Commit e9a2ab11 authored by Douwe Maan's avatar Douwe Maan Committed by Robert Speicher

Merge branch 'ldap-special-chars-fix' into 'master'

Fix identity and user retrieval when special characters are used

Fixes #4023

I also added tests to make sure the user with special characters in his name is returned correctly.

@rspeicher this probably should be added to 8.3 as a patch.

See merge request !2176
parent 2af68713
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
class Identity < ActiveRecord::Base class Identity < ActiveRecord::Base
include Sortable include Sortable
include CaseSensitivity
belongs_to :user belongs_to :user
validates :provider, presence: true validates :provider, presence: true
......
...@@ -14,7 +14,7 @@ module Gitlab ...@@ -14,7 +14,7 @@ module Gitlab
# LDAP distinguished name is case-insensitive # LDAP distinguished name is case-insensitive
identity = ::Identity. identity = ::Identity.
where(provider: provider). where(provider: provider).
where('lower(extern_uid) = ?', uid.mb_chars.downcase.to_s).last iwhere(extern_uid: uid).last
identity && identity.user identity && identity.user
end end
end end
...@@ -31,7 +31,7 @@ module Gitlab ...@@ -31,7 +31,7 @@ module Gitlab
def find_by_uid_and_provider def find_by_uid_and_provider
self.class.find_by_uid_and_provider( self.class.find_by_uid_and_provider(
auth_hash.uid.downcase, auth_hash.provider) auth_hash.uid, auth_hash.provider)
end end
def find_by_email def find_by_email
...@@ -47,7 +47,7 @@ module Gitlab ...@@ -47,7 +47,7 @@ module Gitlab
# find_or_initialize_by doesn't update `gl_user.identities`, and isn't autosaved. # 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.find { |identity| identity.provider == auth_hash.provider }
identity ||= gl_user.identities.build(provider: auth_hash.provider) identity ||= gl_user.identities.build(provider: auth_hash.provider)
# For a new user set extern_uid to the LDAP DN # For a new user set extern_uid to the LDAP DN
# For an existing user with matching email but changed DN, update the DN. # For an existing user with matching email but changed DN, update the DN.
# For an existing user with no change in DN, this line changes nothing. # For an existing user with no change in DN, this line changes nothing.
......
...@@ -64,7 +64,7 @@ module Gitlab ...@@ -64,7 +64,7 @@ module Gitlab
# If a corresponding person exists with same uid in a LDAP server, # If a corresponding person exists with same uid in a LDAP server,
# set up a Gitlab user with dual LDAP and Omniauth identities. # set up a Gitlab user with dual LDAP and Omniauth identities.
if user = Gitlab::LDAP::User.find_by_uid_and_provider(ldap_person.dn.downcase, ldap_person.provider) 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. # Case when a LDAP user already exists in Gitlab. Add the Omniauth identity to existing account.
user.identities.build(extern_uid: auth_hash.uid, provider: auth_hash.provider) user.identities.build(extern_uid: auth_hash.uid, provider: auth_hash.provider)
else else
......
...@@ -42,6 +42,21 @@ describe Gitlab::LDAP::User, lib: true do ...@@ -42,6 +42,21 @@ describe Gitlab::LDAP::User, lib: true do
end end
end end
describe '.find_by_uid_and_provider' do
it 'retrieves the correct user' do
special_info = {
name: 'John Åström',
email: 'john@example.com',
nickname: 'jastrom'
}
special_hash = OmniAuth::AuthHash.new(uid: 'CN=John Åström,CN=Users,DC=Example,DC=com', provider: 'ldapmain', info: special_info)
special_chars_user = described_class.new(special_hash)
user = special_chars_user.save
expect(described_class.find_by_uid_and_provider(special_hash.uid, special_hash.provider)).to eq user
end
end
describe :find_or_create do describe :find_or_create do
it "finds the user if already existing" do it "finds the user if already existing" do
create(:omniauth_user, extern_uid: 'my-uid', provider: 'ldapmain') create(:omniauth_user, extern_uid: 'my-uid', provider: 'ldapmain')
......
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