From fee2d3b0d68f8d45ad8097bbe93f50084d76256d Mon Sep 17 00:00:00 2001 From: Firstyear Date: Tue, 7 Jan 2025 16:05:14 +1000 Subject: [PATCH] Resolve passkey regression (#3343) During other testing I noticed that passkeys no longer worked on a reauthentication. This was due to a regression in you guessed it, cookies, where the auth session id wasn't being removed properly. --- proto/src/internal/error.rs | 16 ++++++++++++++++ server/core/src/https/views/login.rs | 16 +++++++--------- server/lib/src/idm/authsession.rs | 24 +++++++++++++++++------- 3 files changed, 40 insertions(+), 16 deletions(-) diff --git a/proto/src/internal/error.rs b/proto/src/internal/error.rs index c14aa6a56..95309648e 100644 --- a/proto/src/internal/error.rs +++ b/proto/src/internal/error.rs @@ -142,6 +142,13 @@ pub enum OperationError { DatabaseLockAcquisitionTimeout, // Specific internal errors. + AU0001InvalidState, + AU0002JwsSerialisation, + AU0003JwsSignature, + AU0004UserAuthTokenInvalid, + AU0005DelayedProcessFailure, + AU0006CredentialMayNotReauthenticate, + AU0007UserAuthTokenInvalid, // Kanidm Generic Errors KG001TaskTimeout, @@ -347,6 +354,15 @@ impl OperationError { Self::TransactionAlreadyCommitted => None, Self::ValueDenyName => None, Self::DatabaseLockAcquisitionTimeout => Some("Unable to acquire a database lock - the current server may be too busy. Try again later.".into()), + + Self::AU0001InvalidState => Some("Invalid authentication session state for request".into()), + Self::AU0002JwsSerialisation => Some("JWS serialisation failed".into()), + Self::AU0003JwsSignature => Some("JWS signature failed".into()), + Self::AU0004UserAuthTokenInvalid => Some("User auth token was unable to be generated".into()), + Self::AU0005DelayedProcessFailure => Some("Delaying processing failure, unable to proceed".into()), + Self::AU0006CredentialMayNotReauthenticate => Some("Credential may not reauthenticate".into()), + Self::AU0007UserAuthTokenInvalid => Some("User auth token was unable to be generated".into()), + Self::CU0001WebauthnAttestationNotTrusted => None, Self::CU0002WebauthnRegistrationError => None, Self::CU0003WebauthnUserNotVerified => Some("User Verification bit not set while registering credential, you may need to configure a PIN on this device.".into()), diff --git a/server/core/src/https/views/login.rs b/server/core/src/https/views/login.rs index e9066e2a6..300a14798 100644 --- a/server/core/src/https/views/login.rs +++ b/server/core/src/https/views/login.rs @@ -12,7 +12,7 @@ use axum::{ response::{IntoResponse, Redirect, Response}, Extension, Form, Json, }; -use axum_extra::extract::cookie::{Cookie, CookieJar, SameSite}; +use axum_extra::extract::cookie::{CookieJar, SameSite}; use kanidm_proto::internal::{ COOKIE_AUTH_SESSION_ID, COOKIE_BEARER_TOKEN, COOKIE_CU_SESSION_TOKEN, COOKIE_OAUTH2_REQ, COOKIE_USERNAME, @@ -835,10 +835,8 @@ async fn view_login_step( break res; } AuthState::Continue(allowed) => { - // Reauth inits its session here so we need to be able to add cookie here ig. - if jar.get(COOKIE_AUTH_SESSION_ID).is_none() { - jar = add_session_cookie(&state, jar, &session_context)?; - } + // Reauth inits its session here so we need to be able to add it's cookie here. + jar = add_session_cookie(&state, jar, &session_context)?; let res = match allowed.len() { // Shouldn't be possible. @@ -936,9 +934,9 @@ async fn view_login_step( jar }; - jar = jar - .add(bearer_cookie) - .remove(Cookie::from(COOKIE_AUTH_SESSION_ID)); + jar = jar.add(bearer_cookie); + + jar = cookies::destroy(jar, COOKIE_AUTH_SESSION_ID, &state); // Now, we need to decided where to go. let res = if jar.get(COOKIE_OAUTH2_REQ).is_some() { @@ -955,7 +953,7 @@ async fn view_login_step( } AuthState::Denied(reason) => { debug!("🧩 -> AuthState::Denied"); - jar = jar.remove(Cookie::from(COOKIE_AUTH_SESSION_ID)); + jar = cookies::destroy(jar, COOKIE_AUTH_SESSION_ID, &state); break LoginDeniedView { display_ctx, diff --git a/server/lib/src/idm/authsession.rs b/server/lib/src/idm/authsession.rs index a3a225417..0d544019e 100644 --- a/server/lib/src/idm/authsession.rs +++ b/server/lib/src/idm/authsession.rs @@ -1383,7 +1383,17 @@ impl AuthSession { | AuthSessionState::InProgress(CredHandler::PasswordSecurityKey { .. }) | AuthSessionState::InProgress(CredHandler::Passkey { .. }) | AuthSessionState::InProgress(CredHandler::AttestedPasskey { .. }) => Ok(None), - _ => Err(OperationError::InvalidState), + + AuthSessionState::Init(_) => { + debug!( + "Request for credential uuid invalid as auth session state not yet initialised" + ); + Err(OperationError::AU0001InvalidState) + } + AuthSessionState::Success | AuthSessionState::Denied(_) => { + debug!("Request for credential uuid invalid as auth session state has progressed"); + Err(OperationError::AU0001InvalidState) + } } } @@ -1485,13 +1495,13 @@ impl AuthSession { let jwt = Jws::into_json(&uat).map_err(|e| { admin_error!(?e, "Failed to serialise into Jws"); - OperationError::InvalidState + OperationError::AU0002JwsSerialisation })?; // Now encrypt and prepare the token for return to the client. let token = self.key_object.jws_es256_sign(&jwt, time).map_err(|e| { admin_error!(?e, "Failed to sign UserAuthToken to Jwt"); - OperationError::InvalidState + OperationError::AU0003JwsSignature })?; ( @@ -1586,7 +1596,7 @@ impl AuthSession { let uat = self .account .to_userauthtoken(session_id, scope, time, &self.account_policy) - .ok_or(OperationError::InvalidState)?; + .ok_or(OperationError::AU0004UserAuthTokenInvalid)?; // Queue the session info write. // This is dependent on the type of authentication factors @@ -1619,7 +1629,7 @@ impl AuthSession { .map_err(|e| { debug!(?e, "queue failure"); admin_error!("unable to queue failing authentication as the session will not validate ... "); - OperationError::InvalidState + OperationError::AU0005DelayedProcessFailure })?; } }; @@ -1635,7 +1645,7 @@ impl AuthSession { let scope = match auth_type { AuthType::Anonymous | AuthType::GeneratedPassword => { error!("AuthType used in Reauth is not valid for session re-issuance. Rejecting"); - return Err(OperationError::InvalidState); + return Err(OperationError::AU0006CredentialMayNotReauthenticate); } AuthType::Password | AuthType::PasswordTotp @@ -1654,7 +1664,7 @@ impl AuthSession { time, &self.account_policy, ) - .ok_or(OperationError::InvalidState)?; + .ok_or(OperationError::AU0007UserAuthTokenInvalid)?; Ok(uat) }