fix(auth): the admin MFA reset accepted the caller's own account (BACKLOG #1022)

docs/BACKLOG.md

@@ -4815,7 +4815,7 @@ Retiring the tree costs the engine nothing operationally: **`tests/test_ech_egre
 > **One bounded residual, stated so nobody reports it as worse than it is:** the guard is **check-then-act** (read then write, both `await`s), so it is not fail-closed under concurrency. Two racing requests could each see a surviving factor and both remove one. The consequence is **forced re-enrollment, not single-factor access**, which is why it is a residual rather than a blocker.
 > **RESIDUAL SCOPE, written 2026-08-21 (dispatcher), verified on the code at `origin/main` `75e20d43`. THE TITLED DEFECT IS BUILT -- what remains is one small code limb and three records the fix made FALSE.**
 > **BUILT, verified at the definition, at BOTH call sites, and in both polarities of test.** `disable_mfa` (`auth/service.py:2253`) reads `list_webauthn_credentials` then raises when `_mfa_required_for` holds and no factor would survive -- the exact guard shape this item specified, keyed on the Identity's roles with no extra store read. Call sites: `api/auth_routes.py:443-446` maps it to a 400 with a `#1022` comment, so the 500 this item predicted does not occur; `messagefoundry_webconsole/routes/account.py:282-294` renders it as a page and re-raises 429 to preserve Retry-After. Tests: `tests/test_mfa.py:369` asserts the refusal AND that the store was not mutated anyway; `:392` is the labelled POSITIVE CONTROL asserting the disable is ALLOWED when `require_mfa=False`; `:407` asserts the admin recovery path was NOT narrowed. Route level: `tests/test_api_auth.py:1444` and the web console suite.
-> **BUILT 2026-08-25, self-exclusion limb.** `POST /users/{user_id}/reset-mfa` now refuses (400) when `user_id` is the caller's own; cross-user reset is untouched, pinned by its own test. **Stays OPEN regardless** -- per standing dispatcher guidance this row is not a clean closure. Of the three OWED records below, (1) and (2) are corrected on main; (3) is NOT -- the ADR gained retraction prose at the same spot but no new formal `AC-N` entry (checked directly: the file's ACs run AC-10 through AC-16, none added for this refusal). Whoever eventually closes this row still owes that one line.
+> **BUILT (PR #590), self-exclusion limb.** `POST /users/{user_id}/reset-mfa` now refuses (400) when `user_id` is the caller's own; cross-user reset is untouched, pinned by its own test. **Stays OPEN regardless** -- per standing dispatcher guidance this row is not a clean closure. Of the three OWED records below, (1) and (2) are corrected **in this same PR** -- not, as an earlier draft of this note said, already on main; that draft was written before this PR's own ledger edit existed and landed without one, which is the mechanism recorded and corrected at length in `#1022`'s own entry on `main` as of 2026-08-26. (3) is NOT corrected here either -- the ADR gains retraction prose at the same spot but no new formal `AC-N` entry (checked directly: the file's ACs run AC-10 through AC-16, none added for this refusal). Whoever eventually closes this row still owes that one line.
 > **OWED -- code, and it is small.** `POST /users/{user_id}/reset-mfa` (`api/auth_routes.py:786-804`) still has **no self-exclusion**, while its sibling `reset_user_password` 25 lines above (`:761-784`, guard at `:770-773`) does. So an Administrator can still zero their own factors. **The fix is self-exclusion at the ROUTE, never a guard in the service method** -- `admin_reset_mfa` is deliberately unguarded as the always-available recovery for a locked-out passkey user, its docstring says so, and `tests/test_mfa.py:407` exists specifically to prove the new guard did not narrow it.
 > **OWED -- three records the landed fix made false.** (1) `docs/SECURITY.md` still asserts of `DELETE /me/mfa` that there is "no last-factor guard" and that it does not refuse a zero-factor state -- **a shipped security document making a false NEGATIVE claim about a control that now exists.** (2) `docs/adr/0068-*.md:140` still says TOTP-disable "keeps its existing behavior this lane (parity follow-up recorded)". (3) ADR 0068 should gain an acceptance criterion for the `disable_mfa` refusal, pointing at the named test.
 > **DO NOT go looking for the AC-10 / AC-11 contradiction this item alleges -- IT IS NOT THERE.** The body above says ADR 0068 "contradicts itself four lines apart" because AC-10 was amended unscoped. On `origin/main` AC-10 IS scoped -- it conditions on deleting a WebAuthn credential that would remove the user's last second factor -- so it does not collide with AC-11's mandate that `admin_reset_mfa` clears both factor types. **The real ADR defect is under-description, not contradiction.** A builder sent to resolve a contradiction that does not exist will either invent one or return empty-handed.

docs/SECURITY.md

@@ -341,7 +341,7 @@ tuple: they act only on the caller's own account.
 | `GET` | `/me/mfa` | `require` | |
 | `POST` | `/me/mfa/enroll` | `require_reauth_only_action` (action `mfa_enroll`) | password-only step-up — the MFA gate is skipped so a required-but-unenrolled user cannot deadlock |
 | `POST` | `/me/mfa/confirm` | `require_reauth_only_action` (action `mfa_confirm`) | per-actor ceremony limiter; password-only step-up |
-| `DELETE` | `/me/mfa` | `require_step_up_action` (action `mfa_disable`) | step-up bound to the disable action (current factor + a fresh password). ⚠️ **No last-factor guard** — this is the TOTP path (`disable_mfa`), and it does **not** refuse when it would leave the account with zero enrolled factors. The passkey removal path does refuse; see BACKLOG #1022 for the asymmetry |
+| `DELETE` | `/me/mfa` | `require_step_up_action` (action `mfa_disable`) | step-up bound to the disable action (current factor + a fresh password). **Refuses (400) when TOTP is your last second factor and MFA is required for your account** — the same refusal, on the same condition, as the passkey removal path (`AuthService.disable_mfa`, ADR 0068 decision 5). The asymmetry this row used to record is closed (BACKLOG #1022) |
 | `GET` | `/me/sessions` | `require` | |
 | `GET` | `/me/security-events` | `require` | |
 | `DELETE` | `/me/sessions/{session_id}` | `require_reauth_only_action` (action `session_terminate`) | password-only step-up, bound to the action (ASVS 7.5.2): a login-seeded window does not unlock a terminate |
@@ -364,7 +364,7 @@ tuple: they act only on the caller's own account.
 | `DELETE` | `/users/{user_id}/sessions` | `users:manage` | `require_step_up` |
 | `PUT` | `/users/{user_id}/roles` | `users:manage` | `require_step_up` |
 | `POST` | `/users/{user_id}/reset-password` | `users:manage` | `require_step_up_action` (action `admin_reset_password`) |
-| `POST` | `/users/{user_id}/reset-mfa` | `users:manage` | `require_step_up_action` (action `admin_reset_mfa`) |
+| `POST` | `/users/{user_id}/reset-mfa` | `users:manage` | `require_step_up_action` (action `admin_reset_mfa`); **refuses (400) when `user_id` is the caller's own** — use the self-service MFA settings instead. Targeting yourself here was a third route to zero factors that skipped the last-factor refusal both self-service paths make (BACKLOG #1022). Cross-user reset is untouched: it is the always-available recovery for a locked-out passkey user (ADR 0068 §2) |
 | `GET` | `/users/{user_id}/channel-scope` | `users:manage` | `require` (a read on the `users:manage` tier, not `users:read`) |
 | `PUT` | `/users/{user_id}/channel-scope` | `users:manage` | `require_step_up` |
 | `GET` | `/ad-group-map` | `users:manage` | `require` |

docs/adr/0068-browser-webauthn-passkeys-offloopback.md

@@ -136,8 +136,12 @@ TOTP-specific (a WebAuthn-only user's TOTP code gets "not enrolled", never a loc
 **No WebAuthn recovery codes** — they are phishable knowledge secrets that undercut the
 phishing-resistant tier. Recovery = enroll ≥2 passkeys (UI nudge) / keep TOTP alongside /
 `admin_reset_mfa`, which is **extended to also delete all WebAuthn credentials**. Deleting the
-**last remaining second factor while MFA is required is refused** ("enroll another factor first");
-TOTP-disable keeps its existing behavior this lane (parity follow-up recorded). Documented
+**last remaining second factor while MFA is required is refused** ("enroll another factor first").
+*The parity follow-up this lane deferred has since landed: `AuthService.disable_mfa` now refuses on
+the same condition and with the same message, so the two self-service paths to zero factors are
+symmetric. The ADMIN reset additionally refuses when an operator targets their own account, which
+was a third route to zero factors that skipped both (BACKLOG #1022) — `admin_reset_mfa` itself stays
+unguarded and cross-user reset is unchanged, so the recovery path below is intact.* Documented
 consequence: a passkey-only local user cannot satisfy the TOTP-shaped JSON `/auth/mfa-verify`, so
 desktop-console step-up actions become unavailable to them (enroll-page warning; owner-accepted).
 An extra-less install with enrolled credentials gets a **startup advisory** naming

messagefoundry/api/auth_routes.py

@@ -213,7 +213,7 @@
         )
 
 
 def add_auth_routes(app: FastAPI) -> AdminHandlers:
     """Register the auth + user-administration JSON routes and RETURN the admin handler bundle.
 
     The /ui admin/account/audit pages themselves live in the ``messagefoundry_webconsole`` package now
@@ -812,6 +812,32 @@
     ) -> SimpleMessage:
         """Admin MFA reset (lost authenticator + no recovery codes): clear the user's TOTP enrollment
         and revoke their sessions so they re-enroll. The acting admin is itself step-up + MFA gated."""
+        # SELF-EXCLUSION, AND IT BELONGS HERE RATHER THAN IN THE SERVICE (BACKLOG #1022).
+        #
+        # `AuthService.admin_reset_mfa` is the ALWAYS-AVAILABLE RECOVERY for a locked-out passkey
+        # user and is deliberately unguarded (its own docstring, and ADR 0068 §2 via
+        # auth/webauthn.py). An actor check inside it would narrow the one path that exists when a
+        # user has lost every factor. This route is the only caller today, so the distinction is
+        # about WHICH CASE is refused, not about which function holds the line: cross-user recovery
+        # still reaches the service untouched, and only self-targeting is refused here.
+        #
+        # SELF-TARGETING IS NEVER RECOVERY, which is what makes the refusal safe. Reaching this
+        # route at all requires passing `require_step_up_action(... ADMIN_RESET_MFA ...)`, which is
+        # itself MFA-gated -- so a genuinely locked-out operator cannot call it in the first place.
+        # What the refusal removes is a THIRD ROUTE TO ZERO FACTORS: `disable_totp` and
+        # `delete_webauthn_credential` both refuse when they would drop the caller to zero factors
+        # while MFA is required (ADR 0068 decision 5), and using the ADMIN reset on yourself skipped
+        # that check entirely. `AuthService.disable_totp`'s docstring names this item for exactly
+        # that reason: two self-service routes to zero factors behind one step-up gate, and only one
+        # of them asked.
+        #
+        # Copied in shape from reset_user_password above, which has carried the same guard since
+        # ASVS 6.4.6 -- the two admin routes now refuse the same case the same way.
+        if user_id == identity.user_id:
+            raise HTTPException(
+                status.HTTP_400_BAD_REQUEST,
+                "use the self-service MFA settings for your own account",
+            )
         try:
             await service.admin_reset_mfa(user_id, actor=identity.username)
         except ValueError as exc:

tests/test_api_auth.py

@@ -1568,3 +1568,67 @@ async def test_admin_reset_mfa_requires_an_action_bound_proof_and_keeps_the_mfa_
             "the MFA gate is gone from admin reset-MFA -- require_step_up_action was replaced with a "
             "reauth-only factory, which is the weakening #1148's research recommended"
         )
+
+
+async def test_admin_reset_mfa_refuses_to_target_the_caller(engine: Engine) -> None:
+    """BACKLOG #1022: the admin MFA reset was a THIRD, unasked route to zero factors.
+
+    `disable_totp` and `delete_webauthn_credential` both refuse when they would drop the caller to
+    zero factors while MFA is required (ADR 0068 decision 5). Pointing the ADMIN reset at your own
+    account skipped that check entirely -- it clears TOTP, every recovery code and every passkey in
+    one call. `AuthService.disable_totp`'s own docstring names this item: two self-service routes to
+    zero factors behind one step-up gate, and only one of them asked.
+
+    THE SIBLING ROUTE HAS CARRIED THIS GUARD SINCE ASVS 6.4.6. `reset_user_password` refuses
+    `user_id == identity.user_id` twenty-five lines above; reset-MFA, the sharper of the two, did not.
+
+    WHY REFUSING SELF-TARGETING DOES NOT COST A RECOVERY PATH, which is the thing to get right:
+    `AuthService.admin_reset_mfa` stays unguarded and is still the always-available recovery for a
+    locked-out passkey user (ADR 0068 section 2). Cross-user reset is untouched -- the assertion
+    below pins that. And self-targeting was never recovery: reaching this route requires passing
+    `require_step_up_action(... ADMIN_RESET_MFA ...)`, which is itself MFA-gated, so an operator who
+    has genuinely lost every factor cannot call it at all.
+    """
+    service = await _service(
+        engine, AuthSettings(require_mfa=False, login_rate_limit_enabled=False)
+    )
+    # `_add` discards the id it creates, and this test is specifically ABOUT the caller's own id --
+    # so root is created the long way, exactly as `_add` does internally, to keep it.
+    root_id = await service.create_local_user(
+        username="root",
+        password=PW,
+        display_name=None,
+        email=None,
+        roles=[Role.ADMINISTRATOR.value],
+        actor="test",
+    )
+    await _clear_must_change(service, root_id)
+    target = await service.create_local_user(
+        username="mallory",
+        password=PW,
+        display_name=None,
+        email=None,
+        roles=["viewer"],
+        actor="root",
+    )
+    async with _client(engine, service) as c:
+        tok = (await _login(c, "root")).json()["token"]
+        h = _auth(tok)
+
+        # A grant bound to THIS action, so the request reaches the route body rather than the gate.
+        assert (await _reauth(c, tok, purpose="admin_reset_mfa")).status_code == 200
+        mine = await c.post(f"/users/{root_id}/reset-mfa", headers=h)
+        assert mine.status_code == 400, (
+            "the admin MFA reset accepted the caller's own id. That is a route to zero factors "
+            "which bypasses the last-factor refusal both self-service paths make."
+        )
+        assert "own account" in mine.json()["detail"]
+
+        # THE OTHER HALF, and without it this test would pass just as well if the route were broken
+        # outright: a DIFFERENT user is still resettable, so recovery is intact.
+        assert (await _reauth(c, tok, purpose="admin_reset_mfa")).status_code == 200
+        other = await c.post(f"/users/{target}/reset-mfa", headers=h)
+        assert other.status_code == 200, (
+            "cross-user admin MFA reset broke. That is the always-available recovery for a "
+            "locked-out passkey user (ADR 0068 section 2) and the self-exclusion must not touch it."
+        )