Skip to content
Snippets Groups Projects
Commit 05c2f783 authored by catch's avatar catch
Browse files

Revert: Issue #3410582 by catch, Prashant.c, heddn, alexpott, smustgrave,...

Revert: Issue #3410582 by catch, Prashant.c, heddn, alexpott, smustgrave, claudiu.cristea, longwave, quietone: Optimize user logins by avoiding duplicate entity queries
parent f64efcf3
No related branches found
No related tags found
No related merge requests found
...@@ -132,6 +132,7 @@ public function buildForm(array $form, FormStateInterface $form_state) { ...@@ -132,6 +132,7 @@ public function buildForm(array $form, FormStateInterface $form_state) {
$form['actions'] = ['#type' => 'actions']; $form['actions'] = ['#type' => 'actions'];
$form['actions']['submit'] = ['#type' => 'submit', '#value' => $this->t('Log in')]; $form['actions']['submit'] = ['#type' => 'submit', '#value' => $this->t('Log in')];
$form['#validate'][] = '::validateName';
$form['#validate'][] = '::validateAuthentication'; $form['#validate'][] = '::validateAuthentication';
$form['#validate'][] = '::validateFinal'; $form['#validate'][] = '::validateFinal';
...@@ -144,8 +145,8 @@ public function buildForm(array $form, FormStateInterface $form_state) { ...@@ -144,8 +145,8 @@ public function buildForm(array $form, FormStateInterface $form_state) {
* {@inheritdoc} * {@inheritdoc}
*/ */
public function submitForm(array &$form, FormStateInterface $form_state) { public function submitForm(array &$form, FormStateInterface $form_state) {
$uid = $form_state->get('uid');
if (!$uid) { if (empty($uid = $form_state->get('uid'))) {
return; return;
} }
$account = $this->userStorage->load($uid); $account = $this->userStorage->load($uid);
...@@ -166,12 +167,8 @@ public function submitForm(array &$form, FormStateInterface $form_state) { ...@@ -166,12 +167,8 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
/** /**
* Sets an error if supplied username has been blocked. * 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) { 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'))) { if (!$form_state->isValueEmpty('name') && user_is_blocked($form_state->getValue('name'))) {
// Blocked in user administration. // 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')])); $form_state->setErrorByName('name', $this->t('The username %name has not been activated or is blocked.', ['%name' => $form_state->getValue('name')]));
...@@ -186,7 +183,6 @@ public function validateName(array &$form, FormStateInterface $form_state) { ...@@ -186,7 +183,6 @@ public function validateName(array &$form, FormStateInterface $form_state) {
public function validateAuthentication(array &$form, FormStateInterface $form_state) { public function validateAuthentication(array &$form, FormStateInterface $form_state) {
$password = trim($form_state->getValue('pass')); $password = trim($form_state->getValue('pass'));
$flood_config = $this->config('user.flood'); $flood_config = $this->config('user.flood');
$account = FALSE;
if (!$form_state->isValueEmpty('name') && strlen($password) > 0) { if (!$form_state->isValueEmpty('name') && strlen($password) > 0) {
// Do not allow any login from the current user's IP if the limit has been // 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 // reached. Default is 50 failed attempts allowed in one hour. This is
...@@ -197,12 +193,9 @@ public function validateAuthentication(array &$form, FormStateInterface $form_st ...@@ -197,12 +193,9 @@ public function validateAuthentication(array &$form, FormStateInterface $form_st
$form_state->set('flood_control_triggered', 'ip'); $form_state->set('flood_control_triggered', 'ip');
return; return;
} }
$accounts = $this->userStorage->loadByProperties(['name' => $form_state->getValue('name')]); $accounts = $this->userStorage->loadByProperties(['name' => $form_state->getValue('name'), 'status' => 1]);
$account = reset($accounts); $account = reset($accounts);
if ($account && $account->isBlocked()) { if ($account) {
$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')) { if ($flood_config->get('uid_only')) {
// Register flood events based on the uid only, so they apply for any // Register flood events based on the uid only, so they apply for any
// IP address. This is the most secure option. // IP address. This is the most secure option.
...@@ -233,12 +226,11 @@ public function validateAuthentication(array &$form, FormStateInterface $form_st ...@@ -233,12 +226,11 @@ public function validateAuthentication(array &$form, FormStateInterface $form_st
else { else {
$form_state->set('flood_control_skip_clear', 'user'); $form_state->set('flood_control_skip_clear', 'user');
} }
// We are not limited by flood control, so try to authenticate.
// 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());
}
} }
// 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);
} }
} }
......
...@@ -47,27 +47,20 @@ public function authenticate($username, #[\SensitiveParameter] $password) { ...@@ -47,27 +47,20 @@ public function authenticate($username, #[\SensitiveParameter] $password) {
$account_search = $this->entityTypeManager->getStorage('user')->loadByProperties(['name' => $username]); $account_search = $this->entityTypeManager->getStorage('user')->loadByProperties(['name' => $username]);
if ($account = reset($account_search)) { if ($account = reset($account_search)) {
if ($this->authenticateAccount($account, $password)) { if ($this->passwordChecker->check($password, $account->getPassword())) {
// Successful authentication.
$uid = $account->id(); $uid = $account->id();
// Update user to new password scheme if needed.
if ($this->passwordChecker->needsRehash($account->getPassword())) {
$account->setPassword($password);
$account->save();
}
} }
} }
} }
return $uid;
}
/** 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 FALSE;
} }
} }
...@@ -20,20 +20,4 @@ interface UserAuthInterface { ...@@ -20,20 +20,4 @@ interface UserAuthInterface {
*/ */
public function authenticate($username, #[\SensitiveParameter] $password); 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;
} }
...@@ -194,9 +194,11 @@ public function testLogin(): void { ...@@ -194,9 +194,11 @@ public function testLogin(): void {
$expected_queries = [ $expected_queries = [
'SELECT "name", "value" FROM "key_value" WHERE "name" IN ( "system.maintenance_mode" ) AND "collection" = "state"', '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 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"."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"."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 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")', '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"', '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"', 'SELECT "name", "value" FROM "key_value" WHERE "name" IN ( "drupal.test_wait_terminate" ) AND "collection" = "state"',
...@@ -219,7 +221,7 @@ public function testLogin(): void { ...@@ -219,7 +221,7 @@ public function testLogin(): void {
]; ];
$recorded_queries = $performance_data->getQueries(); $recorded_queries = $performance_data->getQueries();
$this->assertSame($expected_queries, $recorded_queries); $this->assertSame($expected_queries, $recorded_queries);
$this->assertSame(23, $performance_data->getQueryCount()); $this->assertSame(25, $performance_data->getQueryCount());
$this->assertSame(64, $performance_data->getCacheGetCount()); $this->assertSame(64, $performance_data->getCacheGetCount());
$this->assertSame(1, $performance_data->getCacheSetCount()); $this->assertSame(1, $performance_data->getCacheSetCount());
$this->assertSame(1, $performance_data->getCacheDeleteCount()); $this->assertSame(1, $performance_data->getCacheDeleteCount());
...@@ -259,13 +261,15 @@ public function testLoginBlock(): void { ...@@ -259,13 +261,15 @@ public function testLoginBlock(): void {
'SELECT "name", "value" FROM "key_value" WHERE "name" IN ( "twig_extension_hash_prefix" ) AND "collection" = "state"', 'SELECT "name", "value" FROM "key_value" WHERE "name" IN ( "twig_extension_hash_prefix" ) AND "collection" = "state"',
'SELECT "name", "value" FROM "key_value" WHERE "name" IN ( "theme:stark" ) AND "collection" = "config.entity.key_store.block"', '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 "config"."name" AS "name" FROM "config" "config" WHERE ("collection" = "") AND ("name" LIKE "search.page.%" ESCAPE ' . "'\\\\'" . ') ORDER BY "collection" ASC, "name" ASC',
'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 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"."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"."status" IN (1)) 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 "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 "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__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 "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 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")', '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"', '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"', 'SELECT "name", "value" FROM "key_value" WHERE "name" IN ( "drupal.test_wait_terminate" ) AND "collection" = "state"',
...@@ -284,7 +288,7 @@ public function testLoginBlock(): void { ...@@ -284,7 +288,7 @@ public function testLoginBlock(): void {
]; ];
$recorded_queries = $performance_data->getQueries(); $recorded_queries = $performance_data->getQueries();
$this->assertSame($expected_queries, $recorded_queries); $this->assertSame($expected_queries, $recorded_queries);
$this->assertSame(27, $performance_data->getQueryCount()); $this->assertSame(29, $performance_data->getQueryCount());
$this->assertSame(108, $performance_data->getCacheGetCount()); $this->assertSame(108, $performance_data->getCacheGetCount());
$this->assertSame(1, $performance_data->getCacheSetCount()); $this->assertSame(1, $performance_data->getCacheSetCount());
$this->assertSame(1, $performance_data->getCacheDeleteCount()); $this->assertSame(1, $performance_data->getCacheDeleteCount());
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment