From fe0f808aeab45932d5efbe593d24c90cb7057855 Mon Sep 17 00:00:00 2001 From: jaasen-livefront Date: Thu, 12 Mar 2026 17:04:35 -0700 Subject: [PATCH 1/4] delete attachments from deleted ciphers --- .../OrganizationDeleteCommand.cs | 15 +++++++ .../Services/Implementations/CipherService.cs | 16 +++++++ .../OrganizationDeleteCommandTests.cs | 14 +++++- .../Vault/Services/CipherServiceTests.cs | 45 +++++++++++++++++++ 4 files changed, 89 insertions(+), 1 deletion(-) diff --git a/src/Core/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommand.cs index f73c49c81102..549f0b803aae 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommand.cs @@ -6,23 +6,30 @@ using Bit.Core.Exceptions; using Bit.Core.Repositories; using Bit.Core.Services; +using Bit.Core.Vault.Repositories; namespace Bit.Core.AdminConsole.OrganizationFeatures.Organizations; public class OrganizationDeleteCommand : IOrganizationDeleteCommand { private readonly IApplicationCacheService _applicationCacheService; + private readonly IAttachmentStorageService _attachmentStorageService; + private readonly ICipherRepository _cipherRepository; private readonly IOrganizationRepository _organizationRepository; private readonly IStripePaymentService _paymentService; private readonly ISsoConfigRepository _ssoConfigRepository; public OrganizationDeleteCommand( IApplicationCacheService applicationCacheService, + IAttachmentStorageService attachmentStorageService, + ICipherRepository cipherRepository, IOrganizationRepository organizationRepository, IStripePaymentService paymentService, ISsoConfigRepository ssoConfigRepository) { _applicationCacheService = applicationCacheService; + _attachmentStorageService = attachmentStorageService; + _cipherRepository = cipherRepository; _organizationRepository = organizationRepository; _paymentService = paymentService; _ssoConfigRepository = ssoConfigRepository; @@ -43,8 +50,16 @@ public async Task DeleteAsync(Organization organization) catch (GatewayException) { } } + // Fetch cipher IDs before DB deletion so we can clean up attachment storage + var orgCiphers = await _cipherRepository.GetManyByOrganizationIdAsync(organization.Id); + await _organizationRepository.DeleteAsync(organization); await _applicationCacheService.DeleteOrganizationAbilityAsync(organization.Id); + + foreach (var cipher in orgCiphers) + { + await _attachmentStorageService.DeleteAttachmentsForCipherAsync(cipher.Id); + } } private async Task ValidateDeleteOrganizationAsync(Organization organization) diff --git a/src/Core/Vault/Services/Implementations/CipherService.cs b/src/Core/Vault/Services/Implementations/CipherService.cs index 5f235879c676..4308f63dcc20 100644 --- a/src/Core/Vault/Services/Implementations/CipherService.cs +++ b/src/Core/Vault/Services/Implementations/CipherService.cs @@ -458,6 +458,12 @@ public async Task DeleteManyAsync(IEnumerable cipherIds, Guid deletingUser await _cipherRepository.DeleteAsync(deletingCiphers.Select(c => c.Id), deletingUserId); } + // Clean up attachment files from storage + foreach (var cipher in deletingCiphers) + { + await _attachmentStorageService.DeleteAttachmentsForCipherAsync(cipher.Id); + } + var events = deletingCiphers.Select(c => new Tuple(c, EventType.Cipher_Deleted, null)); foreach (var eventsBatch in events.Chunk(100)) @@ -492,7 +498,17 @@ public async Task PurgeAsync(Guid organizationId) { throw new NotFoundException(); } + + // Fetch cipher IDs before DB deletion so we can clean up attachment storage + var orgCiphers = await _cipherRepository.GetManyByOrganizationIdAsync(organizationId); + await _cipherRepository.DeleteByOrganizationIdAsync(organizationId); + + foreach (var cipher in orgCiphers) + { + await _attachmentStorageService.DeleteAttachmentsForCipherAsync(cipher.Id); + } + await _eventService.LogOrganizationEventAsync(org, EventType.Organization_PurgedVault); } diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommandTests.cs index 0a83bb89d88e..515930b992f9 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommandTests.cs @@ -8,6 +8,8 @@ using Bit.Core.Repositories; using Bit.Core.Services; using Bit.Core.Test.AutoFixture.OrganizationFixtures; +using Bit.Core.Vault.Entities; +using Bit.Core.Vault.Repositories; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; using NSubstitute; @@ -19,15 +21,25 @@ namespace Bit.Core.Test.AdminConsole.OrganizationFeatures.Organizations; public class OrganizationDeleteCommandTests { [Theory, PaidOrganizationCustomize, BitAutoData] - public async Task Delete_Success(Organization organization, SutProvider sutProvider) + public async Task Delete_Success(Organization organization, List ciphers, + SutProvider sutProvider) { var organizationRepository = sutProvider.GetDependency(); var applicationCacheService = sutProvider.GetDependency(); + var attachmentStorageService = sutProvider.GetDependency(); + + sutProvider.GetDependency() + .GetManyByOrganizationIdAsync(organization.Id) + .Returns(ciphers); await sutProvider.Sut.DeleteAsync(organization); await organizationRepository.Received().DeleteAsync(organization); await applicationCacheService.Received().DeleteOrganizationAbilityAsync(organization.Id); + foreach (var cipher in ciphers) + { + await attachmentStorageService.Received(1).DeleteAttachmentsForCipherAsync(cipher.Id); + } } [Theory, PaidOrganizationCustomize, BitAutoData] diff --git a/test/Core.Test/Vault/Services/CipherServiceTests.cs b/test/Core.Test/Vault/Services/CipherServiceTests.cs index 56430d7d7320..c02d8502da46 100644 --- a/test/Core.Test/Vault/Services/CipherServiceTests.cs +++ b/test/Core.Test/Vault/Services/CipherServiceTests.cs @@ -1802,6 +1802,12 @@ await sutProvider.GetDependency() .Received(1) .DeleteByIdsOrganizationIdAsync(Arg.Is>(ids => ids.Count() == cipherIds.Count() && ids.All(id => cipherIds.Contains(id))), organizationId); + foreach (var cipher in ciphers) + { + await sutProvider.GetDependency() + .Received(1) + .DeleteAttachmentsForCipherAsync(cipher.Id); + } await sutProvider.GetDependency() .Received(1) .LogCipherEventsAsync(Arg.Any>>()); @@ -1840,6 +1846,12 @@ await sutProvider.GetDependency() .Received(1) .DeleteAsync(Arg.Is>(ids => ids.Count() == cipherIds.Count() && ids.All(id => cipherIds.Contains(id))), deletingUserId); + foreach (var cipher in ciphers) + { + await sutProvider.GetDependency() + .Received(1) + .DeleteAttachmentsForCipherAsync(cipher.Id); + } await sutProvider.GetDependency() .Received(1) .LogCipherEventsAsync(Arg.Any>>()); @@ -1980,6 +1992,39 @@ await sutProvider.GetDependency() .PushSyncCiphersAsync(deletingUserId); } + [Theory] + [BitAutoData] + public async Task PurgeAsync_WithOrganizationId_DeletesCiphersAndAttachments( + Organization org, List ciphers, SutProvider sutProvider) + { + foreach (var cipher in ciphers) + { + cipher.OrganizationId = org.Id; + } + + sutProvider.GetDependency() + .GetByIdAsync(org.Id) + .Returns(org); + sutProvider.GetDependency() + .GetManyByOrganizationIdAsync(org.Id) + .Returns(ciphers); + + await sutProvider.Sut.PurgeAsync(org.Id); + + await sutProvider.GetDependency() + .Received(1) + .DeleteByOrganizationIdAsync(org.Id); + foreach (var cipher in ciphers) + { + await sutProvider.GetDependency() + .Received(1) + .DeleteAttachmentsForCipherAsync(cipher.Id); + } + await sutProvider.GetDependency() + .Received(1) + .LogOrganizationEventAsync(org, EventType.Organization_PurgedVault); + } + [Theory] [BitAutoData] public async Task SoftDeleteAsync_WithPersonalCipherOwner_SoftDeletesCipher( From 834a1b92a4b327202cd94c5d628a34522acd5ec7 Mon Sep 17 00:00:00 2001 From: jaasen-livefront Date: Wed, 18 Mar 2026 14:26:14 -0700 Subject: [PATCH 2/4] add DeleteAttachmentsForOrganizationAsync to cipher service. update specs --- .../OrganizationDeleteCommand.cs | 22 +++-------- src/Core/Vault/Services/ICipherService.cs | 1 + .../Services/Implementations/CipherService.cs | 16 +++++--- .../OrganizationDeleteCommandTests.cs | 20 +++------- .../Vault/Services/CipherServiceTests.cs | 39 +++++++++++++++++++ 5 files changed, 63 insertions(+), 35 deletions(-) diff --git a/src/Core/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommand.cs index 549f0b803aae..b23dc62d2673 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommand.cs @@ -6,33 +6,30 @@ using Bit.Core.Exceptions; using Bit.Core.Repositories; using Bit.Core.Services; -using Bit.Core.Vault.Repositories; +using Bit.Core.Vault.Services; namespace Bit.Core.AdminConsole.OrganizationFeatures.Organizations; public class OrganizationDeleteCommand : IOrganizationDeleteCommand { private readonly IApplicationCacheService _applicationCacheService; - private readonly IAttachmentStorageService _attachmentStorageService; - private readonly ICipherRepository _cipherRepository; private readonly IOrganizationRepository _organizationRepository; private readonly IStripePaymentService _paymentService; private readonly ISsoConfigRepository _ssoConfigRepository; + private readonly ICipherService _cipherService; public OrganizationDeleteCommand( IApplicationCacheService applicationCacheService, - IAttachmentStorageService attachmentStorageService, - ICipherRepository cipherRepository, IOrganizationRepository organizationRepository, IStripePaymentService paymentService, - ISsoConfigRepository ssoConfigRepository) + ISsoConfigRepository ssoConfigRepository, + ICipherService cipherService) { _applicationCacheService = applicationCacheService; - _attachmentStorageService = attachmentStorageService; - _cipherRepository = cipherRepository; _organizationRepository = organizationRepository; _paymentService = paymentService; _ssoConfigRepository = ssoConfigRepository; + _cipherService = cipherService; } public async Task DeleteAsync(Organization organization) @@ -50,16 +47,9 @@ public async Task DeleteAsync(Organization organization) catch (GatewayException) { } } - // Fetch cipher IDs before DB deletion so we can clean up attachment storage - var orgCiphers = await _cipherRepository.GetManyByOrganizationIdAsync(organization.Id); - + await _cipherService.DeleteAttachmentsForOrganizationAsync(organization.Id); await _organizationRepository.DeleteAsync(organization); await _applicationCacheService.DeleteOrganizationAbilityAsync(organization.Id); - - foreach (var cipher in orgCiphers) - { - await _attachmentStorageService.DeleteAttachmentsForCipherAsync(cipher.Id); - } } private async Task ValidateDeleteOrganizationAsync(Organization organization) diff --git a/src/Core/Vault/Services/ICipherService.cs b/src/Core/Vault/Services/ICipherService.cs index ae5d622f1f33..2fe9ac4e0332 100644 --- a/src/Core/Vault/Services/ICipherService.cs +++ b/src/Core/Vault/Services/ICipherService.cs @@ -38,4 +38,5 @@ Task> ShareManyAsync(IEnumerable<(CipherDetails ciphe Task ValidateCipherAttachmentFile(Cipher cipher, CipherAttachment.MetaData attachmentData); Task ValidateBulkCollectionAssignmentAsync(IEnumerable collectionIds, IEnumerable cipherIds, Guid userId); Task ValidateCipherEditForAttachmentAsync(Cipher cipher, Guid savingUserId, bool orgAdmin, long requestLength); + Task DeleteAttachmentsForOrganizationAsync(Guid organizationId); } diff --git a/src/Core/Vault/Services/Implementations/CipherService.cs b/src/Core/Vault/Services/Implementations/CipherService.cs index 0a030629a683..1f0ae123f489 100644 --- a/src/Core/Vault/Services/Implementations/CipherService.cs +++ b/src/Core/Vault/Services/Implementations/CipherService.cs @@ -501,17 +501,23 @@ public async Task PurgeAsync(Guid organizationId) throw new NotFoundException(); } - // Fetch cipher IDs before DB deletion so we can clean up attachment storage - var orgCiphers = await _cipherRepository.GetManyByOrganizationIdAsync(organizationId); + + await DeleteAttachmentsForOrganizationAsync(organizationId); await _cipherRepository.DeleteByOrganizationIdAsync(organizationId); - foreach (var cipher in orgCiphers) + await _eventService.LogOrganizationEventAsync(org, EventType.Organization_PurgedVault); + } + + public async Task DeleteAttachmentsForOrganizationAsync(Guid organizationId) + { + var ciphersWithAttachments = (await _cipherRepository.GetManyByOrganizationIdAsync(organizationId)) + .Where(c => c.GetAttachments()?.Count > 0); + + foreach (var cipher in ciphersWithAttachments) { await _attachmentStorageService.DeleteAttachmentsForCipherAsync(cipher.Id); } - - await _eventService.LogOrganizationEventAsync(org, EventType.Organization_PurgedVault); } public async Task MoveManyAsync(IEnumerable cipherIds, Guid? destinationFolderId, Guid movingUserId) diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommandTests.cs index 515930b992f9..0e09de2f17e1 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommandTests.cs @@ -8,8 +8,7 @@ using Bit.Core.Repositories; using Bit.Core.Services; using Bit.Core.Test.AutoFixture.OrganizationFixtures; -using Bit.Core.Vault.Entities; -using Bit.Core.Vault.Repositories; +using Bit.Core.Vault.Services; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; using NSubstitute; @@ -21,25 +20,18 @@ namespace Bit.Core.Test.AdminConsole.OrganizationFeatures.Organizations; public class OrganizationDeleteCommandTests { [Theory, PaidOrganizationCustomize, BitAutoData] - public async Task Delete_Success(Organization organization, List ciphers, + public async Task Delete_Success(Organization organization, SutProvider sutProvider) { var organizationRepository = sutProvider.GetDependency(); var applicationCacheService = sutProvider.GetDependency(); - var attachmentStorageService = sutProvider.GetDependency(); - - sutProvider.GetDependency() - .GetManyByOrganizationIdAsync(organization.Id) - .Returns(ciphers); + var cipherService = sutProvider.GetDependency(); await sutProvider.Sut.DeleteAsync(organization); - await organizationRepository.Received().DeleteAsync(organization); - await applicationCacheService.Received().DeleteOrganizationAbilityAsync(organization.Id); - foreach (var cipher in ciphers) - { - await attachmentStorageService.Received(1).DeleteAttachmentsForCipherAsync(cipher.Id); - } + await cipherService.Received(1).DeleteAttachmentsForOrganizationAsync(organization.Id); + await organizationRepository.Received(1).DeleteAsync(organization); + await applicationCacheService.Received(1).DeleteOrganizationAbilityAsync(organization.Id); } [Theory, PaidOrganizationCustomize, BitAutoData] diff --git a/test/Core.Test/Vault/Services/CipherServiceTests.cs b/test/Core.Test/Vault/Services/CipherServiceTests.cs index 3544343d93d2..dd1f1c2bc67b 100644 --- a/test/Core.Test/Vault/Services/CipherServiceTests.cs +++ b/test/Core.Test/Vault/Services/CipherServiceTests.cs @@ -2769,4 +2769,43 @@ public async Task GetAttachmentDownloadDataAsync_ReturnsUrlFromStorageService( Assert.Equal(expectedUrl, result.Url); Assert.Equal(attachmentId, result.Id); } + + [Theory, BitAutoData] + public async Task DeleteAttachmentsForOrganizationAsync_OnlyDeletesAttachmentsForCiphersWithAttachments( + SutProvider sutProvider, + Guid organizationId, + List ciphersWithAttachments, + List ciphersWithoutAttachments) + { + foreach (var cipher in ciphersWithAttachments) + { + cipher.Attachments = JsonSerializer.Serialize( + new Dictionary { { "attachment1", new CipherAttachment.MetaData() } }); + } + + foreach (var cipher in ciphersWithoutAttachments) + { + cipher.Attachments = null; + } + + sutProvider.GetDependency() + .GetManyByOrganizationIdAsync(organizationId) + .Returns(ciphersWithAttachments.Concat(ciphersWithoutAttachments).ToList()); + + await sutProvider.Sut.DeleteAttachmentsForOrganizationAsync(organizationId); + + foreach (var cipher in ciphersWithAttachments) + { + await sutProvider.GetDependency() + .Received(1) + .DeleteAttachmentsForCipherAsync(cipher.Id); + } + + foreach (var cipher in ciphersWithoutAttachments) + { + await sutProvider.GetDependency() + .DidNotReceive() + .DeleteAttachmentsForCipherAsync(cipher.Id); + } + } } From 5dcb0f2e7c764ebf4508668760a0ec6e86ac8535 Mon Sep 17 00:00:00 2001 From: jaasen-livefront Date: Wed, 18 Mar 2026 14:45:27 -0700 Subject: [PATCH 3/4] fix test --- test/Core.Test/Vault/Services/CipherServiceTests.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/Core.Test/Vault/Services/CipherServiceTests.cs b/test/Core.Test/Vault/Services/CipherServiceTests.cs index dd1f1c2bc67b..c5ad4e5b084c 100644 --- a/test/Core.Test/Vault/Services/CipherServiceTests.cs +++ b/test/Core.Test/Vault/Services/CipherServiceTests.cs @@ -2000,6 +2000,8 @@ public async Task PurgeAsync_WithOrganizationId_DeletesCiphersAndAttachments( foreach (var cipher in ciphers) { cipher.OrganizationId = org.Id; + cipher.Attachments = JsonSerializer.Serialize( + new Dictionary { { "attachment1", new CipherAttachment.MetaData() } }); } sutProvider.GetDependency() From 2cac61e19494e97cfc50825079964926b965df55 Mon Sep 17 00:00:00 2001 From: jaasen-livefront Date: Wed, 18 Mar 2026 15:05:28 -0700 Subject: [PATCH 4/4] only grab cipherId for attachments deletion --- src/Core/Vault/Services/Implementations/CipherService.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Core/Vault/Services/Implementations/CipherService.cs b/src/Core/Vault/Services/Implementations/CipherService.cs index 1f0ae123f489..2082dad1ef5f 100644 --- a/src/Core/Vault/Services/Implementations/CipherService.cs +++ b/src/Core/Vault/Services/Implementations/CipherService.cs @@ -511,12 +511,12 @@ public async Task PurgeAsync(Guid organizationId) public async Task DeleteAttachmentsForOrganizationAsync(Guid organizationId) { - var ciphersWithAttachments = (await _cipherRepository.GetManyByOrganizationIdAsync(organizationId)) - .Where(c => c.GetAttachments()?.Count > 0); + var cipherIdsWithAttachments = (await _cipherRepository.GetManyByOrganizationIdAsync(organizationId)) + .Where(c => c.GetAttachments()?.Count > 0).Select(c => c.Id); - foreach (var cipher in ciphersWithAttachments) + foreach (var cipherId in cipherIdsWithAttachments) { - await _attachmentStorageService.DeleteAttachmentsForCipherAsync(cipher.Id); + await _attachmentStorageService.DeleteAttachmentsForCipherAsync(cipherId); } }