fix(mfa): bind the challenge to its tenant, and scope every MFA key by it

.cargo/mutants.toml

@@ -65,12 +65,20 @@ additional_cargo_args = ["--all-features"]
 #    drop the compiled twin's mutant, which the suite does kill. If `builder.rs` shifts, the
 #    anchor stops matching and the mutant reappears as a survivor — a loud failure, not a
 #    silent one. Re-anchor it then.
-# 6. `token_manager.rs:632` `issue_mfa_temp_token` — the `cfg(not(feature = "mfa"))` twin of
+# 6. `token_manager.rs:1033` `issue_mfa_temp_token` — the `cfg(not(feature = "mfa"))` twin of
 #    the method below it, anchored for the same reason as 5. Re-anchored from 618 on 2026-07-28
-#    when security logging pushed the method down: the sweep reported both of its mutants as
-#    survivors, which is the loud failure this anchoring was chosen for. Confirm with
-#    `cargo mutants --list --all-features | grep issue_mfa_temp_token` — exactly one line must
-#    remain, and it is the `cfg(feature = "mfa")` twin the suite kills.
+#    when security logging pushed the method down. Re-anchored again from 632 on 2026-08-10,
+#    when the tenant claim was threaded onto the challenge token — and this time the anchor had
+#    ALREADY been stale before that change: the method sat at 1022 on `main` while the anchor
+#    still read 632, so the post-merge sweep had been reporting both of these mutants as
+#    survivors against a floor that is supposed to block a release. The "loud failure" this
+#    line-anchoring was chosen for did fire; what it did not do is stop anyone shipping, because
+#    the sweep runs post-merge and a survivor in the report is not a red PR. Worth knowing when
+#    reading this list: a stale anchor here does not fail closed. Confirm with
+#    `cargo mutants --list | grep issue_mfa_temp_token` — exactly one line must remain, and it
+#    is the `cfg(feature = "mfa")` twin the suite kills. Note the config already supplies
+#    `--all-features`; passing it again on the command line fails the baseline build (see the
+#    `additional_cargo_args` note below).
 # 7. The seven `NoOpEmailProvider` sends — the provider's whole job is to do nothing, and each
 #    body is one `tracing::debug!` line followed by `Ok(())`. Replacing a body with `Ok(())`
 #    removes only that debug line, which no assertion reaches without installing a `tracing`
@@ -122,7 +130,7 @@ exclude_re = [
     'delete match arm "inactive" in assert_not_blocked',
     'replace == with != in validate_password',
     'builder\.rs:212:9: replace AuthEngineBuilder::redis_stores',
-    'token_manager\.rs:632:9: replace TokenManagerService::issue_mfa_temp_token',
+    'token_manager\.rs:1033:9: replace TokenManagerService::issue_mfa_temp_token',
     'replace <impl EmailProvider for NoOpEmailProvider>::\w+ -> Result<\(\), EmailError> with Ok\(\(\)\)',
     'replace AuthHooks::\w+ -> Result<\(\), HookError> with Ok\(\(\)\)',
     'replace < with <= in MfaService::challenge_platform',

bindings/bymax-auth-wasm/src/jwt_edge.rs

@@ -374,6 +374,7 @@ mod tests {
             token_type: MfaTempType::MfaChallenge,
             epoch: 0,
             context: MfaContext::Platform,
+            tenant_id: None,
             iat: 1_000,
             exp: 2_000,
         };
@@ -502,6 +503,7 @@ mod tests {
                 token_type: MfaTempType::MfaChallenge,
                 epoch: 0,
                 context: MfaContext::Dashboard,
+                tenant_id: Some("t_1".to_owned()),
                 iat: 1_000,
                 exp: 2_000,
             },
@@ -637,6 +639,7 @@ mod tests {
             token_type: MfaTempType::MfaChallenge,
             epoch: 0,
             context: MfaContext::Platform,
+            tenant_id: None,
             iat: 1_000,
             exp: 9_999_999_999,
         }

conformance/wire-contract.json

@@ -122,8 +122,42 @@
     "refusedError": "auth.refresh_token_invalid"
   },
 
