From 2c14ac3263636d2fdc8919b4785b6cf8599db6ab Mon Sep 17 00:00:00 2001 From: Isaiah Inuwa Date: Thu, 3 Jul 2025 17:11:14 -0500 Subject: [PATCH 01/11] Update to latest libwebauthn --- .../Cargo.lock | 24 +++++++++++++------ .../Cargo.toml | 2 +- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/xyz-iinuwa-credential-manager-portal-gtk/Cargo.lock b/xyz-iinuwa-credential-manager-portal-gtk/Cargo.lock index 68a06f25..37b9b787 100644 --- a/xyz-iinuwa-credential-manager-portal-gtk/Cargo.lock +++ b/xyz-iinuwa-credential-manager-portal-gtk/Cargo.lock @@ -831,7 +831,7 @@ dependencies = [ "heapless-bytes", "iso7816", "serde", - "serde-indexed", + "serde-indexed 0.1.1", "serde_bytes", "serde_repr", ] @@ -1928,7 +1928,7 @@ dependencies = [ [[package]] name = "libwebauthn" version = "0.1.2" -source = "git+https://github.com/linux-credentials/libwebauthn?rev=528af8de3adfbc329b6bd6dca7bf48714e674f96#528af8de3adfbc329b6bd6dca7bf48714e674f96" +source = "git+https://github.com/linux-credentials/libwebauthn?rev=c61492dcc66cc53b33e9d3eb3377017019332964#c61492dcc66cc53b33e9d3eb3377017019332964" dependencies = [ "aes", "async-trait", @@ -1955,7 +1955,7 @@ dependencies = [ "p256", "rand 0.8.5", "serde", - "serde-indexed", + "serde-indexed 0.2.0", "serde_bytes", "serde_cbor", "serde_derive", @@ -2944,6 +2944,17 @@ dependencies = [ "syn", ] +[[package]] +name = "serde-indexed" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8f68cf7478db8b81abcf71b6d195a34a4891bd3d39868731c4d73194d74ec7a3" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "serde-xml-rs" version = "0.6.0" @@ -3101,17 +3112,16 @@ checksum = "8917285742e9f3e1683f0a9c4e6b57960b7314d0b08d30d1ecd426713ee2eee9" [[package]] name = "snow" -version = "0.10.0-alpha.1" +version = "0.10.0-beta.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e55510cf1cf5094776adcdc1141531281f3a7e3f4840c02f0495b489b766f7aa" +checksum = "7c689cc478c547a02061bbf874efca827c63ef1112ad396957411326742dccc8" dependencies = [ "aes-gcm", "blake2", - "byteorder", "chacha20poly1305", "curve25519-dalek", + "getrandom 0.3.2", "p256", - "rand_core 0.6.4", "ring", "rustc_version", "sha2", diff --git a/xyz-iinuwa-credential-manager-portal-gtk/Cargo.toml b/xyz-iinuwa-credential-manager-portal-gtk/Cargo.toml index 7feec3fb..c7a1ca1e 100644 --- a/xyz-iinuwa-credential-manager-portal-gtk/Cargo.toml +++ b/xyz-iinuwa-credential-manager-portal-gtk/Cargo.toml @@ -20,7 +20,7 @@ serde_json = "1.0.140" tracing = "0.1.41" tracing-subscriber = "0.3" zbus = { version = "5.5.0", default-features = false, features = ["blocking-api", "tokio"] } -libwebauthn = { git = "https://github.com/linux-credentials/libwebauthn", rev = "528af8de3adfbc329b6bd6dca7bf48714e674f96" } +libwebauthn = { git = "https://github.com/linux-credentials/libwebauthn", rev = "c61492dcc66cc53b33e9d3eb3377017019332964" } async-trait = "0.1.88" tokio = { version = "1.45.0", features = ["rt-multi-thread"] } futures-lite = "2.6.0" From 264975d90ece5c14cda97f7474a3f45d9cb643b8 Mon Sep 17 00:00:00 2001 From: Isaiah Inuwa Date: Tue, 17 Jun 2025 13:49:12 -0500 Subject: [PATCH 02/11] Add error page --- .../data/resources/ui/window.ui | 18 ++++++++++++++++++ .../src/view_model/gtk/mod.rs | 7 +++++++ .../src/view_model/mod.rs | 8 ++++++-- .../src/window.rs | 10 ++++++++++ 4 files changed, 41 insertions(+), 2 deletions(-) diff --git a/xyz-iinuwa-credential-manager-portal-gtk/data/resources/ui/window.ui b/xyz-iinuwa-credential-manager-portal-gtk/data/resources/ui/window.ui index 3a554c77..49be43dc 100644 --- a/xyz-iinuwa-credential-manager-portal-gtk/data/resources/ui/window.ui +++ b/xyz-iinuwa-credential-manager-portal-gtk/data/resources/ui/window.ui @@ -210,6 +210,24 @@ + + + failed + Something went wrong. + + + vertical + + + Something went wrong while retrieving a credential. Please try again later or use a different authenticator. + + + + + + + + diff --git a/xyz-iinuwa-credential-manager-portal-gtk/src/view_model/gtk/mod.rs b/xyz-iinuwa-credential-manager-portal-gtk/src/view_model/gtk/mod.rs index 171ddfa8..3f05f325 100644 --- a/xyz-iinuwa-credential-manager-portal-gtk/src/view_model/gtk/mod.rs +++ b/xyz-iinuwa-credential-manager-portal-gtk/src/view_model/gtk/mod.rs @@ -48,6 +48,9 @@ mod imp { #[property(get, set)] pub completed: RefCell, + #[property(get, set)] + pub failed: RefCell, + // pub(super) vm: RefCell>, pub(super) rx: RefCell>>, pub(super) tx: RefCell>>, @@ -172,6 +175,10 @@ impl ViewModel { view_model.set_qr_spinner_visible(false); view_model.set_completed(true); } + ViewUpdate::Failed => { + view_model.set_qr_spinner_visible(false); + view_model.set_failed(true); + } } } Err(e) => { diff --git a/xyz-iinuwa-credential-manager-portal-gtk/src/view_model/mod.rs b/xyz-iinuwa-credential-manager-portal-gtk/src/view_model/mod.rs index 27ee2657..7e73dd0b 100644 --- a/xyz-iinuwa-credential-manager-portal-gtk/src/view_model/mod.rs +++ b/xyz-iinuwa-credential-manager-portal-gtk/src/view_model/mod.rs @@ -386,16 +386,20 @@ pub enum ViewUpdate { SetTitle(String), SetDevices(Vec), SetCredentials(Vec), + WaitingForDevice(Device), + SelectingDevice, + UsbNeedsPin { attempts_left: Option }, UsbNeedsUserVerification { attempts_left: Option }, UsbNeedsUserPresence, - Completed, - SelectingDevice, HybridNeedsQrCode(String), HybridConnecting, HybridConnected, + + Completed, + Failed, } pub enum BackgroundEvent { diff --git a/xyz-iinuwa-credential-manager-portal-gtk/src/window.rs b/xyz-iinuwa-credential-manager-portal-gtk/src/window.rs index 2ec2f531..fac3b8f0 100644 --- a/xyz-iinuwa-credential-manager-portal-gtk/src/window.rs +++ b/xyz-iinuwa-credential-manager-portal-gtk/src/window.rs @@ -190,6 +190,16 @@ impl ExampleApplicationWindow { } )); + view_model.connect_failed_notify(clone!( + #[weak] + stack, + move |vm| { + if vm.completed() { + stack.set_visible_child_name("failed"); + } + } + )); + view_model.connect_credentials_notify(clone!( #[weak] stack, From a5ac36bb561635d41807d590bbc0b0bb1b25d28c Mon Sep 17 00:00:00 2001 From: Isaiah Inuwa Date: Tue, 17 Jun 2025 13:49:12 -0500 Subject: [PATCH 03/11] Remove unnecessary match --- .../src/view_model/mod.rs | 56 ++----------------- 1 file changed, 5 insertions(+), 51 deletions(-) diff --git a/xyz-iinuwa-credential-manager-portal-gtk/src/view_model/mod.rs b/xyz-iinuwa-credential-manager-portal-gtk/src/view_model/mod.rs index 7e73dd0b..0757af28 100644 --- a/xyz-iinuwa-credential-manager-portal-gtk/src/view_model/mod.rs +++ b/xyz-iinuwa-credential-manager-portal-gtk/src/view_model/mod.rs @@ -166,35 +166,10 @@ impl ViewModel { async_std::task::spawn(async move { // TODO: add cancellation while let Some(usb_state) = stream.next().await { + // forward to background event loop tx.send(BackgroundEvent::UsbStateChanged(usb_state)) .await .unwrap(); - /* - Ok(usb_state) => { - let state = usb_state.into(); - if prev_state != state { - println!("{:?}", state); - tx.send(BackgroundEvent::UsbStateChanged(state.clone())) - .await - .unwrap(); - } - prev_state = state; - match prev_state { - UsbState::Completed => break, - UsbState::UserCancelled => break, - _ => {} - }; - async_std::task::sleep(Duration::from_millis(50)).await; - } - Err(err) => { - // TODO: move to error page - tracing::error!( - "There was an error trying to get credentials from USB: {}", - err - ); - break; - } - */ } }); } @@ -204,31 +179,10 @@ impl ViewModel { let mut stream = cred_service.lock().await.get_hybrid_credential().await; async_std::task::spawn(async move { while let Some(state) = stream.next().await { - let state = state.into(); - match state { - HybridState::Idle => {} - HybridState::Started(_) => { - tx.send(BackgroundEvent::HybridQrStateChanged(state)) - .await - .unwrap(); - } - HybridState::Connecting => { - tx.send(BackgroundEvent::HybridQrStateChanged(state)) - .await - .unwrap(); - } - HybridState::Connected => { - tx.send(BackgroundEvent::HybridQrStateChanged(state)) - .await - .unwrap(); - } - HybridState::Completed => { - tx.send(BackgroundEvent::HybridQrStateChanged(state)) - .await - .unwrap(); - } - HybridState::UserCancelled => break, - }; + // forward to background event loop + tx.send(BackgroundEvent::HybridQrStateChanged(state.into())) + .await + .unwrap(); } tracing::debug!("Broke out of hybrid QR state stream"); }); From 0fdcb7dc1addd676569ae8082ce644acf54ed215 Mon Sep 17 00:00:00 2001 From: Isaiah Inuwa Date: Tue, 17 Jun 2025 13:49:12 -0500 Subject: [PATCH 04/11] Update some comments --- .../src/credential_service/hybrid.rs | 6 +++++- .../src/credential_service/usb.rs | 5 +++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/hybrid.rs b/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/hybrid.rs index 137f1ac5..902d0a2f 100644 --- a/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/hybrid.rs +++ b/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/hybrid.rs @@ -102,6 +102,7 @@ impl HybridHandler for InternalHybridHandler { } } +/// Used to communicate privileged state between handler and credential service. #[derive(Clone, Debug)] pub(super) enum HybridStateInternal { /// Awaiting BLE advert from phone. Content is the FIDO string to be @@ -123,10 +124,13 @@ pub(super) enum HybridStateInternal { UserCancelled, } +// this is here to prevent making HybridStateInternal public to the whole crate. +/// Messages between hybrid handler and credential service. pub struct HybridEvent { pub(super) state: HybridStateInternal, } +/// Used to communicate privileged state between credential service and UI. #[derive(Clone, Debug)] pub enum HybridState { /// Awaiting BLE advert from phone. Content is the FIDO string to be displayed to the user, which contains QR secret @@ -139,7 +143,7 @@ pub enum HybridState { /// Tunnel is established, waiting for user to release credential on their device. Connected, - /// Authenticator data + /// Authenticator data has been received Completed, // This isn't actually sent from the server. diff --git a/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/usb.rs b/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/usb.rs index 64d3b275..40e19278 100644 --- a/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/usb.rs +++ b/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/usb.rs @@ -331,10 +331,13 @@ impl UsbHandler for InProcessUsbHandler { } } +// this exists to prevent making UsbStateInternal type public to the whole crate. +/// A message between USB handler and credential service pub struct UsbEvent { pub(super) state: UsbStateInternal, } +/// Used to share internal state between handler and credential service #[derive(Clone, Debug, Default)] pub(super) enum UsbStateInternal { /// Not polling for FIDO USB device. @@ -378,6 +381,7 @@ pub(super) enum UsbStateInternal { SelectingDevice(Vec), } +/// Used to share public state between credential service and UI. #[derive(Clone, Debug, Default)] pub enum UsbState { /// Not polling for FIDO USB device. @@ -527,6 +531,7 @@ async fn handle_usb_updates( debug!("USB update channel closed."); } +/// Messages sent between USB authenticator and handler for UV enum UsbUvMessage { NeedsPin { attempts_left: Option, From fa0ed297e85e891f15926fd62b5a3a68b6230f4f Mon Sep 17 00:00:00 2001 From: Isaiah Inuwa Date: Tue, 17 Jun 2025 13:49:12 -0500 Subject: [PATCH 05/11] Send Failed status from HybridHandler --- .../src/credential_service/hybrid.rs | 54 +++++++++++++++---- .../src/credential_service/mod.rs | 6 +++ .../src/view_model/mod.rs | 8 +++ 3 files changed, 57 insertions(+), 11 deletions(-) diff --git a/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/hybrid.rs b/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/hybrid.rs index 902d0a2f..ee3b5042 100644 --- a/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/hybrid.rs +++ b/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/hybrid.rs @@ -8,7 +8,7 @@ use libwebauthn::webauthn::{Error as WebAuthnError, WebAuthn}; use crate::dbus::CredentialRequest; -use super::AuthenticatorResponse; +use super::{AuthenticatorResponse, Error}; pub(crate) trait HybridHandler { fn start( @@ -30,6 +30,7 @@ impl HybridHandler for InternalHybridHandler { &self, request: &CredentialRequest, ) -> impl Stream + Unpin + Send + Sized + 'static { + tracing::debug!("Starting hybrid operation"); let request = request.clone(); let (tx, rx) = async_std::channel::unbounded(); tokio::spawn(async move { @@ -58,19 +59,31 @@ impl HybridHandler for InternalHybridHandler { if let Err(err) = tx.send(HybridStateInternal::Connected).await { tracing::error!("Failed to send caBLE update: {:?}", err) } - let response: AuthenticatorResponse = loop { + tracing::debug!("Polling hybrid channel for updates."); + let response: Result = loop { match &request { CredentialRequest::CreatePublicKeyCredentialRequest(make_request) => { match channel.webauthn_make_credential(make_request).await { Ok(response) => break Ok(response.into()), Err(WebAuthnError::Ctap(ctap_error)) => { if ctap_error.is_retryable_user_error() { - tracing::debug!("Oops, try again! Error: {}", ctap_error); + tracing::debug!("Retrying credential creation operation because of CTAP error: {:?}", ctap_error); continue; + } else { + tracing::error!( + "Received CTAP unrecoverable CTAP error: {:?}", + ctap_error + ); + break Err(Error::AuthenticatorError); } - break Err(WebAuthnError::Ctap(ctap_error)); } - Err(err) => break Err(err), + Err(err) => { + tracing::error!( + "Received unrecoverable error from authenticator: {:?}", + err + ); + break Err(Error::AuthenticatorError); + } }; } CredentialRequest::GetPublicKeyCredentialRequest(get_request) => { @@ -78,18 +91,32 @@ impl HybridHandler for InternalHybridHandler { Ok(response) => break Ok(response.into()), Err(WebAuthnError::Ctap(ctap_error)) => { if ctap_error.is_retryable_user_error() { - println!("Oops, try again! Error: {}", ctap_error); + tracing::debug!("Retrying assertion operation because of CTAP error: {:?}", ctap_error); continue; + } else { + tracing::error!( + "Received CTAP unrecoverable CTAP error: {:?}", + ctap_error + ); + break Err(Error::AuthenticatorError); } - break Err(WebAuthnError::Ctap(ctap_error)); } - Err(err) => break Err(err), + Err(err) => { + tracing::error!( + "Received unrecoverable error from authenticator: {:?}", + err + ); + break Err(Error::AuthenticatorError); + } }; } } - } - .unwrap(); - if let Err(err) = tx.send(HybridStateInternal::Completed(response)).await { + }; + let terminal_state = match response { + Ok(auth_response) => HybridStateInternal::Completed(auth_response), + Err(_) => HybridStateInternal::Failed, + }; + if let Err(err) = tx.send(terminal_state).await { tracing::error!("Failed to send caBLE update: {:?}", err) } }); @@ -118,6 +145,7 @@ pub(super) enum HybridStateInternal { /// Authenticator data Completed(AuthenticatorResponse), + Failed, // TODO(cancellation) // This isn't actually sent from the server. #[allow(dead_code)] @@ -146,6 +174,9 @@ pub enum HybridState { /// Authenticator data has been received Completed, + /// Hybrid operation failed. + Failed, + // This isn't actually sent from the server. UserCancelled, } @@ -158,6 +189,7 @@ impl From for HybridState { HybridStateInternal::Connected => HybridState::Connected, HybridStateInternal::Completed(_) => HybridState::Completed, HybridStateInternal::UserCancelled => HybridState::UserCancelled, + HybridStateInternal::Failed => HybridState::Failed, } } } diff --git a/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/mod.rs b/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/mod.rs index 998424bf..3328b48d 100644 --- a/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/mod.rs +++ b/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/mod.rs @@ -194,6 +194,12 @@ enum AuthenticatorResponse { CredentialsAsserted(GetAssertionResponse), } +#[derive(Debug, Clone)] +pub enum Error { + AuthenticatorError, + Unknown, +} + impl From for AuthenticatorResponse { fn from(value: MakeCredentialResponse) -> Self { Self::CredentialCreated(value) diff --git a/xyz-iinuwa-credential-manager-portal-gtk/src/view_model/mod.rs b/xyz-iinuwa-credential-manager-portal-gtk/src/view_model/mod.rs index 0757af28..b860a27f 100644 --- a/xyz-iinuwa-credential-manager-portal-gtk/src/view_model/mod.rs +++ b/xyz-iinuwa-credential-manager-portal-gtk/src/view_model/mod.rs @@ -321,6 +321,10 @@ impl ViewModel { HybridState::UserCancelled => { self.hybrid_qr_code_data = None; } + HybridState::Failed => { + self.hybrid_qr_code_data = None; + self.tx_update.send(ViewUpdate::Failed).await.unwrap(); + } }; } }; @@ -412,6 +416,9 @@ pub enum HybridState { // This isn't actually sent from the server. UserCancelled, + + /// Failed to receive a credential + Failed, } impl From for HybridState { @@ -426,6 +433,7 @@ impl From for HybridState { crate::credential_service::hybrid::HybridState::UserCancelled => { HybridState::UserCancelled } + crate::credential_service::hybrid::HybridState::Failed => HybridState::Failed, } } } From af4f9418db00d5677fe3053808f2f862cd83a7c9 Mon Sep 17 00:00:00 2001 From: Isaiah Inuwa Date: Tue, 17 Jun 2025 16:26:07 -0500 Subject: [PATCH 06/11] Log errors on winking USB devices --- .../src/credential_service/usb.rs | 42 +++++++++++++------ 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/usb.rs b/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/usb.rs index 40e19278..5f1baef7 100644 --- a/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/usb.rs +++ b/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/usb.rs @@ -61,18 +61,36 @@ impl InProcessUsbHandler { for mut device in hid_devices { let tx = blinking_tx.clone(); tokio::spawn(async move { - let (mut channel, _state_rx) = device.channel().await.unwrap(); - let res = channel - .blink_and_wait_for_user_presence(Duration::from_secs(300)) - .await; - drop(channel); - match res { - Ok(true) => { - let _ = tx.send(Some(device)).await; - } - Ok(false) | Err(_) => { - let _ = tx.send(None).await; - } + let res = match device.channel().await { + Ok((ref mut channel, _)) => channel + .blink_and_wait_for_user_presence(Duration::from_secs(300)) + .await + .map_err(|err| { + format!( + "Failed to send wink request to authenticator: {:?}", + err + ) + }) + .and_then(|blinking| { + if blinking { + Ok(()) + } else { + Err("Authenticator was not able to blink".to_string()) + } + }), + Err(err) => Err(format!( + "Failed to create channel for USB authenticator: {:?}", + err + )), + } + .inspect_err(|err| tracing::warn!(err)) + .ok(); + + if let Err(err) = tx.send(res.map(|_| device)).await { + tracing::error!( + "Failed to send notification of wink response: {:?}", + err, + ); } }); } From 8645c2cca9ebde75042efe38176dbe49404e5d29 Mon Sep 17 00:00:00 2001 From: Isaiah Inuwa Date: Tue, 17 Jun 2025 16:26:07 -0500 Subject: [PATCH 07/11] Add some error logs to USB handler --- .../src/credential_service/mod.rs | 1 - .../src/credential_service/usb.rs | 177 +++++++++++------- 2 files changed, 111 insertions(+), 67 deletions(-) diff --git a/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/mod.rs b/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/mod.rs index 3328b48d..566a1eeb 100644 --- a/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/mod.rs +++ b/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/mod.rs @@ -197,7 +197,6 @@ enum AuthenticatorResponse { #[derive(Debug, Clone)] pub enum Error { AuthenticatorError, - Unknown, } impl From for AuthenticatorResponse { diff --git a/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/usb.rs b/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/usb.rs index 5f1baef7..fcf746fd 100644 --- a/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/usb.rs +++ b/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/usb.rs @@ -38,20 +38,33 @@ impl InProcessUsbHandler { let (signal_tx, mut signal_rx) = mpsc::channel(256); let (cred_tx, mut cred_rx) = mpsc::channel(1); debug!("polling for USB status"); + let mut failures = 0; loop { tracing::debug!("current usb state: {:?}", state); let prev_usb_state = state; let next_usb_state = match prev_usb_state { UsbStateInternal::Idle | UsbStateInternal::Waiting => { - let mut hid_devices = - libwebauthn::transport::hid::list_devices().await.unwrap(); - if hid_devices.is_empty() { - let state = UsbStateInternal::Waiting; - Ok(state) - } else if hid_devices.len() == 1 { - Ok(UsbStateInternal::Connected(hid_devices.swap_remove(0))) - } else { - Ok(UsbStateInternal::SelectingDevice(hid_devices)) + match libwebauthn::transport::hid::list_devices().await { + Ok(mut hid_devices) => { + if hid_devices.is_empty() { + let state = UsbStateInternal::Waiting; + Ok(state) + } else if hid_devices.len() == 1 { + Ok(UsbStateInternal::Connected(hid_devices.swap_remove(0))) + } else { + Ok(UsbStateInternal::SelectingDevice(hid_devices)) + } + } + Err(err) => { + failures += 1; + if failures == 5 { + Err(format!("Failed to list USB authenticators: {:?}. Cancelling USB state updates.", err)) + } else { + tracing::warn!("Failed to list USB authenticators: {:?}. Throttling USB state updates", err); + tokio::time::sleep(Duration::from_secs(1)).await; + Ok(prev_usb_state) + } + } } } UsbStateInternal::SelectingDevice(hid_devices) => { @@ -265,54 +278,79 @@ async fn handle_events( mut device: HidDevice, signal_tx: &Sender>, ) { - let (mut channel, state_rx) = device.channel().await.unwrap(); - let signal_tx2 = signal_tx.clone().downgrade(); - tokio::spawn(async move { - handle_usb_updates(&signal_tx2, state_rx).await; - debug!("Reached end of USB update task"); - }); - match cred_request { - CredentialRequest::CreatePublicKeyCredentialRequest(make_cred_request) => loop { - match channel.webauthn_make_credential(make_cred_request).await { - Ok(response) => { - notify_ceremony_completed( - signal_tx, - AuthenticatorResponse::CredentialCreated(response), - ) - .await; - break; - } - Err(WebAuthnError::Ctap(ctap_error)) if ctap_error.is_retryable_user_error() => { - warn!("Retrying WebAuthn make credential operation"); - continue; - } - Err(err) => { - notify_ceremony_failed(signal_tx, err.to_string()).await; - break; - } - }; - }, - CredentialRequest::GetPublicKeyCredentialRequest(get_cred_request) => loop { - match channel.webauthn_get_assertion(get_cred_request).await { - Ok(response) => { - notify_ceremony_completed( - signal_tx, - AuthenticatorResponse::CredentialsAsserted(response), - ) - .await; - break; - } - Err(WebAuthnError::Ctap(ctap_error)) if ctap_error.is_retryable_user_error() => { - warn!("Retrying WebAuthn get credential operation"); - continue; - } - Err(err) => { - notify_ceremony_failed(signal_tx, err.to_string()).await; - break; - } + let device_debug = device.to_string(); + match device.channel().await { + Err(err) => { + tracing::error!("Failed to open channel to USB authenticator, cannot receive user verification events: {:?}", err); + } + Ok((mut channel, state_rx)) => { + let signal_tx2 = signal_tx.clone().downgrade(); + tokio::spawn(async move { + handle_usb_updates(&signal_tx2, state_rx).await; + debug!("Reached end of USB update task"); + }); + match cred_request { + CredentialRequest::CreatePublicKeyCredentialRequest(make_cred_request) => loop { + tracing::debug!( + "Polling for credential from USB authenticator {}", + &device_debug + ); + match channel.webauthn_make_credential(make_cred_request).await { + Ok(response) => { + tracing::debug!("Received attestation from USB authenticator"); + notify_ceremony_completed( + signal_tx, + AuthenticatorResponse::CredentialCreated(response), + ) + .await; + break; + } + Err(WebAuthnError::Ctap(ctap_error)) + if ctap_error.is_retryable_user_error() => + { + warn!("Retrying WebAuthn make credential operation"); + continue; + } + Err(err) => { + tracing::warn!( + "Failed to create credential with USB authenticator: {:?}", + err + ); + notify_ceremony_failed(signal_tx, err.to_string()).await; + break; + } + }; + }, + CredentialRequest::GetPublicKeyCredentialRequest(get_cred_request) => loop { + match channel.webauthn_get_assertion(get_cred_request).await { + Ok(response) => { + tracing::debug!("Received assertion from USB authenticator"); + notify_ceremony_completed( + signal_tx, + AuthenticatorResponse::CredentialsAsserted(response), + ) + .await; + break; + } + Err(WebAuthnError::Ctap(ctap_error)) + if ctap_error.is_retryable_user_error() => + { + tracing::warn!("Retrying WebAuthn get credential operation"); + continue; + } + Err(err) => { + tracing::warn!( + "Failed to get credential from USB authenticator: {:?}", + err + ); + notify_ceremony_failed(signal_tx, err.to_string()).await; + break; + } + }; + }, }; - }, - }; + } + } } async fn notify_ceremony_completed( @@ -326,7 +364,9 @@ async fn notify_ceremony_completed( } async fn notify_ceremony_failed(signal_tx: &Sender>, err: String) { - signal_tx.send(Err(err)).await.unwrap(); + if let Err(tx_err) = signal_tx.send(Err(err)).await { + tracing::error!("Failed to notify that ceremony failed: {:?}", tx_err); + } } impl UsbHandler for InProcessUsbHandler { @@ -511,25 +551,31 @@ async fn handle_usb_updates( }; match msg { UxUpdate::UvRetry { attempts_left } => { - signal_tx + if let Err(err) = signal_tx .send(Ok(UsbUvMessage::NeedsUserVerification { attempts_left })) .await - .unwrap(); + { + tracing::error!("Authenticator requested user verficiation, but we cannot relay the message to credential service: {:?}", err); + } } UxUpdate::PinRequired(pin_update) => { if pin_update.attempts_left.is_some_and(|num| num <= 1) { // TODO: cancel authenticator operation - signal_tx.send(Err("No more PIN attempts allowed. Select a different authenticator or try again later.".to_string())).await.unwrap(); + if let Err(err) = signal_tx.send(Err("No more PIN attempts allowed. Select a different authenticator or try again later.".to_string())).await { + tracing::error!("Authenticator cannot process anymore PIN requests, but we cannot relay the message to credential service: {:?}", err); + } continue; } let (pin_tx, mut pin_rx) = mpsc::channel(1); - signal_tx + if let Err(err) = signal_tx .send(Ok(UsbUvMessage::NeedsPin { pin_tx, attempts_left: pin_update.attempts_left, })) .await - .unwrap(); + { + tracing::error!("Authenticator requested a PIN from the user, but we cannot relay the message to the credential service: {:?}", err); + } match pin_rx.recv().await { Some(pin) => match pin_update.send_pin(&pin) { Ok(()) => {} @@ -539,10 +585,9 @@ async fn handle_usb_updates( } } UxUpdate::PresenceRequired => { - signal_tx - .send(Ok(UsbUvMessage::NeedsUserPresence)) - .await - .unwrap(); + if let Err(err) = signal_tx.send(Ok(UsbUvMessage::NeedsUserPresence)).await { + tracing::error!("Authenticator requested user presence, but we cannot relay the message to the credential service: {:?}", err); + } } } } From a41206dec523c81c9fa62a47d48d92ab5f79ca1e Mon Sep 17 00:00:00 2001 From: Isaiah Inuwa Date: Tue, 17 Jun 2025 17:57:54 -0500 Subject: [PATCH 08/11] Push errors up out of background tasks. --- .../src/credential_service/mod.rs | 4 + .../src/credential_service/usb.rs | 152 ++++++++---------- 2 files changed, 67 insertions(+), 89 deletions(-) diff --git a/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/mod.rs b/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/mod.rs index 566a1eeb..8c5552a9 100644 --- a/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/mod.rs +++ b/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/mod.rs @@ -197,6 +197,10 @@ enum AuthenticatorResponse { #[derive(Debug, Clone)] pub enum Error { AuthenticatorError, + NoCredentials, + PinAttemptsExhausted, + UserVerficiationAttemptsExhausted, + Internal(String), } impl From for AuthenticatorResponse { diff --git a/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/usb.rs b/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/usb.rs index fcf746fd..ed8b2c1c 100644 --- a/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/usb.rs +++ b/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/usb.rs @@ -5,6 +5,7 @@ use base64::{self, engine::general_purpose::URL_SAFE_NO_PAD, Engine}; use futures_lite::Stream; use libwebauthn::{ ops::webauthn::GetAssertionResponse, + proto::CtapError, transport::{hid::HidDevice, Device}, webauthn::{Error as WebAuthnError, WebAuthn}, UxUpdate, @@ -17,7 +18,7 @@ use crate::{ view_model::Credential, }; -use super::{AuthenticatorResponse, CredentialResponse}; +use super::{AuthenticatorResponse, CredentialResponse, Error}; pub(crate) trait UsbHandler { fn start( @@ -33,7 +34,7 @@ impl InProcessUsbHandler { async fn process( tx: Sender, cred_request: CredentialRequest, - ) -> Result<(), String> { + ) -> Result<(), Error> { let mut state = UsbStateInternal::Idle; let (signal_tx, mut signal_rx) = mpsc::channel(256); let (cred_tx, mut cred_rx) = mpsc::channel(1); @@ -58,7 +59,7 @@ impl InProcessUsbHandler { Err(err) => { failures += 1; if failures == 5 { - Err(format!("Failed to list USB authenticators: {:?}. Cancelling USB state updates.", err)) + Err(Error::Internal(format!("Failed to list USB authenticators: {:?}. Cancelling USB state updates.", err))) } else { tracing::warn!("Failed to list USB authenticators: {:?}. Throttling USB state updates", err); tokio::time::sleep(Duration::from_secs(1)).await; @@ -173,7 +174,7 @@ impl InProcessUsbHandler { } }, Some(Err(err)) => Err(err.clone()), - None => Err("Channel disconnected".to_string()), + None => Err(Error::Internal("Channel disconnected".to_string())), } } UsbStateInternal::NeedsPin { .. } @@ -218,7 +219,7 @@ impl InProcessUsbHandler { } }, }, - None => Err("USB UV handler channel closed".to_string()), + None => Err(Error::Internal("USB UV handler channel closed".to_string())), }, UsbStateInternal::Completed(_) => Ok(prev_usb_state), UsbStateInternal::SelectCredential { @@ -253,19 +254,21 @@ impl InProcessUsbHandler { ), ), )), - None => Err("Selected credential not found.".to_string()), + None => Err(Error::NoCredentials), } } None => { tracing::debug!("cred channel closed before receiving cred from client."); - Err("Cred channel disconnected".to_string()) + Err(Error::Internal( + "Cred channel disconnected prematurely".to_string(), + )) } }, }; state = next_usb_state?; - tx.send(state.clone()) - .await - .map_err(|_| "Receiver channel closed".to_string())?; + tx.send(state.clone()).await.map_err(|_| { + Error::Internal("USB state channel receiver closed prematurely".to_string()) + })?; if let UsbStateInternal::Completed(_) = state { break Ok(()); } @@ -276,7 +279,7 @@ impl InProcessUsbHandler { async fn handle_events( cred_request: &CredentialRequest, mut device: HidDevice, - signal_tx: &Sender>, + signal_tx: &Sender>, ) { let device_debug = device.to_string(); match device.channel().await { @@ -289,86 +292,54 @@ async fn handle_events( handle_usb_updates(&signal_tx2, state_rx).await; debug!("Reached end of USB update task"); }); - match cred_request { - CredentialRequest::CreatePublicKeyCredentialRequest(make_cred_request) => loop { - tracing::debug!( - "Polling for credential from USB authenticator {}", - &device_debug - ); - match channel.webauthn_make_credential(make_cred_request).await { - Ok(response) => { - tracing::debug!("Received attestation from USB authenticator"); - notify_ceremony_completed( - signal_tx, - AuthenticatorResponse::CredentialCreated(response), - ) - .await; - break; - } - Err(WebAuthnError::Ctap(ctap_error)) - if ctap_error.is_retryable_user_error() => - { - warn!("Retrying WebAuthn make credential operation"); - continue; - } - Err(err) => { - tracing::warn!( - "Failed to create credential with USB authenticator: {:?}", - err - ); - notify_ceremony_failed(signal_tx, err.to_string()).await; - break; - } - }; - }, - CredentialRequest::GetPublicKeyCredentialRequest(get_cred_request) => loop { - match channel.webauthn_get_assertion(get_cred_request).await { - Ok(response) => { - tracing::debug!("Received assertion from USB authenticator"); - notify_ceremony_completed( - signal_tx, - AuthenticatorResponse::CredentialsAsserted(response), - ) - .await; - break; - } - Err(WebAuthnError::Ctap(ctap_error)) - if ctap_error.is_retryable_user_error() => - { - tracing::warn!("Retrying WebAuthn get credential operation"); - continue; - } - Err(err) => { - tracing::warn!( - "Failed to get credential from USB authenticator: {:?}", - err - ); - notify_ceremony_failed(signal_tx, err.to_string()).await; - break; - } - }; - }, - }; + tracing::debug!( + "Polling for credential from USB authenticator {}", + &device_debug + ); + let response: Result = loop { + let response = match cred_request { + CredentialRequest::CreatePublicKeyCredentialRequest(make_cred_request) => { + channel + .webauthn_make_credential(make_cred_request) + .await + .map(|response| UsbUvMessage::ReceivedCredentials(response.into())) + } + CredentialRequest::GetPublicKeyCredentialRequest(get_cred_request) => channel + .webauthn_get_assertion(get_cred_request) + .await + .map(|response| UsbUvMessage::ReceivedCredentials(response.into())), + }; + match response { + Ok(response) => { + tracing::debug!("Received credential from USB authenticator"); + break Ok(response); + } + Err(WebAuthnError::Ctap(ctap_error)) + if ctap_error.is_retryable_user_error() => + { + warn!("Retrying WebAuthn credential operation"); + continue; + } + Err(err) => { + tracing::warn!( + "Failed to make/get credential with USB authenticator: {:?}", + err + ); + break Err(err); + } + } + } + .map_err(|err| match err { + WebAuthnError::Ctap(CtapError::NoCredentials) => Error::NoCredentials, + _ => Error::AuthenticatorError, + }); + if let Err(err) = signal_tx.send(response).await { + tracing::error!("Failed to notify that ceremony completed: {:?}", err); + } } } } -async fn notify_ceremony_completed( - signal_tx: &Sender>, - response: AuthenticatorResponse, -) { - signal_tx - .send(Ok(UsbUvMessage::ReceivedCredentials(response))) - .await - .unwrap(); -} - -async fn notify_ceremony_failed(signal_tx: &Sender>, err: String) { - if let Err(tx_err) = signal_tx.send(Err(err)).await { - tracing::error!("Failed to notify that ceremony failed: {:?}", tx_err); - } -} - impl UsbHandler for InProcessUsbHandler { fn start( &self, @@ -377,6 +348,8 @@ impl UsbHandler for InProcessUsbHandler { let request = request.clone(); let (tx, mut rx) = mpsc::channel(32); tokio::spawn(async move { + // TODO: instead of logging error here, push the errors into the + // stream so credential service can handle/forward them to the UI if let Err(err) = InProcessUsbHandler::process(tx, request).await { tracing::error!("Error getting credential from USB: {:?}", err); } @@ -541,7 +514,7 @@ impl From for UsbState { } async fn handle_usb_updates( - signal_tx: &WeakSender>, + signal_tx: &WeakSender>, mut state_rx: Receiver, ) { while let Some(msg) = state_rx.recv().await { @@ -561,7 +534,8 @@ async fn handle_usb_updates( UxUpdate::PinRequired(pin_update) => { if pin_update.attempts_left.is_some_and(|num| num <= 1) { // TODO: cancel authenticator operation - if let Err(err) = signal_tx.send(Err("No more PIN attempts allowed. Select a different authenticator or try again later.".to_string())).await { + // Err("No more PIN attempts allowed. Select a different authenticator or try again later.".to_string()) + if let Err(err) = signal_tx.send(Err(Error::PinAttemptsExhausted)).await { tracing::error!("Authenticator cannot process anymore PIN requests, but we cannot relay the message to credential service: {:?}", err); } continue; From 59789524623f165cfb5a473b7d3ea73ed06331d7 Mon Sep 17 00:00:00 2001 From: Isaiah Inuwa Date: Thu, 3 Jul 2025 16:59:13 -0500 Subject: [PATCH 09/11] Add 'Failed' USB state --- .../src/credential_service/mod.rs | 2 +- .../src/credential_service/usb.rs | 52 +++++++++---------- .../src/view_model/mod.rs | 9 ++-- 3 files changed, 29 insertions(+), 34 deletions(-) diff --git a/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/mod.rs b/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/mod.rs index 8c5552a9..093e2619 100644 --- a/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/mod.rs +++ b/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/mod.rs @@ -199,7 +199,7 @@ pub enum Error { AuthenticatorError, NoCredentials, PinAttemptsExhausted, - UserVerficiationAttemptsExhausted, + // TODO: We may want to hide the details on this variant from the public API. Internal(String), } diff --git a/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/usb.rs b/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/usb.rs index ed8b2c1c..f1951986 100644 --- a/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/usb.rs +++ b/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/usb.rs @@ -221,7 +221,6 @@ impl InProcessUsbHandler { }, None => Err(Error::Internal("USB UV handler channel closed".to_string())), }, - UsbStateInternal::Completed(_) => Ok(prev_usb_state), UsbStateInternal::SelectCredential { response, cred_tx: _, @@ -264,14 +263,13 @@ impl InProcessUsbHandler { )) } }, + UsbStateInternal::Completed(_) => break Ok(()), + UsbStateInternal::Failed(err) => break Err(err), }; - state = next_usb_state?; + state = next_usb_state.map_or_else(|err| UsbStateInternal::Failed(err), |s| s); tx.send(state.clone()).await.map_err(|_| { Error::Internal("USB state channel receiver closed prematurely".to_string()) })?; - if let UsbStateInternal::Completed(_) = state { - break Ok(()); - } } } } @@ -330,6 +328,7 @@ async fn handle_events( } } .map_err(|err| match err { + WebAuthnError::Ctap(CtapError::PINAuthBlocked) => Error::PinAttemptsExhausted, WebAuthnError::Ctap(CtapError::NoCredentials) => Error::NoCredentials, _ => Error::AuthenticatorError, }); @@ -378,6 +377,10 @@ pub(super) enum UsbStateInternal { /// Awaiting FIDO USB device to be plugged in. Waiting, + /// When we encounter multiple devices, we let all of them blink and continue + /// with the one that was tapped. + SelectingDevice(Vec), + /// USB device connected, prompt user to tap Connected(HidDevice), @@ -388,14 +391,12 @@ pub(super) enum UsbStateInternal { }, /// The device needs on-device user verification. - NeedsUserVerification { - attempts_left: Option, - }, + NeedsUserVerification { attempts_left: Option }, /// The device needs evidence of user presence (e.g. touch) to release the credential. NeedsUserPresence, - // Multiple credentials have been found and the user has to select which to use + /// Multiple credentials have been found and the user has to select which to use SelectCredential { response: GetAssertionResponse, cred_tx: mpsc::Sender, @@ -403,13 +404,12 @@ pub(super) enum UsbStateInternal { /// USB tapped, received credential Completed(CredentialResponse), + + /// There was an error while interacting with the authenticator. + Failed(Error), // TODO: implement cancellation // This isn't actually sent from the server. //UserCancelled, - - // When we encounter multiple devices, we let all of them blink and continue - // with the one that was tapped. - SelectingDevice(Vec), } /// Used to share public state between credential service and UI. @@ -422,6 +422,10 @@ pub enum UsbState { /// Awaiting FIDO USB device to be plugged in. Waiting, + // When we encounter multiple devices, we let all of them blink and continue + // with the one that was tapped. + SelectingDevice, + /// USB device connected, prompt user to tap Connected, @@ -438,23 +442,22 @@ pub enum UsbState { /// The device needs evidence of user presence (e.g. touch) to release the credential. NeedsUserPresence, - - /// USB tapped, received credential - Completed, // TODO: implement cancellation // This isn't actually sent from the server. //UserCancelled, - // When we encounter multiple devices, we let all of them blink and continue - // with the one that was tapped. - SelectingDevice, - // Multiple credentials have been found and the user has to select which to use // List of user-identities to decide which to use. SelectCredential { creds: Vec, cred_tx: mpsc::Sender, }, + + /// USB tapped, received credential + Completed, + + /// Interaction with the authenticator failed. + Failed(Error), } impl From for UsbState { @@ -509,6 +512,7 @@ impl From for UsbState { cred_tx, } } + UsbStateInternal::Failed(err) => UsbState::Failed(err), } } } @@ -532,14 +536,6 @@ async fn handle_usb_updates( } } UxUpdate::PinRequired(pin_update) => { - if pin_update.attempts_left.is_some_and(|num| num <= 1) { - // TODO: cancel authenticator operation - // Err("No more PIN attempts allowed. Select a different authenticator or try again later.".to_string()) - if let Err(err) = signal_tx.send(Err(Error::PinAttemptsExhausted)).await { - tracing::error!("Authenticator cannot process anymore PIN requests, but we cannot relay the message to credential service: {:?}", err); - } - continue; - } let (pin_tx, mut pin_rx) = mpsc::channel(1); if let Err(err) = signal_tx .send(Ok(UsbUvMessage::NeedsPin { diff --git a/xyz-iinuwa-credential-manager-portal-gtk/src/view_model/mod.rs b/xyz-iinuwa-credential-manager-portal-gtk/src/view_model/mod.rs index b860a27f..727aed1d 100644 --- a/xyz-iinuwa-credential-manager-portal-gtk/src/view_model/mod.rs +++ b/xyz-iinuwa-credential-manager-portal-gtk/src/view_model/mod.rs @@ -235,11 +235,7 @@ impl ViewModel { } } - Event::Background(BackgroundEvent::UsbPressed) => { - println!("UsbPressed"); - } Event::Background(BackgroundEvent::UsbStateChanged(state)) => { - // TODO: do we need to store the USB state? match state { UsbState::Connected => { info!("Found USB device") @@ -284,6 +280,10 @@ impl ViewModel { .await .unwrap(); } + // TODO: Provide more specific error messages using the wrapped Error. + UsbState::Failed(_) => { + self.tx_update.send(ViewUpdate::Failed).await.unwrap() + } } } Event::Background(BackgroundEvent::HybridQrStateChanged(state)) => { @@ -361,7 +361,6 @@ pub enum ViewUpdate { } pub enum BackgroundEvent { - UsbPressed, UsbStateChanged(UsbState), HybridQrStateChanged(HybridState), } From 7d3beebbbba90240c731111c3928d0a280e42efb Mon Sep 17 00:00:00 2001 From: Isaiah Inuwa Date: Fri, 4 Jul 2025 17:31:28 -0500 Subject: [PATCH 10/11] Finish wiring up USB error page --- .idea/.gitignore | 8 ++++++++ .idea/linux-webauthn-platform-api.iml | 12 +++++++++++ .idea/modules.xml | 8 ++++++++ .idea/vcs.xml | 6 ++++++ .../data/resources/ui/window.ui | 7 +++++++ .../src/credential_service/usb.rs | 15 ++++++++------ .../src/view_model/gtk/mod.rs | 3 ++- .../src/view_model/mod.rs | 20 ++++++++++++++----- .../src/window.rs | 2 +- 9 files changed, 68 insertions(+), 13 deletions(-) create mode 100644 .idea/.gitignore create mode 100644 .idea/linux-webauthn-platform-api.iml create mode 100644 .idea/modules.xml create mode 100644 .idea/vcs.xml diff --git a/.idea/.gitignore b/.idea/.gitignore new file mode 100644 index 00000000..13566b81 --- /dev/null +++ b/.idea/.gitignore @@ -0,0 +1,8 @@ +# Default ignored files +/shelf/ +/workspace.xml +# Editor-based HTTP Client requests +/httpRequests/ +# Datasource local storage ignored files +/dataSources/ +/dataSources.local.xml diff --git a/.idea/linux-webauthn-platform-api.iml b/.idea/linux-webauthn-platform-api.iml new file mode 100644 index 00000000..5e2b0d78 --- /dev/null +++ b/.idea/linux-webauthn-platform-api.iml @@ -0,0 +1,12 @@ + + + + + + + + + + + + \ No newline at end of file diff --git a/.idea/modules.xml b/.idea/modules.xml new file mode 100644 index 00000000..41b72fe7 --- /dev/null +++ b/.idea/modules.xml @@ -0,0 +1,8 @@ + + + + + + + + \ No newline at end of file diff --git a/.idea/vcs.xml b/.idea/vcs.xml new file mode 100644 index 00000000..35eb1ddf --- /dev/null +++ b/.idea/vcs.xml @@ -0,0 +1,6 @@ + + + + + + \ No newline at end of file diff --git a/xyz-iinuwa-credential-manager-portal-gtk/data/resources/ui/window.ui b/xyz-iinuwa-credential-manager-portal-gtk/data/resources/ui/window.ui index 49be43dc..555028b4 100644 --- a/xyz-iinuwa-credential-manager-portal-gtk/data/resources/ui/window.ui +++ b/xyz-iinuwa-credential-manager-portal-gtk/data/resources/ui/window.ui @@ -219,6 +219,13 @@ vertical + + + + ExampleApplicationWindow + + + Something went wrong while retrieving a credential. Please try again later or use a different authenticator. diff --git a/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/usb.rs b/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/usb.rs index f1951986..9bb99f78 100644 --- a/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/usb.rs +++ b/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/usb.rs @@ -180,19 +180,21 @@ impl InProcessUsbHandler { UsbStateInternal::NeedsPin { .. } | UsbStateInternal::NeedsUserVerification { .. } | UsbStateInternal::NeedsUserPresence => match signal_rx.recv().await { - Some(msg) => match msg? { - UsbUvMessage::NeedsPin { + Some(msg) => match msg { + Ok(UsbUvMessage::NeedsPin { attempts_left, pin_tx, - } => Ok(UsbStateInternal::NeedsPin { + }) => Ok(UsbStateInternal::NeedsPin { attempts_left, pin_tx, }), - UsbUvMessage::NeedsUserVerification { attempts_left } => { + Ok(UsbUvMessage::NeedsUserVerification { attempts_left }) => { Ok(UsbStateInternal::NeedsUserVerification { attempts_left }) } - UsbUvMessage::NeedsUserPresence => Ok(UsbStateInternal::NeedsUserPresence), - UsbUvMessage::ReceivedCredentials(response) => match response { + Ok(UsbUvMessage::NeedsUserPresence) => { + Ok(UsbStateInternal::NeedsUserPresence) + } + Ok(UsbUvMessage::ReceivedCredentials(response)) => match response { AuthenticatorResponse::CredentialCreated(make_credential_response) => { Ok(UsbStateInternal::Completed( CredentialResponse::from_make_credential( @@ -218,6 +220,7 @@ impl InProcessUsbHandler { } } }, + Err(err) => Err(err), }, None => Err(Error::Internal("USB UV handler channel closed".to_string())), }, diff --git a/xyz-iinuwa-credential-manager-portal-gtk/src/view_model/gtk/mod.rs b/xyz-iinuwa-credential-manager-portal-gtk/src/view_model/gtk/mod.rs index 3f05f325..b5f23254 100644 --- a/xyz-iinuwa-credential-manager-portal-gtk/src/view_model/gtk/mod.rs +++ b/xyz-iinuwa-credential-manager-portal-gtk/src/view_model/gtk/mod.rs @@ -175,9 +175,10 @@ impl ViewModel { view_model.set_qr_spinner_visible(false); view_model.set_completed(true); } - ViewUpdate::Failed => { + ViewUpdate::Failed(error_msg) => { view_model.set_qr_spinner_visible(false); view_model.set_failed(true); + view_model.set_prompt(error_msg); } } } diff --git a/xyz-iinuwa-credential-manager-portal-gtk/src/view_model/mod.rs b/xyz-iinuwa-credential-manager-portal-gtk/src/view_model/mod.rs index 727aed1d..c33b91b2 100644 --- a/xyz-iinuwa-credential-manager-portal-gtk/src/view_model/mod.rs +++ b/xyz-iinuwa-credential-manager-portal-gtk/src/view_model/mod.rs @@ -10,7 +10,9 @@ use async_std::{ use tokio::sync::mpsc; use tracing::{error, info}; -use crate::credential_service::{CredentialServiceClient, UsbState}; +use crate::credential_service::{ + CredentialServiceClient, Error as CredentialServiceError, UsbState, +}; #[derive(Debug)] pub(crate) struct ViewModel @@ -281,8 +283,16 @@ impl ViewModel { .unwrap(); } // TODO: Provide more specific error messages using the wrapped Error. - UsbState::Failed(_) => { - self.tx_update.send(ViewUpdate::Failed).await.unwrap() + UsbState::Failed(err) => { + let error_msg = String::from(match err { + CredentialServiceError::NoCredentials => "No matching credentials found on this authenticator.", + CredentialServiceError::PinAttemptsExhausted => "No more PIN attempts allowed. Try removing your device and plugging it back in.", + CredentialServiceError::AuthenticatorError | CredentialServiceError::Internal(_) => "Something went wrong while retrieving a credential. Please try again later or use a different authenticator.", + }); + self.tx_update + .send(ViewUpdate::Failed(error_msg)) + .await + .unwrap() } } } @@ -323,7 +333,7 @@ impl ViewModel { } HybridState::Failed => { self.hybrid_qr_code_data = None; - self.tx_update.send(ViewUpdate::Failed).await.unwrap(); + self.tx_update.send(ViewUpdate::Failed(String::from("Something went wrong. Try again later or use a different authenticator."))).await.unwrap(); } }; } @@ -357,7 +367,7 @@ pub enum ViewUpdate { HybridConnected, Completed, - Failed, + Failed(String), } pub enum BackgroundEvent { diff --git a/xyz-iinuwa-credential-manager-portal-gtk/src/window.rs b/xyz-iinuwa-credential-manager-portal-gtk/src/window.rs index fac3b8f0..4da4aa50 100644 --- a/xyz-iinuwa-credential-manager-portal-gtk/src/window.rs +++ b/xyz-iinuwa-credential-manager-portal-gtk/src/window.rs @@ -194,7 +194,7 @@ impl ExampleApplicationWindow { #[weak] stack, move |vm| { - if vm.completed() { + if vm.failed() { stack.set_visible_child_name("failed"); } } From a9e5a224f8b1351c2eee0b04ec9ba81fc5994f44 Mon Sep 17 00:00:00 2001 From: Isaiah Inuwa Date: Fri, 4 Jul 2025 22:10:57 -0500 Subject: [PATCH 11/11] Add some comments --- .../src/credential_service/mod.rs | 8 ++++++++ .../src/credential_service/usb.rs | 5 +++++ 2 files changed, 13 insertions(+) diff --git a/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/mod.rs b/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/mod.rs index 093e2619..fe3f49cb 100644 --- a/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/mod.rs +++ b/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/mod.rs @@ -196,10 +196,18 @@ enum AuthenticatorResponse { #[derive(Debug, Clone)] pub enum Error { + /// Some unknown error with the authenticator occurred. AuthenticatorError, + /// No matching credentials were found on the device. NoCredentials, + /// Too many incorrect PIN attempts, and authenticator must be removed and + /// reinserted to continue any more PIN attempts. + /// + /// Note that this is different than exhausting the PIN count that fully + /// locks out the device. PinAttemptsExhausted, // TODO: We may want to hide the details on this variant from the public API. + /// Something went wrong with the credential service itself, not the authenticator. Internal(String), } diff --git a/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/usb.rs b/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/usb.rs index 9bb99f78..5de60a82 100644 --- a/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/usb.rs +++ b/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/usb.rs @@ -40,6 +40,8 @@ impl InProcessUsbHandler { let (cred_tx, mut cred_rx) = mpsc::channel(1); debug!("polling for USB status"); let mut failures = 0; + // act on current USB USB state, send state changes to the stream, and + // loop until a credential or error is returned. loop { tracing::debug!("current usb state: {:?}", state); let prev_usb_state = state; @@ -177,6 +179,9 @@ impl InProcessUsbHandler { None => Err(Error::Internal("Channel disconnected".to_string())), } } + // TODO: This match arm does basically the same thing as above, we + // should refactor this so we don't have to update things in two + // places. UsbStateInternal::NeedsPin { .. } | UsbStateInternal::NeedsUserVerification { .. } | UsbStateInternal::NeedsUserPresence => match signal_rx.recv().await {