Move verification of IdToken to before accessing tokens

[krzysiek1507]

Closes #27

[krzysiek1507]

@m0n9oose friendly ping ;)

[krzysiek1507]

Any chance for a new release?

[m0n9oose]

@krzysiek1507 yeah, sure. I was going to ask you to test current master branch one more time before i push this update to the rubygems. Can you help me with this?

[krzysiek1507]

@m0n9oose I've tested my changes on in my application already.

[krzysiek1507]

Without this change, my application throws a validation error all the time.

[m0n9oose]

I've tested my changes on in my application already.

yeah, i know but this release has more changes and it would be great to test it properly before push to public

Without this change, my application throws a validation error all the time.

Sorry i didn't get it. Current master is stable for you?

[krzysiek1507]

@m0n9oose tested master and it works for me.

[Eric-Guo]

@m0n9oose I testing the master, same as v0.3.2 (definitely), because it's same code...

I still can not catch the purpose of this PR (#28)

undefined method `count' for nil:NilClass

Extracted source (around line #102):
100
101
102
103
104
105
              
    class << self
      def decode_compact_serialized(jwt_string, key_or_secret, algorithms = nil, encryption_methods = nil, allow_blank_payload = false)
        case jwt_string.count('.') + 1
        when JWS::NUM_OF_SEGMENTS
          JWS.decode_compact_serialized jwt_string, key_or_secret, algorithms, allow_blank_payload
        when JWE::NUM_OF_SEGMENTS

Rails.root: /Users/guochunzhong/git/sso/omniauth_openid_connect-demo

Application Trace | Framework Trace | Full Trace
json-jwt (1.10.2) lib/json/jwt.rb:102:in `decode_compact_serialized'
json-jwt (1.10.2) lib/json/jose.rb:63:in `decode'
openid_connect (1.1.8) lib/openid_connect/response_object/id_token.rb:70:in `decode'
omniauth_openid_connect (0.3.2) lib/omniauth/strategies/openid_connect.rb:210:in `decode_id_token'
omniauth_openid_connect (0.3.2) lib/omniauth/strategies/openid_connect.rb:116:in `callback_phase'

[kmeister2000]

After updating from 0.3.1 to 0.3.2 I am receiving a similar error to @Eric-Guo in a Rails 5.2 app:

undefined method `count' for nil:NilClass, line: "case jwt_string.count('.') + 1"

 class << self
      def decode_compact_serialized(jwt_string, key_or_secret, algorithms = nil, encryption_methods = nil, allow_blank_payload = false)
        case jwt_string.count('.') + 1
        when JWS::NUM_OF_SEGMENTS
          JWS.decode_compact_serialized jwt_string, key_or_secret, algorithms, allow_blank_payload
        when JWE::NUM_OF_SEGMENTS

Backtrace

json-jwt (1.10.2) lib/json/jwt.rb:102:in `decode_compact_serialized'
json-jwt (1.10.2) lib/json/jose.rb:63:in `decode'
openid_connect (1.1.8) lib/openid_connect/response_object/id_token.rb:70:in `decode'
omniauth_openid_connect (0.3.2) lib/omniauth/strategies/openid_connect.rb:210:in `decode_id_token'
omniauth_openid_connect (0.3.2) lib/omniauth/strategies/openid_connect.rb:116:in `callback_phase'
omniauth (1.9.0) lib/omniauth/strategy.rb:238:in `callback_call'
omniauth (1.9.0) lib/omniauth/strategy.rb:189:in `call!'
omniauth (1.9.0) lib/omniauth/strategy.rb:169:in `call'
omniauth (1.9.0) lib/omniauth/builder.rb:64:in `call'
rack (2.0.6) lib/rack/tempfile_reaper.rb:15:in `call'
rack (2.0.6) lib/rack/etag.rb:25:in `call'
rack (2.0.6) lib/rack/conditional_get.rb:25:in `call'
rack (2.0.6) lib/rack/head.rb:12:in `call'
actionpack (5.2.2.1) lib/action_dispatch/http/content_security_policy.rb:18:in `call'
rack (2.0.6) lib/rack/session/abstract/id.rb:232:in `context'
rack (2.0.6) lib/rack/session/abstract/id.rb:226:in `call'
actionpack (5.2.2.1) lib/action_dispatch/middleware/cookies.rb:670:in `call'
actionpack (5.2.2.1) lib/action_dispatch/middleware/callbacks.rb:28:in `block in call'
activesupport (5.2.2.1) lib/active_support/callbacks.rb:98:in `run_callbacks'
actionpack (5.2.2.1) lib/action_dispatch/middleware/callbacks.rb:26:in `call'
actionpack (5.2.2.1) lib/action_dispatch/middleware/executor.rb:14:in `call'
actionpack (5.2.2.1) lib/action_dispatch/middleware/debug_exceptions.rb:61:in `call'
actionpack (5.2.2.1) lib/action_dispatch/middleware/show_exceptions.rb:33:in `call'
railties (5.2.2.1) lib/rails/rack/logger.rb:38:in `call_app'
railties (5.2.2.1) lib/rails/rack/logger.rb:28:in `call'
actionpack (5.2.2.1) lib/action_dispatch/middleware/remote_ip.rb:81:in `call'
actionpack (5.2.2.1) lib/action_dispatch/middleware/request_id.rb:27:in `call'
rack (2.0.6) lib/rack/method_override.rb:22:in `call'
rack (2.0.6) lib/rack/runtime.rb:22:in `call'
activesupport (5.2.2.1) lib/active_support/cache/strategy/local_cache_middleware.rb:29:in `call'
actionpack (5.2.2.1) lib/action_dispatch/middleware/executor.rb:14:in `call'
actionpack (5.2.2.1) lib/action_dispatch/middleware/static.rb:127:in `call'
rack (2.0.6) lib/rack/sendfile.rb:111:in `call'
sentry-raven (2.7.4) lib/raven/integrations/rack.rb:51:in `call'
railties (5.2.2.1) lib/rails/engine.rb:524:in `call'
railties (5.2.2.1) lib/rails/railtie.rb:190:in `public_send'
railties (5.2.2.1) lib/rails/railtie.rb:190:in `method_missing'
/usr/local/rbenv/versions/2.2.7/lib/ruby/gems/2.2.0/gems/passenger-5.0.28/src/ruby_supportlib/phusion_passenger/rack/thread_handler_extension.rb:97:in `process_request'
/usr/local/rbenv/versions/2.2.7/lib/ruby/gems/2.2.0/gems/passenger-5.0.28/src/ruby_supportlib/phusion_passenger/request_handler/thread_handler.rb:152:in `accept_and_process_next_request'
/usr/local/rbenv/versions/2.2.7/lib/ruby/gems/2.2.0/gems/passenger-5.0.28/src/ruby_supportlib/phusion_passenger/request_handler/thread_handler.rb:113:in `main_loop'
/usr/local/rbenv/versions/2.2.7/lib/ruby/gems/2.2.0/gems/passenger-5.0.28/src/ruby_supportlib/phusion_passenger/request_handler.rb:416:in `block (3 levels) in start_threads'
/usr/local/rbenv/versions/2.2.7/lib/ruby/gems/2.2.0/gems/passenger-5.0.28/src/ruby_supportlib/phusion_passenger/utils.rb:113:in `block in create_thread_and_abort_on_exception'

I'm reverting and locking to 0.3.1 until I have more time to see if I can help figure out the cause of the regression.

Also, thanks for this gem - it's been a key part of helping migrate several old Rails apps from rubycas to openid connect.

[krzysiek1507]

@Eric-Guo @kmeister2000 what is your config?

[m0n9oose]

Got it. Will take a look as soon as I can.

[krzysiek1507]

I found the problem. I have in my config response_type: %i[id_token code] so it worked ok, but you guys probably don't have id_token there.

[krzysiek1507]

I've sent PR #34. Sorry!

[stanhu]

I ran into the same problem as #28 (comment) while trying to upgrade GitLab to use v0.3.2. It looks like the work in #34 has carried over to #38. Can we help get that moving?

[m0n9oose]

@stanhu could you take a look at this PR and let me know if it works for you

[stanhu]

@m0n9oose #40 seems to have worked. I also needed to ensure #36 was there in v0.3.2.