Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Alignments with OpenID4VP Draft 22 #507

Open
wants to merge 10 commits into
base: versione-corrente
Choose a base branch
from

Conversation

peppelinux
Copy link
Member

@peppelinux peppelinux commented Nov 25, 2024

This PR:

In general, this PR has introduced the following breaking changes:

  • removed custom RP metadata parameterpresentation_definition_supported
  • renamed metadata type name wallet_relying_party to openid_credential_verifier
  • aligned JARM ref to the latest release
  • definitively removed deprecated client_id_scheme request parameter
  • additional clarifications about the importance of the parameter state in the presentation request
  • introduces dcql query, removed the dependency from presentation_definition
  • removed scopes in the rp requests
  • added client_id_schemes_supported in the wallet metadata during presentation
  • added client_id_schemes_supported in wallet attestation
  • typ value vc+sd-jwt renamed to dc+sd-jwt
  • when DCQL is used, vp_token is not an array but a json object
  • add further clarifications against the endpoint mixup attacks and using the endpoints attested within the federation trust chain metadata
  • fix the request-uri request including the wallet metadata using the application/x-www-form-urlencoded

docs/en/remote-flow.rst Outdated Show resolved Hide resolved
docs/en/remote-flow.rst Outdated Show resolved Hide resolved
docs/en/remote-flow.rst Outdated Show resolved Hide resolved
docs/common/standards.rst Outdated Show resolved Hide resolved
docs/en/remote-flow.rst Outdated Show resolved Hide resolved
docs/en/remote-flow.rst Outdated Show resolved Hide resolved
docs/en/remote-flow.rst Outdated Show resolved Hide resolved
docs/en/remote-flow.rst Outdated Show resolved Hide resolved
@@ -232,16 +238,27 @@ Below a non-normative example of the HTTP Request to the status endpoint, where

When the status endpoint returns **200 OK**, it means that the User authentication is successful and the JavaScript code will use the returned location where the user-agent will be redirect to continue the navigation.

Even if an adversary were to steal the random value used in the request to the status endpoint, their user-agent would be rejected due to the missing cookie in the request.


Request Object Details
----------------------

Below a non-normative example of HTTP request made by the Wallet Instance to the Relying Party.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Normative details of the Request URI request are missing

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see my previous comment. I'd prefer to not redefine those parameters but reference the section where they are defined.

"client_id": "https://relying-party.example.org",
"response_mode": "direct_post.jwt",
"response_type": "vp_token",
"dcql_query": {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Normative details of the DCQL query are missing

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -559,7 +572,8 @@ When the Wallet sends a response using ``direct_post.jwt`` to the Relying Party,
Redirect URI
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Normative details of the response at the Authorization Response endpoint are missing

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my suggestion below

@@ -303,14 +343,12 @@ The JWT payload parameters are described herein:

* - **Name**
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wallet_nonce is missing

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wallet_nonce is optional, I have added it anyway.

@giadas
Copy link
Collaborator

giadas commented Dec 23, 2024

The current structure of the Remote Flow section is not easy to follow. For example, it starts with the details of the Request URI POST, even if in the flow this request occurs after the Authorization Request. To follow the order of the requests and responses presented in the flow, I suggest restructuring this section. Here a proposal

Remote Flow
[...]

  1. Authorization Request
  2. Request URI Request
    With HTTP GET
    With HTTP POST
  3. Request URI Response
    Request Object Details
    Request URI Endpoint Errors
  4. “Authorization Response” Request
    SD-JWT Presentation
  5. “Authorization Response” Response
    Redirect URI
    Redirect URI Errors
    Authorization Response Errors
  6. Device Flow Status Checks and Security

@@ -329,29 +367,21 @@ The JWT payload parameters are described herein:
- String determining the HTTP method to be used with the `request_uri` endpoint to provide the Wallet Instance metadata to the Relying Party. The value is case-insensitive and can be set to: `get` or `post`. The GET method, as defined in [@RFC9101], involves the Wallet Instance sending a GET request to retrieve a Request Object. The POST method involves the Wallet Instance requesting the creation of a new Request Object by sending an HTTP POST request, with its metadata, to the request URI of the Relying Party.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that the request_uri_method should be removed from the request object. The Wallet has already performed the GET/POST request to the Request URI to obtain the Request Object, it does not need this info here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the way we have included it in the qrcode is pretty custom and in the form of an hint to anticipate its use and then to have a more efficient flow

The place of the request_uri_method is in the request response object

Co-authored-by: Giada Sciarretta <[email protected]>
Even if an adversary were to steal the random value used in the request to the status endpoint, their user-agent would be rejected due to the missing cookie in the request.


Request URI Request
----------------------

Copy link
Member Author

@peppelinux peppelinux Dec 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The request and its parameters are defined in Section number 5 (Authorization Request) of `OpenID4VP`_.

docs/en/remote-flow.rst Outdated Show resolved Hide resolved
Comment on lines 349 to 350
* - **response_type**
- It MUST be set to ``vp_token``.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* - **response_type**
- It MUST be set to ``vp_token``.
* - **response_type**
- It MUST be set to ``vp_token``.
* - **wallet_nonce**
- OPTIONAL. A String value used to mitigate replay attacks of the Authorization Request, as defined in Section 5.11 (Request URI Method) of `OpenID4VP`_.

@@ -559,7 +569,8 @@ When the Wallet sends a response using ``direct_post.jwt`` to the Relying Party,
Redirect URI
------------

When the Relying Party provides the redirect URI, the Wallet Instance MUST send the user-agent to this redirect URI. The redirect URI allows the Relying Party to continue the interaction with the End-User on the device where the Wallet Instance resides after the Wallet Instance has sent the Authorization Response to the response URI.
When the Relying Party provides the redirect URI, the Wallet Instance MUST redirect the user-agent to the location defined in the redirect URI.
The redirect URI allows the Relying Party to continue the interaction with the End-User on the device where the Wallet Instance resides after the Wallet Instance has sent the Authorization Response to the response URI.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The response provided by Relying Party, and the parameters and values contained in it, are defined in Section 7.2. (Response Mode "direct_post") of the OpenID4VP specification.

@peppelinux peppelinux requested a review from giadas December 23, 2024 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Progress
2 participants