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

Instantiate RSA/EC Algorithm with both keys #147

Merged
merged 3 commits into from
Mar 14, 2017
Merged

Conversation

lbalmaceda
Copy link
Contributor

@lbalmaceda lbalmaceda commented Mar 10, 2017

Before this PR, the Algorithm instance received one key (public or private). With the introduced changes, the Algorithm is 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.

  • If both keys are null, IllegalArgumentException is thrown upon instantiation.
  • If the public key is null when the user calls verify, IllegalStateException is thrown.
  • If the private key is null when the user calls sign, IllegalStateException is thrown.

@lbalmaceda lbalmaceda requested a review from hzalaz March 10, 2017 19:29
@NE-SmallTown
Copy link

NE-SmallTown commented Mar 11, 2017

I thinks this is greater.Please merge soon,I can't wait to use it now 👏

public static Algorithm RSA256(RSAKey key) throws IllegalArgumentException {
return new RSAAlgorithm("RS256", "SHA256withRSA", key);
RSAPublicKey publicKey = key instanceof PublicKey ? (RSAPublicKey) key : null;

Choose a reason for hiding this comment

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

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;

Choose a reason for hiding this comment

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

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;

Choose a reason for hiding this comment

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

same

public static Algorithm ECDSA256(ECKey key) throws IllegalArgumentException {
return new ECDSAAlgorithm("ES256", "SHA256withECDSA", 32, key);
ECPublicKey publicKey = key instanceof PublicKey ? (ECPublicKey) key : null;

Choose a reason for hiding this comment

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

same

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;

Choose a reason for hiding this comment

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

same

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.");
Copy link

@nikolaseu nikolaseu Mar 13, 2017

Choose a reason for hiding this comment

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

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.");

Choose a reason for hiding this comment

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

IllegalStateException

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.");

Choose a reason for hiding this comment

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

IllegalStateException

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.");

Choose a reason for hiding this comment

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

IllegalStateException

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.");

Choose a reason for hiding this comment

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

IllegalStateException

@hzalaz hzalaz added this to the v3-Next milestone Mar 13, 2017
@lbalmaceda lbalmaceda merged commit aa5ff04 into master Mar 14, 2017
@lbalmaceda lbalmaceda deleted the accept-both-keys branch March 14, 2017 14:26
@Gungrave223 Gungrave223 mentioned this pull request Mar 29, 2017
@lbalmaceda lbalmaceda modified the milestones: v3-Next, 3.2.0 May 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants