diff --git a/src/Core/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommand.cs index f73c49c81102..b23dc62d2673 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommand.cs @@ -6,6 +6,7 @@ using Bit.Core.Exceptions; using Bit.Core.Repositories; using Bit.Core.Services; +using Bit.Core.Vault.Services; namespace Bit.Core.AdminConsole.OrganizationFeatures.Organizations; @@ -15,17 +16,20 @@ public class OrganizationDeleteCommand : IOrganizationDeleteCommand private readonly IOrganizationRepository _organizationRepository; private readonly IStripePaymentService _paymentService; private readonly ISsoConfigRepository _ssoConfigRepository; + private readonly ICipherService _cipherService; public OrganizationDeleteCommand( IApplicationCacheService applicationCacheService, IOrganizationRepository organizationRepository, IStripePaymentService paymentService, - ISsoConfigRepository ssoConfigRepository) + ISsoConfigRepository ssoConfigRepository, + ICipherService cipherService) { _applicationCacheService = applicationCacheService; _organizationRepository = organizationRepository; _paymentService = paymentService; _ssoConfigRepository = ssoConfigRepository; + _cipherService = cipherService; } public async Task DeleteAsync(Organization organization) @@ -43,6 +47,7 @@ public async Task DeleteAsync(Organization organization) catch (GatewayException) { } } + await _cipherService.DeleteAttachmentsForOrganizationAsync(organization.Id); await _organizationRepository.DeleteAsync(organization); await _applicationCacheService.DeleteOrganizationAbilityAsync(organization.Id); } 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 92ccd47c70c7..2082dad1ef5f 100644 --- a/src/Core/Vault/Services/Implementations/CipherService.cs +++ b/src/Core/Vault/Services/Implementations/CipherService.cs @@ -460,6 +460,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)) @@ -494,10 +500,26 @@ public async Task PurgeAsync(Guid organizationId) { throw new NotFoundException(); } + + + await DeleteAttachmentsForOrganizationAsync(organizationId); + await _cipherRepository.DeleteByOrganizationIdAsync(organizationId); + await _eventService.LogOrganizationEventAsync(org, EventType.Organization_PurgedVault); } + public async Task DeleteAttachmentsForOrganizationAsync(Guid organizationId) + { + var cipherIdsWithAttachments = (await _cipherRepository.GetManyByOrganizationIdAsync(organizationId)) + .Where(c => c.GetAttachments()?.Count > 0).Select(c => c.Id); + + foreach (var cipherId in cipherIdsWithAttachments) + { + await _attachmentStorageService.DeleteAttachmentsForCipherAsync(cipherId); + } + } + public async Task MoveManyAsync(IEnumerable cipherIds, Guid? destinationFolderId, Guid movingUserId) { if (destinationFolderId.HasValue) diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommandTests.cs index 0a83bb89d88e..0e09de2f17e1 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommandTests.cs @@ -8,6 +8,7 @@ using Bit.Core.Repositories; using Bit.Core.Services; using Bit.Core.Test.AutoFixture.OrganizationFixtures; +using Bit.Core.Vault.Services; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; using NSubstitute; @@ -19,15 +20,18 @@ 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, + SutProvider sutProvider) { var organizationRepository = sutProvider.GetDependency(); var applicationCacheService = sutProvider.GetDependency(); + var cipherService = sutProvider.GetDependency(); await sutProvider.Sut.DeleteAsync(organization); - await organizationRepository.Received().DeleteAsync(organization); - await applicationCacheService.Received().DeleteOrganizationAbilityAsync(organization.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 5a2762ba8782..c5ad4e5b084c 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,41 @@ 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; + cipher.Attachments = JsonSerializer.Serialize( + new Dictionary { { "attachment1", new CipherAttachment.MetaData() } }); + } + + 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( @@ -2724,4 +2771,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); + } + } }