Merge branch 'ldap_connection_reuse' into 'master'

Ldap Connection Reuse

CHANGELOG-EE

@@ -1,3 +1,6 @@
+v 6.7.0
+  - Improve LDAP sign-in speed by reusing connections
+
 v 6.5.0
   - Add reset permissions button to Group#members page
 

app/controllers/application_controller.rb

@@ -183,14 +183,16 @@ class ApplicationController < ActionController::Base
 
   def ldap_security_check
     if current_user && current_user.ldap_user? && current_user.requires_ldap_check?
-      if gitlab_ldap_access.allowed?(current_user)
-        gitlab_ldap_access.update_permissions(current_user)
-        current_user.last_credential_check_at = Time.now
-        current_user.save
-      else
-        sign_out current_user
-        flash[:alert] = "Access denied for your LDAP account."
-        redirect_to new_user_session_path
+      gitlab_ldap_access do |access|
+        if access.allowed?(current_user)
+          access.update_permissions(current_user)
+          current_user.last_credential_check_at = Time.now
+          current_user.save
+        else
+          sign_out current_user
+          flash[:alert] = "Access denied for your LDAP account."
+          redirect_to new_user_session_path
+        end
       end
     end
   end
@@ -200,8 +202,8 @@ class ApplicationController < ActionController::Base
     @event_filter ||= EventFilter.new(filters)
   end
 
-  def gitlab_ldap_access
-    Gitlab::LDAP::Access.new
+  def gitlab_ldap_access(&block)
+    Gitlab::LDAP::Access.open { |access| block.call(access) }
   end
 
   # JSON for infinite scroll via Pager object

app/controllers/omniauth_callbacks_controller.rb

@@ -21,12 +21,14 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
     @user = Gitlab::LDAP::User.find_or_create(oauth)
     @user.remember_me = true if @user.persisted?
 
-    if gitlab_ldap_access.allowed?(@user)
-      gitlab_ldap_access.update_permissions(@user)
-      sign_in_and_redirect(@user)
-    else
-      flash[:alert] = "Access denied for your LDAP account."
-      redirect_to new_user_session_path
+    gitlab_ldap_access do |access|
+      if access.allowed?(@user)
+        access.update_permissions(@user)
+        sign_in_and_redirect(@user)
+      else
+        flash[:alert] = "Access denied for your LDAP account."
+        redirect_to new_user_session_path
+      end
     end
   end
 

lib/gitlab/ldap/access.rb

@@ -7,8 +7,20 @@
 module Gitlab
   module LDAP
     class Access
+      attr_reader :adapter
+
+      def self.open(&block)
+        Gitlab::LDAP::Adapter.open do |adapter|
+          block.call(self.new(adapter))
+        end
+      end
+
+      def initialize(adapter=nil)
+        @adapter = adapter
+      end
+
       def allowed?(user)
-        !!Gitlab::LDAP::Person.find_by_dn(user.extern_uid)
+        !!Gitlab::LDAP::Person.find_by_dn(user.extern_uid, adapter)
       rescue
         false
       end
@@ -19,13 +31,13 @@ module Gitlab
         return true unless Gitlab.config.ldap['group_base'].present?
 
         # Get LDAP user entry
-        ldap_user = Gitlab::LDAP::Person.find_by_dn(user.extern_uid)
+        ldap_user = Gitlab::LDAP::Person.find_by_dn(user.extern_uid, adapter)
 
         # Get all GitLab groups with activated LDAP
         groups = ::Group.where('ldap_cn IS NOT NULL')
 
         # Get LDAP groups based on cn from GitLab groups
-        ldap_groups = groups.pluck(:ldap_cn).map { |cn| Gitlab::LDAP::Group.find_by_cn(cn) }
+        ldap_groups = groups.pluck(:ldap_cn).map { |cn| Gitlab::LDAP::Group.find_by_cn(cn, adapter) }
         ldap_groups = ldap_groups.compact.uniq
 
         # Iterate over ldap groups and check user membership

lib/gitlab/ldap/adapter.rb

@@ -9,7 +9,17 @@ module Gitlab
     class Adapter
       attr_reader :ldap
 
-      def initialize
+      def self.open(&block)
+        Net::LDAP.open(adapter_options) do |ldap|
+          block.call(self.new(ldap))
+        end
+      end
+
+      def self.config
+        Gitlab.config.ldap
+      end
+
+      def self.adapter_options
         encryption = config['method'].to_s == 'ssl' ? :simple_tls : nil
 
         options = {
@@ -29,8 +39,12 @@ module Gitlab
         if config['password'] || config['bind_dn']
           options.merge!(auth_options)
         end
+        options
+      end
 
-        @ldap = Net::LDAP.new(options)
+
+      def initialize(ldap=nil)
+        @ldap = ldap || Net::LDAP.new(self.class.adapter_options)
       end
 
       # Get LDAP groups from ou=Groups
@@ -95,7 +109,7 @@ module Gitlab
       private
 
       def config
-        @config ||= Gitlab.config.ldap
+        @config ||= self.class.config
       end
     end
   end

lib/gitlab/ldap/group.rb

@@ -7,8 +7,9 @@
 module Gitlab
   module LDAP
     class Group
-      def self.find_by_cn(cn)
-        Gitlab::LDAP::Adapter.new.group(cn)
+      def self.find_by_cn(cn, adapter=nil)
+        adapter ||= Gitlab::LDAP::Adapter.new
+        adapter.group(cn)
       end
 
       def initialize(entry)

lib/gitlab/ldap/person.rb

@@ -7,12 +7,14 @@
 module Gitlab
   module LDAP
     class Person
-      def self.find_by_uid(uid)
-        Gitlab::LDAP::Adapter.new.user(config.uid, uid)
+      def self.find_by_uid(uid, adapter=nil)
+        adapter ||= Gitlab::LDAP::Adapter.new
+        adapter.user(config.uid, uid)
       end
 
-      def self.find_by_dn(dn)
-        Gitlab::LDAP::Adapter.new.user('dn', dn)
+      def self.find_by_dn(dn, adapter=nil)
+        adapter ||= Gitlab::LDAP::Adapter.new
+        adapter.user('dn', dn)
       end
 
       def initialize(entry)