Issue #3410419 by catch: Only clear flood attempts when necessary during user login

core/modules/user/src/Form/UserLoginForm.php

@@ -209,11 +209,22 @@ public function validateAuthentication(array &$form, FormStateInterface $form_st
         }
         $form_state->set('flood_control_user_identifier', $identifier);
 
-        // Don't allow login if the limit for this user has been reached.
-        // Default is to allow 5 failed attempts every 6 hours.
-        if (!$this->userFloodControl->isAllowed('user.failed_login_user', $flood_config->get('user_limit'), $flood_config->get('user_window'), $identifier)) {
-          $form_state->set('flood_control_triggered', 'user');
-          return;
+        // If there are zero flood records for this user, then we don't need to
+        // clear any failed login attempts after a successful login, so check
+        // for this case first before checking the actual flood limit and store
+        // the result in form state.
+        if (!$this->userFloodControl->isAllowed('user.failed_login_user', 1, $flood_config->get('user_window'), $identifier)) {
+          // Now check the actual limit for the user. Default is to allow 5
+          // failed attempts every 6 hours. This means we check the flood table
+          // twice if flood control has already been triggered by a previous
+          // login attempt, bu this should be the less common case.
+          if (!$this->userFloodControl->isAllowed('user.failed_login_user', $flood_config->get('user_limit'), $flood_config->get('user_window'), $identifier)) {
+            $form_state->set('flood_control_triggered', 'user');
+            return;
+          }
+        }
+        else {
+          $form_state->set('flood_control_skip_clear', 'user');
         }
       }
       // We are not limited by flood control, so try to authenticate.
@@ -263,7 +274,7 @@ public function validateFinal(array &$form, FormStateInterface $form_state) {
         }
       }
     }
-    elseif ($flood_control_user_identifier = $form_state->get('flood_control_user_identifier')) {
+    elseif (!$form_state->get('flood_control_skip_clear') && $flood_control_user_identifier = $form_state->get('flood_control_user_identifier')) {
       // Clear past failures for this user so as not to block a user who might
       // log in and out more than once in an hour.
       $this->userFloodControl->clear('user.failed_login_user', $flood_control_user_identifier);

core/profiles/standard/tests/src/FunctionalJavascript/StandardPerformanceTest.php

@@ -131,8 +131,8 @@ public function testLogin(): void {
     // random test failures, assert greater than equal the highest and lowest
     // number of queries observed during test runs.
     // See https://www.drupal.org/project/drupal/issues/3402610
-    $this->assertLessThanOrEqual(39, $performance_data->getQueryCount());
-    $this->assertGreaterThanOrEqual(39, $performance_data->getQueryCount());
+    $this->assertLessThanOrEqual(41, $performance_data->getQueryCount());
+    $this->assertGreaterThanOrEqual(38, $performance_data->getQueryCount());
     $this->assertSame(28, $performance_data->getCacheGetCount());
     $this->assertLessThanOrEqual(2, $performance_data->getCacheSetCount());
     $this->assertGreaterThanOrEqual(1, $performance_data->getCacheSetCount());
@@ -163,8 +163,8 @@ public function testLoginBlock(): void {
     $performance_data = $this->collectPerformanceData(function () use ($account) {
       $this->submitLoginForm($account);
     });
-    $this->assertLessThanOrEqual(50, $performance_data->getQueryCount());
-    $this->assertGreaterThanOrEqual(48, $performance_data->getQueryCount());
+    $this->assertLessThanOrEqual(51, $performance_data->getQueryCount());
+    $this->assertGreaterThanOrEqual(47, $performance_data->getQueryCount());
     // This test observes a variable number of cache operations, so to avoid random
     // test failures, assert greater than equal the highest and lowest number
     // observed during test runs.

[Fabian Franz]

nit - bu => but