Skip to content

Commit 4be0473

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

3 files changed

Lines changed: 47 additions & 34 deletions

File tree

app/controlplane/pkg/biz/orginvitation.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -411,13 +411,13 @@ 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)
416-
if err != nil && !IsNotFound(err) {
414+
// Check if the user already has membership in the project (consider inherited memberships)
415+
membershipExists, err := uc.projectRepo.ExistsProjectMembershipEffective(ctx, orgUUID, *projectID, userUUID, authz.MembershipTypeUser)
416+
if err != nil {
417417
return fmt.Errorf("error checking project membership for user %s: %w", userUUID, err)
418418
}
419419

420-
if existingMembership != nil {
420+
if membershipExists {
421421
// User is already a member of the project, nothing to do
422422
uc.logger.Infow("msg", "User already in project", "invitation_id", invitation.ID.String(), "org_id", invitation.Org.ID, "user_id", userUUID.String(), "project_id", projectID.String())
423423
return nil

app/controlplane/pkg/biz/project.go

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,9 @@ type ProjectsRepo interface {
4545
UpdateMemberRoleInProject(ctx context.Context, orgID uuid.UUID, projectID uuid.UUID, memberID uuid.UUID, membershipType authz.MembershipType, newRole authz.Role) (*ProjectMembership, error)
4646
// FindProjectMembershipByProjectAndID finds a project membership by project ID and member ID (user or group).
4747
FindProjectMembershipByProjectAndID(ctx context.Context, orgID uuid.UUID, projectID uuid.UUID, memberID uuid.UUID, membershipType authz.MembershipType) (*ProjectMembership, error)
48+
// ExistsProjectMembershipEffective checks if a project membership by project ID and member ID (user or group).
49+
// considering inherited (effective) memberships as well.
50+
ExistsProjectMembershipEffective(ctx context.Context, orgID uuid.UUID, projectID uuid.UUID, memberID uuid.UUID, membershipType authz.MembershipType) (bool, error)
4851
// ListPendingInvitationsByProject retrieves a list of pending invitations for a project.
4952
ListPendingInvitationsByProject(ctx context.Context, orgID uuid.UUID, projectID uuid.UUID, paginationOpts *pagination.OffsetPaginationOpts) ([]*OrgInvitation, int, error)
5053
}
@@ -342,12 +345,13 @@ func (uc *ProjectUseCase) addUserToProject(ctx context.Context, orgID uuid.UUID,
342345

343346
userUUID := uuid.MustParse(userMembership.User.ID)
344347

345-
// Check if the user is already a member of the project
346-
existingMembership, err := uc.projectsRepository.FindProjectMembershipByProjectAndID(ctx, orgID, projectID, userUUID, authz.MembershipTypeUser)
347-
if err != nil && !IsNotFound(err) {
348+
// Check if the user is already a member of the project (including inherited memberships)
349+
membershipExists, err := uc.projectsRepository.ExistsProjectMembershipEffective(ctx, orgID, projectID, userUUID, authz.MembershipTypeUser)
350+
if err != nil {
348351
return nil, fmt.Errorf("failed to check existing membership: %w", err)
349352
}
350-
if existingMembership != nil {
353+
354+
if membershipExists {
351355
return nil, NewErrAlreadyExistsStr("user is already a member of this project")
352356
}
353357

@@ -428,12 +432,13 @@ 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)
433-
if err != nil && !IsNotFound(err) {
435+
// Check if the group is already a member of the project (including inherited memberships)
436+
membershipExists, err := uc.projectsRepository.ExistsProjectMembershipEffective(ctx, orgID, projectID, resolvedGroupID, authz.MembershipTypeGroup)
437+
if err != nil {
434438
return nil, fmt.Errorf("failed to check existing group membership: %w", err)
435439
}
436-
if existingMembership != nil {
440+
441+
if membershipExists {
437442
return nil, NewErrAlreadyExistsStr("group is already a member of this project")
438443
}
439444

app/controlplane/pkg/data/project.go

Lines changed: 30 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -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.buildMembershipQuery(orgID, projectID, memberID, membershipType, true).Only(ctx)
233233

234234
if err != nil {
235235
if ent.IsNotFound(err) {
@@ -248,8 +248,8 @@ func (r *ProjectRepo) RemoveMemberFromProject(ctx context.Context, orgID uuid.UU
248248

249249
// FindProjectMembershipByProjectAndID finds a project membership by project ID and member ID (user or group)
250250
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)
251+
// Find the membership (direct only)
252+
m, err := r.buildMembershipQuery(orgID, projectID, memberID, membershipType, true).Only(ctx)
253253

254254
if err != nil {
255255
if ent.IsNotFound(err) {
@@ -258,38 +258,28 @@ 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-
269261
switch membershipType {
270262
case authz.MembershipTypeUser:
271-
// Fetch the user details for user memberships
272263
u, err := r.data.DB.User.Get(ctx, memberID)
273264
if err != nil {
274265
if ent.IsNotFound(err) {
275266
return nil, biz.NewErrNotFound("user")
276267
}
277268
return nil, fmt.Errorf("failed to find user: %w", err)
278269
}
279-
projectMembership.User = entUserToBizUser(u)
270+
return entProjectMembershipToBiz(m, u, nil), nil
280271
case authz.MembershipTypeGroup:
281-
// Fetch the group details for group memberships
282272
g, err := r.data.DB.Group.Query().Where(group.ID(memberID), group.DeletedAtIsNil()).Only(ctx)
283273
if err != nil {
284274
if ent.IsNotFound(err) {
285275
return nil, biz.NewErrNotFound("group")
286276
}
287277
return nil, fmt.Errorf("failed to find group: %w", err)
288278
}
289-
projectMembership.Group = entGroupToBiz(g)
279+
return entProjectMembershipToBiz(m, nil, g), nil
280+
default:
281+
return entProjectMembershipToBiz(m, nil, nil), nil
290282
}
291-
292-
return projectMembership, nil
293283
}
294284

295285
// UpdateMemberRoleInProject updates the role of a member in a project
@@ -308,7 +298,7 @@ func (r *ProjectRepo) UpdateMemberRoleInProject(ctx context.Context, orgID uuid.
308298
}
309299

310300
// Find the membership to update
311-
m, err := r.queryMembership(orgID, projectID, memberID, membershipType).Only(ctx)
301+
m, err := r.buildMembershipQuery(orgID, projectID, memberID, membershipType, true).Only(ctx)
312302

313303
if err != nil {
314304
if ent.IsNotFound(err) {
@@ -326,9 +316,10 @@ func (r *ProjectRepo) UpdateMemberRoleInProject(ctx context.Context, orgID uuid.
326316
return entProjectMembershipToBiz(m, nil, nil), nil
327317
}
328318

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 {
331-
return r.data.DB.Membership.Query().
319+
// buildMembershipQuery constructs a query that can find both direct and inherited memberships
320+
func (r *ProjectRepo) buildMembershipQuery(orgID uuid.UUID, projectID uuid.UUID, memberID uuid.UUID, membershipType authz.MembershipType, directMembershipOnly bool) *ent.MembershipQuery {
321+
// By default, all memberships are considered, both direct and inherited.
322+
baseQuery := r.data.DB.Membership.Query().
332323
Where(
333324
membership.HasOrganizationWith(
334325
organization.ID(orgID),
@@ -337,8 +328,25 @@ func (r *ProjectRepo) queryMembership(orgID uuid.UUID, projectID uuid.UUID, memb
337328
membership.MemberID(memberID),
338329
membership.ResourceTypeEQ(authz.ResourceTypeProject),
339330
membership.ResourceID(projectID),
340-
membership.ParentIDIsNil(), // Only top-level memberships
341331
).WithOrganization()
332+
333+
// Only consider direct memberships (parent is nil)
334+
if directMembershipOnly {
335+
baseQuery = baseQuery.Where(membership.ParentIDIsNil())
336+
}
337+
338+
return baseQuery
339+
}
340+
341+
// ExistsProjectMembershipEffective checks if a project membership exists considering both direct and inherited
342+
// memberships for the member on the project.
343+
func (r *ProjectRepo) ExistsProjectMembershipEffective(ctx context.Context, orgID uuid.UUID, projectID uuid.UUID, memberID uuid.UUID, membershipType authz.MembershipType) (bool, error) {
344+
exists, err := r.buildMembershipQuery(orgID, projectID, memberID, membershipType, false).Exist(ctx)
345+
if err != nil {
346+
return exists, fmt.Errorf("failed to find membership: %w", err)
347+
}
348+
349+
return exists, nil
342350
}
343351

344352
// entProjectToBiz converts an ent.Project to a biz.Project

0 commit comments

Comments
 (0)