Addressing review comments

Signed-off-by: Tiago Nascimento <tiago.nascimento@spruceid.com>
spruceid · Aug 1, 2024 · ee052ef · ee052ef
1 parent 2673005
commit ee052ef
Show file tree

Hide file tree

Showing 6 changed files with 44 additions and 57 deletions.
diff --git a/src/authorization.rs b/src/authorization.rs
@@ -97,7 +97,7 @@ mod test {
     use serde_json::json;
 
     use crate::{
-        core::profiles::{w3c, CoreProfilesAuthorizationDetails, FormatAuthorizationDetails},
+        core::profiles::{w3c, CoreProfilesAuthorizationDetails, ValueAuthorizationDetails},
         metadata::CredentialUrl,
     };
 
@@ -218,8 +218,8 @@ mod test {
         let state = CsrfToken::new("state".into());
         let authorization_details = vec![AuthorizationDetail {
             r#type: AuthorizationDetailType::OpenidCredential,
-            addition_profile_fields: CoreProfilesAuthorizationDetails::Format(
-                FormatAuthorizationDetails::JWTVC(w3c::jwt::AuthorizationDetails::new(
+            addition_profile_fields: CoreProfilesAuthorizationDetails::Value(
+                ValueAuthorizationDetails::JWTVC(w3c::jwt::AuthorizationDetails::new(
                     w3c::CredentialDefinition::new(vec![
                         "VerifiableCredential".into(),
                         "UniversityDegreeCredential".into(),

diff --git a/src/core/profiles/mod.rs b/src/core/profiles/mod.rs
@@ -36,16 +36,16 @@ impl CredentialMetadataProfile for CoreProfilesMetadata {
     fn to_request(&self) -> Self::Request {
         match self {
             CoreProfilesMetadata::JWTVC(m) => {
-                Self::Request::Format(FormatRequest::JWTVC(m.to_request()))
+                Self::Request::Value(ValueRequest::JWTVC(m.to_request()))
             }
             CoreProfilesMetadata::JWTLDVC(m) => {
-                Self::Request::Format(FormatRequest::JWTLDVC(m.to_request()))
+                Self::Request::Value(ValueRequest::JWTLDVC(m.to_request()))
             }
             CoreProfilesMetadata::LDVC(m) => {
-                Self::Request::Format(FormatRequest::LDVC(m.to_request()))
+                Self::Request::Value(ValueRequest::LDVC(m.to_request()))
             }
             CoreProfilesMetadata::ISOmDL(m) => {
-                Self::Request::Format(FormatRequest::ISOmDL(m.to_request()))
+                Self::Request::Value(ValueRequest::ISOmDL(m.to_request()))
             }
         }
     }
@@ -95,7 +95,7 @@ impl ReferencedAuthorizationDetails {
 
 #[derive(Clone, Debug, Deserialize, PartialEq, Serialize)]
 #[serde(tag = "format")]
-pub enum FormatAuthorizationDetails {
+pub enum ValueAuthorizationDetails {
     #[serde(rename = "jwt_vc_json")]
     JWTVC(w3c::jwt::AuthorizationDetails),
     #[serde(rename = "jwt_vc_json-ld")]
@@ -109,7 +109,7 @@ pub enum FormatAuthorizationDetails {
 #[derive(Clone, Debug, Deserialize, PartialEq, Serialize)]
 #[serde(untagged)]
 pub enum CoreProfilesAuthorizationDetails {
-    Format(FormatAuthorizationDetails),
+    Value(ValueAuthorizationDetails),
     Referenced(ReferencedAuthorizationDetails),
 }
 impl AuthorizationDetailsProfile for CoreProfilesAuthorizationDetails {}
@@ -144,7 +144,7 @@ impl ReferencedRequest {
 
 #[derive(Clone, Debug, Deserialize, PartialEq, Serialize)]
 #[serde(tag = "format")]
-pub enum FormatRequest {
+pub enum ValueRequest {
     #[serde(rename = "jwt_vc_json")]
     JWTVC(w3c::jwt::Request),
     #[serde(rename = "jwt_vc_json-ld")]
@@ -158,7 +158,7 @@ pub enum FormatRequest {
 #[derive(Clone, Debug, Deserialize, PartialEq, Serialize)]
 #[serde(untagged)]
 pub enum CoreProfilesRequest {
-    Format(FormatRequest),
+    Value(ValueRequest),
     Referenced(ReferencedRequest),
 }
 

diff --git a/src/deny_field.rs b/src/deny_field.rs
@@ -1,5 +1,13 @@
 use serde::Deserializer;
 
+/// When using flattened structs with `serde`, it is not possible
+/// to also use #[serde(deny_unknown_fields)] in the same struct
+/// definition, but it is possible to create a custom deserializer
+/// that just errors to be able to deny a specific field instead.
+/// In this library, this is used mainly to implement parameter
+/// either/or situations, such as when either `format` or
+/// `credential_configuration_id` must be present, but not
+/// both in `AuthorizationDetails`.
 pub(crate) fn deny_field<'de, D>(_deserializer: D) -> Result<(), D::Error>
 where
     D: Deserializer<'de>,

diff --git a/src/metadata.rs b/src/metadata.rs
@@ -23,7 +23,7 @@ use crate::{
     credential_response_encryption::CredentialResponseEncryptionMetadata,
     http_utils::{check_content_type, MIME_TYPE_JSON},
     profiles::CredentialMetadataProfile,
-    proof_of_possession::KeyProofTypeWrapper,
+    proof_of_possession::KeyProofTypesSupported,
     types::ImageUrl,
 };
 
@@ -251,7 +251,7 @@ where
     scope: Option<Scope>,
     cryptographic_binding_methods_supported: Option<Vec<CryptographicBindingMethod>>,
     #[serde_as(as = "Option<KeyValueMap<_>>")]
-    proof_types_supported: Option<Vec<KeyProofTypeWrapper>>,
+    proof_types_supported: Option<Vec<KeyProofTypesSupported>>,
     display: Option<Vec<CredentialMetadataDisplay>>,
     #[serde(bound = "CM: CredentialMetadataProfile")]
     #[serde(flatten)]
@@ -288,7 +288,7 @@ where
         pub self [self] ["credential metadata value"] {
             set_scope -> scope[Option<Scope>],
             set_cryptographic_binding_methods_supported -> cryptographic_binding_methods_supported[Option<Vec<CryptographicBindingMethod>>],
-            set_proof_types_suuported -> proof_types_supported[Option<Vec<KeyProofTypeWrapper>>],
+            set_proof_types_suuported -> proof_types_supported[Option<Vec<KeyProofTypesSupported>>],
             set_display -> display[Option<Vec<CredentialMetadataDisplay>>],
             set_additional_fields -> additional_fields[CM],
         }
@@ -446,6 +446,7 @@ impl AuthorizationMetadata {
             .set_token_endpoint(Some(token_endpoint)),
         )
     }
+
     pub fn discover<HC, RE, CM, JT, JE, JA>(
         issuer_metadata: &IssuerMetadata<CM, JT, JE, JA>,
         http_client: HC,
@@ -459,6 +460,7 @@ impl AuthorizationMetadata {
         JA: JweKeyManagementAlgorithm + Clone,
     {
         let issuer_url = (match &issuer_metadata.authorization_servers {
+            // TODO: respond with the appropriate authorization server
             Some(v) => v.clone().into_iter().next(),
             _ => None,
         })
@@ -487,6 +489,7 @@ impl AuthorizationMetadata {
         JA: JweKeyManagementAlgorithm + Clone,
     {
         let issuer_url = (match &issuer_metadata.authorization_servers {
+            // TODO: respond with the appropriate authorization server
             Some(v) => v.clone().into_iter().next(),
             _ => None,
         })
@@ -572,13 +575,18 @@ impl AuthorizationMetadata {
 #[cfg(test)]
 mod test {
     use crate::core::profiles::CoreProfilesMetadata;
+    use serde_json::json;
 
     use super::*;
 
     #[test]
     fn example_issuer_metadata() {
-        let jd = &mut serde_json::Deserializer::from_str(
-            r##"{
+        let _: IssuerMetadata<
+            CoreProfilesMetadata,
+            CoreJsonWebKeyType,
+            CoreJweContentEncryptionAlgorithm,
+            CoreJweKeyManagementAlgorithm,
+        > = serde_json::from_value(json!({
             "credential_issuer": "https://credential-issuer.example.com",
             "authorization_servers": [ "https://server.example.com" ],
             "credential_endpoint": "https://credential-issuer.example.com",
@@ -666,20 +674,12 @@ mod test {
                     ]
                 }
             }
-        }"##,
-        );
-        let _: IssuerMetadata<
-            CoreProfilesMetadata,
-            CoreJsonWebKeyType,
-            CoreJweContentEncryptionAlgorithm,
-            CoreJweKeyManagementAlgorithm,
-        > = serde_path_to_error::deserialize(jd).unwrap();
+        })).unwrap();
     }
 
     #[test]
     fn example_credential_metadata_jwt() {
-        let jd = &mut serde_json::Deserializer::from_str(
-            r##"{
+        let _: CredentialMetadata<CoreProfilesMetadata> = serde_json::from_value(json!({
             "format": "jwt_vc_json",
             "id": "UniversityDegree_JWT",
             "cryptographic_binding_methods_supported": [
@@ -739,16 +739,13 @@ mod test {
                     "text_color": "#FFFFFF"
                 }
             ]
-        }"##,
-        );
-        let _: CredentialMetadata<CoreProfilesMetadata> =
-            serde_path_to_error::deserialize(jd).unwrap();
+        }))
+        .unwrap();
     }
 
     #[test]
     fn example_credential_metadata_ldp() {
-        let jd = &mut serde_json::Deserializer::from_str(
-            r##"{
+        let _: CredentialMetadata<CoreProfilesMetadata> = serde_json::from_value(json!({
             "format": "ldp_vc",
             "@context": [
                 "https://www.w3.org/2018/credentials/v1",
@@ -812,16 +809,13 @@ mod test {
                     "text_color": "#FFFFFF"
                 }
             ]
-        }"##,
-        );
-        let _: CredentialMetadata<CoreProfilesMetadata> =
-            serde_path_to_error::deserialize(jd).unwrap();
+        }))
+        .unwrap();
     }
 
     #[test]
     fn example_credential_metadata_isomdl() {
-        let jd = &mut serde_json::Deserializer::from_str(
-            r##"{
+        let _: CredentialMetadata<CoreProfilesMetadata> = serde_json::from_value(json!({
             "format": "mso_mdoc",
             "doctype": "org.iso.18013.5.1.mDL",
             "cryptographic_binding_methods_supported": [
@@ -880,9 +874,7 @@ mod test {
                     "organ_donor": {}
                 }
             }
-        }"##,
-        );
-        let _: CredentialMetadata<CoreProfilesMetadata> =
-            serde_path_to_error::deserialize(jd).unwrap();
+        }))
+        .unwrap();
     }
 }
diff --git a/src/proof_of_possession.rs b/src/proof_of_possession.rs
@@ -14,7 +14,7 @@ const JWS_TYPE: &str = "openid4vci-proof+jwt";
 pub type ProofSigningAlgValuesSupported = Vec<ssi_jwk::Algorithm>;
 
 #[derive(Clone, Debug, Deserialize, PartialEq, Serialize)]
-pub struct KeyProofTypeWrapper {
+pub struct KeyProofTypesSupported {
     #[serde(rename = "$key$")]
     key: KeyProofType,
     proof_signing_alg_values_supported: Vec<ssi_jwk::Algorithm>,

diff --git a/src/token.rs b/src/token.rs
@@ -38,7 +38,7 @@ pub enum Request {
 
 #[serde_as]
 #[skip_serializing_none]
-#[derive(Debug, Deserialize, Serialize)]
+#[derive(Debug, Default, Deserialize, Serialize)]
 pub struct ExtraResponseTokenFields<AD>
 where
     AD: AuthorizationDetailsProfile,
@@ -49,19 +49,6 @@ where
     pub authorization_details: Option<Vec<AuthorizationDetail<AD>>>,
 }
 
-impl<AD> Default for ExtraResponseTokenFields<AD>
-where
-    AD: AuthorizationDetailsProfile,
-{
-    fn default() -> Self {
-        Self {
-            c_nonce: None,
-            c_nonce_expires_in: None,
-            authorization_details: None,
-        }
-    }
-}
-
 pub type Response = StandardTokenResponse<
     ExtraResponseTokenFields<CoreProfilesAuthorizationDetails>,
     CoreTokenType,