Skip to content

Commit

Permalink
Merge pull request #22 from bitwarden/PM-8752-CONTRIB-PM-7145-stop-ha…
Browse files Browse the repository at this point in the history
…rdcoding-uv-values-sent-to-authenticator

Stop hardcoding uv values sent to authenticator
  • Loading branch information
Progdrasil authored Jun 11, 2024
2 parents 1ae384f + 64afac7 commit 8e5467b
Show file tree
Hide file tree
Showing 2 changed files with 92 additions and 3 deletions.
12 changes: 9 additions & 3 deletions passkey-client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,11 @@ use coset::{iana::EnumI64, Algorithm};
use passkey_authenticator::{Authenticator, CredentialStore, UserValidationMethod};
use passkey_types::{
crypto::sha256,
ctap2, encoding, webauthn,
webauthn::{AuthenticatorExtensionsClientOutputs, CredentialPropertiesOutput},
ctap2, encoding,
webauthn::{
self, AuthenticatorExtensionsClientOutputs, CredentialPropertiesOutput,
UserVerificationRequirement,
},
Passkey,
};
use typeshare::typeshare;
Expand Down Expand Up @@ -209,6 +212,9 @@ where
None
};

let uv = request.authenticator_selection.map(|s| s.user_verification)
!= Some(UserVerificationRequirement::Discouraged);

let ctap2_response = self
.authenticator
.make_credential(ctap2::make_credential::Request {
Expand All @@ -224,7 +230,7 @@ where
options: ctap2::make_credential::Options {
rk: true,
up: true,
uv: true,
uv,
},
pin_auth: None,
pin_protocol: None,
Expand Down
83 changes: 83 additions & 0 deletions passkey-client/src/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -287,3 +287,86 @@ fn validate_domain_with_private_list_provider() -> Result<(), ParseError> {

Ok(())
}

fn user_mock_with_uv() -> MockUserValidationMethod {
let mut user_mock = MockUserValidationMethod::new();
user_mock
.expect_is_verification_enabled()
.returning(|| Some(true));
user_mock
.expect_check_user_verification()
.returning(|| Box::pin(async { true }));
// Always called by `get_info`
user_mock
.expect_is_verification_enabled()
.returning(|| Some(true));
user_mock.expect_is_presence_enabled().returning(|| true);
user_mock
}

fn user_mock_without_uv() -> MockUserValidationMethod {
let mut user_mock = MockUserValidationMethod::new();
user_mock
.expect_check_user_presence()
.returning(|| Box::pin(async { true }));
// Always called by `get_info`
user_mock
.expect_is_verification_enabled()
.returning(|| Some(true));
user_mock.expect_is_presence_enabled().returning(|| true);
user_mock
}

#[tokio::test]
async fn client_register_triggers_uv_when_uv_is_required() {
// Arrange
let auth = Authenticator::new(
ctap2::Aaguid::new_empty(),
MemoryStore::new(),
user_mock_with_uv(),
);
let mut client = Client::new(auth);
let origin = Url::parse("https://future.1password.com").unwrap();
let mut options = webauthn::CredentialCreationOptions {
public_key: good_credential_creation_options(),
};
options.public_key.authenticator_selection = Some(webauthn::AuthenticatorSelectionCriteria {
user_verification: UserVerificationRequirement::Required,
authenticator_attachment: Default::default(),
resident_key: Default::default(),
require_resident_key: Default::default(),
});

// Act & Assert
client
.register(&origin, options, None)
.await
.expect("failed to register with options");
}

#[tokio::test]
async fn client_register_does_not_trigger_uv_when_uv_is_discouraged() {
// Arrange
let auth = Authenticator::new(
ctap2::Aaguid::new_empty(),
MemoryStore::new(),
user_mock_without_uv(),
);
let mut client = Client::new(auth);
let origin = Url::parse("https://future.1password.com").unwrap();
let mut options = webauthn::CredentialCreationOptions {
public_key: good_credential_creation_options(),
};
options.public_key.authenticator_selection = Some(webauthn::AuthenticatorSelectionCriteria {
user_verification: UserVerificationRequirement::Discouraged,
authenticator_attachment: Default::default(),
resident_key: Default::default(),
require_resident_key: Default::default(),
});

// Act & Assert
client
.register(&origin, options, None)
.await
.expect("failed to register with options");
}

0 comments on commit 8e5467b

Please sign in to comment.