diff --git a/app/workers/ldap_group_links_worker.rb b/app/workers/ldap_group_links_worker.rb new file mode 100644 index 0000000000000000000000000000000000000000..a685da86be022c2685315174a3ac744897c138cd --- /dev/null +++ b/app/workers/ldap_group_links_worker.rb @@ -0,0 +1,10 @@ +class LdapGroupLinksWorker + include Sidekiq::Worker + + def perform(user_id) + user = User.find(user_id) + logger.info "Updating LDAP group memberships for user #{user.id} (#{user.email})" + access = Gitlab::LDAP::Access.new(user) + access.update_ldap_group_links + end +end diff --git a/app/workers/ldap_sync_worker.rb b/app/workers/ldap_sync_worker.rb index df71217ef033b88f8074f3055b538bc0e205c697..a03011a63da815394337dca10b86b2474a6ecc69 100644 --- a/app/workers/ldap_sync_worker.rb +++ b/app/workers/ldap_sync_worker.rb @@ -8,7 +8,9 @@ class LdapSyncWorker Rails.logger.info "Performing daily LDAP sync task." User.ldap.find_each(batch_size: 100).each do |ldap_user| Rails.logger.debug "Syncing user #{ldap_user.username}, #{ldap_user.email}" - Gitlab::LDAP::Access.allowed?(ldap_user) + # Use the 'update_ldap_group_links_synchronously' option to avoid creating a ton + # of new Sidekiq jobs all at once. + Gitlab::LDAP::Access.allowed?(ldap_user, update_ldap_group_links_synchronously: true) end end end diff --git a/lib/gitlab/ldap/access.rb b/lib/gitlab/ldap/access.rb index eb924a92b4ff45bc179f5f400b11efea2bab13ba..8fe73ce1d80ae6496e95d26c3a59446a6af3ffd9 100644 --- a/lib/gitlab/ldap/access.rb +++ b/lib/gitlab/ldap/access.rb @@ -14,11 +14,11 @@ module Gitlab end end - def self.allowed?(user) + def self.allowed?(user, options={}) self.open(user) do |access| # Whether user is allowed, or not, we should update # permissions to keep things clean - access.update_permissions + access.update_permissions(options) if access.allowed? access.update_user user.last_credential_check_at = Time.now @@ -75,8 +75,14 @@ module Gitlab update_kerberos_identity if import_kerberos_identities? end - def update_permissions - update_ldap_group_links if group_base.present? + def update_permissions(options) + if group_base.present? + if options[:update_ldap_group_links_synchronously] + update_ldap_group_links + else + LdapGroupLinksWorker.perform_async(user.id) + end + end update_admin_status if admin_group.present? end diff --git a/spec/lib/gitlab/ldap/access_spec.rb b/spec/lib/gitlab/ldap/access_spec.rb index 0174700ddec6ba28a6ec288335fb2a159b20f3dc..371d42bf57f7660518804135b2edaacc7eb99a52 100644 --- a/spec/lib/gitlab/ldap/access_spec.rb +++ b/spec/lib/gitlab/ldap/access_spec.rb @@ -117,11 +117,12 @@ describe Gitlab::LDAP::Access, lib: true do end describe '#update_permissions' do - subject { access.update_permissions } + subject { access.update_permissions({}) } it 'does update group permissions with a group base configured' do allow(access).to receive_messages(group_base: 'my-group-base') - expect(access).to receive(:update_ldap_group_links) + expect(access).not_to receive(:update_ldap_group_links) + expect(LdapGroupLinksWorker).to receive(:perform_async).with(user.id) subject end @@ -129,6 +130,7 @@ describe Gitlab::LDAP::Access, lib: true do it 'does not update group permissions without a group base configured' do allow(access).to receive_messages(group_base: '') expect(access).not_to receive(:update_ldap_group_links) + expect(LdapGroupLinksWorker).not_to receive(:perform_async) subject end @@ -146,6 +148,16 @@ describe Gitlab::LDAP::Access, lib: true do subject end + + context 'when synchronously updating group permissions' do + it 'updates group permissions directly' do + allow(access).to receive_messages(group_base: 'my-group-base') + expect(LdapGroupLinksWorker).not_to receive(:perform_async) + expect(access).to receive(:update_ldap_group_links) + + access.update_permissions(update_ldap_group_links_synchronously: true) + end + end end describe :update_kerberos_identity do