From ab8ef8d97791c11e645e81cc9ffd07a015043cfa Mon Sep 17 00:00:00 2001
From: Firstyear <william@blackhats.net.au>
Date: Mon, 16 Dec 2024 10:28:00 +1000
Subject: [PATCH] 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.
---
 proto/src/internal/error.rs                    | 16 +++++++++++++++-
 server/core/src/https/views/constants.rs       |  2 ++
 server/core/templates/unrecoverable_error.html |  3 ++-
 server/lib/src/idm/credupdatesession.rs        |  7 +++----
 4 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/proto/src/internal/error.rs b/proto/src/internal/error.rs
index 5e3d66854..ce7e96800 100644
--- a/proto/src/internal/error.rs
+++ b/proto/src/internal/error.rs
@@ -153,6 +153,15 @@ pub enum OperationError {
     CU0001WebauthnAttestationNotTrusted,
     CU0002WebauthnRegistrationError,
     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
     VS0001IncomingReplSshPublicKey,
     VS0002CertificatePublicKeyDigest,
@@ -271,7 +280,7 @@ impl Display for OperationError {
 
 impl OperationError {
     /// Return the message associated with the error if there is one.
-    fn message(&self) -> Option<String> {
+    pub fn message(&self) -> Option<String> {
         match self {
             Self::SessionExpired => None,
             Self::EmptyRequest => None,
@@ -340,6 +349,11 @@ impl OperationError {
             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()),
+
+            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::DB0002MismatchedRestoreVersion => None,
             Self::DB0003FilterResolveCacheBuild => None,
diff --git a/server/core/src/https/views/constants.rs b/server/core/src/https/views/constants.rs
index d04737ce7..3d3811015 100644
--- a/server/core/src/https/views/constants.rs
+++ b/server/core/src/https/views/constants.rs
@@ -24,6 +24,7 @@ impl std::fmt::Display for UiMessage {
 pub(crate) enum Urls {
     Apps,
     CredReset,
+    CredResetError,
     Profile,
     UpdateCredentials,
     Oauth2Resume,
@@ -36,6 +37,7 @@ impl AsRef<str> for Urls {
         match self {
             Self::Apps => "/ui/apps",
             Self::CredReset => "/ui/reset",
+            Self::CredResetError => "/ui/reset/err",
             Self::Profile => "/ui/profile",
             Self::UpdateCredentials => "/ui/update_credentials",
             Self::Oauth2Resume => "/ui/oauth2/resume",
diff --git a/server/core/templates/unrecoverable_error.html b/server/core/templates/unrecoverable_error.html
index 068d8fc89..39d6b9098 100644
--- a/server/core/templates/unrecoverable_error.html
+++ b/server/core/templates/unrecoverable_error.html
@@ -9,8 +9,9 @@
 	<h2>Error</h2>
 	<main id="main">
 		<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>Error Code: (( err_code ))</p>
+		<a href=((Urls::Ui))>Return</a>
 	</main>
 (% endblock %)
 
diff --git a/server/lib/src/idm/credupdatesession.rs b/server/lib/src/idm/credupdatesession.rs
index 0898f3198..064c24647 100644
--- a/server/lib/src/idm/credupdatesession.rs
+++ b/server/lib/src/idm/credupdatesession.rs
@@ -1311,8 +1311,7 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
                 "Session is unable to commit due to: {}",
                 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::InvalidState);
+            return Err(OperationError::CU0004SessionInconsistent);
         }
 
         // Setup mods for the various bits. We always assert an *exact* state.
@@ -1339,7 +1338,7 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
                 }) => {
                     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.");
-                        return Err(OperationError::InvalidState);
+                        return Err(OperationError::CU0005IntentTokenConflict);
                     } else {
                         *max_ttl
                     }
@@ -1351,7 +1350,7 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
                 })
                 | None => {
                     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);
                 }
             };