diff --git a/lib/omniauth/strategies/openid_connect.rb b/lib/omniauth/strategies/openid_connect.rb index e4705c90..a3383ba5 100644 --- a/lib/omniauth/strategies/openid_connect.rb +++ b/lib/omniauth/strategies/openid_connect.rb @@ -43,7 +43,7 @@ class OpenIDConnect option :send_scope_to_token_endpoint, true option :client_auth_method - uid { user_info.sub } + uid { id_token.sub } info do { @@ -151,11 +151,18 @@ def discover! end def user_info - @user_info ||= access_token.userinfo! + @user_info ||= begin + _user_info = access_token.userinfo! + if id_token.sub.eql? _user_info.sub + _user_info + else + ::OpenIDConnect::ResponseObject::UserInfo.new(sub: '😶', warning: 'UserInfo subject does not match ID Token subject. Discard UserInfo response.') + end + end end - def access_token - @access_token ||= lambda { + def access_and_id_tokens + @tokens ||= begin _access_token = client.access_token!( scope: (options.scope if options.send_scope_to_token_endpoint), client_auth_method: options.client_auth_method @@ -166,8 +173,16 @@ def access_token client_id: client_options.identifier, nonce: stored_nonce ) - _access_token - }.call() + [_access_token, _id_token] + end + end + + def access_token + @access_token ||= access_and_id_tokens[0] + end + + def id_token + @id_token ||= access_and_id_tokens[1] 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 index 0deda94d..2934918a 100644 --- a/test/lib/omniauth/strategies/openid_connect_test.rb +++ b/test/lib/omniauth/strategies/openid_connect_test.rb @@ -43,7 +43,34 @@ def test_request_phase_with_discovery end def test_uid - assert_equal user_info.sub, strategy.uid + code = SecureRandom.hex(16) + state = SecureRandom.hex(16) + nonce = SecureRandom.hex(16) + sub = SecureRandom.hex(16) + request.stubs(:params).returns({'code' => code,'state' => state}) + request.stubs(:path_info).returns('') + + strategy.options.issuer = 'example.com' + strategy.options.client_signing_alg = :RS256 + strategy.options.client_jwk_signing_key = File.read('test/fixtures/jwks.json') + + id_token = stub('OpenIDConnect::ResponseObject::IdToken') + id_token.stubs(:verify!).with({:issuer => strategy.options.issuer, :client_id => @identifier, :nonce => nonce}).returns(true) + id_token.stubs(:sub).returns(sub) + ::OpenIDConnect::ResponseObject::IdToken.stubs(:decode).returns(id_token) + + strategy.unstub(:user_info) + access_token = stub('OpenIDConnect::AccessToken') + access_token.stubs(:access_token) + access_token.stubs(:refresh_token) + access_token.stubs(:expires_in) + access_token.stubs(:scope) + access_token.stubs(:id_token).returns(File.read('test/fixtures/id_token.txt')) + client.expects(:access_token!).at_least_once.returns(access_token) + + strategy.call!({'rack.session' => {'omniauth.state' => state, 'omniauth.nonce' => nonce}}) + + assert_equal sub, strategy.uid end def test_callback_phase(session = {}, params = {}) @@ -59,6 +86,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) + id_token.stubs(:sub).returns(user_info.sub) ::OpenIDConnect::ResponseObject::IdToken.stubs(:decode).returns(id_token) strategy.unstub(:user_info) @@ -102,6 +130,7 @@ def test_callback_phase_with_discovery id_token = stub('OpenIDConnect::ResponseObject::IdToken') id_token.stubs(:verify!).with({:issuer => 'https://example.com/', :client_id => @identifier, :nonce => nonce}).returns(true) + id_token.stubs(:sub).returns(user_info.sub) ::OpenIDConnect::ResponseObject::IdToken.stubs(:decode).returns(id_token) strategy.unstub(:user_info) @@ -189,6 +218,7 @@ def test_callback_phase_with_socket_error end def test_info + user_info.sub info = strategy.info assert_equal user_info.name, info[:name] assert_equal user_info.email, info[:email] @@ -201,6 +231,60 @@ def test_info assert_equal({ website: user_info.website }, info[:urls]) end + def test_info_when_subject_does_not_match_id_token_subject + code = SecureRandom.hex(16) + state = SecureRandom.hex(16) + nonce = SecureRandom.hex(16) + request.stubs(:params).returns({'code' => code,'state' => state}) + request.stubs(:path_info).returns('') + + strategy.options.issuer = 'example.com' + strategy.options.client_signing_alg = :RS256 + strategy.options.client_jwk_signing_key = File.read('test/fixtures/jwks.json') + + id_token = stub('OpenIDConnect::ResponseObject::IdToken') + id_token.stubs(:verify!).with({:issuer => strategy.options.issuer, :client_id => @identifier, :nonce => nonce}).returns(true) + id_token.stubs(:sub).returns(user_info.sub) + ::OpenIDConnect::ResponseObject::IdToken.stubs(:decode).returns(id_token) + + strategy.unstub(:user_info) + access_token = stub('OpenIDConnect::AccessToken') + access_token.stubs(:access_token) + access_token.stubs(:refresh_token) + access_token.stubs(:expires_in) + access_token.stubs(:scope) + access_token.stubs(:id_token).returns(File.read('test/fixtures/id_token.txt')) + client.expects(:access_token!).at_least_once.returns(access_token) + _user_info = OpenIDConnect::ResponseObject::UserInfo.new( + sub: SecureRandom.hex(16), + name: Faker::Name.name, + email: Faker::Internet.email, + nickname: Faker::Name.first_name, + preferred_username: Faker::Internet.user_name, + given_name: Faker::Name.first_name, + family_name: Faker::Name.last_name, + gender: 'female', + picture: Faker::Internet.url + ".png", + phone_number: Faker::PhoneNumber.phone_number, + website: Faker::Internet.url, + ) + access_token.expects(:userinfo!).returns(_user_info) + + strategy.call!({'rack.session' => {'omniauth.state' => state, 'omniauth.nonce' => nonce}}) + + info = strategy.info + assert_equal nil, info[:name] + assert_equal nil, info[:email] + assert_equal nil, info[:nickname] + assert_equal nil, info[:first_name] + assert_equal nil, info[:last_name] + assert_equal nil, info[:gender] + assert_equal nil, info[:image] + assert_equal nil, info[:phone] + assert_equal({ website: nil }, info[:urls]) + assert_equal({ raw_info: {sub: "😶", warning: "UserInfo subject does not match ID Token subject. Discard UserInfo response."}}, strategy.extra) + end + def test_extra assert_equal({ raw_info: user_info.as_json }, strategy.extra) end