- Refactor all source-code to apply almost rubocop rules

- Remove unnecessary instance variables by using `config` property
- Add optional parameter `only` into `find_*` methods to get one result
- Refactor all unit-tests to apply almost `rubocop` rules

README.rdoc

@@ -34,10 +34,10 @@ It exposes these methods:
  valid_group?(uid)
  returns true if the group provided exists
 
- find_user(uid)
+ find_user(uid, only = nil)
  returns the LDAP entry of the user if found, nil if not found
 
- find_group(gid)
+ find_group(gid, only = nil)
  returns the LDAP entry of the group if found, nil if not found
 
 These methods are handy for using LDAP for both authentication and authorization.
@@ -49,12 +49,12 @@ Your global configuration must provide information about your LDAP host to funct
  host: # ip address or hostname
  port: # port
  encryption: # blank, :simple_tls, or :start_tls
- base_dn: # base DN for LDAP auth, eg dc=redhat,dc=com
+ base_dn:  # base DN for LDAP auth, eg dc=redhat,dc=com
  group_base: # base DN for your LDAP groups, eg ou=Groups,dc=redhat,dc=com
  use_netgroups: # false by default, use true if you want to use netgroup triples,
  # supported only for server type :free_ipa and :posix
- server_type: # type of server. default == :posix. :active_directory, :posix, :free_ipa
- ad_domain: # domain for your users if using active directory, eg redhat.com
+ server_type:  # type of server. default == :posix. :active_directory, :posix, :free_ipa
+ ad_domain:  # domain for your users if using active directory, eg redhat.com
  service_user: # service account for authenticating LDAP calls. required unless you enable anon
  service_pass: # service password for authenticating LDAP calls. required unless you enable anon
  anon_queries: # false by default, true if you don't want to use the service user

lib/ldap_fluff.rb

@@ -23,20 +23,9 @@ class LdapFluff
  attr_accessor :ldap, :instrumentation_service
 
  def initialize(config = {})
- config = LdapFluff::Config.new(config)
-
- @ldap =
- case config.server_type
- when :posix
- Posix.new(config)
- when :active_directory
- ActiveDirectory.new(config)
- when :free_ipa
- FreeIPA.new(config)
- else
- raise 'unknown server_type'
- end
+ config = Config.new(config)
 
+ @ldap = create_provider(config)
  @instrumentation_service = config.instrumentation_service
  end
 
@@ -45,87 +34,99 @@ def initialize(config = {})
  # @return [Boolean]
  def authenticate?(uid, password)
  instrument('authenticate.ldap_fluff', uid: uid) do |payload|
- if password.nil? || password.empty?
- false
- else
- [email protected]?(uid, password)
- end
+ !password || password.empty? ? false : ldap.bind?(payload[:uid], password)
  end
  end
 
  def test
- instrument('test.ldap_fluff') do |payload|
- @ldap.ldap.open {}
+ instrument('test.ldap_fluff') do # |payload|
+ ldap.ldap.open {}
  end
  end
 
  # @param [String] gid
  # @return [Array<String>] a list of users for a given gid
  def user_list(gid)
  instrument('user_list.ldap_fluff', gid: gid) do |payload|
- @ldap.users_for_gid(gid)
+ ldap.users_for_gid payload[:gid]
  end
  end
 
  # @param [String] uid
  # @return [Array<String>] a list of groups for a given uid
  def group_list(uid)
  instrument('group_list.ldap_fluff', uid: uid) do |payload|
- @ldap.groups_for_uid(uid)
+ ldap.groups_for_uid payload[:uid]
  end
  end
 
  # @param [String] uid
  # @return [Boolean] true if a user is in all of the groups in grouplist
  def user_in_groups?(uid, grouplist)
  instrument('user_in_groups?.ldap_fluff', uid: uid, grouplist: grouplist) do |payload|
- @ldap.user_in_groups?(uid, grouplist, true)
+ ldap.user_in_groups? payload[:uid], payload[:grouplist], true
  end
  end
 
  # @param [String] uid
  # @return [Boolean] true if uid exists
  def valid_user?(uid)
  instrument('valid_user?.ldap_fluff', uid: uid) do |payload|
