Merge branch 'ee-48932-disable-saml-if-omniauth-is-disabled' into 'master'

EE: Resolve "Disable SAML and Bitbucket if OmniAuth is disabled"

See merge request gitlab-org/gitlab-ee!6501

app/controllers/sessions_controller.rb

@@ -158,6 +158,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?
 

app/helpers/auth_helper.rb

@@ -9,7 +9,7 @@ module AuthHelper
   end
 
   def omniauth_enabled?
-    Gitlab.config.omniauth.enabled
+    Gitlab::Auth.omniauth_enabled?
   end
 
   def provider_has_icon?(name)

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

changelogs/unreleased/48932-disable-saml-if-omniauth-is-disabled.yml

+---
+title: Disable SAML and Bitbucket if OmniAuth is disabled
+merge_request: 20608
+author:
+type: fixed

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

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

ee/lib/ee/gitlab/auth.rb

@@ -3,23 +3,6 @@ module EE
     module Auth
       extend ::Gitlab::Utils::Override
 
-      override :omniauth_customized_providers
-      def omniauth_customized_providers
-        @omniauth_customized_providers ||=
-          super.concat(%w[kerberos_spnego group_saml])
-      end
-
-      override :omniauth_setup_a_provider
-      def omniauth_setup_a_provider(provider)
-        super
-
-        if provider == 'group_saml'
-          OmniAuth.config.on_failure =
-            ::Gitlab::Auth::GroupSaml::FailureHandler.new(
-              OmniAuth.config.on_failure)
-        end
-      end
-
       override :find_with_user_password
       def find_with_user_password(login, password)
         if Devise.omniauth_providers.include?(:kerberos)

ee/lib/ee/gitlab/omniauth_initializer.rb

+module EE
+  module Gitlab
+    module OmniauthInitializer
+      extend ::Gitlab::Utils::Override
+
+      override :build_omniauth_customized_providers
+      def build_omniauth_customized_providers
+        super.concat(%i[kerberos_spnego group_saml])
+      end
+
+      override :setup_provider
+      def setup_provider(provider)
+        super
+
+        if provider == :group_saml
+          OmniAuth.config.on_failure =
+            ::Gitlab::Auth::GroupSaml::FailureHandler.new(
+              OmniAuth.config.on_failure)
+        end
+      end
+    end
+  end
+end

lib/gitlab/auth.rb

@@ -16,23 +16,8 @@ module Gitlab
     class << self
       prepend EE::Gitlab::Auth
 
-      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:)

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)

lib/gitlab/omniauth_initializer.rb

 module Gitlab
   class OmniauthInitializer
-    def self.enabled?
-      Gitlab.config.omniauth.enabled ||
-        Gitlab.config.omniauth.auto_sign_in_with_provider.present?
-    end
+    prepend ::EE::Gitlab::OmniauthInitializer
 
     def initialize(devise_config)
       @devise_config = devise_config
@@ -11,13 +8,16 @@ module Gitlab
 
     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 +76,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

lib/gitlab/usage_data.rb

@@ -132,7 +132,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?
         }

lib/tasks/gitlab/info.rake

@@ -69,8 +69,8 @@ namespace :gitlab do
       puts "Geo:\t\t#{Gitlab::Geo.enabled? ? "yes".color(:green) : "no"}"
       puts "Geo node:\t#{geo_node_type}" if Gitlab::Geo.enabled?
       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"

spec/lib/gitlab/usage_data_spec.rb

@@ -153,7 +153,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)