Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

security issue in returning post parameters from session in callback phase #867

Merged
merged 1 commit into from Jan 17, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/omniauth/strategy.rb
Expand Up @@ -198,7 +198,7 @@ def request_call # rubocop:disable CyclomaticComplexity, MethodLength, Perceived
setup_phase
log :info, 'Request phase initiated.'
# store query params from the request url, extracted in the callback_phase
session['omniauth.params'] = request.params
session['omniauth.params'] = request.GET
OmniAuth.config.before_request_phase.call(env) if OmniAuth.config.before_request_phase
if options.form.respond_to?(:call)
log :info, 'Rendering form from supplied Rack endpoint.'
Expand Down Expand Up @@ -265,7 +265,7 @@ def mock_call!(*)
def mock_request_call
setup_phase

session['omniauth.params'] = request.params
session['omniauth.params'] = request.GET
OmniAuth.config.before_request_phase.call(env) if OmniAuth.config.before_request_phase
if request.params['origin']
@env['rack.session']['omniauth.origin'] = request.params['origin']
Expand Down
13 changes: 12 additions & 1 deletion spec/omniauth/strategy_spec.rb
Expand Up @@ -685,13 +685,24 @@ def make_env(path = '/auth/test', props = {})
expect(strategy.env['foobar']).to eq('baz')
end

it 'sets omniauth.params on the request phase' do
it 'sets omniauth.params with query params on the request phase' do
OmniAuth.config.mock_auth[:test] = {}

strategy.call(make_env('/auth/test', 'QUERY_STRING' => 'foo=bar'))
expect(strategy.env['rack.session']['omniauth.params']).to eq('foo' => 'bar')
end

it 'does not set body parameters of POST request on the request phase' do
OmniAuth.config.mock_auth[:test] = {}

props = {
'REQUEST_METHOD' => 'POST',
'rack.input' => StringIO.new('foo=bar')
}
strategy.call(make_env('/auth/test', props))
expect(strategy.env['rack.session']['omniauth.params']).to eq({})
end

it 'executes request hook on the request phase' do
OmniAuth.config.mock_auth[:test] = {}
OmniAuth.config.before_request_phase do |env|
Expand Down