Add better LDAP connection handling

4 files changed, 47 insertions, 8 deletions

diff --git a/lib/gitlab/auth/ldap/access.rb b/lib/gitlab/auth/ldap/access.rb
index 77c0ddc2d48..34286900e72 100644
--- a/lib/gitlab/auth/ldap/access.rb
+++ b/lib/gitlab/auth/ldap/access.rb
@@ -52,6 +52,8 @@ module Gitlab
             block_user(user, 'does not exist anymore')
             false
           end
+        rescue LDAPConnectionError
+          false
         end
 
         def adapter
diff --git a/lib/gitlab/auth/ldap/adapter.rb b/lib/gitlab/auth/ldap/adapter.rb
index caf2d18c668..82ff1e77e5c 100644
--- a/lib/gitlab/auth/ldap/adapter.rb
+++ b/lib/gitlab/auth/ldap/adapter.rb
@@ -2,6 +2,9 @@ module Gitlab
   module Auth
     module LDAP
       class Adapter
+        SEARCH_RETRY_FACTOR = [1, 1, 2, 3].freeze
+        MAX_SEARCH_RETRIES = Rails.env.test? ? 1 : SEARCH_RETRY_FACTOR.size.freeze
+
         attr_reader :provider, :ldap
 
         def self.open(provider, &block)
@@ -16,7 +19,7 @@ module Gitlab
 
         def initialize(provider, ldap = nil)
           @provider = provider
-          @ldap = ldap || Net::LDAP.new(config.adapter_options)
+          @ldap = ldap || renew_connection_adapter
         end
 
         def config
@@ -47,8 +50,10 @@ module Gitlab
         end
 
         def ldap_search(*args)
+          retries ||= 0
+
           # Net::LDAP's `time` argument doesn't work. Use Ruby `Timeout` instead.
-          Timeout.timeout(config.timeout) do
+          Timeout.timeout(timeout_time(retries)) do
             results = ldap.search(*args)
 
             if results.nil?
@@ -63,16 +68,26 @@ module Gitlab
               results
             end
           end
-        rescue Net::LDAP::Error => error
-          Rails.logger.warn("LDAP search raised exception #{error.class}: #{error.message}")
-          []
-        rescue Timeout::Error
-          Rails.logger.warn("LDAP search timed out after #{config.timeout} seconds")
-          []
+        rescue Net::LDAP::Error, Timeout::Error => error
+          retries += 1
+          error_message = connection_error_message(error)
+
+          Rails.logger.warn(error_message)
+
+          if retries < MAX_SEARCH_RETRIES
+            renew_connection_adapter
+            retry
+          else
+            raise LDAPConnectionError, error_message
+          end
         end
 
         private
 
+        def timeout_time(retry_number)
+          SEARCH_RETRY_FACTOR[retry_number] * config.timeout
+        end
+
         def user_options(fields, value, limit)
           options = {
             attributes: Gitlab::Auth::LDAP::Person.ldap_attributes(config),
@@ -104,6 +119,18 @@ module Gitlab
             filter
           end
         end
+
+        def connection_error_message(exception)
+          if exception.is_a?(Timeout::Error)
+            "LDAP search timed out after #{config.timeout} seconds"
+          else
+            "LDAP search raised exception #{exception.class}: #{exception.message}"
+          end
+        end
+
+        def renew_connection_adapter
+          @ldap = Net::LDAP.new(config.adapter_options)
+        end
       end
     end
   end
diff --git a/lib/gitlab/auth/ldap/ldap_connection_error.rb b/lib/gitlab/auth/ldap/ldap_connection_error.rb
new file mode 100644
index 00000000000..ef0a695742b
--- /dev/null
+++ b/lib/gitlab/auth/ldap/ldap_connection_error.rb
@@ -0,0 +1,7 @@
+module Gitlab
+  module Auth
+    module LDAP
+      LDAPConnectionError = Class.new(StandardError)
+    end
+  end
+end
diff --git a/lib/gitlab/auth/o_auth/user.rb b/lib/gitlab/auth/o_auth/user.rb
index b6a96081278..d0c6b0386ba 100644
--- a/lib/gitlab/auth/o_auth/user.rb
+++ b/lib/gitlab/auth/o_auth/user.rb
@@ -124,6 +124,9 @@ module Gitlab
           Gitlab::Auth::LDAP::Person.find_by_uid(auth_hash.uid, adapter) ||
             Gitlab::Auth::LDAP::Person.find_by_email(auth_hash.uid, adapter) ||
             Gitlab::Auth::LDAP::Person.find_by_dn(auth_hash.uid, adapter)
+
+        rescue Gitlab::Auth::LDAP::LDAPConnectionError
+          nil
         end
 
         def ldap_config