Sphereon-Opensource / SIOP-OID4VP

Self Issued OpenID Provider v2 (SIOP) with optional OpenID for Verifiable Presentations (OpenID4VP)
77 stars 25 forks source link

OpenID4VP ID1 + JWT VC Presentation Profile #29

Open siacomuzzi opened 1 year ago

siacomuzzi commented 1 year ago

Hi,

Do you have plans to support OpenID4VP ID1/JWT VC Presentation Profile spec?

I know that there are a lot of specifications and different versions about that, but it seems like this specific one is used by Microsoft Entra.

Thank you

nklomp commented 1 year ago

Hi @siacomuzzi

Yes on both accounts. We are working on both of these. In https://github.com/Sphereon-Opensource/did-auth-siop/tree/release/v2.0.0 you can see some things landing for the VC Presentation Profile, like domain linkage etc.

The OID4VP work is just starting at this point, but we need something concrete to show interop over the next few weeks, so we will have something pretty fast.

siacomuzzi commented 1 year ago

Thank you @nklomp!

I just installed that branch in my webapp:

yarn add "github:Sphereon-Opensource/did-auth-siop#release/v2.0.0"

But it's looks like the build script is not executed:

image

Am I missing something?

nklomp commented 1 year ago

Yeah, unfortunately the name of the branch is a bit off. There is no release yet of that version. You could build it locally I guess. We will soon merge it.

siacomuzzi commented 1 year ago

I was able to build it locally:

did-auth-siop % yarn build
yarn run v1.22.10
$ run-p build:*
$ tsc -p tsconfig.build.json
$ node --loader ts-node/esm generator/schemaGenerator.ts
(node:40683) ExperimentalWarning: --experimental-loader is an experimental feature. This feature could change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
✨  Done in 10.63s.

But, after link the module into my webapp (yarn link "@sphereon/did-auth-siop") and execute it, I'm getting some runtime errors:

warn  - ../did-auth-siop/node_modules/node-fetch/lib/index.js
Module not found: Can't resolve 'encoding' in '/Users/xyz/Documents/did-auth-siop/node_modules/node-fetch/lib'

../did-auth-siop/node_modules/ws/lib/buffer-util.js
Module not found: Can't resolve 'bufferutil' in '/Users/xyz/Documents/did-auth-siop/node_modules/ws/lib'

../did-auth-siop/node_modules/ws/lib/validation.js
Module not found: Can't resolve 'utf-8-validate' in '/Users/xyz/Documents/did-auth-siop/node_modules/ws/lib'
error - Error: ENOENT: no such file or directory, open '(api)/../did-auth-siop/node_modules/jsonpath/include/module.js'
    at Object.openSync (node:fs:585:3)
    at Object.readFileSync (node:fs:453:35)
    at eval (webpack-internal:///(api)/../did-auth-siop/node_modules/jsonpath/lib/grammar.js:102:30)
    at Object.(api)/../did-auth-siop/node_modules/jsonpath/lib/grammar.js (/Users/xyz/Documents/vps-requester-tool-poc/.next/server/pages/api/vp_request.js:9526:1)
    at __webpack_require__ (/Users/xyz/Documents/vps-requester-tool-poc/.next/server/webpack-api-runtime.js:33:43)
    at eval (webpack-internal:///(api)/../did-auth-siop/node_modules/jsonpath/lib/parser.js:1:15)
    at Object.(api)/../did-auth-siop/node_modules/jsonpath/lib/parser.js (/Users/xyz/Documents/vps-requester-tool-poc/.next/server/pages/api/vp_request.js:9556:1)
    at __webpack_require__ (/Users/xyz/Documents/vps-requester-tool-poc/.next/server/webpack-api-runtime.js:33:43)
    at eval (webpack-internal:///(api)/../did-auth-siop/node_modules/jsonpath/lib/index.js:3:14)
    at Object.(api)/../did-auth-siop/node_modules/jsonpath/lib/index.js (/Users/xyz/Documents/vps-requester-tool-poc/.next/server/pages/api/vp_request.js:9546:1) {
  errno: -2,
  syscall: 'open',
  code: 'ENOENT',
  path: '(api)/../did-auth-siop/node_modules/jsonpath/include/module.js'
}
warn  - ../did-auth-siop/node_modules/node-fetch/lib/index.js
Module not found: Can't resolve 'encoding' in '/Users/xyz/Documents/did-auth-siop/node_modules/node-fetch/lib'

../did-auth-siop/node_modules/ws/lib/buffer-util.js
Module not found: Can't resolve 'bufferutil' in '/Users/xyz/Documents/did-auth-siop/node_modules/ws/lib'

../did-auth-siop/node_modules/ws/lib/validation.js
Module not found: Can't resolve 'utf-8-validate' in '/Users/xyz/Documents/did-auth-siop/node_modules/ws/lib'
wait  - compiling /_error (client and server)...
warn  - ../did-auth-siop/node_modules/node-fetch/lib/index.js
Module not found: Can't resolve 'encoding' in '/Users/xyz/Documents/did-auth-siop/node_modules/node-fetch/lib'

../did-auth-siop/node_modules/ws/lib/buffer-util.js
Module not found: Can't resolve 'bufferutil' in '/Users/xyz/Documents/did-auth-siop/node_modules/ws/lib'

../did-auth-siop/node_modules/ws/lib/validation.js
Module not found: Can't resolve 'utf-8-validate' in '/Users/xyz/Documents/did-auth-siop/node_modules/ws/lib'
nklomp commented 1 year ago

Hmzz, could you try out this version? https://www.npmjs.com/package/@sphereon/did-auth-siop/v/0.2.14-unstable.2

I build it from the v2 branch just now.

If that yields the same results, could you provide you tsconfig and node/typescript versions?

siacomuzzi commented 1 year ago

@sphereon/did-auth-siop@0.2.14-unstable.2 isn't listed, should I use @sphereon/did-auth-siop@0.2.14-unstable.1 instead?

yarn add @sphereon/did-auth-siop@0.2.14-unstable.2
yarn add v1.22.10
[1/4] 🔍  Resolving packages...
Couldn't find any versions for "@sphereon/did-auth-siop" that matches "0.2.14-unstable.2"
? Please choose a version of "@sphereon/did-auth-siop" from this list: (Use arrow keys)
❯ 0.2.14-unstable.1
  0.2.13
  0.2.12
  0.2.12-next.4
  0.2.12-next.3
  0.2.12-next.2
  0.2.12-next.1
  0.2.12-UNSTABLE.0
  0.2.11
  0.2.11-UNSTABLE.0
  0.2.10
  0.2.9
  0.2.9-unstable.1
  0.2.9-unstable.0
  0.2.8
  0.2.7
  0.2.7-unstable.0
  0.2.6
  0.2.5
  0.2.4
  0.2.4-unstable.1
  0.2.3
  0.2.2
  0.2.1
  0.2.0
  0.1.1
  0.1.0
  0.0.1-temp-sp2
  0.0.1-temp-sp1
siacomuzzi commented 1 year ago

nvm,@sphereon/did-auth-siop@0.2.14-unstable.2 is listed now

nklomp commented 1 year ago

Hehe probably you were too fast. Sometimes it takes a bit. But indeed..1 also works. It only misses the last merge from today

siacomuzzi commented 1 year ago

It's working now, thank you for publishing it! I've started playing with this library and Microsoft Entra and these are the first differences I've found so far:

  1. pex library is enforcing that input_descriptors[x].schema.uri must be a valid uri, but accoriding to the spec, simple strings are valid too, like "schema": [ { "uri": "InteropExampleVC" } ]. See https://identity.foundation/jwt-vc-presentation-profile/#requesting-verifiable-credentials.
  2. include client_name and client_purpose attributes as part of the registration. This info will be shown by wallet to holder during presentation flow.
  3. Issuer is https://self-issued.me/v2/openid-vc (following the spec), but the library is expecting https://self-isued.me/v2 (we have the same problem with did-jwt)
  4. Pass header object when resolving issuer did
  5. sub_type claim isn't specified in the id_token, so library returns SIOPErrors.NO_SUB_TYPE
  6. id_token has _vp_token claim, but library expects vp_token
  7. It's not cleat to me how assertValidVerifiablePresentations takes presentation_submission object (located inside the id_token) to validate the vp_token. See https://identity.foundation/jwt-vc-presentation-profile/#structure-of-authentication-response
nklomp commented 1 year ago

Thanks for the valuable feedback. We will definitely address these. This lib predates the jwt-vc interop profile and was based on older SIOPv2 and OIDC4VP specs. Hence there are still some incompatibilities.

We certainly will make it compatible with the interop profile, since we need it ourselves as well ;)

CC @sksadjad @BtencateSphereon @maikel-maas

nklomp commented 1 year ago

Regarding 3. It seems like we are now in a situation, where the latest SIOPV2 spec, ditched the value altogether and the JWT VC Interop profile using a different value than the SIOPv2 Implementors Draft 1 version it was based on. I asked for clarification.

I asked for clarification in the interop profile Github: https://github.com/decentralized-identity/jwt-vc-presentation-profile/issues/63

nklomp commented 1 year ago

Regarding 1. I think this is an error in the interop profile example. The schema comes from Presentation Exchange v1 specification and it quite clearly should be a URI. It also doesn't make much sense to me to have a simple string instead of a URI for what is supposed to link a schema. I have created a ticket in the interop github for clarification: https://github.com/decentralized-identity/jwt-vc-presentation-profile/issues/64

siacomuzzi commented 1 year ago

Regarding 1. I think this is an error in the interop profile example. The schema comes from Presentation Exchange v1 specification and it quite clearly should be a URI. It also doesn't make much sense to me to have a simple string instead of a URI for what is supposed to link a schema. I have created a ticket in the interop github for clarification: decentralized-identity/jwt-vc-presentation-profile#64

Please note that the "format": "uri" attribute isn't specified in the Presentation Exchange v1 specification:

image

So something like { "uri": "InteropExampleVC" } should be valid.

embesozzi commented 1 year ago

Hi, Is there any update on this ? Nowadays Microsoft Entra uses an string for the schema.uri, for example:

{
      id: "12bda93d-c33f-463d-9043-2bf7d4b715ab",
      input_descriptors: [
       {
            id: "VerifiedCredentialEmployee",
            name: "VerifiedCredentialEmployee",
            purpose: "So we can see that your Corporate Verified Employee",
            schema: [{
                "uri": "VerifiedCredentialEmployee"
            }],
            constraints: {
                fields: [
                {
                    ...
                ]
            },
        }]
}  

Is there a workaround for this? - BTW I'm using the latest version ^0.3.x of the library

Thanks,

nklomp commented 1 year ago

Hi @embesozzi. We are going to support strings as well, to ensure interop. We have this on our backlog together with some changes we need to make to the lower level Presentation Exchange at a later point.

But let me expedite this a bit, as it should be a simple change on our end.

nklomp commented 1 year ago

The 0.3.0-unstable.2 release now supports strings for schema.uri.

Leaving this open for now, as there probably will be some more issues with Entra compatibility. We will soon write some integration and regression tests for entra and ID1.

embesozzi commented 1 year ago

Hi @nklomp, first of all thanks for your quick fix. I confirm you that I was able to send the authorization request without any problem. Furthermore, Google Authenticator presented the requested credential (this vc was issued with MS Entra services). And then, I validated the authorization response (verifyDidJWT method): jwt vp_token (response.body.vp_token) and the jwt vc (jwt.payload.vp.verifiableCredential[0]) using an ION resolver. So, I can confirm that the library works well with VC issued with MS Entra :ok_hand:. This means that a Verifier app can request and validate the vc (without having any MS Entra and Azure requirements).

Great work guys ! :clap:

nklomp commented 1 year ago

Cool, thanks for the feedback @embesozzi!

I was expecting some issues with some of the discovery metadata, as we still have some work to do there. Be adviced that there might be some small external interface changes in the builders coming the next few weeks. We are finishing some TODOs of the v0.3 refactor, hence the current 0.3.0-unstable pre-releases.

Will keep this ticket open to update for changes and keep track whether everything still works for ID1 (and entra)