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(keys): add functionality to extract public key to KeyParsers #4360

Conversation

paullatzelsperger
Copy link
Member

What this PR changes/adds

Adds a parsePublic method to they keyparsers, that attempt to extract the public key from an encoded string, leaving behind any potential private key parts.

Why it does that

LocalPublicKeyServiceImpl should be able to extract a public key from any serialized format

Further notes

List other areas of code that have changed but are not necessarily linked to the main feature. This could be method
signature changes, package declarations, bugs that were encountered and were fixed inline, etc.

Linked Issue(s)

Closes # <-- insert Issue number if one exists

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

@paullatzelsperger paullatzelsperger added the enhancement New feature or request label Jul 12, 2024
@paullatzelsperger paullatzelsperger force-pushed the feat/add_toPublic_method_to_keyparsers branch from a98897a to b5269fe Compare July 12, 2024 08:50
@@ -48,6 +48,10 @@
.orElseGet(() -> Result.failure("No public key could be resolved for key-ID '%s'".formatted(id)));
}

public Result<Void> addRawKey(String id, String rawKey) {
return parseKey(rawKey).onSuccess((pk) -> cachedKeys.put(id, pk)).mapTo();

Check notice

Code scanning / CodeQL

Deprecated method or constructor invocation Note

Invoking
Result.mapTo
should be avoided because it has been deprecated.
@paullatzelsperger paullatzelsperger force-pushed the feat/add_toPublic_method_to_keyparsers branch 2 times, most recently from fbc28f7 to 23279e6 Compare July 12, 2024 09:04
@paullatzelsperger paullatzelsperger force-pushed the feat/add_toPublic_method_to_keyparsers branch from 23279e6 to a885770 Compare July 12, 2024 09:19
@codecov-commenter
Copy link

codecov-commenter commented Jul 12, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 63.15789% with 14 lines in your changes missing coverage. Please review.

Project coverage is 75.22%. Comparing base (7f20ba5) to head (1708555).
Report is 355 commits behind head on main.

Files Patch % Lines
...ava/org/eclipse/edc/keys/keyparsers/PemParser.java 65.00% 4 Missing and 3 partials ⚠️
...va/org/eclipse/edc/keys/KeyParserRegistryImpl.java 0.00% 4 Missing ⚠️
...ava/org/eclipse/edc/keys/keyparsers/JwkParser.java 75.00% 3 Missing ⚠️

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

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4360      +/-   ##
==========================================
+ Coverage   71.74%   75.22%   +3.48%     
==========================================
  Files         919     1055     +136     
  Lines       18457    21132    +2675     
  Branches     1037     1161     +124     
==========================================
+ Hits        13242    15897    +2655     
+ Misses       4756     4714      -42     
- Partials      459      521      +62     

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

@paullatzelsperger paullatzelsperger force-pushed the feat/add_toPublic_method_to_keyparsers branch from 952badf to 7f56306 Compare July 12, 2024 09:48
public Result<PublicKey> parsePublic(String encoded) {
return parsers.stream().filter(kp -> kp.canHandle(encoded))
.findFirst()
.map(kp -> kp.parsePublic(encoded).compose(k -> Result.success((PublicKey) k)))
Copy link
Member

Choose a reason for hiding this comment

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

the type validation that was in LocalPublicKeyServiceImpl is missing

Copy link
Member Author

@paullatzelsperger paullatzelsperger Jul 12, 2024

Choose a reason for hiding this comment

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

as per the contract of the parsePublic method this can never be something other than a PublicKey, even though it returns a Key. This is necessary to be able to have the parsePublic method as default in the interface. Not all key parsers support this feature.

@paullatzelsperger paullatzelsperger merged commit f7e1aaf into eclipse-edc:main Jul 12, 2024
23 checks passed
@paullatzelsperger paullatzelsperger deleted the feat/add_toPublic_method_to_keyparsers branch July 12, 2024 10:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants