From ad358fff860184f75a85712e2a8cbd8873126f0a Mon Sep 17 00:00:00 2001 From: catch Date: Mon, 12 Dec 2022 16:53:33 +0000 Subject: [PATCH] Issue #2828724 by Spokje, alexpott, ravi.shankar, Lal_, malcomio, ElusiveMind, smaz, yogeshmpawar, ridhimaabrol24, semiaddict, piggito, f.mazeikis, tvhung, tatarbj, ranjith_kumar_k_u, vijaycs85, baikho, Jelle_S, kleinmp, bbrala, Mike_info, David_Rothstein, pwolanin, cburschka: Username enumeration via one time login route (cherry picked from commit ae061b3974522f4f93b75ffbae1354f2a6ce3c6c) --- .../user/src/Controller/UserController.php | 73 ++++++++++++++----- .../src/Functional/UserLoginHttpTest.php | 4 + .../src/Functional/UserPasswordResetTest.php | 17 ++++- 3 files changed, 71 insertions(+), 23 deletions(-) diff --git a/core/modules/user/src/Controller/UserController.php b/core/modules/user/src/Controller/UserController.php index 625969e295a..ec5b1ccfad0 100644 --- a/core/modules/user/src/Controller/UserController.php +++ b/core/modules/user/src/Controller/UserController.php @@ -14,6 +14,7 @@ use Drupal\user\UserInterface; use Drupal\user\UserStorageInterface; use Psr\Log\LoggerInterface; use Symfony\Component\DependencyInjection\ContainerInterface; +use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; @@ -149,6 +150,12 @@ class UserController extends ControllerBase { } } + /** @var \Drupal\user\UserInterface $reset_link_user */ + $reset_link_user = $this->userStorage->load($uid); + if ($redirect = $this->determineErrorRedirect($reset_link_user, $timestamp, $hash)) { + return $redirect; + } + $session = $request->getSession(); $session->set('pass_reset_hash', $hash); $session->set('pass_reset_timeout', $timestamp); @@ -222,11 +229,54 @@ class UserController extends ControllerBase { * If $uid is for a blocked user or invalid user ID. */ public function resetPassLogin($uid, $timestamp, $hash, Request $request) { - // The current user is not logged in, so check the parameters. - $current = REQUEST_TIME; /** @var \Drupal\user\UserInterface $user */ $user = $this->userStorage->load($uid); + if ($redirect = $this->determineErrorRedirect($user, $timestamp, $hash)) { + return $redirect; + } + user_login_finalize($user); + $this->logger->notice('User %name used one-time login link at time %timestamp.', ['%name' => $user->getDisplayName(), '%timestamp' => $timestamp]); + $this->messenger()->addStatus($this->t('You have just used your one-time login link. It is no longer necessary to use this link to log in. Please set your password.')); + // Let the user's password be changed without the current password + // check. + $token = Crypt::randomBytesBase64(55); + $request->getSession()->set('pass_reset_' . $user->id(), $token); + // Clear any flood events for this user. + $this->flood->clear('user.password_request_user', $uid); + return $this->redirect( + 'entity.user.edit_form', + ['user' => $user->id()], + [ + 'query' => ['pass-reset-token' => $token], + 'absolute' => TRUE, + ] + ); + } + + /** + * Validates user, hash, and timestamp. + * + * This method allows the 'user.reset' and 'user.reset.login' routes to use + * the same logic to check the user, timestamp and hash and redirect to the + * same location with the same messages. + * + * @param \Drupal\user\UserInterface|null $user + * User requesting reset. NULL if the user does not exist. + * @param int $timestamp + * The current timestamp. + * @param string $hash + * Login link hash. + * + * @return \Symfony\Component\HttpFoundation\RedirectResponse|null + * Returns a redirect if the information is incorrect. It redirects to + * 'user.pass' route with a message for the user. + * + * @throws \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException + * If $uid is for a blocked user or invalid user ID. + */ + protected function determineErrorRedirect(?UserInterface $user, int $timestamp, string $hash): ?RedirectResponse { + $current = REQUEST_TIME; // Verify that the user exists and is active. if ($user === NULL || !$user->isActive()) { // Blocked or invalid user ID, so deny access. The parameters will be in @@ -242,23 +292,8 @@ class UserController extends ControllerBase { return $this->redirect('user.pass'); } elseif ($user->isAuthenticated() && ($timestamp >= $user->getLastLoginTime()) && ($timestamp <= $current) && hash_equals($hash, user_pass_rehash($user, $timestamp))) { - user_login_finalize($user); - $this->logger->notice('User %name used one-time login link at time %timestamp.', ['%name' => $user->getDisplayName(), '%timestamp' => $timestamp]); - $this->messenger()->addStatus($this->t('You have just used your one-time login link. It is no longer necessary to use this link to log in. Please set your password.')); - // Let the user's password be changed without the current password - // check. - $token = Crypt::randomBytesBase64(55); - $request->getSession()->set('pass_reset_' . $user->id(), $token); - // Clear any flood events for this user. - $this->flood->clear('user.password_request_user', $uid); - return $this->redirect( - 'entity.user.edit_form', - ['user' => $user->id()], - [ - 'query' => ['pass-reset-token' => $token], - 'absolute' => TRUE, - ] - ); + // The information provided is valid. + return NULL; } $this->messenger()->addError($this->t('You have tried to use a one-time login link that has either been used or is no longer valid. Please request a new one using the form below.')); diff --git a/core/modules/user/tests/src/Functional/UserLoginHttpTest.php b/core/modules/user/tests/src/Functional/UserLoginHttpTest.php index b2f87f28d13..0760d50b161 100644 --- a/core/modules/user/tests/src/Functional/UserLoginHttpTest.php +++ b/core/modules/user/tests/src/Functional/UserLoginHttpTest.php @@ -106,6 +106,8 @@ class UserLoginHttpTest extends BrowserTestBase { // Enable serialization so we have access to additional formats. $this->container->get('module_installer')->install(['serialization']); + $this->rebuildAll(); + $this->doTestLogin('json'); $this->doTestLogin('xml'); } @@ -243,6 +245,7 @@ class UserLoginHttpTest extends BrowserTestBase { // Enable serialization so we have access to additional formats. $this->container->get('module_installer')->install(['serialization']); + $this->rebuildAll(); $this->doTestPasswordReset('json', $account); $this->doTestPasswordReset('xml', $account); @@ -572,6 +575,7 @@ class UserLoginHttpTest extends BrowserTestBase { $resetURL = $urls[0]; $this->drupalGet($resetURL); $this->submitForm([], 'Log in'); + $this->assertSession()->pageTextContains('You have just used your one-time login link. It is no longer necessary to use this link to log in. Please set your password.'); } } diff --git a/core/modules/user/tests/src/Functional/UserPasswordResetTest.php b/core/modules/user/tests/src/Functional/UserPasswordResetTest.php index 6229dbb97cd..5bb8f3fe9b8 100644 --- a/core/modules/user/tests/src/Functional/UserPasswordResetTest.php +++ b/core/modules/user/tests/src/Functional/UserPasswordResetTest.php @@ -151,7 +151,8 @@ class UserPasswordResetTest extends BrowserTestBase { // Log out, and try to log in again using the same one-time link. $this->drupalLogout(); $this->drupalGet($resetURL); - $this->submitForm([], 'Log in'); + $this->assertSession()->pageTextContains('You have tried to use a one-time login link that has either been used or is no longer valid. Please request a new one using the form below.'); + $this->drupalGet($resetURL . '/login'); $this->assertSession()->pageTextContains('You have tried to use a one-time login link that has either been used or is no longer valid. Please request a new one using the form below.'); // Request a new password again, this time using the email address. @@ -177,7 +178,8 @@ class UserPasswordResetTest extends BrowserTestBase { $bogus_timestamp = REQUEST_TIME - $timeout - 60; $_uid = $this->account->id(); $this->drupalGet("user/reset/$_uid/$bogus_timestamp/" . user_pass_rehash($this->account, $bogus_timestamp)); - $this->submitForm([], 'Log in'); + $this->assertSession()->pageTextContains('You have tried to use a one-time login link that has expired. Please request a new one using the form below.'); + $this->drupalGet("user/reset/$_uid/$bogus_timestamp/" . user_pass_rehash($this->account, $bogus_timestamp) . '/login'); $this->assertSession()->pageTextContains('You have tried to use a one-time login link that has expired. Please request a new one using the form below.'); // Create a user, block the account, and verify that a login link is denied. @@ -186,6 +188,8 @@ class UserPasswordResetTest extends BrowserTestBase { $blocked_account->save(); $this->drupalGet("user/reset/" . $blocked_account->id() . "/$timestamp/" . user_pass_rehash($blocked_account, $timestamp)); $this->assertSession()->statusCodeEquals(403); + $this->drupalGet("user/reset/" . $blocked_account->id() . "/$timestamp/" . user_pass_rehash($blocked_account, $timestamp) . '/login'); + $this->assertSession()->statusCodeEquals(403); // Verify a blocked user can not request a new password. $this->drupalGet('user/password'); @@ -203,7 +207,8 @@ class UserPasswordResetTest extends BrowserTestBase { $this->account->setEmail("1" . $this->account->getEmail()); $this->account->save(); $this->drupalGet($old_email_reset_link); - $this->submitForm([], 'Log in'); + $this->assertSession()->pageTextContains('You have tried to use a one-time login link that has either been used or is no longer valid. Please request a new one using the form below.'); + $this->drupalGet($old_email_reset_link . '/login'); $this->assertSession()->pageTextContains('You have tried to use a one-time login link that has either been used or is no longer valid. Please request a new one using the form below.'); // Verify a password reset link will automatically log a user when /login is @@ -563,7 +568,11 @@ class UserPasswordResetTest extends BrowserTestBase { $reset_url = user_pass_reset_url($user1); $attack_reset_url = str_replace("user/reset/{$user1->id()}", "user/reset/{$user2->id()}", $reset_url); $this->drupalGet($attack_reset_url); - $this->submitForm([], 'Log in'); + // Verify that the invalid password reset page does not show the user name. + $this->assertSession()->pageTextNotContains($user2->getAccountName()); + $this->assertSession()->addressEquals('user/password'); + $this->assertSession()->pageTextContains('You have tried to use a one-time login link that has either been used or is no longer valid. Please request a new one using the form below.'); + $this->drupalGet($attack_reset_url . '/login'); // Verify that the invalid password reset page does not show the user name. $this->assertSession()->pageTextNotContains($user2->getAccountName()); $this->assertSession()->addressEquals('user/password');