openwallet-foundation / acapy-plugins

aries-acapy-plugins
https://hyperledger.github.io/aries-acapy-plugins/
Apache License 2.0
6 stars 27 forks source link

OpenID4VP minor issues (client ID, client_id_scheme, verifier metadata) #1162

Open gmulhearn opened 4 weeks ago

gmulhearn commented 4 weeks ago

Hi, appreciating the OpenID4VC plugin, just reporting some compatibility issues from testing against the OpenID4VC plugin and comparing with credo. NOTE: I'm not entirely sure which draft version of OpenID4VP is targeted with the current plugin, so the solution to some of these may vary depend on that (e.g. in some draft versions client_id_scheme doesn't exist). https://openid.net/specs/openid-4-verifiable-presentations-1_0.html#name-document-history

1. the returned URI from create_oid4vp_request does not contain a client_id parameter alongside the request_uri.

This might be considered as redundant, since the client_id is available in the request JWT (JAR) payload, but the JAR RFC specifies that client_id param is still required in the uri: https://www.rfc-editor.org/rfc/rfc9101.pdf

2. client_id_scheme is not included in the JAR payload.

Again, whether this is a problem depends on what draft version of the spec was targeted, but in draft 21, i believe client_id_scheme should be specified in order to tell the consumer (wallet) that the client_id is a did based client ID (which informs how the wallet should go about verifying the JAR). If no client_id_scheme is provided, then i believe the default scheme is pre-registered: https://openid.net/specs/openid-4-verifiable-presentations-1_0.html#section-5.7 - i don't think this is what the acapy plugin is going for, it should be using a did scheme.

With that being said... Draft 22+ (unpublished) removes client_id_scheme, so it's worth being aware of that: https://openid.github.io/OpenID4VP/openid-4-verifiable-presentations-wg-draft.html#section-5.10.4

3. JAR payload has verifier/client metadata at root level

The current JAR payload (returned by get_request public_routes) has the client/verifier metadata (https://openid.net/specs/openid-4-verifiable-presentations-1_0.html#name-verifier-metadata-client-me) at the root of the object rather than embedded within the client_metadata field/object.

For instance, the plugin puts vp_formats at the root level, compared to a draft 21 spec example:

{
  "client_id": "did:example:123",
  "client_id_scheme": "did",
  "response_type": "vp_token",
  "redirect_uri": "https://client.example.org/callback",
  "nonce": "n-0S6_WzA2Mj",
  "presentation_definition": "...",
  "client_metadata": {
    "vp_formats": {
      "jwt_vp": {
        "alg": [
          "EdDSA",
          "ES256K"
        ]
      },
      "ldp_vp": {
        "proof_type": [
          "Ed25519Signature2018"
        ]
      }
    }
  }
}

There are also other parameters included in the acapy JAR that i'm unsure about. For instance scope: vp_token is included, i'm not sure this is the intended usage of scope, i believe they're meant to be used to specify mutually known presentation definitions: https://openid.net/specs/openid-4-verifiable-presentations-1_0.html#section-5.3

Such a scope value MUST be an alias for a well-defined Presentation Definition that will be referred to in the presentation_submission response parameter.

4. URI scheme for auth requests (openid vs openid4vp)

This one is more of a question for my understanding, there's probably a gap in my understanding

the plugin currently uses openid:// as the scheme for the returned auth request. This scheme maps to a static configuration defined in the SIOPv2 spec: https://openid.net/specs/openid-connect-self-issued-v2-1_0.html#name-a-set-of-static-configuratio . I believe this scheme is designed for communicating with wallets that support SIOPv2 (id_token) & OpenID4VP (vp_token). Would it be more appropriate to use the scheme defined in the OpenID4VP spec openid4vp://: https://openid.net/specs/openid-4-verifiable-presentations-1_0.html#section-11.1.2, which targets just vp_token responses (matches the capabilities of the acapy plugin verifier). In my testing with credo, openid4vp:// was being used.

dbluhm commented 3 weeks ago

@mepeltier @TheTechmage fyi