Add delegation_type support to OpenID4VP presentation matcher - #215
Add delegation_type support to OpenID4VP presentation matcher#215danieljkim1 wants to merge 1 commit into
Conversation
97c6ecf to
bcdb50f
Compare
|
Btw, not ready for review yet. |
|
Ready for review |
QZHelen
left a comment
There was a problem hiding this comment.
Can you rebase and make sure only delegation_type related changes are included in this PR?
66f5973 to
a41a554
Compare
Done! |
| set_index: i32, | ||
| delegation_type: i32, | ||
| ); | ||
| fn set_delegation_type_for_entry_in_set( |
There was a problem hiding this comment.
Do you need to additionally expose this API? Will the update to add_entry_to_set suffice?
There was a problem hiding this comment.
removed set_delegation_type_for_entry_in_set to use add_entry_to_set only
| field_display_value.to_string(), | ||
| )); | ||
| } | ||
| fn add_payment_entry_to_set_v2( |
There was a problem hiding this comment.
Have you made sure that the same support also work for when there's a Payment credential involved?
There was a problem hiding this comment.
Haven't verified for payment credential. Did you want to include support for the payment credential as well?
| set_id_c.as_ptr(), | ||
| set_index, | ||
| ); | ||
| if delegation_type != 0 { |
There was a problem hiding this comment.
Check the wasm version to safely make this call. If the wasm version returns a lower version than when the support is added, invoking this method will cause the the wasm to fail, unable to find the exported wasm function
| id, | ||
| matched: vec![MatchedCredential { | ||
| id, | ||
| delegation_type: 0, |
There was a problem hiding this comment.
0 is used as a magic number in multiple places to represent delegation type none. At minimum, make it a constant, or ideally it should be a typed enum.
There was a problem hiding this comment.
Are there any tests added that test the behavior of non-zero delegation type?
| metadata: &str, | ||
| set_id: &str, | ||
| set_index: i32, | ||
| delegation_type: i32, |
There was a problem hiding this comment.
How are we handling old GMSCore versions where the function signature is different?
ae1ac4d to
df1d78b
Compare
- Define DelegationType enum (None = 0, Full = 1) with DeJson deserialization - Check get_wasm_version() >= 7 before calling SetDelegationTypeForEntryInSet in add_entry_to_set and add_payment_entry_to_set_v2 - Propagate delegation_type through DCQL to reporter for standard and payment entries - Add unit tests for DelegationType in openid4vp_models and reporter
b538ce1 to
8923460
Compare
This PR adds support for provider
delegation_typein the OpenID4VP presentation matcher (matcher-rs).Summary of Changes:
bindings.rs: DeclaredSetDelegationTypeForEntryInSetFFI function imported from the Android Credential Manager host runtime.openid4vp_models.rs: DefinedDelegationTypeenum (None = 0,Full = 1) withnanoserde::DeJsondeserialization (defaulting toNone), and addedpub delegation_type: DelegationTypetoRegistryCredentialandMatchedCredential.credman.rs: Updatedadd_entry_to_setandadd_payment_entry_to_set_v2to acceptdelegation_type: DelegationType, and invokeSetDelegationTypeForEntryInSetgated byif self.get_wasm_version() >= 7 && delegation_type != DelegationType::None.dcql.rs&reporter.rs: Propagatedcandidate.delegation_typethrough DCQL matching toreporter.rswhen populating presentationentrySetitems (both standard verification entries and payment entries) viacredman.add_entry_to_set(...)andcredman.add_payment_entry_to_set_v2(...).DelegationTypedeserialization inopenid4vp_models.rs, standard entry reporting with delegation inreporter.rs, and payment entry reporting with delegation inreporter.rs.TAG=agy
CONV=0b44330c-f824-447d-83ba-c4685a5977a5