- @ldap.user_exists? uid
+ ldap.user_exists? payload[:uid]
  end
  end
 
  # @param [String] gid
  # @return [Boolean] true if group exists
  def valid_group?(gid)
  instrument('valid_group?.ldap_fluff', gid: gid) do |payload|
- @ldap.group_exists? gid
+ ldap.group_exists? payload[:gid]
  end
  end
 
  # @param [String] uid
- # @return [Array, Net::LDAP::Entry]
- def find_user(uid)
+ # @return [Array<Net::LDAP::Entry>, Net::LDAP::Entry]
+ def find_user(uid, only = nil)
  instrument('find_user.ldap_fluff', uid: uid) do |payload|
- @ldap.member_service.find_user(uid)
+ ldap.member_service.find_user payload[:uid], only
  end
  end
 
  # @param [String] gid
- # @return [Array, Net::LDAP::Entry]
- def find_group(gid)
+ # @return [Array<Net::LDAP::Entry>, Net::LDAP::Entry]
+ def find_group(gid, only = nil)
  instrument('find_group.ldap_fluff', gid: gid) do |payload|
- @ldap.member_service.find_group(gid)
+ ldap.member_service.find_group payload[:gid], only
  end
  end
 
  private
 
+ # @param [Config] config
+ # @return [Generic]
+ # @raise [RuntimeError]
+ def create_provider(config)
+ case config.server_type
+ when :posix
+ Posix.new(config)
+ when :active_directory
+ ActiveDirectory.new(config)
+ when :free_ipa
+ FreeIPA.new(config)
+ else
+ raise 'unknown server_type'
+ end
+ end
+
  # @param [String] event
  # @param [Hash] payload
  # @yieldreturn [Hash]
  def instrument(event, payload = {})
  payload = payload ? payload.dup : {}
 
  if instrumentation_service
- instrumentation_service.instrument(event, payload) do |payload|
- payload[:result] = yield(payload) if block_given?
+ instrumentation_service.instrument(event, payload) do |data|
+ data[:result] = yield(data) if block_given?
  end
  elsif block_given?
  yield(payload)

lib/ldap_fluff/active_directory.rb

@@ -1,51 +1,64 @@
 # frozen_string_literal: true
 
 class LdapFluff::ActiveDirectory < LdapFluff::Generic
+ # @param [String] uid
+ # @param [String] password
+ # @param [Hash] opts
+ # @return [Boolean]
  def bind?(uid = nil, password = nil, opts = {})
- unless uid.include?(',') || uid.include?('\\') || opts[:search] == false
+ if opts[:search] != false && uid && !(uid.include?(',') || uid.include?('\\'))
  service_bind
- user = @member_service.find_user(uid)
- uid = user.first.dn if user && user.first
+ user = member_service.find_user(uid, true)
+
+ uid = user.dn if user
  end
- @ldap.auth(uid, password)
- @ldap.bind
+
+ super(uid, password)
  end
 
  # active directory stores group membership on a users model
  # TODO: query by group individually not like this
+ #
+ # @param [String] uid
+ # @param [Array<String>] gids
+ # @return [Boolean]
  def user_in_groups?(uid, gids = [], all = false)
- service_bind
- return true if !gids || gids.empty?
-
- begin
- groups = @member_service.find_user_groups(uid)
- intersection = gids & groups
- all ? (intersection == gids) : !intersection.empty?
- rescue MemberService::UIDNotFoundException
- false
- end
+ super
+ rescue MemberService::UIDNotFoundException
+ false
  end
 
  private
 
+ # @param [Net::LDAP::Entry] search
+ # @param [Symbol] method
+ # @return [Array<String>]
  def users_from_search_results(search, method)
- users = []
+ members = search.send method
 
- search.send(method).each do |member|
+ # @type [Array<String>]
+ users = members.map do |member|
  begin
- entry = @member_service.find_by_dn(member).first
+ entry = member_service.find_by_dn(member, true)
  rescue MemberService::UIDNotFoundException
- next
+ entry = nil
  end
