Attempt to link saml users to ldap by email

changelogs/unreleased/33493-attempt-to-link-saml-users-to-ldap-by-email.yml

@@ -0,0 +1,5 @@
+---
+title: Link SAML users to LDAP by email.
+merge_request: 14216
+author:
+type: changed

lib/gitlab/ldap/person.rb

@@ -17,6 +17,10 @@ module Gitlab
         adapter.user('dn', dn)
       end
 
+      def self.find_by_email(email, adapter)
+        Array(adapter.config.attributes['email']).find { |attr| adapter.user(attr, email) }
+      end
+
       def self.disabled_via_active_directory?(dn, adapter)
         adapter.dn_matches_filter?(dn, AD_USER_DISABLED)
       end

lib/gitlab/o_auth/user.rb

@@ -108,9 +108,12 @@ module Gitlab
       end
 
       def find_ldap_person(auth_hash, adapter)
-        by_uid = Gitlab::LDAP::Person.find_by_uid(auth_hash.uid, adapter)
+        person = Gitlab::LDAP::Person.find_by_uid(auth_hash.uid, adapter)
         # The `uid` might actually be a DN. Try it next.
-        by_uid || Gitlab::LDAP::Person.find_by_dn(auth_hash.uid, adapter)
+        person ||= Gitlab::LDAP::Person.find_by_dn(auth_hash.uid, adapter)
+
+        # The `uid` might actually be a Email. Try it next.
+        person || Gitlab::LDAP::Person.find_by_email(auth_hash.uid, adapter)
       end
 
       def ldap_config

lib/gitlab/saml/user.rb

@@ -11,16 +11,16 @@ module Gitlab
       end
 
       def gl_user
-        if auto_link_ldap_user?
+        if auto_link_saml_user?
+          @user ||= find_by_email
+        end
+
+        if auto_link_ldap_user? && !@user&.ldap_user?
           @user ||= find_or_create_ldap_user
         end
 
         @user ||= find_by_uid_and_provider
 
-        if auto_link_saml_user?
-          @user ||= find_by_email
-        end
-
         if signup_enabled?
           @user ||= build_new_user
         end
@@ -42,7 +42,11 @@ module Gitlab
       def find_by_email
         if auth_hash.has_attribute?(:email)
           user = ::User.find_by(email: auth_hash.email.downcase)
-          user.identities.new(extern_uid: auth_hash.uid, provider: auth_hash.provider) if user
+
+          if user&.identities&.empty?
+            user.identities.new(extern_uid: auth_hash.uid, provider: auth_hash.provider)
+          end
+
           user
         end
       end

spec/lib/gitlab/saml/user_spec.rb

@@ -170,6 +170,7 @@ describe Gitlab::Saml::User do
               allow(ldap_user).to receive(:dn) { 'uid=user1,ou=People,dc=example' }
               allow(Gitlab::LDAP::Person).to receive(:find_by_uid).and_return(ldap_user)
               allow(Gitlab::LDAP::Person).to receive(:find_by_dn).and_return(ldap_user)
+              allow(Gitlab::LDAP::Person).to receive(:find_by_email).and_return(ldap_user)
             end
 
             context 'and no account for the LDAP user' do
@@ -195,6 +196,82 @@ describe Gitlab::Saml::User do
                        username: 'john')
               end
 
+              shared_examples 'find ldap person' do |uid_type, uid|
+                before do
+                  allow(Gitlab::LDAP::Person).to receive(:"find_by_#{uid_type}").and_return(ldap_user)
+                end
+
+                it 'adds the omniauth identity to the LDAP account' do
+                  identities = [
+                    { provider: 'ldapmain', extern_uid: 'uid=user1,ou=People,dc=example' },
+                    { provider: 'saml', extern_uid: extern_uid }
+                  ]
+
+                  identities_as_hash = gl_user.identities.map do |id|
+                    { provider: id.provider, extern_uid: id.extern_uid }
+                  end
+
+                  saml_user.save
+
+                  expect(gl_user).to be_valid
+                  expect(gl_user.username).to eql 'john'
+                  expect(gl_user.email).to eql 'john@mail.com'
+                  expect(gl_user.identities.length).to be 2
+                  expect(identities_as_hash).to match_array(identities)
+                end
+              end
+
+              context 'when uid is an uid' do
+                it_behaves_like 'find ldap person', 'uid' do
+                  let(:extern_uid) { uid }
+                  let(:auth_hash) do
+                    OmniAuth::AuthHash.new(
+                      uid: uid,
+                      provider: provider,
+                      info: info_hash,
+                      extra: {
+                        raw_info: OneLogin::RubySaml::Attributes.new(
+                          { 'groups' => %w(Developers Freelancers Designers) }
+                        )
+                      })
+                  end
+                end
+              end
+
+              context 'when uid is a dn' do
+                it_behaves_like 'find ldap person', 'email' do
+                  let(:extern_uid) { 'uid=user1,ou=People,dc=example' }
+                  let(:auth_hash) do
+                    OmniAuth::AuthHash.new(
+                      uid: extern_uid,
+                      provider: provider,
+                      info: info_hash,
+                      extra: {
+                        raw_info: OneLogin::RubySaml::Attributes.new(
+                          { 'groups' => %w(Developers Freelancers Designers) }
+                        )
+                      })
+                  end
+                end
+              end
+
+              context 'when uid is an email' do
+                it_behaves_like 'find ldap person', 'email' do
+                  let(:extern_uid) { 'john@mail.com' }
+                  let(:auth_hash) do
+                    OmniAuth::AuthHash.new(
+                      uid: extern_uid,
+                      provider: provider,
+                      info: info_hash,
+                      extra: {
+                        raw_info: OneLogin::RubySaml::Attributes.new(
+                          { 'groups' => %w(Developers Freelancers Designers) }
+                        )
+                      })
+                  end
+                end
+              end
+
               it 'adds the omniauth identity to the LDAP account' do
                 saml_user.save