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

JWT::InvalidIatError on some requests #195

Closed
tmazeika opened this issue Sep 29, 2015 · 25 comments
Closed

JWT::InvalidIatError on some requests #195

tmazeika opened this issue Sep 29, 2015 · 25 comments

Comments

@tmazeika
Copy link

I get the following error in Ruby on Rails on many requests when authenticating with Google:

Ruby on Rails error

I use GitHub and Google OAuth; GitHub works fine while omniauth-google-oauth2 seems to be having some issues. It doesn't seem to happen all the time, but it occurred after server restarts (both through Puma and the actual machine). I am convinced it is not my code as it seems to error out before it ever reaches my controller.

Unfortunately I cannot pinpoint a specific action that causes it to happen.

@hongmingChm
Copy link

I was binding.pry through jwt. And some strange thing happen.

payload['iat'].is_a?(Integer) && payload['iat'].to_i <= Time.now.to_i

for this line of code that complaint the error, when I check the payload variable fast enough, I will be able to see that it return false. But as i keep running the same code over and over again ( press up and enter again in console ), it will return true after few tries and I am logged in.

To me it seems like there's some delay in the payload variable. I am still digging in into whats happening.

And during the pry more. Pry was run twice. Seems like it was called twice for some reason.

@hongmingChm
Copy link

Update
During the binding.pry
I did
puts "#{payload['iat'].to_i} #{Time.now.to_i}"
as soon i get into pry mode.
1443514977 1443514964
Above is the output
it seems theres a slight time different that caused the time comparison to be false
payload['iat'].to_i <= Time.now.to_i
So this explain why after wait few second the authentication pass

@jchatel
Copy link

jchatel commented Sep 29, 2015

+1 Got this problem too. On version 0.2.2
This is a very recent problem

@jchatel
Copy link

jchatel commented Sep 29, 2015

