Sphereon-Opensource / SIOP-OID4VP

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

Remove crypto and did resolver related functionality #55

Open TimoGlastra opened 1 year ago

TimoGlastra commented 1 year ago

I think we've sort of discussed this before, but just wanted to open an issue to continue the discussion.

We're currently integrating this library into Aries Framework JavaScript (AFJ), and one of the things that was really nice about the OID4VCI and PEX libraries is that they're really lightweight and don't pull in any crypto or did related libraries.

In AFJ we have a did resolver, JWT signer/verifier, and support for several crypto suites. That way we don't have to pull in all the dependencies for this. (I think especially did-jwt pulls in a lot of crypto related dependencies).

nklomp commented 1 year ago

Yeah we discussed this before. A lot of deps have already been removed and we are planning to split the current lib into 2 versions. One more or less current functionality which then is extending the second, providing some callbacks and interfaces for the functionality mentioned, thus removing more deps.

In all honesty I am not sure we will be able to do that inbthe next 2 weeks though. Probably gonna be roughly at the end of the month.

TimoGlastra commented 1 year ago

Splitting in two packages sounds good to me!

In all honesty I am not sure we will be able to do that in the next 2 weeks though. Probably gonna be roughly at the end of the month.

Yeah that makes sense, no rush in doing this. My understanding of the library now is that you can use your own did resolvers, crypto, JWT signing/verification. If that's the case we're good 👍

nklomp commented 1 year ago

Yes that is indeed correct.

TimoGlastra commented 1 year ago

It seems that for verification of the Authorization Request JWT, you can't provide a custom verification callback. Is that correct?

Here's the call:

    const verifiedAuthorizationRequest = await op.verifyAuthorizationRequest(
      options.authorizationRequest,
      {
        verification: {
          mode: VerificationMode.EXTERNAL,
          revocationOpts: {
            revocationVerification: RevocationVerification.NEVER,
          },
          resolveOpts: {
            resolver: this.getResolver(agentContext),
            noUniversalResolverFallback: true,
            // TODO: do we need to add all supported did methods here?
            // subjectSyntaxTypesSupported: []
            // TODO: do we need any?
            // jwtVerifyOpts: {}
          },
        },
      }
    )

The README shows an verifyCallback in the interface, but this doesn't seem present in the code of the library.

Is VerificationMethod.External related to verifying the JWT yourself, or is that currently not supported by the library?

nklomp commented 1 year ago

Hmzz, it seems that in my head we had more options than we actually do have. External is meant for cloud based integrations, to offload it. But that is not really implemented anyway.

Looking at the code, the path relies on verifyDidJWT which then unconditionally delegates to the did-jwt library.

You could extend the enum, with a 'PROVIDED' and then add a callback interface. It would be relatively easy to pass that on to the AuthorizationRequest verify method and then in the verifyDidJWT not delegate to did-jwt if the Mode is PROVIDED and instead execute the callback