diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index f238881b..1683281f 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -4733,7 +4733,7 @@ Retiring the tree costs the engine nothing operationally: **`tests/test_ech_egre ## 1021. The MFA enrollment confirm verifies the activating TOTP through a bool wrapper that discards the step, so it is never consumed (ASVS 6.5.1) -> ๐Ÿ”ข **Filed 2026-08-04 โ€” not started.** Value **6/10** ยท Difficulty **4/10** ยท _quick win_. `confirm_mfa_enrollment` verifies the enrolling code with `totp.verify_totp`, a documented thin bool wrapper that computes the matched time-step and then collapses it to a bool, so the step cannot be recorded. `enable_totp` leaves `last_totp_step` NULL, and `consume_totp_step` rejects only when `last is not None and last >= step` โ€” so on first deployment the activating code would remain usable on the login path for the remainder of its own step. +> โœ… **Fixed 2026-08-06 โ€” enrollment now consumes the activating TOTP step (`verify_totp_step` + `consume_totp_step`), mirroring the login path.** Value **6/10** ยท Difficulty **4/10** ยท _quick win_. `confirm_mfa_enrollment` proved the enrolling code through the `totp.verify_totp` bool wrapper, which computed the matched time-step then collapsed it to a bool, so the step was never recorded; with `last_totp_step` left NULL by `enable_totp`, the activating code would have remained usable on the login path for the remainder of its own step on first deployment. The confirm site now takes the matched step from `verify_totp_step` and requires `consume_totp_step` before minting recovery codes / `enable_totp`, so the step is single-use (ASVS 6.5.1) and enable stays atomic. **Cluster:** Security / authentication. **Priority:** P2. **Verdict:** build (small). **Severity:** would leave a narrow second-factor replay window at enrollment on first deployment โ€” bounded, not a bypass. diff --git a/messagefoundry/auth/service.py b/messagefoundry/auth/service.py index 54a6c2b1..4b1cb607 100644 --- a/messagefoundry/auth/service.py +++ b/messagefoundry/auth/service.py @@ -2004,7 +2004,8 @@ async def confirm_mfa_enrollment( """Confirm a staged enrollment by proving a live TOTP code. On success: activate MFA, mint the single-use recovery codes (returned **once**, plaintext, for the user to save), mark the current session MFA-verified, audit + notify. Returns the recovery codes, or ``None`` when the - code was wrong. Raises :class:`ValueError` if no enrollment is staged / the user isn't local.""" + code was wrong or its time-step was already consumed (single-use, BACKLOG #1021). Raises + :class:`ValueError` if no enrollment is staged / the user isn't local.""" user = await self._store.get_user(identity.user_id) if user is None or user.auth_provider != AuthProvider.LOCAL.value: raise ValueError("only local users can enroll a TOTP authenticator") @@ -2014,8 +2015,19 @@ async def confirm_mfa_enrollment( # Verify the enrollment proof under the SAME configured clock-skew window as a login (BACKLOG # #187): default 0 = strict current-step only. Enrolling under the same window a login uses # avoids the trap of a skewed-clock authenticator that confirms enrollment yet then fails every - # login (the mismatch surfaces at enroll time instead). - if not totp.verify_totp(secret, code.strip(), window=self._settings.totp_skew_steps): + # login (the mismatch surfaces at enroll time instead). The matched step is then CONSUMED + # (BACKLOG #1021), single-use per ASVS 6.5.1, mirroring _verify_second_factor's verify-then- + # consume so the activating code can't be replayed on POST /auth/mfa-verify inside its step + # window (verify_totp alone discarded the step, which would leave a second-factor replay window + # at enrollment on first deployment). Consume BEFORE minting recovery codes / enable_totp keeps + # enable atomic: a step that no longer advances the high-water mark fails on the same + # phase=enroll branch and MFA is not enabled. + matched_step = totp.verify_totp_step( + secret, code.strip(), window=self._settings.totp_skew_steps + ) + if matched_step is None or not await self._store.consume_totp_step( + identity.user_id, matched_step + ): await self._audit( "auth.mfa_failed", actor=identity.username, diff --git a/packaging/messagefoundry-webconsole/tests/test_ui_mfa_gate.py b/packaging/messagefoundry-webconsole/tests/test_ui_mfa_gate.py index e7d0dda3..8cc8e593 100644 --- a/packaging/messagefoundry-webconsole/tests/test_ui_mfa_gate.py +++ b/packaging/messagefoundry-webconsole/tests/test_ui_mfa_gate.py @@ -13,6 +13,7 @@ from __future__ import annotations import httpx +import pytest from messagefoundry.api import create_app from messagefoundry.auth import Role, totp @@ -23,6 +24,29 @@ PW = "a-strong-test-passphrase" # >=15, no app/vendor terms โ€” satisfies the ASVS policy (WP-3) +class _PinnedClock: + """Minimal ``time`` stand-in exposing only ``time()`` at a fixed instant. + + A local twin of ``tests/_totp_clock._PinnedClock`` โ€” this package has its own test root and does + not import the engine suite's helpers. ``totp`` reads the wall clock solely as ``time.time()``, so + swapping the module reference pins the TOTP step deterministically while leaving every other clock + real. + """ + + def __init__(self, instant: float) -> None: + self._instant = instant + + def time(self) -> float: + return self._instant + + +def _pin_totp_clock(monkeypatch: pytest.MonkeyPatch, instant: float) -> None: + """Pin the ``totp`` module clock so an enroll ceremony and a later /ui/mfa gate verify land in + distinct, provably-adjacent steps โ€” needed now that enrollment consumes the activating step + (BACKLOG #1021), so a gate code from the SAME step would be refused as a replay.""" + monkeypatch.setattr(totp, "time", _PinnedClock(instant)) + + async def _service(engine: Engine, **kw: object) -> AuthService: service = AuthService(engine.store, AuthSettings(login_rate_limit_enabled=False, **kw)) # type: ignore[arg-type] await service.initialize() @@ -185,7 +209,9 @@ async def test_the_page_asks_for_a_code_once_a_factor_exists(engine: Engine) -> assert 'name="password"' not in r.text -async def test_a_satisfied_session_is_bounced_off_the_page(engine: Engine) -> None: +async def test_a_satisfied_session_is_bounced_off_the_page( + engine: Engine, monkeypatch: pytest.MonkeyPatch +) -> None: """RED when: the mfa_satisfied early-return is removed from GET /ui/mfa. Without it a verified operator who navigates back to /ui/mfa is asked to re-verify forever. @@ -193,22 +219,36 @@ async def test_a_satisfied_session_is_bounced_off_the_page(engine: Engine) -> No service = await _service(engine) await _add(service, "op", Role.OPERATOR) async with _client(engine, service) as c: + # Enroll consumes the activating TOTP step (BACKLOG #1021), so the gate code must sit in a + # strictly later step: pin the enroll ceremony to t0 and the gate verify to t0+period. + t0 = 1_000_000.0 + _pin_totp_clock(monkeypatch, t0) secret = await _enroll_totp(service) await _login(c) - assert (await c.post("/ui/mfa", data={"code": totp.totp(secret)})).status_code == 303 + t1 = t0 + totp.DEFAULT_PERIOD + _pin_totp_clock(monkeypatch, t1) + gate = await c.post("/ui/mfa", data={"code": totp.totp(secret, now=t1)}) + assert gate.status_code == 303 r = await c.get("/ui/mfa") assert r.status_code == 303 and r.headers["location"] == "/ui" -async def test_a_valid_code_clears_the_gate(engine: Engine) -> None: +async def test_a_valid_code_clears_the_gate( + engine: Engine, monkeypatch: pytest.MonkeyPatch +) -> None: """RED when: POST /ui/mfa stops calling verify_mfa (or stops redirecting on success).""" service = await _service(engine) await _add(service, "op", Role.OPERATOR) async with _client(engine, service) as c: + # Enroll consumes the activating step (BACKLOG #1021); the gate code lives in a later step. + t0 = 1_000_000.0 + _pin_totp_clock(monkeypatch, t0) secret = await _enroll_totp(service) await _login(c) assert (await c.get("/ui/messages")).status_code == 303 # confined - r = await c.post("/ui/mfa", data={"code": totp.totp(secret)}) + t1 = t0 + totp.DEFAULT_PERIOD + _pin_totp_clock(monkeypatch, t1) + r = await c.post("/ui/mfa", data={"code": totp.totp(secret, now=t1)}) assert r.status_code == 303 and r.headers["location"] == "/ui" assert (await c.get("/ui/messages")).status_code == 200 # released diff --git a/tests/_totp_clock.py b/tests/_totp_clock.py index 08dd588c..7c75af6d 100644 --- a/tests/_totp_clock.py +++ b/tests/_totp_clock.py @@ -31,9 +31,11 @@ import time +import pytest + from messagefoundry.auth import totp -__all__ = ["fresh_totp", "step_remaining"] +__all__ = ["fresh_totp", "pin_totp_clock", "step_remaining"] #: Seconds that must remain in the current step before a code is generated. Comfortably larger than #: any plausible generate โ†’ verify gap (a store read plus an HMAC; the argon2id recovery-code path @@ -65,3 +67,33 @@ def fresh_totp( # keeps a coarse clock from returning the step we just left. time.sleep(remaining + 0.05) return totp.totp(secret, period=period) + + +class _PinnedClock: + """A minimal stand-in for the ``time`` module exposing only ``time()`` at a fixed instant. + + ``totp`` reads the wall clock solely as ``time.time()`` (two call sites: :func:`totp.totp` and + :func:`totp.verify_totp_step`), so swapping the module reference for this pins the TOTP step + deterministically while leaving every OTHER clock โ€” the service's session-expiry, rate-limiting + and audit timestamps, all on their own ``time`` imports โ€” real. That surgical scope is what lets + a test place enrollment and a later verify in DISTINCT, provably-adjacent steps without the 30 s + boundary flake :func:`fresh_totp` can only narrow. + """ + + def __init__(self, instant: float) -> None: + self._instant = instant + + def time(self) -> float: + return self._instant + + +def pin_totp_clock(monkeypatch: pytest.MonkeyPatch, instant: float) -> None: + """Pin the ``totp`` module's clock to ``instant`` for the rest of the test (monkeypatch restores + the real clock at teardown). + + Generate codes with ``totp.totp(secret, now=instant)`` so they land in the same step the pinned + service-side verify computes. Unlike :func:`fresh_totp` โ€” which guarantees headroom WITHIN a step + but cannot advance one โ€” this can place two verifies in strictly different steps, which is needed + now that enrollment consumes the activating step (BACKLOG #1021): a later login code must live in + a higher step than the consumed enrollment step to be accepted.""" + monkeypatch.setattr(totp, "time", _PinnedClock(instant)) diff --git a/tests/test_admin_new_ip.py b/tests/test_admin_new_ip.py index 1fd49aab..e4874a04 100644 --- a/tests/test_admin_new_ip.py +++ b/tests/test_admin_new_ip.py @@ -17,10 +17,10 @@ import httpx import pytest -from _totp_clock import fresh_totp +from _totp_clock import pin_totp_clock from messagefoundry.api import create_app -from messagefoundry.auth import Role +from messagefoundry.auth import Role, totp from messagefoundry.auth.identity import Identity from messagefoundry.auth.notifications import ADMIN_NEW_IP, SecurityEvent from messagefoundry.auth.service import AuthService @@ -196,7 +196,9 @@ async def test_loopback_addresses_treated_as_same_host() -> None: await store.close() -async def test_verify_mfa_reanchors_session_to_the_new_ip() -> None: +async def test_verify_mfa_reanchors_session_to_the_new_ip( + monkeypatch: pytest.MonkeyPatch, +) -> None: """Completing the second factor (TOTP) from a new address re-anchors the session, like reauth โ€” so an MFA-required admin who roamed clears the new-IP signal with one credential proof, not two.""" store = await MessageStore.open(":memory:") @@ -208,13 +210,21 @@ async def test_verify_mfa_reanchors_session_to_the_new_ip() -> None: assert out.ok and out.identity is not None and out.token is not None identity, token = out.identity, out.token enroll = await service.begin_mfa_enrollment(identity) + # Pin the TOTP clock so the enrollment confirm and the later verify_mfa sit in distinct steps + # (enrollment now consumes the activating step, BACKLOG #1021). + t0 = 1_000_000.0 + pin_totp_clock(monkeypatch, t0) await service.confirm_mfa_enrollment( - identity, fresh_totp(enroll.secret), token=token, client="10.1.1.1" + identity, totp.totp(enroll.secret, now=t0), token=token, client="10.1.1.1" ) # Roam to a new address โ†’ flagged. assert await service.flag_new_client_ip(token, "10.2.2.2", path="/users") is True - # Completing MFA from the new address re-anchors the session (parity with reauth). - assert await service.verify_mfa(token, fresh_totp(enroll.secret), client="10.2.2.2") is True + # Completing MFA from the new address re-anchors the session (parity with reauth), using a code + # in a strictly later step than enrollment consumed. + t1 = t0 + totp.DEFAULT_PERIOD + pin_totp_clock(monkeypatch, t1) + code = totp.totp(enroll.secret, now=t1) + assert await service.verify_mfa(token, code, client="10.2.2.2") is True assert await service.flag_new_client_ip(token, "10.2.2.2", path="/users") is False finally: await store.close() diff --git a/tests/test_api_auth.py b/tests/test_api_auth.py index 06defb59..104f0839 100644 --- a/tests/test_api_auth.py +++ b/tests/test_api_auth.py @@ -9,10 +9,10 @@ import httpx import pytest -from _totp_clock import fresh_totp +from _totp_clock import fresh_totp, pin_totp_clock from messagefoundry.api import create_app -from messagefoundry.auth import Role +from messagefoundry.auth import Role, totp from messagefoundry.auth.ldap import AdPrincipal from messagefoundry.auth.service import AuthService from messagefoundry.auth.tokens import hash_token @@ -145,7 +145,9 @@ async def test_security_events_feed_is_scoped_to_caller(engine: Engine) -> None: assert "auth.login_failed" in bob_actions # bob sees his own failure -async def test_mfa_enroll_confirm_and_step_up_gate(engine: Engine) -> None: +async def test_mfa_enroll_confirm_and_step_up_gate( + engine: Engine, monkeypatch: pytest.MonkeyPatch +) -> None: # WP-14 / ASVS 6.3.3: the full TOTP lifecycle over the API + the step-up MFA gate. An MFA-required # session 403s on a require_step_up route with X-MFA-Required until POST /auth/mfa-verify. service = await _service(engine, AuthSettings(login_rate_limit_enabled=False)) @@ -160,9 +162,16 @@ async def test_mfa_enroll_confirm_and_step_up_gate(engine: Engine) -> None: assert r.status_code == 200 secret = r.json()["secret"] - # Confirm with a live code โ†’ activates MFA + returns the one-time recovery codes. + # Confirm with a live code โ†’ activates MFA + returns the one-time recovery codes. Pin the TOTP + # clock so the activating confirm code and the later /auth/mfa-verify code sit in distinct steps + # (enrollment now consumes the activating step, BACKLOG #1021). The in-process ASGI server + # shares this totp module, so the pin covers its server-side verify too. assert (await _reauth(c, tok, purpose="mfa_confirm")).status_code == 200 - r = await c.post("/me/mfa/confirm", json={"code": fresh_totp(secret)}, headers=_auth(tok)) + t0 = 1_000_000.0 + pin_totp_clock(monkeypatch, t0) + r = await c.post( + "/me/mfa/confirm", json={"code": totp.totp(secret, now=t0)}, headers=_auth(tok) + ) assert r.status_code == 200 and len(r.json()["recovery_codes"]) == 10 st = (await c.get("/me/mfa", headers=_auth(tok))).json() assert st["enabled"] is True and st["required"] is True @@ -175,7 +184,12 @@ async def test_mfa_enroll_confirm_and_step_up_gate(engine: Engine) -> None: # A require_step_up route is blocked with X-MFA-Required until the 2nd factor is verified. r = await c.put("/ad-group-map", json={"entries": []}, headers=_auth(tok2)) assert r.status_code == 403 and r.headers.get("X-MFA-Required") == "1" - r = await c.post("/auth/mfa-verify", json={"code": fresh_totp(secret)}, headers=_auth(tok2)) + # Verify from a strictly later step than enrollment consumed. + t1 = t0 + totp.DEFAULT_PERIOD + pin_totp_clock(monkeypatch, t1) + r = await c.post( + "/auth/mfa-verify", json={"code": totp.totp(secret, now=t1)}, headers=_auth(tok2) + ) assert r.status_code == 200 # Now it passes (password step-up satisfied at login; MFA now satisfied). r = await c.put("/ad-group-map", json={"entries": []}, headers=_auth(tok2)) diff --git a/tests/test_mfa.py b/tests/test_mfa.py index 007635c9..2b0eceae 100644 --- a/tests/test_mfa.py +++ b/tests/test_mfa.py @@ -12,7 +12,8 @@ import asyncio -from _totp_clock import fresh_totp +import pytest +from _totp_clock import fresh_totp, pin_totp_clock from messagefoundry.auth import totp from messagefoundry.auth.identity import AuthProvider, Identity @@ -72,31 +73,68 @@ async def test_enroll_confirm_status_and_recovery_codes() -> None: await store.close() -async def test_login_requires_second_factor_after_enrollment() -> None: +async def test_login_requires_second_factor_after_enrollment( + monkeypatch: pytest.MonkeyPatch, +) -> None: store = await _store() try: service = AuthService(store, AuthSettings(mfa_recovery_code_count=2)) identity, token, password = await _bootstrap_login(service) enroll = await service.begin_mfa_enrollment(identity) - await service.confirm_mfa_enrollment(identity, fresh_totp(enroll.secret), token=token) + # Pin the TOTP clock so the enrollment confirm and the later login verify sit in distinct, + # provably-adjacent steps: enrollment now consumes the activating step (BACKLOG #1021), so a + # login code from the SAME step would be refused as a replay, not accepted. + t0 = 1_000_000.0 + pin_totp_clock(monkeypatch, t0) + activating = totp.totp(enroll.secret, now=t0) + await service.confirm_mfa_enrollment(identity, activating, token=token) out = await service.login("admin", password) assert out.ok and out.mfa_required is True and out.token is not None assert await service.mfa_satisfied(out.token) is False # step-up gate would 403 - code = totp.totp(enroll.secret) - wrong = "000000" if code != "000000" else "111111" + wrong = "000000" if activating != "000000" else "111111" assert await service.verify_mfa(out.token, wrong) is False assert await service.mfa_satisfied(out.token) is False - # Recompute the TOTP immediately before the success-path verify: the wrong-code verify above - # runs two argon2id recovery-code checks (~100-200 ms), long enough that a 30 s TOTP step - # boundary could stale the code computed at enrollment time and flake this assertion. - assert await service.verify_mfa(out.token, fresh_totp(enroll.secret)) is True + # The successful login verify must sit in a strictly later step than enrollment consumed. + t1 = t0 + totp.DEFAULT_PERIOD + pin_totp_clock(monkeypatch, t1) + assert await service.verify_mfa(out.token, totp.totp(enroll.secret, now=t1)) is True assert await service.mfa_satisfied(out.token) is True finally: await store.close() +async def test_enrollment_consumes_the_activating_step(monkeypatch: pytest.MonkeyPatch) -> None: + # BACKLOG #1021: the code that activates MFA is a live second factor, so it must be single-use like + # any login code (ASVS 6.5.1). Before the fix, confirm went through the bool verify_totp wrapper, + # which discarded the matched step and never consumed it โ€” leaving the activating code replayable + # on POST /auth/mfa-verify for the rest of its ~30 s step on first deployment. Pin the clock so the + # confirm and the replay land in the SAME step S0: the replay is refused because enrollment already + # spent S0, not because the code went stale at a boundary. + store = await _store() + try: + service = AuthService(store, AuthSettings(mfa_recovery_code_count=1)) + identity, _token, password = await _bootstrap_login(service) + enroll = await service.begin_mfa_enrollment(identity) + + t0 = 1_000_000.0 + pin_totp_clock(monkeypatch, t0) + activating = totp.totp(enroll.secret, now=t0) + # Confirm succeeds and consumes step S0 (returns the recovery codes, not None). + assert await service.confirm_mfa_enrollment(identity, activating, token=_token) is not None + + # A fresh login, then replay the SAME activating code while still pinned to step S0: refused, + # because enrollment already consumed S0 (the login path advances the high-water mark to S0 + # at enroll, so this replay resolves to a non-greater step). + out = await service.login("admin", password) + assert out.token is not None + assert await service.verify_mfa(out.token, activating) is False + assert await service.mfa_satisfied(out.token) is False + finally: + await store.close() + + async def test_require_mfa_forces_admin_even_unenrolled() -> None: store = await _store() try: @@ -136,7 +174,9 @@ async def test_recovery_code_single_use() -> None: await store.close() -async def test_totp_code_is_single_use_within_its_window() -> None: +async def test_totp_code_is_single_use_within_its_window( + monkeypatch: pytest.MonkeyPatch, +) -> None: # ASVS 6.5.1: a TOTP code is consumed on first use; replaying the SAME code (still valid inside its # ~30 s step window) on a fresh session is rejected, so a captured code can't be reused. store = await _store() @@ -144,9 +184,16 @@ async def test_totp_code_is_single_use_within_its_window() -> None: service = AuthService(store, AuthSettings()) identity, token, password = await _bootstrap_login(service) enroll = await service.begin_mfa_enrollment(identity) - await service.confirm_mfa_enrollment(identity, fresh_totp(enroll.secret), token=token) - - code = totp.totp(enroll.secret) + t0 = 1_000_000.0 + pin_totp_clock(monkeypatch, t0) + activating = totp.totp(enroll.secret, now=t0) + await service.confirm_mfa_enrollment(identity, activating, token=token) + + # Move to a step later than the one enrollment consumed (BACKLOG #1021); the login code and its + # replay both live in THIS step, so the replay is refused for reuse, not for staleness. + t1 = t0 + totp.DEFAULT_PERIOD + pin_totp_clock(monkeypatch, t1) + code = totp.totp(enroll.secret, now=t1) out = await service.login("admin", password) assert out.token is not None assert await service.verify_mfa(out.token, code) is True # consumes the step @@ -175,7 +222,7 @@ async def test_consume_totp_step_is_monotonic() -> None: await store.close() -async def test_disable_and_admin_reset_clear_mfa() -> None: +async def test_disable_and_admin_reset_clear_mfa(monkeypatch: pytest.MonkeyPatch) -> None: store = await _store() try: notifier = _FakeNotifier() @@ -184,15 +231,29 @@ async def test_disable_and_admin_reset_clear_mfa() -> None: ) identity, token, _ = await _bootstrap_login(service) enroll = await service.begin_mfa_enrollment(identity) - await service.confirm_mfa_enrollment(identity, fresh_totp(enroll.secret), token=token) + t0 = 1_000_000.0 + pin_totp_clock(monkeypatch, t0) + activating = totp.totp(enroll.secret, now=t0) + await service.confirm_mfa_enrollment(identity, activating, token=token) await service.disable_mfa(identity) assert (await service.mfa_status(identity)).enabled is False assert any(e.event_type == MFA_DISABLED for e in notifier.events) - # Re-enroll, then an admin reset clears it again and revokes sessions. + # Re-enroll, then an admin reset clears it again and revokes sessions. The single-use high-water + # mark PERSISTS across disable (disable_totp does not clear last_totp_step โ€” correct and + # conservative, do NOT clear it), so the re-enroll confirm must land in a LATER step than the + # first enrollment consumed or it would be rejected as a replay and silently leave MFA disabled + # (BACKLOG #1021). Assert it actually re-enabled so the admin reset below is proven to clear a + # live enrollment, not a no-op. enroll2 = await service.begin_mfa_enrollment(identity) - await service.confirm_mfa_enrollment(identity, fresh_totp(enroll2.secret), token=token) + t1 = t0 + totp.DEFAULT_PERIOD + pin_totp_clock(monkeypatch, t1) + reenrolled = await service.confirm_mfa_enrollment( + identity, totp.totp(enroll2.secret, now=t1), token=token + ) + assert reenrolled is not None + assert (await service.mfa_status(identity)).enabled is True await service.admin_reset_mfa(identity.user_id, actor="admin") assert (await service.mfa_status(identity)).enabled is False finally: diff --git a/tests/test_step_up.py b/tests/test_step_up.py index 478de440..76fdac0a 100644 --- a/tests/test_step_up.py +++ b/tests/test_step_up.py @@ -15,7 +15,7 @@ import pytest from messagefoundry.api import create_app -from messagefoundry.auth import Role +from messagefoundry.auth import Role, totp from messagefoundry.auth.ldap import AdPrincipal from messagefoundry.auth.service import AuthService from messagefoundry.auth.tokens import hash_token @@ -295,10 +295,12 @@ async def test_admin_user_update_opt_out_uses_window(engine: Engine) -> None: assert r.status_code == 200, r.text -async def test_login_and_verify_mfa_never_grant_an_action(engine: Engine) -> None: +async def test_login_and_verify_mfa_never_grant_an_action( + engine: Engine, monkeypatch: pytest.MonkeyPatch +) -> None: """AC-3: neither login nor verify_mfa mints a per-action grant โ€” only reauth(purpose=โ€ฆ) does.""" - from _totp_clock import fresh_totp + from _totp_clock import pin_totp_clock service = await _service(engine) await _add_admin(service, "boss") @@ -309,13 +311,20 @@ async def test_login_and_verify_mfa_never_grant_an_action(engine: Engine) -> Non # Login stamped the session window but no action grant. assert await service.has_recent_step_up(token) is True assert await service.has_action_step_up(token, "mfa_enroll") is False - # Enroll + confirm TOTP (drives the service directly, past the HTTP step-up). + # Enroll + confirm TOTP (drives the service directly, past the HTTP step-up). Pin the TOTP clock + # so the later verify_mfa can use a strictly-later step than enrollment consumes (BACKLOG #1021). + t0 = 1_000_000.0 + pin_totp_clock(monkeypatch, t0) enroll = await service.begin_mfa_enrollment(identity) - await service.confirm_mfa_enrollment(identity, fresh_totp(enroll.secret), token=token) + await service.confirm_mfa_enrollment( + identity, totp.totp(enroll.secret, now=t0), token=token + ) # A fresh MFA-required login, then verify_mfa: it seeds the session window but NOT an action grant. + t1 = t0 + totp.DEFAULT_PERIOD + pin_totp_clock(monkeypatch, t1) token2 = (await service.login("boss", PW)).token assert token2 is not None - assert await service.verify_mfa(token2, fresh_totp(enroll.secret)) is True + assert await service.verify_mfa(token2, totp.totp(enroll.secret, now=t1)) is True assert await service.has_recent_step_up(token2) is True # verify_mfa re-anchored the window assert await service.has_action_step_up(token2, "mfa_disable") is False # but no grant # Only reauth(purpose=โ€ฆ) mints one โ€” and it is single-use.