Use specific errors for intent token revoked (#3291)

Rather than the generic 'invalid state' error, we now return
proper site-specific errors for credential commit failures, with
error messages to explain what went wrong.
This commit is contained in:
Firstyear 2024-12-16 10:28:00 +10:00 committed by GitHub
parent 5d75c9b247
commit 6c3b8500a2
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 22 additions and 6 deletions

View file

@ -152,6 +152,15 @@ pub enum OperationError {
CU0001WebauthnAttestationNotTrusted, CU0001WebauthnAttestationNotTrusted,
CU0002WebauthnRegistrationError, CU0002WebauthnRegistrationError,
CU0003WebauthnUserNotVerified, CU0003WebauthnUserNotVerified,
// The session is inconsistent and can't be committed, but the errors
// can be resolved.
CU0004SessionInconsistent,
// Another session used this intent token, and so it can't be committed.
CU0005IntentTokenConflict,
// The intent token was invalidated before we could commit.
CU0006IntentTokenInvalidated,
// ValueSet errors // ValueSet errors
VS0001IncomingReplSshPublicKey, VS0001IncomingReplSshPublicKey,
VS0002CertificatePublicKeyDigest, VS0002CertificatePublicKeyDigest,
@ -295,7 +304,7 @@ impl Display for OperationError {
impl OperationError { impl OperationError {
/// Return the message associated with the error if there is one. /// Return the message associated with the error if there is one.
fn message(&self) -> Option<String> { pub fn message(&self) -> Option<String> {
match self { match self {
Self::SessionExpired => None, Self::SessionExpired => None,
Self::EmptyRequest => None, Self::EmptyRequest => None,
@ -364,6 +373,11 @@ impl OperationError {
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()),
Self::CU0004SessionInconsistent => Some("The session is unable to be committed due to unresolved warnings.".into()),
Self::CU0005IntentTokenConflict => Some("The intent token used to create this session has been reused in another browser/tab and may not proceed.".into()),
Self::CU0006IntentTokenInvalidated => Some("The intent token has been invalidated/revoked before the commit could be accepted. Has it been used in another browser or tab?".into()),
Self::DB0001MismatchedRestoreVersion => None, Self::DB0001MismatchedRestoreVersion => None,
Self::DB0002MismatchedRestoreVersion => None, Self::DB0002MismatchedRestoreVersion => None,
Self::DB0003FilterResolveCacheBuild => None, Self::DB0003FilterResolveCacheBuild => None,

View file

@ -24,6 +24,7 @@ impl std::fmt::Display for UiMessage {
pub(crate) enum Urls { pub(crate) enum Urls {
Apps, Apps,
CredReset, CredReset,
CredResetError,
Profile, Profile,
UpdateCredentials, UpdateCredentials,
Oauth2Resume, Oauth2Resume,
@ -36,6 +37,7 @@ impl AsRef<str> for Urls {
match self { match self {
Self::Apps => "/ui/apps", Self::Apps => "/ui/apps",
Self::CredReset => "/ui/reset", Self::CredReset => "/ui/reset",
Self::CredResetError => "/ui/reset/err",
Self::Profile => "/ui/profile", Self::Profile => "/ui/profile",
Self::UpdateCredentials => "/ui/update_credentials", Self::UpdateCredentials => "/ui/update_credentials",
Self::Oauth2Resume => "/ui/oauth2/resume", Self::Oauth2Resume => "/ui/oauth2/resume",

View file

@ -9,8 +9,9 @@
<h2>Error</h2> <h2>Error</h2>
<main id="main"> <main id="main">
<p>An unrecoverable error occurred. Please contact your administrator with the details below.</p> <p>An unrecoverable error occurred. Please contact your administrator with the details below.</p>
<p>Error Code: (( err_code ))</p>
<p>Operation ID: (( operation_id ))</p> <p>Operation ID: (( operation_id ))</p>
<p>Error Code: (( err_code ))</p>
<a href=((Urls::Ui))>Return</a>
</main> </main>
(% endblock %) (% endblock %)

View file

@ -1311,8 +1311,7 @@ impl IdmServerProxyWriteTransaction<'_> {
"Session is unable to commit due to: {}", "Session is unable to commit due to: {}",
commit_failure_reasons commit_failure_reasons
); );
// TODO: perhaps it would be more helpful to add a new operation error that describes what the issue is return Err(OperationError::CU0004SessionInconsistent);
return Err(OperationError::InvalidState);
} }
// Setup mods for the various bits. We always assert an *exact* state. // Setup mods for the various bits. We always assert an *exact* state.
@ -1339,7 +1338,7 @@ impl IdmServerProxyWriteTransaction<'_> {
}) => { }) => {
if *session_id != session_token.sessionid { if *session_id != session_token.sessionid {
security_info!("Session originated from an intent token, but the intent token has initiated a conflicting second update session. Refusing to commit changes."); security_info!("Session originated from an intent token, but the intent token has initiated a conflicting second update session. Refusing to commit changes.");
return Err(OperationError::InvalidState); return Err(OperationError::CU0005IntentTokenConflict);
} else { } else {
*max_ttl *max_ttl
} }
@ -1351,7 +1350,7 @@ impl IdmServerProxyWriteTransaction<'_> {
}) })
| None => { | None => {
security_info!("Session originated from an intent token, but the intent token has transitioned to an invalid state. Refusing to commit changes."); security_info!("Session originated from an intent token, but the intent token has transitioned to an invalid state. Refusing to commit changes.");
return Err(OperationError::InvalidState); return Err(OperationError::CU0006IntentTokenInvalidated);
} }
}; };