Commit bf8f4c13 authored by Stan Hu's avatar Stan Hu

Fix OmniAuth OAuth2Generic strategy not loading

In https://github.com/rails/rails/commit/83b767ce, Rails 5.1 removed
support for using a String to specify a middleware. When the
strategy_class argument is passed from the GitLab YAML config to Devise,
Devise passes the string value straight through to Rails, and GitLab
would crash with a NoMethodError inside ActionDispatch::MiddlewareStack.

To make this OmniAuth strategy work again, we normalize the arguments by
converting the strategy_class value into an actual Class.

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/62216
parent dd454852
---
title: Fix OmniAuth OAuth2Generic strategy not loading
merge_request: 28680
author:
type: fixed
...@@ -36,12 +36,25 @@ module Gitlab ...@@ -36,12 +36,25 @@ module Gitlab
hash_arguments = provider['args'].merge(provider_defaults(provider)) hash_arguments = provider['args'].merge(provider_defaults(provider))
# A Hash from the configuration will be passed as is. # A Hash from the configuration will be passed as is.
provider_arguments << hash_arguments.symbolize_keys provider_arguments << normalize_hash_arguments(hash_arguments)
end end
provider_arguments provider_arguments
end end
def normalize_hash_arguments(args)
args.symbolize_keys!
# Rails 5.1 deprecated the use of string names in the middleware
# (https://github.com/rails/rails/commit/83b767ce), so we need to
# pass in the actual class to Devise.
if args[:strategy_class].is_a?(String)
args[:strategy_class] = args[:strategy_class].constantize
end
args
end
def provider_defaults(provider) def provider_defaults(provider)
case provider['name'] case provider['name']
when 'cas3' when 'cas3'
......
...@@ -38,6 +38,28 @@ describe Gitlab::OmniauthInitializer do ...@@ -38,6 +38,28 @@ describe Gitlab::OmniauthInitializer do
subject.execute([hash_config]) subject.execute([hash_config])
end end
it 'normalizes a String strategy_class' do
hash_config = { 'name' => 'hash', 'args' => { strategy_class: 'OmniAuth::Strategies::OAuth2Generic' } }
expect(devise_config).to receive(:omniauth).with(:hash, strategy_class: OmniAuth::Strategies::OAuth2Generic)
subject.execute([hash_config])
end
it 'allows a class to be specified in strategy_class' do
hash_config = { 'name' => 'hash', 'args' => { strategy_class: OmniAuth::Strategies::OAuth2Generic } }
expect(devise_config).to receive(:omniauth).with(:hash, strategy_class: OmniAuth::Strategies::OAuth2Generic)
subject.execute([hash_config])
end
it 'throws an error for an invalid strategy_class' do
hash_config = { 'name' => 'hash', 'args' => { strategy_class: 'OmniAuth::Strategies::Bogus' } }
expect { subject.execute([hash_config]) }.to raise_error(NameError)
end
it 'configures fail_with_empty_uid for shibboleth' do it 'configures fail_with_empty_uid for shibboleth' do
shibboleth_config = { 'name' => 'shibboleth', 'args' => {} } shibboleth_config = { 'name' => 'shibboleth', 'args' => {} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment