From 4be0473fd20d3661da76a081a400af2c1fbcfaf3 Mon Sep 17 00:00:00 2001 From: Javier Rodriguez Date: Wed, 20 Aug 2025 12:38:22 +0200 Subject: [PATCH] feat(projects): When adding members check all memberships Signed-off-by: Javier Rodriguez --- app/controlplane/pkg/biz/orginvitation.go | 8 ++-- app/controlplane/pkg/biz/project.go | 21 +++++---- app/controlplane/pkg/data/project.go | 52 +++++++++++++---------- 3 files changed, 47 insertions(+), 34 deletions(-) diff --git a/app/controlplane/pkg/biz/orginvitation.go b/app/controlplane/pkg/biz/orginvitation.go index ddab056fa..be77b9518 100644 --- a/app/controlplane/pkg/biz/orginvitation.go +++ b/app/controlplane/pkg/biz/orginvitation.go @@ -411,13 +411,13 @@ func (uc *OrgInvitationUseCase) processProjectMembership(ctx context.Context, in role = authz.RoleProjectViewer } - // Check if the user is already a member of the project - existingMembership, err := uc.projectRepo.FindProjectMembershipByProjectAndID(ctx, orgUUID, *projectID, userUUID, authz.MembershipTypeUser) - if err != nil && !IsNotFound(err) { + // Check if the user already has membership in the project (consider inherited memberships) + membershipExists, err := uc.projectRepo.ExistsProjectMembershipEffective(ctx, orgUUID, *projectID, userUUID, authz.MembershipTypeUser) + if err != nil { return fmt.Errorf("error checking project membership for user %s: %w", userUUID, err) } - if existingMembership != nil { + if membershipExists { // User is already a member of the project, nothing to do 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()) return nil diff --git a/app/controlplane/pkg/biz/project.go b/app/controlplane/pkg/biz/project.go index a07abfc27..077e621a7 100644 --- a/app/controlplane/pkg/biz/project.go +++ b/app/controlplane/pkg/biz/project.go @@ -45,6 +45,9 @@ type ProjectsRepo interface { UpdateMemberRoleInProject(ctx context.Context, orgID uuid.UUID, projectID uuid.UUID, memberID uuid.UUID, membershipType authz.MembershipType, newRole authz.Role) (*ProjectMembership, error) // FindProjectMembershipByProjectAndID finds a project membership by project ID and member ID (user or group). FindProjectMembershipByProjectAndID(ctx context.Context, orgID uuid.UUID, projectID uuid.UUID, memberID uuid.UUID, membershipType authz.MembershipType) (*ProjectMembership, error) + // ExistsProjectMembershipEffective checks if a project membership by project ID and member ID (user or group). + // considering inherited (effective) memberships as well. + ExistsProjectMembershipEffective(ctx context.Context, orgID uuid.UUID, projectID uuid.UUID, memberID uuid.UUID, membershipType authz.MembershipType) (bool, error) // ListPendingInvitationsByProject retrieves a list of pending invitations for a project. ListPendingInvitationsByProject(ctx context.Context, orgID uuid.UUID, projectID uuid.UUID, paginationOpts *pagination.OffsetPaginationOpts) ([]*OrgInvitation, int, error) } @@ -342,12 +345,13 @@ func (uc *ProjectUseCase) addUserToProject(ctx context.Context, orgID uuid.UUID, userUUID := uuid.MustParse(userMembership.User.ID) - // Check if the user is already a member of the project - existingMembership, err := uc.projectsRepository.FindProjectMembershipByProjectAndID(ctx, orgID, projectID, userUUID, authz.MembershipTypeUser) - if err != nil && !IsNotFound(err) { + // Check if the user is already a member of the project (including inherited memberships) + membershipExists, err := uc.projectsRepository.ExistsProjectMembershipEffective(ctx, orgID, projectID, userUUID, authz.MembershipTypeUser) + if err != nil { return nil, fmt.Errorf("failed to check existing membership: %w", err) } - if existingMembership != nil { + + if membershipExists { return nil, NewErrAlreadyExistsStr("user is already a member of this project") } @@ -428,12 +432,13 @@ func (uc *ProjectUseCase) addGroupToProject(ctx context.Context, orgID uuid.UUID return nil, fmt.Errorf("failed to validate group reference: %w", err) } - // Check if the group already has membership in the project - existingMembership, err := uc.projectsRepository.FindProjectMembershipByProjectAndID(ctx, orgID, projectID, resolvedGroupID, authz.MembershipTypeGroup) - if err != nil && !IsNotFound(err) { + // Check if the group is already a member of the project (including inherited memberships) + membershipExists, err := uc.projectsRepository.ExistsProjectMembershipEffective(ctx, orgID, projectID, resolvedGroupID, authz.MembershipTypeGroup) + if err != nil { return nil, fmt.Errorf("failed to check existing group membership: %w", err) } - if existingMembership != nil { + + if membershipExists { return nil, NewErrAlreadyExistsStr("group is already a member of this project") } diff --git a/app/controlplane/pkg/data/project.go b/app/controlplane/pkg/data/project.go index 2156df3e8..c4c2e2962 100644 --- a/app/controlplane/pkg/data/project.go +++ b/app/controlplane/pkg/data/project.go @@ -229,7 +229,7 @@ func (r *ProjectRepo) RemoveMemberFromProject(ctx context.Context, orgID uuid.UU } // Find the membership to delete - m, err := r.queryMembership(orgID, projectID, memberID, membershipType).Only(ctx) + m, err := r.buildMembershipQuery(orgID, projectID, memberID, membershipType, true).Only(ctx) if err != nil { if ent.IsNotFound(err) { @@ -248,8 +248,8 @@ func (r *ProjectRepo) RemoveMemberFromProject(ctx context.Context, orgID uuid.UU // FindProjectMembershipByProjectAndID finds a project membership by project ID and member ID (user or group) func (r *ProjectRepo) FindProjectMembershipByProjectAndID(ctx context.Context, orgID uuid.UUID, projectID uuid.UUID, memberID uuid.UUID, membershipType authz.MembershipType) (*biz.ProjectMembership, error) { - // Find the membership - m, err := r.queryMembership(orgID, projectID, memberID, membershipType).Only(ctx) + // Find the membership (direct only) + m, err := r.buildMembershipQuery(orgID, projectID, memberID, membershipType, true).Only(ctx) if err != nil { if ent.IsNotFound(err) { @@ -258,17 +258,8 @@ func (r *ProjectRepo) FindProjectMembershipByProjectAndID(ctx context.Context, o return nil, fmt.Errorf("failed to find membership: %w", err) } - // Build the membership response based on the membership type - projectMembership := &biz.ProjectMembership{ - MembershipType: m.MembershipType, - Role: m.Role, - CreatedAt: &m.CreatedAt, - UpdatedAt: &m.UpdatedAt, - } - switch membershipType { case authz.MembershipTypeUser: - // Fetch the user details for user memberships u, err := r.data.DB.User.Get(ctx, memberID) if err != nil { if ent.IsNotFound(err) { @@ -276,9 +267,8 @@ func (r *ProjectRepo) FindProjectMembershipByProjectAndID(ctx context.Context, o } return nil, fmt.Errorf("failed to find user: %w", err) } - projectMembership.User = entUserToBizUser(u) + return entProjectMembershipToBiz(m, u, nil), nil case authz.MembershipTypeGroup: - // Fetch the group details for group memberships g, err := r.data.DB.Group.Query().Where(group.ID(memberID), group.DeletedAtIsNil()).Only(ctx) if err != nil { if ent.IsNotFound(err) { @@ -286,10 +276,10 @@ func (r *ProjectRepo) FindProjectMembershipByProjectAndID(ctx context.Context, o } return nil, fmt.Errorf("failed to find group: %w", err) } - projectMembership.Group = entGroupToBiz(g) + return entProjectMembershipToBiz(m, nil, g), nil + default: + return entProjectMembershipToBiz(m, nil, nil), nil } - - return projectMembership, nil } // UpdateMemberRoleInProject updates the role of a member in a project @@ -308,7 +298,7 @@ func (r *ProjectRepo) UpdateMemberRoleInProject(ctx context.Context, orgID uuid. } // Find the membership to update - m, err := r.queryMembership(orgID, projectID, memberID, membershipType).Only(ctx) + m, err := r.buildMembershipQuery(orgID, projectID, memberID, membershipType, true).Only(ctx) if err != nil { if ent.IsNotFound(err) { @@ -326,9 +316,10 @@ func (r *ProjectRepo) UpdateMemberRoleInProject(ctx context.Context, orgID uuid. return entProjectMembershipToBiz(m, nil, nil), nil } -// queryMembership is a helper function to build a common membership query -func (r *ProjectRepo) queryMembership(orgID uuid.UUID, projectID uuid.UUID, memberID uuid.UUID, membershipType authz.MembershipType) *ent.MembershipQuery { - return r.data.DB.Membership.Query(). +// buildMembershipQuery constructs a query that can find both direct and inherited memberships +func (r *ProjectRepo) buildMembershipQuery(orgID uuid.UUID, projectID uuid.UUID, memberID uuid.UUID, membershipType authz.MembershipType, directMembershipOnly bool) *ent.MembershipQuery { + // By default, all memberships are considered, both direct and inherited. + baseQuery := r.data.DB.Membership.Query(). Where( membership.HasOrganizationWith( organization.ID(orgID), @@ -337,8 +328,25 @@ func (r *ProjectRepo) queryMembership(orgID uuid.UUID, projectID uuid.UUID, memb membership.MemberID(memberID), membership.ResourceTypeEQ(authz.ResourceTypeProject), membership.ResourceID(projectID), - membership.ParentIDIsNil(), // Only top-level memberships ).WithOrganization() + + // Only consider direct memberships (parent is nil) + if directMembershipOnly { + baseQuery = baseQuery.Where(membership.ParentIDIsNil()) + } + + return baseQuery +} + +// ExistsProjectMembershipEffective checks if a project membership exists considering both direct and inherited +// memberships for the member on the project. +func (r *ProjectRepo) ExistsProjectMembershipEffective(ctx context.Context, orgID uuid.UUID, projectID uuid.UUID, memberID uuid.UUID, membershipType authz.MembershipType) (bool, error) { + exists, err := r.buildMembershipQuery(orgID, projectID, memberID, membershipType, false).Exist(ctx) + if err != nil { + return exists, fmt.Errorf("failed to find membership: %w", err) + } + + return exists, nil } // entProjectToBiz converts an ent.Project to a biz.Project