From: Go MAEDA <maeda@farend.jp>
Date: Wed, 2 Dec 2020 13:56:15 +0000 (+0000)
Subject: Changes User.try_to_login to catch and log AuthSourceExceptions, and introduces User... 
X-Git-Tag: 4.2.0~271
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=1dcebf8ce02dffe34f8b1f08682119d8a4860032;p=redmine.git

Changes User.try_to_login to catch and log AuthSourceExceptions, and introduces User.try_to_login! replicating the original behavior (#34071).

Patch by Jens Krämer.


git-svn-id: http://svn.redmine.org/redmine/trunk@20547 e93f8b46-1217-0410-a6f0-8f06a7374b81
---

diff --git a/app/controllers/account_controller.rb b/app/controllers/account_controller.rb
index bc0792abb..9f46db64a 100644
--- a/app/controllers/account_controller.rb
+++ b/app/controllers/account_controller.rb
@@ -308,7 +308,7 @@ class AccountController < ApplicationController
   end
 
   def password_authentication
-    user = User.try_to_login(params[:username], params[:password], false)
+    user = User.try_to_login!(params[:username], params[:password], false)
 
     if user.nil?
       invalid_credentials
diff --git a/app/models/user.rb b/app/models/user.rb
index 2f88713ca..82d58bc0c 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -221,7 +221,17 @@ class User < Principal
   end
 
   # Returns the user that matches provided login and password, or nil
+  # AuthSource errors are caught, logged and nil is returned.
   def self.try_to_login(login, password, active_only=true)
+    try_to_login!(login, password, active_only)
+  rescue AuthSourceException => e
+    logger.error "An error occured when authenticating #{login}: #{e.message}"
+    nil
+  end
+
+  # Returns the user that matches provided login and password, or nil
+  # AuthSource errors are passed through.
+  def self.try_to_login!(login, password, active_only=true)
     login = login.to_s.strip
     password = password.to_s
 
diff --git a/test/unit/user_test.rb b/test/unit/user_test.rb
index 56e4c5ecf..3e25fee1d 100644
--- a/test/unit/user_test.rb
+++ b/test/unit/user_test.rb
@@ -695,13 +695,31 @@ class UserTest < ActiveSupport::TestCase
     assert_equal "ADMIN", user.login
   end
 
-  if ldap_configured?
-    test "#try_to_login using LDAP with failed connection to the LDAP server" do
-      auth_source = AuthSourceLdap.find(1)
-      AuthSource.any_instance.stubs(:initialize_ldap_con).raises(Net::LDAP::Error, 'Cannot connect')
+  test "#try_to_login! using LDAP with existing user and failed connection to the LDAP server" do
+    auth_source = AuthSourceLdap.find(1)
+    user = users(:users_001)
+    user.update_column :auth_source_id, auth_source.id
+    AuthSource.any_instance.stubs(:initialize_ldap_con).raises(Net::LDAP::Error, 'Cannot connect')
+    assert_raise(AuthSourceException){ User.try_to_login!('admin', 'admin') }
+  end
 
-      assert_nil User.try_to_login('edavis', 'wrong')
-    end
+  test "#try_to_login using LDAP with existing user and failed connection to the LDAP server" do
+    auth_source = AuthSourceLdap.find(1)
+    user = users(:users_001)
+    user.update_column :auth_source_id, auth_source.id
+    AuthSource.any_instance.stubs(:initialize_ldap_con).raises(Net::LDAP::Error, 'Cannot connect')
+    assert_nil User.try_to_login('admin', 'admin')
+  end
+
+  test "#try_to_login using LDAP with new user and failed connection to the LDAP server" do
+    auth_source = AuthSourceLdap.find(1)
+    auth_source.update onthefly_register: true
+    AuthSource.any_instance.stubs(:initialize_ldap_con).raises(Net::LDAP::Error, 'Cannot connect')
+
+    assert_nil User.try_to_login('edavis', 'wrong')
+  end
+
+  if ldap_configured?
 
     test "#try_to_login using LDAP" do
       assert_nil User.try_to_login('edavis', 'wrong')