Use dn lookup for person. Collect group members dn when check for membership

lib/gitlab/ldap/access.rb

@@ -12,7 +12,7 @@ module Gitlab
         # if instance does not use group_base setting
         return true unless Gitlab.config.ldap['group_base'].present?
 
-        ldap_user = Gitlab::LDAP::Person.find(user.extern_uid)
+        ldap_user = Gitlab::LDAP::Person.find_by_dn(user.extern_uid)
         ldap_groups = ldap_user.groups
         ldap_groups_cn = ldap_groups.map(&:name)
         groups = ::Group.where(ldap_cn: ldap_groups_cn)

lib/gitlab/ldap/adapter.rb

@@ -51,18 +51,18 @@ module Gitlab
         end
       end
 
-      def users(opts)
-        if opts.respond_to? :key?
-          key, uid = opts.keys.first, opts.values.first
+      def users(field, value)
+        if field.to_sym == :dn
+          options = {
+            base: value
+          }
         else
-          key, uid = config.uid, opts || "*"
+          options = {
+            base: config['base'],
+            filter: Net::LDAP::Filter.eq(field, value)
+          }
         end
 
-        options = {
-          base: config['base'],
-          filter: Net::LDAP::Filter.eq(key, uid)
-        }
-
         entries = ldap.search(options).select do |entry|
           entry.respond_to? config.uid
         end
@@ -72,8 +72,8 @@ module Gitlab
         end
       end
 
-      def user(opts)
-        users(opts).first
+      def user(*args)
+        users(*args).first
       end
 
       private

lib/gitlab/ldap/group.rb

@@ -19,24 +19,14 @@ module Gitlab
         name.parameterize
       end
 
-      def members
-        member_uids.map do |opts|
-          adapter.user(opts)
-        end.compact
+      def memberuid?
+        entry.respond_to? :memberuid
       end
 
       def member_uids
-        if entry.respond_to? :memberuid
-          entry.memberuid
-        else
-          member_dns.map do |dn|
-            dn_to_opts(dn)
-          end
-        end.compact
+        entry.memberuid
       end
 
-      private
-
       def member_dns
         if entry.respond_to? :member
           entry.member
@@ -49,6 +39,8 @@ module Gitlab
         end
       end
 
+      private
+
       def entry
         @entry
       end
@@ -56,10 +48,6 @@ module Gitlab
       def adapter
         @adapter ||= Gitlab::LDAP::Adapter.new
       end
-
-      def dn_to_opts(dn)
-        dn.split(",").first.split("=")
-      end
     end
   end
 end

lib/gitlab/ldap/person.rb

@@ -7,10 +7,12 @@
 module Gitlab
   module LDAP
     class Person
-      def self.find(user_uid)
-        id = user_uid.split(",").first
-        key, value = id.split("=")
-        Gitlab::LDAP::Adapter.new.user(key => value)
+      def self.find_by_uid(uid)
+        Gitlab::LDAP::Adapter.new.user(config.uid, uid)
+      end
+
+      def self.find_by_dn(dn)
+        Gitlab::LDAP::Adapter.new.user('dn', dn)
       end
 
       def initialize(entry)
@@ -29,9 +31,17 @@ module Gitlab
         uid
       end
 
+      def dn
+        entry.dn
+      end
+
       def groups
         adapter.groups.select do |group|
-          group.member_uids.include?(uid)
+          if group.memberuid?
+            group.member_uids.include?(uid)
+          else
+            group.member_dns.include?(dn)
+          end
         end
       end