Proposal of simple JWK support #289
Conversation
|
Hello, @anakinj! This is your first Pull Request that will be reviewed by Ebert, an automatic Code Review service. It will leave comments on this diff with potential issues and style violations found in the code as you push new commits. You can also see all the issues found on this Pull Request on its review page. Please check our documentation for more information. |
![sourcelevel-bot[bot]](https://avatars.githubusercontent.com/in/597?s=60&v=4){kind=small}
|
Ebert has finished reviewing this Pull Request and has found:
You can see more details about this review at https://ebertapp.io/github/jwt/ruby-jwt/pulls/289. |
|
Hi @anakinj, thanks for the PR! This looks very good. It would be great if you could include an update in the README.md that JWK is supported for RSA keys. Please ignore the build errors in travis. This seems to be related to the used OpenSSL version in the containers. |
|
Hi @excpt. Thanks for the feedback. I'll do something related to the documentation and was also thinking to add support for EC public keys. It seems that the Travis build failure is because Ruby 2.2 does not support the openssl gem specified in the gemspec. Maybe we could just drop Ruby 2.2 support (drop the build), the road for ruby 2.2 is ending anyway (https://www.ruby-lang.org/en/news/2018/06/20/support-of-ruby-2-2-has-ended/). |
| def initialize(options) | ||
| jwks_or_loader = options[:jwks] | ||
| @jwks = jwks_or_loader if jwks_or_loader.is_a?(Hash) | ||
| @jwk_loader = jwks_or_loader if jwks_or_loader.respond_to?(:call) |
There was a problem hiding this comment.
JWT::JWK::KeyFinder#initialize manually dispatches method call
There was a problem hiding this comment.
I would not like to have the same thing given as two different parameters, therefore the check if it's lazy-loadable or not.
anakinj
force-pushed
the
jwk-support
branch
2 times, most recently
from
55a2d06 to
417e32c
Compare
|
Got some time to refactor this, moved the JWK related things into its own module and separated the RSA handling to a completely own class. Also added a code example to the README.md. Did not add the EC support just yet, i did not quite understand how the parameters should be represented for those keys. |
|
Sweet! Thanks @anakinj for working on this. Is this just pending review atm? |
|
@anthonylebrun, I have nothing pending related to this anymore. Additional algorithms and such can be added later, if there is a need for it. |
|
Great to see this merged. When will it be released? |
|
Also keen to have this released! |
|
Just wanted to drop in here and thank @anakinj and everyone that helped get this over the line. I just used this in a new integration and found it really straight forward to decode JWTs and validate with JWKs. 🙏 thank you for allowing the JWKs to be loaded from a |
|
Great to hear that this giving value and thank you for the kind words @bjeanes |
|
There is a beta release available. Changelog: https://github.com/jwt/ruby-jwt/releases/tag/v2.2.0.pre.beta.0 |
|
Thanks @excpt ! |
|
Thanks! We've been using the gem from github source and it's been working well for us. Thanks for your work on this. |
This PR is related to issue #158
Been using this gem for handling JWT tokens but with the lack of JWK support been forced to also include another JWT gem in the picture (https://rubygems.org/gems/json-jwt).
With something like this we could get rid of needing to include a second JWT dependency in the mix.
Currently this only supports RSA keys, but could easily be extended if considered a good approach.