Merge branch 'ee_adapter_mixin' into 'master'

Move most of the EE-specific LDAP code to EE-specific classes/modules

This will prevent or reduce merge errors. This doesn't include *all* of the EE-specific LDAP code, but it's the low-hanging fruit. I will get the less-straightforward stuff in a subsequent merge request to avoid making this one unbearably large. 

## Why is this necessary?

I'm starting work on some of the priority LDAP features/fixes. I was about to start stuffing additional EE-specific methods into `Gitlab::LDAP::Adapter` and it felt like this could be a CE-EE merge danger zone. This seemed like a potentially quick fix to that worry so I thought I'd take a moment to see if it was sane. 

@DouweM and I talked briefly about this. It seems to be the way we want to go in the future anyway so this gets the ball rolling. 

If this goes through and everyone is happy, I'll also create a subsequent merge request to add some development docs about organizing EE code. 

FYI @vsizov @jacobvosmaer-gitlab 

See merge request !511

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(