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

Options hash uses both symbols and strings as keys. #95

Closed
aj-michael opened this issue Aug 5, 2015 · 6 comments
Closed

Options hash uses both symbols and strings as keys. #95

aj-michael opened this issue Aug 5, 2015 · 6 comments

Comments

@aj-michael
Copy link
Member

I tried to verify the issuer on a JWT that I was decoding today, and I found this line (and others like it) very ugly.

if options[:verify_iss] && options['iss']

This means that my code to decode the JWT looked like this:

JWT.decode(token, public_key, true, :verify_iss => true, 'iss' => 'my issuer')

In my opinion, the consumer of this API should be be able to do one of the following:

  1. Always use symbols as keys in the options hash.
  2. Always use strings as keys in the options hash.
  3. Be able to either use symbols or keys in the options in hash and have our library deal with it under the hood.

Thoughts?

@yordis
Copy link

yordis commented Sep 4, 2015

👍 for symbol only until someone complain about it 😄

@excpt
Copy link
Member

excpt commented Sep 4, 2015

👍 symbols only :)

@skippy
Copy link
Contributor

skippy commented Sep 15, 2015

+1

This is an issue beyond semantics... @aj-michael it is a security hole.

I pass in the following:
{:aud=>"test_aud_id", :verify_aud=>true}

and it decodes the token successfully. The hole is it decodes regardless of what the aud is. You can see this bug in line jwt.rb#172 if options[:verify_aud] && options['aud']

if I pass in:
{:aud=>"test_aud_id", :verify_aud=>true, :verify_iat=>true, :sub=>"test_sub_id", :verify_sub=>true}

this fails because how the verify_sub checks, it doesn't allow it to silently fail (even though sub is set in the token)

@excpt
Copy link
Member

excpt commented Sep 16, 2015

@aj-michael @yordis @skippy :)

@skippy
Copy link
Contributor

skippy commented Sep 16, 2015

@excpt fair enough! I wasn't trying to be snarky. Thank you for the fast PR!

@yordis
Copy link

yordis commented Sep 17, 2015

@excpt awesome, very good clean there 👍

@excpt excpt added this to the Version 2.0.0 milestone Sep 17, 2015
@excpt excpt closed this as completed Sep 17, 2015
@anakinj anakinj removed this from the Version 3.0.0 milestone Feb 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants