Merge branch '33493-attempt-to-link-saml-users-to-ldap-by-email' into 'master'

Attempt to link saml users to ldap by email

Closes #33493

See merge request gitlab-org/gitlab-ce!14216

3 files changed, 21 insertions, 33 deletions

diff --git a/lib/gitlab/ldap/adapter.rb b/lib/gitlab/ldap/adapter.rb
index cd7e4ca7b7e..0afaa2306b5 100644
--- a/lib/gitlab/ldap/adapter.rb
+++ b/lib/gitlab/ldap/adapter.rb
@@ -22,8 +22,8 @@ module Gitlab
         Gitlab::LDAP::Config.new(provider)
       end
 
-      def users(field, value, limit = nil)
-        options = user_options(field, value, limit)
+      def users(fields, value, limit = nil)
+        options = user_options(Array(fields), value, limit)
 
         entries = ldap_search(options).select do |entry|
           entry.respond_to? config.uid
@@ -72,20 +72,24 @@ module Gitlab
 
       private
 
-      def user_options(field, value, limit)
-        options = { attributes: Gitlab::LDAP::Person.ldap_attributes(config).compact.uniq }
+      def user_options(fields, value, limit)
+        options = {
+          attributes: Gitlab::LDAP::Person.ldap_attributes(config).compact.uniq,
+          base: config.base
+        }
+
         options[:size] = limit if limit
 
-        if field.to_sym == :dn
+        if fields.include?('dn')
+          raise ArgumentError, 'It is not currently possible to search the DN and other fields at the same time.' if fields.size > 1
+
           options[:base] = value
           options[:scope] = Net::LDAP::SearchScope_BaseObject
-          options[:filter] = user_filter
         else
-          options[:base] = config.base
-          options[:filter] = user_filter(Net::LDAP::Filter.eq(field, value))
+          filter = fields.map { |field| Net::LDAP::Filter.eq(field, value) }.inject(:|)
         end
 
-        options
+        options.merge(filter: user_filter(filter))
       end
 
       def user_filter(filter = nil)
diff --git a/lib/gitlab/ldap/person.rb b/lib/gitlab/ldap/person.rb
index 4d6f8ac79de..9a6f7827b16 100644
--- a/lib/gitlab/ldap/person.rb
+++ b/lib/gitlab/ldap/person.rb
@@ -17,6 +17,12 @@ module Gitlab
         adapter.user('dn', dn)
       end
 
+      def self.find_by_email(email, adapter)
+        email_fields = adapter.config.attributes['email']
+
+        adapter.user(email_fields, email)
+      end
+
       def self.disabled_via_active_directory?(dn, adapter)
         adapter.dn_matches_filter?(dn, AD_USER_DISABLED)
       end
diff --git a/lib/gitlab/ldap/user.rb b/lib/gitlab/ldap/user.rb
index 3bf27b37ae6..1793097363e 100644
--- a/lib/gitlab/ldap/user.rb
+++ b/lib/gitlab/ldap/user.rb
@@ -17,41 +17,19 @@ module Gitlab
         end
       end
 
-      def initialize(auth_hash)
-        super
-        update_user_attributes
-      end
-
       def save
         super('LDAP')
       end
 
       # instance methods
-      def gl_user
-        @gl_user ||= find_by_uid_and_provider || find_by_email || build_new_user
+      def find_user
+        find_by_uid_and_provider || find_by_email || build_new_user
       end
 
       def find_by_uid_and_provider
         self.class.find_by_uid_and_provider(auth_hash.uid, auth_hash.provider)
       end
 
-      def find_by_email
-        ::User.find_by(email: auth_hash.email.downcase) if auth_hash.has_attribute?(:email)
-      end
-
-      def update_user_attributes
-        if persisted?
-          # find_or_initialize_by doesn't update `gl_user.identities`, and isn't autosaved.
-          identity = gl_user.identities.find { |identity|  identity.provider == auth_hash.provider }
-          identity ||= gl_user.identities.build(provider: auth_hash.provider)
-
-          # For a new identity set extern_uid to the LDAP DN
-          # For an existing identity with matching email but changed DN, update the DN.
-          # For an existing identity with no change in DN, this line changes nothing.
-          identity.extern_uid = auth_hash.uid
-        end
-      end
-
       def changed?
         gl_user.changed? || gl_user.identities.any?(&:changed?)
       end