From 9461f4ab6f58d4ec40fd2bdf2d4772caeaecb0e5 Mon Sep 17 00:00:00 2001 From: Lee Rowlands <lee.rowlands@previousnext.com.au> Date: Tue, 9 Jun 2020 08:54:48 +1000 Subject: [PATCH] Issue #2983395 by mcdruid, narendra.rajwar27, andypost, yogeshmpawar, vijaycs85, prabha1997, swatichouhan012, amjad1233, SunnyGambino, larowlan, borisson_, anavarre, kim.pepper, alexpott: user module's flood controls should do better logging --- .../UserAuthenticationController.php | 32 ++-- core/modules/user/src/Event/UserEvents.php | 43 +++++ .../modules/user/src/Event/UserFloodEvent.php | 165 ++++++++++++++++++ .../EventSubscriber/UserFloodSubscriber.php | 72 ++++++++ core/modules/user/src/Form/UserLoginForm.php | 46 +++-- core/modules/user/src/UserFloodControl.php | 98 +++++++++++ .../user/src/UserFloodControlInterface.php | 12 ++ .../src/Functional/UserLoginHttpTest.php | 16 +- .../tests/src/Functional/UserLoginTest.php | 13 ++ .../UserAuthenticationControllerTest.php | 52 ++++++ .../src/Kernel/Form/UserLoginFormTest.php | 41 +++++ core/modules/user/user.services.yml | 11 ++ 12 files changed, 565 insertions(+), 36 deletions(-) create mode 100644 core/modules/user/src/Event/UserEvents.php create mode 100644 core/modules/user/src/Event/UserFloodEvent.php create mode 100644 core/modules/user/src/EventSubscriber/UserFloodSubscriber.php create mode 100644 core/modules/user/src/UserFloodControl.php create mode 100644 core/modules/user/src/UserFloodControlInterface.php create mode 100644 core/modules/user/tests/src/Kernel/Controller/UserAuthenticationControllerTest.php create mode 100644 core/modules/user/tests/src/Kernel/Form/UserLoginFormTest.php diff --git a/core/modules/user/src/Controller/UserAuthenticationController.php b/core/modules/user/src/Controller/UserAuthenticationController.php index f45ec3a7ac7b..16cd0a5ba757 100644 --- a/core/modules/user/src/Controller/UserAuthenticationController.php +++ b/core/modules/user/src/Controller/UserAuthenticationController.php @@ -5,9 +5,9 @@ use Drupal\Core\Access\CsrfTokenGenerator; use Drupal\Core\Controller\ControllerBase; use Drupal\Core\DependencyInjection\ContainerInjectionInterface; -use Drupal\Core\Flood\FloodInterface; use Drupal\Core\Routing\RouteProviderInterface; use Drupal\user\UserAuthInterface; +use Drupal\user\UserFloodControlInterface; use Drupal\user\UserInterface; use Drupal\user\UserStorageInterface; use Psr\Log\LoggerInterface; @@ -39,11 +39,11 @@ class UserAuthenticationController extends ControllerBase implements ContainerIn const LOGGED_OUT = 0; /** - * The flood controller. + * The user flood control service. * - * @var \Drupal\Core\Flood\FloodInterface + * @var \Drupal\user\UserFloodControl */ - protected $flood; + protected $userFloodControl; /** * The user storage. @@ -97,8 +97,8 @@ class UserAuthenticationController extends ControllerBase implements ContainerIn /** * Constructs a new UserAuthenticationController object. * - * @param \Drupal\Core\Flood\FloodInterface $flood - * The flood controller. + * @param \Drupal\user\UserFloodControlInterface $user_flood_control + * The user flood control service. * @param \Drupal\user\UserStorageInterface $user_storage * The user storage. * @param \Drupal\Core\Access\CsrfTokenGenerator $csrf_token @@ -114,8 +114,12 @@ class UserAuthenticationController extends ControllerBase implements ContainerIn * @param \Psr\Log\LoggerInterface $logger * A logger instance. */ - public function __construct(FloodInterface $flood, UserStorageInterface $user_storage, CsrfTokenGenerator $csrf_token, UserAuthInterface $user_auth, RouteProviderInterface $route_provider, Serializer $serializer, array $serializer_formats, LoggerInterface $logger) { - $this->flood = $flood; + public function __construct($user_flood_control, UserStorageInterface $user_storage, CsrfTokenGenerator $csrf_token, UserAuthInterface $user_auth, RouteProviderInterface $route_provider, Serializer $serializer, array $serializer_formats, LoggerInterface $logger) { + if (!$user_flood_control instanceof UserFloodControlInterface) { + @trigger_error('Passing the flood service to ' . __METHOD__ . ' is deprecated in drupal:9.1.0 and is replaced by user.flood_control in drupal:10.0.0. See https://www.drupal.org/node/3067148', E_USER_DEPRECATED); + $user_flood_control = \Drupal::service('user.flood_control'); + } + $this->userFloodControl = $user_flood_control; $this->userStorage = $user_storage; $this->csrfToken = $csrf_token; $this->userAuth = $user_auth; @@ -140,7 +144,7 @@ public static function create(ContainerInterface $container) { } return new static( - $container->get('flood'), + $container->get('user.flood_control'), $container->get('entity_type.manager')->getStorage('user'), $container->get('csrf_token'), $container->get('user.auth'), @@ -183,7 +187,7 @@ public function login(Request $request) { } if ($uid = $this->userAuth->authenticate($credentials['name'], $credentials['pass'])) { - $this->flood->clear('user.http_login', $this->getLoginFloodIdentifier($request, $credentials['name'])); + $this->userFloodControl->clear('user.http_login', $this->getLoginFloodIdentifier($request, $credentials['name'])); /** @var \Drupal\user\UserInterface $user */ $user = $this->userStorage->load($uid); $this->userLoginFinalize($user); @@ -212,10 +216,10 @@ public function login(Request $request) { $flood_config = $this->config('user.flood'); if ($identifier = $this->getLoginFloodIdentifier($request, $credentials['name'])) { - $this->flood->register('user.http_login', $flood_config->get('user_window'), $identifier); + $this->userFloodControl->register('user.http_login', $flood_config->get('user_window'), $identifier); } // Always register an IP-based failed login event. - $this->flood->register('user.failed_login_ip', $flood_config->get('ip_window')); + $this->userFloodControl->register('user.failed_login_ip', $flood_config->get('ip_window')); throw new BadRequestHttpException('Sorry, unrecognized username or password.'); } @@ -354,14 +358,14 @@ protected function getRequestFormat(Request $request) { */ protected function floodControl(Request $request, $username) { $flood_config = $this->config('user.flood'); - if (!$this->flood->isAllowed('user.failed_login_ip', $flood_config->get('ip_limit'), $flood_config->get('ip_window'))) { + if (!$this->userFloodControl->isAllowed('user.failed_login_ip', $flood_config->get('ip_limit'), $flood_config->get('ip_window'))) { throw new AccessDeniedHttpException('Access is blocked because of IP based flood prevention.', NULL, Response::HTTP_TOO_MANY_REQUESTS); } if ($identifier = $this->getLoginFloodIdentifier($request, $username)) { // Don't allow login if the limit for this user has been reached. // Default is to allow 5 failed attempts every 6 hours. - if (!$this->flood->isAllowed('user.http_login', $flood_config->get('user_limit'), $flood_config->get('user_window'), $identifier)) { + if (!$this->userFloodControl->isAllowed('user.http_login', $flood_config->get('user_limit'), $flood_config->get('user_window'), $identifier)) { if ($flood_config->get('uid_only')) { $error_message = sprintf('There have been more than %s failed login attempts for this account. It is temporarily blocked. Try again later or request a new password.', $flood_config->get('user_limit')); } diff --git a/core/modules/user/src/Event/UserEvents.php b/core/modules/user/src/Event/UserEvents.php new file mode 100644 index 000000000000..7c17e272d346 --- /dev/null +++ b/core/modules/user/src/Event/UserEvents.php @@ -0,0 +1,43 @@ +<?php + +namespace Drupal\user\Event; + +/** + * Defines events for the user module. + */ +final class UserEvents { + + /** + * The name of the event fired when a login is blocked by flood control. + * + * This event allows modules to perform an action whenever flood control has + * been triggered by excessive login attempts for a particular user account. + * The event listener method receives a \Drupal\user\Event\UserFloodEvent + * instance. + * + * @Event + * + * @see: \Drupal\user\UserFloodControl::isAllowed + * @see: \Drupal\user\EventSubscriber\UserFloodSubscriber + * + * @var string + */ + const FLOOD_BLOCKED_USER = 'user.flood_blocked_user'; + + /** + * The name of the event fired when a login is blocked by flood control. + * + * This event allows modules to perform an action whenever flood control has + * been triggered by excessive login attempts from a particular IP. The event + * listener method receives a \Drupal\user\Event\UserFloodEvent instance. + * + * @Event + * + * @see: \Drupal\user\UserFloodControl::isAllowed + * @see: \Drupal\user\EventSubscriber\UserFloodSubscriber + * + * @var string + */ + const FLOOD_BLOCKED_IP = 'user.flood_blocked_ip'; + +} diff --git a/core/modules/user/src/Event/UserFloodEvent.php b/core/modules/user/src/Event/UserFloodEvent.php new file mode 100644 index 000000000000..2ad7a648948b --- /dev/null +++ b/core/modules/user/src/Event/UserFloodEvent.php @@ -0,0 +1,165 @@ +<?php + +namespace Drupal\user\Event; + +use Symfony\Component\EventDispatcher\Event; + +/** + * Provides a user flood event for event listeners. + */ +class UserFloodEvent extends Event { + + /** + * Flood event name. + * + * @var string + */ + protected $name; + + /** + * Flood event threshold. + * + * @var int + */ + protected $threshold; + + /** + * Flood event window. + * + * @var int + */ + protected $window; + + /** + * Flood event identifier. + * + * @var string + */ + protected $identifier; + + /** + * Flood event uid. + * + * @var int + */ + protected $uid = NULL; + + /** + * Flood event IP. + * + * @var string + */ + protected $ip = NULL; + + /** + * Constructs a user flood event object. + * + * @param string $name + * The name of the flood event. + * @param int $threshold + * The threshold for the flood event. + * @param int $window + * The window for the flood event. + * @param string $identifier + * The identifier of the flood event. + */ + public function __construct($name, $threshold, $window, $identifier) { + $this->name = $name; + $this->threshold = $threshold; + $this->window = $window; + $this->identifier = $identifier; + // The identifier could be a uid or an IP, or a composite of both. + if (is_numeric($identifier)) { + $this->uid = $identifier; + return; + } + if (strpos($identifier, '-') !== FALSE) { + list($uid, $ip) = explode('-', $identifier); + $this->uid = $uid; + $this->ip = $ip; + return; + } + $this->ip = $identifier; + } + + /** + * Gets the name of the user flood event object. + * + * @return string + * The name of the flood event. + */ + public function getName() { + return $this->name; + } + + /** + * Gets the threshold for the user flood event object. + * + * @return int + * The threshold for the flood event. + */ + public function getThreshold() { + return $this->threshold; + } + + /** + * Gets the window for the user flood event object. + * + * @return int + * The window for the flood event. + */ + public function getWindow() { + return $this->window; + } + + /** + * Gets the identifier of the user flood event object. + * + * @return string + * The identifier of the flood event. + */ + public function getIdentifier() { + return $this->identifier; + } + + /** + * Gets the IP of the user flood event object. + * + * @return string + * The IP of the flood event. + */ + public function getIp() { + return $this->ip; + } + + /** + * Gets the uid of the user flood event object. + * + * @return int + * The uid of the flood event. + */ + public function getUid() { + return $this->uid; + } + + /** + * Is the user flood event associated with an IP? + * + * @return bool + * Whether the event has an IP. + */ + public function hasIp() { + return !empty($this->ip); + } + + /** + * Is the user flood event associated with a uid? + * + * @return bool + * Whether the event has a uid. + */ + public function hasUid() { + return !empty($this->uid); + } + +} diff --git a/core/modules/user/src/EventSubscriber/UserFloodSubscriber.php b/core/modules/user/src/EventSubscriber/UserFloodSubscriber.php new file mode 100644 index 000000000000..ba0c324c1ae2 --- /dev/null +++ b/core/modules/user/src/EventSubscriber/UserFloodSubscriber.php @@ -0,0 +1,72 @@ +<?php + +namespace Drupal\user\EventSubscriber; + +use Drupal\user\Event\UserEvents; +use Drupal\user\Event\UserFloodEvent; +use Drupal\Core\Site\Settings; +use Symfony\Component\EventDispatcher\EventSubscriberInterface; +use Psr\Log\LoggerInterface; + +/** + * Logs details of User Flood Control events. + */ +class UserFloodSubscriber implements EventSubscriberInterface { + + /** + * The default logger service. + * + * @var \Psr\Log\LoggerInterface + */ + protected $logger; + + /** + * Constructs a UserFloodSubscriber. + * + * @param \Psr\Log\LoggerInterface $logger + * A logger instance. + */ + public function __construct(LoggerInterface $logger = NULL) { + $this->logger = $logger; + } + + /** + * {@inheritdoc} + */ + public static function getSubscribedEvents() { + $events[UserEvents::FLOOD_BLOCKED_USER][] = ['blockedUser']; + $events[UserEvents::FLOOD_BLOCKED_IP][] = ['blockedIp']; + return $events; + } + + /** + * An attempt to login has been blocked based on user name. + * + * @param \Drupal\user\Event\UserFloodEvent $floodEvent + * The flood event. + */ + public function blockedUser(UserFloodEvent $floodEvent) { + if (Settings::get('log_user_flood', TRUE)) { + $uid = $floodEvent->getUid(); + if ($floodEvent->hasIp()) { + $ip = $floodEvent->getIp(); + $this->logger->notice('Flood control blocked login attempt for uid %uid from %ip', ['%uid' => $uid, '%ip' => $ip]); + return; + } + $this->logger->notice('Flood control blocked login attempt for uid %uid', ['%uid' => $uid]); + } + } + + /** + * An attempt to login has been blocked based on IP. + * + * @param \Drupal\user\Event\UserFloodEvent $floodEvent + * The flood event. + */ + public function blockedIp(UserFloodEvent $floodEvent) { + if (Settings::get('log_user_flood', TRUE)) { + $this->logger->notice('Flood control blocked login attempt from %ip', ['%ip' => $floodEvent->getIp()]); + } + } + +} diff --git a/core/modules/user/src/Form/UserLoginForm.php b/core/modules/user/src/Form/UserLoginForm.php index 6f1a19fc0428..b2fb91a290ee 100644 --- a/core/modules/user/src/Form/UserLoginForm.php +++ b/core/modules/user/src/Form/UserLoginForm.php @@ -2,7 +2,6 @@ namespace Drupal\user\Form; -use Drupal\Core\Flood\FloodInterface; use Drupal\Core\Form\FormBase; use Drupal\Core\Form\FormStateInterface; use Drupal\Core\Render\RendererInterface; @@ -10,7 +9,9 @@ use Drupal\user\UserAuthInterface; use Drupal\user\UserInterface; use Drupal\user\UserStorageInterface; +use Drupal\user\UserFloodControlInterface; use Symfony\Component\DependencyInjection\ContainerInterface; +use Symfony\Component\HttpFoundation\Response; /** * Provides a user login form. @@ -20,11 +21,11 @@ class UserLoginForm extends FormBase { /** - * The flood service. + * The user flood control service. * - * @var \Drupal\Core\Flood\FloodInterface + * @var \Drupal\user\UserFloodControl */ - protected $flood; + protected $userFloodControl; /** * The user storage. @@ -50,8 +51,8 @@ class UserLoginForm extends FormBase { /** * Constructs a new UserLoginForm. * - * @param \Drupal\Core\Flood\FloodInterface $flood - * The flood service. + * @param \Drupal\user\UserFloodControlInterface $user_flood_control + * The user flood control service. * @param \Drupal\user\UserStorageInterface $user_storage * The user storage. * @param \Drupal\user\UserAuthInterface $user_auth @@ -59,8 +60,12 @@ class UserLoginForm extends FormBase { * @param \Drupal\Core\Render\RendererInterface $renderer * The renderer. */ - public function __construct(FloodInterface $flood, UserStorageInterface $user_storage, UserAuthInterface $user_auth, RendererInterface $renderer) { - $this->flood = $flood; + public function __construct($user_flood_control, UserStorageInterface $user_storage, UserAuthInterface $user_auth, RendererInterface $renderer) { + if (!$user_flood_control instanceof UserFloodControlInterface) { + @trigger_error('Passing the flood service to ' . __METHOD__ . ' is deprecated in drupal:9.1.0 and is replaced by user.flood_control in drupal:10.0.0. See https://www.drupal.org/node/3067148', E_USER_DEPRECATED); + $user_flood_control = \Drupal::service('user.flood_control'); + } + $this->userFloodControl = $user_flood_control; $this->userStorage = $user_storage; $this->userAuth = $user_auth; $this->renderer = $renderer; @@ -71,7 +76,7 @@ public function __construct(FloodInterface $flood, UserStorageInterface $user_st */ public static function create(ContainerInterface $container) { return new static( - $container->get('flood'), + $container->get('user.flood_control'), $container->get('entity_type.manager')->getStorage('user'), $container->get('user.auth'), $container->get('renderer') @@ -131,9 +136,13 @@ public function buildForm(array $form, FormStateInterface $form_state) { * {@inheritdoc} */ public function submitForm(array &$form, FormStateInterface $form_state) { - $account = $this->userStorage->load($form_state->get('uid')); - // A destination was set, probably on an exception controller, + if (empty($uid = $form_state->get('uid'))) { + return; + } + $account = $this->userStorage->load($uid); + + // A destination was set, probably on an exception controller. if (!$this->getRequest()->request->has('destination')) { $form_state->setRedirect( 'entity.user.canonical', @@ -171,7 +180,7 @@ public function validateAuthentication(array &$form, FormStateInterface $form_st // independent of the per-user limit to catch attempts from one IP to log // in to many different user accounts. We have a reasonably high limit // since there may be only one apparent IP for all users at an institution. - if (!$this->flood->isAllowed('user.failed_login_ip', $flood_config->get('ip_limit'), $flood_config->get('ip_window'))) { + if (!$this->userFloodControl->isAllowed('user.failed_login_ip', $flood_config->get('ip_limit'), $flood_config->get('ip_window'))) { $form_state->set('flood_control_triggered', 'ip'); return; } @@ -193,7 +202,7 @@ public function validateAuthentication(array &$form, FormStateInterface $form_st // Don't allow login if the limit for this user has been reached. // Default is to allow 5 failed attempts every 6 hours. - if (!$this->flood->isAllowed('user.failed_login_user', $flood_config->get('user_limit'), $flood_config->get('user_window'), $identifier)) { + if (!$this->userFloodControl->isAllowed('user.failed_login_user', $flood_config->get('user_limit'), $flood_config->get('user_window'), $identifier)) { $form_state->set('flood_control_triggered', 'user'); return; } @@ -214,20 +223,21 @@ public function validateFinal(array &$form, FormStateInterface $form_state) { $flood_config = $this->config('user.flood'); if (!$form_state->get('uid')) { // Always register an IP-based failed login event. - $this->flood->register('user.failed_login_ip', $flood_config->get('ip_window')); + $this->userFloodControl->register('user.failed_login_ip', $flood_config->get('ip_window')); // Register a per-user failed login event. if ($flood_control_user_identifier = $form_state->get('flood_control_user_identifier')) { - $this->flood->register('user.failed_login_user', $flood_config->get('user_window'), $flood_control_user_identifier); + $this->userFloodControl->register('user.failed_login_user', $flood_config->get('user_window'), $flood_control_user_identifier); } if ($flood_control_triggered = $form_state->get('flood_control_triggered')) { if ($flood_control_triggered == 'user') { - $form_state->setErrorByName('name', $this->formatPlural($flood_config->get('user_limit'), 'There has been more than one failed login attempt for this account. It is temporarily blocked. Try again later or <a href=":url">request a new password</a>.', 'There have been more than @count failed login attempts for this account. It is temporarily blocked. Try again later or <a href=":url">request a new password</a>.', [':url' => Url::fromRoute('user.pass')->toString()])); + $message = $this->formatPlural($flood_config->get('user_limit'), 'There has been more than one failed login attempt for this account. It is temporarily blocked. Try again later or <a href=":url">request a new password</a>.', 'There have been more than @count failed login attempts for this account. It is temporarily blocked. Try again later or <a href=":url">request a new password</a>.', [':url' => Url::fromRoute('user.pass')->toString()]); } else { // We did not find a uid, so the limit is IP-based. - $form_state->setErrorByName('name', $this->t('Too many failed login attempts from your IP address. This IP address is temporarily blocked. Try again later or <a href=":url">request a new password</a>.', [':url' => Url::fromRoute('user.pass')->toString()])); + $message = $this->t('Too many failed login attempts from your IP address. This IP address is temporarily blocked. Try again later or <a href=":url">request a new password</a>.', [':url' => Url::fromRoute('user.pass')->toString()]); } + $form_state->setResponse(new Response($message, 403)); } else { // Use $form_state->getUserInput() in the error message to guarantee @@ -251,7 +261,7 @@ public function validateFinal(array &$form, FormStateInterface $form_state) { elseif ($flood_control_user_identifier = $form_state->get('flood_control_user_identifier')) { // Clear past failures for this user so as not to block a user who might // log in and out more than once in an hour. - $this->flood->clear('user.failed_login_user', $flood_control_user_identifier); + $this->userFloodControl->clear('user.failed_login_user', $flood_control_user_identifier); } } diff --git a/core/modules/user/src/UserFloodControl.php b/core/modules/user/src/UserFloodControl.php new file mode 100644 index 000000000000..c6d6bc07ddd8 --- /dev/null +++ b/core/modules/user/src/UserFloodControl.php @@ -0,0 +1,98 @@ +<?php + +namespace Drupal\user; + +use Drupal\user\Event\UserEvents; +use Drupal\user\Event\UserFloodEvent; +use Drupal\Core\Flood\FloodInterface; +use Symfony\Component\EventDispatcher\EventDispatcherInterface; +use Symfony\Component\HttpFoundation\RequestStack; + +/** + * User Flood Control service. + * + * @see: \Drupal\Core\Flood\DatabaseBackend + */ +class UserFloodControl implements UserFloodControlInterface { + + /** + * The decorated flood service. + * + * @var \Drupal\Core\Flood\FloodInterface + */ + protected $flood; + + /** + * Event dispatcher. + * + * @var \Symfony\Component\EventDispatcher\EventDispatcherInterface + */ + protected $eventDispatcher; + + /** + * The request stack. + * + * @var \Symfony\Component\HttpFoundation\RequestStack + */ + protected $requestStack; + + /** + * Construct the UserFloodControl. + * + * @param \Drupal\Core\Flood\FloodInterface $flood + * The flood service. + * @param \Symfony\Component\EventDispatcher\EventDispatcherInterface $event_dispatcher + * The event dispatcher service. + * @param \Symfony\Component\HttpFoundation\RequestStack $request_stack + * The request stack used to retrieve the current request. + */ + public function __construct(FloodInterface $flood, EventDispatcherInterface $event_dispatcher, RequestStack $request_stack) { + $this->flood = $flood; + $this->eventDispatcher = $event_dispatcher; + $this->requestStack = $request_stack; + } + + /** + * {@inheritdoc} + */ + public function isAllowed($name, $threshold, $window = 3600, $identifier = NULL) { + if ($this->flood->isAllowed($name, $threshold, $window, $identifier)) { + return TRUE; + } + // Register flood control blocked login event. + $event_map['user.failed_login_ip'] = UserEvents::FLOOD_BLOCKED_IP; + $event_map['user.failed_login_user'] = UserEvents::FLOOD_BLOCKED_USER; + $event_map['user.http_login'] = UserEvents::FLOOD_BLOCKED_USER; + + if (isset($event_map[$name])) { + if (empty($identifier)) { + $identifier = $this->requestStack->getCurrentRequest()->getClientIp(); + } + $event = new UserFloodEvent($name, $threshold, $window, $identifier); + $this->eventDispatcher->dispatch($event_map[$name], $event); + } + return FALSE; + } + + /** + * {@inheritdoc} + */ + public function register($name, $window = 3600, $identifier = NULL) { + return $this->flood->register($name, $window, $identifier); + } + + /** + * {@inheritdoc} + */ + public function clear($name, $identifier = NULL) { + return $this->flood->clear($name, $identifier); + } + + /** + * {@inheritdoc} + */ + public function garbageCollection() { + return $this->flood->garbageCollection(); + } + +} diff --git a/core/modules/user/src/UserFloodControlInterface.php b/core/modules/user/src/UserFloodControlInterface.php new file mode 100644 index 000000000000..116470ee8928 --- /dev/null +++ b/core/modules/user/src/UserFloodControlInterface.php @@ -0,0 +1,12 @@ +<?php + +namespace Drupal\user; + +use Drupal\Core\Flood\FloodInterface; + +/** + * Defines an interface for user flood controllers. + */ +interface UserFloodControlInterface extends FloodInterface { + +} diff --git a/core/modules/user/tests/src/Functional/UserLoginHttpTest.php b/core/modules/user/tests/src/Functional/UserLoginHttpTest.php index 1544b953d0f1..cf25febbee24 100644 --- a/core/modules/user/tests/src/Functional/UserLoginHttpTest.php +++ b/core/modules/user/tests/src/Functional/UserLoginHttpTest.php @@ -30,7 +30,7 @@ class UserLoginHttpTest extends BrowserTestBase { * * @var array */ - protected static $modules = ['hal']; + protected static $modules = ['hal', 'dblog']; /** * {@inheritdoc} @@ -288,6 +288,7 @@ protected function resetFlood() { * @see \Drupal\user\Tests\UserLoginTest::testGlobalLoginFloodControl */ public function testGlobalLoginFloodControl() { + $database = \Drupal::database(); $this->config('user.flood') ->set('ip_limit', 2) // Set a high per-user limit out so that it is not relevant in the test. @@ -307,6 +308,8 @@ public function testGlobalLoginFloodControl() { // IP limit has reached to its limit. Even valid user credentials will fail. $response = $this->loginRequest($user->getAccountName(), $user->passRaw); $this->assertHttpResponseWithMessage($response, '403', 'Access is blocked because of IP based flood prevention.'); + $last_log = $database->queryRange('SELECT message FROM {watchdog} WHERE type = :type ORDER BY wid DESC', 0, 1, [':type' => 'user'])->fetchField(); + $this->assertEquals('Flood control blocked login attempt from %ip', $last_log, 'A watchdog message was logged for the login attempt blocked by flood control per IP.'); } /** @@ -348,6 +351,7 @@ protected function assertHttpResponseWithMessage(ResponseInterface $response, $e * @see \Drupal\basic_auth\Tests\Authentication\BasicAuthTest::testPerUserLoginFloodControl */ public function testPerUserLoginFloodControl() { + $database = \Drupal::database(); foreach ([TRUE, FALSE] as $uid_only_setting) { $this->config('user.flood') // Set a high global limit out so that it is not relevant in the test. @@ -389,12 +393,16 @@ public function testPerUserLoginFloodControl() { $response = $this->loginRequest($user1->getAccountName(), $user1->passRaw); // Depending on the uid_only setting the error message will be different. if ($uid_only_setting) { - $excepted_message = 'There have been more than 3 failed login attempts for this account. It is temporarily blocked. Try again later or request a new password.'; + $expected_message = 'There have been more than 3 failed login attempts for this account. It is temporarily blocked. Try again later or request a new password.'; + $expected_log = 'Flood control blocked login attempt for uid %uid'; } else { - $excepted_message = 'Too many failed login attempts from your IP address. This IP address is temporarily blocked.'; + $expected_message = 'Too many failed login attempts from your IP address. This IP address is temporarily blocked.'; + $expected_log = 'Flood control blocked login attempt for uid %uid from %ip'; } - $this->assertHttpResponseWithMessage($response, 403, $excepted_message); + $this->assertHttpResponseWithMessage($response, 403, $expected_message); + $last_log = $database->queryRange('SELECT message FROM {watchdog} WHERE type = :type ORDER BY wid DESC', 0, 1, [':type' => 'user'])->fetchField(); + $this->assertEquals($expected_log, $last_log, 'A watchdog message was logged for the login attempt blocked by flood control per user.'); } } diff --git a/core/modules/user/tests/src/Functional/UserLoginTest.php b/core/modules/user/tests/src/Functional/UserLoginTest.php index e067915867e9..4e2c534bf64f 100644 --- a/core/modules/user/tests/src/Functional/UserLoginTest.php +++ b/core/modules/user/tests/src/Functional/UserLoginTest.php @@ -18,6 +18,13 @@ class UserLoginTest extends BrowserTestBase { */ protected $defaultTheme = 'stark'; + /** + * Modules to install. + * + * @var array + */ + public static $modules = ['dblog']; + /** * Tests login with destination. */ @@ -161,6 +168,7 @@ public function testPasswordRehashOnLogin() { * - Set to NULL to expect a failed login. */ public function assertFailedLogin($account, $flood_trigger = NULL) { + $database = \Drupal::database(); $edit = [ 'name' => $account->getAccountName(), 'pass' => $account->passRaw, @@ -168,15 +176,20 @@ public function assertFailedLogin($account, $flood_trigger = NULL) { $this->drupalPostForm('user/login', $edit, t('Log in')); $this->assertNoFieldByXPath("//input[@name='pass' and @value!='']", NULL, 'Password value attribute is blank.'); if (isset($flood_trigger)) { + $this->assertSession()->statusCodeEquals(403); + $last_log = $database->queryRange('SELECT message FROM {watchdog} WHERE type = :type ORDER BY wid DESC', 0, 1, [':type' => 'user'])->fetchField(); 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 <a href=":url">request a new password</a>.', 'There have been more than @count failed login attempts for this account. It is temporarily blocked. Try again later or <a href=":url">request a new password</a>.', [':url' => Url::fromRoute('user.pass')->toString()])); + $this->assertEquals('Flood control blocked login attempt for uid %uid from %ip', $last_log, 'A watchdog message was logged for the login attempt blocked by flood control per user.'); } 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 <a href=":url">request a new password</a>.', [':url' => Url::fromRoute('user.pass')->toString()])); + $this->assertEquals('Flood control blocked login attempt from %ip', $last_log, 'A watchdog message was logged for the login attempt blocked by flood control per IP.'); } } else { + $this->assertSession()->statusCodeEquals(200); $this->assertText(t('Unrecognized username or password. Forgot your password?')); } } diff --git a/core/modules/user/tests/src/Kernel/Controller/UserAuthenticationControllerTest.php b/core/modules/user/tests/src/Kernel/Controller/UserAuthenticationControllerTest.php new file mode 100644 index 000000000000..b3c941cef155 --- /dev/null +++ b/core/modules/user/tests/src/Kernel/Controller/UserAuthenticationControllerTest.php @@ -0,0 +1,52 @@ +<?php + +namespace Drupal\Tests\user\Kernel\Controller; + +use Drupal\Core\Access\CsrfTokenGenerator; +use Drupal\Core\Flood\FloodInterface; +use Drupal\Core\Routing\RouteProviderInterface; +use Drupal\KernelTests\KernelTestBase; +use Drupal\user\Controller\UserAuthenticationController; +use Drupal\user\UserAuthInterface; +use Drupal\user\UserStorageInterface; +use Psr\Log\LoggerInterface; +use Symfony\Component\Serializer\Serializer; + +/** + * @coversDefaultClass \Drupal\user\Controller\UserController + * @group user + */ +class UserAuthenticationControllerTest extends KernelTestBase { + + /** + * {@inheritdoc} + */ + public static $modules = ['user']; + + /** + * @group legacy + * @expectedDeprecation Passing the flood service to Drupal\user\Controller\UserAuthenticationController::__construct is deprecated in drupal:9.1.0 and is replaced by user.flood_control in drupal:10.0.0. See https://www.drupal.org/node/3067148 + */ + public function testConstructorDeprecations() { + $flood = $this->prophesize(FloodInterface::class); + $user_storage = $this->prophesize(UserStorageInterface::class); + $csrf_token = $this->prophesize(CsrfTokenGenerator::class); + $user_auth = $this->prophesize(UserAuthInterface::class); + $route_provider = $this->prophesize(RouteProviderInterface::class); + $serializer = $this->prophesize(Serializer::class); + $serializer_formats = []; + $logger = $this->prophesize(LoggerInterface::class); + $controller = new UserAuthenticationController( + $flood->reveal(), + $user_storage->reveal(), + $csrf_token->reveal(), + $user_auth->reveal(), + $route_provider->reveal(), + $serializer->reveal(), + $serializer_formats, + $logger->reveal() + ); + $this->assertNotNull($controller); + } + +} diff --git a/core/modules/user/tests/src/Kernel/Form/UserLoginFormTest.php b/core/modules/user/tests/src/Kernel/Form/UserLoginFormTest.php new file mode 100644 index 000000000000..7737433d49bd --- /dev/null +++ b/core/modules/user/tests/src/Kernel/Form/UserLoginFormTest.php @@ -0,0 +1,41 @@ +<?php + +namespace Drupal\Tests\user\Kernel\Form; + +use Drupal\Core\Flood\FloodInterface; +use Drupal\Core\Render\RendererInterface; +use Drupal\KernelTests\KernelTestBase; +use Drupal\user\Form\UserLoginForm; +use Drupal\user\UserAuthInterface; +use Drupal\user\UserStorageInterface; + +/** + * @coversDefaultClass \Drupal\user\Form\UserLoginForm + * @group user + */ +class UserLoginFormTest extends KernelTestBase { + + /** + * {@inheritdoc} + */ + public static $modules = ['user']; + + /** + * @group legacy + * @expectedDeprecation Passing the flood service to Drupal\user\Form\UserLoginForm::__construct is deprecated in drupal:9.1.0 and is replaced by user.flood_control in drupal:10.0.0. See https://www.drupal.org/node/3067148 + */ + public function testConstructorDeprecations() { + $flood = $this->prophesize(FloodInterface::class); + $user_storage = $this->prophesize(UserStorageInterface::class); + $user_auth = $this->prophesize(UserAuthInterface::class); + $renderer = $this->prophesize(RendererInterface::class); + $form = new UserLoginForm( + $flood->reveal(), + $user_storage->reveal(), + $user_auth->reveal(), + $renderer->reveal() + ); + $this->assertNotNull($form); + } + +} diff --git a/core/modules/user/user.services.yml b/core/modules/user/user.services.yml index 0646064078c5..0697f24acce4 100644 --- a/core/modules/user/user.services.yml +++ b/core/modules/user/user.services.yml @@ -59,3 +59,14 @@ services: user.toolbar_link_builder: class: Drupal\user\ToolbarLinkBuilder arguments: ['@current_user'] + user.flood_control: + class: Drupal\user\UserFloodControl + arguments: ['@flood', '@event_dispatcher', '@request_stack'] + user.flood_subcriber: + class: Drupal\user\EventSubscriber\UserFloodSubscriber + arguments: ['@logger.channel.user'] + tags: + - { name: 'event_subscriber' } + logger.channel.user: + parent: logger.channel_base + arguments: ['user'] -- GitLab