fix(otp): pin the stored-code format in the shared contract, and fix the tests it broke

conformance/wire-contract.json

@@ -21,15 +21,24 @@
       "keyed with it. Three things are pinned at once: the ':' separator between label and",
       "secret, the SHA-256, and the fact that the HMAC is keyed with the hex TEXT of the digest",
       "rather than its raw bytes. Any of the three drifting produces different Redis keys for",
-      "every lockout, OTP, resend cooldown, MFA setup, and anti-replay record."
+      "every lockout, OTP, resend cooldown, MFA setup, and anti-replay record.",
+      "",
+      "The same vector continues into the OTP record. The stored value is never the code",
+      "itself — six digits is a keyspace of a million, reversible offline from a Redis dump —",
+      "but the hmac-sha256 of `{identifier}:{code}` under that derived key. Pinned here",
+      "because both implementations write and compare the SAME `otp:` record, so a drift is",
+      "not a parse error on the other side: it is every code minted on one backend failing to",
+      "verify on the other, in both directions."
     ],
     "label": "bymax-auth:hmac-key:v1",
     "vectors": [
       {
         "secret": "0123456789abcdef0123456789abcdef",
         "derivedKeyHex": "0dd66555bd2d89e0eb4ce050f1fef427bea6799bec27fb8e313f69ab965048c1",
         "identifierMessage": "tenant-a:user@example.com",
-        "identifierHex": "609a759522bd8b397748fad2dbde07957cea580fe4f4f1f0ce0f526485de2b6d"
+        "identifierHex": "609a759522bd8b397748fad2dbde07957cea580fe4f4f1f0ce0f526485de2b6d",
+        "otpCode": "123456",
+        "otpRecordCodeHex": "d3483e9ed6fe3d6fc54ab28ac5d69dc6a60399d3519fdc0de95dbe77b9065327"
       }
     ]
   },
@@ -353,6 +362,7 @@
     "passwordHash": "PHC — see the passwordHashFormat section, which pins it by vector",
     "totpSecretAtRest": "aes-256-gcm over the BASE32 TEXT of the secret",
     "recoveryCodeDigest": "hex hmac-sha256 of the code under the derived identifier key",
+    "otpRecordCode": "hex hmac-sha256 of `{identifier}:{code}` under the derived identifier key",
     "wsTicket": "64 lowercase hex characters (32 CSPRNG bytes), single-use, 30 s lifetime"
   },
 

crates/bymax-auth-axum/tests/adapter.rs

