diff --git a/crates/outer/src/auth/oauth.rs b/crates/outer/src/auth/oauth.rs index 3a8eb5b..8f55937 100644 --- a/crates/outer/src/auth/oauth.rs +++ b/crates/outer/src/auth/oauth.rs @@ -560,7 +560,9 @@ async fn authorize_post( } } -/// Allow and deny redirects carry `iss` (RFC 9207) plus `state` when sent. +/// Allow and deny hand the browser back to the client (see +/// `pages::return_page` for why it isn't a 302). The URL carries `iss` +/// (RFC 9207) plus `state` when sent. fn redirect(base: &str, params: &[(&str, &str)], state: Option<&str>, issuer: &str) -> Response { let mut url = match Url::parse(base) { Ok(url) => url, @@ -582,7 +584,15 @@ fn redirect(base: &str, params: &[(&str, &str)], state: Option<&str>, issuer: &s } pairs.append_pair("iss", issuer); } - (StatusCode::FOUND, [("location", url.to_string())], "").into_response() + let host = match url.host_str() { + Some(host) => host.to_string(), + None => format!("{}:", url.scheme()), + }; + html( + StatusCode::OK, + pages::return_page(url.as_str(), &host), + None, + ) } /// `POST /oauth/register` (RFC 7591): public clients only, HTTPS or loopback diff --git a/crates/outer/src/auth/pages.rs b/crates/outer/src/auth/pages.rs index 1df8a9e..d971e27 100644 --- a/crates/outer/src/auth/pages.rs +++ b/crates/outer/src/auth/pages.rs @@ -150,6 +150,33 @@ pub fn consent_page(client: &ClientDoc, scope: &str, request_id: &str, csrf: &st ) } +/// After Allow/Deny: hand the browser back to the client with a page that +/// navigates by itself (``) instead of a 302 answer to the form +/// POST. Browsers apply CSP `form-action` to every hop of a form +/// submission's redirect chain, and some clients bounce through more than one +/// origin before landing (Gemini: oauth-redirect.googleusercontent.com, then +/// Google's own pages), so a 302 gets the user stuck on the consent page. A +/// refresh is a fresh navigation that `form-action` doesn't govern. The link +/// is the fallback when refresh is disabled. +pub fn return_page(url: &str, host: &str) -> Markup { + html! { + (maud::DOCTYPE) + html lang="en" { + head { + meta charset="utf-8"; + meta name="viewport" content="width=device-width, initial-scale=1"; + meta http-equiv="refresh" content=(format!("0;url={url}")); + title { "Returning · Outer" } + } + body style="font-family:system-ui,sans-serif;max-width:28rem;margin:3rem auto;padding:0 1rem;line-height:1.5;" { + h1 style="font-size:1.25rem;" { "Outer" } + p { "Returning to " strong { (host) } "…" } + p { a href=(url) { "Continue to " (host) } } + } + } + } +} + pub fn error_page(title: &str, message: &str) -> Markup { layout( title, diff --git a/crates/outer/src/auth/tests.rs b/crates/outer/src/auth/tests.rs index 2597780..6071e8d 100644 --- a/crates/outer/src/auth/tests.rs +++ b/crates/outer/src/auth/tests.rs @@ -108,19 +108,16 @@ fn raw_set_session(headers: &HeaderMap) -> String { .expect("session cookie set") } -fn location_query(headers: &HeaderMap) -> (StatusCode, HashMap) { - let location = headers - .get("location") - .unwrap() - .to_str() - .unwrap() - .to_string(); - let params = url::Url::parse(&format!("http://redirect.invalid{location}")) +/// The client URL the Allow/Deny page navigates to (its ``). +fn return_query(body: &str) -> HashMap { + let start = body.find("0;url=").expect("refresh target") + "0;url=".len(); + let end = start + body[start..].find('"').expect("closing quote"); + let target = body[start..end].replace("&", "&"); + url::Url::parse(&target) .unwrap() .query_pairs() .map(|(key, value)| (key.to_string(), value.to_string())) - .collect(); - (StatusCode::FOUND, params) + .collect() } fn app_with(state: &AuthState) -> Router { @@ -316,9 +313,9 @@ async fn allow( ]), ); req.headers_mut().insert("cookie", session.parse().unwrap()); - let (status, headers, _) = send(&flow.app, req).await; - assert_eq!(status, StatusCode::FOUND); - let (_, params) = location_query(&headers); + let (status, _, body) = send(&flow.app, req).await; + assert_eq!(status, StatusCode::OK); + let params = return_query(&body); assert_eq!(params.get("state").map(String::as_str), Some("xyz-state")); assert_eq!(params.get("iss").map(String::as_str), Some(ISSUER)); assert!(params.contains_key("code")); @@ -450,9 +447,9 @@ async fn full_authorize_flow_mints_a_working_token() { ]), ); req.headers_mut().insert("cookie", session.parse().unwrap()); - let (status, headers, _) = send(&flow.app, req).await; - assert_eq!(status, StatusCode::FOUND); - let (_, params) = location_query(&headers); + let (status, _, body) = send(&flow.app, req).await; + assert_eq!(status, StatusCode::OK); + let params = return_query(&body); assert_eq!( params.get("error").map(String::as_str), Some("access_denied")