Skip to content

Commit 86a5bfa

Browse files
committed
Push errors up out of background tasks.
1 parent 70a0b89 commit 86a5bfa

3 files changed

Lines changed: 164 additions & 89 deletions

File tree

.idea/workspace.xml

Lines changed: 97 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/mod.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,10 @@ enum AuthenticatorResponse {
197197
#[derive(Debug, Clone)]
198198
pub enum Error {
199199
AuthenticatorError,
200+
NoCredentials,
201+
PinAttemptsExhausted,
202+
UserVerficiationAttemptsExhausted,
203+
Internal(String),
200204
}
201205

202206
impl From<MakeCredentialResponse> for AuthenticatorResponse {

xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/usb.rs

Lines changed: 63 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use base64::{self, engine::general_purpose::URL_SAFE_NO_PAD, Engine};
55
use futures_lite::Stream;
66
use libwebauthn::{
77
ops::webauthn::GetAssertionResponse,
8+
proto::CtapError,
89
transport::{hid::HidDevice, Device},
910
webauthn::{Error as WebAuthnError, WebAuthn},
1011
UxUpdate,
@@ -17,7 +18,7 @@ use crate::{
1718
view_model::Credential,
1819
};
1920

20-
use super::{AuthenticatorResponse, CredentialResponse};
21+
use super::{AuthenticatorResponse, CredentialResponse, Error};
2122

2223
pub(crate) trait UsbHandler {
2324
fn start(
@@ -33,7 +34,7 @@ impl InProcessUsbHandler {
3334
async fn process(
3435
tx: Sender<UsbStateInternal>,
3536
cred_request: CredentialRequest,
36-
) -> Result<(), String> {
37+
) -> Result<(), Error> {
3738
let mut state = UsbStateInternal::Idle;
3839
let (signal_tx, mut signal_rx) = mpsc::channel(256);
3940
let (cred_tx, mut cred_rx) = mpsc::channel(1);
@@ -58,7 +59,7 @@ impl InProcessUsbHandler {
5859
Err(err) => {
5960
failures += 1;
6061
if failures == 5 {
61-
Err(format!("Failed to list USB authenticators: {:?}. Cancelling USB state updates.", err))
62+
Err(Error::Internal(format!("Failed to list USB authenticators: {:?}. Cancelling USB state updates.", err)))
6263
} else {
6364
tracing::warn!("Failed to list USB authenticators: {:?}. Throttling USB state updates", err);
6465
tokio::time::sleep(Duration::from_secs(1)).await;
@@ -173,7 +174,7 @@ impl InProcessUsbHandler {
173174
}
174175
},
175176
Some(Err(err)) => Err(err.clone()),
176-
None => Err("Channel disconnected".to_string()),
177+
None => Err(Error::Internal("Channel disconnected".to_string())),
177178
}
178179
}
179180
UsbStateInternal::NeedsPin { .. }
@@ -218,7 +219,7 @@ impl InProcessUsbHandler {
218219
}
219220
},
220221
},
221-
None => Err("USB UV handler channel closed".to_string()),
222+
None => Err(Error::Internal("USB UV handler channel closed".to_string())),
222223
},
223224
UsbStateInternal::Completed(_) => Ok(prev_usb_state),
224225
UsbStateInternal::SelectCredential {
@@ -253,19 +254,21 @@ impl InProcessUsbHandler {
253254
),
254255
),
255256
)),
256-
None => Err("Selected credential not found.".to_string()),
257+
None => Err(Error::NoCredentials),
257258
}
258259
}
259260
None => {
260261
tracing::debug!("cred channel closed before receiving cred from client.");
261-
Err("Cred channel disconnected".to_string())
262+
Err(Error::Internal(
263+
"Cred channel disconnected prematurely".to_string(),
264+
))
262265
}
263266
},
264267
};
265268
state = next_usb_state?;
266-
tx.send(state.clone())
267-
.await
268-
.map_err(|_| "Receiver channel closed".to_string())?;
269+
tx.send(state.clone()).await.map_err(|_| {
270+
Error::Internal("USB state channel receiver closed prematurely".to_string())
271+
})?;
269272
if let UsbStateInternal::Completed(_) = state {
270273
break Ok(());
271274
}
@@ -276,7 +279,7 @@ impl InProcessUsbHandler {
276279
async fn handle_events(
277280
cred_request: &CredentialRequest,
278281
mut device: HidDevice,
279-
signal_tx: &Sender<Result<UsbUvMessage, String>>,
282+
signal_tx: &Sender<Result<UsbUvMessage, Error>>,
280283
) {
281284
let device_debug = device.to_string();
282285
match device.channel().await {
@@ -289,86 +292,54 @@ async fn handle_events(
289292
handle_usb_updates(&signal_tx2, state_rx).await;
290293
debug!("Reached end of USB update task");
291294
});
292-
match cred_request {
293-
CredentialRequest::CreatePublicKeyCredentialRequest(make_cred_request) => loop {
294-
tracing::debug!(
295-
"Polling for credential from USB authenticator {}",
296-
&device_debug
297-
);
298-
match channel.webauthn_make_credential(make_cred_request).await {
299-
Ok(response) => {
300-
tracing::debug!("Received attestation from USB authenticator");
301-
notify_ceremony_completed(
302-
signal_tx,
303-
AuthenticatorResponse::CredentialCreated(response),
304-
)
305-
.await;
306-
break;
307-
}
308-
Err(WebAuthnError::Ctap(ctap_error))
309-
if ctap_error.is_retryable_user_error() =>
310-
{
311-
warn!("Retrying WebAuthn make credential operation");
312-
continue;
313-
}
314-
Err(err) => {
315-
tracing::warn!(
316-
"Failed to create credential with USB authenticator: {:?}",
317-
err
318-
);
319-
notify_ceremony_failed(signal_tx, err.to_string()).await;
320-
break;
321-
}
322-
};
323-
},
324-
CredentialRequest::GetPublicKeyCredentialRequest(get_cred_request) => loop {
325-
match channel.webauthn_get_assertion(get_cred_request).await {
326-
Ok(response) => {
327-
tracing::debug!("Received assertion from USB authenticator");
328-
notify_ceremony_completed(
329-
signal_tx,
330-
AuthenticatorResponse::CredentialsAsserted(response),
331-
)
332-
.await;
333-
break;
334-
}
335-
Err(WebAuthnError::Ctap(ctap_error))
336-
if ctap_error.is_retryable_user_error() =>
337-
{
338-
tracing::warn!("Retrying WebAuthn get credential operation");
339-
continue;
340-
}
341-
Err(err) => {
342-
tracing::warn!(
343-
"Failed to get credential from USB authenticator: {:?}",
344-
err
345-
);
346-
notify_ceremony_failed(signal_tx, err.to_string()).await;
347-
break;
348-
}
349-
};
350-
},
351-
};
295+
tracing::debug!(
296+
"Polling for credential from USB authenticator {}",
297+
&device_debug
298+
);
299+
let response: Result<UsbUvMessage, Error> = loop {
300+
let response = match cred_request {
301+
CredentialRequest::CreatePublicKeyCredentialRequest(make_cred_request) => {
302+
channel
303+
.webauthn_make_credential(make_cred_request)
304+
.await
305+
.map(|response| UsbUvMessage::ReceivedCredentials(response.into()))
306+
}
307+
CredentialRequest::GetPublicKeyCredentialRequest(get_cred_request) => channel
308+
.webauthn_get_assertion(get_cred_request)
309+
.await
310+
.map(|response| UsbUvMessage::ReceivedCredentials(response.into())),
311+
};
312+
match response {
313+
Ok(response) => {
314+
tracing::debug!("Received credential from USB authenticator");
315+
break Ok(response);
316+
}
317+
Err(WebAuthnError::Ctap(ctap_error))
318+
if ctap_error.is_retryable_user_error() =>
319+
{
320+
warn!("Retrying WebAuthn credential operation");
321+
continue;
322+
}
323+
Err(err) => {
324+
tracing::warn!(
325+
"Failed to make/get credential with USB authenticator: {:?}",
326+
err
327+
);
328+
break Err(err);
329+
}
330+
}
331+
}
332+
.map_err(|err| match err {
333+
WebAuthnError::Ctap(CtapError::NoCredentials) => Error::NoCredentials,
334+
_ => Error::AuthenticatorError,
335+
});
336+
if let Err(err) = signal_tx.send(response).await {
337+
tracing::error!("Failed to notify that ceremony completed: {:?}", err);
338+
}
352339
}
353340
}
354341
}
355342

