diff --git a/app/controlplane/cmd/wire_gen.go b/app/controlplane/cmd/wire_gen.go index cd3ab2814..843d62557 100644 --- a/app/controlplane/cmd/wire_gen.go +++ b/app/controlplane/cmd/wire_gen.go @@ -83,7 +83,7 @@ func wireApp(bootstrap *conf.Bootstrap, readerWriter credentials.ReaderWriter, l integrationUseCase := biz.NewIntegrationUseCase(newIntegrationUseCaseOpts) v := bootstrap.Onboarding organizationUseCase := biz.NewOrganizationUseCase(organizationRepo, casBackendUseCase, auditorUseCase, integrationUseCase, membershipRepo, v, logger) - membershipUseCase := biz.NewMembershipUseCase(membershipRepo, organizationUseCase, auditorUseCase, userRepo, logger) + membershipUseCase := biz.NewMembershipUseCase(membershipRepo, organizationUseCase, auditorUseCase, userRepo, authzUseCase, logger) allowList := newAuthAllowList(bootstrap) userAccessSyncerUseCase := biz.NewUserAccessSyncerUseCase(logger, userRepo, allowList) newUserUseCaseParams := &biz.NewUserUseCaseParams{ diff --git a/app/controlplane/internal/service/organization.go b/app/controlplane/internal/service/organization.go index 422c38dc5..7242de52e 100644 --- a/app/controlplane/internal/service/organization.go +++ b/app/controlplane/internal/service/organization.go @@ -19,6 +19,7 @@ import ( "context" pb "github.com/chainloop-dev/chainloop/app/controlplane/api/controlplane/v1" + "github.com/chainloop-dev/chainloop/app/controlplane/internal/usercontext" "github.com/chainloop-dev/chainloop/app/controlplane/internal/usercontext/entities" "github.com/chainloop-dev/chainloop/app/controlplane/pkg/authz" "github.com/chainloop-dev/chainloop/app/controlplane/pkg/biz" @@ -196,7 +197,8 @@ func (s *OrganizationService) DeleteMembership(ctx context.Context, req *pb.Orga return nil, err } - if err := s.membershipUC.DeleteOther(ctx, currentOrg.ID, currentUser.ID, req.MembershipId); err != nil { + callerRole := authz.Role(usercontext.CurrentAuthzSubject(ctx)) + if err := s.membershipUC.DeleteOther(ctx, currentOrg.ID, currentUser.ID, req.MembershipId, callerRole); err != nil { return nil, handleUseCaseErr(err, s.log) } @@ -214,7 +216,8 @@ func (s *OrganizationService) UpdateMembership(ctx context.Context, req *pb.Orga return nil, err } - m, err := s.membershipUC.UpdateRole(ctx, currentOrg.ID, currentUser.ID, req.MembershipId, biz.PbRoleToBiz(req.Role)) + callerRole := authz.Role(usercontext.CurrentAuthzSubject(ctx)) + m, err := s.membershipUC.UpdateRole(ctx, currentOrg.ID, currentUser.ID, req.MembershipId, biz.PbRoleToBiz(req.Role), callerRole) if err != nil { return nil, handleUseCaseErr(err, s.log) } diff --git a/app/controlplane/pkg/authz/authz.go b/app/controlplane/pkg/authz/authz.go index df914ebef..3a2adee97 100644 --- a/app/controlplane/pkg/authz/authz.go +++ b/app/controlplane/pkg/authz/authz.go @@ -1,5 +1,5 @@ // -// Copyright 2024-2025 The Chainloop Authors. +// Copyright 2024-2026 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -36,11 +36,12 @@ func (r Role) IsAdmin() bool { const ( // Actions - ActionRead = "read" - ActionList = "list" - ActionCreate = "create" - ActionUpdate = "update" - ActionDelete = "delete" + ActionRead = "read" + ActionList = "list" + ActionCreate = "create" + ActionUpdate = "update" + ActionDelete = "delete" + ActionManageOwners = "manage_owners" // Resources @@ -175,6 +176,9 @@ var ( PolicyProjectRemoveMemberships = &Policy{ResourceProjectMembership, ActionDelete} // Organization Invitations PolicyOrganizationInvitationsCreate = &Policy{ResourceOrganizationInvitations, ActionCreate} + + // Manage owners (promote to/demote from owner, remove owners) + PolicyOrganizationManageOwners = &Policy{OrganizationMemberships, ActionManageOwners} ) // RolesMap The default list of policies for each role @@ -193,6 +197,7 @@ var RolesMap = map[Role][]*Policy{ }, RoleOwner: { PolicyOrganizationDelete, + PolicyOrganizationManageOwners, }, // RoleAdmin is an org-scoped role that provides super admin privileges (it's the higher role) RoleAdmin: { diff --git a/app/controlplane/pkg/biz/membership.go b/app/controlplane/pkg/biz/membership.go index 032ff942b..d4e93a3ef 100644 --- a/app/controlplane/pkg/biz/membership.go +++ b/app/controlplane/pkg/biz/membership.go @@ -1,5 +1,5 @@ // -// Copyright 2024-2025 The Chainloop Authors. +// Copyright 2024-2026 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -90,15 +90,17 @@ type MembershipUseCase struct { // Use Cases orgUseCase *OrganizationUseCase auditor *AuditorUseCase + authzUC *AuthzUseCase } -func NewMembershipUseCase(repo MembershipRepo, orgUC *OrganizationUseCase, auditor *AuditorUseCase, userRepo UserRepo, logger log.Logger) *MembershipUseCase { - return &MembershipUseCase{repo: repo, orgUseCase: orgUC, logger: log.NewHelper(logger), userRepo: userRepo, auditor: auditor} +func NewMembershipUseCase(repo MembershipRepo, orgUC *OrganizationUseCase, auditor *AuditorUseCase, userRepo UserRepo, authzUC *AuthzUseCase, logger log.Logger) *MembershipUseCase { + return &MembershipUseCase{repo: repo, orgUseCase: orgUC, logger: log.NewHelper(logger), userRepo: userRepo, auditor: auditor, authzUC: authzUC} } // DeleteOther just deletes a membership from the database -// but ensures that the user is not deleting itself from the org -func (uc *MembershipUseCase) DeleteOther(ctx context.Context, orgID, userID, membershipID string) error { +// but ensures that the user is not deleting itself from the org. +// callerRole is the authorization subject of the caller (e.g. their org role). +func (uc *MembershipUseCase) DeleteOther(ctx context.Context, orgID, userID, membershipID string, callerRole authz.Role) error { membershipUUID, err := uuid.Parse(membershipID) if err != nil { return NewErrInvalidUUID(err) @@ -120,6 +122,13 @@ func (uc *MembershipUseCase) DeleteOther(ctx context.Context, orgID, userID, mem return NewErrValidationStr("cannot delete yourself from the org") } + // Only owners can remove other owners + if m.Role == authz.RoleOwner { + if err := uc.enforceManageOwners(ctx, callerRole); err != nil { + return err + } + } + uc.logger.Infow("msg", "Deleting membership", "org_id", orgID, "membership_id", m.ID.String()) // Delete the main membership - this will also remove the user from all groups in the org @@ -131,7 +140,7 @@ func (uc *MembershipUseCase) DeleteOther(ctx context.Context, orgID, userID, mem return nil } -func (uc *MembershipUseCase) UpdateRole(ctx context.Context, orgID, userID, membershipID string, role authz.Role) (*Membership, error) { +func (uc *MembershipUseCase) UpdateRole(ctx context.Context, orgID, userID, membershipID string, role authz.Role, callerRole authz.Role) (*Membership, error) { // If it has ben overrode by the user, validate it if role == "" { return nil, NewErrValidationStr("role is required") @@ -160,6 +169,13 @@ func (uc *MembershipUseCase) UpdateRole(ctx context.Context, orgID, userID, memb return nil, NewErrValidationStr("cannot update yourself") } + // Only owners can modify owner memberships or promote users to owner + if m.Role == authz.RoleOwner || role == authz.RoleOwner { + if err := uc.enforceManageOwners(ctx, callerRole); err != nil { + return nil, err + } + } + userUUID, err := uuid.Parse(m.User.ID) if err != nil { return nil, NewErrInvalidUUID(err) @@ -187,6 +203,20 @@ func (uc *MembershipUseCase) UpdateRole(ctx context.Context, orgID, userID, memb return updatedMembership, nil } +// enforceManageOwners checks that the caller has the manage_owners policy. +func (uc *MembershipUseCase) enforceManageOwners(ctx context.Context, callerRole authz.Role) error { + ok, err := uc.authzUC.Enforce(ctx, string(callerRole), authz.PolicyOrganizationManageOwners) + if err != nil { + return fmt.Errorf("failed to enforce manage owners policy: %w", err) + } + + if !ok { + return NewErrUnauthorizedStr("only organization owners can manage owner memberships") + } + + return nil +} + type membershipCreateOpts struct { current bool role authz.Role diff --git a/app/controlplane/pkg/biz/membership_integration_test.go b/app/controlplane/pkg/biz/membership_integration_test.go index 27d3242d7..af32d91da 100644 --- a/app/controlplane/pkg/biz/membership_integration_test.go +++ b/app/controlplane/pkg/biz/membership_integration_test.go @@ -1,5 +1,5 @@ // -// Copyright 2024-2025 The Chainloop Authors. +// Copyright 2024-2026 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -229,12 +229,12 @@ func (s *membershipIntegrationTestSuite) TestDeleteOther() { s.NoError(err) s.T().Run("I can not delete my own membership", func(t *testing.T) { - err := s.Membership.DeleteOther(ctx, sharedOrg.ID, user2.ID, mUser2SharedOrg.ID.String()) + err := s.Membership.DeleteOther(ctx, sharedOrg.ID, user2.ID, mUser2SharedOrg.ID.String(), authz.RoleOwner) s.ErrorContains(err, "cannot delete yourself from the org") }) s.T().Run("I can't find the membership", func(t *testing.T) { - err := s.Membership.DeleteOther(ctx, otherOrg.ID, user2.ID, mUser2SharedOrg.ID.String()) + err := s.Membership.DeleteOther(ctx, otherOrg.ID, user2.ID, mUser2SharedOrg.ID.String(), authz.RoleOwner) s.True(biz.IsNotFound(err)) }) @@ -244,7 +244,7 @@ func (s *membershipIntegrationTestSuite) TestDeleteOther() { s.Len(memberships, 1) s.Equal(1, count) - err = s.Membership.DeleteOther(ctx, sharedOrg.ID, user.ID, mUser2SharedOrg.ID.String()) + err = s.Membership.DeleteOther(ctx, sharedOrg.ID, user.ID, mUser2SharedOrg.ID.String(), authz.RoleOwner) s.NoError(err) memberships, count, err = s.Membership.ByOrg(ctx, sharedOrg.ID, &biz.ListByOrgOpts{}, nil) @@ -270,17 +270,17 @@ func (s *membershipIntegrationTestSuite) TestUpdateRole() { // empty role s.T().Run("a role is required", func(t *testing.T) { - _, err := s.Membership.UpdateRole(ctx, sharedOrg.ID, user.ID, mUser2SharedOrg.ID.String(), "") + _, err := s.Membership.UpdateRole(ctx, sharedOrg.ID, user.ID, mUser2SharedOrg.ID.String(), "", authz.RoleOwner) s.ErrorContains(err, "role is required") }) s.T().Run("I can not update my own membership", func(t *testing.T) { - _, err := s.Membership.UpdateRole(ctx, sharedOrg.ID, user2.ID, mUser2SharedOrg.ID.String(), authz.RoleAdmin) + _, err := s.Membership.UpdateRole(ctx, sharedOrg.ID, user2.ID, mUser2SharedOrg.ID.String(), authz.RoleAdmin, authz.RoleOwner) s.ErrorContains(err, "cannot update yourself") }) s.T().Run("I can't find the membership in another org", func(t *testing.T) { - _, err := s.Membership.UpdateRole(ctx, otherOrg.ID, user.ID, mUser2SharedOrg.ID.String(), authz.RoleAdmin) + _, err := s.Membership.UpdateRole(ctx, otherOrg.ID, user.ID, mUser2SharedOrg.ID.String(), authz.RoleAdmin, authz.RoleOwner) s.True(biz.IsNotFound(err)) }) @@ -291,12 +291,81 @@ func (s *membershipIntegrationTestSuite) TestUpdateRole() { s.Equal(1, count) s.Equal(authz.RoleViewer, memberships[0].Role) - got, err := s.Membership.UpdateRole(ctx, sharedOrg.ID, user.ID, mUser2SharedOrg.ID.String(), authz.RoleAdmin) + got, err := s.Membership.UpdateRole(ctx, sharedOrg.ID, user.ID, mUser2SharedOrg.ID.String(), authz.RoleAdmin, authz.RoleOwner) s.NoError(err) s.Equal(authz.RoleAdmin, got.Role) }) } +func (s *membershipIntegrationTestSuite) TestOwnerGuards() { + ctx := context.Background() + + owner, err := s.User.UpsertByEmail(ctx, "owner@test.com", nil) + s.NoError(err) + admin, err := s.User.UpsertByEmail(ctx, "admin@test.com", nil) + s.NoError(err) + target, err := s.User.UpsertByEmail(ctx, "target@test.com", nil) + s.NoError(err) + + org, err := s.Organization.CreateWithRandomName(ctx) + s.NoError(err) + + // owner is org owner, admin is org admin, target starts as viewer + _, err = s.Membership.Create(ctx, org.ID, owner.ID, biz.WithMembershipRole(authz.RoleOwner)) + s.NoError(err) + _, err = s.Membership.Create(ctx, org.ID, admin.ID, biz.WithMembershipRole(authz.RoleAdmin)) + s.NoError(err) + mTarget, err := s.Membership.Create(ctx, org.ID, target.ID, biz.WithMembershipRole(authz.RoleViewer)) + s.NoError(err) + + s.Run("admin cannot promote user to owner", func() { + _, err := s.Membership.UpdateRole(ctx, org.ID, admin.ID, mTarget.ID.String(), authz.RoleOwner, authz.RoleAdmin) + s.Error(err) + s.True(biz.IsErrUnauthorized(err)) + s.Contains(err.Error(), "only organization owners can manage owner memberships") + }) + + s.Run("owner can promote user to owner", func() { + got, err := s.Membership.UpdateRole(ctx, org.ID, owner.ID, mTarget.ID.String(), authz.RoleOwner, authz.RoleOwner) + s.NoError(err) + s.Equal(authz.RoleOwner, got.Role) + }) + + s.Run("admin cannot demote an owner", func() { + _, err := s.Membership.UpdateRole(ctx, org.ID, admin.ID, mTarget.ID.String(), authz.RoleAdmin, authz.RoleAdmin) + s.Error(err) + s.True(biz.IsErrUnauthorized(err)) + s.Contains(err.Error(), "only organization owners can manage owner memberships") + }) + + s.Run("owner can demote another owner", func() { + got, err := s.Membership.UpdateRole(ctx, org.ID, owner.ID, mTarget.ID.String(), authz.RoleAdmin, authz.RoleOwner) + s.NoError(err) + s.Equal(authz.RoleAdmin, got.Role) + }) + + s.Run("admin cannot delete an owner membership", func() { + // First promote target back to owner + _, err := s.Membership.UpdateRole(ctx, org.ID, owner.ID, mTarget.ID.String(), authz.RoleOwner, authz.RoleOwner) + s.NoError(err) + + err = s.Membership.DeleteOther(ctx, org.ID, admin.ID, mTarget.ID.String(), authz.RoleAdmin) + s.Error(err) + s.True(biz.IsErrUnauthorized(err)) + s.Contains(err.Error(), "only organization owners can manage owner memberships") + }) + + s.Run("owner can delete another owner membership", func() { + err := s.Membership.DeleteOther(ctx, org.ID, owner.ID, mTarget.ID.String(), authz.RoleOwner) + s.NoError(err) + + memberships, count, err := s.Membership.ByOrg(ctx, org.ID, &biz.ListByOrgOpts{}, nil) + s.NoError(err) + s.Equal(2, count) // only owner and admin remain + s.Len(memberships, 2) + }) +} + func (s *membershipIntegrationTestSuite) TestCreateMembership() { assert := assert.New(s.T()) ctx := context.Background() diff --git a/app/controlplane/pkg/biz/testhelpers/wire_gen.go b/app/controlplane/pkg/biz/testhelpers/wire_gen.go index 29666ff42..997c5d4a9 100644 --- a/app/controlplane/pkg/biz/testhelpers/wire_gen.go +++ b/app/controlplane/pkg/biz/testhelpers/wire_gen.go @@ -72,7 +72,16 @@ func WireTestData(testDatabase *TestDatabase, t *testing.T, logger log.Logger, r integrationUseCase := biz.NewIntegrationUseCase(newIntegrationUseCaseOpts) organizationUseCase := biz.NewOrganizationUseCase(organizationRepo, casBackendUseCase, auditorUseCase, integrationUseCase, membershipRepo, arg, logger) userRepo := data.NewUserRepo(dataData, logger) - membershipUseCase := biz.NewMembershipUseCase(membershipRepo, organizationUseCase, auditorUseCase, userRepo, logger) + config := authzConfig() + casbinEnforcer, err := authz.NewCasbinEnforcer(config) + if err != nil { + cleanup() + return nil, nil, err + } + apiTokenRepo := data.NewAPITokenRepo(dataData, logger) + bizAuthzUseCaseConfig := authzUseCaseConfig(bootstrap, casbinEnforcer, apiTokenRepo, logger) + authzUseCase := biz.NewAuthzUseCase(bizAuthzUseCaseConfig) + membershipUseCase := biz.NewMembershipUseCase(membershipRepo, organizationUseCase, auditorUseCase, userRepo, authzUseCase, logger) workflowContractRepo := data.NewWorkflowContractRepo(dataData, logger) v := NewPolicyProviderConfig(bootstrap) registry, err := policies.NewRegistry(logger, v...) @@ -136,16 +145,7 @@ func WireTestData(testDatabase *TestDatabase, t *testing.T, logger log.Logger, r cleanup() return nil, nil, err } - apiTokenRepo := data.NewAPITokenRepo(dataData, logger) apiTokenJWTConfig := newJWTConfig(auth) - config := authzConfig() - casbinEnforcer, err := authz.NewCasbinEnforcer(config) - if err != nil { - cleanup() - return nil, nil, err - } - bizAuthzUseCaseConfig := authzUseCaseConfig(bootstrap, casbinEnforcer, apiTokenRepo, logger) - authzUseCase := biz.NewAuthzUseCase(bizAuthzUseCaseConfig) apiTokenUseCase, err := biz.NewAPITokenUseCase(apiTokenRepo, apiTokenJWTConfig, authzUseCase, organizationUseCase, auditorUseCase, logger) if err != nil { cleanup()