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.
This commit is contained in:
Firstyear 2025-01-07 16:05:14 +10:00 committed by GitHub
parent ccf6792104
commit 1983ce19e9
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 40 additions and 16 deletions

View file

@ -142,6 +142,13 @@ pub enum OperationError {
DatabaseLockAcquisitionTimeout, DatabaseLockAcquisitionTimeout,
// Specific internal errors. // Specific internal errors.
AU0001InvalidState,
AU0002JwsSerialisation,
AU0003JwsSignature,
AU0004UserAuthTokenInvalid,
AU0005DelayedProcessFailure,
AU0006CredentialMayNotReauthenticate,
AU0007UserAuthTokenInvalid,
// Kanidm Generic Errors // Kanidm Generic Errors
KG001TaskTimeout, KG001TaskTimeout,
@ -371,6 +378,15 @@ impl OperationError {
Self::TransactionAlreadyCommitted => None, Self::TransactionAlreadyCommitted => None,
Self::ValueDenyName => 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::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::CU0001WebauthnAttestationNotTrusted => None,
Self::CU0002WebauthnRegistrationError => 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()), Self::CU0003WebauthnUserNotVerified => Some("User Verification bit not set while registering credential, you may need to configure a PIN on this device.".into()),

View file

@ -12,7 +12,7 @@ use axum::{
response::{IntoResponse, Redirect, Response}, response::{IntoResponse, Redirect, Response},
Extension, Form, Json, Extension, Form, Json,
}; };
use axum_extra::extract::cookie::{Cookie, CookieJar, SameSite}; use axum_extra::extract::cookie::{CookieJar, SameSite};
use kanidm_proto::internal::{ use kanidm_proto::internal::{
COOKIE_AUTH_SESSION_ID, COOKIE_BEARER_TOKEN, COOKIE_CU_SESSION_TOKEN, COOKIE_OAUTH2_REQ, COOKIE_AUTH_SESSION_ID, COOKIE_BEARER_TOKEN, COOKIE_CU_SESSION_TOKEN, COOKIE_OAUTH2_REQ,
COOKIE_USERNAME, COOKIE_USERNAME,
@ -835,10 +835,8 @@ async fn view_login_step(
break res; break res;
} }
AuthState::Continue(allowed) => { AuthState::Continue(allowed) => {
// Reauth inits its session here so we need to be able to add cookie here ig. // Reauth inits its session here so we need to be able to add it's cookie here.
if jar.get(COOKIE_AUTH_SESSION_ID).is_none() {
jar = add_session_cookie(&state, jar, &session_context)?; jar = add_session_cookie(&state, jar, &session_context)?;
}
let res = match allowed.len() { let res = match allowed.len() {
// Shouldn't be possible. // Shouldn't be possible.
@ -936,9 +934,9 @@ async fn view_login_step(
jar jar
}; };
jar = jar jar = jar.add(bearer_cookie);
.add(bearer_cookie)
.remove(Cookie::from(COOKIE_AUTH_SESSION_ID)); jar = cookies::destroy(jar, COOKIE_AUTH_SESSION_ID, &state);
// Now, we need to decided where to go. // Now, we need to decided where to go.
let res = if jar.get(COOKIE_OAUTH2_REQ).is_some() { let res = if jar.get(COOKIE_OAUTH2_REQ).is_some() {
@ -955,7 +953,7 @@ async fn view_login_step(
} }
AuthState::Denied(reason) => { AuthState::Denied(reason) => {
debug!("🧩 -> AuthState::Denied"); debug!("🧩 -> AuthState::Denied");
jar = jar.remove(Cookie::from(COOKIE_AUTH_SESSION_ID)); jar = cookies::destroy(jar, COOKIE_AUTH_SESSION_ID, &state);
break LoginDeniedView { break LoginDeniedView {
display_ctx, display_ctx,

View file

@ -1383,7 +1383,17 @@ impl AuthSession {
| AuthSessionState::InProgress(CredHandler::PasswordSecurityKey { .. }) | AuthSessionState::InProgress(CredHandler::PasswordSecurityKey { .. })
| AuthSessionState::InProgress(CredHandler::Passkey { .. }) | AuthSessionState::InProgress(CredHandler::Passkey { .. })
| AuthSessionState::InProgress(CredHandler::AttestedPasskey { .. }) => Ok(None), | 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| { let jwt = Jws::into_json(&uat).map_err(|e| {
admin_error!(?e, "Failed to serialise into Jws"); admin_error!(?e, "Failed to serialise into Jws");
OperationError::InvalidState OperationError::AU0002JwsSerialisation
})?; })?;
// Now encrypt and prepare the token for return to the client. // Now encrypt and prepare the token for return to the client.
let token = self.key_object.jws_es256_sign(&jwt, time).map_err(|e| { let token = self.key_object.jws_es256_sign(&jwt, time).map_err(|e| {
admin_error!(?e, "Failed to sign UserAuthToken to Jwt"); admin_error!(?e, "Failed to sign UserAuthToken to Jwt");
OperationError::InvalidState OperationError::AU0003JwsSignature
})?; })?;
( (
@ -1586,7 +1596,7 @@ impl AuthSession {
let uat = self let uat = self
.account .account
.to_userauthtoken(session_id, scope, time, &self.account_policy) .to_userauthtoken(session_id, scope, time, &self.account_policy)
.ok_or(OperationError::InvalidState)?; .ok_or(OperationError::AU0004UserAuthTokenInvalid)?;
// Queue the session info write. // Queue the session info write.
// This is dependent on the type of authentication factors // This is dependent on the type of authentication factors
@ -1619,7 +1629,7 @@ impl AuthSession {
.map_err(|e| { .map_err(|e| {
debug!(?e, "queue failure"); debug!(?e, "queue failure");
admin_error!("unable to queue failing authentication as the session will not validate ... "); 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 { let scope = match auth_type {
AuthType::Anonymous | AuthType::GeneratedPassword => { AuthType::Anonymous | AuthType::GeneratedPassword => {
error!("AuthType used in Reauth is not valid for session re-issuance. Rejecting"); error!("AuthType used in Reauth is not valid for session re-issuance. Rejecting");
return Err(OperationError::InvalidState); return Err(OperationError::AU0006CredentialMayNotReauthenticate);
} }
AuthType::Password AuthType::Password
| AuthType::PasswordTotp | AuthType::PasswordTotp
@ -1654,7 +1664,7 @@ impl AuthSession {
time, time,
&self.account_policy, &self.account_policy,
) )
.ok_or(OperationError::InvalidState)?; .ok_or(OperationError::AU0007UserAuthTokenInvalid)?;
Ok(uat) Ok(uat)
} }