From c741f95a3b599971aaf3dfc1ff3ca43a8ba938b8 Mon Sep 17 00:00:00 2001
From: James Edwards-Jones <jedwardsjones@gitlab.com>
Date: Mon, 23 Apr 2018 00:15:48 +0100
Subject: [PATCH] Exclude LDAP from OmniauthCallbackController base methods

---
 .../omniauth_callbacks_controller.rb          |  4 ++--
 app/helpers/auth_helper.rb                    |  8 +++++--
 spec/helpers/auth_helper_spec.rb              | 24 +++++++++++++++++++
 3 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb
index 9137bc92810..40d9fa18a10 100644
--- a/app/controllers/omniauth_callbacks_controller.rb
+++ b/app/controllers/omniauth_callbacks_controller.rb
@@ -8,8 +8,8 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
     omniauth_flow(Gitlab::Auth::OAuth)
   end
 
-  Gitlab.config.omniauth.providers.each do |provider|
-    alias_method provider['name'], :handle_omniauth
+  AuthHelper.providers_for_base_controller.each do |provider|
+    alias_method provider, :handle_omniauth
   end
 
   # Extend the standard implementation to also increment
diff --git a/app/helpers/auth_helper.rb b/app/helpers/auth_helper.rb
index c109954f3a3..d2daee22aba 100644
--- a/app/helpers/auth_helper.rb
+++ b/app/helpers/auth_helper.rb
@@ -1,6 +1,6 @@
 module AuthHelper
   PROVIDERS_WITH_ICONS = %w(twitter github gitlab bitbucket google_oauth2 facebook azure_oauth2 authentiq).freeze
-  FORM_BASED_PROVIDERS = [/\Aldap/, 'crowd'].freeze
+  LDAP_PROVIDER = /\Aldap/
 
   def ldap_enabled?
     Gitlab::Auth::LDAP::Config.enabled?
@@ -23,7 +23,7 @@ module AuthHelper
   end
 
   def form_based_provider?(name)
-    FORM_BASED_PROVIDERS.any? { |pattern| pattern === name.to_s }
+    [LDAP_PROVIDER, 'crowd'].any? { |pattern| pattern === name.to_s }
   end
 
   def form_based_providers
@@ -38,6 +38,10 @@ module AuthHelper
     auth_providers.reject { |provider| form_based_provider?(provider) }
   end
 
+  def providers_for_base_controller
+    auth_providers.reject { |provider| LDAP_PROVIDER === provider }
+  end
+
   def enabled_button_based_providers
     disabled_providers = Gitlab::CurrentSettings.disabled_oauth_sign_in_sources || []
 
diff --git a/spec/helpers/auth_helper_spec.rb b/spec/helpers/auth_helper_spec.rb
index c94fedd615b..120b23e66ac 100644
--- a/spec/helpers/auth_helper_spec.rb
+++ b/spec/helpers/auth_helper_spec.rb
@@ -18,6 +18,30 @@ describe AuthHelper do
     end
   end
 
+  describe "providers_for_base_controller" do
+    it 'returns all enabled providers from devise' do
+      allow(helper).to receive(:auth_providers) { [:twitter, :github] }
+      expect(helper.providers_for_base_controller).to include(*[:twitter, :github])
+    end
+
+    it 'excludes ldap providers' do
+      allow(helper).to receive(:auth_providers) { [:twitter, :ldapmain] }
+      expect(helper.providers_for_base_controller).not_to include(:ldapmain)
+    end
+  end
+
+  describe "form_based_providers" do
+    it 'includes LDAP providers' do
+      allow(helper).to receive(:auth_providers) { [:twitter, :ldapmain] }
+      expect(helper.form_based_providers).to eq %i(ldapmain)
+    end
+
+    it 'includes crowd provider' do
+      allow(helper).to receive(:auth_providers) { [:twitter, :crowd] }
+      expect(helper.form_based_providers).to eq %i(crowd)
+    end
+  end
+
   describe 'enabled_button_based_providers' do
     before do
       allow(helper).to receive(:auth_providers) { [:twitter, :github] }