[PM-1198] Modify AuthRequest Purge Job (#3048)

* Add PasswordlessAuth Settings

* Update Repository Method to Take TimeSpan

* Update AuthRequest_DeleteIfExpired

- Take Configurable Expiration
- Add Special Cases for AdminApproval AuthRequests

* Add AuthRequestRepositoryTests

* Run Formatting

* Remove Comment

* Fix Bug in EF Repo

* Add Test Covering Expired Rejected AuthRequest

* Use Longer Param Names

* Use Longer Names in Test Helpers

src/Admin/Auth/Jobs/DeleteAuthRequestsJob.cs

@@ -1,6 +1,7 @@
 using Bit.Core;
 using Bit.Core.Jobs;
 using Bit.Core.Repositories;
+using Bit.Core.Settings;
 using Quartz;
 
 namespace Bit.Admin.Auth.Jobs;
@@ -8,20 +9,26 @@ namespace Bit.Admin.Auth.Jobs;
 public class DeleteAuthRequestsJob : BaseJob
 {
     private readonly IAuthRequestRepository _authRepo;
+    private readonly IGlobalSettings _globalSettings;
 
     public DeleteAuthRequestsJob(
         IAuthRequestRepository authrepo,
+        IGlobalSettings globalSettings,
         ILogger<DeleteAuthRequestsJob> logger)
         : base(logger)
     {
         _authRepo = authrepo;
+        _globalSettings = globalSettings;
     }
 
     protected async override Task ExecuteJobAsync(IJobExecutionContext context)
     {
         _logger.LogInformation(Constants.BypassFiltersEventId, "Execute job task: DeleteAuthRequestsJob: Start");
-        var count = await _authRepo.DeleteExpiredAsync();
+        var count = await _authRepo.DeleteExpiredAsync(
-        _logger.LogInformation(Constants.BypassFiltersEventId, $"{count} records deleted from AuthRequests.");
+            _globalSettings.PasswordlessAuth.UserRequestExpiration,
+            _globalSettings.PasswordlessAuth.AdminRequestExpiration,
+            _globalSettings.PasswordlessAuth.AfterAdminApprovalExpiration);
+        _logger.LogInformation(Constants.BypassFiltersEventId, "{Count} records deleted from AuthRequests.", count);
         _logger.LogInformation(Constants.BypassFiltersEventId, "Execute job task: DeleteAuthRequestsJob: End");
     }
 }

src/Core/Auth/Repositories/IAuthRequestRepository.cs

@@ -5,7 +5,7 @@ namespace Bit.Core.Repositories;
 
 public interface IAuthRequestRepository : IRepository<AuthRequest, Guid>
 {
-    Task<int> DeleteExpiredAsync();
+    Task<int> DeleteExpiredAsync(TimeSpan userRequestExpiration, TimeSpan adminRequestExpiration, TimeSpan afterAdminApprovalExpiration);
     Task<ICollection<AuthRequest>> GetManyByUserIdAsync(Guid userId);
     Task<ICollection<OrganizationAdminAuthRequest>> GetManyPendingByOrganizationIdAsync(Guid organizationId);
     Task<ICollection<OrganizationAdminAuthRequest>> GetManyAdminApprovalRequestsByManyIdsAsync(Guid organizationId, IEnumerable<Guid> ids);

src/Core/Auth/Settings/IPasswordlessAuthSettings.cs

@@ -3,4 +3,7 @@
 public interface IPasswordlessAuthSettings
 {
     bool KnownDevicesOnly { get; set; }
+    TimeSpan UserRequestExpiration { get; set; }
+    TimeSpan AdminRequestExpiration { get; set; }
+    TimeSpan AfterAdminApprovalExpiration { get; set; }
 }

src/Core/Settings/GlobalSettings.cs

@@ -534,6 +534,9 @@ public class GlobalSettings : IGlobalSettings
     public class PasswordlessAuthSettings : IPasswordlessAuthSettings
     {
         public bool KnownDevicesOnly { get; set; } = true;
+        public TimeSpan UserRequestExpiration { get; set; } = TimeSpan.FromMinutes(15);
+        public TimeSpan AdminRequestExpiration { get; set; } = TimeSpan.FromDays(7);
+        public TimeSpan AfterAdminApprovalExpiration { get; set; } = TimeSpan.FromHours(12);
     }
 
     public class DomainVerificationSettings : IDomainVerificationSettings

src/Infrastructure.Dapper/Auth/Repositories/AuthRequestRepository.cs

@@ -19,13 +19,19 @@ public class AuthRequestRepository : Repository<AuthRequest, Guid>, IAuthRequest
         : base(connectionString, readOnlyConnectionString)
     { }
 
-    public async Task<int> DeleteExpiredAsync()
+    public async Task<int> DeleteExpiredAsync(
+        TimeSpan userRequestExpiration, TimeSpan adminRequestExpiration, TimeSpan afterAdminApprovalExpiration)
     {
         using (var connection = new SqlConnection(ConnectionString))
         {
             return await connection.ExecuteAsync(
                 $"[{Schema}].[AuthRequest_DeleteIfExpired]",
-                null,
+                new
+                {
+                    UserExpirationSeconds = (int)userRequestExpiration.TotalSeconds,
+                    AdminExpirationSeconds = (int)adminRequestExpiration.TotalSeconds,
+                    AdminApprovalExpirationSeconds = (int)afterAdminApprovalExpiration.TotalSeconds,
+                },
                 commandType: CommandType.StoredProcedure);
         }
     }

src/Infrastructure.EntityFramework/Auth/Repositories/AuthRequestRepository.cs

@@ -15,15 +15,20 @@ public class AuthRequestRepository : Repository<Core.Auth.Entities.AuthRequest,
     public AuthRequestRepository(IServiceScopeFactory serviceScopeFactory, IMapper mapper)
         : base(serviceScopeFactory, mapper, (DatabaseContext context) => context.AuthRequests)
     { }
-    public async Task<int> DeleteExpiredAsync()
+    public async Task<int> DeleteExpiredAsync(
+        TimeSpan userRequestExpiration, TimeSpan adminRequestExpiration, TimeSpan afterAdminApprovalExpiration)
     {
+
         using (var scope = ServiceScopeFactory.CreateScope())
         {
             var dbContext = GetDatabaseContext(scope);
-            var expiredRequests = await dbContext.AuthRequests.Where(a => a.CreationDate < DateTime.Now.AddMinutes(-15)).ToListAsync();
+            var expiredRequests = await dbContext.AuthRequests
+                .Where(a => (a.Type != AuthRequestType.AdminApproval && a.CreationDate.AddSeconds(userRequestExpiration.TotalSeconds) < DateTime.UtcNow)
+                    || (a.Type == AuthRequestType.AdminApproval && a.Approved != true && a.CreationDate.AddSeconds(adminRequestExpiration.TotalSeconds) < DateTime.UtcNow)
+                    || (a.Type == AuthRequestType.AdminApproval && a.Approved == true && a.ResponseDate.Value.AddSeconds(afterAdminApprovalExpiration.TotalSeconds) < DateTime.UtcNow))
+                .ToListAsync();
             dbContext.AuthRequests.RemoveRange(expiredRequests);
-            await dbContext.SaveChangesAsync();
+            return await dbContext.SaveChangesAsync();
-            return 1;
         }
     }
 

src/Sql/Auth/dbo/Stored Procedures/AuthRequest_DeleteIfExpired.sql

@@ -1,6 +1,19 @@
-CREATE PROCEDURE [dbo].[AuthRequest_DeleteIfExpired]
+-- UserExpirationSeconds to 15 minutes (15 * 60)
+-- AdminExpirationSeconds to 7 days (7 * 24 * 60 * 60)
+-- AdminApprovalExpirationSeconds to 12 hour (12 * 60 * 60)
+
+CREATE PROCEDURE [dbo].[AuthRequest_DeleteIfExpired]
+    @UserExpirationSeconds INT = 900,
+    @AdminExpirationSeconds INT = 604800,
+    @AdminApprovalExpirationSeconds INT = 43200
 AS
 BEGIN
     SET NOCOUNT OFF
-    DELETE FROM [dbo].[AuthRequest] WHERE [CreationDate] < DATEADD(minute, -15, GETUTCDATE());
+    DELETE FROM [dbo].[AuthRequest]
+        -- User requests expire after 15 minutes (by default) of their creation
+    WHERE ([Type] != 2 AND DATEADD(second, @UserExpirationSeconds, [CreationDate]) < GETUTCDATE())
+        -- Admin requests expire after 7 days (by default) of their creation if they have not been approved
+        OR ([Type] = 2 AND ([Approved] IS NULL OR [Approved] = 0) AND DATEADD(second, @AdminExpirationSeconds,[CreationDate]) < GETUTCDATE())
+        -- Admin requests expire after 12 hours (by default) of their approval
+        OR ([Type] = 2 AND [Approved] = 1 AND DATEADD(second, @AdminApprovalExpirationSeconds, [ResponseDate]) < GETUTCDATE());
 END

test/Infrastructure.IntegrationTest/Auth/Repositories/AuthRequestRepositoryTests.cs

@@ -0,0 +1,99 @@
+using Bit.Core.Auth.Entities;
+using Bit.Core.Auth.Enums;
+using Bit.Core.Entities;
+using Bit.Core.Repositories;
+using Xunit;
+
+namespace Bit.Infrastructure.IntegrationTest.Auth.Repositories;
+
+public class AuthRequestRepositoryTests
+{
+    private readonly static TimeSpan _userRequestExpiration = TimeSpan.FromMinutes(15);
+    private readonly static TimeSpan _adminRequestExpiration = TimeSpan.FromDays(6);
+    private readonly static TimeSpan _afterAdminApprovalExpiration = TimeSpan.FromHours(12);
+
+    [DatabaseTheory, DatabaseData]
+    public async Task DeleteExpiredAsync_Works(
+        IAuthRequestRepository authRequestRepository,
+        IUserRepository userRepository,
+        ITestDatabaseHelper helper)
+    {
+        var user = await userRepository.CreateAsync(new User
+        {
+            Name = "Test User",
+            Email = $"test+{Guid.NewGuid()}@email.com",
+            ApiKey = "TEST",
+            SecurityStamp = "stamp",
+        });
+
+        // A user auth request type that has passed it's expiration time, should be deleted.
+        var userExpiredAuthRequest = await authRequestRepository.CreateAsync(
+            CreateAuthRequest(user.Id, AuthRequestType.AuthenticateAndUnlock, CreateExpiredDate(_userRequestExpiration)));
+
+        // An AdminApproval request that hasn't had any action taken on it and has passed it's expiration time, should be deleted.
+        var adminApprovalExpiredAuthRequest = await authRequestRepository.CreateAsync(
+            CreateAuthRequest(user.Id, AuthRequestType.AdminApproval, CreateExpiredDate(_adminRequestExpiration)));
+
+        // An AdminApproval request that was approved before it expired but the user has been approved for too long, should be deleted.
+        var adminApprovedExpiredAuthRequest = await authRequestRepository.CreateAsync(
+            CreateAuthRequest(user.Id, AuthRequestType.AdminApproval, DateTime.UtcNow.AddDays(-6), true, CreateExpiredDate(_afterAdminApprovalExpiration)));
+
+        // An AdminApproval request that was rejected within it's allowed lifetime but has no gone past it's expiration time, should be deleted.
+        var adminRejectedExpiredAuthRequest = await authRequestRepository.CreateAsync(
+            CreateAuthRequest(user.Id, AuthRequestType.AdminApproval, CreateExpiredDate(_adminRequestExpiration), false, DateTime.UtcNow.AddHours(-1)));
+
+        // A User AuthRequest that was created just a minute ago.
+        var notExpiredUserAuthRequest = await authRequestRepository.CreateAsync(
+            CreateAuthRequest(user.Id, AuthRequestType.Unlock, DateTime.UtcNow.AddMinutes(-1)));
+
+        // An AdminApproval AuthRequest that was create 6 days 23 hours 59 minutes 59 seconds ago which is right on the edge of still being valid
+        var notExpiredAdminApprovalRequest = await authRequestRepository.CreateAsync(
+            CreateAuthRequest(user.Id, AuthRequestType.AdminApproval, DateTime.UtcNow.Add(new TimeSpan(days: 6, hours: 23, minutes: 59, seconds: 59))));
+
+        // An AdminApproval AuthRequest that was created a week ago but just approved 11 hours ago.
+        var notExpiredApprovedAdminApprovalRequest = await authRequestRepository.CreateAsync(
+            CreateAuthRequest(user.Id, AuthRequestType.AdminApproval, DateTime.UtcNow.AddDays(7), true, DateTime.UtcNow.AddHours(11)));
+
+        helper.ClearTracker();
+
+        var numberOfDeleted = await authRequestRepository.DeleteExpiredAsync(_userRequestExpiration, _adminRequestExpiration, _afterAdminApprovalExpiration);
+
+        // Ensure all the AuthRequests that should have been deleted, have been deleted.
+        Assert.Null(await authRequestRepository.GetByIdAsync(userExpiredAuthRequest.Id));
+        Assert.Null(await authRequestRepository.GetByIdAsync(adminApprovalExpiredAuthRequest.Id));
+        Assert.Null(await authRequestRepository.GetByIdAsync(adminApprovedExpiredAuthRequest.Id));
+        Assert.Null(await authRequestRepository.GetByIdAsync(adminRejectedExpiredAuthRequest.Id));
+
+        // Ensure that all the AuthRequests that should have been left alone, were.
+        Assert.NotNull(await authRequestRepository.GetByIdAsync(notExpiredUserAuthRequest.Id));
+        Assert.NotNull(await authRequestRepository.GetByIdAsync(notExpiredAdminApprovalRequest.Id));
+        Assert.NotNull(await authRequestRepository.GetByIdAsync(notExpiredApprovedAdminApprovalRequest.Id));
+
+        // Ensure the repository responds with the amount of items it deleted and it deleted the right amount.
+        // NOTE: On local development this might fail on it's first run because the developer could have expired AuthRequests
+        // on their machine but aren't running the job that would delete them. The second run of this test should succeed.
+        Assert.Equal(4, numberOfDeleted);
+    }
+
+    private static AuthRequest CreateAuthRequest(Guid userId, AuthRequestType authRequestType, DateTime creationDate, bool? approved = null, DateTime? responseDate = null)
+    {
+        return new AuthRequest
+        {
+            UserId = userId,
+            Type = authRequestType,
+            Approved = approved,
+            RequestDeviceIdentifier = "something", // TODO: EF Doesn't enforce this as not null
+            RequestIpAddress = "1.1.1.1", // TODO: EF Doesn't enforce this as not null
+            AccessCode = "test_access_code", // TODO: EF Doesn't enforce this as not null
+            PublicKey = "test_public_key", // TODO: EF Doesn't enforce this as not null
+            CreationDate = creationDate,
+            ResponseDate = responseDate,
+        };
+    }
+
+    private static DateTime CreateExpiredDate(TimeSpan expirationPeriod)
+    {
+        var exp = expirationPeriod + TimeSpan.FromMinutes(1);
+        return DateTime.UtcNow.Add(exp.Negate());
+    }
+}

util/Migrator/DbScripts/2023-06-27_00_AuthRequestExpirationUpdates.sql

@@ -0,0 +1,25 @@
+IF OBJECT_ID('[dbo].[AuthRequest_DeleteIfExpired]') IS NOT NULL
+    BEGIN
+        DROP PROCEDURE [dbo].[AuthRequest_DeleteIfExpired]
+    END
+GO
+
+-- UserExpirationSeconds to 15 minutes (15 * 60)
+-- AdminExpirationSeconds to 7 days (7 * 24 * 60 * 60)
+-- AdminApprovalExpirationSeconds to 12 hour (12 * 60 * 60)
+
+CREATE PROCEDURE [dbo].[AuthRequest_DeleteIfExpired]
+    @UserExpirationSeconds INT = 900,
+    @AdminExpirationSeconds INT = 604800,
+    @AdminApprovalExpirationSeconds INT = 43200
+AS
+BEGIN
+    SET NOCOUNT OFF
+    DELETE FROM [dbo].[AuthRequest]
+        -- User requests expire after 15 minutes (by default) of their creation
+    WHERE ([Type] != 2 AND DATEADD(second, @UserExpirationSeconds, [CreationDate]) < GETUTCDATE())
+        -- Admin requests expire after 7 days (by default) of their creation if they have not been approved
+        OR ([Type] = 2 AND ([Approved] IS NULL OR [Approved] = 0) AND DATEADD(second, @AdminExpirationSeconds,[CreationDate]) < GETUTCDATE())
+        -- Admin requests expire after 12 hours (by default) of their approval
+        OR ([Type] = 2 AND [Approved] = 1 AND DATEADD(second, @AdminApprovalExpirationSeconds, [ResponseDate]) < GETUTCDATE());
+END