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

3 files changed, 16 insertions, 14 deletions

diff --git a/doc/integration/omniauth.md b/doc/integration/omniauth.md
index 69bbd05c367..2932c884d04 100644
--- a/doc/integration/omniauth.md
+++ b/doc/integration/omniauth.md
@@ -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.
 
-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
   gitlab_rails['sync_profile_from_provider'] = ['twitter', 'google_oauth2']
diff --git a/lib/gitlab/auth/o_auth/user.rb b/lib/gitlab/auth/o_auth/user.rb
index f38c5d57c44..09d1d79fefc 100644
--- a/lib/gitlab/auth/o_auth/user.rb
+++ b/lib/gitlab/auth/o_auth/user.rb
@@ -146,7 +146,6 @@ module Gitlab
           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_dn(auth_hash.uid, adapter)
-
         rescue Gitlab::Auth::LDAP::LDAPConnectionError
           nil
         end
@@ -200,22 +199,19 @@ module Gitlab
           # Give preference to LDAP for sensitive information when creating a linked account
           if creating_linked_ldap_user?
             username = ldap_person.username.presence
+            name = ldap_person.name.presence
             email = ldap_person.email.first.presence
           end
 
           username ||= auth_hash.username
+          name ||= auth_hash.name
           email ||= auth_hash.email
 
           valid_username = ::Namespace.clean_path(username)
-
-          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?
+          valid_username = Uniquify.new.string(valid_username) { |s| !NamespacePathValidator.valid_path?(s) }
 
           {
-            name:                       name,
+            name:                       name.strip.presence || valid_username,
             username:                   valid_username,
             email:                      email,
             password:                   auth_hash.password,
@@ -248,8 +244,9 @@ module Gitlab
             metadata.provider = auth_hash.provider
           end
 
-          if creating_linked_ldap_user? && gl_user.email == ldap_person.email.first
-            metadata.set_attribute_synced(:email, true)
+          if creating_linked_ldap_user?
+            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
           end
         end
diff --git a/spec/lib/gitlab/auth/o_auth/user_spec.rb b/spec/lib/gitlab/auth/o_auth/user_spec.rb
index dcbd12fe190..b765c265e69 100644
--- a/spec/lib/gitlab/auth/o_auth/user_spec.rb
+++ b/spec/lib/gitlab/auth/o_auth/user_spec.rb
@@ -207,6 +207,7 @@ describe Gitlab::Auth::OAuth::User do
             before do
               allow(ldap_user).to receive(:uid) { 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(:dn) { dn }
             end
@@ -221,6 +222,7 @@ describe Gitlab::Auth::OAuth::User do
               it "creates a user with dual LDAP and omniauth identities" do
                 expect(gl_user).to be_valid
                 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.identities.length).to be 2
                 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
                 )
               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
               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
               end
 
@@ -246,7 +250,7 @@ describe Gitlab::Auth::OAuth::User do
             end
 
             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
                 allow(Gitlab::Auth::LDAP::Person).to receive(:find_by_uid).and_return(ldap_user)
 
@@ -254,6 +258,7 @@ describe Gitlab::Auth::OAuth::User do
 
                 expect(gl_user).to be_valid
                 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.identities.length).to be 2
                 identities_as_hash = gl_user.identities.map { |id| { provider: id.provider, extern_uid: id.extern_uid } }