From 35bc7ba3de37b550a0dea7d72e9e012d7a8a0ab5 Mon Sep 17 00:00:00 2001 From: "Jose I. Paris" Date: Tue, 15 Jul 2025 16:45:18 +0200 Subject: [PATCH 1/4] err if illegal combination Signed-off-by: Jose I. Paris --- app/controlplane/pkg/biz/group.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/controlplane/pkg/biz/group.go b/app/controlplane/pkg/biz/group.go index 3f506410b..db89cfee7 100644 --- a/app/controlplane/pkg/biz/group.go +++ b/app/controlplane/pkg/biz/group.go @@ -453,6 +453,11 @@ func (uc *GroupUseCase) AddMemberToGroup(ctx context.Context, orgID uuid.UUID, o return nil, fmt.Errorf("failed to find user by email: %w", err) } + // Illegal. Org viewers cannot become maintainers + if userMembership != nil && userMembership.Role == authz.RoleViewer && opts.Maintainer { + return nil, NewErrValidationStr("org viewers cannot become group maintainers") + } + // If the user is not found in the organization, send an invitation if userMembership == nil { // We need a requester for creating invitations From 595d85cf6673af8d2a00944bee2bb1d59f150518 Mon Sep 17 00:00:00 2001 From: "Jose I. Paris" Date: Tue, 15 Jul 2025 16:49:35 +0200 Subject: [PATCH 2/4] fix tests Signed-off-by: Jose I. Paris --- .../pkg/biz/group_integration_test.go | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/app/controlplane/pkg/biz/group_integration_test.go b/app/controlplane/pkg/biz/group_integration_test.go index b1c5cd901..36571a566 100644 --- a/app/controlplane/pkg/biz/group_integration_test.go +++ b/app/controlplane/pkg/biz/group_integration_test.go @@ -912,7 +912,6 @@ func (s *groupMembersIntegrationTestSuite) TestAddMemberToGroup() { }, UserEmail: "add-user2@example.com", RequesterID: uuid.MustParse(s.user.ID), - Maintainer: true, } _, err := s.Group.AddMemberToGroup(ctx, uuid.MustParse(s.org.ID), opts) @@ -928,6 +927,22 @@ func (s *groupMembersIntegrationTestSuite) TestAddMemberToGroup() { s.NoError(err) s.Equal(3, count) // still the original 3 members }) + + s.Run("a viewer cannot be maintainer", func() { + // Try to add user2 again (who we added in the first test) + opts := &biz.AddMemberToGroupOpts{ + IdentityReference: &biz.IdentityReference{ + ID: &s.group.ID, + }, + UserEmail: "add-user2@example.com", + RequesterID: uuid.MustParse(s.user.ID), + Maintainer: true, + } + + _, err := s.Group.AddMemberToGroup(ctx, uuid.MustParse(s.org.ID), opts) + s.Error(err) + s.True(biz.IsErrValidation(err)) + }) } // Test removing members from groups From 8e1e97d17f23d9bc1ef6be10c73ec1aed0a55bcd Mon Sep 17 00:00:00 2001 From: "Jose I. Paris" Date: Tue, 15 Jul 2025 16:55:00 +0200 Subject: [PATCH 3/4] fix updates Signed-off-by: Jose I. Paris --- app/controlplane/pkg/biz/group.go | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/app/controlplane/pkg/biz/group.go b/app/controlplane/pkg/biz/group.go index db89cfee7..760ba3c18 100644 --- a/app/controlplane/pkg/biz/group.go +++ b/app/controlplane/pkg/biz/group.go @@ -769,6 +769,7 @@ func (uc *GroupUseCase) UpdateMemberMaintainerStatus(ctx context.Context, orgID // Find the user by reference or email var userUUID uuid.UUID var userEmail string + var userMembership *Membership // If UserReference is provided, use it to resolve the user ID if opts.UserReference != nil && (opts.UserReference.ID != nil || opts.UserReference.Name != nil) { @@ -776,17 +777,17 @@ func (uc *GroupUseCase) UpdateMemberMaintainerStatus(ctx context.Context, orgID if opts.UserReference.ID != nil { userUUID = *opts.UserReference.ID // Look up the user to verify they exist and get their email - user, err := uc.membershipRepo.FindByOrgAndUser(ctx, orgID, userUUID) + userMembership, err = uc.membershipRepo.FindByOrgAndUser(ctx, orgID, userUUID) if err != nil { return fmt.Errorf("failed to find user by ID: %w", err) } - if user == nil { + if userMembership == nil { return NewErrNotFound("user") } - userEmail = user.User.Email + userEmail = userMembership.User.Email } else if opts.UserReference.Name != nil { // If name (email) is provided, look up the user - userMembership, err := uc.membershipRepo.FindByOrgIDAndUserEmail(ctx, orgID, *opts.UserReference.Name) + userMembership, err = uc.membershipRepo.FindByOrgIDAndUserEmail(ctx, orgID, *opts.UserReference.Name) if err != nil && !IsNotFound(err) { return fmt.Errorf("failed to find user by email: %w", err) } @@ -798,7 +799,7 @@ func (uc *GroupUseCase) UpdateMemberMaintainerStatus(ctx context.Context, orgID } } else { // Fall back to using UserEmail - userMembership, err := uc.membershipRepo.FindByOrgIDAndUserEmail(ctx, orgID, *opts.UserReference.Name) + userMembership, err = uc.membershipRepo.FindByOrgIDAndUserEmail(ctx, orgID, *opts.UserReference.Name) if err != nil && !IsNotFound(err) { return fmt.Errorf("failed to find user by email: %w", err) } @@ -809,6 +810,11 @@ func (uc *GroupUseCase) UpdateMemberMaintainerStatus(ctx context.Context, orgID userEmail = *opts.UserReference.Name } + // illegal combination: org viewers cannot become maintainers + if userMembership != nil && userMembership.Role == authz.RoleViewer && opts.IsMaintainer { + return NewErrValidationStr("org viewers cannot become group maintainers") + } + // Check if the user is a member of the group existingMembership, err := uc.groupRepo.FindGroupMembershipByGroupAndID(ctx, resolvedGroupID, userUUID) if err != nil && !IsNotFound(err) { From 4c53c913f83d9fad6c3fdc4a91c3e6a1099a748d Mon Sep 17 00:00:00 2001 From: "Jose I. Paris" Date: Tue, 15 Jul 2025 16:58:56 +0200 Subject: [PATCH 4/4] fix tests Signed-off-by: Jose I. Paris --- app/controlplane/pkg/biz/group_integration_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/controlplane/pkg/biz/group_integration_test.go b/app/controlplane/pkg/biz/group_integration_test.go index 36571a566..bd68422c5 100644 --- a/app/controlplane/pkg/biz/group_integration_test.go +++ b/app/controlplane/pkg/biz/group_integration_test.go @@ -1531,8 +1531,7 @@ func (s *groupMembersIntegrationTestSuite) TestUpdateMemberMaintainerStatus() { UserReference: &biz.IdentityReference{ Name: &nonMemberEmail, }, - RequesterID: uuid.MustParse(s.user.ID), - IsMaintainer: true, + RequesterID: uuid.MustParse(s.user.ID), } err = s.Group.UpdateMemberMaintainerStatus(ctx, uuid.MustParse(s.org.ID), updateOpts)