356-
async fn notify_ceremony_completed(
357-
signal_tx: &Sender<Result<UsbUvMessage, String>>,
358-
response: AuthenticatorResponse,
359-
) {
360-
signal_tx
361-
.send(Ok(UsbUvMessage::ReceivedCredentials(response)))
362-
.await
363-
.unwrap();
364-
}
365-
366-
async fn notify_ceremony_failed(signal_tx: &Sender<Result<UsbUvMessage, String>>, err: String) {
367-
if let Err(tx_err) = signal_tx.send(Err(err)).await {
368-
tracing::error!("Failed to notify that ceremony failed: {:?}", tx_err);
369-
}
370-
}
371-
372343
impl UsbHandler for InProcessUsbHandler {
373344
fn start(
374345
&self,
@@ -377,6 +348,8 @@ impl UsbHandler for InProcessUsbHandler {
377348
let request = request.clone();
378349
let (tx, mut rx) = mpsc::channel(32);
379350
tokio::spawn(async move {
351+
// TODO: instead of logging error here, push the errors into the
352+
// stream so credential service can handle/forward them to the UI
380353
if let Err(err) = InProcessUsbHandler::process(tx, request).await {
381354
tracing::error!("Error getting credential from USB: {:?}", err);
382355
}
@@ -541,7 +514,7 @@ impl From<UsbStateInternal> for UsbState {
541514
}
542515

543516
async fn handle_usb_updates(
544-
signal_tx: &WeakSender<Result<UsbUvMessage, String>>,
517+
signal_tx: &WeakSender<Result<UsbUvMessage, Error>>,
545518
mut state_rx: Receiver<UxUpdate>,
546519
) {
547520
while let Some(msg) = state_rx.recv().await {
@@ -561,7 +534,8 @@ async fn handle_usb_updates(
561534
UxUpdate::PinRequired(pin_update) => {
562535
if pin_update.attempts_left.is_some_and(|num| num <= 1) {
563536
// TODO: cancel authenticator operation
564-
if let Err(err) = signal_tx.send(Err("No more PIN attempts allowed. Select a different authenticator or try again later.".to_string())).await {
537+
// Err("No more PIN attempts allowed. Select a different authenticator or try again later.".to_string())
538+
if let Err(err) = signal_tx.send(Err(Error::PinAttemptsExhausted)).await {
565539
tracing::error!("Authenticator cannot process anymore PIN requests, but we cannot relay the message to credential service: {:?}", err);
566540
}
567541
continue;

0 commit comments

Comments
 (0)