diff --git a/core/modules/user/src/Controller/UserController.php b/core/modules/user/src/Controller/UserController.php index a972bd23f100d2713e98b1bb4ac2cd538145cb8c..7516da93c6e90fd9ee06db75825ad866a9f582e3 100644 --- a/core/modules/user/src/Controller/UserController.php +++ b/core/modules/user/src/Controller/UserController.php @@ -6,6 +6,7 @@ use Drupal\Component\Utility\Xss; use Drupal\Core\Controller\ControllerBase; use Drupal\Core\Datetime\DateFormatterInterface; +use Drupal\Core\Flood\FloodInterface; use Drupal\Core\Url; use Drupal\user\Form\UserPasswordResetForm; use Drupal\user\UserDataInterface; @@ -49,6 +50,13 @@ class UserController extends ControllerBase { */ protected $logger; + /** + * The flood service. + * + * @var \Drupal\Core\Flood\FloodInterface + */ + protected $flood; + /** * Constructs a UserController object. * @@ -60,12 +68,19 @@ class UserController extends ControllerBase { * The user data service. * @param \Psr\Log\LoggerInterface $logger * A logger instance. + * @param \Drupal\Core\Flood\FloodInterface $flood + * The flood service. */ - public function __construct(DateFormatterInterface $date_formatter, UserStorageInterface $user_storage, UserDataInterface $user_data, LoggerInterface $logger) { + public function __construct(DateFormatterInterface $date_formatter, UserStorageInterface $user_storage, UserDataInterface $user_data, LoggerInterface $logger, FloodInterface $flood = NULL) { $this->dateFormatter = $date_formatter; $this->userStorage = $user_storage; $this->userData = $user_data; $this->logger = $logger; + if (!$flood) { + @trigger_error('Calling ' . __METHOD__ . ' without the $flood parameter is deprecated in drupal:8.8.0 and is required in drupal:9.0.0. See https://www.drupal.org/node/1681832', E_USER_DEPRECATED); + $flood = \Drupal::service('flood'); + } + $this->flood = $flood; } /** @@ -76,7 +91,8 @@ public static function create(ContainerInterface $container) { $container->get('date.formatter'), $container->get('entity_type.manager')->getStorage('user'), $container->get('user.data'), - $container->get('logger.factory')->get('user') + $container->get('logger.factory')->get('user'), + $container->get('flood') ); } @@ -235,6 +251,8 @@ public function resetPassLogin($uid, $timestamp, $hash) { // check. $token = Crypt::randomBytesBase64(55); $_SESSION['pass_reset_' . $user->id()] = $token; + // Clear any flood events for this user. + $this->flood->clear('user.password_request_user', $uid); return $this->redirect( 'entity.user.edit_form', ['user' => $user->id()], diff --git a/core/modules/user/src/Form/UserPasswordForm.php b/core/modules/user/src/Form/UserPasswordForm.php index 8291ad5de4c29f378025e133e60f59316e555ef7..abe0b6226f3c0c341f269fb5754f108fc46984f5 100644 --- a/core/modules/user/src/Form/UserPasswordForm.php +++ b/core/modules/user/src/Form/UserPasswordForm.php @@ -2,6 +2,8 @@ namespace Drupal\user\Form; +use Drupal\Core\Config\ConfigFactory; +use Drupal\Core\Flood\FloodInterface; use Drupal\Core\Form\FormBase; use Drupal\Core\Form\FormStateInterface; use Drupal\Core\Language\LanguageManagerInterface; @@ -33,6 +35,13 @@ class UserPasswordForm extends FormBase { */ protected $languageManager; + /** + * The flood service. + * + * @var \Drupal\Core\Flood\FloodInterface + */ + protected $flood; + /** * Constructs a UserPasswordForm object. * @@ -40,10 +49,24 @@ class UserPasswordForm extends FormBase { * The user storage. * @param \Drupal\Core\Language\LanguageManagerInterface $language_manager * The language manager. + * @param \Drupal\Core\Config\ConfigFactory $config_factory + * The config factory. + * @param \Drupal\Core\Flood\FloodInterface $flood + * The flood service. */ - public function __construct(UserStorageInterface $user_storage, LanguageManagerInterface $language_manager) { + public function __construct(UserStorageInterface $user_storage, LanguageManagerInterface $language_manager, ConfigFactory $config_factory = NULL, FloodInterface $flood = NULL) { $this->userStorage = $user_storage; $this->languageManager = $language_manager; + if (!$config_factory) { + @trigger_error('Calling ' . __METHOD__ . ' without the $config_factory is deprecated in drupal:8.8.0 and is required before drupal:9.0.0. See https://www.drupal.org/node/1681832', E_USER_DEPRECATED); + $config_factory = \Drupal::configFactory(); + } + $this->configFactory = $config_factory; + if (!$flood) { + @trigger_error('Calling ' . __METHOD__ . ' without the $flood parameter is deprecated in drupal:8.8.0 and is required before drupal:9.0.0. See https://www.drupal.org/node/1681832', E_USER_DEPRECATED); + $flood = \Drupal::service('flood'); + } + $this->flood = $flood; } /** @@ -52,7 +75,9 @@ public function __construct(UserStorageInterface $user_storage, LanguageManagerI public static function create(ContainerInterface $container) { return new static( $container->get('entity_type.manager')->getStorage('user'), - $container->get('language_manager') + $container->get('language_manager'), + $container->get('config.factory'), + $container->get('flood') ); } @@ -110,6 +135,12 @@ public function buildForm(array $form, FormStateInterface $form_state) { * {@inheritdoc} */ public function validateForm(array &$form, FormStateInterface $form_state) { + $flood_config = $this->configFactory->get('user.flood'); + if (!$this->flood->isAllowed('user.password_request_ip', $flood_config->get('ip_limit'), $flood_config->get('ip_window'))) { + $form_state->setErrorByName('name', $this->t('Too many password recovery requests from your IP address. It is temporarily blocked. Try again later or contact the site administrator.')); + return; + } + $this->flood->register('user.password_request_ip', $flood_config->get('ip_window')); $name = trim($form_state->getValue('name')); // Try to load by email. $users = $this->userStorage->loadByProperties(['mail' => $name]); @@ -124,6 +155,15 @@ public function validateForm(array &$form, FormStateInterface $form_state) { $form_state->setErrorByName('name', $this->t('%name is blocked or has not been activated yet.', ['%name' => $name])); } 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); } } diff --git a/core/modules/user/tests/src/Functional/UserPasswordResetTest.php b/core/modules/user/tests/src/Functional/UserPasswordResetTest.php index 866b2d324287db619b85947351ea98e8239a5f07..1bcbd1d1416d89253c0141d569237c295864b19e 100644 --- a/core/modules/user/tests/src/Functional/UserPasswordResetTest.php +++ b/core/modules/user/tests/src/Functional/UserPasswordResetTest.php @@ -72,21 +72,15 @@ public function testUserPasswordReset() { // Try to reset the password for an invalid account. $this->drupalGet('user/password'); - - $edit = ['name' => $this->randomMachineName(32)]; + $edit = ['name' => $this->randomMachineName()]; $this->drupalPostForm(NULL, $edit, t('Submit')); - - $this->assertText(t('@name is not recognized as a username or an email address.', ['@name' => $edit['name']]), 'Validation error message shown when trying to request password for invalid account.'); - $this->assertEqual(count($this->drupalGetMails(['id' => 'user_password_reset'])), 0, 'No email was sent when requesting a password for an invalid account.'); + $this->assertNoValidPasswordReset($edit['name']); // Reset the password by username via the password reset page. - $edit['name'] = $this->account->getAccountName(); + $this->drupalGet('user/password'); + $edit = ['name' => $this->account->getAccountName()]; $this->drupalPostForm(NULL, $edit, t('Submit')); - - // Verify that the user was sent an email. - $this->assertMail('to', $this->account->getEmail(), 'Password email sent to user.'); - $subject = t('Replacement login information for @username at @site', ['@username' => $this->account->getAccountName(), '@site' => $this->config('system.site')->get('name')]); - $this->assertMail('subject', $subject, 'Password reset email subject is correct.'); + $this->assertValidPasswordReset($edit['name']); $resetURL = $this->getResetURL(); $this->drupalGet($resetURL); @@ -126,11 +120,12 @@ public function testUserPasswordReset() { $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.'); // Request a new password again, this time using the email address. - $this->drupalGet('user/password'); // Count email messages before to compare with after. $before = count($this->drupalGetMails(['id' => 'user_password_reset'])); + $this->drupalGet('user/password'); $edit = ['name' => $this->account->getEmail()]; $this->drupalPostForm(NULL, $edit, t('Submit')); + $this->assertValidPasswordReset($edit['name']); $this->assertTrue(count($this->drupalGetMails(['id' => 'user_password_reset'])) === $before + 1, 'Email sent when requesting password reset using email address.'); // Visit the user edit page without pass-reset-token and make sure it does @@ -284,6 +279,136 @@ public function testUserResetPasswordTextboxFilled() { $this->assertNoFieldByName('name', $edit['name'], 'User name not found.'); } + /** + * Tests password reset flood control for one user. + */ + public function testUserResetPasswordUserFloodControl() { + \Drupal::configFactory()->getEditable('user.flood') + ->set('user_limit', 3) + ->save(); + + $edit = ['name' => $this->account->getAccountName()]; + + // Try 3 requests that should not trigger flood control. + for ($i = 0; $i < 3; $i++) { + $this->drupalGet('user/password'); + $this->drupalPostForm(NULL, $edit, t('Submit')); + $this->assertValidPasswordReset($edit['name']); + $this->assertNoPasswordUserFlood(); + } + + // The next request should trigger flood control. + $this->drupalGet('user/password'); + $this->drupalPostForm(NULL, $edit, t('Submit')); + $this->assertPasswordUserFlood(); + } + + /** + * Tests password reset flood control for one IP. + */ + public function testUserResetPasswordIpFloodControl() { + \Drupal::configFactory()->getEditable('user.flood') + ->set('ip_limit', 3) + ->save(); + + // Try 3 requests that should not trigger flood control. + for ($i = 0; $i < 3; $i++) { + $this->drupalGet('user/password'); + $edit = ['name' => $this->randomMachineName()]; + $this->drupalPostForm(NULL, $edit, t('Submit')); + // Because we're testing with a random name, the password reset will not be valid. + $this->assertNoValidPasswordReset($edit['name']); + $this->assertNoPasswordIpFlood(); + } + + // The next request should trigger flood control. + $this->drupalGet('user/password'); + $edit = ['name' => $this->randomMachineName()]; + $this->drupalPostForm(NULL, $edit, t('Submit')); + $this->assertPasswordIpFlood(); + } + + /** + * Tests user password reset flood control is cleared on successful reset. + */ + public function testUserResetPasswordUserFloodControlIsCleared() { + \Drupal::configFactory()->getEditable('user.flood') + ->set('user_limit', 3) + ->save(); + + $edit = ['name' => $this->account->getAccountName()]; + + // Try 3 requests that should not trigger flood control. + for ($i = 0; $i < 3; $i++) { + $this->drupalGet('user/password'); + $this->drupalPostForm(NULL, $edit, t('Submit')); + $this->assertValidPasswordReset($edit['name']); + $this->assertNoPasswordUserFlood(); + } + + // Use the last password reset URL which was generated. + $reset_url = $this->getResetURL(); + $this->drupalGet($reset_url . '/login'); + $this->assertLink(t('Log out')); + $this->assertTitle(t('@name | @site', ['@name' => $this->account->getAccountName(), '@site' => $this->config('system.site')->get('name')]), 'Logged in using password reset link.'); + $this->drupalLogout(); + + // The next request should *not* trigger flood control, since a successful + // password reset should have cleared flood events for this user. + $this->drupalGet('user/password'); + $this->drupalPostForm(NULL, $edit, t('Submit')); + $this->assertValidPasswordReset($edit['name']); + $this->assertNoPasswordUserFlood(); + } + + /** + * 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(t('Sorry, @name is not recognized as a username or an e-mail address.', ['@name' => $name]), 'Validation error message shown when trying to request password for invalid account.'); + $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.'); + } + + /** + * Helper function to make assertions about an invalid password reset. + */ + public function assertNoValidPasswordReset($name) { + // Make sure the error text is displayed and no email sent. + $this->assertText(t('@name is not recognized as a username or an email address.', ['@name' => $name]), 'Validation error message shown when trying to request password for invalid account.'); + $this->assertEqual(count($this->drupalGetMails(['id' => 'user_password_reset'])), 0, 'No e-mail was sent when requesting a password for an invalid account.'); + } + + /** + * Helper function to make assertions about a password reset triggering user flood cotrol. + */ + public function assertPasswordUserFlood() { + $this->assertText(t('Too many password recovery requests for this account. It is temporarily blocked. Try again later or contact the site administrator.'), 'User password reset flood error message shown.'); + } + + /** + * Helper function to make assertions about a password reset not triggering user flood control. + */ + public function assertNoPasswordUserFlood() { + $this->assertNoText(t('Too many password recovery requests for this account. It is temporarily blocked. Try again later or contact the site administrator.'), 'User password reset flood error message not shown.'); + } + + /** + * Helper function to make assertions about a password reset triggering IP flood cotrol. + */ + public function assertPasswordIpFlood() { + $this->assertText(t('Too many password recovery requests from your IP address. It is temporarily blocked. Try again later or contact the site administrator.'), 'IP password reset flood error message shown.'); + } + + /** + * Helper function to make assertions about a password reset not triggering IP flood control. + */ + public function assertNoPasswordIpFlood() { + $this->assertNoText(t('Too many password recovery requests from your IP address. It is temporarily blocked. Try again later or contact the site administrator.'), 'IP password reset flood error message not shown.'); + } + /** * Make sure that users cannot forge password reset URLs of other users. */ diff --git a/core/modules/user/tests/src/Kernel/Controller/UserControllerTest.php b/core/modules/user/tests/src/Kernel/Controller/UserControllerTest.php new file mode 100644 index 0000000000000000000000000000000000000000..f2f0146654b6b373cbe07306ac6e92b7483e4360 --- /dev/null +++ b/core/modules/user/tests/src/Kernel/Controller/UserControllerTest.php @@ -0,0 +1,36 @@ +<?php + +namespace Drupal\Tests\user\Kernel\Controller; + +use Drupal\Core\Datetime\DateFormatterInterface; +use Drupal\KernelTests\KernelTestBase; +use Drupal\user\Controller\UserController; +use Drupal\user\UserDataInterface; +use Drupal\user\UserStorageInterface; +use Psr\Log\LoggerInterface; + +/** + * @coversDefaultClass \Drupal\user\Controller\UserController + * @group user + */ +class UserControllerTest extends KernelTestBase { + + /** + * @group legacy + * @expectedDeprecation Calling Drupal\user\Controller\UserController::__construct without the $flood parameter is deprecated in drupal:8.8.0 and is required in drupal:9.0.0. See https://www.drupal.org/node/1681832 + */ + public function testConstructorDeprecations() { + $date_formatter = $this->prophesize(DateFormatterInterface::class); + $user_storage = $this->prophesize(UserStorageInterface::class); + $user_data = $this->prophesize(UserDataInterface::class); + $logger = $this->prophesize(LoggerInterface::class); + $controller = new UserController( + $date_formatter->reveal(), + $user_storage->reveal(), + $user_data->reveal(), + $logger->reveal() + ); + $this->assertNotNull($controller); + } + +} diff --git a/core/modules/user/tests/src/Kernel/Form/UserPasswordFormTest.php b/core/modules/user/tests/src/Kernel/Form/UserPasswordFormTest.php new file mode 100644 index 0000000000000000000000000000000000000000..0876677cc889dc4ebfdc5ab627262ce06222de1a --- /dev/null +++ b/core/modules/user/tests/src/Kernel/Form/UserPasswordFormTest.php @@ -0,0 +1,28 @@ +<?php + +namespace Drupal\Tests\user\Kernel\Form; + +use Drupal\Core\Language\LanguageManagerInterface; +use Drupal\KernelTests\KernelTestBase; +use Drupal\user\Form\UserPasswordForm; +use Drupal\user\UserStorageInterface; + +/** + * @coversDefaultClass \Drupal\user\Form\UserPasswordForm + * @group user + */ +class UserPasswordFormTest extends KernelTestBase { + + /** + * @group legacy + * @expectedDeprecation Calling Drupal\user\Form\UserPasswordForm::__construct without the $config_factory is deprecated in drupal:8.8.0 and is required before drupal:9.0.0. See https://www.drupal.org/node/1681832 + * @expectedDeprecation Calling Drupal\user\Form\UserPasswordForm::__construct without the $flood parameter is deprecated in drupal:8.8.0 and is required before drupal:9.0.0. See https://www.drupal.org/node/1681832 + */ + public function testConstructorDeprecations() { + $user_storage = $this->prophesize(UserStorageInterface::class); + $language_manager = $this->prophesize(LanguageManagerInterface::class); + $form = new UserPasswordForm($user_storage->reveal(), $language_manager->reveal()); + $this->assertNotNull($form); + } + +}