haha totally spot on. To get pass through on my local machine (because I need to work), I copy text in my console (I'm on windows, and when you do that, it stops processing of the app). This introduce some lag, then I can cancel my copy, my server resumes and I can finally log in.

@hongmingChm
Copy link

Hi all,
I know this is not very helpful.
But what our team decision is.
We shall wait JWT to implement the leeway methods as while we looking at JWT gem, the latest version will be implementing the leeway for the iat timestamp.
https://github.com/jwt/ruby-jwt/blob/master/lib/jwt.rb#L170
And also we will be waiting ominiauth google gem to implement the leeway method.
So for now, we are bumping it back to version 0.2.6 untill there's update from both side.

@zquestz
Copy link
Owner

zquestz commented Oct 1, 2015

So for now, we could easily disable the timestamp checking, better than flakey behavior.

I assume the bug is here:

JWT.decode(
            access_token['id_token'], nil, false, {
              :verify_iss => true,
              'iss' => 'accounts.google.com',
              :verify_aud => true,
              'aud' => options.client_id,
              :verify_sub => false,
              :verify_expiration => true,
              :verify_not_before => true,
              :verify_iat => true,
              :verify_jti => false
            }).first

If we change verify_iat to false I am guessing this would resolve the issue. What do you guys think? I can put out a fix pretty quick.

@zquestz
Copy link
Owner

zquestz commented Oct 1, 2015

I could also put a gate on the JWT parsing, and have that need to be explicitly enabled, as many users don't require it. Going to actually do that now.

@zquestz
Copy link
Owner

zquestz commented Oct 1, 2015

Ok I have a fix, I think we should disable the JWT processing by default and have the user opt in if they require this functionality. This work for you guys?

@jchatel
Copy link

jchatel commented Oct 1, 2015

I fixed it by simply updating the clock on my PC.

@zquestz
Copy link
Owner

zquestz commented Oct 1, 2015

In many environments, especially VM environments there can be time drift that is very hard to avoid. I think that many users won't require the extra fields. If they need them they can enable the JWT processing and either make sure their clocks are sync'd or use the new JWT gem that implements leeway support.

@zquestz
Copy link
Owner

zquestz commented Oct 1, 2015

@bionicrm @hongmingChm have you tried syncing your clock? Does that resolve it for you?

@zquestz
Copy link
Owner

zquestz commented Oct 1, 2015

Actually decided to just allow the user to opt out of JWT decoding. The default behavior is the same as today. That way I don't break the contract for existing users, but can provide a way for you to avoid your issue.

@zquestz
Copy link
Owner

zquestz commented Oct 1, 2015

Issue now has a usable workaround. Closing. Can reopen if you need anything else on this ticket.

@zquestz zquestz closed this as completed Oct 1, 2015
@tmazeika
Copy link
Author

tmazeika commented Oct 1, 2015

@zquestz Thanks. I tried syncing the time and then restarting which seemed to work, but the same thing happened last time and it seemed to stop working after a while. I am on a VM with Vagrant.

@zquestz
Copy link
Owner

zquestz commented Oct 2, 2015

Well now you can just set :skip_jwt to true in the options if you don't need those fields. =)

@gf59ru
Copy link

gf59ru commented Oct 6, 2015

you can just set :skip_jwt to true

Where I can set this option if I using devise?

In devise.rb:

Devise.setup do |config|
  config.omniauth :google_oauth2, ENV['CLIENT_ID'], ENV['CLIENT_SECRET']

In user model:

class User < ActiveRecord::Base
  devise ... :omniauth_providers => [:google_oauth2, ...]

Or somewhere else?

@dmz006
Copy link

dmz006 commented Oct 7, 2015

From the gem README it shows an example of addition the hash of options. I assume you can add a :skip_jwt => true

Rails.application.config.middleware.use OmniAuth::Builder do
provider :google_oauth2, ENV["GOOGLE_CLIENT_ID"], ENV["GOOGLE_CLIENT_SECRET"],
{
:name => "google",
:scope => "email, profile, plus.me, http://gdata.youtube.com",
:prompt => "select_account",
:image_aspect_ratio => "square",
:image_size => 50,
:skip_jwt => true
}
end

However it didn't seem to help me any

@zquestz
Copy link
Owner

zquestz commented Oct 7, 2015

Make sure you are using the latest release of the gem and you can do:

config.omniauth :google_oauth2, "GOOGLE_CLIENT_ID", "GOOGLE_CLIENT_SECRET", { :skip_jwt => true }

@dmz006
Copy link

dmz006 commented Oct 7, 2015

Upgrading to 0.2.8 fixed it for me with the omniauth options.

@gf59ru
Copy link

gf59ru commented Oct 7, 2015

Thank You, @zquestz! it works!

@blimmer
Copy link

blimmer commented Feb 4, 2016

I'm suddenly getting this error with a new version of discourse (bug here). Adding :skip_jwt => true resolves the issue, but it seems that it shouldn't 500 the server without this. Any ideas?

@zquestz
Copy link
Owner

zquestz commented Feb 4, 2016

Sync your time. If your local time is off, then this is the side effect.

@ttilberg
Copy link

@zquestz I've been learning about google oAuth stuff today and came across this series of issues in my app, throwing the InvalidIat... I dug in a little further, and found the time Google was sending to be in the future by about a minute -- as you are implying above.

I don't currently understand what the JWT really provides here, but skipping it entirely seems heavy handed when there is a config option in the JWT Class to define a leeway amount.

Would it make sense to instead send a leeway parameter to initialize JWT with instead of skipping entirely?

Some quick references:

leeway defaults to 0 in JWT#decode options


JWT::Verify#verify_iat

    def verify_iat
      return unless @payload.include?('iat')

      if !(@payload['iat'].is_a?(Integer)) || @payload['iat'].to_i > (Time.now.to_i + leeway)
        fail(JWT::InvalidIatError, 'Invalid iat')
      end
    end

Would this make jwt_iat_leeway: 300 (five minutes) feasible as a param to pass to config.OmniAuth ?

 if !options[:skip_jwt] && !access_token['id_token'].nil?
          hash[:id_info] = JWT.decode(
            access_token['id_token'], nil, false, {
              :verify_iss => true,
              'iss' => 'accounts.google.com',
              :verify_aud => true,
              'aud' => options.client_id,
              :verify_sub => false,
              :verify_expiration => true,
              :verify_not_before => true,
              :verify_iat => true,
              :verify_jti => false,
              :leeway: options[:jwt_iat_leeway]  # or some other named option that is obvious what's up.. I don't know how fluent general people are with JWT and IAT aside from getting the red screen of death ;)
            }).first

I know this, and other related issues have been closed, but I wonder if this wouldn't be a better solution. If I'm following the source correclty, It wouldn't break existing implementations, and it would still allow for JWT decoding and verification (for whatever it does haha).

I've not yet contributed to gem development, so I'm not able to test this theory, and at this time, know literally 10 minutes worth of oauth, so I hope you don't see this as wasted time.

@zquestz
Copy link
Owner

zquestz commented Mar 11, 2016

So the leeway options are definitely good now. However that was just recently added to the JWT library. We can update the dependencies to use that version, and then update omniauth-google-oauth2 to pass these options. The commit to add leeway support to ruby-jwt was only added in July 2015.

jwt/ruby-jwt@9c720a6

I will try to get these changes done today. Seems enough people will want it. =)

@ttilberg
Copy link

I've not made a public PR before, but I got my private fork working as expected, so I'm submitting it your way.

More noobness: I don't really know how to write an rspec test for this situation, or in what ways it would be appropriate to test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants