Disable SAML if OmniAuth is disabled48932-disable-saml-if-omniauth-is-disabled

We also try to unify the way we setup OmniAuth, and how we check
if it's enabled or not.

12 files changed, 41 insertions, 38 deletions

diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb
index 9dd652206fe..4ca42e2d4a2 100644
--- a/app/controllers/sessions_controller.rb
+++ b/app/controllers/sessions_controller.rb
@@ -157,6 +157,8 @@ class SessionsController < Devise::SessionsController
   end
 
   def auto_sign_in_with_provider
+    return unless Gitlab::Auth.omniauth_enabled?
+
     provider = Gitlab.config.omniauth.auto_sign_in_with_provider
     return unless provider.present?
 
diff --git a/app/helpers/auth_helper.rb b/app/helpers/auth_helper.rb
index d2daee22aba..18f0979fc86 100644
--- a/app/helpers/auth_helper.rb
+++ b/app/helpers/auth_helper.rb
@@ -7,7 +7,7 @@ module AuthHelper
   end
 
   def omniauth_enabled?
-    Gitlab.config.omniauth.enabled
+    Gitlab::Auth.omniauth_enabled?
   end
 
   def provider_has_icon?(name)
diff --git a/app/views/admin/dashboard/index.html.haml b/app/views/admin/dashboard/index.html.haml
index 18f2c1a509f..fac61f9d249 100644
--- a/app/views/admin/dashboard/index.html.haml
+++ b/app/views/admin/dashboard/index.html.haml
@@ -91,10 +91,10 @@
               %span.light.float-right
                 = boolean_to_icon gravatar_enabled?
             - omniauth = "OmniAuth"
-            %p{ "aria-label" => "#{omniauth}: status " + (Gitlab.config.omniauth.enabled ? "on" : "off") }
+            %p{ "aria-label" => "#{omniauth}: status " + (Gitlab::Auth.omniauth_enabled? ? "on" : "off") }
               = omniauth
               %span.light.float-right
-                = boolean_to_icon Gitlab.config.omniauth.enabled
+                = boolean_to_icon Gitlab::Auth.omniauth_enabled?
             - reply_email = "Reply by email"
             %p{ "aria-label" => "#{reply_email}: status " + (Gitlab::IncomingEmail.enabled? ? "on" : "off") }
               = reply_email
diff --git a/changelogs/unreleased/48932-disable-saml-if-omniauth-is-disabled.yml b/changelogs/unreleased/48932-disable-saml-if-omniauth-is-disabled.yml
new file mode 100644
index 00000000000..0466debea65
--- /dev/null
+++ b/changelogs/unreleased/48932-disable-saml-if-omniauth-is-disabled.yml
@@ -0,0 +1,5 @@
+---
+title: Disable SAML and Bitbucket if OmniAuth is disabled
+merge_request: 20608
+author:
+type: fixed
diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb
index e5772c33307..c41b2db722c 100644
--- a/config/initializers/devise.rb
+++ b/config/initializers/devise.rb
@@ -219,7 +219,7 @@ Devise.setup do |config|
     end
   end
 
-  if Gitlab::OmniauthInitializer.enabled?
+  if Gitlab::Auth.omniauth_enabled?
     Gitlab::OmniauthInitializer.new(config).execute(Gitlab.config.omniauth.providers)
   end
 end
diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb
index c558eb28ced..ef23ca065c6 100644
--- a/config/initializers/omniauth.rb
+++ b/config/initializers/omniauth.rb
@@ -16,8 +16,3 @@ OmniAuth.config.allowed_request_methods << :get if Gitlab.config.omniauth.auto_s
 OmniAuth.config.before_request_phase do |env|
   Gitlab::RequestForgeryProtection.call(env)
 end
-
-if Gitlab::OmniauthInitializer.enabled?
-  provider_names = Gitlab.config.omniauth.providers.map(&:name)
-  Gitlab::Auth.omniauth_setup_providers(provider_names)
-end
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index 7de66539848..111e18b2076 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -14,23 +14,8 @@ module Gitlab
     DEFAULT_SCOPES = [:api].freeze
 
     class << self
-      def omniauth_customized_providers
-        @omniauth_customized_providers ||= %w[bitbucket jwt]
-      end
-
-      def omniauth_setup_providers(provider_names)
-        provider_names.each do |provider|
-          omniauth_setup_a_provider(provider)
-        end
-      end
-
-      def omniauth_setup_a_provider(provider)
-        case provider
-        when 'kerberos'
-          require 'omniauth-kerberos'
-        when *omniauth_customized_providers
-          require_dependency "omni_auth/strategies/#{provider}"
-        end
+      def omniauth_enabled?
+        Gitlab.config.omniauth.enabled
       end
 
       def find_for_git_client(login, password, project:, ip:)
diff --git a/lib/gitlab/auth/o_auth/provider.rb b/lib/gitlab/auth/o_auth/provider.rb
index 5fb61ffe00d..e73743944a9 100644
--- a/lib/gitlab/auth/o_auth/provider.rb
+++ b/lib/gitlab/auth/o_auth/provider.rb
@@ -30,7 +30,7 @@ module Gitlab
         def self.enabled?(name)
           return true if name == 'database'
 
