Skip to content

Commit

Permalink
Updates to rust and clippy lints
Browse files Browse the repository at this point in the history
  • Loading branch information
Progdrasil committed Dec 9, 2024
1 parent 90c1c28 commit e56f6a6
Show file tree
Hide file tree
Showing 21 changed files with 94 additions and 88 deletions.
10 changes: 0 additions & 10 deletions passkey-authenticator/src/authenticator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,16 +118,6 @@ where
}
}

/// Set the authenticator's display name which will be returned if [`webauthn::CredentialPropertiesOutput`] is requested.
pub fn set_display_name(&mut self, name: String) {
self.extensions.display_name.replace(name);
}

/// Get a reference to the authenticators display name to return in [`webauthn::CredentialPropertiesOutput`].
pub fn display_name(&self) -> Option<&String> {
self.extensions.display_name.as_ref()
}

/// Set whether the authenticator should save new credentials with a signature counter.
///
/// NOTE: Using a counter with a credential that will sync is not recommended and can cause friction
Expand Down
3 changes: 0 additions & 3 deletions passkey-authenticator/src/authenticator/extensions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,6 @@ use crate::Authenticator;
#[derive(Debug, Default)]
#[non_exhaustive]
pub(super) struct Extensions {
/// The display name given when a [`webauthn::CredentialPropertiesOutput`] is requested
pub display_name: Option<String>,

/// Extension to retrieve a symmetric secret from the authenticator.
pub hmac_secret: Option<HmacSecretConfig>,
}
Expand Down
2 changes: 0 additions & 2 deletions passkey-authenticator/src/authenticator/get_assertion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use passkey_types::{
AuthenticatorData, Ctap2Error, Flags, StatusCode,
},
webauthn::PublicKeyCredentialUserEntity,
Passkey,
};

use crate::{private_key_from_cose_key, Authenticator, CredentialStore, UserValidationMethod};
Expand All @@ -14,7 +13,6 @@ impl<S: CredentialStore + Sync, U> Authenticator<S, U>
where
S: CredentialStore + Sync,
U: UserValidationMethod<PasskeyItem = <S as CredentialStore>::PasskeyItem> + Sync,
Passkey: TryFrom<<S as CredentialStore>::PasskeyItem> + Clone,
{
/// This method is used by a host to request cryptographic proof of user authentication as well
/// as user consent to a given transaction, using a previously generated credential that is
Expand Down
1 change: 0 additions & 1 deletion passkey-authenticator/src/user_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ pub struct UserCheck {

/// Pluggable trait for the [`Authenticator`] to do user interaction and verification.
#[cfg_attr(any(test, feature = "testable"), mockall::automock(type PasskeyItem = Passkey;))]
#[cfg_attr(any(test, feature = "testable"), allow(clippy::unused_async))] // Generated by the `mockall` macro.
#[async_trait::async_trait]
pub trait UserValidationMethod {
/// The type of the passkey item that can be used to display additional information about the operation to the user.
Expand Down
49 changes: 37 additions & 12 deletions passkey-client/src/android.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use nom::{
use std::{borrow::Cow, fmt::Debug, str::from_utf8};
use url::Url;

#[derive(Debug)]
#[derive(Debug, Clone)]
/// An Unverified asset link.
pub struct UnverifiedAssetLink<'a> {
/// Application package name.
Expand All @@ -27,19 +27,21 @@ impl<'a> UnverifiedAssetLink<'a> {
package_name: impl Into<Cow<'a, str>>,
sha256_cert_fingerprint: &str,
host: impl Into<Cow<'a, str>>,
asset_link_url: Option<Url>,
asset_link_url: Url,
) -> Result<Self, ValidationError> {
// Is this correct?
// It looks like you can set your own url path.
// https://developers.google.com/digital-asset-links/v1/statements#scaling-to-dozens-of-statements-or-more
if !valid_asset_link_url(&asset_link_url) {
return Err(ValidationError::InvalidAssetLinkUrl);
}
let host = host.into();
let url = match asset_link_url {
Some(u) => u,
None => Url::parse(&format!("https://{host}/.well-known/assetlinks.json",))
.map_err(|e| ValidationError::InvalidAssetLinkUrl(e.to_string()))?,
};

valid_fingerprint(sha256_cert_fingerprint).map(|sha256_cert_fingerprint| Self {
package_name: package_name.into(),
sha256_cert_fingerprint,
host,
asset_link_url: url,
asset_link_url,
})
}

Expand Down Expand Up @@ -71,8 +73,8 @@ pub enum ValidationError {
ParseFailed(String),
/// The fingerprint had an invalid length.
InvalidLength,
/// The asset link url could not be parsed.
InvalidAssetLinkUrl(String),
/// The asset link url is not secure or incorrect path.
InvalidAssetLinkUrl,
}

impl<T> From<nom::Err<nom::error::Error<T>>> for ValidationError {
Expand Down Expand Up @@ -120,10 +122,15 @@ pub fn valid_fingerprint(fingerprint: &str) -> Result<Vec<u8>, ValidationError>
.ok_or(ValidationError::InvalidLength)
}

/// Check for secure and expected path.
fn valid_asset_link_url(url: &Url) -> bool {
url.scheme() == "https" && url.path() == "/.well-known/assetlinks.json"
}

#[cfg(test)]
mod test {
use super::valid_fingerprint;
use crate::ValidationError;
use super::{valid_asset_link_url, valid_fingerprint, ValidationError};
use url::Url;

#[test]
fn check_valid_fingerprint() {
Expand Down Expand Up @@ -154,4 +161,22 @@ mod test {
"Should be valid fingerprint"
);
}

#[test]
fn asset_link_url_ok() {
let url = Url::parse("https://www.facebook.com/.well-known/assetlinks.json").unwrap();
assert!(valid_asset_link_url(&url));
}

#[test]
fn asset_link_url_not_secure() {
let url = Url::parse("http://www.facebook.com/.well-known/assetlinks.json").unwrap();
assert!(!valid_asset_link_url(&url));
}

#[test]
fn asset_link_url_unexpected_path() {
let url = Url::parse("https://www.facebook.com/assetlinks.json").unwrap();
assert!(!valid_asset_link_url(&url));
}
}
3 changes: 0 additions & 3 deletions passkey-client/src/extensions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ use passkey_types::{
AuthenticationExtensionsClientInputs, AuthenticationExtensionsClientOutputs,
CredentialPropertiesOutput, PublicKeyCredentialRequestOptions,
},
Passkey,
};

use crate::{Client, WebauthnError};
Expand All @@ -29,7 +28,6 @@ where
S: CredentialStore + Sync,
U: UserValidationMethod + Sync,
P: public_suffix::EffectiveTLDProvider + Sync + 'static,
Passkey: TryFrom<<S as CredentialStore>::PasskeyItem>,
{
/// Create the extension inputs to be passed to an authenticator over CTAP2
/// during a registration request.
Expand All @@ -55,7 +53,6 @@ where

Some(CredentialPropertiesOutput {
discoverable: Some(discoverable),
authenticator_display_name: self.authenticator.display_name().cloned(),
})
} else {
None
Expand Down
59 changes: 41 additions & 18 deletions passkey-client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
mod client_data;
pub use client_data::*;

use std::{borrow::Cow, fmt::Display};
use std::{borrow::Cow, fmt::Display, ops::ControlFlow};

use ciborium::{cbor, value::Value};
use coset::{iana::EnumI64, Algorithm};
Expand All @@ -35,8 +35,6 @@ use typeshare::typeshare;
use url::Url;

mod extensions;
mod quirks;
use quirks::QuirkyRp;

#[cfg(feature = "android-asset-validation")]
mod android;
Expand Down Expand Up @@ -160,7 +158,6 @@ where
S: CredentialStore + Sync,
U: UserValidationMethod + Sync,
P: public_suffix::EffectiveTLDProvider + Sync + 'static,
Passkey: TryFrom<<S as CredentialStore>::PasskeyItem>,
{
authenticator: Authenticator<S, U>,
rp_id_verifier: RpIdVerifier<P>,
Expand All @@ -187,7 +184,6 @@ where
S: CredentialStore + Sync,
U: UserValidationMethod<PasskeyItem = <S as CredentialStore>::PasskeyItem> + Sync,
P: public_suffix::EffectiveTLDProvider + Sync + 'static,
Passkey: TryFrom<<S as CredentialStore>::PasskeyItem>,
{
/// Create a `Client` with a given `Authenticator` and a custom TLD provider
/// that implements `[public_suffix::EffectiveTLDProvider]`.
Expand Down Expand Up @@ -353,9 +349,7 @@ where
client_extension_results,
};

// Sanitize output before sending it back to the RP
let maybe_quirky_rp = QuirkyRp::from_rp_id(rp_id);
Ok(maybe_quirky_rp.map_create_credential(response))
Ok(response)
}

/// Authenticate a Webauthn request.
Expand Down Expand Up @@ -549,30 +543,59 @@ where
effective_domain = rp_id;
}

// guard against localhost effective domain, return early
if effective_domain == "localhost" {
return if self.allows_insecure_localhost {
Ok(effective_domain)
} else {
Err(WebauthnError::InsecureLocalhostNotAllowed)
};
// Guard against local host and assert rp_id is not part of the public suffix list
if let ControlFlow::Break(res) = self.assert_valid_rp_id(effective_domain) {
return res;
}

// Make sure origin uses https://
if !(origin.scheme().eq_ignore_ascii_case("https")) {
return Err(WebauthnError::UnprotectedOrigin);
}

Ok(effective_domain)
}

fn assert_valid_rp_id<'a>(
&self,
rp_id: &'a str,
) -> ControlFlow<Result<&'a str, WebauthnError>, ()> {
// guard against localhost effective domain, return early
if rp_id == "localhost" {
return if self.allows_insecure_localhost {
ControlFlow::Break(Ok(rp_id))
} else {
ControlFlow::Break(Err(WebauthnError::InsecureLocalhostNotAllowed))
};
}

// assert rp_id is not part of the public suffix list and is a registerable domain.
if decode_host(effective_domain)
if decode_host(rp_id)
.as_ref()
.and_then(|s| self.tld_provider.effective_tld_plus_one(s).ok())
.is_none()
{
return Err(WebauthnError::InvalidRpId);
return ControlFlow::Break(Err(WebauthnError::InvalidRpId));
}

Ok(effective_domain)
ControlFlow::Continue(())
}

/// Parse a given Relying Party ID and assert that it is valid to act as such.
///
/// This method is only to assert that an RP ID passes the required checks.
/// In order to ensure that a request's origin is in accordance with it's claimed RP ID,
/// [`Self::assert_domain`] should be used.
///
/// There are several checks that an RP ID must pass:
/// 1. An RP ID set to `localhost` is only allowed when explicitly enabled with [`Self::allows_insecure_localhost`].
/// 1. An RP ID must not be part of the [public suffix list],
/// since that would allow it to act as a credential for unrelated services by other entities.
pub fn is_valid_rp_id(&self, rp_id: &str) -> bool {
match self.assert_valid_rp_id(rp_id) {
ControlFlow::Continue(_) | ControlFlow::Break(Ok(_)) => true,
ControlFlow::Break(Err(_)) => false,
}
}

#[cfg(feature = "android-asset-validation")]
Expand Down
2 changes: 1 addition & 1 deletion passkey-types/src/ctap2/aaguid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ impl<'de> Deserialize<'de> for Aaguid {
D: serde::Deserializer<'de>,
{
struct AaguidVisitior;
impl<'de> serde::de::Visitor<'de> for AaguidVisitior {
impl serde::de::Visitor<'_> for AaguidVisitior {
type Value = Aaguid;

fn expecting(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
Expand Down
2 changes: 1 addition & 1 deletion passkey-types/src/ctap2/attestation_fmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ impl<'de> Deserialize<'de> for AuthenticatorData {
D: serde::Deserializer<'de>,
{
struct Visitor;
impl<'v> serde::de::Visitor<'v> for Visitor {
impl serde::de::Visitor<'_> for Visitor {
type Value = AuthenticatorData;

fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
Expand Down
2 changes: 1 addition & 1 deletion passkey-types/src/ctap2/get_assertion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ pub use crate::ctap2::make_credential::Options;
#[cfg(doc)]
use {
crate::webauthn::{CollectedClientData, PublicKeyCredentialRequestOptions},
ciborium::Value,
ciborium::value::Value,
};

use super::extensions::{AuthenticatorPrfGetOutputs, AuthenticatorPrfInputs, HmacGetSecretInput};
Expand Down
2 changes: 1 addition & 1 deletion passkey-types/src/ctap2/get_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ impl Default for Options {
}

/// CTAP versions supported
#[allow(non_camel_case_types)]
#[expect(non_camel_case_types)]
#[derive(Debug, Serialize, Deserialize, PartialEq, Eq)]
pub enum Version {
/// Universal 2nd Factor version 1.2
Expand Down
2 changes: 1 addition & 1 deletion passkey-types/src/u2f.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ pub enum ResponseStatusWords {
}

impl From<ResponseStatusWords> for u16 {
#[allow(clippy::as_conversions)]
#[expect(clippy::as_conversions)]
fn from(sw: ResponseStatusWords) -> Self {
sw as u16
}
Expand Down
4 changes: 2 additions & 2 deletions passkey-types/src/u2f/authenticate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ pub enum AuthenticationParameter {
}

impl From<AuthenticationParameter> for u8 {
#[allow(clippy::as_conversions)]
#[expect(clippy::as_conversions)]
fn from(src: AuthenticationParameter) -> Self {
src as u8
}
Expand Down Expand Up @@ -72,7 +72,7 @@ pub struct AuthenticationRequest {
impl AuthenticationRequest {
/// Try parsing a data payload into an authentication request with the given parameter taken from
/// the u2f message frame.
#[allow(clippy::as_conversions)]
#[expect(clippy::as_conversions)]
pub fn try_from(
data: &[u8],
parameter: impl Into<AuthenticationParameter>,
Expand Down
2 changes: 1 addition & 1 deletion passkey-types/src/u2f/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ const REQUEST_HEADER_LEN: usize = 6;
impl TryFrom<&[u8]> for Request {
type Error = ResponseStatusWords;

#[allow(clippy::as_conversions)]
#[expect(clippy::as_conversions)]
fn try_from(value: &[u8]) -> Result<Self, Self::Error> {
if value.len() < REQUEST_HEADER_LEN {
return Err(ResponseStatusWords::WrongLength);
Expand Down
2 changes: 1 addition & 1 deletion passkey-types/src/u2f/register.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ pub struct PublicKey {

impl RegisterResponse {
/// Encode the Response to it's binary format for a successfull response
#[allow(clippy::as_conversions)]
#[expect(clippy::as_conversions)]
pub fn encode(self) -> Vec<u8> {
[0x05] // Reserved magic byte
.into_iter()
Expand Down
3 changes: 2 additions & 1 deletion passkey-types/src/utils/repr_enum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ impl<I: Display> Display for CodeOutOfRange<I> {
/// Generate an enum with associated values, plus conversion methods
macro_rules! repr_enum {
( $(#[$attr:meta])* $enum_name:ident: $repr:ident {$($(#[$fattr:meta])* $name:ident: $val:expr,)* } ) => {
#[allow(clippy::allow_attributes, reason = "the macro doesn't always play nicely with expect()")]
#[allow(non_camel_case_types)]
$(#[$attr])*
#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Copy)]
Expand All @@ -33,7 +34,7 @@ macro_rules! repr_enum {
}
}
impl From<$enum_name> for $repr {
#[allow(clippy::as_conversions)]
#[expect(clippy::as_conversions)]
fn from(src: $enum_name) -> Self {
src as $repr
}
Expand Down
4 changes: 2 additions & 2 deletions passkey-types/src/utils/serde.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ pub mod i64_to_iana {

struct StringOrNum<T>(pub std::marker::PhantomData<T>);

impl<'de, T> Visitor<'de> for StringOrNum<T>
impl<T> Visitor<'_> for StringOrNum<T>
where
T: FromStr + TryFrom<i64> + TryFrom<u64>,
{
Expand Down Expand Up @@ -220,7 +220,7 @@ where
where
E: Error,
{
#[allow(clippy::as_conversions)]
#[expect(clippy::as_conversions)]
// Ensure the float has an integer representation,
// or be 0 if it is a non-integer number
self.visit_i64(if v.is_normal() { v as i64 } else { 0 })
Expand Down
Loading

0 comments on commit e56f6a6

Please sign in to comment.