Fix process time difference between valid and invalid user login (#503)

code/backend/Cleanuparr.Api.Tests/CustomWebApplicationFactory.cs

@@ -1,20 +1,17 @@
-using Cleanuparr.Persistence;
+using Cleanuparr.Shared.Helpers;
 using Microsoft.AspNetCore.Hosting;
 using Microsoft.AspNetCore.Mvc.Testing;
-using Microsoft.EntityFrameworkCore;
-using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Hosting;
 using Xunit;
 
-// Integration tests share file-system state (config-dir users.db used by SetupGuardMiddleware),
-// so we must run them sequentially to avoid interference between factories.
+// Integration tests share file-system state (config-dir used by SetupGuardMiddleware),
+// so they must be run sequentially to avoid interference between factories.
 [assembly: CollectionBehavior(DisableTestParallelization = true)]
 
 namespace Cleanuparr.Api.Tests;
 
 /// <summary>
-/// Custom WebApplicationFactory that uses an isolated SQLite database for each test fixture.
-/// The database file is created in a temp directory so both DI and static contexts share the same data.
+/// Custom WebApplicationFactory that redirects all database contexts to an isolated temp directory
 /// </summary>
 public class CustomWebApplicationFactory : WebApplicationFactory<Program>
 {
@@ -24,6 +21,8 @@ public class CustomWebApplicationFactory : WebApplicationFactory<Program>
     {
         _tempDir = Path.Combine(Path.GetTempPath(), $"cleanuparr-test-{Guid.NewGuid():N}");
         Directory.CreateDirectory(_tempDir);
+
+        ConfigurationPathProvider.SetConfigPath(_tempDir);
     }
 
     protected override void ConfigureWebHost(IWebHostBuilder builder)
@@ -32,35 +31,12 @@ public class CustomWebApplicationFactory : WebApplicationFactory<Program>
 
         builder.ConfigureServices(services =>
         {
-            // Remove the existing UsersContext registration
-            var descriptor = services.SingleOrDefault(d => d.ServiceType == typeof(DbContextOptions<UsersContext>));
-            if (descriptor != null) services.Remove(descriptor);
-
-            // Also remove the DbContext registration itself
-            var contextDescriptor = services.SingleOrDefault(d => d.ServiceType == typeof(UsersContext));
-            if (contextDescriptor != null) services.Remove(contextDescriptor);
-
-            var dbPath = Path.Combine(_tempDir, "users.db");
-
-            services.AddDbContext<UsersContext>(options =>
-            {
-                options.UseSqlite($"Data Source={dbPath}");
-            });
-
             // Remove all hosted services (Quartz scheduler, BackgroundJobManager) to prevent
             // Quartz.Logging.LogProvider.ResolvedLogProvider (a cached Lazy<T>) from being accessed
-            // with a disposed ILoggerFactory from the previous factory lifecycle.
-            // Auth tests don't depend on background job scheduling, so this is safe.
             foreach (var hostedService in services.Where(d => d.ServiceType == typeof(IHostedService)).ToList())
+            {
                 services.Remove(hostedService);
-
-            // Ensure DB is created using a minimal isolated context (not the full app DI container)
-            // to avoid any residual static state contamination.
-            using var db = new UsersContext(
-                new DbContextOptionsBuilder<UsersContext>()
-                    .UseSqlite($"Data Source={dbPath}")
-                    .Options);
-            db.Database.EnsureCreated();
+            }
         });
     }
 

code/backend/Cleanuparr.Api.Tests/Features/Auth/AccountControllerOidcTests.cs

@@ -10,7 +10,6 @@ using Microsoft.AspNetCore.Hosting;
 using Microsoft.AspNetCore.Mvc.Testing;
 using Microsoft.EntityFrameworkCore;
 using Microsoft.Extensions.DependencyInjection;
-using Microsoft.Extensions.Hosting;
 using Shouldly;
 
 namespace Cleanuparr.Api.Tests.Features.Auth;
@@ -281,76 +280,18 @@ public class AccountControllerOidcTests : IClassFixture<AccountControllerOidcTes
 
     #region Test Infrastructure
 
-    public class OidcLinkWebApplicationFactory : WebApplicationFactory<Program>
+    public class OidcLinkWebApplicationFactory : CustomWebApplicationFactory
     {
-        private readonly string _configDir;
-
-        public OidcLinkWebApplicationFactory()
-        {
-            _configDir = Cleanuparr.Shared.Helpers.ConfigurationPathProvider.GetConfigPath();
-
-            // Delete both databases (and their WAL sidecar files) so each test run starts clean.
-            // users.db must be at the config path so CreateStaticInstance() (used by
-            // SetupGuardMiddleware) reads the same file as the DI-injected UsersContext.
-            // ClearAllPools() releases any SQLite connections held by the previous test factory's
-            // connection pool, which would otherwise prevent file deletion on Windows or cause
-            // SQLite to reconstruct a partial database from stale WAL files on other platforms.
-            Microsoft.Data.Sqlite.SqliteConnection.ClearAllPools();
-            foreach (var name in new[] {
-                "users.db", "users.db-shm", "users.db-wal",
-                "cleanuparr.db", "cleanuparr.db-shm", "cleanuparr.db-wal" })
-            {
-                var path = Path.Combine(_configDir, name);
-                if (File.Exists(path))
-                    try { File.Delete(path); } catch { /* best effort */ }
-            }
-        }
-
         protected override void ConfigureWebHost(IWebHostBuilder builder)
         {
-            builder.UseEnvironment("Testing");
+            base.ConfigureWebHost(builder);
 
             builder.ConfigureServices(services =>
             {
-                var descriptor = services.SingleOrDefault(d => d.ServiceType == typeof(DbContextOptions<UsersContext>));
-                if (descriptor != null) services.Remove(descriptor);
-
-                var contextDescriptor = services.SingleOrDefault(d => d.ServiceType == typeof(UsersContext));
-                if (contextDescriptor != null) services.Remove(contextDescriptor);
-
-                // Use the config-path db so SetupGuardMiddleware (CreateStaticInstance) sees
-                // the same data as the DI-injected context. Apply the same naming conventions
-                // as CreateStaticInstance() so the schemas match.
-                var dbPath = Path.Combine(_configDir, "users.db");
-                services.AddDbContext<UsersContext>(options =>
-                {
-                    options
-                        .UseSqlite($"Data Source={dbPath}")
-                        .UseLowerCaseNamingConvention()
-                        .UseSnakeCaseNamingConvention();
-                });
-
                 var oidcDescriptor = services.SingleOrDefault(d => d.ServiceType == typeof(IOidcAuthService));
                 if (oidcDescriptor != null) services.Remove(oidcDescriptor);
 
                 services.AddSingleton<IOidcAuthService, MockOidcAuthService>();
-
-                // Remove all hosted services (Quartz scheduler, BackgroundJobManager) to prevent
-                // Quartz.Logging.LogProvider.ResolvedLogProvider (a cached Lazy<T>) from being accessed
-                // with a disposed ILoggerFactory from the previous factory lifecycle.
-                // Auth tests don't depend on background job scheduling, so this is safe.
-                foreach (var hostedService in services.Where(d => d.ServiceType == typeof(IHostedService)).ToList())
-                    services.Remove(hostedService);
-
-                // Ensure DB is created using a minimal isolated context (not the full app DI container)
-                // to avoid any residual static state contamination.
-                using var db = new UsersContext(
-                    new DbContextOptionsBuilder<UsersContext>()
-                        .UseSqlite($"Data Source={dbPath}")
-                        .UseLowerCaseNamingConvention()
-                        .UseSnakeCaseNamingConvention()
-                        .Options);
-                db.Database.EnsureCreated();
             });
         }
 
@@ -410,22 +351,6 @@ public class AccountControllerOidcTests : IClassFixture<AccountControllerOidcTes
             return user?.Oidc.ExclusiveMode ?? false;
         }
 
-        protected override void Dispose(bool disposing)
-        {
-            base.Dispose(disposing);
-            if (disposing)
-            {
-                Microsoft.Data.Sqlite.SqliteConnection.ClearAllPools();
-                foreach (var name in new[] {
-                    "users.db", "users.db-shm", "users.db-wal",
-                    "cleanuparr.db", "cleanuparr.db-shm", "cleanuparr.db-wal" })
-                {
-                    var path = Path.Combine(_configDir, name);
-                    if (File.Exists(path))
-                        try { File.Delete(path); } catch { /* best effort */ }
-                }
-            }
-        }
     }
 
     private sealed class MockOidcAuthService : IOidcAuthService

