Isolate EE LDAP library code in EE module (Part 1)

CHANGELOG-EE

@@ -3,6 +3,7 @@ Please view this file on the master branch, on stable branches it's out of date.
 v 8.10.0 (unreleased)
   - Rename Git Hooks to Push Rules
   - Fix EE keys fingerprint add index migration if came from CE
+  - Isolate EE LDAP library code in EE module (Part 1) !511
 
 v 8.9.5
   - Fix of quoted text in lock tooltip. !518

app/workers/ldap_group_sync_worker.rb

@@ -5,7 +5,7 @@ class LdapGroupSyncWorker
 
   def perform
     logger.info 'Started LDAP group sync'
-    Gitlab::LDAP::GroupSync.execute
+    EE::Gitlab::LDAP::GroupSync.execute
     logger.info 'Finished LDAP group sync'
   end
 end

lib/ee/gitlab/ldap/access_levels.rb

+module EE
+  module Gitlab
+    module LDAP
+      # Create a hash map of member DNs to access levels. The highest
+      # access level is retained in cases where `set` is called multiple times
+      # for the same DN.
+      class AccessLevels < Hash
+        def set(dns, to:)
+          dns.each do |dn|
+            current = self[dn]
+
+            # Keep the higher of the access values.
+            self[dn] = to if current.nil? || to > current
+          end
+        end
+      end
+    end
+  end
+end

lib/ee/gitlab/ldap/adapter.rb

+# LDAP connection adapter EE mixin
+#
+# This module is intended to encapsulate EE-specific adapter methods
+# and be included in the `Gitlab::LDAP::Adapter` class.
+module EE
+  module Gitlab
+    module LDAP
+      module Adapter
+        # Get LDAP groups from ou=Groups
+        #
+        # cn - filter groups by name
+        #
+        # Ex.
+        #   groups("dev*") # return all groups start with 'dev'
+        #
+        def groups(cn = "*", size = nil)
+          options = {
+            base: config.group_base,
+            filter: Net::LDAP::Filter.eq("cn", cn)
+          }
+
+          options.merge!(size: size) if size
+
+          ldap_search(options).map do |entry|
+            Group.new(entry, self)
+          end
+        end
+
+        def group(*args)
+          groups(*args).first
+        end
+
+        def dn_matches_filter?(dn, filter)
+          ldap_search(
+            base: dn,
+            filter: filter,
+            scope: Net::LDAP::SearchScope_BaseObject,
+            attributes: %w{dn}
+          ).any?
+        end
+      end
+    end
+  end
+end

lib/ee/gitlab/ldap/group.rb

+module EE
+  module Gitlab
+    module LDAP
+      class Group
+        attr_accessor :adapter
+
+        def self.find_by_cn(cn, adapter)
+          cn = Net::LDAP::Filter.escape(cn)
+          adapter.group(cn)
+        end
+
+        def initialize(entry, adapter=nil)
+          Rails.logger.debug { "Instantiating #{self.class.name} with LDIF:\n#{entry.to_ldif}" }
+          @entry = entry
+          @adapter = adapter
+        end
+
+        def active_directory?
+          adapter.config.active_directory
+        end
+
+        def cn
+          entry.cn.first
+        end
+
+        def name
+          cn
+        end
+
+        def path
+          name.parameterize
+        end
+
+        def memberuid?
+          entry.respond_to? :memberuid
+        end
+
+        def member_uids
+          entry.memberuid
+        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)
+          end
+
+          if (entry.respond_to? :member) && (entry.respond_to? :submember)
+            dns.concat(entry.member + entry.submember)
+          elsif entry.respond_to? :member
+            dns.concat(entry.member)
+          elsif entry.respond_to? :uniquemember
+            dns.concat(entry.uniquemember)
+          elsif entry.respond_to? :memberof
+            dns.concat(entry.memberof)
+          else
+            Rails.logger.warn("Could not find member DNs for LDAP group #{entry.inspect}")
+          end
+          dns.uniq
+        end
+
+        private
+
+        # We use the ActiveDirectory LDAP_MATCHING_RULE_IN_CHAIN matching rule; see
+        # http://msdn.microsoft.com/en-us/library/aa746475%28VS.85%29.aspx#code-snippet-5
+        def active_directory_recursive_memberof_filter
+          Net::LDAP::Filter.ex("memberOf:1.2.840.113556.1.4.1941", entry.dn)
+        end
+
+        def entry
+          @entry
+        end
+      end
+    end
+  end
+end

lib/gitlab/ldap/access_levels.rb

