Skip to content

Commit

Permalink
Fix redirect_uri so that it does not include code or state
Browse files Browse the repository at this point in the history
I think this is a better fix for omniauth#28, and saves many OAuth2 strategies
broken by omniauth#70.
  • Loading branch information
knu committed Nov 14, 2016
1 parent 464fcef commit a4edc6f
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 0 deletions.
18 changes: 18 additions & 0 deletions lib/omniauth/strategies/oauth2.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
31 changes: 31 additions & 0 deletions spec/omniauth/strategies/oauth2_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit a4edc6f

Please sign in to comment.