[PM-22104] Migrate default collection when org user is removed (#6135)

* migrate default collection to a shared collection when users are removed * remove redundant logic * fix test * fix tests * fix test * clean up * add migrations * run dotnet format * clean up, refactor duplicate logic to sproc, wip integration test * fix sql * add migration for new sproc * integration test wip * integration test wip * integration test wip * integration test wip * fix integration test LINQ expression * fix using wrong Id * wip integration test for DeleteManyAsync * fix LINQ * only set DefaultUserEmail when it is null in sproc * check for null * spelling, separate create and update request models * fix test * fix child class * refactor sproc * clean up * more cleanup * fix tests * fix user email * remove unneccesary test * add DefaultUserCollectionEmail to EF query * fix test * fix EF logic to match sprocs * clean up logic * cleanup
2025-12-22 19:23:45 +00:00 · 2025-08-19 14:12:34 -04:00
parent 29d6288b27
commit c189e4aaf5
16 changed files with 1001 additions and 195 deletions
--- a/src/Api/Controllers/CollectionsController.cs
+++ b/src/Api/Controllers/CollectionsController.cs
@@ -146,7 +146,7 @@ public class CollectionsController : Controller
    }

    [HttpPost("")]
-    public async Task<CollectionResponseModel> Post(Guid orgId, [FromBody] CollectionRequestModel model)
+    public async Task<CollectionResponseModel> Post(Guid orgId, [FromBody] CreateCollectionRequestModel model)
    {
        var collection = model.ToCollection(orgId);

@@ -174,7 +174,7 @@ public class CollectionsController : Controller

    [HttpPut("{id}")]
    [HttpPost("{id}")]
-    public async Task<CollectionResponseModel> Put(Guid orgId, Guid id, [FromBody] CollectionRequestModel model)
+    public async Task<CollectionResponseModel> Put(Guid orgId, Guid id, [FromBody] UpdateCollectionRequestModel model)
    {
        var collection = await _collectionRepository.GetByIdAsync(id);
        var authorized = (await _authorizationService.AuthorizeAsync(User, collection, BulkCollectionOperations.Update)).Succeeded;
--- a/src/Api/Models/Request/CollectionRequestModel.cs
+++ b/src/Api/Models/Request/CollectionRequestModel.cs
@@ -7,7 +7,7 @@ using Bit.Core.Utilities;

 namespace Bit.Api.Models.Request;

-public class CollectionRequestModel
+public class CreateCollectionRequestModel
 {
    [Required]
    [EncryptedString]
@@ -40,7 +40,7 @@ public class CollectionBulkDeleteRequestModel
    public IEnumerable<Guid> Ids { get; set; }
 }

-public class CollectionWithIdRequestModel : CollectionRequestModel
+public class CollectionWithIdRequestModel : CreateCollectionRequestModel
 {
    public Guid? Id { get; set; }

@@ -50,3 +50,21 @@ public class CollectionWithIdRequestModel : CollectionRequestModel
        return base.ToCollection(existingCollection);
    }
 }
+
+public class UpdateCollectionRequestModel : CreateCollectionRequestModel
+{
+    [EncryptedString]
+    [EncryptedStringLength(1000)]
+    public new string Name { get; set; }
+
+    public override Collection ToCollection(Collection existingCollection)
+    {
+        if (string.IsNullOrEmpty(existingCollection.DefaultUserCollectionEmail) && !string.IsNullOrWhiteSpace(Name))
+        {
+            existingCollection.Name = Name;
+        }
+        existingCollection.ExternalId = ExternalId;
+        return existingCollection;
+    }
+
+}
--- a/src/Api/Models/Response/CollectionResponseModel.cs
+++ b/src/Api/Models/Response/CollectionResponseModel.cs
@@ -49,6 +49,7 @@ public class CollectionDetailsResponseModel : CollectionResponseModel
        ReadOnly = collectionDetails.ReadOnly;
        HidePasswords = collectionDetails.HidePasswords;
        Manage = collectionDetails.Manage;
+        DefaultUserCollectionEmail = collectionDetails.DefaultUserCollectionEmail;
    }

    public bool ReadOnly { get; set; }
--- a/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationUserRepository.cs
+++ b/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationUserRepository.cs
@@ -5,6 +5,7 @@ using AutoMapper;
 using Bit.Core.AdminConsole.Enums;
 using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.InviteUsers.Models;
 using Bit.Core.Enums;
