Skip to content

Commit 7fd9d92

Browse files
committed
fix: handle excludeCredentials with credentials with an empty store
1 parent 2677e25 commit 7fd9d92

File tree

2 files changed

+125
-3
lines changed

2 files changed

+125
-3
lines changed

passkey-authenticator/src/authenticator/make_credential.rs

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,16 +32,24 @@ where
3232
.filter(|list| !list.is_empty())
3333
.is_some()
3434
{
35-
if let Some(excluded_credential) = self
35+
// Handle the case where find_credentials returns NoCredentials error.
36+
// An empty credential store should not prevent credential creation when checking
37+
// the exclude list. NoCredentials simply means there are no credentials to exclude.
38+
let excluded_credentials = match self
3639
.store()
3740
.find_credentials(
3841
input.exclude_list.as_deref(),
3942
&input.rp.id,
4043
Some(&input.user.id),
4144
)
42-
.await?
43-
.first()
45+
.await
4446
{
47+
Ok(creds) => creds,
48+
Err(status) if status == StatusCode::from(Ctap2Error::NoCredentials) => vec![],
49+
Err(e) => return Err(e),
50+
};
51+
52+
if let Some(excluded_credential) = excluded_credentials.first() {
4553
self.check_user(
4654
UiHint::InformExcludedCredentialFound(excluded_credential),
4755
&input.options,

passkey-authenticator/src/authenticator/make_credential/tests.rs

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -438,3 +438,117 @@ async fn make_credential_returns_err_when_rk_is_requested_but_not_supported() {
438438
// Assert
439439
assert_eq!(err, Ctap2Error::UnsupportedOption.into());
440440
}
441+
#[tokio::test]
442+
async fn empty_store_with_exclude_credentials_succeeds() {
443+
// This test verifies the fix for the issue where an empty credential store
444+
// would return NoCredentials error when checking excludeCredentials,
445+
// causing credential creation to fail incorrectly.
446+
447+
let cred_id: Bytes = random_vec(16).into();
448+
let request = Request {
449+
exclude_list: Some(vec![webauthn::PublicKeyCredentialDescriptor {
450+
ty: webauthn::PublicKeyCredentialType::PublicKey,
451+
id: cred_id.clone(),
452+
transports: Some(vec![webauthn::AuthenticatorTransport::Usb]),
453+
}]),
454+
..good_request()
455+
};
456+
457+
// Use an empty store - previously this would cause NoCredentials error
458+
let shared_store = Arc::new(Mutex::new(MemoryStore::new()));
459+
let user_mock = MockUserValidationMethod::verified_user_with_hint(
460+
1,
461+
MockUiHint::RequestNewCredential(request.user.clone().into(), request.rp.clone()),
462+
);
463+
464+
let mut authenticator =
465+
Authenticator::new(Aaguid::new_empty(), shared_store.clone(), user_mock);
466+
467+
// This should succeed - an empty store means no credentials to exclude
468+
authenticator
469+
.make_credential(request)
470+
.await
471+
.expect("make_credential should succeed with empty store and exclude_credentials");
472+
473+
// Verify the credential was created
474+
assert_eq!(shared_store.lock().await.len(), 1);
475+
}
476+
477+
#[tokio::test]
478+
async fn empty_exclude_credentials_with_empty_store_succeeds() {
479+
// Edge case: empty exclude list with empty store should also succeed
480+
let request = Request {
481+
exclude_list: Some(vec![]),
482+
..good_request()
483+
};
484+
485+
let shared_store = Arc::new(Mutex::new(MemoryStore::new()));
486+
let user_mock = MockUserValidationMethod::verified_user_with_hint(
487+
1,
488+
MockUiHint::RequestNewCredential(request.user.clone().into(), request.rp.clone()),
489+
);
490+
491+
let mut authenticator =
492+
Authenticator::new(Aaguid::new_empty(), shared_store.clone(), user_mock);
493+
494+
authenticator
495+
.make_credential(request)
496+
.await
497+
.expect("make_credential should succeed with empty exclude list");
498+
499+
assert_eq!(shared_store.lock().await.len(), 1);
500+
}
501+
502+
#[tokio::test]
503+
async fn store_with_credentials_not_in_exclude_list_succeeds() {
504+
// This test verifies that when the store contains credentials,
505+
// but none of them are in the excludeCredentials list,
506+
// credential creation should succeed.
507+
508+
let stored_cred_id: Bytes = random_vec(16).into();
509+
let excluded_cred_id: Bytes = random_vec(16).into();
510+
511+
// Create a passkey that will be stored (with different ID than excluded)
512+
let passkey = Passkey {
513+
key: Default::default(),
514+
rp_id: "future.1password.com".into(),
515+
credential_id: stored_cred_id.clone(),
516+
user_handle: Some(random_vec(16).into()),
517+
counter: None,
518+
extensions: Default::default(),
519+
};
520+
521+
let request = Request {
522+
exclude_list: Some(vec![webauthn::PublicKeyCredentialDescriptor {
523+
ty: webauthn::PublicKeyCredentialType::PublicKey,
524+
id: excluded_cred_id.clone(),
525+
transports: Some(vec![webauthn::AuthenticatorTransport::Usb]),
526+
}]),
527+
..good_request()
528+
};
529+
530+
let shared_store = Arc::new(Mutex::new(MemoryStore::new()));
531+
532+
// Insert the credential into the store
533+
shared_store
534+
.lock()
535+
.await
536+
.insert(stored_cred_id.into(), passkey);
537+
538+
let user_mock = MockUserValidationMethod::verified_user_with_hint(
539+
1,
540+
MockUiHint::RequestNewCredential(request.user.clone().into(), request.rp.clone()),
541+
);
542+
543+
let mut authenticator =
544+
Authenticator::new(Aaguid::new_empty(), shared_store.clone(), user_mock);
545+
546+
// This should succeed - the store contains credentials, but not the excluded one
547+
authenticator
548+
.make_credential(request)
549+
.await
550+
.expect("make_credential should succeed when store has credentials not in exclude list");
551+
552+
// Verify a new credential was created (now 2 credentials in store)
553+
assert_eq!(shared_store.lock().await.len(), 2);
554+
}

0 commit comments

Comments
 (0)