', - '#markup' => $this->t('Password reset instructions will be mailed to %email. You must log out to use the password reset link in the email.', array('%email' => $user->getEmail())), - '#suffix' => '
', + // When a user requests a password reset we check for username and email + // conflicts using a multistep form. + $step_value = $form_state->getValue('step'); + if (empty($step_value)) { + $form['step'] = array( + '#type' => 'hidden', + '#value' => 1, ); + $form_state->setValue('step', 1); } - else { - $form['mail'] = array( - '#prefix' => '', - '#markup' => $this->t('Password reset instructions will be sent to your registered e-mail address.'), - '#suffix' => '
', + + if ($form_state->getValue('step') == 1) { + $form['name'] = array( + '#type' => 'textfield', + '#title' => t('Username or email address'), + '#size' => 60, + '#maxlength' => max(USERNAME_MAX_LENGTH, Email::EMAIL_MAX_LENGTH), + '#required' => TRUE, + '#attributes' => array( + 'autocorrect' => 'off', + 'autocapitalize' => 'off', + 'spellcheck' => 'false', + 'autofocus' => 'autofocus', + ), ); + // Allow logged in users to request this also. + $user = $this->currentUser(); + if ($user->isAuthenticated()) { + $form['name']['#type'] = 'value'; + $form['name']['#value'] = $user->getEmail(); + $form['mail'] = array( + '#prefix' => '', + '#markup' => $this->t('Password reset instructions will be mailed to %email. You must log out to use the password reset link in the e-mail.', array('%email' => $user->getEmail())), + '#suffix' => '
', + ); + } $form['name']['#default_value'] = $this->getRequest()->query->get('name'); } + else { + // Where there is a conflict between the username and email address for two + // users we supply both accounts as an option for the password reset. + $accounts = $form_state->getStorage()['accounts']; + $options = array(); + foreach ($accounts as $account) { + $label = $this->t('The account with the username: @name', array('@name' => $account->getUsername())); + if ($account->getEmail() === $form_state->getStorage()['name']) { + $label = $this->t('The account with the email address: @email', array('@email' => $account->getEmail())); + } + $options[Crypt::hashBase64(Settings::getHashSalt() . $account->id())] = $label; + } + $form['choose_account'] = array( + '#type' => 'radios', + '#title' => t('Choose account'), + '#required' => TRUE, + '#prefix' => "" . $this->t("There is a username conflict with the email address @email. Please select which account password to reset.", array('@email' => $form_state->getStorage()['name'])) . "
", + '#options' => $options, + '#default_value' => Crypt::hashBase64(Settings::getHashSalt() . reset($accounts)->id()), + ); + + $form['step'] = array( + '#type' => 'hidden', + '#value' => 2, + ); + } $form['actions'] = array('#type' => 'actions'); - $form['actions']['submit'] = array('#type' => 'submit', '#value' => $this->t('Submit')); + if ($form_state->getValue('step') == 2) { + $form['actions']['cancel'] = array( + '#type' => 'submit', + '#value' => $this->t('Cancel'), + '#name' => 'cancel', + '#limit_validation_errors' => array(), + '#weight' => 5, + ); + } + $form['actions']['submit'] = array( + '#type' => 'submit', + '#value' => $this->t('Submit'), + '#name' => 'submit' + ); return $form; } @@ -114,25 +163,45 @@ public function buildForm(array $form, FormStateInterface $form_state) { * {@inheritdoc} */ public function validateForm(array &$form, FormStateInterface $form_state) { - $name = trim($form_state->getValue('name')); - // Try to load by email. - $users = $this->userStorage->loadByProperties(array('mail' => $name)); - if (empty($users)) { - // No success, try to load by name. - $users = $this->userStorage->loadByProperties(array('name' => $name)); - } - $account = reset($users); - if ($account && $account->id()) { - // Blocked accounts cannot request a new password. - if (!$account->isActive()) { - $form_state->setErrorByName('name', $this->t('%name is blocked or has not been activated yet.', array('%name' => $name))); + if ($form_state->getValue('step') == 1) { + $name = trim($form_state->getValue('name')); + $accounts = array(); + // Try to load by email. + $users = $this->userStorage->loadByProperties(array('mail' => $name)); + $account_by_email = reset($users); + if ($account_by_email) { + $accounts[Crypt::hashBase64(Settings::getHashSalt() . $account_by_email->id())] = $account_by_email; + } + // Also try to load by user name, but only when the user is not logged in. + $user = $this->currentUser(); + if ($user->id() == 0) { + $users = $this->userStorage->loadByProperties(array('name' => $name)); + $account_by_name = reset($users); + if ($account_by_name) { + $accounts[Crypt::hashBase64(Settings::getHashSalt() . $account_by_name->id())] = $account_by_name; + } + } + if (!empty($accounts)) { + if (count($accounts) == 1) { + $account = reset($accounts); + // Blocked accounts cannot request a new password. + if (!$account->isActive()) { + $form_state->setErrorByName('name', $this->t('%name is blocked or has not been activated yet.', array('%name' => $account->getUsername()))); + } + } + $form_state->setValue('accounts', $accounts); } else { - $form_state->setValueForElement(array('#parents' => array('account')), $account); + $form_state->setErrorByName('name', $this->t('Sorry, %name is not recognized as a username or an e-mail address.', array('%name' => $name))); } } - else { - $form_state->setErrorByName('name', $this->t('Sorry, %name is not recognized as a username or an email address.', array('%name' => $name))); + else if ($form_state->getValue('step') == 2) { + $chosen_account = $form_state->getValue('choose_account'); + $account = $form_state->getStorage()['accounts'][$chosen_account]; + // Blocked accounts cannot request a new password. + if (!$account->isActive()) { + $form_state->setErrorByName('choose_account', $this->t('%name is blocked or has not been activated yet.', array('%name' => $account->getUsername()))); + } } } @@ -140,17 +209,42 @@ public function validateForm(array &$form, FormStateInterface $form_state) { * {@inheritdoc} */ public function submitForm(array &$form, FormStateInterface $form_state) { - $langcode = $this->languageManager->getCurrentLanguage()->getId(); - - $account = $form_state->getValue('account'); - // Mail one time login URL and instructions using current language. - $mail = _user_mail_notify('password_reset', $account, $langcode); - if (!empty($mail)) { - $this->logger('user')->notice('Password reset instructions mailed to %name at %email.', array('%name' => $account->getUsername(), '%email' => $account->getEmail())); - drupal_set_message($this->t('Further instructions have been sent to your email address.')); + $language_interface = \Drupal::languageManager()->getCurrentLanguage(LanguageInterface::TYPE_INTERFACE); + + if ($form_state->getValue('step') == 1) { + $accounts = $form_state->getValue('accounts'); + if (count($accounts) > 1) { + $form_state->setValue('step', 2); + $form_state->setStorage(array( + 'name' => $form_state->getValue('name'), + 'accounts' => $accounts, + )); + $form_state->setRebuild(); + } + else { + $account = reset($accounts); + } + } + else { + if ($form_state->getTriggeringElement()['#name'] == 'submit') { + $chosen_account = $form_state->getValue('choose_account'); + $account = $form_state->getStorage()['accounts'][$chosen_account]; + } + else { + $form_state->setRedirectUrl(Url::fromRoute('user.pass')); + } } + if (isset($account)) { + // Mail one-time login URL and instructions using current language. + $mail = _user_mail_notify('password_reset', $account, $language_interface->getId()); + if (!empty($mail)) { + \Drupal::logger('user')->notice('Password reset instructions mailed to %name at %email.', array('%name' => $account->name, '%email' => $account->mail)); + drupal_set_message($this->t('Further instructions have been sent to your e-mail address.')); + } - $form_state->setRedirect('user.page'); + $form_state->setRedirectUrl(Url::fromRoute('user.page')); + } + return; } } diff --git a/core/modules/user/src/Tests/UserEditTest.php b/core/modules/user/src/Tests/UserEditTest.php index 46502f4..457c169 100644 --- a/core/modules/user/src/Tests/UserEditTest.php +++ b/core/modules/user/src/Tests/UserEditTest.php @@ -102,4 +102,5 @@ function testUserWithoutEmailEdit() { $this->drupalPostForm("user/" . $user1->id() . "/edit", array('mail' => ''), t('Save')); $this->assertRaw(t("The changes have been saved.")); } + } diff --git a/core/modules/user/src/Tests/UserPasswordResetTest.php b/core/modules/user/src/Tests/UserPasswordResetTest.php index 07c7fcf..d8e98d2 100644 --- a/core/modules/user/src/Tests/UserPasswordResetTest.php +++ b/core/modules/user/src/Tests/UserPasswordResetTest.php @@ -7,6 +7,8 @@ namespace Drupal\user\Tests; +use Drupal\Component\Utility\Crypt; +use Drupal\Core\Site\Settings; use Drupal\system\Tests\Cache\PageCacheTagsTestBase; use Drupal\user\Entity\User; @@ -70,6 +72,61 @@ protected function setUp() { } /** + * Attempts to reset a password when an email address matches two accounts. + */ + function testUserPasswordResetDuplicateUsers() { + $user_settings = $this->config('user.settings'); + + // Don't require email validation for new accounts. + $user_settings->set('verify_mail', FALSE)->save(); + + // Don't require admin approval for new accounts. + $user_settings->set('register', USER_REGISTER_VISITORS)->save(); + // Create two users. + $user_with_email = $this->drupalCreateUser(); + $user_with_name = $this->drupalCreateUser(); + + // Change the second user's username to the same value as the first user's + // email address. + $user_with_name->name = $user_with_email->getEmail(); + $user_with_name->save(); + + // Try and reset based on the duplicated email. + $edit = array(); + $edit['name'] = $user_with_email->getEmail(); + $this->drupalPostForm('user/password', $edit, t('Submit')); + // There should be a field prompting the user to pick and account. + $this->assertField('choose_account', 'User is prompted to pick an account when email matches two accounts.'); + // We should be sure to not expose another user's email to the user. + $this->assertNoText($user_with_name->getEmail(), "Duplicated user's email is not exposed to the other user."); + + // Select the account with the username matching the entered email. + $edit = array(); + $edit['choose_account'] = Crypt::hashBase64(Settings::getHashSalt() . $user_with_email->id()); + $this->drupalPostForm(NULL, $edit, t('Submit')); + $this->assertText(t('Further instructions have been sent to your e-mail address.'), 'User is notified that password reset was sent.'); + + // Make sure that right user was sent a reset email. + $this->assertEqual(count($this->drupalGetMails(array('key' => 'password_reset', 'to' => $user_with_email->getEmail()))), 1, 'The right user was sent a password reset mail.'); + // Make sure that the other user was not sent an email. + $this->assertEqual(count($this->drupalGetMails(array('key' => 'password_reset', 'to' => $user_with_name->getEmail()))), 0, 'The other user was not sent a password reset mail.'); + + // If the user is logged in, they should not have to choose an account to + // reset their password. + $this->drupalLogin($user_with_name); + $this->drupalGet('user/password'); + // There should not be a form element for name. + $this->assertNoField('name', 'Duplicate user is not asked for a name when resetting password while logged in.'); + $this->drupalPostForm(NULL, array(), t('Submit')); + // Make sure the user with the matching username was sent an email. + $this->assertText(t('Further instructions have been sent to your e-mail address.'), 'User is notified that password reset was sent when logged in.'); + $this->assertEqual(count($this->drupalGetMails(array('key' => 'password_reset', 'to' => $user_with_name->getEmail()))), 1, 'The right user was sent a password reset mail when logged in.'); + // Make sure that the user with the matching email address was not sent an + // email. (An email was already sent to this user earlier.) + $this->assertEqual(count($this->drupalGetMails(array('key' => 'password_reset', 'to' => $user_with_email->getEmail()))), 1, 'The other user was not sent a password reset mail when logged in.'); + } + + /** * Tests password reset functionality. */ function testUserPasswordReset() { @@ -79,7 +136,7 @@ function testUserPasswordReset() { $edit = array('name' => $this->randomMachineName(32)); $this->drupalPostForm(NULL, $edit, t('Submit')); - $this->assertText(t('Sorry, @name is not recognized as a username or an email address.', array('@name' => $edit['name'])), 'Validation error message shown when trying to request password for invalid account.'); + $this->assertRaw(t('Sorry, %name is not recognized as a username or an e-mail address.', array('%name' => $edit['name'])), 'Validation error message shown when trying to request password for invalid account.'); $this->assertEqual(count($this->drupalGetMails(array('id' => 'user_password_reset'))), 0, 'No email was sent when requesting a password for an invalid account.'); // Reset the password by username via the password reset page. diff --git a/core/modules/user/src/Tests/UserRegistrationTest.php b/core/modules/user/src/Tests/UserRegistrationTest.php index cc5785a..927f40f 100644 --- a/core/modules/user/src/Tests/UserRegistrationTest.php +++ b/core/modules/user/src/Tests/UserRegistrationTest.php @@ -150,6 +150,47 @@ function testRegistrationEmailDuplicates() { $this->assertText(t('The email address @email is already taken.', array('@email' => $duplicate_user->getEmail())), 'Supplying a duplicate email address with added whitespace displays an error message'); } + /** + * Ensure that a new account cannot be created when the username matches an + * existing user's email address, or when the email address matches an + * existing user's username. + */ + function testRegistrationConflicts() { + $user_settings = $this->config('user.settings'); + // Don't require email validation for new accounts. + $user_settings->set('verify_mail', FALSE)->save(); + + // Don't require admin approval for new accounts. + $user_settings->set('register', USER_REGISTER_VISITORS)->save(); + + // Set up a user to check for duplicates. + $duplicate_user = $this->drupalCreateUser(); + + $edit = array(); + $edit['name'] = $duplicate_user->getEmail(); + $edit['mail'] = $this->randomMachineName() . '@example.com'; + $edit['pass[pass1]'] = $password = $this->randomMachineName(); + $edit['pass[pass2]'] = $password; + + // Attempt to create a new account using a username that matches an + // existing email. + $this->drupalPostForm('user/register', $edit, t('Create new account')); + $this->assertText(t('The name @name is already taken.', array('@name' => $edit['name'])), "A user cannot be created when their username matches an existing user's email address."); + + // Change the username to an email address. + $duplicate_user->name = $name = $this->randomMachineName() . '@example.com'; + $duplicate_user->save(); + + $edit = array(); + $edit['name'] = $this->randomMachineName(); + $edit['mail'] = $name; + + // Attempt to create a new account using an email that matches an existing + // username. + $this->drupalPostForm('user/register', $edit, t('Create new account')); + $this->assertText(t('The email address @email is already registered.', array('@email' => $edit['mail'])), "A user cannot be created when their email address matches an existing username."); + } + function testRegistrationDefaultValues() { // Don't require email verification and allow registration by site visitors // without administrator approval. diff --git a/core/modules/user/user.module b/core/modules/user/user.module index cfbfdcd..3e95399 100644 --- a/core/modules/user/user.module +++ b/core/modules/user/user.module @@ -18,6 +18,8 @@ use Drupal\user\Entity\User; use Drupal\user\RoleInterface; use Drupal\user\UserInterface; +use Drupal\Core\Language\LanguageInterface; +use Symfony\Component\HttpKernel\HttpKernelInterface; /** * @file