+  "mfaSubjectPreimages": {
+    "dashboard": "dashboard:{tenantId}:{userId}",
+    "platform": "platform:{userId}",
+    "$comment": [
+      "The subject EVERY MFA store key and MFA failure counter below is derived from. One",
+      "definition rather than eight copies: eight keys share this shape, and a copy that drifts is",
+      "a key that silently stops matching the one the other implementation derives.",
+      "",
+      "The tenant is part of the dashboard arm because neither library may assume the consumer's",
+      "user ids are unique ACROSS tenants — `findById` takes a tenant precisely because they may",
+      "not be, and a host that numbers users per tenant gives every tenant a user `1`. Keyed on",
+      "`{plane}:{userId}` alone, two tenants' accounts shared all five store keys and all three",
+      "failure counters. The counters were the worst of the eight and it is not close: the other",
+      "five cost a collision between two accounts that happen to share an id, while the counters",
+      "cost a CREDENTIAL-FREE CROSS-TENANT LOCKOUT — failures against one tenant's user spent",
+      "another tenant's budget and a success on either side cleared the other's. That is not the",
+      "per-subscriber-account rate limiting NIST SP 800-63B requires, and it is the same bug the",
+      "`identifierPreimages` block below already documents one level up, where a tenant literally",
+      "named `platform` collided with the platform plane and locked an operator out of the console.",
+      "That fix landed for the login counter and stopped there.",
+      "",
+      "The platform arm carries NO tenant segment: its admins are cross-tenant and have none, and",
+      "an invented value would become a lookup key. Both implementations derive the shape from the",
+      "PLANE, not from whether a tenant was supplied, so a caller that passes one on the platform",
+      "plane cannot move the preimage off `platform:{userId}`.",
+      "",
+      "The dashboard MFA temp token carries `tenantId` for this reason. A challenge token without",
+      "it is REFUSED, not defaulted: RFC 8725 §3.9 sets the absent-means-reject pattern, §3.12",
+      "requires validation rules that reject a token of the wrong kind, and ASVS 5.0 6.6.2 requires",
+      "the out-of-band token be bound to the authentication request that generated it. Falling back",
+      "would leave the unscoped derivation reachable by omitting one optional field."
+    ]
+  },
+
   "mfaTransitionLock": {
-    "key": "mfalock:{hmac_sha256(hmacKey, '{plane}:{userId}')}",
+    "key": "mfalock:{hmac_sha256(hmacKey, mfaSubject)}",
     "value": "a per-call nonce (16 CSPRNG bytes, hex) — the release compares against it",
     "ttlSeconds": 10,
     "$comment": [
@@ -153,7 +187,7 @@
   },
 
   "recoveryCodeClaim": {
-    "key": "rcu:{hmac_sha256(hmacKey, '{plane}:{userId}:{code}')}",
+    "key": "rcu:{hmac_sha256(hmacKey, '{mfaSubject}:{code}')}",
     "value": "'1' — presence is the whole meaning",
     "ttlSeconds": 300,
     "$comment": [
@@ -171,6 +205,32 @@
     ]
   },
 
+  "mfaSubjectDerivedKeys": {
+    "mfaSetupRecord": "mfa_setup:{hmac_sha256(hmacKey, mfaSubject)}",
+    "recentAuthMarker": "ra:{hmac_sha256(hmacKey, mfaSubject)}",
+    "totpAntiReplay": "tu:{hmac_sha256(hmacKey, '{mfaSubject}:{code}')}",
+    "challengeFailureCounter": "lf:{hmac_sha256(hmacKey, 'challenge:{mfaSubject}')}",
+    "disableFailureCounter": "lf:{hmac_sha256(hmacKey, 'disable:{mfaSubject}')}",
+    "reauthFailureCounter": "lf:{hmac_sha256(hmacKey, 'reauth:{mfaSubject}')}",
+    "$comment": [
+      "The six keys that derive from `mfaSubjectPreimages` and were NOT pinned here before. They",
+      "drifted apart silently for exactly as long as nothing named them: a contract detects drift",
+      "only in what it lists, which is how the OTP stored value moved without this file noticing.",
+      "Six of the eight MFA subject-derived keys were in that blind spot.",
+      "",
+      "`totpAntiReplay` and `recoveryCodeClaim` share ONE preimage and are separated only by their",
+      "key prefix — both implementations derive them from the same string, so the two entries are",
+      "one derivation described twice, not two derivations that happen to look alike.",
+      "",
+      "The three failure counters live under the `lf:` prefix alongside the LOGIN counter that",
+      "`identifierPreimages` describes, which is why that block looked like it covered them and",
+      "did not: it defines the email-keyed login preimage, these are user-id-keyed MFA preimages.",
+      "The `challenge:` / `disable:` / `reauth:` segments keep the three isolated from each other,",
+      "so the pre-auth counter an attacker can drive cannot exhaust the authenticated user's",
+      "management budget, nor the password re-proof that gates enrolment."
+    ]
+  },
+
   "identifierPreimages": {
     "$comment": [
       "The preimage each plane HMACs into its `lf:` counter key. The identity PLANE is part of",

crates/bymax-auth-axum/tests/common/mod.rs

@@ -1118,6 +1118,7 @@ pub async fn enable_mfa_flag(harness: &Harness, user_id: &str) {
         .users
         .update_mfa(
             user_id,
+            None,
             UpdateMfaData {
                 mfa_enabled: true,
                 mfa_secret: Some("encrypted-secret".to_owned()),

crates/bymax-auth-core/src/services/auth/login.rs

@@ -197,7 +197,11 @@ impl AuthEngine {
         if user.mfa_enabled {
             let mfa_temp_token = self
                 .tokens()
-                .issue_mfa_temp_token(&user.id, MfaContext::Dashboard)
+                // The tenant comes from the AUTHENTICATED account, never from the request that
+                // named it: the challenge resolves the account by `(id, tenant)`, so a
+                // caller-supplied value here would let the second step be pointed at a
+                // different tenant's row than the password step authenticated.
+                .issue_mfa_temp_token(&user.id, MfaContext::Dashboard, Some(&user.tenant_id))
                 .await?;
             let tenant = log_safe(&tenant_id);
             tracing::info!(user_id = %user.id, tenant_id = %tenant, "login: MFA challenge issued");

crates/bymax-auth-core/src/services/auth/mod.rs

@@ -551,6 +551,7 @@ pub(crate) mod test_support {
                     .users
                     .update_mfa(
                         &user.id,
+                        None,
                         UpdateMfaData {
                             mfa_enabled: true,
                             mfa_secret: Some("encrypted-secret".to_owned()),

crates/bymax-auth-core/src/services/auth/password_reset.rs

@@ -2239,6 +2239,7 @@ mod tests {
         async fn update_mfa(
             &self,
             _id: &str,
+            _tenant_id: Option<&str>,
             _data: bymax_auth_types::UpdateMfaData,
         ) -> Result<(), crate::RepositoryError> {
             Ok(())
@@ -2332,6 +2333,7 @@ mod tests {
         assert!(
             repo.update_mfa(
                 "x",
+                None,
                 bymax_auth_types::UpdateMfaData {
                     mfa_enabled: false,
                     mfa_secret: None,

crates/bymax-auth-core/src/services/auth/session_ops.rs

@@ -500,6 +500,7 @@ mod tests {
             h.users
                 .update_mfa(
                     &id,
+                    None,
                     bymax_auth_types::UpdateMfaData {
                         mfa_enabled: true,
                         mfa_secret: Some("encrypted-secret".to_owned()),

crates/bymax-auth-core/src/services/mfa/challenge.rs

@@ -63,18 +63,51 @@ impl MfaService {
         ip: &str,
         user_agent: &str,
     ) -> Result<LoginResultMfa, AuthError> {
-        let MfaTempVerified { user_id, jti, .. } = verified;
-        let bf_id = self.challenge_bf_id(MfaContext::Dashboard, &user_id);
+        let MfaTempVerified {
+            user_id,
+            jti,
+            tenant_id,
+            ..
+        } = verified;
+        // Always `Some` on this plane — `verify_mfa_temp_token` refuses a dashboard token
+        // without it rather than resolving the account by id alone.
+        let tenant_id = tenant_id.as_deref();
+        let bf_id = self.challenge_bf_id(MfaContext::Dashboard, tenant_id, &user_id);
         self.assert_not_locked("challenge", &user_id, &bf_id)
             .await?;
 
         // Fetch the dashboard user concretely; the combined guard rejects both a missing user
-        // and one without MFA configured.
-        let user = self
+        // and one without MFA configured. Scoped by `(id, tenant)`, never by id alone. Passing `None` here resolved the account
+        // by id across every tenant, and everything below runs on what came back: the status
+        // gate, `mfa_enabled`, the secret that gets decrypted, the recovery digests that get
+        // scanned, and the account the session is finally minted for. Under a host schema that
+        // numbers users per tenant, every tenant has a user `1`.
+        let answer = self
             .user_repo
-            .find_by_id(&user_id, None)
+            .find_by_id(&user_id, tenant_id)
             .await
-            .map_err(repository_error)?
+            .map_err(repository_error)?;
+
+        // The tenant the repository ANSWERED with must be the tenant that was asked for.
+        // Passing the argument is a request, not an enforcement: the repository is the host's,
+        // and a single-tenant host writing `find_by_id` that ignores its second argument is the
+        // shape nobody notices — which is exactly the deployment where ids collide across
+        // tenants and this whole change matters. `login` already refuses such an answer
+        // (services/auth/login.rs), for the same reason and with the same wording; the challenge
+        // is the other door into the same account and had no such check. Without it, everything
+        // below — the status gate, `mfa_enabled`, the secret, the recovery digests, the account
+        // the session is minted for — still runs on a row from another tenant.
+        if answer
+            .as_ref()
+            .is_some_and(|candidate| Some(candidate.tenant_id.as_str()) != tenant_id)
+        {
+            tracing::warn!(
+                "mfa challenge: repository returned an account outside the token's tenant — \
+                 check that UserRepository::find_by_id scopes by its tenant_id argument"
+            );
+        }
+        let user = answer
+            .filter(|candidate| Some(candidate.tenant_id.as_str()) == tenant_id)
             .ok_or(AuthError::MfaNotEnabled)?;
 
         // Re-check the account status. Login gated it before minting the temp token, but that
@@ -99,7 +132,14 @@ impl MfaService {
         // (retryable within its TTL) and only the failure counter advances.
         let recovery_index = if is_totp_code(code) {
             if !self
-                .accept_totp(MfaContext::Dashboard, &user_id, &raw_secret, code, &jti)
+                .accept_totp(
+                    MfaContext::Dashboard,
+                    tenant_id,
+                    &user_id,
+                    &raw_secret,
+                    code,
+                    &jti,
+                )
                 .await?
             {
                 return self.reject_code("challenge", &user_id, &bf_id).await;
@@ -191,7 +231,7 @@ impl MfaService {
         user_agent: &str,
     ) -> Result<LoginResultMfa, AuthError> {
         let MfaTempVerified { user_id, jti, .. } = verified;
-        let bf_id = self.challenge_bf_id(MfaContext::Platform, &user_id);
+        let bf_id = self.challenge_bf_id(MfaContext::Platform, None, &user_id);
         self.assert_not_locked("platform challenge", &user_id, &bf_id)
             .await?;
 
@@ -228,7 +268,14 @@ impl MfaService {
         let recovery_codes = admin.mfa_recovery_codes.clone().unwrap_or_default();
         let recovery_index = if is_totp_code(code) {
             if !self
-                .accept_totp(MfaContext::Platform, &user_id, &raw_secret, code, &jti)
+                .accept_totp(
+                    MfaContext::Platform,
+                    None,
+                    &user_id,
+                    &raw_secret,
+                    code,
+                    &jti,
+                )
                 .await?
             {
                 return self
@@ -299,6 +346,7 @@ impl MfaService {
     async fn accept_totp(
         &self,
         ctx: MfaContext,
+        tenant_id: Option<&str>,
         user_id: &str,
         raw_secret: &[u8],
         code: &str,
@@ -317,7 +365,7 @@ impl MfaService {
         // (same marker already present) or a different still-valid code (its marker is fresh but
         // the temp token is already gone) — is rejected, so exactly one session is issued. The
         // anti-replay TTL is derived from the configured window so the marker outlives the code.
-        let replay = self.replay_id(ctx, user_id, code);
+        let replay = self.replay_id(ctx, tenant_id, user_id, code);
         let jti_marker = to_hex(&bymax_auth_crypto::mac::sha256(jti.as_bytes()));
         self.mfa_store
             .challenge_consume(&replay, &jti_marker, self.anti_replay_ttl_seconds())
@@ -343,7 +391,13 @@ impl MfaService {
         // minting two sessions, the one property a recovery code has. The engine cannot make
         // that repository atomic; it can be atomic in the store it owns. The loser reads as an
         // invalid code, which is what a code already spent is.
-        if !self.claim_recovery_code(ctx, &user.id, code).await? {
+        // The tenant comes from the fetched account rather than being threaded in: the lookup
+        // above is now scoped by `(id, tenant)`, so this IS the token's tenant, and reading it
+        // off the account leaves no second value that could disagree with the first.
+        if !self
+            .claim_recovery_code(ctx, Some(user.tenant_id.as_str()), &user.id, code)
+            .await?
+        {
             return Ok(None);
         }
         Ok(Some(index))
@@ -368,7 +422,8 @@ impl MfaService {
         else {
             return Ok(None);
         };
-        if !self.claim_recovery_code(ctx, user_id, code).await? {
+        // Platform-plane only: its admins are cross-tenant and carry no tenant.
+        if !self.claim_recovery_code(ctx, None, user_id, code).await? {
             return Ok(None);
         }
         Ok(Some(index))
@@ -387,12 +442,13 @@ impl MfaService {
     async fn claim_recovery_code(
         &self,
         ctx: MfaContext,
+        tenant_id: Option<&str>,
         user_id: &str,
         code: &str,
     ) -> Result<bool, AuthError> {
         self.mfa_store
             .claim_recovery_code(
-                &self.replay_id(ctx, user_id, code),
+                &self.replay_id(ctx, tenant_id, user_id, code),
                 super::RECOVERY_CODE_CLAIM_TTL_SECONDS,
             )
             .await