Shouldn't verification of additional claims, like iss, aud etc. be enforced when in options?

[lwe]

First of all - thanks for the great gem!

What I've stumbled upon while using the gem is the verification of additional attributes, like e.g. the "iss" claim. When "iss" is supplied to the options in JWT.decode it is not enforced or checked, even when verify_iss is set to true as long as it's not present in the actual payload. Basically I've had the following decoding code:

options = {
  algorithm: 'HS256',
  verify_iss: true,
  'iss' => 'acme.org'
}
payload, = JWT.decode(token, "secret", true, options)

and provided it a token like:

token = JWT.encode({ data: 'payload' }, 'secret', 'HS256')

and expected it to raise an error, because iss is not set in the payload - is this assumption wrong? Should the presence of these attributes be verified again by the application? Looking at jwt.rb#166 the issuer is only verified when it's present in the payload. From my point of view - and I think the more "safe" variant - would be to always trust the options and when "iss" is present in the options then enforcing the check against the issuer.

PS: I'm not savvy with the JWT spec, so it can be that the JWT spec actually proposes the current design. Then just ignore & close my issue :)
PPS: And if this proposal makes sense, I'd be happy to create a PR

[lwe]

In the meantime I've checked a few other libraries, noteably:

• Java, https://bitbucket.org/b_c/jose4j/src/ab4080d997c9eb56b47f0ca942cdd21de2586178/src/main/java/org/jose4j/jwt/consumer/IssValidator.java?at=master
• Python
• Node.JS, https://github.com/auth0/node-jsonwebtoken/blob/master/index.js#L181

and they all check the issuer claim when it's present in the options.

[excpt]

This is a bug. After inspecting the code, the verification is triggered only when the iss payload is present. See: https://github.com/progrium/ruby-jwt/blob/master/lib/jwt.rb#L166

This affects also all the other verifications.

Thanks for reporting. If you find the time to provide a PR I'd be happy to merge it.

[lwe]

All right, I'd propose the following

1. Setting verify_iss and verify_aud by default to true
2. Change the conditions to be like if options[:verify_iss] && options["iss"]...

This change then has verification of issuer enabled - as soon as iss option is set, or would you prefer to leave verify_iss to be false by default?

PS: Btw, what is the reasoning behind using "iss" as option and not :iss?

[excpt]

I agree with your proposal. With these changes it would become more easy to setup the library and check created tokens. But this would break the current api. In this case it would be required to switch to version 2.0. I don't have a problem with this next version step.

The use of 'iss' instead of :iss was a mistake in the code review process. It simply slipped through and now it is in the current version. A fix for this could be this active support library. http://api.rubyonrails.org/classes/ActiveSupport/HashWithIndifferentAccess.html But this won't be needed if we make the version 2.0 step. In this case we can change everything to use the hash syntax and drop the ruby 1.8.x support.

[lwe]

Then I'd propose the following:

• Bugfix release 1.5.x, where verify_iss and verify_aud are still set to to false -> will create a PR for this part
• Breaking changes to be introduced in a 2.0 release, looking at how the current verification works, I think there are a few other areas that could be slightly adapted (similar issues, e.g. there's no way of saying: Hey, I require an exp)

[excpt]

Sounds excellent. 👍

If you have some ideas for version 2.0 send me a mail.

[excpt]

I just created a google group. https://groups.google.com/forum/#!forum/ruby-jwt Drop your ideas there. :)

[lwe]

Created a PR, so I'm closing this.