fix(auth): prevent 2FA secret overwrite, fix recovery code login, improve rate-limit UX

Author: Akash29g

DocAnalytics.Api.Tests/Controllers/AuthControllerTests.cs

@@ -163,7 +163,7 @@ public async Task SetupTwoFactor_returns_200_with_setup_payload()
         var payload = new TwoFactorSetupResponse("SECRET", "otpauth://totp/x", "SECR ET");
         var userId = Guid.NewGuid();
         var auth = new Mock<IAuthService>();
-        auth.Setup(a => a.SetupTwoFactorAsync(userId, It.IsAny<CancellationToken>())).ReturnsAsync(payload);
+        auth.Setup(a => a.SetupTwoFactorAsync(userId, It.IsAny<CancellationToken>())).ReturnsAsync(((string?)null, payload));
         var currentUser = new Mock<ICurrentUser>();
         currentUser.SetupGet(c => c.UserId).Returns(userId);
 
@@ -173,6 +173,21 @@ public async Task SetupTwoFactor_returns_200_with_setup_payload()
         Assert.Equal("SECRET", Assert.IsType<ApiResponse<TwoFactorSetupResponse>>(ok.Value).Data!.Secret);
     }
 
+    [Fact]
+    public async Task SetupTwoFactor_returns_400_when_already_enabled()
+    {
+        var auth = new Mock<IAuthService>();
+        auth.Setup(a => a.SetupTwoFactorAsync(It.IsAny<Guid>(), It.IsAny<CancellationToken>()))
+            .ReturnsAsync(("Two-factor authentication is already enabled. Disable it first to re-configure.", (TwoFactorSetupResponse?)null));
+
+        var result = await NewController(auth.Object, Mock.Of<ICurrentUser>(), Mock.Of<ILoginLockoutService>())
+            .SetupTwoFactor(default);
+
+        var bad = Assert.IsType<BadRequestObjectResult>(result);
+        Assert.Equal("TWO_FACTOR_ALREADY_ENABLED", Assert.IsType<ApiResponse<object>>(bad.Value).Error!.Code);
+    }
+
+
     [Fact]
     public async Task ConfirmTwoFactor_returns_400_on_error()
     {

DocAnalytics.Api/Controllers/AuthController.cs

@@ -194,10 +194,14 @@ public async Task<IActionResult> LoginTwoFactor([FromBody] TwoFactorLoginRequest
     [HttpPost("2fa/setup")]
     public async Task<IActionResult> SetupTwoFactor(CancellationToken ct)
     {
-        var result = await _auth.SetupTwoFactorAsync(_currentUser.UserId, ct);
-        return Ok(ApiResponse<TwoFactorSetupResponse>.Ok(result));
+        var (error, result) = await _auth.SetupTwoFactorAsync(_currentUser.UserId, ct);
+        if (error is not null)
+            return BadRequest(ApiResponse<object>.Fail("TWO_FACTOR_ALREADY_ENABLED", error));
+
+        return Ok(ApiResponse<TwoFactorSetupResponse>.Ok(result!));
     }
 
+
     /// <summary>Confirms 2FA setup with a valid code: enables 2FA, returns one-time recovery codes.</summary>
     [Authorize]
     [HttpPost("2fa/confirm")]

DocAnalytics.Api/appsettings.json

@@ -63,6 +63,11 @@
       "PermitLimit": 3,
       "WindowSeconds": 60,
       "QueueLimit": 0
+    },
+    "Mfa": {
+      "PermitLimit": 5,
+      "WindowSeconds": 300,
+      "QueueLimit": 0
     }
   }
 

DocAnalytics.Service.Tests/Auth/AuthServiceTests.cs

@@ -183,23 +183,25 @@ public async Task SetupTwoFactorAsync_stores_an_encrypted_secret_and_returns_set
         var user = ActiveUser("a@org.com", "pw");
         var sut = NewSut(Ctx(new[] { user }, Array.Empty<UserSiteAccess>(), Array.Empty<Site>()));
 
-        var result = await sut.SetupTwoFactorAsync(user.Id, default);
+        var (error, result) = await sut.SetupTwoFactorAsync(user.Id, default);
 
-        Assert.False(string.IsNullOrWhiteSpace(result.Secret));
+        Assert.Null(error);
+        Assert.False(string.IsNullOrWhiteSpace(result!.Secret));
         Assert.StartsWith("otpauth://totp/", result.OtpAuthUri);
         Assert.NotNull(user.TwoFactorSecret);
         Assert.NotEqual(result.Secret, user.TwoFactorSecret); // stored value is encrypted, not plaintext
     }
 
+
     [Fact]
     public async Task ConfirmTwoFactorAsync_enables_2fa_and_returns_recovery_codes_on_valid_code()
     {
         var twoFactor = new TwoFactorService();
         var user = ActiveUser("a@org.com", "pw");
         var sut = NewSut(Ctx(new[] { user }, Array.Empty<UserSiteAccess>(), Array.Empty<Site>()), twoFactor: twoFactor);
 
-        var setup = await sut.SetupTwoFactorAsync(user.Id, default);
-        var code = new OtpNet.Totp(OtpNet.Base32Encoding.ToBytes(setup.Secret)).ComputeTotp();
+        var (setupError, setup) = await sut.SetupTwoFactorAsync(user.Id, default);
+        var code = new OtpNet.Totp(OtpNet.Base32Encoding.ToBytes(setup!.Secret)).ComputeTotp();
 
         var (error, result) = await sut.ConfirmTwoFactorAsync(user.Id, code, default);
 

DocAnalytics.Service.Tests/Auth/AuthServiceTwoFactorTests.cs

@@ -199,14 +199,30 @@ public async Task SetupTwoFactorAsync_StoresEncryptedSecret_AndReturnsSetupPaylo
             .Returns(("SECRETBASE32", "otpauth://totp/...", "SECR ETBA SE32"));
         var sut = NewSut(db);
 
-        var response = await sut.SetupTwoFactorAsync(user.Id, CancellationToken.None);
+        var (error, response) = await sut.SetupTwoFactorAsync(user.Id, CancellationToken.None);
 
-        Assert.Equal("SECRETBASE32", response.Secret);
+        Assert.Null(error);
+        Assert.Equal("SECRETBASE32", response!.Secret);
         var stored = await db.Users.SingleAsync();
         Assert.NotNull(stored.TwoFactorSecret);
         Assert.NotEqual("SECRETBASE32", stored.TwoFactorSecret); // must be encrypted
     }
 
+    [Fact]
+    public async Task SetupTwoFactorAsync_ReturnsError_WhenTwoFactorAlreadyEnabled()
+    {
+        using var db = NewDb();
+        var user = NewUser(db, twoFactorEnabled: true);
+        var sut = NewSut(db);
+
+        var (error, response) = await sut.SetupTwoFactorAsync(user.Id, CancellationToken.None);
+
+        Assert.NotNull(error);
+        Assert.Null(response);
+        _twoFactor.Verify(t => t.GenerateSetup(It.IsAny<string>(), It.IsAny<string>()), Times.Never);
+    }
+
+
     [Fact]
     public async Task ConfirmTwoFactorAsync_EnablesTwoFactor_AndIssuesRecoveryCodes_OnValidCode()
     {

DocAnalytics.Service/Auth/AuthService.cs

@@ -86,18 +86,25 @@ public AuthService(
     }
 
     /// <inheritdoc />
-    public async Task<TwoFactorSetupResponse> SetupTwoFactorAsync(Guid userId, CancellationToken ct)
+    public async Task<(string? Error, TwoFactorSetupResponse? Result)> SetupTwoFactorAsync(Guid userId, CancellationToken ct)
     {
         var user = await _db.Users.FirstAsync(u => u.Id == userId, ct);
+
+        if (user.TwoFactorEnabled)
+        {
+            return ("Two-factor authentication is already enabled. Disable it first to re-configure.", null);
+        }
+
         var (secret, uri, manualKey) = _twoFactor.GenerateSetup(user.Email);
 
         // Store encrypted immediately so /confirm can validate against it; NOT enabled until confirmed.
         user.TwoFactorSecret = _protector.Protect(secret);
         await _db.SaveChangesAsync(ct);
 
-        return new TwoFactorSetupResponse(secret, uri, manualKey);
+        return (null, new TwoFactorSetupResponse(secret, uri, manualKey));
     }
 
+
     /// <inheritdoc />
     public async Task<(string? Error, TwoFactorConfirmResponse? Result)> ConfirmTwoFactorAsync(Guid userId, string code, CancellationToken ct)
     {

DocAnalytics.Service/Auth/IAuthService.cs

@@ -32,7 +32,8 @@ public interface IAuthService
 
 
     /// <summary>Starts 2FA setup for an authenticated user: generates + stores an encrypted secret, returns the QR payload.</summary>
-    Task<TwoFactorSetupResponse> SetupTwoFactorAsync(Guid userId, CancellationToken ct);
+    Task<(string? Error, TwoFactorSetupResponse? Result)> SetupTwoFactorAsync(Guid userId, CancellationToken ct);
+
 
     /// <summary>Confirms 2FA setup with a valid TOTP code: flips TwoFactorEnabled, returns one-time recovery codes.</summary>
     Task<(string? Error, TwoFactorConfirmResponse? Result)> ConfirmTwoFactorAsync(Guid userId, string code, CancellationToken ct);

docanalytics-web/src/app/features/auth/login.component.ts

@@ -45,6 +45,8 @@ export class LoginComponent {
   }
 
   submit(): void {
+    if (this.loading()) return; // ⬅️ guard against double/rapid submits
+
     this.errorMessage.set(null);
 
     if (this.form.invalid) {
@@ -70,7 +72,6 @@ export class LoginComponent {
           return;
         }
 
-        // Forced first-login password reset takes priority over everything.
         if (data.must_change_password) {
           this.router.navigate(['/change-password']);
           return;
@@ -81,10 +82,11 @@ export class LoginComponent {
           this.auth.logout();
         }
       },
-      // Login 401 is handled HERE locally (not via the global "Session expired" toast).
       error: (err: HttpErrorResponse) => {
         this.loading.set(false);
-        if (err.status === 401) {
+        if (err.status === 429) {
+          this.errorMessage.set(this.rateLimitMessage(err));
+        } else if (err.status === 401) {
           this.errorMessage.set('Invalid email or password.');
         } else if (err.status === 0) {
           this.errorMessage.set('Cannot reach the server. Check your connection and try again.');
@@ -118,13 +120,28 @@ export class LoginComponent {
           this.auth.logout();
         }
       },
-      error: () => {
+      error: (err: HttpErrorResponse) => {
         this.mfaLoading.set(false);
-        this.mfaError.set('Invalid or expired code. Try again or use a recovery code.');
+        if (err.status === 429) {
+          this.mfaError.set(this.rateLimitMessage(err));
+        } else {
+          this.mfaError.set('Invalid or expired code. Try again or use a recovery code.');
+        }
       },
     });
   }
 
+  /** Builds a friendly rate-limit message, using Retry-After header if the server sends one. */
+  private rateLimitMessage(err: HttpErrorResponse): string {
+    const retryAfter = err.headers?.get?.('Retry-After');
+    const seconds = retryAfter ? parseInt(retryAfter, 10) : null;
+    if (seconds && !isNaN(seconds) && seconds > 0) {
+      return `Too many attempts. Please wait ${seconds}s and try again.`;
+    }
+    return 'Too many attempts. Please wait a moment and try again.';
+  }
+
+
   backToCredentials(): void {
     this.step.set('credentials');
     this.challengeToken.set(null);

docanalytics-web/src/app/features/security/two-factor-setup.component.ts

@@ -34,6 +34,10 @@ export class TwoFactorSetupComponent {
   private startSetup(): void {
     this.auth.setupTwoFactor().subscribe({
       next: async (res) => {
+        if (res.error) {
+          this.error.set(res.error.message);
+          return;
+        }
         if (!res.data) {
           this.error.set('Could not start 2FA setup.');
           return;
@@ -47,6 +51,7 @@ export class TwoFactorSetupComponent {
     });
   }
 
+
   protected confirm(): void {
     if (this.code().length !== 6) return;
     this.loading.set(true);

perf-results/perf-report.csv

@@ -1,7 +1,7 @@
 operation,samples,p50_ms,p90_ms,max_ms
-batch_list_page,10,0.9,3.2,23.3
-concurrent_dashboard_summary,10,1.1,12.9,23.9
-concurrent_total_wall_time,1,53.7,53.7,53.7
-dashboard_summary,10,1.4,21.8,103.0
-error_list_page,10,197.1,318.3,497.7
-recent_failures_page,10,62.9,189.4,230.4
+batch_list_page,10,0.8,3.0,30.8
+concurrent_dashboard_summary,10,1.1,12.7,25.1
+concurrent_total_wall_time,1,54.4,54.4,54.4
+dashboard_summary,10,1.2,21.8,101.1
+error_list_page,10,196.3,306.2,530.3
+recent_failures_page,10,58.8,191.4,228.5