@@ -2225,7 +2225,6 @@ async fn invitation_create_with_an_unknown_role_hits_the_error_arm() {
 async fn password_reset_otp_two_step_success_flow() {
     // forgot-password mints an OTP; verify-otp exchanges it for a verified token (success arm);
     // reset-password with that token succeeds (204). Uses the in-memory OTP peek.
-    use bymax_auth_core::traits::OtpPurpose;
     let Some(h) = build(EngineSpec::default()) else { return };
     let app = router(&h);
     seed_user(&h, "pw@e.com", "glidingwalnut42", "USER").await;
@@ -2236,12 +2235,20 @@ async fn password_reset_otp_two_step_success_flow() {
         .send(&app)
         .await;
 
-    // Recover the OTP from the in-memory store (the engine derives the identifier internally).
-    // Asserted rather than skipped: the OTP existing is what proves the route reached the
+    // Recovered from the mail the engine actually sent, which is where the code is. The stored
+    // record holds a keyed fingerprint, not the code, so reading the store back would hand the
+    // test the wrong string — and a leaked Redis dump would hand an attacker nothing, which is
+    // the point of storing it that way.
+    //
+    // Asserted rather than skipped: the code existing is what proves the route reached the
     // engine at all, and a skip here would pass just as happily against a handler that did
     // nothing but answer 200.
-    let otp = common::peek_otp(&h, OtpPurpose::PasswordReset, "pw@e.com").unwrap_or_default();
-    assert!(!otp.is_empty(), "forgot-password minted no reset OTP");
+    let otp = h
+        .emails
+        .await_password_reset_code()
+        .await
+        .unwrap_or_default();
+    assert!(!otp.is_empty(), "forgot-password mailed no reset OTP");
 
     let verify = Req::post("/auth/password/verify-otp")
         .json(serde_json::json!({ "email": "pw@e.com", "otp": otp, "tenantId": TENANT }))
@@ -2650,7 +2657,6 @@ async fn oauth_callback_mfa_branch_without_redirect_returns_json() {
 #[tokio::test]
 async fn verify_email_success_with_a_live_otp() {
     // The verify-email happy path: a registered user with a real verification OTP verifies (204).
-    use bymax_auth_core::traits::OtpPurpose;
     let Some(h) = build(EngineSpec {
         verification_required: true,
         ..EngineSpec::default()
@@ -2665,9 +2671,12 @@ async fn verify_email_success_with_a_live_otp() {
         }))
         .send(&app)
         .await;
-    let Some(otp) = common::peek_otp(&h, OtpPurpose::EmailVerification, "vfy@e.com") else {
-        return;
-    };
+    // From the mail, not the store: the record holds a keyed fingerprint of the code. Awaited
+    // and asserted — the send is detached, and an `else { return }` on a lost race is a test
+    // that skips itself and reports success.
+    let mailed = h.emails.await_verification_code().await;
+    assert!(mailed.is_some(), "register mailed no verification code");
+    let Some(otp) = mailed else { return };
     let verify = Req::post("/auth/verify-email")
         .json(serde_json::json!({ "email": "vfy@e.com", "otp": otp, "tenantId": TENANT }))
         .send(&app)
@@ -3633,3 +3642,71 @@ async fn mfa_management_error_arms_with_a_real_account() {
         assert_eq!(resp.json()["error"]["code"], code, "{path}");
     }
 }
+
+#[tokio::test]
+async fn the_capturing_mailer_records_the_codes_and_ignores_the_rest() {
+    // The harness's double keeps the two codes a flow has to submit back — the OTP record holds
+    // a keyed fingerprint, so the mailbox is the only place the plaintext exists — and no-ops
+    // the rest. Driven end to end so the object-safe impl is covered, and so the recording
+    // halves are asserted rather than assumed: a double that quietly kept nothing would make
+    // every test reading a code from it skip its own assertions and still pass.
+    use bymax_auth_core::traits::EmailProvider;
+
+    let mailer = common::CapturingEmails::default();
+    assert!(mailer.verification_code().is_none());
+    assert!(mailer.password_reset_code().is_none());
+
+    let provider: &dyn EmailProvider = &mailer;
+    assert!(
+        provider
+            .send_email_verification_otp("t1", "u@e.com", "123456", None)
+            .await
+            .is_ok()
+    );
+    assert!(
+        provider
+            .send_password_reset_otp("t1", "u@e.com", "654321", Some("pt-BR"))
+            .await
+            .is_ok()
+    );
+    assert_eq!(mailer.verification_code().as_deref(), Some("123456"));
+    assert_eq!(mailer.password_reset_code().as_deref(), Some("654321"));
+
+    // The rest carry nothing a test submits back, so they are no-ops.
+    assert!(
+        provider
+            .send_password_reset_token("t1", "u@e.com", "tok", None)
+            .await
+            .is_ok()
+    );
+    assert!(
+        provider
+            .send_email_change_verification("t1", "new@e.com", "tok", None)
+            .await
+            .is_ok()
+    );
+    assert!(
+        provider
+            .send_mfa_enabled("t1", "u@e.com", None)
+            .await
+            .is_ok()
+    );
+    assert!(
+        provider
+            .send_mfa_disabled("t1", "u@e.com", None)
+            .await
+            .is_ok()
+    );
+    let invite = bymax_auth_core::traits::InviteData {
+        inviter_name: "Owner".to_owned(),
+        tenant_name: "Acme".to_owned(),
+        invite_token: "0".repeat(64),
+        expires_at: time::OffsetDateTime::UNIX_EPOCH,
+    };
+    assert!(
+        provider
+            .send_invitation("t1", "u@e.com", &invite, None)
+            .await
+            .is_ok()
+    );
+}

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

@@ -124,6 +124,136 @@ pub fn hash_password(plain: &str) -> String {
     bymax_auth_crypto::password::hash(plain.as_bytes(), &params).unwrap_or_default()
 }
 
+/// An email provider that keeps the codes it was asked to send.
+///
+/// The OTP record no longer holds the plaintext code — it holds a keyed fingerprint, so a test
+/// cannot read the code back out of the store and never could once the store stopped being a
+/// place a leak would hand an attacker the code. The recipient's mailbox is where the code
+/// actually is, so that is where a test that drives the real flow has to get it.
+#[derive(Default)]
+pub struct CapturingEmails {
+    verification: Mutex<Option<String>>,
+    password_reset: Mutex<Option<String>>,
+}
+
+impl CapturingEmails {
+    /// Wait for the detached send to deliver the verification code, up to a deadline.
+    ///
+    /// The engine mails fire-and-forget, so reading the mailbox the instant a route returns is
+    /// a race — and one a caller loses SILENTLY when it unwraps with `else { return }`. Polling
+    /// rather than a fixed sleep keeps it from becoming a flake on a slower runner.
+    pub async fn await_verification_code(&self) -> Option<String> {
+        for _ in 0..40 {
+            if let Some(code) = self.verification_code() {
+                return Some(code);
+            }
+            tokio::time::sleep(std::time::Duration::from_millis(25)).await;
+        }
+        None
+    }
+
+    /// The password-reset twin of [`Self::await_verification_code`].
+    pub async fn await_password_reset_code(&self) -> Option<String> {
+        for _ in 0..40 {
+            if let Some(code) = self.password_reset_code() {
+                return Some(code);
+            }
+            tokio::time::sleep(std::time::Duration::from_millis(25)).await;
+        }
+        None
+    }
+
+    /// Take the last email-verification code sent, leaving the mailbox empty.
+    ///
+    /// Consuming, not peeking: a flow that mails twice would otherwise have the second read
+    /// answer instantly with the FIRST code, and the test would submit a stale one.
+    pub fn verification_code(&self) -> Option<String> {
+        self.verification.lock().ok().and_then(|mut c| c.take())
+    }
+
+    /// Take the last password-reset code sent. See [`Self::verification_code`].
+    pub fn password_reset_code(&self) -> Option<String> {
+        self.password_reset.lock().ok().and_then(|mut c| c.take())
+    }
+}
+
+#[async_trait::async_trait]
+impl bymax_auth_core::traits::EmailProvider for CapturingEmails {
+    async fn send_email_verification_otp(
+        &self,
+        _tenant_id: &str,
+        _email: &str,
+        otp: &str,
+        _locale: Option<&str>,
+    ) -> Result<(), bymax_auth_core::traits::EmailError> {
+        if let Ok(mut slot) = self.verification.lock() {
+            *slot = Some(otp.to_owned());
+        }
+        Ok(())
+    }
+
+    async fn send_password_reset_otp(
+        &self,
+        _tenant_id: &str,
+        _email: &str,
+        otp: &str,
+        _locale: Option<&str>,
+    ) -> Result<(), bymax_auth_core::traits::EmailError> {
+        if let Ok(mut slot) = self.password_reset.lock() {
+            *slot = Some(otp.to_owned());
+        }
+        Ok(())
+    }
+
+    async fn send_password_reset_token(
+        &self,
+        _tenant_id: &str,
+        _email: &str,
+        _token: &str,
+        _locale: Option<&str>,
+    ) -> Result<(), bymax_auth_core::traits::EmailError> {
+        Ok(())
+    }
+
+    async fn send_email_change_verification(
+        &self,
+        _tenant_id: &str,
+        _new_email: &str,
+        _token: &str,
+        _locale: Option<&str>,
+    ) -> Result<(), bymax_auth_core::traits::EmailError> {
+        Ok(())
+    }
+
+    async fn send_mfa_enabled(
+        &self,
+        _tenant_id: &str,
+        _email: &str,
+        _locale: Option<&str>,
+    ) -> Result<(), bymax_auth_core::traits::EmailError> {
+        Ok(())
+    }
+
+    async fn send_mfa_disabled(
+        &self,
+        _tenant_id: &str,
+        _email: &str,
+        _locale: Option<&str>,
+    ) -> Result<(), bymax_auth_core::traits::EmailError> {
+        Ok(())
+    }
+
+    async fn send_invitation(
+        &self,
+        _tenant_id: &str,
+        _email: &str,
+        _invite: &bymax_auth_core::traits::InviteData,
+        _locale: Option<&str>,
+    ) -> Result<(), bymax_auth_core::traits::EmailError> {
+        Ok(())
+    }
+}
+
 /// The built engine plus the concrete in-memory collaborators a test seeds/inspects.
 pub struct Harness {
     pub engine: Arc<AuthEngine>,
@@ -132,6 +262,8 @@ pub struct Harness {
     pub stores: Arc<InMemoryStores>,
     /// The cookie-domain resolver, present only when `EngineSpec::cookie_domains` asked for one.
     pub domain_resolver: Option<Arc<RecordingDomains>>,
+    /// The codes the engine actually mailed, for flows whose store holds only a fingerprint.
+    pub emails: Arc<CapturingEmails>,
 }
 
 /// Build an engine + router over in-memory stores per `spec`.
@@ -197,11 +329,13 @@ pub fn build(spec: EngineSpec) -> Option<Harness> {
         ]));
     }
 
+    let emails = Arc::new(CapturingEmails::default());
     let mut builder = AuthEngine::builder()
         .config(config)
         .environment(Environment::Test)
         .user_repository(users.clone())
         .platform_user_repository(admins.clone())
+        .email_provider(emails.clone())
         .redis_stores(stores.clone());
 
     if spec.oauth {
@@ -220,6 +354,7 @@ pub fn build(spec: EngineSpec) -> Option<Harness> {
         admins,
         stores,
         domain_resolver,
+        emails,
     })
 }
 
@@ -256,6 +391,7 @@ pub fn build_oauth_with_redirects() -> Option<Harness> {
         admins,
         stores,
         domain_resolver: None,
+        emails: Arc::new(CapturingEmails::default()),
     })
 }
 
