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

fix: require key-id when resolving key material #3655

Conversation

paullatzelsperger
Copy link
Member

@paullatzelsperger paullatzelsperger commented Nov 24, 2023

What this PR changes/adds

This PR changes the public key resolution, in that the kid is only required, if the DID contains multiple verification methods.

Why it does that

Taking the first verification method if no kid is given is dangerous and wrong.

Further notes

  • In addition, the JwtPresentationVerifier now uses the DidPublicKeyResolver instead of the DidResolverRegistry, which is better, because it limits the API surface

Linked Issue(s)

Closes #3649

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

@codecov-commenter
Copy link

codecov-commenter commented Nov 24, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (66ded5e) 71.76% compared to head (604e1b8) 71.74%.

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

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3655      +/-   ##
==========================================
- Coverage   71.76%   71.74%   -0.02%     
==========================================
  Files         916      916              
  Lines       18356    18353       -3     
  Branches     1041     1041              
==========================================
- Hits        13173    13168       -5     
- Misses       4727     4729       +2     
  Partials      456      456              

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

@paullatzelsperger paullatzelsperger force-pushed the fix/require_kid_in_did_verificationmethod branch from d63e076 to 604e1b8 Compare November 24, 2023 12:27
@paullatzelsperger paullatzelsperger merged commit 1f76ae9 into eclipse-edc:main Nov 24, 2023
17 checks passed
@paullatzelsperger paullatzelsperger deleted the fix/require_kid_in_did_verificationmethod branch November 24, 2023 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working core feature dcp
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing kid when resolving DIDs with multiple keys should fail
3 participants