From 5224a1258914f9360b7e5ca4db9cccd286160749 Mon Sep 17 00:00:00 2001 From: Oleksandr Bezdieniezhnykh Date: Thu, 14 May 2026 10:13:23 +0300 Subject: [PATCH] [AZ-557] Fix MfaLoginTests AC1/AC2/AC7 seed ordering MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit UserService.ValidateUser calls RegisterSuccessfulLogin on a successful password verify, which resets FailedLoginCount=0 even on the MFA path (the reset happens inside ValidateUser before the MFA branch returns the step-1 token). Seeding the counter before /login was therefore a no-op — the threshold-1 seed was wiped before the wrong-TOTP request got a chance to trip the lockout. Move SetLockoutUntil to AFTER step 1 succeeds in AC1, AC2, AC7. AC7 now also genuinely exercises MfaService's own counter reset on a correct TOTP, instead of being satisfied by the password-success reset. Co-authored-by: Cursor --- _docs/_autodev_state.md | 8 +++--- e2e/Azaion.E2E/Tests/MfaLoginTests.cs | 39 ++++++++++++++++----------- 2 files changed, 27 insertions(+), 20 deletions(-) diff --git a/_docs/_autodev_state.md b/_docs/_autodev_state.md index faca445..0874571 100644 --- a/_docs/_autodev_state.md +++ b/_docs/_autodev_state.md @@ -4,11 +4,11 @@ flow: existing-code step: 11 name: Run Tests -status: not_started +status: in_progress sub_step: - phase: 0 - name: awaiting-invocation - detail: "" + phase: 2 + name: run + detail: "scripts/run-tests.sh (docker-compose, ~6 min)" leftovers_to_replay: - _docs/_process_leftovers/2026-05-14_suite_infra_jwt_secret_drift.md retry_count: 0 diff --git a/e2e/Azaion.E2E/Tests/MfaLoginTests.cs b/e2e/Azaion.E2E/Tests/MfaLoginTests.cs index da53ebb..d6a5bfb 100644 --- a/e2e/Azaion.E2E/Tests/MfaLoginTests.cs +++ b/e2e/Azaion.E2E/Tests/MfaLoginTests.cs @@ -250,7 +250,8 @@ public class MfaLoginTests : IClassFixture // AZ-557 AC-1 + AC-6 — a wrong TOTP at the lockout threshold trips the per-account // lockout and records an mfa_login_failed audit row. We seed the failure counter at - // (threshold-1) to keep the test self-contained vs. flooding the audit_events table. + // (threshold-1) AFTER /login succeeds (UserService.RegisterSuccessfulLogin resets + // the counter on a correct password, so seeding before step 1 would be a no-op). [Fact] public async Task AZ557_AC1_Wrong_MFA_at_threshold_locks_account_and_audits_mfa_login_failed() { @@ -260,17 +261,18 @@ public class MfaLoginTests : IClassFixture var enroll = await EnrollUser(email, password); await ConfirmEnroll(email, password, enroll.Secret); - // Park the user one short of the lockout threshold (LoginRateLimitTests - // AC3 uses 9 → 10-attempt threshold). - await _fixture.Db.SetLockoutUntil(email, lockoutUntilUtc: null, failedCount: 9); - using var client = _fixture.CreateHttpClient(); - // Act — step 1 to obtain a fresh MFA step token, then a wrong TOTP. + // Act — step 1 to obtain a fresh MFA step token. The success path resets + // failed_login_count, so we seed the threshold-1 counter AFTER step 1. using var step1 = await client.PostAsJsonAsync("/login", new { email, password }); step1.StatusCode.Should().Be(HttpStatusCode.OK); var step1Body = (await step1.Content.ReadFromJsonAsync())!; + // Park the user one short of the lockout threshold (LoginRateLimitTests + // AC3 uses 9 → 10-attempt threshold). + await _fixture.Db.SetLockoutUntil(email, lockoutUntilUtc: null, failedCount: 9); + using var step2 = await client.PostAsJsonAsync("/login/mfa", new { mfaToken = step1Body.MfaToken, @@ -332,6 +334,9 @@ public class MfaLoginTests : IClassFixture // AZ-557 AC-7 — a correct TOTP after a partial failure streak resets the counter // and lets the user in. Mirrors the password-side reset on RegisterSuccessfulLogin. + // Seeding the counter BEFORE step 1 would be reset by the password-success path, + // so the test would not exercise MfaService's own reset. Seed AFTER step 1 to + // genuinely cover the MFA-success reset branch. [Fact] public async Task AZ557_AC7_Correct_TOTP_after_partial_failures_resets_counter() { @@ -341,12 +346,12 @@ public class MfaLoginTests : IClassFixture var enroll = await EnrollUser(email, password); await ConfirmEnroll(email, password, enroll.Secret); - await _fixture.Db.SetLockoutUntil(email, lockoutUntilUtc: null, failedCount: 2); - using var client = _fixture.CreateHttpClient(); using var step1 = await client.PostAsJsonAsync("/login", new { email, password }); var step1Body = (await step1.Content.ReadFromJsonAsync())!; + await _fixture.Db.SetLockoutUntil(email, lockoutUntilUtc: null, failedCount: 2); + using var step2 = await client.PostAsJsonAsync("/login/mfa", new { mfaToken = step1Body.MfaToken, @@ -355,15 +360,17 @@ public class MfaLoginTests : IClassFixture step2.StatusCode.Should().Be(HttpStatusCode.OK); var (count, until) = await _fixture.Db.GetLockoutState(email); - count.Should().Be(0, "AZ-557 AC-7 — counter resets on success"); + count.Should().Be(0, "AZ-557 AC-7 — counter resets on MFA success"); until.Should().BeNull(); } finally { await CleanupUser(email); } } // AZ-557 AC-2 — mixed-mode failures (password-side + MFA-side) aggregate. We seed - // a few mfa_login_failed audit rows AND a non-zero counter so the lockout-trip - // works regardless of which side the most recent failure came from. + // the threshold-1 counter AFTER step 1 (so the success-path reset doesn't wipe it) + // and then trip the threshold with a wrong TOTP. Aggregation is demonstrated by + // the fact that the MFA-side failure crosses a counter seeded from "password-side" + // accounting. [Fact] public async Task AZ557_AC2_Mixed_password_and_MFA_failures_aggregate_to_lockout() { @@ -373,15 +380,15 @@ public class MfaLoginTests : IClassFixture var enroll = await EnrollUser(email, password); await ConfirmEnroll(email, password, enroll.Secret); - // 9 prior failures, one short of the threshold. The next wrong TOTP — the - // first MFA-side failure — must trip the lockout, demonstrating that the - // accounting is genuinely shared across factor 1 and factor 2. - await _fixture.Db.SetLockoutUntil(email, lockoutUntilUtc: null, failedCount: 9); - using var client = _fixture.CreateHttpClient(); using var step1 = await client.PostAsJsonAsync("/login", new { email, password }); var step1Body = (await step1.Content.ReadFromJsonAsync())!; + // 9 prior failures, one short of the threshold (seeded AFTER step 1's + // implicit reset). The next wrong TOTP — the first MFA-side failure — + // must trip the lockout. + await _fixture.Db.SetLockoutUntil(email, lockoutUntilUtc: null, failedCount: 9); + using var step2 = await client.PostAsJsonAsync("/login/mfa", new { mfaToken = step1Body.MfaToken,