Skip to content

Commit a4edc6f

Browse files
committed
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.
1 parent 464fcef commit a4edc6f

File tree

2 files changed

+49
-0
lines changed

2 files changed

+49
-0
lines changed

lib/omniauth/strategies/oauth2.rb

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,24 @@ def client
3636
::OAuth2::Client.new(options.client_id, options.client_secret, deep_symbolize(options.client_options))
3737
end
3838

39+
def callback_url
40+
# If redirect_uri is configured in token_params, use that
41+
# value.
42+
token_params.to_hash(:symbolize_keys => true)[:redirect_uri] || super
43+
end
44+
45+
def query_string
46+
# This method is called by callback_url, only if redirect_uri
47+
# is omitted in token_params.
48+
if request.params['code']
49+
# If this is a callback, ignore query parameters added by
50+
# the provider.
51+
''
52+
else
53+
super
54+
end
55+
end
56+
3957
credentials do
4058
hash = {"token" => access_token.token}
4159
hash.merge!("refresh_token" => access_token.refresh_token) if access_token.expires? && access_token.refresh_token

spec/omniauth/strategies/oauth2_spec.rb

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,37 @@ def app
8787
instance.callback_phase
8888
end
8989
end
90+
91+
describe "#callback_url" do
92+
subject { fresh_strategy }
93+
94+
it "returns the value in token_params, if given" do
95+
instance = subject.new('abc', 'def', :token_params => {:redirect_uri => 'http://test/foo?bar=1'})
96+
allow(instance).to receive(:request) do
97+
double('Request', :params => {'code' => 'codecodecode', 'state' => 'statestatestate'})
98+
end
99+
expect(instance.callback_url).to eq('http://test/foo?bar=1')
100+
end
101+
102+
it "does not include any query parameters like 'code' and 'state'" do
103+
instance = subject.new('abc', 'def')
104+
allow(instance).to receive(:full_host) do
105+
"http://test"
106+
end
107+
allow(instance).to receive(:script_name) do
108+
'/foo'
109+
end
110+
allow(instance).to receive(:callback_path) do
111+
'/bar/callback'
112+
end
113+
allow(instance).to receive(:request) do
114+
double('Request',
115+
:params => {'code' => 'codecodecode', 'state' => 'statestatestate'},
116+
:query_string => 'code=codecodecode&state=statestatestate')
117+
end
118+
expect(instance.callback_url).to eq('http://test/foo/bar/callback')
119+
end
120+
end
90121
end
91122

92123
describe OmniAuth::Strategies::OAuth2::CallbackError do

0 commit comments

Comments
 (0)