Commit 42603af3 authored by Robert Speicher's avatar Robert Speicher

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
parents 03b7fe71 b7def88c
...@@ -35,7 +35,7 @@ module Gitlab ...@@ -35,7 +35,7 @@ module Gitlab
end end
def find_by_email def find_by_email
::User.find_by(email: auth_hash.email) ::User.find_by(email: auth_hash.email.downcase)
end end
def update_user_attributes def update_user_attributes
......
...@@ -13,6 +13,17 @@ describe Gitlab::LDAP::User do ...@@ -13,6 +13,17 @@ describe Gitlab::LDAP::User do
let(:auth_hash) do let(:auth_hash) do
OmniAuth::AuthHash.new(uid: 'my-uid', provider: 'ldapmain', info: info) OmniAuth::AuthHash.new(uid: 'my-uid', provider: 'ldapmain', info: info)
end 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 describe :changed? do
it "marks existing ldap user as changed" do it "marks existing ldap user as changed" do
...@@ -57,6 +68,16 @@ describe Gitlab::LDAP::User do ...@@ -57,6 +68,16 @@ describe Gitlab::LDAP::User do
expect(existing_user.id).to eql ldap_user.gl_user.id expect(existing_user.id).to eql ldap_user.gl_user.id
end 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 it 'maintains an identity per provider' do
existing_user = create(:omniauth_user, email: 'john@example.com', provider: 'twitter') existing_user = create(:omniauth_user, email: 'john@example.com', provider: 'twitter')
expect(existing_user.identities.count).to eql(1) expect(existing_user.identities.count).to eql(1)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment