Skip to content

Commit

Permalink
Merge branch 'main' into refactor-extension-types
Browse files Browse the repository at this point in the history
  • Loading branch information
Progdrasil committed Jul 25, 2024
2 parents 5cb2eed + 94db2aa commit 6fce95c
Show file tree
Hide file tree
Showing 15 changed files with 243 additions and 41 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@
- Removed: `UserValidationMethod::check_user_verification`
- Added: `UserValidationMethod::check_user`. This function now performs both user presence and user verification checks.
The function now also returns which validations were performed, even if they were not requested.
- Added: Support for discoverable credentials
- ⚠ BREAKING: Added: `CredentialStore::get_info` which returns `StoreInfo` containing `DiscoverabilitySupport`.
- ⚠ BREAKING: Changed: `CredentialStore::save_credential` now also takes `Options`.
- Changed: `Authenticator::make_credentials` now returns an error if a discoverable credential was requested but not supported by the store.

### passkey-client

Expand All @@ -35,6 +39,7 @@ handles client data and its hash.

- `CollectedClientData` is now generic and supports additional strongly typed fields. ([#28](https://github.com/1Password/passkey-rs/pull/28))
- Changed: `CollectedClientData` has changed to `CollectedClientData<E = ()>`
- The `Client` now returns `CredProps::rk` depending on the authenticator's capabilities. ([#29](https://github.com/1Password/passkey-rs/pull/29))
- ⚠ BREAKING: Rename webauthn extension outputs to be consistent with inputs. ([#33](https://github.com/1Password/passkey-rs/pull/33))
- ⚠ BREAKING: Create new extension inputs for the CTAP authenticator inputs. ([#33](https://github.com/1Password/passkey-rs/pull/33))

Expand Down
3 changes: 3 additions & 0 deletions passkey-authenticator/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ tokio = { version = "1", features = ["sync"], optional = true }

[dev-dependencies]
mockall = { version = "0.11" }
passkey-types = { path = "../passkey-types", version = "0.2", features = [
"testable",
] }
tokio = { version = "1", features = ["sync", "macros", "rt"] }
generic-array = { version = "0.14", default-features = false }
signature = { version = "2", features = ["rand_core"] }
8 changes: 7 additions & 1 deletion passkey-authenticator/src/authenticator/get_assertion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,12 @@ where
// Note that because this specification defines normative behaviors for them, all
// authenticators MUST understand the "rk", "up", and "uv" options.

// 4. If the "rk" option is present then:
// 1. Return CTAP2_ERR_UNSUPPORTED_OPTION.
if input.options.rk {
return Err(Ctap2Error::UnsupportedOption.into());
}

// 6. TODO, if the extensions parameter is present, process any extensions that this
// authenticator supports. Authenticator extension outputs generated by the authenticator
// extension processing are returned in the authenticator data.
Expand Down Expand Up @@ -187,7 +193,7 @@ mod tests {
options: Options {
up: true,
uv: true,
rk: true,
rk: false,
},
}
}
Expand Down
9 changes: 6 additions & 3 deletions passkey-authenticator/src/authenticator/get_info.rs
Original file line number Diff line number Diff line change
@@ -1,17 +1,20 @@
use passkey_types::ctap2::get_info::{Options, Response};

use crate::{Authenticator, CredentialStore, UserValidationMethod};
use crate::{
credential_store::DiscoverabilitySupport, Authenticator, CredentialStore, UserValidationMethod,
};

impl<S: CredentialStore, U: UserValidationMethod> Authenticator<S, U> {
/// Using this method, the host can request that the authenticator report a list of all
/// supported protocol versions, supported extensions, AAGUID of the device, and its capabilities.
pub fn get_info(&self) -> Response {
pub async fn get_info(&self) -> Response {
Response {
versions: vec!["FIDO_2_0".into(), "U2F_V2".into()],
extensions: None,
aaguid: *self.aaguid(),
options: Some(Options {
rk: true,
rk: self.store.get_info().await.discoverability
!= DiscoverabilitySupport::OnlyNonDiscoverable,
uv: self.user_validation.is_verification_enabled(),
up: self.user_validation.is_presence_enabled(),
..Default::default()
Expand Down
80 changes: 75 additions & 5 deletions passkey-authenticator/src/authenticator/make_credential.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,13 @@ where
// return CTAP2_ERR_INVALID_OPTION. Ignore any options that are not understood.
// Note that because this specification defines normative behaviors for them, all
// authenticators MUST understand the "rk", "up", and "uv" options.
// NB: this is handled at the very begining of the method
// NOTE: Some of this step is handled at the very begining of the method

// 4. If the "rk" option is present then:
// 1. If the rk option ID is not present in authenticatorGetInfo response, end the operation by returning CTAP2_ERR_UNSUPPORTED_OPTION.
if input.options.rk && !self.get_info().await.options.unwrap_or_default().rk {
return Err(Ctap2Error::UnsupportedOption.into());
}

// 4. TODO, if the extensions parameter is present, process any extensions that this
// authenticator supports. Authenticator extension outputs generated by the authenticator
Expand Down Expand Up @@ -141,7 +147,7 @@ where

// 10
self.store_mut()
.save_credential(passkey, input.user.into(), input.rp)
.save_credential(passkey, input.user.into(), input.rp, input.options)
.await?;

Ok(response)
Expand All @@ -154,16 +160,24 @@ mod tests {

use coset::iana;
use passkey_types::{
ctap2::make_credential::{Options, PublicKeyCredentialRpEntity},
ctap2::Aaguid,
ctap2::{
make_credential::{
Options, PublicKeyCredentialRpEntity, PublicKeyCredentialUserEntity,
},
Aaguid,
},
rand::random_vec,
webauthn, Bytes,
};

use tokio::sync::Mutex;

use super::*;
use crate::{user_validation::MockUserValidationMethod, MemoryStore};
use crate::{
credential_store::{DiscoverabilitySupport, StoreInfo},
user_validation::MockUserValidationMethod,
MemoryStore,
};

fn good_request() -> Request {
Request {
Expand Down Expand Up @@ -288,4 +302,60 @@ mod tests {
let store = shared_store.lock().await;
assert_eq!(store.as_ref().and_then(|c| c.counter).unwrap(), 0);
}

#[tokio::test]
async fn make_credential_returns_err_when_rk_is_requested_but_not_supported() {
struct StoreWithoutDiscoverableSupport;
#[async_trait::async_trait]
impl CredentialStore for StoreWithoutDiscoverableSupport {
type PasskeyItem = Passkey;

async fn find_credentials(
&self,
_id: Option<&[webauthn::PublicKeyCredentialDescriptor]>,
_rp_id: &str,
) -> Result<Vec<Self::PasskeyItem>, StatusCode> {
#![allow(clippy::unimplemented)]
unimplemented!("The test should not call find_credentials")
}

async fn save_credential(
&mut self,
_cred: Passkey,
_user: PublicKeyCredentialUserEntity,
_rp: PublicKeyCredentialRpEntity,
_options: Options,
) -> Result<(), StatusCode> {
#![allow(clippy::unimplemented)]
unimplemented!("The test should not call save_credential")
}

async fn update_credential(&mut self, _cred: Passkey) -> Result<(), StatusCode> {
#![allow(clippy::unimplemented)]
unimplemented!("The test should not call update_credential")
}

async fn get_info(&self) -> StoreInfo {
StoreInfo {
discoverability: DiscoverabilitySupport::OnlyNonDiscoverable,
}
}
}

// Arrange
let store = StoreWithoutDiscoverableSupport;
let user_mock = MockUserValidationMethod::verified_user(1);
let request = good_request();
let mut authenticator = Authenticator::new(Aaguid::new_empty(), store, user_mock);
authenticator.set_make_credentials_with_signature_counter(true);

// Act
let err = authenticator
.make_credential(request)
.await
.expect_err("Succeeded with unsupported rk");

// Assert
assert_eq!(err, Ctap2Error::UnsupportedOption.into());
}
}
85 changes: 79 additions & 6 deletions passkey-authenticator/src/credential_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,36 @@ use std::sync::Arc;

use passkey_types::{
ctap2::{
make_credential::PublicKeyCredentialRpEntity,
make_credential::PublicKeyCredentialUserEntity, Ctap2Error, StatusCode,
get_assertion::Options,
make_credential::{PublicKeyCredentialRpEntity, PublicKeyCredentialUserEntity},
Ctap2Error, StatusCode,
},
webauthn::PublicKeyCredentialDescriptor,
Passkey,
};

/// A struct that defines the capabilities of a store.
pub struct StoreInfo {
/// How the store handles discoverability.
pub discoverability: DiscoverabilitySupport,
}

/// Enum to define how the store handles discoverability.
/// Note that this is does not say anything about which storage mode will be used.
#[derive(PartialEq)]
pub enum DiscoverabilitySupport {
/// The store supports both discoverable and non-credentials.
Full,

/// The store only supports non-discoverable credentials.
/// An error will be returned if a discoverable credential is requested.
OnlyNonDiscoverable,

/// The store only supports discoverable credential.
/// No error will be returned if a non-discoverable credential is requested.
ForcedDiscoverable,
}

/// Use this on a type that enables storage and fetching of credentials
#[async_trait::async_trait]
pub trait CredentialStore {
Expand All @@ -32,10 +55,14 @@ pub trait CredentialStore {
cred: Passkey,
user: PublicKeyCredentialUserEntity,
rp: PublicKeyCredentialRpEntity,
options: Options,
) -> Result<(), StatusCode>;

/// Update the credential in your store
async fn update_credential(&mut self, cred: Passkey) -> Result<(), StatusCode>;

/// Get information about the store
async fn get_info(&self) -> StoreInfo;
}

/// In-memory store for Passkeys
Expand Down Expand Up @@ -70,6 +97,7 @@ impl CredentialStore for MemoryStore {
cred: Passkey,
_user: PublicKeyCredentialUserEntity,
_rp: PublicKeyCredentialRpEntity,
_options: Options,
) -> Result<(), StatusCode> {
self.insert(cred.credential_id.clone().into(), cred);
Ok(())
Expand All @@ -79,6 +107,12 @@ impl CredentialStore for MemoryStore {
self.insert(cred.credential_id.clone().into(), cred);
Ok(())
}

async fn get_info(&self) -> StoreInfo {
StoreInfo {
discoverability: DiscoverabilitySupport::ForcedDiscoverable,
}
}
}

#[async_trait::async_trait]
Expand Down Expand Up @@ -107,6 +141,7 @@ impl CredentialStore for Option<Passkey> {
cred: Passkey,
_user: PublicKeyCredentialUserEntity,
_rp: PublicKeyCredentialRpEntity,
_options: Options,
) -> Result<(), StatusCode> {
self.replace(cred);
Ok(())
Expand All @@ -116,6 +151,12 @@ impl CredentialStore for Option<Passkey> {
self.replace(cred);
Ok(())
}

async fn get_info(&self) -> StoreInfo {
StoreInfo {
discoverability: DiscoverabilitySupport::ForcedDiscoverable,
}
}
}

#[cfg(any(feature = "tokio", test))]
Expand All @@ -138,13 +179,21 @@ impl<S: CredentialStore<PasskeyItem = Passkey> + Send + Sync> CredentialStore
cred: Passkey,
user: PublicKeyCredentialUserEntity,
rp: PublicKeyCredentialRpEntity,
options: Options,
) -> Result<(), StatusCode> {
self.lock().await.save_credential(cred, user, rp).await
self.lock()
.await
.save_credential(cred, user, rp, options)
.await
}

async fn update_credential(&mut self, cred: Passkey) -> Result<(), StatusCode> {
self.lock().await.update_credential(cred).await
}

async fn get_info(&self) -> StoreInfo {
self.lock().await.get_info().await
}
}

#[cfg(any(feature = "tokio", test))]
Expand All @@ -167,13 +216,21 @@ impl<S: CredentialStore<PasskeyItem = Passkey> + Send + Sync> CredentialStore
cred: Passkey,
user: PublicKeyCredentialUserEntity,
rp: PublicKeyCredentialRpEntity,
options: Options,
) -> Result<(), StatusCode> {
self.write().await.save_credential(cred, user, rp).await
self.write()
.await
.save_credential(cred, user, rp, options)
.await
}

async fn update_credential(&mut self, cred: Passkey) -> Result<(), StatusCode> {
self.write().await.update_credential(cred).await
}

async fn get_info(&self) -> StoreInfo {
self.read().await.get_info().await
}
}

#[cfg(any(feature = "tokio", test))]
Expand All @@ -196,13 +253,21 @@ impl<S: CredentialStore<PasskeyItem = Passkey> + Send + Sync> CredentialStore
cred: Passkey,
user: PublicKeyCredentialUserEntity,
rp: PublicKeyCredentialRpEntity,
options: Options,
) -> Result<(), StatusCode> {
self.lock().await.save_credential(cred, user, rp).await
self.lock()
.await
.save_credential(cred, user, rp, options)
.await
}

async fn update_credential(&mut self, cred: Passkey) -> Result<(), StatusCode> {
self.lock().await.update_credential(cred).await
}

async fn get_info(&self) -> StoreInfo {
self.lock().await.get_info().await
}
}

#[cfg(any(feature = "tokio", test))]
Expand All @@ -225,11 +290,19 @@ impl<S: CredentialStore<PasskeyItem = Passkey> + Send + Sync> CredentialStore
cred: Passkey,
user: PublicKeyCredentialUserEntity,
rp: PublicKeyCredentialRpEntity,
options: Options,
) -> Result<(), StatusCode> {
self.write().await.save_credential(cred, user, rp).await
self.write()
.await
.save_credential(cred, user, rp, options)
.await
}

async fn update_credential(&mut self, cred: Passkey) -> Result<(), StatusCode> {
self.write().await.update_credential(cred).await
}

async fn get_info(&self) -> StoreInfo {
self.read().await.get_info().await
}
}
2 changes: 1 addition & 1 deletion passkey-authenticator/src/ctap2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ where
U: UserValidationMethod + Sync + Send,
{
async fn get_info(&self) -> get_info::Response {
self.get_info()
self.get_info().await
}

async fn make_credential(
Expand Down
2 changes: 1 addition & 1 deletion passkey-authenticator/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ use passkey_types::{ctap2::Ctap2Error, Bytes};

pub use self::{
authenticator::Authenticator,
credential_store::{CredentialStore, MemoryStore},
credential_store::{CredentialStore, DiscoverabilitySupport, MemoryStore, StoreInfo},
ctap2::Ctap2Api,
u2f::U2fApi,
user_validation::{UserCheck, UserValidationMethod},
Expand Down
Loading

0 comments on commit 6fce95c

Please sign in to comment.