Moved `find_or_create_ldap_user` method to parent class and added logging.

3 files changed, 13 insertions, 31 deletions

diff --git a/lib/gitlab/o_auth/user.rb b/lib/gitlab/o_auth/user.rb
index 268ee115028..5e52979093d 100644
--- a/lib/gitlab/o_auth/user.rb
+++ b/lib/gitlab/o_auth/user.rb
@@ -69,13 +69,19 @@ module Gitlab
         return unless ldap_person
 
         # If a corresponding person exists with same uid in a LDAP server,
-        # set up a Gitlab user with dual LDAP and Omniauth identities.
-        if user = Gitlab::LDAP::User.find_by_uid_and_provider(ldap_person.dn, ldap_person.provider)
-          # Case when a LDAP user already exists in Gitlab. Add the Omniauth identity to existing account.
+        # check if the user already has a GitLab account.
+        if (user = Gitlab::LDAP::User.find_by_uid_and_provider(ldap_person.dn, ldap_person.provider))
+          # Case when a LDAP user already exists in Gitlab. Add the OAuth identity to existing account.
+          log.info "LDAP account found for user #{user.username}. Building new identity."
           user.identities.build(extern_uid: auth_hash.uid, provider: auth_hash.provider)
         else
-          # No account in Gitlab yet: create it and add the LDAP identity
-          user = build_new_user
+          log.info 'No existing LDAP account was found in GitLab. Checking for OAuth account.'
+          user = find_by_uid_and_provider
+          if user.nil?
+            log.info 'No user found with the specified OAuth provider. Creating a new one.'
+            user = build_new_user
+          end
+          log.info "Correct account has been found. Adding LDAP identity to user: #{user.username}."
           user.identities.new(provider: ldap_person.provider, extern_uid: ldap_person.dn)
         end
 
@@ -96,7 +102,7 @@ module Gitlab
         # Look for a corresponding person with same uid in any of the configured LDAP providers
         Gitlab::LDAP::Config.providers.each do |provider|
           adapter = Gitlab::LDAP::Adapter.new(provider)
-          @ldap_person = Gitlab::LDAP::Person.find_by_dn(auth_hash.uid, adapter)
+          @ldap_person = Gitlab::LDAP::Person.find_by_uid(auth_hash.uid, adapter)
           break if @ldap_person
         end
         @ldap_person
diff --git a/lib/gitlab/saml/user.rb b/lib/gitlab/saml/user.rb
index 6f7d4825ae5..8943022612c 100644
--- a/lib/gitlab/saml/user.rb
+++ b/lib/gitlab/saml/user.rb
@@ -62,30 +62,6 @@ module Gitlab
         !Gitlab::Saml::Config.external_groups.nil?
       end
 
-      def find_or_create_ldap_user
-        return unless ldap_person
-
-        # If a corresponding person exists with same uid in a LDAP server,
-        # check if the user already has a GitLab account
-        user = Gitlab::LDAP::User.find_by_uid_and_provider(ldap_person.dn, ldap_person.provider)
-        if user
-          # Case when a LDAP user already exists in Gitlab. Add the SAML identity to existing account.
-          user.identities.build(extern_uid: auth_hash.uid, provider: auth_hash.provider)
-        else
-          # No account found using LDAP in Gitlab yet: check if there is a SAML account with
-          # the passed uid and provider
-          user = find_by_uid_and_provider
-          if user.nil?
-            # No SAML account found, build a new user.
-            user = build_new_user
-          end
-          # Correct account is present, add the LDAP Identity to the user.
-          user.identities.new(provider: ldap_person.provider, extern_uid: ldap_person.dn)
-        end
-
-        user
-      end
-
       def auth_hash=(auth_hash)
         @auth_hash = Gitlab::Saml::AuthHash.new(auth_hash)
       end
diff --git a/spec/lib/gitlab/saml/user_spec.rb b/spec/lib/gitlab/saml/user_spec.rb
index f0a17244ff6..5957998e0f3 100644
--- a/spec/lib/gitlab/saml/user_spec.rb
+++ b/spec/lib/gitlab/saml/user_spec.rb
@@ -182,7 +182,7 @@ describe Gitlab::Saml::User, lib: true do
             context 'user has SAML user, and wants to add their LDAP identity' do
               it 'adds the LDAP identity to the existing SAML user' do
                 create(:omniauth_user, email: 'john@mail.com', extern_uid: 'uid=user1,ou=People,dc=example', provider: 'saml', username: 'john')
-                local_hash = OmniAuth::AuthHash.new(uid: 'uid=user1,ou=People,dc=example', provider: provider, info: info_hash, extra: { raw_info: OneLogin::RubySaml::Attributes.new({ 'groups' => %w(Developers Freelancers Designers) }) })
+                local_hash = OmniAuth::AuthHash.new(uid: 'uid=user1,ou=People,dc=example', provider: provider, info: info_hash)
                 local_saml_user = described_class.new(local_hash)
 
                 local_saml_user.save