Move verification of IdToken to before accessing tokens (omniauth#28)

Eric-Guo · Aug 2, 2019 · c6c64a4 · c6c64a4
1 parent bd8b57f
commit c6c64a4
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 7 deletions.
diff --git a/lib/omniauth/strategies/openid_connect.rb b/lib/omniauth/strategies/openid_connect.rb
@@ -112,6 +112,12 @@ def callback_phase
         return fail!(:missing_code, OmniAuth::OpenIDConnect::MissingCodeError.new(params['error'])) unless params['code']
 
         options.issuer = issuer if options.issuer.nil? || options.issuer.empty?
+
+        decode_id_token(params['id_token'])
+          .verify! issuer: options.issuer,
+                   client_id: client_options.identifier,
+                   nonce: stored_nonce
+
         discover!
         client.redirect_uri = redirect_uri
         client.authorization_code = authorization_code
@@ -197,13 +203,6 @@ def access_token
           scope: (options.scope if options.send_scope_to_token_endpoint),
           client_auth_method: options.client_auth_method
         )
-        id_token = decode_id_token(@access_token.id_token)
-        id_token.verify!(
-          issuer: options.issuer,
-          client_id: client_options.identifier,
-          nonce: stored_nonce
-        )
-        @access_token
       end
 
       def decode_id_token(id_token)

diff --git a/test/lib/omniauth/strategies/openid_connect_test.rb b/test/lib/omniauth/strategies/openid_connect_test.rb
@@ -136,6 +136,7 @@ def test_callback_phase(session = {}, params = {})
         id_token = stub('OpenIDConnect::ResponseObject::IdToken')
         id_token.stubs(:verify!).with(issuer: strategy.options.issuer, client_id: @identifier, nonce: nonce).returns(true)
         ::OpenIDConnect::ResponseObject::IdToken.stubs(:decode).returns(id_token)
+        id_token.expects(:verify!)
 
         strategy.unstub(:user_info)
         access_token = stub('OpenIDConnect::AccessToken')
@@ -241,6 +242,11 @@ def test_callback_phase_with_timeout
         strategy.stubs(:access_token).raises(::Timeout::Error.new('error'))
         strategy.call!('rack.session' => { 'omniauth.state' => state, 'omniauth.nonce' => nonce })
         strategy.expects(:fail!)
+
+        id_token = stub('OpenIDConnect::ResponseObject::IdToken')
+        id_token.stubs(:verify!).with(issuer: 'example.com', client_id: @identifier, nonce: nonce).returns(true)
+        ::OpenIDConnect::ResponseObject::IdToken.stubs(:decode).returns(id_token)
+
         strategy.callback_phase
       end
 
@@ -256,6 +262,11 @@ def test_callback_phase_with_etimeout
         strategy.stubs(:access_token).raises(::Errno::ETIMEDOUT.new('error'))
         strategy.call!('rack.session' => { 'omniauth.state' => state, 'omniauth.nonce' => nonce })
         strategy.expects(:fail!)
+
+        id_token = stub('OpenIDConnect::ResponseObject::IdToken')
+        id_token.stubs(:verify!).with(issuer: 'example.com', client_id: @identifier, nonce: nonce).returns(true)
+        ::OpenIDConnect::ResponseObject::IdToken.stubs(:decode).returns(id_token)
+
         strategy.callback_phase
       end
 
@@ -271,6 +282,11 @@ def test_callback_phase_with_socket_error
         strategy.stubs(:access_token).raises(::SocketError.new('error'))
         strategy.call!('rack.session' => { 'omniauth.state' => state, 'omniauth.nonce' => nonce })
         strategy.expects(:fail!)
+
+        id_token = stub('OpenIDConnect::ResponseObject::IdToken')
+        id_token.stubs(:verify!).with(issuer: 'example.com', client_id: @identifier, nonce: nonce).returns(true)
+        ::OpenIDConnect::ResponseObject::IdToken.stubs(:decode).returns(id_token)
+
         strategy.callback_phase
       end
 
@@ -286,6 +302,11 @@ def test_callback_phase_with_rack_oauth2_client_error
         strategy.stubs(:access_token).raises(::Rack::OAuth2::Client::Error.new('error', error: 'Unknown'))
         strategy.call!('rack.session' => { 'omniauth.state' => state, 'omniauth.nonce' => nonce })
         strategy.expects(:fail!)
+
+        id_token = stub('OpenIDConnect::ResponseObject::IdToken')
+        id_token.stubs(:verify!).with(issuer: 'example.com', client_id: @identifier, nonce: nonce).returns(true)
+        ::OpenIDConnect::ResponseObject::IdToken.stubs(:decode).returns(id_token)
+
         strategy.callback_phase
       end