Skip to content

Commit

Permalink
[PM-9498] let user check and pick credential for creation return user…
Browse files Browse the repository at this point in the history
… check (#874)

## 🎟️ Tracking

<!-- Paste the link to the Jira or GitHub issue or otherwise describe /
point to where this change is coming from. -->

## 📔 Objective

`check_user_and_pick_credential_for_creation` needs to return the result
of the `check_user` part of the function

## 📸 Screenshots

<!-- Required for any UI changes; delete if not applicable. Use fixed
width images for better display. -->

## ⏰ Reminders before review

- Contributor guidelines followed
- All formatters and local linters executed and passed
- Written new unit and / or integration tests where applicable
- Protected functional changes with optionality (feature flags)
- Used internationalization (i18n) for all UI strings
- CI builds passed
- Communicated to DevOps any deployment requirements
- Updated any necessary documentation (Confluence, contributing docs) or
informed the documentation
  team

## 🦮 Reviewer guidelines

<!-- Suggested interactions but feel free to use (or not) as you desire!
-->

- 👍 (`:+1:`) or similar for great changes
- 📝 (`:memo:`) or ℹ️ (`:information_source:`) for notes or general info
- ❓ (`:question:`) for questions
- 🤔 (`:thinking:`) or 💭 (`:thought_balloon:`) for more open inquiry
that's not quite a confirmed
  issue and could potentially benefit from discussion
- 🎨 (`:art:`) for suggestions / improvements
- ❌ (`:x:`) or ⚠️ (`:warning:`) for more significant problems or
concerns needing attention
- 🌱 (`:seedling:`) or ♻️ (`:recycle:`) for future improvements or
indications of technical debt
- ⛏ (`:pick:`) for minor or nitpick changes
  • Loading branch information
coroiu authored Jul 4, 2024
1 parent c7120e6 commit f9fb74c
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 15 deletions.
9 changes: 3 additions & 6 deletions crates/bitwarden-fido/src/authenticator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use passkey::{
use thiserror::Error;

use super::{
try_from_credential_new_view, types::*, CheckUserOptions, CheckUserResult, CipherViewContainer,
try_from_credential_new_view, types::*, CheckUserOptions, CipherViewContainer,
Fido2CredentialStore, Fido2UserInterface, SelectedCredential, UnknownEnum, AAGUID,
};
use crate::{
Expand Down Expand Up @@ -613,7 +613,7 @@ impl passkey::authenticator::UserValidationMethod for UserValidationMethodImpl<'
let new_credential = try_from_credential_new_view(user, rp)
.map_err(|_| Ctap2Error::InvalidCredential)?;

let cipher_view = self
let (cipher_view, user_check) = self
.authenticator
.user_interface
.check_user_and_pick_credential_for_creation(options, new_credential)
Expand All @@ -626,10 +626,7 @@ impl passkey::authenticator::UserValidationMethod for UserValidationMethodImpl<'
.expect("Mutex is not poisoned")
.replace(cipher_view);

Ok(CheckUserResult {
user_present: true,
user_verified: verification != UV::Discouraged,
})
Ok(user_check)
}
_ => {
self.authenticator
Expand Down
2 changes: 1 addition & 1 deletion crates/bitwarden-fido/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ pub trait Fido2UserInterface: Send + Sync {
&self,
options: CheckUserOptions,
new_credential: Fido2CredentialNewView,
) -> Result<CipherView, Fido2CallbackError>;
) -> Result<(CipherView, CheckUserResult), Fido2CallbackError>;
async fn is_verification_enabled(&self) -> bool;
}

Expand Down
27 changes: 20 additions & 7 deletions crates/bitwarden-uniffi/src/platform/fido2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,22 @@ pub struct CheckUserResult {
user_verified: bool,
}

impl From<CheckUserResult> for bitwarden::fido::CheckUserResult {
fn from(val: CheckUserResult) -> Self {
Self {
user_present: val.user_present,
user_verified: val.user_verified,
}
}
}

#[allow(dead_code)]
#[derive(uniffi::Record)]
pub struct CheckUserAndPickCredentialForCreationResult {
cipher: CipherViewWrapper,
check_user_result: CheckUserResult,
}

#[derive(Debug, thiserror::Error, uniffi::Error)]
pub enum Fido2CallbackError {
#[error("The operation requires user interaction")]
Expand Down Expand Up @@ -227,7 +243,7 @@ pub trait Fido2UserInterface: Send + Sync {
&self,
options: CheckUserOptions,
new_credential: Fido2CredentialNewView,
) -> Result<CipherViewWrapper, Fido2CallbackError>;
) -> Result<CheckUserAndPickCredentialForCreationResult, Fido2CallbackError>;
async fn is_verification_enabled(&self) -> bool;
}

Expand Down Expand Up @@ -326,10 +342,7 @@ impl bitwarden::fido::Fido2UserInterface for UniffiTraitBridge<&dyn Fido2UserInt
self.0
.check_user(options.clone(), hint.into())
.await
.map(|r| bitwarden::fido::CheckUserResult {
user_present: r.user_present,
user_verified: r.user_verified,
})
.map(Into::into)
.map_err(Into::into)
}
async fn pick_credential_for_authentication(
Expand All @@ -346,11 +359,11 @@ impl bitwarden::fido::Fido2UserInterface for UniffiTraitBridge<&dyn Fido2UserInt
&self,
options: CheckUserOptions,
new_credential: Fido2CredentialNewView,
) -> Result<CipherView, BitFido2CallbackError> {
) -> Result<(CipherView, bitwarden::fido::CheckUserResult), BitFido2CallbackError> {
self.0
.check_user_and_pick_credential_for_creation(options, new_credential)
.await
.map(|v| v.cipher)
.map(|v| (v.cipher.cipher, v.check_user_result.into()))
.map_err(Into::into)
}
async fn is_verification_enabled(&self) -> bool {
Expand Down
2 changes: 1 addition & 1 deletion languages/swift/iOS/App/ContentView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ class Fido2UserInterfaceImpl: Fido2UserInterface {
abort()
}

func checkUserAndPickCredentialForCreation(options: BitwardenSdk.CheckUserOptions, newCredential: BitwardenSdk.Fido2CredentialNewView) async throws -> BitwardenSdk.CipherViewWrapper {
func checkUserAndPickCredentialForCreation(options: BitwardenSdk.CheckUserOptions, newCredential: BitwardenSdk.Fido2CredentialNewView) async throws -> BitwardenSdk.CheckUserAndPickCredentialForCreationResult {
abort()
}

Expand Down

0 comments on commit f9fb74c

Please sign in to comment.