diff --git a/core/modules/user/src/Controller/UserController.php b/core/modules/user/src/Controller/UserController.php index be15fac..5b9ef00 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\user\Form\UserPasswordResetForm; use Drupal\user\UserDataInterface; use Drupal\user\UserInterface; @@ -49,6 +50,13 @@ class UserController extends ControllerBase { protected $logger; /** + * A flood service instance. + * + * @var Drupal\Core\Flood\FloodInterface + */ + protected $flood; + + /** * Constructs a UserController object. * * @param \Drupal\Core\Datetime\DateFormatterInterface $date_formatter @@ -59,12 +67,15 @@ class UserController extends ControllerBase { * The user data service. * @param \Psr\Log\LoggerInterface $logger * A logger instance. + * @param Drupal\Core\Flood\FloodInterface $flood + * A logger instance. */ - 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) { $this->dateFormatter = $date_formatter; $this->userStorage = $user_storage; $this->userData = $user_data; $this->logger = $logger; + $this->flood = $flood; } /** @@ -75,7 +86,8 @@ public static function create(ContainerInterface $container) { $container->get('date.formatter'), $container->get('entity.manager')->getStorage('user'), $container->get('user.data'), - $container->get('logger.factory')->get('user') + $container->get('logger.factory')->get('user'), + $container->get('flood') ); } @@ -186,6 +198,8 @@ public function getResetPassForm(Request $request, $uid) { /** * Validates user, hash, and timestamp; logs the user in if correct. * + * @param \Symfony\Component\HttpFoundation\Request $request + * The request. * @param int $uid * User ID of the user requesting reset. * @param int $timestamp @@ -201,7 +215,7 @@ public function getResetPassForm(Request $request, $uid) { * @throws \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException * If $uid is for a blocked user or invalid user ID. */ - public function resetPassLogin($uid, $timestamp, $hash) { + public function resetPassLogin(Request $request, $uid, $timestamp, $hash) { // The current user is not logged in, so check the parameters. $current = REQUEST_TIME; /** @var \Drupal\user\UserInterface $user */ @@ -222,6 +236,21 @@ public function resetPassLogin($uid, $timestamp, $hash) { return $this->redirect('user.pass'); } elseif ($user->isAuthenticated() && ($timestamp >= $user->getLastLoginTime()) && ($timestamp <= $current) && Crypt::hashEquals($hash, user_pass_rehash($user, $timestamp))) { + $flood_config = $this->config('user.flood'); + if ($flood_config->get('uid_only')) { + // Register flood events based on the uid only, so they apply for any + // IP address. This is the most secure option. + $identifier = $user->id(); + } + else { + // The default identifier is a combination of uid and IP address. This + // is less secure but more resistant to denial-of-service attacks that + // could lock out all users with public user names. + $identifier = $user->id() . '-' . $request->getClientIP(); + } + $this->flood->clear('user.failed_login_ip'); + $this->flood->clear('user.failed_login_user', $identifier); + user_login_finalize($user); $this->logger->notice('User %name used one-time login link at time %timestamp.', ['%name' => $user->getDisplayName(), '%timestamp' => $timestamp]); drupal_set_message($this->t('You have just used your one-time login link. It is no longer necessary to use this link to log in. Please change your password.')); diff --git a/core/modules/user/src/Tests/UserLoginTest.php b/core/modules/user/src/Tests/UserLoginTest.php index d92c1c7..2177d26 100644 --- a/core/modules/user/src/Tests/UserLoginTest.php +++ b/core/modules/user/src/Tests/UserLoginTest.php @@ -63,16 +63,24 @@ public function testGlobalLoginFloodControl() { // A login with the correct password should also result in a flood error // message. $this->assertFailedLogin($user1, 'ip'); + $this->resetUserPassword($user1); + $this->drupalLogout(); + + // Try to login as user 1, it should be successful. + $this->drupalLogin($user1); + $this->assertNoRaw('Too many failed login attempts from your IP address.'); } /** * Test the per-user login flood control. */ public function testPerUserLoginFloodControl() { + $user_limit = 3; + $this->config('user.flood') // Set a high global limit out so that it is not relevant in the test. ->set('ip_limit', 4000) - ->set('user_limit', 3) + ->set('user_limit', $user_limit) ->save(); $user1 = $this->drupalCreateUser([]); @@ -103,6 +111,12 @@ public function testPerUserLoginFloodControl() { // Try one more attempt for user 1, it should be rejected, even if the // correct password has been used. $this->assertFailedLogin($user1, 'user'); + $this->resetUserPassword($user1); + $this->drupalLogout(); + + // Try to login as user 1, it should be successful. + $this->drupalLogin($user1); + $this->assertNoRaw('There have been more than ' . $user_limit . ' failed login attempts for this account.'); } /** @@ -159,20 +173,39 @@ public function assertFailedLogin($account, $flood_trigger = NULL) { 'name' => $account->getUsername(), 'pass' => $account->pass_raw, ]; - $this->drupalPostForm('user/login', $edit, t('Log in')); + $this->drupalPostForm('user/login', $edit, 'Log in'); $this->assertNoFieldByXPath("//input[@name='pass' and @value!='']", NULL, 'Password value attribute is blank.'); if (isset($flood_trigger)) { if ($flood_trigger == 'user') { - $this->assertRaw(\Drupal::translation()->formatPlural($this->config('user.flood')->get('user_limit'), 'There has been more than one failed login attempt for this account. It is temporarily blocked. Try again later or request a new password.', 'There have been more than @count failed login attempts for this account. It is temporarily blocked. Try again later or request a new password.', [':url' => \Drupal::url('user.pass')])); + $this->assertRaw('There have been more than ' . $this->config('user.flood')->get('user_limit') . ' failed login attempts for this account.'); } else { // No uid, so the limit is IP-based. - $this->assertRaw(t('Too many failed login attempts from your IP address. This IP address is temporarily blocked. Try again later or request a new password.', [':url' => \Drupal::url('user.pass')])); + $this->assertRaw('Too many failed login attempts from your IP address. This IP address is temporarily blocked.'); } } else { - $this->assertText(t('Unrecognized username or password. Forgot your password?')); + $this->assertText('Unrecognized username or password. Forgot your password?'); } } + /** + * Reset user password. + * + * @param object $user + * A user object. + */ + public function resetUserPassword($user) { + $this->drupalGet('user/password'); + $edit['name'] = $user->getUsername(); + $this->drupalPostForm(NULL, $edit, 'Submit'); + $_emails = $this->drupalGetMails(); + $email = end($_emails); + $urls = []; + preg_match('#.+user/reset/.+#', $email['body'], $urls); + $resetURL = $urls[0]; + $this->drupalGet($resetURL); + $this->drupalPostForm(NULL, NULL, 'Log in'); + } + }