diff --git a/code/backend/Cleanuparr.Api.Tests/Features/Auth/AuthControllerTests.cs b/code/backend/Cleanuparr.Api.Tests/Features/Auth/AuthControllerTests.cs index 0bdcfe7d..f41daa63 100644 --- a/code/backend/Cleanuparr.Api.Tests/Features/Auth/AuthControllerTests.cs +++ b/code/backend/Cleanuparr.Api.Tests/Features/Auth/AuthControllerTests.cs @@ -178,6 +178,61 @@ public class AuthControllerTests : IClassFixture response.StatusCode.ShouldBe(HttpStatusCode.OK); } + [Fact, TestPriority(11)] + public async Task Setup_2FAGenerate_AfterCompletion_IsBlocked() + { + var response = await _client.PostAsJsonAsync("/api/auth/setup/2fa/generate", new { }); + + // Blocked by middleware (403) or controller defense-in-depth (409) + new[] { HttpStatusCode.Forbidden, HttpStatusCode.Conflict } + .ShouldContain(response.StatusCode); + } + + [Fact, TestPriority(12)] + public async Task Setup_PlexPin_AfterCompletion_IsBlocked() + { + var response = await _client.PostAsync("/api/auth/setup/plex/pin", null); + + // Blocked by middleware (403) or controller defense-in-depth (409) + new[] { HttpStatusCode.Forbidden, HttpStatusCode.Conflict } + .ShouldContain(response.StatusCode); + } + + [Fact, TestPriority(13)] + public async Task Setup_Complete_AfterCompletion_IsBlocked() + { + var response = await _client.PostAsJsonAsync("/api/auth/setup/complete", new { }); + + // Blocked by middleware (403) or controller defense-in-depth (409) + new[] { HttpStatusCode.Forbidden, HttpStatusCode.Conflict } + .ShouldContain(response.StatusCode); + } + + [Fact, TestPriority(14)] + public async Task Login_NotBlockedByMiddleware_AfterSetupEndpointsBlocked() + { + var response = await _client.PostAsJsonAsync("/api/auth/login", new + { + username = "admin", + password = "TestPassword123!" + }); + + // Login endpoint must NOT be blocked by the middleware (403). + // It may return OK (200) or TooManyRequests (429) due to brute force lockout from earlier tests. + response.StatusCode.ShouldNotBe(HttpStatusCode.Forbidden); + } + + [Fact, TestPriority(15)] + public async Task AuthStatus_StillWorks_AfterSetupEndpointsBlocked() + { + var response = await _client.GetAsync("/api/auth/status"); + + response.StatusCode.ShouldBe(HttpStatusCode.OK); + + var body = await response.Content.ReadFromJsonAsync(); + body.GetProperty("setupCompleted").GetBoolean().ShouldBeTrue(); + } + #region TOTP helpers private static string _totpSecret = ""; diff --git a/code/backend/Cleanuparr.Api/Features/Auth/Controllers/AuthController.cs b/code/backend/Cleanuparr.Api/Features/Auth/Controllers/AuthController.cs index 02eaa9f4..d7bf1d45 100644 --- a/code/backend/Cleanuparr.Api/Features/Auth/Controllers/AuthController.cs +++ b/code/backend/Cleanuparr.Api/Features/Auth/Controllers/AuthController.cs @@ -119,9 +119,9 @@ public sealed class AuthController : ControllerBase return BadRequest(new { error = "Create an account first" }); } - if (user.SetupCompleted && user.TotpEnabled) + if (user.SetupCompleted) { - return Conflict(new { error = "2FA is already configured" }); + return Conflict(new { error = "Setup already completed. Use account settings to manage 2FA." }); } // Generate new TOTP secret @@ -176,6 +176,11 @@ public sealed class AuthController : ControllerBase return BadRequest(new { error = "Create an account first" }); } + if (user.SetupCompleted) + { + return Conflict(new { error = "Setup already completed. Use account settings to manage 2FA." }); + } + if (string.IsNullOrEmpty(user.TotpSecret)) { return BadRequest(new { error = "Generate 2FA setup first" }); @@ -212,6 +217,11 @@ public sealed class AuthController : ControllerBase return BadRequest(new { error = "Create an account first" }); } + if (user.SetupCompleted) + { + return Conflict(new { error = "Setup already completed" }); + } + user.SetupCompleted = true; user.UpdatedAt = DateTime.UtcNow; await _usersContext.SaveChangesAsync(); @@ -382,6 +392,11 @@ public sealed class AuthController : ControllerBase return BadRequest(new { error = "Create an account first" }); } + if (user.SetupCompleted) + { + return Conflict(new { error = "Setup already completed. Use account settings to manage Plex." }); + } + var pin = await _plexAuthService.RequestPin(); return Ok(new PlexPinStatusResponse @@ -412,6 +427,11 @@ public sealed class AuthController : ControllerBase return BadRequest(new { error = "Create an account first" }); } + if (user.SetupCompleted) + { + return Conflict(new { error = "Setup already completed. Use account settings to manage Plex." }); + } + user.PlexAccountId = plexAccount.AccountId; user.PlexUsername = plexAccount.Username; user.PlexEmail = plexAccount.Email; @@ -472,7 +492,10 @@ public sealed class AuthController : ControllerBase return Unauthorized(new { error = "Plex account does not match the linked account" }); } - // Plex login bypasses 2FA + // Plex OAuth acts as a trusted identity provider — the user explicitly linked their + // Plex account during setup or via account settings (both require authentication). + // Since Plex login verifies the exact same Plex account ID that was linked, + // 2FA is not required for Plex login. _logger.LogInformation("User {Username} logged in via Plex", user.Username); var tokenResponse = await GenerateTokenResponse(user); diff --git a/code/backend/Cleanuparr.Api/Middleware/SetupGuardMiddleware.cs b/code/backend/Cleanuparr.Api/Middleware/SetupGuardMiddleware.cs index 96dad4b3..9715af74 100644 --- a/code/backend/Cleanuparr.Api/Middleware/SetupGuardMiddleware.cs +++ b/code/backend/Cleanuparr.Api/Middleware/SetupGuardMiddleware.cs @@ -15,52 +15,75 @@ public class SetupGuardMiddleware public async Task InvokeAsync(HttpContext context) { - // Fast path: setup already completed + string path = context.Request.Path.Value?.ToLowerInvariant() ?? ""; + + // Always allow health checks and non-API paths (static files, SPA, etc.) + if (path.StartsWith("/health") || !path.StartsWith("/api/")) + { + await _next(context); + return; + } + + // Setup-only paths (/api/auth/setup/*) require setup to NOT be complete + if (IsSetupOnlyPath(path)) + { + if (await IsSetupCompleted()) + { + context.Response.StatusCode = StatusCodes.Status403Forbidden; + context.Response.ContentType = "application/json"; + await context.Response.WriteAsJsonAsync(new { error = "Setup already completed" }); + return; + } + + await _next(context); + return; + } + + // Non-setup auth paths (login, refresh, logout, status) are always allowed + if (path.StartsWith("/api/auth/") || path == "/api/auth") + { + await _next(context); + return; + } + + // All other API paths require setup to be complete + if (!await IsSetupCompleted()) + { + context.Response.StatusCode = StatusCodes.Status403Forbidden; + context.Response.ContentType = "application/json"; + await context.Response.WriteAsJsonAsync(new { error = "Setup required" }); + return; + } + + await _next(context); + } + + public void ResetSetupState() + { + _setupCompleted = false; + } + + private async Task IsSetupCompleted() + { if (_setupCompleted) { - await _next(context); - return; + return true; } - var path = context.Request.Path.Value?.ToLowerInvariant() ?? ""; - - // Always allow these paths regardless of setup state - if (IsAllowedPath(path)) - { - await _next(context); - return; - } - - // Check database for setup completion await using var usersContext = UsersContext.CreateStaticInstance(); var user = await usersContext.Users.AsNoTracking().FirstOrDefaultAsync(); if (user is { SetupCompleted: true }) { _setupCompleted = true; - await _next(context); - return; + return true; } - // Setup not complete - block non-auth requests - context.Response.StatusCode = StatusCodes.Status403Forbidden; - context.Response.ContentType = "application/json"; - await context.Response.WriteAsJsonAsync(new { error = "Setup required" }); + return false; } - /// - /// Resets the cached setup state. Call this if the user database is reset. - /// - public void ResetSetupState() + private static bool IsSetupOnlyPath(string path) { - _setupCompleted = false; - } - - private static bool IsAllowedPath(string path) - { - return path.StartsWith("/api/auth/") - || path == "/api/auth" - || path.StartsWith("/health") - || !path.StartsWith("/api/"); + return path.StartsWith("/api/auth/setup/") || path == "/api/auth/setup"; } }