[PM-33501] Prevent orphaned Sends during user and org deletion

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,10 @@
 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;
 
 namespace Bit.Core.AdminConsole.OrganizationFeatures.Organizations;
@@ -19,6 +24,8 @@ 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<OrganizationDeleteCommand> _logger;
 
     public OrganizationDeleteCommand(
@@ -28,6 +35,8 @@ public OrganizationDeleteCommand(
         ISsoConfigRepository ssoConfigRepository,
         ISubscriberService subscriberService,
         IFeatureService featureService,
+        ISendRepository sendRepository,
+        ISendFileStorageService sendFileStorageService,
         ILogger<OrganizationDeleteCommand> logger)
     {
         _applicationCacheService = applicationCacheService;
@@ -36,6 +45,8 @@ public OrganizationDeleteCommand(
         _ssoConfigRepository = ssoConfigRepository;
         _subscriberService = subscriberService;
         _featureService = featureService;
+        _sendRepository = sendRepository;
+        _sendFileStorageService = sendFileStorageService;
         _logger = logger;
     }
 
@@ -66,6 +77,8 @@ public async Task DeleteAsync(Organization organization)
             }
         }
 
+
+        await DeleteOrganizationOwnedSendFilesAsync(organization);
         await _organizationRepository.DeleteAsync(organization);
         await _applicationCacheService.DeleteOrganizationAbilityAsync(organization.Id);
     }
@@ -78,4 +91,19 @@ private async Task ValidateDeleteOrganizationAsync(Organization organization)
             throw new BadRequestException("You cannot delete an Organization that is using Key Connector.");
         }
     }
+
+    // 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))
+        {
+            var data = send.Data != null ? JsonSerializer.Deserialize<SendFileData>(send.Data) : null;
+            if (data?.Id != null)
+            {
+                await _sendFileStorageService.DeleteFileAsync(send, data.Id);
+            }
+        }
+    }
 }

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 @@
     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 @@
         IPolicyRequirementQuery policyRequirementQuery,
         IPricingClient pricingClient,
         IHasPremiumAccessQuery hasPremiumAccessQuery,
-        ISubscriberService subscriberService)
+        ISubscriberService subscriberService,
+        ISendRepository sendRepository,
+        ISendFileStorageService sendFileStorageService)
         : base(
               store,
               optionsAccessor,
@@ -141,6 +149,8 @@
         _pricingClient = pricingClient;
         _hasPremiumAccessQuery = hasPremiumAccessQuery;
         _subscriberService = subscriberService;
+        _sendRepository = sendRepository;
+        _sendFileStorageService = sendFileStorageService;
     }
 
     public Guid? GetProperUserId(ClaimsPrincipal principal)
@@ -281,6 +291,8 @@
             catch (BillingException) { }
         }
 
+        await DeleteSendFilesForUserAsync(user);
+
         await _userRepository.DeleteAsync(user);
         await _pushService.PushLogOutAsync(user.Id);
         return IdentityResult.Success;
@@ -732,7 +744,7 @@
     {
         if (!CoreHelpers.FixedTimeEquals(
                 user.TwoFactorRecoveryCode,
                 recoveryCode.Replace(" ", string.Empty).Trim().ToLower()))
         {
             return false;
         }
@@ -1151,14 +1163,14 @@
 
     public async Task<bool> ActiveNewDeviceVerificationException(Guid userId)
     {
         var cacheKey = string.Format(AuthConstants.NewDeviceVerificationExceptionCacheKeyFormat, userId.ToString());
         var cacheValue = await _distributedCache.GetAsync(cacheKey);
         return cacheValue != null;
     }
 
     public async Task ToggleNewDeviceVerificationException(Guid userId)
     {
         var cacheKey = string.Format(AuthConstants.NewDeviceVerificationExceptionCacheKeyFormat, userId.ToString());
         var cacheValue = await _distributedCache.GetAsync(cacheKey);
         if (cacheValue != null)
         {
@@ -1186,4 +1198,19 @@
             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<SendFileData>(send.Data) : null;
+            if (data?.Id != null)
+            {
+                await _sendFileStorageService.DeleteFileAsync(send, data.Id);
+            }
+        }
+    }
 }

src/Core/Tools/Repositories/ISendRepository.cs

@@ -23,6 +23,18 @@ public interface ISendRepository : IRepository<Send, Guid>
     /// </returns>
     Task<ICollection<Send>> GetManyByUserIdAsync(Guid userId);
 
+    /// <summary>
+    /// Loads all <see cref="Send"/>s owned by an organization.
+    /// </summary>
+    /// <param name="organizationId">
+    /// Identifies the organization.
+    /// </param>
+    /// <returns>
+    /// A task that completes once the <see cref="Send"/>s have been loaded.
+    /// The task's result contains the loaded <see cref="Send"/>s.
+    /// </returns>
+    Task<ICollection<Send>> GetManyByOrganizationIdAsync(Guid organizationId);
+
     /// <summary>
     /// Loads <see cref="Send"/>s scheduled for deletion.
     /// </summary>

src/Core/Tools/SendFeatures/Commands/NonAnonymousSendCommand.cs

@@ -137,12 +137,15 @@ public async Task UploadFileToExistingSendAsync(Stream stream, Send send)
     }
     public async Task DeleteSendAsync(Send send)
     {
-        await _sendRepository.DeleteAsync(send);
-        if (send.Type == Enums.SendType.File)
+        if (send.Type == Enums.SendType.File && send.Data != null)
         {
             var data = JsonSerializer.Deserialize<SendFileData>(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);
     }
 

src/Infrastructure.Dapper/Tools/Repositories/SendRepository.cs

@@ -52,6 +52,22 @@ public async Task<ICollection<Send>> GetManyByUserIdAsync(Guid userId)
         }
     }
 
+    /// <inheritdoc />
+    public async Task<ICollection<Send>> GetManyByOrganizationIdAsync(Guid organizationId)
+    {
+        using (var connection = new SqlConnection(ConnectionString))
+        {
+            var results = await connection.QueryAsync<Send>(
+                $"[{Schema}].[Send_ReadByOrganizationId]",
+                new { OrganizationId = organizationId },
+                commandType: CommandType.StoredProcedure);
+
+            var sends = results.ToList();
+            UnprotectData(sends);
+            return sends;
+        }
+    }
+
     /// <inheritdoc />
     public async Task<ICollection<Send>> GetManyByDeletionDateAsync(DateTime deletionDateBefore)
     {

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();

src/Infrastructure.EntityFramework/Tools/Repositories/SendRepository.cs

@@ -71,6 +71,17 @@ public SendRepository(IServiceScopeFactory serviceScopeFactory, IMapper mapper)
         }
     }
 
+    /// <inheritdoc />
+    public async Task<ICollection<Core.Tools.Entities.Send>> 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<List<Core.Tools.Entities.Send>>(results);
+        }
+    }
+
     /// <inheritdoc />
     public UpdateEncryptedDataForKeyRotation UpdateForKeyRotation(Guid userId,
         IEnumerable<Core.Tools.Entities.Send> sends)

src/Sql/dbo/Stored Procedures/Organization_DeleteById.sql

@@ -151,6 +151,13 @@ BEGIN
     WHERE
         [OrganizationId] = @Id
 
+    -- Delete Organization Owned Sends
+    DELETE
+    FROM
+        [dbo].[Send]
+    WHERE
+        [OrganizationId] = @Id
+
     DELETE
     FROM
         [dbo].[Organization]

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

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<IOrganizationRepository>().Received(1).DeleteAsync(organization);
     }
+
+    [Theory, PaidOrganizationCustomize, BitAutoData]
+    public async Task Delete_WithFileSends_DeletesFilesBeforeDbRecords(
+        Organization organization,
+        SutProvider<OrganizationDeleteCommand> 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<ISendRepository>()
+            .GetManyByOrganizationIdAsync(organization.Id)
+            .Returns(new List<Send> { fileSend, textSend });
+
+        var callOrder = new List<string>();
+        sutProvider.GetDependency<ISendFileStorageService>()
+            .DeleteFileAsync(fileSend, fileData.Id)
+            .Returns(Task.CompletedTask)
+            .AndDoes(_ => callOrder.Add("file"));
+        sutProvider.GetDependency<IOrganizationRepository>()
+            .DeleteAsync(organization)
+            .Returns(Task.CompletedTask)
+            .AndDoes(_ => callOrder.Add("db"));
+
+        await sutProvider.Sut.DeleteAsync(organization);
+
+        await sutProvider.GetDependency<ISendFileStorageService>()
+            .Received(1).DeleteFileAsync(fileSend, fileData.Id);
+        await sutProvider.GetDependency<IOrganizationRepository>()
+            .Received(1).DeleteAsync(organization);
+        Assert.Equal(new[] { "file", "db" }, callOrder);
+    }
 }