feat(auth): require a recent login to change a credential (#31)

crates/auth-core/src/lib.rs

@@ -19,6 +19,12 @@
 //! 3. **A signing key's private half never reaches an output.**
 //!    `tokens::SigningKeys` prints key ids only, and the published JWKS
 //!    is built from the re-derived public point (issue #9).
+//! 4. **Changing how somebody signs in needs a recent login, not just a
+//!    live session** (issue #31). [`require_recent_authentication`] is the
+//!    one rule, and the window is one key —
+//!    `AUTH_CORE_STEP_UP_WINDOW_SECS`, default 900 — because a deployment
+//!    that hardened one login method and forgot another would have
+//!    hardened nothing.
 
 #![forbid(unsafe_code)]
 
@@ -46,9 +52,11 @@ pub use secrets::{
     verify_password, verify_secret,
 };
 pub use sessions::{
-    ABSOLUTE_CAP_DAYS, COOKIE_NAME, IssuedSession, Login, SESSION_INVALID, SESSION_VALUE_BYTES,
-    SLIDE_AFTER_SECS, SLIDE_WINDOW_DAYS, Session, SessionError, ValidSession, clear_cookie,
-    cookie_value, issue, revoke_all, set_cookie, ua_family, validate,
+    ABSOLUTE_CAP_DAYS, COOKIE_NAME, DEFAULT_STEP_UP_WINDOW_SECS, IssuedSession, Login,
+    MAX_STEP_UP_WINDOW_SECS, MIN_STEP_UP_WINDOW_SECS, REAUTHENTICATION_REQUIRED, SESSION_INVALID,
+    SESSION_VALUE_BYTES, SLIDE_AFTER_SECS, SLIDE_WINDOW_DAYS, Session, SessionError, ValidSession,
+    clear_cookie, cookie_value, issue, require_recent_authentication, revoke_all, set_cookie,
+    step_up_window_secs, ua_family, validate,
 };
 pub use store::{
     Bytes, CLIENT_CONFIDENTIAL, CLIENT_PUBLIC, CREDENTIAL_PASSKEY, CREDENTIAL_PASSWORD,

crates/auth-core/src/sessions.rs

@@ -53,6 +53,32 @@ pub const SESSION_INVALID: cratefield_core::ProblemDef = cratefield_core::Proble
     description: "Missing, unknown, revoked or expired session — not distinguished",
 };
 
+/// Changing how somebody signs in needs an authentication newer than this,
+/// not merely a live session (issue #31). Fifteen minutes is the common
+/// default: long enough to add a passkey without signing in twice, short
+/// enough that a session left open on a shared machine cannot be turned
+/// into permanent access.
+pub const DEFAULT_STEP_UP_WINDOW_SECS: i64 = 900;
+/// Below a minute the window is unusable: a person cannot finish a WebAuthn
+/// ceremony inside it.
+pub const MIN_STEP_UP_WINDOW_SECS: i64 = 60;
+/// Above a day it is not a step-up, it is a session lifetime.
+pub const MAX_STEP_UP_WINDOW_SECS: i64 = 86_400;
+
+/// The session is real, but the login behind it is too old to change how
+/// this account signs in.
+///
+/// `403`, not `401`, and that distinction is the point: a client that got a
+/// `401` should send the person through a full login, while this one should
+/// re-authenticate them and retry. Answering both with
+/// [`SESSION_INVALID`] would make a step-up look like a logout.
+pub const REAUTHENTICATION_REQUIRED: cratefield_core::ProblemDef = cratefield_core::ProblemDef {
+    slug: "auth/reauthentication-required",
+    status: StatusCode::FORBIDDEN,
+    title: "Sign in again to change how you sign in",
+    description: "The session is valid, but the authentication behind it is older than the step-up window",
+};
+
 /// Failure while issuing a session: entropy or database. The cookie
 /// value never leaves the failing call.
 #[derive(Debug, thiserror::Error)]
@@ -75,6 +101,74 @@ pub enum SessionError {
 pub struct ValidSession {
     pub id: String,
     pub user_id: String,
+    /// When the login behind this session happened — the session's own
+    /// `created_at`, which sliding never moves.
+    pub authenticated_at: OffsetDateTime,
+    /// RFC 8176 method references recorded at that login, empty when the
+    /// login method wired none. Read rather than ignored so a later rule
+    /// can ask *how* somebody authenticated, not only how long ago.
+    pub amr: Vec<String>,
+}
+
+impl ValidSession {
+    /// Whether the login behind this session is newer than `window_secs`.
+    ///
+    /// A session whose `created_at` is in the future counts as recent: that
+    /// is clock skew between the writer and the reader, and the safe
+    /// failure for skew is not to lock somebody out of their own account
+    /// page. The window itself is validated on the way in, so it cannot be
+    /// negative here.
+    #[must_use]
+    pub fn authenticated_within(&self, now: OffsetDateTime, window_secs: i64) -> bool {
+        now - self.authenticated_at <= time::Duration::seconds(window_secs)
+    }
+}
+
+/// The step-up guard: a live session is not enough to change how somebody
+/// signs in (issue #31).
+///
+/// # Errors
+///
+/// [`REAUTHENTICATION_REQUIRED`] when the login behind the session is older
+/// than `window_secs`. The caller adds its own `instance`.
+pub fn require_recent_authentication(
+    session: &ValidSession,
+    now: OffsetDateTime,
+    window_secs: i64,
+) -> Result<(), Problem> {
+    if session.authenticated_within(now, window_secs) {
+        Ok(())
+    } else {
+        Err(Problem::new(&REAUTHENTICATION_REQUIRED))
+    }
+}
+
+/// Resolves the step-up window from configuration.
+///
+/// Owned by `auth-core` and keyed `AUTH_CORE_STEP_UP_WINDOW_SECS` rather
+/// than per module, because every login method has to apply the same rule:
+/// a deployment that hardened passkeys and forgot passwords would have
+/// hardened nothing.
+///
+/// # Errors
+///
+/// A message naming the key when the value is not a whole number of seconds
+/// inside [`MIN_STEP_UP_WINDOW_SECS`]..=[`MAX_STEP_UP_WINDOW_SECS`].
+pub fn step_up_window_secs(cfg: &dyn cratefield_core::Config) -> Result<i64, String> {
+    let module = cratefield_core::ModuleConfig::new("auth-core", cfg);
+    match module.get_opt("STEP_UP_WINDOW_SECS") {
+        None => Ok(DEFAULT_STEP_UP_WINDOW_SECS),
+        Some(raw) => match raw.trim().parse::<i64>() {
+            Ok(secs) if (MIN_STEP_UP_WINDOW_SECS..=MAX_STEP_UP_WINDOW_SECS).contains(&secs) => {
+                Ok(secs)
+            }
+            _ => Err(format!(
+                "{} must be between {MIN_STEP_UP_WINDOW_SECS} and \
+                 {MAX_STEP_UP_WINDOW_SECS} seconds, got {raw:?}",
+                module.key("STEP_UP_WINDOW_SECS")
+            )),
+        },
+    }
 }
 
 pub(crate) fn iso(t: OffsetDateTime) -> String {
@@ -357,9 +451,23 @@ pub async fn validate(
         }
     }
 
+    let authenticated_at = OffsetDateTime::parse(&row.created_at, &Rfc3339)
+        .map_err(|_| DbError::Query("session created_at is malformed".to_owned()))?;
+    // A malformed `amr` is not worth refusing a session over: it is
+    // provenance, not authority. Log it and carry on with none.
+    let amr = match row.amr.as_deref() {
+        None => Vec::new(),
+        Some(raw) => serde_json::from_str::<Vec<String>>(raw).unwrap_or_else(|_| {
+            tracing::warn!(session = %row.id, "session amr is not a JSON array of strings");
+            Vec::new()
+        }),
+    };
+
     Ok(Some(ValidSession {
         id: row.id,
         user_id: row.user_id,
+        authenticated_at,
+        amr,
     }))
 }
 

crates/auth-core/tests/sessions.rs

@@ -198,12 +198,20 @@ async fn issue_stores_only_the_hash_and_builds_the_host_cookie() {
     let ok = validate(&*kit.db, &at(EPOCH + 60), &issued.value)
         .await
         .expect("validate");
+    let ok = ok.expect("the session validates");
+    assert_eq!(ok.id, issued.session_id);
+    assert_eq!(ok.user_id, "u1");
+    // The login behind the session, not the moment it was last used: this
+    // is what the step-up rule reads, and sliding must never move it.
     assert_eq!(
-        ok,
-        Some(factory0_auth_core::ValidSession {
-            id: issued.session_id.clone(),
-            user_id: "u1".to_owned()
-        })
+        ok.authenticated_at.unix_timestamp(),
+        EPOCH,
+        "authenticated_at is not the login time"
+    );
+    assert_eq!(
+        ok.amr,
+        vec!["passkey".to_owned()],
+        "the amr recorded at login was lost"
     );
     assert!(
         validate(

crates/auth-passkeys/src/lib.rs

@@ -228,6 +228,9 @@ impl RelyingParty {
 pub(crate) struct ModuleState {
     pub ctx: Arc<ModuleContext>,
     pub rp: Option<RelyingParty>,
+    /// How recent a login has to be to add or remove a passkey (issue #31).
+    /// Resolved by `auth-core` so every login method reads one key.
+    pub step_up_window_secs: i64,
 }
 
 impl ModuleState {
@@ -295,16 +298,23 @@ impl Module for Passkeys {
     }
 
     fn validate_config(&self, cfg: &dyn Config) -> Result<(), ConfigError> {
-        match RelyingParty::from_config(cfg) {
-            Ok(_) => Ok(()),
-            Err(problems) => {
-                let mut errors = ConfigError::default();
-                for problem in problems {
-                    errors.push(format!("auth-passkeys: {problem}"));
-                }
-                Err(errors)
+        let mut errors = ConfigError::default();
+        if let Err(problems) = RelyingParty::from_config(cfg) {
+            for problem in problems {
+                errors.push(format!("auth-passkeys: {problem}"));
             }
         }
+        // The step-up window is auth-core's key, but a deployment that
+        // mistypes it should hear about it from whichever module enforces
+        // it rather than silently getting the default.
+        if let Err(problem) = factory0_auth_core::step_up_window_secs(cfg) {
+            errors.push(format!("auth-passkeys: {problem}"));
+        }
+        if errors.is_empty() {
+            Ok(())
+        } else {
+            Err(errors)
+        }
     }
 
     fn router(&self, ctx: ModuleContext) -> axum::Router {
@@ -321,9 +331,18 @@ impl Module for Passkeys {
                 None
             }
         };
+        // A bad value is reported by `validate_config`; here it degrades to
+        // the default rather than taking the module down, and the default
+        // is the stricter of the two outcomes anyway.
+        let step_up_window_secs = factory0_auth_core::step_up_window_secs(&*ctx.config)
+            .unwrap_or_else(|problem| {
+                tracing::error!(problem, "the step-up window is unusable; using the default");
+                factory0_auth_core::DEFAULT_STEP_UP_WINDOW_SECS
+            });
         let state = Arc::new(ModuleState {
             ctx: Arc::new(ctx),
             rp,
+            step_up_window_secs,
         });
         register::router().merge(login::router()).with_state(state)
     }

crates/auth-passkeys/src/register.rs

@@ -24,7 +24,9 @@ use webauthn_rs_proto::{
 };
 
 use crate::challenge::{self, PURPOSE_REGISTER};
-use crate::request::{b64u, ceremony_failed, hex, internal, ok, ports, require_session};
+use crate::request::{
+    b64u, ceremony_failed, hex, internal, ok, ports, require_recent_session, require_session,
+};
 use crate::webauthn::{UserVerification, verify_registration};
 use crate::{COSE_EDDSA, COSE_ES256, COSE_RS256, ModuleState};
 
@@ -52,7 +54,7 @@ async fn options(
     scope: Scope,
     headers: HeaderMap,
 ) -> Result<Response, Problem> {
-    let session = require_session(&state, &headers, &scope).await?;
+    let session = require_recent_session(&state, &headers, &scope).await?;
     let rp = state.rp()?;
     let (db, clock, id_gen) = ports(&state)?;
 
@@ -162,7 +164,7 @@ async fn verify(
     headers: HeaderMap,
     Json(body): Json<VerifyBody>,
 ) -> Result<Response, Problem> {
-    let session = require_session(&state, &headers, &scope).await?;
+    let session = require_recent_session(&state, &headers, &scope).await?;
     let rp = state.rp()?;
     let (db, clock, id_gen) = ports(&state)?;
 
@@ -344,7 +346,7 @@ async fn remove(
     headers: HeaderMap,
     Path(id): Path<String>,
 ) -> Result<Response, Problem> {
-    let session = require_session(&state, &headers, &scope).await?;
+    let session = require_recent_session(&state, &headers, &scope).await?;
     let (db, _, _) = ports(&state)?;
 
     let credentials = credentials_by_user(db, &session.user_id)

crates/auth-passkeys/src/request.rs

@@ -50,6 +50,33 @@ pub(crate) async fn require_session(
     }
 }
 
+/// The signed-in user, for an endpoint that **changes how they sign in**.
+///
+/// A live session is not enough here (issue #31). Adding a passkey to a
+/// hijacked session is the classic persistence step: the passkey outlives
+/// the password change and the session revocation that follow, and nothing
+/// about it looks unusual on an account page. So the login behind the
+/// session has to be recent, not merely valid.
+///
+/// The refusal is `403 auth/reauthentication-required`, distinct from the
+/// `401` a signed-out caller gets, so a client re-authenticates and retries
+/// instead of dropping the person into a full login.
+pub(crate) async fn require_recent_session(
+    state: &ModuleState,
+    headers: &HeaderMap,
+    scope: &Scope,
+) -> Result<ValidSession, Problem> {
+    let session = require_session(state, headers, scope).await?;
+    let (_, clock, _) = ports(state)?;
+    factory0_auth_core::require_recent_authentication(
+        &session,
+        clock.now(),
+        state.step_up_window_secs,
+    )
+    .map_err(|problem| problem.instance(&scope.request_id))?;
+    Ok(session)
+}
+
 /// The rate limit on the two endpoints anyone can call. Keyed on the client
 /// address **only**: keying on the email as well would let anyone lock a
 /// named account out of its own logins, which trades one denial of service