From 1cde4f95a4d85a0ee4f5ca76c6b5bcc98534268e Mon Sep 17 00:00:00 2001 From: John Harrington <84741727+harr1424@users.noreply.github.com> Date: Fri, 3 Apr 2026 09:17:00 -0700 Subject: [PATCH 1/6] initial work to prevent orphaned sends on user/org deletion --- .../OrganizationDeleteCommand.cs | 5 + .../Services/Implementations/UserService.cs | 22 ++- .../Commands/NonAnonymousSendCommand.cs | 2 +- .../Services/AzureSendFileStorageService.cs | 16 ++ .../Repositories/OrganizationRepository.cs | 3 + .../Organization_DeleteById.sql | 6 + ...OrganizationDeleteById_AddSendDeletion.sql | 168 ++++++++++++++++++ 7 files changed, 220 insertions(+), 2 deletions(-) create mode 100644 util/Migrator/DbScripts/2026-04-06_00_OrganizationDeleteById_AddSendDeletion.sql diff --git a/src/Core/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommand.cs index 5254bd2ca62d..508f52253cdd 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.Tools.Services; using Microsoft.Extensions.Logging; namespace Bit.Core.AdminConsole.OrganizationFeatures.Organizations; @@ -19,6 +20,7 @@ public class OrganizationDeleteCommand : IOrganizationDeleteCommand private readonly ISsoConfigRepository _ssoConfigRepository; private readonly ISubscriberService _subscriberService; private readonly IFeatureService _featureService; + private readonly ISendFileStorageService _sendFileStorageService; private readonly ILogger _logger; public OrganizationDeleteCommand( @@ -28,6 +30,7 @@ public OrganizationDeleteCommand( ISsoConfigRepository ssoConfigRepository, ISubscriberService subscriberService, IFeatureService featureService, + ISendFileStorageService sendFileStorageService, ILogger logger) { _applicationCacheService = applicationCacheService; @@ -36,6 +39,7 @@ public OrganizationDeleteCommand( _ssoConfigRepository = ssoConfigRepository; _subscriberService = subscriberService; _featureService = featureService; + _sendFileStorageService = sendFileStorageService; _logger = logger; } @@ -66,6 +70,7 @@ public async Task DeleteAsync(Organization organization) } } + await _sendFileStorageService.DeleteFilesForOrganizationAsync(organization.Id); await _organizationRepository.DeleteAsync(organization); await _applicationCacheService.DeleteOrganizationAbilityAsync(organization.Id); } diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index 50273e281dea..c7969e53d760 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -30,6 +30,10 @@ using Bit.Core.Platform.Push; using Bit.Core.Repositories; using Bit.Core.Settings; +using Bit.Core.Tools.Enums; +using Bit.Core.Tools.Models.Data; +using Bit.Core.Tools.Repositories; +using Bit.Core.Tools.Services; using Bit.Core.Utilities; using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.Caching.Distributed; @@ -69,6 +73,8 @@ public class UserService : UserManager, IUserService private readonly IPricingClient _pricingClient; private readonly IHasPremiumAccessQuery _hasPremiumAccessQuery; private readonly ISubscriberService _subscriberService; + private readonly ISendRepository _sendRepository; + private readonly ISendFileStorageService _sendFileStorageService; public UserService( IUserRepository userRepository, @@ -102,7 +108,9 @@ public UserService( IPolicyRequirementQuery policyRequirementQuery, IPricingClient pricingClient, IHasPremiumAccessQuery hasPremiumAccessQuery, - ISubscriberService subscriberService) + ISubscriberService subscriberService, + ISendRepository sendRepository, + ISendFileStorageService sendFileStorageService) : base( store, optionsAccessor, @@ -141,6 +149,8 @@ public UserService( _pricingClient = pricingClient; _hasPremiumAccessQuery = hasPremiumAccessQuery; _subscriberService = subscriberService; + _sendRepository = sendRepository; + _sendFileStorageService = sendFileStorageService; } public Guid? GetProperUserId(ClaimsPrincipal principal) @@ -281,6 +291,16 @@ await _subscriberService.CancelSubscription( catch (BillingException) { } } + var sends = await _sendRepository.GetManyByUserIdAsync(user.Id); + foreach (var send in sends.Where(s => s.Type == SendType.File)) + { + var data = JsonSerializer.Deserialize(send.Data); + if (data?.Id != null) + { + await _sendFileStorageService.DeleteFileAsync(send, data.Id); + } + } + await _userRepository.DeleteAsync(user); await _pushService.PushLogOutAsync(user.Id); return IdentityResult.Success; diff --git a/src/Core/Tools/SendFeatures/Commands/NonAnonymousSendCommand.cs b/src/Core/Tools/SendFeatures/Commands/NonAnonymousSendCommand.cs index 21ca1ca3fb00..5c682ebae1d7 100644 --- a/src/Core/Tools/SendFeatures/Commands/NonAnonymousSendCommand.cs +++ b/src/Core/Tools/SendFeatures/Commands/NonAnonymousSendCommand.cs @@ -137,12 +137,12 @@ public async Task UploadFileToExistingSendAsync(Stream stream, Send send) } public async Task DeleteSendAsync(Send send) { - await _sendRepository.DeleteAsync(send); if (send.Type == Enums.SendType.File) { var data = JsonSerializer.Deserialize(send.Data); await _sendFileStorageService.DeleteFileAsync(send, data.Id); } + await _sendRepository.DeleteAsync(send); await _pushNotificationService.PushSyncSendDeleteAsync(send); } diff --git a/src/Core/Tools/SendFeatures/Services/AzureSendFileStorageService.cs b/src/Core/Tools/SendFeatures/Services/AzureSendFileStorageService.cs index 63405b8cdf66..5ca504643d47 100644 --- a/src/Core/Tools/SendFeatures/Services/AzureSendFileStorageService.cs +++ b/src/Core/Tools/SendFeatures/Services/AzureSendFileStorageService.cs @@ -66,11 +66,27 @@ public async Task DeleteBlobAsync(string blobName) public async Task DeleteFilesForOrganizationAsync(Guid organizationId) { await InitAsync(); + await DeleteBlobsByMetadataAsync("organizationId", organizationId.ToString()); } public async Task DeleteFilesForUserAsync(Guid userId) { await InitAsync(); + await DeleteBlobsByMetadataAsync("userId", userId.ToString()); + } + + private async Task DeleteBlobsByMetadataAsync(string metadataKey, string metadataValue) + { + await foreach (var blobItem in _sendFilesContainerClient!.GetBlobsAsync(BlobTraits.Metadata)) + { + if (blobItem.Metadata != null && + blobItem.Metadata.TryGetValue(metadataKey, out var value) && + string.Equals(value, metadataValue, StringComparison.OrdinalIgnoreCase)) + { + var blob = _sendFilesContainerClient.GetBlobClient(blobItem.Name); + await blob.DeleteIfExistsAsync(); + } + } } public async Task GetSendFileDownloadUrlAsync(Send send, string fileId) diff --git a/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationRepository.cs b/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationRepository.cs index 48dd04f62af4..8e72fe78abaf 100644 --- a/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationRepository.cs +++ b/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationRepository.cs @@ -267,6 +267,9 @@ await dbContext.NotificationStatuses.Where(ns => ns.Notification.OrganizationId await dbContext.Notifications.Where(n => n.OrganizationId == organization.Id) .ExecuteDeleteAsync(); + await dbContext.Sends.Where(s => s.OrganizationId == organization.Id) + .ExecuteDeleteAsync(); + // The below section are 3 SPROCS in SQL Server but are only called by here await dbContext.OrganizationApiKeys.Where(oa => oa.OrganizationId == organization.Id) .ExecuteDeleteAsync(); diff --git a/src/Sql/dbo/Stored Procedures/Organization_DeleteById.sql b/src/Sql/dbo/Stored Procedures/Organization_DeleteById.sql index 50ad247c1afb..ff693287e58a 100644 --- a/src/Sql/dbo/Stored Procedures/Organization_DeleteById.sql +++ b/src/Sql/dbo/Stored Procedures/Organization_DeleteById.sql @@ -150,6 +150,12 @@ BEGIN [dbo].[OrganizationReport] WHERE [OrganizationId] = @Id + -- Delete Organization Owned Sends + DELETE + FROM + [dbo].[Send] + WHERE + [OrganizationId] = @Id DELETE FROM diff --git a/util/Migrator/DbScripts/2026-04-06_00_OrganizationDeleteById_AddSendDeletion.sql b/util/Migrator/DbScripts/2026-04-06_00_OrganizationDeleteById_AddSendDeletion.sql new file mode 100644 index 000000000000..f529769f948a --- /dev/null +++ b/util/Migrator/DbScripts/2026-04-06_00_OrganizationDeleteById_AddSendDeletion.sql @@ -0,0 +1,168 @@ +CREATE OR ALTER PROCEDURE [dbo].[Organization_DeleteById] + @Id UNIQUEIDENTIFIER +WITH RECOMPILE +AS +BEGIN + SET NOCOUNT ON + + EXEC [dbo].[User_BumpAccountRevisionDateByOrganizationId] @Id + + DECLARE @BatchSize INT = 100 + WHILE @BatchSize > 0 + BEGIN + BEGIN TRANSACTION Organization_DeleteById_Ciphers + + DELETE TOP(@BatchSize) + FROM + [dbo].[Cipher] + WHERE + [UserId] IS NULL + AND [OrganizationId] = @Id + + SET @BatchSize = @@ROWCOUNT + + COMMIT TRANSACTION Organization_DeleteById_Ciphers + END + + BEGIN TRANSACTION Organization_DeleteById + + DELETE + FROM + [dbo].[AuthRequest] + WHERE + [OrganizationId] = @Id + + DELETE + FROM + [dbo].[SsoUser] + WHERE + [OrganizationId] = @Id + + DELETE + FROM + [dbo].[SsoConfig] + WHERE + [OrganizationId] = @Id + + DELETE CU + FROM + [dbo].[CollectionUser] CU + INNER JOIN + [dbo].[OrganizationUser] OU ON [CU].[OrganizationUserId] = [OU].[Id] + WHERE + [OU].[OrganizationId] = @Id + + DELETE AP + FROM + [dbo].[AccessPolicy] AP + INNER JOIN + [dbo].[OrganizationUser] OU ON [AP].[OrganizationUserId] = [OU].[Id] + WHERE + [OU].[OrganizationId] = @Id + + DELETE GU + FROM + [dbo].[GroupUser] GU + INNER JOIN + [dbo].[OrganizationUser] OU ON [GU].[OrganizationUserId] = [OU].[Id] + WHERE + [OU].[OrganizationId] = @Id + + DELETE + FROM + [dbo].[OrganizationUser] + WHERE + [OrganizationId] = @Id + + DELETE + FROM + [dbo].[ProviderOrganization] + WHERE + [OrganizationId] = @Id + + EXEC [dbo].[OrganizationApiKey_OrganizationDeleted] @Id + EXEC [dbo].[OrganizationConnection_OrganizationDeleted] @Id + EXEC [dbo].[OrganizationSponsorship_OrganizationDeleted] @Id + EXEC [dbo].[OrganizationDomain_OrganizationDeleted] @Id + EXEC [dbo].[OrganizationIntegration_OrganizationDeleted] @Id + + DELETE + FROM + [dbo].[Project] + WHERE + [OrganizationId] = @Id + + DELETE + FROM + [dbo].[Secret] + WHERE + [OrganizationId] = @Id + + DELETE AK + FROM + [dbo].[ApiKey] AK + INNER JOIN + [dbo].[ServiceAccount] SA ON [AK].[ServiceAccountId] = [SA].[Id] + WHERE + [SA].[OrganizationId] = @Id + + DELETE AP + FROM + [dbo].[AccessPolicy] AP + INNER JOIN + [dbo].[ServiceAccount] SA ON [AP].[GrantedServiceAccountId] = [SA].[Id] + WHERE + [SA].[OrganizationId] = @Id + + DELETE + FROM + [dbo].[ServiceAccount] + WHERE + [OrganizationId] = @Id + + -- Delete Notification Status + DELETE + NS + FROM + [dbo].[NotificationStatus] NS + INNER JOIN + [dbo].[Notification] N ON N.[Id] = NS.[NotificationId] + WHERE + N.[OrganizationId] = @Id + + -- Delete Notification + DELETE + FROM + [dbo].[Notification] + WHERE + [OrganizationId] = @Id + + -- Delete Organization Application + DELETE + FROM + [dbo].[OrganizationApplication] + WHERE + [OrganizationId] = @Id + + -- Delete Organization Report + DELETE + FROM + [dbo].[OrganizationReport] + WHERE + [OrganizationId] = @Id + + -- Delete Organization Owned Sends + DELETE + FROM + [dbo].[Send] + WHERE + [OrganizationId] = @Id + + DELETE + FROM + [dbo].[Organization] + WHERE + [Id] = @Id + + COMMIT TRANSACTION Organization_DeleteById +END From 4c11ab805d40190b001b75d94bcafbe4dd3308a0 Mon Sep 17 00:00:00 2001 From: John Harrington <84741727+harr1424@users.noreply.github.com> Date: Fri, 3 Apr 2026 14:57:52 -0700 Subject: [PATCH 2/6] format SP --- src/Sql/dbo/Stored Procedures/Organization_DeleteById.sql | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Sql/dbo/Stored Procedures/Organization_DeleteById.sql b/src/Sql/dbo/Stored Procedures/Organization_DeleteById.sql index ff693287e58a..87d14a1013f6 100644 --- a/src/Sql/dbo/Stored Procedures/Organization_DeleteById.sql +++ b/src/Sql/dbo/Stored Procedures/Organization_DeleteById.sql @@ -150,6 +150,7 @@ BEGIN [dbo].[OrganizationReport] WHERE [OrganizationId] = @Id + -- Delete Organization Owned Sends DELETE FROM From 74e8a75743b9da4cb37245e0857a015ce3c2b80c Mon Sep 17 00:00:00 2001 From: John Harrington <84741727+harr1424@users.noreply.github.com> Date: Fri, 3 Apr 2026 17:15:49 -0700 Subject: [PATCH 3/6] add ReadByOrgId SP and align deletion strategies, add null guard, and test coverage --- .../OrganizationDeleteCommand.cs | 20 +++++- .../Tools/Repositories/ISendRepository.cs | 12 ++++ .../Commands/NonAnonymousSendCommand.cs | 7 +- .../Tools/Repositories/SendRepository.cs | 16 +++++ .../Tools/Repositories/SendRepository.cs | 11 +++ .../Send_ReadByOrganizationId.sql | 13 ++++ .../OrganizationDeleteCommandTests.cs | 56 ++++++++++++++- test/Core.Test/Services/UserServiceTests.cs | 62 ++++++++++++++++ .../Services/LocalSendStorageServiceTests.cs | 70 +++++++++++++++++++ .../Services/NonAnonymousSendCommandTests.cs | 32 +++++++++ ...6-04-06_01_AddSendReadByOrganizationId.sql | 13 ++++ 11 files changed, 307 insertions(+), 5 deletions(-) create mode 100644 src/Sql/dbo/Tools/Stored Procedures/Send_ReadByOrganizationId.sql create mode 100644 test/Core.Test/Tools/Services/LocalSendStorageServiceTests.cs create mode 100644 util/Migrator/DbScripts/2026-04-06_01_AddSendReadByOrganizationId.sql diff --git a/src/Core/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommand.cs index 508f52253cdd..916e6948b86a 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommand.cs @@ -1,4 +1,5 @@ -using Bit.Core.AdminConsole.Entities; +using System.Text.Json; +using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.OrganizationFeatures.Organizations.Interfaces; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Repositories; @@ -7,6 +8,9 @@ using Bit.Core.Exceptions; using Bit.Core.Repositories; using Bit.Core.Services; +using Bit.Core.Tools.Enums; +using Bit.Core.Tools.Models.Data; +using Bit.Core.Tools.Repositories; using Bit.Core.Tools.Services; using Microsoft.Extensions.Logging; @@ -20,6 +24,7 @@ public class OrganizationDeleteCommand : IOrganizationDeleteCommand private readonly ISsoConfigRepository _ssoConfigRepository; private readonly ISubscriberService _subscriberService; private readonly IFeatureService _featureService; + private readonly ISendRepository _sendRepository; private readonly ISendFileStorageService _sendFileStorageService; private readonly ILogger _logger; @@ -30,6 +35,7 @@ public OrganizationDeleteCommand( ISsoConfigRepository ssoConfigRepository, ISubscriberService subscriberService, IFeatureService featureService, + ISendRepository sendRepository, ISendFileStorageService sendFileStorageService, ILogger logger) { @@ -39,6 +45,7 @@ public OrganizationDeleteCommand( _ssoConfigRepository = ssoConfigRepository; _subscriberService = subscriberService; _featureService = featureService; + _sendRepository = sendRepository; _sendFileStorageService = sendFileStorageService; _logger = logger; } @@ -70,7 +77,16 @@ public async Task DeleteAsync(Organization organization) } } - await _sendFileStorageService.DeleteFilesForOrganizationAsync(organization.Id); + var sends = await _sendRepository.GetManyByOrganizationIdAsync(organization.Id); + foreach (var send in sends.Where(s => s.Type == SendType.File)) + { + var data = send.Data != null ? JsonSerializer.Deserialize(send.Data) : null; + if (data?.Id != null) + { + await _sendFileStorageService.DeleteFileAsync(send, data.Id); + } + } + await _organizationRepository.DeleteAsync(organization); await _applicationCacheService.DeleteOrganizationAbilityAsync(organization.Id); } diff --git a/src/Core/Tools/Repositories/ISendRepository.cs b/src/Core/Tools/Repositories/ISendRepository.cs index 6de89f03748e..53957112f65d 100644 --- a/src/Core/Tools/Repositories/ISendRepository.cs +++ b/src/Core/Tools/Repositories/ISendRepository.cs @@ -23,6 +23,18 @@ public interface ISendRepository : IRepository /// Task> GetManyByUserIdAsync(Guid userId); + /// + /// Loads all s owned by an organization. + /// + /// + /// Identifies the organization. + /// + /// + /// A task that completes once the s have been loaded. + /// The task's result contains the loaded s. + /// + Task> GetManyByOrganizationIdAsync(Guid organizationId); + /// /// Loads s scheduled for deletion. /// diff --git a/src/Core/Tools/SendFeatures/Commands/NonAnonymousSendCommand.cs b/src/Core/Tools/SendFeatures/Commands/NonAnonymousSendCommand.cs index 5c682ebae1d7..eb7e22b9d3a5 100644 --- a/src/Core/Tools/SendFeatures/Commands/NonAnonymousSendCommand.cs +++ b/src/Core/Tools/SendFeatures/Commands/NonAnonymousSendCommand.cs @@ -137,10 +137,13 @@ public async Task UploadFileToExistingSendAsync(Stream stream, Send send) } public async Task DeleteSendAsync(Send send) { - if (send.Type == Enums.SendType.File) + if (send.Type == Enums.SendType.File && send.Data != null) { var data = JsonSerializer.Deserialize(send.Data); - await _sendFileStorageService.DeleteFileAsync(send, data.Id); + if (data?.Id != null) + { + await _sendFileStorageService.DeleteFileAsync(send, data.Id); + } } await _sendRepository.DeleteAsync(send); await _pushNotificationService.PushSyncSendDeleteAsync(send); diff --git a/src/Infrastructure.Dapper/Tools/Repositories/SendRepository.cs b/src/Infrastructure.Dapper/Tools/Repositories/SendRepository.cs index 144e08021d71..e551b4ed4b99 100644 --- a/src/Infrastructure.Dapper/Tools/Repositories/SendRepository.cs +++ b/src/Infrastructure.Dapper/Tools/Repositories/SendRepository.cs @@ -52,6 +52,22 @@ public async Task> GetManyByUserIdAsync(Guid userId) } } + /// + public async Task> GetManyByOrganizationIdAsync(Guid organizationId) + { + using (var connection = new SqlConnection(ConnectionString)) + { + var results = await connection.QueryAsync( + $"[{Schema}].[Send_ReadByOrganizationId]", + new { OrganizationId = organizationId }, + commandType: CommandType.StoredProcedure); + + var sends = results.ToList(); + UnprotectData(sends); + return sends; + } + } + /// public async Task> GetManyByDeletionDateAsync(DateTime deletionDateBefore) { diff --git a/src/Infrastructure.EntityFramework/Tools/Repositories/SendRepository.cs b/src/Infrastructure.EntityFramework/Tools/Repositories/SendRepository.cs index adf3fcc1f1cd..d13a57132348 100644 --- a/src/Infrastructure.EntityFramework/Tools/Repositories/SendRepository.cs +++ b/src/Infrastructure.EntityFramework/Tools/Repositories/SendRepository.cs @@ -71,6 +71,17 @@ public SendRepository(IServiceScopeFactory serviceScopeFactory, IMapper mapper) } } + /// + public async Task> GetManyByOrganizationIdAsync(Guid organizationId) + { + using (var scope = ServiceScopeFactory.CreateScope()) + { + var dbContext = GetDatabaseContext(scope); + var results = await dbContext.Sends.Where(s => s.OrganizationId == organizationId).ToListAsync(); + return Mapper.Map>(results); + } + } + /// public UpdateEncryptedDataForKeyRotation UpdateForKeyRotation(Guid userId, IEnumerable sends) diff --git a/src/Sql/dbo/Tools/Stored Procedures/Send_ReadByOrganizationId.sql b/src/Sql/dbo/Tools/Stored Procedures/Send_ReadByOrganizationId.sql new file mode 100644 index 000000000000..81b1647ebefe --- /dev/null +++ b/src/Sql/dbo/Tools/Stored Procedures/Send_ReadByOrganizationId.sql @@ -0,0 +1,13 @@ +CREATE PROCEDURE [dbo].[Send_ReadByOrganizationId] + @OrganizationId UNIQUEIDENTIFIER +AS +BEGIN + SET NOCOUNT ON + + SELECT + * + FROM + [dbo].[SendView] + WHERE + [OrganizationId] = @OrganizationId +END diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommandTests.cs index e0149a8407fe..deddce69ba5b 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommandTests.cs @@ -1,4 +1,5 @@ -using Bit.Core.AdminConsole.Entities; +using System.Text.Json; +using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.OrganizationFeatures.Organizations; using Bit.Core.Auth.Entities; using Bit.Core.Auth.Enums; @@ -10,6 +11,11 @@ using Bit.Core.Repositories; using Bit.Core.Services; using Bit.Core.Test.AutoFixture.OrganizationFixtures; +using Bit.Core.Tools.Entities; +using Bit.Core.Tools.Enums; +using Bit.Core.Tools.Models.Data; +using Bit.Core.Tools.Repositories; +using Bit.Core.Tools.Services; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; using NSubstitute; @@ -144,4 +150,52 @@ public async Task Delete_WhenFlagDisabled_HandlesBillingException( await sutProvider.GetDependency().Received(1).DeleteAsync(organization); } + + [Theory, PaidOrganizationCustomize, BitAutoData] + public async Task Delete_WithFileSends_DeletesFilesBeforeDbRecords( + Organization organization, + SutProvider sutProvider) + { + // Ensuring that the file is deleted first avoids the following situation: + // 1. DB row is deleted successfully + // 2. File blob fails to delete + // 3. File blob still exists but with no parent Send + var fileData = new SendFileData { Id = "file1", FileName = "test.txt", Size = 100 }; + var fileSend = new Send + { + Id = Guid.NewGuid(), + OrganizationId = organization.Id, + Type = SendType.File, + Data = JsonSerializer.Serialize(fileData) + }; + var textSend = new Send + { + Id = Guid.NewGuid(), + OrganizationId = organization.Id, + Type = SendType.Text, + Data = "{}" + }; + + sutProvider.GetDependency() + .GetManyByOrganizationIdAsync(organization.Id) + .Returns(new List { fileSend, textSend }); + + var callOrder = new List(); + sutProvider.GetDependency() + .DeleteFileAsync(fileSend, fileData.Id) + .Returns(Task.CompletedTask) + .AndDoes(_ => callOrder.Add("file")); + sutProvider.GetDependency() + .DeleteAsync(organization) + .Returns(Task.CompletedTask) + .AndDoes(_ => callOrder.Add("db")); + + await sutProvider.Sut.DeleteAsync(organization); + + await sutProvider.GetDependency() + .Received(1).DeleteFileAsync(fileSend, fileData.Id); + await sutProvider.GetDependency() + .Received(1).DeleteAsync(organization); + Assert.Equal(new[] { "file", "db" }, callOrder); + } } diff --git a/test/Core.Test/Services/UserServiceTests.cs b/test/Core.Test/Services/UserServiceTests.cs index 1a1425d3dc1f..fa1b4e0ccc9f 100644 --- a/test/Core.Test/Services/UserServiceTests.cs +++ b/test/Core.Test/Services/UserServiceTests.cs @@ -25,6 +25,11 @@ using Bit.Core.Services; using Bit.Core.Settings; using Bit.Core.Test.AdminConsole.AutoFixture; +using Bit.Core.Tools.Entities; +using Bit.Core.Tools.Enums; +using Bit.Core.Tools.Models.Data; +using Bit.Core.Tools.Repositories; +using Bit.Core.Tools.Services; using Bit.Core.Utilities; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; @@ -699,6 +704,63 @@ await sutProvider.GetDependency() .DidNotReceiveWithAnyArgs() .CancelSubscription(default, default, default); } + + [Theory, BitAutoData] + public async Task DeleteAsync_WithFileSends_DeletesFilesBeforeDbRecords( + User user, + SutProvider sutProvider) + { + // Ensuring that the file is deleted first avoids the following situation: + // 1. DB row is deleted successfully + // 2. File blob fails to delete + // 3. File blob still exists but with no parent Send + user.GatewaySubscriptionId = null; + + sutProvider.GetDependency() + .GetCountByOnlyOwnerAsync(user.Id) + .Returns(0); + + sutProvider.GetDependency() + .GetCountByOnlyOwnerAsync(user.Id) + .Returns(0); + + var fileData = new SendFileData { Id = "file1", FileName = "test.txt", Size = 100 }; + var fileSend = new Send + { + Id = Guid.NewGuid(), + UserId = user.Id, + Type = SendType.File, + Data = JsonSerializer.Serialize(fileData) + }; + var textSend = new Send + { + Id = Guid.NewGuid(), + UserId = user.Id, + Type = SendType.Text, + Data = "{}" + }; + + sutProvider.GetDependency() + .GetManyByUserIdAsync(user.Id) + .Returns(new List { fileSend, textSend }); + + var callOrder = new List(); + sutProvider.GetDependency() + .DeleteFileAsync(fileSend, fileData.Id) + .Returns(Task.CompletedTask) + .AndDoes(_ => callOrder.Add("file")); + sutProvider.GetDependency() + .DeleteAsync(user) + .Returns(Task.CompletedTask) + .AndDoes(_ => callOrder.Add("db")); + + var result = await sutProvider.Sut.DeleteAsync(user); + + Assert.True(result.Succeeded); + await sutProvider.GetDependency() + .Received(1).DeleteFileAsync(fileSend, fileData.Id); + Assert.Equal(new[] { "file", "db" }, callOrder); + } } public static class UserServiceSutProviderExtensions diff --git a/test/Core.Test/Tools/Services/LocalSendStorageServiceTests.cs b/test/Core.Test/Tools/Services/LocalSendStorageServiceTests.cs new file mode 100644 index 000000000000..f1b5b0420e4d --- /dev/null +++ b/test/Core.Test/Tools/Services/LocalSendStorageServiceTests.cs @@ -0,0 +1,70 @@ +using Bit.Core.Tools.Entities; +using Bit.Core.Tools.Enums; +using Bit.Core.Tools.Services; +using Xunit; + +namespace Bit.Core.Test.Tools.Services; + +public class LocalSendStorageServiceTests +{ + [Fact] + public async Task DeleteFileAsync_FileExists_DeletesFileAndEmptyDirectory() + { + using var tempDirectory = new TempDirectory(); + var sut = CreateSut(tempDirectory); + + var send = new Send { Id = Guid.NewGuid(), Type = SendType.File }; + var fileId = "testfile"; + + // Create the file on disk + var dirPath = Path.Combine(tempDirectory.Directory, send.Id.ToString()); + Directory.CreateDirectory(dirPath); + var filePath = Path.Combine(dirPath, fileId); + await File.WriteAllTextAsync(filePath, "file contents"); + + await sut.DeleteFileAsync(send, fileId); + + Assert.False(File.Exists(filePath)); + Assert.False(Directory.Exists(dirPath)); + } + + [Fact] + public async Task DeleteFileAsync_FileDoesNotExist_DoesNotThrow() + { + using var tempDirectory = new TempDirectory(); + var sut = CreateSut(tempDirectory); + + var send = new Send { Id = Guid.NewGuid(), Type = SendType.File }; + + await sut.DeleteFileAsync(send, "nonexistent"); + } + + [Fact] + public async Task DeleteFileAsync_DirectoryHasOtherFiles_KeepsDirectory() + { + using var tempDirectory = new TempDirectory(); + var sut = CreateSut(tempDirectory); + + var send = new Send { Id = Guid.NewGuid(), Type = SendType.File }; + var fileId = "testfile"; + + var dirPath = Path.Combine(tempDirectory.Directory, send.Id.ToString()); + Directory.CreateDirectory(dirPath); + await File.WriteAllTextAsync(Path.Combine(dirPath, fileId), "delete me"); + await File.WriteAllTextAsync(Path.Combine(dirPath, "otherfile"), "keep me"); + + await sut.DeleteFileAsync(send, fileId); + + Assert.False(File.Exists(Path.Combine(dirPath, fileId))); + Assert.True(Directory.Exists(dirPath)); + Assert.True(File.Exists(Path.Combine(dirPath, "otherfile"))); + } + + private static LocalSendStorageService CreateSut(TempDirectory tempDirectory) + { + var globalSettings = new Bit.Core.Settings.GlobalSettings(); + globalSettings.Send.BaseDirectory = tempDirectory.Directory; + globalSettings.Send.BaseUrl = "https://localhost/sends"; + return new LocalSendStorageService(globalSettings); + } +} diff --git a/test/Core.Test/Tools/Services/NonAnonymousSendCommandTests.cs b/test/Core.Test/Tools/Services/NonAnonymousSendCommandTests.cs index 9bebe5560c93..b7dcca346f3c 100644 --- a/test/Core.Test/Tools/Services/NonAnonymousSendCommandTests.cs +++ b/test/Core.Test/Tools/Services/NonAnonymousSendCommandTests.cs @@ -1416,4 +1416,36 @@ public void SendCanBeAccessed_WithNullExpirationDate_ReturnsTrue() // Assert Assert.True(result); } + + [Fact] + public async Task DeleteSendAsync_FileSend_DeletesFileBeforeDbRecord() + { + // Ensuring that the file is deleted first avoids the following situation: + // 1. DB row is deleted successfully + // 2. File blob fails to delete + // 3. File blob still exists but with no parent Send + var fileData = new SendFileData { Id = "file123", FileName = "test.txt", Size = 100 }; + var send = new Send + { + Id = Guid.NewGuid(), + Type = SendType.File, + Data = JsonSerializer.Serialize(fileData), + UserId = Guid.NewGuid() + }; + + var callOrder = new List(); + _sendFileStorageService.DeleteFileAsync(send, fileData.Id) + .Returns(Task.CompletedTask) + .AndDoes(_ => callOrder.Add("file")); + _sendRepository.DeleteAsync(send) + .Returns(Task.CompletedTask) + .AndDoes(_ => callOrder.Add("db")); + + await _nonAnonymousSendCommand.DeleteSendAsync(send); + + await _sendFileStorageService.Received(1).DeleteFileAsync(send, fileData.Id); + await _sendRepository.Received(1).DeleteAsync(send); + await _pushNotificationService.Received(1).PushSyncSendDeleteAsync(send); + Assert.Equal(new[] { "file", "db" }, callOrder); + } } diff --git a/util/Migrator/DbScripts/2026-04-06_01_AddSendReadByOrganizationId.sql b/util/Migrator/DbScripts/2026-04-06_01_AddSendReadByOrganizationId.sql new file mode 100644 index 000000000000..708f61461eea --- /dev/null +++ b/util/Migrator/DbScripts/2026-04-06_01_AddSendReadByOrganizationId.sql @@ -0,0 +1,13 @@ +CREATE OR ALTER PROCEDURE [dbo].[Send_ReadByOrganizationId] + @OrganizationId UNIQUEIDENTIFIER +AS +BEGIN + SET NOCOUNT ON + + SELECT + * + FROM + [dbo].[SendView] + WHERE + [OrganizationId] = @OrganizationId +END From a5dc9bae0bab8023526898824a8c0bd91d9caa69 Mon Sep 17 00:00:00 2001 From: John Harrington <84741727+harr1424@users.noreply.github.com> Date: Mon, 6 Apr 2026 10:17:22 -0700 Subject: [PATCH 4/6] extract org owned send deltion logic to private method --- .../OrganizationDeleteCommand.cs | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/src/Core/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommand.cs index 916e6948b86a..1a7400a78771 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommand.cs @@ -77,16 +77,8 @@ public async Task DeleteAsync(Organization organization) } } - var sends = await _sendRepository.GetManyByOrganizationIdAsync(organization.Id); - foreach (var send in sends.Where(s => s.Type == SendType.File)) - { - var data = send.Data != null ? JsonSerializer.Deserialize(send.Data) : null; - if (data?.Id != null) - { - await _sendFileStorageService.DeleteFileAsync(send, data.Id); - } - } + await DeleteOrganizationOwnedSendsAsync(organization); await _organizationRepository.DeleteAsync(organization); await _applicationCacheService.DeleteOrganizationAbilityAsync(organization.Id); } @@ -99,4 +91,17 @@ private async Task ValidateDeleteOrganizationAsync(Organization organization) throw new BadRequestException("You cannot delete an Organization that is using Key Connector."); } } + + private async Task DeleteOrganizationOwnedSendsAsync(Organization organization) + { + var sends = await _sendRepository.GetManyByOrganizationIdAsync(organization.Id); + foreach (var send in sends.Where(s => s.Type == SendType.File)) + { + var data = send.Data != null ? JsonSerializer.Deserialize(send.Data) : null; + if (data?.Id != null) + { + await _sendFileStorageService.DeleteFileAsync(send, data.Id); + } + } + } } From e66a9aae816ea860c2463f7f057e0af12203fafd Mon Sep 17 00:00:00 2001 From: John Harrington <84741727+harr1424@users.noreply.github.com> Date: Mon, 6 Apr 2026 15:30:42 -0700 Subject: [PATCH 5/6] add comments and rename helper methods for clarity --- .../OrganizationDeleteCommand.cs | 6 +++-- .../Services/Implementations/UserService.cs | 25 ++++++++++++------- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/src/Core/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommand.cs index 1a7400a78771..d7cdf7283ef2 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/Organizations/OrganizationDeleteCommand.cs @@ -78,7 +78,7 @@ public async Task DeleteAsync(Organization organization) } - await DeleteOrganizationOwnedSendsAsync(organization); + await DeleteOrganizationOwnedSendFilesAsync(organization); await _organizationRepository.DeleteAsync(organization); await _applicationCacheService.DeleteOrganizationAbilityAsync(organization.Id); } @@ -92,7 +92,9 @@ private async Task ValidateDeleteOrganizationAsync(Organization organization) } } - private async Task DeleteOrganizationOwnedSendsAsync(Organization organization) + // Only deletes Send Files + // Send objects are deleted via the call to _organizationRepository.DeleteAsync() + private async Task DeleteOrganizationOwnedSendFilesAsync(Organization organization) { var sends = await _sendRepository.GetManyByOrganizationIdAsync(organization.Id); foreach (var send in sends.Where(s => s.Type == SendType.File)) diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index c7969e53d760..8d812736235f 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -291,15 +291,7 @@ await _subscriberService.CancelSubscription( catch (BillingException) { } } - var sends = await _sendRepository.GetManyByUserIdAsync(user.Id); - foreach (var send in sends.Where(s => s.Type == SendType.File)) - { - var data = JsonSerializer.Deserialize(send.Data); - if (data?.Id != null) - { - await _sendFileStorageService.DeleteFileAsync(send, data.Id); - } - } + await DeleteSendFilesForUserAsync(user); await _userRepository.DeleteAsync(user); await _pushService.PushLogOutAsync(user.Id); @@ -1206,4 +1198,19 @@ private async Task SendAppropriateWelcomeEmailAsync(User user, string initiation await _mailService.SendWelcomeEmailAsync(user); } } + + // Only deletes Send Files + // Send objects are deleted via the call to _userRepository.DeleteAsync() + private async Task DeleteSendFilesForUserAsync(User user) + { + var sends = await _sendRepository.GetManyByUserIdAsync(user.Id); + foreach (var send in sends.Where(s => s.Type == SendType.File)) + { + var data = send.Data != null ? JsonSerializer.Deserialize(send.Data) : null; + if (data?.Id != null) + { + await _sendFileStorageService.DeleteFileAsync(send, data.Id); + } + } + } } From 361e984551ae736647e72718746a0943f4aced16 Mon Sep 17 00:00:00 2001 From: John Harrington <84741727+harr1424@users.noreply.github.com> Date: Mon, 6 Apr 2026 15:42:27 -0700 Subject: [PATCH 6/6] remove dead code --- .../Services/AzureSendFileStorageService.cs | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/src/Core/Tools/SendFeatures/Services/AzureSendFileStorageService.cs b/src/Core/Tools/SendFeatures/Services/AzureSendFileStorageService.cs index 5ca504643d47..63405b8cdf66 100644 --- a/src/Core/Tools/SendFeatures/Services/AzureSendFileStorageService.cs +++ b/src/Core/Tools/SendFeatures/Services/AzureSendFileStorageService.cs @@ -66,27 +66,11 @@ public async Task DeleteBlobAsync(string blobName) public async Task DeleteFilesForOrganizationAsync(Guid organizationId) { await InitAsync(); - await DeleteBlobsByMetadataAsync("organizationId", organizationId.ToString()); } public async Task DeleteFilesForUserAsync(Guid userId) { await InitAsync(); - await DeleteBlobsByMetadataAsync("userId", userId.ToString()); - } - - private async Task DeleteBlobsByMetadataAsync(string metadataKey, string metadataValue) - { - await foreach (var blobItem in _sendFilesContainerClient!.GetBlobsAsync(BlobTraits.Metadata)) - { - if (blobItem.Metadata != null && - blobItem.Metadata.TryGetValue(metadataKey, out var value) && - string.Equals(value, metadataValue, StringComparison.OrdinalIgnoreCase)) - { - var blob = _sendFilesContainerClient.GetBlobClient(blobItem.Name); - await blob.DeleteIfExistsAsync(); - } - } } public async Task GetSendFileDownloadUrlAsync(Send send, string fileId)