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

7 files changed, 50 insertions, 9 deletions

diff --git a/app/models/auth_source.rb b/app/models/auth_source.rb
index ec7d3dad9..bfec5f20c 100644
--- a/app/models/auth_source.rb
+++ b/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
diff --git a/app/models/auth_source_ldap.rb b/app/models/auth_source_ldap.rb
index adfb66e8b..d49e00dc6 100644
--- a/app/models/auth_source_ldap.rb
+++ b/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)
-      logger.debug "Authentication successful for '#{login}'" if logger && logger.debug?
-      return attrs.except(:dn)
+    with_timeout do
+      attrs = get_user_dn(login, password)
+      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)
-    ldap_con.open { }
-  rescue  Net::LDAP::LdapError => e
-    raise "LdapError: " + e.message
+    with_timeout do
+      ldap_con = initialize_ldap_con(self.account, self.account_password)
+      ldap_con.open { }
+    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)
diff --git a/app/views/auth_sources/_form_auth_source_ldap.html.erb b/app/views/auth_sources/_form_auth_source_ldap.html.erb
index 3ddf43a9a..9a6afb36b 100644
--- a/app/views/auth_sources/_form_auth_source_ldap.html.erb
+++ b/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>
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 03a850677..4609cfba3 100644
--- a/config/locales/en.yml
+++ b/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
diff --git a/config/locales/fr.yml b/config/locales/fr.yml
index b341da627..ca06fac06 100644
--- a/config/locales/fr.yml
+++ b/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
diff --git a/db/migrate/20120707064544_add_auth_sources_timeout.rb b/db/migrate/20120707064544_add_auth_sources_timeout.rb
new file mode 100644
index 000000000..40f467810
--- /dev/null
+++ b/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
diff --git a/test/unit/auth_source_ldap_test.rb b/test/unit/auth_source_ldap_test.rb
index 783ea6faf..60558f798 100644
--- a/test/unit/auth_source_ldap_test.rb
+++ b/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