Clarify an empty object in a VP Token cannot be used to signify an error response - #745
Clarify an empty object in a VP Token cannot be used to signify an error response#745awoie wants to merge 13 commits into
Conversation
|
We probably will need to apply this logic to DC API as well. I will need to double check if this is included with the current proposed text. |
fkj
left a comment
There was a problem hiding this comment.
If people don't object to the rewriting of the definition, I agree that this makes it easier to understand what we actually mean.
|
wg discussion, should be in the errata as well. |
Co-authored-by: Kristina <52878547+Sakurann@users.noreply.github.com>
New test module oid4vp-1final-wallet-negative-test-required-non-matching-credential sends a DCQL query whose credential_sets require both the real credential and a non-existent one. As the query as a whole cannot be satisfied, the wallet must not return a vp_token - neither an empty object nor a partial one containing only the real credential - as clarified in openid/OpenID4VP#745. The module accepts either an error response (access_denied expected; other codes give a warning; encrypted error responses are supported in the .jwt response modes) or the wallet rejecting the request and displaying an error, verified via a screenshot placeholder. AddOptionalNonMatchingCredentialToDcqlQuery is refactored into an abstract base shared with the new required variant, and the browser API response parsing/rejection handling in AbstractVP1FinalWalletTest is extracted into reusable helpers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Per WG consensus on #743, an empty VP Token (i.e., {}) MUST NOT be used, including to signify an error. When the Wallet has no Presentation to return - e.g., no consent, no matching Credentials, or a DCQL query satisfiable without any Presentation because all unfulfilled Credential Queries are optional - any response it returns MUST be an error response rather than a VP Token. Also fixes the truncated sentence in the vp_token requirements, applies the same clarification to 1.0 as an errata set 1 change with changelog entries in both documents, and removes the accidentally committed .DS_Store.
|
Please review again @jogu @paulbastian @Sakurann . @jogu Note that if my memory is correct that the WG agreed to prohibit empty vp_token's even for the case where the DCQL query has only optional queries. |
An empty vp_token object cannot be used to signify an error response; the wallet must return an authorization error response instead. This is being clarified for OID4VP 1.1 and the 1.0 errata in openid/OpenID4VP#745 (which fixes openid/OpenID4VP#743). Previously an empty object was rejected by the entry-count check with a misleading 'seems to contain more than one credential' message. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…iple'
Per OID4VP 1.0 section 8.1 the array for a Credential Query must contain
exactly one Presentation when 'multiple' is omitted or false ('only one'
in the 1.0 text, reworded to 'exactly one' in
openid/OpenID4VP#745). Previously any extra
presentations in the array were silently ignored.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
New test module oid4vp-1final-wallet-negative-test-required-non-matching-credential sends a DCQL query whose credential_sets require both the real credential and a non-existent one. As the query as a whole cannot be satisfied, the wallet must not return a vp_token - neither an empty object nor a partial one containing only the real credential - as clarified in openid/OpenID4VP#745. The module accepts either an error response (access_denied expected; other codes give a warning; encrypted error responses are supported in the .jwt response modes) or the wallet rejecting the request and displaying an error, verified via a screenshot placeholder. AddOptionalNonMatchingCredentialToDcqlQuery is refactored into an abstract base shared with the new required variant, and the browser API response parsing/rejection handling in AbstractVP1FinalWalletTest is extracted into reusable helpers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
An empty vp_token object cannot be used to signify an error response; the wallet must return an authorization error response instead. This is being clarified for OID4VP 1.1 and the 1.0 errata in openid/OpenID4VP#745 (which fixes openid/OpenID4VP#743). Previously an empty object was rejected by the entry-count check with a misleading 'seems to contain more than one credential' message. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…iple'
Per OID4VP 1.0 section 8.1 the array for a Credential Query must contain
exactly one Presentation when 'multiple' is omitted or false ('only one'
in the 1.0 text, reworded to 'exactly one' in
openid/OpenID4VP#745). Previously any extra
presentations in the array were silently ignored.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
New test module oid4vp-1final-wallet-negative-test-required-non-matching-credential sends a DCQL query whose credential_sets require both the real credential and a non-existent one. As the query as a whole cannot be satisfied, the wallet must not return a vp_token - neither an empty object nor a partial one containing only the real credential - as clarified in openid/OpenID4VP#745. The module accepts either an error response (access_denied expected; other codes give a warning; encrypted error responses are supported in the .jwt response modes) or the wallet rejecting the request and displaying an error, verified via a screenshot placeholder. AddOptionalNonMatchingCredentialToDcqlQuery is refactored into an abstract base shared with the new required variant, and the browser API response parsing/rejection handling in AbstractVP1FinalWalletTest is extracted into reusable helpers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@paulbastian will review as well |
…ed privacy considerations of dc api, so it applies to regular flow as well Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Joseph Heenan <joseph@authlete.com>
Co-authored-by: Joseph Heenan <joseph@authlete.com>
|
@jogu You're right, DC API specificity: Partly agreed. The underlying consideration isn't unique to the DC API. A Wallet in a redirect-based flow that returns an error depending on whether it can match the request leaks the same information, which is what the two SHOULD NOT paragraphs below the list already address. But you're right that the bullets were phrased as unconditional, and that absolute only holds for the DC API. Rather than moving them back under the DC API heading, I scoped the lead-in:
and added a note after the list:
The bullets themselves are unchanged. Let me know if you'd rather have them back in the DC API subsection. Also synced the same changes plus your two earlier suggestions into 1.1, which had drifted from 1.0. |
|
Discussed in DCP WG call. Changes have been made and @jogu needs to re-review. |
jogu
left a comment
There was a problem hiding this comment.
Looks good to me I think - spotted to unrelated rendering issues that would be good to include.
fkj
left a comment
There was a problem hiding this comment.
Minor editorial suggestions, but otherwise looks fine to me.
Co-authored-by: Joseph Heenan <joseph@authlete.com>
Co-authored-by: Joseph Heenan <joseph@authlete.com>
Co-authored-by: Frederik Krogsdal Jacobsen <fkj@users.noreply.github.com>
Co-authored-by: Frederik Krogsdal Jacobsen <fkj@users.noreply.github.com>
|
@dastoikov @brentzundel @Sakurann this might be good to merge now. |
| A VP Token is only returned if the corresponding Authorization Request contained a `dcql_query` parameter or a `scope` parameter representing a DCQL Query (as defined in #vp_token_request). | ||
|
|
||
| The Wallet MUST return a VP Token only if it contains a set of | ||
| Presentations that satisfy the requirements of the DCQL query according to |
There was a problem hiding this comment.
| Presentations that satisfy the requirements of the DCQL query according to | |
| Presentations that satisfy the requirements of the DCQL query according to |
| A VP Token is only returned if the corresponding Authorization Request contained a `dcql_query` parameter or a `scope` parameter representing a DCQL Query, as defined in (#vp_token_request). | ||
|
|
||
| The Wallet MUST return a VP Token only if it contains a set of | ||
| Presentations that satisfy the requirements of the DCQL query according to |
There was a problem hiding this comment.
| Presentations that satisfy the requirements of the DCQL query according to | |
| Presentations that satisfy the requirements of the DCQL query according to |
| (#error-response). In particular, an empty VP Token (a JSON object without | ||
| any entries) MUST NOT be used to signify an error. Privacy considerations that | ||
| apply when returning an error response, in particular with regard to End-User | ||
| consent, are defined in (#error-responses). |
There was a problem hiding this comment.
Should we say something about the error to use here? I'd say access_denied is the natural choice.
Fixes #743
I also improved the vp_token definition because it is a quite important part of the spec and the readability was not great.
I removed this editorially because I don't think it had any meaning anymore: