20250205 3369 firefox pin (#3403)

Improve error message when passkey is missing PIN

Firefox still doesn't support setting a PIN on new devices. Because
of this we need a way to return a better error message for devices
that don't have UV configured.

proto/src/internal/credupdate.rs

@@ -160,6 +160,7 @@ pub enum CURegWarning {
     AttestedResidentKeyRequired,
     Unsatisfiable,
     WebauthnAttestationUnsatisfiable,
+    WebauthnUserVerificationRequired,
 }
 
 #[derive(Debug, Clone, Serialize, Deserialize, ToSchema)]

server/core/src/https/views/reset.rs

@@ -73,6 +73,7 @@ struct CredStatusView {
 #[template(path = "credentials_update_partial.html")]
 struct CredResetPartialView {
     ext_cred_portal: CUExtPortal,
+    can_commit: bool,
     warnings: Vec<CURegWarning>,
     attested_passkeys_state: CUCredState,
     passkeys_state: CUCredState,
@@ -899,6 +900,7 @@ pub(crate) async fn view_reset_get(
 fn get_cu_partial(cu_status: CUStatus) -> CredResetPartialView {
     let CUStatus {
         ext_cred_portal,
+        can_commit,
         warnings,
         passkeys_state,
         attested_passkeys_state,
@@ -913,6 +915,7 @@ fn get_cu_partial(cu_status: CUStatus) -> CredResetPartialView {
 
     CredResetPartialView {
         ext_cred_portal,
+        can_commit,
         warnings,
         attested_passkeys_state,
         passkeys_state,

server/core/templates/credentials_update_partial.html

@@ -49,6 +49,9 @@
                 (% when CURegWarning::Unsatisfiable %)
                 An account policy conflict has occurred and you will not be able
                 to save your credentials.
+                (% when CURegWarning::WebauthnUserVerificationRequired %)
+                The passkey you attempted to register did not provide user verification. Please
+                ensure that you have a PIN or alternative configured on your authenticator.
                 (% endmatch %)
 
                 (% if is_danger %)
@@ -158,7 +161,7 @@
                                 type="submit"
                                 hx-post="/ui/api/cu_commit"
                                 hx-boost="false"
-                                (% if !warnings.is_empty() %)disabled(% endif
+                                (% if !can_commit %)disabled(% endif
                                 %)>Save Changes</button>
                         </span>
                     </div>

server/lib/src/idm/credupdatesession.rs

@@ -203,6 +203,7 @@ impl CredentialUpdateSession {
     // Vec of the issues with the current session so that UI's can highlight properly how to proceed.
     fn can_commit(&self) -> (bool, Vec<CredentialUpdateSessionStatusWarnings>) {
         let mut warnings = Vec::with_capacity(0);
+        let mut can_proceed = true;
 
         let cred_type_min = self.resolved_account_policy.credential_policy();
 
@@ -219,6 +220,7 @@ impl CredentialUpdateSession {
                     // parts.
                     .unwrap_or(false)
                 {
+                    can_proceed = false;
                     warnings.push(CredentialUpdateSessionStatusWarnings::MfaRequired);
                 }
             }
@@ -226,12 +228,14 @@ impl CredentialUpdateSession {
                 // NOTE: Technically this is unreachable, but we keep it for correctness.
                 // Primary can't be set at all.
                 if self.primary.is_some() {
+                    can_proceed = false;
                     warnings.push(CredentialUpdateSessionStatusWarnings::PasskeyRequired);
                 }
             }
             CredentialType::AttestedPasskey => {
                 // Also unreachable - during these sessions, there will be no values present here.
                 if !self.passkeys.is_empty() || self.primary.is_some() {
+                    can_proceed = false;
                     warnings.push(CredentialUpdateSessionStatusWarnings::AttestedPasskeyRequired);
                 }
             }
@@ -241,12 +245,14 @@ impl CredentialUpdateSession {
                     || !self.passkeys.is_empty()
                     || self.primary.is_some()
                 {
+                    can_proceed = false;
                     warnings
                         .push(CredentialUpdateSessionStatusWarnings::AttestedResidentKeyRequired);
                 }
             }
             CredentialType::Invalid => {
                 // special case, must always deny all changes.
+                can_proceed = false;
                 warnings.push(CredentialUpdateSessionStatusWarnings::Unsatisfiable)
             }
         }
@@ -258,7 +264,7 @@ impl CredentialUpdateSession {
             }
         }
 
-        (warnings.is_empty(), warnings)
+        (can_proceed, warnings)
     }
 }
 
@@ -296,6 +302,7 @@ pub enum CredentialUpdateSessionStatusWarnings {
     AttestedResidentKeyRequired,
     Unsatisfiable,
     WebauthnAttestationUnsatisfiable,
+    WebauthnUserVerificationRequired,
 }
 
 impl Display for CredentialUpdateSessionStatusWarnings {
@@ -319,6 +326,9 @@ impl From<CredentialUpdateSessionStatusWarnings> for CURegWarning {
             CredentialUpdateSessionStatusWarnings::WebauthnAttestationUnsatisfiable => {
                 CURegWarning::WebauthnAttestationUnsatisfiable
             }
+            CredentialUpdateSessionStatusWarnings::WebauthnUserVerificationRequired => {
+                CURegWarning::WebauthnUserVerificationRequired
+            }
         }
     }
 }
@@ -350,6 +360,13 @@ pub struct CredentialUpdateSessionStatus {
 }
 
 impl CredentialUpdateSessionStatus {
+    /// Append a single warning to this session status, which will only be displayed to the
+    /// user once. This is different to other warnings that are derived from the state of the
+    /// session as a whole.
+    pub fn append_ephemeral_warning(&mut self, warning: CredentialUpdateSessionStatusWarnings) {
+        self.warnings.push(warning)
+    }
+
     pub fn can_commit(&self) -> bool {
         self.can_commit
     }
@@ -2172,30 +2189,36 @@ impl IdmServerCredUpdateTransaction<'_> {
 
         match &session.mfaregstate {
             MfaRegState::Passkey(_ccr, pk_reg) => {
-                let result = self
+                let reg_result = self.webauthn.finish_passkey_registration(reg, pk_reg);
-                    .webauthn
-                    .finish_passkey_registration(reg, pk_reg)
-                    .map_err(|e| {
-                        error!(eclass=?e, emsg=%e, "Unable to complete passkey registration");
-                        match e {
-                            WebauthnError::UserNotVerified => {
-                                OperationError::CU0003WebauthnUserNotVerified
-                            }
-                            _ => OperationError::CU0002WebauthnRegistrationError,
-                        }
-                    });
 
-                // The reg is done. Clean up state before returning errors.
+                // Clean up state before returning results.
                 session.mfaregstate = MfaRegState::None;
 
-                let passkey = result?;
+                match reg_result {
+                    Ok(passkey) => {
+                        let pk_id = Uuid::new_v4();
+                        session.passkeys.insert(pk_id, (label, passkey));
 
-                let pk_id = Uuid::new_v4();
+                        let cu_status: CredentialUpdateSessionStatus = session.deref().into();
-                session.passkeys.insert(pk_id, (label, passkey));
+                        Ok(cu_status)
-
+                    }
-                Ok(session.deref().into())
+                    Err(WebauthnError::UserNotVerified) => {
+                        let mut cu_status: CredentialUpdateSessionStatus = session.deref().into();
+                        cu_status.append_ephemeral_warning(
+                            CredentialUpdateSessionStatusWarnings::WebauthnUserVerificationRequired,
+                        );
+                        Ok(cu_status)
+                    }
+                    Err(err) => {
+                        error!(eclass=?err, emsg=%err, "Unable to complete passkey registration");
+                        Err(OperationError::CU0002WebauthnRegistrationError)
+                    }
+                }
+            }
+            invalid_state => {
+                warn!(?invalid_state);
+                Err(OperationError::InvalidRequestState)
             }
-            _ => Err(OperationError::InvalidRequestState),
         }
     }
 

tools/cli/src/cli/lib.rs

@@ -74,7 +74,9 @@ pub(crate) fn handle_group_account_policy_error(response: ClientError, _output_m
     use kanidm_proto::internal::OperationError::SchemaViolation;
     use kanidm_proto::internal::SchemaError::AttributeNotValidForClass;
 
-    if let ClientError::Http(_status, Some(SchemaViolation(AttributeNotValidForClass(att))), opid) = response {
+    if let ClientError::Http(_status, Some(SchemaViolation(AttributeNotValidForClass(att))), opid) =
+        response
+    {
         error!("OperationId: {:?}", opid);
         error!("Cannot update account-policy attribute {att}. Is account-policy enabled on this group?");
     } else {

tools/cli/src/cli/person.rs

@@ -1282,6 +1282,11 @@ fn display_warnings(warnings: &[CURegWarning]) {
             CURegWarning::Unsatisfiable => {
                 println!("Account policy is unsatisfiable. Contact your administrator.");
             }
+            CURegWarning::WebauthnUserVerificationRequired => {
+                println!(
+                    "The passkey you attempted to register did not provide user verification, please ensure a PIN or equivalent is set."
+                );
+            }
         }
     }
 }