Request only the LDAP attributes we need

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
 

lib/ee/gitlab/ldap/adapter.rb

@@ -16,7 +16,8 @@ module EE
         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 @@ module EE
           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

lib/ee/gitlab/ldap/person.rb

+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

lib/gitlab/ldap/adapter.rb

@@ -29,31 +29,7 @@ module Gitlab
       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 @@ module Gitlab
         users(*args).first
       end
 
-      def dns_for_filter(filter)
-        ldap_search(
-          base: config.base,
+      def dn_matches_filter?(dn, filter)
+        ldap_search(base: dn,
                     filter: filter,
-          scope: Net::LDAP::SearchScope_WholeSubtree,
-          attributes: %w{dn}
-        ).map(&:dn)
+                    scope: Net::LDAP::SearchScope_BaseObject,
+                    attributes: %w{dn}).any?
       end
 
       def ldap_search(*args)
@@ -98,6 +72,38 @@ module Gitlab
         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

lib/gitlab/ldap/person.rb

 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 @@ module Gitlab
         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

spec/lib/ee/gitlab/ldap/adapter_spec.rb

 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

spec/lib/ee/gitlab/ldap/group_spec.rb

 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 @@ describe EE::Gitlab::LDAP::Group, lib: true do
       )
     end
 
-    def adapter
-      @adapter ||= Gitlab::LDAP::Adapter.new('ldapmain')
-    end
-
     let(:group) { described_class.new(ldif, adapter) }
     let(:recursive_dns) do
       %w(

spec/lib/gitlab/ldap/person_spec.rb → spec/lib/ee/gitlab/ldap/person_spec.rb

-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 @@ describe Gitlab::LDAP::Person do
 
     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 @@ describe Gitlab::LDAP::Person do
       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

spec/lib/ee/gitlab/ldap/sync/admin_users_spec.rb

@@ -3,6 +3,8 @@ require 'spec_helper'
 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)) }
 

spec/lib/ee/gitlab/ldap/sync/external_users_spec.rb

@@ -4,6 +4,7 @@ describe EE::Gitlab::LDAP::Sync::ExternalUsers, lib: true do
   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) }

spec/lib/ee/gitlab/ldap/sync/group_spec.rb

@@ -3,6 +3,7 @@ require 'spec_helper'
 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) }
 

spec/lib/ee/gitlab/ldap/sync/groups_spec.rb

@@ -3,6 +3,7 @@ require 'spec_helper'
 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

spec/lib/ee/gitlab/ldap/sync/proxy_spec.rb

@@ -4,6 +4,7 @@ require 'net/ldap/dn'
 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

spec/lib/gitlab/ldap/adapter_spec.rb

 require 'spec_helper'
 
 describe Gitlab::LDAP::Adapter, lib: true do
-  let(:adapter) { Gitlab::LDAP::Adapter.new 'ldapmain' }
+  include LdapHelpers
 
-  describe '#dn_matches_filter?' do
   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
     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

spec/support/ee/ldap_helpers.rb

+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

spec/support/ldap_helpers.rb

 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 @@ module LdapHelpers
   #
   #  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