@@ -293,6 +429,7 @@ pub fn build_oauth_with_failing_state_store() -> Option<Harness> {
         admins,
         stores: inert,
         domain_resolver: None,
+        emails: Arc::new(CapturingEmails::default()),
     })
 }
 
@@ -868,6 +1005,7 @@ pub fn build_failing() -> Option<Harness> {
         admins,
         stores: inert,
         domain_resolver: None,
+        emails: Arc::new(CapturingEmails::default()),
     })
 }
 
@@ -903,20 +1041,10 @@ pub fn build_failing_blacklist() -> Option<Harness> {
         admins,
         stores: inert,
         domain_resolver: None,
+        emails: Arc::new(CapturingEmails::default()),
     })
 }
 
-/// Peek the engine-generated OTP for a `(tenant, email)` pair under a purpose (the in-memory
-/// store keeps the plaintext code so the reset/verify flows can be driven end to end).
-pub fn peek_otp(
-    harness: &Harness,
-    purpose: bymax_auth_core::traits::OtpPurpose,
-    email: &str,
-) -> Option<String> {
-    let identifier = harness.engine.hashed_identifier_for(TENANT, email);
-    harness.stores.peek_otp(purpose, &identifier)
-}
-
 /// Seed an active platform admin with the given role; returns its id.
 pub async fn seed_admin(harness: &Harness, email: &str, role: &str) -> String {
     use bymax_auth_types::AuthPlatformUser;

crates/bymax-auth-core/src/engine/builder.rs

@@ -462,7 +462,10 @@ impl AuthEngineBuilder {
             brute_max_attempts,
             brute_window_secs,
         ));
-        let otp = OtpService::new(otp_store.clone());
+        let otp = OtpService::new(
+            otp_store.clone(),
+            zeroize::Zeroizing::new(*config.hmac_key()),
+        );
         let sessions = Arc::new(SessionService::new(
             session_store.clone(),
             user_repository.clone(),