Verify ISS should be off by default

[k1w1]

It looks like 1.4.0 adds a new feature to verify the iss claim. The problem is that this feature is enabled by default, so if existing code doesn't specify an iss value for the options then the verification fails.

It seems that for backwards compatibility :verify_iss should be false.

[excpt]

Can you please provide sample code when this is the case in your application?

[ojab]

The same here. Basically I'm doing this:

 token = OAuth2Client.auth_code.get_token(params[:code], redirect_uri: 'https://domain.tld/oauth2callback')
JWT.decode(token['id_token'], nil, false)

[ZhangHanDong]

@ojab whether your token['id_token'] include iss ?

example_payload = {"hello" => "world"}
 example_secret = 'secret'
jwt = JWT.encode(example_payload, example_secret)
JWT.decode(jwt, example_secret)
=> [{"hello"=>"world"}, {"typ"=>"JWT", "alg"=>"HS256"}]

example_payload = {"hello" => "world", "iss" => 'token'}
jwt = JWT.encode(example_payload, example_secret)
JWT.decode(jwt, example_secret)  #=>  JWT::InvalidIssuerError: Invalid issuer

source code:

if options[:verify_iss] && payload.include?('iss')
      raise JWT::InvalidIssuerError.new("Invalid issuer. Expected #{options['iss']}, received #{payload['iss']}") unless payload['iss'].to_s == options['iss'].to_s
end

[excpt]

@ojab @k1w1 It's a bug. Thanks for reporting.

If verify is set to false the token will still be validated when iss is present in the payload.

My test code:

token = JWT.encode({iss: 'test_me', id: 1}, 'password')

puts token

begin
  puts JWT.decode token, 'password'
rescue => e
  puts 'I wont work - you didnt tell me the original issuer who created that token.'
  puts e.inspect
end

begin
  puts JWT.decode token, 'password', true, {'iss' => 'test_me'}
  puts 'I work because you told me who the original issuer is to validate against.'
rescue => e
  puts e.inspect
end

# this will raise an exception
# <JWT::InvalidIssuerError: Invalid issuer. Expected , received test_me>
begin
  puts JWT.decode token, 'password', false
  puts 'I should work because I dont care if my values are okay.'
rescue => e
  # BUT I DONT WORK!
  puts e.inspect
end

I think we should set the default option to the verify value when you call decode.

verify = false

JWT.decode my_token, my_password, verify

JWT.decode should set all default_options to false but by default they are always set to true. This causes the crashes.

You can enable specific verifications using the options parameter in the JWT.decode method.

verify = false
options = {iss: 'my_issuer', verify_iss: true}

JWT.decode my_token, my_password, verify, options

This should be the correct way.

@k1w1 and @ojab have to change just a line of code.

Old:

JWT.decode token, password

New:

JWT.decode token, password, false

That should to the trick when the fix is implemented.

[danleyden]

I'm unconvinced that the verification method is actually correct per the spec.

https://tools.ietf.org/html/draft-ietf-oauth-json-web-token-32#section-4.1.1 states that "The processing of this claim is generally application specific.", so likely not a good candidate for implementing in a library in this way.

Take, for example, the case where the issuer may be one of thousands of possible issuers, where e.g. the issuer might be an individual user or third party client systems. In that case, application may choose to allow any of the authorized list of issuers. In the current implementation, it would be incumbent upon the application to first interrogate the token, then confirm that it is in the 'allowed list', then pass that value back in to the decoder to do a string match, essentially making the additional check in this library redundant.

An alternative implementation that might work more generally (and be in line with the spec) would be to simply verify that the claim exists and has a valid value (is a string) and pass the value to a callback provided by the application to validate the value.

There is a similar issue with the current verification of the sub claim.

[excpt]

Thank you for taking the time to clarify the specification details.

In this case I think we should drop the verification for aud, sub, iss and jti (#68) until we have a solution.

[ZhangHanDong]

@danleyden thanks for your explain.
@excpt
Sorry for pull in the solutions of defects , beacuse I did'nt consider the case of more .

[brancusi]

Thanks, having the same issue here. Specifically I cannot verify against sub but it's in the payload.

[danleyden]

@ZhangHanDong - no worries. I've read through the RFCs, and had to re-read them several times as they are quite extensive and not immediately obvious, particularly with the interplay of the different RFCs involved here. I can see where you were going and, for many applications, it is exactly the right thing to do.

[excpt]

Quick fix:

I set the default behavior for the the optional claim verification to false and release 1.4.1 to get rid of this bug so everyone can keep on using the jwt gem without breaking the code.

As a next step we should bump our heads together and find a solution to make these validatons work without introducing new bugs.

[excpt]

@k1w1 @ojab @brancusi 1.4.1 Released. Optional claim verification set to false by default.