diff --git a/core/modules/user/src/AccountSettingsForm.php b/core/modules/user/src/AccountSettingsForm.php index fb8fac2..84efdce 100644 --- a/core/modules/user/src/AccountSettingsForm.php +++ b/core/modules/user/src/AccountSettingsForm.php @@ -308,7 +308,7 @@ public function buildForm(array $form, FormStateInterface $form_state) { '#title' => $this->t('Email change notification'), '#collapsible' => TRUE, '#collapsed' => TRUE, - '#description' => $this->t("Edit the email messages sent to users' old email address when the email address is changed.") . ' ' . $email_token_help, + '#description' => $this->t("Edit the email message sent to the user old email address when the email address is changed.") . ' ' . $email_token_help, '#group' => 'email', '#weight' => 11, ); diff --git a/core/modules/user/src/Controller/ChangeEmailController.php b/core/modules/user/src/Controller/ChangeEmailController.php index 3984b90..aa6784a 100644 --- a/core/modules/user/src/Controller/ChangeEmailController.php +++ b/core/modules/user/src/Controller/ChangeEmailController.php @@ -2,12 +2,14 @@ /** * @file - * Contains \Drupal\user\Controller\UserController. + * Contains \Drupal\user\Controller\ChangeEmailController. */ namespace Drupal\user\Controller; use Drupal\Core\Controller\ControllerBase; +use Drupal\Core\Url; +use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; /** * Controller routines for email change routes. @@ -33,27 +35,28 @@ class ChangeEmailController extends ControllerBase { * If the timestamp passed is in the future. */ public function changeEmailPage($uid, $timestamp, $new_mail, $hash) { - $account = \Drupal::entityManager()->getStorage('user')->load($uid); + /** @var \Drupal\user\UserInterface $account */ + $account = $this->entityTypeManager()->getStorage('user')->load($uid); - // We need to set the new email here to validate the hash correctly, - // which is created using the new mail adress. We only save the account - // if the hash matches. + // We need to set the new email here to validate the hash correctly, which + // is created using the new mail adress. We only save the account if the + // hash matches. $account->setEmail($new_mail); $timeout = 24 * 60 * 60; - if ($timestamp < REQUEST_TIME) { + if ($account->isActive() && $timestamp < REQUEST_TIME) { if (REQUEST_TIME - $timestamp > $timeout) { - drupal_set_message($this->t('You have tried to use a one-time email address change link for %account that has expired -- your change of email address was not completed. Please visit your account edit page if you wish to attempt the change again.', array('%account' => $account->name)), 'error'); + drupal_set_message($this->t('You have tried to use a one-time email address change link for %account that has expired -- your change of email address was not completed. Please visit your account edit page if you wish to attempt the change again.', ['%account' => $account->getAccountName()]), 'error'); } elseif ($this->currentUser()->id() && $this->currentUser()->id() != $account->id()) { - drupal_set_message($this->t('You are currently logged in as %user, and are attempting to confirm an email address change for %account, which is not allowed. Please log in as %account and initiate a new change of email request.', array('%user' => $user->name, '%account' => $account->name)), 'error'); + drupal_set_message($this->t('You are currently logged in as %user, and are attempting to confirm an email address change for %account, which is not allowed. Please log in as %account and initiate a new change of email request.', ['%user' => $this->currentUser()->getAccountName(), '%account' => $account->getAccountName()]), 'error'); } elseif ($hash != user_pass_rehash($account, $timestamp)) { drupal_set_message($this->t('There was a problem validating the used link. Please visit your account edit page and retry changing your email address.'), 'error'); } elseif ($timestamp > $account->getLastLoginTime() && $timestamp < REQUEST_TIME) { $account->save(); - drupal_set_message($this->t('Your email address has been changed to %mail.', array('%mail' => $new_mail))); + drupal_set_message($this->t('Your email address has been changed to %mail.', ['%mail' => $new_mail])); } } else { @@ -67,21 +70,21 @@ public function changeEmailPage($uid, $timestamp, $new_mail, $hash) { /** * Generates a unique URL for a one time email change confirmation. * - * @param object $account + * @param \Drupal\user\Entity\User $account * An object containing the user account. * @param array $options * (optional) A keyed array of settings. Supported options are: * - langcode: A language code to be used when generating locale-sensitive - * URLs. If langcode is NULL the users preferred language is used. + * URLs. If langcode is NULL the users preferred language is used. * - * @return + * @return \Drupal\Core\Url * A unique URL that provides a one-time email change confirmation for the * user. */ - public static function changeEmailUrl($account, $options = []) { + public static function changeEmailUrl(\Drupal\user\Entity\User $account, array $options = []) { $langcode = isset($options['langcode']) ? $options['langcode'] : $account->getPreferredLangcode(); - $url_options = array('absolute' => TRUE, 'language' => \Drupal::languageManager()->getLanguage($langcode)); - return \Drupal::url('user.change_email', [ + $url_options = ['absolute' => TRUE, 'language' => \Drupal::getContainer()->get('language_manager')->getLanguage($langcode)]; + return Url::fromRoute('user.change_email', [ 'uid' => $account->id(), 'timestamp' => REQUEST_TIME, 'new_mail' => $account->getEmail(), diff --git a/core/modules/user/src/Controller/UserController.php b/core/modules/user/src/Controller/UserController.php index d3d18f2..772884b 100644 --- a/core/modules/user/src/Controller/UserController.php +++ b/core/modules/user/src/Controller/UserController.php @@ -71,7 +71,7 @@ public static function create(ContainerInterface $container) { } /** - * Provides a callback for user email change page. + * Returns the user password reset page. * * @param int $uid * UID of user requesting reset. @@ -112,6 +112,7 @@ public function resetPass($uid, $timestamp, $hash) { // The current user is not logged in, so check the parameters. // Time out, in seconds, until login URL expires. $timeout = $config->get('password_reset_timeout'); + $current = REQUEST_TIME; /* @var \Drupal\user\UserInterface $user */ $user = $this->userStorage->load($uid); @@ -119,11 +120,11 @@ public function resetPass($uid, $timestamp, $hash) { // Verify that the user exists and is active. if ($user && $user->isActive()) { // No time out for first time login. - if ($user->getLastLoginTime() && REQUEST_TIME - $timestamp > $timeout) { + if ($user->getLastLoginTime() && $current - $timestamp > $timeout) { 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 <= REQUEST_TIME) && Crypt::hashEquals($hash, user_pass_rehash($user, $timestamp))) { + elseif ($user->isAuthenticated() && ($timestamp >= $user->getLastLoginTime()) && ($timestamp <= $current) && Crypt::hashEquals($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); } @@ -209,7 +210,7 @@ public function confirmCancel(UserInterface $user, $timestamp = 0, $hashed_pass return batch_process(''); } else { - drupal_set_message($this->t('You have tried to use an account cancellation link that has expired. Please request a new one using the form below.'), 'error'); + drupal_set_message(t('You have tried to use an account cancellation link that has expired. Please request a new one using the form below.'), 'error'); return $this->redirect('entity.user.cancel_form', ['user' => $user->id()], ['absolute' => TRUE]); } } diff --git a/core/modules/user/src/Tests/UserChangeEmailTest.php b/core/modules/user/src/Tests/UserChangeEmailTest.php new file mode 100644 index 0000000..34eaf0d --- /dev/null +++ b/core/modules/user/src/Tests/UserChangeEmailTest.php @@ -0,0 +1,182 @@ +drupalCreateUser(['change own username']); + $this->drupalLogin($account); + + // Save a new email and verify that the user was sent an email. + $new_mail = $this->getRandomEmail(); + $edit = array(); + $edit['mail'] = $new_mail; + $edit['current_pass'] = $account->pass_raw; + $this->drupalPostForm('user/' . $account->id() . '/edit', $edit, t('Save')); + + // Take email settings from user.mail.yml. + $mail_settings = Yaml::parse(file_get_contents(__DIR__ . '/../../config/install/user.mail.yml')); + + // Verify that the user was sent a notification email. + $this->assertMail('to', $account->getEmail(), 'Notification email with link to change the email has sent to user.'); + $subject = $token_service->replace($mail_settings['mail_change_notification']['subject'], ['user' => $account]); + $this->assertMail('subject', $subject, 'Notification subject of email to change email is correct.'); + + // Verify that the user was sent a verification email. + $this->assertMailString('to', $new_mail, 2, 'Verification email with link to change the email has sent to user.'); + $subject = $token_service->replace($mail_settings['mail_change_verification']['subject'], ['user' => $account]); + $this->assertMailString('subject', $subject, 2, 'Verification subject of email to change email is correct.'); + + $changeMailURL = $this->getChangeEmailURLFromEmail('user_mail_change_verification'); + + // Check that the email has been successfully updated. + $this->drupalGet($changeMailURL); + $this->assertRaw(t('Your email address has been changed to %mail.', ['%mail' => $new_mail])); + + $this->drupalGet($changeMailURL); + $this->assertFalse($this->drupalGetHeader('X-Drupal-Cache')); + + // Ensure the change email URL is not cached. + $this->drupalGet($changeMailURL); + $this->assertFalse($this->drupalGetHeader('X-Drupal-Cache')); + } + + /** + * Tests change of email for blocked users. + */ + public function testEmailChangeForBlockedUser() { + $timestamp = REQUEST_TIME - 1; + $account = $this->drupalCreateUser(['change own username'])->block(); + $account->save(); + $this->drupalGet($this->getChangeEmailURL($account, $timestamp)); + $this->assertResponse(403); + } + + /** + * Tests change of email for expired timestamp. + */ + public function testEmailChangeExpiredTimestamp() { + $timestamp = REQUEST_TIME - 30 * 60 * 60 * 10; + $account = $this->drupalCreateUser(['change own username']); + $this->drupalGet($this->getChangeEmailURL($account, $timestamp)); + $this->assertRaw(t('You have tried to use a one-time email address change link for %account that has expired -- your change of email address was not completed. Please visit your account edit page if you wish to attempt the change again.', ['%account' => $account->getAccountName()])); + } + + /** + * Tests change of email when other user is logged in. + */ + public function testEmailImpersonation() { + $timestamp = REQUEST_TIME - 1; + $account = $this->drupalCreateUser(['change own username']); + + // Create other account and login with it. + $other_account = $this->drupalCreateUser(); + $this->drupalLogin($other_account); + + // Try to change the email for the first account when the other account is + // logged in. + $this->drupalGet($this->getChangeEmailURL($account, $timestamp)); + $this->assertRaw(t('You are currently logged in as %user, and are attempting to confirm an email address change for %account, which is not allowed. Please log in as %account and initiate a new change of email request.', ['%user' => $other_account->getAccountName(), '%account' => $account->getAccountName()])); + } + + /** + * Tests change of email for timestamp in the future. + */ + public function testEmailChangeForFutureTimestamp() { + $timestamp = REQUEST_TIME + 30 * 60 * 60; + $account = $this->drupalCreateUser(['change own username']); + $this->drupalGet($this->getChangeEmailURL($account, $timestamp)); + $this->assertResponse(403); + } + + /** + * Tests change of email with the wrong hash. + */ + public function testEmailChangeWithWrongHash() { + $timestamp = REQUEST_TIME - 1; + $account = $this->drupalCreateUser(['change own username']); + + // Generate the hash for other user. + $other_account = $this->drupalCreateUser(['change own username']); + $hash = user_pass_rehash($other_account, $timestamp); + + $this->drupalGet($this->getChangeEmailURL($account, $timestamp, $hash)); + $this->assertText(t('There was a problem validating the used link. Please visit your account edit page and retry changing your email address.')); + } + + /** + * Retrieves the change email and extracts the link. + */ + public function getChangeEmailURLFromEmail($mail_id) { + // Assume the most recent email. + $_emails = $this->drupalGetMails(['id' => $mail_id]); + $email = end($_emails); + $urls = []; + preg_match('#.+user/change-mail/.+#', $email['body'], $urls); + return $urls[0]; + } + + /** + * Generates random email. + * + * @return string + * Random email. + */ + public function getRandomEmail() { + return $this->randomMachineName() . '@example.com'; + } + + /** + * Generates the change link. + * + * @param \Drupal\user\UserInterface $account + * The account for which the link is created. + * @param int $timestamp + * Timestamp when hash is created. + * @param string $hash + * Unique hash. + * + * @return string + * Random email. + */ + public function getChangeEmailURL(\Drupal\user\UserInterface $account, $timestamp, $hash = NULL) { + $hash = empty($hash) ? user_pass_rehash($account, $timestamp) : $hash; + return '/user/change-mail/' . $account->id() . "/$timestamp/" . $this->getRandomEmail() . '/' . $hash; + } + +} diff --git a/core/modules/user/src/Tests/UserEditedOwnAccountTest.php b/core/modules/user/src/Tests/UserEditedOwnAccountTest.php index df05bec..017ca85 100644 --- a/core/modules/user/src/Tests/UserEditedOwnAccountTest.php +++ b/core/modules/user/src/Tests/UserEditedOwnAccountTest.php @@ -10,7 +10,7 @@ use Drupal\simpletest\WebTestBase; /** - * Tests the user edit form for the current user. + * Tests user edited own account can still log in. * * @group user */ @@ -23,9 +23,6 @@ class UserEditedOwnAccountTest extends WebTestBase { */ public static $modules = array('user_form_test'); - /** - * Tests user edited own account can still log in. - */ function testUserEditedOwnAccount() { // Change account setting 'Who can register accounts?' to Administrators // only. @@ -37,8 +34,7 @@ function testUserEditedOwnAccount() { // Change own username. $edit = array(); - $name = $this->randomMachineName(); - $edit['name'] = $name; + $edit['name'] = $this->randomMachineName(); $this->drupalPostForm('user/' . $account->id() . '/edit', $edit, t('Save')); // Log out. @@ -47,20 +43,5 @@ function testUserEditedOwnAccount() { // Set the new name on the user account and attempt to log back in. $account->name = $edit['name']; $this->drupalLogin($account); - - // Set a new email address for the user account. - $new_email = $this->randomMachineName() . '@example.com'; - $edit = array( - 'current_pass' => $account->pass_raw, - 'mail' => $new_email, - ); - $this->drupalPostForm('user/' . $account->id() . '/edit', $edit, t('Save')); - - $site_name = $this->config('system.site')->get('name'); - $user_mail = $this->drupalGetMails(array( - 'to' => $new_email, - 'subject' => "Email change information for $name at $site_name", - )); - $this->assertTrue(count($user_mail), 'A verification email was sent to the user.'); } } diff --git a/core/modules/user/src/Tests/UserTokenReplaceTest.php b/core/modules/user/src/Tests/UserTokenReplaceTest.php index 9d380ca..32cad7a 100644 --- a/core/modules/user/src/Tests/UserTokenReplaceTest.php +++ b/core/modules/user/src/Tests/UserTokenReplaceTest.php @@ -135,7 +135,7 @@ function testUserTokenReplacement() { // Generate login and cancel link. $tests = array(); $tests['[user:one-time-login-url]'] = user_pass_reset_url($account); - $tests['[user:mail-change-login-url]'] = ChangeEmailController::changeEmailUrl($account); + $tests['[user:mail-change-login-url]'] = ChangeEmailController::changeEmailUrl($account)->toString(); $tests['[user:cancel-url]'] = user_cancel_url($account); // Generate tokens with interface language. diff --git a/core/modules/user/user.module b/core/modules/user/user.module index 0877422..32e2096 100644 --- a/core/modules/user/user.module +++ b/core/modules/user/user.module @@ -948,7 +948,7 @@ function user_mail($key, &$message, $params) { function user_mail_tokens(&$replacements, $data, $options) { if (isset($data['user'])) { $replacements['[user:one-time-login-url]'] = user_pass_reset_url($data['user'], $options); - $replacements['[user:mail-change-login-url]'] = ChangeEmailController::changeEmailUrl($data['user'], $options); + $replacements['[user:mail-change-login-url]'] = ChangeEmailController::changeEmailUrl($data['user'], $options)->toString(); $replacements['[user:cancel-url]'] = user_cancel_url($data['user'], $options); } } diff --git a/core/modules/user/user.routing.yml b/core/modules/user/user.routing.yml index 47677fd..e241dce 100644 --- a/core/modules/user/user.routing.yml +++ b/core/modules/user/user.routing.yml @@ -152,7 +152,7 @@ user.reset: no_cache: TRUE user.change_email: - path: 'user/change-mail/{uid}/{timestamp}/{new_mail}/{hash}' + path: '/user/change-mail/{uid}/{timestamp}/{new_mail}/{hash}' defaults: _controller: '\Drupal\user\Controller\ChangeEmailController::changeEmailPage' _title: 'Change email address'