-module Gitlab
-  module LDAP
-    # Create a hash map of member DNs to access levels. The highest
-    # access level is retained in cases where `set` is called multiple times
-    # for the same DN.
-    class AccessLevels < Hash
-      def set(dns, to:)
-        dns.each do |dn|
-          current = self[dn]
-
-          # Keep the higher of the access values.
-          self[dn] = to if current.nil? || to > current
-        end
-      end
-    end
-  end
-end

lib/gitlab/ldap/adapter.rb

+# LDAP connection adapter
+#
+# Contains methods common to both GitLab CE and EE.
+# All EE methods should be in `EE::Gitlab::LDAP::Adapter` only.
 module Gitlab
   module LDAP
     class Adapter
+      include EE::Gitlab::LDAP::Adapter
+
       attr_reader :provider, :ldap
 
       def self.open(provider, &block)
@@ -22,30 +28,6 @@ module Gitlab
         Gitlab::LDAP::Config.new(provider)
       end
 
-      # Get LDAP groups from ou=Groups
-      #
-      # cn - filter groups by name
-      #
-      # Ex.
-      #   groups("dev*") # return all groups start with 'dev'
-      #
-      def groups(cn = "*", size = nil)
-        options = {
-          base: config.group_base,
-          filter: Net::LDAP::Filter.eq("cn", cn)
-        }
-
-        options.merge!(size: size) if size
-
-        ldap_search(options).map do |entry|
-          Gitlab::LDAP::Group.new(entry, self)
-        end
-      end
-
-      def group(*args)
-        groups(*args).first
-      end
-
       def users(field, value, limit = nil)
         if field.to_sym == :dn
           options = {
@@ -86,13 +68,6 @@ module Gitlab
         users(*args).first
       end
 
-      def dn_matches_filter?(dn, filter)
-        ldap_search(base: dn,
-                    filter: filter,
-                    scope: Net::LDAP::SearchScope_BaseObject,
-                    attributes: %w{dn}).any?
-      end
-
       def dns_for_filter(filter)
         ldap_search(
           base: config.base,

lib/gitlab/ldap/group.rb

-module Gitlab
-  module LDAP
-    class Group
-      attr_accessor :adapter
-
-      def self.find_by_cn(cn, adapter)
-        cn = Net::LDAP::Filter.escape(cn)
-        adapter.group(cn)
-      end
-
-      def initialize(entry, adapter=nil)
-        Rails.logger.debug { "Instantiating #{self.class.name} with LDIF:\n#{entry.to_ldif}" }
-        @entry = entry
-        @adapter = adapter
-      end
-
-      def active_directory?
-        adapter.config.active_directory
-      end
-
-      def cn
-        entry.cn.first
-      end
-
-      def name
-        cn
-      end
-
-      def path
-        name.parameterize
-      end
-
-      def memberuid?
-        entry.respond_to? :memberuid
-      end
-
-      def member_uids
-        entry.memberuid
-      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)
-        end
-
-        if (entry.respond_to? :member) && (entry.respond_to? :submember)
-          dns.concat(entry.member + entry.submember)
-        elsif entry.respond_to? :member
-          dns.concat(entry.member)
-        elsif entry.respond_to? :uniquemember
-          dns.concat(entry.uniquemember)
-        elsif entry.respond_to? :memberof
-          dns.concat(entry.memberof)
-        else
-          Rails.logger.warn("Could not find member DNs for LDAP group #{entry.inspect}")
-        end
-        dns.uniq
-      end
-
-      private
-
-      # We use the ActiveDirectory LDAP_MATCHING_RULE_IN_CHAIN matching rule; see
-      # http://msdn.microsoft.com/en-us/library/aa746475%28VS.85%29.aspx#code-snippet-5
-      def active_directory_recursive_memberof_filter
-        Net::LDAP::Filter.ex("memberOf:1.2.840.113556.1.4.1941", entry.dn)
-      end
-
-      def entry
-        @entry
-      end
-    end
-  end
-end

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

 require 'spec_helper'
 
-describe Gitlab::LDAP::AccessLevels, lib: true do
+describe EE::Gitlab::LDAP::AccessLevels, lib: true do
   describe '#set' do
-    let(:access_levels) { Gitlab::LDAP::AccessLevels.new }
+    let(:access_levels) { described_class.new }
     let(:dns) do
       %w(
       uid=johndoe,ou=users,dc=example,dc=com

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' }
+
+  it { is_expected.to include_module(EE::Gitlab::LDAP::Adapter) }
+end

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

 require 'spec_helper'
 
-describe Gitlab::LDAP::Group, lib: true do
+describe EE::Gitlab::LDAP::Group, lib: true do
   describe '#member_dns' do
     def ldif
       Net::LDAP::Entry.from_single_ldif_string(