diff --git a/.gitignore b/.gitignore index 467b7402..8c53097b 100644 --- a/.gitignore +++ b/.gitignore @@ -15,3 +15,4 @@ test/tmp test/version_tmp tmp .ruby-version +Gemfile.lock diff --git a/lib/omniauth/strategies/openid_connect.rb b/lib/omniauth/strategies/openid_connect.rb index e4705c90..aecb33a1 100644 --- a/lib/omniauth/strategies/openid_connect.rb +++ b/lib/omniauth/strategies/openid_connect.rb @@ -89,10 +89,13 @@ def request_phase def callback_phase error = request.params['error_reason'] || request.params['error'] + original_omniauth_state = session['omniauth.state'] + if error raise CallbackError.new(request.params['error'], request.params['error_description'] || request.params['error_reason'], request.params['error_uri']) elsif request.params['state'].to_s.empty? || request.params['state'] != stored_state - return Rack::Response.new(['401 Unauthorized'], 401).finish + #return Rack::Response.new(['401 Unauthorized'], 401).finish + raise StateError, "Invalid state: #{request.params['state']} (original state: #{original_omniauth_state})" elsif !request.params["code"] return fail!(:missing_code, OmniAuth::OpenIDConnect::MissingCodeError.new(request.params["error"])) else @@ -109,6 +112,8 @@ def callback_phase fail!(:timeout, e) rescue ::SocketError => e fail!(:failed_to_connect, e) + rescue StateError => e + fail!(:state_error, e) end @@ -124,6 +129,8 @@ def authorize_uri state: new_state, nonce: (new_nonce if options.send_nonce), hd: options.hd, + login_hint: options.login_hint, + acr_values: options.acr_values } client.authorization_uri(opts.reject{|k,v| v.nil?}) end @@ -171,6 +178,7 @@ def access_token end def decode_id_token(id_token) + Rails.logger.info "id_token: #{id_token} | public_key: #{public_key}" ::OpenIDConnect::ResponseObject::IdToken.decode(id_token, public_key) end @@ -244,6 +252,8 @@ def message [error, error_reason, error_uri].compact.join(' | ') end end + + class StateError < StandardError; end end end end diff --git a/omniauth-openid-connect.gemspec b/omniauth-openid-connect.gemspec index 5fad920c..fad2e434 100644 --- a/omniauth-openid-connect.gemspec +++ b/omniauth-openid-connect.gemspec @@ -18,8 +18,8 @@ Gem::Specification.new do |spec| spec.test_files = spec.files.grep(%r{^(test|spec|features)/}) spec.require_paths = ["lib"] - spec.add_dependency 'omniauth', '~> 1.1' - spec.add_dependency 'openid_connect', '~> 0.9.2' + spec.add_dependency 'omniauth' + spec.add_dependency 'openid_connect', '~> 1.1.6' spec.add_dependency 'addressable', '~> 2.3' spec.add_development_dependency "bundler", "~> 1.5" spec.add_development_dependency "minitest" diff --git a/solano.yml b/solano.yml new file mode 100644 index 00000000..c92f0bd6 --- /dev/null +++ b/solano.yml @@ -0,0 +1,5 @@ +--- +ruby_version: ruby-2.2.5 +bundler_version: 1.13.2 # IMPORTANT: set ruby bundler version to use +environment: + RAILS_LOG_LEVEL: '2' \ No newline at end of file diff --git a/test/lib/omniauth/strategies/openid_connect_test.rb b/test/lib/omniauth/strategies/openid_connect_test.rb index 0deda94d..ef308eb5 100644 --- a/test/lib/omniauth/strategies/openid_connect_test.rb +++ b/test/lib/omniauth/strategies/openid_connect_test.rb @@ -9,9 +9,11 @@ def test_client_options_defaults end def test_request_phase - expected_redirect = /^https:\/\/example\.com\/authorize\?client_id=1234&nonce=[\w\d]{32}&response_type=code&scope=openid&state=[\w\d]{32}$/ + expected_redirect = /^https:\/\/example\.com\/authorize\?acr_values=authorize%3A1&client_id=1234&login_hint=example%40example.com&nonce=[\w\d]{32}&response_type=code&scope=openid&state=[\w\d]{32}$/ strategy.options.issuer = 'example.com' strategy.options.client_options.host = 'example.com' + strategy.options.login_hint = 'example@example.com' + strategy.options.acr_values = 'authorize:1' strategy.expects(:redirect).with(regexp_matches(expected_redirect)) strategy.request_phase end @@ -139,8 +141,10 @@ def test_callback_phase_with_invalid_state strategy.call!({'rack.session' => {'omniauth.state' => state, 'omniauth.nonce' => nonce}}) result = strategy.callback_phase - assert result.kind_of?(Array) - assert result.first == 401, "Expecting unauthorized" + #assert result.kind_of?(Array) + #assert result.first == 401, "Expecting unauthorized" + strategy.expects(:fail!) + strategy.callback_phase end def test_callback_phase_with_timeout @@ -275,8 +279,10 @@ def test_state result = strategy.callback_phase - assert result.kind_of?(Array) - assert result.first == 401, "Expecting unauthorized" + #assert result.kind_of?(Array) + #assert result.first == 401, "Expecting unauthorized" + strategy.expects(:fail!) + strategy.callback_phase end def test_option_client_auth_method