Skip to content

Commit 99e2ab7

Browse files
committed
feat(projects): When adding members check all memberships
Signed-off-by: Javier Rodriguez <javier@chainloop.dev>
1 parent 063e952 commit 99e2ab7

3 files changed

Lines changed: 88 additions & 36 deletions

File tree

app/controlplane/pkg/biz/orginvitation.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -411,8 +411,8 @@ func (uc *OrgInvitationUseCase) processProjectMembership(ctx context.Context, in
411411
role = authz.RoleProjectViewer
412412
}
413413

414-
// Check if the user is already a member of the project
415-
existingMembership, err := uc.projectRepo.FindProjectMembershipByProjectAndID(ctx, orgUUID, *projectID, userUUID, authz.MembershipTypeUser)
414+
// Check if the user already has membership in the project (consider inherited memberships)
415+
existingMembership, err := uc.projectRepo.FindProjectMembershipEffective(ctx, orgUUID, *projectID, userUUID, authz.MembershipTypeUser)
416416
if err != nil && !IsNotFound(err) {
417417
return fmt.Errorf("error checking project membership for user %s: %w", userUUID, err)
418418
}

app/controlplane/pkg/biz/project.go

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,12 @@ type ProjectsRepo interface {
4343
RemoveMemberFromProject(ctx context.Context, orgID uuid.UUID, projectID uuid.UUID, memberID uuid.UUID, membershipType authz.MembershipType) error
4444
// UpdateMemberRoleInProject updates the role of a user or group in a project.
4545
UpdateMemberRoleInProject(ctx context.Context, orgID uuid.UUID, projectID uuid.UUID, memberID uuid.UUID, membershipType authz.MembershipType, newRole authz.Role) (*ProjectMembership, error)
46-
// FindProjectMembershipByProjectAndID finds a project membership by project ID and member ID (user or group).
47-
FindProjectMembershipByProjectAndID(ctx context.Context, orgID uuid.UUID, projectID uuid.UUID, memberID uuid.UUID, membershipType authz.MembershipType) (*ProjectMembership, error)
46+
// FindProjectMembershipDirect finds a project membership by project ID and member ID (user or group).
47+
// It only considers direct memberships (non-inherited).
48+
FindProjectMembershipDirect(ctx context.Context, orgID uuid.UUID, projectID uuid.UUID, memberID uuid.UUID, membershipType authz.MembershipType) (*ProjectMembership, error)
49+
// FindProjectMembershipEffective finds a project membership by project ID and member ID (user or group).
50+
// considering inherited (effective) memberships as well.
51+
FindProjectMembershipEffective(ctx context.Context, orgID uuid.UUID, projectID uuid.UUID, memberID uuid.UUID, membershipType authz.MembershipType) (*ProjectMembership, error)
4852
// ListPendingInvitationsByProject retrieves a list of pending invitations for a project.
4953
ListPendingInvitationsByProject(ctx context.Context, orgID uuid.UUID, projectID uuid.UUID, paginationOpts *pagination.OffsetPaginationOpts) ([]*OrgInvitation, int, error)
5054
}
@@ -342,8 +346,8 @@ func (uc *ProjectUseCase) addUserToProject(ctx context.Context, orgID uuid.UUID,
342346

343347
userUUID := uuid.MustParse(userMembership.User.ID)
344348

345-
// Check if the user is already a member of the project
346-
existingMembership, err := uc.projectsRepository.FindProjectMembershipByProjectAndID(ctx, orgID, projectID, userUUID, authz.MembershipTypeUser)
349+
// Check if the user is already a member of the project (consider inherited memberships)
350+
existingMembership, err := uc.projectsRepository.FindProjectMembershipEffective(ctx, orgID, projectID, userUUID, authz.MembershipTypeUser)
347351
if err != nil && !IsNotFound(err) {
348352
return nil, fmt.Errorf("failed to check existing membership: %w", err)
349353
}
@@ -428,8 +432,8 @@ func (uc *ProjectUseCase) addGroupToProject(ctx context.Context, orgID uuid.UUID
428432
return nil, fmt.Errorf("failed to validate group reference: %w", err)
429433
}
430434

431-
// Check if the group already has membership in the project
432-
existingMembership, err := uc.projectsRepository.FindProjectMembershipByProjectAndID(ctx, orgID, projectID, resolvedGroupID, authz.MembershipTypeGroup)
435+
// Check if the group already has membership in the project (consider inherited memberships)
436+
existingMembership, err := uc.projectsRepository.FindProjectMembershipEffective(ctx, orgID, projectID, resolvedGroupID, authz.MembershipTypeGroup)
433437
if err != nil && !IsNotFound(err) {
434438
return nil, fmt.Errorf("failed to check existing group membership: %w", err)
435439
}
@@ -513,7 +517,7 @@ func (uc *ProjectUseCase) removeUserFromProject(ctx context.Context, orgID uuid.
513517
userUUID := uuid.MustParse(userMembership.User.ID)
514518

515519
// Check if the user is a member of the project
516-
existingMembership, err := uc.projectsRepository.FindProjectMembershipByProjectAndID(ctx, orgID, projectID, userUUID, authz.MembershipTypeUser)
520+
existingMembership, err := uc.projectsRepository.FindProjectMembershipDirect(ctx, orgID, projectID, userUUID, authz.MembershipTypeUser)
517521
if err != nil && !IsNotFound(err) {
518522
return fmt.Errorf("failed to check existing membership: %w", err)
519523
}
@@ -548,7 +552,7 @@ func (uc *ProjectUseCase) removeGroupFromProject(ctx context.Context, orgID uuid
548552
}
549553

550554
// Check if the group has membership in the project
551-
existingMembership, err := uc.projectsRepository.FindProjectMembershipByProjectAndID(ctx, orgID, projectID, resolvedGroupID, authz.MembershipTypeGroup)
555+
existingMembership, err := uc.projectsRepository.FindProjectMembershipDirect(ctx, orgID, projectID, resolvedGroupID, authz.MembershipTypeGroup)
552556
if err != nil && !IsNotFound(err) {
553557
return fmt.Errorf("failed to check existing group membership: %w", err)
554558
}
@@ -749,7 +753,7 @@ func (uc *ProjectUseCase) updateUserRoleInProject(ctx context.Context, orgID uui
749753
userUUID := uuid.MustParse(userMembership.User.ID)
750754

751755
// Check if the user is a member of the project
752-
existingMembership, err := uc.projectsRepository.FindProjectMembershipByProjectAndID(ctx, orgID, projectID, userUUID, authz.MembershipTypeUser)
756+
existingMembership, err := uc.projectsRepository.FindProjectMembershipDirect(ctx, orgID, projectID, userUUID, authz.MembershipTypeUser)
753757
if err != nil && !IsNotFound(err) {
754758
return fmt.Errorf("failed to check existing membership: %w", err)
755759
}
@@ -791,7 +795,7 @@ func (uc *ProjectUseCase) updateGroupRoleInProject(ctx context.Context, orgID uu
791795
}
792796

793797
// Check if the group has membership in the project
794-
existingMembership, err := uc.projectsRepository.FindProjectMembershipByProjectAndID(ctx, orgID, projectID, resolvedGroupID, authz.MembershipTypeGroup)
798+
existingMembership, err := uc.projectsRepository.FindProjectMembershipDirect(ctx, orgID, projectID, resolvedGroupID, authz.MembershipTypeGroup)
795799
if err != nil && !IsNotFound(err) {
796800
return fmt.Errorf("failed to check existing group membership: %w", err)
797801
}

app/controlplane/pkg/data/project.go

Lines changed: 72 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ func (r *ProjectRepo) AddMemberToProject(ctx context.Context, orgID uuid.UUID, p
214214
}
215215

216216
// Return the created membership
217-
return r.FindProjectMembershipByProjectAndID(ctx, orgID, projectID, memberID, membershipType)
217+
return r.FindProjectMembershipDirect(ctx, orgID, projectID, memberID, membershipType)
218218
}
219219

220220
// RemoveMemberFromProject removes a user or group from a project
@@ -229,7 +229,7 @@ func (r *ProjectRepo) RemoveMemberFromProject(ctx context.Context, orgID uuid.UU
229229
}
230230

231231
// Find the membership to delete
232-
m, err := r.queryMembership(orgID, projectID, memberID, membershipType).Only(ctx)
232+
m, err := r.buildDirectMembershipQuery(orgID, projectID, memberID, membershipType).Only(ctx)
233233

234234
if err != nil {
235235
if ent.IsNotFound(err) {
@@ -246,10 +246,10 @@ func (r *ProjectRepo) RemoveMemberFromProject(ctx context.Context, orgID uuid.UU
246246
return nil
247247
}
248248

249-
// FindProjectMembershipByProjectAndID finds a project membership by project ID and member ID (user or group)
250-
func (r *ProjectRepo) FindProjectMembershipByProjectAndID(ctx context.Context, orgID uuid.UUID, projectID uuid.UUID, memberID uuid.UUID, membershipType authz.MembershipType) (*biz.ProjectMembership, error) {
251-
// Find the membership
252-
m, err := r.queryMembership(orgID, projectID, memberID, membershipType).Only(ctx)
249+
// FindProjectMembershipDirect finds a project membership by project ID and member ID (user or group)
250+
func (r *ProjectRepo) FindProjectMembershipDirect(ctx context.Context, orgID uuid.UUID, projectID uuid.UUID, memberID uuid.UUID, membershipType authz.MembershipType) (*biz.ProjectMembership, error) {
251+
// Find the membership (direct only)
252+
m, err := r.buildDirectMembershipQuery(orgID, projectID, memberID, membershipType).Only(ctx)
253253

254254
if err != nil {
255255
if ent.IsNotFound(err) {
@@ -258,38 +258,29 @@ func (r *ProjectRepo) FindProjectMembershipByProjectAndID(ctx context.Context, o
258258
return nil, fmt.Errorf("failed to find membership: %w", err)
259259
}
260260

261-
// Build the membership response based on the membership type
262-
projectMembership := &biz.ProjectMembership{
263-
MembershipType: m.MembershipType,
264-
Role: m.Role,
265-
CreatedAt: &m.CreatedAt,
266-
UpdatedAt: &m.UpdatedAt,
267-
}
268-
261+
// Use centralized converter and fetch associated user/group when needed
269262
switch membershipType {
270263
case authz.MembershipTypeUser:
271-
// Fetch the user details for user memberships
272264
u, err := r.data.DB.User.Get(ctx, memberID)
273265
if err != nil {
274266
if ent.IsNotFound(err) {
275267
return nil, biz.NewErrNotFound("user")
276268
}
277269
return nil, fmt.Errorf("failed to find user: %w", err)
278270
}
279-
projectMembership.User = entUserToBizUser(u)
271+
return entProjectMembershipToBiz(m, u, nil), nil
280272
case authz.MembershipTypeGroup:
281-
// Fetch the group details for group memberships
282273
g, err := r.data.DB.Group.Query().Where(group.ID(memberID), group.DeletedAtIsNil()).Only(ctx)
283274
if err != nil {
284275
if ent.IsNotFound(err) {
285276
return nil, biz.NewErrNotFound("group")
286277
}
287278
return nil, fmt.Errorf("failed to find group: %w", err)
288279
}
289-
projectMembership.Group = entGroupToBiz(g)
280+
return entProjectMembershipToBiz(m, nil, g), nil
281+
default:
282+
return entProjectMembershipToBiz(m, nil, nil), nil
290283
}
291-
292-
return projectMembership, nil
293284
}
294285

295286
// UpdateMemberRoleInProject updates the role of a member in a project
@@ -308,7 +299,7 @@ func (r *ProjectRepo) UpdateMemberRoleInProject(ctx context.Context, orgID uuid.
308299
}
309300

310301
// Find the membership to update
311-
m, err := r.queryMembership(orgID, projectID, memberID, membershipType).Only(ctx)
302+
m, err := r.buildDirectMembershipQuery(orgID, projectID, memberID, membershipType).Only(ctx)
312303

313304
if err != nil {
314305
if ent.IsNotFound(err) {
@@ -326,8 +317,24 @@ func (r *ProjectRepo) UpdateMemberRoleInProject(ctx context.Context, orgID uuid.
326317
return entProjectMembershipToBiz(m, nil, nil), nil
327318
}
328319

329-
// queryMembership is a helper function to build a common membership query
330-
func (r *ProjectRepo) queryMembership(orgID uuid.UUID, projectID uuid.UUID, memberID uuid.UUID, membershipType authz.MembershipType) *ent.MembershipQuery {
320+
// buildDirectMembershipQuery constructs a query that only considers direct memberships
321+
func (r *ProjectRepo) buildDirectMembershipQuery(orgID uuid.UUID, projectID uuid.UUID, memberID uuid.UUID, membershipType authz.MembershipType) *ent.MembershipQuery {
322+
return r.data.DB.Membership.Query().
323+
Where(
324+
membership.HasOrganizationWith(
325+
organization.ID(orgID),
326+
),
327+
membership.MembershipTypeEQ(membershipType),
328+
membership.MemberID(memberID),
329+
membership.ResourceTypeEQ(authz.ResourceTypeProject),
330+
membership.ResourceID(projectID),
331+
// only direct memberships (parent is nil)
332+
membership.ParentIDIsNil(),
333+
).WithOrganization()
334+
}
335+
336+
// buildEffectiveMembershipQuery constructs a query that considers direct and inherited memberships
337+
func (r *ProjectRepo) buildEffectiveMembershipQuery(orgID uuid.UUID, projectID uuid.UUID, memberID uuid.UUID, membershipType authz.MembershipType) *ent.MembershipQuery {
331338
return r.data.DB.Membership.Query().
332339
Where(
333340
membership.HasOrganizationWith(
@@ -337,10 +344,51 @@ func (r *ProjectRepo) queryMembership(orgID uuid.UUID, projectID uuid.UUID, memb
337344
membership.MemberID(memberID),
338345
membership.ResourceTypeEQ(authz.ResourceTypeProject),
339346
membership.ResourceID(projectID),
340-
membership.ParentIDIsNil(), // Only top-level memberships
347+
// include both direct (parent nil) and indirect (parent not nil) memberships
348+
membership.Or(
349+
membership.ParentIDIsNil(),
350+
membership.ParentIDNotNil(),
351+
),
341352
).WithOrganization()
342353
}
343354

355+
// FindProjectMembershipEffective finds a project membership considering both direct and inherited
356+
// memberships for the member on the project.
357+
func (r *ProjectRepo) FindProjectMembershipEffective(ctx context.Context, orgID uuid.UUID, projectID uuid.UUID, memberID uuid.UUID, membershipType authz.MembershipType) (*biz.ProjectMembership, error) {
358+
m, err := r.buildEffectiveMembershipQuery(orgID, projectID, memberID, membershipType).Only(ctx)
359+
360+
if err != nil {
361+
if ent.IsNotFound(err) {
362+
return nil, nil // Return nil when no membership found
363+
}
364+
return nil, fmt.Errorf("failed to find membership: %w", err)
365+
}
366+
367+
// Use centralized converter and fetch associated user/group when needed
368+
switch membershipType {
369+
case authz.MembershipTypeUser:
370+
u, err := r.data.DB.User.Get(ctx, memberID)
371+
if err != nil {
372+
if ent.IsNotFound(err) {
373+
return nil, biz.NewErrNotFound("user")
374+
}
375+
return nil, fmt.Errorf("failed to find user: %w", err)
376+
}
377+
return entProjectMembershipToBiz(m, u, nil), nil
378+
case authz.MembershipTypeGroup:
379+
g, err := r.data.DB.Group.Query().Where(group.ID(memberID), group.DeletedAtIsNil()).Only(ctx)
380+
if err != nil {
381+
if ent.IsNotFound(err) {
382+
return nil, biz.NewErrNotFound("group")
383+
}
384+
return nil, fmt.Errorf("failed to find group: %w", err)
385+
}
386+
return entProjectMembershipToBiz(m, nil, g), nil
387+
default:
388+
return entProjectMembershipToBiz(m, nil, nil), nil
389+
}
390+
}
391+
344392
// entProjectToBiz converts an ent.Project to a biz.Project
345393
func entProjectToBiz(pro *ent.Project) *biz.Project {
346394
return &biz.Project{

0 commit comments

Comments
 (0)