From a47a6d6fd56f92497b164d45ff9b53023202c4b1 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Fri, 31 Mar 2023 09:01:53 +0000 Subject: Add latest changes from gitlab-org/gitlab@15-10-stable-ee --- app/models/user_synced_attributes_metadata.rb | 26 ++++++--- config/initializers/1_settings.rb | 3 +- lib/gitlab/auth/ldap/config.rb | 4 ++ lib/gitlab/auth/o_auth/user.rb | 2 +- spec/lib/gitlab/auth/o_auth/user_spec.rb | 81 +++++++++++++++++---------- 5 files changed, 76 insertions(+), 40 deletions(-) diff --git a/app/models/user_synced_attributes_metadata.rb b/app/models/user_synced_attributes_metadata.rb index 4cd0e3fb828..6b23bce6406 100644 --- a/app/models/user_synced_attributes_metadata.rb +++ b/app/models/user_synced_attributes_metadata.rb @@ -14,7 +14,7 @@ class UserSyncedAttributesMetadata < ApplicationRecord def read_only_attributes return [] unless sync_profile_from_provider? - self.class.syncable_attributes.select { |key| synced?(key) } + SYNCABLE_ATTRIBUTES.select { |key| synced?(key) } end def synced?(attribute) @@ -26,12 +26,11 @@ class UserSyncedAttributesMetadata < ApplicationRecord end class << self - def syncable_attributes - if Gitlab.config.ldap.enabled && !Gitlab.config.ldap.sync_name - SYNCABLE_ATTRIBUTES - %i[name] - else - SYNCABLE_ATTRIBUTES - end + def syncable_attributes(provider = nil) + return SYNCABLE_ATTRIBUTES unless provider && ldap_provider?(provider) + return SYNCABLE_ATTRIBUTES if ldap_sync_name?(provider) + + SYNCABLE_ATTRIBUTES - %i[name] end end @@ -40,4 +39,17 @@ class UserSyncedAttributesMetadata < ApplicationRecord def sync_profile_from_provider? Gitlab::Auth::OAuth::Provider.sync_profile_from_provider?(provider) end + + class << self + def ldap_provider?(provider) + Gitlab::Auth::OAuth::Provider.ldap_provider?(provider) + end + + def ldap_sync_name?(provider) + return false unless provider + + config = Gitlab::Auth::Ldap::Config.new(provider) + config.enabled? && config.sync_name + end + end end diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 9cb1be45b68..35931a3caaa 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -27,7 +27,6 @@ end # backwards compatibility, we only have one host if Settings.ldap['enabled'] || Rails.env.test? - Settings.ldap['sync_name'] = true if Settings.ldap['sync_name'].nil? if Settings.ldap['host'].present? # We detected old LDAP configuration syntax. Update the config to make it # look like it was entered with the new syntax. @@ -68,6 +67,8 @@ if Settings.ldap['enabled'] || Rails.env.test? # `ssl_version` in favor of `tls_options` hash option. server['tls_options'] ||= {} + server['sync_name'] = true if server['sync_name'].nil? + if server['ssl_version'] || server['ca_file'] Gitlab::AppLogger.warn 'DEPRECATED: LDAP options `ssl_version` and `ca_file` should be nested within `tls_options`' end diff --git a/lib/gitlab/auth/ldap/config.rb b/lib/gitlab/auth/ldap/config.rb index 6c99b505797..30896637eff 100644 --- a/lib/gitlab/auth/ldap/config.rb +++ b/lib/gitlab/auth/ldap/config.rb @@ -184,6 +184,10 @@ module Gitlab options['lowercase_usernames'] end + def sync_name + options['sync_name'] + end + def name_proc if allow_username_or_email_login proc { |name| name.gsub(/@.*\z/, '') } diff --git a/lib/gitlab/auth/o_auth/user.rb b/lib/gitlab/auth/o_auth/user.rb index bb47b4236fb..3981594478d 100644 --- a/lib/gitlab/auth/o_auth/user.rb +++ b/lib/gitlab/auth/o_auth/user.rb @@ -258,7 +258,7 @@ module Gitlab metadata = gl_user.build_user_synced_attributes_metadata if sync_profile_from_provider? - UserSyncedAttributesMetadata.syncable_attributes.each do |key| + UserSyncedAttributesMetadata.syncable_attributes(auth_hash.provider).each do |key| if auth_hash.has_attribute?(key) && gl_user.sync_attribute?(key) gl_user.public_send("#{key}=".to_sym, auth_hash.public_send(key)) # rubocop:disable GitlabSecurity/PublicSend metadata.set_attribute_synced(key, true) diff --git a/spec/lib/gitlab/auth/o_auth/user_spec.rb b/spec/lib/gitlab/auth/o_auth/user_spec.rb index 8a86b306604..78e0df91103 100644 --- a/spec/lib/gitlab/auth/o_auth/user_spec.rb +++ b/spec/lib/gitlab/auth/o_auth/user_spec.rb @@ -320,6 +320,38 @@ RSpec.describe Gitlab::Auth::OAuth::User, feature_category: :system_access do end include_examples "to verify compliance with allow_single_sign_on" + + context 'and other providers' do + context 'when sync_name is disabled' do + before do + stub_ldap_config(sync_name: false) + end + + let!(:existing_user) { create(:omniauth_user, name: 'John Swift', email: 'john@example.com', extern_uid: dn, provider: 'twitter', username: 'john') } + + it "updates the gl_user name" do + oauth_user.save # rubocop:disable Rails/SaveBang + + expect(gl_user).to be_valid + expect(gl_user.name).to eql 'John' + end + end + + context 'when sync_name is enabled' do + before do + stub_ldap_config(sync_name: true) + end + + let!(:existing_user) { create(:omniauth_user, name: 'John Swift', email: 'john@example.com', extern_uid: dn, provider: 'twitter', username: 'john') } + + it "updates the gl_user name" do + oauth_user.save # rubocop:disable Rails/SaveBang + + expect(gl_user).to be_valid + expect(gl_user.name).to eql 'John' + end + end + end end context "with auto_link_ldap_user enabled" do @@ -418,54 +450,41 @@ RSpec.describe Gitlab::Auth::OAuth::User, feature_category: :system_access do end context "and LDAP user has an account already" do + let(:provider) { 'ldapmain' } + + before do + allow(Gitlab::Auth::Ldap::Person).to receive(:find_by_uid).and_return(ldap_user) + stub_omniauth_config(sync_profile_attributes: true) + allow(Gitlab.config.ldap).to receive(:enabled).and_return(true) + end + context 'when sync_name is disabled' do before do - allow(Gitlab.config.ldap).to receive(:enabled).and_return(true) - allow(Gitlab.config.ldap).to receive(:sync_name).and_return(false) + stub_ldap_config(sync_name: false) end - 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) + let!(:existing_user) { create(:omniauth_user, name: 'John Deo', email: 'john@example.com', extern_uid: dn, provider: 'ldapmain', username: 'john') } + it "does not update the user name" do oauth_user.save # rubocop:disable Rails/SaveBang 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 } } - expect(identities_as_hash).to match_array( - [ - { provider: 'ldapmain', extern_uid: dn }, - { provider: 'twitter', extern_uid: uid } - ] - ) + expect(gl_user.name).to eql 'John Deo' end end context 'when sync_name is enabled' do - let!(:existing_user) { create(:omniauth_user, name: 'John Swift', email: 'john@example.com', extern_uid: dn, provider: 'ldapmain', username: 'john') } + before do + stub_ldap_config(sync_name: true) + end - it "adds the omniauth identity to the LDAP account" do - allow(Gitlab::Auth::Ldap::Person).to receive(:find_by_uid).and_return(ldap_user) + let!(:existing_user) { create(:omniauth_user, name: 'John Swift', email: 'john@example.com', extern_uid: dn, provider: 'ldapmain', username: 'john') } + it "updates the user name" do oauth_user.save # rubocop:disable Rails/SaveBang expect(gl_user).to be_valid - expect(gl_user.username).to eql 'john' - expect(gl_user.name).to eql 'John Swift' - 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 } } - expect(identities_as_hash).to match_array( - [ - { provider: 'ldapmain', extern_uid: dn }, - { provider: 'twitter', extern_uid: uid } - ] - ) + expect(gl_user.name).to eql 'John' end end end -- cgit v1.2.1