From 3f0c8ea97a3a82987a90c26c2b6e3a9ab5946390 Mon Sep 17 00:00:00 2001 From: Drew Blessing Date: Thu, 3 Mar 2016 16:20:45 -0600 Subject: [PATCH] Move group sync to a separate worker process and revamp the entire strategy --- app/models/group.rb | 4 + app/models/identity.rb | 2 + app/models/members/group_member.rb | 4 + app/models/user.rb | 3 + app/workers/ldap_group_links_worker.rb | 10 - app/workers/ldap_group_sync_worker.rb | 11 + config/initializers/1_settings.rb | 3 + ..._add_secondary_extern_uid_to_identities.rb | 5 + ...0317191509_add_last_sync_time_to_groups.rb | 6 + db/schema.rb | 7 +- lib/gitlab/ldap/access.rb | 83 --- lib/gitlab/ldap/adapter.rb | 10 + lib/gitlab/ldap/group.rb | 22 +- lib/gitlab/ldap/group_sync.rb | 318 ++++++++++ spec/lib/gitlab/ldap/access_spec.rb | 302 ---------- spec/lib/gitlab/ldap/group_sync_spec.rb | 544 ++++++++++++++++++ 16 files changed, 923 insertions(+), 411 deletions(-) delete mode 100644 app/workers/ldap_group_links_worker.rb create mode 100644 app/workers/ldap_group_sync_worker.rb create mode 100644 db/migrate/20160303210802_add_secondary_extern_uid_to_identities.rb create mode 100644 db/migrate/20160317191509_add_last_sync_time_to_groups.rb create mode 100644 lib/gitlab/ldap/group_sync.rb create mode 100644 spec/lib/gitlab/ldap/group_sync_spec.rb diff --git a/app/models/group.rb b/app/models/group.rb index 64b82e411197de..6bcc62890ee145 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -36,6 +36,10 @@ class Group < Namespace after_create :post_create_hook after_destroy :post_destroy_hook + scope :where_group_links_with_provider, ->(provider) do + joins(:ldap_group_links).where(ldap_group_links: { provider: provider }) + end + class << self # Searches for groups matching the given query. # diff --git a/app/models/identity.rb b/app/models/identity.rb index e1915b079d439d..cd1cf9a9e9859b 100644 --- a/app/models/identity.rb +++ b/app/models/identity.rb @@ -19,6 +19,8 @@ class Identity < ActiveRecord::Base validates :extern_uid, allow_blank: true, uniqueness: { scope: :provider } validates :user_id, uniqueness: { scope: :provider } + scope :with_provider, ->(provider) { where(provider: provider) } + def ldap? provider.starts_with?('ldap') end diff --git a/app/models/members/group_member.rb b/app/models/members/group_member.rb index a6bae8e9716873..fb8dbfe8c7eb8a 100644 --- a/app/models/members/group_member.rb +++ b/app/models/members/group_member.rb @@ -31,6 +31,10 @@ class GroupMember < Member scope :with_group, ->(group) { where(source_id: group.id) } scope :with_user, ->(user) { where(user_id: user.id) } scope :with_ldap_dn, -> { joins(user: :identities).where("identities.provider LIKE ?", 'ldap%') } + scope :select_access_level_and_user, -> { select(:access_level, :user_id) } + scope :with_identity_provider, ->(provider) do + joins(user: :identities).where(identities: { provider: provider }) + end def self.access_level_roles Gitlab::Access.options_with_owner diff --git a/app/models/user.rb b/app/models/user.rb index d60d040381ede7..d27020962fd86a 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -227,6 +227,9 @@ def blocked? scope :ldap, -> { joins(:identities).where('identities.provider LIKE ?', 'ldap%') } scope :with_two_factor, -> { where(two_factor_enabled: true) } scope :without_two_factor, -> { where(two_factor_enabled: false) } + scope :with_provider, ->(provider) do + joins(:identities).where(identities: { provider: provider }) + end # # Class methods diff --git a/app/workers/ldap_group_links_worker.rb b/app/workers/ldap_group_links_worker.rb deleted file mode 100644 index a685da86be022c..00000000000000 --- a/app/workers/ldap_group_links_worker.rb +++ /dev/null @@ -1,10 +0,0 @@ -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_group_sync_worker.rb b/app/workers/ldap_group_sync_worker.rb new file mode 100644 index 00000000000000..4f1a715126910b --- /dev/null +++ b/app/workers/ldap_group_sync_worker.rb @@ -0,0 +1,11 @@ +class LdapGroupSyncWorker + include Sidekiq::Worker + + sidekiq_options retry: false + + def perform + logger.info 'Started LDAP group sync' + Gitlab::LDAP::GroupSync.execute + logger.info 'Finished LDAP group sync' + end +end diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 8c399eed07f31c..9d840f15f71902 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -330,6 +330,9 @@ def host(url) Settings.cron_jobs['ldap_sync_worker'] ||= Settingslogic.new({}) Settings.cron_jobs['ldap_sync_worker']['cron'] ||= '30 1 * * *' Settings.cron_jobs['ldap_sync_worker']['job_class'] = 'LdapSyncWorker' +Settings.cron_jobs['ldap_group_sync_worker'] ||= Settingslogic.new({}) +Settings.cron_jobs['ldap_group_sync_worker']['cron'] ||= '0 * * * *' +Settings.cron_jobs['ldap_group_sync_worker']['job_class'] = 'LdapGroupSyncWorker' Settings.cron_jobs['geo_bulk_notify_worker'] ||= Settingslogic.new({}) Settings.cron_jobs['geo_bulk_notify_worker']['cron'] ||= '*/10 * * * * *' Settings.cron_jobs['geo_bulk_notify_worker']['job_class'] ||= 'GeoBulkNotifyWorker' diff --git a/db/migrate/20160303210802_add_secondary_extern_uid_to_identities.rb b/db/migrate/20160303210802_add_secondary_extern_uid_to_identities.rb new file mode 100644 index 00000000000000..92aa7f043e33f2 --- /dev/null +++ b/db/migrate/20160303210802_add_secondary_extern_uid_to_identities.rb @@ -0,0 +1,5 @@ +class AddSecondaryExternUidToIdentities < ActiveRecord::Migration + def change + add_column :identities, :secondary_extern_uid, :string + end +end diff --git a/db/migrate/20160317191509_add_last_sync_time_to_groups.rb b/db/migrate/20160317191509_add_last_sync_time_to_groups.rb new file mode 100644 index 00000000000000..9a4cea5ea439f7 --- /dev/null +++ b/db/migrate/20160317191509_add_last_sync_time_to_groups.rb @@ -0,0 +1,6 @@ +class AddLastSyncTimeToGroups < ActiveRecord::Migration + def change + add_column :namespaces, :last_ldap_sync_at, :datetime + add_index :namespaces, :last_ldap_sync_at + end +end diff --git a/db/schema.rb b/db/schema.rb index cfc31bf383bb80..4c7ab0b84b0ff4 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20160316124047) do +ActiveRecord::Schema.define(version: 20160317191509) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -455,6 +455,7 @@ t.integer "user_id" t.datetime "created_at" t.datetime "updated_at" + t.string "secondary_extern_uid" end add_index "identities", ["created_at", "id"], name: "index_identities_on_created_at_and_id", using: :btree @@ -676,9 +677,11 @@ t.string "avatar" t.boolean "membership_lock", default: false t.boolean "share_with_group_lock", default: false + t.datetime "last_ldap_sync_at" end add_index "namespaces", ["created_at", "id"], name: "index_namespaces_on_created_at_and_id", using: :btree + add_index "namespaces", ["last_ldap_sync_at"], name: "index_namespaces_on_last_ldap_sync_at", using: :btree add_index "namespaces", ["name"], name: "index_namespaces_on_name", unique: true, using: :btree add_index "namespaces", ["name"], name: "index_namespaces_on_name_trigram", using: :gin, opclasses: {"name"=>"gin_trgm_ops"} add_index "namespaces", ["owner_id"], name: "index_namespaces_on_owner_id", using: :btree @@ -831,7 +834,7 @@ t.boolean "pending_delete", default: false t.boolean "public_builds", default: true, null: false t.string "main_language" - t.integer "pushes_since_gc", default: 0 + t.integer "pushes_since_gc", default: 0 end add_index "projects", ["builds_enabled", "shared_runners_enabled"], name: "index_projects_on_builds_enabled_and_shared_runners_enabled", using: :btree diff --git a/lib/gitlab/ldap/access.rb b/lib/gitlab/ldap/access.rb index 2024fecc58e8f3..f67b5328bed387 100644 --- a/lib/gitlab/ldap/access.rb +++ b/lib/gitlab/ldap/access.rb @@ -18,7 +18,6 @@ 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(options) if access.allowed? access.update_user user.last_credential_check_at = Time.now @@ -75,17 +74,6 @@ def update_user update_kerberos_identity if import_kerberos_identities? end - 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 - # Update user ssh keys if they changed in LDAP def update_ssh_keys remove_old_ssh_keys @@ -148,63 +136,6 @@ def update_email user.update(email: ldap_email) end - def update_admin_status - admin_group = Gitlab::LDAP::Group.find_by_cn(ldap_config.admin_group, adapter) - admin_user = Gitlab::LDAP::Person.find_by_dn(user.ldap_identity.extern_uid, adapter) - - if admin_group && admin_group.has_member?(admin_user) - unless user.admin? - user.admin = true - user.save - end - else - if user.admin? - user.admin = false - user.save - end - end - end - - # Loop through all ldap connected groups, and update the users link with it - # - # We documented what sort of queries an LDAP server can expect from - # GitLab EE in doc/integration/ldap.md. Please remember to update that - # documentation if you change the algorithm below. - def update_ldap_group_links - gitlab_groups_with_ldap_link.each do |group| - active_group_links = group.ldap_group_links.where(cn: cns_with_access) - - if active_group_links.any? - max_access = active_group_links.maximum(:group_access) - - # Ensure we don't leave a group without an owner - if max_access < Gitlab::Access::OWNER && group.last_owner?(user) - logger.warn "#{self.class.name}: LDAP group sync cannot demote #{user.name} (#{user.id}) from group #{group.name} (#{group.id}) as this is the group's last owner" - else - group.add_users([user.id], max_access, skip_notification: true) - end - elsif group.last_owner?(user) - logger.warn "#{self.class.name}: LDAP group sync cannot remove #{user.name} (#{user.id}) from group #{group.name} (#{group.id}) as this is the group's last owner" - else - group.users.delete(user) - end - end - end - - def ldap_groups - @ldap_groups ||= ::LdapGroupLink.with_provider(provider).distinct(:cn).pluck(:cn).map do |cn| - Gitlab::LDAP::Group.find_by_cn(cn, adapter) - end.compact - end - - # returns a collection of cn strings to which the user has access - def cns_with_access - return [] unless ldap_user.present? - @ldap_groups_with_access ||= ldap_groups.select do |ldap_group| - ldap_group.has_member?(ldap_user) - end.map(&:cn) - end - def sync_ssh_keys? ldap_config.sync_ssh_keys? end @@ -214,22 +145,8 @@ def import_kerberos_identities? ldap_config.active_directory && (Gitlab.config.kerberos.enabled || AuthHelper.kerberos_enabled? ) end - def group_base - ldap_config.group_base - end - - def admin_group - ldap_config.admin_group - end - private - def gitlab_groups_with_ldap_link - ::Group.includes(:ldap_group_links).references(:ldap_group_links). - where.not(ldap_group_links: { id: nil }). - where(ldap_group_links: { provider: provider }) - end - def logger Rails.logger end diff --git a/lib/gitlab/ldap/adapter.rb b/lib/gitlab/ldap/adapter.rb index 0c9a2db4fd0db6..c2025d51c05e95 100644 --- a/lib/gitlab/ldap/adapter.rb +++ b/lib/gitlab/ldap/adapter.rb @@ -93,6 +93,16 @@ def dn_matches_filter?(dn, filter) attributes: %w{dn}).any? end + def dns_for_filter(filter) + ldap_search( + base: config.base, + filter: filter, + scope: Net::LDAP::SearchScope_WholeSubtree, + attributes: %w{dn} + ).map(&:dn) + end + + def ldap_search(*args) # Net::LDAP's `time` argument doesn't work. Use Ruby `Timeout` instead. Timeout.timeout(config.timeout) do diff --git a/lib/gitlab/ldap/group.rb b/lib/gitlab/ldap/group.rb index 53ec0d585a4db4..03a0ab636ebf0b 100644 --- a/lib/gitlab/ldap/group.rb +++ b/lib/gitlab/ldap/group.rb @@ -14,6 +14,10 @@ def initialize(entry, adapter=nil) @adapter = adapter end + def active_directory? + adapter.config.active_directory + end + def cn entry.cn.first end @@ -34,22 +38,12 @@ def member_uids entry.memberuid end - def has_member?(user) - user_uid = user.uid.downcase - user_dn = user.dn.downcase - - if memberuid? - member_uids.any? { |member_uid| member_uid.downcase == user_uid } - elsif member_dns.any? { |member_dn| member_dn.downcase == user_dn } - true - elsif member_dns.any? { |member_dn| member_dn.downcase == "uid=" + user_uid } - true - elsif adapter.config.active_directory - adapter.dn_matches_filter?(user.dn, active_directory_recursive_memberof_filter) + def member_dns + if active_directory? + dns = adapter.dns_for_filter(active_directory_recursive_memberof_filter) + return dns unless dns.empty? end - end - def member_dns if (entry.respond_to? :member) && (entry.respond_to? :submember) entry.member + entry.submember elsif entry.respond_to? :member diff --git a/lib/gitlab/ldap/group_sync.rb b/lib/gitlab/ldap/group_sync.rb new file mode 100644 index 00000000000000..903c97b81abffb --- /dev/null +++ b/lib/gitlab/ldap/group_sync.rb @@ -0,0 +1,318 @@ +module Gitlab + module LDAP + class GroupSync + attr_reader :provider + + # Open a connection so we can run all queries through it. + # It's more efficient than the default of opening/closing per LDAP query. + def self.open(provider, &block) + Gitlab::LDAP::Adapter.open(provider) do |adapter| + block.call(self.new(provider, adapter)) + end + end + + def self.execute + # Shuffle providers to prevent a scenario where sync fails after a time + # and only the first provider or two get synced. This shuffles the order + # so subsequent syncs should eventually get to all providers. Obviously + # we should avoid failure, but this is an additional safeguard. + Gitlab::LDAP::Config.providers.shuffle.each do |provider| + self.open(provider) do |group_sync| + group_sync.update_permissions + end + end + + true + end + + def initialize(provider, adapter = nil) + @adapter = adapter + @provider = provider + end + + def update_permissions + if group_base.present? + logger.debug { "Performing LDAP group sync for '#{provider}' provider" } + sync_groups + logger.debug { "Finished LDAP group sync for '#{provider}' provider" } + else + logger.debug { "No `group_base` configured for '#{provider}' provider. Skipping" } + end + + if admin_group.present? + logger.debug { "Syncing admin users for '#{provider}' provider" } + sync_admin_users + logger.debug { "Finished syncing admin users for '#{provider}' provider" } + else + logger.debug { "No `admin_group` configured for '#{provider}' provider. Skipping" } + end + + nil + end + + # Iterate of all GitLab groups with LDAP links. Build an access hash + # representing a user's highest access level among the LDAP links within + # the same GitLab group. + def sync_groups + # Order results by last_ldap_sync_at ASC so groups with older last + # sync time are handled first + groups_where_group_links_with_provider_ordered.each do |group| + lease = Gitlab::ExclusiveLease.new( + "ldap_group_sync:#{provider}:#{group.id}", + timeout: 3600 + ) + next unless lease.try_obtain + + logger.debug { "Syncing '#{group.name}' group" } + access_hash = {} + + # Only iterate over group links for the current provider + group.ldap_group_links.with_provider(provider).each do |group_link| + if member_dns = dns_for_group_cn(group_link.cn) + members_to_access_hash( + access_hash, member_dns, group_link.group_access + ) + + logger.debug { "Resolved '#{group.name}' group member access: #{access_hash}" } + end + end + + update_existing_group_membership(group, access_hash) + add_new_members(group, access_hash) + + group.update(last_ldap_sync_at: Time.now) + + logger.debug { "Finished syncing '#{group.name}' group" } + end + end + + # Update global administrators based on the specified admin group CN + def sync_admin_users + admin_group_member_dns = dns_for_group_cn(admin_group) + current_admin_users = ::User.admins.with_provider(provider) + verified_admin_users = [] + + # Verify existing admin users and add new ones. + admin_group_member_dns.each do |member_dn| + user = Gitlab::LDAP::User.find_by_uid_and_provider(member_dn, provider) + + if user.present? + user.admin = true + user.save + verified_admin_users << user + else + logger.debug do + <<-MSG.strip_heredoc.gsub(/\n/, ' ') + #{self.class.name}: User with DN `#{member_dn}` should have admin + access but there is no user in GitLab with that identity. + Membership will be updated once the user signs in for the first time. + MSG + end + end + end + + # Revoke the unverified admins. + current_admin_users.each do |user| + unless verified_admin_users.include?(user) + user.admin = false + user.save + end + end + end + + def members_to_access_hash(access_hash, member_dns, group_access) + member_dns.each do |member_dn| + current_access = access_hash[member_dn] + + # Keep the higher of the access values. + if current_access.nil? || group_access > current_access + access_hash[member_dn] = group_access + end + end + access_hash + end + + private + + # Cache LDAP group member DNs so we don't query LDAP groups more than once. + def dns_for_group_cn(group_cn) + @dns_for_group_cn ||= Hash.new { |h, k| h[k] = ldap_group_member_dns(k) } + @dns_for_group_cn[group_cn] + end + + # Cache user DN so we don't generate excess queries to map UID to DN + def dn_for_uid(uid) + @dn_for_uid ||= Hash.new { |h, k| h[k] = member_uid_to_dn(k) } + @dn_for_uid[uid] + end + + def adapter + @adapter ||= Gitlab::LDAP::Adapter.new(provider) + end + + def config + @config ||= Gitlab::LDAP::Config.new(provider) + end + + def group_base + config.group_base + end + + def admin_group + config.admin_group + end + + def ldap_group_member_dns(ldap_group_cn) + ldap_group = Gitlab::LDAP::Group.find_by_cn(ldap_group_cn, adapter) + unless ldap_group.present? + logger.warn { "Cannot find LDAP group with CN '#{ldap_group_cn}'. Skipping" } + return [] + end + + member_dns = ldap_group.member_dns + if member_dns.empty? + # Group must be empty + return [] unless ldap_group.memberuid? + + members = ldap_group.member_uids + member_dns = members.map { |uid| dn_for_uid(uid) }.compact + end + + logger.debug { "Members in '#{ldap_group.name}' LDAP group: #{member_dns}" } + + member_dns + end + + def member_uid_to_dn(uid) + identity = Identity.find_by(provider: provider, secondary_extern_uid: uid) + + if identity.present? + # Use the DN on record in GitLab when it's available + identity.extern_uid + else + ldap_user = Gitlab::LDAP::Person.find_by_uid(uid, adapter) + + # Can't find a matching user for group entry + return nil unless ldap_user.present? + + # Update user identity so we don't have to go through this again + update_identity(ldap_user.dn, uid) + + ldap_user.dn + end + end + + def update_identity(dn, uid) + identity = + Identity.find_by(provider: provider, extern_uid: dn) + + # User may not exist in GitLab yet. Skip. + return unless identity.present? + + identity.secondary_extern_uid = uid + identity.save + end + + def update_existing_group_membership(group, access_hash) + logger.debug { "Updating existing membership for '#{group.name}' group" } + + select_and_preload_group_members(group).each do |member| + user = member.user + identity = user.identities.select(:id, :extern_uid) + .with_provider(provider).first + member_dn = identity.extern_uid + + # Skip if this is not an LDAP user with a valid `extern_uid`. + next unless member_dn.present? + + # Prevent shifting group membership, in case where user is a member + # of two LDAP groups from different providers linked to the same + # GitLab group. This is not ideal, but preserves existing behavior. + if user.ldap_identity.id != identity.id + access_hash.delete(member_dn) + next + end + + desired_access = access_hash[member_dn] + + # Don't do anything if the user already has the desired access level + if member.access_level == desired_access + access_hash.delete(member_dn) + next + end + + # Check and update the access level. If `desired_access` is `nil` + # we need to delete the user from the group. + if desired_access.present? + add_or_update_user_membership(user, group, desired_access) + + # Delete this entry from the hash now that we've acted on it + access_hash.delete(member_dn) + elsif group.last_owner?(user) + warn_cannot_remove_last_owner(user, group) + else + group.users.delete(user) + end + end + end + + def add_new_members(group, access_hash) + logger.debug { "Adding new members to '#{group.name}' group" } + + access_hash.each do |member_dn, access_level| + user = Gitlab::LDAP::User.find_by_uid_and_provider(member_dn, provider) + + if user.present? + add_or_update_user_membership(user, group, access_level) + else + logger.debug do + <<-MSG.strip_heredoc.gsub(/\n/, ' ') + #{self.class.name}: User with DN `#{member_dn}` should have access + to '#{group.name}' group but there is no user in GitLab with that + identity. Membership will be updated once the user signs in for + the first time. + MSG + end + end + end + end + + def add_or_update_user_membership(user, group, access) + # Prevent the last owner of a group from being demoted + if access < Gitlab::Access::OWNER && group.last_owner?(user) + warn_cannot_remove_last_owner(user, group) + else + # If you pass the user object, instead of just user ID, + # it saves an extra user database query. + group.add_users([user], access, skip_notification: true) + end + end + + def warn_cannot_remove_last_owner(user, group) + logger.warn do + <<-MSG.strip_heredoc.gsub(/\n/, ' ') + #{self.class.name}: LDAP group sync cannot remove #{user.name} + (#{user.id}) from group #{group.name} (#{group.id}) as this is + the group's last owner + MSG + end + end + + def select_and_preload_group_members(group) + group.members.select_access_level_and_user + .with_identity_provider(provider).preload(:user) + end + + def groups_where_group_links_with_provider_ordered + ::Group.where_group_links_with_provider(provider) + .preload(:ldap_group_links) + .reorder('last_ldap_sync_at ASC, namespaces.id ASC') + .distinct + end + + def logger + Rails.logger + end + end + end +end diff --git a/spec/lib/gitlab/ldap/access_spec.rb b/spec/lib/gitlab/ldap/access_spec.rb index 2d2c8fc54b44e7..efd34b14f19ded 100644 --- a/spec/lib/gitlab/ldap/access_spec.rb +++ b/spec/lib/gitlab/ldap/access_spec.rb @@ -116,50 +116,6 @@ end end - describe '#update_permissions' do - 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).not_to receive(:update_ldap_group_links) - expect(LdapGroupLinksWorker).to receive(:perform_async).with(user.id) - - subject - end - - 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 - - it 'does update admin group permissions if admin group is configured' do - allow(access).to receive_messages(admin_group: 'my-admin-group') - expect(access).to receive(:update_admin_status) - - subject - end - - it 'does not update admin status when admin group is not configured' do - allow(access).to receive_messages(admin_group: '') - expect(access).not_to receive(:update_admin_status) - - 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 let(:entry) do Net::LDAP::Entry.from_single_ldif_string("dn: cn=foo, dc=bar, dc=com") @@ -277,262 +233,4 @@ expect{ access.update_email }.to change(user, :email) end end - - - describe :update_admin_status do - before do - allow(access).to receive_messages(admin_group: "GLAdmins") - ldap_user_entry = Net::LDAP::Entry.new - allow_any_instance_of(Gitlab::LDAP::Adapter).to receive(:user) { Gitlab::LDAP::Person.new(ldap_user_entry, user.ldap_identity.provider) } - allow_any_instance_of(Gitlab::LDAP::Person).to receive(:uid) { 'admin2' } - end - - it "should give admin privileges to an User" do - admin_group = Net::LDAP::Entry.from_single_ldif_string( - %Q{dn: cn=#{access.admin_group},ou=groups,dc=bar,dc=com -cn: #{access.admin_group} -description: GitLab admins -gidnumber: 42 -memberuid: admin1 -memberuid: admin2 -memberuid: admin3 -objectclass: top -objectclass: posixGroup - }) - allow_any_instance_of(Gitlab::LDAP::Adapter).to receive(:group) { Gitlab::LDAP::Group.new(admin_group) } - - expect{ access.update_admin_status }.to change(user, :admin?).to(true) - end - - it "should remove admin privileges from an User" do - user.update_attribute(:admin, true) - admin_group = Net::LDAP::Entry.from_single_ldif_string( - %Q{dn: cn=#{access.admin_group},ou=groups,dc=bar,dc=com -cn: #{access.admin_group} -description: GitLab admins -gidnumber: 42 -memberuid: admin1 -memberuid: admin3 -objectclass: top -objectclass: posixGroup - }) - allow_any_instance_of(Gitlab::LDAP::Adapter).to receive(:group) { Gitlab::LDAP::Group.new(admin_group) } - expect{ access.update_admin_status }.to change(user, :admin?).to(false) - end - end - - - describe :update_ldap_group_links do - let(:cns_with_access) { %w(ldap-group1 ldap-group2) } - let(:gitlab_group_1) { create :group } - let(:gitlab_group_2) { create :group } - - before do - allow(access).to receive_messages(cns_with_access: cns_with_access) - end - - context "non existing access for group-1, allowed via ldap-group1 as MASTER" do - before do - gitlab_group_1.ldap_group_links.create({ - cn: 'ldap-group1', group_access: Gitlab::Access::MASTER, provider: 'ldapmain' }) - end - - it "gives the user master access for group 1" do - access.update_ldap_group_links - expect( gitlab_group_1.has_master?(user) ).to be_truthy - end - - it "doesn't send a notification email" do - expect { access.update_ldap_group_links }.not_to \ - change { ActionMailer::Base.deliveries } - end - end - - context "existing access as guest for group-1, allowed via ldap-group1 as DEVELOPER" do - before do - gitlab_group_1.group_members.guests.create(user_id: user.id) - gitlab_group_1.ldap_group_links.create({ - cn: 'ldap-group1', group_access: Gitlab::Access::MASTER, provider: 'ldapmain' }) - end - - it "upgrades the users access to master for group 1" do - expect { access.update_ldap_group_links }.to \ - change{ gitlab_group_1.has_master?(user) }.from(false).to(true) - end - - it "doesn't send a notification email" do - expect { access.update_ldap_group_links }.not_to \ - change { ActionMailer::Base.deliveries } - end - end - - context 'existing access as MASTER for group-1, allowed via ldap-group1 as DEVELOPER' do - before do - gitlab_group_1.group_members.masters.create(user_id: user.id) - gitlab_group_1.ldap_group_links.create({ - cn: 'ldap-group1', group_access: Gitlab::Access::DEVELOPER, provider: 'ldapmain' }) - end - - it 'downgrades the users access' do - expect { access.update_ldap_group_links }.to \ - change{ gitlab_group_1.has_master?(user) }.from(true).to(false) - end - - it 'does not send a notification email' do - expect { access.update_ldap_group_links }.not_to \ - change { ActionMailer::Base.deliveries } - end - end - - context 'existing access as master for group-1, not allowed via LDAP' do - before do - gitlab_group_1.group_members.masters.create(user_id: user.id) - gitlab_group_1.ldap_group_links.create(cn: 'ldap-group1', group_access: Gitlab::Access::MASTER, provider: 'ldapmain') - allow(access).to receive_messages(cns_with_access: ['ldap-group2']) - end - - it 'removes user from gitlab_group_1' do - expect { access.update_ldap_group_links }.to \ - change{ gitlab_group_1.members.where(user_id: user).any? }.from(true).to(false) - end - end - - context 'existing access as owner for group-1 with no other owner, not allowed via LDAP' do - before do - gitlab_group_1.group_members.owners.create(user_id: user.id) - gitlab_group_1.ldap_group_links.create(cn: 'ldap-group1', group_access: Gitlab::Access::OWNER, provider: 'ldapmain') - allow(access).to receive_messages(cns_with_access: ['ldap-group2']) - end - - # Note: Don't use `has_owner?` or `has_master?` in this expectation. - # It leads to false negatives. - it 'does not remove the user from gitlab_group_1 since its the last owner' do - expect { access.update_ldap_group_links }.not_to \ - change { gitlab_group_1.members.owners.where(user_id: user).any? } - end - end - - context 'existing access as owner for group-1 with no other owner, allowed via ldap-group1 as DEVELOPER' do - before do - gitlab_group_1.group_members.owners.create(user_id: user.id) - gitlab_group_1.ldap_group_links.create(cn: 'ldap-group1', group_access: Gitlab::Access::DEVELOPER, provider: 'ldapmain') - allow(access).to receive_messages(cns_with_access: ['ldap-group1']) - end - - # Note: Don't use `has_owner?` or `has_master?` in this expectation. - # It leads to false negatives. - it 'does not remove the user from gitlab_group_1 since its the last owner' do - expect { access.update_ldap_group_links }.not_to \ - change { gitlab_group_1.members.owners.where(user_id: user).any? } - end - end - - context 'existing access as owner for group-1 while other owners present, not allowed via LDAP' do - before do - owner2 = create(:user) # a 2nd owner - gitlab_group_1.group_members.owners.create([{ user_id: user.id }, { user_id: owner2.id }]) - gitlab_group_1.ldap_group_links.create(cn: 'ldap-group1', group_access: Gitlab::Access::OWNER, provider: 'ldapmain') - allow(access).to receive_messages(cns_with_access: ['ldap-group2']) - end - - it 'removes user from gitlab_group_1' do - expect { access.update_ldap_group_links }.to \ - change{ gitlab_group_1.members.where(user_id: user).any? }.from(true).to(false) - end - end - end - - describe 'ldap_groups' do - let(:ldap_group_1) do - Net::LDAP::Entry.from_single_ldif_string( - %Q{dn: cn=#{access.ldap_config.admin_group},ou=groups,dc=bar,dc=com -cn: #{access.ldap_config.admin_group} -description: GitLab group 1 -gidnumber: 42 -memberuid: user1 -memberuid: user2 -objectclass: top -objectclass: posixGroup - }) - end - - it "returns an interator of LDAP Groups" do - ::LdapGroupLink.create({ - cn: 'example', group_access: Gitlab::Access::DEVELOPER, group_id: 42, provider: 'ldapmain' }) - allow_any_instance_of(Gitlab::LDAP::Adapter).to receive(:group) { Gitlab::LDAP::Group.new(ldap_group_1) } - - expect(access.ldap_groups.first).to be_a Gitlab::LDAP::Group - end - - it "only returns found ldap groups" do - ::LdapGroupLink.create cn: 'example', group_access: Gitlab::Access::DEVELOPER, group_id: 42 - allow(Gitlab::LDAP::Group).to receive_messages(find_by_cn: nil) # group not found - - expect(access.ldap_groups).to be_empty - end - end - - describe :cns_with_access do - let(:ldap_group_response_1) do - Net::LDAP::Entry.from_single_ldif_string( - %Q{dn: cn=group1,ou=groups,dc=bar,dc=com -cn: group1 -description: GitLab group 1 -gidnumber: 21 -uniquemember: #{ldap_user.dn.downcase} -uniquemember: uid=user2,ou=people,dc=example -objectclass: top -objectclass: posixGroup - }) - end - - let(:ldap_group_response_2) do - Net::LDAP::Entry.from_single_ldif_string( - %Q{dn: cn=group2,ou=groups,dc=bar,dc=com -cn: group2 -description: GitLab group 2 -gidnumber: 42 -memberuid: user3 -memberuid: user4 -objectclass: top -objectclass: posixGroup - }) - end - - let(:ldap_groups) do - [ - Gitlab::LDAP::Group.new(ldap_group_response_1), - Gitlab::LDAP::Group.new(ldap_group_response_2) - ] - end - - before do - allow(ldap_user).to receive(:dn) { 'uid=user1,ou=People,dc=example' } - allow(access).to receive_messages(ldap_groups: ldap_groups) - end - - context 'when the LDAP user exists' do - let(:ldap_user) { Gitlab::LDAP::Person.new(Net::LDAP::Entry.new, user.ldap_identity.provider) } - - before do - allow(access).to receive_messages(ldap_user: ldap_user) - allow(ldap_user).to receive(:uid) { 'user1' } - end - - it 'only returns ldap cns to which the user has access' do - expect(access.cns_with_access).to eq(['group1']) - end - end - - context 'when the LADP user does not exist' do - let(:ldap_user) { nil } - before do - allow(access).to receive_messages(ldap_user: ldap_user) - end - - it 'returns an empty array' do - expect(access.cns_with_access).to eq([]) - end - end - end end diff --git a/spec/lib/gitlab/ldap/group_sync_spec.rb b/spec/lib/gitlab/ldap/group_sync_spec.rb new file mode 100644 index 00000000000000..121b76eefe2aba --- /dev/null +++ b/spec/lib/gitlab/ldap/group_sync_spec.rb @@ -0,0 +1,544 @@ +require 'spec_helper' + +describe Gitlab::LDAP::GroupSync, lib: true do + let(:group_sync) { Gitlab::LDAP::GroupSync.new('ldapmain') } + let(:config) { double(:config, active_directory: false) } + let(:adapter) { double(:adapter, config: config) } + subject { group_sync } + + before do + allow_any_instance_of(Gitlab::ExclusiveLease) + .to receive(:try_obtain).and_return(true) + end + + describe '#update_permissions' do + before do + allow(group_sync) + .to receive_messages(sync_groups: true, sync_admin_users: true) + end + after { group_sync.update_permissions } + + context 'when group_base is present but admin_group is not' do + before do + allow(group_sync) + .to receive_messages(group_base: 'my-group-base', admin_group: nil) + end + + it { is_expected.to receive(:sync_groups) } + it { is_expected.not_to receive(:sync_admin_users) } + end + + context 'when admin_group is present but group_base is not' do + before do + allow(group_sync) + .to receive_messages(group_base: nil, admin_group: 'my-admin-group') + end + + it { is_expected.to receive(:sync_admin_users) } + it { is_expected.not_to receive(:sync_groups) } + end + end + + describe '#sync_groups' do + let(:user1) { create(:user) } + let(:user2) { create(:user) } + let(:group1) { create(:group) } + let(:group2) { create(:group) } + let(:ldap_group1) do + Net::LDAP::Entry.from_single_ldif_string( + <<-EOS.strip_heredoc + dn: cn=ldap_group1,ou=groups,dc=example,dc=com + cn: ldap_group1 + description: LDAP Group 1 + gidnumber: 42 + uniqueMember: uid=#{user1.username},ou=users,dc=example,dc=com + uniqueMember: uid=#{user2.username},ou=users,dc=example,dc=com + objectclass: top + objectclass: groupOfNames + EOS + ) + end + + context 'with all functionality against one LDAP group type' do + before do + allow_any_instance_of(Gitlab::LDAP::Group) + .to receive(:adapter).and_return(adapter) + + user1.identities.create( + provider: 'ldapmain', + extern_uid: "uid=#{user1.username},ou=users,dc=example,dc=com" + ) + user2.identities.create( + provider: 'ldapmain', + extern_uid: "uid=#{user2.username},ou=users,dc=example,dc=com" + ) + + allow(Gitlab::LDAP::Group) + .to receive(:find_by_cn) + .with('ldap_group1', kind_of(Gitlab::LDAP::Adapter)) + .and_return(Gitlab::LDAP::Group.new(ldap_group1)) + + group1.ldap_group_links.create( + cn: 'ldap_group1', + group_access: Gitlab::Access::DEVELOPER, + provider: 'ldapmain' + ) + group2.ldap_group_links.create( + cn: 'ldap_group1', + group_access: Gitlab::Access::OWNER, + provider: 'ldapmain' + ) + end + + context 'with basic add/update actions' do + before do + # Pre-populate the group with some users + group1.add_users([user1.id], + Gitlab::Access::MASTER, skip_notification: true) + group2.add_users([user2.id], + Gitlab::Access::DEVELOPER, skip_notification: true) + end + + it 'adds new members' do + expect { group_sync.sync_groups } + .to change { group1.members.where(user_id: user2.id).any? } + .from(false).to(true) + end + + it 'downgrades existing member access' do + expect { group_sync.sync_groups } + .to change { + group1.members.where( + user_id: user1.id, + access_level: Gitlab::Access::DEVELOPER + ).any? + }.from(false).to(true) + end + + it 'upgrades existing member access' do + expect { group_sync.sync_groups } + .to change { + group2.members.where( + user_id: user2.id, + access_level: Gitlab::Access::OWNER + ).any? + }.from(false).to(true) + end + + it 'does not send a notification email' do + expect { group_sync.sync_groups } + .not_to change { ActionMailer::Base.deliveries } + end + end + + context 'when existing user is no longer in LDAP group' do + let(:user_without_group) { create(:user) } + before do + user_without_group.identities + .create(provider: group_sync.provider, + extern_uid: "uid=johndoe,ou=users,dc=example,dc=com" ) + group1.add_users([user_without_group.id], + Gitlab::Access::MASTER, skip_notification: true) + group2.add_users([user_without_group.id], + Gitlab::Access::OWNER, skip_notification: true) + end + + it 'removes the user from the group' do + expect { group_sync.sync_groups } + .to change { group1.members.where(user_id: user_without_group.id).any? } + .from(true).to(false) + end + + it 'refuses to delete the last owner' do + expect { group_sync.sync_groups } + .not_to change { group2.members.where(user_id: user_without_group.id).any? } + end + end + + context 'when user is the last owner' do + before do + group1.ldap_group_links.create( + cn: 'ldap_group1', + group_access: Gitlab::Access::DEVELOPER, + provider: 'ldapmain' + ) + group1.add_users([user1.id], + Gitlab::Access::OWNER, skip_notification: true) + end + + it 'refuses to downgrade the last owner' do + expect { group_sync.sync_groups } + .not_to change { + group1.members.where( + user_id: user1.id, + access_level: Gitlab::Access::OWNER + ).any? + } + end + + context 'when user is a member of two groups from different providers' do + let(:config) { double(:config, active_directory: false, provider: 'ldapsecondary') } + let(:adapter) { double(:adapter, config: config) } + let(:secondary_group_sync) do + Gitlab::LDAP::GroupSync.new('ldapsecondary', adapter) + end + let(:ldap_secondary_group1) do + Net::LDAP::Entry.from_single_ldif_string( + <<-EOS.strip_heredoc + dn: cn=ldap_secondary_group1,ou=groups,dc=example,dc=com + cn: ldap_secondary_group1 + description: LDAP Group 1 + gidnumber: 42 + uniqueMember: uid=#{user1.username},ou=users,dc=example,dc=com + uniqueMember: uid=#{user2.username},ou=users,dc=example,dc=com + objectclass: top + objectclass: groupOfNames + EOS + ) + end + let(:user_w_multiple_ids) { create(:user) } + + before do + allow(Gitlab::LDAP::Group) + .to receive(:find_by_cn) + .with('ldap_group1', any_args) + .and_return(Gitlab::LDAP::Group.new(ldap_group1)) + allow(Gitlab::LDAP::Group) + .to receive(:find_by_cn) + .with('ldap_secondary_group1', any_args) + .and_return(Gitlab::LDAP::Group.new(ldap_secondary_group1)) + user_w_multiple_ids.identities.create( + [ + { + provider: 'ldapsecondary', + extern_uid: "uid=#{user1.username},ou=users,dc=example,dc=com" + }, + { + provider: 'ldapprimary', + extern_uid: "uid=#{user1.username},ou=users,dc=example,dc=com" + } + ] + ) + group1.ldap_group_links.create( + cn: 'ldap_group1', + group_access: Gitlab::Access::DEVELOPER, + provider: 'ldapprimary' + ) + group1.ldap_group_links.create( + cn: 'ldap_secondary_group1', + group_access: Gitlab::Access::OWNER, + provider: 'ldapsecondary' + ) + group1.add_users([user_w_multiple_ids.id], + Gitlab::Access::DEVELOPER, skip_notification: true) + end + + it 'does not change user permissions for secondary group link' do + expect { secondary_group_sync.sync_groups } + .not_to change { + group1.members.where( + user_id: user_w_multiple_ids.id, + access_level: Gitlab::Access::OWNER + ).any? + } + end + end + end + end + + # Test that membership can be resolved for all different type of LDAP groups + context 'with different LDAP group types' do + let(:secondary_extern_uid) { nil } + + before do + allow_any_instance_of(Gitlab::LDAP::Group) + .to receive(:adapter).and_return(adapter) + allow(Gitlab::LDAP::Group) + .to receive(:find_by_cn) + .with(ldap_group.cn, any_args) + .and_return(ldap_group) + user1.identities.create( + provider: 'ldapmain', + extern_uid: "uid=#{user1.username},ou=users,dc=example,dc=com", + secondary_extern_uid: secondary_extern_uid + ) + group1.ldap_group_links.create( + cn: ldap_group.cn, + group_access: Gitlab::Access::DEVELOPER, + provider: 'ldapmain' + ) + end + + # GroupOfNames - OpenLDAP + context 'with groupOfNames style LDAP group' do + let(:ldap_group) do + Gitlab::LDAP::Group.new( + Net::LDAP::Entry.from_single_ldif_string( + <<-EOS.strip_heredoc + dn: cn=ldap_group1,ou=groups,dc=example,dc=com + cn: ldap_group1 + description: LDAP Group 1 + member: uid=#{user1.username},ou=users,dc=example,dc=com + objectclass: top + objectclass: groupOfNames + EOS + ) + ) + end + + it 'adds the user to the group' do + expect { group_sync.sync_groups } + .to change { group1.members.where(user_id: user1.id).any? } + .from(false).to(true) + end + end + + # posixGroup - Apple Open Directory + context 'with posixGroup style LDAP group' do + let(:ldap_group) do + Gitlab::LDAP::Group.new( + Net::LDAP::Entry.from_single_ldif_string( + <<-EOS.strip_heredoc + dn: cn=ldap_group1,ou=groups,dc=example,dc=com + cn: ldap_group1 + description: LDAP Group 1 + memberuid: #{user1.username} + objectclass: top + objectclass: posixGroup + EOS + ) + ) + end + let(:ldap_user) do + Gitlab::LDAP::Person.new( + Net::LDAP::Entry.from_single_ldif_string( + "dn: uid=#{user1.username},ou=users,dc=example,dc=com" + ), + 'ldapmain' + ) + end + + before do + allow(Gitlab::LDAP::Person) + .to receive(:find_by_uid) + .with(user1.username, any_args) + .and_return(ldap_user) + end + + it 'adds the user to the group' do + expect { group_sync.sync_groups } + .to change { group1.members.where(user_id: user1.id).any? } + .from(false).to(true) + end + + it 'expects Gitlab::LDAP::Person to be called' do + expect(Gitlab::LDAP::Person).to receive(:find_by_uid) + + group_sync.sync_groups + end + + it do + expect { group_sync.sync_groups } + .to change { + user1.identities.find_by( + provider: group_sync.provider, + extern_uid: ldap_user.dn + ).secondary_extern_uid + }.from(nil).to(user1.username) + end + + context 'when the uid is stored in the database' do + let(:secondary_extern_uid) { user1.username } + + it 'expects Gitlab::LDAP::Person will not be called' do + expect(Gitlab::LDAP::Person) + .not_to receive(:find_by_uid) + .with(user1.username, any_args) + + group_sync.sync_groups + end + end + + context 'when a DN for UID is requesting multiple times' do + let(:secondary_extern_uid) { user1.username } + + before do + # Group 1 link was created above. Create another here. + group2.ldap_group_links.create( + cn: ldap_group.cn, + group_access: Gitlab::Access::DEVELOPER, + provider: 'ldapmain' + ) + end + + it 'expects the identity will be retrieved from the database once' do + expect(Identity).to receive(:find_by) + .with( + provider: 'ldapmain', + secondary_extern_uid: secondary_extern_uid + ).once.and_call_original + + group_sync.sync_groups + end + + it 'expects Gitlab::LDAP::Person will not be called' do + expect(Gitlab::LDAP::Person) + .not_to receive(:find_by_uid) + .with(user1.username, any_args) + + group_sync.sync_groups + end + end + end + + context 'with groupOfUniqueNames style LDAP group' do + let(:ldap_group) do + Gitlab::LDAP::Group.new( + Net::LDAP::Entry.from_single_ldif_string( + <<-EOS.strip_heredoc + dn: cn=ldap_group1,ou=groups,dc=example,dc=com + cn: ldap_group1 + description: LDAP Group 1 + uniquemember: uid=#{user1.username},ou=users,dc=example,dc=com + objectclass: top + objectclass: groupOfUniqueNames + EOS + ) + ) + end + + it 'adds the user to the group' do + expect { group_sync.sync_groups } + .to change { group1.members.where(user_id: user1.id).any? } + .from(false).to(true) + end + end + + context 'with an empty LDAP group' do + let(:ldap_group) do + Gitlab::LDAP::Group.new( + Net::LDAP::Entry.from_single_ldif_string( + <<-EOS.strip_heredoc + dn: cn=ldap_group1,ou=groups,dc=example,dc=com + cn: ldap_group1 + description: LDAP Group 1 + objectclass: top + objectclass: groupOfUniqueNames + EOS + ) + ) + end + + it 'does nothing, without failure' do + expect { group_sync.sync_groups } + .not_to change { group1.members.count } + end + end + end + end + + describe '#sync_admin_users' do + let(:user1) { create(:user) } + let(:user2) { create(:user) } + let(:user3) { create(:user) } + + let(:admin_group) do + Net::LDAP::Entry.from_single_ldif_string( + <<-EOS.strip_heredoc + dn: cn=admin_group,ou=groups,dc=example,dc=com + cn: admin_group + description: Admin Group + gidnumber: 42 + uniqueMember: uid=#{user2.username},ou=users,dc=example,dc=com + objectclass: top + objectclass: groupOfNames + EOS + ) + end + + before do + user1.admin = true + user1.save + user3.admin = true + user3.save + + allow_any_instance_of(Gitlab::LDAP::Group) + .to receive(:adapter).and_return(adapter) + allow(Gitlab::LDAP::Group) + .to receive(:find_by_cn).with(admin_group.cn, any_args) + allow(Gitlab::LDAP::Group) + .to receive(:find_by_cn) + .with('admin_group', kind_of(Gitlab::LDAP::Adapter)) + .and_return(Gitlab::LDAP::Group.new(admin_group)) + + user1.identities.create( + provider: 'ldapmain', + extern_uid: "uid=#{user1.username},ou=users,dc=example,dc=com" + ) + user2.identities.create( + provider: 'ldapmain', + extern_uid: "uid=#{user2.username},ou=users,dc=example,dc=com" + ) + + allow(group_sync).to receive_messages(admin_group: 'admin_group') + end + + it 'adds new admin users' do + expect { group_sync.sync_admin_users } + .to change { User.admins.where(id: user2.id).any? }.from(false).to(true) + end + + it 'removes users that are not in the LDAP group' do + expect { group_sync.sync_admin_users } + .to change { User.admins.where(id: user1.id).any? }.from(true).to(false) + end + + it 'leaves admins that do not have the LDAP provider' do + expect { group_sync.sync_admin_users } + .not_to change { User.admins.where(id: user3.id).any? } + end + end + + describe '#members_to_access_hash' do + let(:group_access) { Gitlab::Access::DEVELOPER } + let(:member_dns) do + %w( + uid=johndoe,ou=users,dc=example,dc=com + uid=janedoe,ou=users,dc=example,dc=com + ) + end + + subject { group_sync.members_to_access_hash(access_hash, member_dns, group_access) } + + context 'when access_hash is empty' do + let(:access_hash) { Hash.new } + + it do + is_expected + .to eq({ + 'uid=janedoe,ou=users,dc=example,dc=com' => 30, + 'uid=johndoe,ou=users,dc=example,dc=com' => 30 + }) + end + end + + context 'when access_hash has existing entries' do + let(:access_hash) do + { + 'uid=janedoe,ou=users,dc=example,dc=com' => 40, + 'uid=johndoe,ou=users,dc=example,dc=com' => 20, + 'uid=jamesdoe,ou=users,dc=example,dc=com' => 40, + } + end + + it 'keeps the higher of all access values' do + is_expected + .to eq({ + 'uid=janedoe,ou=users,dc=example,dc=com' => 40, + 'uid=johndoe,ou=users,dc=example,dc=com' => 30, + 'uid=jamesdoe,ou=users,dc=example,dc=com' => 40 + }) + end + end + end +end -- GitLab