From 22cd7ccf3e6d8ab27c1c36c6303fa5c89bb69ebc Mon Sep 17 00:00:00 2001
From: Martin Gruner <mg@zammad.com>
Date: Thu, 30 Jun 2022 10:57:06 +0200
Subject: [PATCH] Maintenance: Refactored handling of login_failed counter.

---
 lib/auth.rb                   | 17 ++++++++++-------
 spec/lib/auth_spec.rb         |  7 ++++++-
 spec/system/js/q_unit_spec.rb |  2 +-
 3 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/lib/auth.rb b/lib/auth.rb
index 4d48128208a9..a1b4ae30836e 100644
--- a/lib/auth.rb
+++ b/lib/auth.rb
@@ -8,6 +8,8 @@ class Auth
 
   attr_accessor :increase_login_failed_attempts
 
+  BRUTE_FORCE_SLEEP = 1.second
+
   # Initializes a Auth object for the given user.
   #
   # @param username [String] the user name for the user object which needs an authentication.
@@ -28,20 +30,21 @@ def initialize(username, password)
   #
   # @return [Boolean] true if the user was authenticated, otherwise false.
   def valid?
-    if !auth_user || !auth_user.can_login?
-      avoid_brute_force_attack
+    # Wrap in a lock to synchronize concurrent requests.
+    validated = auth_user&.user&.with_lock do
+      next false if !auth_user.can_login?
+      next true if backends.valid?
 
-      return false
+      auth_user.increase_login_failed if increase_login_failed_attempts
+      false
     end
 
-    if backends.valid?
+    if validated
       auth_user.update_last_login
       return true
     end
 
     avoid_brute_force_attack
-
-    auth_user.increase_login_failed if increase_login_failed_attempts
     false
   end
 
@@ -49,7 +52,7 @@ def valid?
 
   # Sleep for a second to avoid brute force attacks.
   def avoid_brute_force_attack
-    sleep 1
+    sleep BRUTE_FORCE_SLEEP
   end
 
   def backends
diff --git a/spec/lib/auth_spec.rb b/spec/lib/auth_spec.rb
index 303b983df102..726cdcda827a 100644
--- a/spec/lib/auth_spec.rb
+++ b/spec/lib/auth_spec.rb
@@ -7,6 +7,10 @@
   let(:user)     { create(:user, password: password) }
   let(:instance) { described_class.new(user.login, password) }
 
+  before do
+    stub_const('Auth::BRUTE_FORCE_SLEEP', 0)
+  end
+
   describe '.valid?' do
     it 'responds to valid?' do
       expect(instance).to respond_to(:valid?)
@@ -83,7 +87,8 @@
         it 'failed login avoids brute force attack' do
           allow(instance).to receive(:sleep)
           instance.valid?
-          expect(instance).to have_received(:sleep).with(1)
+          # sleep receives the stubbed value.
+          expect(instance).to have_received(:sleep).with(0)
         end
       end
 
diff --git a/spec/system/js/q_unit_spec.rb b/spec/system/js/q_unit_spec.rb
index 530583a46b80..7380508e8d83 100644
--- a/spec/system/js/q_unit_spec.rb
+++ b/spec/system/js/q_unit_spec.rb
@@ -2,7 +2,7 @@
 
 require 'rails_helper'
 
-RSpec.describe 'QUnit', type: :system, authenticated_as: false, set_up: true, websocket: false, time_zone: 'Europe/London' do
+RSpec.describe 'QUnit', type: :system, authenticated_as: false, set_up: true, time_zone: 'Europe/London' do
   matcher :pass_qunit_test do
     match do
       actual.has_css?('.total', wait: 120)