Complete MFA and Webauthn handlers (#360)

Fixes #357 - this allows the password MFA handler to correct handle a mixed totp or webauthn credential with passwords. This is likely the "majority" of accounts we will see on the service.

kanidm_client/src/lib.rs

@@ -1068,6 +1068,23 @@ impl KanidmClient {
         }
     }
 
+    pub fn idm_account_primary_credential_remove_webauthn(
+        &self,
+        id: &str,
+        label: &str,
+    ) -> Result<bool, ClientError> {
+        let r = SetCredentialRequest::WebauthnRemove(label.to_string());
+        let res: Result<SetCredentialResponse, ClientError> = self.perform_put_request(
+            format!("/v1/account/{}/_credential/primary", id).as_str(),
+            r,
+        );
+        match res {
+            Ok(SetCredentialResponse::Success) => Ok(true),
+            Ok(_) => Err(ClientError::EmptyResponse),
+            Err(e) => Err(e),
+        }
+    }
+
     pub fn idm_account_radius_credential_get(
         &self,
         id: &str,

kanidm_client/tests/proto_v1_test.rs

@@ -788,6 +788,103 @@ fn test_server_rest_totp_auth_lifecycle() {
 
 #[test]
 fn test_server_rest_webauthn_auth_lifecycle() {
+    run_test(|mut rsclient: KanidmClient| {
+        let res = rsclient.auth_simple_password("admin", ADMIN_TEST_PASSWORD);
+        assert!(res.is_ok());
+
+        // Not recommended in production!
+        rsclient
+            .idm_group_add_members("idm_admins", &["admin"])
+            .unwrap();
+
+        // Create a new account
+        rsclient
+            .idm_account_create("demo_account", "Deeeeemo")
+            .unwrap();
+
+        // Enroll a soft token to the account webauthn.
+        let mut wa_softtok = WebauthnAuthenticator::new(U2FSoft::new());
+
+        // Do the challenge
+        let (sessionid, regchal) = rsclient
+            .idm_account_primary_credential_register_webauthn("demo_account", "softtok")
+            .unwrap();
+
+        let rego = wa_softtok
+            .do_registration("https://idm.example.com", regchal)
+            .expect("Failed to register to softtoken");
+
+        // Enroll the cred after signing.
+        rsclient
+            .idm_account_primary_credential_complete_webuthn_registration(
+                "demo_account",
+                rego,
+                sessionid,
+            )
+            .unwrap();
+
+        // ====== Reg a second token.
+        let mut wa_softtok_2 = WebauthnAuthenticator::new(U2FSoft::new());
+
+        // Do the challenge
+        let (sessionid, regchal) = rsclient
+            .idm_account_primary_credential_register_webauthn("demo_account", "softtok_2")
+            .unwrap();
+
+        let rego = wa_softtok_2
+            .do_registration("https://idm.example.com", regchal)
+            .expect("Failed to register to softtoken");
+
+        // Enroll the cred after signing.
+        rsclient
+            .idm_account_primary_credential_complete_webuthn_registration(
+                "demo_account",
+                rego,
+                sessionid,
+            )
+            .unwrap();
+
+        // Now do an auth
+        let mut rsclient_good = rsclient.new_session().unwrap();
+
+        let pkr = rsclient_good.auth_webauthn_begin("demo_account").unwrap();
+
+        // Get the auth chal.
+        let auth = wa_softtok_2
+            .do_authentication("https://idm.example.com", pkr)
+            .expect("Failed to auth to softtoken");
+
+        // Submit the webauthn auth.
+        rsclient_good
+            .auth_webauthn_complete(auth)
+            .expect("Failed to authenticate");
+
+        // ======== remove the second softtok.
+
+        rsclient
+            .idm_account_primary_credential_remove_webauthn("demo_account", "softtok_2")
+            .expect("failed to remove softtoken");
+
+        // All good, check first tok auth.
+
+        let mut rsclient_good = rsclient.new_session().unwrap();
+
+        let pkr = rsclient_good.auth_webauthn_begin("demo_account").unwrap();
+
+        // Get the auth chal.
+        let auth = wa_softtok
+            .do_authentication("https://idm.example.com", pkr)
+            .expect("Failed to auth to softtoken");
+
+        // Submit the webauthn auth.
+        rsclient_good
+            .auth_webauthn_complete(auth)
+            .expect("Failed to authenticate");
+    });
+}
+
+#[test]
+fn test_server_rest_webauthn_mfa_auth_lifecycle() {
     run_test(|mut rsclient: KanidmClient| {
         let res = rsclient.auth_simple_password("admin", ADMIN_TEST_PASSWORD);
         assert!(res.is_ok());
@@ -836,7 +933,23 @@ fn test_server_rest_webauthn_auth_lifecycle() {
         // Submit the webauthn auth.
         rsclient_good
             .auth_webauthn_complete(auth)
-            .expect("Failed to authenticate")
+            .expect("Failed to authenticate");
+
+        // Set a password to cause the state to change to PasswordMFA
+        assert!(rsclient
+            .idm_account_primary_credential_set_password("demo_account", "sohdi3iuHo6mai7noh0a")
+            .is_ok());
+
+        // Now remove Webauthn ...
+        rsclient
+            .idm_account_primary_credential_remove_webauthn("demo_account", "softtok")
+            .expect("failed to remove softtoken");
+
+        // Check pw only
+        let mut rsclient_good = rsclient.new_session().unwrap();
+        assert!(rsclient_good
+            .auth_simple_password("demo_account", "sohdi3iuHo6mai7noh0a")
+            .is_ok());
     });
 }
 

kanidm_proto/src/v1.rs

@@ -583,6 +583,8 @@ pub enum SetCredentialRequest {
     WebauthnBegin(String),
     // Finish it.
     WebauthnRegister(Uuid, RegisterPublicKeyCredential),
+    // Remove
+    WebauthnRemove(String),
 }
 
 #[derive(Debug, Clone, Serialize, Deserialize)]

kanidm_tools/src/cli/account.rs

@@ -16,6 +16,7 @@ impl AccountOpt {
                 AccountCredential::SetPassword(acs) => acs.copt.debug,
                 AccountCredential::GeneratePassword(acs) => acs.copt.debug,
                 AccountCredential::RegisterWebauthn(acs) => acs.copt.debug,
+                AccountCredential::RemoveWebauthn(acs) => acs.copt.debug,
                 AccountCredential::RegisterTOTP(acs) => acs.copt.debug,
                 AccountCredential::RemoveTOTP(acs) => acs.copt.debug,
             },
@@ -126,6 +127,20 @@ impl AccountOpt {
                         }
                     }
                 }
+                AccountCredential::RemoveWebauthn(acsopt) => {
+                    let client = acsopt.copt.to_client();
+                    match client.idm_account_primary_credential_remove_webauthn(
+                        acsopt.aopts.account_id.as_str(),
+                        acsopt.tag.as_str(),
+                    ) {
+                        Ok(_) => {
+                            println!("Webauthn removal success.");
+                        }
+                        Err(e) => {
+                            eprintln!("Error Removing Webauthn from account -> {:?}", e);
+                        }
+                    }
+                }
                 AccountCredential::RegisterTOTP(acsopt) => {
                     let client = acsopt.copt.to_client();
                     let (session, tok) = match client.idm_account_primary_credential_generate_totp(

kanidm_tools/src/opt/kanidm.rs

@@ -156,6 +156,8 @@ pub enum AccountCredential {
     GeneratePassword(AccountCredentialSet),
     #[structopt(name = "register_webauthn")]
     RegisterWebauthn(AccountNamedTagOpt),
+    #[structopt(name = "remove_webauthn")]
+    RemoveWebauthn(AccountNamedTagOpt),
     /// Set the TOTP credential of the account. If a TOTP already exists, on a successful
     /// registration, this will replace it.
     #[structopt(name = "set_totp")]

kanidmd/src/lib/actors/v1_write.rs

@@ -9,8 +9,8 @@ use crate::event::{
 };
 use crate::idm::event::{
     GeneratePasswordEvent, GenerateTOTPEvent, PasswordChangeEvent, RegenerateRadiusSecretEvent,
-    RemoveTOTPEvent, UnixPasswordChangeEvent, VerifyTOTPEvent, WebauthnDoRegisterEvent,
-    WebauthnInitRegisterEvent,
+    RemoveTOTPEvent, RemoveWebauthnEvent, UnixPasswordChangeEvent, VerifyTOTPEvent,
+    WebauthnDoRegisterEvent, WebauthnInitRegisterEvent,
 };
 use crate::modify::{Modify, ModifyInvalid, ModifyList};
 use crate::value::{PartialValue, Value};
@@ -703,6 +703,26 @@ impl QueryServerWriteV1 {
                             .reg_account_webauthn_complete(&mut audit, &wre)
                             .and_then(|r| idms_prox_write.commit(&mut audit).map(|_| r))
                     }
+                    SetCredentialRequest::WebauthnRemove(label) => {
+                        let rwe = RemoveWebauthnEvent::from_parts(
+                            &mut audit,
+                            &idms_prox_write.qs_write,
+                            msg.uat.as_ref(),
+                            target_uuid,
+                            label,
+                        )
+                        .map_err(|e| {
+                            ladmin_error!(
+                                audit,
+                                "Failed to begin internal_credential_set_message: {:?}",
+                                e
+                            );
+                            e
+                        })?;
+                        idms_prox_write
+                            .remove_account_webauthn(&mut audit, &rwe)
+                            .and_then(|r| idms_prox_write.commit(&mut audit).map(|_| r))
+                    }
                 }
             }
         );

kanidmd/src/lib/credential/mod.rs

@@ -380,6 +380,57 @@ impl Credential {
         })
     }
 
