Skip to content

Commit

Permalink
Merge branch 'main' into support-prf-on-make-credential
Browse files Browse the repository at this point in the history
  • Loading branch information
Progdrasil committed Jul 25, 2024
2 parents 105d90f + 101902c commit 72960c5
Show file tree
Hide file tree
Showing 23 changed files with 521 additions and 83 deletions.
23 changes: 18 additions & 5 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,21 @@
- 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

- Changed: The `Client` no longer hardcodes the UV value sent to the `Authenticator` ([#22](https://github.com/1Password/passkey-rs/pull/22)).
- Changed: The `Client` no longer hardcodes the RK value sent to the `Authenticator` ([#27](https://github.com/1Password/passkey-rs/pull/27)).
- The client now supports additional user-defined properties in the client data, while also clarifying how the client
handles client data and its hash.
- ⚠ BREAKING: Changed: `register` and `authenticate` take `ClientData<E>` instead of `Option<Vec<u8>>`.
- ⚠ BREAKING: Changed: Custom client data hashes are now specified using `DefaultClientDataWithCustomHash(Vec<u8>)` instead of
`Some(Vec<u8>)`.
- Added: Additional fields can be added to the client data using `DefaultClientDataWithExtra(ExtraData)`.
- Added: The `Client` now has the ability to adjust the response for quirky relying parties
when a fully featured response would break their server side validation. ([#31](https://github.com/1Password/passkey-rs/pull/31))
- ⚠ BREAKING: Added the `Origin` enum which is now the origin parameter for the following methods ([#32](https://github.com/1Password/passkey-rs/pull/27)):
Expand All @@ -25,12 +35,15 @@
- `RpIdValidator::assert_domain` takes an `&Origin` instead of a `&Url`
- ⚠ BREAKING: The collected client data will now have the android app signature as the origin when a request comes from an app directly. ([#32](https://github.com/1Password/passkey-rs/pull/27))

### passkey-types
## passkey-types

- ⚠ BREAKING: Rename webauthn extension outputs to be consistent with inputs.
- ⚠ BREAKING: Create new extension inputs for the CTAP authenticator inputs.
- ⚠ BREAKING: Add unsigned extension outputs for the CTAP authenticator outputs.
- ⚠ BREAKING: Add ability for `Passkey` to store associated extension data.
- `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))
- ⚠ BREAKING: Add unsigned extension outputs for the CTAP authenticator outputs. ([#34](https://github.com/1Password/passkey-rs/pull/33))
- ⚠ BREAKING: Add ability for `Passkey` to store associated extension data. ([#36](https://github.com/1Password/passkey-rs/pull/36))

## Passkey v0.2.0
### passkey-types v0.2.0
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ let request = CredentialCreationOptions {
};

// Now create the credential.
let my_webauthn_credential: CreatedPublicKeyCredential = my_client.register(origin, request).await?;
let my_webauthn_credential: CreatedPublicKeyCredential = my_client.register(origin, request, DefaultClientData).await?;

```

Expand Down
6 changes: 4 additions & 2 deletions passkey-authenticator/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ workspace = true
[features]
default = []
tokio = ["dep:tokio"]
testable = ["dep:mockall", "passkey-types/mocks"]
testable = ["dep:mockall", "passkey-types/testable"]

[dependencies]
async-trait = "0.1"
Expand All @@ -31,7 +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"] }
passkey-types = { path = "../passkey-types", version = "0.2", features = ["mocks"]}
2 changes: 1 addition & 1 deletion passkey-authenticator/src/authenticator/extensions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ pub use hmac_secret::{HmacSecretConfig, HmacSecretCredentialSupport};
#[cfg(test)]
pub(crate) use hmac_secret::tests::prf_eval_request;

#[cfg(docs)]
#[cfg(doc)]
use passkey_types::webauthn;

use crate::Authenticator;
Expand Down
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 @@ -199,7 +205,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: self.extensions.list_extensions(),
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
78 changes: 74 additions & 4 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 @@ -146,7 +152,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 @@ -161,7 +167,10 @@ mod tests {
use passkey_types::{
ctap2::{
extensions::{AuthenticatorPrfInputs, AuthenticatorPrfValues},
make_credential::{ExtensionInputs, Options, PublicKeyCredentialRpEntity},
make_credential::{
ExtensionInputs, Options, PublicKeyCredentialRpEntity,
PublicKeyCredentialUserEntity,
},
Aaguid,
},
rand::random_vec,
Expand All @@ -171,7 +180,12 @@ mod tests {
use tokio::sync::Mutex;

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

fn good_request() -> Request {
Request {
Expand Down Expand Up @@ -509,4 +523,60 @@ mod tests {
assert!(prf.enabled);
assert!(prf.results.is_none())
}

#[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());
}
}
Loading

0 comments on commit 72960c5

Please sign in to comment.