diff --git a/CHANGELOG-EE b/CHANGELOG-EE index a9583715081c8e5d3c23de0978e6a2d17aefad78..b6e826293077cf540780b1cc0f3144a7257ca92b 100644 --- a/CHANGELOG-EE +++ b/CHANGELOG-EE @@ -9,6 +9,7 @@ v 8.7.0 (unreleased) v 8.6.6 - Fix LDAP group sync regression for groups with member value `uid=` !335 + - Concat AD group recursive member results with regular member results v 8.6.5 - No EE-specific changes diff --git a/lib/gitlab/ldap/group.rb b/lib/gitlab/ldap/group.rb index 03a0ab636ebf0b1dc9c57623d07d86ca7e62c2ce..c5b28782bfa1a91956baa83f710e735e70c19606 100644 --- a/lib/gitlab/ldap/group.rb +++ b/lib/gitlab/ldap/group.rb @@ -39,23 +39,27 @@ def member_uids end def member_dns + dns = [] + + # There's an edge-case with AD where sometimes a recursive search + # doesn't return all users at the top-level. Concat recursive results + # with regular results to be safe. See gitlab-ee#484 if active_directory? dns = adapter.dns_for_filter(active_directory_recursive_memberof_filter) - return dns unless dns.empty? end if (entry.respond_to? :member) && (entry.respond_to? :submember) - entry.member + entry.submember + dns.concat(entry.member + entry.submember) elsif entry.respond_to? :member - entry.member + dns.concat(entry.member) elsif entry.respond_to? :uniquemember - entry.uniquemember + dns.concat(entry.uniquemember) elsif entry.respond_to? :memberof - entry.memberof + dns.concat(entry.memberof) else Rails.logger.warn("Could not find member DNs for LDAP group #{entry.inspect}") - [] end + dns.uniq end private diff --git a/spec/lib/gitlab/ldap/group_spec.rb b/spec/lib/gitlab/ldap/group_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..bff80e54f5993fc88e00ea763f8fa6819c88983e --- /dev/null +++ b/spec/lib/gitlab/ldap/group_spec.rb @@ -0,0 +1,50 @@ +require 'spec_helper' + +describe Gitlab::LDAP::Group, lib: true do + describe '#member_dns' do + def ldif + 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,ou=users,dc=example,dc=com + member: uid=user2,ou=users,dc=example,dc=com + member: uid=user3,ou=users,dc=example,dc=com + objectclass: top + objectclass: groupOfNames + EOS + ) + end + + def adapter + @adapter ||= Gitlab::LDAP::Adapter.new('ldapmain') + end + + let(:group) { described_class.new(ldif, adapter) } + let(:recursive_dns) do + %w( + uid=user3,ou=users,dc=example,dc=com + uid=user4,ou=users,dc=example,dc=com + uid=user5,ou=users,dc=example,dc=com + ) + end + + it 'concatenates recursive and regular results and returns uniq' do + allow(group).to receive(:active_directory?).and_return(true) + allow(adapter).to receive(:dns_for_filter).and_return(recursive_dns) + + expect(group.member_dns) + .to match_array( + %w( + uid=user1,ou=users,dc=example,dc=com + uid=user2,ou=users,dc=example,dc=com + uid=user3,ou=users,dc=example,dc=com + uid=user4,ou=users,dc=example,dc=com + uid=user5,ou=users,dc=example,dc=com + ) + ) + end + + end +end