Skip to content
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.

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

Change where we transform UV #786

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
29 changes: 12 additions & 17 deletions crates/bitwarden/src/platform/fido2/authenticator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

use super::{
crypto::attested_credential_data_into_iter, types::*, CheckUserOptions, CipherViewContainer,
Fido2CredentialStore, Fido2UserInterface, SelectedCredential, Verification, AAGUID,
Fido2CredentialStore, Fido2UserInterface, SelectedCredential, AAGUID,
};
use crate::{
error::{require, Error, Result},
Expand Down Expand Up @@ -42,6 +42,14 @@
.expect("Mutex is not poisoned")
.replace(request.options.uv);

let verification_enabled = self.user_interface.is_verification_enabled().await;
let uv = match (request.options.uv, verification_enabled) {
(UV::Preferred, true) => true,
(UV::Preferred, false) => false,
(UV::Required, _) => true,
(UV::Discouraged, _) => false,

Check warning on line 50 in crates/bitwarden/src/platform/fido2/authenticator.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/platform/fido2/authenticator.rs#L45-L50

Added lines #L45 - L50 were not covered by tests
};

let response = authenticator
.make_credential(ctap2::make_credential::Request {
client_data_hash: request.client_data_hash.into(),
Expand Down Expand Up @@ -70,7 +78,7 @@
options: passkey::types::ctap2::make_credential::Options {
rk: request.options.rk,
up: true,
uv: request.options.uv != UV::Discouraged,
uv,

Check warning on line 81 in crates/bitwarden/src/platform/fido2/authenticator.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/platform/fido2/authenticator.rs#L81

Added line #L81 was not covered by tests
},
pin_auth: None,
pin_protocol: None,
Expand Down Expand Up @@ -398,30 +406,17 @@
// make_credential Should we validate that it matches with what we stored?
_verification: bool,
) -> Result<UserCheck, Ctap2Error> {
let verification_enabled = self
.authenticator
.user_interface
.is_verification_enabled()
.await;

let selected_uv = self
let verification = self

Check warning on line 409 in crates/bitwarden/src/platform/fido2/authenticator.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/platform/fido2/authenticator.rs#L409

Added line #L409 was not covered by tests
.authenticator
.selected_uv
.lock()
.expect("Mutex is not poisoned")
.take()
.ok_or(Ctap2Error::UserVerificationInvalid)?;

let require_verification = match (selected_uv, verification_enabled) {
(UV::Preferred, true) => Verification::Required,
(UV::Preferred, false) => Verification::Discouraged,
(UV::Required, _) => Verification::Required,
(UV::Discouraged, _) => Verification::Discouraged,
};

let options = CheckUserOptions {
require_presence: presence,
require_verification,
require_verification: verification.into(),

Check warning on line 419 in crates/bitwarden/src/platform/fido2/authenticator.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/platform/fido2/authenticator.rs#L419

Added line #L419 was not covered by tests
};

let result = self
Expand Down
12 changes: 11 additions & 1 deletion crates/bitwarden/src/platform/fido2/types.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use serde::Serialize;

use super::{get_enum_from_string_name, SelectedCredential};
use super::{get_enum_from_string_name, SelectedCredential, Verification};

#[cfg_attr(feature = "mobile", derive(uniffi::Record))]
pub struct PublicKeyCredentialRpEntity {
Expand Down Expand Up @@ -101,6 +101,16 @@
Required,
}

impl From<UV> for Verification {
fn from(value: UV) -> Self {
match value {
UV::Discouraged => Verification::Discouraged,
UV::Preferred => Verification::Preferred,
UV::Required => Verification::Required,

Check warning on line 109 in crates/bitwarden/src/platform/fido2/types.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/platform/fido2/types.rs#L105-L109

Added lines #L105 - L109 were not covered by tests
}
}

Check warning on line 111 in crates/bitwarden/src/platform/fido2/types.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/platform/fido2/types.rs#L111

Added line #L111 was not covered by tests
}

#[cfg_attr(feature = "mobile", derive(uniffi::Record))]
pub struct GetAssertionResult {
pub credential_id: Vec<u8>,
Expand Down
Loading