Merge pull request #89 from 1Password/various-cleanup

Various cleanup

Author: Progdrasil

passkey-authenticator/src/authenticator/make_credential/tests.rs

@@ -440,6 +440,7 @@ async fn make_credential_returns_err_when_rk_is_requested_but_not_supported() {
     // Assert
     assert_eq!(err, Ctap2Error::UnsupportedOption.into());
 }
+
 #[tokio::test]
 async fn empty_store_with_exclude_credentials_succeeds() {
     // This test verifies the fix for the issue where an empty credential store

passkey-client/src/lib.rs

@@ -80,6 +80,8 @@ pub enum WebauthnError {
     RedirectError,
     /// Related Origins endpoint contains a number of labels exceeding the max limit
     ExceedsMaxLabelLimit,
+    /// JSON serialization error
+    SerializationError,
 }
 
 impl WebauthnError {
@@ -253,8 +255,8 @@ where
             unknown_keys: Default::default(),
         };
 
-        // SAFETY: it is a developer error if serializing this struct fails.
-        let client_data_json = serde_json::to_string(&collected_client_data).unwrap();
+        let client_data_json = serde_json::to_string(&collected_client_data)
+            .map_err(|_| WebauthnError::SerializationError)?;
         let client_data_json_hash = client_data
             .client_data_hash()
             .unwrap_or_else(|| sha256(client_data_json.as_bytes()).to_vec());
@@ -374,8 +376,8 @@ where
             unknown_keys: Default::default(),
         };
 
-        // SAFETY: it is a developer error if serializing this struct fails.
-        let client_data_json = serde_json::to_string(&collected_client_data).unwrap();
+        let client_data_json = serde_json::to_string(&collected_client_data)
+            .map_err(|_| WebauthnError::SerializationError)?;
         let client_data_json_hash = client_data
             .client_data_hash()
             .unwrap_or_else(|| sha256(client_data_json.as_bytes()).to_vec());

passkey-client/src/quirks.rs

@@ -1,6 +1,6 @@
 //! <https://fidoalliance.org/specs/fido-v2.0-ps-20190130/fido-client-to-authenticator-protocol-v2.0-ps-20190130.html#authenticatorMakeCredential>
 
-use ciborium::{Value, cbor};
+use ciborium::cbor;
 use serde::{Deserialize, Serialize};
 
 use crate::{
@@ -11,8 +11,11 @@ use crate::{
 };
 
 #[cfg(doc)]
-use crate::webauthn::{
-    CollectedClientData, PublicKeyCredentialCreationOptions, PublicKeyCredentialDescriptor,
+use {
+    crate::webauthn::{
+        CollectedClientData, PublicKeyCredentialCreationOptions, PublicKeyCredentialDescriptor,
+    },
+    ciborium::value::Value,
 };
 
 use super::extensions::{AuthenticatorPrfInputs, AuthenticatorPrfMakeOutputs, HmacGetSecretInput};
@@ -280,7 +283,7 @@ serde_workaround! {
         // TODO: Change to a flattened enum when `content, type` serde enums can use numbers as
         // the keys
         #[serde(rename = 0x03)]
-        pub att_stmt: Value,
+        pub att_stmt: ciborium::value::Value,
 
         /// Indicates whether an enterprise attestation was returned for this credential.
         /// If `ep_att` is absent or present and set to false, then an enterprise attestation was not returned.
@@ -323,7 +326,7 @@ impl Response {
                "fmt" => "none",
                 "attStmt" => {},
                 // Explicitly define these fields as bytes since specialization is still fairly far
-               "authData" => Value::Bytes(self.auth_data.to_vec()),
+               "authData" => ciborium::value::Value::Bytes(self.auth_data.to_vec()),
         })
         .unwrap();
         ciborium::ser::into_writer(&attestation_object_value, &mut attestation_object).unwrap();

passkey-types/src/ctap2/make_credential.rs

@@ -143,6 +143,8 @@ impl<'de> Deserialize<'de> for Bytes {
             where
                 E: serde::de::Error,
             {
+                // There have been some whitespace seen in incoming base64 encodings.
+                let v = v.trim();
                 v.try_into().map_err(|_| {
                     E::invalid_value(
                         serde::de::Unexpected::Str(v),

passkey-types/src/utils/bytes.rs

@@ -83,7 +83,6 @@ where
     Ok(de
         .deserialize_seq(IgnoreUnknown(std::marker::PhantomData))
         .unwrap_or_default())
-    // de.deserialize_seq(IgnoreUnknown(std::marker::PhantomData))
 }
 
 pub(crate) fn ignore_unknown_vec<'de, D, T>(de: D) -> Result<Vec<T>, D::Error>
@@ -141,15 +140,12 @@ where
     where
         E: Error,
     {
-        match FromStr::from_str(v) {
-            Ok(v) => Ok(v),
-            _ => {
-                if let Ok(v) = f64::from_str(v) {
-                    self.visit_f64(v)
-                } else {
-                    Err(E::custom("Was not a stringified number"))
-                }
-            }
+        if let Ok(v) = FromStr::from_str(v) {
+            Ok(v)
+        } else if let Ok(v) = f64::from_str(v) {
+            self.visit_f64(v)
+        } else {
+            Err(E::custom("Was not a stringified number"))
         }
     }
 
@@ -279,5 +275,6 @@ where
 {
     de.deserialize_any(StringOrBool)
 }
+
 #[cfg(test)]
 mod tests;