diff --git a/CHANGELOG.md b/CHANGELOG.md index 5dec938..e2d796c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,35 @@ version bump. ### Security +- **The account-status gate is scoped to the token's tenant.** `AuthEngine::assert_user_active` + resolved the account with `find_by_id(sub, None)` — and `UserRepository` says in as many words + that `None` is for "internal admin flows where cross-tenant access is intentional". A status + gate on a request path is not one of those. A repository id is unique only *within* a tenant, + so with a host whose ids are per-tenant serials the gate could resolve a **different tenant's** + account and decide on that account's status: admitting a caller whose own account is banned, or + refusing one who is fine. The verified token carries `tenant_id`, so there was nothing to guess. + `assert_user_active` now takes the tenant and the `UserStatus` extractor passes it. + +- **MFA management now requires a verified address, not just a live token.** `POST /auth/mfa/setup`, + `/verify-enable`, `/disable` and `/recovery-codes` took `AuthUser` — a valid token and nothing + else — so an account whose address nobody had proven could enrol, remove or re-roll a second + factor. Binding a factor to an unproven address binds it, and the recovery path that runs back + through that same address, to a mailbox that may not be the holder's. They now take a new + `VerifiedUser` extractor: account status **and** address verification. + + `POST /auth/mfa/challenge` is deliberately **not** gated — it is part of signing in, not of + managing the factor. `GET /auth/me` is deliberately left on `AuthUser` alone: a pending, + suspended or unverified client still has to read its own profile to render the "verify your + email" or "suspended" screen, and `SafeAuthUser` carries `status` and `emailVerified` for + exactly that. Gating it would leave that client a 403 and nothing to render. + + The verification half is conditional on `email_verification.required`, exactly as the login + path is: a deployment that does not ask for verification never marks anyone verified, so an + unconditional gate would make these routes permanently unreachable there. + + `UserStatus` is unchanged and still gates status alone — ws-ticket, password change and the + session routes keep their current behaviour rather than silently acquiring a second gate. + - **A delivery failure no longer logs the catalogue's subject.** `DefaultAuthEmailProvider` reported a failed send with the rendered subject line, and the subject comes from `AuthEmailCatalogue` — which is the host's. Putting the code in the subject is an ordinary diff --git a/crates/bymax-auth-axum/src/extractors/mod.rs b/crates/bymax-auth-axum/src/extractors/mod.rs index e5ba642..451260c 100644 --- a/crates/bymax-auth-axum/src/extractors/mod.rs +++ b/crates/bymax-auth-axum/src/extractors/mod.rs @@ -24,7 +24,7 @@ pub use mfa::MfaSatisfied; pub use optional::OptionalAuthUser; pub use role::{RequireRole, Role}; pub use self_or_admin::{AdminRole, SelfOrAdmin}; -pub use status::UserStatus; +pub use status::{UserStatus, VerifiedUser}; #[cfg(feature = "platform")] pub use platform::{PlatformRole, PlatformUser, RequirePlatformRole}; diff --git a/crates/bymax-auth-axum/src/extractors/status.rs b/crates/bymax-auth-axum/src/extractors/status.rs index efb4f4a..e1401dd 100644 --- a/crates/bymax-auth-axum/src/extractors/status.rs +++ b/crates/bymax-auth-axum/src/extractors/status.rs @@ -26,7 +26,49 @@ where async fn from_request_parts(parts: &mut Parts, state: &S) -> Result { let auth_state = AuthState::from_ref(state); let claims = verified_dashboard_claims(parts, &auth_state).await?; - auth_state.engine().assert_user_active(&claims.sub).await?; + // The tenant comes from the verified token, never from the request: a repository id is + // unique only within a tenant, so an unscoped lookup can resolve someone else's account. + auth_state + .engine() + .assert_user_active(&claims.sub, Some(&claims.tenant_id)) + .await?; + Ok(Self(claims)) + } +} + +/// Requires everything [`UserStatus`] does **and** that the account's address is verified. +/// +/// Separate from [`UserStatus`] rather than folded into it, because the two gates protect +/// different things and the routes that want them differ. `UserStatus` guards operations a +/// signed-in account may perform regardless of whether its address is proven — listing its own +/// sessions, changing its password, opening a socket. This one guards the operations that must +/// not be reachable from an address nobody has proven, MFA enrolment above all: enrolling a +/// second factor on an unverified account binds it to a mailbox that was never shown to belong +/// to the person, and the recovery path for that factor runs back through the same address. +/// +/// `GET /auth/me` deliberately takes neither: a pending or suspended client still has to be able +/// to read its own profile to render the "verify your email" or "suspended" screen, and +/// `SafeAuthUser` carries `status` and `email_verified` for exactly that. +/// +/// The verified half is conditional on `email_verification.required`; see +/// `AuthEngine::assert_user_active_and_verified`. +#[derive(Debug, Clone)] +pub struct VerifiedUser(pub DashboardClaims); + +impl FromRequestParts for VerifiedUser +where + AuthState: FromRef, + S: Send + Sync, +{ + type Rejection = AuthRejection; + + async fn from_request_parts(parts: &mut Parts, state: &S) -> Result { + let auth_state = AuthState::from_ref(state); + let claims = verified_dashboard_claims(parts, &auth_state).await?; + auth_state + .engine() + .assert_user_active_and_verified(&claims.sub, Some(&claims.tenant_id)) + .await?; Ok(Self(claims)) } } @@ -35,7 +77,7 @@ where mod tests { use super::*; use crate::response::AuthRejection; - use crate::test_support::{dashboard_token, parts_with_cookie, scaffold, seed}; + use crate::test_support::{dashboard_token, mint_token, parts_with_cookie, scaffold, seed}; use bymax_auth_core::config::TokenDelivery; use bymax_auth_core::traits::UserRepository; use bymax_auth_types::AuthError; @@ -59,4 +101,87 @@ mod tests { Err(AuthRejection(AuthError::AccountBanned)) )); } + + #[tokio::test] + async fn the_status_lookup_is_scoped_to_the_token_tenant() { + // A repository id is unique only WITHIN a tenant, and the repository contract says to + // pass `None` only for internal admin flows. This gate used to pass `None`, so a host + // whose ids are per-tenant serials could have it resolve a different tenant's row and + // decide on that account's status. The in-memory repository honours the tenant argument, + // so a token whose tenant does not hold the id must be refused rather than silently + // answered by whatever row shares the id. + let Some(s) = scaffold(TokenDelivery::Cookie) else { return }; + let id = seed(&s.users, "scoped@e.com", "USER").await; + let token = dashboard_token(&s, &id).await; + + // The seeded account lives in `t1`, and the token says `t1`: it resolves. + let mut parts = parts_with_cookie(&token); + assert!(matches!( + UserStatus::from_request_parts(&mut parts, &s.state).await, + Ok(UserStatus(_)) + )); + + // The regression this guards against is the EXTRACTOR passing `None`, so it has to be + // driven through the extractor. Calling the engine directly with another tenant would + // only prove the engine scopes — and would keep passing if the extractor stopped + // sending the tenant at all, which is exactly the bug. + // + // A token is minted whose claims name a tenant the account does not belong to. Scoped, + // the lookup finds nothing and the gate refuses; unscoped, it finds the row by bare id + // and lets the request through. + let elsewhere = DashboardClaims { + iss: None, + aud: None, + sub: id.clone(), + jti: "jti-other-tenant".to_owned(), + tenant_id: "some-other-tenant".to_owned(), + role: "USER".to_owned(), + token_type: bymax_auth_types::DashboardType::Dashboard, + status: "ACTIVE".to_owned(), + mfa_enabled: false, + mfa_verified: false, + iat: 1_700_000_000, + exp: 4_102_444_800, + epoch: 0, + }; + let mut parts = parts_with_cookie(&mint_token(&elsewhere)); + let refused = UserStatus::from_request_parts(&mut parts, &s.state).await; + assert!( + matches!(refused, Err(AuthRejection(AuthError::TokenInvalid))), + "the gate resolved an id outside the token's tenant: {refused:?}" + ); + } + + #[tokio::test] + async fn verified_user_refuses_an_unproven_address_and_passes_a_proven_one() { + // The MFA-management gate. `seed` creates verified accounts, so the pass arm is the + // seeded one; flipping the flag off is what proves the check is load-bearing rather + // than a status gate wearing a different name. + let Some(s) = scaffold(TokenDelivery::Cookie) else { return }; + let id = seed(&s.users, "vfy@e.com", "USER").await; + let token = dashboard_token(&s, &id).await; + + let mut parts = parts_with_cookie(&token); + assert!(matches!( + VerifiedUser::from_request_parts(&mut parts, &s.state).await, + Ok(VerifiedUser(_)) + )); + + let _ = s.users.update_email_verified(&id, false).await; + let mut parts = parts_with_cookie(&token); + let denied = VerifiedUser::from_request_parts(&mut parts, &s.state).await; + assert!( + matches!(denied, Err(AuthRejection(AuthError::EmailNotVerified))), + "an unverified address reached an MFA-management route: {denied:?}" + ); + + // The status half still applies, so the two gates compose rather than replace. + let _ = s.users.update_email_verified(&id, true).await; + let _ = s.users.update_status(&id, "SUSPENDED").await; + let mut parts = parts_with_cookie(&token); + assert!(matches!( + VerifiedUser::from_request_parts(&mut parts, &s.state).await, + Err(AuthRejection(AuthError::AccountSuspended)) + )); + } } diff --git a/crates/bymax-auth-axum/src/lib.rs b/crates/bymax-auth-axum/src/lib.rs index 19e2a35..c10146d 100644 --- a/crates/bymax-auth-axum/src/lib.rs +++ b/crates/bymax-auth-axum/src/lib.rs @@ -48,7 +48,7 @@ pub use dto::{ }; pub use extractors::{ AdminRole, AuthUser, CurrentUser, MfaSatisfied, OptionalAuthUser, RequireRole, Role, - SelfOrAdmin, UserStatus, + SelfOrAdmin, UserStatus, VerifiedUser, }; pub use rate_limit::{RateLimit, RateLimitConfig}; pub use response::{AuthRejection, error_response}; diff --git a/crates/bymax-auth-axum/src/routes/mfa.rs b/crates/bymax-auth-axum/src/routes/mfa.rs index 2682864..6c576f7 100644 --- a/crates/bymax-auth-axum/src/routes/mfa.rs +++ b/crates/bymax-auth-axum/src/routes/mfa.rs @@ -21,7 +21,7 @@ use crate::delivery::TokenDelivery; use crate::dto::{ MfaChallengeDto, MfaDisableDto, MfaRegenerateRecoveryCodesDto, MfaSetupDto, MfaVerifyDto, }; -use crate::extractors::AuthUser; +use crate::extractors::{AuthUser, VerifiedUser}; use crate::response::error_response; use crate::routes::{CookieDomains, RequestMeta}; use crate::state::{AuthState, AxumAuthConfig, ClientIpSource}; @@ -69,6 +69,11 @@ pub(crate) fn routes(config: &AxumAuthConfig, ip_source: ClientIpSource) -> Rout async fn setup( State(state): State, user: AuthUser, + // Status + email-verified, alongside the token: enrolling or removing a second factor on an + // account whose address nobody has proven binds the factor — and its recovery path — to a + // mailbox that may not be the holder's. `challenge` below is deliberately NOT gated: it is + // part of signing in, not of managing the factor. + _gate: VerifiedUser, headers: http::HeaderMap, body: axum::body::Bytes, ) -> Response { @@ -81,7 +86,16 @@ async fn setup( }; match state .engine() - .mfa_setup(&user.0.sub, MfaContext::Dashboard, dto.password.as_deref()) + .mfa_setup( + &user.0.sub, + MfaContext::Dashboard, + // The operation targets the account named by the VERIFIED token, in the tenant that + // token names. Without it the read and the writes behind it resolve by bare id, + // which the gate above no longer does — leaving the gate checking one account and + // the operation touching another. + Some(user.0.tenant_id.as_str()), + dto.password.as_deref(), + ) .await { Ok(result) => ( @@ -101,6 +115,11 @@ async fn setup( async fn verify_enable( State(state): State, user: AuthUser, + // Status + email-verified, alongside the token: enrolling or removing a second factor on an + // account whose address nobody has proven binds the factor — and its recovery path — to a + // mailbox that may not be the holder's. `challenge` below is deliberately NOT gated: it is + // part of signing in, not of managing the factor. + _gate: VerifiedUser, RequestMeta(ctx): RequestMeta, ValidatedJson(dto): ValidatedJson, ) -> Response { @@ -112,6 +131,7 @@ async fn verify_enable( &ctx.ip, &ctx.user_agent, MfaContext::Dashboard, + Some(user.0.tenant_id.as_str()), ) .await { @@ -183,6 +203,11 @@ fn mfa_temp_cookie(cookies: &tower_cookies::Cookies) -> Option { async fn disable( State(state): State, user: AuthUser, + // Status + email-verified, alongside the token: enrolling or removing a second factor on an + // account whose address nobody has proven binds the factor — and its recovery path — to a + // mailbox that may not be the holder's. `challenge` below is deliberately NOT gated: it is + // part of signing in, not of managing the factor. + _gate: VerifiedUser, RequestMeta(ctx): RequestMeta, ValidatedJson(dto): ValidatedJson, ) -> Response { @@ -194,6 +219,7 @@ async fn disable( &ctx.ip, &ctx.user_agent, MfaContext::Dashboard, + Some(user.0.tenant_id.as_str()), ) .await { @@ -207,6 +233,11 @@ async fn disable( async fn recovery_codes( State(state): State, user: AuthUser, + // Status + email-verified, alongside the token: enrolling or removing a second factor on an + // account whose address nobody has proven binds the factor — and its recovery path — to a + // mailbox that may not be the holder's. `challenge` below is deliberately NOT gated: it is + // part of signing in, not of managing the factor. + _gate: VerifiedUser, RequestMeta(ctx): RequestMeta, ValidatedJson(dto): ValidatedJson, ) -> Response { @@ -218,6 +249,7 @@ async fn recovery_codes( &ctx.ip, &ctx.user_agent, MfaContext::Dashboard, + Some(user.0.tenant_id.as_str()), ) .await { diff --git a/crates/bymax-auth-axum/src/routes/platform_mfa.rs b/crates/bymax-auth-axum/src/routes/platform_mfa.rs index c930e1f..97954ff 100644 --- a/crates/bymax-auth-axum/src/routes/platform_mfa.rs +++ b/crates/bymax-auth-axum/src/routes/platform_mfa.rs @@ -61,7 +61,13 @@ async fn setup( }; match state .engine() - .mfa_setup(&user.0.sub, MfaContext::Platform, dto.password.as_deref()) + .mfa_setup( + &user.0.sub, + MfaContext::Platform, + // A platform admin is cross-tenant by definition, so there is no tenant to scope by. + None, + dto.password.as_deref(), + ) .await { Ok(result) => ( @@ -92,6 +98,8 @@ async fn verify_enable( &ctx.ip, &ctx.user_agent, MfaContext::Platform, + // A platform admin is cross-tenant by definition, so there is no tenant to scope by. + None, ) .await { @@ -115,6 +123,8 @@ async fn disable( &ctx.ip, &ctx.user_agent, MfaContext::Platform, + // A platform admin is cross-tenant by definition, so there is no tenant to scope by. + None, ) .await { @@ -138,6 +148,8 @@ async fn recovery_codes( &ctx.ip, &ctx.user_agent, MfaContext::Platform, + // A platform admin is cross-tenant by definition, so there is no tenant to scope by. + None, ) .await { diff --git a/crates/bymax-auth-axum/tests/adapter.rs b/crates/bymax-auth-axum/tests/adapter.rs index a9d8644..74e89dd 100644 --- a/crates/bymax-auth-axum/tests/adapter.rs +++ b/crates/bymax-auth-axum/tests/adapter.rs @@ -1137,7 +1137,8 @@ async fn mfa_setup_verify_enable_and_challenge_error_arms() { .json(serde_json::json!({ "code": "000000" })) .send(&app) .await; - assert_ne!(recov.status, StatusCode::OK); + assert_eq!(recov.status, StatusCode::UNAUTHORIZED); + assert_eq!(recov.json()["error"]["code"], "auth.token_invalid"); } #[tokio::test] @@ -2751,9 +2752,18 @@ async fn dashboard_mfa_disable_and_recovery_success() { } #[tokio::test] -async fn mfa_handler_error_arms_with_a_ghost_subject() { - // A token for a non-existent subject drives the mfa setup/verify error arms (the engine - // fetches the user and errors). +async fn a_ghost_subject_is_turned_away_before_any_mfa_handler_runs() { + // Renamed from "...error_arms_with_a_ghost_subject": it no longer drives those arms, and a + // test whose name outlives what it proves is how a gap gets recorded as covered. The status + // gate on these routes resolves the account BEFORE the handler, so a token minted for a + // subject no repository row backs is refused at the door rather than reaching the engine. + // What this pins now is that refusal. The handlers' error arms are driven by a real account + // in `mfa_management_error_arms_with_a_real_account`. + // + // The assertions pin the exact refusal. `assert_ne!(status, )` would not have + // distinguished a gate rejection from an engine error — the handlers refuse a ghost too, so + // the test would stay green with the gate removed, which is precisely the claim it is here + // to make. let Some(h) = build(EngineSpec { mfa: true, ..EngineSpec::default() @@ -2767,21 +2777,24 @@ async fn mfa_handler_error_arms_with_a_ghost_subject() { .cookie("access_token", &ghost) .send(&app) .await; - assert_ne!(setup.status, StatusCode::CREATED); + assert_eq!(setup.status, StatusCode::UNAUTHORIZED); + assert_eq!(setup.json()["error"]["code"], "auth.token_invalid"); let verify = Req::post("/auth/mfa/verify-enable") .cookie("access_token", &ghost) .json(serde_json::json!({ "code": "000000" })) .send(&app) .await; - assert_ne!(verify.status, StatusCode::NO_CONTENT); + assert_eq!(verify.status, StatusCode::UNAUTHORIZED); + assert_eq!(verify.json()["error"]["code"], "auth.token_invalid"); let disable = Req::post("/auth/mfa/disable") .cookie("access_token", &ghost) .json(serde_json::json!({ "code": "000000" })) .send(&app) .await; - assert_ne!(disable.status, StatusCode::NO_CONTENT); + assert_eq!(disable.status, StatusCode::UNAUTHORIZED); + assert_eq!(disable.json()["error"]["code"], "auth.token_invalid"); let recov = Req::post("/auth/mfa/recovery-codes") .cookie("access_token", &ghost) @@ -3567,3 +3580,56 @@ async fn each_route_is_served_under_the_limit_it_declares() { "recovery-codes must NOT be served under the `mfa_setup` limit" ); } + +#[tokio::test] +async fn mfa_management_error_arms_with_a_real_account() { + // The handlers' error arms, driven by an account that PASSES every gate and simply asks for + // something the engine refuses. The suite used to reach them with a "ghost" token — one + // minted for an id no repository row backs — but the status gate now resolves the account + // before the handler runs, so a ghost is turned away at the door. That is the better + // behaviour and it is why these need a real subject: an error arm only proves it renders the + // envelope if the request actually reaches it. + let Some(h) = build(EngineSpec { + mfa: true, + ..EngineSpec::default() + }) else { + return; + }; + let app = router(&h); + let reg = Req::post("/auth/register") + .json(serde_json::json!({ + "email": "arms@e.com", "password": "glidingwalnut42", "name": "Arms", "tenantId": TENANT + })) + .send(&app) + .await; + let access = reg.cookie_value("access_token").unwrap_or_default(); + assert!(!access.is_empty()); + + // Enrolment re-proves the password, so a wrong one is refused — by the handler, not the gate. + let setup = Req::post("/auth/mfa/setup") + .cookie("access_token", &access) + .json(serde_json::json!({ "password": "not-the-password" })) + .send(&app) + .await; + assert_eq!(setup.status, StatusCode::UNAUTHORIZED); + assert_eq!(setup.json()["error"]["code"], "auth.invalid_credentials"); + + // The remaining three act on a second factor this account has never enrolled. + for (path, code) in [ + ("/auth/mfa/verify-enable", "auth.mfa_setup_required"), + ("/auth/mfa/disable", "auth.mfa_not_enabled"), + ("/auth/mfa/recovery-codes", "auth.mfa_not_enabled"), + ] { + let resp = Req::post(path) + .cookie("access_token", &access) + .json(serde_json::json!({ "code": "000000" })) + .send(&app) + .await; + assert_ne!( + resp.status, + StatusCode::FORBIDDEN, + "{path} was stopped by the gate, so its error arm never ran" + ); + assert_eq!(resp.json()["error"]["code"], code, "{path}"); + } +} diff --git a/crates/bymax-auth-core/src/services/adapter_api.rs b/crates/bymax-auth-core/src/services/adapter_api.rs index e531a73..13964e3 100644 --- a/crates/bymax-auth-core/src/services/adapter_api.rs +++ b/crates/bymax-auth-core/src/services/adapter_api.rs @@ -90,19 +90,66 @@ impl AuthEngine { /// uses (`banned → AccountBanned`, etc.). A subject that no longer exists is treated as an /// invalid token (no enumeration oracle). /// + /// `tenant_id` scopes the lookup and callers on the request path must supply it. A + /// repository id is unique only *within* a tenant — [`crate::traits::UserRepository`] says + /// so, and adds that `None` is for "internal admin flows where cross-tenant access is + /// intentional". A status gate is not one of those: this call used to pass `None`, so with + /// a host whose ids are per-tenant serials it could resolve a DIFFERENT tenant's account + /// and decide on that account's status — admitting a caller whose own account is banned, + /// or refusing one who is fine. The token carries the tenant, so there is nothing to guess. + /// /// # Errors /// /// Returns the status-specific [`AuthError`] (`AccountBanned`/`AccountInactive`/ /// `AccountSuspended`/`PendingApproval`) for a blocked account, [`AuthError::TokenInvalid`] /// for an unknown subject, or a store [`AuthError`] on a repository failure. - pub async fn assert_user_active(&self, sub: &str) -> Result<(), AuthError> { + pub async fn assert_user_active( + &self, + sub: &str, + tenant_id: Option<&str>, + ) -> Result<(), AuthError> { + self.gated_user(sub, tenant_id).await.map(|_| ()) + } + + /// As [`Self::assert_user_active`], and additionally refuse an account whose address is + /// still unproven — the gate for operations that must not be reachable before the address + /// is verified. + /// + /// Conditional on `email_verification.required`, exactly as the login path is: a deployment + /// that does not ask for verification never marks anyone verified, so gating on it + /// unconditionally would make the guarded routes permanently unreachable there. + /// + /// # Errors + /// + /// As [`Self::assert_user_active`], plus [`AuthError::EmailNotVerified`] when the + /// deployment requires verification and this account has none. + pub async fn assert_user_active_and_verified( + &self, + sub: &str, + tenant_id: Option<&str>, + ) -> Result<(), AuthError> { + let user = self.gated_user(sub, tenant_id).await?; + if self.config().config().email_verification.required && !user.email_verified { + return Err(AuthError::EmailNotVerified); + } + Ok(()) + } + + /// Fetch the account behind `sub` within `tenant_id` and apply the status gate, returning + /// it so a caller can apply further gates without a second repository round-trip. + async fn gated_user( + &self, + sub: &str, + tenant_id: Option<&str>, + ) -> Result { let user = self .user_repository() - .find_by_id(sub, None) + .find_by_id(sub, tenant_id) .await .map_err(map_repository_error)? .ok_or(AuthError::TokenInvalid)?; - self.assert_user_not_blocked(&user.status) + self.assert_user_not_blocked(&user.status)?; + Ok(user) } /// List the caller's active sessions for the HTTP `GET /auth/sessions` route. The current @@ -297,11 +344,12 @@ impl AuthEngine { &self, user_id: &str, ctx: MfaContext, + tenant_id: Option<&str>, password: Option<&str>, ) -> Result { self.mfa() .ok_or(AuthError::MfaNotEnabled)? - .setup(user_id, ctx, password) + .setup(user_id, ctx, tenant_id, password) .await } @@ -319,10 +367,11 @@ impl AuthEngine { ip: &str, user_agent: &str, ctx: MfaContext, + tenant_id: Option<&str>, ) -> Result<(), AuthError> { self.mfa() .ok_or(AuthError::MfaNotEnabled)? - .verify_and_enable(user_id, code, ip, user_agent, ctx) + .verify_and_enable(user_id, code, ip, user_agent, ctx, tenant_id) .await } @@ -408,10 +457,11 @@ impl AuthEngine { ip: &str, user_agent: &str, ctx: MfaContext, + tenant_id: Option<&str>, ) -> Result<(), AuthError> { self.mfa() .ok_or(AuthError::MfaNotEnabled)? - .disable(user_id, code, ip, user_agent, ctx) + .disable(user_id, code, ip, user_agent, ctx, tenant_id) .await } @@ -429,10 +479,11 @@ impl AuthEngine { ip: &str, user_agent: &str, ctx: MfaContext, + tenant_id: Option<&str>, ) -> Result, AuthError> { self.mfa() .ok_or(AuthError::MfaNotEnabled)? - .regenerate_recovery_codes(user_id, code, ip, user_agent, ctx) + .regenerate_recovery_codes(user_id, code, ip, user_agent, ctx, tenant_id) .await } @@ -655,7 +706,7 @@ mod tests { assert!(matches!(verified, Ok(claims) if claims.sub == sub)); // An active account passes the status gate; a garbage token fails verification. - assert!(h.engine.assert_user_active(&sub).await.is_ok()); + assert!(h.engine.assert_user_active(&sub, Some("t1")).await.is_ok()); assert!(matches!( h.engine.verify_access_token("not-a-jwt").await, Err(AuthError::TokenInvalid) @@ -665,7 +716,9 @@ mod tests { // are both rejected. Asserting only the admitting side would pass against a gate // that admitted everything — which is the whole failure this guards. assert!(matches!( - h.engine.assert_user_active("no-such-user").await, + h.engine + .assert_user_active("no-such-user", Some("t1")) + .await, Err(AuthError::TokenInvalid) )); let banned = h @@ -676,7 +729,7 @@ mod tests { }) .await; assert!(matches!( - h.engine.assert_user_active(&banned).await, + h.engine.assert_user_active(&banned, Some("t1")).await, Err(AuthError::AccountBanned) )); @@ -870,7 +923,10 @@ mod tests { let claims = seeded_claims(&h).await; h.users.fail_next_reads(1); - let gated = h.engine.assert_user_active(&claims.sub).await; + let gated = h + .engine + .assert_user_active(&claims.sub, Some(&claims.tenant_id)) + .await; let matched = matches!(gated, Err(AuthError::Internal(_))); assert!( matched, @@ -1084,12 +1140,14 @@ mod tests { use bymax_auth_types::MfaContext; let Some(h) = harness(base_config(), None) else { return }; assert!(matches!( - h.engine.mfa_setup("u", MfaContext::Dashboard, None).await, + h.engine + .mfa_setup("u", MfaContext::Dashboard, Some("t1"), None) + .await, Err(AuthError::MfaNotEnabled) )); assert!(matches!( h.engine - .mfa_verify_enable("u", "000000", "ip", "ua", MfaContext::Dashboard) + .mfa_verify_enable("u", "000000", "ip", "ua", MfaContext::Dashboard, Some("t1")) .await, Err(AuthError::MfaNotEnabled) )); @@ -1099,13 +1157,20 @@ mod tests { )); assert!(matches!( h.engine - .mfa_disable("u", "000000", "ip", "ua", MfaContext::Dashboard) + .mfa_disable("u", "000000", "ip", "ua", MfaContext::Dashboard, Some("t1")) .await, Err(AuthError::MfaNotEnabled) )); assert!(matches!( h.engine - .mfa_regenerate_recovery_codes("u", "000000", "ip", "ua", MfaContext::Dashboard) + .mfa_regenerate_recovery_codes( + "u", + "000000", + "ip", + "ua", + MfaContext::Dashboard, + Some("t1") + ) .await, Err(AuthError::MfaNotEnabled) )); diff --git a/crates/bymax-auth-core/src/services/mfa/challenge.rs b/crates/bymax-auth-core/src/services/mfa/challenge.rs index e5c3380..9e32d79 100644 --- a/crates/bymax-auth-core/src/services/mfa/challenge.rs +++ b/crates/bymax-auth-core/src/services/mfa/challenge.rs @@ -144,16 +144,21 @@ impl MfaService { // A TOTP challenge persists nothing on its own, so the rewrite needs its own write. // Serialized like every other transition, and carrying the code list from the // record inside the lock so the rewrite cannot roll back a concurrent regenerate. - self.transition_mfa_record(&user_id, MfaContext::Dashboard, |current| { - if !current.mfa_enabled { - return None; - } - Some(( - true, - Some(stored_secret), - current.mfa_recovery_codes.clone(), - )) - }) + self.transition_mfa_record( + &user_id, + MfaContext::Dashboard, + Some(user.tenant_id.as_str()), + |current| { + if !current.mfa_enabled { + return None; + } + Some(( + true, + Some(stored_secret), + current.mfa_recovery_codes.clone(), + )) + }, + ) .await?; } @@ -263,7 +268,7 @@ impl MfaService { // The platform twin of the dashboard splice, serialized and re-located by value // against the record inside the lock for exactly the same reasons. let spent = recovery_codes.get(index).cloned(); - self.transition_mfa_record(&admin.id, MfaContext::Platform, |current| { + self.transition_mfa_record(&admin.id, MfaContext::Platform, None, |current| { if !current.mfa_enabled { return None; } @@ -413,23 +418,28 @@ impl MfaService { .as_ref() .and_then(|codes| codes.get(index)) .cloned(); - self.transition_mfa_record(&user.id, MfaContext::Dashboard, |current| { - // The account stopped having MFA while this challenge was in flight — a `disable` - // that has already completed. Writing here would re-enable it with the pre-disable - // secret, so the code stays spent (its `rcu:` claim already stands) and nothing is - // written back. - if !current.mfa_enabled { - return None; - } - let mut codes = current.mfa_recovery_codes.clone().unwrap_or_default(); - // Re-locate by value: the index computed against the earlier read may name a - // different code, or none, after a concurrent write. - let live = spent - .as_ref() - .and_then(|d| codes.iter().position(|c| c == d))?; - codes.remove(live); - Some((true, Some(encrypted_secret.to_owned()), Some(codes))) - }) + self.transition_mfa_record( + &user.id, + MfaContext::Dashboard, + Some(user.tenant_id.as_str()), + |current| { + // The account stopped having MFA while this challenge was in flight — a `disable` + // that has already completed. Writing here would re-enable it with the pre-disable + // secret, so the code stays spent (its `rcu:` claim already stands) and nothing is + // written back. + if !current.mfa_enabled { + return None; + } + let mut codes = current.mfa_recovery_codes.clone().unwrap_or_default(); + // Re-locate by value: the index computed against the earlier read may name a + // different code, or none, after a concurrent write. + let live = spent + .as_ref() + .and_then(|d| codes.iter().position(|c| c == d))?; + codes.remove(live); + Some((true, Some(encrypted_secret.to_owned()), Some(codes))) + }, + ) .await .map(|_| ()) } diff --git a/crates/bymax-auth-core/src/services/mfa/manage.rs b/crates/bymax-auth-core/src/services/mfa/manage.rs index b2f2769..37ed38e 100644 --- a/crates/bymax-auth-core/src/services/mfa/manage.rs +++ b/crates/bymax-auth-core/src/services/mfa/manage.rs @@ -27,13 +27,14 @@ impl MfaService { ip: &str, user_agent: &str, ctx: MfaContext, + tenant_id: Option<&str>, ) -> Result<(), AuthError> { - let view = self.fetch_user_mfa(user_id, ctx).await?; + let view = self.fetch_user_mfa(user_id, ctx, tenant_id).await?; self.reauth_gate(ctx, user_id, code, &view).await?; // The TOTP code verified; clear MFA, revoke sessions, and notify. Serialized against // every other MFA transition so a challenge that read the record a moment earlier // cannot splice `mfa_enabled: true` and the old secret back on top of this. - self.transition_mfa_record(user_id, ctx, |_| Some((false, None, None))) + self.transition_mfa_record(user_id, ctx, tenant_id, |_| Some((false, None, None))) .await?; // Revoke every refresh session AND advance the token epoch: an auth-state change // revokes everything issued under the previous state, in both directions — the same @@ -84,8 +85,13 @@ impl MfaService { /// Returns [`AuthError::MfaNotEnabled`] if no user with that id exists in the given plane /// (the same answer the rest of this service gives for an unresolvable subject), or an /// internal/store [`AuthError`]. - pub async fn reset_mfa(&self, user_id: &str, ctx: MfaContext) -> Result<(), AuthError> { - let view = self.fetch_user_mfa(user_id, ctx).await?; + pub async fn reset_mfa( + &self, + user_id: &str, + ctx: MfaContext, + tenant_id: Option<&str>, + ) -> Result<(), AuthError> { + let view = self.fetch_user_mfa(user_id, ctx, tenant_id).await?; if !view.mfa_enabled { tracing::info!( target: "bymax_auth::mfa", @@ -93,7 +99,7 @@ impl MfaService { ); return Ok(()); } - self.transition_mfa_record(user_id, ctx, |_| Some((false, None, None))) + self.transition_mfa_record(user_id, ctx, tenant_id, |_| Some((false, None, None))) .await?; self.session_store .revoke_all(session_kind(ctx), user_id) @@ -130,8 +136,9 @@ impl MfaService { ip: &str, user_agent: &str, ctx: MfaContext, + tenant_id: Option<&str>, ) -> Result, AuthError> { - let view = self.fetch_user_mfa(user_id, ctx).await?; + let view = self.fetch_user_mfa(user_id, ctx, tenant_id).await?; self.reauth_gate(ctx, user_id, totp_code, &view).await?; // Generate a fresh set with the same entropy/format as setup; persist only the digests. let plain_codes: Vec = (0..self.recovery_code_count) @@ -148,7 +155,7 @@ impl MfaService { // list spliced it back on top of this write. The secret is taken from the record as it // stands INSIDE the lock rather than the copy read above, for the same reason. let replaced = self - .transition_mfa_record(user_id, ctx, |current| { + .transition_mfa_record(user_id, ctx, tenant_id, |current| { // MFA was disabled while the new codes were being derived. Writing them would // re-enable it with the pre-disable secret, so the transition is abandoned. if !current.mfa_enabled { diff --git a/crates/bymax-auth-core/src/services/mfa/mod.rs b/crates/bymax-auth-core/src/services/mfa/mod.rs index 34e6c72..501427d 100644 --- a/crates/bymax-auth-core/src/services/mfa/mod.rs +++ b/crates/bymax-auth-core/src/services/mfa/mod.rs @@ -550,16 +550,22 @@ impl MfaService { /// Returns [`AuthError::MfaNotEnabled`] for a misconfigured platform context or a missing /// account, the status error when the account is blocked, or a repository /// [`AuthError::Internal`] on a backend failure. + /// + /// `tenant_id` scopes the dashboard lookup and the caller supplies the one carried by the + /// verified token. A repository id is unique only *within* a tenant, so an unscoped read + /// here would let an operation authorized against one tenant read — and then mutate — the + /// same id in another. The platform arm passes nothing: an operator is not tenant-scoped. async fn fetch_user_mfa( &self, user_id: &str, ctx: MfaContext, + tenant_id: Option<&str>, ) -> Result { match ctx { MfaContext::Dashboard => { let user = self .user_repo - .find_by_id(user_id, None) + .find_by_id(user_id, tenant_id) .await .map_err(repository_error)? .ok_or(AuthError::MfaNotEnabled)?; @@ -645,6 +651,7 @@ impl MfaService { &self, user_id: &str, ctx: MfaContext, + tenant_id: Option<&str>, mutate: F, ) -> Result where @@ -664,7 +671,9 @@ impl MfaService { { return Err(AuthError::MfaStateConflict); } - let outcome = self.transition_locked(user_id, ctx, mutate).await; + let outcome = self + .transition_locked(user_id, ctx, tenant_id, mutate) + .await; // Released on every exit, including the error one: a failed transition must not leave // the account unchangeable for the lock's whole TTL. self.mfa_store @@ -679,6 +688,7 @@ impl MfaService { &self, user_id: &str, ctx: MfaContext, + tenant_id: Option<&str>, mutate: F, ) -> Result where @@ -686,7 +696,7 @@ impl MfaService { { // Re-read inside the lock. The caller's copy was read before the lock existed and may // already be stale — reusing it would leave exactly the window this closes. - let current = self.fetch_user_mfa(user_id, ctx).await?; + let current = self.fetch_user_mfa(user_id, ctx, tenant_id).await?; let Some((enabled, secret, codes)) = mutate(¤t) else { return Ok(false); }; diff --git a/crates/bymax-auth-core/src/services/mfa/setup.rs b/crates/bymax-auth-core/src/services/mfa/setup.rs index 3266fe8..031a1dd 100644 --- a/crates/bymax-auth-core/src/services/mfa/setup.rs +++ b/crates/bymax-auth-core/src/services/mfa/setup.rs @@ -24,9 +24,10 @@ impl MfaService { &self, user_id: &str, ctx: MfaContext, + tenant_id: Option<&str>, password: Option<&str>, ) -> Result { - let view = self.fetch_user_mfa(user_id, ctx).await?; + let view = self.fetch_user_mfa(user_id, ctx, tenant_id).await?; if view.mfa_enabled { return Err(AuthError::MfaAlreadyEnabled); } @@ -100,8 +101,9 @@ impl MfaService { ip: &str, user_agent: &str, ctx: MfaContext, + tenant_id: Option<&str>, ) -> Result<(), AuthError> { - let view = self.fetch_user_mfa(user_id, ctx).await?; + let view = self.fetch_user_mfa(user_id, ctx, tenant_id).await?; if view.mfa_enabled { return Err(AuthError::MfaAlreadyEnabled); } @@ -144,7 +146,7 @@ impl MfaService { // already makes the enable one-per-record among concurrent verify calls; this puts it // in the same queue as `disable` and the challenge splice, which write the same three // fields over the same record. - self.transition_mfa_record(user_id, ctx, |_| { + self.transition_mfa_record(user_id, ctx, tenant_id, |_| { Some((true, Some(data.encrypted_secret), Some(data.hashed_codes))) }) .await?; diff --git a/crates/bymax-auth-core/src/services/mfa/tests.rs b/crates/bymax-auth-core/src/services/mfa/tests.rs index ba0ecd2..1cb2245 100644 --- a/crates/bymax-auth-core/src/services/mfa/tests.rs +++ b/crates/bymax-auth-core/src/services/mfa/tests.rs @@ -252,7 +252,10 @@ async fn full_dashboard_lifecycle() { }; let Some(mfa) = h.engine.mfa() else { return }; - let Ok(setup) = mfa.setup(&uid, MfaContext::Dashboard, Some(PASSWORD)).await else { + let Ok(setup) = mfa + .setup(&uid, MfaContext::Dashboard, Some("t1"), Some(PASSWORD)) + .await + else { return; }; assert_eq!(setup.recovery_codes.len(), 8); @@ -266,7 +269,10 @@ async fn full_dashboard_lifecycle() { ); // Idempotent setup returns the same material (fast-path). - let Ok(again) = mfa.setup(&uid, MfaContext::Dashboard, Some(PASSWORD)).await else { + let Ok(again) = mfa + .setup(&uid, MfaContext::Dashboard, Some("t1"), Some(PASSWORD)) + .await + else { return; }; assert_eq!(setup.secret, again.secret); @@ -284,13 +290,21 @@ async fn full_dashboard_lifecycle() { // Enable with a valid code; the success value carries no secret. assert!( - mfa.verify_and_enable(&uid, &enable_code, "1.2.3.4", "ua", MfaContext::Dashboard) - .await - .is_ok() + mfa.verify_and_enable( + &uid, + &enable_code, + "1.2.3.4", + "ua", + MfaContext::Dashboard, + Some("t1") + ) + .await + .is_ok() ); // No read path re-exposes the secret: a further setup is rejected, never re-returning it. assert!(matches!( - mfa.setup(&uid, MfaContext::Dashboard, Some(PASSWORD)).await, + mfa.setup(&uid, MfaContext::Dashboard, Some("t1"), Some(PASSWORD)) + .await, Err(AuthError::MfaAlreadyEnabled) )); @@ -330,7 +344,14 @@ async fn full_dashboard_lifecycle() { // Regenerate: a fresh set, the old codes invalidated, sessions NOT revoked. let Ok(new_codes) = mfa - .regenerate_recovery_codes(&uid, ®en_code, "1.2.3.4", "ua", MfaContext::Dashboard) + .regenerate_recovery_codes( + &uid, + ®en_code, + "1.2.3.4", + "ua", + MfaContext::Dashboard, + Some("t1"), + ) .await else { return; @@ -340,9 +361,16 @@ async fn full_dashboard_lifecycle() { // Disable with a fourth distinct step. assert!( - mfa.disable(&uid, &disable_code, "1.2.3.4", "ua", MfaContext::Dashboard) - .await - .is_ok() + mfa.disable( + &uid, + &disable_code, + "1.2.3.4", + "ua", + MfaContext::Dashboard, + Some("t1") + ) + .await + .is_ok() ); // After disable the user is no longer MFA-enabled. let after = h.users.find_by_id(&uid, None).await; @@ -497,7 +525,10 @@ async fn every_mfa_state_change_alerts_the_account_owner() { return; }; let Some(mfa) = h.engine.mfa() else { return }; - let Ok(setup) = mfa.setup(&uid, MfaContext::Dashboard, Some(PASSWORD)).await else { + let Ok(setup) = mfa + .setup(&uid, MfaContext::Dashboard, Some("t1"), Some(PASSWORD)) + .await + else { return; }; let base = now_secs(); @@ -507,7 +538,8 @@ async fn every_mfa_state_change_alerts_the_account_owner() { &code_at(&setup.secret, base), "1.2.3.4", "ua", - MfaContext::Dashboard + MfaContext::Dashboard, + Some("t1"), ) .await .is_ok() @@ -521,7 +553,8 @@ async fn every_mfa_state_change_alerts_the_account_owner() { &code_at(&setup.secret, base + 30), "1.2.3.4", "ua", - MfaContext::Dashboard + MfaContext::Dashboard, + Some("t1"), ) .await .is_ok() @@ -532,7 +565,8 @@ async fn every_mfa_state_change_alerts_the_account_owner() { &code_at(&setup.secret, base + 60), "1.2.3.4", "ua", - MfaContext::Dashboard + MfaContext::Dashboard, + Some("t1"), ) .await .is_ok() @@ -579,7 +613,10 @@ async fn a_challenge_registers_its_session_with_the_session_service() { return; }; let Some(mfa) = h.engine.mfa() else { return }; - let Ok(setup) = mfa.setup(&uid, MfaContext::Dashboard, Some(PASSWORD)).await else { + let Ok(setup) = mfa + .setup(&uid, MfaContext::Dashboard, Some("t1"), Some(PASSWORD)) + .await + else { return; }; let base = now_secs(); @@ -589,7 +626,8 @@ async fn a_challenge_registers_its_session_with_the_session_service() { &code_at(&setup.secret, base), "1.2.3.4", "ua", - MfaContext::Dashboard + MfaContext::Dashboard, + Some("t1"), ) .await .is_ok() @@ -627,14 +665,24 @@ async fn anti_replay_rejects_a_code_already_used_on_enable() { return; }; let Some(mfa) = h.engine.mfa() else { return }; - let Ok(setup) = mfa.setup(&uid, MfaContext::Dashboard, Some(PASSWORD)).await else { + let Ok(setup) = mfa + .setup(&uid, MfaContext::Dashboard, Some("t1"), Some(PASSWORD)) + .await + else { return; }; let enable_code = code(&setup.secret, 0); assert!( - mfa.verify_and_enable(&uid, &enable_code, "1.2.3.4", "ua", MfaContext::Dashboard) - .await - .is_ok() + mfa.verify_and_enable( + &uid, + &enable_code, + "1.2.3.4", + "ua", + MfaContext::Dashboard, + Some("t1") + ) + .await + .is_ok() ); let Some(temp) = login_temp_token(&h.engine, "replay@example.com").await else { return; @@ -655,16 +703,21 @@ async fn setup_rejects_already_enabled_and_a_platform_context_without_a_repo() { let Some(mfa) = h.engine.mfa() else { return }; // No platform repository is wired, so a platform context fails fast. assert!(matches!( - mfa.setup(&uid, MfaContext::Platform, Some(PASSWORD)).await, + mfa.setup(&uid, MfaContext::Platform, None, Some(PASSWORD)) + .await, Err(AuthError::MfaNotEnabled) )); // An unknown user is also `MfaNotEnabled`. assert!(matches!( - mfa.setup("ghost", MfaContext::Dashboard, None).await, + mfa.setup("ghost", MfaContext::Dashboard, Some("t1"), None) + .await, Err(AuthError::MfaNotEnabled) )); // Enable, then a second setup is rejected. - let Ok(setup) = mfa.setup(&uid, MfaContext::Dashboard, Some(PASSWORD)).await else { + let Ok(setup) = mfa + .setup(&uid, MfaContext::Dashboard, Some("t1"), Some(PASSWORD)) + .await + else { return; }; assert!( @@ -673,13 +726,15 @@ async fn setup_rejects_already_enabled_and_a_platform_context_without_a_repo() { &code(&setup.secret, 0), "1.2.3.4", "ua", - MfaContext::Dashboard + MfaContext::Dashboard, + Some("t1"), ) .await .is_ok() ); assert!(matches!( - mfa.setup(&uid, MfaContext::Dashboard, Some(PASSWORD)).await, + mfa.setup(&uid, MfaContext::Dashboard, Some("t1"), Some(PASSWORD)) + .await, Err(AuthError::MfaAlreadyEnabled) )); assert!(matches!( @@ -688,7 +743,8 @@ async fn setup_rejects_already_enabled_and_a_platform_context_without_a_repo() { &code(&setup.secret, 30), "1.2.3.4", "ua", - MfaContext::Dashboard + MfaContext::Dashboard, + Some("t1"), ) .await, Err(AuthError::MfaAlreadyEnabled) @@ -704,17 +760,34 @@ async fn enable_requires_a_pending_record_and_rejects_a_wrong_code() { let Some(mfa) = h.engine.mfa() else { return }; // No setup yet -> no pending record. assert!(matches!( - mfa.verify_and_enable(&uid, "000000", "1.2.3.4", "ua", MfaContext::Dashboard) - .await, + mfa.verify_and_enable( + &uid, + "000000", + "1.2.3.4", + "ua", + MfaContext::Dashboard, + Some("t1") + ) + .await, Err(AuthError::MfaSetupRequired) )); - let Ok(setup) = mfa.setup(&uid, MfaContext::Dashboard, Some(PASSWORD)).await else { + let Ok(setup) = mfa + .setup(&uid, MfaContext::Dashboard, Some("t1"), Some(PASSWORD)) + .await + else { return; }; // A wrong code does not enable and does not consume the pending record. assert!(matches!( - mfa.verify_and_enable(&uid, "not-a-code", "1.2.3.4", "ua", MfaContext::Dashboard) - .await, + mfa.verify_and_enable( + &uid, + "not-a-code", + "1.2.3.4", + "ua", + MfaContext::Dashboard, + Some("t1") + ) + .await, Err(AuthError::MfaInvalidCode) )); // The record survived, so a correct code still enables. @@ -724,7 +797,8 @@ async fn enable_requires_a_pending_record_and_rejects_a_wrong_code() { &code(&setup.secret, 0), "1.2.3.4", "ua", - MfaContext::Dashboard + MfaContext::Dashboard, + Some("t1"), ) .await .is_ok() @@ -819,7 +893,10 @@ async fn challenge_locks_out_after_repeated_wrong_codes() { return; }; let Some(mfa) = h.engine.mfa() else { return }; - let Ok(setup) = mfa.setup(&uid, MfaContext::Dashboard, Some(PASSWORD)).await else { + let Ok(setup) = mfa + .setup(&uid, MfaContext::Dashboard, Some("t1"), Some(PASSWORD)) + .await + else { return; }; assert!( @@ -828,7 +905,8 @@ async fn challenge_locks_out_after_repeated_wrong_codes() { &code(&setup.secret, 0), "1.2.3.4", "ua", - MfaContext::Dashboard + MfaContext::Dashboard, + Some("t1"), ) .await .is_ok() @@ -852,7 +930,10 @@ async fn challenge_locks_out_after_repeated_wrong_codes() { let Some(other) = register(&h.engine, "other@example.com").await else { return; }; - let Ok(other_setup) = mfa.setup(&other, MfaContext::Dashboard, None).await else { + let Ok(other_setup) = mfa + .setup(&other, MfaContext::Dashboard, Some("t1"), None) + .await + else { return; }; let base = now_secs(); @@ -862,7 +943,8 @@ async fn challenge_locks_out_after_repeated_wrong_codes() { &code_at(&other_setup.secret, base), "1.2.3.4", "ua", - MfaContext::Dashboard + MfaContext::Dashboard, + Some("t1"), ) .await .is_ok() @@ -896,10 +978,16 @@ async fn two_users_setting_up_never_share_a_pending_record() { }; let Some(mfa) = h.engine.mfa() else { return }; - let Ok(a) = mfa.setup(&first, MfaContext::Dashboard, None).await else { + let Ok(a) = mfa + .setup(&first, MfaContext::Dashboard, Some("t1"), None) + .await + else { return; }; - let Ok(b) = mfa.setup(&second, MfaContext::Dashboard, None).await else { + let Ok(b) = mfa + .setup(&second, MfaContext::Dashboard, Some("t1"), None) + .await + else { return; }; assert_ne!(a.secret, b.secret); @@ -913,7 +1001,8 @@ async fn two_users_setting_up_never_share_a_pending_record() { &code_at(&a.secret, base), "1.2.3.4", "ua", - MfaContext::Dashboard + MfaContext::Dashboard, + Some("t1"), ) .await .is_ok() @@ -924,7 +1013,8 @@ async fn two_users_setting_up_never_share_a_pending_record() { &code_at(&b.secret, base), "1.2.3.4", "ua", - MfaContext::Dashboard + MfaContext::Dashboard, + Some("t1"), ) .await .is_ok() @@ -940,11 +1030,21 @@ async fn disable_is_totp_only_and_regenerate_keeps_sessions() { let Some(mfa) = h.engine.mfa() else { return }; // disable before enable -> not enabled. assert!(matches!( - mfa.disable(&uid, "000000", "1.2.3.4", "ua", MfaContext::Dashboard) - .await, + mfa.disable( + &uid, + "000000", + "1.2.3.4", + "ua", + MfaContext::Dashboard, + Some("t1") + ) + .await, Err(AuthError::MfaNotEnabled) )); - let Ok(setup) = mfa.setup(&uid, MfaContext::Dashboard, Some(PASSWORD)).await else { + let Ok(setup) = mfa + .setup(&uid, MfaContext::Dashboard, Some("t1"), Some(PASSWORD)) + .await + else { return; }; assert!( @@ -953,7 +1053,8 @@ async fn disable_is_totp_only_and_regenerate_keeps_sessions() { &code(&setup.secret, 0), "1.2.3.4", "ua", - MfaContext::Dashboard + MfaContext::Dashboard, + Some("t1"), ) .await .is_ok() @@ -961,8 +1062,15 @@ async fn disable_is_totp_only_and_regenerate_keeps_sessions() { // A recovery code can never disable MFA (it is not a TOTP). let recovery = setup.recovery_codes[0].clone(); assert!(matches!( - mfa.disable(&uid, &recovery, "1.2.3.4", "ua", MfaContext::Dashboard) - .await, + mfa.disable( + &uid, + &recovery, + "1.2.3.4", + "ua", + MfaContext::Dashboard, + Some("t1") + ) + .await, Err(AuthError::MfaInvalidCode) )); // Regenerate keeps the secret and replaces the codes; an old code no longer verifies. @@ -973,6 +1081,7 @@ async fn disable_is_totp_only_and_regenerate_keeps_sessions() { "1.2.3.4", "ua", MfaContext::Dashboard, + Some("t1"), ) .await else { @@ -993,7 +1102,8 @@ async fn disable_is_totp_only_and_regenerate_keeps_sessions() { &code(&setup.secret, 60), "1.2.3.4", "ua", - MfaContext::Dashboard + MfaContext::Dashboard, + Some("t1"), ) .await .is_ok() @@ -1007,7 +1117,10 @@ async fn disable_locks_out_after_repeated_wrong_codes() { return; }; let Some(mfa) = h.engine.mfa() else { return }; - let Ok(setup) = mfa.setup(&uid, MfaContext::Dashboard, Some(PASSWORD)).await else { + let Ok(setup) = mfa + .setup(&uid, MfaContext::Dashboard, Some("t1"), Some(PASSWORD)) + .await + else { return; }; assert!( @@ -1016,21 +1129,36 @@ async fn disable_locks_out_after_repeated_wrong_codes() { &code(&setup.secret, 0), "1.2.3.4", "ua", - MfaContext::Dashboard + MfaContext::Dashboard, + Some("t1"), ) .await .is_ok() ); for _ in 0..5 { assert!(matches!( - mfa.disable(&uid, "wrong-totp", "1.2.3.4", "ua", MfaContext::Dashboard) - .await, + mfa.disable( + &uid, + "wrong-totp", + "1.2.3.4", + "ua", + MfaContext::Dashboard, + Some("t1") + ) + .await, Err(AuthError::MfaInvalidCode) )); } assert!(matches!( - mfa.disable(&uid, "wrong-totp", "1.2.3.4", "ua", MfaContext::Dashboard) - .await, + mfa.disable( + &uid, + "wrong-totp", + "1.2.3.4", + "ua", + MfaContext::Dashboard, + Some("t1") + ) + .await, Err(AuthError::AccountLocked { .. }) )); @@ -1040,7 +1168,10 @@ async fn disable_locks_out_after_repeated_wrong_codes() { let Some(other) = register(&h.engine, "dislock2@example.com").await else { return; }; - let Ok(other_setup) = mfa.setup(&other, MfaContext::Dashboard, None).await else { + let Ok(other_setup) = mfa + .setup(&other, MfaContext::Dashboard, Some("t1"), None) + .await + else { return; }; let base = now_secs(); @@ -1050,7 +1181,8 @@ async fn disable_locks_out_after_repeated_wrong_codes() { &code_at(&other_setup.secret, base), "1.2.3.4", "ua", - MfaContext::Dashboard + MfaContext::Dashboard, + Some("t1"), ) .await .is_ok() @@ -1061,7 +1193,8 @@ async fn disable_locks_out_after_repeated_wrong_codes() { &code_at(&other_setup.secret, base + 30), "1.2.3.4", "ua", - MfaContext::Dashboard + MfaContext::Dashboard, + Some("t1"), ) .await .is_ok() @@ -1092,7 +1225,7 @@ async fn the_platform_recovery_splice_abandons_when_mfa_vanished_under_the_lock( h.platform.insert(admin); let Some(mfa) = h.engine.mfa() else { return }; let Ok(setup) = mfa - .setup("p-abandon", MfaContext::Platform, Some(PASSWORD)) + .setup("p-abandon", MfaContext::Platform, None, Some(PASSWORD)) .await else { return; @@ -1103,7 +1236,8 @@ async fn the_platform_recovery_splice_abandons_when_mfa_vanished_under_the_lock( &code(&setup.secret, 0), "1.2.3.4", "ua", - MfaContext::Platform + MfaContext::Platform, + None, ) .await .is_ok() @@ -1156,7 +1290,10 @@ async fn platform_context_routes_to_the_platform_repository() { }; h.platform.insert(admin); let Some(mfa) = h.engine.mfa() else { return }; - let Ok(setup) = mfa.setup("p1", MfaContext::Platform, Some(PASSWORD)).await else { + let Ok(setup) = mfa + .setup("p1", MfaContext::Platform, None, Some(PASSWORD)) + .await + else { return; }; assert!( @@ -1165,7 +1302,8 @@ async fn platform_context_routes_to_the_platform_repository() { &code(&setup.secret, 0), "1.2.3.4", "ua", - MfaContext::Platform + MfaContext::Platform, + None, ) .await .is_ok() @@ -1180,7 +1318,8 @@ async fn platform_context_routes_to_the_platform_repository() { &code(&setup.secret, 30), "1.2.3.4", "ua", - MfaContext::Platform + MfaContext::Platform, + None, ) .await .is_ok() @@ -1191,7 +1330,8 @@ async fn platform_context_routes_to_the_platform_repository() { &code(&setup.secret, 60), "1.2.3.4", "ua", - MfaContext::Platform + MfaContext::Platform, + None, ) .await .is_ok() @@ -1228,14 +1368,24 @@ async fn platform_challenge_exchanges_a_temp_token_for_a_full_platform_session() // Enable MFA on the platform admin so a challenge has a secret to verify against. let base = now_secs(); - let Ok(setup) = mfa.setup("p1", MfaContext::Platform, Some(PASSWORD)).await else { + let Ok(setup) = mfa + .setup("p1", MfaContext::Platform, None, Some(PASSWORD)) + .await + else { return; }; let enable_code = code_at(&setup.secret, base); assert!( - mfa.verify_and_enable("p1", &enable_code, "1.2.3.4", "ua", MfaContext::Platform) - .await - .is_ok() + mfa.verify_and_enable( + "p1", + &enable_code, + "1.2.3.4", + "ua", + MfaContext::Platform, + Some("t1") + ) + .await + .is_ok() ); // Mint a PLATFORM temp token (what the platform login plants for an MFA-enabled admin) and @@ -1326,7 +1476,10 @@ async fn platform_challenge_rejects_a_wrong_code_and_keeps_the_temp_token_alive( }); let Some(mfa) = h.engine.mfa() else { return }; let base = now_secs(); - let Ok(setup) = mfa.setup("p2", MfaContext::Platform, Some(PASSWORD)).await else { + let Ok(setup) = mfa + .setup("p2", MfaContext::Platform, None, Some(PASSWORD)) + .await + else { return; }; assert!( @@ -1335,7 +1488,8 @@ async fn platform_challenge_rejects_a_wrong_code_and_keeps_the_temp_token_alive( &code_at(&setup.secret, base), "1.2.3.4", "ua", - MfaContext::Platform + MfaContext::Platform, + None, ) .await .is_ok() @@ -1481,12 +1635,22 @@ async fn a_transition_is_refused_while_another_one_holds_the_lock() { // `setup` writes only the pending record, so it does not contend; `verify_and_enable` is // the first call that rewrites the account, and it is the one refused. - let Ok(setup) = mfa.setup(&uid, MfaContext::Dashboard, None).await else { + let Ok(setup) = mfa + .setup(&uid, MfaContext::Dashboard, Some("t1"), None) + .await + else { return; }; let code = code_at(&setup.secret, now_secs()); let refused = mfa - .verify_and_enable(&uid, &code, "1.2.3.4", "ua", MfaContext::Dashboard) + .verify_and_enable( + &uid, + &code, + "1.2.3.4", + "ua", + MfaContext::Dashboard, + Some("t1"), + ) .await; assert!( matches!(refused, Err(AuthError::MfaStateConflict)), @@ -1576,12 +1740,22 @@ async fn the_transition_releases_with_the_token_it_acquired_with() { // Enrolment on a passwordless account takes a recent authentication, so this test has to // arrange one — without it `setup` refuses and the case below silently becomes a no-op. plant_recent_auth(&mfa, &uid).await; - let Ok(setup) = mfa.setup(&uid, MfaContext::Dashboard, None).await else { + let Ok(setup) = mfa + .setup(&uid, MfaContext::Dashboard, Some("t1"), None) + .await + else { return; }; let code = code_at(&setup.secret, now_secs()); let enabled = mfa - .verify_and_enable(&uid, &code, "1.2.3.4", "ua", MfaContext::Dashboard) + .verify_and_enable( + &uid, + &code, + "1.2.3.4", + "ua", + MfaContext::Dashboard, + Some("t1"), + ) .await; assert!( enabled.is_ok(), @@ -1634,7 +1808,10 @@ async fn the_recovery_splice_abandons_when_mfa_vanished_under_the_lock() { }; let Some(mfa) = h.engine.mfa() else { return }; let base = now_secs(); - let Ok(setup) = mfa.setup(&uid, MfaContext::Dashboard, Some(PASSWORD)).await else { + let Ok(setup) = mfa + .setup(&uid, MfaContext::Dashboard, Some("t1"), Some(PASSWORD)) + .await + else { return; }; let enabled = mfa @@ -1644,6 +1821,7 @@ async fn the_recovery_splice_abandons_when_mfa_vanished_under_the_lock() { "1.2.3.4", "ua", MfaContext::Dashboard, + Some("t1"), ) .await; assert!(enabled.is_ok(), "enrolment should succeed: {enabled:?}"); @@ -1703,13 +1881,23 @@ async fn a_transition_abandons_when_mfa_is_disabled_under_the_lock() { // Enrol first: the account really does have MFA when the caller starts. let base = now_secs(); - let Ok(setup) = mfa.setup(&uid, MfaContext::Dashboard, None).await else { + let Ok(setup) = mfa + .setup(&uid, MfaContext::Dashboard, Some("t1"), None) + .await + else { return; }; let enable_code = code_at(&setup.secret, base); let regen_code = code_at(&setup.secret, base + 60); let enabled = mfa - .verify_and_enable(&uid, &enable_code, "1.2.3.4", "ua", MfaContext::Dashboard) + .verify_and_enable( + &uid, + &enable_code, + "1.2.3.4", + "ua", + MfaContext::Dashboard, + Some("t1"), + ) .await; assert!(enabled.is_ok(), "enrolment should succeed: {enabled:?}"); @@ -1719,7 +1907,14 @@ async fn a_transition_abandons_when_mfa_is_disabled_under_the_lock() { *armed = true; } let regenerated = mfa - .regenerate_recovery_codes(&uid, ®en_code, "1.2.3.4", "ua", MfaContext::Dashboard) + .regenerate_recovery_codes( + &uid, + ®en_code, + "1.2.3.4", + "ua", + MfaContext::Dashboard, + Some("t1"), + ) .await; assert!( matches!(regenerated, Err(AuthError::MfaNotEnabled)), @@ -2163,7 +2358,9 @@ async fn setup_returns_the_winner_record_after_a_lost_nx_race() { }); let svc = service_over(store, users); plant_recent_auth(&svc, &uid).await; - let result = svc.setup(&uid, MfaContext::Dashboard, None).await; + let result = svc + .setup(&uid, MfaContext::Dashboard, Some("t1"), None) + .await; assert!(matches!(&result, Ok(r) if r.recovery_codes == ["WINNER-0000-CODE"])); } @@ -2182,7 +2379,8 @@ async fn setup_errors_when_the_record_vanishes_after_a_lost_race() { let svc = service_over(store, users); plant_recent_auth(&svc, &uid).await; assert!(matches!( - svc.setup(&uid, MfaContext::Dashboard, None).await, + svc.setup(&uid, MfaContext::Dashboard, Some("t1"), None) + .await, Err(AuthError::Internal(_)) )); } @@ -2204,7 +2402,9 @@ async fn setup_fast_path_rejects_a_corrupt_or_undecryptable_record() { let garbage_svc = service_over(garbage, users.clone()); plant_recent_auth(&garbage_svc, &uid).await; assert!(matches!( - garbage_svc.setup(&uid, MfaContext::Dashboard, None).await, + garbage_svc + .setup(&uid, MfaContext::Dashboard, Some("t1"), None) + .await, Err(AuthError::Internal(_)) )); // Well-formed record whose ciphertext will not decrypt under the key. @@ -2223,7 +2423,7 @@ async fn setup_fast_path_rejects_a_corrupt_or_undecryptable_record() { plant_recent_auth(&undecryptable_svc, &uid).await; assert!(matches!( undecryptable_svc - .setup(&uid, MfaContext::Dashboard, None) + .setup(&uid, MfaContext::Dashboard, Some("t1"), None) .await, Err(AuthError::Internal(_)) )); @@ -2240,7 +2440,7 @@ async fn setup_fast_path_rejects_a_corrupt_or_undecryptable_record() { plant_recent_auth(&codes_undecryptable_svc, &uid).await; assert!(matches!( codes_undecryptable_svc - .setup(&uid, MfaContext::Dashboard, None) + .setup(&uid, MfaContext::Dashboard, Some("t1"), None) .await, Err(AuthError::Internal(_)) )); @@ -2259,7 +2459,7 @@ async fn setup_fast_path_rejects_a_corrupt_or_undecryptable_record() { plant_recent_auth(&codes_undecodable_svc, &uid).await; assert!(matches!( codes_undecodable_svc - .setup(&uid, MfaContext::Dashboard, None) + .setup(&uid, MfaContext::Dashboard, Some("t1"), None) .await, Err(AuthError::Internal(_)) )); @@ -2299,8 +2499,15 @@ async fn enable_fails_when_the_completion_gate_is_lost() { // `winner_record` encrypts the raw secret `[1u8; 20]`, so a code for those bytes verifies. let valid = raw_code(&[1u8; 20], now_secs()); assert!(matches!( - svc.verify_and_enable(&uid, &valid, "1.2.3.4", "ua", MfaContext::Dashboard) - .await, + svc.verify_and_enable( + &uid, + &valid, + "1.2.3.4", + "ua", + MfaContext::Dashboard, + Some("t1") + ) + .await, Err(AuthError::MfaSetupRequired) )); } @@ -2312,14 +2519,20 @@ async fn challenge_rejects_a_wrong_six_digit_totp_code() { let Some(h) = build(false, false) else { return }; let Some(uid) = register(&h.engine, "wrong-totp@example.com").await else { return }; let Some(mfa) = h.engine.mfa() else { return }; - let Ok(setup) = mfa.setup(&uid, MfaContext::Dashboard, Some(PASSWORD)).await else { return }; + let Ok(setup) = mfa + .setup(&uid, MfaContext::Dashboard, Some("t1"), Some(PASSWORD)) + .await + else { + return; + }; assert!( mfa.verify_and_enable( &uid, &code(&setup.secret, 0), "1.2.3.4", "ua", - MfaContext::Dashboard + MfaContext::Dashboard, + Some("t1"), ) .await .is_ok() @@ -2339,14 +2552,20 @@ async fn challenge_succeeds_with_session_tracking_disabled() { let Some(h) = build(false, false) else { return }; let Some(uid) = register(&h.engine, "nosess@example.com").await else { return }; let Some(mfa) = h.engine.mfa() else { return }; - let Ok(setup) = mfa.setup(&uid, MfaContext::Dashboard, Some(PASSWORD)).await else { return }; + let Ok(setup) = mfa + .setup(&uid, MfaContext::Dashboard, Some("t1"), Some(PASSWORD)) + .await + else { + return; + }; assert!( mfa.verify_and_enable( &uid, &code(&setup.secret, 0), "1.2.3.4", "ua", - MfaContext::Dashboard + MfaContext::Dashboard, + Some("t1"), ) .await .is_ok() @@ -2366,14 +2585,20 @@ async fn challenge_collapses_an_undecryptable_secret_to_an_opaque_error() { let Some(h) = build(false, false) else { return }; let Some(uid) = register(&h.engine, "decrypt@example.com").await else { return }; let Some(mfa) = h.engine.mfa() else { return }; - let Ok(setup) = mfa.setup(&uid, MfaContext::Dashboard, Some(PASSWORD)).await else { return }; + let Ok(setup) = mfa + .setup(&uid, MfaContext::Dashboard, Some("t1"), Some(PASSWORD)) + .await + else { + return; + }; assert!( mfa.verify_and_enable( &uid, &code(&setup.secret, 0), "1.2.3.4", "ua", - MfaContext::Dashboard + MfaContext::Dashboard, + Some("t1"), ) .await .is_ok() @@ -2491,7 +2716,10 @@ async fn concurrent_distinct_valid_codes_issue_one_session() { let secret; { let Some(mfa) = h.engine.mfa() else { return }; - let Ok(setup) = mfa.setup(&uid, MfaContext::Dashboard, Some(PASSWORD)).await else { + let Ok(setup) = mfa + .setup(&uid, MfaContext::Dashboard, Some("t1"), Some(PASSWORD)) + .await + else { return; }; if mfa @@ -2501,6 +2729,7 @@ async fn concurrent_distinct_valid_codes_issue_one_session() { "1.2.3.4", "ua", MfaContext::Dashboard, + Some("t1"), ) .await .is_err() @@ -2974,7 +3203,10 @@ async fn an_mfa_state_change_kills_the_outstanding_access_tokens() { // Enable MFA. Distinct TOTP steps per verification, as in the lifecycle test. let Some(mfa) = h.engine.mfa() else { return }; - let Ok(setup) = mfa.setup(&uid, MfaContext::Dashboard, Some(PASSWORD)).await else { + let Ok(setup) = mfa + .setup(&uid, MfaContext::Dashboard, Some("t1"), Some(PASSWORD)) + .await + else { return; }; let base = now_secs(); @@ -2984,7 +3216,8 @@ async fn an_mfa_state_change_kills_the_outstanding_access_tokens() { &code_at(&setup.secret, base), "1.2.3.4", "ua", - MfaContext::Dashboard + MfaContext::Dashboard, + Some("t1"), ) .await .is_ok() @@ -3024,7 +3257,8 @@ async fn an_mfa_state_change_kills_the_outstanding_access_tokens() { &code_at(&setup.secret, base + 60), "1.2.3.4", "ua", - MfaContext::Dashboard + MfaContext::Dashboard, + Some("t1"), ) .await .is_ok() @@ -3299,7 +3533,7 @@ async fn enrolment_re_authenticates_against_the_account_password() { // login returns, so an attacker holding a stolen token learns nothing new. for attempt in [None, Some("wrong")] { let refused = service - .setup(&user.id, MfaContext::Dashboard, attempt) + .setup(&user.id, MfaContext::Dashboard, Some("t1"), attempt) .await; assert!( matches!(refused, Err(AuthError::InvalidCredentials)), @@ -3309,7 +3543,7 @@ async fn enrolment_re_authenticates_against_the_account_password() { // The correct password enrols. let allowed = service - .setup(&user.id, MfaContext::Dashboard, Some(password)) + .setup(&user.id, MfaContext::Dashboard, Some("t1"), Some(password)) .await; assert!( allowed.is_ok(), @@ -3339,7 +3573,9 @@ async fn enrolment_on_a_passwordless_account_takes_a_recent_authentication_inste let service = service_over(Arc::new(InMemoryStores::new()), users); // No marker: the caller holds a token but has not proved it authenticated recently. - let refused = service.setup(&uid, MfaContext::Dashboard, None).await; + let refused = service + .setup(&uid, MfaContext::Dashboard, Some("t1"), None) + .await; assert!( matches!(refused, Err(AuthError::ReauthenticationRequired)), "a stolen token alone must not enrol a factor, got {refused:?}" @@ -3347,7 +3583,9 @@ async fn enrolment_on_a_passwordless_account_takes_a_recent_authentication_inste // …and after a real sign-in, the same call proceeds — the gate is a delay, not a wall. plant_recent_auth(&service, &uid).await; - let enrolled = service.setup(&uid, MfaContext::Dashboard, None).await; + let enrolled = service + .setup(&uid, MfaContext::Dashboard, Some("t1"), None) + .await; assert!( enrolled.is_ok(), "a recently authenticated OAuth account must still be able to enrol, got {enrolled:?}" @@ -3590,7 +3828,10 @@ async fn reset_mfa_removes_the_factor_without_a_code_and_tells_the_owner() { return; }; let Some(mfa) = h.engine.mfa() else { return }; - let Ok(setup) = mfa.setup(&uid, MfaContext::Dashboard, Some(PASSWORD)).await else { + let Ok(setup) = mfa + .setup(&uid, MfaContext::Dashboard, Some("t1"), Some(PASSWORD)) + .await + else { return; }; assert!( @@ -3599,13 +3840,18 @@ async fn reset_mfa_removes_the_factor_without_a_code_and_tells_the_owner() { &code(&setup.secret, 0), "1.2.3.4", "ua", - MfaContext::Dashboard + MfaContext::Dashboard, + Some("t1"), ) .await .is_ok() ); - assert!(mfa.reset_mfa(&uid, MfaContext::Dashboard).await.is_ok()); + assert!( + mfa.reset_mfa(&uid, MfaContext::Dashboard, Some("t1")) + .await + .is_ok() + ); // The factor is actually gone, not merely reported gone: `disable` answers "not enabled", // which it can only do by reading the record back. @@ -3615,7 +3861,8 @@ async fn reset_mfa_removes_the_factor_without_a_code_and_tells_the_owner() { &code(&setup.secret, 30), "1.2.3.4", "ua", - MfaContext::Dashboard + MfaContext::Dashboard, + Some("t1"), ) .await, Err(AuthError::MfaNotEnabled) @@ -3646,12 +3893,21 @@ async fn reset_mfa_is_idempotent_and_refuses_an_unknown_subject() { let Some(mfa) = h.engine.mfa() else { return }; // No second factor was ever enrolled. - assert!(mfa.reset_mfa(&uid, MfaContext::Dashboard).await.is_ok()); + assert!( + mfa.reset_mfa(&uid, MfaContext::Dashboard, Some("t1")) + .await + .is_ok() + ); // And again, for the retry. - assert!(mfa.reset_mfa(&uid, MfaContext::Dashboard).await.is_ok()); + assert!( + mfa.reset_mfa(&uid, MfaContext::Dashboard, Some("t1")) + .await + .is_ok() + ); assert!(matches!( - mfa.reset_mfa("nobody-at-all", MfaContext::Dashboard).await, + mfa.reset_mfa("nobody-at-all", MfaContext::Dashboard, Some("t1")) + .await, Err(AuthError::MfaNotEnabled) )); } diff --git a/crates/bymax-auth-redis/tests/mfa_lifecycle_e2e.rs b/crates/bymax-auth-redis/tests/mfa_lifecycle_e2e.rs index 6e97788..bd05ef1 100644 --- a/crates/bymax-auth-redis/tests/mfa_lifecycle_e2e.rs +++ b/crates/bymax-auth-redis/tests/mfa_lifecycle_e2e.rs @@ -132,7 +132,12 @@ async fn full_lifecycle_against_real_redis() { // setup → enable. Compute the lifecycle's distinct TOTP codes from one captured base // (steps s, s+1, s+2, s-1), so they never collide as the clock advances mid-test. - let Ok(setup) = mfa.setup(&uid, MfaContext::Dashboard, None).await else { return }; + let Ok(setup) = mfa + .setup(&uid, MfaContext::Dashboard, Some("t1"), None) + .await + else { + return; + }; assert_eq!(setup.recovery_codes.len(), 8); let base = now_secs(); let enable_code = code_at(&setup.secret, base); @@ -140,9 +145,16 @@ async fn full_lifecycle_against_real_redis() { let regen_code = code_at(&setup.secret, base + 60); let disable_code = code_at(&setup.secret, base - 30); assert!( - mfa.verify_and_enable(&uid, &enable_code, "1.2.3.4", "ua", MfaContext::Dashboard) - .await - .is_ok() + mfa.verify_and_enable( + &uid, + &enable_code, + "1.2.3.4", + "ua", + MfaContext::Dashboard, + Some("t1") + ) + .await + .is_ok() ); // challenge via TOTP (a fresh step so the anti-replay marker is new). @@ -175,7 +187,14 @@ async fn full_lifecycle_against_real_redis() { // Regenerate atomically: the old codes are invalidated wholesale. let Ok(fresh) = mfa - .regenerate_recovery_codes(&uid, ®en_code, "1.2.3.4", "ua", MfaContext::Dashboard) + .regenerate_recovery_codes( + &uid, + ®en_code, + "1.2.3.4", + "ua", + MfaContext::Dashboard, + Some("t1"), + ) .await else { return; @@ -191,9 +210,16 @@ async fn full_lifecycle_against_real_redis() { // disable. assert!( - mfa.disable(&uid, &disable_code, "1.2.3.4", "ua", MfaContext::Dashboard) - .await - .is_ok() + mfa.disable( + &uid, + &disable_code, + "1.2.3.4", + "ua", + MfaContext::Dashboard, + Some("t1") + ) + .await + .is_ok() ); // A subsequent login no longer challenges (MFA is off): it succeeds outright. let input = LoginInput { @@ -217,7 +243,12 @@ async fn concurrent_correct_totp_yields_one_session() { let secret; { let Some(mfa) = engine.mfa() else { return }; - let Ok(setup) = mfa.setup(&uid, MfaContext::Dashboard, None).await else { return }; + let Ok(setup) = mfa + .setup(&uid, MfaContext::Dashboard, Some("t1"), None) + .await + else { + return; + }; if mfa .verify_and_enable( &uid, @@ -225,6 +256,7 @@ async fn concurrent_correct_totp_yields_one_session() { "1.2.3.4", "ua", MfaContext::Dashboard, + Some("t1"), ) .await .is_err() @@ -279,7 +311,12 @@ async fn concurrent_distinct_valid_codes_yield_one_session() { let secret; { let Some(mfa) = engine.mfa() else { return }; - let Ok(setup) = mfa.setup(&uid, MfaContext::Dashboard, None).await else { return }; + let Ok(setup) = mfa + .setup(&uid, MfaContext::Dashboard, Some("t1"), None) + .await + else { + return; + }; if mfa .verify_and_enable( &uid, @@ -287,6 +324,7 @@ async fn concurrent_distinct_valid_codes_yield_one_session() { "1.2.3.4", "ua", MfaContext::Dashboard, + Some("t1"), ) .await .is_err() diff --git a/crates/bymax-auth-redis/tests/platform_identity_e2e.rs b/crates/bymax-auth-redis/tests/platform_identity_e2e.rs index 0475d6f..da696fd 100644 --- a/crates/bymax-auth-redis/tests/platform_identity_e2e.rs +++ b/crates/bymax-auth-redis/tests/platform_identity_e2e.rs @@ -304,7 +304,9 @@ async fn platform_mfa_challenge_exchange_issues_a_full_session_against_redis() { let base = now_secs(); // Enrolment re-authenticates: this admin has a password, so it must be re-proved // before a factor is minted. - let setup_result = mfa.setup(&id, MfaContext::Platform, Some(PASSWORD)).await; + let setup_result = mfa + .setup(&id, MfaContext::Platform, None, Some(PASSWORD)) + .await; assert!( setup_result.is_ok(), "platform MFA setup must succeed: {setup_result:?}" @@ -318,7 +320,8 @@ async fn platform_mfa_challenge_exchange_issues_a_full_session_against_redis() { &code_at(&setup.secret, base), "1.2.3.4", "ua", - MfaContext::Platform + MfaContext::Platform, + None, ) .await .is_ok()