From 32392c1c7dca7bcd2ea1d61e19fe92bec80b9ef3 Mon Sep 17 00:00:00 2001 From: Drew Blessing Date: Fri, 2 Sep 2016 15:52:32 -0500 Subject: [PATCH] Request only the LDAP attributes we need --- CHANGELOG-EE | 1 + lib/ee/gitlab/ldap/adapter.rb | 11 +-- lib/ee/gitlab/ldap/person.rb | 36 +++++++++ lib/gitlab/ldap/adapter.rb | 70 +++++++++-------- lib/gitlab/ldap/person.rb | 31 +------- spec/lib/ee/gitlab/ldap/adapter_spec.rb | 41 +++++++++- spec/lib/ee/gitlab/ldap/group_spec.rb | 8 +- spec/lib/{ => ee}/gitlab/ldap/person_spec.rb | 52 +++++++------ .../ee/gitlab/ldap/sync/admin_users_spec.rb | 2 + .../gitlab/ldap/sync/external_users_spec.rb | 1 + spec/lib/ee/gitlab/ldap/sync/group_spec.rb | 1 + spec/lib/ee/gitlab/ldap/sync/groups_spec.rb | 1 + spec/lib/ee/gitlab/ldap/sync/proxy_spec.rb | 1 + spec/lib/gitlab/ldap/adapter_spec.rb | 71 +++++++++++++++++- spec/support/ee/ldap_helpers.rb | 64 ++++++++++++++++ spec/support/ldap_helpers.rb | 75 ++----------------- 16 files changed, 297 insertions(+), 169 deletions(-) create mode 100644 lib/ee/gitlab/ldap/person.rb rename spec/lib/{ => ee}/gitlab/ldap/person_spec.rb (61%) create mode 100644 spec/support/ee/ldap_helpers.rb diff --git a/CHANGELOG-EE b/CHANGELOG-EE index 9da01b3d9848d7..9e76fb49aa3c62 100644 --- a/CHANGELOG-EE +++ b/CHANGELOG-EE @@ -2,6 +2,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 8.12.0 (Unreleased) - Reduce UPDATE queries when moving between import states on projects - [ES] Instrument Elasticsearch::Git::Repository + - Request only the LDAP attributes we need - [ES] Instrument other Gitlab::Elastic classes - [ES] Fix: Elasticsearch does not find partial matches in project names diff --git a/lib/ee/gitlab/ldap/adapter.rb b/lib/ee/gitlab/ldap/adapter.rb index c6c20efd425395..a2aa35e25063e9 100644 --- a/lib/ee/gitlab/ldap/adapter.rb +++ b/lib/ee/gitlab/ldap/adapter.rb @@ -16,7 +16,8 @@ module Adapter def groups(cn = "*", size = nil) options = { base: config.group_base, - filter: Net::LDAP::Filter.eq("cn", cn) + filter: Net::LDAP::Filter.eq("cn", cn), + attributes: %w(dn cn memberuid member submember uniquemember memberof) } options.merge!(size: size) if size @@ -30,13 +31,13 @@ def group(*args) groups(*args).first end - def dn_matches_filter?(dn, filter) + def dns_for_filter(filter) ldap_search( - base: dn, + base: config.base, filter: filter, - scope: Net::LDAP::SearchScope_BaseObject, + scope: Net::LDAP::SearchScope_WholeSubtree, attributes: %w{dn} - ).any? + ).map(&:dn) end end end diff --git a/lib/ee/gitlab/ldap/person.rb b/lib/ee/gitlab/ldap/person.rb new file mode 100644 index 00000000000000..8dea43b50ccac2 --- /dev/null +++ b/lib/ee/gitlab/ldap/person.rb @@ -0,0 +1,36 @@ +module EE + module Gitlab + module LDAP + module Person + def ssh_keys + if config.sync_ssh_keys? && entry.respond_to?(config.sync_ssh_keys) + entry[config.sync_ssh_keys.to_sym]. + map { |key| key[/(ssh|ecdsa)-[^ ]+ [^\s]+/] }. + compact + else + [] + end + end + + def kerberos_principal + # The following is only meaningful for Active Directory + return unless entry.respond_to?(:sAMAccountName) + entry[:sAMAccountName].first + '@' + windows_domain_name.upcase + end + + def windows_domain_name + # The following is only meaningful for Active Directory + require 'net/ldap/dn' + dn_components = [] + Net::LDAP::DN.new(dn).each_pair { |name, value| dn_components << { name: name, value: value } } + dn_components. + reverse. + take_while { |rdn| rdn[:name].casecmp('DC').zero? }. # Domain Component + map { |rdn| rdn[:value] }. + reverse. + join('.') + end + end + end + end +end diff --git a/lib/gitlab/ldap/adapter.rb b/lib/gitlab/ldap/adapter.rb index a91cdd2e800ede..68f61fa293caa3 100644 --- a/lib/gitlab/ldap/adapter.rb +++ b/lib/gitlab/ldap/adapter.rb @@ -29,31 +29,7 @@ def config end def users(field, value, limit = nil) - if field.to_sym == :dn - options = { - base: value, - scope: Net::LDAP::SearchScope_BaseObject - } - else - options = { - base: config.base, - filter: Net::LDAP::Filter.eq(field, value) - } - end - - if config.user_filter.present? - user_filter = Net::LDAP::Filter.construct(config.user_filter) - - options[:filter] = if options[:filter] - Net::LDAP::Filter.join(options[:filter], user_filter) - else - user_filter - end - end - - if limit.present? - options.merge!(size: limit) - end + options = user_options(field, value, limit) entries = ldap_search(options).select do |entry| entry.respond_to? config.uid @@ -68,13 +44,11 @@ def user(*args) users(*args).first end - def dns_for_filter(filter) - ldap_search( - base: config.base, - filter: filter, - scope: Net::LDAP::SearchScope_WholeSubtree, - attributes: %w{dn} - ).map(&:dn) + def dn_matches_filter?(dn, filter) + ldap_search(base: dn, + filter: filter, + scope: Net::LDAP::SearchScope_BaseObject, + attributes: %w{dn}).any? end def ldap_search(*args) @@ -98,6 +72,38 @@ def ldap_search(*args) Rails.logger.warn("LDAP search timed out after #{config.timeout} seconds") [] end + + private + + def user_options(field, value, limit) + options = { attributes: %W(#{config.uid} cn mail dn) } + options[:size] = limit if limit + + if field.to_sym == :dn + options[:base] = value + options[:scope] = Net::LDAP::SearchScope_BaseObject + options[:filter] = user_filter + else + options[:base] = config.base + options[:filter] = user_filter(Net::LDAP::Filter.eq(field, value)) + end + + options + end + + def user_filter(filter = nil) + if config.user_filter.present? + user_filter = Net::LDAP::Filter.construct(config.user_filter) + end + + if user_filter && filter + Net::LDAP::Filter.join(filter, user_filter) + elsif user_filter + user_filter + else + filter + end + end end end end diff --git a/lib/gitlab/ldap/person.rb b/lib/gitlab/ldap/person.rb index 82ad86cf2f9fad..7a039d6c18d4b5 100644 --- a/lib/gitlab/ldap/person.rb +++ b/lib/gitlab/ldap/person.rb @@ -1,6 +1,8 @@ module Gitlab module LDAP class Person + include EE::Gitlab::LDAP::Person + # Active Directory-specific LDAP filter that checks if bit 2 of the # userAccountControl attribute is set. # Source: http://ctogonewild.com/2009/09/03/bitmask-searches-in-ldap/ @@ -47,35 +49,6 @@ def dn entry.dn end - def ssh_keys - if config.sync_ssh_keys? && entry.respond_to?(config.sync_ssh_keys) - entry[config.sync_ssh_keys.to_sym]. - map { |key| key[/(ssh|ecdsa)-[^ ]+ [^\s]+/] }. - compact - else - [] - end - end - - def kerberos_principal - # The following is only meaningful for Active Directory - return unless entry.respond_to?(:sAMAccountName) - entry[:sAMAccountName].first + '@' + windows_domain_name.upcase - end - - def windows_domain_name - # The following is only meaningful for Active Directory - require 'net/ldap/dn' - dn_components = [] - Net::LDAP::DN.new(dn).each_pair { |name, value| dn_components << { name: name, value: value } } - dn_components. - reverse. - take_while { |rdn| rdn[:name].casecmp('DC').zero? }. # Domain Component - map { |rdn| rdn[:value] }. - reverse. - join('.') - end - private def entry diff --git a/spec/lib/ee/gitlab/ldap/adapter_spec.rb b/spec/lib/ee/gitlab/ldap/adapter_spec.rb index 5e7db6f5c39a08..5296cb28fa25f8 100644 --- a/spec/lib/ee/gitlab/ldap/adapter_spec.rb +++ b/spec/lib/ee/gitlab/ldap/adapter_spec.rb @@ -1,9 +1,42 @@ require 'spec_helper' -# Test things specific to the EE mixin, but run the actual tests -# against the main adapter class to ensure it's properly included describe Gitlab::LDAP::Adapter, lib: true do - subject { Gitlab::LDAP::Adapter.new 'ldapmain' } + include LdapHelpers - it { is_expected.to include_module(EE::Gitlab::LDAP::Adapter) } + it 'includes the EE module' do + expect(Gitlab::LDAP::Adapter).to include_module(EE::Gitlab::LDAP::Adapter) + end + + describe '#groups' do + let(:adapter) { ldap_adapter('ldapmain') } + + before do + stub_ldap_config( + group_base: 'ou=groups,dc=example,dc=com', + active_directory: false + ) + end + + it 'searches with the proper options' do + # Requires this expectation style to match the filter + expect(adapter).to receive(:ldap_search) do |arg| + expect(arg[:filter].to_s).to eq('(cn=*)') + expect(arg[:base]).to eq('ou=groups,dc=example,dc=com') + expect(arg[:attributes]).to match(%w(dn cn memberuid member submember uniquemember memberof)) + end.and_return({}) + + adapter.groups + end + + it 'returns a group object if search returns a result' do + entry = ldap_group_entry(['john', 'mary'], cn: 'group1') + allow(adapter).to receive(:ldap_search).and_return([entry]) + + results = adapter.groups('group1') + + expect(results.first).to be_a(EE::Gitlab::LDAP::Group) + expect(results.first.cn).to eq('group1') + expect(results.first.member_dns).to match_array(%w(john mary)) + end + end end diff --git a/spec/lib/ee/gitlab/ldap/group_spec.rb b/spec/lib/ee/gitlab/ldap/group_spec.rb index 8d360d7ab8de8b..3ea5cb87f7fbf0 100644 --- a/spec/lib/ee/gitlab/ldap/group_spec.rb +++ b/spec/lib/ee/gitlab/ldap/group_spec.rb @@ -1,6 +1,10 @@ require 'spec_helper' describe EE::Gitlab::LDAP::Group, lib: true do + include LdapHelpers + + let(:adapter) { ldap_adapter } + describe '#member_dns' do def ldif Net::LDAP::Entry.from_single_ldif_string( @@ -17,10 +21,6 @@ def ldif ) end - def adapter - @adapter ||= Gitlab::LDAP::Adapter.new('ldapmain') - end - let(:group) { described_class.new(ldif, adapter) } let(:recursive_dns) do %w( diff --git a/spec/lib/gitlab/ldap/person_spec.rb b/spec/lib/ee/gitlab/ldap/person_spec.rb similarity index 61% rename from spec/lib/gitlab/ldap/person_spec.rb rename to spec/lib/ee/gitlab/ldap/person_spec.rb index 704c29447998ba..2f099e69d3f91a 100644 --- a/spec/lib/gitlab/ldap/person_spec.rb +++ b/spec/lib/ee/gitlab/ldap/person_spec.rb @@ -1,7 +1,11 @@ -require "spec_helper" +require 'spec_helper' describe Gitlab::LDAP::Person do - describe "#kerberos_principal" do + it 'includes the EE module' do + expect(Gitlab::LDAP::Person).to include(EE::Gitlab::LDAP::Person) + end + + describe '#kerberos_principal' do let(:entry) do ldif = "dn: cn=foo, dc=bar, dc=com\n" ldif += "sAMAccountName: #{sam_account_name}\n" if sam_account_name @@ -10,25 +14,25 @@ subject { Gitlab::LDAP::Person.new(entry, 'ldapmain') } - context "when sAMAccountName is not defined (non-AD LDAP server)" do + context 'when sAMAccountName is not defined (non-AD LDAP server)' do let(:sam_account_name) { nil } - it "returns nil" do + it 'returns nil' do expect(subject.kerberos_principal).to be_nil end end - context "when sAMAccountName is defined (AD server)" do - let(:sam_account_name) { "mylogin" } + context 'when sAMAccountName is defined (AD server)' do + let(:sam_account_name) { 'mylogin' } - it "returns the principal combining sAMAccountName and DC components of the distinguishedName" do - expect(subject.kerberos_principal).to eq("mylogin@BAR.COM") + it 'returns the principal combining sAMAccountName and DC components of the distinguishedName' do + expect(subject.kerberos_principal).to eq('mylogin@BAR.COM') end end end - describe "#ssh_keys" do - let(:ssh_key) { "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQCrSQHff6a1rMqBdHFt+FwIbytMZ+hJKN3KLkTtOWtSvNIriGhnTdn4rs+tjD/w+z+revytyWnMDM9dS7J8vQi006B16+hc9Xf82crqRoPRDnBytgAFFQY1G/55ql2zdfsC5yvpDOFzuwIJq5dNGsojS82t6HNmmKPq130fzsenFnj5v1pl3OJvk513oduUyKiZBGTroWTn7H/eOPtu7s9MD7pAdEjqYKFLeaKmyidiLmLqQlCRj3Tl2U9oyFg4PYNc0bL5FZJ/Z6t0Ds3i/a2RanQiKxrvgu3GSnUKMx7WIX373baL4jeM7cprRGiOY/1NcS+1cAjfJ8oaxQF/1dYj" } + describe '#ssh_keys' do + let(:ssh_key) { 'ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQCrSQHff6a1rMqBdHFt+FwIbytMZ+hJKN3KLkTtOWtSvNIriGhnTdn4rs+tjD/w+z+revytyWnMDM9dS7J8vQi006B16+hc9Xf82crqRoPRDnBytgAFFQY1G/55ql2zdfsC5yvpDOFzuwIJq5dNGsojS82t6HNmmKPq130fzsenFnj5v1pl3OJvk513oduUyKiZBGTroWTn7H/eOPtu7s9MD7pAdEjqYKFLeaKmyidiLmLqQlCRj3Tl2U9oyFg4PYNc0bL5FZJ/Z6t0Ds3i/a2RanQiKxrvgu3GSnUKMx7WIX373baL4jeM7cprRGiOY/1NcS+1cAjfJ8oaxQF/1dYj' } let(:ssh_key_attribute_name) { 'altSecurityIdentities' } let(:entry) do Net::LDAP::Entry.from_single_ldif_string("dn: cn=foo, dc=bar, dc=com\n#{keys}") @@ -40,53 +44,53 @@ allow_any_instance_of(Gitlab::LDAP::Config).to receive_messages(sync_ssh_keys: ssh_key_attribute_name) end - context "when the SSH key is literal" do + context 'when the SSH key is literal' do let(:keys) { "#{ssh_key_attribute_name}: #{ssh_key}" } - it "includes the SSH key" do + it 'includes the SSH key' do expect(subject.ssh_keys).to include(ssh_key) end end - context "when the SSH key is prefixed" do + context 'when the SSH key is prefixed' do let(:keys) { "#{ssh_key_attribute_name}: SSHKey:#{ssh_key}" } - it "includes the SSH key" do + it 'includes the SSH key' do expect(subject.ssh_keys).to include(ssh_key) end end - context "when the SSH key is suffixed" do + context 'when the SSH key is suffixed' do let(:keys) { "#{ssh_key_attribute_name}: #{ssh_key} (SSH key)" } - it "includes the SSH key" do + it 'includes the SSH key' do expect(subject.ssh_keys).to include(ssh_key) end end - context "when the SSH key is followed by a newline" do + context 'when the SSH key is followed by a newline' do let(:keys) { "#{ssh_key_attribute_name}: #{ssh_key}\n" } - it "includes the SSH key" do + it 'includes the SSH key' do expect(subject.ssh_keys).to include(ssh_key) end end - context "when the key is not an SSH key" do + context 'when the key is not an SSH key' do let(:keys) { "#{ssh_key_attribute_name}: KerberosKey:bogus" } - it "is empty" do + it 'is empty' do expect(subject.ssh_keys).to be_empty end end - context "when there are multiple keys" do + context 'when there are multiple keys' do let(:keys) { "#{ssh_key_attribute_name}: #{ssh_key}\n#{ssh_key_attribute_name}: KerberosKey:bogus\n#{ssh_key_attribute_name}: ssh-rsa keykeykey" } - it "includes both SSH keys" do + it 'includes both SSH keys' do expect(subject.ssh_keys).to include(ssh_key) - expect(subject.ssh_keys).to include("ssh-rsa keykeykey") - expect(subject.ssh_keys).not_to include("KerberosKey:bogus") + expect(subject.ssh_keys).to include('ssh-rsa keykeykey') + expect(subject.ssh_keys).not_to include('KerberosKey:bogus') end end end diff --git a/spec/lib/ee/gitlab/ldap/sync/admin_users_spec.rb b/spec/lib/ee/gitlab/ldap/sync/admin_users_spec.rb index 130e87efce51dd..c3cfd534ed1fce 100644 --- a/spec/lib/ee/gitlab/ldap/sync/admin_users_spec.rb +++ b/spec/lib/ee/gitlab/ldap/sync/admin_users_spec.rb @@ -3,6 +3,8 @@ describe EE::Gitlab::LDAP::Sync::AdminUsers, lib: true do include LdapHelpers + let(:adapter) { ldap_adapter } + describe '#update_permissions' do let(:sync_admin) { described_class.new(proxy(adapter)) } diff --git a/spec/lib/ee/gitlab/ldap/sync/external_users_spec.rb b/spec/lib/ee/gitlab/ldap/sync/external_users_spec.rb index 35e376332ca701..0507ea7eb6ba0f 100644 --- a/spec/lib/ee/gitlab/ldap/sync/external_users_spec.rb +++ b/spec/lib/ee/gitlab/ldap/sync/external_users_spec.rb @@ -4,6 +4,7 @@ include LdapHelpers describe '#update_permissions' do + let(:adapter) { ldap_adapter } let(:sync_external) { described_class.new(proxy(adapter)) } let(:user1) { create(:user) } let(:user2) { create(:user) } diff --git a/spec/lib/ee/gitlab/ldap/sync/group_spec.rb b/spec/lib/ee/gitlab/ldap/sync/group_spec.rb index f615c8f1260f73..834c594fa6b678 100644 --- a/spec/lib/ee/gitlab/ldap/sync/group_spec.rb +++ b/spec/lib/ee/gitlab/ldap/sync/group_spec.rb @@ -3,6 +3,7 @@ describe EE::Gitlab::LDAP::Sync::Group, lib: true do include LdapHelpers + let(:adapter) { ldap_adapter } let(:sync_group) { described_class.new(group, proxy(adapter)) } let(:user) { create(:user) } diff --git a/spec/lib/ee/gitlab/ldap/sync/groups_spec.rb b/spec/lib/ee/gitlab/ldap/sync/groups_spec.rb index 09871eab546620..5a34766a42e8e9 100644 --- a/spec/lib/ee/gitlab/ldap/sync/groups_spec.rb +++ b/spec/lib/ee/gitlab/ldap/sync/groups_spec.rb @@ -3,6 +3,7 @@ describe EE::Gitlab::LDAP::Sync::Groups, lib: true do include LdapHelpers + let(:adapter) { ldap_adapter } let(:group_sync) { described_class.new(proxy(adapter)) } describe '#update_permissions' do diff --git a/spec/lib/ee/gitlab/ldap/sync/proxy_spec.rb b/spec/lib/ee/gitlab/ldap/sync/proxy_spec.rb index cb2b3913b253be..8fcd75bb038523 100644 --- a/spec/lib/ee/gitlab/ldap/sync/proxy_spec.rb +++ b/spec/lib/ee/gitlab/ldap/sync/proxy_spec.rb @@ -4,6 +4,7 @@ describe EE::Gitlab::LDAP::Sync::Proxy, lib: true do include LdapHelpers + let(:adapter) { ldap_adapter } let(:sync_proxy) { EE::Gitlab::LDAP::Sync::Proxy.new('ldapmain', adapter) } before do diff --git a/spec/lib/gitlab/ldap/adapter_spec.rb b/spec/lib/gitlab/ldap/adapter_spec.rb index 4847b5f3b0e62e..0600893f4cffce 100644 --- a/spec/lib/gitlab/ldap/adapter_spec.rb +++ b/spec/lib/gitlab/ldap/adapter_spec.rb @@ -1,12 +1,77 @@ require 'spec_helper' describe Gitlab::LDAP::Adapter, lib: true do - let(:adapter) { Gitlab::LDAP::Adapter.new 'ldapmain' } + include LdapHelpers + + let(:ldap) { double(:ldap) } + let(:adapter) { ldap_adapter('ldapmain', ldap) } + + describe '#users' do + before do + stub_ldap_config(base: 'dc=example,dc=com') + end + + it 'searches with the proper options when searching by uid' do + # Requires this expectation style to match the filter + expect(adapter).to receive(:ldap_search) do |arg| + expect(arg[:filter].to_s).to eq('(uid=johndoe)') + expect(arg[:base]).to eq('dc=example,dc=com') + expect(arg[:attributes]).to match(%w{uid cn mail dn}) + end.and_return({}) + + adapter.users('uid', 'johndoe') + end + + it 'searches with the proper options when searching by dn' do + expect(adapter).to receive(:ldap_search).with( + base: 'uid=johndoe,ou=users,dc=example,dc=com', + scope: Net::LDAP::SearchScope_BaseObject, + attributes: %w{uid cn mail dn}, + filter: nil + ).and_return({}) + + adapter.users('dn', 'uid=johndoe,ou=users,dc=example,dc=com') + end + + it 'searches with the proper options when searching with a limit' do + expect(adapter) + .to receive(:ldap_search).with(hash_including(size: 100)).and_return({}) + + adapter.users('uid', 'johndoe', 100) + end + + it 'returns an LDAP::Person if search returns a result' do + entry = ldap_user_entry('johndoe') + allow(adapter).to receive(:ldap_search).and_return([entry]) + + results = adapter.users('uid', 'johndoe') + + expect(results.size).to eq(1) + expect(results.first.uid).to eq('johndoe') + end + + it 'returns empty array if search entry does not respond to uid' do + entry = Net::LDAP::Entry.new + entry['dn'] = user_dn('johndoe') + allow(adapter).to receive(:ldap_search).and_return([entry]) + + results = adapter.users('uid', 'johndoe') + + expect(results).to be_empty + end + + it 'uses the right uid attribute when non-default' do + stub_ldap_config(uid: 'sAMAccountName') + expect(adapter).to receive(:ldap_search).with( + hash_including(attributes: %w{sAMAccountName cn mail dn}) + ).and_return({}) + + adapter.users('sAMAccountName', 'johndoe') + end + end describe '#dn_matches_filter?' do - let(:ldap) { double(:ldap) } subject { adapter.dn_matches_filter?(:dn, :filter) } - before { allow(adapter).to receive(:ldap).and_return(ldap) } context "when the search is successful" do context "and the result is non-empty" do diff --git a/spec/support/ee/ldap_helpers.rb b/spec/support/ee/ldap_helpers.rb new file mode 100644 index 00000000000000..590a63bd0440ac --- /dev/null +++ b/spec/support/ee/ldap_helpers.rb @@ -0,0 +1,64 @@ +module EE + module LdapHelpers + def proxy(adapter, provider = 'ldapmain') + EE::Gitlab::LDAP::Sync::Proxy.new(provider, adapter) + end + + # Stub an LDAP group search and provide the return entry. Specify `nil` for + # `entry` to simulate when an LDAP group is not found + # + # Example: + # adapter = ::Gitlab::LDAP::Adapter.new('ldapmain', double(:ldap)) + # ldap_group1 = ldap_group_entry('uid=user,ou=users,dc=example,dc=com') + # + # stub_ldap_group_find_by_cn('ldap_group1', ldap_group1, adapter) + def stub_ldap_group_find_by_cn(cn, entry, adapter = nil) + if entry.present? + return_value = EE::Gitlab::LDAP::Group.new(entry, adapter) + end + + allow(EE::Gitlab::LDAP::Group) + .to receive(:find_by_cn) + .with(cn, kind_of(::Gitlab::LDAP::Adapter)).and_return(return_value) + end + + # Create an LDAP group entry with any number of members. By default, creates + # a groupOfNames style entry. Change the style by specifying the object class + # and member attribute name. The last example below shows how to specify a + # posixGroup (Apple Open Directory) entry. `members` can be nil to create + # an empty group. + # + # Example: + # ldap_group_entry('uid=user,ou=users,dc=example,dc=com') + # + # ldap_group_entry( + # 'uid=user1,ou=users,dc=example,dc=com', + # 'uid=user2,ou=users,dc=example,dc=com' + # ) + # + # ldap_group_entry( + # [ 'user1', 'user2' ], + # cn: 'my_group' + # objectclass: 'posixGroup', + # member_attr: 'memberUid' + # ) + def ldap_group_entry( + members, + cn: 'ldap_group1', + objectclass: 'groupOfNames', + member_attr: 'uniqueMember' + ) + entry = Net::LDAP::Entry.from_single_ldif_string(<<-EOS.strip_heredoc) + dn: cn=#{cn},ou=groups,dc=example,dc=com + cn: #{cn} + description: LDAP Group #{cn} + objectclass: top + objectclass: #{objectclass} + EOS + + members = [members].flatten + entry[member_attr] = members if members.any? + entry + end + end +end diff --git a/spec/support/ldap_helpers.rb b/spec/support/ldap_helpers.rb index c49544acd1781f..e1c877329beb55 100644 --- a/spec/support/ldap_helpers.rb +++ b/spec/support/ldap_helpers.rb @@ -1,10 +1,8 @@ module LdapHelpers - def adapter(provider = 'ldapmain') - ::Gitlab::LDAP::Adapter.new(provider, double(:ldap)) - end + include EE::LdapHelpers - def proxy(adapter, provider = 'ldapmain') - EE::Gitlab::LDAP::Sync::Proxy.new(provider, adapter) + def ldap_adapter(provider = 'ldapmain', ldap = double(:ldap)) + ::Gitlab::LDAP::Adapter.new(provider, ldap) end def user_dn(uid) @@ -34,77 +32,18 @@ def stub_ldap_config(messages) # # stub_ldap_person_find_by_uid('john_doe', ldap_user_entry, adapter) def stub_ldap_person_find_by_uid(uid, entry, provider = 'ldapmain') - return_value = if entry.present? - ::Gitlab::LDAP::Person.new(entry, provider) - else - nil - end + return_value = ::Gitlab::LDAP::Person.new(entry, provider) if entry.present? allow(::Gitlab::LDAP::Person) .to receive(:find_by_uid).with(uid, any_args).and_return(return_value) end - # Stub an LDAP group search and provide the return entry. Specify `nil` for - # `entry` to simulate when an LDAP group is not found - # - # Example: - # adapter = ::Gitlab::LDAP::Adapter.new('ldapmain', double(:ldap)) - # ldap_group1 = ldap_group_entry('uid=user,ou=users,dc=example,dc=com') - # - # stub_ldap_group_find_by_cn('ldap_group1', ldap_group1, adapter) - def stub_ldap_group_find_by_cn(cn, entry, adapter = nil) - return_value = if entry.present? - EE::Gitlab::LDAP::Group.new(entry, adapter) - else - nil - end - - allow(EE::Gitlab::LDAP::Group) - .to receive(:find_by_cn) - .with(cn, kind_of(::Gitlab::LDAP::Adapter)).and_return(return_value) - end - # Create a simple LDAP user entry. def ldap_user_entry(uid) - Net::LDAP::Entry.from_single_ldif_string("dn: #{user_dn(uid)}") - end - - # Create an LDAP group entry with any number of members. By default, creates - # a groupOfNames style entry. Change the style by specifying the object class - # and member attribute name. The last example below shows how to specify a - # posixGroup (Apple Open Directory) entry. `members` can be nil to create - # an empty group. - # - # Example: - # ldap_group_entry('uid=user,ou=users,dc=example,dc=com') - # - # ldap_group_entry( - # 'uid=user1,ou=users,dc=example,dc=com', - # 'uid=user2,ou=users,dc=example,dc=com' - # ) - # - # ldap_group_entry( - # [ 'user1', 'user2' ], - # cn: 'my_group' - # objectclass: 'posixGroup', - # member_attr: 'memberUid' - # ) - def ldap_group_entry( - members, - cn: 'ldap_group1', - objectclass: 'groupOfNames', - member_attr: 'uniqueMember' - ) - entry = Net::LDAP::Entry.from_single_ldif_string(<<-EOS.strip_heredoc) - dn: cn=#{cn},ou=groups,dc=example,dc=com - cn: #{cn} - description: LDAP Group #{cn} - objectclass: top - objectclass: #{objectclass} - EOS + entry = Net::LDAP::Entry.new + entry['dn'] = user_dn(uid) + entry['uid'] = uid - members = [members].flatten - entry[member_attr] = members if members.any? entry end end -- GitLab