Patch #19520

LDAP authentication exception handling

Added by Karel Pičman over 7 years ago. Updated over 7 years ago.

Status:NewStart date:
Priority:NormalDue date:
Assignee:-% Done:


Target version:-


In case of problems with a domain controler used for LDAP authentication Errno::ECONNRESET exception might be thrown. However, only AuthSourceException is caugth in the account controller. Therefore an Internal error is displayed to users after their logn attempt. I'd recomend either adding another exception handling or changing to a general exception as in the attached patch.

Errno::ECONNRESET (Connection reset by peer):
  app/models/auth_source_ldap.rb:178:in `get_user_dn'
  app/models/auth_source_ldap.rb:42:in `block in authenticate'
  app/models/auth_source_ldap.rb:98:in `block in with_timeout'
  app/models/auth_source_ldap.rb:97:in `with_timeout'
  app/models/auth_source_ldap.rb:41:in `authenticate'
  app/models/user.rb:271:in `check_password?'
  app/controllers/account_controller.rb:186:in `authenticate_user'
  app/controllers/account_controller.rb:40:in `login'

auth_exception.patch Magnifier - Account controller patch (490 Bytes) Karel Pičman, 2015-03-30 09:25


#1 Updated by Toshi MARUYAMA over 7 years ago

I don't like catching all exception in controller.
I think it is better catching in model.

diff --git a/app/models/auth_source.rb b/app/models/auth_source.rb
--- 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 AuthSourceConnectResetException < AuthSourceException; end
 class AuthSourceTimeoutException < AuthSourceException; end

 class AuthSource < ActiveRecord::Base
diff --git a/app/models/auth_source_ldap.rb b/app/models/auth_source_ldap.rb
--- a/app/models/auth_source_ldap.rb
+++ b/app/models/auth_source_ldap.rb
@@ -97,6 +97,8 @@ class AuthSourceLdap < AuthSource
     Timeout.timeout(timeout) do
       return yield
+  rescue Errno::ECONNRESET => e
+    raise
   rescue Timeout::Error => e

#2 Updated by Toshi MARUYAMA over 7 years ago

  • Subject changed from Authentication exception handling to LDAP authentication exception handling

Also available in: Atom PDF