-          providers.include?(name.to_sym)
+          Gitlab::Auth.omniauth_enabled? && providers.include?(name.to_sym)
         end
 
         def self.ldap_provider?(name)
diff --git a/lib/gitlab/omniauth_initializer.rb b/lib/gitlab/omniauth_initializer.rb
index a71acda8701..f33ea0880df 100644
--- a/lib/gitlab/omniauth_initializer.rb
+++ b/lib/gitlab/omniauth_initializer.rb
@@ -1,23 +1,21 @@
 module Gitlab
   class OmniauthInitializer
-    def self.enabled?
-      Gitlab.config.omniauth.enabled ||
-        Gitlab.config.omniauth.auto_sign_in_with_provider.present?
-    end
-
     def initialize(devise_config)
       @devise_config = devise_config
     end
 
     def execute(providers)
       providers.each do |provider|
-        add_provider(provider['name'].to_sym, *arguments_for(provider))
+        name = provider['name'].to_sym
+
+        add_provider_to_devise(name, *arguments_for(provider))
+        setup_provider(name)
       end
     end
 
     private
 
-    def add_provider(*args)
+    def add_provider_to_devise(*args)
       @devise_config.omniauth(*args)
     end
 
@@ -76,5 +74,23 @@ module Gitlab
         end
       end
     end
+
+    def omniauth_customized_providers
+      @omniauth_customized_providers ||= build_omniauth_customized_providers
+    end
+
+    # We override this in EE
+    def build_omniauth_customized_providers
+      %i[bitbucket jwt]
+    end
+
+    def setup_provider(provider)
+      case provider
+      when :kerberos
+        require 'omniauth-kerberos'
+      when *omniauth_customized_providers
+        require_dependency "omni_auth/strategies/#{provider}"
+      end
+    end
   end
 end
diff --git a/lib/gitlab/usage_data.rb b/lib/gitlab/usage_data.rb
index dff0c97eeb4..22c9638ecc0 100644
--- a/lib/gitlab/usage_data.rb
+++ b/lib/gitlab/usage_data.rb
@@ -95,7 +95,7 @@ module Gitlab
           gravatar_enabled: Gitlab::CurrentSettings.gravatar_enabled?,
           ldap_enabled: Gitlab.config.ldap.enabled,
           mattermost_enabled: Gitlab.config.mattermost.enabled,
-          omniauth_enabled: Gitlab.config.omniauth.enabled,
+          omniauth_enabled: Gitlab::Auth.omniauth_enabled?,
           reply_by_email_enabled: Gitlab::IncomingEmail.enabled?,
           signup_enabled: Gitlab::CurrentSettings.allow_signup?
         }
diff --git a/lib/tasks/gitlab/info.rake b/lib/tasks/gitlab/info.rake
index 6de739e9515..e97d77d20e0 100644
--- a/lib/tasks/gitlab/info.rake
+++ b/lib/tasks/gitlab/info.rake
@@ -54,8 +54,8 @@ namespace :gitlab do
       puts "HTTP Clone URL:\t#{http_clone_url}"
       puts "SSH Clone URL:\t#{ssh_clone_url}"
       puts "Using LDAP:\t#{Gitlab.config.ldap.enabled ? "yes".color(:green) : "no"}"
-      puts "Using Omniauth:\t#{Gitlab.config.omniauth.enabled ? "yes".color(:green) : "no"}"
-      puts "Omniauth Providers: #{omniauth_providers.join(', ')}" if Gitlab.config.omniauth.enabled
+      puts "Using Omniauth:\t#{Gitlab::Auth.omniauth_enabled? ? "yes".color(:green) : "no"}"
+      puts "Omniauth Providers: #{omniauth_providers.join(', ')}" if Gitlab::Auth.omniauth_enabled?
 
       # check Gitolite version
       gitlab_shell_version_file = "#{Gitlab.config.gitlab_shell.hooks_path}/../VERSION"
diff --git a/spec/lib/gitlab/usage_data_spec.rb b/spec/lib/gitlab/usage_data_spec.rb
index 20def4fefe2..a19b3c0ba66 100644
--- a/spec/lib/gitlab/usage_data_spec.rb
+++ b/spec/lib/gitlab/usage_data_spec.rb
@@ -133,7 +133,7 @@ describe Gitlab::UsageData do
       expect(subject[:signup_enabled]).to eq(Gitlab::CurrentSettings.allow_signup?)
       expect(subject[:ldap_enabled]).to eq(Gitlab.config.ldap.enabled)
       expect(subject[:gravatar_enabled]).to eq(Gitlab::CurrentSettings.gravatar_enabled?)
-      expect(subject[:omniauth_enabled]).to eq(Gitlab.config.omniauth.enabled)
+      expect(subject[:omniauth_enabled]).to eq(Gitlab::Auth.omniauth_enabled?)
       expect(subject[:reply_by_email_enabled]).to eq(Gitlab::IncomingEmail.enabled?)
       expect(subject[:container_registry_enabled]).to eq(Gitlab.config.registry.enabled)
       expect(subject[:gitlab_shared_runners_enabled]).to eq(Gitlab.config.gitlab_ci.shared_runners_enabled)