Skip to content

Implement credential selection #43

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
iinuwa merged 6 commits into from
Jun 13, 2025
Merged

Implement credential selection #43

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f10d953
Implement credential selection
msirringhaus Jun 12, 2025
7561542
Also show optional display_name, if present
msirringhaus Jun 12, 2025
3fa38d8
Correct comment for hybrid credential selection
msirringhaus Jun 13, 2025
ae51f48
Hash credential ID before sending it to the UI components
msirringhaus Jun 13, 2025
9cf2e4f
Remove unneeded selected_credential infrastructure
msirringhaus Jun 13, 2025
dbce932
Remove unnecessary TODO comment
iinuwa Jun 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion xyz-iinuwa-credential-manager-portal-gtk/data/resources/ui/window.ui
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@
<property name="orientation">vertical</property>
<child>
<object class="GtkLabel">
<property name="label">Credentials</property>
<property name="label">Choose credential</property>
</object>
</child>
<child>
Expand Down
58 changes: 22 additions & 36 deletions xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,7 @@ use libwebauthn::{

use crate::{
credential_service::{hybrid::HybridEvent, usb::UsbEvent},
dbus::{
CredentialRequest, CredentialResponse, GetAssertionResponseInternal,
MakeCredentialResponseInternal,
},
dbus::{CredentialRequest, CredentialResponse},
view_model::{Device, Transport},
};

Expand Down Expand Up @@ -132,7 +129,25 @@ where
Poll::Pending => Poll::Pending,
Poll::Ready(Some(HybridEvent { state })) => {
if let HybridStateInternal::Completed(hybrid_response) = &state {
let response = hybrid_response.as_cred_response(&["hybrid"], "cross-platform");
let response = match hybrid_response {
AuthenticatorResponse::CredentialCreated(make_credential_response) => {
CredentialResponse::from_make_credential(
make_credential_response,
&["hybrid"],
"cross-platform",
)
}
AuthenticatorResponse::CredentialsAsserted(get_assertion_response) => {
CredentialResponse::from_get_assertion(
// When doing hybrid, the authenticator is capable of displaying it's own UI.
// So we assume here, it only ever returns one assertion.
// In case this doesn't hold true, we have to implement credential selection here,
// as is done for USB.
&get_assertion_response.assertions[0],
"cross-platform",
)
}
};
let mut cred_response = cred_response.lock().unwrap();
cred_response.replace(response);
}
Expand Down Expand Up @@ -163,9 +178,8 @@ where
Poll::Pending => Poll::Pending,
Poll::Ready(Some(UsbEvent { state })) => {
if let UsbStateInternal::Completed(response) = &state {
let response = response.as_cred_response(&["usb"], "cross-platform");
let mut cred_response = cred_response.lock().unwrap();
cred_response.replace(response);
cred_response.replace(response.clone());
}
Poll::Ready(Some(state.into()))
}
Expand All @@ -179,32 +193,6 @@ enum AuthenticatorResponse {
CredentialCreated(MakeCredentialResponse),
CredentialsAsserted(GetAssertionResponse),
}
impl AuthenticatorResponse {
fn as_cred_response(&self, transports: &[&str], modality: &str) -> CredentialResponse {
match self {
AuthenticatorResponse::CredentialCreated(make_response) => {
CredentialResponse::CreatePublicKeyCredentialResponse(
MakeCredentialResponseInternal::new(
make_response.clone(),
transports.iter().map(|s| s.to_string()).collect(),
modality.to_string(),
),
)
}
AuthenticatorResponse::CredentialsAsserted(GetAssertionResponse { assertions })
if assertions.len() == 1 =>
{
CredentialResponse::GetPublicKeyCredentialResponse(
GetAssertionResponseInternal::new(assertions[0].clone(), modality.to_string()),
)
}
AuthenticatorResponse::CredentialsAsserted(GetAssertionResponse { assertions }) => {
assert!(!assertions.is_empty());
todo!("need to support selection from multiple credentials");
}
}
}
}

impl From<MakeCredentialResponse> for AuthenticatorResponse {
fn from(value: MakeCredentialResponse) -> Self {
Expand Down Expand Up @@ -298,9 +286,7 @@ mod test {
origin: Some("webauthn.io".to_string()),
is_same_origin: Some(true),
r#type: "public-key".to_string(),
public_key: Some(CreatePublicKeyCredentialRequest {
request_json: request_json,
}),
public_key: Some(CreatePublicKeyCredentialRequest { request_json }),
}
.try_into_ctap2_request()
.unwrap();
Expand Down
159 changes: 148 additions & 11 deletions xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/usb.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,23 @@
use std::time::Duration;

use async_stream::stream;
use base64::{self, engine::general_purpose::URL_SAFE_NO_PAD, Engine};
use futures_lite::Stream;
use libwebauthn::{
ops::webauthn::GetAssertionResponse,
transport::{hid::HidDevice, Device},
webauthn::{Error as WebAuthnError, WebAuthn},
UxUpdate,
};
use tokio::sync::mpsc::{self, Receiver, Sender, WeakSender};
use tracing::{debug, warn};

use crate::dbus::CredentialRequest;
use crate::{
dbus::{CredentialRequest, GetAssertionResponseInternal},
view_model::Credential,
};

use super::AuthenticatorResponse;
use super::{AuthenticatorResponse, CredentialResponse};

pub(crate) trait UsbHandler {
fn start(
Expand All @@ -31,6 +36,7 @@ impl InProcessUsbHandler {
) -> Result<(), String> {
let mut state = UsbStateInternal::Idle;
let (signal_tx, mut signal_rx) = mpsc::channel(256);
let (cred_tx, mut cred_rx) = mpsc::channel(1);
debug!("polling for USB status");
loop {
tracing::debug!("current usb state: {:?}", state);
Expand Down Expand Up @@ -109,9 +115,32 @@ impl InProcessUsbHandler {
Some(Ok(UsbUvMessage::NeedsUserPresence)) => {
Ok(UsbStateInternal::NeedsUserPresence)
}
Some(Ok(UsbUvMessage::ReceivedCredential(response))) => {
Ok(UsbStateInternal::Completed(response.clone()))
}
Some(Ok(UsbUvMessage::ReceivedCredentials(response))) => match response {
AuthenticatorResponse::CredentialCreated(make_credential_response) => {
Ok(UsbStateInternal::Completed(
CredentialResponse::from_make_credential(
&make_credential_response,
&["usb"],
"cross-platform",
),
))
}
AuthenticatorResponse::CredentialsAsserted(get_assertion_response) => {
if get_assertion_response.assertions.len() == 1 {
Ok(UsbStateInternal::Completed(
CredentialResponse::from_get_assertion(
&get_assertion_response.assertions[0],
"cross-platform",
),
))
} else {
Ok(UsbStateInternal::SelectCredential {
response: get_assertion_response,
cred_tx: cred_tx.clone(),
})
}
}
},
Some(Err(err)) => Err(err.clone()),
None => Err("Channel disconnected".to_string()),
}
Expand Down Expand Up @@ -140,13 +169,76 @@ impl InProcessUsbHandler {
Ok(UsbStateInternal::NeedsUserVerification { attempts_left })
}
UsbUvMessage::NeedsUserPresence => Ok(UsbStateInternal::NeedsUserPresence),
UsbUvMessage::ReceivedCredential(response) => {
Ok(UsbStateInternal::Completed(response.clone()))
}
UsbUvMessage::ReceivedCredentials(response) => match response {
AuthenticatorResponse::CredentialCreated(make_credential_response) => {
Ok(UsbStateInternal::Completed(
CredentialResponse::from_make_credential(
&make_credential_response,
&["usb"],
"cross-platform",
),
))
}
AuthenticatorResponse::CredentialsAsserted(get_assertion_response) => {
if get_assertion_response.assertions.len() == 1 {
Ok(UsbStateInternal::Completed(
CredentialResponse::from_get_assertion(
&get_assertion_response.assertions[0],
"cross-platform",
),
))
} else {
Ok(UsbStateInternal::SelectCredential {
response: get_assertion_response,
cred_tx: cred_tx.clone(),
})
}
}
},
},
None => Err("USB UV handler channel closed".to_string()),
},
UsbStateInternal::Completed(_) => Ok(prev_usb_state),
UsbStateInternal::SelectCredential {
response,
cred_tx: _,
} => match cred_rx.recv().await {
Some(cred_id) => {
let assertion = response
.assertions
.iter()
.find(|c| {
c.credential_id
.as_ref()
.map(|c| {
// In order to not expose the credential ID to the untrusted UI component,
// we hashed it, before sending it. So we have to re-hash all our credential
// IDs to identify the selected one.
URL_SAFE_NO_PAD.encode(ring::digest::digest(
&ring::digest::SHA256,
&c.id,
)) == cred_id
})
.unwrap_or_default()
})
.cloned();
match assertion {
Some(assertion) => Ok(UsbStateInternal::Completed(
CredentialResponse::GetPublicKeyCredentialResponse(
GetAssertionResponseInternal::new(
assertion,
"cross-platform".to_string(),
),
),
)),
None => Err("Selected credential not found.".to_string()),
}
}
None => {
tracing::debug!("cred channel closed before receiving cred from client.");
Err("Cred channel disconnected".to_string())
}
},
};
state = next_usb_state?;
tx.send(state.clone())
Expand Down Expand Up @@ -219,7 +311,7 @@ async fn notify_ceremony_completed(
response: AuthenticatorResponse,
) {
signal_tx
.send(Ok(UsbUvMessage::ReceivedCredential(response)))
.send(Ok(UsbUvMessage::ReceivedCredentials(response)))
.await
.unwrap();
}
Expand Down Expand Up @@ -278,8 +370,14 @@ pub(super) enum UsbStateInternal {
/// 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
SelectCredential {
response: GetAssertionResponse,
cred_tx: mpsc::Sender<String>,
},

/// USB tapped, received credential
Completed(AuthenticatorResponse),
Completed(CredentialResponse),
// TODO: implement cancellation
// This isn't actually sent from the server.
//UserCancelled,
Expand Down Expand Up @@ -324,6 +422,13 @@ pub enum UsbState {
// 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<Credential>,
cred_tx: mpsc::Sender<String>,
},
}

impl From<UsbStateInternal> for UsbState {
Expand All @@ -346,6 +451,38 @@ impl From<UsbStateInternal> for UsbState {
UsbStateInternal::Completed(_) => UsbState::Completed,
// UsbStateInternal::UserCancelled => UsbState:://UserCancelled,
UsbStateInternal::SelectingDevice(_) => UsbState::SelectingDevice,
UsbStateInternal::SelectCredential { response, cred_tx } => {
UsbState::SelectCredential {
creds: response
.assertions
.iter()
.map(|x| Credential {
id: x
Copy link
Copy Markdown
Member

@iinuwa iinuwa Jun 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will need to be used as a handle for the UI to return which credential the user selected. So this should return an error (unwrap is fine for now) rather than mapping to a non-unique string.

Separately, I think we should use an opaque ID here so that the credential ID is not leaked to the (untrusted) UI code.

Copy link
Copy Markdown
Collaborator Author

@msirringhaus msirringhaus Jun 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch regarding the <unknown>-fallback!
I also hashed the ID now, before sending it.

.credential_id
.as_ref()
.map(|i| {
// In order to not expose the credential ID to the untrusted UI components,
// we hash and then encode it into a String.
URL_SAFE_NO_PAD
.encode(ring::digest::digest(&ring::digest::SHA256, &i.id))
})
.unwrap(),

name: x
.user
.as_ref()
.and_then(|u| u.name.clone())
.unwrap_or_else(|| String::from("<unknown>")),
username: x
.user
.as_ref()
.map(|u| u.display_name.clone())
.unwrap_or_default(),
})
.collect(),
cred_tx,
}
}
}
}
}
Expand Down Expand Up @@ -408,5 +545,5 @@ enum UsbUvMessage {
attempts_left: Option<u32>,
},
NeedsUserPresence,
ReceivedCredential(AuthenticatorResponse),
ReceivedCredentials(AuthenticatorResponse),
}
21 changes: 21 additions & 0 deletions xyz-iinuwa-credential-manager-portal-gtk/src/dbus.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,27 @@ pub(crate) enum CredentialResponse {
GetPublicKeyCredentialResponse(GetAssertionResponseInternal),
}

impl CredentialResponse {
pub(crate) fn from_make_credential(
response: &MakeCredentialResponse,
transports: &[&str],
modality: &str,
) -> CredentialResponse {
CredentialResponse::CreatePublicKeyCredentialResponse(MakeCredentialResponseInternal::new(
response.clone(),
transports.iter().map(|s| s.to_string()).collect(),
modality.to_string(),
))
}

pub(crate) fn from_get_assertion(assertion: &Assertion, modality: &str) -> CredentialResponse {
CredentialResponse::GetPublicKeyCredentialResponse(GetAssertionResponseInternal::new(
assertion.clone(),
modality.to_string(),
))
}
}

#[derive(Clone, Debug)]
pub(crate) struct MakeCredentialResponseInternal {
ctap: MakeCredentialResponse,
Expand Down
Loading