decentralized-identity / veramo

A JavaScript Framework for Verifiable Data
https://veramo.io
Apache License 2.0
438 stars 131 forks source link

[proposal] W3C VC format is not checked for EIP712 credentials #1100

Open mirceanis opened 1 year ago

mirceanis commented 1 year ago

Is your feature request related to a problem? Please describe. JWT and JSON-LD credentials modules are both able to verify the W3C VC data model when verifying a credential or presentation. This functionality is not fully implemented in the credentials-eip712 module.

Describe the solution you'd like VC format should be validated by default when verifying

phoniks commented 7 months ago

If I'm understanding this issue correctly, it sounds like the data model is validated for both JWT and JSON-LD credentials/presentations already and we want to implement the same functionality for eip712 credentials. In looking at the repo, I was able to find the following modules which I think are the 3 relevant ones:

  1. https://github.com/decentralized-identity/veramo/tree/next/packages/credential-eip712
  2. https://github.com/decentralized-identity/veramo/tree/next/packages/credential-ld
  3. https://github.com/decentralized-identity/veramo/tree/next/packages/credential-w3c

My assumption is that I should be able to look at 2 & 3 as examples of how to implement the validation of the data model for 1. Does this seem like an accurate assumption? Can anyone point to where exactly the validation takes place in the modules that already have this functionality?

@mirceanis or @italobb ?

nickreynolds commented 6 months ago

@phoniks so the "credential-w3c" package actually handles all credential types and then sends the credential to the appropriate plugin for verification. You can see this in credential-w3c/src/action-handler.ts verifyCredential. Using the credential-ld package as a comparison would be a good idea, though.

However, to really understand how the verification works, you'll have to see that the credential-ld package actually uses another package to verify credentials, specifically @digitalcredentials/vc (you can see this in credential-ld/src/ld-credential-module.ts line 175).

So, I think what this issue is really asking you to do, is to add some of the work being done in that package ( https://github.com/digitalcredentials/vc/blob/51d1f3fd8a578864c2ea1e22de1f9dd7227ed99d/lib/index.js#L298 ) into the credential-eip712 verifyCredential function. This may be as simple as adding the checks seen in _checkCredential ( https://github.com/digitalcredentials/vc/blob/51d1f3fd8a578864c2ea1e22de1f9dd7227ed99d/lib/index.js#L612 )

phoniks commented 6 months ago

Thanks @nickreynolds - this is super helpful context. I think it should be enough for me to be able to put together a PR and get some feedback.