Skip to content
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: add verification/validation to IdentityAndTrustService #3528

Merged

Conversation

paullatzelsperger
Copy link
Member

@paullatzelsperger paullatzelsperger commented Oct 10, 2023

What this PR changes/adds

This PR adds basic scaffolding and collaborator interfaces for performing the validation and verification of VerifiablePresentations and SI tokens in the following ways

  • validating incoming SI tokens (structural)
  • verifying incoming SI tokens (cryptographically) re-using the DidResolverRegistry
  • verifying VPs (cryptographically, currently stubbed)
  • validating VCs (structurally), extensibility not yet implemented

Why it does that

IATP adoption.

Further notes

Much of the actual code has been stubbed, to keep the PR relatively small and focused.
The following things are not yet implemented (currently mocked) and will follow either later or in subsequent PRs:

  • making the VP Request (http) against the CredentialService
  • actual cryptographic verification of VPs (JSON-LD, JWT). The SignatureSuite implementations are there, but there are some open questions about the formats. So we need to connect the dots.
  • checking the StatusList2021 (revocation)
  • pluggable additional VC validations (e.g. validating the credentialSubject)
  • [edit] Validating and verifying JWTs has been implemented in similar ways several times already, in a future PR we should harmonize this Refactor multiple modules handling JWT generation and validation #1357

Linked Issue(s)

Closes #3496

Please be sure to take a look at the contributing guidelines and our etiquette for pull requests.

* @param format indicates whether the VP is present in JWT or JSON-LD format
* @param credential the {@link VerifiablePresentation}, as it was deserialized from the raw VP string. Note that JSON-LD and JWT VCs
* have to be deserialized differently
* @param rawVp A String containing the VP in its raw format. This must be exactly how it was originally received by the holder.

Check notice

Code scanning / CodeQL

Spurious Javadoc @param tags Note

@param tag "rawVp" does not match any actual type parameter of type "VerifiablePresentationContainer".
* @param credential the {@link VerifiablePresentation}, as it was deserialized from the raw VP string. Note that JSON-LD and JWT VCs
* have to be deserialized differently
* @param rawVp A String containing the VP in its raw format. This must be exactly how it was originally received by the holder.
* @param format indicates whether the VP is present in JWT or JSON-LD format

Check notice

Code scanning / CodeQL

Spurious Javadoc @param tags Note

@param tag "format" does not match any actual type parameter of type "VerifiablePresentationContainer".
@codecov-commenter
Copy link

codecov-commenter commented Oct 10, 2023

Codecov Report

Attention: 42 lines in your changes are missing coverage. Please review.

Comparison is base (2bfcdd7) 72.21% compared to head (37b2dd8) 72.24%.
Report is 3 commits behind head on main.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3528      +/-   ##
==========================================
+ Coverage   72.21%   72.24%   +0.02%     
==========================================
  Files         853      863      +10     
  Lines       17162    17304     +142     
  Branches      965      985      +20     
==========================================
+ Hits        12394    12501     +107     
- Misses       4359     4392      +33     
- Partials      409      411       +2     
Files Coverage Δ
...entitytrust/core/IatpDefaultServicesExtension.java 100.00% <ø> (ø)
...trust/core/service/EmbeddedSecureTokenService.java 66.66% <ø> (ø)
...identitytrust/validation/rules/HasValidIssuer.java 100.00% <100.00%> (ø)
...titytrust/validation/rules/HasValidSubjectIds.java 100.00% <100.00%> (ø)
.../iam/identitytrust/validation/rules/IsRevoked.java 100.00% <100.00%> (ø)
...o/JsonObjectToVerifiableCredentialTransformer.java 90.00% <100.00%> (ø)
.../edc/identitytrust/model/VerifiableCredential.java 50.00% <ø> (ø)
...dc/identitytrust/model/VerifiablePresentation.java 58.62% <100.00%> (ø)
...tytrust/model/VerifiablePresentationContainer.java 0.00% <ø> (ø)
...edc/iam/identitytrust/IdentityAndTrustService.java 97.61% <97.61%> (ø)
... and 8 more

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

return result.map(u -> extractClaimToken(credentials));
}

private ClaimToken extractClaimToken(List<VerifiableCredential> credentials) {

Check notice

Code scanning / CodeQL

Useless parameter Note

The parameter 'credentials' is never used.
@paullatzelsperger paullatzelsperger changed the title feat: add VP verification/validation feat: add verification/validation to IdentityAndTrustService Oct 10, 2023
.filter(vm -> DidConstants.ALLOWED_VERIFICATION_TYPES.contains(vm.getType()))
.findFirst();
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed this implementation is almost similar to the verifyJwtToken of the DecentralizedIdentityService

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, there are many similarities, also regarding JWT validation and verification. I'd prefer to do the harmonization and clean up later, so that the PR doesn't get blown out of proportion.

@paullatzelsperger paullatzelsperger merged commit 67d3e09 into eclipse-edc:main Oct 12, 2023
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IATP: Validate consumer VP
4 participants