Issue #1521996 by quietone, dww, Sophie.SK, Carsten Müller, Spokje, kim.pepper, izus, mjpa, ashu1629, AlanHDev, mangy.fox, Vivek Panicker, robpowell, jozzy_a, tar_inet, ravi.shankar, sokru, zuhair_ak, cjgratacos, aerozeppelin, darrenwh, lomasr, luismagr, mohit1604, ronald.garcia, matteodem, webchuck, gordon, nginex, jain_deepak, Aunion, ankitgarg, Elendev, Kristen Pol, alexpott, YesCT, dpi, no2e, larowlan, longwave, markpavlitski, acbramley, C_Logemann, cilefen, claudiu.cristea, penyaskito, David_Rothstein, Leeteq, anagp, jlbellido, pdrake, klausi: Password reset form reveals whether an email or username is in use

merge-requests/259/head
Alex Pott 2021-01-16 10:54:57 +00:00
parent d99b12fb9e
commit 5f0435769a
No known key found for this signature in database
GPG Key ID: 31905460D4A69276
2 changed files with 128 additions and 53 deletions

View File

@ -2,12 +2,15 @@
namespace Drupal\user\Form;
use Drupal\Component\Utility\EmailValidatorInterface;
use Drupal\Core\Config\ConfigFactory;
use Drupal\Core\Field\BaseFieldDefinition;
use Drupal\Core\Flood\FloodInterface;
use Drupal\Core\Form\FormBase;
use Drupal\Core\Form\FormStateInterface;
use Drupal\Core\Language\LanguageManagerInterface;
use Drupal\Core\Render\Element\Email;
use Drupal\Core\TypedData\TypedDataManagerInterface;
use Drupal\user\UserInterface;
use Drupal\user\UserStorageInterface;
use Symfony\Component\DependencyInjection\ContainerInterface;
@ -42,6 +45,20 @@ class UserPasswordForm extends FormBase {
*/
protected $flood;
/**
* The typed data manager.
*
* @var \Drupal\Core\TypedData\TypedDataManagerInterface
*/
protected $typedDataManager;
/**
* The email validator service.
*
* @var \Drupal\Component\Utility\EmailValidatorInterface
*/
protected $emailValidator;
/**
* Constructs a UserPasswordForm object.
*
@ -53,12 +70,26 @@ class UserPasswordForm extends FormBase {
* The config factory.
* @param \Drupal\Core\Flood\FloodInterface $flood
* The flood service.
* @param \Drupal\Core\TypedData\TypedDataManagerInterface $typed_data_manager
* The typed data manager.
* @param \Drupal\Component\Utility\EmailValidatorInterface $email_validator
* The email validator service.
*/
public function __construct(UserStorageInterface $user_storage, LanguageManagerInterface $language_manager, ConfigFactory $config_factory, FloodInterface $flood) {
public function __construct(UserStorageInterface $user_storage, LanguageManagerInterface $language_manager, ConfigFactory $config_factory, FloodInterface $flood, TypedDataManagerInterface $typed_data_manager = NULL, EmailValidatorInterface $email_validator = NULL) {
$this->userStorage = $user_storage;
$this->languageManager = $language_manager;
$this->configFactory = $config_factory;
$this->flood = $flood;
if (is_null($typed_data_manager)) {
@trigger_error('Calling ' . __METHOD__ . ' without the $typed_data_manager argument is deprecated in drupal:9.2.0 and will be required in drupal:10.0.0. See https://www.drupal.org/node/3189310', E_USER_DEPRECATED);
$typed_data_manager = \Drupal::typedDataManager();
}
$this->typedDataManager = $typed_data_manager;
if (is_null($email_validator)) {
@trigger_error('Calling ' . __METHOD__ . ' without the $email_validator argument is deprecated in drupal:9.2.0 and will be required in drupal:10.0.0. See https://www.drupal.org/node/3189310', E_USER_DEPRECATED);
$email_validator = \Drupal::service('email.validator');
}
$this->emailValidator = $email_validator;
}
/**
@ -69,7 +100,9 @@ class UserPasswordForm extends FormBase {
$container->get('entity_type.manager')->getStorage('user'),
$container->get('language_manager'),
$container->get('config.factory'),
$container->get('flood')
$container->get('flood'),
$container->get('typed_data_manager'),
$container->get('email.validator')
);
}
@ -133,7 +166,20 @@ class UserPasswordForm extends FormBase {
return;
}
$this->flood->register('user.password_request_ip', $flood_config->get('ip_window'));
// First, see if the input is possibly valid as a username.
$name = trim($form_state->getValue('name'));
$definition = BaseFieldDefinition::create('string')
->addConstraint('UserName', []);
$data = $this->typedDataManager->create($definition);
$data->setValue($name);
$violations = $data->validate();
// Usernames have a maximum length shorter than email addresses. Only print
// this error if the input is not valid as a username or email address.
if ($violations->count() > 0 && !$this->emailValidator->isValid($name)) {
$form_state->setErrorByName('name', $this->t("The username or email address is invalid."));
return;
}
// Try to load by email.
$users = $this->userStorage->loadByProperties(['mail' => $name]);
if (empty($users)) {
@ -141,26 +187,17 @@ class UserPasswordForm extends FormBase {
$users = $this->userStorage->loadByProperties(['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.', ['%name' => $name]));
// Blocked accounts cannot request a new password.
if ($account && $account->id() && $account->isActive()) {
// Register flood events based on the uid only, so they apply for any
// IP address. This allows them to be cleared on successful reset (from
// any IP).
$identifier = $account->id();
if (!$this->flood->isAllowed('user.password_request_user', $flood_config->get('user_limit'), $flood_config->get('user_window'), $identifier)) {
return;
}
else {
// Register flood events based on the uid only, so they apply for any
// IP address. This allows them to be cleared on successful reset (from
// any IP).
$identifier = $account->id();
if (!$this->flood->isAllowed('user.password_request_user', $flood_config->get('user_limit'), $flood_config->get('user_window'), $identifier)) {
$form_state->setErrorByName('name', $this->t('Too many password recovery requests for this account. It is temporarily blocked. Try again later or contact the site administrator.'));
return;
}
$this->flood->register('user.password_request_user', $flood_config->get('user_window'), $identifier);
$form_state->setValueForElement(['#parents' => ['account']], $account);
}
}
else {
$form_state->setErrorByName('name', $this->t('%name is not recognized as a username or an email address.', ['%name' => $name]));
$this->flood->register('user.password_request_user', $flood_config->get('user_window'), $identifier);
$form_state->setValueForElement(['#parents' => ['account']], $account);
}
}
@ -169,12 +206,29 @@ class UserPasswordForm extends FormBase {
*/
public function submitForm(array &$form, FormStateInterface $form_state) {
$account = $form_state->getValue('account');
// Mail one time login URL and instructions using current language.
$mail = _user_mail_notify('password_reset', $account);
if (!empty($mail)) {
$this->logger('user')->notice('Password reset instructions mailed to %name at %email.', ['%name' => $account->getAccountName(), '%email' => $account->getEmail()]);
$this->messenger()->addStatus($this->t('Further instructions have been sent to your email address.'));
if ($account) {
// Mail one time login URL and instructions using current language.
$mail = _user_mail_notify('password_reset', $account);
if (!empty($mail)) {
$this->logger('user')
->notice('Password reset instructions mailed to %name at %email.', [
'%name' => $account->getAccountName(),
'%email' => $account->getEmail(),
]);
}
}
else {
$this->logger('user')
->notice('Password reset form was submitted with an unknown or inactive account: %name.', [
'%name' => $form_state->getValue('name'),
]);
}
// Make sure the status text is displayed even if no email was sent. This
// message is deliberately the same as the success message for privacy.
$this->messenger()
->addStatus($this->t('If %identifier is a valid account, an email will be sent with instructions to reset your password.', [
'%identifier' => $form_state->getValue('name'),
]));
$form_state->setRedirect('<front>');
}

View File

@ -9,6 +9,7 @@ use Drupal\Core\Url;
use Drupal\language\Entity\ConfigurableLanguage;
use Drupal\Tests\BrowserTestBase;
use Drupal\user\Entity\User;
use Drupal\user\UserInterface;
/**
* Ensure that password reset methods work as expected.
@ -87,11 +88,29 @@ class UserPasswordResetTest extends BrowserTestBase {
$this->drupalGet(Url::fromRoute('user.reset.form', ['uid' => $this->account->id()]));
$this->assertSession()->statusCodeEquals(403);
// Try to reset the password for a completely invalid username.
$this->drupalGet('user/password');
$long_name = $this->randomMachineName(UserInterface::USERNAME_MAX_LENGTH + 10);
$edit = ['name' => $long_name];
$this->submitForm($edit, 'Submit');
$this->assertCount(0, $this->drupalGetMails(['id' => 'user_password_reset']), 'No e-mail was sent when requesting a password for an invalid user name.');
$this->assertSession()->pageTextContains("The username or email address is invalid.");
// Try to reset the password for an invalid account.
$this->drupalGet('user/password');
$edit = ['name' => $this->randomMachineName()];
$random_name = $this->randomMachineName();
$edit = ['name' => $random_name];
$this->submitForm($edit, 'Submit');
$this->assertNoValidPasswordReset($edit['name']);
$this->assertNoValidPasswordReset($random_name);
// Try to reset the password for a valid email address longer than
// UserInterface::USERNAME_MAX_LENGTH (invalid username, valid email).
// This should pass validation and print the generic message.
$this->drupalGet('user/password');
$long_name = $this->randomMachineName(UserInterface::USERNAME_MAX_LENGTH) . '@example.com';
$edit = ['name' => $long_name];
$this->submitForm($edit, 'Submit');
$this->assertNoValidPasswordReset($long_name);
// Reset the password by username via the password reset page.
$this->drupalGet('user/password');
@ -175,7 +194,6 @@ class UserPasswordResetTest extends BrowserTestBase {
$before = count($this->drupalGetMails(['id' => 'user_password_reset']));
$edit = ['name' => $blocked_account->getAccountName()];
$this->submitForm($edit, 'Submit');
$this->assertRaw(t('%name is blocked or has not been activated yet.', ['%name' => $blocked_account->getAccountName()]));
$this->assertCount($before, $this->drupalGetMails(['id' => 'user_password_reset']), '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.
@ -379,18 +397,25 @@ class UserPasswordResetTest extends BrowserTestBase {
$edit = ['name' => $this->account->getAccountName()];
// Count email messages before to compare with after.
$before = count($this->drupalGetMails(['id' => 'user_password_reset']));
// Try 3 requests that should not trigger flood control.
for ($i = 0; $i < 3; $i++) {
$this->drupalGet('user/password');
$this->submitForm($edit, 'Submit');
$this->assertValidPasswordReset($edit['name']);
$this->assertNoPasswordUserFlood();
}
// Ensure 3 emails were sent.
$this->assertCount($before + 3, $this->drupalGetMails(['id' => 'user_password_reset']), '3 emails sent without triggering flood control.');
// The next request should trigger flood control.
$this->drupalGet('user/password');
$this->submitForm($edit, 'Submit');
$this->assertPasswordUserFlood();
// Ensure no further emails were sent.
$this->assertCount($before + 3, $this->drupalGetMails(['id' => 'user_password_reset']), 'No further email was sent after triggering flood control.');
}
/**
@ -404,10 +429,11 @@ class UserPasswordResetTest extends BrowserTestBase {
// Try 3 requests that should not trigger flood control.
for ($i = 0; $i < 3; $i++) {
$this->drupalGet('user/password');
$edit = ['name' => $this->randomMachineName()];
$random_name = $this->randomMachineName();
$edit = ['name' => $random_name];
$this->submitForm($edit, 'Submit');
// Because we're testing with a random name, the password reset will not be valid.
$this->assertNoValidPasswordReset($edit['name']);
$this->assertNoValidPasswordReset($random_name);
$this->assertNoPasswordIpFlood();
}
@ -428,14 +454,19 @@ class UserPasswordResetTest extends BrowserTestBase {
$edit = ['name' => $this->account->getAccountName()];
// Count email messages before to compare with after.
$before = count($this->drupalGetMails(['id' => 'user_password_reset']));
// Try 3 requests that should not trigger flood control.
for ($i = 0; $i < 3; $i++) {
$this->drupalGet('user/password');
$this->submitForm($edit, 'Submit');
$this->assertValidPasswordReset($edit['name']);
$this->assertNoPasswordUserFlood();
}
// Ensure 3 emails were sent.
$this->assertCount($before + 3, $this->drupalGetMails(['id' => 'user_password_reset']), '3 emails sent without triggering flood control.');
// Use the last password reset URL which was generated.
$reset_url = $this->getResetURL();
$this->drupalGet($reset_url . '/login');
@ -448,15 +479,16 @@ class UserPasswordResetTest extends BrowserTestBase {
$this->drupalGet('user/password');
$this->submitForm($edit, 'Submit');
$this->assertValidPasswordReset($edit['name']);
$this->assertNoPasswordUserFlood();
// Ensure another email was sent.
$this->assertCount($before + 4, $this->drupalGetMails(['id' => 'user_password_reset']), 'Another email was sent after clearing flood control.');
}
/**
* Helper function to make assertions about a valid password reset.
*/
public function assertValidPasswordReset($name) {
// Make sure the error text is not displayed and email sent.
$this->assertNoText("Sorry, $name is not recognized as a username or an e-mail address.");
$this->assertSession()->pageTextContains("If $name is a valid account, an email will be sent with instructions to reset your password.");
$this->assertMail('to', $this->account->getEmail(), 'Password e-mail sent to user.');
$subject = t('Replacement login information for @username at @site', ['@username' => $this->account->getAccountName(), '@site' => \Drupal::config('system.site')->get('name')]);
$this->assertMail('subject', $subject, 'Password reset e-mail subject is correct.');
@ -464,27 +496,16 @@ class UserPasswordResetTest extends BrowserTestBase {
/**
* Helper function to make assertions about an invalid password reset.
*
* @param string $name
*/
public function assertNoValidPasswordReset($name) {
// Make sure the error text is displayed and no email sent.
$this->assertText($name . ' is not recognized as a username or an email address.');
// This message is the same as the valid reset for privacy reasons.
$this->assertSession()->pageTextContains("If $name is a valid account, an email will be sent with instructions to reset your password.");
// The difference is that no email is sent.
$this->assertCount(0, $this->drupalGetMails(['id' => 'user_password_reset']), 'No e-mail was sent when requesting a password for an invalid account.');
}
/**
* Makes assertions about a password reset triggering user flood control.
*/
public function assertPasswordUserFlood() {
$this->assertText('Too many password recovery requests for this account. It is temporarily blocked. Try again later or contact the site administrator.');
}
/**
* Makes assertions about a password reset not triggering user flood control.
*/
public function assertNoPasswordUserFlood() {
$this->assertNoText('Too many password recovery requests for this account. It is temporarily blocked. Try again later or contact the site administrator.');
}
/**
* Makes assertions about a password reset triggering IP flood control.
*/