diff --git a/migrations/20260621000000_magic_link_request_challenge.sql b/migrations/20260621000000_magic_link_request_challenge.sql new file mode 100644 index 00000000..87a196ac --- /dev/null +++ b/migrations/20260621000000_magic_link_request_challenge.sql @@ -0,0 +1,30 @@ +-- ════════════════════════════════════════════════════════════════════════════ +-- Device-bound magic-link redemption (PR 22) +-- ════════════════════════════════════════════════════════════════════════════ +-- Login-via-email tokens (the ones the user requests themselves from their +-- own browser) now carry a per-request challenge that mirrors a cookie +-- set on the originating browser. On redemption the server compares the +-- inbound cookie against this column: +-- +-- - Cookie present and matches → redeem instantly (common case, zero +-- UX change for the user clicking from the same browser). +-- - Cookie absent or mismatched → show a confirmation page; user +-- clicks Continue to redeem anyway. Audit-logged as +-- `cross_browser_confirmed`. +-- +-- Invitation tokens (the ones a sharer mints for a recipient who has no +-- prior browser context with the server) leave this column NULL — they +-- are cross-device by design and bypass the cookie check entirely. +-- +-- See docs/architecture/magic-link-auth.md and auth-simplification.md +-- (PR 22) for the threat model and full design. + +ALTER TABLE auth.magic_link_tokens + ADD COLUMN request_challenge TEXT NULL; + +COMMENT ON COLUMN auth.magic_link_tokens.request_challenge IS + 'Random per-request value mirrored into the oxicloud_magic_request + cookie on the originating browser. NULL for invitation tokens + (cross-device by design); non-NULL for login-via-email tokens + (browser-bound). Compared on redemption to bind the magic-link to + the device that requested it.'; diff --git a/src/application/services/auth_application_service.rs b/src/application/services/auth_application_service.rs index f5c46cd8..6069a526 100644 --- a/src/application/services/auth_application_service.rs +++ b/src/application/services/auth_application_service.rs @@ -63,6 +63,26 @@ pub enum RegisterResult { EmailTaken, } +/// Outcome of a `redeem_magic_link` call (PR 22). +/// +/// - `Allowed(redemption)` — the token is valid and the browser +/// binding either matched or was overridden via the user's +/// explicit cross-browser confirmation. The token has been +/// atomically marked used. +/// - `NeedsCrossBrowserConfirm` — the token carries a +/// `request_challenge` but the incoming cookie didn't match. +/// The handler should render a confirmation page; the user +/// clicks Continue and we re-redeem with `cross_browser_confirmed = true`. +/// The token is NOT marked used yet — it stays redeemable. +#[derive(Debug)] +pub enum MagicLinkRedeemResult { + /// Boxed to keep the enum's stack size small — `MagicLinkRedemption` + /// is ~350 bytes while `NeedsCrossBrowserConfirm` is zero-sized. + /// One redemption per request; the heap indirection is negligible. + Allowed(Box), + NeedsCrossBrowserConfirm, +} + #[derive(Debug, Clone)] pub struct MagicLinkRedemption { pub auth: AuthResponseDto, @@ -385,7 +405,9 @@ impl AuthApplicationService { is_external = false, "🛂 user registered", ); - Ok(RegisterResult::Created(Box::new(UserDto::from(created_user)))) + Ok(RegisterResult::Created(Box::new(UserDto::from( + created_user, + )))) } /// Create the first admin user during initial system setup. @@ -627,7 +649,17 @@ impl AuthApplicationService { /// /// Returns `ServiceUnavailable` (mapped from `NotImplemented`) when /// the magic-link repo isn't wired — the handler maps that to HTTP 503. - pub async fn redeem_magic_link(&self, token: &str) -> Result { + /// + /// `incoming_challenge` is the value the handler read from the + /// browser's `oxicloud_magic_request` cookie (or `None` if absent). + /// `cross_browser_confirmed` is `true` when the user has clicked + /// through the cross-browser confirmation page (PR 22). + pub async fn redeem_magic_link( + &self, + token: &str, + incoming_challenge: Option<&str>, + cross_browser_confirmed: bool, + ) -> Result { let repo = self.magic_link_repo.as_ref().ok_or_else(|| { DomainError::new( ErrorKind::NotImplemented, @@ -692,6 +724,31 @@ impl AuthApplicationService { )); } + // PR 22 — browser binding for login-via-email tokens. When the + // token carries a `request_challenge`, compare it against the + // cookie the handler extracted. Mismatch surfaces as a + // cross-browser confirmation page (the handler renders the + // HTML); the user clicks Continue and we re-enter with + // `cross_browser_confirmed = true`. Invitation tokens have no + // challenge — they bypass this check entirely (cross-device by + // design). The token is NOT marked used on the prompt path — + // it stays redeemable for the confirm round-trip. + if let Some(expected) = mlt.request_challenge() + && !cross_browser_confirmed + && incoming_challenge != Some(expected) + { + tracing::info!( + target: "audit", + event = "magic_link.cross_browser_prompt", + token_id = %mlt.id(), + user_id = %mlt.user_id(), + incoming_present = incoming_challenge.is_some(), + "🔗 magic-link cross-browser: cookie absent or mismatched for user {}", + mlt.user_id(), + ); + return Ok(MagicLinkRedeemResult::NeedsCrossBrowserConfirm); + } + let consumed = repo.mark_used(mlt.id()).await?; if !consumed { // Either a concurrent redemption beat us, or the row was @@ -759,6 +816,7 @@ impl AuthApplicationService { is_external = user.is_external(), resource_kind = ?mlt.resource_kind(), resource_id = ?mlt.resource_id(), + cross_browser_confirmed = cross_browser_confirmed, ); let auth = AuthResponseDto { @@ -769,11 +827,11 @@ impl AuthApplicationService { expires_in: self.token_service.refresh_token_expiry_secs(), }; - Ok(MagicLinkRedemption { + Ok(MagicLinkRedeemResult::Allowed(Box::new(MagicLinkRedemption { auth, resource_kind: mlt.resource_kind(), resource_id: mlt.resource_id(), - }) + }))) } /// Verifies username/password credentials without creating a session. diff --git a/src/application/services/magic_link_invite_service.rs b/src/application/services/magic_link_invite_service.rs index 3be01b21..dcf909f1 100644 --- a/src/application/services/magic_link_invite_service.rs +++ b/src/application/services/magic_link_invite_service.rs @@ -239,10 +239,15 @@ impl MagicLinkInviteService { Resource::Folder(id) => (MagicLinkResourceKind::Folder, id), Resource::File(id) => (MagicLinkResourceKind::File, id), }; + // Invitation tokens are cross-device by design (recipient has + // no prior browser context with the server) — no challenge + // cookie. Long TTL (default 24h) because recipients may not + // check their email for a while. let token = MagicLinkToken::new( recipient.id(), - self.magic_link_cfg.ttl_hours, + chrono::Duration::hours(self.magic_link_cfg.invite_ttl_hours as i64), Some((kind, resource_id)), + None, ); self.magic_link_repo.create(&token).await?; @@ -273,7 +278,7 @@ impl MagicLinkInviteService { inviter = inviter_username, kind = kind_label, link = link, - ttl = self.magic_link_cfg.ttl_hours, + ttl = self.magic_link_cfg.invite_ttl_hours, now = Utc::now().to_rfc3339(), ); @@ -333,7 +338,21 @@ impl MagicLinkInviteService { /// `no_account`, `oidc_user`, `has_password` — so operators can see the truth /// while the API stays anti-enumeration-safe. A fourth outcome /// `send_failed` is logged at `warn` level when SMTP errors. - pub async fn send_login_link(&self, raw_email: &str) -> Result<(), DomainError> { + /// + /// `request_challenge` is the per-request random value the handler + /// already set as the `oxicloud_magic_request` cookie on the + /// originating browser. The service mirrors it into the token row; + /// the redemption endpoint compares it against the inbound cookie + /// to bind the magic-link to the device that requested it. + /// Anti-enumeration: the handler passes the same challenge whether + /// or not the user exists / is eligible — the token row is just + /// not created in those branches, so nothing is leaked by the + /// presence or absence of the cookie. + pub async fn send_login_link( + &self, + raw_email: &str, + request_challenge: &str, + ) -> Result<(), DomainError> { let normalised = match normalize_email(raw_email) { Ok(n) => n, Err(e) => { @@ -403,9 +422,16 @@ impl MagicLinkInviteService { return Ok(()); } - // Mint a NULL-resource token. The redemption handler lands - // NULL-resource tokens on /#/sharedwithme (see PR 8). - let token = MagicLinkToken::new(user.id(), self.magic_link_cfg.ttl_hours, None); + // Mint a NULL-resource token bound to the requesting browser + // via `request_challenge` (PR 22). Short TTL (default 10 min) + // — the user just clicked the button, so a slow click is + // almost certainly someone else with access to the inbox. + let token = MagicLinkToken::new( + user.id(), + chrono::Duration::minutes(self.magic_link_cfg.login_ttl_minutes as i64), + None, + Some(request_challenge.to_string()), + ); self.magic_link_repo.create(&token).await?; let link = format!( @@ -418,7 +444,8 @@ impl MagicLinkInviteService { "Hello,\n\ \n\ Use the link below to sign in to OxiCloud. The link works \ - once and expires in {ttl} hours.\n\ + once and expires in {ttl} minutes. Open it on the same \ + device where you requested it.\n\ \n\ {link}\n\ \n\ @@ -426,7 +453,7 @@ impl MagicLinkInviteService { ignore this message — no further action is needed.\n\ \n\ — OxiCloud, {now}\n", - ttl = self.magic_link_cfg.ttl_hours, + ttl = self.magic_link_cfg.login_ttl_minutes, link = link, now = Utc::now().to_rfc3339(), ); diff --git a/src/common/config.rs b/src/common/config.rs index f6143a84..c9a512e2 100644 --- a/src/common/config.rs +++ b/src/common/config.rs @@ -688,9 +688,24 @@ impl SmtpConfig { /// the invite-by-email / login-via-email flow. #[derive(Debug, Clone)] pub struct MagicLinkConfig { - /// How long a freshly-minted magic-link token stays valid before the - /// background sweeper marks it expired. Default: 24 hours. - pub ttl_hours: u64, + /// TTL for **login-via-email** tokens (the ones a user requests + /// themselves from their own browser). Short by design — the user + /// just clicked the button moments before; if they take >10 minutes + /// to click the link, something's wrong. Combined with the per- + /// request challenge cookie (PR 22), this bounds the window for + /// mailbox compromise to turn into a session. + /// + /// Default: 10 minutes. + pub login_ttl_minutes: u64, + /// TTL for **invitation** tokens (the ones a sharer mints via + /// `POST /api/grants` for a recipient who has no prior browser + /// context with the server). Long because the recipient may not + /// check their email for hours or days. Cross-device by design; + /// no challenge cookie. + /// + /// Default: 24 hours. The legacy `OXICLOUD_MAGIC_LINK_TTL_HOURS` + /// env var is a deprecated alias that writes here. + pub invite_ttl_hours: u64, /// Kill switch for the whole magic-link flow. When `false`: /// - `POST /api/grants` rejects `subject.type = "email"` for unknown /// email addresses (no lazy external-user creation). @@ -751,7 +766,8 @@ pub struct MagicLinkConfig { impl Default for MagicLinkConfig { fn default() -> Self { Self { - ttl_hours: 24, + login_ttl_minutes: 10, + invite_ttl_hours: 24, allow_external_users: true, allowed_email_domains: Vec::new(), invite_per_caller_per_hour: 50, @@ -1370,11 +1386,32 @@ impl AppConfig { } // Magic-link configuration + // Legacy `OXICLOUD_MAGIC_LINK_TTL_HOURS` is preserved as a + // deprecated alias for `OXICLOUD_MAGIC_LINK_INVITE_TTL_HOURS`. + // Existing deployments keep working with their old env var; + // the new explicit var wins if both are set. if let Ok(v) = env::var("OXICLOUD_MAGIC_LINK_TTL_HOURS") && let Ok(h) = v.parse::() && h > 0 { - config.magic_link.ttl_hours = h; + tracing::warn!( + "OXICLOUD_MAGIC_LINK_TTL_HOURS is deprecated — \ + use OXICLOUD_MAGIC_LINK_INVITE_TTL_HOURS (invitations) \ + and OXICLOUD_MAGIC_LINK_LOGIN_TTL_MINUTES (login-via-email)." + ); + config.magic_link.invite_ttl_hours = h; + } + if let Ok(v) = env::var("OXICLOUD_MAGIC_LINK_INVITE_TTL_HOURS") + && let Ok(h) = v.parse::() + && h > 0 + { + config.magic_link.invite_ttl_hours = h; + } + if let Ok(v) = env::var("OXICLOUD_MAGIC_LINK_LOGIN_TTL_MINUTES") + && let Ok(m) = v.parse::() + && m > 0 + { + config.magic_link.login_ttl_minutes = m; } if let Ok(v) = env::var("OXICLOUD_ALLOW_EXTERNAL_USERS") { config.magic_link.allow_external_users = v.parse::().unwrap_or(true); diff --git a/src/domain/entities/magic_link_token.rs b/src/domain/entities/magic_link_token.rs index 55174527..2ff74364 100644 --- a/src/domain/entities/magic_link_token.rs +++ b/src/domain/entities/magic_link_token.rs @@ -106,22 +106,34 @@ pub struct MagicLinkToken { /// schema-level error guarded by the DB CHECK `magic_link_tokens_resource_pair`. resource_kind: Option, resource_id: Option, + /// Per-request challenge (PR 22). Mirrors the `oxicloud_magic_request` + /// cookie set on the originating browser when the user requests a + /// login-via-email link. Compared on redemption to bind the + /// magic-link to the device that requested it. + /// + /// `Some` for login-via-email tokens (browser-bound); `None` for + /// invitation tokens (cross-device by design — recipient has no + /// prior browser context with the server). + request_challenge: Option, } impl MagicLinkToken { /// Mint a fresh pending token. Generates 32 CSPRNG bytes, encodes them /// URL-safe base64 (no padding), and stamps `issued_at = now`, - /// `expires_at = now + ttl_hours`. + /// `expires_at = now + ttl`. /// /// `resource` is `Some((kind, id))` for invitations (deep-link to a /// specific file/folder) or `None` for login-via-email (lands on - /// `/shared-with-me`). The XOR-on-NULL DB CHECK enforces this - /// invariant; the entity exposes it as a single `Option` for - /// clarity. + /// `/shared-with-me` or `/files` depending on `is_external`). + /// + /// `request_challenge` carries the per-request value mirrored into + /// the originating browser's cookie. Pass `Some` for login-via-email + /// (browser-bound), `None` for invitations (cross-device). pub fn new( user_id: Uuid, - ttl_hours: u64, + ttl: Duration, resource: Option<(MagicLinkResourceKind, Uuid)>, + request_challenge: Option, ) -> Self { let mut bytes = [0u8; 32]; OsRng.fill_bytes(&mut bytes); @@ -139,10 +151,11 @@ impl MagicLinkToken { user_id, status: MagicLinkStatus::Pending, issued_at: now, - expires_at: now + Duration::hours(ttl_hours as i64), + expires_at: now + ttl, used_at: None, resource_kind, resource_id, + request_challenge, } } @@ -158,6 +171,7 @@ impl MagicLinkToken { used_at: Option>, resource_kind: Option, resource_id: Option, + request_challenge: Option, ) -> Self { Self { id, @@ -169,6 +183,7 @@ impl MagicLinkToken { used_at, resource_kind, resource_id, + request_challenge, } } @@ -210,6 +225,14 @@ impl MagicLinkToken { self.resource_id } + /// Per-request challenge for browser binding (PR 22). `Some` for + /// login-via-email tokens — the redemption endpoint compares this + /// with the inbound `oxicloud_magic_request` cookie. `None` for + /// invitation tokens — they bypass the cookie check entirely. + pub fn request_challenge(&self) -> Option<&str> { + self.request_challenge.as_deref() + } + // ── Business logic ─────────────────────────────────────────── /// `true` once `expires_at < now`. The status column may still be @@ -235,7 +258,7 @@ mod tests { #[test] fn new_token_is_pending_and_within_ttl() { let user_id = Uuid::new_v4(); - let token = MagicLinkToken::new(user_id, 24, None); + let token = MagicLinkToken::new(user_id, Duration::hours(24), None, None); assert_eq!(token.status(), MagicLinkStatus::Pending); assert_eq!(token.user_id(), user_id); assert!(token.resource_kind().is_none()); @@ -252,8 +275,9 @@ mod tests { let folder_id = Uuid::new_v4(); let token = MagicLinkToken::new( user_id, - 24, + Duration::hours(24), Some((MagicLinkResourceKind::Folder, folder_id)), + None, ); assert_eq!(token.resource_kind(), Some(MagicLinkResourceKind::Folder)); assert_eq!(token.resource_id(), Some(folder_id)); @@ -262,8 +286,8 @@ mod tests { #[test] fn each_token_is_unique() { let user_id = Uuid::new_v4(); - let a = MagicLinkToken::new(user_id, 24, None); - let b = MagicLinkToken::new(user_id, 24, None); + let a = MagicLinkToken::new(user_id, Duration::hours(24), None, None); + let b = MagicLinkToken::new(user_id, Duration::hours(24), None, None); assert_ne!(a.token(), b.token()); assert_ne!(a.id(), b.id()); } diff --git a/src/infrastructure/repositories/pg/magic_link_token_pg_repository.rs b/src/infrastructure/repositories/pg/magic_link_token_pg_repository.rs index 7061cb70..bc62ac38 100644 --- a/src/infrastructure/repositories/pg/magic_link_token_pg_repository.rs +++ b/src/infrastructure/repositories/pg/magic_link_token_pg_repository.rs @@ -37,6 +37,7 @@ impl MagicLinkTokenPgRepository { let resource_type: Option = row.try_get("resource_type").ok(); let resource_kind = resource_type.and_then(|s| MagicLinkResourceKind::parse(&s)); let resource_id: Option = row.try_get("resource_id").ok(); + let request_challenge: Option = row.try_get("request_challenge").ok(); Ok(MagicLinkToken::from_raw( row.try_get("id").unwrap(), @@ -48,6 +49,7 @@ impl MagicLinkTokenPgRepository { row.try_get("used_at").ok(), resource_kind, resource_id, + request_challenge, )) } } @@ -60,11 +62,13 @@ impl MagicLinkTokenRepository for MagicLinkTokenPgRepository { INSERT INTO auth.magic_link_tokens ( id, token, user_id, status, issued_at, expires_at, used_at, - resource_type, resource_id + resource_type, resource_id, + request_challenge ) VALUES ( $1, $2, $3, $4::auth.magic_link_status, $5, $6, $7, - $8, $9 + $8, $9, + $10 ) "#, ) @@ -77,6 +81,7 @@ impl MagicLinkTokenRepository for MagicLinkTokenPgRepository { .bind(token.used_at()) .bind(token.resource_kind().map(|k| k.as_str())) .bind(token.resource_id()) + .bind(token.request_challenge()) .execute(self.pool.as_ref()) .await .map_err(|e| DomainError::internal_error("MagicLinkToken", format!("insert: {}", e)))?; @@ -88,7 +93,8 @@ impl MagicLinkTokenRepository for MagicLinkTokenPgRepository { r#" SELECT id, token, user_id, status::text AS status, issued_at, expires_at, used_at, - resource_type, resource_id + resource_type, resource_id, + request_challenge FROM auth.magic_link_tokens WHERE token = $1 "#, diff --git a/src/interfaces/api/cookie_auth.rs b/src/interfaces/api/cookie_auth.rs index 950e9d21..6cfb4d52 100644 --- a/src/interfaces/api/cookie_auth.rs +++ b/src/interfaces/api/cookie_auth.rs @@ -24,6 +24,12 @@ pub const REFRESH_COOKIE: &str = "oxicloud_refresh"; pub const CSRF_COOKIE: &str = "oxicloud_csrf"; /// Header the frontend must send with the CSRF token value. pub const CSRF_HEADER: &str = "x-csrf-token"; +/// Per-request challenge cookie for browser-bound magic-link +/// redemption (PR 22). Set by `POST /api/auth/magic-link/send` on +/// the requesting browser; checked by `GET /magic/v1/{token}` against +/// the token row's `request_challenge` column. Limited to `/magic` +/// so it only travels back on the redemption endpoint. +pub const MAGIC_REQUEST_COOKIE: &str = "oxicloud_magic_request"; /// Whether the `Secure` flag should be set on cookies. /// @@ -167,6 +173,47 @@ pub fn append_csrf_cookie(headers: &mut HeaderMap, access_expiry_secs: i64) { } } +/// Generate a per-request challenge for the magic-link browser +/// binding (PR 22). 128-bit UUIDv4 — same shape as `generate_csrf_token`, +/// plenty of entropy to make brute-force matching infeasible during +/// the 10-minute login TTL. The value is set as a cookie on the +/// originating browser AND mirrored into the token row so the +/// redemption endpoint can compare them. +pub fn generate_magic_request_challenge() -> String { + uuid::Uuid::new_v4().to_string() +} + +/// Append the `oxicloud_magic_request` cookie that binds a +/// login-via-email magic-link to the originating browser (PR 22). +/// HttpOnly + SameSite=Strict + Path=/magic — only sent back when +/// the user clicks the redemption link, never on cross-site +/// navigations. `value` is a random URL-safe string the handler +/// also mirrors into `auth.magic_link_tokens.request_challenge`. +pub fn append_magic_request_cookie(headers: &mut HeaderMap, value: &str, max_age_secs: i64) { + if let Ok(val) = HeaderValue::from_str(&build_cookie( + MAGIC_REQUEST_COOKIE, + value, + "/magic", + max_age_secs, + "Strict", + )) { + headers.append(SET_COOKIE, val); + } +} + +/// Clear the `oxicloud_magic_request` cookie after redemption — the +/// challenge is single-use, so we don't want a stale cookie on the +/// browser confusing a later flow. +pub fn append_clear_magic_request_cookie(headers: &mut HeaderMap) { + let secure = if cookie_secure() { "; Secure" } else { "" }; + let val = format!( + "{MAGIC_REQUEST_COOKIE}=; HttpOnly; SameSite=Strict; Path=/magic; Max-Age=0{secure}", + ); + if let Ok(hv) = HeaderValue::from_str(&val) { + headers.append(SET_COOKIE, hv); + } +} + /// Clear the CSRF cookie (on logout). pub fn append_clear_csrf_cookie(headers: &mut HeaderMap) { let secure = if cookie_secure() { "; Secure" } else { "" }; diff --git a/src/interfaces/api/handlers/auth_handler.rs b/src/interfaces/api/handlers/auth_handler.rs index de206f1c..967156be 100644 --- a/src/interfaces/api/handlers/auth_handler.rs +++ b/src/interfaces/api/handlers/auth_handler.rs @@ -175,11 +175,14 @@ pub async fn register( match result { RegisterResult::Created(user) => { - // Email-only signup: dispatch the welcome magic-link. - // Best-effort — SMTP failures don't roll back the user. + // Email-only signup: dispatch the welcome magic-link with + // a fresh browser-binding challenge (PR 22). Best-effort — + // SMTP failures don't roll back the user. + let challenge = cookie_auth::generate_magic_request_challenge(); + let login_ttl_secs = (state.core.config.magic_link.login_ttl_minutes * 60) as i64; if was_passwordless && let Some(invite) = state.magic_link_invite_service.as_ref() - && let Err(e) = invite.send_login_link(&email).await + && let Err(e) = invite.send_login_link(&email, &challenge).await { tracing::warn!( target: "audit", @@ -192,8 +195,19 @@ pub async fn register( } if smtp_enabled { // Anti-enumeration mode: hide success-vs-collision behind - // the uniform "check your email" cover story. - Ok(uniform_ok()) + // the uniform "check your email" cover story. Attach the + // browser-binding challenge cookie on every email-only + // path — preserves the "did a mail go out" anti-enum + // property at the cookie level too. + let mut resp = uniform_ok(); + if was_passwordless { + cookie_auth::append_magic_request_cookie( + resp.headers_mut(), + &challenge, + login_ttl_secs, + ); + } + Ok(resp) } else { // Classic mode: clear 201 + UserDto so the frontend can // log the user in directly with the password they just @@ -1076,11 +1090,28 @@ pub async fn send_magic_link( ) })?; + // Per-request browser-binding challenge (PR 22). Generated for + // every request and set as a cookie on every 200 response — + // including the silent-rate-limit paths — so the cookie's + // presence is uniform and can't be used as an enumeration oracle. + // The corresponding token row only carries the challenge when a + // token is actually minted; cookie-without-token simply fails to + // match on the eventual redemption. + let challenge = cookie_auth::generate_magic_request_challenge(); + let login_ttl_secs = (state.core.config.magic_link.login_ttl_minutes * 60) as i64; + let challenge_for_closure = challenge.clone(); + let uniform_ok = || { let payload = serde_json::json!({ "message": "If an account exists for that email, a sign-in link will be sent.", }); - (StatusCode::OK, Json(payload)).into_response() + let mut resp = (StatusCode::OK, Json(payload)).into_response(); + cookie_auth::append_magic_request_cookie( + resp.headers_mut(), + &challenge_for_closure, + login_ttl_secs, + ); + resp }; if !is_authenticated { @@ -1128,7 +1159,7 @@ pub async fn send_magic_link( // via the audit channel; we surface only an internal error (DB down, // etc.). Anti-enumeration means we always return the same body. invite_svc - .send_login_link(&body.email) + .send_login_link(&body.email, &challenge) .await .map_err(AppError::from)?; diff --git a/src/interfaces/api/handlers/magic_link_handler.rs b/src/interfaces/api/handlers/magic_link_handler.rs index 84d69166..6f1c06e2 100644 --- a/src/interfaces/api/handlers/magic_link_handler.rs +++ b/src/interfaces/api/handlers/magic_link_handler.rs @@ -25,13 +25,16 @@ use std::sync::Arc; use axum::{ Router, - extract::{Path, State}, - http::{HeaderValue, StatusCode, header::CONTENT_TYPE, header::LOCATION}, + extract::{Path, Query, State}, + http::{HeaderMap, HeaderValue, StatusCode, header::CONTENT_TYPE, header::LOCATION}, response::{IntoResponse, Response}, routing::get, }; +use serde::Deserialize; -use crate::application::services::auth_application_service::MagicLinkRedemption; +use crate::application::services::auth_application_service::{ + MagicLinkRedeemResult, MagicLinkRedemption, +}; use crate::common::di::AppState; use crate::common::errors::ErrorKind; use crate::domain::entities::magic_link_token::MagicLinkResourceKind; @@ -44,11 +47,21 @@ pub fn magic_link_routes() -> Router> { Router::new().route("/magic/v1/{token}", get(redeem_magic_link)) } +#[derive(Debug, Deserialize)] +struct RedeemQuery { + /// PR 22: `?confirm=1` means the user clicked the cross-browser + /// confirmation prompt's Continue button. The service skips the + /// challenge-cookie check on this re-entry. + #[serde(default)] + confirm: Option, +} + #[utoipa::path( get, path = "/magic/v1/{token}", params(("token" = String, Path, description = "Opaque magic-link token")), responses( + (status = 200, description = "Cross-browser confirmation prompt (HTML page)"), (status = 302, description = "Redemption succeeded — redirects to the resource or to /#/sharedwithme"), (status = 410, description = "Token is unknown, expired, or already used"), (status = 503, description = "Magic-link feature is not configured on this server"), @@ -58,6 +71,8 @@ pub fn magic_link_routes() -> Router> { async fn redeem_magic_link( State(state): State>, Path(token): Path, + Query(query): Query, + headers: HeaderMap, ) -> Response { let Some(auth_svc) = state.auth_service.as_ref() else { return error_page( @@ -66,12 +81,35 @@ async fn redeem_magic_link( ); }; + // PR 22 browser binding: read the per-request challenge from the + // cookie (set by `POST /api/auth/magic-link/send` on the originating + // browser). The service compares it to the token's stored + // challenge. `confirm=1` means the user just clicked through the + // cross-browser prompt and is fine redeeming from a different + // browser anyway. + let incoming_challenge = + cookie_auth::extract_cookie_value(&headers, cookie_auth::MAGIC_REQUEST_COOKIE); + let cross_browser_confirmed = query + .confirm + .as_deref() + .map(|v| v == "1" || v == "true") + .unwrap_or(false); + match auth_svc .auth_application_service - .redeem_magic_link(&token) + .redeem_magic_link( + &token, + incoming_challenge.as_deref(), + cross_browser_confirmed, + ) .await { - Ok(redemption) => build_success_response(&state, redemption), + Ok(MagicLinkRedeemResult::Allowed(redemption)) => { + build_success_response(&state, *redemption) + } + Ok(MagicLinkRedeemResult::NeedsCrossBrowserConfirm) => { + cross_browser_confirmation_page(&token) + } Err(e) => { // Log the cause for ops; the user gets a generic page so the // outcome can't be used as an enumeration oracle. @@ -113,10 +151,53 @@ fn build_success_response(state: &Arc, redemption: MagicLinkRedemption state.core.config.auth.refresh_token_expiry_secs, ); cookie_auth::append_csrf_cookie(response.headers_mut(), redemption.auth.expires_in); + // Clear the request-challenge cookie — it's single-use and we don't + // want a stale value on the browser confusing a later flow. + cookie_auth::append_clear_magic_request_cookie(response.headers_mut()); response } +/// Render the cross-browser confirmation page (PR 22). Shown when the +/// magic-link token carries a `request_challenge` (login-via-email) +/// but the inbound cookie didn't match — typically because the user +/// requested the link from one browser and clicked it from another +/// (phone vs desktop, work vs personal). The Continue button submits +/// back to the same endpoint with `?confirm=1` so the service skips +/// the challenge check and proceeds with redemption. Audit-logged at +/// `magic_link.redeemed reason="cross_browser_confirmed"`. +fn cross_browser_confirmation_page(token: &str) -> Response { + let confirm_url = format!("/magic/v1/{}?confirm=1", html_escape(token)); + let body = format!( + "\ + Sign in — OxiCloud\ + \ + \ +

Continue signing in on this device?

\ +

You opened this sign-in link in a different browser or device than \ + the one where you requested it.

\ +

If you requested this link, it's safe to continue. \ + If you didn't request it, close this page — clicking Continue would sign \ + someone else into your account.

\ +

Continue and sign in

\ + ", + confirm_url = confirm_url, + ); + + let mut response = (StatusCode::OK, body).into_response(); + response.headers_mut().insert( + CONTENT_TYPE, + HeaderValue::from_static("text/html; charset=utf-8"), + ); + response +} + /// Build the SPA hash-route the redemption should land on. Mirrors the /// front-end's `deserializeHash()` parser at `static/js/app/main.js`. /// diff --git a/tests/api/external_users.hurl b/tests/api/external_users.hurl index 61ece2e4..d07b42f3 100644 --- a/tests/api/external_users.hurl +++ b/tests/api/external_users.hurl @@ -353,10 +353,25 @@ jsonpath "$.text_body" matches "/magic/v1/[A-Za-z0-9_-]+" [Captures] login_magic_url: jsonpath "$.text_body" regex "(https?://[^\\s]+/magic/v1/[A-Za-z0-9_-]+)" -# 15c — Redeem the login link. Lands on /#/sharedwithme since the -# token has no resource target. +# 15c-i — PR 22: the token is browser-bound. Hitting the +# redemption URL without the matching cookie shows the +# cross-browser confirmation page (200 + HTML) rather +# than redeeming. Audit-logs `magic_link.cross_browser_prompt`. +# The token is NOT marked used on this branch. GET {{login_magic_url}} +HTTP 200 +[Asserts] +header "content-type" startsWith "text/html" +body contains "different browser" + +# 15c-ii — Same token, with `?confirm=1` to acknowledge the +# cross-browser redemption. PR 22 audit-logs +# `cross_browser_confirmed=true` on the success line. +# Lands on /#/sharedwithme since the token has no +# resource target. +GET {{login_magic_url}}?confirm=1 + HTTP 302 [Asserts] header "Location" == "/#/sharedwithme" diff --git a/tests/api/registration.hurl b/tests/api/registration.hurl index 6ed108d6..925de708 100644 --- a/tests/api/registration.hurl +++ b/tests/api/registration.hurl @@ -70,6 +70,12 @@ Content-Type: application/json } HTTP 200 +[Captures] +# PR 22 — capture the browser-binding cookie so the redemption can +# replay it. Hurl's automatic cookie jar doesn't reliably attach +# Path-scoped cookies in this test setup, so we wire it through +# explicitly via the Set-Cookie header. +pr18_magic_cookie: header "set-cookie" regex "oxicloud_magic_request=([^;]+)" [Asserts] jsonpath "$.message" contains "request received" @@ -89,13 +95,27 @@ pr18_magic_url: jsonpath "$.text_body" regex "(https?://[^\\s]+/magic/v1/[A-Za-z # ───────────────────────────────────────────────────────────── -# Step 5 — Redeem the welcome link. Internal user with no -# resource target → lands on `/#/files` (NOT -# `/#/sharedwithme`, which is the external-user -# landing). +# Step 5a — Redeem the welcome link WITHOUT the browser-binding +# cookie. PR 22 shows the cross-browser confirmation +# page (HTTP 200, HTML) rather than redeeming. # ───────────────────────────────────────────────────────────── GET {{pr18_magic_url}} +HTTP 200 +[Asserts] +header "content-type" startsWith "text/html" +body contains "different browser" + + +# ───────────────────────────────────────────────────────────── +# Step 5b — Same link, this time with the matching cookie. +# PR 22 binds the magic-link to the requesting browser; +# a matching cookie redeems instantly. Internal user +# with no resource target → lands on `/#/files`. +# ───────────────────────────────────────────────────────────── +GET {{pr18_magic_url}} +Cookie: oxicloud_magic_request={{pr18_magic_cookie}} + HTTP 302 [Asserts] header "Location" == "/#/files"