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
This commit is contained in:
Douwe Maan 2015-12-24 16:35:00 +00:00
commit 835333c426
4 changed files with 20 additions and 4 deletions

View file

@ -12,6 +12,7 @@
class Identity < ActiveRecord::Base
include Sortable
include CaseSensitivity
belongs_to :user
validates :provider, presence: true

View file

@ -14,7 +14,7 @@ module Gitlab
# LDAP distinguished name is case-insensitive
identity = ::Identity.
where(provider: provider).
where('lower(extern_uid) = ?', uid.mb_chars.downcase.to_s).last
iwhere(extern_uid: uid).last
identity && identity.user
end
end
@ -31,7 +31,7 @@ module Gitlab
def 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
def find_by_email
@ -47,7 +47,7 @@ module Gitlab
# 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 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 no change in DN, this line changes nothing.

View file

@ -64,7 +64,7 @@ module Gitlab
# If a corresponding person exists with same uid in a LDAP server,
# 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.
user.identities.build(extern_uid: auth_hash.uid, provider: auth_hash.provider)
else

View file

@ -42,6 +42,21 @@ describe Gitlab::LDAP::User, lib: true do
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
it "finds the user if already existing" do
create(:omniauth_user, extern_uid: 'my-uid', provider: 'ldapmain')