From 3f9af34458319b728ad01abc226aed4d3e3b93ab Mon Sep 17 00:00:00 2001 From: Jacob Date: Fri, 31 Jan 2025 15:59:41 +0000 Subject: [PATCH] 18013-7: Handle missing requested fields. (#106) --- src/oid4vp/iso_18013_7/mod.rs | 1 + src/oid4vp/iso_18013_7/prepare_response.rs | 20 +++- src/oid4vp/iso_18013_7/requested_values.rs | 106 +++++++++++++-------- 3 files changed, 86 insertions(+), 41 deletions(-) diff --git a/src/oid4vp/iso_18013_7/mod.rs b/src/oid4vp/iso_18013_7/mod.rs index 5404791..126f08a 100644 --- a/src/oid4vp/iso_18013_7/mod.rs +++ b/src/oid4vp/iso_18013_7/mod.rs @@ -198,6 +198,7 @@ impl InProgressRequest180137 { &self.request, credential, approved_fields, + &request_match.missing_fields, field_map, mdoc_generated_nonce.clone(), )?; diff --git a/src/oid4vp/iso_18013_7/prepare_response.rs b/src/oid4vp/iso_18013_7/prepare_response.rs index afff566..2b2147a 100644 --- a/src/oid4vp/iso_18013_7/prepare_response.rs +++ b/src/oid4vp/iso_18013_7/prepare_response.rs @@ -6,6 +6,7 @@ use isomdl::{ cbor, cose::sign1::PreparedCoseSign1, definitions::{ + device_response::DocumentErrorCode, device_signed::{DeviceAuthentication, DeviceNamespaces}, helpers::{ByteStr, NonEmptyMap, NonEmptyVec, Tag24}, session::SessionTranscript as SessionTranscriptTrait, @@ -117,6 +118,7 @@ pub fn prepare_response( request: &AuthorizationRequestObject, credential: &Mdoc, approved_fields: Vec, + missing_fields: &BTreeMap, mut field_map: FieldMap, mdoc_generated_nonce: String, ) -> Result { @@ -203,6 +205,22 @@ pub fn prepare_response( device_auth, }; + let mut errors: BTreeMap> = BTreeMap::new(); + for (namespace, element_identifier) in missing_fields { + if let Some(elems) = errors.get_mut(namespace) { + elems.insert( + element_identifier.clone(), + DocumentErrorCode::DataNotReturned, + ); + } else { + let element_map = NonEmptyMap::new( + element_identifier.clone(), + DocumentErrorCode::DataNotReturned, + ); + errors.insert(namespace.clone(), element_map); + } + } + let document = Document { doc_type: mdoc.mso.doc_type.clone(), issuer_signed: IssuerSigned { @@ -210,7 +228,7 @@ pub fn prepare_response( namespaces: Some(revealed_namespaces), }, device_signed, - errors: None, + errors: NonEmptyMap::maybe_new(errors), }; let documents = NonEmptyVec::new(document); diff --git a/src/oid4vp/iso_18013_7/requested_values.rs b/src/oid4vp/iso_18013_7/requested_values.rs index 23e40b1..8876777 100644 --- a/src/oid4vp/iso_18013_7/requested_values.rs +++ b/src/oid4vp/iso_18013_7/requested_values.rs @@ -20,6 +20,7 @@ pub struct RequestMatch180137 { pub credential_id: Uuid, pub field_map: FieldMap, pub requested_fields: Vec, + pub missing_fields: BTreeMap, } uniffi::custom_newtype!(FieldId180137, String); @@ -76,11 +77,7 @@ where credentials .filter_map( |credential| match find_match(input_descriptor, credential) { - Ok((field_map, requested_fields)) => Some(Arc::new(RequestMatch180137 { - field_map, - requested_fields, - credential_id: credential.id(), - })), + Ok(m) => Some(Arc::new(m)), Err(e) => { tracing::info!("credential did not match: {e}"); None @@ -90,10 +87,7 @@ where .collect() } -fn find_match( - input_descriptor: &InputDescriptor, - credential: &Mdoc, -) -> Result<(FieldMap, Vec)> { +fn find_match(input_descriptor: &InputDescriptor, credential: &Mdoc) -> Result { let mdoc = credential.document(); if mdoc.mso.doc_type != input_descriptor.id { @@ -152,6 +146,7 @@ fn find_match( ); let mut requested_fields = BTreeMap::new(); + let mut missing_fields = BTreeMap::new(); let elements_json_ref = &elements_json; @@ -210,32 +205,50 @@ fn find_match( }, ); } - None if field.is_required() => bail!( - "missing requested field: {}", - field.path.as_ref()[0].to_string() - ), - None => (), + None => { + let json_path = field.path.as_ref()[0].to_string(); + if let Some((namespace, element_identifier)) = split_json_path(&json_path) { + missing_fields.insert(namespace, element_identifier); + } else { + tracing::warn!("invalid JSON path expression: {json_path}") + } + } } } let mut seen_age_over_attestations = 0; + let requested_fields = requested_fields + .into_values() + // According to the rules in ISO/IEC 18013-5 Section 7.2.5, don't respond with more + // than 2 age over attestations. + .filter(|field| { + if field.displayable_name.starts_with("age_over_") { + seen_age_over_attestations += 1; + seen_age_over_attestations < 3 + } else { + true + } + }) + .collect(); - Ok(( + Ok(RequestMatch180137 { + credential_id: credential.id(), field_map, - requested_fields - .into_values() - // According to the rules in ISO/IEC 18013-5 Section 7.2.5, don't respond with more - // than 2 age over attestations. - .filter(|field| { - if field.displayable_name.starts_with("age_over_") { - seen_age_over_attestations += 1; - seen_age_over_attestations < 3 - } else { - true - } - }) - .collect(), - )) + requested_fields, + missing_fields, + }) +} + +fn split_json_path(json_path: &str) -> Option<(String, String)> { + // Find the namespace between "$['" and "']['"". + let (namespace, rest) = json_path.strip_prefix("$['")?.split_once("']['")?; + // Find the element identifier up to "']". + let (element_id, "") = rest.split_once("']")? else { + // Unexpected trailing characters. + return None; + }; + + Some((namespace.to_string(), element_id.to_string())) } fn cbor_to_string(cbor: &Cbor) -> Option { @@ -397,13 +410,13 @@ mod test { use super::{parse_request, reverse_mapping}; #[rstest] - #[case::valid("tests/examples/18013_7_presentation_definition.json", true)] - #[case::invalid( - "tests/examples/18013_7_presentation_definition_age_over_25.json", - false - )] + #[case::valid("tests/examples/18013_7_presentation_definition.json", 0)] + #[case::missing("tests/examples/18013_7_presentation_definition_age_over_25.json", 1)] #[tokio::test] - async fn mdl_matches_presentation_definition(#[case] filepath: &str, #[case] valid: bool) { + async fn mdl_matches_presentation_definition( + #[case] filepath: &str, + #[case] missing_fields: usize, + ) { let key_manager = Arc::new(RustTestKeyManager::default()); let key_alias = KeyAlias("".to_string()); @@ -420,12 +433,11 @@ mod test { let request = parse_request(&presentation_definition, credentials.iter()); - assert_eq!(request.len() == 1, valid); + assert_eq!(request.len(), 1); - if valid { - let request = &request[0]; - assert_eq!(request.requested_fields.len(), 12) - } + let request = &request[0]; + assert_eq!(request.requested_fields.len(), 12 - missing_fields); + assert_eq!(request.missing_fields.len(), missing_fields); } #[test] @@ -444,4 +456,18 @@ mod test { _ => panic!("unexpected value"), }) } + + #[rstest] + #[case::valid("$['namespace']['element_id']", true)] + #[case::invalid("$.namespace.element_id", false)] + #[case::trailing("$['namespace']['element_id']['extra']", false)] + fn json_path_splitting(#[case] path: &str, #[case] is_some: bool) { + let Some((namespace, element_id)) = super::split_json_path(path) else { + assert!(!is_some); + return; + }; + + assert_eq!(namespace, "namespace"); + assert_eq!(element_id, "element_id"); + } }