fix(auth-core): make the fixation revoke fire on the form_post path (#36)

crates/auth-core/src/federated.rs

@@ -50,6 +50,9 @@ pub struct Caller<'a> {
     /// The session cookie the request presented, revoked before a new
     /// session exists (the fixation defence in [`issue`]).
     pub presented_cookie: Option<&'a str>,
+    /// The id of that session, for a `form_post` callback where the cookie
+    /// itself cannot arrive. See [`Login::presented_session_id`].
+    pub presented_session_id: Option<&'a str>,
     pub ip: Option<&'a str>,
     pub user_agent: Option<&'a str>,
 }
@@ -200,6 +203,7 @@ pub async fn complete(
             ip: caller.ip,
             user_agent: caller.user_agent,
             presented_cookie: caller.presented_cookie,
+            presented_session_id: caller.presented_session_id,
             amr,
         },
     )

crates/auth-core/src/sessions.rs

@@ -175,10 +175,24 @@ pub struct Login<'a> {
     pub ip: Option<&'a str>,
     pub user_agent: Option<&'a str>,
     /// The raw cookie the login request presented, if any: the session it
-    /// names is revoked before the new one exists (fixation). `None` on a
-    /// `form_post` callback, where the `SameSite=Lax` cookie does not arrive,
-    /// so the revoke does not fire there (see [`issue`]; auth #36).
+    /// names is revoked before the new one exists (fixation).
     pub presented_cookie: Option<&'a str>,
+    /// The **id** of the session the browser had, for the paths where the
+    /// cookie itself cannot arrive.
+    ///
+    /// On a `form_post` callback the session cookie is `SameSite=Lax` and
+    /// does not come with a cross-site POST, so `presented_cookie` is
+    /// always `None` there and the revoke above could never fire (auth
+    /// #36). The provider modules seal this id into the signed flow at
+    /// `/start`, which *is* same-site, and hand it back here.
+    ///
+    /// It is the id and not the value on purpose: the value is a bearer
+    /// credential, and putting a second copy of it inside another cookie
+    /// widens its exposure for nothing. An id is useless without the row.
+    ///
+    /// Ignored when `presented_cookie` is `Some` — that path already
+    /// identifies the same session, from the stronger evidence.
+    pub presented_session_id: Option<&'a str>,
     /// Authentication-method references (RFC 8176) for this login —
     /// `passkey` methods say `["user","passkey"]`-style values when the
     /// login-method issues wire them (issues #13-#22). Stored as the
@@ -208,19 +222,23 @@ fn audit(action: &str, user_id: &str) {
 /// Issues a session on a successful login: 32 random bytes in the
 /// cookie, only the SHA-256 stored, expiry `now + 30 days`.
 ///
-/// **Fixation defence, and its one blind spot.** A session whose raw
-/// cookie the request presents ([`Login::presented_cookie`]) is revoked
-/// first, so a fixed pre-login value dies here. This does not fire on a
-/// `form_post` callback (Apple): the session cookie is `SameSite=Lax` and
-/// does not arrive on a cross-site POST, so `presented_cookie` is `None`
-/// there. That is not a hole — the cookie is `__Host-` and `HttpOnly`, so
-/// it cannot be planted cross-site, which is the attack the revoke exists
-/// for; the browser has also just overwritten it with the new value. The
-/// only residue is the old server-side row living until it expires. Firing
-/// the revoke on every path needs the session **id** sealed into the flow
-/// at `/start` and a `presented_session_id` on [`Login`]
-/// ([auth #36](https://github.com/Factory-Zero/auth/issues/36)); until then,
-/// do not read this as "always revokes".
+/// **Fixation defence.** The session the browser was carrying is revoked
+/// before the new one exists, so a fixed pre-login value dies here. It is
+/// identified either by the raw cookie the request presented
+/// ([`Login::presented_cookie`]) or, where that cannot arrive, by the id
+/// the caller carried across ([`Login::presented_session_id`]).
+///
+/// The second exists because a `form_post` callback is a cross-site POST
+/// and the session cookie is `SameSite=Lax`, so it is simply not sent: for
+/// Apple and Meta the cookie path can never fire (auth #36). Those modules
+/// seal the session id into their signed flow at `/start`, which is
+/// same-site, and pass it back here.
+///
+/// Revoking is deliberate rather than incidental. The browser overwrites
+/// its own cookie either way, so what this buys is the server-side row not
+/// outliving the login that replaced it — a session listing that tells the
+/// truth, and a revoked row that cannot be resurrected by anyone who
+/// captured the old value earlier.
 ///
 /// # Errors
 ///
@@ -248,10 +266,18 @@ pub async fn issue(
         }
     }
 
-    if let Some(presented) = login.presented_cookie
-        && let Some(old) =
+    // The cookie is the stronger evidence, so it wins where both are
+    // present; the id is the fallback for the paths a cookie cannot reach.
+    let superseded = match login.presented_cookie {
+        Some(presented) => {
             store::session_by_token_hash(db, &sha256_raw(presented.as_bytes())).await?
-    {
+        }
+        None => match login.presented_session_id {
+            Some(id) => store::session_by_id(db, id).await?,
+            None => None,
+        },
+    };
+    if let Some(old) = superseded {
         store::revoke_session(db, &old.id, &iso(clock.now())).await?;
         audit("session.revoke", login.user_id);
     }

crates/auth-core/tests/authorize.rs

@@ -154,6 +154,7 @@ async fn signed_in(kit: &TestHarness, user_id: &str) -> String {
             ip: None,
             user_agent: None,
             presented_cookie: None,
+            presented_session_id: None,
             amr: &[],
         },
     )

crates/auth-core/tests/sessions.rs

@@ -78,6 +78,7 @@ fn a_session_is_never_issued_to_an_account_that_is_not_active() {
                 ip: None,
                 user_agent: None,
                 presented_cookie: None,
+                presented_session_id: None,
                 amr: &["passkey"],
             },
         )
@@ -98,6 +99,7 @@ fn a_session_is_never_issued_to_an_account_that_is_not_active() {
                 ip: None,
                 user_agent: None,
                 presented_cookie: None,
+                presented_session_id: None,
                 amr: &["passkey"],
             },
         )
@@ -127,6 +129,7 @@ async fn login(
             ip: Some("203.0.113.7"),
             user_agent: Some("Mozilla/5.0 Macintosh Safari/605.1.15"),
             presented_cookie: presented,
+            presented_session_id: None,
             amr: &["passkey"],
         },
     )
@@ -439,6 +442,7 @@ async fn the_expired_shape_through_the_router_is_the_same_401() {
             ip: None,
             user_agent: None,
             presented_cookie: None,
+            presented_session_id: None,
             amr: &[],
         },
     )
@@ -472,6 +476,7 @@ async fn listing_shows_ip_hash_ua_family_and_the_current_flag() {
             ip: Some("198.51.100.9"),
             user_agent: Some("Mozilla/5.0 Windows Chrome/120.0 Safari/537.36"),
             presented_cookie: None,
+            presented_session_id: None,
             amr: &[],
         },
     )

crates/auth-core/tests/tokens.rs

@@ -121,6 +121,7 @@ async fn seed_session(
             ip: Some("203.0.113.7"),
             user_agent: Some("Mozilla/5.0 Macintosh Safari/605.1.15"),
             presented_cookie: None,
+            presented_session_id: None,
             amr,
         },
     )

crates/auth-magic-link/src/handlers.rs

@@ -521,6 +521,9 @@ async fn spend(
                 .get(header::USER_AGENT)
                 .and_then(|value| value.to_str().ok()),
             presented_cookie: presented.as_deref(),
+            // A top-level GET from the mailed link, so the cookie above
+            // arrives and names the session itself (auth #36).
+            presented_session_id: None,
             amr: &AMR,
         },
     )

crates/auth-meta/src/flow.rs

@@ -48,6 +48,16 @@ pub(crate) struct Flow {
     /// the answer `/start` saw is the one the linking rules asked about.
     #[serde(default, skip_serializing_if = "Option::is_none")]
     pub signed_in_user: Option<String>,
+    /// The id of the session that person was carrying at `/start`.
+    ///
+    /// Carried for the same reason as `signed_in_user` and revoked at the
+    /// callback: the fixation defence in `auth-core`'s `issue` needs to
+    /// name the superseded session, and on a `form_post` callback the
+    /// cookie that would name it never arrives (auth #36). The id, not the
+    /// value — the value is a bearer credential and does not belong in a
+    /// second cookie.
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub signed_in_session_id: Option<String>,
 }
 
 impl Flow {
@@ -135,6 +145,7 @@ mod tests {
             return_to: "/".to_owned(),
             expires_at,
             signed_in_user: None,
+            signed_in_session_id: None,
         }
     }
 

crates/auth-meta/src/handlers.rs

@@ -207,14 +207,19 @@ async fn start(
     };
 
     // Who is signed in now, read here because `/start` is same-site.
-    let signed_in_user = match (ctx.ports.db.as_deref(), session_cookie_value(&headers)) {
+    // Both halves of the answer: who it is, and which session row it is,
+    // so the callback can revoke that row even though the cookie naming it
+    // will not arrive there (auth #36).
+    let signed_in = match (ctx.ports.db.as_deref(), session_cookie_value(&headers)) {
         (Some(db), Some(value)) => factory0_auth_core::validate(db, clock, &value)
             .await
             .ok()
-            .flatten()
-            .map(|session| session.user_id),
+            .flatten(),
         _ => None,
     };
+    let (signed_in_user, signed_in_session_id) = signed_in.map_or((None, None), |session| {
+        (Some(session.user_id), Some(session.id))
+    });
 
     let challenge =
         PkceCodeChallenge::from_code_verifier_sha256(&PkceCodeVerifier::new(verifier.clone()));
@@ -234,6 +239,7 @@ async fn start(
             .unwrap_or_else(|| settings.default_return_to.clone()),
         expires_at: clock.now().unix_timestamp() + flow::TTL_SECS,
         signed_in_user,
+        signed_in_session_id,
     }
     .seal(signer);
 
@@ -389,6 +395,7 @@ async fn callback(
         &Caller {
             current_user: current_user.as_deref(),
             presented_cookie: presented.as_deref(),
+            presented_session_id: flow.signed_in_session_id.as_deref(),
             ip: ip.as_deref(),
             user_agent,
         },

crates/auth-oidc/src/flow.rs

@@ -55,6 +55,16 @@ pub(crate) struct Flow {
     /// only carries the answer across, inside a payload we signed.
     #[serde(default, skip_serializing_if = "Option::is_none")]
     pub signed_in_user: Option<String>,
+    /// The id of the session that person was carrying at `/start`.
+    ///
+    /// Carried for the same reason as `signed_in_user` and revoked at the
+    /// callback: the fixation defence in `auth-core`'s `issue` needs to
+    /// name the superseded session, and on a `form_post` callback the
+    /// cookie that would name it never arrives (auth #36). The id, not the
+    /// value — the value is a bearer credential and does not belong in a
+    /// second cookie.
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub signed_in_session_id: Option<String>,
 }
 
 impl Flow {
@@ -207,6 +217,7 @@ mod tests {
             return_to: "/v1/auth-core/authorize?x=1".to_owned(),
             expires_at,
             signed_in_user: None,
+            signed_in_session_id: None,
         }
     }
 

crates/auth-oidc/src/handlers.rs

@@ -239,14 +239,19 @@ async fn start(
     // arrives here even for a provider whose callback will not carry it.
     // Sealing the answer into the flow is what makes "add this provider to
     // my account" work for a `form_post` provider at all.
-    let signed_in_user = match (ctx.ports.db.as_deref(), session_cookie_value(&headers)) {
+    // Both halves of the answer: who it is, and which session row it is,
+    // so the callback can revoke that row even though the cookie naming it
+    // will not arrive there (auth #36).
+    let signed_in = match (ctx.ports.db.as_deref(), session_cookie_value(&headers)) {
         (Some(db), Some(value)) => factory0_auth_core::validate(db, clock, &value)
             .await
             .ok()
-            .flatten()
-            .map(|session| session.user_id),
+            .flatten(),
         _ => None,
     };
+    let (signed_in_user, signed_in_session_id) = signed_in.map_or((None, None), |session| {
+        (Some(session.user_id), Some(session.id))
+    });
 
     let sealed = Flow {
         provider: provider.slug.to_owned(),
@@ -257,6 +262,7 @@ async fn start(
             .unwrap_or_else(|| settings.default_return_to.clone()),
         expires_at: clock.now().unix_timestamp() + flow::TTL_SECS,
         signed_in_user,
+        signed_in_session_id,
     }
     .seal(signer);
 
@@ -443,6 +449,7 @@ async fn complete_callback(
         &session::Caller {
             current_user: current_user.as_deref(),
             presented_cookie: presented.as_deref(),
+            presented_session_id: flow.signed_in_session_id.as_deref(),
             ip: ip.as_deref(),
             user_agent,
         },

crates/auth-oidc/src/session.rs

@@ -50,6 +50,10 @@ pub(crate) struct Caller<'a> {
     /// The session cookie the request presented, revoked before a new
     /// session exists (auth-core's fixation defence).
     pub presented_cookie: Option<&'a str>,
+    /// The id of that session, sealed into the flow at `/start`. On a
+    /// `form_post` callback the cookie above never arrives, so this is the
+    /// only thing that can name the session being superseded (auth #36).
+    pub presented_session_id: Option<&'a str>,
     pub ip: Option<&'a str>,
     pub user_agent: Option<&'a str>,
 }
@@ -86,6 +90,7 @@ pub(crate) async fn complete(
         &CoreCaller {
             current_user: caller.current_user,
             presented_cookie: caller.presented_cookie,
+            presented_session_id: caller.presented_session_id,
             ip: caller.ip,
             user_agent: caller.user_agent,
         },