From 3a3bfa543cbd9c1adea8d14e1c51ea6207d81341 Mon Sep 17 00:00:00 2001 From: Akinori MUSHA Date: Tue, 15 Nov 2016 15:42:09 +0900 Subject: [PATCH 1/6] Update the list of rubies Obsolete versions are removed since they always fail in bundle. --- .travis.yml | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/.travis.yml b/.travis.yml index d2e47a0..7fde4c0 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,15 +4,11 @@ env: - JRUBY_OPTS="$JRUBY_OPTS --debug" language: ruby rvm: - - 1.8.7 - - 1.9.3 - - 2.0.0 - 2.1 - 2.2 - - jruby-18mode - - jruby-19mode + - 2.3.1 + - jruby-9 - jruby-head - - rbx-2 - ruby-head matrix: allow_failures: From 6c4ddaa443b0f5c3dbb720d55b6750d6568d4362 Mon Sep 17 00:00:00 2001 From: Akinori MUSHA Date: Tue, 15 Nov 2016 15:42:31 +0900 Subject: [PATCH 2/6] Guard rack and public_suffix for old rubies --- omniauth-oauth2.gemspec | 2 ++ 1 file changed, 2 insertions(+) diff --git a/omniauth-oauth2.gemspec b/omniauth-oauth2.gemspec index 53983a4..b7939f2 100644 --- a/omniauth-oauth2.gemspec +++ b/omniauth-oauth2.gemspec @@ -5,6 +5,8 @@ require "omniauth-oauth2/version" Gem::Specification.new do |gem| gem.add_dependency "oauth2", "~> 1.0" gem.add_dependency "omniauth", "~> 1.2" + gem.add_dependency "public_suffix", "< 2.0.4" if RUBY_VERSION < "2.0" + gem.add_dependency "rack", "< 2" if RUBY_VERSION < "2.2.2" gem.add_development_dependency "bundler", "~> 1.0" From 126e70be7b6b209e391b5ddc215a2eae96fdc1ca Mon Sep 17 00:00:00 2001 From: Akinori MUSHA Date: Mon, 14 Nov 2016 21:40:31 +0900 Subject: [PATCH 3/6] Fix redirect_uri so that it does not include `code` or `state` I think this is a better fix for #28, and saves many OAuth2 strategies broken by #70. --- lib/omniauth/strategies/oauth2.rb | 18 ++++++++++++++ spec/omniauth/strategies/oauth2_spec.rb | 31 +++++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/lib/omniauth/strategies/oauth2.rb b/lib/omniauth/strategies/oauth2.rb index 3ffff1b..a2bc06b 100644 --- a/lib/omniauth/strategies/oauth2.rb +++ b/lib/omniauth/strategies/oauth2.rb @@ -36,6 +36,24 @@ def client ::OAuth2::Client.new(options.client_id, options.client_secret, deep_symbolize(options.client_options)) end + def callback_url + # If redirect_uri is configured in token_params, use that + # value. + token_params.to_hash(:symbolize_keys => true)[:redirect_uri] || super + end + + def query_string + # This method is called by callback_url, only if redirect_uri + # is omitted in token_params. + if request.params["code"] + # If this is a callback, ignore query parameters added by + # the provider. + "" + else + super + end + end + credentials do hash = {"token" => access_token.token} hash.merge!("refresh_token" => access_token.refresh_token) if access_token.expires? && access_token.refresh_token diff --git a/spec/omniauth/strategies/oauth2_spec.rb b/spec/omniauth/strategies/oauth2_spec.rb index 0bffcde..de05c60 100644 --- a/spec/omniauth/strategies/oauth2_spec.rb +++ b/spec/omniauth/strategies/oauth2_spec.rb @@ -87,6 +87,37 @@ def app instance.callback_phase end end + + describe "#callback_url" do + subject { fresh_strategy } + + it "returns the value in token_params, if given" do + instance = subject.new("abc", "def", :token_params => {:redirect_uri => "http://test/foo?bar=1"}) + allow(instance).to receive(:request) do + double("Request", :params => {"code" => "codecodecode", "state" => "statestatestate"}) + end + expect(instance.callback_url).to eq("http://test/foo?bar=1") + end + + it "does not include any query parameters like \"code\" and \"state\"" do + instance = subject.new("abc", "def") + allow(instance).to receive(:full_host) do + "http://test" + end + allow(instance).to receive(:script_name) do + "/foo" + end + allow(instance).to receive(:callback_path) do + "/bar/callback" + end + allow(instance).to receive(:request) do + double("Request", + :params => {"code" => "codecodecode", "state" => "statestatestate"}, + :query_string => "code=codecodecode&state=statestatestate") + end + expect(instance.callback_url).to eq("http://test/foo/bar/callback") + end + end end describe OmniAuth::Strategies::OAuth2::CallbackError do From a483a8a34c5eac5fcd377f6f8d68a12ee6c087bc Mon Sep 17 00:00:00 2001 From: Akinori MUSHA Date: Tue, 15 Nov 2016 15:48:39 +0900 Subject: [PATCH 4/6] Fix the obsolete cop Style/TrailingComma --- .rubocop.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.rubocop.yml b/.rubocop.yml index 8dbbbf0..9b181f8 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -38,5 +38,8 @@ Style/SpaceInsideHashLiteralBraces: Style/StringLiterals: EnforcedStyle: double_quotes -Style/TrailingComma: +Style/TrailingCommaInLiteral: + EnforcedStyleForMultiline: 'comma' + +Style/TrailingCommaInArguments: EnforcedStyleForMultiline: 'comma' From 16bd81a9059e61f762526596dce519413b8f3b31 Mon Sep 17 00:00:00 2001 From: Akinori MUSHA Date: Tue, 15 Nov 2016 15:53:36 +0900 Subject: [PATCH 5/6] Fix things I didn't break to shut up Rubocop --- lib/omniauth-oauth2/version.rb | 2 +- lib/omniauth/strategies/oauth2.rb | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/omniauth-oauth2/version.rb b/lib/omniauth-oauth2/version.rb index 5a79a34..739f739 100644 --- a/lib/omniauth-oauth2/version.rb +++ b/lib/omniauth-oauth2/version.rb @@ -1,5 +1,5 @@ module OmniAuth module OAuth2 - VERSION = "1.4.0" + VERSION = "1.4.0".freeze end end diff --git a/lib/omniauth/strategies/oauth2.rb b/lib/omniauth/strategies/oauth2.rb index a2bc06b..0d5348a 100644 --- a/lib/omniauth/strategies/oauth2.rb +++ b/lib/omniauth/strategies/oauth2.rb @@ -56,9 +56,9 @@ def query_string credentials do hash = {"token" => access_token.token} - hash.merge!("refresh_token" => access_token.refresh_token) if access_token.expires? && access_token.refresh_token - hash.merge!("expires_at" => access_token.expires_at) if access_token.expires? - hash.merge!("expires" => access_token.expires?) + hash["refresh_token"] = access_token.refresh_token if access_token.expires? && access_token.refresh_token + hash["expires_at"] = access_token.expires_at if access_token.expires? + hash["expires"] = access_token.expires? hash end From 23ffea5d4e9f13d838570c89c5939f6525546e22 Mon Sep 17 00:00:00 2001 From: Akinori MUSHA Date: Tue, 15 Nov 2016 19:36:14 +0900 Subject: [PATCH 6/6] Add one more spec --- spec/omniauth/strategies/oauth2_spec.rb | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/spec/omniauth/strategies/oauth2_spec.rb b/spec/omniauth/strategies/oauth2_spec.rb index de05c60..85b40b4 100644 --- a/spec/omniauth/strategies/oauth2_spec.rb +++ b/spec/omniauth/strategies/oauth2_spec.rb @@ -99,7 +99,26 @@ def app expect(instance.callback_url).to eq("http://test/foo?bar=1") end - it "does not include any query parameters like \"code\" and \"state\"" do + it "does not contain any parameters" do + instance = subject.new("abc", "def") + allow(instance).to receive(:full_host) do + "http://test" + end + allow(instance).to receive(:script_name) do + "/foo" + end + allow(instance).to receive(:callback_path) do + "/bar/callback" + end + allow(instance).to receive(:request) do + double("Request", + :params => {}, + :query_string => "") + end + expect(instance.callback_url).to eq("http://test/foo/bar/callback") + end + + it "does not include any query parameters when invoked from within a callback" do instance = subject.new("abc", "def") allow(instance).to receive(:full_host) do "http://test"