Harden login session rotation

e2e-tests/tests/gateway_session_test.rs

@@ -0,0 +1,200 @@
+//! Session-security regressions for the gateway.
+
+use std::time::Duration;
+
+use anyhow::{Context, ensure};
+use e2e_tests::common::{GatewayEnv, SharedServer};
+use reqwest::header::{COOKIE, SET_COOKIE};
+
+const TEST_PASSWORD: &str = "synthetic-gateway-test-password";
+const TEST_USER_ID: &str = "00000000-0000-0000-0000-000000000001";
+const SESSION_COOKIE: &str = "rwf_session";
+
+static GATEWAY: SharedServer<GatewayEnv> = SharedServer::new();
+
+async fn get_gateway() -> anyhow::Result<&'static GatewayEnv> {
+    GATEWAY.get(|| async { GatewayEnv::start().await }).await
+}
+
+fn test_client() -> anyhow::Result<reqwest::Client> {
+    reqwest::Client::builder()
+        .timeout(Duration::from_secs(5))
+        .build()
+        .context("failed to build reqwest client")
+}
+
+fn response_cookie(response: &reqwest::Response, name: &str) -> anyhow::Result<String> {
+    for header in response.headers().get_all(SET_COOKIE) {
+        let header = header.to_str().context("Set-Cookie header was not UTF-8")?;
+        let Some((cookie_name, rest)) = header.split_once('=') else {
+            continue;
+        };
+        if cookie_name.trim() != name {
+            continue;
+        }
+
+        let value = rest.split(';').next().unwrap_or_default();
+        ensure!(!value.is_empty(), "{name} cookie had an empty value");
+        return Ok(value.to_owned());
+    }
+
+    anyhow::bail!("response did not set the {name} cookie")
+}
+
+async fn rotate_refresh(
+    client: &reqwest::Client,
+    base: &str,
+    refresh_token: &str,
+) -> anyhow::Result<String> {
+    let rotation: serde_json::Value = client
+        .post(format!("{base}/auth/refresh"))
+        .json(&serde_json::json!({ "refresh_token": refresh_token }))
+        .send()
+        .await
+        .context("failed to rotate refresh token")?
+        .error_for_status()
+        .context("refresh-token rotation failed")?
+        .json()
+        .await
+        .context("refresh-token rotation returned invalid JSON")?;
+
+    rotation["refresh_token"]
+        .as_str()
+        .context("refresh-token rotation did not return a replacement")
+        .map(str::to_owned)
+}
+
+#[tokio::test]
+async fn login_rotates_pre_auth_session_id_and_invalidates_the_old_cookie() -> anyhow::Result<()> {
+    let gateway = get_gateway().await?;
+    let base = gateway.base_url();
+    let client = test_client()?;
+
+    // CSRF bootstrap deliberately creates useful pre-authentication session
+    // state. Capture that cookie as if another party already knew its ID.
+    let csrf_response = client
+        .get(format!("{base}/auth/csrf"))
+        .send()
+        .await
+        .context("failed to bootstrap CSRF session")?
+        .error_for_status()
+        .context("CSRF bootstrap failed")?;
+    let old_session = response_cookie(&csrf_response, SESSION_COOKIE)?;
+
+    // Authenticate while presenting the known pre-authentication session ID.
+    let login_response = client
+        .post(format!("{base}/auth/login"))
+        .header(COOKIE, format!("{SESSION_COOKIE}={old_session}"))
+        .json(&serde_json::json!({
+            "user_id": TEST_USER_ID,
+            "password": TEST_PASSWORD,
+        }))
+        .send()
+        .await
+        .context("failed to login with pre-authentication session")?
+        .error_for_status()
+        .context("login failed")?;
+    let new_session = response_cookie(&login_response, SESSION_COOKIE)?;
+
+    ensure!(
+        old_session != new_session,
+        "login retained the pre-authentication session ID"
+    );
+
+    // The exact old credential must no longer resolve to authenticated state.
+    let stale_whoami: serde_json::Value = client
+        .get(format!("{base}/session/whoami"))
+        .header(COOKIE, format!("{SESSION_COOKIE}={old_session}"))
+        .send()
+        .await
+        .context("failed to query stale session")?
+        .error_for_status()
+        .context("stale session whoami failed")?
+        .json()
+        .await
+        .context("stale session whoami returned invalid JSON")?;
+    ensure!(
+        stale_whoami["user_id"].is_null(),
+        "old pre-authentication session inherited authenticated state: {stale_whoami}"
+    );
+
+    // The rotated credential must still carry the authenticated session state.
+    let active_whoami: serde_json::Value = client
+        .get(format!("{base}/session/whoami"))
+        .header(COOKIE, format!("{SESSION_COOKIE}={new_session}"))
+        .send()
+        .await
+        .context("failed to query rotated session")?
+        .error_for_status()
+        .context("rotated session whoami failed")?
+        .json()
+        .await
+        .context("rotated session whoami returned invalid JSON")?;
+    ensure!(
+        active_whoami["user_id"] == TEST_USER_ID,
+        "rotated session did not retain authenticated state: {active_whoami}"
+    );
+
+    Ok(())
+}
+
+#[tokio::test]
+async fn replaying_rotated_refresh_token_revokes_the_active_family() -> anyhow::Result<()> {
+    let gateway = get_gateway().await?;
+    let base = gateway.base_url();
+    let client = test_client()?;
+
+    let login: serde_json::Value = client
+        .post(format!("{base}/auth/login"))
+        .json(&serde_json::json!({
+            "user_id": TEST_USER_ID,
+            "password": TEST_PASSWORD,
+        }))
+        .send()
+        .await
+        .context("failed to login before refresh rotation")?
+        .error_for_status()
+        .context("login before refresh rotation failed")?
+        .json()
+        .await
+        .context("login response returned invalid JSON")?;
+    let original_refresh = login["refresh_token"]
+        .as_str()
+        .context("login response did not contain a refresh token")?
+        .to_owned();
+
+    // Rotate twice before replaying anything. The second successful rotation is
+    // an observable proof that the first replacement was genuinely usable; this
+    // prevents the final 401 assertion from false-greening on an already-dead
+    // replacement token.
+    let first_replacement = rotate_refresh(&client, &base, &original_refresh).await?;
+    let active_descendant = rotate_refresh(&client, &base, &first_replacement).await?;
+
+    // Reusing the exact original capability is the replay signal. It must fail
+    // and revoke every still-active descendant in that token family.
+    let replay = client
+        .post(format!("{base}/auth/refresh"))
+        .json(&serde_json::json!({ "refresh_token": original_refresh }))
+        .send()
+        .await
+        .context("failed to replay rotated refresh token")?;
+    ensure!(
+        replay.status() == reqwest::StatusCode::UNAUTHORIZED,
+        "replayed refresh token was accepted: {}",
+        replay.status()
+    );
+
+    let descendant_after_replay = client
+        .post(format!("{base}/auth/refresh"))
+        .json(&serde_json::json!({ "refresh_token": active_descendant }))
+        .send()
+        .await
+        .context("failed to probe active descendant after replay")?;
+    ensure!(
+        descendant_after_replay.status() == reqwest::StatusCode::UNAUTHORIZED,
+        "refresh-token replay did not revoke the active family: {}",
+        descendant_after_replay.status()
+    );
+
+    Ok(())
+}

gateway/src/auth/handlers.rs

@@ -139,15 +139,42 @@ pub async fn login_handler(
     .await
     .map_err(|e| AppError::internal("refresh token insert", e))?;
 
-    session
+    // A pre-authentication session ID may already be known to another party
+    // (for example after bootstrapping a CSRF token). Rotate it before attaching
+    // authenticated state so possession of the old ID cannot inherit the login.
+    if session.id().is_some()
+        && let Err(error) = session.cycle_id().await
+    {
+        remove_unissued_refresh_token(pool, refresh_jti).await;
+        return Err(AppError::internal("session id rotation", error));
+    }
+
+    if let Err(error) = session
         .insert(
             crate::session::SESSION_USER_KEY,
             crate::session::SessionUser {
                 user_id: user_id.clone(),
             },
         )
         .await
-        .map_err(|e| AppError::internal("session insert", e))?;
+    {
+        remove_unissued_refresh_token(pool, refresh_jti).await;
+        return Err(AppError::internal("session insert", error));
+    }
+
+    // tower-sessions normally persists modified state after the handler returns.
+    // Persist once here as well so a store failure is observable while we can
+    // still compensate the refresh-token insert rather than returning a valid
+    // refresh credential for a login whose authenticated session was not saved.
+    if let Err(error) = session.save().await {
+        // The outer SessionManagerLayer will still inspect this Session after the
+        // handler returns. Remove authenticated in-memory state first so a
+        // transient retry by that middleware cannot turn this 500 response into
+        // a successfully persisted authenticated session/cookie.
+        session.clear().await;
+        remove_unissued_refresh_token(pool, refresh_jti).await;
+        return Err(AppError::internal("session save", error));
+    }
 
     Ok(Json(LoginResponse {
         token,
@@ -156,6 +183,23 @@ pub async fn login_handler(
     }))
 }
 
+/// Remove a refresh credential that was persisted for a login which could not
+/// complete its session transition. Cleanup is best-effort because the original
+/// session-store error remains the request's primary failure.
+async fn remove_unissued_refresh_token(pool: &sqlx::PgPool, refresh_jti: Uuid) {
+    if let Err(error) = sqlx::query("DELETE FROM refresh_tokens WHERE jti = $1")
+        .bind(refresh_jti)
+        .execute(pool)
+        .await
+    {
+        tracing::error!(
+            refresh_jti = %refresh_jti,
+            error = %error,
+            "failed to remove refresh token after login session failure"
+        );
+    }
+}
+
 #[utoipa::path(
     post,
     path = "/auth/refresh",