Skip to content

Commit ebb0be4

Browse files
authored
feat(group/project): Only org admin/owners can invite new users (#2212)
Signed-off-by: Javier Rodriguez <[email protected]>
1 parent df06653 commit ebb0be4

File tree

7 files changed

+126
-36
lines changed

7 files changed

+126
-36
lines changed

app/controlplane/cmd/wire_gen.go

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

app/controlplane/pkg/authz/authz.go

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -35,22 +35,23 @@ const (
3535

3636
// Resources
3737

38-
ResourceWorkflowContract = "workflow_contract"
39-
ResourceCASArtifact = "cas_artifact"
40-
ResourceCASBackend = "cas_backend"
41-
ResourceReferrer = "referrer"
42-
ResourceAvailableIntegration = "integration_available"
43-
ResourceRegisteredIntegration = "integration_registered"
44-
ResourceAttachedIntegration = "integration_attached"
45-
ResourceOrgMetric = "metrics_org"
46-
ResourceRobotAccount = "robot_account"
47-
ResourceWorkflowRun = "workflow_run"
48-
ResourceWorkflow = "workflow"
49-
Organization = "organization"
50-
ResourceGroup = "group"
51-
ResourceGroupMembership = "group_membership"
52-
ResourceProjectAPIToken = "project_api_token"
53-
ResourceProjectMembership = "project_membership"
38+
ResourceWorkflowContract = "workflow_contract"
39+
ResourceCASArtifact = "cas_artifact"
40+
ResourceCASBackend = "cas_backend"
41+
ResourceReferrer = "referrer"
42+
ResourceAvailableIntegration = "integration_available"
43+
ResourceRegisteredIntegration = "integration_registered"
44+
ResourceAttachedIntegration = "integration_attached"
45+
ResourceOrgMetric = "metrics_org"
46+
ResourceRobotAccount = "robot_account"
47+
ResourceWorkflowRun = "workflow_run"
48+
ResourceWorkflow = "workflow"
49+
Organization = "organization"
50+
ResourceGroup = "group"
51+
ResourceGroupMembership = "group_membership"
52+
ResourceProjectAPIToken = "project_api_token"
53+
ResourceProjectMembership = "project_membership"
54+
ResourceOrganizationInvitations = "organization_invitations"
5455

5556
// We have for now three roles, viewer, admin and owner
5657
// The owner of an org
@@ -158,6 +159,8 @@ var (
158159
PolicyProjectAddMemberships = &Policy{ResourceProjectMembership, ActionCreate}
159160
PolicyProjectUpdateMemberships = &Policy{ResourceProjectMembership, ActionUpdate}
160161
PolicyProjectRemoveMemberships = &Policy{ResourceProjectMembership, ActionDelete}
162+
// Organization Invitations
163+
PolicyOrganizationInvitationsCreate = &Policy{ResourceOrganizationInvitations, ActionCreate}
161164
)
162165

163166
// RolesMap The default list of policies for each role
@@ -200,6 +203,8 @@ var RolesMap = map[Role][]*Policy{
200203
// We do a manual check in the artifact upload endpoint
201204
// so we need the actual policy in place skipping it is not enough
202205
PolicyArtifactUpload,
206+
// We manually check this policy to be able to know if the user can invite users to the system
207+
PolicyOrganizationInvitationsCreate,
203208
// + all the policies from the viewer role inherited automatically
204209
},
205210
// RoleOrgMember is an org-scoped role that enables RBAC in the underlying resources. Users with this role at

app/controlplane/pkg/biz/group.go

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,8 @@ type UpdateMemberMaintainerStatusOpts struct {
166166

167167
type GroupUseCase struct {
168168
// logger is used to log messages.
169-
logger *log.Helper
169+
logger *log.Helper
170+
enforcer *authz.Enforcer
170171
// Repositories
171172
groupRepo GroupRepo
172173
membershipRepo MembershipRepo
@@ -177,7 +178,7 @@ type GroupUseCase struct {
177178
auditorUC *AuditorUseCase
178179
}
179180

180-
func NewGroupUseCase(logger log.Logger, groupRepo GroupRepo, membershipRepo MembershipRepo, userRepo UserRepo, orgInvitationUC *OrgInvitationUseCase, auditorUC *AuditorUseCase, invitationRepo OrgInvitationRepo) *GroupUseCase {
181+
func NewGroupUseCase(logger log.Logger, groupRepo GroupRepo, membershipRepo MembershipRepo, userRepo UserRepo, orgInvitationUC *OrgInvitationUseCase, auditorUC *AuditorUseCase, invitationRepo OrgInvitationRepo, enforcer *authz.Enforcer) *GroupUseCase {
181182
return &GroupUseCase{
182183
logger: log.NewHelper(log.With(logger, "component", "biz/group")),
183184
groupRepo: groupRepo,
@@ -186,6 +187,7 @@ func NewGroupUseCase(logger log.Logger, groupRepo GroupRepo, membershipRepo Memb
186187
orgInvitationUC: orgInvitationUC,
187188
auditorUC: auditorUC,
188189
orgInvitationRepo: invitationRepo,
190+
enforcer: enforcer,
189191
}
190192
}
191193

@@ -501,6 +503,21 @@ func (uc *GroupUseCase) handleNonExistingUser(ctx context.Context, orgID, groupI
501503
return nil, NewErrAlreadyExistsStr("user is already invited to the organization")
502504
}
503505

506+
// Check if the requester is an admin or owner of the organization
507+
requesterMembership, err := uc.membershipRepo.FindByOrgAndUser(ctx, orgID, opts.RequesterID)
508+
if err != nil {
509+
return nil, fmt.Errorf("failed to check requester's role: %w", err)
510+
}
511+
512+
pass, err := uc.enforcer.Enforce(string(requesterMembership.Role), authz.PolicyOrganizationInvitationsCreate)
513+
if err != nil {
514+
return nil, fmt.Errorf("failed to check requester's role: %w", err)
515+
}
516+
517+
if !pass {
518+
return nil, NewErrValidationStr("only organization admins or owners can invite new users")
519+
}
520+
504521
// Create an organization invitation with group context
505522
invitationContext := &OrgInvitationContext{
506523
GroupIDToJoin: groupID,

app/controlplane/pkg/biz/group_integration_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -566,7 +566,7 @@ func (s *groupMembersIntegrationTestSuite) SetupTest() {
566566
assert.NoError(err)
567567

568568
// Add user to organization
569-
_, err = s.Membership.Create(ctx, s.org.ID, s.user.ID)
569+
_, err = s.Membership.Create(ctx, s.org.ID, s.user.ID, biz.WithMembershipRole(authz.RoleAdmin))
570570
assert.NoError(err)
571571

572572
// Create a group for membership tests

app/controlplane/pkg/biz/project.go

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,8 @@ type ProjectsRepo interface {
5252

5353
// ProjectUseCase is a use case for projects
5454
type ProjectUseCase struct {
55-
logger *log.Helper
55+
logger *log.Helper
56+
enforcer *authz.Enforcer
5657
// Use Cases
5758
auditorUC *AuditorUseCase
5859
groupUC *GroupUseCase
@@ -142,7 +143,7 @@ type AddMemberToProjectResult struct {
142143
InvitationSent bool
143144
}
144145

145-
func NewProjectsUseCase(logger log.Logger, projectsRepository ProjectsRepo, membershipRepository MembershipRepo, auditorUC *AuditorUseCase, groupUC *GroupUseCase, membershipUC *MembershipUseCase, orgInvitationUC *OrgInvitationUseCase, orgInvitationRepo OrgInvitationRepo) *ProjectUseCase {
146+
func NewProjectsUseCase(logger log.Logger, projectsRepository ProjectsRepo, membershipRepository MembershipRepo, auditorUC *AuditorUseCase, groupUC *GroupUseCase, membershipUC *MembershipUseCase, orgInvitationUC *OrgInvitationUseCase, orgInvitationRepo OrgInvitationRepo, enforcer *authz.Enforcer) *ProjectUseCase {
146147
return &ProjectUseCase{
147148
logger: servicelogger.ScopedHelper(logger, "biz/project"),
148149
projectsRepository: projectsRepository,
@@ -152,6 +153,7 @@ func NewProjectsUseCase(logger log.Logger, projectsRepository ProjectsRepo, memb
152153
membershipUC: membershipUC,
153154
orgInvitationUC: orgInvitationUC,
154155
orgInvitationRepo: orgInvitationRepo,
156+
enforcer: enforcer,
155157
}
156158
}
157159

@@ -337,6 +339,7 @@ func (uc *ProjectUseCase) addUserToProject(ctx context.Context, orgID uuid.UUID,
337339
}
338340

339341
// handleNonExistingUser creates an invitation for a user not yet in the organization with project context
342+
// Only sends an invitation if the requester is an admin or owner of the organization
340343
func (uc *ProjectUseCase) handleNonExistingUser(ctx context.Context, orgID, projectID uuid.UUID, opts *AddMemberToProjectOpts) (*AddMemberToProjectResult, error) {
341344
// Check if the user email is already invited to the organization
342345
invitation, err := uc.orgInvitationRepo.PendingInvitation(ctx, orgID, opts.UserEmail)
@@ -349,6 +352,21 @@ func (uc *ProjectUseCase) handleNonExistingUser(ctx context.Context, orgID, proj
349352
return nil, NewErrAlreadyExistsStr("user is already invited to the organization")
350353
}
351354

355+
// Check if the requester is an admin or owner of the organization
356+
requesterMembership, err := uc.membershipUC.FindByOrgAndUser(ctx, orgID.String(), opts.RequesterID.String())
357+
if err != nil {
358+
return nil, fmt.Errorf("failed to check requester's role: %w", err)
359+
}
360+
361+
pass, err := uc.enforcer.Enforce(string(requesterMembership.Role), authz.PolicyOrganizationInvitationsCreate)
362+
if err != nil {
363+
return nil, fmt.Errorf("failed to check requester's role: %w", err)
364+
}
365+
366+
if !pass {
367+
return nil, NewErrValidationStr("only organization admins or owners can invite new users")
368+
}
369+
352370
// Create an organization invitation with project context
353371
invitationContext := &OrgInvitationContext{
354372
ProjectIDToJoin: projectID,

app/controlplane/pkg/biz/project_integration_test.go

Lines changed: 61 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,7 @@ func (s *projectMembersIntegrationTestSuite) TestAddMemberToProject() {
309309
result, err := s.Project.AddMemberToProject(ctx, uuid.MustParse(s.org.ID), opts)
310310
s.NoError(err)
311311
s.NotNil(result)
312-
s.True(result.InvitationSent, "An invitation should be sent for users not in the organization")
312+
s.True(result.InvitationSent, "An invitation should be sent for users not in the organization when requester is an org admin")
313313
s.Nil(result.Membership, "No membership should be created directly")
314314

315315
// Verify an invitation was created
@@ -334,23 +334,73 @@ func (s *projectMembersIntegrationTestSuite) TestAddMemberToProject() {
334334
s.Equal(authz.RoleProjectViewer, foundInvitation.Context.ProjectRole)
335335
})
336336

337-
s.Run("add member who is already in the project", func() {
338-
// Try to add user2 again (who we added in the first test)
339-
opts := &biz.AddMemberToProjectOpts{
337+
s.Run("project admin (non-org admin) should not be able to invite new users", func() {
338+
// Create a project admin who is not an org admin
339+
projectAdminUser, err := s.User.UpsertByEmail(ctx, "[email protected]", nil)
340+
require.NoError(s.T(), err)
341+
342+
// Add user to organization as a regular member (not org admin)
343+
_, err = s.Membership.Create(ctx, s.org.ID, projectAdminUser.ID, biz.WithMembershipRole(authz.RoleOrgMember), biz.WithCurrentMembership())
344+
require.NoError(s.T(), err)
345+
346+
// Add user to project as a project admin
347+
projectAdminOpts := &biz.AddMemberToProjectOpts{
340348
ProjectReference: projectRef,
341-
UserEmail: "add-user2@example.com",
342-
RequesterID: uuid.MustParse(s.user.ID),
349+
UserEmail: "project-admin@example.com",
350+
RequesterID: uuid.MustParse(s.user.ID), // Using the org admin to add them
343351
Role: authz.RoleProjectAdmin,
344352
}
353+
_, err = s.Project.AddMemberToProject(ctx, uuid.MustParse(s.org.ID), projectAdminOpts)
354+
require.NoError(s.T(), err)
345355

346-
_, err := s.Project.AddMemberToProject(ctx, uuid.MustParse(s.org.ID), opts)
356+
// Create a new user who is not in the organization
357+
nonExistingEmail := "[email protected]"
358+
_, err = s.User.UpsertByEmail(ctx, nonExistingEmail, nil)
359+
require.NoError(s.T(), err)
360+
361+
// Try to add the non-existing user using the project admin
362+
opts := &biz.AddMemberToProjectOpts{
363+
ProjectReference: projectRef,
364+
UserEmail: nonExistingEmail,
365+
RequesterID: uuid.MustParse(projectAdminUser.ID),
366+
Role: authz.RoleProjectViewer,
367+
}
368+
369+
// The invitation should be rejected
370+
_, err = s.Project.AddMemberToProject(ctx, uuid.MustParse(s.org.ID), opts)
347371
s.Error(err)
348-
s.True(biz.IsErrAlreadyExists(err))
372+
s.True(biz.IsErrValidation(err))
373+
s.Contains(err.Error(), "only organization admins or owners can invite new users")
374+
})
349375

350-
// Verify the number of members hasn't changed
351-
_, count, err := s.Project.ListMembers(ctx, uuid.MustParse(s.org.ID), projectRef, nil)
376+
s.Run("org owner should be able to invite new users", func() {
377+
// Create an org owner
378+
orgOwnerUser, err := s.User.UpsertByEmail(ctx, "[email protected]", nil)
379+
require.NoError(s.T(), err)
380+
381+
// Add user to organization as an owner
382+
_, err = s.Membership.Create(ctx, s.org.ID, orgOwnerUser.ID, biz.WithMembershipRole(authz.RoleOwner), biz.WithCurrentMembership())
383+
require.NoError(s.T(), err)
384+
385+
// Create a new user who is not in the organization
386+
nonExistingEmail := "[email protected]"
387+
_, err = s.User.UpsertByEmail(ctx, nonExistingEmail, nil)
388+
require.NoError(s.T(), err)
389+
390+
// Try to add the non-existing user using the org owner
391+
opts := &biz.AddMemberToProjectOpts{
392+
ProjectReference: projectRef,
393+
UserEmail: nonExistingEmail,
394+
RequesterID: uuid.MustParse(orgOwnerUser.ID),
395+
Role: authz.RoleProjectViewer,
396+
}
397+
398+
// The invitation should be sent successfully
399+
result, err := s.Project.AddMemberToProject(ctx, uuid.MustParse(s.org.ID), opts)
352400
s.NoError(err)
353-
s.Equal(2, count) // still the original 2 members
401+
s.NotNil(result)
402+
s.True(result.InvitationSent, "An invitation should be sent for users not in the organization when requester is an org owner")
403+
s.Nil(result.Membership, "No membership should be created directly")
354404
})
355405
}
356406

app/controlplane/pkg/biz/testhelpers/wire_gen.go

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)