Prevent caching of OIDC redirects

Author: lovasoa

CHANGELOG.md

@@ -3,6 +3,7 @@
 ## unreleased
 
 - **Access logs now go to stdout.** SQLPage now writes the single per-request completion log line to stdout with the target `sqlpage::access`, matching common application-server and container logging conventions. Diagnostic logs, warnings, and internal errors still go to stderr. If your `LOG_LEVEL` or `RUST_LOG` filter is scoped to a specific old target such as `sqlpage::webserver::http=info`, add `sqlpage::access=info` so request-completion logs are still emitted. If your log pipeline only collects stderr, update it to collect stdout too.
+- **OIDC redirects are no longer cacheable.** Authorization redirects contain one-time state and post-login redirects set session cookies. SQLPage now sends `Cache-Control: no-store` for these responses, preventing a browser or intermediary from replaying an expired authorization redirect.
 
 ## v0.44.1
 

src/webserver/oidc.rs

@@ -870,6 +870,9 @@ fn build_auth_provider_redirect_response(
     .finish();
     let mut response = HttpResponse::SeeOther();
     response.append_header((header::LOCATION, url.to_string()));
+    // The location contains a one-time CSRF state. A cached redirect would
+    // replay it after its state cookie has been consumed.
+    response.append_header((header::CACHE_CONTROL, "no-store"));
     if let Ok(cookies) = request.cookies() {
         for mut cookie in get_tmp_login_flow_state_cookies_to_evict(&cookies).cloned() {
             cookie.make_removal();
@@ -884,6 +887,7 @@ fn build_auth_provider_redirect_response(
 fn build_redirect_response(target_url: String) -> HttpResponse {
     HttpResponse::SeeOther()
         .append_header(("Location", target_url))
+        .append_header((header::CACHE_CONTROL, "no-store"))
         .body("Redirecting...")
 }
 

tests/oidc/mod.rs

@@ -264,6 +264,17 @@ fn get_query_param(url: &Url, name: &str) -> String {
         .to_string()
 }
 
+fn permits_storage_after_proxy_adds_freshness(headers: &header::HeaderMap) -> bool {
+    // The supplied HAR has `Cache-Control: max-age=86400` on the OIDC 303s,
+    // despite SQLPage not setting it. This models a proxy adding that directive:
+    // `no-store` still wins when both directives are present.
+    !headers
+        .get_all(header::CACHE_CONTROL)
+        .filter_map(|value| value.to_str().ok())
+        .flat_map(|value| value.split(','))
+        .any(|directive| directive.trim().eq_ignore_ascii_case("no-store"))
+}
+
 macro_rules! request_with_cookies {
     ($app:expr, $req:expr, $cookies:expr) => {{
         let mut req = $req;
@@ -325,13 +336,79 @@ async fn setup_oidc_test(
     (app, provider)
 }
 
+#[actix_web::test]
+async fn test_oidc_cached_authorization_redirect_cannot_replay_consumed_state() {
+    let (app, provider) = setup_oidc_test(|_| {}).await;
+    let mut cookies: Vec<Cookie<'static>> = Vec::new();
+
+    // Chrome's private cache can replay a cached 303 without reapplying its
+    // Set-Cookie headers. This is the sequence captured in #1341's HAR.
+    let initial_response = request_with_cookies!(app, test::TestRequest::get().uri("/"), cookies);
+    let initial_auth_url = Url::parse(
+        initial_response
+            .headers()
+            .get(header::LOCATION)
+            .unwrap()
+            .to_str()
+            .unwrap(),
+    )
+    .unwrap();
+    let cached_authorization_url =
+        permits_storage_after_proxy_adds_freshness(initial_response.headers())
+            .then_some(initial_auth_url.clone());
+
+    let state = get_query_param(&initial_auth_url, "state");
+    let nonce = get_query_param(&initial_auth_url, "nonce");
+    let redirect_uri = get_query_param(&initial_auth_url, "redirect_uri");
+    let callback_path = Url::parse(&redirect_uri).unwrap().path().to_owned();
+    provider.store_auth_code("first-code".to_string(), nonce.clone());
+    let callback_uri = format!("{callback_path}?code=first-code&state={state}");
+    let callback_response =
+        request_with_cookies!(app, test::TestRequest::get().uri(&callback_uri), cookies);
+    assert_eq!(callback_response.status(), StatusCode::SEE_OTHER);
+
+    if let Some(stale_authorization_url) = cached_authorization_url {
+        // A fresh Entra code is returned for the authorization URL cached by
+        // Chrome, but its state is the state that the successful callback just
+        // consumed. Before this fix, SQLPage restarted OIDC here, creating the
+        // observed redirect loop.
+        let stale_state = get_query_param(&stale_authorization_url, "state");
+        provider.store_auth_code("stale-code".to_string(), nonce);
+        let stale_callback_uri = format!("{callback_path}?code=stale-code&state={stale_state}");
+        let stale_callback_response = request_with_cookies!(
+            app,
+            test::TestRequest::get().uri(&stale_callback_uri),
+            cookies
+        );
+        panic!(
+            "a cacheable authorization redirect replays a consumed state; stale callback redirected to {}",
+            stale_callback_response
+                .headers()
+                .get(header::LOCATION)
+                .unwrap()
+                .to_str()
+                .unwrap()
+        );
+    }
+
+    // With `no-store`, Chrome re-requests `/` after login and sends its new
+    // auth cookie instead of following the original authorization redirect.
+    let final_response = request_with_cookies!(app, test::TestRequest::get().uri("/"), cookies);
+    assert_eq!(final_response.status(), StatusCode::OK);
+}
+
 #[actix_web::test]
 async fn test_oidc_happy_path() {
     let (app, provider) = setup_oidc_test(|_| {}).await;
     let mut cookies: Vec<Cookie<'static>> = Vec::new();
 
     let resp = request_with_cookies!(app, test::TestRequest::get().uri("/"), cookies);
     assert_eq!(resp.status(), StatusCode::SEE_OTHER);
+    assert_eq!(
+        resp.headers().get(header::CACHE_CONTROL).unwrap(),
+        "no-store",
+        "the authorization redirect contains a one-time state and must not be cached"
+    );
     let auth_url = Url::parse(resp.headers().get("location").unwrap().to_str().unwrap()).unwrap();
 
     let state = get_query_param(&auth_url, "state");
@@ -347,6 +424,11 @@ async fn test_oidc_happy_path() {
     let callback_resp =
         request_with_cookies!(app, test::TestRequest::get().uri(&callback_uri), cookies);
     assert_eq!(callback_resp.status(), StatusCode::SEE_OTHER);
+    assert_eq!(
+        callback_resp.headers().get(header::CACHE_CONTROL).unwrap(),
+        "no-store",
+        "the post-login redirect must not re-enter a cached authorization redirect"
+    );
 
     let final_resp = request_with_cookies!(app, test::TestRequest::get().uri("/"), cookies);
     assert_eq!(final_resp.status(), StatusCode::OK);