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

3 files changed, 41 insertions, 1 deletions

diff --git a/changelogs/unreleased/sh-fix-omniauth-generic-strategy.yml b/changelogs/unreleased/sh-fix-omniauth-generic-strategy.yml
new file mode 100644
index 00000000000..561c19c9685
--- /dev/null
+++ b/changelogs/unreleased/sh-fix-omniauth-generic-strategy.yml
@@ -0,0 +1,5 @@
+---
+title: Fix OmniAuth OAuth2Generic strategy not loading
+merge_request: 28680
+author:
+type: fixed
diff --git a/lib/gitlab/omniauth_initializer.rb b/lib/gitlab/omniauth_initializer.rb
index e0ac9eec1f2..2a2083ebae0 100644
--- a/lib/gitlab/omniauth_initializer.rb
+++ b/lib/gitlab/omniauth_initializer.rb
@@ -36,12 +36,25 @@ module Gitlab
         hash_arguments = provider['args'].merge(provider_defaults(provider))
 
         # A Hash from the configuration will be passed as is.
-        provider_arguments << hash_arguments.symbolize_keys
+        provider_arguments << normalize_hash_arguments(hash_arguments)
       end
 
       provider_arguments
     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)
       case provider['name']
       when 'cas3'
diff --git a/spec/lib/gitlab/omniauth_initializer_spec.rb b/spec/lib/gitlab/omniauth_initializer_spec.rb
index d808b4d49e0..f9c0daf1ef1 100644
--- a/spec/lib/gitlab/omniauth_initializer_spec.rb
+++ b/spec/lib/gitlab/omniauth_initializer_spec.rb
@@ -38,6 +38,28 @@ describe Gitlab::OmniauthInitializer do
       subject.execute([hash_config])
     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
       shibboleth_config = { 'name' => 'shibboleth', 'args' => {} }