+using Bit.Core.Exceptions;
 using Bit.Core.KeyManagement.UserKey;
 using Bit.Core.Models.Data;
 using Bit.Core.Models.Data.Organizations.OrganizationUsers;
@@ -73,53 +74,91 @@ public class OrganizationUserRepository : Repository<Core.Entities.OrganizationU
        return organizationUsers.Select(u => u.Id).ToList();
    }

-    public override async Task DeleteAsync(Core.Entities.OrganizationUser organizationUser) => await DeleteAsync(organizationUser.Id);
-    public async Task DeleteAsync(Guid organizationUserId)
+    public override async Task DeleteAsync(Core.Entities.OrganizationUser organizationUser)
    {
        using (var scope = ServiceScopeFactory.CreateScope())
        {
            var dbContext = GetDatabaseContext(scope);
-            await dbContext.UserBumpAccountRevisionDateByOrganizationUserIdAsync(organizationUserId);
            var orgUser = await dbContext.OrganizationUsers
-                .Where(ou => ou.Id == organizationUserId)
-                .FirstAsync();
+                .Where(ou => ou.Id == organizationUser.Id)
+                .Select(ou => new
+                {
+                    ou.Id,
+                    ou.UserId,
+                    OrgEmail = ou.Email,
+                    UserEmail = ou.User.Email
+                })
+                .FirstOrDefaultAsync();

-            var organizationId = orgUser?.OrganizationId;
+            if (orgUser == null)
+            {
+                throw new NotFoundException("User not found.");
+            }
+
+            var email = !string.IsNullOrEmpty(orgUser.OrgEmail)
+                ? orgUser.OrgEmail
+                : orgUser.UserEmail;
+            var organizationId = organizationUser?.OrganizationId;
            var userId = orgUser?.UserId;
+            var utcNow = DateTime.UtcNow;

-            if (orgUser?.OrganizationId != null && orgUser?.UserId != null)
+            using var transaction = await dbContext.Database.BeginTransactionAsync();
+
+            try
            {
-                var ssoUsers = dbContext.SsoUsers
-                    .Where(su => su.UserId == userId && su.OrganizationId == organizationId);
-                dbContext.SsoUsers.RemoveRange(ssoUsers);
+                await dbContext.Collections
+                    .Where(c => c.Type == CollectionType.DefaultUserCollection
+                             && c.CollectionUsers.Any(cu => cu.OrganizationUserId == organizationUser.Id))
+                    .ExecuteUpdateAsync(setters => setters
+                        .SetProperty(c => c.Type, CollectionType.SharedCollection)
+                        .SetProperty(c => c.RevisionDate, utcNow)
+                        .SetProperty(c => c.DefaultUserCollectionEmail,
+                            c => c.DefaultUserCollectionEmail == null ? email : c.DefaultUserCollectionEmail));
+
+                await dbContext.CollectionUsers
+                    .Where(cu => cu.OrganizationUserId == organizationUser.Id)
+                    .ExecuteDeleteAsync();
+
+                await dbContext.GroupUsers
+                    .Where(gu => gu.OrganizationUserId == organizationUser.Id)
+                    .ExecuteDeleteAsync();
+
+                await dbContext.SsoUsers
+                    .Where(su => su.UserId == userId && su.OrganizationId == organizationId)
+                    .ExecuteDeleteAsync();
+
+                await dbContext.UserProjectAccessPolicy
+                    .Where(ap => ap.OrganizationUserId == organizationUser.Id)
+                    .ExecuteDeleteAsync();
+
+                await dbContext.UserServiceAccountAccessPolicy
+                    .Where(ap => ap.OrganizationUserId == organizationUser.Id)
+                    .ExecuteDeleteAsync();
+
+                await dbContext.UserSecretAccessPolicy
+                    .Where(ap => ap.OrganizationUserId == organizationUser.Id)
+                    .ExecuteDeleteAsync();
+
+                await dbContext.OrganizationSponsorships
+                    .Where(os => os.SponsoringOrganizationUserId == organizationUser.Id)
+                    .ExecuteDeleteAsync();
+
+                await dbContext.Users
+                    .Where(u => u.Id == orgUser.UserId)
+                    .ExecuteUpdateAsync(setters => setters
+                        .SetProperty(u => u.AccountRevisionDate, utcNow));
+
+                await dbContext.OrganizationUsers
+                    .Where(ou => ou.Id == organizationUser.Id)
+                    .ExecuteDeleteAsync();
+
+                await transaction.CommitAsync();
            }
-
-            var collectionUsers = dbContext.CollectionUsers
-                .Where(cu => cu.OrganizationUserId == organizationUserId);
-            dbContext.CollectionUsers.RemoveRange(collectionUsers);
-
-            var groupUsers = dbContext.GroupUsers
-                .Where(gu => gu.OrganizationUserId == organizationUserId);
-            dbContext.GroupUsers.RemoveRange(groupUsers);
-
-            dbContext.UserProjectAccessPolicy.RemoveRange(
-                dbContext.UserProjectAccessPolicy.Where(ap => ap.OrganizationUserId == organizationUserId));
-            dbContext.UserServiceAccountAccessPolicy.RemoveRange(
-                dbContext.UserServiceAccountAccessPolicy.Where(ap => ap.OrganizationUserId == organizationUserId));
-            dbContext.UserSecretAccessPolicy.RemoveRange(
-                dbContext.UserSecretAccessPolicy.Where(ap => ap.OrganizationUserId == organizationUserId));
-
-            var orgSponsorships = await dbContext.OrganizationSponsorships
-                .Where(os => os.SponsoringOrganizationUserId == organizationUserId)
-                .ToListAsync();
-
-            foreach (var orgSponsorship in orgSponsorships)
+            catch
            {
-                orgSponsorship.ToDelete = true;
+                await transaction.RollbackAsync();
+                throw;
            }
-
-            dbContext.OrganizationUsers.Remove(orgUser);
-            await dbContext.SaveChangesAsync();
        }
    }

@@ -130,31 +169,92 @@ public class OrganizationUserRepository : Repository<Core.Entities.OrganizationU
        var dbContext = GetDatabaseContext(scope);

        var transaction = await dbContext.Database.BeginTransactionAsync();
-        await dbContext.UserBumpAccountRevisionDateByOrganizationUserIdsAsync(targetOrganizationUserIds);

-        await dbContext.CollectionUsers
-            .Where(cu => targetOrganizationUserIds.Contains(cu.OrganizationUserId))
-            .ExecuteDeleteAsync();
+        try
+        {
+            await dbContext.UserBumpAccountRevisionDateByOrganizationUserIdsAsync(targetOrganizationUserIds);

-        await dbContext.GroupUsers
-            .Where(gu => targetOrganizationUserIds.Contains(gu.OrganizationUserId))
-            .ExecuteDeleteAsync();
+            var organizationUsersToDelete = await dbContext.OrganizationUsers
+                .Where(ou => targetOrganizationUserIds.Contains(ou.Id))
+                .Include(ou => ou.User)
+                .ToListAsync();

-        await dbContext.UserProjectAccessPolicy
-            .Where(ap => targetOrganizationUserIds.Contains(ap.OrganizationUserId!.Value))
-            .ExecuteDeleteAsync();
-        await dbContext.UserServiceAccountAccessPolicy
-            .Where(ap => targetOrganizationUserIds.Contains(ap.OrganizationUserId!.Value))
-            .ExecuteDeleteAsync();
-        await dbContext.UserSecretAccessPolicy
-            .Where(ap => targetOrganizationUserIds.Contains(ap.OrganizationUserId!.Value))
-            .ExecuteDeleteAsync();
+            var collectionUsers = await dbContext.CollectionUsers
+                .Where(cu => targetOrganizationUserIds.Contains(cu.OrganizationUserId))
+                .ToListAsync();

-        await dbContext.OrganizationUsers
-            .Where(ou => targetOrganizationUserIds.Contains(ou.Id)).ExecuteDeleteAsync();
+            var collectionIds = collectionUsers.Select(cu => cu.CollectionId).Distinct().ToList();

-        await dbContext.SaveChangesAsync();
-        await transaction.CommitAsync();
+            var collections = await dbContext.Collections
+                .Where(c => collectionIds.Contains(c.Id))
+                .ToListAsync();
+
+            var collectionsToUpdate = collections
+                .Where(c => c.Type == CollectionType.DefaultUserCollection)
+                .ToList();
+
+            var collectionUserLookup = collectionUsers.ToLookup(cu => cu.CollectionId);
+
+            foreach (var collection in collectionsToUpdate)
+            {
+                var collectionUser = collectionUserLookup[collection.Id].FirstOrDefault();
+                if (collectionUser != null)
+                {
+                    var orgUser = organizationUsersToDelete.FirstOrDefault(ou => ou.Id == collectionUser.OrganizationUserId);
+
+                    if (orgUser?.User != null)
+                    {
+                        if (string.IsNullOrEmpty(collection.DefaultUserCollectionEmail))
+                        {
+                            var emailToUse = !string.IsNullOrEmpty(orgUser.Email)
+                                ? orgUser.Email
+                                : orgUser.User.Email;
+
+                            if (!string.IsNullOrEmpty(emailToUse))
+                            {
+                                collection.DefaultUserCollectionEmail = emailToUse;
+                            }
+                        }
+                        collection.Type = CollectionType.SharedCollection;
+                    }
+                }
+            }
+
+            await dbContext.CollectionUsers
+                .Where(cu => targetOrganizationUserIds.Contains(cu.OrganizationUserId))
+                .ExecuteDeleteAsync();
+
+            await dbContext.GroupUsers
+                .Where(gu => targetOrganizationUserIds.Contains(gu.OrganizationUserId))
+                .ExecuteDeleteAsync();
+
+            await dbContext.UserProjectAccessPolicy
+                .Where(ap => targetOrganizationUserIds.Contains(ap.OrganizationUserId!.Value))
+                .ExecuteDeleteAsync();
+
+            await dbContext.UserServiceAccountAccessPolicy
+                .Where(ap => targetOrganizationUserIds.Contains(ap.OrganizationUserId!.Value))
+                .ExecuteDeleteAsync();
+
+            await dbContext.UserSecretAccessPolicy
+                .Where(ap => targetOrganizationUserIds.Contains(ap.OrganizationUserId!.Value))
+                .ExecuteDeleteAsync();
+
+            await dbContext.OrganizationSponsorships
+                .Where(os => targetOrganizationUserIds.Contains(os.SponsoringOrganizationUserId))
+                .ExecuteDeleteAsync();
+
+            await dbContext.OrganizationUsers
+                .Where(ou => targetOrganizationUserIds.Contains(ou.Id)).ExecuteDeleteAsync();
+
+            await dbContext.SaveChangesAsync();
+            await transaction.CommitAsync();
+        }
+        catch
+        {
+            await transaction.RollbackAsync();
+            throw;
+        }
    }

    public async Task<Tuple<Core.Entities.OrganizationUser, ICollection<CollectionAccessSelection>>> GetByIdWithCollectionsAsync(Guid id)
--- a/src/Infrastructure.EntityFramework/Repositories/CollectionRepository.cs
+++ b/src/Infrastructure.EntityFramework/Repositories/CollectionRepository.cs
@@ -325,7 +325,8 @@ public class CollectionRepository : Repository<Core.Entities.Collection, Collect
                        c.CreationDate,
                        c.RevisionDate,
                        c.ExternalId,
-                        c.Unmanaged
+                        c.Unmanaged,
+                        c.DefaultUserCollectionEmail
                    }).Select(collectionGroup => new CollectionAdminDetails
                    {
                        Id = collectionGroup.Key.Id,
@@ -339,7 +340,8 @@ public class CollectionRepository : Repository<Core.Entities.Collection, Collect
                            Convert.ToBoolean(collectionGroup.Min(c => Convert.ToInt32(c.HidePasswords))),
                        Manage = Convert.ToBoolean(collectionGroup.Max(c => Convert.ToInt32(c.Manage))),
                        Assigned = Convert.ToBoolean(collectionGroup.Max(c => Convert.ToInt32(c.Assigned))),
-                        Unmanaged = collectionGroup.Key.Unmanaged
+                        Unmanaged = collectionGroup.Key.Unmanaged,
+                        DefaultUserCollectionEmail = collectionGroup.Key.DefaultUserCollectionEmail
                    }).ToList();
            }
            else
@@ -353,7 +355,8 @@ public class CollectionRepository : Repository<Core.Entities.Collection, Collect
                                         c.CreationDate,
                                         c.RevisionDate,
                                         c.ExternalId,
-                                         c.Unmanaged
+                                         c.Unmanaged,
+                                         c.DefaultUserCollectionEmail
                                     }
                    into collectionGroup
                                     select new CollectionAdminDetails
@@ -369,7 +372,8 @@ public class CollectionRepository : Repository<Core.Entities.Collection, Collect
                                             Convert.ToBoolean(collectionGroup.Min(c => Convert.ToInt32(c.HidePasswords))),
                                         Manage = Convert.ToBoolean(collectionGroup.Max(c => Convert.ToInt32(c.Manage))),
                                         Assigned = Convert.ToBoolean(collectionGroup.Max(c => Convert.ToInt32(c.Assigned))),
-                                         Unmanaged = collectionGroup.Key.Unmanaged
+                                         Unmanaged = collectionGroup.Key.Unmanaged,
+                                         DefaultUserCollectionEmail = collectionGroup.Key.DefaultUserCollectionEmail
                                     }).ToListAsync();
            }

--- a/src/Infrastructure.EntityFramework/Repositories/Queries/CollectionAdminDetailsQuery.cs
+++ b/src/Infrastructure.EntityFramework/Repositories/Queries/CollectionAdminDetailsQuery.cs
@@ -81,6 +81,7 @@ public class CollectionAdminDetailsQuery : IQuery<CollectionAdminDetails>
            ExternalId = x.c.ExternalId,
            CreationDate = x.c.CreationDate,
            RevisionDate = x.c.RevisionDate,
+            DefaultUserCollectionEmail = x.c.DefaultUserCollectionEmail,
            ReadOnly = (bool?)x.cu.ReadOnly ?? (bool?)x.cg.ReadOnly ?? false,
            HidePasswords = (bool?)x.cu.HidePasswords ?? (bool?)x.cg.HidePasswords ?? false,
            Manage = (bool?)x.cu.Manage ?? (bool?)x.cg.Manage ?? false,
--- a/Procedures/OrganizationUser_DeleteById.sql
+++ b/Procedures/OrganizationUser_DeleteById.sql
@@ -1,4 +1,4 @@
-CREATE PROCEDURE [dbo].[OrganizationUser_DeleteById]
+CREATE PROCEDURE [dbo].[OrganizationUser_DeleteById]
    @Id UNIQUEIDENTIFIER
 AS
 BEGIN
@@ -17,6 +17,11 @@ BEGIN
    WHERE
        [Id] = @Id

+    -- Migrate DefaultUserCollection to SharedCollection
+    DECLARE @Ids [dbo].[GuidIdArray]
+    INSERT INTO @Ids (Id) VALUES (@Id)
+    EXEC [dbo].[OrganizationUser_MigrateDefaultCollection] @Ids
+
    IF @OrganizationId IS NOT NULL AND @UserId IS NOT NULL
    BEGIN
        EXEC [dbo].[SsoUser_Delete] @UserId, @OrganizationId
--- a/Procedures/OrganizationUser_DeleteByIds.sql
+++ b/Procedures/OrganizationUser_DeleteByIds.sql
@@ -6,6 +6,9 @@ BEGIN

    EXEC [dbo].[User_BumpAccountRevisionDateByOrganizationUserIds] @Ids

+    -- Migrate DefaultCollection to SharedCollection
+    EXEC [dbo].[OrganizationUser_MigrateDefaultCollection] @Ids
+
    DECLARE @UserAndOrganizationIds [dbo].[TwoGuidIdArray]

    INSERT INTO @UserAndOrganizationIds
--- a/Procedures/OrganizationUser_MigrateDefaultCollection.sql
+++ b/Procedures/OrganizationUser_MigrateDefaultCollection.sql
@@ -0,0 +1,22 @@
+CREATE PROCEDURE [dbo].[OrganizationUser_MigrateDefaultCollection]
+    @Ids [dbo].[GuidIdArray] READONLY
+AS
+BEGIN
+    SET NOCOUNT ON
+    DECLARE @UtcNow DATETIME2(7) = GETUTCDATE();
+
+    UPDATE c
+    SET
+        [DefaultUserCollectionEmail] = CASE WHEN c.[DefaultUserCollectionEmail] IS NULL THEN u.[Email] ELSE c.[DefaultUserCollectionEmail] END,
+        [RevisionDate] = @UtcNow,
+        [Type] = 0
+    FROM
+        [dbo].[Collection] c
+        INNER JOIN [dbo].[CollectionUser] cu ON c.[Id] = cu.[CollectionId]
+        INNER JOIN [dbo].[OrganizationUser] ou ON cu.[OrganizationUserId] = ou.[Id]
+        INNER JOIN [dbo].[User] u ON ou.[UserId] = u.[Id]
+        INNER JOIN @Ids i ON ou.[Id] = i.[Id]
+    WHERE
+        c.[Type] = 1
+END
+GO