refactor: remove private/public key from commons and use typed keys expected by the algorithms #243
Conversation
This file contains hidden or 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
Break inheritance with Key superclass as the byte[] is all we need. All the time the key is 32 of length and by using a type for SecretKey we don't need to check as the code does not allow to pass a SecretKey instance to a PrivateKey instance. Typing to the rescue.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the key handling system by removing the dubious PrivateKey and PublicKey commons classes and the abstract Key superclass, replacing them with strict typing using Java's built-in key interfaces. The refactor eliminates ambiguity between byte array and key instance handling while maintaining the SecretKey class for raw byte array operations.
- Replaces generic key wrappers with specific Java security interface types (EdECPrivateKey, EdECPublicKey, ECPrivateKey, ECPublicKey, RSAPrivateKey, RSAPublicKey)
- Removes version and purpose validation checks that are now enforced through type safety
- Simplifies key constructors and eliminates deprecated byte array constructors for newer versions
Reviewed Changes
Copilot reviewed 35 out of 35 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
version4/src/main/java/org/paseto4j/version4/PasetoPublic.java |
Updates to use EdECPrivateKey/EdECPublicKey interfaces directly, removes validation checks |
version4/src/main/java/org/paseto4j/version4/PasetoLocal.java |
Removes version/purpose validation, updates key access methods |
version4/src/main/java/org/paseto4j/version4/Paseto.java |
Updates method signatures to use typed key interfaces |
version4/src/main/java/org/paseto4j/version4/CryptoFunctions.java |
Updates signature methods to accept EdECPrivateKey directly |
version3/src/main/java/org/paseto4j/version3/PasetoPublic.java |
Updates to use ECPrivateKey/ECPublicKey interfaces, adds key conversion logic |
version3/src/main/java/org/paseto4j/version3/PasetoLocal.java |
Removes validation checks, updates key access |
version3/src/main/java/org/paseto4j/version3/Paseto.java |
Updates method signatures for EC key types |
version3/src/main/java/org/paseto4j/version3/CryptoFunctions.java |
Updates crypto functions to work with EC key interfaces |
version2/src/main/java/org/paseto4j/version2/PrivateKey.java |
Adds new version-specific PrivateKey record with validation |
version2/src/main/java/org/paseto4j/version2/PublicKey.java |
Adds new version-specific PublicKey record with validation |
version2/src/main/java/org/paseto4j/version2/PasetoPublic.java |
Updates to use new key records, removes validation |
version2/src/main/java/org/paseto4j/version2/PasetoLocal.java |
Updates key access methods |
version1/src/main/java/org/paseto4j/version1/PasetoPublic.java |
Updates to use RSA key interfaces directly |
version1/src/main/java/org/paseto4j/version1/PasetoLocal.java |
Removes validation checks |
version1/src/main/java/org/paseto4j/version1/Paseto.java |
Updates method signatures for RSA key types |
version1/src/main/java/org/paseto4j/version1/CryptoFunctions.java |
Adds key conversion utilities, updates crypto functions |
commons/src/main/java/org/paseto4j/commons/SecretKey.java |
Converts to record with built-in validation |
commons/src/main/java/org/paseto4j/commons/TokenAlgorithm.java |
Converts to record |
| String expectedToken) | ||
| throws IOException, SignatureException { | ||
| Reader rdr = new StringReader(publicKeyPem); | ||
| Object parsed = new PEMParser(rdr).readObject(); |
There was a problem hiding this comment.
[nitpick] The removed debug print statement System.out.println(parsed); suggests this may have been used for debugging. Consider adding proper logging if debugging information is needed in production.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
|
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 removes the strange behavior of the commons classes surrounding
PrivateKeyandPublicKey. In some cases andbyte[]could be passed and in some cases a instance of ajava.security.Keywas expected. The keys are now strictly typed. We also removed theKeysuperclass as it no longer plays a role.The class
SecretKeystays behind as we only work with the rawbyte[]in all algorithms.Due to the strict typing we no longer need to check for purpose explicitly as
PasetoLocalandPasetoPublicas the typing forces a correct key to be used. The superclass which might introduce the wrong usage of the API no longer exists.