diff --git a/core/modules/user/src/Controller/UserController.php b/core/modules/user/src/Controller/UserController.php index dc669b25877..03dcb102974 100644 --- a/core/modules/user/src/Controller/UserController.php +++ b/core/modules/user/src/Controller/UserController.php @@ -123,7 +123,7 @@ class UserController extends ControllerBase { drupal_set_message($this->t('You have tried to use a one-time login link that has expired. Please request a new one using the form below.'), 'error'); return $this->redirect('user.pass'); } - elseif ($user->isAuthenticated() && ($timestamp >= $user->getLastLoginTime()) && ($timestamp <= $current) && ($hash === user_pass_rehash($user->getPassword(), $timestamp, $user->getLastLoginTime(), $user->id()))) { + elseif ($user->isAuthenticated() && ($timestamp >= $user->getLastLoginTime()) && ($timestamp <= $current) && ($hash === user_pass_rehash($user, $timestamp))) { $expiration_date = $user->getLastLoginTime() ? $this->dateFormatter->format($timestamp + $timeout) : NULL; return $this->formBuilder()->getForm('Drupal\user\Form\UserPasswordResetForm', $user, $expiration_date, $timestamp, $hash); } @@ -197,7 +197,7 @@ class UserController extends ControllerBase { $account_data = $this->userData->get('user', $user->id()); if (isset($account_data['cancel_method']) && !empty($timestamp) && !empty($hashed_pass)) { // Validate expiration and hashed password/login. - if ($timestamp <= $current && $current - $timestamp < $timeout && $user->id() && $timestamp >= $user->getLastLoginTime() && $hashed_pass == user_pass_rehash($user->getPassword(), $timestamp, $user->getLastLoginTime(), $user->id())) { + if ($timestamp <= $current && $current - $timestamp < $timeout && $user->id() && $timestamp >= $user->getLastLoginTime() && $hashed_pass == user_pass_rehash($user, $timestamp)) { $edit = array( 'user_cancel_notify' => isset($account_data['cancel_notify']) ? $account_data['cancel_notify'] : $this->config('user.settings')->get('notify.status_canceled'), ); diff --git a/core/modules/user/src/Tests/UserCancelTest.php b/core/modules/user/src/Tests/UserCancelTest.php index a6a161bb8b3..53570134097 100644 --- a/core/modules/user/src/Tests/UserCancelTest.php +++ b/core/modules/user/src/Tests/UserCancelTest.php @@ -59,7 +59,7 @@ class UserCancelTest extends WebTestBase { // Attempt bogus account cancellation request confirmation. $timestamp = $account->getLastLoginTime(); - $this->drupalGet("user/" . $account->id() . "/cancel/confirm/$timestamp/" . user_pass_rehash($account->getPassword(), $timestamp, $account->getLastLoginTime(), $account->id())); + $this->drupalGet("user/" . $account->id() . "/cancel/confirm/$timestamp/" . user_pass_rehash($account, $timestamp)); $this->assertResponse(403, 'Bogus cancelling request rejected.'); $user_storage->resetCache(array($account->id())); $account = $user_storage->load($account->id()); @@ -165,7 +165,7 @@ class UserCancelTest extends WebTestBase { // Attempt bogus account cancellation request confirmation. $bogus_timestamp = $timestamp + 60; - $this->drupalGet("user/" . $account->id() . "/cancel/confirm/$bogus_timestamp/" . user_pass_rehash($account->getPassword(), $bogus_timestamp, $account->getLastLoginTime(), $account->id())); + $this->drupalGet("user/" . $account->id() . "/cancel/confirm/$bogus_timestamp/" . user_pass_rehash($account, $bogus_timestamp)); $this->assertText(t('You have tried to use an account cancellation link that has expired. Please request a new one using the form below.'), 'Bogus cancelling request rejected.'); $user_storage->resetCache(array($account->id())); $account = $user_storage->load($account->id()); @@ -173,7 +173,7 @@ class UserCancelTest extends WebTestBase { // Attempt expired account cancellation request confirmation. $bogus_timestamp = $timestamp - 86400 - 60; - $this->drupalGet("user/" . $account->id() . "/cancel/confirm/$bogus_timestamp/" . user_pass_rehash($account->getPassword(), $bogus_timestamp, $account->getLastLoginTime(), $account->id())); + $this->drupalGet("user/" . $account->id() . "/cancel/confirm/$bogus_timestamp/" . user_pass_rehash($account, $bogus_timestamp)); $this->assertText(t('You have tried to use an account cancellation link that has expired. Please request a new one using the form below.'), 'Expired cancel account request rejected.'); $user_storage->resetCache(array($account->id())); $account = $user_storage->load($account->id()); @@ -214,7 +214,7 @@ class UserCancelTest extends WebTestBase { $this->assertText(t('A confirmation request to cancel your account has been sent to your email address.'), 'Account cancellation request mailed message displayed.'); // Confirm account cancellation request. - $this->drupalGet("user/" . $account->id() . "/cancel/confirm/$timestamp/" . user_pass_rehash($account->getPassword(), $timestamp, $account->getLastLoginTime(), $account->id())); + $this->drupalGet("user/" . $account->id() . "/cancel/confirm/$timestamp/" . user_pass_rehash($account, $timestamp)); $user_storage->resetCache(array($account->id())); $account = $user_storage->load($account->id()); $this->assertTrue($account->isBlocked(), 'User has been blocked.'); @@ -272,7 +272,7 @@ class UserCancelTest extends WebTestBase { $this->assertText(t('A confirmation request to cancel your account has been sent to your email address.'), 'Account cancellation request mailed message displayed.'); // Confirm account cancellation request. - $this->drupalGet("user/" . $account->id() . "/cancel/confirm/$timestamp/" . user_pass_rehash($account->getPassword(), $timestamp, $account->getLastLoginTime(), $account->id())); + $this->drupalGet("user/" . $account->id() . "/cancel/confirm/$timestamp/" . user_pass_rehash($account, $timestamp)); $user_storage->resetCache(array($account->id())); $account = $user_storage->load($account->id()); $this->assertTrue($account->isBlocked(), 'User has been blocked.'); @@ -348,7 +348,7 @@ class UserCancelTest extends WebTestBase { $this->assertText(t('A confirmation request to cancel your account has been sent to your email address.'), 'Account cancellation request mailed message displayed.'); // Confirm account cancellation request. - $this->drupalGet("user/" . $account->id() . "/cancel/confirm/$timestamp/" . user_pass_rehash($account->getPassword(), $timestamp, $account->getLastLoginTime(), $account->id())); + $this->drupalGet("user/" . $account->id() . "/cancel/confirm/$timestamp/" . user_pass_rehash($account, $timestamp)); $user_storage->resetCache(array($account->id())); $this->assertFalse($user_storage->load($account->id()), 'User is not found in the database.'); @@ -427,7 +427,7 @@ class UserCancelTest extends WebTestBase { $this->assertText(t('A confirmation request to cancel your account has been sent to your email address.'), 'Account cancellation request mailed message displayed.'); // Confirm account cancellation request. - $this->drupalGet("user/" . $account->id() . "/cancel/confirm/$timestamp/" . user_pass_rehash($account->getPassword(), $timestamp, $account->getLastLoginTime(), $account->id())); + $this->drupalGet("user/" . $account->id() . "/cancel/confirm/$timestamp/" . user_pass_rehash($account, $timestamp)); $user_storage->resetCache(array($account->id())); $this->assertFalse($user_storage->load($account->id()), 'User is not found in the database.'); diff --git a/core/modules/user/src/Tests/UserPasswordResetTest.php b/core/modules/user/src/Tests/UserPasswordResetTest.php index 32339189fd2..f5bd131ecad 100644 --- a/core/modules/user/src/Tests/UserPasswordResetTest.php +++ b/core/modules/user/src/Tests/UserPasswordResetTest.php @@ -144,14 +144,14 @@ class UserPasswordResetTest extends PageCacheTagsTestBase { $timeout = $this->config('user.settings')->get('password_reset_timeout'); $bogus_timestamp = REQUEST_TIME - $timeout - 60; $_uid = $this->account->id(); - $this->drupalGet("user/reset/$_uid/$bogus_timestamp/" . user_pass_rehash($this->account->getPassword(), $bogus_timestamp, $this->account->getLastLoginTime(), $this->account->id())); + $this->drupalGet("user/reset/$_uid/$bogus_timestamp/" . user_pass_rehash($this->account, $bogus_timestamp)); $this->assertText(t('You have tried to use a one-time login link that has expired. Please request a new one using the form below.'), 'Expired password reset request rejected.'); // Create a user, block the account, and verify that a login link is denied. $timestamp = REQUEST_TIME - 1; $blocked_account = $this->drupalCreateUser()->block(); $blocked_account->save(); - $this->drupalGet("user/reset/" . $blocked_account->id() . "/$timestamp/" . user_pass_rehash($blocked_account->getPassword(), $timestamp, $blocked_account->getLastLoginTime(), $this->account->id())); + $this->drupalGet("user/reset/" . $blocked_account->id() . "/$timestamp/" . user_pass_rehash($blocked_account, $timestamp)); $this->assertResponse(403); // Verify a blocked user can not request a new password. @@ -162,6 +162,16 @@ class UserPasswordResetTest extends PageCacheTagsTestBase { $this->drupalPostForm(NULL, $edit, t('Submit')); $this->assertRaw(t('%name is blocked or has not been activated yet.', array('%name' => $blocked_account->getUsername())), 'Notified user blocked accounts can not request a new password'); $this->assertTrue(count($this->drupalGetMails(array('id' => 'user_password_reset'))) === $before, 'No email was sent when requesting password reset for a blocked account'); + + // Verify a password reset link is invalidated when the user's email address changes. + $this->drupalGet('user/password'); + $edit = array('name' => $this->account->getUsername()); + $this->drupalPostForm(NULL, $edit, t('Submit')); + $old_email_reset_link = $this->getResetURL(); + $this->account->setEmail("1" . $this->account->getEmail()); + $this->account->save(); + $this->drupalGet($old_email_reset_link); + $this->assertText(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.'), 'One-time link is no longer valid.'); } /** diff --git a/core/modules/user/user.module b/core/modules/user/user.module index cfbfdcd4774..b157f1ffe25 100644 --- a/core/modules/user/user.module +++ b/core/modules/user/user.module @@ -574,7 +574,7 @@ function user_pass_reset_url($account, $options = array()) { array( 'uid' => $account->id(), 'timestamp' => $timestamp, - 'hash' => user_pass_rehash($account->getPassword(), $timestamp, $account->getLastLoginTime(), $account->id()), + 'hash' => user_pass_rehash($account, $timestamp), ), array( 'absolute' => TRUE, @@ -587,11 +587,7 @@ function user_pass_reset_url($account, $options = array()) { * Generates a URL to confirm an account cancellation request. * * @param \Drupal\user\UserInterface $account - * The user account object, which must contain at least the following - * properties: - * - uid: The user ID number. - * - pass: The hashed user password string. - * - login: The UNIX timestamp of the user's last login. + * The user account object. * @param array $options * (optional) A keyed array of settings. Supported options are: * - langcode: A language code to be used when generating locale-sensitive @@ -604,14 +600,14 @@ function user_pass_reset_url($account, $options = array()) { * @see user_mail_tokens() * @see \Drupal\user\Controller\UserController::confirmCancel() */ -function user_cancel_url($account, $options = array()) { +function user_cancel_url(UserInterface $account, $options = array()) { $timestamp = REQUEST_TIME; $langcode = isset($options['langcode']) ? $options['langcode'] : $account->getPreferredLangcode(); $url_options = array('absolute' => TRUE, 'language' => \Drupal::languageManager()->getLanguage($langcode)); return \Drupal::url('user.cancel_confirm', [ 'user' => $account->id(), 'timestamp' => $timestamp, - 'hashed_pass' => user_pass_rehash($account->getPassword(), $timestamp, $account->getLastLoginTime(), $account->id()) + 'hashed_pass' => user_pass_rehash($account, $timestamp) ], $url_options); } @@ -621,26 +617,26 @@ function user_cancel_url($account, $options = array()) { * This hash is normally used to build a unique and secure URL that is sent to * the user by email for purposes such as resetting the user's password. In * order to validate the URL, the same hash can be generated again, from the - * same information, and compared to the hash value from the URL. The URL - * normally contains both the time stamp and the numeric user ID. The login - * timestamp and hashed password are retrieved from the database as necessary. + * same information, and compared to the hash value from the URL. The hash + * contains the time stamp, the user's last login time, the numeric user ID, + * and the user's email address. * For a usage example, see user_cancel_url() and * \Drupal\user\Controller\UserController::confirmCancel(). * - * @param string $password - * The hashed user account password value. + * @param \Drupal\user\UserInterface $account + * An object containing the user account. * @param int $timestamp * A UNIX timestamp, typically REQUEST_TIME. - * @param int $login - * The UNIX timestamp of the user's last login. - * @param int $uid - * The user ID. * * @return string * A string that is safe for use in URLs and SQL statements. */ -function user_pass_rehash($password, $timestamp, $login, $uid) { - return Crypt::hmacBase64($timestamp . $login . $uid, Settings::getHashSalt() . $password); +function user_pass_rehash(UserInterface $account, $timestamp) { + $data = $timestamp; + $data .= $account->getLastLoginTime(); + $data .= $account->id(); + $data .= $account->getEmail(); + return Crypt::hmacBase64($data, Settings::getHashSalt() . $account->getPassword()); } /**