- objectclasses = entry.objectclass.map(&:downcase)
 
- if !(%w[organizationalperson person userproxy] & objectclasses).empty?
- users << @member_service.get_login_from_entry(entry)
- elsif !(%w[organizationalunit group] & objectclasses).empty?
- users << users_for_gid(entry.cn.first)
- end
+ entry ? get_users_for_entry(entry) : nil
  end
 
- users.flatten.uniq
+ users.flatten.compact.uniq
+ end
+
+ # @param [Net::LDAP::Entry] entry
+ # @return [Array<String>, String]
+ def get_users_for_entry(entry)
+ objectclasses = entry[:objectclass].map(&:downcase)
+
+ if !(%w[organizationalperson person userproxy] & objectclasses).empty?
+ member_service.get_login_from_entry(entry)
+ elsif !(%w[organizationalunit group] & objectclasses).empty?
+ users_for_gid(entry[:cn].first)
+ end
  end
 end

lib/ldap_fluff/ad_member_service.rb

@@ -5,47 +5,59 @@ class LdapFluff::ActiveDirectory::MemberService < LdapFluff::GenericMemberServic
  # @param [Net::LDAP] ldap
  # @param [Config] config
  def initialize(ldap, config)
- @attr_login = (config.attr_login || 'samaccountname')
+ config.instance_variable_set(:@attr_login, 'samaccountname') unless config.attr_login
  super
  end
 
- # get a list [] of LDAP groups for a given user
- # in active directory, this means a recursive lookup
+ # get a list of LDAP groups for a given user in active directory, this means a recursive lookup
+ # @param [String] uid
+ # @return [Array<String>]
  def find_user_groups(uid)
- data = find_user(uid)
- _groups_from_ldap_data(data.first)
+ # @type [Net::LDAP::Entry]
+ data = find_user(uid, true)
+ groups_from_ldap_data(data)
  end
 
- # return the :memberof attrs + parents, recursively
- def _groups_from_ldap_data(payload)
- data = []
- if !payload.nil?
- first_level = payload[:memberof]
- total_groups, = _walk_group_ancestry(first_level, first_level)
- data = (get_groups(first_level + total_groups)).uniq
- end
- data
+ private
+
+ # @param [Net::LDAP::Entry] payload
+ # @return [Array<String>] the :memberof attrs + parents, recursively
+ def groups_from_ldap_data(payload)
+ return [] unless payload
+
+ first_level = payload[:memberof]
+ total_groups, = walk_group_ancestry(first_level, first_level)
+
+ get_groups(first_level + total_groups).uniq
  end
 
  # recursively loop over the parent list
- def _walk_group_ancestry(group_dns = [], known_groups = [])
- set = []
+ # @param [Array<String>] group_dns
+ # @param [Array<String>] known_groups
+ # @return [Array<Array<String>>]
+ def walk_group_ancestry(group_dns = [], known_groups = [], set = [])
  group_dns.each do |group_dn|
- search = @ldap.search(base: group_dn, scope: Net::LDAP::SearchScope_BaseObject, attributes: ['memberof'])
- if !search.nil? && !search.first.nil?
- groups = search.first[:memberof] - known_groups
-  known_groups += groups
-  next_level, = _walk_group_ancestry(groups, known_groups)
-  set += next_level
- set += groups
-  known_groups += next_level
- end
+ groups = find_parent_groups(group_dn)
+ next unless groups
+
+ groups -= known_groups
+ known_groups += groups
+ next_level, = walk_group_ancestry(groups, known_groups) # new_known_groups
+
+ set += next_level + groups
+ known_groups += next_level
  end
+
  [set, known_groups]
  end
 
- def class_filter
- Net::LDAP::Filter.eq('objectclass', 'group')
+ # @param [String] group_dn
+ # @return [Array<String>]
+ def find_parent_groups(group_dn)
+ search = ldap.search(base: group_dn, scope: Net::LDAP::SearchScope_BaseObject, attributes: ['memberof'])
+
+ search = search.first if search
+ search ? search[:memberof] : nil
  end
 
  class UIDNotFoundException < LdapFluff::Error