diff --git a/app/controlplane/pkg/biz/membership.go b/app/controlplane/pkg/biz/membership.go index 9379767b9..6ef8e8d2e 100644 --- a/app/controlplane/pkg/biz/membership.go +++ b/app/controlplane/pkg/biz/membership.go @@ -154,6 +154,9 @@ func (uc *MembershipUseCase) DeleteOther(ctx context.Context, orgID, userID, mem } uc.logger.Infow("msg", "Deleting membership", "org_id", orgID, "membership_id", m.ID.String()) + + // Delete the main membership - this will also remove the user from all groups in the org + // and clean up associated resource memberships in the data layer if err := uc.repo.Delete(ctx, membershipUUID); err != nil { return fmt.Errorf("failed to delete membership: %w", err) } diff --git a/app/controlplane/pkg/biz/membership_integration_test.go b/app/controlplane/pkg/biz/membership_integration_test.go index b9b61a51b..3ec61a4d5 100644 --- a/app/controlplane/pkg/biz/membership_integration_test.go +++ b/app/controlplane/pkg/biz/membership_integration_test.go @@ -277,6 +277,215 @@ func (s *membershipIntegrationTestSuite) TestCreateMembership() { }) } +func (s *membershipIntegrationTestSuite) TestDeleteCleanup() { + ctx := context.Background() + + // Create users + user, err := s.User.UpsertByEmail(ctx, "cleanup-test@example.com", nil) + s.NoError(err) + adminUser, err := s.User.UpsertByEmail(ctx, "admin-user@example.com", nil) + s.NoError(err) + + // Create an organization + org, err := s.Organization.CreateWithRandomName(ctx) + s.NoError(err) + + // Add users to organization with different roles + membershipUser, err := s.Membership.Create(ctx, org.ID, user.ID, biz.WithCurrentMembership()) + s.NoError(err) + _, err = s.Membership.Create(ctx, org.ID, adminUser.ID, biz.WithMembershipRole(authz.RoleAdmin), biz.WithCurrentMembership()) + s.NoError(err) + + // Create a project in the organization + project, err := s.Project.Create(ctx, org.ID, "test-cleanup-project") + s.NoError(err) + + // Add user to the project + projectRef := &biz.IdentityReference{ID: &project.ID} + projectOpts := &biz.AddMemberToProjectOpts{ + ProjectReference: projectRef, + UserEmail: "cleanup-test@example.com", + RequesterID: uuid.MustParse(adminUser.ID), + Role: authz.RoleProjectViewer, + } + _, err = s.Project.AddMemberToProject(ctx, uuid.MustParse(org.ID), projectOpts) + s.NoError(err) + + // Create a group in the organization + userUUID := uuid.MustParse(user.ID) + group, err := s.Group.Create(ctx, uuid.MustParse(org.ID), "test-cleanup-group", "Group for cleanup testing", &userUUID) + s.NoError(err) + + // Create another user to add to the group + otherUser, err := s.User.UpsertByEmail(ctx, "other-member@example.com", nil) + s.NoError(err) + _, err = s.Membership.Create(ctx, org.ID, otherUser.ID) + s.NoError(err) + + // Add other user to the group + groupRef := &biz.IdentityReference{ID: &group.ID} + groupOpts := &biz.AddMemberToGroupOpts{ + IdentityReference: groupRef, + UserEmail: "other-member@example.com", + RequesterID: uuid.MustParse(user.ID), + Maintainer: false, + } + _, err = s.Group.AddMemberToGroup(ctx, uuid.MustParse(org.ID), groupOpts) + s.NoError(err) + + // Verify initial state + s.Run("verify initial state", func() { + // Verify user is in the project + members, count, err := s.Project.ListMembers(ctx, uuid.MustParse(org.ID), projectRef, nil) + s.NoError(err) + s.Equal(1, count) + s.Equal(1, len(members)) + s.Equal(user.ID, members[0].User.ID) + + // Verify user is in the group as maintainer + groupMembers, groupCount, err := s.Group.ListMembers(ctx, uuid.MustParse(org.ID), &biz.ListMembersOpts{ + IdentityReference: groupRef, + }, nil) + s.NoError(err) + s.Equal(2, groupCount) // User + otherUser + userFound := false + for _, member := range groupMembers { + if member.User.ID == user.ID { + s.True(member.Maintainer) + userFound = true + break + } + } + s.True(userFound, "User should be found in the group as a maintainer") + }) + + // Delete the user's membership + s.Run("delete user membership", func() { + err := s.Membership.LeaveAndDeleteOrg(ctx, user.ID, membershipUser.ID.String()) + s.NoError(err) + + // Check that the organization still exists (since there's still admin user) + _, err = s.Organization.FindByID(ctx, org.ID) + s.NoError(err) + + // Verify user is removed from project + projectMembers, projectCount, err := s.Project.ListMembers(ctx, uuid.MustParse(org.ID), projectRef, nil) + s.NoError(err) + s.Equal(0, projectCount) + s.Equal(0, len(projectMembers)) + + // Verify user is removed from group but other member remains + groupMembers, groupCount, err := s.Group.ListMembers(ctx, uuid.MustParse(org.ID), &biz.ListMembersOpts{ + IdentityReference: groupRef, + }, nil) + s.NoError(err) + s.Equal(1, groupCount) // Only otherUser should remain + s.Equal(1, len(groupMembers)) + s.Equal(otherUser.ID, groupMembers[0].User.ID) + s.False(groupMembers[0].Maintainer) + + // Verify group membership has been decremented + updatedGroup, err := s.Group.Get(ctx, uuid.MustParse(org.ID), &biz.IdentityReference{ID: &group.ID}) + s.NoError(err) + s.Equal(1, updatedGroup.MemberCount) + }) +} + +func (s *membershipIntegrationTestSuite) TestDeleteWithGroups() { + ctx := context.Background() + + // Create a user + user, err := s.User.UpsertByEmail(ctx, "groups-test@example.com", nil) + s.NoError(err) + userUUID := uuid.MustParse(user.ID) + + // Create an organization + org, err := s.Organization.CreateWithRandomName(ctx) + s.NoError(err) + orgUUID := uuid.MustParse(org.ID) + + // Add user to organization + membershipUser, err := s.Membership.Create(ctx, org.ID, user.ID, biz.WithMembershipRole(authz.RoleAdmin), biz.WithCurrentMembership()) + s.NoError(err) + + // Create multiple groups with the user as maintainer + group1, err := s.Group.Create(ctx, orgUUID, "group-1", "First group", &userUUID) + s.NoError(err) + group2, err := s.Group.Create(ctx, orgUUID, "group-2", "Second group", &userUUID) + s.NoError(err) + + // Add the groups to a project + pr, err := s.Project.Create(ctx, org.ID, "test-groups-project") + s.NoError(err) + projectRef := &biz.IdentityReference{ID: &pr.ID} + + // Add group1 to project + groupProjectOpts := &biz.AddMemberToProjectOpts{ + ProjectReference: projectRef, + GroupReference: &biz.IdentityReference{ID: &group1.ID}, + RequesterID: userUUID, + Role: authz.RoleProjectAdmin, + } + _, err = s.Project.AddMemberToProject(ctx, orgUUID, groupProjectOpts) + s.NoError(err) + + // Verify initial state + s.Run("verify initial state with groups", func() { + // Check user is a maintainer in both groups + group1Members, _, err := s.Group.ListMembers(ctx, orgUUID, &biz.ListMembersOpts{ + IdentityReference: &biz.IdentityReference{ID: &group1.ID}, + }, nil) + s.NoError(err) + s.Equal(1, len(group1Members)) + s.Equal(user.ID, group1Members[0].User.ID) + s.True(group1Members[0].Maintainer) + + group2Members, _, err := s.Group.ListMembers(ctx, orgUUID, &biz.ListMembersOpts{ + IdentityReference: &biz.IdentityReference{ID: &group2.ID}, + }, nil) + s.NoError(err) + s.Equal(1, len(group2Members)) + s.Equal(user.ID, group2Members[0].User.ID) + s.True(group2Members[0].Maintainer) + + // Check group1 is in the project + projectMembers, _, err := s.Project.ListMembers(ctx, orgUUID, projectRef, nil) + s.NoError(err) + s.Equal(1, len(projectMembers)) + s.Equal(group1.ID, projectMembers[0].Group.ID) + }) + + // Delete the user's membership + s.Run("delete user membership with groups", func() { + err := s.Membership.LeaveAndDeleteOrg(ctx, user.ID, membershipUser.ID.String()) + s.NoError(err) + + // The organization should be deleted since this was the only user + _, err = s.Organization.FindByID(ctx, org.ID) + s.True(biz.IsNotFound(err), "Organization should be deleted") + + // All groups should be soft-deleted + _, err = s.Group.Get(ctx, orgUUID, &biz.IdentityReference{ID: &group1.ID}) + s.True(biz.IsNotFound(err), "Group 1 should be deleted") + + _, err = s.Group.Get(ctx, orgUUID, &biz.IdentityReference{ID: &group2.ID}) + s.True(biz.IsNotFound(err), "Group 2 should be deleted") + + // The project should be deleted + _, err = s.Project.FindProjectByReference(ctx, org.ID, &biz.IdentityReference{ID: projectRef.ID}) + s.True(biz.IsNotFound(err), "Project should be deleted") + + // Verify group memberships are marked as deleted + group1Mem, group1Err := s.Repos.GroupRepo.FindGroupMembershipByGroupAndID(ctx, group1.ID, userUUID) + s.True(biz.IsNotFound(group1Err)) + s.Nil(group1Mem) + + group2Mem, group2Err := s.Repos.GroupRepo.FindGroupMembershipByGroupAndID(ctx, group2.ID, userUUID) + s.True(biz.IsNotFound(group2Err)) + s.Nil(group2Mem) + }) +} + // Run the tests func TestMembershipUseCase(t *testing.T) { suite.Run(t, new(membershipIntegrationTestSuite)) diff --git a/app/controlplane/pkg/data/membership.go b/app/controlplane/pkg/data/membership.go index ef78cec06..c61b32ece 100644 --- a/app/controlplane/pkg/data/membership.go +++ b/app/controlplane/pkg/data/membership.go @@ -18,10 +18,12 @@ package data import ( "context" "fmt" + "time" "github.com/chainloop-dev/chainloop/app/controlplane/pkg/authz" "github.com/chainloop-dev/chainloop/app/controlplane/pkg/biz" "github.com/chainloop-dev/chainloop/app/controlplane/pkg/data/ent" + "github.com/chainloop-dev/chainloop/app/controlplane/pkg/data/ent/group" "github.com/chainloop-dev/chainloop/app/controlplane/pkg/data/ent/groupmembership" "github.com/chainloop-dev/chainloop/app/controlplane/pkg/data/ent/membership" "github.com/chainloop-dev/chainloop/app/controlplane/pkg/data/ent/organization" @@ -240,8 +242,67 @@ func (r *MembershipRepo) SetRole(ctx context.Context, membershipID uuid.UUID, ro } // Delete deletes a membership by ID. +// When deleting a membership, it's important to ensure we're not leaving any dangling references. func (r *MembershipRepo) Delete(ctx context.Context, id uuid.UUID) error { - return r.data.DB.Membership.DeleteOneID(id).Exec(ctx) + // First, fetch the membership to understand what we're deleting + membershipToDelete, err := r.data.DB.Membership.Query().Where(membership.ID(id)).WithOrganization().Only(ctx) + if err != nil { + if ent.IsNotFound(err) { + return nil // Already deleted, nothing to do + } + return fmt.Errorf("failed to get membership: %w", err) + } + + return WithTx(ctx, r.data.DB, func(tx *ent.Tx) error { + // Delete the specific membership + if err := tx.Membership.DeleteOneID(id).Exec(ctx); err != nil { + return fmt.Errorf("failed to delete membership: %w", err) + } + + // If this is an organization membership and the member type is a user, + // we also need to clean up any resource memberships for this user in the organization + if membershipToDelete.ResourceType == authz.ResourceTypeOrganization && membershipToDelete.MembershipType == authz.MembershipTypeUser { + // Extract the organization id and user ID from the membership + orgID := membershipToDelete.Edges.Organization.ID + userID := membershipToDelete.MemberID + + // Delete all other resource memberships for this user in the organization + // This will cover all membership types including group-related ones + if _, err := tx.Membership.Delete().Where( + membership.IDNEQ(id), // Don't try to delete the one we already deleted + membership.MemberID(userID), + membership.HasOrganizationWith(organization.ID(orgID)), + ).Exec(ctx); err != nil { + return fmt.Errorf("failed to delete related memberships: %w", err) + } + + // Remove the user from all groups in the organization by soft-deleting group memberships + now := time.Now() + + // Soft delete all group memberships for this user in this organization + if _, err := tx.GroupMembership.Update().Where( + groupmembership.UserID(userID), + groupmembership.DeletedAtIsNil(), + groupmembership.HasGroupWith(group.OrganizationID(orgID)), + ).SetDeletedAt(now).SetUpdatedAt(now).Save(ctx); err != nil { + return fmt.Errorf("failed to delete group memberships for user %s in organization %s: %w", userID, orgID, err) + } + + // Decrement the member count of each group this user was a member of + // We don't need a separate check for groups the user was a maintainer of + // because all memberships were already deleted above + if _, err := tx.Group.Update(). + Where( + group.HasMembersWith(user.ID(userID)), + group.HasOrganizationWith(organization.ID(orgID)), + ). + AddMemberCount(-1).Save(ctx); err != nil { + return fmt.Errorf("failed to decrement group member count: %w", err) + } + } + + return nil + }) } // RBAC methods