Issue #1681832 by mcdruid, k_zoltan, dougvann, ZenDoodles, Cyberschorsch,...

Issue #1681832 by mcdruid, k_zoltan, dougvann, ZenDoodles, Cyberschorsch, kim.pepper, mohit1604, drumm, pdenooijer, eshta, mihai_brb, phiit, grwgreg, larowlan, amateescu, David_Rothstein, klausi, Damien Tournoud, coltrane, sun, aiypz, alippai: Password reset form has no flood protection

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()],

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);
       }
     }

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.
    */

core/modules/user/tests/src/Kernel/Controller/UserControllerTest.php

+<?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);
+  }
+
+}

core/modules/user/tests/src/Kernel/Form/UserPasswordFormTest.php

+<?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);
+  }
+
+}