code/backend/Cleanuparr.Api.Tests/Features/Auth/LoginTimingTests.cs

@@ -0,0 +1,169 @@
+using System.Diagnostics;
+using System.Net;
+using System.Net.Http.Json;
+using Shouldly;
+
+namespace Cleanuparr.Api.Tests.Features.Auth;
+
+/// <summary>
+/// Tests that the login endpoint always runs BCrypt verification regardless of
+/// username validity, preventing timing-based username enumeration.
+/// </summary>
+[Collection("Login Timing Tests")]
+[TestCaseOrderer("Cleanuparr.Api.Tests.PriorityOrderer", "Cleanuparr.Api.Tests")]
+public class LoginTimingTests : IClassFixture<TimingTestWebApplicationFactory>
+{
+    private readonly HttpClient _client;
+    private readonly TimingTestWebApplicationFactory _factory;
+
+    public LoginTimingTests(TimingTestWebApplicationFactory factory)
+    {
+        _factory = factory;
+        _client = factory.CreateClient();
+    }
+
+    [Fact, TestPriority(0)]
+    public async Task Login_NoUserExists_StillCallsPasswordVerification()
+    {
+        _factory.TrackingPasswordService.Reset();
+
+        var response = await _client.PostAsJsonAsync("/api/auth/login", new
+        {
+            username = "nouser",
+            password = "SomePassword123!"
+        });
+
+        response.StatusCode.ShouldBe(HttpStatusCode.Unauthorized);
+        _factory.TrackingPasswordService.VerifyPasswordCallCount.ShouldBeGreaterThanOrEqualTo(1);
+    }
+
+    [Fact, TestPriority(1)]
+    public async Task Setup_CreateAccountAndComplete()
+    {
+        var createResponse = await _client.PostAsJsonAsync("/api/auth/setup/account", new
+        {
+            username = "timingtest",
+            password = "TimingTestPassword123!"
+        });
+        createResponse.StatusCode.ShouldBe(HttpStatusCode.Created);
+
+        var completeResponse = await _client.PostAsJsonAsync("/api/auth/setup/complete", new { });
+        completeResponse.StatusCode.ShouldBe(HttpStatusCode.OK);
+    }
+
+    [Fact, TestPriority(2)]
+    public async Task Login_ValidUsername_CallsPasswordVerification()
+    {
+        _factory.TrackingPasswordService.Reset();
+
+        await _client.PostAsJsonAsync("/api/auth/login", new
+        {
+            username = "timingtest",
+            password = "TimingTestPassword123!"
+        });
+
+        _factory.TrackingPasswordService.VerifyPasswordCallCount.ShouldBeGreaterThanOrEqualTo(1);
+    }
+
+    [Fact, TestPriority(3)]
+    public async Task Login_NonexistentUsername_StillCallsPasswordVerification()
+    {
+        _factory.TrackingPasswordService.Reset();
+
+        var response = await _client.PostAsJsonAsync("/api/auth/login", new
+        {
+            username = "doesnotexist",
+            password = "SomePassword123!"
+        });
+
+        response.StatusCode.ShouldBe(HttpStatusCode.Unauthorized);
+        _factory.TrackingPasswordService.VerifyPasswordCallCount.ShouldBeGreaterThanOrEqualTo(1);
+    }
+
+    [Fact, TestPriority(4)]
+    public async Task Login_LockedOutUser_StillCallsPasswordVerification()
+    {
+        // Trigger lockout by making several failed login attempts
+        for (var i = 0; i < 5; i++)
+        {
+            await _client.PostAsJsonAsync("/api/auth/login", new
+            {
+                username = "timingtest",
+                password = "WrongPassword!"
+            });
+        }
+
+        _factory.TrackingPasswordService.Reset();
+
+        var response = await _client.PostAsJsonAsync("/api/auth/login", new
+        {
+            username = "timingtest",
+            password = "WrongPassword!"
+        });
+
+        response.StatusCode.ShouldBe(HttpStatusCode.TooManyRequests);
+        _factory.TrackingPasswordService.VerifyPasswordCallCount.ShouldBeGreaterThanOrEqualTo(1);
+    }
+
+    [Fact, TestPriority(5)]
+    public async Task Login_TimingConsistency_InvalidAndValidUsernamesTakeSimilarTime()
+    {
+        const int iterations = 10;
+
+        // Warm up the server and BCrypt static init
+        await _client.PostAsJsonAsync("/api/auth/login", new
+        {
+            username = "warmup",
+            password = "WarmupPassword123!"
+        });
+
+        var invalidTimings = new List<long>(iterations);
+        var validTimings = new List<long>(iterations);
+
+        for (var i = 0; i < iterations; i++)
+        {
+            // Alternate to avoid ordering bias
+            var invalidSw = Stopwatch.StartNew();
+            await _client.PostAsJsonAsync("/api/auth/login", new
+            {
+                username = $"nonexistent_{i}",
+                password = "SomePassword123!"
+            });
+            invalidSw.Stop();
+            invalidTimings.Add(invalidSw.ElapsedMilliseconds);
+
+            var validSw = Stopwatch.StartNew();
+            await _client.PostAsJsonAsync("/api/auth/login", new
+            {
+                username = "timingtest",
+                password = "WrongPasswordForTiming!"
+            });
+            validSw.Stop();
+            validTimings.Add(validSw.ElapsedMilliseconds);
+        }
+
+        var invalidMedian = Median(invalidTimings);
+        var validMedian = Median(validTimings);
+
+        // The invalid-username path must not be suspiciously fast
+        invalidMedian.ShouldBeGreaterThan(50,
+            $"Non-existent username median too fast ({invalidMedian}ms) — BCrypt may have been skipped");
+
+        // Medians should be in the same ballpark
+        var ratio = invalidMedian > validMedian
+            ? (double)invalidMedian / validMedian
+            : (double)validMedian / invalidMedian;
+
+        ratio.ShouldBeLessThan(3.0,
+            $"Timing difference too large: invalid median={invalidMedian}ms, valid median={validMedian}ms (ratio={ratio:F1}x)");
+    }
+
+    private static long Median(List<long> values)
+    {
+        values.Sort();
+        var mid = values.Count / 2;
+        return values.Count % 2 == 0
+            ? (values[mid - 1] + values[mid]) / 2
+            : values[mid];
+    }
+}

code/backend/Cleanuparr.Api.Tests/Features/Auth/OidcAuthControllerTests.cs

@@ -4,6 +4,7 @@ using System.Text.Json;
 using Cleanuparr.Infrastructure.Features.Auth;
 using Cleanuparr.Persistence;
 using Cleanuparr.Persistence.Models.Auth;
+using Cleanuparr.Shared.Helpers;
 using Microsoft.AspNetCore.Hosting;
 using Microsoft.AspNetCore.Mvc.Testing;
 using Microsoft.EntityFrameworkCore;
@@ -451,16 +452,8 @@ public class OidcAuthControllerTests : IClassFixture<OidcAuthControllerTests.Oid
             _tempDir = Path.Combine(Path.GetTempPath(), $"cleanuparr-oidc-test-{Guid.NewGuid():N}");
             Directory.CreateDirectory(_tempDir);
 
-            // Clean up any existing DataContext DB from previous test runs.
-            // DataContext.CreateStaticInstance() uses ConfigurationPathProvider which
-            // resolves to {AppContext.BaseDirectory}/config/cleanuparr.db.
-            // We need to ensure a clean state for our tests.
-            var configDir = Cleanuparr.Shared.Helpers.ConfigurationPathProvider.GetConfigPath();
-            var dataDbPath = Path.Combine(configDir, "cleanuparr.db");
-            if (File.Exists(dataDbPath))
-            {
-                try { File.Delete(dataDbPath); } catch { /* best effort */ }
-            }
+            // Redirect all database contexts to this factory's temp directory.
+            ConfigurationPathProvider.SetConfigPath(_tempDir);
         }
 
         protected override void ConfigureWebHost(IWebHostBuilder builder)
@@ -469,19 +462,6 @@ public class OidcAuthControllerTests : IClassFixture<OidcAuthControllerTests.Oid
 
             builder.ConfigureServices(services =>
             {
-                // Remove existing UsersContext registration
-                var descriptor = services.SingleOrDefault(d => d.ServiceType == typeof(DbContextOptions<UsersContext>));
-                if (descriptor != null) services.Remove(descriptor);
-
-                var contextDescriptor = services.SingleOrDefault(d => d.ServiceType == typeof(UsersContext));
-                if (contextDescriptor != null) services.Remove(contextDescriptor);
-
-                var dbPath = Path.Combine(_tempDir, "users.db");
-                services.AddDbContext<UsersContext>(options =>
-                {
-                    options.UseSqlite($"Data Source={dbPath}");
-                });
-
                 // Replace IOidcAuthService with mock
                 var oidcDescriptor = services.SingleOrDefault(d => d.ServiceType == typeof(IOidcAuthService));
                 if (oidcDescriptor != null) services.Remove(oidcDescriptor);
@@ -494,14 +474,6 @@ public class OidcAuthControllerTests : IClassFixture<OidcAuthControllerTests.Oid
                 // Auth tests don't depend on background job scheduling, so this is safe.
                 foreach (var hostedService in services.Where(d => d.ServiceType == typeof(IHostedService)).ToList())
                     services.Remove(hostedService);
-
-                // Ensure DB is created using a minimal isolated context (not the full app DI container)
-                // to avoid any residual static state contamination.
-                using var db = new UsersContext(
-                    new DbContextOptionsBuilder<UsersContext>()
-                        .UseSqlite($"Data Source={dbPath}")
-                        .Options);
-                db.Database.EnsureCreated();
             });
         }
 

code/backend/Cleanuparr.Api.Tests/Features/Auth/TimingTestWebApplicationFactory.cs

@@ -0,0 +1,29 @@
+using Cleanuparr.Infrastructure.Features.Auth;
+using Microsoft.AspNetCore.Hosting;
+using Microsoft.Extensions.DependencyInjection;
+
+namespace Cleanuparr.Api.Tests.Features.Auth;
+
+/// <summary>
+/// Factory variant that replaces <see cref="IPasswordService"/> with a
+/// <see cref="TrackingPasswordService"/> spy so tests can assert that
+/// password verification is always called regardless of username validity.
+/// </summary>
+public class TimingTestWebApplicationFactory : CustomWebApplicationFactory
+{
+    public TrackingPasswordService TrackingPasswordService { get; } = new();
+
+    protected override void ConfigureWebHost(IWebHostBuilder builder)
+    {
+        base.ConfigureWebHost(builder);
+
+        builder.ConfigureServices(services =>
+        {
+            // Replace IPasswordService with our tracking spy
+            var descriptor = services.SingleOrDefault(d => d.ServiceType == typeof(IPasswordService));
+            if (descriptor != null) services.Remove(descriptor);
+
+            services.AddSingleton<IPasswordService>(TrackingPasswordService);
+        });
+    }
+}

code/backend/Cleanuparr.Api.Tests/Features/Auth/TrackingPasswordService.cs

@@ -0,0 +1,33 @@
+using Cleanuparr.Infrastructure.Features.Auth;
+
+namespace Cleanuparr.Api.Tests.Features.Auth;
+
+/// <summary>
+/// Spy wrapper around <see cref="PasswordService"/> that tracks calls to
+/// <see cref="VerifyPassword"/> for behavioral assertions in timing tests.
+/// </summary>
+public sealed class TrackingPasswordService : IPasswordService
+{
+    private readonly PasswordService _inner = new();
+    private int _verifyPasswordCallCount;
+
+    public int VerifyPasswordCallCount => _verifyPasswordCallCount;
+    
+    public string DummyHash => _inner.DummyHash;
+
+    public string HashPassword(string password)
+    {
+        return _inner.HashPassword(password);
+    }
+
+    public bool VerifyPassword(string password, string hash)
+    {
+        Interlocked.Increment(ref _verifyPasswordCallCount);
+        return _inner.VerifyPassword(password, hash);
+    }
+
+    public void Reset()
+    {
+        Interlocked.Exchange(ref _verifyPasswordCallCount, 0);
+    }
+}

code/backend/Cleanuparr.Api/Features/Auth/Controllers/AuthController.cs

@@ -264,6 +264,11 @@ public sealed class AuthController : ControllerBase
 
         var user = await _usersContext.Users.AsNoTracking().FirstOrDefaultAsync();
 
+        // Always verify the submitted password to prevent timing-based username enumeration
+        var userHasPassword = user?.PasswordHash is not null;
+        var passwordHash = user?.PasswordHash ?? _passwordService.DummyHash;
+        var passwordValid = _passwordService.VerifyPassword(request.Password, passwordHash) && userHasPassword;
+
         if (user is null || !user.SetupCompleted)
         {
             return Unauthorized(new { error = "Invalid credentials" });
@@ -272,12 +277,11 @@ public sealed class AuthController : ControllerBase
         // Check lockout
         if (user.LockoutEnd.HasValue && user.LockoutEnd.Value > DateTime.UtcNow)
         {
-            var remaining = (int)(user.LockoutEnd.Value - DateTime.UtcNow).TotalSeconds;
+            var remaining = (int)Math.Ceiling((user.LockoutEnd.Value - DateTime.UtcNow).TotalSeconds);
             return StatusCode(429, new { error = "Account is locked", retryAfterSeconds = remaining });
         }
 
-        if (!_passwordService.VerifyPassword(request.Password, user.PasswordHash) ||
-            !string.Equals(user.Username, request.Username, StringComparison.OrdinalIgnoreCase))
+        if (!passwordValid || !string.Equals(user.Username, request.Username, StringComparison.OrdinalIgnoreCase))
         {
             var retryAfterSeconds = await IncrementFailedAttempts(user.Id);
             return Unauthorized(new { error = "Invalid credentials", retryAfterSeconds });

code/backend/Cleanuparr.Infrastructure/Features/Auth/IPasswordService.cs

@@ -2,6 +2,9 @@ namespace Cleanuparr.Infrastructure.Features.Auth;
 
 public interface IPasswordService
 {
+    string DummyHash { get; }
+    
     string HashPassword(string password);
+    
     bool VerifyPassword(string password, string hash);
 }

code/backend/Cleanuparr.Infrastructure/Features/Auth/PasswordService.cs

@@ -4,6 +4,11 @@ public sealed class PasswordService : IPasswordService
 {
     private const int WorkFactor = 12;
 
+    /// <summary>
+    /// Pre-computed BCrypt hash with a work factor of 12 used as a fallback when no user exists
+    /// </summary>
+    public string DummyHash => "$2a$12$tQw4MgGGq7WTFro3Me4mQOekctJ0mIOYmFMn.XEmEbyZhBq0i4qKy";
+
     public string HashPassword(string password)
     {
         return BCrypt.Net.BCrypt.HashPassword(password, WorkFactor);

code/backend/Cleanuparr.Shared/Helpers/ConfigurationPathProvider.cs

@@ -48,4 +48,19 @@ public static class ConfigurationPathProvider
     {
         return _configPath ?? InitializeConfigPath();
     }
+
+    public static void SetConfigPath(string path)
+    {
+        if (string.IsNullOrWhiteSpace(path))
+        {
+            throw new ArgumentException("Value cannot be null or whitespace.", nameof(path));
+        }
+        
+        if (!Directory.Exists(path))
+        {
+            Directory.CreateDirectory(path);
+        }
+
+        _configPath = path;
+    }
 }