[ELY-2929] Initial elliptic curve code and test for JWT usage.#2415
[ELY-2929] Initial elliptic curve code and test for JWT usage.#2415rsearls wants to merge 2 commits intowildfly-security:2.xfrom
Conversation
There was a problem hiding this comment.
Thanks @rsearls ! I just added minor comments. I've noticed that this is marked as feature request in jira. Should this follow feature development process or are we good to proceed without it?
And I was just wondering if we should use an external library like nimbus jose jwt to add EC support to avoid bugs and having to maintain this. But for RSA we also do not use external library so this might be something to consider later. So this lgtm
|
|
||
| // Perform the verification | ||
| boolean verfiedSig = verifier.verify(derSignature); | ||
| System.out.println("Signature verifcation is: " + verfiedSig); |
There was a problem hiding this comment.
just a minor, change this to logging instead
There was a problem hiding this comment.
you can also update this line to use the logger instead
| /** | ||
| * Process an RSA or EC (Ellipitic Curve) based JWT token. | ||
| * @param evidence | ||
| * @return |
There was a problem hiding this comment.
just a minor, missing return javadoc
| @@ -0,0 +1,35 @@ | |||
| package org.wildfly.security.evidence; | |||
| @@ -0,0 +1,50 @@ | |||
| package org.wildfly.security.evidence; | |||
|
Darran created the jira. We should ask him if this should be treated as As to switching to a 3rd party JSON library. I used the pre-existing JSON |
|
I have converted the issue to an Enhancement, I think for changes like this we will need to be able to merge to Elytron to keep our codebase relevent - trying to map this to stability levels would intriduce more pain than benefits. |
Okay sounds good, I've approved this before, if you agree you can merge before it gets obsolete |
3 participants
https://issues.redhat.com/browse/ELY-2929