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

Add alg RS256 support for JWT generator and validator. #281

Closed
wants to merge 1 commit into from
Closed

Add alg RS256 support for JWT generator and validator. #281

wants to merge 1 commit into from

Conversation

clems4ever
Copy link
Contributor

Currently, the JWT library only support creating and validating HS256 tokens. I implemented the creation and validation of RS256 tokens.

The interface is accepting a shared pointer of RSA as input for RSA keys to be as generic as possible but I also provide helper functions to convert PEM representation of keys into RSA keys so that it is easy to read the PEM on disk. I will probably implement the JWK representation of keys in a coming review.

@clems4ever
Copy link
Contributor Author

Since this change is not really trivial, I will go through the contributing process. I'm currently creating my accounts.

@tillt
Copy link
Contributor

tillt commented Apr 14, 2018

@clems4ever this looks really good - let's get it into a committable state. I have added you to Apache Mesos as a contributor and assigned me as a shepherd to your improvement.

@clems4ever
Copy link
Contributor Author

clems4ever commented Apr 14, 2018

Hello @tillt , thank you for having a look. Don't you need to merge the following PR to make me be a contributor: https://github.com/apache/mesos/pull/282/commits?

@tillt
Copy link
Contributor

tillt commented Apr 14, 2018

@clems4ever I totally missed your second PR - sorry for that. Merged it now.

@clems4ever
Copy link
Contributor Author

@tillt , no pb, I will create a review in the review board now. Thanks.

@tillt
Copy link
Contributor

tillt commented Apr 14, 2018

@clems4ever can we have tests as well? :)

@clems4ever
Copy link
Contributor Author

@tilt, what do you mean? There are tests in the commit already.

@tillt
Copy link
Contributor

tillt commented Apr 14, 2018

@clems4ever and missed that as well :D - it was hidden due to its size (large diff).

@clems4ever
Copy link
Contributor Author

@tilt, yes, large diff because I refactored a bit and added as many tests as there are for HS256. :)

@clems4ever
Copy link
Contributor Author

@tilt, here you go: https://reviews.apache.org/r/66621/.

@clems4ever
Copy link
Contributor Author

Closing this PR since reviews are in the review board.

@clems4ever clems4ever closed this Apr 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants