VCR: JWT support for credentials and presentations #2520
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Weirdness; with a VP from the Sphereon Wallet, the signer of the VP is a did:key, but the holder of the contained credential is a did:jwt. So the presenter is not the holder. Weird? JWT VP: |
reinkrul
requested review from
woutslakhorst,
stevenvegt and
gerardsn
and removed request for
woutslakhorst and
stevenvegt
did:key and did:jwk are both a did representation of a public key, so they could refer to the same public key. |
woutslakhorst
requested changes
Oct 2, 2023
There was a problem hiding this comment.
@gerardsn mentioned that it's still unclear if the jwt formats are called jwt_vc or jwt_vc_json. Maybe we should be able to accept both?
Comment on lines
+166
to
+176
| if keyID != "" && strings.Split(keyID, "#")[0] != credential.Issuer.String() { | ||
| return errVerificationMethodNotOfIssuer | ||
| } |
There was a problem hiding this comment.
Is this the only validation that needs to be done according to spec?
There was a problem hiding this comment.
The spec does not even state this (it vaguely hints it could do something like this), but we do it for JSON-LD, do we should do the same for JWT to avoid confusion (leading to vulnerabilities)
I decoded and compared both, the |
|
Tests are failing though |
reinkrul
force-pushed
the
vcr/jwt-support
branch
from
0b614f9 to
175d83e
Compare
reinkrul
force-pushed
the
vcr/jwt-support
branch
from
8218a6f to
f96ada5
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds support for creating and verifying VCs and VPs in JWT format.
TODO:
Requires nuts-foundation/go-did#87
Part of #2451
Note on VC/VP-JWT
The VC data model v1 specification of JWTs makes implementations complex:
issv.s.vc.issuer), this leads to unspecified behavior when both are present.This leads to interpretation of individual software vendors or the creation of profiles (https://identity.foundation/jwt-vc-presentation-profile/). However, given the current capabilities and target use of the Nuts node, we can safely address these:
kidheader fordid:nutsanddid:web. Make an exception fordid:jwk: it holds a single key, so it can be derived from theissJWT claim (appending#0).VC data model v2
VC data model v2 (which is in draft) reworks JWT encoding through https://w3c.github.io/vc-jose-cose/. Although this specification is more explicit and thus should be easier to implement, it is still in draft. So we should stick with VC data model v1 JWT of things (we also don't support VCs in v2 format).
Compatibility
Regarding compatibility: it Sphereon produces JWT VCs that seem compatible with both v1 and v2 (for v1 it includes the
vcclaim, and for v2 it puts the VC properties directly inside the JWT claims (duplication)).