Refactor OmniauthCallbacksController to remove duplication

Moves LDAP to its own controller with tests
Provides path forward for implementing GroupSaml

6 files changed, 101 insertions, 2 deletions

diff --git a/lib/gitlab/auth/ldap/user.rb b/lib/gitlab/auth/ldap/user.rb
index 068212d9a21..604c2d222e9 100644
--- a/lib/gitlab/auth/ldap/user.rb
+++ b/lib/gitlab/auth/ldap/user.rb
@@ -8,6 +8,8 @@ module Gitlab
   module Auth
     module LDAP
       class User < Gitlab::Auth::OAuth::User
+        extend ::Gitlab::Utils::Override
+
         class << self
           def find_by_uid_and_provider(uid, provider)
             identity = ::Identity.with_extern_uid(provider, uid).take
@@ -33,6 +35,11 @@ module Gitlab
           gl_user.changed? || gl_user.identities.any?(&:changed?)
         end
 
+        override :omniauth_should_save?
+        def omniauth_should_save?
+          changed? && super
+        end
+
         def block_after_signup?
           ldap_config.block_auto_created_users
         end
@@ -41,6 +48,10 @@ module Gitlab
           Gitlab::Auth::LDAP::Access.allowed?(gl_user)
         end
 
+        def valid_sign_in?
+          allowed?
+        end
+
         def ldap_config
           Gitlab::Auth::LDAP::Config.new(auth_hash.provider)
         end
diff --git a/lib/gitlab/auth/o_auth/identity_linker.rb b/lib/gitlab/auth/o_auth/identity_linker.rb
new file mode 100644
index 00000000000..cfa83ba2a55
--- /dev/null
+++ b/lib/gitlab/auth/o_auth/identity_linker.rb
@@ -0,0 +1,15 @@
+module Gitlab
+  module Auth
+    module OAuth
+      class IdentityLinker < OmniauthIdentityLinkerBase
+        def create_or_update
+          current_user.identities
+                      .with_extern_uid(oauth['provider'], oauth['uid'])
+                      .first_or_create(extern_uid: oauth['uid'])
+
+          @created = true
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/auth/o_auth/user.rb b/lib/gitlab/auth/o_auth/user.rb
index d0c6b0386ba..f2923c29163 100644
--- a/lib/gitlab/auth/o_auth/user.rb
+++ b/lib/gitlab/auth/o_auth/user.rb
@@ -30,6 +30,10 @@ module Gitlab
           gl_user.try(:valid?)
         end
 
+        def valid_sign_in?
+          valid? && persisted?
+        end
+
         def save(provider = 'OAuth')
           raise SigninDisabledForProviderError if oauth_provider_disabled?
           raise SignupDisabledError unless gl_user
@@ -64,8 +68,18 @@ module Gitlab
           user
         end
 
+        def find_and_update!
+          save if omniauth_should_save?
+
+          gl_user
+        end
+
         protected
 
+        def omniauth_should_save?
+          true
+        end
+
         def add_or_update_user_identities
           return unless gl_user
 
diff --git a/lib/gitlab/auth/omniauth_identity_linker_base.rb b/lib/gitlab/auth/omniauth_identity_linker_base.rb
new file mode 100644
index 00000000000..c60d9f70a99
--- /dev/null
+++ b/lib/gitlab/auth/omniauth_identity_linker_base.rb
@@ -0,0 +1,21 @@
+module Gitlab
+  module Auth
+    class OmniauthIdentityLinkerBase
+      attr_reader :current_user, :oauth
+
+      def initialize(current_user, oauth)
+        @current_user = current_user
+        @oauth = oauth
+        @created = false
+      end
+
+      def created?
+        @created
+      end
+
+      def create_or_update
+        raise NotImplementedError
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/auth/saml/identity_linker.rb b/lib/gitlab/auth/saml/identity_linker.rb
new file mode 100644
index 00000000000..d5f97f01df3
--- /dev/null
+++ b/lib/gitlab/auth/saml/identity_linker.rb
@@ -0,0 +1,27 @@
+module Gitlab
+  module Auth
+    module Saml
+      class IdentityLinker < OmniauthIdentityLinkerBase
+        def create_or_update
+          if find_saml_identity.nil?
+            create_saml_identity
+
+            @created = true
+          else
+            @created = false
+          end
+        end
+
+        protected
+
+        def find_saml_identity
+          current_user.identities.with_extern_uid(:saml, oauth['uid']).take
+        end
+
+        def create_saml_identity
+          current_user.identities.create(extern_uid: oauth['uid'], provider: :saml)
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/auth/saml/user.rb b/lib/gitlab/auth/saml/user.rb
index d4024e9ec39..557e6aa21a4 100644
--- a/lib/gitlab/auth/saml/user.rb
+++ b/lib/gitlab/auth/saml/user.rb
@@ -7,6 +7,8 @@ module Gitlab
   module Auth
     module Saml
       class User < Gitlab::Auth::OAuth::User
+        extend ::Gitlab::Utils::Override
+
         def save
           super('SAML')
         end
@@ -21,7 +23,7 @@ module Gitlab
           if external_users_enabled? && user
             # Check if there is overlap between the user's groups and the external groups
             # setting then set user as external or internal.
-            user.external = !(auth_hash.groups & Gitlab::Auth::Saml::Config.external_groups).empty?
+            user.external = !(auth_hash.groups & saml_config.external_groups).empty?
           end
 
           user
@@ -33,14 +35,23 @@ module Gitlab
           gl_user.changed? || gl_user.identities.any?(&:changed?)
         end
 
+        override :omniauth_should_save?
+        def omniauth_should_save?
+          changed? && super
+        end
+
         protected
 
+        def saml_config
+          Gitlab::Auth::Saml::Config
+        end
+
         def auto_link_saml_user?
           Gitlab.config.omniauth.auto_link_saml_user
         end
 
         def external_users_enabled?
-          !Gitlab::Auth::Saml::Config.external_groups.nil?
+          !saml_config.external_groups.nil?
         end
 
         def auth_hash=(auth_hash)