diff options
| -rw-r--r-- | doc/integration/omniauth.md | 2 | ||||
| -rw-r--r-- | lib/gitlab/auth/o_auth/user.rb | 17 | ||||
| -rw-r--r-- | spec/lib/gitlab/auth/o_auth/user_spec.rb | 11 |
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 } } |