From 74d22629621c89a849941ac90e6d8d38374de2f8 Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Mon, 2 Nov 2026 21:27:46 -0500 Subject: [PATCH 01/17] test: reproduce cross-site body-less posts reaching cookie routes A body-less POST with Sec-Fetch-Site same-site or absent reaches a cookie-authenticated route under SameSite=Lax, the origin check never runs outside SameSite=None, and an empty configured allowlist is read as not configured. Refs #24 --- src/tests/mod.rs | 1 + src/tests/origin.rs | 138 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 139 insertions(+) create mode 100644 src/tests/origin.rs diff --git a/src/tests/mod.rs b/src/tests/mod.rs index e583f52..a1c1385 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -7,6 +7,7 @@ mod guard; mod magic_link; mod manifest; mod notices; +mod origin; mod recovery; mod verifier; diff --git a/src/tests/origin.rs b/src/tests/origin.rs new file mode 100644 index 0000000..9fe08d9 --- /dev/null +++ b/src/tests/origin.rs @@ -0,0 +1,138 @@ +//! Cross-site state changes against cookie-authenticated routes (#24). + +use serde_json::json; + +use super::*; +use crate::SameSite; + +fn access() -> String { + signed_cookie("seamless-access", json!({ "sub": "u1", "token": "t" })) +} + +/// A body-less POST to a cookie-authenticated route, as an HTML form on another +/// site can send it. +fn form_post() -> Call { + call("POST", "/recovery-codes").cookie(&access()) +} + +async fn started() -> FakeApi { + let api = FakeApi::start().await; + api.on( + "POST", + "/recovery-codes", + respond(200, json!({ "recoveryCodes": ["a"] })), + ); + api +} + +#[tokio::test] +async fn a_same_site_form_post_under_lax_cookies_is_refused() { + let api = started().await; + let adapter = api.adapter(|b| b.same_site(SameSite::Lax)); + + let r = form_post() + .header("sec-fetch-site", "same-site") + .header("origin", "https://evil.example.com") + .to(&adapter) + .await; + + assert_eq!(r.status, 415, "{}", r.text); + assert!(api.calls_to("POST", "/recovery-codes").is_empty()); +} + +#[tokio::test] +async fn a_form_post_without_fetch_metadata_is_refused() { + let api = started().await; + let adapter = api.adapter(|b| b.same_site(SameSite::Lax)); + + let r = form_post() + .header("origin", "https://evil.example") + .to(&adapter) + .await; + + assert_eq!(r.status, 415, "{}", r.text); + assert!(api.calls_to("POST", "/recovery-codes").is_empty()); +} + +#[tokio::test] +async fn the_origin_check_runs_whatever_the_same_site_setting() { + let api = started().await; + for same_site in [SameSite::Lax, SameSite::Strict] { + let adapter = api.adapter(|b| { + b.same_site(same_site) + .allowed_origins(["https://app.example.com"]) + }); + + let r = form_post() + .header("sec-fetch-site", "cross-site") + .header("content-type", "application/json") + .to(&adapter) + .await; + assert_eq!(r.status, 403, "{same_site:?} cross-site: {}", r.text); + + let r = form_post() + .header("sec-fetch-site", "same-site") + .header("origin", "https://evil.example.com") + .header("content-type", "application/json") + .to(&adapter) + .await; + assert_eq!(r.status, 403, "{same_site:?} same-site: {}", r.text); + + let r = form_post() + .header("origin", "https://evil.example.com") + .header("content-type", "application/json") + .to(&adapter) + .await; + assert_eq!(r.status, 403, "{same_site:?} no fetch metadata: {}", r.text); + } + assert!(api.calls_to("POST", "/recovery-codes").is_empty()); +} + +#[tokio::test] +async fn an_empty_configured_allowlist_is_strict() { + let api = started().await; + let adapter = api.adapter(|b| b.allowed_origins(Vec::::new())); + + let r = form_post() + .header("sec-fetch-site", "same-site") + .header("origin", "https://evil.example.com") + .header("content-type", "application/json") + .to(&adapter) + .await; + assert_eq!(r.status, 403, "same-site: {}", r.text); + + let r = form_post() + .header("origin", "https://evil.example.com") + .header("content-type", "application/json") + .to(&adapter) + .await; + assert_eq!(r.status, 403, "no fetch metadata: {}", r.text); + + assert!(api.calls_to("POST", "/recovery-codes").is_empty()); +} + +#[tokio::test] +async fn a_body_less_unsafe_request_passes_with_json_or_the_transport_header() { + let api = started().await; + let adapter = api.adapter(|b| b); + + let r = form_post() + .header("sec-fetch-site", "same-origin") + .header("content-type", "application/json") + .to(&adapter) + .await; + assert_eq!(r.status, 200, "json: {}", r.text); + + let r = form_post() + .header("sec-fetch-site", "same-origin") + .header("x-seamless-auth-transport", "cookie") + .to(&adapter) + .await; + assert_eq!(r.status, 200, "transport header: {}", r.text); + + let r = form_post() + .header("sec-fetch-site", "same-origin") + .to(&adapter) + .await; + assert_eq!(r.status, 415, "neither: {}", r.text); +} From 7931898175c8bc60ffbe1a1f80e7afe7a24fedb2 Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Mon, 2 Nov 2026 21:29:24 -0500 Subject: [PATCH 02/17] fix: check Origin on unsafe methods whatever the SameSite setting The cross-site check ran only with SameSite=None cookies, and the JSON content type was required only when a body was present, so a body-less form POST from a sibling subdomain or a browser without fetch metadata reached cookie-authenticated routes. - run the origin check on unsafe methods for every SameSite setting - when allowed_origins is set, enforce it for same-site requests and requests without fetch metadata, and treat an empty list as strict - require Content-Type: application/json or x-seamless-auth-transport on unsafe manifest routes even with no body, which forces a preflight Refs #24 --- README.md | 13 +++++++++---- src/adapter.rs | 2 +- src/options.rs | 16 +++++++++------- src/origin.rs | 29 +++++++++++++---------------- src/routes.rs | 15 +++++++++++---- src/tests/adapter.rs | 5 ++++- src/tests/magic_link.rs | 5 ++++- src/tests/notices.rs | 1 + 8 files changed, 52 insertions(+), 34 deletions(-) diff --git a/README.md b/README.md index c40f4bc..a5ef09f 100644 --- a/README.md +++ b/README.md @@ -86,8 +86,9 @@ state come from that verified token, so a cookie forged with a leaked cookie sec claims of its own. When the JWKS cannot be fetched the session is refused. A request the guard admits on the session cookie also gets the auth routes' cross-site check. It -applies only while cookies are `SameSite=None`, and only to state-changing methods (not `GET`, -`HEAD` or `OPTIONS`); a refused request answers 403 `cross_site_request_blocked`: +applies whatever the cookies' `SameSite` setting (a sibling subdomain is same-site, so `Lax` and +`Strict` cookies still reach it), and only to state-changing methods (not `GET`, `HEAD` or +`OPTIONS`); a refused request answers 403 `cross_site_request_blocked`: - `Sec-Fetch-Site: cross-site` is refused, and `same-origin` (or `none`) passes. - `Sec-Fetch-Site: same-site` also covers a sibling subdomain, so when `allowed_origins` is set @@ -95,6 +96,7 @@ applies only while cookies are `SameSite=None`, and only to state-changing metho `allowed_origins` it passes. - With no `Sec-Fetch-Site` (an older browser or a server), a request with no `Origin` passes, a `null` `Origin` is refused, and when `allowed_origins` is set the `Origin` must be on it. +- An empty `allowed_origins` is still set: it allows no cross-origin caller. A bearer token is never attached by a browser, so it is not checked. @@ -138,7 +140,10 @@ get(|user: Option| async move { /* signed in or not */ }) The auth routes forward a request body to the auth API as JSON, so a body must be sent as `application/json` (or a `+json` type). Anything else answers 415 `unsupported_media_type`: a cross-site form can post a `text/plain` body shaped like JSON with no CORS preflight, and read as -JSON it would be a sign-in the user never made. The client SDKs already send JSON. +JSON it would be a sign-in the user never made. A state-changing request with no body (`POST`, +`PUT`, `PATCH` or `DELETE`) to a manifest route must still send `Content-Type: application/json` or the +`x-seamless-auth-transport` header, or it answers 415 too: either one forces a CORS preflight, +which an HTML form on another site, sibling subdomains included, cannot send. ### Errors and logout @@ -163,7 +168,7 @@ API cannot be reached. | `cookie_domain` | none | Cookie `Domain` | | `insecure_cookies` | `false` | Drops `Secure`, for local development over HTTP, and allows example secrets | | `same_site` | `None`, or `Lax` with `insecure_cookies` | Cookie `SameSite` | -| `allowed_origins` | none | The only cross-origin (including same-site) callers allowed to change state while cookies are `SameSite=None` | +| `allowed_origins` | none | The only cross-origin (including same-site) callers allowed to change state; an empty list allows none | | `session_cookie_names` | `seamless-access`, `seamless-refresh` | Session cookie names | | `flow_cookie_names` | `seamless-ephemeral` | Sign-in flow cookie names | | `deliver` | none | Sends OTP codes and magic links through your own transports | diff --git a/src/adapter.rs b/src/adapter.rs index 9bcfa18..17ff24d 100644 --- a/src/adapter.rs +++ b/src/adapter.rs @@ -24,7 +24,7 @@ use crate::origin::check_origin; use crate::refresh::Refresher; use crate::service_token::ServiceTokens; -const TRANSPORT_HEADER: &str = "x-seamless-auth-transport"; +pub(crate) const TRANSPORT_HEADER: &str = "x-seamless-auth-transport"; pub(crate) const DELIVERY_MODE_HEADER: &str = "x-seamless-auth-delivery-mode"; pub(crate) const DELIVERY_KINDS_HEADER: &str = "x-seamless-auth-delivery-kinds"; pub(crate) const MAX_BODY_BYTES: usize = 1 << 20; diff --git a/src/options.rs b/src/options.rs index 80cdeca..c5f50c5 100644 --- a/src/options.rs +++ b/src/options.rs @@ -116,7 +116,7 @@ pub struct Builder { cookie_domain: Option, insecure_cookies: bool, same_site: Option, - allowed_origins: Vec, + allowed_origins: Option>, access_cookie_name: String, registration_cookie_name: String, refresh_cookie_name: String, @@ -149,7 +149,7 @@ impl Builder { cookie_domain: None, insecure_cookies: false, same_site: None, - allowed_origins: Vec::new(), + allowed_origins: None, access_cookie_name: "seamless-access".into(), registration_cookie_name: "seamless-ephemeral".into(), refresh_cookie_name: "seamless-refresh".into(), @@ -219,15 +219,16 @@ impl Builder { self } - /// The only cross-origin callers allowed to change state while cookies are - /// `SameSite=None`. With it set, a `Sec-Fetch-Site: same-site` request (which - /// includes a sibling subdomain) needs an `Origin` on the list as well. + /// The only cross-origin callers allowed to change state. With it set, a + /// `Sec-Fetch-Site: same-site` request (which includes a sibling subdomain), + /// or a request without fetch metadata that carries an `Origin`, needs that + /// `Origin` on the list. An empty list allows no cross-origin caller. pub fn allowed_origins(mut self, origins: I) -> Self where I: IntoIterator, S: Into, { - self.allowed_origins = origins.into_iter().map(Into::into).collect(); + self.allowed_origins = Some(origins.into_iter().map(Into::into).collect()); self } @@ -438,7 +439,8 @@ pub(crate) struct Config { pub cookie_domain: Option, pub secure_cookies: bool, pub same_site: SameSite, - pub allowed_origins: Vec, + /// `None` when not configured; an empty list is configured and allows none. + pub allowed_origins: Option>, pub access_cookie_name: String, pub registration_cookie_name: String, pub refresh_cookie_name: String, diff --git a/src/origin.rs b/src/origin.rs index ceb6fac..5af7062 100644 --- a/src/origin.rs +++ b/src/origin.rs @@ -1,14 +1,11 @@ use axum::http::{HeaderMap, Method, StatusCode}; use crate::adapter::Reply; -use crate::options::{Config, SameSite}; +use crate::options::Config; -/// Blocks cross-site state changes while cookies are `SameSite=None`, which is -/// when the browser would otherwise attach them to such a request. +/// Blocks cross-site state changes whatever the cookies' `SameSite` setting: a +/// sibling subdomain is same-site, so `Lax` and `Strict` cookies still reach it. pub(crate) fn check_origin(config: &Config, method: &Method, headers: &HeaderMap) -> Option { - if config.same_site != SameSite::None { - return None; - } if matches!(*method, Method::GET | Method::HEAD | Method::OPTIONS) { return None; } @@ -25,13 +22,14 @@ pub(crate) fn check_origin(config: &Config, method: &Method, headers: &HeaderMap if site.eq_ignore_ascii_case(b"cross-site") { return blocked(); } - // A sibling subdomain is same-site too, so with an allowlist the Origin - // has to be on it. - if !site.eq_ignore_ascii_case(b"same-site") || config.allowed_origins.is_empty() { + if !site.eq_ignore_ascii_case(b"same-site") { return None; } + let Some(allowed) = &config.allowed_origins else { + return None; + }; return match headers.get("origin") { - Some(origin) if is_allowed(config, origin.as_bytes()) => None, + Some(origin) if is_allowed(allowed, origin.as_bytes()) => None, _ => blocked(), }; } @@ -44,21 +42,20 @@ pub(crate) fn check_origin(config: &Config, method: &Method, headers: &HeaderMap if origin.to_str().is_err() { return blocked(); } - if config.allowed_origins.is_empty() || is_allowed(config, origin.as_bytes()) { - return None; + match &config.allowed_origins { + Some(allowed) if !is_allowed(allowed, origin.as_bytes()) => blocked(), + _ => None, } - blocked() } /// Whether an Origin is on the allowlist. An Origin, or an allowlist entry, that /// does not normalize never matches, so two unparseable values cannot pass for /// one another. -fn is_allowed(config: &Config, origin: &[u8]) -> bool { +fn is_allowed(allowed: &[String], origin: &[u8]) -> bool { let Some(origin) = std::str::from_utf8(origin).ok().and_then(normalize_origin) else { return false; }; - config - .allowed_origins + allowed .iter() .filter_map(|allowed| normalize_origin(allowed)) .any(|allowed| allowed == origin) diff --git a/src/routes.rs b/src/routes.rs index 9a8b8f8..a449ed3 100644 --- a/src/routes.rs +++ b/src/routes.rs @@ -9,8 +9,8 @@ use axum::http::{ use serde_json::{Map, Value, json}; use crate::adapter::{ - BodyError, Forwarded, Inner, Reply, UpstreamCall, bearer_token, failure, is_json_content_type, - read_json, read_request_body, + BodyError, Forwarded, Inner, Reply, TRANSPORT_HEADER, UpstreamCall, bearer_token, failure, + is_json_content_type, read_json, read_request_body, }; use crate::cookies::{ CookieWrite, cookie_kind, ephemeral_cookie, raw_cookie, read_cookie, session_cookies, @@ -190,8 +190,15 @@ impl Inner { }; // The body goes upstream as JSON, so it has to be JSON. A cross-site form // can send a text/plain body shaped like JSON with no CORS preflight; read - // as JSON, that would be a sign-in the victim never made. - if body.is_some() && !is_explicit_json(parts.headers.get(CONTENT_TYPE)) { + // as JSON, that would be a sign-in the victim never made. A body-less + // state change needs the JSON type or the transport header all the same: + // either forces a preflight, which a form on a sibling subdomain cannot + // pass, and a form can send no body at all. + let json = is_explicit_json(parts.headers.get(CONTENT_TYPE)); + let unsafe_method = !matches!(parts.method, Method::GET | Method::HEAD | Method::OPTIONS); + if (body.is_some() && !json) + || (unsafe_method && !json && !parts.headers.contains_key(TRANSPORT_HEADER)) + { return Reply::error(StatusCode::UNSUPPORTED_MEDIA_TYPE, "unsupported_media_type"); } diff --git a/src/tests/adapter.rs b/src/tests/adapter.rs index 4fa1a13..2fde5eb 100644 --- a/src/tests/adapter.rs +++ b/src/tests/adapter.rs @@ -428,6 +428,7 @@ async fn delivery_routes_hand_the_message_to_the_application() { }); let r = call("POST", "/otp/generate-login-email-otp") + .header("content-type", "application/json") .cookie(&pre_auth()) .to(&adapter) .await; @@ -505,6 +506,7 @@ async fn a_failed_delivery_is_reported() { let adapter = api.adapter(|b| b.deliver(|_| async { Err::<(), _>("smtp down") })); let r = call("POST", "/magic-link") + .header("content-type", "application/json") .cookie(&pre_auth()) .to(&adapter) .await; @@ -523,6 +525,7 @@ async fn without_deliver_the_api_sends_the_message() { ); call("POST", "/magic-link") + .header("content-type", "application/json") .cookie(&pre_auth()) .to(&api.adapter(|b| b)) .await; @@ -755,7 +758,7 @@ async fn the_origin_guard_follows_sec_fetch_site() { &lax, "POST", &[("sec-fetch-site", "cross-site")], - false, + true, ), ( "same-site from a listed Origin", diff --git a/src/tests/magic_link.rs b/src/tests/magic_link.rs index 2385d9b..7796ab6 100644 --- a/src/tests/magic_link.rs +++ b/src/tests/magic_link.rs @@ -108,7 +108,10 @@ async fn confirm_is_proxied_without_a_credential() { let confirm = format!("{VERIFY}/confirm"); api.on("POST", &confirm, respond(200, ok())); - let r = call("POST", &confirm).to(&api.adapter(|b| b)).await; + let r = call("POST", &confirm) + .header("content-type", "application/json") + .to(&api.adapter(|b| b)) + .await; assert_eq!(r.status, 200, "{}", r.text); assert_eq!(r.body, ok()); diff --git a/src/tests/notices.rs b/src/tests/notices.rs index 8d06cfa..315dd30 100644 --- a/src/tests/notices.rs +++ b/src/tests/notices.rs @@ -283,6 +283,7 @@ async fn an_unknown_delivery_kind_is_skipped() { .await; assert_eq!(r.status, 200, "{}", r.text); let r = call("POST", "/otp/generate-login-email-otp") + .header("content-type", "application/json") .cookie(&pre_auth()) .to(&adapter) .await; From 120a428d7c9dc87faff74f29be880c96ae55f139 Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Mon, 2 Nov 2026 21:29:32 -0500 Subject: [PATCH 03/17] test: reproduce cached kids waiting behind a slow JWKS fetch While a refetch triggered by an unknown kid is in flight, verifying a token whose kid is cached and fresh waits for that fetch to finish. Refs #25 --- src/jwks.rs | 8 +++++ src/tests/jwks.rs | 85 +++++++++++++++++++++++++++++++++++++++++++++++ src/tests/mod.rs | 1 + 3 files changed, 94 insertions(+) create mode 100644 src/tests/jwks.rs diff --git a/src/jwks.rs b/src/jwks.rs index 2a6b22d..f74f65e 100644 --- a/src/jwks.rs +++ b/src/jwks.rs @@ -114,6 +114,14 @@ impl JwksCache { .ok_or_else(|| format!("unknown key id {kid:?}")) } + /// Makes the last fetch attempt look older than the refetch interval, so the + /// next unknown kid fetches again. + #[cfg(test)] + pub(crate) async fn backdate_attempt(&self) { + let mut state = self.state.lock().await; + state.attempted_at = Instant::now().checked_sub(REFETCH_INTERVAL + Duration::from_secs(1)); + } + async fn fetch(&self) -> Result, String> { let res = self .client diff --git a/src/tests/jwks.rs b/src/tests/jwks.rs new file mode 100644 index 0000000..616116d --- /dev/null +++ b/src/tests/jwks.rs @@ -0,0 +1,85 @@ +//! The JWKS cache under a slow key endpoint (#25). + +use std::sync::Arc; +use std::sync::atomic::{AtomicUsize, Ordering}; +use std::time::{Duration, Instant}; + +use axum::routing::get; +use serde_json::json; + +use super::*; +use crate::jwks::JwksCache; + +const SLOW_FETCH: Duration = Duration::from_secs(3); + +/// A JWKS endpoint that answers the first fetch at once and every later one +/// after `SLOW_FETCH`. +async fn slow_jwks() -> (String, Arc) { + let fetches = Arc::new(AtomicUsize::new(0)); + let counter = fetches.clone(); + let app = Router::new().route( + "/jwks", + get(move || { + let counter = counter.clone(); + async move { + if counter.fetch_add(1, Ordering::SeqCst) > 0 { + tokio::time::sleep(SLOW_FETCH).await; + } + json_reply( + 200, + json!({ "keys": [{ "kty": "RSA", "kid": "k1", "alg": "RS256", "use": "sig", "n": TEST_KEY_N.trim(), "e": "AQAB" }] }), + ) + } + }), + ); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let url = format!("http://{}/jwks", listener.local_addr().unwrap()); + tokio::spawn(async move { axum::serve(listener, app).await.unwrap() }); + (url, fetches) +} + +#[tokio::test] +async fn a_cached_kid_does_not_wait_on_a_slow_refetch() { + let (url, fetches) = slow_jwks().await; + let cache = Arc::new(JwksCache::at(url, reqwest::Client::new())); + cache.key("k1").await.unwrap(); + cache.backdate_attempt().await; + + let unknown = { + let cache = cache.clone(); + tokio::spawn(async move { cache.key("forged").await }) + }; + while fetches.load(Ordering::SeqCst) < 2 { + tokio::time::sleep(Duration::from_millis(5)).await; + } + + let started = Instant::now(); + let cached = tokio::time::timeout(Duration::from_millis(500), cache.key("k1")).await; + assert!( + matches!(cached, Ok(Ok(_))), + "a cached, fresh kid waited {:?} behind the refetch", + started.elapsed() + ); + + assert!(unknown.await.unwrap().is_err()); +} + +#[tokio::test] +async fn unknown_kids_share_one_refetch() { + let (url, fetches) = slow_jwks().await; + let cache = Arc::new(JwksCache::at(url, reqwest::Client::new())); + cache.key("k1").await.unwrap(); + cache.backdate_attempt().await; + + let lookups: Vec<_> = (0..5) + .map(|i| { + let cache = cache.clone(); + tokio::spawn(async move { cache.key(&format!("forged-{i}")).await }) + }) + .collect(); + for lookup in lookups { + assert!(lookup.await.unwrap().is_err()); + } + + assert_eq!(fetches.load(Ordering::SeqCst), 2); +} diff --git a/src/tests/mod.rs b/src/tests/mod.rs index a1c1385..9d3bf0f 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -4,6 +4,7 @@ mod adapter; mod client_ip; mod console; mod guard; +mod jwks; mod magic_link; mod manifest; mod notices; From d86f78acec0d9cf9c4a181ff60d72785fa582412 Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Mon, 2 Nov 2026 21:30:45 -0500 Subject: [PATCH 04/17] fix: do not hold the JWKS cache lock across the network fetch Every verification took the cache lock, which a refetch held for up to the 5 second fetch timeout, so a slow JWKS endpoint queued every verification behind it, including tokens whose kid was cached and fresh. The key set is now behind a lock held only to read or update it. A cached kid is served without waiting on a fetch in progress, and the refetch runs single-flight on its own lock, which an unknown kid waits on so one fetch answers all of them. Refs #25 --- src/jwks.rs | 108 ++++++++++++++++++++++++++++++++----------- src/tests/refresh.rs | 100 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 180 insertions(+), 28 deletions(-) create mode 100644 src/tests/refresh.rs diff --git a/src/jwks.rs b/src/jwks.rs index f74f65e..aa78cfa 100644 --- a/src/jwks.rs +++ b/src/jwks.rs @@ -1,10 +1,10 @@ use std::collections::HashMap; +use std::sync::{Mutex, MutexGuard}; use std::time::{Duration, Instant}; use jsonwebtoken::{Algorithm, DecodingKey, Validation}; use serde::Deserialize; use serde_json::Value; -use tokio::sync::Mutex; use crate::jwt::{CLOCK_SKEW_SECS, Claims}; @@ -22,7 +22,10 @@ const MAX_JWKS_BYTES: usize = 256 * 1024; pub(crate) struct JwksCache { url: String, client: reqwest::Client, + /// Held only to read or update the key set, never across a fetch. state: Mutex, + /// Held across a fetch, so only one runs at a time. + fetching: tokio::sync::Mutex<()>, } #[derive(Default)] @@ -63,6 +66,17 @@ struct JwkSet { keys: Vec, } +/// What the key set holds for a kid. +enum Lookup { + Fresh(Key), + /// A key from a set past its maximum age, or none, and whether a fetch is + /// due. + Cached { + key: Option, + due: bool, + }, +} + impl JwksCache { pub fn new(auth_server_url: &str, client: reqwest::Client) -> Self { Self::at(format!("{auth_server_url}/.well-known/jwks.json"), client) @@ -73,16 +87,61 @@ impl JwksCache { url, client, state: Mutex::new(KeySet::default()), + fetching: tokio::sync::Mutex::new(()), } } pub async fn key(&self, kid: &str) -> Result { - let mut state = self.state.lock().await; + let cached = match self.lookup(kid) { + Lookup::Fresh(key) + | Lookup::Cached { + key: Some(key), + due: false, + } => return Ok(key), + Lookup::Cached { key, .. } => key, + }; + // A kid already cached is served while another caller fetches, so a slow + // JWKS endpoint never holds up tokens it would verify anyway. An unknown + // kid waits for the fetch in flight, which may bring it. + let _fetching = match cached { + Some(key) => match self.fetching.try_lock() { + Ok(guard) => guard, + Err(_) => return Ok(key), + }, + None => self.fetching.lock().await, + }; + match self.lookup(kid) { + Lookup::Fresh(key) => return Ok(key), + Lookup::Cached { key, due: false } => { + return key.ok_or_else(|| format!("unknown key id {kid:?}")); + } + Lookup::Cached { due: true, .. } => {} + } + + self.lock_state().attempted_at = Some(Instant::now()); + let fetched = self.fetch().await; + let mut state = self.lock_state(); + match fetched { + Ok(keys) => { + state.keys = Some(keys); + state.fetched_at = Some(Instant::now()); + } + // A stale key set keeps verifying while the API cannot be reached, + // rather than signing every user out. + Err(err) if state.keys.is_some() => { + tracing::warn!("[seamless-auth] Could not refresh the JWKS ({err})."); + } + Err(err) => return Err(err), + } + known(&state, kid) + } + fn lookup(&self, kid: &str) -> Lookup { + let state = self.lock_state(); let fresh = state.fetched_at.is_some_and(|at| at.elapsed() < MAX_AGE); - let known = state.keys.as_ref().and_then(|keys| keys.get(kid)).cloned(); - if let (true, Some(key)) = (fresh, &known) { - return Ok(key.clone()); + let key = state.keys.as_ref().and_then(|keys| keys.get(kid)).cloned(); + if let (true, Some(key)) = (fresh, &key) { + return Lookup::Fresh(key.clone()); } let backoff = if state.keys.is_some() { @@ -90,36 +149,20 @@ impl JwksCache { } else { FAILED_FETCH_BACKOFF }; - if state.attempted_at.is_none_or(|at| at.elapsed() >= backoff) { - state.attempted_at = Some(Instant::now()); - match self.fetch().await { - Ok(keys) => { - state.keys = Some(keys); - state.fetched_at = Some(Instant::now()); - } - // A stale key set keeps verifying while the API cannot be reached, - // rather than signing every user out. - Err(err) if state.keys.is_some() => { - tracing::warn!("[seamless-auth] Could not refresh the JWKS ({err})."); - } - Err(err) => return Err(err), - } - } + let due = state.attempted_at.is_none_or(|at| at.elapsed() >= backoff); + Lookup::Cached { key, due } + } - state - .keys - .as_ref() - .and_then(|keys| keys.get(kid)) - .cloned() - .ok_or_else(|| format!("unknown key id {kid:?}")) + fn lock_state(&self) -> MutexGuard<'_, KeySet> { + self.state.lock().unwrap_or_else(|e| e.into_inner()) } /// Makes the last fetch attempt look older than the refetch interval, so the /// next unknown kid fetches again. #[cfg(test)] pub(crate) async fn backdate_attempt(&self) { - let mut state = self.state.lock().await; - state.attempted_at = Instant::now().checked_sub(REFETCH_INTERVAL + Duration::from_secs(1)); + self.lock_state().attempted_at = + Instant::now().checked_sub(REFETCH_INTERVAL + Duration::from_secs(1)); } async fn fetch(&self) -> Result, String> { @@ -187,3 +230,12 @@ impl JwksCache { .map_err(|e| e.to_string()) } } + +fn known(state: &KeySet, kid: &str) -> Result { + state + .keys + .as_ref() + .and_then(|keys| keys.get(kid)) + .cloned() + .ok_or_else(|| format!("unknown key id {kid:?}")) +} diff --git a/src/tests/refresh.rs b/src/tests/refresh.rs new file mode 100644 index 0000000..d77dee0 --- /dev/null +++ b/src/tests/refresh.rs @@ -0,0 +1,100 @@ +//! Refresh sharing ends when the rotation does (#26). + +use std::sync::Arc; +use std::sync::atomic::{AtomicUsize, Ordering}; +use std::time::Duration; + +use serde_json::json; + +use super::*; + +/// The auth API's rotation: the first presentation of a refresh token rotates +/// it, and any later one is reuse. +fn rotate_once(api: &FakeApi, delay: Duration) -> Arc { + let calls = Arc::new(AtomicUsize::new(0)); + let counter = calls.clone(); + let session = api.session("u1"); + api.on( + "POST", + "/refresh", + Arc::new(move |_| { + if !delay.is_zero() { + std::thread::sleep(delay); + } + if counter.fetch_add(1, Ordering::SeqCst) == 0 { + json_reply(200, session.clone()) + } else { + json_reply(401, json!({ "error": "refresh_token_reused" })) + } + }), + ); + calls +} + +#[tokio::test] +async fn a_bearer_refresh_token_presented_after_rotation_reaches_the_api() { + let api = FakeApi::start().await; + let calls = rotate_once(&api, Duration::ZERO); + let adapter = api.adapter(|b| b); + let refresh = || { + call("POST", "/refresh") + .header("x-seamless-auth-transport", "bearer") + .header("authorization", "Bearer r1") + .to(&adapter) + }; + + let first = refresh().await; + assert_eq!(first.status, 200, "{}", first.text); + + let replay = refresh().await; + assert_eq!( + calls.load(Ordering::SeqCst), + 2, + "the replay never reached the API" + ); + assert_eq!(replay.status, 401, "{}", replay.text); + assert_eq!(replay.body["error"], "refresh_token_reused"); + assert!(replay.body.get("refreshToken").is_none()); +} + +#[tokio::test] +async fn a_refresh_cookie_presented_after_rotation_reaches_the_api() { + let api = FakeApi::start().await; + let calls = rotate_once(&api, Duration::ZERO); + let adapter = api.adapter(|b| b); + let cookie = signed_cookie( + "seamless-refresh", + json!({ "sub": "u1", "refreshToken": "r1" }), + ); + + let first = call("POST", "/refresh").cookie(&cookie).to(&adapter).await; + assert_eq!(first.status, 200, "{}", first.text); + + let replay = call("POST", "/refresh").cookie(&cookie).to(&adapter).await; + assert_eq!( + calls.load(Ordering::SeqCst), + 2, + "the replay never reached the API" + ); + assert_eq!(replay.status, 401, "{}", replay.text); + assert!(replay.cleared("seamless-access") && replay.cleared("seamless-refresh")); +} + +#[tokio::test] +async fn concurrent_bearer_refreshes_share_the_rotation_in_flight() { + let api = FakeApi::start().await; + let calls = rotate_once(&api, Duration::from_millis(200)); + let adapter = api.adapter(|b| b); + let refresh = || { + call("POST", "/refresh") + .header("x-seamless-auth-transport", "bearer") + .header("authorization", "Bearer r1") + .to(&adapter) + }; + + let (a, b) = tokio::join!(refresh(), refresh()); + + assert_eq!(a.status, 200, "{}", a.text); + assert_eq!(b.body, a.body); + assert_eq!(calls.load(Ordering::SeqCst), 1); +} From a07658459ed829bf0a4dac64f193a416d050f66d Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Mon, 2 Nov 2026 21:30:57 -0500 Subject: [PATCH 05/17] test: reproduce a completed refresh rotation replayed to later callers After a rotation succeeds, presenting the same refresh token or cookie again within 5 seconds gets the new session without the auth API being called, so its reuse detection never sees the replay. Refs #26 --- src/tests/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/tests/mod.rs b/src/tests/mod.rs index 9d3bf0f..c0d46a2 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -10,6 +10,7 @@ mod manifest; mod notices; mod origin; mod recovery; +mod refresh; mod verifier; use std::collections::HashMap; From e4de955ed51e4cda4a591227dbe72ebef36c595d Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Mon, 2 Nov 2026 21:31:35 -0500 Subject: [PATCH 06/17] fix: stop replaying a completed refresh rotation to later callers After a rotation succeeded, any caller presenting the same refresh token or cookie within 5 seconds got the new session without the auth API being called, so a stolen token replayed in that window took over the session and the API's reuse detection never saw it. A rotation is now shared only with callers that arrive while it is in flight. Once it settles nothing is kept, and a later caller reaches the auth API, which answers a replayed token as reuse. Refs #26 --- AGENTS.md | 2 +- src/refresh.rs | 56 +++++++++++--------------------------------- src/tests/adapter.rs | 22 ----------------- 3 files changed, 15 insertions(+), 65 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 4d93d5c..3d6f615 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -64,7 +64,7 @@ so CI builds what was tested. - `src/adapter.rs`: `Adapter`, the router, the request pipeline, the upstream call, writing replies. - `src/routes.rs`: manifest routes, credential resolution (with silent refresh), session verification, what a cookie-transport body may contain. -- `src/refresh.rs`: refresh sharing (one result per refresh token for 5 seconds, on its own task), +- `src/refresh.rs`: refresh sharing (one rotation per refresh token while in flight, on its own task), `POST /refresh`, logout. - `src/manifest.rs`: parsing, matching, the live and embedded manifest. - `src/guard.rs`: `require_auth`, `authenticate`, the `User` extractor. diff --git a/src/refresh.rs b/src/refresh.rs index 5138c4d..926774c 100644 --- a/src/refresh.rs +++ b/src/refresh.rs @@ -1,7 +1,7 @@ use std::collections::HashMap; use std::future::Future; use std::sync::{Arc, Mutex}; -use std::time::{Duration, Instant}; +use std::time::Duration; use axum::http::header::RETRY_AFTER; use axum::http::{HeaderMap, HeaderValue, Method, StatusCode}; @@ -13,10 +13,6 @@ use crate::cookies::{CookieWrite, KIND_REFRESH, raw_cookie, read_cookie, session use crate::jwt::{Claims, str_claim}; use crate::routes::{carries_session, cookie_transport_body}; -/// A refresh token presented again within this window gets the rotation it -/// already got, so parallel requests from several tabs with one expired session -/// all succeed instead of tripping the auth API's reuse detection. -const REUSE_WINDOW: Duration = Duration::from_secs(5); const FLIGHT_TIMEOUT: Duration = Duration::from_secs(30); #[derive(Clone, Debug)] @@ -34,7 +30,6 @@ pub(crate) enum RefreshOutcome { struct Flight { id: u64, rx: watch::Receiver>, - finished: Option, } #[derive(Default)] @@ -44,9 +39,11 @@ pub(crate) struct Refresher { } impl Refresher { - /// Runs `rotation` once per key at a time, and hands a successful outcome to - /// every caller with the same key for the reuse window afterwards. A failure - /// is not kept, so each later caller sees it from the auth API itself. + /// Runs `rotation` once per key at a time, and hands its outcome to every + /// caller that presents the same key while it is in flight, so parallel + /// requests from several tabs with one expired session all succeed instead of + /// tripping the auth API's reuse detection. Nothing is kept once it settles: a + /// later caller is sent to the auth API, which sees a replayed token as reuse. /// /// The rotation runs on its own task: a caller that goes away mid-flight must /// not drop a rotation the auth API has already made, or every other tab would @@ -61,13 +58,6 @@ impl Refresher { { let mut rx = { let mut flights = self.flights.lock().unwrap_or_else(|e| e.into_inner()); - let now = Instant::now(); - flights.retain(|_, flight| { - flight - .finished - .is_none_or(|at| now.duration_since(at) <= REUSE_WINDOW) - }); - match flights.get(&key) { Some(flight) => flight.rx.clone(), None => { @@ -77,14 +67,7 @@ impl Refresher { *next }; let (tx, rx) = watch::channel(None); - flights.insert( - key.clone(), - Flight { - id, - rx: rx.clone(), - finished: None, - }, - ); + flights.insert(key.clone(), Flight { id, rx: rx.clone() }); let this = self.clone(); let rotation = rotation(); @@ -95,11 +78,7 @@ impl Refresher { let outcome = tokio::time::timeout(FLIGHT_TIMEOUT, rotation) .await .unwrap_or_else(|_| RefreshOutcome::Failed("refresh timed out".into())); - this.settle( - &flight_key, - id, - matches!(outcome, RefreshOutcome::Session(_)), - ); + this.settle(&flight_key, id); let _ = tx.send(Some(outcome)); }); rx @@ -107,14 +86,13 @@ impl Refresher { } }; - match rx.wait_for(Option::is_some).await { - Ok(outcome) => outcome - .clone() - .unwrap_or_else(|| RefreshOutcome::Failed("refresh lost".into())), + let settled = rx.wait_for(Option::is_some).await.map(|o| o.clone()); + match settled { + Ok(outcome) => outcome.unwrap_or_else(|| RefreshOutcome::Failed("refresh lost".into())), // The rotation task ended without an answer (it panicked). Err(_) => { let mut flights = self.flights.lock().unwrap_or_else(|e| e.into_inner()); - if flights.get(&key).is_some_and(|f| f.finished.is_none()) { + if flights.get(&key).is_some_and(|f| f.rx.same_channel(&rx)) { flights.remove(&key); } RefreshOutcome::Failed("refresh task ended".into()) @@ -122,16 +100,10 @@ impl Refresher { } } - fn settle(&self, key: &str, id: u64, succeeded: bool) { + fn settle(&self, key: &str, id: u64) { let mut flights = self.flights.lock().unwrap_or_else(|e| e.into_inner()); if flights.get(key).is_some_and(|f| f.id == id) { - if succeeded { - if let Some(flight) = flights.get_mut(key) { - flight.finished = Some(Instant::now()); - } - } else { - flights.remove(key); - } + flights.remove(key); } } } diff --git a/src/tests/adapter.rs b/src/tests/adapter.rs index 2fde5eb..b21c256 100644 --- a/src/tests/adapter.rs +++ b/src/tests/adapter.rs @@ -340,28 +340,6 @@ async fn post_refresh_rotates_both_cookies_and_returns_no_tokens() { ); } -#[tokio::test] -async fn bearer_refresh_is_shared_for_one_refresh_token() { - let api = FakeApi::start().await; - api.on("POST", "/refresh", respond(200, api.session("u1"))); - let adapter = api.adapter(|b| b); - let refresh = || { - call("POST", "/refresh") - .header("x-seamless-auth-transport", "bearer") - .header("authorization", "Bearer r1") - .to(&adapter) - }; - - let first = refresh().await; - let second = refresh().await; - - assert_eq!(first.status, 200); - assert_eq!(first.body["refreshToken"], "refresh-u1"); - assert_eq!(second.body, first.body); - assert_eq!(api.calls_to("POST", "/refresh").len(), 1); - assert!(first.cookies.is_empty()); -} - #[tokio::test] async fn bearer_refresh_without_a_token_is_401() { let api = FakeApi::start().await; From 78aee0b7bb5a0c5aa553af914fb4f7294870bd7f Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Mon, 2 Nov 2026 21:32:05 -0500 Subject: [PATCH 07/17] test: cover the refresh and logout exemptions from the content-type gate POST /refresh, DELETE /logout and DELETE /logout/all pass body-less with no content type or transport header, as the client SDKs send them, but still get the origin check under every SameSite setting, and a refused logout clears no cookies. Refs #24 --- src/tests/origin.rs | 77 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/src/tests/origin.rs b/src/tests/origin.rs index 9fe08d9..a66b8f8 100644 --- a/src/tests/origin.rs +++ b/src/tests/origin.rs @@ -136,3 +136,80 @@ async fn a_body_less_unsafe_request_passes_with_json_or_the_transport_header() { .await; assert_eq!(r.status, 415, "neither: {}", r.text); } + +/// What the JS client sends today: body-less, with no content type and no +/// transport header. +fn session_call(method: &str, path: &str) -> Call { + let refresh = signed_cookie( + "seamless-refresh", + json!({ "sub": "u1", "refreshToken": "r1" }), + ); + call(method, path).cookie(&format!("{}; {refresh}", access())) +} + +#[tokio::test] +async fn refresh_and_logout_are_exempt_from_the_content_type_gate() { + let api = started().await; + api.on("POST", "/refresh", respond(200, api.session("u1"))); + api.on( + "DELETE", + "/logout", + respond(200, json!({ "message": "ok" })), + ); + api.on( + "DELETE", + "/logout/all", + respond(200, json!({ "message": "ok" })), + ); + let adapter = api.adapter(|b| b.same_site(SameSite::Lax)); + + let r = session_call("POST", "/refresh").to(&adapter).await; + assert_eq!(r.status, 200, "refresh: {}", r.text); + let r = session_call("DELETE", "/logout").to(&adapter).await; + assert_eq!(r.status, 204, "logout: {}", r.text); + let r = session_call("DELETE", "/logout/all").to(&adapter).await; + assert_eq!(r.status, 204, "logout/all: {}", r.text); +} + +#[tokio::test] +async fn refresh_and_logout_still_get_the_origin_check() { + let api = started().await; + for same_site in [SameSite::None, SameSite::Lax, SameSite::Strict] { + let adapter = api.adapter(|b| b.same_site(same_site)); + for (method, path) in [ + ("POST", "/refresh"), + ("DELETE", "/logout"), + ("DELETE", "/logout/all"), + ] { + let r = session_call(method, path) + .header("sec-fetch-site", "cross-site") + .to(&adapter) + .await; + assert_eq!(r.status, 403, "{same_site:?} {method} {path}: {}", r.text); + assert_eq!(r.body, json!({ "error": "cross_site_request_blocked" })); + assert!( + r.cookies.is_empty(), + "{same_site:?} {method} {path} touched cookies: {:?}", + r.cookies + ); + } + } + assert!(api.calls_to("POST", "/refresh").is_empty()); + assert!(api.calls_to("DELETE", "/logout").is_empty()); + assert!(api.calls_to("DELETE", "/logout/all").is_empty()); +} + +#[tokio::test] +async fn other_body_less_auth_routes_get_the_content_type_gate() { + let api = started().await; + let adapter = api.adapter(|b| b); + + for (method, path) in [("POST", "/recovery-codes"), ("POST", "/totp/enroll/start")] { + let r = session_call(method, path) + .header("sec-fetch-site", "same-origin") + .to(&adapter) + .await; + assert_eq!(r.status, 415, "{method} {path}: {}", r.text); + assert_eq!(r.body, json!({ "error": "unsupported_media_type" })); + } +} From d1b8d94c219a7f02f1ce7372d433b069b992fb23 Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Mon, 2 Nov 2026 21:32:05 -0500 Subject: [PATCH 08/17] docs: describe the content-type gate exemptions and transport header values Refs #24 --- README.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index a5ef09f..0a2d1ea 100644 --- a/README.md +++ b/README.md @@ -143,7 +143,11 @@ cross-site form can post a `text/plain` body shaped like JSON with no CORS prefl JSON it would be a sign-in the user never made. A state-changing request with no body (`POST`, `PUT`, `PATCH` or `DELETE`) to a manifest route must still send `Content-Type: application/json` or the `x-seamless-auth-transport` header, or it answers 415 too: either one forces a CORS preflight, -which an HTML form on another site, sibling subdomains included, cannot send. +which an HTML form on another site, sibling subdomains included, cannot send. The header may +carry any value; only `bearer` selects bearer transport. `POST /refresh`, `DELETE /logout` and +`DELETE /logout/all` are exempt from this rule, since the client SDKs send them with no body and no +header, but they still get the cross-site check. Routes behind `require_auth` get the cross-site +check only. ### Errors and logout From a191b6dd012200fbb5f5c86a8026765c4094c659 Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Mon, 2 Nov 2026 21:41:51 -0500 Subject: [PATCH 09/17] test: reproduce Origin null passing on same-site requests A Sec-Fetch-Site: same-site request carrying Origin: null passes when no allowlist is configured, while every other unsafe request with a null Origin is refused. Refs #24 --- src/tests/origin.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/tests/origin.rs b/src/tests/origin.rs index a66b8f8..6cdadd0 100644 --- a/src/tests/origin.rs +++ b/src/tests/origin.rs @@ -213,3 +213,20 @@ async fn other_body_less_auth_routes_get_the_content_type_gate() { assert_eq!(r.body, json!({ "error": "unsupported_media_type" })); } } + +#[tokio::test] +async fn a_null_origin_is_refused_on_same_site_requests_without_an_allowlist() { + let api = started().await; + let adapter = api.adapter(|b| b); + + let r = form_post() + .header("sec-fetch-site", "same-site") + .header("origin", "null") + .header("content-type", "application/json") + .to(&adapter) + .await; + + assert_eq!(r.status, 403, "{}", r.text); + assert_eq!(r.body, json!({ "error": "cross_site_request_blocked" })); + assert!(api.calls_to("POST", "/recovery-codes").is_empty()); +} From 02f6f5fc8408616a899729f4e335a3c1c2acf133 Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Mon, 2 Nov 2026 21:42:17 -0500 Subject: [PATCH 10/17] fix: refuse Origin null on same-site requests A null Origin is opaque or sandboxed, so it is refused on every unsafe request, including a Sec-Fetch-Site: same-site one with no allowlist. Refs #24 --- README.md | 2 +- src/origin.rs | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 0a2d1ea..db6e255 100644 --- a/README.md +++ b/README.md @@ -93,7 +93,7 @@ applies whatever the cookies' `SameSite` setting (a sibling subdomain is same-si - `Sec-Fetch-Site: cross-site` is refused, and `same-origin` (or `none`) passes. - `Sec-Fetch-Site: same-site` also covers a sibling subdomain, so when `allowed_origins` is set the `Origin` must be on it, and a request with no `Origin` is refused. Without - `allowed_origins` it passes. + `allowed_origins` it passes, unless its `Origin` is `null`. - With no `Sec-Fetch-Site` (an older browser or a server), a request with no `Origin` passes, a `null` `Origin` is refused, and when `allowed_origins` is set the `Origin` must be on it. - An empty `allowed_origins` is still set: it allows no cross-origin caller. diff --git a/src/origin.rs b/src/origin.rs index 5af7062..f140365 100644 --- a/src/origin.rs +++ b/src/origin.rs @@ -25,6 +25,9 @@ pub(crate) fn check_origin(config: &Config, method: &Method, headers: &HeaderMap if !site.eq_ignore_ascii_case(b"same-site") { return None; } + if headers.get("origin").is_some_and(|origin| origin == "null") { + return blocked(); + } let Some(allowed) = &config.allowed_origins else { return None; }; From 5e725b26915414240141cbad9e9190dc7cb30a1c Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Mon, 2 Nov 2026 21:48:26 -0500 Subject: [PATCH 11/17] test: reproduce Origin null passing with same-origin or none fetch metadata A null Origin, in any case or with surrounding space, should be refused on every unsafe request before Sec-Fetch-Site and the allowlist are consulted. Sec-Fetch-Site same-origin or none lets it through. Refs #24 --- src/tests/origin.rs | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/tests/origin.rs b/src/tests/origin.rs index 6cdadd0..6c0b9c9 100644 --- a/src/tests/origin.rs +++ b/src/tests/origin.rs @@ -230,3 +230,27 @@ async fn a_null_origin_is_refused_on_same_site_requests_without_an_allowlist() { assert_eq!(r.body, json!({ "error": "cross_site_request_blocked" })); assert!(api.calls_to("POST", "/recovery-codes").is_empty()); } + +#[tokio::test] +async fn a_null_origin_is_refused_first_whatever_the_fetch_metadata() { + let api = started().await; + let open = api.adapter(|b| b); + let listed = api.adapter(|b| b.allowed_origins(["https://app.example.com"])); + + for adapter in [&open, &listed] { + for site in [Some("same-origin"), Some("none"), Some("same-site"), None] { + for origin in ["null", "NULL", " Null "] { + let mut request = form_post() + .header("origin", origin) + .header("content-type", "application/json"); + if let Some(site) = site { + request = request.header("sec-fetch-site", site); + } + let r = request.to(adapter).await; + assert_eq!(r.status, 403, "{site:?} {origin:?}: {}", r.text); + assert_eq!(r.body, json!({ "error": "cross_site_request_blocked" })); + } + } + } + assert!(api.calls_to("POST", "/recovery-codes").is_empty()); +} From 9890782a5c7b76f000d180da81d648528240ba53 Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Mon, 2 Nov 2026 21:48:47 -0500 Subject: [PATCH 12/17] fix: refuse Origin null first on every unsafe request A null Origin, trimmed and in any case, is now refused before Sec-Fetch-Site and the allowlist are consulted, so same-origin or none fetch metadata no longer lets it through. This folds the earlier same-site special case into the single early check. Refs #24 --- README.md | 8 +++++--- src/origin.rs | 15 +++++++++------ 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index db6e255..6b701b6 100644 --- a/README.md +++ b/README.md @@ -90,12 +90,14 @@ applies whatever the cookies' `SameSite` setting (a sibling subdomain is same-si `Strict` cookies still reach it), and only to state-changing methods (not `GET`, `HEAD` or `OPTIONS`); a refused request answers 403 `cross_site_request_blocked`: +- A `null` `Origin` (in any case) is refused first, whatever `Sec-Fetch-Site` and + `allowed_origins` say. - `Sec-Fetch-Site: cross-site` is refused, and `same-origin` (or `none`) passes. - `Sec-Fetch-Site: same-site` also covers a sibling subdomain, so when `allowed_origins` is set the `Origin` must be on it, and a request with no `Origin` is refused. Without - `allowed_origins` it passes, unless its `Origin` is `null`. -- With no `Sec-Fetch-Site` (an older browser or a server), a request with no `Origin` passes, a - `null` `Origin` is refused, and when `allowed_origins` is set the `Origin` must be on it. + `allowed_origins` it passes. +- With no `Sec-Fetch-Site` (an older browser or a server), a request with no `Origin` passes, and + when `allowed_origins` is set the `Origin` must be on it. - An empty `allowed_origins` is still set: it allows no cross-origin caller. A bearer token is never attached by a browser, so it is not checked. diff --git a/src/origin.rs b/src/origin.rs index f140365..87b3d03 100644 --- a/src/origin.rs +++ b/src/origin.rs @@ -17,6 +17,15 @@ pub(crate) fn check_origin(config: &Config, method: &Method, headers: &HeaderMap )) }; + // A null Origin is opaque or sandboxed, never the adopter's own page, so it + // is refused whatever Sec-Fetch-Site and the allowlist say. + if headers + .get("origin") + .is_some_and(|origin| origin.as_bytes().trim_ascii().eq_ignore_ascii_case(b"null")) + { + return blocked(); + } + if let Some(site) = headers.get("sec-fetch-site") { let site = site.as_bytes(); if site.eq_ignore_ascii_case(b"cross-site") { @@ -25,9 +34,6 @@ pub(crate) fn check_origin(config: &Config, method: &Method, headers: &HeaderMap if !site.eq_ignore_ascii_case(b"same-site") { return None; } - if headers.get("origin").is_some_and(|origin| origin == "null") { - return blocked(); - } let Some(allowed) = &config.allowed_origins else { return None; }; @@ -38,9 +44,6 @@ pub(crate) fn check_origin(config: &Config, method: &Method, headers: &HeaderMap } let origin = headers.get("origin")?; - if origin == "null" { - return blocked(); - } // An Origin a browser would never send is not one to trust. if origin.to_str().is_err() { return blocked(); From 3021fbd3fce3ed0dc6ee039061987da3242bc169 Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Mon, 2 Nov 2026 22:07:53 -0500 Subject: [PATCH 13/17] test: reproduce rotated cookies dropped on upstream session errors and ported proxy hops An access route that silently refreshes and then gets an unverifiable or unstorable session back answers 502 without the rotated access and refresh cookies. A trusted X-Forwarded-For hop written as ip:port or [v6]:port resolves to no client address. Logout clearing a pre-auth cookie with its own name is already covered by logout_clears_a_pre_auth_cookie_with_its_own_name since c6edea8. Refs #27 --- src/tests/adapter.rs | 51 ++++++++++++++++++++++++++++++++++++++++++ src/tests/client_ip.rs | 30 +++++++++++++++++++++++++ 2 files changed, 81 insertions(+) diff --git a/src/tests/adapter.rs b/src/tests/adapter.rs index b21c256..8b97238 100644 --- a/src/tests/adapter.rs +++ b/src/tests/adapter.rs @@ -1315,3 +1315,54 @@ async fn logout_clears_a_shared_flow_cookie_once() { }); assert_eq!(headers.count(), 1); } + +async fn switch_after_a_silent_refresh(upstream: impl FnOnce(&FakeApi) -> Value) -> Reply { + let api = FakeApi::start().await; + api.on("POST", "/refresh", respond(200, api.session("u1"))); + api.on( + "POST", + "/organizations/o2/switch", + respond(200, upstream(&api)), + ); + let refresh = signed_cookie( + "seamless-refresh", + json!({ "sub": "u1", "refreshToken": "r1" }), + ); + call("POST", "/organizations/o2/switch") + .cookie(&refresh) + .body("{}") + .to(&api.adapter(|b| b)) + .await +} + +#[tokio::test] +async fn an_unverifiable_upstream_session_keeps_the_rotated_cookies() { + let r = switch_after_a_silent_refresh(|_| { + json!({ + "sub": "u1", + "token": "not-a-jwt", + "ttl": 900, + }) + }) + .await; + assert_eq!(r.status, 502, "{}", r.text); + assert_eq!(r.body["error"], "invalid_upstream_session"); + for name in ["seamless-access", "seamless-refresh"] { + assert!(r.cookies.contains_key(name), "rotated {name} dropped"); + } +} + +#[tokio::test] +async fn an_unstorable_upstream_session_keeps_the_rotated_cookies() { + let r = switch_after_a_silent_refresh(|api| { + let mut session = api.session("u1"); + session["ttl"] = json!("never"); + session + }) + .await; + assert_eq!(r.status, 502, "{}", r.text); + assert_eq!(r.body["error"], "invalid_upstream_session"); + for name in ["seamless-access", "seamless-refresh"] { + assert!(r.cookies.contains_key(name), "rotated {name} dropped"); + } +} diff --git a/src/tests/client_ip.rs b/src/tests/client_ip.rs index e7e784d..8612bd1 100644 --- a/src/tests/client_ip.rs +++ b/src/tests/client_ip.rs @@ -72,3 +72,33 @@ fn refuses_what_it_cannot_read() { } assert!(TrustedProxies::new(["0.0.0.0/0", "::/0"]).is_ok()); } + +#[test] +fn reads_hops_written_with_a_port() { + let proxies = TrustedProxies::new(["10.0.0.0/8", "2001:db8::/32"]).unwrap(); + + assert_eq!( + proxies.resolve(&forwarded(&["203.0.113.50, 10.0.0.7:443"]), ip("10.0.0.1")), + ip("203.0.113.50") + ); + assert_eq!( + proxies.resolve(&forwarded(&["203.0.113.50:51234"]), ip("10.0.0.1")), + ip("203.0.113.50") + ); + assert_eq!( + proxies.resolve( + &forwarded(&["[2001:db9::7]:51234, [2001:db8::5]:443"]), + ip("2001:db8::2") + ), + ip("2001:db9::7") + ); + assert_eq!( + proxies.resolve(&forwarded(&["[2001:db9::7]"]), ip("2001:db8::2")), + ip("2001:db9::7") + ); + // Every hop trusted, the left-most written with a port. + assert_eq!( + proxies.resolve(&forwarded(&["10.0.0.5:80, 10.0.0.6"]), ip("10.0.0.1")), + ip("10.0.0.5") + ); +} From 8f3933bd6f2ca73d7b1fa2bf46909fac0b723dee Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Mon, 2 Nov 2026 22:08:33 -0500 Subject: [PATCH 14/17] fix: keep rotated cookies on upstream session errors and read proxy hops with ports A manifest route that silently refreshed the session and then got back a session it could not verify or store answered 502 without the rotated access and refresh cookies. The browser kept the spent refresh token and was signed out by reuse detection on its next request. Both invalid_upstream_session replies now carry the rotated cookies. TrustedProxies read an X-Forwarded-For hop written as ip:port or [v6]:port as unreadable and gave no client address, so the auth API limited on the adapter's own address. Hops are now read past the port and brackets; a hop whose port is not a port is still unreadable. Refs #27 --- src/client_ip.rs | 42 +++++++++++++++++++++++++++++++++++++----- src/routes.rs | 6 ++++-- src/tests/client_ip.rs | 13 +++++++++++++ 3 files changed, 54 insertions(+), 7 deletions(-) diff --git a/src/client_ip.rs b/src/client_ip.rs index 662d9f2..a0cd97e 100644 --- a/src/client_ip.rs +++ b/src/client_ip.rs @@ -115,21 +115,53 @@ impl TrustedProxies { .collect(); for hop in hops.iter().rev() { - match hop.parse::() { - Ok(ip) if self.trusted(ip) => continue, - Ok(ip) => return Some(ip), + match hop_address(hop) { + Some(ip) if self.trusted(ip) => continue, + Some(ip) => return Some(ip), // Not an address, so not a trusted proxy either: whoever wrote it // is the client, and it has no usable address. - Err(_) => return None, + None => return None, } } match hops.first() { - Some(first) => first.parse().ok(), + Some(first) => hop_address(first), None => Some(peer), } } } +/// A hop's address, read past the port some proxies append (`ip:port`, +/// `[v6]:port`) and the brackets around a bare `[v6]`. +fn hop_address(hop: &str) -> Option { + if let Ok(ip) = hop.parse() { + return Some(ip); + } + let (host, port) = match hop.strip_prefix('[') { + Some(rest) => { + let (host, after) = rest.split_once(']')?; + match after { + "" => (host, None), + _ => (host, Some(after.strip_prefix(':')?)), + } + } + None => { + let (host, port) = hop.split_once(':')?; + // More than one colon without brackets is an IPv6 address, which + // already failed to parse above. + if port.contains(':') { + return None; + } + (host, Some(port)) + } + }; + if let Some(port) = port + && port.parse::().is_err() + { + return None; + } + host.parse().ok() +} + fn width(addr: IpAddr) -> u8 { match addr { IpAddr::V4(_) => 32, diff --git a/src/routes.rs b/src/routes.rs index a449ed3..85e4bbd 100644 --- a/src/routes.rs +++ b/src/routes.rs @@ -335,7 +335,8 @@ impl Inner { route.method, route.path ); - return Reply::error(StatusCode::BAD_GATEWAY, "invalid_upstream_session"); + return Reply::error(StatusCode::BAD_GATEWAY, "invalid_upstream_session") + .with_set(std::mem::take(&mut reply.set)); } }; @@ -351,7 +352,8 @@ impl Inner { Ok(cookies) => reply.set.extend(cookies), Err(err) => { tracing::warn!("[seamless-auth] {} {}: {err}", route.method, route.path); - return Reply::error(StatusCode::BAD_GATEWAY, "invalid_upstream_session"); + return Reply::error(StatusCode::BAD_GATEWAY, "invalid_upstream_session") + .with_set(std::mem::take(&mut reply.set)); } } } diff --git a/src/tests/client_ip.rs b/src/tests/client_ip.rs index 8612bd1..6e0767b 100644 --- a/src/tests/client_ip.rs +++ b/src/tests/client_ip.rs @@ -101,4 +101,17 @@ fn reads_hops_written_with_a_port() { proxies.resolve(&forwarded(&["10.0.0.5:80, 10.0.0.6"]), ip("10.0.0.1")), ip("10.0.0.5") ); + // A port that is not one leaves the hop unreadable. + for bad in [ + "203.0.113.50:", + "203.0.113.50:http", + "[2001:db9::7]x", + "[2001:db9::7]:99999", + ] { + assert_eq!( + proxies.resolve(&forwarded(&[bad]), ip("10.0.0.1")), + None, + "{bad}" + ); + } } From 60b16883aafd925f5ed79b9ed6fe4f7e786f7cbd Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Tue, 3 Nov 2026 10:56:52 -0500 Subject: [PATCH 15/17] ci: hold conformance to the content-type gate contract --- .github/workflows/ci.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a32f348..0f4c6db 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -89,6 +89,9 @@ jobs: - name: Conformance env: SEAMLESS_API_DIR: ${{ github.workspace }}/seamless-auth-api + # This adapter ships the content-type gate, so hold it to the pending + # contract (seamless-cli verify/harness/lib/pendingContracts.ts). + SEAMLESS_VERIFY_CONTRACTS: content-type-gate run: node seamless-cli/dist/index.js verify --adapter-url=http://localhost:8080 - name: Reference app log if: always() From b876560508d8bcdecb2155386cf2251cefcf6a78 Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Tue, 3 Nov 2026 11:39:10 -0500 Subject: [PATCH 16/17] test: reproduce a cookie refresh replay failing inside the multi-tab window A tab that presents a refresh cookie just after another tab rotated it must get the same rotated session without a call to the auth API, for five seconds. After that, and for bearer transport always, the replay reaches the API. Expired entries must be pruned. Refs #26 --- src/tests/refresh.rs | 95 +++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 90 insertions(+), 5 deletions(-) diff --git a/src/tests/refresh.rs b/src/tests/refresh.rs index d77dee0..d602f66 100644 --- a/src/tests/refresh.rs +++ b/src/tests/refresh.rs @@ -1,4 +1,5 @@ -//! Refresh sharing ends when the rotation does (#26). +//! Refresh sharing (#26): any caller shares a rotation in flight, and a cookie +//! caller shares a completed one for five seconds. use std::sync::Arc; use std::sync::atomic::{AtomicUsize, Ordering}; @@ -7,6 +8,7 @@ use std::time::Duration; use serde_json::json; use super::*; +use crate::jwt::{str_claim, verify_hs256}; /// The auth API's rotation: the first presentation of a refresh token rotates /// it, and any later one is reuse. @@ -57,18 +59,50 @@ async fn a_bearer_refresh_token_presented_after_rotation_reaches_the_api() { assert!(replay.body.get("refreshToken").is_none()); } +fn refresh_cookie(token: &str) -> String { + signed_cookie( + "seamless-refresh", + json!({ "sub": "u1", "refreshToken": token }), + ) +} + #[tokio::test] -async fn a_refresh_cookie_presented_after_rotation_reaches_the_api() { +async fn a_refresh_cookie_replayed_within_the_window_shares_the_completed_rotation() { let api = FakeApi::start().await; let calls = rotate_once(&api, Duration::ZERO); let adapter = api.adapter(|b| b); - let cookie = signed_cookie( - "seamless-refresh", - json!({ "sub": "u1", "refreshToken": "r1" }), + let cookie = refresh_cookie("r1"); + + let first = call("POST", "/refresh").cookie(&cookie).to(&adapter).await; + assert_eq!(first.status, 200, "{}", first.text); + + let replay = call("POST", "/refresh").cookie(&cookie).to(&adapter).await; + assert_eq!(replay.status, 200, "{}", replay.text); + assert_eq!( + calls.load(Ordering::SeqCst), + 1, + "the replay reached the API" ); + assert_eq!(replay.body, first.body); + for name in ["seamless-access", "seamless-refresh"] { + let a = verify_hs256(&first.cookie_value(name), TEST_SECRET).unwrap(); + let b = verify_hs256(&replay.cookie_value(name), TEST_SECRET).unwrap(); + for claim in ["sub", "token", "refreshToken", "sessionId"] { + assert_eq!(str_claim(&a, claim), str_claim(&b, claim), "{name} {claim}"); + } + } +} + +#[tokio::test] +async fn a_refresh_cookie_replayed_after_the_window_reaches_the_api() { + let api = FakeApi::start().await; + let calls = rotate_once(&api, Duration::ZERO); + let adapter = api.adapter(|b| b); + let cookie = refresh_cookie("r1"); let first = call("POST", "/refresh").cookie(&cookie).to(&adapter).await; assert_eq!(first.status, 200, "{}", first.text); + adapter.inner.refreshes.backdate_completed(); let replay = call("POST", "/refresh").cookie(&cookie).to(&adapter).await; assert_eq!( @@ -80,6 +114,57 @@ async fn a_refresh_cookie_presented_after_rotation_reaches_the_api() { assert!(replay.cleared("seamless-access") && replay.cleared("seamless-refresh")); } +#[tokio::test] +async fn a_bearer_replay_within_a_cookie_window_reaches_the_api() { + let api = FakeApi::start().await; + let calls = rotate_once(&api, Duration::ZERO); + let adapter = api.adapter(|b| b); + + let first = call("POST", "/refresh") + .cookie(&refresh_cookie("r1")) + .to(&adapter) + .await; + assert_eq!(first.status, 200, "{}", first.text); + + let replay = call("POST", "/refresh") + .header("x-seamless-auth-transport", "bearer") + .header("authorization", "Bearer r1") + .to(&adapter) + .await; + assert_eq!( + calls.load(Ordering::SeqCst), + 2, + "the replay never reached the API" + ); + assert_eq!(replay.status, 401, "{}", replay.text); + assert_eq!(replay.body["error"], "refresh_token_reused"); +} + +#[tokio::test] +async fn expired_completed_rotations_are_pruned() { + let api = FakeApi::start().await; + let session = api.session("u1"); + api.on("POST", "/refresh", respond(200, session)); + let adapter = api.adapter(|b| b); + + for token in ["r1", "r2"] { + let r = call("POST", "/refresh") + .cookie(&refresh_cookie(token)) + .to(&adapter) + .await; + assert_eq!(r.status, 200, "{}", r.text); + } + assert_eq!(adapter.inner.refreshes.completed_len(), 2); + + adapter.inner.refreshes.backdate_completed(); + let r = call("POST", "/refresh") + .cookie(&refresh_cookie("r3")) + .to(&adapter) + .await; + assert_eq!(r.status, 200, "{}", r.text); + assert_eq!(adapter.inner.refreshes.completed_len(), 1); +} + #[tokio::test] async fn concurrent_bearer_refreshes_share_the_rotation_in_flight() { let api = FakeApi::start().await; From bf77f1ef0861108f4756547bfe65e51dd2de9043 Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Tue, 3 Nov 2026 11:40:18 -0500 Subject: [PATCH 17/17] fix: keep a completed refresh for cookie transport for five seconds A tab that sends the old refresh cookie just after another tab rotated it now gets the same rotated session without calling the auth API, for five seconds after the rotation settles. The completed result is stored under the same lock that ends the flight, so no caller can slip between the two and replay the token. Only successful, verified rotations are kept, entries are pruned on every access, and bearer transport never uses the window: a bearer replay reaches the API, whose reuse detection revokes the family. Refs #26 --- AGENTS.md | 5 +- src/refresh.rs | 130 ++++++++++++++++++++++++++++++++++++++++--------- src/routes.rs | 2 +- 3 files changed, 112 insertions(+), 25 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 3d6f615..012d0b1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -64,8 +64,9 @@ so CI builds what was tested. - `src/adapter.rs`: `Adapter`, the router, the request pipeline, the upstream call, writing replies. - `src/routes.rs`: manifest routes, credential resolution (with silent refresh), session verification, what a cookie-transport body may contain. -- `src/refresh.rs`: refresh sharing (one rotation per refresh token while in flight, on its own task), - `POST /refresh`, logout. +- `src/refresh.rs`: refresh sharing (one rotation per refresh token while in flight, on its own task, + and for cookie transport only a successful rotation kept for 5 seconds for other tabs; bearer + replays always reach the API), `POST /refresh`, logout. - `src/manifest.rs`: parsing, matching, the live and embedded manifest. - `src/guard.rs`: `require_auth`, `authenticate`, the `User` extractor. - `src/console.rs`: `console_router`, the admin dashboard proxy. diff --git a/src/refresh.rs b/src/refresh.rs index 926774c..ae8425a 100644 --- a/src/refresh.rs +++ b/src/refresh.rs @@ -1,7 +1,7 @@ use std::collections::HashMap; use std::future::Future; use std::sync::{Arc, Mutex}; -use std::time::Duration; +use std::time::{Duration, Instant}; use axum::http::header::RETRY_AFTER; use axum::http::{HeaderMap, HeaderValue, Method, StatusCode}; @@ -14,6 +14,8 @@ use crate::jwt::{Claims, str_claim}; use crate::routes::{carries_session, cookie_transport_body}; const FLIGHT_TIMEOUT: Duration = Duration::from_secs(30); +/// How long a completed cookie-transport rotation is handed to later callers. +const COOKIE_WINDOW: Duration = Duration::from_secs(5); #[derive(Clone, Debug)] pub(crate) enum RefreshOutcome { @@ -32,9 +34,26 @@ struct Flight { rx: watch::Receiver>, } +struct Completed { + at: Instant, + outcome: RefreshOutcome, +} + +#[derive(Default)] +struct Flights { + running: HashMap, + completed: HashMap, +} + +impl Flights { + fn prune(&mut self) { + self.completed.retain(|_, c| c.at.elapsed() < COOKIE_WINDOW); + } +} + #[derive(Default)] pub(crate) struct Refresher { - flights: Mutex>, + flights: Mutex, next_id: Mutex, } @@ -42,8 +61,13 @@ impl Refresher { /// Runs `rotation` once per key at a time, and hands its outcome to every /// caller that presents the same key while it is in flight, so parallel /// requests from several tabs with one expired session all succeed instead of - /// tripping the auth API's reuse detection. Nothing is kept once it settles: a - /// later caller is sent to the auth API, which sees a replayed token as reuse. + /// tripping the auth API's reuse detection. + /// + /// With `keep` (cookie transport), a successful rotation is also handed to + /// callers that present the key within five seconds of it settling, for tabs + /// that sent the old refresh cookie just after another tab rotated it. + /// Otherwise nothing is kept once it settles: a later caller is sent to the + /// auth API, which sees a replayed token as reuse. /// /// The rotation runs on its own task: a caller that goes away mid-flight must /// not drop a rotation the auth API has already made, or every other tab would @@ -51,14 +75,21 @@ impl Refresher { pub async fn share( self: &Arc, key: String, + keep: bool, rotation: impl FnOnce() -> Fut, ) -> RefreshOutcome where Fut: Future + Send + 'static, { let mut rx = { - let mut flights = self.flights.lock().unwrap_or_else(|e| e.into_inner()); - match flights.get(&key) { + let mut flights = self.lock(); + if keep { + flights.prune(); + if let Some(done) = flights.completed.get(&key) { + return done.outcome.clone(); + } + } + match flights.running.get(&key) { Some(flight) => flight.rx.clone(), None => { let id = { @@ -67,7 +98,9 @@ impl Refresher { *next }; let (tx, rx) = watch::channel(None); - flights.insert(key.clone(), Flight { id, rx: rx.clone() }); + flights + .running + .insert(key.clone(), Flight { id, rx: rx.clone() }); let this = self.clone(); let rotation = rotation(); @@ -78,7 +111,7 @@ impl Refresher { let outcome = tokio::time::timeout(FLIGHT_TIMEOUT, rotation) .await .unwrap_or_else(|_| RefreshOutcome::Failed("refresh timed out".into())); - this.settle(&flight_key, id); + this.settle(&flight_key, id, keep, &outcome); let _ = tx.send(Some(outcome)); }); rx @@ -91,21 +124,57 @@ impl Refresher { Ok(outcome) => outcome.unwrap_or_else(|| RefreshOutcome::Failed("refresh lost".into())), // The rotation task ended without an answer (it panicked). Err(_) => { - let mut flights = self.flights.lock().unwrap_or_else(|e| e.into_inner()); - if flights.get(&key).is_some_and(|f| f.rx.same_channel(&rx)) { - flights.remove(&key); + let mut flights = self.lock(); + if flights + .running + .get(&key) + .is_some_and(|f| f.rx.same_channel(&rx)) + { + flights.running.remove(&key); } RefreshOutcome::Failed("refresh task ended".into()) } } } - fn settle(&self, key: &str, id: u64) { - let mut flights = self.flights.lock().unwrap_or_else(|e| e.into_inner()); - if flights.get(key).is_some_and(|f| f.id == id) { - flights.remove(key); + /// Ends a flight. A kept outcome is stored under the same lock, so no caller + /// can find the key neither running nor completed and replay it. + fn settle(&self, key: &str, id: u64, keep: bool, outcome: &RefreshOutcome) { + let mut flights = self.lock(); + if flights.running.get(key).is_some_and(|f| f.id == id) { + flights.running.remove(key); + } + flights.prune(); + if keep && matches!(outcome, RefreshOutcome::Session(_)) { + flights.completed.insert( + key.to_string(), + Completed { + at: Instant::now(), + outcome: outcome.clone(), + }, + ); } } + + fn lock(&self) -> std::sync::MutexGuard<'_, Flights> { + self.flights.lock().unwrap_or_else(|e| e.into_inner()) + } + + /// Makes every completed rotation look older than the window. + #[cfg(test)] + pub(crate) fn backdate_completed(&self) { + let at = Instant::now().checked_sub(COOKIE_WINDOW + Duration::from_secs(1)); + for done in self.lock().completed.values_mut() { + if let Some(at) = at { + done.at = at; + } + } + } + + #[cfg(test)] + pub(crate) fn completed_len(&self) -> usize { + self.lock().completed.len() + } } pub(crate) enum SilentRefreshError { @@ -134,15 +203,22 @@ impl Inner { return Err(SilentRefreshError::Failed("invalid refresh cookie".into())); } let subject = payload.as_ref().map(|p| str_claim(p, "sub")).unwrap_or(""); - self.refresh_session(format!("cookie:{raw}"), refresh_token, subject, forwarded) - .await + self.refresh_session( + format!("cookie:{raw}"), + true, + refresh_token, + subject, + forwarded, + ) + .await } - /// Rotates a refresh token, shared under `key`, and returns the verified - /// session with its cookies. + /// Rotates a refresh token, shared under `key` (and kept for the cookie + /// window with `keep`), and returns the verified session with its cookies. pub(crate) async fn refresh_session( self: &Arc, key: String, + keep: bool, refresh_token: &str, subject: &str, forwarded: &Forwarded, @@ -160,8 +236,18 @@ impl Inner { let forwarded = forwarded.clone(); let outcome = self .refreshes - .share(key, move || async move { - inner.rotate(&forwarded, &refresh_token, service).await + .share(key, keep, move || async move { + // Verified before it settles, so a session the adapter would refuse + // is never kept. + match inner.rotate(&forwarded, &refresh_token, service).await { + RefreshOutcome::Session(session) => { + match inner.verify_session(&session, "access").await { + Ok(_) => RefreshOutcome::Session(session), + Err(err) => RefreshOutcome::Failed(err), + } + } + other => other, + } }) .await; @@ -237,7 +323,7 @@ impl Inner { let forwarded = forwarded.clone(); let outcome = self .refreshes - .share(format!("bearer:{token}"), move || async move { + .share(format!("bearer:{token}"), false, move || async move { inner.rotate(&forwarded, &refresh_token, service).await }) .await; diff --git a/src/routes.rs b/src/routes.rs index 85e4bbd..a0b6bc9 100644 --- a/src/routes.rs +++ b/src/routes.rs @@ -447,7 +447,7 @@ impl Inner { // the browser holds, so the one it just issued is the one to spend. Some((token, _)) if token.is_empty() => return, Some((token, subject)) => { - self.refresh_session(format!("fresh:{token}"), &token, &subject, forwarded) + self.refresh_session(format!("fresh:{token}"), false, &token, &subject, forwarded) .await } None => self.silent_refresh(headers, forwarded).await,