Repository navigation
Conversation
|
📄 Editor's Copy preview Preview of 479d69a; updated on every push. For review only — the official specifications are published at https://openid.net/specs/ |
There was a problem hiding this comment.
We need to update the definition of 'allow_multiple' - it currently says "Allow returning multiple credentials of the same type" and my presumption is with this change you end up allowing any of the types in the array. (This falls out of it being semantically equivalent to the longer form non-array form of repeating the query.)
Note this will clash with #57 - we could use stacked PRs here: https://docs.github.com/en/pull-requests/how-tos/stacked-pull-requests (i.e. built on top of my branch, and assuming mine is merged first GitHub should auto retarget yours to main when mine is merged). The a query matches credentials whose docType or vct "is identical to the requested value" in mine will need to be updated too.
| | `encryption_ref` | `7` | `int` | O | Reference to an entry in `additional_encryption_contexts`; absent means the main `encryption_context` is used | | ||
|
|
||
| Listing multiple values in `credential_type` is a size optimization for requesting the same data elements from credential types that are closely related (e.g., newer versions of the same credential type). The same effect could be achieved by defining multiple `CredentialQuery` entries. | ||
| The verifier MUST ensure that every element in `elements_dict` is defined for each listed credential type. |
There was a problem hiding this comment.
Discussed on today's WG call:
Martijn will make a comment/suggestion. It's not a "MUST" but for the query to work this needs to be the case, e.g. if one credential has an extension and the other doesn't.
Co-authored-by: Joseph Heenan <joseph@heenan.me.uk> Co-authored-by: Oliver Terbu <o.terbu@gmail.com>
|
Not sure what gh did from committing a 2 word change (via UI suggestions in gh) to creating a 1200 line commit that doesn't change anything but 2 words... I'll force push remove the last commit if this doesn't fix itself |
3b3211d to
a9ce0fc
Compare
| |---|---|---|---|---| | ||
| | `credential_format` | `1` | `tstr` | M | Format identifier (e.g., `"mso_mdoc"`, `"vc+sd-jwt"`) | | ||
| | `credential_type` | `2` | `tstr` | M | Credential type (e.g., `"org.iso.18013.5.1.mDL"`) | | ||
| | `credential_types` | `2` | `[ + tstr ]` | M | Acceptable credential types (e.g., `["org.iso.18013.5.1.mDL"]`). The wallet may present a credential of any listed type | |
There was a problem hiding this comment.
The "the wallet may present a credential of any listed type" seems like a processing rule that probably needs to be centralized somewhere else. Also does "may" need to be "MAY"?
| |---|---|---|---|---| | ||
| | `credential_format` | `1` | `tstr` | M | Format identifier (e.g., `"mso_mdoc"`, `"vc+sd-jwt"`) | | ||
| | `credential_type` | `2` | `tstr` | M | Credential type (e.g., `"org.iso.18013.5.1.mDL"`) | | ||
| | `credential_types` | `2` | `[ + tstr ]` | M | Acceptable credential types (e.g., `["org.iso.18013.5.1.mDL"]`). The wallet may present a credential of any listed type | |
There was a problem hiding this comment.
Discussed on today's call:
The 'may' sentence is normative and probably sits better in the processing rules section. Remove the second sentence from here.
Martijn to check if we have an issue for adding that processing rules section.
jogu
left a comment
jogu
left a comment
There was a problem hiding this comment.
LGTM , two very editorial nits.
Co-authored-by: Joseph Heenan <joseph@heenan.me.uk>
| | `encryption_ref` | `7` | `int` | O | Reference to an entry in `additional_encryption_contexts`; absent means the main `encryption_context` is used | | ||
|
|
||
| Listing multiple values in `credential_types` is a size optimization for requesting the same data elements from credential types that are closely related (e.g., newer versions of the same credential type). It is equivalent to defining one `CredentialQuery` per listed credential type, identical in all other respects, and offering them as alternatives. | ||
| The verifier shall ensure that every element in `elements_dict` is defined for each listed credential type. |
There was a problem hiding this comment.
I agree with the sentiment, but normatively saying when you can and cannot put multiple credential types in the array is difficult, especially since normatively saying when something is or isn't defined for a credential type is likely ambiguous.
To give another extreme example, if you ask for:
credential_type = [type_a, typeb]
and then ask for the following data elements:
element_only_in_type_a OR element_only_in_type_b
it will still work, even though you are doing something that you shouldn't do.
In general I feel that putting multiple credential types in a single credential type makes it more difficult for the relying party that creates the query and therefore I would recommend not to use the feature. However I also don't think normatively it's a problem because of the very clear processing rule.
|
|
||
| Listing multiple values in `credential_types` is a size optimization for requesting the same data elements from credential types that are closely related (e.g., newer versions of the same credential type). It is equivalent to defining one `CredentialQuery` per listed credential type, identical in all other respects, and offering them as alternatives. | ||
| The verifier shall ensure that every element in `elements_dict` is defined for each listed credential type. | ||
| If the requirements differ between credential types, the verifier shall use separate `CredentialQuery` entries instead. |
There was a problem hiding this comment.
As with the previous comment, I agree with the sentiment, but I don't think we should normatively say this. What I would like to say is "probably you should just use separate credential queries to unless you have a good reason to put them in a single query." but then with better words.
Co-authored-by: Martijn Haring <62745275+martijnharing@users.noreply.github.com>
| The verifier shall ensure that every element in `elements_dict` is defined for each listed credential type. | ||
| If the requirements differ between credential types, the verifier shall use separate `CredentialQuery` entries instead. |
There was a problem hiding this comment.
Discussed on today's WG call; consensus on call:
We don't have similar text for other forms of queries, we can just remove the two lines.
This branch has not been deployed
7 participants
Closes #46