Support OpenID4VP multi-signed request in Rust matcher - #45
Conversation
| pub dcql_query: Option<DcqlQuery>, | ||
| pub offer: Option<JsonValue>, | ||
| pub transaction_data: Vec<String>, | ||
| pub request: String, |
There was a problem hiding this comment.
I'd like to avoid using JsonValue as much as possible. It removes the clarity and guardrails provided by static typing.
We should make request an enum of either a string or a struct with the right fields, and implement a custom DeJson for it, just like how ProtocolRequestData is done.
In addition, the payload field seems to be a mistake. Judging from the test data, it is a subfield of request instead.
10fefde to
090ffde
Compare
| } | ||
|
|
||
| #[derive(Debug, Clone)] | ||
| pub enum OpenId4VpRequestData { |
There was a problem hiding this comment.
This struct is only for signed requests, right? Let's add "signed" to the name so it is clear to readers that unsigned requests have no use for this struct.
There was a problem hiding this comment.
I was referring to it as a struct, but it is actually an enum. Sorry for the mistake. The naming comment still stands.
|
|
||
| #[derive(DeJson, Debug, Clone, Default)] | ||
| #[nserde(default)] | ||
| pub struct OpenId4VpRequestObject { |
There was a problem hiding this comment.
Similarly, if this is only valid in the context multisigned, we should add multisigned to its name. If this struct is valid for both single-signed and multi-signed cases, we should add signed to its name instead.
090ffde to
fe11207
Compare
|
@netheril96 please take a look at this update. I decided not to update any model structure and just parse the request directly into the existing structure, as there's no reason that we need to keep the original payload / signature for now. |
| pr.request.as_str() | ||
| } else { | ||
| return Err("Missing multisigned request data".into()); | ||
| }; |
There was a problem hiding this comment.
This whole blocks let json_str = ... looks a lot like the ones in the beginning of parse_protocol_request_data. Shall we extract them out into a function?
|
|
||
| let parsed: JsonValue = DeJson::deserialize_json(json_str)?; | ||
|
|
||
| let payload = match &parsed { |
There was a problem hiding this comment.
Instead of match &parsed, write match parsed, then you don't need to clone the strings anymore.
2 participants
No description provided.