From 3b207ee77caf47953fb436099b384099c97ff16b Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Sat, 7 Jul 2012 09:36:04 +0000 Subject: [PATCH] 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 | 1 + app/models/auth_source_ldap.rb | 34 ++++++++++++++----- .../_form_auth_source_ldap.html.erb | 3 ++ config/locales/en.yml | 1 + config/locales/fr.yml | 1 + ...20120707064544_add_auth_sources_timeout.rb | 9 +++++ test/unit/auth_source_ldap_test.rb | 10 ++++++ 7 files changed, 50 insertions(+), 9 deletions(-) create mode 100644 db/migrate/20120707064544_add_auth_sources_timeout.rb 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 @@

<%= text_field 'auth_source', 'filter', :size => 60 %>

+

+<%= text_field 'auth_source', 'timeout', :size => 4 %>

+

<%= check_box 'auth_source', 'onthefly_register' %>

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 -- 2.39.5