diff --git a/src/Core/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommand.cs index 5254bd2ca62d..f44d464e6cd2 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommand.cs @@ -7,6 +7,7 @@ using Bit.Core.Exceptions; using Bit.Core.Repositories; using Bit.Core.Services; +using Bit.Core.Vault.Services; using Microsoft.Extensions.Logging; namespace Bit.Core.AdminConsole.OrganizationFeatures.Organizations; @@ -17,6 +18,7 @@ public class OrganizationDeleteCommand : IOrganizationDeleteCommand private readonly IOrganizationRepository _organizationRepository; private readonly IStripePaymentService _paymentService; private readonly ISsoConfigRepository _ssoConfigRepository; + private readonly ICipherService _cipherService; private readonly ISubscriberService _subscriberService; private readonly IFeatureService _featureService; private readonly ILogger _logger; @@ -26,6 +28,7 @@ public OrganizationDeleteCommand( IOrganizationRepository organizationRepository, IStripePaymentService paymentService, ISsoConfigRepository ssoConfigRepository, + ICipherService cipherService, ISubscriberService subscriberService, IFeatureService featureService, ILogger logger) @@ -34,6 +37,7 @@ public OrganizationDeleteCommand( _organizationRepository = organizationRepository; _paymentService = paymentService; _ssoConfigRepository = ssoConfigRepository; + _cipherService = cipherService; _subscriberService = subscriberService; _featureService = featureService; _logger = logger; @@ -66,6 +70,7 @@ public async Task DeleteAsync(Organization organization) } } + 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 714d00cdc699..4e5bca4bd1e1 100644 --- a/src/Core/Vault/Services/Implementations/CipherService.cs +++ b/src/Core/Vault/Services/Implementations/CipherService.cs @@ -451,6 +451,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)) @@ -485,10 +491,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 e0149a8407fe..e53151ce6cc7 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommandTests.cs @@ -10,6 +10,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; @@ -22,15 +23,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 31c1e239206c..51e6b8531510 100644 --- a/test/Core.Test/Vault/Services/CipherServiceTests.cs +++ b/test/Core.Test/Vault/Services/CipherServiceTests.cs @@ -1,4 +1,5 @@ -using Bit.Core.AdminConsole.Entities; +using System.Text.Json; +using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.Models.Data.Organizations.Policies; using Bit.Core.AdminConsole.OrganizationFeatures.Policies; @@ -1436,6 +1437,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>>()); @@ -1474,6 +1481,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>>()); @@ -1660,6 +1673,41 @@ await sutProvider.GetDependency() ids.Contains(orgIdNotInCache))); } + [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( @@ -2404,4 +2452,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); + } + } }