Adds a configurable timeout for LDAP authentication (#8978).

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@9931 e93f8b46-1217-0410-a6f0-8f06a7374b81

app/models/auth_source.rb

@@ -18,6 +18,7 @@
 # Generic exception for when the AuthSource can not be reached
 # (eg. can not connect to the LDAP)
 class AuthSourceException < Exception; end
+class AuthSourceTimeoutException < AuthSourceException; end
 
 class AuthSource < ActiveRecord::Base
   include Redmine::SubclassFactory

app/models/auth_source_ldap.rb

@@ -18,6 +18,7 @@
 require 'iconv'
 require 'net/ldap'
 require 'net/ldap/dn'
+require 'timeout'
 
 class AuthSourceLdap < AuthSource
   validates_presence_of :host, :port, :attr_login
@@ -25,6 +26,7 @@ class AuthSourceLdap < AuthSource
   validates_length_of :account, :account_password, :base_dn, :filter, :maximum => 255, :allow_blank => true
   validates_length_of :attr_login, :attr_firstname, :attr_lastname, :attr_mail, :maximum => 30, :allow_nil => true
   validates_numericality_of :port, :only_integer => true
+  validates_numericality_of :timeout, :only_integer => true, :allow_blank => true
   validate :validate_filter
 
   before_validation :strip_ldap_attributes
@@ -44,22 +46,26 @@ class AuthSourceLdap < AuthSource
 
   def authenticate(login, password)
     return nil if login.blank? || password.blank?
-    attrs = get_user_dn(login, password)
 
-    if attrs && attrs[:dn] && authenticate_dn(attrs[:dn], password)
+    with_timeout do
-      logger.debug "Authentication successful for '#{login}'" if logger && logger.debug?
+      attrs = get_user_dn(login, password)
-      return attrs.except(:dn)
+      if attrs && attrs[:dn] && authenticate_dn(attrs[:dn], password)
+        logger.debug "Authentication successful for '#{login}'" if logger && logger.debug?
+        return attrs.except(:dn)
+      end
     end
-  rescue  Net::LDAP::LdapError => e
+  rescue Net::LDAP::LdapError => e
     raise AuthSourceException.new(e.message)
   end
 
   # test the connection to the LDAP
   def test_connection
-    ldap_con = initialize_ldap_con(self.account, self.account_password)
+    with_timeout do
-    ldap_con.open { }
+      ldap_con = initialize_ldap_con(self.account, self.account_password)
-  rescue  Net::LDAP::LdapError => e
+      ldap_con.open { }
-    raise "LdapError: " + e.message
+    end
+  rescue Net::LDAP::LdapError => e
+    raise AuthSourceException.new(e.message)
   end
 
   def auth_method_name
@@ -68,6 +74,16 @@ class AuthSourceLdap < AuthSource
 
   private
 
+  def with_timeout(&block)
+    timeout = self.timeout
+    timeout = 20 unless timeout && timeout > 0
+    Timeout.timeout(timeout) do
+      return yield
+    end
+  rescue Timeout::Error => e
+    raise AuthSourceTimeoutException.new(e.message)
+  end
+
   def ldap_filter
     if filter.present?
       Net::LDAP::Filter.construct(filter)

app/views/auth_sources/_form_auth_source_ldap.html.erb

@@ -26,6 +26,9 @@
 <p><label for="auth_source_custom_filter"><%=l(:field_ldap_filter)%></label>
 <%= text_field 'auth_source', 'filter', :size => 60 %></p>
 
+<p><label for="auth_source_timeout"><%=l(:field_timeout)%></label>
+<%= text_field 'auth_source', 'timeout', :size => 4 %></p>
+
 <p><label for="auth_source_onthefly_register"><%=l(:field_onthefly)%></label>
 <%= check_box 'auth_source', 'onthefly_register' %></p>
 </div>

config/locales/en.yml

@@ -329,6 +329,7 @@ en:
   field_multiple: Multiple values
   field_ldap_filter: LDAP filter
   field_core_fields: Standard fields
+  field_timeout: "Timeout (in seconds)"
 
   setting_app_title: Application title
   setting_app_subtitle: Application subtitle

config/locales/fr.yml

@@ -328,6 +328,7 @@ fr:
   field_multiple: Valeurs multiples
   field_ldap_filter: Filtre LDAP
   field_core_fields: Champs standards
+  field_timeout: "Timeout (en secondes)"
 
   setting_app_title: Titre de l'application
   setting_app_subtitle: Sous-titre de l'application

db/migrate/20120707064544_add_auth_sources_timeout.rb

@@ -0,0 +1,9 @@
+class AddAuthSourcesTimeout < ActiveRecord::Migration
+  def up
+    add_column :auth_sources, :timeout, :integer
+  end
+
+  def self.down
+    remove_column :auth_sources, :timeout
+  end
+end

test/unit/auth_source_ldap_test.rb

@@ -114,6 +114,16 @@ class AuthSourceLdapTest < ActiveSupport::TestCase
         end
       end
     end
+
+    def test_authenticate_should_timeout
+      auth_source = AuthSourceLdap.find(1)
+      auth_source.timeout = 1
+      def auth_source.initialize_ldap_con(*args); sleep(5); end
+
+      assert_raise AuthSourceTimeoutException do
+        auth_source.authenticate 'example1', '123456'
+      end
+    end
   else
     puts '(Test LDAP server not configured)'
   end