Commit 13cd7cd7 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'ce-1974-update-user-name-upon-ldap-sync' into 'master'

Backport 'Update user name upon LDAP sync' from EE

See merge request gitlab-org/gitlab-ce!26432
parents 80a65cb8 45da7dd3
...@@ -256,7 +256,7 @@ gitlab_rails['omniauth_enabled'] = false ...@@ -256,7 +256,7 @@ gitlab_rails['omniauth_enabled'] = false
You can enable profile syncing from selected OmniAuth providers and for all or for specific user information. You can enable profile syncing from selected OmniAuth providers and for all or for specific user information.
When authenticating using LDAP, the user's email is always synced. When authenticating using LDAP, the user's name and email are always synced.
```ruby ```ruby
gitlab_rails['sync_profile_from_provider'] = ['twitter', 'google_oauth2'] gitlab_rails['sync_profile_from_provider'] = ['twitter', 'google_oauth2']
......
...@@ -146,7 +146,6 @@ module Gitlab ...@@ -146,7 +146,6 @@ module Gitlab
Gitlab::Auth::LDAP::Person.find_by_uid(auth_hash.uid, adapter) || Gitlab::Auth::LDAP::Person.find_by_uid(auth_hash.uid, adapter) ||
Gitlab::Auth::LDAP::Person.find_by_email(auth_hash.uid, adapter) || Gitlab::Auth::LDAP::Person.find_by_email(auth_hash.uid, adapter) ||
Gitlab::Auth::LDAP::Person.find_by_dn(auth_hash.uid, adapter) Gitlab::Auth::LDAP::Person.find_by_dn(auth_hash.uid, adapter)
rescue Gitlab::Auth::LDAP::LDAPConnectionError rescue Gitlab::Auth::LDAP::LDAPConnectionError
nil nil
end end
...@@ -200,22 +199,19 @@ module Gitlab ...@@ -200,22 +199,19 @@ module Gitlab
# Give preference to LDAP for sensitive information when creating a linked account # Give preference to LDAP for sensitive information when creating a linked account
if creating_linked_ldap_user? if creating_linked_ldap_user?
username = ldap_person.username.presence username = ldap_person.username.presence
name = ldap_person.name.presence
email = ldap_person.email.first.presence email = ldap_person.email.first.presence
end end
username ||= auth_hash.username username ||= auth_hash.username
name ||= auth_hash.name
email ||= auth_hash.email email ||= auth_hash.email
valid_username = ::Namespace.clean_path(username) valid_username = ::Namespace.clean_path(username)
valid_username = Uniquify.new.string(valid_username) { |s| !NamespacePathValidator.valid_path?(s) }
uniquify = Uniquify.new
valid_username = uniquify.string(valid_username) { |s| !NamespacePathValidator.valid_path?(s) }
name = auth_hash.name
name = valid_username if name.strip.empty?
{ {
name: name, name: name.strip.presence || valid_username,
username: valid_username, username: valid_username,
email: email, email: email,
password: auth_hash.password, password: auth_hash.password,
...@@ -248,8 +244,9 @@ module Gitlab ...@@ -248,8 +244,9 @@ module Gitlab
metadata.provider = auth_hash.provider metadata.provider = auth_hash.provider
end end
if creating_linked_ldap_user? && gl_user.email == ldap_person.email.first if creating_linked_ldap_user?
metadata.set_attribute_synced(:email, true) metadata.set_attribute_synced(:name, true) if gl_user.name == ldap_person.name
metadata.set_attribute_synced(:email, true) if gl_user.email == ldap_person.email.first
metadata.provider = ldap_person.provider metadata.provider = ldap_person.provider
end end
end end
......
...@@ -207,6 +207,7 @@ describe Gitlab::Auth::OAuth::User do ...@@ -207,6 +207,7 @@ describe Gitlab::Auth::OAuth::User do
before do before do
allow(ldap_user).to receive(:uid) { uid } allow(ldap_user).to receive(:uid) { uid }
allow(ldap_user).to receive(:username) { uid } allow(ldap_user).to receive(:username) { uid }
allow(ldap_user).to receive(:name) { 'John Doe' }
allow(ldap_user).to receive(:email) { ['johndoe@example.com', 'john2@example.com'] } allow(ldap_user).to receive(:email) { ['johndoe@example.com', 'john2@example.com'] }
allow(ldap_user).to receive(:dn) { dn } allow(ldap_user).to receive(:dn) { dn }
end end
...@@ -221,6 +222,7 @@ describe Gitlab::Auth::OAuth::User do ...@@ -221,6 +222,7 @@ describe Gitlab::Auth::OAuth::User do
it "creates a user with dual LDAP and omniauth identities" do it "creates a user with dual LDAP and omniauth identities" do
expect(gl_user).to be_valid expect(gl_user).to be_valid
expect(gl_user.username).to eql uid expect(gl_user.username).to eql uid
expect(gl_user.name).to eql 'John Doe'
expect(gl_user.email).to eql 'johndoe@example.com' expect(gl_user.email).to eql 'johndoe@example.com'
expect(gl_user.identities.length).to be 2 expect(gl_user.identities.length).to be 2
identities_as_hash = gl_user.identities.map { |id| { provider: id.provider, extern_uid: id.extern_uid } } identities_as_hash = gl_user.identities.map { |id| { provider: id.provider, extern_uid: id.extern_uid } }
...@@ -232,11 +234,13 @@ describe Gitlab::Auth::OAuth::User do ...@@ -232,11 +234,13 @@ describe Gitlab::Auth::OAuth::User do
) )
end end
it "has email set as synced" do it "has name and email set as synced" do
expect(gl_user.user_synced_attributes_metadata.name_synced).to be_truthy
expect(gl_user.user_synced_attributes_metadata.email_synced).to be_truthy expect(gl_user.user_synced_attributes_metadata.email_synced).to be_truthy
end end
it "has email set as read-only" do it "has name and email set as read-only" do
expect(gl_user.read_only_attribute?(:name)).to be_truthy
expect(gl_user.read_only_attribute?(:email)).to be_truthy expect(gl_user.read_only_attribute?(:email)).to be_truthy
end end
...@@ -246,7 +250,7 @@ describe Gitlab::Auth::OAuth::User do ...@@ -246,7 +250,7 @@ describe Gitlab::Auth::OAuth::User do
end end
context "and LDAP user has an account already" do context "and LDAP user has an account already" do
let!(:existing_user) { create(:omniauth_user, email: 'john@example.com', extern_uid: dn, provider: 'ldapmain', username: 'john') } let!(:existing_user) { create(:omniauth_user, name: 'John Doe', email: 'john@example.com', extern_uid: dn, provider: 'ldapmain', username: 'john') }
it "adds the omniauth identity to the LDAP account" do it "adds the omniauth identity to the LDAP account" do
allow(Gitlab::Auth::LDAP::Person).to receive(:find_by_uid).and_return(ldap_user) allow(Gitlab::Auth::LDAP::Person).to receive(:find_by_uid).and_return(ldap_user)
...@@ -254,6 +258,7 @@ describe Gitlab::Auth::OAuth::User do ...@@ -254,6 +258,7 @@ describe Gitlab::Auth::OAuth::User do
expect(gl_user).to be_valid expect(gl_user).to be_valid
expect(gl_user.username).to eql 'john' expect(gl_user.username).to eql 'john'
expect(gl_user.name).to eql 'John Doe'
expect(gl_user.email).to eql 'john@example.com' expect(gl_user.email).to eql 'john@example.com'
expect(gl_user.identities.length).to be 2 expect(gl_user.identities.length).to be 2
identities_as_hash = gl_user.identities.map { |id| { provider: id.provider, extern_uid: id.extern_uid } } identities_as_hash = gl_user.identities.map { |id| { provider: id.provider, extern_uid: id.extern_uid } }
......
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