-
Notifications
You must be signed in to change notification settings - Fork 245
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: verification of (consumer) VP (VC-JWT) #3545
feat: verification of (consumer) VP (VC-JWT) #3545
Conversation
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #3545 +/- ##
==========================================
+ Coverage 72.43% 72.45% +0.01%
==========================================
Files 867 867
Lines 17387 17417 +30
Branches 988 993 +5
==========================================
+ Hits 12595 12620 +25
- Misses 4376 4380 +4
- Partials 416 417 +1
☔ View full report in Codecov by Sentry. |
When implementing the JWT verifier, and starting on the LDP verifier, i noticed that adding glue code, testing etc. is going to have quite a significant impact in terms of changed files, so I decided to do it in another PR. This one only contains the JWT verifier!! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Few minor comments, but LGTM
} | ||
|
||
} catch (ParseException | JsonProcessingException e) { | ||
throw new RuntimeException(e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really want to throw? Shouldn't we return a failed result instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was sort of undecided, but in practice this can only happen, if the key is in an invalid format, or we are not dealing with valid JSON, at which point any further processing would be pointless.
...ain/java/org/eclipse/edc/iam/identitytrust/verification/MultiFormatPresentationVerifier.java
Outdated
Show resolved
Hide resolved
…c/main/java/org/eclipse/edc/iam/identitytrust/verification/MultiFormatPresentationVerifier.java Co-authored-by: Benjamin Scholtes <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
What this PR changes/adds
This PR adds the cryptographic verification of VerifiablePresentations for the following formats:
Why it does that
Verifiers (e.g. a provider connector) must be able to ensure the cryptographic integrity of a presenter's VP
Further notes
VPs must be homogenous, i.e. if a VP is presented as JWT, it may only contain VC's that are also encoded as JWT.
This is a restriction imposed by the Verfiable Credentials Data Model v1.1.
Linked Issue(s)
Closes #3533
Please be sure to take a look at the contributing guidelines and our etiquette for pull requests.