Merge branch 'main' into subject-display-name

mindersec · Jul 3, 2024 · 401f927 · 401f927
2 parents 0b9ba5f + a75c435
commit 401f927
Show file tree

Hide file tree

Showing 27 changed files with 107 additions and 101 deletions.
diff --git a/.clusterfuzzlite/build.sh b/.clusterfuzzlite/build.sh
@@ -29,4 +29,4 @@ compile_native_go_fuzzer github.com/stacklok/minder/internal/engine/eval/rego Fu
 compile_native_go_fuzzer github.com/stacklok/minder/internal/controlplane FuzzGitHubEventParsers FuzzGitHubEventParsers
 compile_native_go_fuzzer github.com/stacklok/minder/internal/engine/ingester/diff FuzzDiffParse FuzzDiffParse
 compile_native_go_fuzzer github.com/stacklok/minder/internal/crypto FuzzEncryptDecrypt FuzzEncryptDecrypt
-compile_native_go_fuzzer github.com/stacklok/minder/internal/auth FuzzParseAndValidate FuzzParseAndValidate
+compile_native_go_fuzzer github.com/stacklok/minder/internal/auth/jwt FuzzParseAndValidate FuzzParseAndValidate
diff --git a/cmd/dev/app/testserver/testserver.go b/cmd/dev/app/testserver/testserver.go
@@ -30,7 +30,7 @@ import (
 	"github.com/spf13/viper"
 
 	"github.com/stacklok/minder/internal/auth"
-	noopauth "github.com/stacklok/minder/internal/auth/noop"
+	noopauth "github.com/stacklok/minder/internal/auth/jwt/noop"
 	mockauthz "github.com/stacklok/minder/internal/authz/mock"
 	"github.com/stacklok/minder/internal/config"
 	serverconfig "github.com/stacklok/minder/internal/config/server"

diff --git a/cmd/server/app/serve.go b/cmd/server/app/serve.go
@@ -28,6 +28,7 @@ import (
 	"github.com/spf13/viper"
 
 	"github.com/stacklok/minder/internal/auth"
+	"github.com/stacklok/minder/internal/auth/jwt"
 	"github.com/stacklok/minder/internal/auth/keycloak"
 	"github.com/stacklok/minder/internal/authz"
 	"github.com/stacklok/minder/internal/config"
@@ -100,7 +101,7 @@ var serveCmd = &cobra.Command{
 		if err != nil {
 			return fmt.Errorf("failed to create issuer URL: %w\n", err)
 		}
-		jwt, err := auth.NewJwtValidator(ctx, jwksUrl.String(), issUrl.String(), cfg.Identity.Server.Audience)
+		jwt, err := jwt.NewJwtValidator(ctx, jwksUrl.String(), issUrl.String(), cfg.Identity.Server.Audience)
 		if err != nil {
 			return fmt.Errorf("failed to fetch and cache identity provider JWKS: %w\n", err)
 		}

diff --git a/internal/auth/github.go b/internal/auth/github.go
@@ -13,6 +13,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+// Package auth contains the authentication logic for the control plane
 package auth
 
 import (

diff --git a/internal/auth/fuzz_test.go → internal/auth/jwt/fuzz_test.go b/internal/auth/fuzz_test.go → internal/auth/jwt/fuzz_test.go
@@ -13,7 +13,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package auth
+package jwt
 
 import (
 	"crypto/rand"
@@ -24,7 +24,7 @@ import (
 	"github.com/lestrrat-go/jwx/v2/jwk"
 	"go.uber.org/mock/gomock"
 
-	mockjwt "github.com/stacklok/minder/internal/auth/mock"
+	mockjwt "github.com/stacklok/minder/internal/auth/jwt/mock"
 )
 
 var (

diff --git a/internal/auth/jwauth_test.go → internal/auth/jwt/jwauth_test.go b/internal/auth/jwauth_test.go → internal/auth/jwt/jwauth_test.go
@@ -13,8 +13,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// package auth contains the authentication logic for the control plane
-package auth
+package jwt
 
 import (
 	crand "crypto/rand"
@@ -29,7 +28,7 @@ import (
 	"github.com/stretchr/testify/require"
 	"go.uber.org/mock/gomock"
 
-	mockjwt "github.com/stacklok/minder/internal/auth/mock"
+	mockjwt "github.com/stacklok/minder/internal/auth/jwt/mock"
 )
 
 func TestParseAndValidate(t *testing.T) {

diff --git a/internal/auth/jwtauth.go → internal/auth/jwt/jwtauth.go b/internal/auth/jwtauth.go → internal/auth/jwt/jwtauth.go
@@ -13,7 +13,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package auth
+// Package jwt provides the logic for reading and validating JWT tokens
+package jwt
 
 import (
 	"context"
@@ -26,8 +27,8 @@ import (
 
 //go:generate go run go.uber.org/mock/mockgen -package mock_$GOPACKAGE -destination=./mock/$GOFILE -source=./$GOFILE
 
-// JwtValidator provides the functions to validate a JWT
-type JwtValidator interface {
+// Validator provides the functions to validate a JWT
+type Validator interface {
 	ParseAndValidate(tokenString string) (openid.Token, error)
 }
 
@@ -86,7 +87,7 @@ func (j *JwkSetJwtValidator) ParseAndValidate(tokenString string) (openid.Token,
 }
 
 // NewJwtValidator creates a new JWT validator that uses a JWK set URL to validate the tokens
-func NewJwtValidator(ctx context.Context, jwksUrl string, issUrl string, aud string) (JwtValidator, error) {
+func NewJwtValidator(ctx context.Context, jwksUrl string, issUrl string, aud string) (Validator, error) {
 	// Cache the JWK set
 	// The cache will refresh every 15 minutes by default
 	jwks := jwk.NewCache(ctx)

diff --git a/internal/auth/mock/jwtauth.go → internal/auth/jwt/mock/jwtauth.go b/internal/auth/mock/jwtauth.go → internal/auth/jwt/mock/jwtauth.go
diff --git a/internal/auth/noop/jwtauth.go → internal/auth/jwt/noop/jwtauth.go b/internal/auth/noop/jwtauth.go → internal/auth/jwt/noop/jwtauth.go
@@ -13,13 +13,13 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// Package noop provides a no-op implementation of the JwtValidator interface
+// Package noop provides a no-op implementation of the Validator interface
 package noop
 
 import (
 	"github.com/lestrrat-go/jwx/v2/jwt/openid"
 
-	"github.com/stacklok/minder/internal/auth"
+	"github.com/stacklok/minder/internal/auth/jwt"
 )
 
 type noopJwtValidator struct {
@@ -28,7 +28,7 @@ type noopJwtValidator struct {
 }
 
 // NewJwtValidator returns a new instance of the no-op JWT validator
-func NewJwtValidator(subject string) auth.JwtValidator {
+func NewJwtValidator(subject string) jwt.Validator {
 	return &noopJwtValidator{
 		Subject: subject,
 	}

diff --git a/internal/authz/authz.go b/internal/authz/authz.go
@@ -32,7 +32,7 @@ import (
 	"github.com/rs/zerolog"
 	"k8s.io/client-go/transport"
 
-	"github.com/stacklok/minder/internal/auth"
+	"github.com/stacklok/minder/internal/auth/jwt"
 	srvconfig "github.com/stacklok/minder/internal/config/server"
 	minderv1 "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1"
 )
@@ -249,7 +249,7 @@ func (a *ClientWrapper) Check(ctx context.Context, action string, project uuid.U
 	// TODO: set ClientCheckOptions like in
 	// https://openfga.dev/docs/getting-started/perform-check#02-calling-check-api
 	options := fgaclient.ClientCheckOptions{}
-	userString := getUserForTuple(auth.GetUserSubjectFromContext(ctx))
+	userString := getUserForTuple(jwt.GetUserSubjectFromContext(ctx))
 	body := fgaclient.ClientCheckRequest{
 		User:     userString,
 		Relation: action,

diff --git a/internal/authz/authz_test.go b/internal/authz/authz_test.go
@@ -32,7 +32,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
-	"github.com/stacklok/minder/internal/auth"
+	"github.com/stacklok/minder/internal/auth/jwt"
 	"github.com/stacklok/minder/internal/authz"
 	srvconfig "github.com/stacklok/minder/internal/config/server"
 )
@@ -105,7 +105,7 @@ func TestVerifyOneProject(t *testing.T) {
 
 	userJWT := openid.New()
 	assert.NoError(t, userJWT.Set("sub", "user-1"))
-	userctx := auth.WithAuthTokenContext(ctx, userJWT)
+	userctx := jwt.WithAuthTokenContext(ctx, userJWT)
 
 	// verify the project
 	assert.NoError(t, c.Check(userctx, "get", prj), "failed to check project")
@@ -159,7 +159,7 @@ func TestVerifyMultipleProjects(t *testing.T) {
 
 	user1JWT := openid.New()
 	assert.NoError(t, user1JWT.Set("sub", "user-1"))
-	userctx := auth.WithAuthTokenContext(ctx, user1JWT)
+	userctx := jwt.WithAuthTokenContext(ctx, user1JWT)
 
 	// verify the project
 	assert.NoError(t, c.Check(userctx, "get", prj1), "failed to check project")
@@ -178,7 +178,7 @@ func TestVerifyMultipleProjects(t *testing.T) {
 	// verify the project
 	user2JWT := openid.New()
 	assert.NoError(t, user2JWT.Set("sub", "user-2"))
-	assert.NoError(t, c.Check(auth.WithAuthTokenContext(ctx, user2JWT), "get", prj3), "failed to check project")
+	assert.NoError(t, c.Check(jwt.WithAuthTokenContext(ctx, user2JWT), "get", prj3), "failed to check project")
 
 	// verify user-1 cannot operate on project 3
 	assert.Error(t, c.Check(userctx, "get", prj3), "expected user-1 to not be able to operate on project 3")

diff --git a/internal/authz/fuzz_test.go b/internal/authz/fuzz_test.go
@@ -22,7 +22,7 @@ import (
 	"github.com/google/uuid"
 	"github.com/lestrrat-go/jwx/v2/jwt/openid"
 
-	"github.com/stacklok/minder/internal/auth"
+	"github.com/stacklok/minder/internal/auth/jwt"
 	"github.com/stacklok/minder/internal/authz"
 )
 
@@ -52,7 +52,7 @@ func FuzzAllAuthzApis(f *testing.F) {
 			return
 		}
 
-		userctx := auth.WithAuthTokenContext(ctx, userJWT)
+		userctx := jwt.WithAuthTokenContext(ctx, userJWT)
 
 		c.Check(userctx, str3, prj)
 

diff --git a/internal/controlplane/handlers_authz.go b/internal/controlplane/handlers_authz.go
@@ -29,7 +29,7 @@ import (
 	"google.golang.org/protobuf/proto"
 	"google.golang.org/protobuf/types/known/timestamppb"
 
-	"github.com/stacklok/minder/internal/auth"
+	"github.com/stacklok/minder/internal/auth/jwt"
 	"github.com/stacklok/minder/internal/authz"
 	"github.com/stacklok/minder/internal/db"
 	"github.com/stacklok/minder/internal/email"
@@ -111,7 +111,7 @@ func ProjectAuthorizationInterceptor(ctx context.Context, req interface{}, info
 		zerolog.Ctx(ctx).Error().Err(err).Msg("authorization check failed")
 		return nil, util.UserVisibleError(
 			codes.PermissionDenied, "user %q is not authorized to perform this operation on project %q",
-			auth.GetUserSubjectFromContext(ctx), entityCtx.Project.ID)
+			jwt.GetUserSubjectFromContext(ctx), entityCtx.Project.ID)
 	}
 
 	return handler(ctx, req)
@@ -183,7 +183,7 @@ func getDefaultProjectID(
 	store db.Store,
 	authzClient authz.Client,
 ) (uuid.UUID, error) {
-	subject := auth.GetUserSubjectFromContext(ctx)
+	subject := jwt.GetUserSubjectFromContext(ctx)
 
 	userInfo, err := store.GetUserBySubject(ctx, subject)
 	if err != nil {
@@ -348,7 +348,7 @@ func (s *Server) inviteUser(
 ) (*minder.AssignRoleResponse, error) {
 	var userInvite db.UserInvite
 	// Get the sponsor's user information (current user)
-	currentUser, err := s.store.GetUserBySubject(ctx, auth.GetUserSubjectFromContext(ctx))
+	currentUser, err := s.store.GetUserBySubject(ctx, jwt.GetUserSubjectFromContext(ctx))
 	if err != nil {
 		return nil, status.Errorf(codes.Internal, "failed to get user: %s", err)
 	}
@@ -692,7 +692,7 @@ func (s *Server) updateInvite(
 ) (*minder.UpdateRoleResponse, error) {
 	var userInvite db.UserInvite
 	// Get the sponsor's user information (current user)
-	currentUser, err := s.store.GetUserBySubject(ctx, auth.GetUserSubjectFromContext(ctx))
+	currentUser, err := s.store.GetUserBySubject(ctx, jwt.GetUserSubjectFromContext(ctx))
 	if err != nil {
 		return nil, status.Errorf(codes.Internal, "failed to get user: %s", err)
 	}
@@ -847,12 +847,12 @@ func (s *Server) updateRole(
 // isUserSelfUpdating is used to prevent if the user is trying to update their own role
 func isUserSelfUpdating(ctx context.Context, subject, inviteeEmail string) error {
 	if subject != "" {
-		if auth.GetUserSubjectFromContext(ctx) == subject {
+		if jwt.GetUserSubjectFromContext(ctx) == subject {
 			return util.UserVisibleError(codes.InvalidArgument, "cannot update your own role")
 		}
 	}
 	if inviteeEmail != "" {
-		tokenEmail, err := auth.GetUserEmailFromContext(ctx)
+		tokenEmail, err := jwt.GetUserEmailFromContext(ctx)
 		if err != nil {
 			return util.UserVisibleError(codes.Internal, "error getting user email from token: %v", err)
 		}

diff --git a/internal/controlplane/handlers_authz_test.go b/internal/controlplane/handlers_authz_test.go
@@ -39,7 +39,8 @@ import (
 
 	mockdb "github.com/stacklok/minder/database/mock"
 	"github.com/stacklok/minder/internal/auth"
-	"github.com/stacklok/minder/internal/auth/noop"
+	authjwt "github.com/stacklok/minder/internal/auth/jwt"
+	"github.com/stacklok/minder/internal/auth/jwt/noop"
 	"github.com/stacklok/minder/internal/authz"
 	"github.com/stacklok/minder/internal/authz/mock"
 	"github.com/stacklok/minder/internal/db"
@@ -187,7 +188,7 @@ func TestEntityContextProjectInterceptor(t *testing.T) {
 			if tc.buildStubs != nil {
 				tc.buildStubs(t, mockStore)
 			}
-			ctx := auth.WithAuthTokenContext(withRpcOptions(context.Background(), rpcOptions), userJWT)
+			ctx := authjwt.WithAuthTokenContext(withRpcOptions(context.Background(), rpcOptions), userJWT)
 
 			authzClient := &mock.SimpleClient{}
 
@@ -281,7 +282,7 @@ func TestProjectAuthorizationInterceptor(t *testing.T) {
 			}
 			ctx := withRpcOptions(context.Background(), rpcOptions)
 			ctx = engcontext.WithEntityContext(ctx, tc.entityCtx)
-			ctx = auth.WithAuthTokenContext(ctx, userJWT)
+			ctx = authjwt.WithAuthTokenContext(ctx, userJWT)
 			_, err := ProjectAuthorizationInterceptor(ctx, request{}, &grpc.UnaryServerInfo{
 				Server: &server,
 			}, unaryHandler)
@@ -491,7 +492,7 @@ func TestRoleManagement(t *testing.T) {
 			}
 
 			ctx := context.Background()
-			ctx = auth.WithAuthTokenContext(ctx, user)
+			ctx = authjwt.WithAuthTokenContext(ctx, user)
 			ctx = engcontext.WithEntityContext(ctx, &engcontext.EntityContext{
 				Project: engcontext.Project{
 					ID: project,

diff --git a/internal/controlplane/handlers_oauth.go b/internal/controlplane/handlers_oauth.go
@@ -37,6 +37,7 @@ import (
 	"google.golang.org/grpc/status"
 
 	"github.com/stacklok/minder/internal/auth"
+	"github.com/stacklok/minder/internal/auth/jwt"
 	mcrypto "github.com/stacklok/minder/internal/crypto"
 	"github.com/stacklok/minder/internal/db"
 	"github.com/stacklok/minder/internal/engine/engcontext"
@@ -93,7 +94,7 @@ func (s *Server) GetAuthorizationURL(ctx context.Context,
 	span.SetAttributes(attribute.Key("provider").String(providerName))
 	defer span.End()
 
-	user, _ := auth.GetUserClaimFromContext[string](ctx, "gh_id")
+	user, _ := jwt.GetUserClaimFromContext[string](ctx, "gh_id")
 	// If the user's token doesn't have gh_id set yet, we'll pass it through for now.
 	s.mt.AddTokenOpCount(ctx, "issued", user != "")