+    pub fn remove_webauthn(&self, label: &str) -> Result<Self, OperationError> {
+        let type_ = match &self.type_ {
+            CredentialType::Password(_) | CredentialType::GeneratedPassword(_) => {
+                return Err(OperationError::InvalidAttribute(
+                    "Webauthn is not present on this credential".to_string(),
+                ));
+            }
+            CredentialType::PasswordMFA(pw, totp, map) => {
+                let mut nmap = map.clone();
+                if nmap.remove(label).is_none() {
+                    return Err(OperationError::InvalidAttribute(format!(
+                        "Removing Webauthn token with label '{:?}': does not exist",
+                        label
+                    )));
+                }
+                if nmap.is_empty() {
+                    if totp.is_some() {
+                        CredentialType::PasswordMFA(pw.clone(), totp.clone(), nmap)
+                    } else {
+                        CredentialType::Password(pw.clone())
+                    }
+                } else {
+                    CredentialType::PasswordMFA(pw.clone(), totp.clone(), nmap)
+                }
+            }
+            CredentialType::Webauthn(map) => {
+                let mut nmap = map.clone();
+                if nmap.remove(label).is_none() {
+                    return Err(OperationError::InvalidAttribute(format!(
+                        "Removing Webauthn token with label '{:?}': does not exist",
+                        label
+                    )));
+                }
+                if nmap.is_empty() {
+                    return Err(OperationError::InvalidAttribute(format!(
+                        "Removing Webauthn token with label '{:?}': unable to remove, this is the last webauthn token",
+                        label
+                    )));
+                }
+                CredentialType::Webauthn(nmap)
+            }
+        };
+
+        // Check stuff
+        Ok(Credential {
+            type_,
+            claims: self.claims.clone(),
+            uuid: self.uuid,
+        })
+    }
+
     #[allow(clippy::ptr_arg)]
     pub fn update_webauthn_counter(
         &self,
@@ -527,11 +578,7 @@ impl Credential {
             CredentialType::PasswordMFA(_, totp, wan) => {
                 CredentialType::PasswordMFA(pw, totp.clone(), wan.clone())
             }
-            CredentialType::Webauthn(wan) => {
-                // Or should this become PasswordWebauthn?
-                debug_assert!(false);
-                CredentialType::Webauthn(wan.clone())
-            }
+            CredentialType::Webauthn(wan) => CredentialType::PasswordMFA(pw, None, wan.clone()),
         };
         Credential {
             type_,

kanidmd/src/lib/idm/account.rs

@@ -274,6 +274,24 @@ impl Account {
         Ok(ModifyList::new_purge_and_set("primary_credential", vcred))
     }
 
+    pub(crate) fn gen_webauthn_remove_mod(
+        &self,
+        label: &str,
+    ) -> Result<ModifyList<ModifyInvalid>, OperationError> {
+        match &self.primary {
+            // Change the cred
+            Some(primary) => {
+                let ncred = primary.remove_webauthn(label)?;
+                let vcred = Value::new_credential("primary", ncred);
+                Ok(ModifyList::new_purge_and_set("primary_credential", vcred))
+            }
+            None => {
+                // No credential exists, we can't remove what is not real.
+                Err(OperationError::InvalidState)
+            }
+        }
+    }
+
     #[allow(clippy::ptr_arg)]
     pub(crate) fn gen_webauthn_counter_mod(
         &self,

kanidmd/src/lib/idm/authsession.rs

@@ -44,11 +44,12 @@ enum CredVerifyState {
 }
 
 #[derive(Clone, Debug)]
-struct CredTotpPw {
+struct CredMfa {
     pw: Password,
     pw_state: CredVerifyState,
-    totp: TOTP,
-    totp_state: CredVerifyState,
+    totp: Option<TOTP>,
+    wan: Option<(RequestChallengeResponse, AuthenticationState)>,
+    mfa_state: CredVerifyState,
 }
 
 #[derive(Clone, Debug)]
@@ -63,7 +64,7 @@ enum CredHandler {
     Anonymous,
     // AppPassword (?)
     Password(Password),
-    PasswordMFA(CredTotpPw),
+    PasswordMFA(Box<CredMfa>),
     Webauthn(CredWebauthn),
     // Webauthn + Password
 }
@@ -79,15 +80,41 @@ impl CredHandler {
             CredentialType::Password(pw) | CredentialType::GeneratedPassword(pw) => {
                 Ok(CredHandler::Password(pw.clone()))
             }
-            CredentialType::PasswordMFA(pw, Some(totp), _) => {
-                Ok(CredHandler::PasswordMFA(CredTotpPw {
+            CredentialType::PasswordMFA(pw, maybe_totp, maybe_wan) => {
+                let wan = if !maybe_wan.is_empty() {
+                    webauthn
+                        .generate_challenge_authenticate(maybe_wan.values().cloned().collect())
+                        .map(Some)
+                        .map_err(|e| {
+                            lsecurity!(
+                                au,
+                                "Unable to create webauthn authentication challenge -> {:?}",
+                                e
+                            );
+                        })?
+                } else {
+                    None
+                };
+
+                let cmfa = Box::new(CredMfa {
                     pw: pw.clone(),
                     pw_state: CredVerifyState::Init,
-                    totp: totp.clone(),
-                    totp_state: CredVerifyState::Init,
-                }))
+                    totp: maybe_totp.clone(),
+                    wan,
+                    mfa_state: CredVerifyState::Init,
+                });
+
+                // Paranoia. Should NEVER occur.
+                if cmfa.totp.is_none() && cmfa.wan.is_none() {
+                    lsecurity_critical!(
+                        au,
+                        "Unable to create CredHandler::PasswordMFA - totp and webauthn are both not present. Credentials MAY be corrupt!"
+                    );
+                    return Err(());
+                }
+
+                Ok(CredHandler::PasswordMFA(cmfa))
             }
-            CredentialType::PasswordMFA(_, None, _) => Err(()),
             CredentialType::Webauthn(wan) => webauthn
                 .generate_challenge_authenticate(wan.values().cloned().collect())
                 .map(|(chal, wan_state)| {
@@ -177,60 +204,110 @@ impl CredHandler {
         }
     }
 
-    fn validate_totp_password(
+    fn validate_password_mfa(
         au: &mut AuditScope,
         cred: &AuthCredential,
         ts: &Duration,
-        pw_totp: &mut CredTotpPw,
+        pw_mfa: &mut CredMfa,
+        webauthn: &Webauthn<WebauthnDomainConfig>,
         who: Uuid,
         async_tx: &Sender<DelayedAction>,
     ) -> CredState {
-        match (cred, &pw_totp.totp_state, &pw_totp.pw_state) {
-            // Must be done first.
-            (AuthCredential::TOTP(totp_chal), CredVerifyState::Init, CredVerifyState::Init) => {
-                if pw_totp.totp.verify(*totp_chal, ts) {
-                    pw_totp.totp_state = CredVerifyState::Success;
-                    lsecurity!(
-                        au,
-                        "Handler::PasswordMFA -> Result::Continue - TOTP OK, password -"
-                    );
-                    CredState::Continue(vec![AuthAllowed::Password])
-                } else {
-                    pw_totp.totp_state = CredVerifyState::Fail;
-                    lsecurity!(
-                        au,
-                        "Handler::PasswordMFA -> Result::Denied - TOTP Fail, password -"
-                    );
-                    CredState::Denied(BAD_TOTP_MSG)
+        match (&pw_mfa.mfa_state, &pw_mfa.pw_state) {
+            (CredVerifyState::Init, CredVerifyState::Init) => {
+                // MFA first
+                match (cred, pw_mfa.totp.as_ref(), pw_mfa.wan.as_ref()) {
+                    (AuthCredential::Webauthn(resp), _, Some((_, wan_state))) => {
+                        webauthn.authenticate_credential(&resp, wan_state.clone())
+                                .map(|r| {
+                                    pw_mfa.mfa_state = CredVerifyState::Success;
+                                    // Success. Determine if we need to update the counter
+                                    // async from r.
+                                    if let Some((cid, counter)) = r {
+                                        // Do async
+                                        if let Err(_e) = async_tx.send(DelayedAction::WebauthnCounterIncrement(WebauthnCounterIncrement {
+                                            target_uuid: who,
+                                            cid,
+                                            counter,
+                                        })) {
+                                            ladmin_warning!(au, "unable to queue delayed webauthn counter increment, continuing ... ");
+                                        };
+                                    };
+                                    CredState::Continue(vec![AuthAllowed::Password])
+                                })
+                                .unwrap_or_else(|e| {
+                                    pw_mfa.mfa_state = CredVerifyState::Fail;
+                                    // Denied.
+                                    lsecurity!(au, "Handler::Webauthn -> Result::Denied - webauthn error {:?}", e);
+                                    CredState::Denied(BAD_WEBAUTHN_MSG)
+                                })
+                    }
+                    (AuthCredential::TOTP(totp_chal), Some(totp), _) => {
+                        if totp.verify(*totp_chal, ts) {
+                            pw_mfa.mfa_state = CredVerifyState::Success;
+                            lsecurity!(
+                                au,
+                                "Handler::PasswordMFA -> Result::Continue - TOTP OK, password -"
+                            );
+                            CredState::Continue(vec![AuthAllowed::Password])
+                        } else {
+                            pw_mfa.mfa_state = CredVerifyState::Fail;
+                            lsecurity!(
+                                au,
+                                "Handler::PasswordMFA -> Result::Denied - TOTP Fail, password -"
+                            );
+                            CredState::Denied(BAD_TOTP_MSG)
+                        }
+                    }
+                    _ => {
+                        lsecurity!(
+                            au,
+                            "Handler::PasswordMFA -> Result::Denied - invalid cred type for handler"
+                        );
+                        CredState::Denied(BAD_AUTH_TYPE_MSG)
+                    }
                 }
             }
-            // Must only proceed if totp was success.
-            (
-                AuthCredential::Password(cleartext),
-                CredVerifyState::Success,
-                CredVerifyState::Init,
-            ) => {
-                if pw_totp.pw.verify(cleartext.as_str()).unwrap_or(false) {
-                    pw_totp.pw_state = CredVerifyState::Success;
-                    lsecurity!(
-                        au,
-                        "Handler::PasswordMFA -> Result::Success - TOTP OK, password OK"
-                    );
-                    Self::maybe_pw_upgrade(au, &pw_totp.pw, who, cleartext.as_str(), async_tx);
-                    CredState::Success(Vec::new())
-                } else {
-                    pw_totp.pw_state = CredVerifyState::Fail;
-                    lsecurity!(
-                        au,
-                        "Handler::PasswordMFA -> Result::Denied - TOTP OK, password Fail"
-                    );
-                    CredState::Denied(BAD_PASSWORD_MSG)
+            (CredVerifyState::Success, CredVerifyState::Init) => {
+                // PW second.
+                match cred {
+                    AuthCredential::Password(cleartext) => {
+                        if pw_mfa.pw.verify(cleartext.as_str()).unwrap_or(false) {
+                            pw_mfa.pw_state = CredVerifyState::Success;
+                            lsecurity!(
+                                au,
+                                "Handler::PasswordMFA -> Result::Success - TOTP OK, password OK"
+                            );
+                            Self::maybe_pw_upgrade(
+                                au,
+                                &pw_mfa.pw,
+                                who,
+                                cleartext.as_str(),
+                                async_tx,
+                            );
+                            CredState::Success(Vec::new())
+                        } else {
+                            pw_mfa.pw_state = CredVerifyState::Fail;
+                            lsecurity!(
+                                au,
+                                "Handler::PasswordMFA -> Result::Denied - TOTP OK, password Fail"
+                            );
+                            CredState::Denied(BAD_PASSWORD_MSG)
+                        }
+                    }
+                    _ => {
+                        lsecurity!(
+                            au,
+                            "Handler::PasswordMFA -> Result::Denied - invalid cred type for handler"
+                        );
+                        CredState::Denied(BAD_AUTH_TYPE_MSG)
+                    }
                 }
             }
             _ => {
                 lsecurity!(
                     au,
-                    "Handler::PasswordMFA -> Result::Denied - invalid cred type for handler"
+                    "Handler::PasswordMFA -> Result::Denied - invalid credential mfa and pw state"
                 );
                 CredState::Denied(BAD_AUTH_TYPE_MSG)
             }
@@ -304,8 +381,8 @@ impl CredHandler {
             CredHandler::Password(ref mut pw) => {
                 Self::validate_password(au, cred, pw, who, async_tx)
             }
-            CredHandler::PasswordMFA(ref mut pw_totp) => {
-                Self::validate_totp_password(au, cred, ts, pw_totp, who, async_tx)
+            CredHandler::PasswordMFA(ref mut pw_mfa) => {
+                Self::validate_password_mfa(au, cred, ts, pw_mfa, webauthn, who, async_tx)
             }
             CredHandler::Webauthn(ref mut wan_cred) => {
                 Self::validate_webauthn(au, cred, wan_cred, webauthn, who, async_tx)
@@ -317,9 +394,17 @@ impl CredHandler {
         match &self {
             CredHandler::Anonymous => vec![AuthAllowed::Anonymous],
             CredHandler::Password(_) => vec![AuthAllowed::Password],
-            // webauth
-            // mfa
-            CredHandler::PasswordMFA(_) => vec![AuthAllowed::TOTP],
+            CredHandler::PasswordMFA(ref pw_mfa) => pw_mfa
+                .totp
+                .iter()
+                .map(|_| AuthAllowed::TOTP)
+                .chain(
+                    pw_mfa
+                        .wan
+                        .iter()
+                        .map(|(chal, _)| AuthAllowed::Webauthn(chal.clone())),
+                )
+                .collect(),
             CredHandler::Webauthn(webauthn) => vec![AuthAllowed::Webauthn(webauthn.chal.clone())],
         }
     }
@@ -806,7 +891,7 @@ mod tests {
                 $webauthn,
                 duration_from_epoch_now(),
             );
-            let mut session = session.unwrap();
+            let mut session = session.expect("Session was unable to be created.");
 
             if let AuthState::Choose(auth_mechs) = state {
                 assert!(
@@ -823,10 +908,16 @@ mod tests {
                 .start_session($audit, &AuthMech::PasswordMFA)
                 .expect("Failed to select anonymous mech.");
 
+            let mut rchal = None;
+
             if let AuthState::Continue(auth_mechs) = state {
                 assert!(
                     true == auth_mechs.iter().fold(false, |acc, x| match x {
                         // TODO: How to return webauthn chal?
+                        AuthAllowed::Webauthn(chal) => {
+                            rchal = Some(chal.clone());
+                            true
+                        }
                         AuthAllowed::TOTP => true,
                         _ => acc,
                     })
@@ -835,7 +926,7 @@ mod tests {
                 panic!("Invalid auth state")
             }
 
-            session
+            (session, rchal)
         }};
     }
 
@@ -880,7 +971,7 @@ mod tests {
 
         // check send anon (fail)
         {
-            let mut session = start_password_mfa_session!(&mut audit, account, &webauthn);
+            let (mut session, _) = start_password_mfa_session!(&mut audit, account, &webauthn);
 
             match session.validate_creds(
                 &mut audit,
@@ -898,7 +989,7 @@ mod tests {
 
         // Sending a PW first is an immediate fail.
         {
-            let mut session = start_password_mfa_session!(&mut audit, account, &webauthn);
+            let (mut session, _) = start_password_mfa_session!(&mut audit, account, &webauthn);
 
             match session.validate_creds(
                 &mut audit,
@@ -913,7 +1004,7 @@ mod tests {
         }
         // check send bad totp, should fail immediate
         {
-            let mut session = start_password_mfa_session!(&mut audit, account, &webauthn);
+            let (mut session, _) = start_password_mfa_session!(&mut audit, account, &webauthn);
 
             match session.validate_creds(
                 &mut audit,
@@ -930,7 +1021,7 @@ mod tests {
         // check send good totp, should continue
         //      then bad pw, fail pw
         {
-            let mut session = start_password_mfa_session!(&mut audit, account, &webauthn);
+            let (mut session, _) = start_password_mfa_session!(&mut audit, account, &webauthn);
 
             match session.validate_creds(
                 &mut audit,
@@ -957,7 +1048,7 @@ mod tests {
         // check send good totp, should continue
         //      then good pw, success
         {
-            let mut session = start_password_mfa_session!(&mut audit, account, &webauthn);
+            let (mut session, _) = start_password_mfa_session!(&mut audit, account, &webauthn);
 
             match session.validate_creds(
                 &mut audit,
@@ -1033,24 +1124,19 @@ mod tests {
         }};
     }
 
-    #[test]
-    fn test_idm_authsession_webauthn_only_mech() {
-        let mut audit = AuditScope::new(
-            "test_idm_authsession_webauthn_mech",
-            uuid::Uuid::new_v4(),
-            None,
-        );
+    fn setup_webauthn(
+        name: &str,
+    ) -> (
+        webauthn_rs::Webauthn<crate::credential::webauthn::WebauthnDomainConfig>,
+        webauthn_authenticator_rs::WebauthnAuthenticator<U2FSoft>,
+        webauthn_rs::proto::Credential,
+    ) {
         let webauthn = create_webauthn();
-        let (async_tx, mut async_rx) = unbounded();
-        let ts = duration_from_epoch_now();
-        // create the ent
-        let mut account = entry_str_to_account!(JSON_ADMIN_V1);
-
         // Setup a soft token
         let mut wa = WebauthnAuthenticator::new(U2FSoft::new());
 
         let (chal, reg_state) = webauthn
-            .generate_challenge_register(&account.name, Some(UserVerificationPolicy::Discouraged))
+            .generate_challenge_register(name, Some(UserVerificationPolicy::Discouraged))
             .expect("Failed to setup webauthn rego challenge");
 
         let r = wa
@@ -1061,13 +1147,28 @@ mod tests {
             .register_credential(&r, reg_state, |_| Ok(false))
             .expect("Failed to register soft token");
 
+        (webauthn, wa, wan_cred)
+    }
+
+    #[test]
+    fn test_idm_authsession_webauthn_only_mech() {
+        let mut audit = AuditScope::new(
+            "test_idm_authsession_webauthn_only_mech",
+            uuid::Uuid::new_v4(),
+            None,
+        );
+        let (async_tx, mut async_rx) = unbounded();
+        let ts = duration_from_epoch_now();
+        // create the ent
+        let mut account = entry_str_to_account!(JSON_ADMIN_V1);
+
+        let (webauthn, mut wa, wan_cred) = setup_webauthn(account.name.as_str());
+
         // Now create the credential for the account.
         let cred = Credential::new_webauthn_only("soft".to_string(), wan_cred);
         account.primary = Some(cred);
 
-        // now check correct mech was offered. we stash this challenge for later
-        // to help generate a failure.
-        let (_session, inv_chal) = start_webauthn_only_session!(&mut audit, account, &webauthn);
+        // now check correct mech was offered.
 
         // check send anon (fail)
         {
@@ -1114,6 +1215,7 @@ mod tests {
 
         // Check bad challenge.
         {
+            let (_session, inv_chal) = start_webauthn_only_session!(&mut audit, account, &webauthn);
             let (mut session, _chal) = start_webauthn_only_session!(&mut audit, account, &webauthn);
 
             let resp = wa
@@ -1181,4 +1283,389 @@ mod tests {
         assert!(async_rx.blocking_recv().is_none());
         audit.write_log();
     }
+
+    #[test]
+    fn test_idm_authsession_webauthn_password_mech() {
+        let mut audit = AuditScope::new(
+            "test_idm_authsession_webauthn_password_mech",
+            uuid::Uuid::new_v4(),
+            None,
+        );
+        let (async_tx, mut async_rx) = unbounded();
+        let ts = duration_from_epoch_now();
+        // create the ent
+        let mut account = entry_str_to_account!(JSON_ADMIN_V1);
+
+        let (webauthn, mut wa, wan_cred) = setup_webauthn(account.name.as_str());
+        let pw_good = "test_password";
+        let pw_bad = "bad_password";
+
+        // Now create the credential for the account.
+        let p = CryptoPolicy::minimum();
+        let cred = Credential::new_password_only(&p, pw_good)
+            .unwrap()
+            .append_webauthn("soft".to_string(), wan_cred)
+            .unwrap();
+
+        account.primary = Some(cred);
+
+        // check pw first (fail)
+        {
+            let (mut session, _) = start_password_mfa_session!(&mut audit, account, &webauthn);
+
+            match session.validate_creds(
+                &mut audit,
+                &AuthCredential::Password(pw_bad.to_string()),
+                &ts,
+                &async_tx,
+                &webauthn,
+            ) {
+                Ok(AuthState::Denied(msg)) => assert!(msg == BAD_AUTH_TYPE_MSG),
+                _ => panic!(),
+            };
+        }
+
+        // Check totp first attempt fails.
+        {
+            let (mut session, _) = start_password_mfa_session!(&mut audit, account, &webauthn);
+
+            match session.validate_creds(
+                &mut audit,
+                &AuthCredential::TOTP(0),
+                &ts,
+                &async_tx,
+                &webauthn,
+            ) {
+                Ok(AuthState::Denied(msg)) => assert!(msg == BAD_AUTH_TYPE_MSG),
+                _ => panic!(),
+            };
+        }
+
+        // check bad webauthn (fail)
+        // NOTE: We only check bad challenge here as bad softtoken is already
+        // extensively tested.
+        {
+            let (_session, inv_chal) = start_password_mfa_session!(&mut audit, account, &webauthn);
+            let (mut session, _chal) = start_password_mfa_session!(&mut audit, account, &webauthn);
+
+            let inv_chal = inv_chal.unwrap();
+
+            let resp = wa
+                // HERE -> we use inv_chal instead.
+                .do_authentication("https://idm.example.com", inv_chal)
+                .expect("failed to use softtoken to authenticate");
+
+            match session.validate_creds(
+                &mut audit,
+                &AuthCredential::Webauthn(resp),
+                &ts,
+                &async_tx,
+                &webauthn,
+            ) {
+                Ok(AuthState::Denied(msg)) => assert!(msg == BAD_WEBAUTHN_MSG),
+                _ => panic!(),
+            };
+        }
+
+        // check good webauthn/bad pw (fail)
+        {
+            let (mut session, chal) = start_password_mfa_session!(&mut audit, account, &webauthn);
+            let chal = chal.unwrap();
+
+            let resp = wa
+                .do_authentication("https://idm.example.com", chal)
+                .expect("failed to use softtoken to authenticate");
+
+            match session.validate_creds(
+                &mut audit,
+                &AuthCredential::Webauthn(resp),
+                &ts,
+                &async_tx,
+                &webauthn,
+            ) {
+                Ok(AuthState::Continue(cont)) => assert!(cont == vec![AuthAllowed::Password]),
+                _ => panic!(),
+            };
+            match session.validate_creds(
+                &mut audit,
+                &AuthCredential::Password(pw_bad.to_string()),
+                &ts,
+                &async_tx,
+                &webauthn,
+            ) {
+                Ok(AuthState::Denied(msg)) => assert!(msg == BAD_PASSWORD_MSG),
+                _ => panic!(),
+            };
+
+            // Check the async counter update was sent.
+            match async_rx.blocking_recv() {
+                Some(DelayedAction::WebauthnCounterIncrement(_)) => {}
+                _ => assert!(false),
+            }
+        }
+
+        // Check good webauthn/good pw (pass)
+        {
+            let (mut session, chal) = start_password_mfa_session!(&mut audit, account, &webauthn);
+            let chal = chal.unwrap();
+
+            let resp = wa
+                .do_authentication("https://idm.example.com", chal)
+                .expect("failed to use softtoken to authenticate");
+
+            match session.validate_creds(
+                &mut audit,
+                &AuthCredential::Webauthn(resp),
+                &ts,
+                &async_tx,
+                &webauthn,
+            ) {
+                Ok(AuthState::Continue(cont)) => assert!(cont == vec![AuthAllowed::Password]),
+                _ => panic!(),
+            };
+            match session.validate_creds(
+                &mut audit,
+                &AuthCredential::Password(pw_good.to_string()),
+                &ts,
+                &async_tx,
+                &webauthn,
+            ) {
+                Ok(AuthState::Success(_)) => {}
+                _ => panic!(),
+            };
+
+            // Check the async counter update was sent.
+            match async_rx.blocking_recv() {
+                Some(DelayedAction::WebauthnCounterIncrement(_)) => {}
+                _ => assert!(false),
+            }
+        }
+
+        drop(async_tx);
+        assert!(async_rx.blocking_recv().is_none());
+        audit.write_log();
+    }
+
+    #[test]
+    fn test_idm_authsession_webauthn_password_totp_mech() {
+        let mut audit = AuditScope::new(
+            "test_idm_authsession_webauthn_password_totp_mech",
+            uuid::Uuid::new_v4(),
+            None,
+        );
+        let (async_tx, mut async_rx) = unbounded();
+        let ts = duration_from_epoch_now();
+        // create the ent
+        let mut account = entry_str_to_account!(JSON_ADMIN_V1);
+
+        let (webauthn, mut wa, wan_cred) = setup_webauthn(account.name.as_str());
+
+        let totp = TOTP::generate_secure("test_totp".to_string(), TOTP_DEFAULT_STEP);
+        let totp_good = totp
+            .do_totp_duration_from_epoch(&ts)
+            .expect("failed to perform totp.");
+        let totp_bad = totp
+            .do_totp_duration_from_epoch(&Duration::from_secs(1234567))
+            .expect("failed to perform totp.");
+        assert!(totp_bad != totp_good);
+
+        let pw_good = "test_password";
+        let pw_bad = "bad_password";
+
+        // Now create the credential for the account.
+        let p = CryptoPolicy::minimum();
+        let cred = Credential::new_password_only(&p, pw_good)
+            .unwrap()
+            .append_webauthn("soft".to_string(), wan_cred)
+            .unwrap()
+            .update_totp(totp);
+
+        account.primary = Some(cred);
+
+        // check pw first (fail)
+        {
+            let (mut session, _) = start_password_mfa_session!(&mut audit, account, &webauthn);
+
+            match session.validate_creds(
+                &mut audit,
+                &AuthCredential::Password(pw_bad.to_string()),
+                &ts,
+                &async_tx,
+                &webauthn,
+            ) {
+                Ok(AuthState::Denied(msg)) => assert!(msg == BAD_AUTH_TYPE_MSG),
+                _ => panic!(),
+            };
+        }
+
+        // Check bad totp (fail)
+        {
+            let (mut session, _) = start_password_mfa_session!(&mut audit, account, &webauthn);
+
+            match session.validate_creds(
+                &mut audit,
+                &AuthCredential::TOTP(totp_bad),
+                &ts,
+                &async_tx,
+                &webauthn,
+            ) {
+                Ok(AuthState::Denied(msg)) => assert!(msg == BAD_TOTP_MSG),
+                _ => panic!(),
+            };
+        }
+
+        // check bad webauthn (fail)
+        {
+            let (_session, inv_chal) = start_password_mfa_session!(&mut audit, account, &webauthn);
+            let (mut session, _chal) = start_password_mfa_session!(&mut audit, account, &webauthn);
+
+            let inv_chal = inv_chal.unwrap();
+
+            let resp = wa
+                // HERE -> we use inv_chal instead.
+                .do_authentication("https://idm.example.com", inv_chal)
+                .expect("failed to use softtoken to authenticate");
+
+            match session.validate_creds(
+                &mut audit,
+                &AuthCredential::Webauthn(resp),
+                &ts,
+                &async_tx,
+                &webauthn,
+            ) {
+                Ok(AuthState::Denied(msg)) => assert!(msg == BAD_WEBAUTHN_MSG),
+                _ => panic!(),
+            };
+        }
+
+        // check good webauthn/bad pw (fail)
+        {
+            let (mut session, chal) = start_password_mfa_session!(&mut audit, account, &webauthn);
+            let chal = chal.unwrap();
+
+            let resp = wa
+                .do_authentication("https://idm.example.com", chal)
+                .expect("failed to use softtoken to authenticate");
+
+            match session.validate_creds(
+                &mut audit,
+                &AuthCredential::Webauthn(resp),
+                &ts,
+                &async_tx,
+                &webauthn,
+            ) {
+                Ok(AuthState::Continue(cont)) => assert!(cont == vec![AuthAllowed::Password]),
+                _ => panic!(),
+            };
+            match session.validate_creds(
+                &mut audit,
+                &AuthCredential::Password(pw_bad.to_string()),
+                &ts,
+                &async_tx,
+                &webauthn,
+            ) {
+                Ok(AuthState::Denied(msg)) => assert!(msg == BAD_PASSWORD_MSG),
+                _ => panic!(),
+            };
+
+            // Check the async counter update was sent.
+            match async_rx.blocking_recv() {
+                Some(DelayedAction::WebauthnCounterIncrement(_)) => {}
+                _ => assert!(false),
+            }
+        }
+
+        // check good totp/bad pw (fail)
+        {
+            let (mut session, _) = start_password_mfa_session!(&mut audit, account, &webauthn);
+
+            match session.validate_creds(
+                &mut audit,
+                &AuthCredential::TOTP(totp_good),
+                &ts,
+                &async_tx,
+                &webauthn,
+            ) {
+                Ok(AuthState::Continue(cont)) => assert!(cont == vec![AuthAllowed::Password]),
+                _ => panic!(),
+            };
+            match session.validate_creds(
+                &mut audit,
+                &AuthCredential::Password(pw_bad.to_string()),
+                &ts,
+                &async_tx,
+                &webauthn,
+            ) {
+                Ok(AuthState::Denied(msg)) => assert!(msg == BAD_PASSWORD_MSG),
+                _ => panic!(),
+            };
+        }
+
+        // check good totp/good pw (pass)
+        {
+            let (mut session, _) = start_password_mfa_session!(&mut audit, account, &webauthn);
+
+            match session.validate_creds(
+                &mut audit,
+                &AuthCredential::TOTP(totp_good),
+                &ts,
+                &async_tx,
+                &webauthn,
+            ) {
+                Ok(AuthState::Continue(cont)) => assert!(cont == vec![AuthAllowed::Password]),
+                _ => panic!(),
+            };
+            match session.validate_creds(
+                &mut audit,
+                &AuthCredential::Password(pw_good.to_string()),
+                &ts,
+                &async_tx,
+                &webauthn,
+            ) {
+                Ok(AuthState::Success(_)) => {}
+                _ => panic!(),
+            };
+        }
+
+        // Check good webauthn/good pw (pass)
+        {
+            let (mut session, chal) = start_password_mfa_session!(&mut audit, account, &webauthn);
+            let chal = chal.unwrap();
+
+            let resp = wa
+                .do_authentication("https://idm.example.com", chal)
+                .expect("failed to use softtoken to authenticate");
+
+            match session.validate_creds(
+                &mut audit,
+                &AuthCredential::Webauthn(resp),
+                &ts,
+                &async_tx,
+                &webauthn,
+            ) {
+                Ok(AuthState::Continue(cont)) => assert!(cont == vec![AuthAllowed::Password]),
+                _ => panic!(),
+            };
+            match session.validate_creds(
+                &mut audit,
+                &AuthCredential::Password(pw_good.to_string()),
+                &ts,
+                &async_tx,
+                &webauthn,
+            ) {
+                Ok(AuthState::Success(_)) => {}
+                _ => panic!(),
+            };
+
+            // Check the async counter update was sent.
+            match async_rx.blocking_recv() {
+                Some(DelayedAction::WebauthnCounterIncrement(_)) => {}
+                _ => assert!(false),
+            }
+        }
+
+        drop(async_tx);
+        assert!(async_rx.blocking_recv().is_none());
+        audit.write_log();
+    }
 }

kanidmd/src/lib/idm/event.rs

@@ -440,6 +440,42 @@ impl WebauthnDoRegisterEvent {
     }
 }
 
+#[derive(Debug)]
+pub struct RemoveWebauthnEvent {
+    pub event: Event,
+    pub target: Uuid,
+    pub label: String,
+}
+
+impl RemoveWebauthnEvent {
+    pub fn from_parts(
+        audit: &mut AuditScope,
+        qs: &QueryServerWriteTransaction,
+        uat: Option<&UserAuthToken>,
+        target: Uuid,
+        label: String,
+    ) -> Result<Self, OperationError> {
+        let e = Event::from_rw_uat(audit, qs, uat)?;
+
+        Ok(RemoveWebauthnEvent {
+            event: e,
+            target,
+            label,
+        })
+    }
+
+    #[cfg(test)]
+    pub fn new_internal(target: Uuid, label: String) -> Self {
+        let e = Event::from_internal();
+
+        RemoveWebauthnEvent {
+            event: e,
+            target,
+            label,
+        }
+    }
+}
+
 pub struct LdapAuthEvent {
     // pub event: Event,
     pub target: Uuid,

kanidmd/src/lib/idm/server.rs

@@ -9,9 +9,9 @@ use crate::idm::account::Account;
 use crate::idm::authsession::AuthSession;
 use crate::idm::event::{
     GeneratePasswordEvent, GenerateTOTPEvent, LdapAuthEvent, PasswordChangeEvent,
-    RadiusAuthTokenEvent, RegenerateRadiusSecretEvent, RemoveTOTPEvent, UnixGroupTokenEvent,
-    UnixPasswordChangeEvent, UnixUserAuthEvent, UnixUserTokenEvent, VerifyTOTPEvent,
-    WebauthnDoRegisterEvent, WebauthnInitRegisterEvent,
+    RadiusAuthTokenEvent, RegenerateRadiusSecretEvent, RemoveTOTPEvent, RemoveWebauthnEvent,
+    UnixGroupTokenEvent, UnixPasswordChangeEvent, UnixUserAuthEvent, UnixUserTokenEvent,
+    VerifyTOTPEvent, WebauthnDoRegisterEvent, WebauthnInitRegisterEvent,
 };
 use crate::idm::mfareg::{MfaRegCred, MfaRegNext, MfaRegSession};
 use crate::idm::radius::RadiusAccount;
@@ -1211,6 +1211,43 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
         Ok(next)
     }
 
+    pub fn remove_account_webauthn(
+        &mut self,
+        au: &mut AuditScope,
+        rwe: &RemoveWebauthnEvent,
+    ) -> Result<SetCredentialResponse, OperationError> {
+        ltrace!(
+            au,
+            "Attempting to remove webauthn {:?} -> {:?}",
+            rwe.label,
+            rwe.target
+        );
+
+        let account = self.target_to_account(au, &rwe.target)?;
+        let modlist = account
+            .gen_webauthn_remove_mod(rwe.label.as_str())
+            .map_err(|e| {
+                ladmin_error!(au, "Failed to gen webauthn remove mod {:?}", e);
+                e
+            })?;
+        // Perform the mod
+        self.qs_write
+            .impersonate_modify(
+                au,
+                // Filter as executed
+                &filter!(f_eq("uuid", PartialValue::new_uuidr(&account.uuid))),
+                // Filter as intended (acp)
+                &filter_all!(f_eq("uuid", PartialValue::new_uuidr(&account.uuid))),
+                &modlist,
+                &rwe.event,
+            )
+            .map_err(|e| {
+                ladmin_error!(au, "remove_account_webauthn {:?}", e);
+                e
+            })
+            .map(|_| SetCredentialResponse::Success)
+    }
+
     pub fn generate_account_totp(
         &mut self,
         au: &mut AuditScope,
@@ -1460,8 +1497,9 @@ mod tests {
     use crate::idm::delayed::{DelayedAction, WebauthnCounterIncrement};
     use crate::idm::event::{
         GenerateTOTPEvent, PasswordChangeEvent, RadiusAuthTokenEvent, RegenerateRadiusSecretEvent,
-        RemoveTOTPEvent, UnixGroupTokenEvent, UnixPasswordChangeEvent, UnixUserAuthEvent,
-        UnixUserTokenEvent, VerifyTOTPEvent, WebauthnDoRegisterEvent, WebauthnInitRegisterEvent,
+        RemoveTOTPEvent, RemoveWebauthnEvent, UnixGroupTokenEvent, UnixPasswordChangeEvent,
+        UnixUserAuthEvent, UnixUserTokenEvent, VerifyTOTPEvent, WebauthnDoRegisterEvent,
+        WebauthnInitRegisterEvent,
     };
     use crate::idm::AuthState;
     use crate::modify::{Modify, ModifyList};
@@ -3027,6 +3065,30 @@ mod tests {
             });
             let r = task::block_on(idms.delayed_action(au, duration_from_epoch_now(), da));
             assert!(Ok(true) == r);
+
+            // Check we can remove the webauthn device - provided we set a pw.
+            let mut idms_prox_write = idms.proxy_write(ct.clone());
+            let rwe =
+                RemoveWebauthnEvent::new_internal(UUID_ADMIN.clone(), "softtoken".to_string());
+            // This fails because the acc is webauthn only.
+            match idms_prox_write.remove_account_webauthn(au, &rwe) {
+                Err(OperationError::InvalidAttribute(_)) => {
+                    //ok
+                }
+                _ => assert!(false),
+            };
+            // Reg a pw.
+            let pce = PasswordChangeEvent::new_internal(&UUID_ADMIN, TEST_PASSWORD, None);
+            assert!(idms_prox_write.set_account_password(au, &pce).is_ok());
+            // Now remove, it will work.
+            idms_prox_write
+                .remove_account_webauthn(au, &rwe)
+                .expect("Failed to remove webauthn");
+
+            assert!(idms_prox_write.commit(au).is_ok());
+
+            check_admin_password(idms, au, TEST_PASSWORD);
+            // All done!
         })
     }
 }