Merge branch 'fix_email_downcasing' into 'master'

LDAP email address downcasing

Fixes #2960

In the event we cannot match an LDAP user by DN we attempt to find an identity by email address and then update the DN. In this case the identity is matched by email address. 

If the user's email address in LDAP has an upper case character we cannot find a match in the GitLab database. GitLab downcases emails before the user object is saved.

This merge request downcases the email from LDAP before we lookup by email. I also added a test to prevent a regression.

See merge request !1550

2 files changed, 22 insertions, 1 deletions

diff --git a/lib/gitlab/ldap/user.rb b/lib/gitlab/ldap/user.rb
index 1ea7751e27d..4be99dd88c2 100644
--- a/lib/gitlab/ldap/user.rb
+++ b/lib/gitlab/ldap/user.rb
@@ -35,7 +35,7 @@ module Gitlab
       end
 
       def find_by_email
-        ::User.find_by(email: auth_hash.email)
+        ::User.find_by(email: auth_hash.email.downcase)
       end
 
       def update_user_attributes
diff --git a/spec/lib/gitlab/ldap/user_spec.rb b/spec/lib/gitlab/ldap/user_spec.rb
index fd2e5f6d0e1..b5b56a34952 100644
--- a/spec/lib/gitlab/ldap/user_spec.rb
+++ b/spec/lib/gitlab/ldap/user_spec.rb
@@ -13,6 +13,17 @@ describe Gitlab::LDAP::User do
   let(:auth_hash) do
     OmniAuth::AuthHash.new(uid: 'my-uid', provider: 'ldapmain', info: info)
   end
+  let(:ldap_user_upper_case) { Gitlab::LDAP::User.new(auth_hash_upper_case) }
+  let(:info_upper_case) do
+    {
+      name: 'John',
+      email: 'John@Example.com', # Email address has upper case chars
+      nickname: 'john'
+    }
+  end
+  let(:auth_hash_upper_case) do
+    OmniAuth::AuthHash.new(uid: 'my-uid', provider: 'ldapmain', info: info_upper_case)
+  end
 
   describe :changed? do
     it "marks existing ldap user as changed" do
@@ -57,6 +68,16 @@ describe Gitlab::LDAP::User do
       expect(existing_user.id).to eql ldap_user.gl_user.id
     end
 
+    it 'connects to existing ldap user if the extern_uid changes and email address has upper case characters' do
+      existing_user = create(:omniauth_user, email: 'john@example.com', extern_uid: 'old-uid', provider: 'ldapmain')
+      expect{ ldap_user_upper_case.save }.not_to change{ User.count }
+
+      existing_user.reload
+      expect(existing_user.ldap_identity.extern_uid).to eql 'my-uid'
+      expect(existing_user.ldap_identity.provider).to eql 'ldapmain'
+      expect(existing_user.id).to eql ldap_user.gl_user.id
+    end
+
     it 'maintains an identity per provider' do
       existing_user = create(:omniauth_user, email: 'john@example.com', provider: 'twitter')
       expect(existing_user.identities.count).to eql(1)