Issue #2508627 by rvtraveller, pwolanin: Changing email address should...

Issue #2508627 by rvtraveller, pwolanin: Changing email address should invalidate one-time login links

core/modules/user/src/Controller/UserController.php

@@ -123,7 +123,7 @@ public function resetPass($uid, $timestamp, $hash) {
         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 @@ public function confirmCancel(UserInterface $user, $timestamp = 0, $hashed_pass
     $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'),
         );

core/modules/user/src/Tests/UserCancelTest.php

@@ -59,7 +59,7 @@ function testUserCancelWithoutPermission() {
 
     // 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 @@ function testUserCancelInvalid() {
 
     // 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 @@ function testUserCancelInvalid() {
 
     // 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 @@ function testUserBlock() {
     $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 @@ function testUserBlockUnpublish() {
     $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 @@ function testUserAnonymize() {
     $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 @@ function testUserDelete() {
     $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.');
 

core/modules/user/src/Tests/UserPasswordResetTest.php

@@ -144,14 +144,14 @@ function testUserPasswordReset() {
     $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 @@ function testUserPasswordReset() {
     $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.');
   }
 
   /**

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());
 }
 
 /**