Verified Commit 589b8fe8 authored by Alex Pott's avatar Alex Pott
Browse files

Issue #3410582 by catch, heddn, Prashant.c, smustgrave, claudiu.cristea,... 

Issue #3410582 by catch, heddn, Prashant.c, smustgrave, claudiu.cristea, quietone, longwave: Optimize user logins by avoiding duplicate entity queries
parent 47b08da6

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

+17 −9
Original line number Diff line number Diff line
@@ -132,7 +132,6 @@ public function buildForm(array $form, FormStateInterface $form_state) { 
    $form['actions'] = ['#type' => 'actions'];

    $form['actions']['submit'] = ['#type' => 'submit', '#value' => $this->t('Log in')];



    $form['#validate'][] = '::validateName';

    $form['#validate'][] = '::validateAuthentication';

    $form['#validate'][] = '::validateFinal';
@@ -145,8 +144,8 @@ public function buildForm(array $form, FormStateInterface $form_state) { 
   * {@inheritdoc}

   */

  public function submitForm(array &$form, FormStateInterface $form_state) {



    if (empty($uid = $form_state->get('uid'))) {

    $uid = $form_state->get('uid');

    if (!$uid) {

      return;

    }

    $account = $this->userStorage->load($uid);
@@ -167,8 +166,12 @@ public function submitForm(array &$form, FormStateInterface $form_state) { 


  /**

   * Sets an error if supplied username has been blocked.

   *

   * @deprecated in drupal:10.3.0 and is removed from drupal:11.0.0. There is no replacement.

   * @see https://www.drupal.org/node/3410706

   */

  public function validateName(array &$form, FormStateInterface $form_state) {

    @trigger_error(__METHOD__ . ' is deprecated in drupal:10.3.0 and is removed from drupal:11.0.0. There is no replacement. See https://www.drupal.org/node/3410706', E_USER_DEPRECATED);

    if (!$form_state->isValueEmpty('name') && user_is_blocked($form_state->getValue('name'))) {

      // Blocked in user administration.

      $form_state->setErrorByName('name', $this->t('The username %name has not been activated or is blocked.', ['%name' => $form_state->getValue('name')]));
@@ -183,6 +186,7 @@ public function validateName(array &$form, FormStateInterface $form_state) { 
  public function validateAuthentication(array &$form, FormStateInterface $form_state) {

    $password = trim($form_state->getValue('pass'));

    $flood_config = $this->config('user.flood');

    $account = FALSE;

    if (!$form_state->isValueEmpty('name') && strlen($password) > 0) {

      // Do not allow any login from the current user's IP if the limit has been

      // reached. Default is 50 failed attempts allowed in one hour. This is
@@ -193,9 +197,12 @@ public function validateAuthentication(array &$form, FormStateInterface $form_st 
        $form_state->set('flood_control_triggered', 'ip');

        return;

      }

      $accounts = $this->userStorage->loadByProperties(['name' => $form_state->getValue('name'), 'status' => 1]);

      $accounts = $this->userStorage->loadByProperties(['name' => $form_state->getValue('name')]);

      $account = reset($accounts);

      if ($account) {

      if ($account && $account->isBlocked()) {

        $form_state->setErrorByName('name', $this->t('The username %name has not been activated or is blocked.', ['%name' => $form_state->getValue('name')]));

      }

      elseif ($account && $account->isActive()) {

        if ($flood_config->get('uid_only')) {

          // Register flood events based on the uid only, so they apply for any

          // IP address. This is the most secure option.
@@ -226,11 +233,12 @@ public function validateAuthentication(array &$form, FormStateInterface $form_st 
        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);

      $form_state->set('uid', $uid);

        // Store the user ID in form state as a flag for self::validateFinal().

        if ($this->userAuth->authenticateAccount($account, $password)) {

          $form_state->set('uid', $account->id());

        }

      }

    }

  }

core/modules/user/src/UserAuth.php

+16 −9
Original line number Diff line number Diff line
@@ -47,20 +47,27 @@ public function authenticate($username, #[\SensitiveParameter] $password) { 
      $account_search = $this->entityTypeManager->getStorage('user')->loadByProperties(['name' => $username]);



      if ($account = reset($account_search)) {

        if ($this->passwordChecker->check($password, $account->getPassword())) {

          // Successful authentication.

        if ($this->authenticateAccount($account, $password)) {

          $uid = $account->id();

        }

      }

    }

    return $uid;

  }



  /**

   * {@inheritdoc}

   */

  public function authenticateAccount(UserInterface $account, #[\SensitiveParameter] string $password): bool {

    if ($this->passwordChecker->check($password, $account->getPassword())) {

      // Update user to new password scheme if needed.

      if ($this->passwordChecker->needsRehash($account->getPassword())) {

        $account->setPassword($password);

        $account->save();

      }

      return TRUE;

    }

      }

    }



    return $uid;

    return FALSE;

  }



}

core/modules/user/src/UserAuthInterface.php

+16 −0
Original line number Diff line number Diff line
@@ -20,4 +20,20 @@ interface UserAuthInterface { 
   */

  public function authenticate($username, #[\SensitiveParameter] $password);



  /**

   * Validates user authentication credentials for an account.

   *

   * This can be used where the account has already been located using the login

   * credentials.

   *

   * @param \Drupal\Core\Session\AccountInterface $account

   *   The account to authenticate.

   * @param string $password

   *   A plain-text password, such as trimmed text from form values.

   *

   * @return bool

   *   TRUE on success, FALSE on failure.

   */

  public function authenticateAccount(UserInterface $account, #[\SensitiveParameter] string $password): bool;



}

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

+4 −8
Original line number Diff line number Diff line
@@ -195,11 +195,9 @@ public function testLogin(): void { 


    $expected_queries = [

      'SELECT "name", "value" FROM "key_value" WHERE "name" IN ( "system.maintenance_mode" ) AND "collection" = "state"',

      'SELECT "base_table"."uid" AS "uid", "base_table"."uid" AS "base_table_uid" FROM "users" "base_table" INNER JOIN "users_field_data" "users_field_data" ON "users_field_data"."uid" = "base_table"."uid" WHERE ("users_field_data"."name" LIKE "ACCOUNT_NAME" ESCAPE ' . "'\\\\'" . ') AND ("users_field_data"."status" = 0)',

      'SELECT COUNT(*) AS "expression" FROM (SELECT 1 AS "expression" FROM "flood" "f" WHERE ("event" = "user.failed_login_ip") AND ("identifier" = "CLIENT_IP") AND ("timestamp" > "TIMESTAMP")) "subquery"',

      'SELECT "base_table"."uid" AS "uid", "base_table"."uid" AS "base_table_uid" FROM "users" "base_table" INNER JOIN "users_field_data" "users_field_data" ON "users_field_data"."uid" = "base_table"."uid" WHERE ("users_field_data"."name" IN ("ACCOUNT_NAME")) AND ("users_field_data"."status" IN (1)) AND ("users_field_data"."default_langcode" IN (1))',

      'SELECT COUNT(*) AS "expression" FROM (SELECT 1 AS "expression" FROM "flood" "f" WHERE ("event" = "user.failed_login_user") AND ("identifier" = "CLIENT_IP") AND ("timestamp" > "TIMESTAMP")) "subquery"',

      'SELECT "base_table"."uid" AS "uid", "base_table"."uid" AS "base_table_uid" FROM "users" "base_table" INNER JOIN "users_field_data" "users_field_data" ON "users_field_data"."uid" = "base_table"."uid" WHERE ("users_field_data"."name" IN ("ACCOUNT_NAME")) AND ("users_field_data"."default_langcode" IN (1))',

      'SELECT COUNT(*) AS "expression" FROM (SELECT 1 AS "expression" FROM "flood" "f" WHERE ("event" = "user.failed_login_user") AND ("identifier" = "CLIENT_IP") AND ("timestamp" > "TIMESTAMP")) "subquery"',

      'INSERT INTO "watchdog" ("uid", "type", "message", "variables", "severity", "link", "location", "referer", "hostname", "timestamp") VALUES ("2", "user", "Session opened for %name.", "WATCHDOG_DATA", 6, "", "LOCATION", "REFERER", "CLIENT_IP", "TIMESTAMP")',

      'UPDATE "users_field_data" SET "login"="TIMESTAMP" WHERE "uid" = "2"',

      'SELECT "name", "value" FROM "key_value" WHERE "name" IN ( "drupal.test_wait_terminate" ) AND "collection" = "state"',
@@ -222,7 +220,7 @@ public function testLogin(): void { 
    ];

    $recorded_queries = $performance_data->getQueries();

    $this->assertSame($expected_queries, $recorded_queries);

    $this->assertSame(25, $performance_data->getQueryCount());

    $this->assertSame(23, $performance_data->getQueryCount());

    $this->assertSame(64, $performance_data->getCacheGetCount());

    $this->assertSame(1, $performance_data->getCacheSetCount());

    $this->assertSame(1, $performance_data->getCacheDeleteCount());
@@ -263,15 +261,13 @@ public function testLoginBlock(): void { 
      'SELECT "name", "value" FROM "key_value" WHERE "name" IN ( "theme:stark" ) AND "collection" = "config.entity.key_store.block"',

      'SELECT "config"."name" AS "name" FROM "config" "config" WHERE ("collection" = "") AND ("name" LIKE "search.page.%" ESCAPE ' . "'\\\\'" . ') ORDER BY "collection" ASC, "name" ASC',

      'SELECT "menu_tree"."id" AS "id" FROM "menu_tree" "menu_tree" WHERE ("menu_name" = "account") AND ("expanded" = 1) AND ("has_children" = 1) AND ("enabled" = 1) AND ("parent" IN ("")) AND ("id" NOT IN (""))',

      'SELECT "base_table"."uid" AS "uid", "base_table"."uid" AS "base_table_uid" FROM "users" "base_table" INNER JOIN "users_field_data" "users_field_data" ON "users_field_data"."uid" = "base_table"."uid" WHERE ("users_field_data"."name" LIKE "ACCOUNT_NAME" ESCAPE ' . "'\\\\'" . ') AND ("users_field_data"."status" = 0)',

      'SELECT COUNT(*) AS "expression" FROM (SELECT 1 AS "expression" FROM "flood" "f" WHERE ("event" = "user.failed_login_ip") AND ("identifier" = "CLIENT_IP") AND ("timestamp" > "TIMESTAMP")) "subquery"',

      'SELECT "base_table"."uid" AS "uid", "base_table"."uid" AS "base_table_uid" FROM "users" "base_table" INNER JOIN "users_field_data" "users_field_data" ON "users_field_data"."uid" = "base_table"."uid" WHERE ("users_field_data"."name" IN ("ACCOUNT_NAME")) AND ("users_field_data"."status" IN (1)) AND ("users_field_data"."default_langcode" IN (1))',

      'SELECT "base_table"."uid" AS "uid", "base_table"."uid" AS "base_table_uid" FROM "users" "base_table" INNER JOIN "users_field_data" "users_field_data" ON "users_field_data"."uid" = "base_table"."uid" WHERE ("users_field_data"."name" IN ("ACCOUNT_NAME")) AND ("users_field_data"."default_langcode" IN (1))',

      'SELECT "base"."uid" AS "uid", "base"."uuid" AS "uuid", "base"."langcode" AS "langcode" FROM "users" "base" WHERE "base"."uid" IN (2)',

      'SELECT "data".* FROM "users_field_data" "data" WHERE "data"."uid" IN (2) ORDER BY "data"."uid" ASC',

      'SELECT "t".* FROM "user__roles" "t" WHERE ("entity_id" IN (2)) AND ("deleted" = 0) AND ("langcode" IN ("en", "und", "zxx")) ORDER BY "delta" ASC',

      'SELECT "t".* FROM "user__user_picture" "t" WHERE ("entity_id" IN (2)) AND ("deleted" = 0) AND ("langcode" IN ("en", "und", "zxx")) ORDER BY "delta" ASC',

      'SELECT COUNT(*) AS "expression" FROM (SELECT 1 AS "expression" FROM "flood" "f" WHERE ("event" = "user.failed_login_user") AND ("identifier" = "CLIENT_IP") AND ("timestamp" > "TIMESTAMP")) "subquery"',

      'SELECT "base_table"."uid" AS "uid", "base_table"."uid" AS "base_table_uid" FROM "users" "base_table" INNER JOIN "users_field_data" "users_field_data" ON "users_field_data"."uid" = "base_table"."uid" WHERE ("users_field_data"."name" IN ("ACCOUNT_NAME")) AND ("users_field_data"."default_langcode" IN (1))',

      'INSERT INTO "watchdog" ("uid", "type", "message", "variables", "severity", "link", "location", "referer", "hostname", "timestamp") VALUES ("2", "user", "Session opened for %name.", "WATCHDOG_DATA", 6, "", "LOCATION", "REFERER", "CLIENT_IP", "TIMESTAMP")',

      'UPDATE "users_field_data" SET "login"="TIMESTAMP" WHERE "uid" = "2"',

      'SELECT "name", "value" FROM "key_value" WHERE "name" IN ( "drupal.test_wait_terminate" ) AND "collection" = "state"',
@@ -290,7 +286,7 @@ public function testLoginBlock(): void { 
    ];

    $recorded_queries = $performance_data->getQueries();

    $this->assertSame($expected_queries, $recorded_queries);

    $this->assertSame(30, $performance_data->getQueryCount());

    $this->assertSame(28, $performance_data->getQueryCount());

    $this->assertSame(85, $performance_data->getCacheGetCount());

    $this->assertSame(1, $performance_data->getCacheSetCount());

    $this->assertSame(1, $performance_data->getCacheDeleteCount());