Instantiate RSA/EC Algorithm with both keys - #147
Merged
Conversation
|
I thinks this is greater.Please merge soon,I can't wait to use it now 👏 |
nikolaseu
reviewed
Mar 13, 2017
| @Deprecated | ||
| public static Algorithm RSA256(RSAKey key) throws IllegalArgumentException { | ||
| return new RSAAlgorithm("RS256", "SHA256withRSA", key); | ||
| RSAPublicKey publicKey = key instanceof PublicKey ? (RSAPublicKey) key : null; |
There was a problem hiding this comment.
since we are force casting we should use the same type for the instanceof
| public static Algorithm RSA384(RSAKey key) throws IllegalArgumentException { | ||
| return new RSAAlgorithm("RS384", "SHA384withRSA", key); | ||
| RSAPublicKey publicKey = key instanceof PublicKey ? (RSAPublicKey) key : null; | ||
| RSAPrivateKey privateKey = key instanceof PrivateKey ? (RSAPrivateKey) key : null; |
There was a problem hiding this comment.
use same type for instanceof and force casting
| public static Algorithm RSA512(RSAKey key) throws IllegalArgumentException { | ||
| return new RSAAlgorithm("RS512", "SHA512withRSA", key); | ||
| RSAPublicKey publicKey = key instanceof PublicKey ? (RSAPublicKey) key : null; | ||
| RSAPrivateKey privateKey = key instanceof PrivateKey ? (RSAPrivateKey) key : null; |
| @Deprecated | ||
| public static Algorithm ECDSA256(ECKey key) throws IllegalArgumentException { | ||
| return new ECDSAAlgorithm("ES256", "SHA256withECDSA", 32, key); | ||
| ECPublicKey publicKey = key instanceof PublicKey ? (ECPublicKey) key : null; |
| public static Algorithm ECDSA384(ECKey key) throws IllegalArgumentException { | ||
| return new ECDSAAlgorithm("ES384", "SHA384withECDSA", 48, key); | ||
| ECPublicKey publicKey = key instanceof PublicKey ? (ECPublicKey) key : null; | ||
| ECPrivateKey privateKey = key instanceof PrivateKey ? (ECPrivateKey) key : null; |
| if (key == null) { | ||
| throw new IllegalArgumentException("The ECKey cannot be null"); | ||
| if (publicKey == null && privateKey == null) { | ||
| throw new IllegalArgumentException("Both provided Keys cannot be null."); |
There was a problem hiding this comment.
I think IllegalStateException is better, since the keys are not arguments of this method, they are state of the instance.
| if (!(key instanceof ECPublicKey)) { | ||
| throw new IllegalArgumentException("The given ECKey is not an ECPublicKey."); | ||
| if (publicKey == null) { | ||
| throw new IllegalArgumentException("The given Public Key is null."); |
| if (!(key instanceof ECPrivateKey)) { | ||
| throw new IllegalArgumentException("The given ECKey is not a ECPrivateKey."); | ||
| if (privateKey == null) { | ||
| throw new IllegalArgumentException("The given Private Key is null."); |
| if (!(key instanceof PublicKey)) { | ||
| throw new IllegalArgumentException("The given RSAKey is not a RSAPublicKey."); | ||
| if (publicKey == null) { | ||
| throw new IllegalArgumentException("The given Public Key is null."); |
| if (!(key instanceof PrivateKey)) { | ||
| throw new IllegalArgumentException("The given RSAKey is not a RSAPrivateKey."); | ||
| if (privateKey == null) { | ||
| throw new IllegalArgumentException("The given Private Key is null."); |
hzalaz
approved these changes
Mar 13, 2017
lbalmaceda
force-pushed
the
accept-both-keys
branch
from
March 14, 2017 14:14
8dfb783 to
c4f2094
Compare
Closed
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
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.
Before this PR, the
Algorithminstance received one key (public or private). With the introduced changes, theAlgorithmis now instantiated with both keys at the same time. So the user can define the instance once and reuse it as needed for both signing and verifying purposes.IllegalArgumentExceptionis thrown upon instantiation.verify,IllegalStateExceptionis thrown.sign,IllegalStateExceptionis thrown.