From 8daf82a5e6ba4e44b7d588b7e9113aa4dde04a3c Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Tue, 3 Nov 2026 14:28:01 -0500 Subject: [PATCH 1/2] test: refresh failures keep the session cookies unless the auth API answers 401 Reproduces cookie-transport refreshes (the silent refresh on an auth route and POST /refresh) that sign a browser out when the auth API is unreachable, answers another error, returns no usable session, or the rotation times out or its task panics. Only a 401 or an unreadable refresh cookie should clear the session cookies. Turns on tokio's test-util for a paused clock in tests. --- Cargo.toml | 2 +- src/tests/refresh.rs | 231 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 232 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 09a0332..aa17ca6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -46,7 +46,7 @@ tracing = { version = "0.1.40", default-features = false, features = ["std"] } [dev-dependencies] axum = { version = "0.8.4", default-features = false, features = ["tokio", "http1", "json"] } -tokio = { version = "1.38", features = ["macros", "rt-multi-thread", "net"] } +tokio = { version = "1.38", features = ["macros", "rt-multi-thread", "net", "test-util"] } # The fake auth API in the tests signs RS256 with a PEM test key. jsonwebtoken = { version = "11.1.0", default-features = false, features = ["aws_lc_rs", "use_pem"] } tower = { version = "0.5", default-features = false, features = ["util"] } diff --git a/src/tests/refresh.rs b/src/tests/refresh.rs index d602f66..1ff3438 100644 --- a/src/tests/refresh.rs +++ b/src/tests/refresh.rs @@ -183,3 +183,234 @@ async fn concurrent_bearer_refreshes_share_the_rotation_in_flight() { assert_eq!(b.body, a.body); assert_eq!(calls.load(Ordering::SeqCst), 1); } + +/// A refresh the adapter makes on its own: an auth route with an expired access +/// cookie and a live refresh cookie. +async fn silent_refresh(adapter: &crate::Adapter) -> Reply { + call("GET", "/admin/applications") + .cookie(&refresh_cookie("r1")) + .to(adapter) + .await +} + +/// A browser asking for a refresh: `POST /refresh` in cookie transport. +async fn cookie_refresh(adapter: &crate::Adapter) -> Reply { + call("POST", "/refresh") + .cookie(&refresh_cookie("r1")) + .to(adapter) + .await +} + +/// Both cookie-transport refreshes, each against a fresh adapter in front of +/// `api`, labelled for assertion messages. +async fn both_refreshes(api: &FakeApi) -> Vec<(&'static str, Reply)> { + vec![ + ("silent", silent_refresh(&api.adapter(|b| b)).await), + ("cookie", cookie_refresh(&api.adapter(|b| b)).await), + ] +} + +fn assert_kept(name: &str, r: &Reply) { + assert!( + r.cookies.is_empty(), + "{name}: touched cookies {:?}", + r.cookies.keys() + ); +} + +fn assert_unavailable(name: &str, r: &Reply) { + assert_eq!(r.status, 502, "{name}: {}", r.text); + assert_eq!(r.body, json!({ "error": "upstream_unavailable" }), "{name}"); + assert_kept(name, r); +} + +#[tokio::test] +async fn a_refresh_the_api_rejects_with_401_clears_the_session() { + let api = FakeApi::start().await; + api.on( + "POST", + "/refresh", + respond(401, json!({ "error": "refresh_token_reused" })), + ); + let replies = both_refreshes(&api).await; + + let (_, silent) = &replies[0]; + assert_eq!(silent.status, 401, "{}", silent.text); + assert_eq!(silent.body["error"], "Refresh failed"); + let (_, cookie) = &replies[1]; + assert_eq!(cookie.status, 401, "{}", cookie.text); + assert_eq!(cookie.body["error"], "invalid_refresh_token"); + for (name, r) in &replies { + assert!( + r.cleared("seamless-access") && r.cleared("seamless-refresh"), + "{name}: {:?}", + r.cookies + ); + } +} + +#[tokio::test] +async fn an_unreadable_refresh_cookie_clears_the_session() { + let api = FakeApi::start().await; + api.on("POST", "/refresh", respond(200, api.session("u1"))); + let adapter = api.adapter(|b| b); + let forged = "seamless-refresh=not-a-cookie"; + + let silent = call("GET", "/admin/applications") + .cookie(forged) + .to(&adapter) + .await; + assert_eq!(silent.status, 401, "{}", silent.text); + assert_eq!(silent.body["error"], "Refresh failed"); + let cookie = call("POST", "/refresh").cookie(forged).to(&adapter).await; + assert_eq!(cookie.status, 401, "{}", cookie.text); + assert_eq!(cookie.body["error"], "invalid_refresh_token"); + for (name, r) in [("silent", &silent), ("cookie", &cookie)] { + assert!( + r.cleared("seamless-access") && r.cleared("seamless-refresh"), + "{name}: {:?}", + r.cookies + ); + } + assert!(api.calls_to("POST", "/refresh").is_empty()); +} + +#[tokio::test] +async fn a_refresh_that_cannot_reach_the_api_keeps_the_session() { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let url = format!("http://{}", listener.local_addr().unwrap()); + drop(listener); + let adapter = || { + crate::Adapter::builder(&url) + .cookie_secret(TEST_SECRET) + .service_secret(TEST_SERVICE) + .disable_manifest_fetch(true) + .build() + .unwrap() + }; + + assert_unavailable("silent", &silent_refresh(&adapter()).await); + assert_unavailable("cookie", &cookie_refresh(&adapter()).await); +} + +#[tokio::test] +async fn a_refresh_the_api_answers_with_another_error_passes_it_through_and_keeps_the_session() { + let api = FakeApi::start().await; + api.on( + "POST", + "/refresh", + Arc::new(|_| { + let mut res = json_reply(503, json!({ "error": "maintenance" })); + res.headers_mut() + .insert("retry-after", axum::http::HeaderValue::from_static("120")); + res + }), + ); + + for (name, r) in both_refreshes(&api).await { + assert_eq!(r.status, 503, "{name}: {}", r.text); + assert_eq!(r.body, json!({ "error": "maintenance" }), "{name}"); + assert_eq!(r.headers["retry-after"], "120", "{name}"); + assert_kept(name, &r); + } + assert!(api.calls_to("GET", "/admin/applications").is_empty()); +} + +#[tokio::test] +async fn a_failed_refresh_is_not_kept_for_other_tabs() { + let api = FakeApi::start().await; + let calls = Arc::new(AtomicUsize::new(0)); + let counter = calls.clone(); + let session = api.session("u1"); + api.on( + "POST", + "/refresh", + Arc::new(move |_| { + if counter.fetch_add(1, Ordering::SeqCst) == 0 { + json_reply(503, json!({ "error": "maintenance" })) + } else { + json_reply(200, session.clone()) + } + }), + ); + let adapter = api.adapter(|b| b); + + let first = cookie_refresh(&adapter).await; + assert_eq!(first.status, 503, "{}", first.text); + assert_kept("first", &first); + let retry = cookie_refresh(&adapter).await; + assert_eq!(retry.status, 200, "{}", retry.text); + assert_eq!(calls.load(Ordering::SeqCst), 2); + assert!(retry.cookies.contains_key("seamless-access")); +} + +#[tokio::test] +async fn a_refresh_answered_with_no_session_keeps_the_session() { + let api = FakeApi::start().await; + api.on("POST", "/refresh", respond(200, json!({ "message": "ok" }))); + for (name, r) in both_refreshes(&api).await { + assert_unavailable(name, &r); + } +} + +#[tokio::test] +async fn a_refresh_answered_with_a_non_200_success_keeps_the_session() { + let api = FakeApi::start().await; + api.on("POST", "/refresh", respond(201, api.session("u1"))); + for (name, r) in both_refreshes(&api).await { + assert_unavailable(name, &r); + } +} + +#[tokio::test] +async fn a_rotated_session_that_fails_verification_keeps_the_session() { + let api = FakeApi::start().await; + let mut session = api.session("u1"); + session["token"] = json!(sign_rs256_with( + OTHER_KEY_PEM, + json!({ "sub": "u1", "sid": "s-1", "typ": "access", "iss": api.url, "aud": api.url }), + )); + api.on("POST", "/refresh", respond(200, session)); + for (name, r) in both_refreshes(&api).await { + assert_unavailable(name, &r); + } +} + +/// Resolves the auth API's host by hanging or by panicking, so the rotation never +/// settles or its task dies. +struct BrokenResolver { + panics: bool, +} + +impl reqwest::dns::Resolve for BrokenResolver { + fn resolve(&self, _: reqwest::dns::Name) -> reqwest::dns::Resolving { + assert!(!self.panics, "resolver panicked"); + Box::pin(std::future::pending()) + } +} + +fn broken_adapter(panics: bool) -> crate::Adapter { + let client = reqwest::Client::builder() + .dns_resolver(BrokenResolver { panics }) + .build() + .unwrap(); + crate::Adapter::builder("http://auth.invalid") + .cookie_secret(TEST_SECRET) + .service_secret(TEST_SERVICE) + .disable_manifest_fetch(true) + .http_client(client) + .build() + .unwrap() +} + +#[tokio::test(start_paused = true)] +async fn a_refresh_that_times_out_keeps_the_session() { + assert_unavailable("silent", &silent_refresh(&broken_adapter(false)).await); + assert_unavailable("cookie", &cookie_refresh(&broken_adapter(false)).await); +} + +#[tokio::test] +async fn a_refresh_whose_task_panics_keeps_the_session() { + assert_unavailable("silent", &silent_refresh(&broken_adapter(true)).await); + assert_unavailable("cookie", &cookie_refresh(&broken_adapter(true)).await); +} From fafd393dac5f3cfbf52e018213199baf800ed91a Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Tue, 3 Nov 2026 14:29:40 -0500 Subject: [PATCH 2/2] fix: keep the session cookies when a refresh fails transiently In cookie transport, the silent refresh on an auth route and POST /refresh now clear the session cookies only when the auth API answers the refresh with 401 or the refresh cookie cannot be read. An unreachable API, a rotation that times out or whose task panics, a 2xx other than a 200 with a verifiable session, all answer 502 upstream_unavailable; any other error from the API passes through with its status, body and Retry-After. The cookies are kept in every one of these cases so the next request tries again, matching the Go, Node and Python adapters. Bearer refresh is unchanged. --- README.md | 21 ++++++++++++++++++ src/refresh.rs | 58 ++++++++++++++++++++++++++++++++++++++++---------- src/routes.rs | 11 ++++++++-- 3 files changed, 77 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index baa696f..25fc27c 100644 --- a/README.md +++ b/README.md @@ -151,6 +151,27 @@ carry any value; only `bearer` selects bearer transport. `POST /refresh`, `DELET header, but they still get the cross-site check. Routes behind `require_auth` get the cross-site check only. +### Refresh + +In cookie transport the auth routes refresh an expired access cookie from the refresh cookie on +their own, and a browser can also call `POST /auth/refresh`. Parallel requests presenting one +refresh token share a single rotation, and a request presenting the same refresh cookie within 5 +seconds of a successful rotation gets that rotated session, so several tabs with one expired +session all succeed. A bearer client presenting a spent refresh token always reaches the auth API. + +Only a successful rotation is shared for those 5 seconds, and only a refresh the auth API refuses +ends a browser session. Both the silent refresh and `POST /auth/refresh` clear the session cookies +only when the auth API answers the refresh with 401 (the silent refresh answers 401 +`Refresh failed`, `POST /auth/refresh` answers 401 `invalid_refresh_token`) or when the refresh +cookie cannot be read. Any other failure keeps the cookies, so the next request tries again: + +| The refresh | The answer | +| --- | --- | +| does not reach the auth API | 502 `upstream_unavailable` | +| gets another non-2xx answer (a 503 during maintenance, a 429) | that status, body and `Retry-After` | +| gets a 2xx other than 200, a 200 with no session, or a session that fails verification | 502 `upstream_unavailable` | +| does not settle within 30 seconds, or its task panics | 502 `upstream_unavailable` | + ### Errors and logout An error from the auth API goes back with its status, its JSON body and its `Retry-After` header, diff --git a/src/refresh.rs b/src/refresh.rs index ae8425a..c17fa6b 100644 --- a/src/refresh.rs +++ b/src/refresh.rs @@ -179,7 +179,22 @@ impl Refresher { pub(crate) enum SilentRefreshError { NoRefreshCookie, - Failed(String), + /// The auth API refused the refresh token with 401, or the refresh cookie + /// cannot be read. The session is over, so its cookies are cleared. + Rejected(String), + /// The refresh failed without saying the session is over: the auth API was + /// unreachable or answered another error, or the rotation did not settle on a + /// session the adapter can vouch for. The answer to send, with the cookies + /// kept so the next request tries again. + Transient(Box), +} + +fn unavailable(err: &str) -> SilentRefreshError { + tracing::debug!("[seamless-auth] Refresh failed: {err}"); + SilentRefreshError::Transient(Box::new(Reply::error( + StatusCode::BAD_GATEWAY, + "upstream_unavailable", + ))) } impl Inner { @@ -200,7 +215,9 @@ impl Inner { .map(|p| str_claim(p, "refreshToken")) .unwrap_or(""); if refresh_token.is_empty() { - return Err(SilentRefreshError::Failed("invalid refresh cookie".into())); + return Err(SilentRefreshError::Rejected( + "invalid refresh cookie".into(), + )); } let subject = payload.as_ref().map(|p| str_claim(p, "sub")).unwrap_or(""); self.refresh_session( @@ -226,9 +243,10 @@ impl Inner { let mut extra = Claims::new(); extra.insert("refreshToken".into(), Value::from(refresh_token)); let Some(service) = self.tokens.sign(subject, extra) else { - return Err(SilentRefreshError::Failed( - "could not sign the service token".into(), - )); + return Err(SilentRefreshError::Transient(Box::new(Reply::error( + StatusCode::INTERNAL_SERVER_ERROR, + "internal_error", + )))); }; let inner = self.clone(); @@ -253,21 +271,38 @@ impl Inner { let session = match outcome { RefreshOutcome::Session(session) => session, - RefreshOutcome::Rejected { status, .. } => { - return Err(SilentRefreshError::Failed(format!( + RefreshOutcome::Rejected { status, .. } if status == StatusCode::UNAUTHORIZED => { + return Err(SilentRefreshError::Rejected("refresh answered 401".into())); + } + RefreshOutcome::Rejected { status, .. } if status.is_success() => { + return Err(unavailable(&format!( "refresh answered {}", status.as_u16() ))); } - RefreshOutcome::Failed(err) => return Err(SilentRefreshError::Failed(err)), + RefreshOutcome::Rejected { + status, + body, + is_json, + retry_after, + } => { + tracing::debug!( + "[seamless-auth] Refresh failed: refresh answered {}", + status.as_u16() + ); + return Err(SilentRefreshError::Transient(Box::new( + failure(status, body, is_json).with_retry_after(retry_after), + ))); + } + RefreshOutcome::Failed(err) => return Err(unavailable(&err)), }; let session_id = self .verify_session(&session, "access") .await - .map_err(SilentRefreshError::Failed)?; + .map_err(|err| unavailable(&err))?; let cookies = session_cookies(&self.config, &session, &session_id, true) - .map_err(SilentRefreshError::Failed)?; + .map_err(|err| unavailable(&err))?; Ok((session, cookies)) } @@ -356,13 +391,14 @@ impl Inner { Err(SilentRefreshError::NoRefreshCookie) => { Reply::error(StatusCode::UNAUTHORIZED, "refresh token required") } - Err(SilentRefreshError::Failed(err)) => { + Err(SilentRefreshError::Rejected(err)) => { tracing::debug!("[seamless-auth] Refresh failed: {err}"); Reply::error(StatusCode::UNAUTHORIZED, "invalid_refresh_token").with_clear(vec![ self.config.access_cookie_name.clone(), self.config.refresh_cookie_name.clone(), ]) } + Err(SilentRefreshError::Transient(reply)) => *reply, } } diff --git a/src/routes.rs b/src/routes.rs index a0b6bc9..7ea5e18 100644 --- a/src/routes.rs +++ b/src/routes.rs @@ -102,7 +102,8 @@ impl Inner { }); } Err(SilentRefreshError::NoRefreshCookie) => {} - Err(SilentRefreshError::Failed(err)) => { + Err(SilentRefreshError::Transient(reply)) => return Err(reply), + Err(SilentRefreshError::Rejected(err)) => { tracing::debug!("[seamless-auth] Silent refresh failed: {err}"); return Err(Box::new( Reply::error(StatusCode::UNAUTHORIZED, "Refresh failed").with_clear(vec![ @@ -460,9 +461,15 @@ impl Inner { reply.set.extend(cookies); } Err(SilentRefreshError::NoRefreshCookie) => {} - Err(SilentRefreshError::Failed(err)) => { + Err(SilentRefreshError::Rejected(err)) => { tracing::debug!("[seamless-auth] Refresh after recovery enrollment failed: {err}"); } + Err(SilentRefreshError::Transient(reply)) => { + tracing::debug!( + "[seamless-auth] Refresh after recovery enrollment failed: {}", + reply.status.as_u16() + ); + } } } }