Verified Commit 595dce06 authored by Dave Long's avatar Dave Long
Browse files

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

(cherry picked from commit 8e5acc67)
parent dd59d521

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

+17 −6
Original line number Diff line number Diff line
@@ -209,13 +209,24 @@ 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 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.

      // Store $uid in form state as a flag for self::validateFinal().

      $uid = $this->userAuth->authenticate($form_state->getValue('name'), $password);
@@ -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

+4 −4
Original line number Diff line number Diff line
@@ -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.