From 7723097937169c9b42be6e22b6447cbce8427623 Mon Sep 17 00:00:00 2001 From: Prashantkumar Khatri Date: Tue, 30 Jun 2026 00:02:09 +0530 Subject: [PATCH 1/5] refactor: encapsulate project creation logic into the ProjectCreator interface Signed-off-by: Prashantkumar Khatri --- internal/controlplane/handlers_projects.go | 63 +----------- internal/controlplane/handlers_user_test.go | 3 + internal/projects/creator.go | 104 ++++++++++++++++++-- internal/projects/creator_test.go | 13 +-- internal/service/service.go | 9 +- 5 files changed, 117 insertions(+), 75 deletions(-) diff --git a/internal/controlplane/handlers_projects.go b/internal/controlplane/handlers_projects.go index 81b8cf6a37..6568c93780 100644 --- a/internal/controlplane/handlers_projects.go +++ b/internal/controlplane/handlers_projects.go @@ -170,66 +170,9 @@ func (s *Server) CreateProject( return nil, err } - var project *db.Project - if parentProjectID != uuid.Nil { - // Verify permissions if we have a parent - relationName := relationAsName(minderv1.Relation_RELATION_CREATE) - if err := s.authzClient.Check(ctx, relationName, parentProjectID); err != nil { - return nil, util.UserVisibleError( - codes.PermissionDenied, "user %q is not authorized to perform this operation on project %q", - auth.IdentityFromContext(ctx).Human(), parentProjectID) - } - - if !features.ProjectAllowsProjectHierarchyOperations(ctx, s.store, parentProjectID) { - return nil, util.UserVisibleError(codes.PermissionDenied, - "project does not allow project hierarchy operations") - } - tx, err := s.store.BeginTransaction() - if err != nil { - return nil, status.Errorf(codes.Internal, "error starting transaction: %v", err) - } - defer s.store.Rollback(tx) - qtx := s.store.GetQuerierWithTransaction(tx) - - project, err = s.projectCreator.ProvisionChildProject(ctx, qtx, parentProjectID, req.Name) - if err != nil { - return nil, err - } - - if err := s.store.Commit(tx); err != nil { - return nil, status.Errorf(codes.Internal, "error committing transaction: %v", err) - } - - } else { - // This is a top-level project creation request. - // We need to check if the user has the right to create projects in the system. - if !flags.Bool(ctx, s.featureFlags, flags.ProjectCreateDelete) { - return nil, util.UserVisibleError(codes.Unimplemented, "cannot create a new top-level project") - } - - id := auth.IdentityFromContext(ctx) - if id.String() == "" { - return nil, util.UserVisibleError(codes.Unauthenticated, "cannot determine user ID") - } - - tx, err := s.store.BeginTransaction() - if err != nil { - return nil, status.Errorf(codes.Internal, "error starting transaction: %v", err) - } - defer s.store.Rollback(tx) - qtx := s.store.GetQuerierWithTransaction(tx) - project, err = s.projectCreator.ProvisionSelfEnrolledProject(ctx, qtx, req.Name, id.String()) - if err != nil { - return nil, err - } - - if err := s.store.Commit(tx); err != nil { - return nil, status.Errorf(codes.Internal, "error committing transaction: %v", err) - } - } - - if project == nil { - return nil, status.Errorf(codes.Internal, "project is nil after creation") + project, err := s.projectCreator.ProvisionProject(ctx, req.Name, parentProjectID) + if err != nil { + return nil, err } return &minderv1.CreateProjectResponse{ diff --git a/internal/controlplane/handlers_user_test.go b/internal/controlplane/handlers_user_test.go index f861c8cec9..3dee0feb4c 100644 --- a/internal/controlplane/handlers_user_test.go +++ b/internal/controlplane/handlers_user_test.go @@ -43,6 +43,7 @@ import ( pb "github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1" serverconfig "github.com/mindersec/minder/pkg/config/server" "github.com/mindersec/minder/pkg/eventer" + "github.com/mindersec/minder/pkg/flags" ) const ( @@ -271,6 +272,8 @@ func TestCreateUser_gRPC(t *testing.T) { marketplaces.NewNoopMarketplace(), &serverconfig.DefaultProfilesConfig{}, &serverconfig.FeaturesConfig{}, + mockStore, + &flags.FakeClient{}, ), } diff --git a/internal/projects/creator.go b/internal/projects/creator.go index e0eb1d5d8c..63fb6b8ba8 100644 --- a/internal/projects/creator.go +++ b/internal/projects/creator.go @@ -15,11 +15,14 @@ import ( "google.golang.org/grpc/codes" "google.golang.org/grpc/status" + "github.com/mindersec/minder/internal/auth" "github.com/mindersec/minder/internal/authz" "github.com/mindersec/minder/internal/db" "github.com/mindersec/minder/internal/marketplaces" + "github.com/mindersec/minder/internal/projects/features" "github.com/mindersec/minder/internal/util" "github.com/mindersec/minder/pkg/config/server" + "github.com/mindersec/minder/pkg/flags" "github.com/mindersec/minder/pkg/mindpak" ) @@ -28,6 +31,13 @@ import ( // 1. Move the delete operations into this interface // 2. The interface is very GitHub-specific. It needs to be made more generic. type ProjectCreator interface { + // ProvisionProject orchestrates the creation of a project, checking permissions, + // managing the database transaction, and calling the appropriate provisioning logic. + ProvisionProject( + ctx context.Context, + projectName string, + parentProjectID uuid.UUID, + ) (*db.Project, error) // ProvisionSelfEnrolledProject creates the core default components of the project // (project, marketplace subscriptions, etc.). @@ -50,10 +60,12 @@ type ProjectCreator interface { } type projectCreator struct { - authzClient authz.Client - marketplace marketplaces.Marketplace - profilesCfg *server.DefaultProfilesConfig - featuresCfg *server.FeaturesConfig + authzClient authz.Client + marketplace marketplaces.Marketplace + profilesCfg *server.DefaultProfilesConfig + featuresCfg *server.FeaturesConfig + store db.Store + featureFlags flags.Interface } // NewProjectCreator creates a new instance of the project creator @@ -61,12 +73,16 @@ func NewProjectCreator(authzClient authz.Client, marketplace marketplaces.Marketplace, profilesCfg *server.DefaultProfilesConfig, featuresCfg *server.FeaturesConfig, + store db.Store, + featureFlags flags.Interface, ) ProjectCreator { return &projectCreator{ - authzClient: authzClient, - marketplace: marketplace, - profilesCfg: profilesCfg, - featuresCfg: featuresCfg, + authzClient: authzClient, + marketplace: marketplace, + profilesCfg: profilesCfg, + featuresCfg: featuresCfg, + store: store, + featureFlags: featureFlags, } } @@ -204,3 +220,75 @@ func (p *projectCreator) ProvisionChildProject( return &subProject, nil } + +func (p *projectCreator) ProvisionProject( + ctx context.Context, + projectName string, + parentProjectID uuid.UUID, +) (*db.Project, error) { + var project *db.Project + + if parentProjectID != uuid.Nil { + // Verify permissions if we have a parent + if err := p.authzClient.Check(ctx, "create", parentProjectID); err != nil { + return nil, util.UserVisibleError( + codes.PermissionDenied, "user %q is not authorized to perform this operation on project %q", + auth.IdentityFromContext(ctx).Human(), parentProjectID) + } + + if !features.ProjectAllowsProjectHierarchyOperations(ctx, p.store, parentProjectID) { + return nil, util.UserVisibleError(codes.PermissionDenied, + "project does not allow project hierarchy operations") + } + + tx, err := p.store.BeginTransaction() + if err != nil { + return nil, status.Errorf(codes.Internal, "error starting transaction: %v", err) + } + defer p.store.Rollback(tx) + qtx := p.store.GetQuerierWithTransaction(tx) + + project, err = p.ProvisionChildProject(ctx, qtx, parentProjectID, projectName) + if err != nil { + return nil, err + } + + if err := p.store.Commit(tx); err != nil { + return nil, status.Errorf(codes.Internal, "error committing transaction: %v", err) + } + + } else { + // This is a top-level project creation request, so we need to check + // if the user has the right to create projects in the system. + if !flags.Bool(ctx, p.featureFlags, flags.ProjectCreateDelete) { + return nil, util.UserVisibleError(codes.Unimplemented, "cannot create a new top-level project") + } + + id := auth.IdentityFromContext(ctx) + if id.String() == "" { + return nil, util.UserVisibleError(codes.Unauthenticated, "cannot determine user ID") + } + + tx, err := p.store.BeginTransaction() + if err != nil { + return nil, status.Errorf(codes.Internal, "error starting transaction: %v", err) + } + defer p.store.Rollback(tx) + qtx := p.store.GetQuerierWithTransaction(tx) + + project, err = p.ProvisionSelfEnrolledProject(ctx, qtx, projectName, id.String()) + if err != nil { + return nil, err + } + + if err := p.store.Commit(tx); err != nil { + return nil, status.Errorf(codes.Internal, "error committing transaction: %v", err) + } + } + + if project == nil { + return nil, status.Errorf(codes.Internal, "project is nil after creation") + } + + return project, nil +} diff --git a/internal/projects/creator_test.go b/internal/projects/creator_test.go index 7b3a2b970f..a499deddf0 100644 --- a/internal/projects/creator_test.go +++ b/internal/projects/creator_test.go @@ -24,6 +24,7 @@ import ( "github.com/mindersec/minder/internal/projects" "github.com/mindersec/minder/internal/util" "github.com/mindersec/minder/pkg/config/server" + "github.com/mindersec/minder/pkg/flags" ) func TestProvisionSelfEnrolledProject(t *testing.T) { @@ -59,7 +60,7 @@ func TestProvisionSelfEnrolledProject(t *testing.T) { "teamA": "featureA", "teamB": "featureB", }, - }) + }, mockStore, &flags.FakeClient{}) _, err := creator.ProvisionSelfEnrolledProject( ctx, @@ -86,7 +87,7 @@ func TestProvisionSelfEnrolledProjectFailsWritingProjectToDB(t *testing.T) { Return(db.Project{}, fmt.Errorf("failed to create project")) ctx := context.Background() - creator := projects.NewProjectCreator(authzClient, marketplaces.NewNoopMarketplace(), &server.DefaultProfilesConfig{}, &server.FeaturesConfig{}) + creator := projects.NewProjectCreator(authzClient, marketplaces.NewNoopMarketplace(), &server.DefaultProfilesConfig{}, &server.FeaturesConfig{}, mockStore, &flags.FakeClient{}) _, err := creator.ProvisionSelfEnrolledProject( ctx, mockStore, @@ -118,7 +119,7 @@ func TestProvisionSelfEnrolledProjectInvalidName(t *testing.T) { mockStore := mockdb.NewMockStore(ctrl) ctx := context.Background() - creator := projects.NewProjectCreator(authzClient, marketplaces.NewNoopMarketplace(), &server.DefaultProfilesConfig{}, &server.FeaturesConfig{}) + creator := projects.NewProjectCreator(authzClient, marketplaces.NewNoopMarketplace(), &server.DefaultProfilesConfig{}, &server.FeaturesConfig{}, mockStore, &flags.FakeClient{}) for _, tc := range testCases { _, err := creator.ProvisionSelfEnrolledProject( @@ -159,7 +160,7 @@ func TestProvisionChildProject(t *testing.T) { "teamA": "featureA", "teamB": "featureB", }, - }) + }, mockStore, &flags.FakeClient{}) _, err := creator.ProvisionChildProject( ctx, @@ -188,7 +189,7 @@ func TestProvisionChildProjectFailsNoParent(t *testing.T) { Return(db.Project{}, fmt.Errorf("parent not found")) ctx := context.Background() - creator := projects.NewProjectCreator(authzClient, marketplaces.NewNoopMarketplace(), &server.DefaultProfilesConfig{}, &server.FeaturesConfig{}) + creator := projects.NewProjectCreator(authzClient, marketplaces.NewNoopMarketplace(), &server.DefaultProfilesConfig{}, &server.FeaturesConfig{}, mockStore, &flags.FakeClient{}) _, err := creator.ProvisionChildProject( ctx, mockStore, @@ -221,7 +222,7 @@ func TestProvisionChildProjectFailsTooManyParents(t *testing.T) { }, nil) ctx := context.Background() - creator := projects.NewProjectCreator(authzClient, marketplaces.NewNoopMarketplace(), &server.DefaultProfilesConfig{}, &server.FeaturesConfig{}) + creator := projects.NewProjectCreator(authzClient, marketplaces.NewNoopMarketplace(), &server.DefaultProfilesConfig{}, &server.FeaturesConfig{}, mockStore, &flags.FakeClient{}) _, err := creator.ProvisionChildProject( ctx, mockStore, diff --git a/internal/service/service.go b/internal/service/service.go index ac8130c4a3..41718f53c6 100644 --- a/internal/service/service.go +++ b/internal/service/service.go @@ -110,7 +110,14 @@ func AllInOneServerService( fallbackTokenClient := ghprov.NewFallbackTokenClient(cfg.Provider) ghClientFactory := clients.NewGitHubClientFactory(providerMetrics) providerStore := providers.NewProviderStore(store) - projectCreator := projects.NewProjectCreator(authzClient, marketplace, &cfg.DefaultProfiles, &cfg.Features) + projectCreator := projects.NewProjectCreator( + authzClient, + marketplace, + &cfg.DefaultProfiles, + &cfg.Features, + store, + featureFlagClient, + ) propSvc := propService.NewPropertiesService(store) // TODO: isolate GitHub-specific wiring. We'll need to isolate GitHub From 11bed215ee40b3963c0c0a38ba9d536862675719 Mon Sep 17 00:00:00 2001 From: Prashantkumar Khatri Date: Wed, 1 Jul 2026 01:30:17 +0530 Subject: [PATCH 2/5] refactor: use RelationCreate constant for authorization checks in project creator Signed-off-by: Prashantkumar Khatri --- internal/authz/interface.go | 5 +++++ internal/projects/creator.go | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/internal/authz/interface.go b/internal/authz/interface.go index 030d80aa38..7d8d37b9f7 100644 --- a/internal/authz/interface.go +++ b/internal/authz/interface.go @@ -32,6 +32,11 @@ const ( RolePermissionsManager Role = "permissions_manager" ) +const ( + // RelationCreate maps to Relation_RELATION_CREATE [(name) = "create"]. + RelationCreate = "create" +) + // nolint:lll var ( // AllRolesDescriptions is a list of all roles diff --git a/internal/projects/creator.go b/internal/projects/creator.go index 63fb6b8ba8..0e8eab9dc9 100644 --- a/internal/projects/creator.go +++ b/internal/projects/creator.go @@ -230,7 +230,7 @@ func (p *projectCreator) ProvisionProject( if parentProjectID != uuid.Nil { // Verify permissions if we have a parent - if err := p.authzClient.Check(ctx, "create", parentProjectID); err != nil { + if err := p.authzClient.Check(ctx, authz.RelationCreate, parentProjectID); err != nil { return nil, util.UserVisibleError( codes.PermissionDenied, "user %q is not authorized to perform this operation on project %q", auth.IdentityFromContext(ctx).Human(), parentProjectID) From d0a37a0eb3538834a726ada41c3f37ff877728e4 Mon Sep 17 00:00:00 2001 From: Prashantkumar Khatri Date: Thu, 9 Jul 2026 16:15:35 +0530 Subject: [PATCH 3/5] refactor: standardize project creation return types and simplify authz checks in project handlers Signed-off-by: Prashantkumar Khatri --- internal/authz/interface.go | 4 - internal/controlplane/handlers_authz.go | 18 +- internal/controlplane/handlers_projects.go | 8 +- internal/projects/creator.go | 29 ++- internal/projects/creator_test.go | 255 +++++++++++++-------- pkg/api/protobuf/go/minder/v1/api.go | 16 ++ 6 files changed, 189 insertions(+), 141 deletions(-) diff --git a/internal/authz/interface.go b/internal/authz/interface.go index 7d8d37b9f7..4150fc738b 100644 --- a/internal/authz/interface.go +++ b/internal/authz/interface.go @@ -32,10 +32,6 @@ const ( RolePermissionsManager Role = "permissions_manager" ) -const ( - // RelationCreate maps to Relation_RELATION_CREATE [(name) = "create"]. - RelationCreate = "create" -) // nolint:lll var ( diff --git a/internal/controlplane/handlers_authz.go b/internal/controlplane/handlers_authz.go index ba6de3a76b..c03c5b03e9 100644 --- a/internal/controlplane/handlers_authz.go +++ b/internal/controlplane/handlers_authz.go @@ -14,7 +14,6 @@ import ( "google.golang.org/grpc" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" - "google.golang.org/protobuf/proto" "github.com/mindersec/minder/internal/auth" "github.com/mindersec/minder/internal/auth/jwt" @@ -78,7 +77,7 @@ func ProjectAuthorizationInterceptor(ctx context.Context, req interface{}, info relation := opts.GetRelation() - relationName := relationAsName(relation) + relationName := minder.RelationAsName(relation) if relationName == "" { return nil, status.Errorf(codes.Internal, "error getting name for requested relation %v", relation) } @@ -105,21 +104,6 @@ func ProjectAuthorizationInterceptor(ctx context.Context, req interface{}, info return handler(ctx, req) } -// relationAsName returns the OpenFGA relation name for the given relation enum. -// It returns an empty string in the case of error. -func relationAsName(relation minder.Relation) string { - relationValue := relation.Descriptor().Values().ByNumber(relation.Number()) - if relationValue == nil { - return "" - } - extension := proto.GetExtension(relationValue.Options(), minder.E_Name) - relationName, ok := extension.(string) - if !ok { - return "" - } - return relationName -} - // populateEntityContext populates the project in the entity context, by looking at the proto context or // fetching the default project func populateEntityContext( diff --git a/internal/controlplane/handlers_projects.go b/internal/controlplane/handlers_projects.go index 6568c93780..6c915afc64 100644 --- a/internal/controlplane/handlers_projects.go +++ b/internal/controlplane/handlers_projects.go @@ -176,13 +176,7 @@ func (s *Server) CreateProject( } return &minderv1.CreateProjectResponse{ - Project: &minderv1.Project{ - ProjectId: project.ID.String(), - Name: project.Name, - Description: "", - CreatedAt: timestamppb.New(project.CreatedAt), - UpdatedAt: timestamppb.New(project.UpdatedAt), - }, + Project: project, }, nil } diff --git a/internal/projects/creator.go b/internal/projects/creator.go index 0e8eab9dc9..1c738efb84 100644 --- a/internal/projects/creator.go +++ b/internal/projects/creator.go @@ -14,6 +14,7 @@ import ( "github.com/rs/zerolog/log" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" + "google.golang.org/protobuf/types/known/timestamppb" "github.com/mindersec/minder/internal/auth" "github.com/mindersec/minder/internal/authz" @@ -21,6 +22,7 @@ import ( "github.com/mindersec/minder/internal/marketplaces" "github.com/mindersec/minder/internal/projects/features" "github.com/mindersec/minder/internal/util" + minderv1 "github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1" "github.com/mindersec/minder/pkg/config/server" "github.com/mindersec/minder/pkg/flags" "github.com/mindersec/minder/pkg/mindpak" @@ -37,7 +39,7 @@ type ProjectCreator interface { ctx context.Context, projectName string, parentProjectID uuid.UUID, - ) (*db.Project, error) + ) (*minderv1.Project, error) // ProvisionSelfEnrolledProject creates the core default components of the project // (project, marketplace subscriptions, etc.). @@ -47,16 +49,6 @@ type ProjectCreator interface { projectName string, userSub string, ) (outproj *db.Project, projerr error) - - // ProvisionChildProject creates the components of a child project which is nested - // under a parent in the project hierarchy. The parent project is checked for - // existence before creating the child project. - ProvisionChildProject( - ctx context.Context, - qtx db.ExtendQuerier, - parentProjectID uuid.UUID, - projectName string, - ) (outproj *db.Project, projerr error) } type projectCreator struct { @@ -163,7 +155,7 @@ func (p *projectCreator) ProvisionSelfEnrolledProject( return &project, nil } -func (p *projectCreator) ProvisionChildProject( +func (p *projectCreator) provisionChildProject( ctx context.Context, qtx db.ExtendQuerier, parentProjectID uuid.UUID, @@ -225,12 +217,12 @@ func (p *projectCreator) ProvisionProject( ctx context.Context, projectName string, parentProjectID uuid.UUID, -) (*db.Project, error) { +) (*minderv1.Project, error) { var project *db.Project if parentProjectID != uuid.Nil { // Verify permissions if we have a parent - if err := p.authzClient.Check(ctx, authz.RelationCreate, parentProjectID); err != nil { + if err := p.authzClient.Check(ctx, "create", parentProjectID); err != nil { return nil, util.UserVisibleError( codes.PermissionDenied, "user %q is not authorized to perform this operation on project %q", auth.IdentityFromContext(ctx).Human(), parentProjectID) @@ -248,7 +240,7 @@ func (p *projectCreator) ProvisionProject( defer p.store.Rollback(tx) qtx := p.store.GetQuerierWithTransaction(tx) - project, err = p.ProvisionChildProject(ctx, qtx, parentProjectID, projectName) + project, err = p.provisionChildProject(ctx, qtx, parentProjectID, projectName) if err != nil { return nil, err } @@ -290,5 +282,10 @@ func (p *projectCreator) ProvisionProject( return nil, status.Errorf(codes.Internal, "project is nil after creation") } - return project, nil + return &minderv1.Project{ + ProjectId: project.ID.String(), + Name: project.Name, + CreatedAt: timestamppb.New(project.CreatedAt), + UpdatedAt: timestamppb.New(project.UpdatedAt), + }, nil } diff --git a/internal/projects/creator_test.go b/internal/projects/creator_test.go index a499deddf0..73592a4d9e 100644 --- a/internal/projects/creator_test.go +++ b/internal/projects/creator_test.go @@ -5,6 +5,7 @@ package projects_test import ( "context" + "database/sql" "fmt" "reflect" "testing" @@ -15,10 +16,12 @@ import ( "github.com/stretchr/testify/require" "go.uber.org/mock/gomock" "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" mockdb "github.com/mindersec/minder/database/mock" + "github.com/mindersec/minder/internal/auth" "github.com/mindersec/minder/internal/auth/jwt" - "github.com/mindersec/minder/internal/authz/mock" + authmock "github.com/mindersec/minder/internal/authz/mock" "github.com/mindersec/minder/internal/db" "github.com/mindersec/minder/internal/marketplaces" "github.com/mindersec/minder/internal/projects" @@ -30,7 +33,7 @@ import ( func TestProvisionSelfEnrolledProject(t *testing.T) { t.Parallel() - authzClient := &mock.SimpleClient{} + authzClient := &authmock.SimpleClient{} ctrl := gomock.NewController(t) defer ctrl.Finish() @@ -77,7 +80,7 @@ func TestProvisionSelfEnrolledProject(t *testing.T) { func TestProvisionSelfEnrolledProjectFailsWritingProjectToDB(t *testing.T) { t.Parallel() - authzClient := &mock.SimpleClient{} + authzClient := &authmock.SimpleClient{} ctrl := gomock.NewController(t) defer ctrl.Finish() @@ -112,7 +115,7 @@ func TestProvisionSelfEnrolledProjectInvalidName(t *testing.T) { {"longestinvalidnamelongestinvalidnamelongestinvalidnamelongestinvalidnamelongestinvalidname", `name is too long`}, } - authzClient := &mock.SimpleClient{} + authzClient := &authmock.SimpleClient{} ctrl := gomock.NewController(t) defer ctrl.Finish() @@ -133,106 +136,164 @@ func TestProvisionSelfEnrolledProjectInvalidName(t *testing.T) { } -func TestProvisionChildProject(t *testing.T) { +func TestProvisionProject(t *testing.T) { t.Parallel() parentProject := uuid.New() childProj := uuid.New() - authzClient := &mock.SimpleClient{} - - ctrl := gomock.NewController(t) - defer ctrl.Finish() - - mockStore := mockdb.NewMockStore(ctrl) - mockStore.EXPECT().GetProjectByID(gomock.Any(), parentProject). - Return(db.Project{ID: parentProject}, nil) - mockStore.EXPECT().CreateProject(gomock.Any(), gomock.Any()). - Return(db.Project{ - ID: childProj, - }, nil) - mockStore.EXPECT().CreateEntitlements(gomock.Any(), gomock.Any()). - Return(nil) - - ctx := prepareTestToken(context.Background(), t, []any{}) - - creator := projects.NewProjectCreator(authzClient, marketplaces.NewNoopMarketplace(), &server.DefaultProfilesConfig{}, &server.FeaturesConfig{ - MembershipFeatureMapping: map[string]string{ - "teamA": "featureA", - "teamB": "featureB", + tests := []struct { + name string + setupCtx func(t *testing.T) context.Context + setupAuthz func() *authmock.SimpleClient + setupStore func(ctrl *gomock.Controller) *mockdb.MockStore + featureFlags flags.Interface + parentProjectID uuid.UUID + projectName string + wantErrCode codes.Code + wantNoErr bool + }{ + { + name: "child project: authzClient.Check denies", + setupCtx: func(t *testing.T) context.Context { + t.Helper() + return prepareTestToken(context.Background(), t, []any{}) + }, + setupAuthz: func() *authmock.SimpleClient { + return &authmock.SimpleClient{} + }, + setupStore: func(ctrl *gomock.Controller) *mockdb.MockStore { + return mockdb.NewMockStore(ctrl) + }, + featureFlags: &flags.FakeClient{}, + parentProjectID: parentProject, + projectName: "child", + wantErrCode: codes.PermissionDenied, }, - }, mockStore, &flags.FakeClient{}) - - _, err := creator.ProvisionChildProject( - ctx, - mockStore, - parentProject, - "test-child-proj", - ) - assert.NoError(t, err) - - t.Log("ensure project permission was written") - assert.Len(t, authzClient.Adoptions, 1) - assert.Equal(t, authzClient.Adoptions[childProj], parentProject) -} - -func TestProvisionChildProjectFailsNoParent(t *testing.T) { - t.Parallel() - parentProject := uuid.New() - - authzClient := &mock.SimpleClient{} - - ctrl := gomock.NewController(t) - defer ctrl.Finish() - - mockStore := mockdb.NewMockStore(ctrl) - mockStore.EXPECT().GetProjectByID(gomock.Any(), parentProject). - Return(db.Project{}, fmt.Errorf("parent not found")) - - ctx := context.Background() - creator := projects.NewProjectCreator(authzClient, marketplaces.NewNoopMarketplace(), &server.DefaultProfilesConfig{}, &server.FeaturesConfig{}, mockStore, &flags.FakeClient{}) - _, err := creator.ProvisionChildProject( - ctx, - mockStore, - parentProject, - "test-child-proj", - ) - assert.Error(t, err) - - t.Log("ensure project permission was cleaned up") - assert.Len(t, authzClient.Adoptions, 0) -} - -func TestProvisionChildProjectFailsTooManyParents(t *testing.T) { - t.Parallel() - parentProject := uuid.New() - - authzClient := &mock.SimpleClient{} - - ctrl := gomock.NewController(t) - defer ctrl.Finish() - - mockStore := mockdb.NewMockStore(ctrl) - mockStore.EXPECT().GetProjectByID(gomock.Any(), parentProject). - Return(db.Project{ - ID: parentProject, - ParentID: uuid.NullUUID{ - UUID: uuid.New(), - Valid: true, + { + name: "child project: ProjectAllowsProjectHierarchyOperations returns false", + setupCtx: func(t *testing.T) context.Context { + t.Helper() + return prepareTestToken(context.Background(), t, []any{}) }, - }, nil) - - ctx := context.Background() - creator := projects.NewProjectCreator(authzClient, marketplaces.NewNoopMarketplace(), &server.DefaultProfilesConfig{}, &server.FeaturesConfig{}, mockStore, &flags.FakeClient{}) - _, err := creator.ProvisionChildProject( - ctx, - mockStore, - parentProject, - "test-child-proj", - ) - assert.Error(t, err) + setupAuthz: func() *authmock.SimpleClient { + return &authmock.SimpleClient{Allowed: []uuid.UUID{parentProject}} + }, + setupStore: func(ctrl *gomock.Controller) *mockdb.MockStore { + s := mockdb.NewMockStore(ctrl) + s.EXPECT().GetFeatureInProject(gomock.Any(), gomock.Any()). + Return(nil, sql.ErrNoRows) + return s + }, + featureFlags: &flags.FakeClient{}, + parentProjectID: parentProject, + projectName: "child", + wantErrCode: codes.PermissionDenied, + }, + { + name: "top-level: ProjectCreateDelete flag disabled", + setupCtx: func(t *testing.T) context.Context { + t.Helper() + return prepareTestToken(context.Background(), t, []any{}) + }, + setupAuthz: func() *authmock.SimpleClient { + return &authmock.SimpleClient{} + }, + setupStore: func(ctrl *gomock.Controller) *mockdb.MockStore { + return mockdb.NewMockStore(ctrl) + }, + featureFlags: &flags.FakeClient{}, + parentProjectID: uuid.Nil, + projectName: "top", + wantErrCode: codes.Unimplemented, + }, + { + name: "top-level: no identity in context", + setupCtx: func(_ *testing.T) context.Context { + return context.Background() + }, + setupAuthz: func() *authmock.SimpleClient { + return &authmock.SimpleClient{} + }, + setupStore: func(ctrl *gomock.Controller) *mockdb.MockStore { + return mockdb.NewMockStore(ctrl) + }, + featureFlags: &flags.FakeClient{ + Data: map[string]any{ + string(flags.ProjectCreateDelete): true, + }, + }, + parentProjectID: uuid.Nil, + projectName: "top", + wantErrCode: codes.Unauthenticated, + }, + { + name: "top-level: happy path returns proto project", + setupCtx: func(t *testing.T) context.Context { + t.Helper() + ctx := prepareTestToken(context.Background(), t, []any{}) + return auth.WithIdentityContext(ctx, &auth.Identity{UserID: "user-1"}) + }, + setupAuthz: func() *authmock.SimpleClient { + return &authmock.SimpleClient{} + }, + setupStore: func(ctrl *gomock.Controller) *mockdb.MockStore { + s := mockdb.NewMockStore(ctrl) + tx := &sql.Tx{} + s.EXPECT().BeginTransaction().Return(tx, nil) + s.EXPECT().GetQuerierWithTransaction(tx).Return(s) + s.EXPECT().Rollback(tx).Return(nil) + s.EXPECT().CreateProjectWithID(gomock.Any(), gomock.Any()). + Return(db.Project{ID: childProj, Name: "top"}, nil) + s.EXPECT().CreateEntitlements(gomock.Any(), gomock.Any()).Return(nil) + s.EXPECT().Commit(tx).Return(nil) + return s + }, + featureFlags: &flags.FakeClient{ + Data: map[string]any{ + string(flags.ProjectCreateDelete): true, + }, + }, + parentProjectID: uuid.Nil, + projectName: "top", + wantNoErr: true, + }, + } - t.Log("ensure project permission was cleaned up") - assert.Len(t, authzClient.Adoptions, 0) + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + ctx := tc.setupCtx(t) + authzClient := tc.setupAuthz() + mockStore := tc.setupStore(ctrl) + + creator := projects.NewProjectCreator( + authzClient, + marketplaces.NewNoopMarketplace(), + &server.DefaultProfilesConfig{}, + &server.FeaturesConfig{}, + mockStore, + tc.featureFlags, + ) + + proj, err := creator.ProvisionProject(ctx, tc.projectName, tc.parentProjectID) + if tc.wantNoErr { + require.NoError(t, err) + require.NotNil(t, proj) + assert.Equal(t, tc.projectName, proj.Name) + } else { + require.Error(t, err) + st, ok := status.FromError(err) + require.True(t, ok, "expected a gRPC status error, got: %v", err) + assert.Equal(t, tc.wantErrCode, st.Code(), + "expected code %v, got %v: %v", tc.wantErrCode, st.Code(), err) + assert.Nil(t, proj) + } + }) + } } // prepareTestToken creates a JWT token with the specified roles and returns the context with the token. diff --git a/pkg/api/protobuf/go/minder/v1/api.go b/pkg/api/protobuf/go/minder/v1/api.go index dc371c8087..016cb2bb80 100644 --- a/pkg/api/protobuf/go/minder/v1/api.go +++ b/pkg/api/protobuf/go/minder/v1/api.go @@ -12,6 +12,7 @@ import ( "io" "google.golang.org/protobuf/encoding/protojson" + "google.golang.org/protobuf/proto" "google.golang.org/protobuf/reflect/protoreflect" "github.com/mindersec/minder/internal/util/jsonyaml" @@ -179,3 +180,18 @@ func YouMayHaveTheWrongResource(err error) bool { return err != nil && (errors.Is(err, ErrResourceTypeMismatch) || errors.Is(err, ErrNotAResource) || errors.Is(err, ErrInvalidResource) || errors.Is(err, ErrInvalidResourceType)) } + +// RelationAsName returns the OpenFGA relation name for the given Relation enum. +// It returns an empty string if the relation is unknown or has no name extension. +func RelationAsName(relation Relation) string { + relationValue := relation.Descriptor().Values().ByNumber(relation.Number()) + if relationValue == nil { + return "" + } + extension := proto.GetExtension(relationValue.Options(), E_Name) + relationName, ok := extension.(string) + if !ok { + return "" + } + return relationName +} From ffc584418fee0ce0d9d6daa9b40924a95f07452e Mon Sep 17 00:00:00 2001 From: Prashantkumar Khatri Date: Fri, 10 Jul 2026 00:13:14 +0530 Subject: [PATCH 4/5] use RelationAsName helper for authz check in ProvisionProject Co-authored-by: Evan Anderson --- internal/projects/creator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/projects/creator.go b/internal/projects/creator.go index 1c738efb84..0e49ac415b 100644 --- a/internal/projects/creator.go +++ b/internal/projects/creator.go @@ -222,7 +222,7 @@ func (p *projectCreator) ProvisionProject( if parentProjectID != uuid.Nil { // Verify permissions if we have a parent - if err := p.authzClient.Check(ctx, "create", parentProjectID); err != nil { + if err := p.authzClient.Check(ctx, minderv1.RelationAsName(minderv1.Relation_RELATION_CREATE), parentProjectID); err != nil { return nil, util.UserVisibleError( codes.PermissionDenied, "user %q is not authorized to perform this operation on project %q", auth.IdentityFromContext(ctx).Human(), parentProjectID) From 9d7b33c416d145287920fbc2ea895006369bfa29 Mon Sep 17 00:00:00 2001 From: Prashantkumar Khatri Date: Fri, 10 Jul 2026 00:16:07 +0530 Subject: [PATCH 5/5] refactor: remove unnecessary blank line in authz interface definitions Signed-off-by: Prashantkumar Khatri --- internal/authz/interface.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/authz/interface.go b/internal/authz/interface.go index 4150fc738b..030d80aa38 100644 --- a/internal/authz/interface.go +++ b/internal/authz/interface.go @@ -32,7 +32,6 @@ const ( RolePermissionsManager Role = "permissions_manager" ) - // nolint:lll var ( // AllRolesDescriptions is a list of all roles