waiting-for-dev / warden-jwt_auth

JWT token authentication with warden
MIT License
113 stars 56 forks source link

Allow configuration of the algorithm to be used for encoding/decoding #19

Closed hrakotobe closed 5 years ago

hrakotobe commented 5 years ago

Allow the use of different signing algorithm when encoding/decoding JWTs.

HS256 is currently hard coded for the encoding and decoding of the token. This is a proposition to allow overriding the default and specify more robust (in particular RS256) algorithms.

Please tell me if there is any coding style/formatting issues or comment on this approach.

waiting-for-dev commented 5 years ago

@hrakotobe thanks for your work on that. I'm ok with it. Having the option of configuring the algorithm makes a lot of sense. But, just out of curiosity, why do you need to use an asymmetric algorithm? Usually this gem is used in a server which acts both as identity provider and resource provider. Do you have any other actor who needs to verify the token signature? In terms of robustness, if I'm not wrong, there is no difference between HS256 & RS256. You could have more security with HS512 while keeping things symmetric.

About the implementation, I think everything is perfect in the library code. However, I think there is no need to change anything in the test suite. It is just a setting to allow other kind of algorithms, which are all tested in ruby-jwt library. From our end, we are save testing everything works fine with the default algorithm.

hrakotobe commented 5 years ago

@waiting-for-dev thanks for looking at it. For the choice of RS256, yes, we plan to share the token between several system, some of which will only validate the tokens without generating new ones (typically they'll only have the public key)

For the tests, ok, that makes sense. I removed them.

waiting-for-dev commented 5 years ago

Could you just rename again e to exception in the strategy? This makes the build to fail.

hrakotobe commented 5 years ago

It seems there is a configuration conflict on the code style checking between Reek and Rubocop. Should I change the rubocop rule to match Reek's ?

https://travis-ci.org/waiting-for-dev/warden-jwt_auth/jobs/565924531#L649

Analyze with Reek..............................................[Reek] FAILED
650Errors on modified lines:
651  /home/travis/build/waiting-for-dev/warden-jwt_auth/lib/warden/jwt_auth/strategy.rb:24: UncommunicativeVariableName: Warden::JWTAuth::Strategy#authenticate! has the variable name 'e' [https://github.com/troessner/reek/blob/v4.8.2/docs/Uncommunicative-Variable-Name.md]

https://travis-ci.org/waiting-for-dev/warden-jwt_auth/jobs/566357274

Analyze with RuboCop........................................[RuboCop] FAILED
652Errors on modified lines:
653/home/travis/build/waiting-for-dev/warden-jwt_auth/lib/warden/jwt_auth/strategy.rb:24:34: C: Naming/RescuedExceptionsVariableName: Use `e` instead of `exception`.
waiting-for-dev commented 5 years ago

Thanks @hrakotobe for your great work on this. I merged it and released as v0.4.0. If you are using devise-jwt, you'll need v0.6.0.