diff --git a/core/modules/serialization/serialization.services.yml b/core/modules/serialization/serialization.services.yml index a03336e..8b570c0 100644 --- a/core/modules/serialization/serialization.services.yml +++ b/core/modules/serialization/serialization.services.yml @@ -64,8 +64,8 @@ services: class: Drupal\serialization\EntityResolver\TargetIdResolver tags: - { name: entity_resolver} - serialization.exception.default_http: - class: Drupal\serialization\EventSubscriber\ExceptionHttpSubscriber + serialization.exception.default: + class: Drupal\serialization\EventSubscriber\DefaultExceptionSubscriber tags: - { name: event_subscriber } arguments: ['@serializer', '%serializer.formats%'] diff --git a/core/modules/serialization/src/EventSubscriber/DefaultExceptionSubscriber.php b/core/modules/serialization/src/EventSubscriber/DefaultExceptionSubscriber.php new file mode 100644 index 0000000..c2bd175 --- /dev/null +++ b/core/modules/serialization/src/EventSubscriber/DefaultExceptionSubscriber.php @@ -0,0 +1,74 @@ +serializer = $serializer; + $this->serializerFormats = $serializer_formats; + } + + /** + * {@inheritdoc} + */ + protected function getHandledFormats() { + return $this->serializerFormats; + } + + /** + * {@inheritdoc} + */ + protected static function getPriority() { + // This will fire after the most common HTML handler, since HTML requests + // are still more common than HTTP requests. + return -75; + } + + /** + * {@inheritdoc} + */ + public function onException(GetResponseForExceptionEvent $event) { + parent::onException($event); + $format = $event->getRequest()->getRequestFormat(); + $content = ['message' => $event->getException()->getMessage()]; + $encoded_content = $this->serializer->serialize($content, $format); + $exception = $event->getException(); + if ($exception instanceof HttpExceptionInterface && (empty($handled_formats) || in_array($format, $handled_formats))) { + $response = new Response($encoded_content, $exception->getStatusCode()); + $event->setResponse($response); + } + } + +} diff --git a/core/modules/serialization/src/EventSubscriber/ExceptionHttpSubscriber.php b/core/modules/serialization/src/EventSubscriber/ExceptionHttpSubscriber.php deleted file mode 100644 index 9b4cba2..0000000 --- a/core/modules/serialization/src/EventSubscriber/ExceptionHttpSubscriber.php +++ /dev/null @@ -1,124 +0,0 @@ -serializer = $serializer; - $this->serializerFormats = $serializer_formats; - } - - /** - * {@inheritdoc} - */ - protected function getHandledFormats() { - return $this->serializerFormats; - } - - /** - * {@inheritdoc} - */ - protected static function getPriority() { - // This will fire after the most common HTML handler, since HTML requests - // are still more common than HTTP requests. - return -75; - } - - /** - * Handles a 400 error for HTTP. - * - * @param \Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent $event - * The event to process. - */ - public function on400(GetResponseForExceptionEvent $event) { - $this->setEventResponse($event, Response::HTTP_BAD_REQUEST); - } - - /** - * Handles a 403 error for HTTP. - * - * @param \Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent $event - * The event to process. - */ - public function on403(GetResponseForExceptionEvent $event) { - $this->setEventResponse($event, Response::HTTP_FORBIDDEN); - } - - /** - * Handles a 404 error for HTTP. - * - * @param \Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent $event - * The event to process. - */ - public function on404(GetResponseForExceptionEvent $event) { - $this->setEventResponse($event, Response::HTTP_NOT_FOUND); - } - - /** - * Handles a 405 error for HTTP. - * - * @param \Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent $event - * The event to process. - */ - public function on405(GetResponseForExceptionEvent $event) { - $this->setEventResponse($event, Response::HTTP_METHOD_NOT_ALLOWED); - } - - /** - * Handles a 406 error for HTTP. - * - * @param \Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent $event - * The event to process. - */ - public function on406(GetResponseForExceptionEvent $event) { - $this->setEventResponse($event, Response::HTTP_NOT_ACCEPTABLE); - } - - /** - * Sets the Response for the exception event. - * - * @param \Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent $event - * The current exception event. - * @param int $status - * The HTTP status code to set for the response. - */ - protected function setEventResponse(GetResponseForExceptionEvent $event, $status) { - $format = $event->getRequest()->getRequestFormat(); - $content = ['message' => $event->getException()->getMessage()]; - $encoded_content = $this->serializer->serialize($content, $format); - $response = new Response($encoded_content, $status); - $event->setResponse($response); - } - -} diff --git a/core/modules/serialization/src/EventSubscriber/UserRouteAlterSubscriber.php b/core/modules/serialization/src/EventSubscriber/UserRouteAlterSubscriber.php index f6f6abf..d8002d2 100644 --- a/core/modules/serialization/src/EventSubscriber/UserRouteAlterSubscriber.php +++ b/core/modules/serialization/src/EventSubscriber/UserRouteAlterSubscriber.php @@ -9,7 +9,7 @@ use Symfony\Component\Serializer\SerializerInterface; /** - * Alters user authentication routes to add formats supported by this module. + * Alters user authentication routes to support additional serialization formats. */ class UserRouteAlterSubscriber implements EventSubscriberInterface { @@ -44,7 +44,7 @@ public function __construct(SerializerInterface $serializer, array $serializer_f * {@inheritdoc} */ public static function getSubscribedEvents() { - $events[RoutingEvents::ALTER][] = 'onRoutingRouteAlterAddFormats'; + $events[RoutingEvents::ALTER][] = 'onRoutingAlterAddFormats'; return $events; } @@ -54,7 +54,7 @@ public static function getSubscribedEvents() { * @param \Drupal\Core\Routing\RouteBuildEvent $event * The event to process. */ - public function onRoutingRouteAlterAddFormats(RouteBuildEvent $event) { + public function onRoutingAlterAddFormats(RouteBuildEvent $event) { $route_names = [ 'user.login_status.http', 'user.login.http', diff --git a/core/modules/user/src/Controller/UserAuthenticationController.php b/core/modules/user/src/Controller/UserAuthenticationController.php index eaffe47..e5a960e 100644 --- a/core/modules/user/src/Controller/UserAuthenticationController.php +++ b/core/modules/user/src/Controller/UserAuthenticationController.php @@ -134,13 +134,9 @@ public static function create(ContainerInterface $container) { * The request. * * @return \Symfony\Component\HttpFoundation\Response - * Returns a response which contains the ID and CSRF token. + * A response which contains the ID and CSRF token. */ public function login(Request $request) { - $flood_config = $this->config('user.flood'); - if (!$this->flood->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, 403); - } $format = $this->getRequestFormat($request); $content = $request->getContent(); @@ -156,15 +152,14 @@ public function login(Request $request) { throw new BadRequestHttpException('Missing credentials.pass.'); } - if (!$this->isFloodBlocked()) { - throw new BadRequestHttpException('Blocked.'); - } + $this->floodControl($request, $credentials['name']); if ($this->userIsBlocked($credentials['name'])) { throw new BadRequestHttpException('The user has not been activated or is blocked.'); } if ($uid = $this->userAuth->authenticate($credentials['name'], $credentials['pass'])) { + $this->flood->clear('user.http_login', $this->getLoginFloodIdentifier($request, $credentials['name'])); /** @var \Drupal\user\UserInterface $user */ $user = $this->userStorage->load($uid); $this->userLoginFinalize($user); @@ -187,7 +182,9 @@ public function login(Request $request) { } $flood_config = $this->config('user.flood'); - $this->flood->register('user.http_login', $flood_config->get('user_window')); + if ($identifier = $this->getLoginFloodIdentifier($request, $credentials['name'])) { + $this->flood->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')); throw new BadRequestHttpException('Sorry, unrecognized username or password.'); @@ -200,7 +197,7 @@ public function login(Request $request) { * The username. * * @return bool - * Returns TRUE if the user is blocked, otherwise FALSE. + * TRUE if the user is blocked, otherwise FALSE. */ protected function userIsBlocked($name) { return user_is_blocked($name); @@ -217,20 +214,6 @@ protected function userLoginFinalize(UserInterface $user) { } /** - * Checks for flooding. - * - * @return bool - * TRUE if the user is allowed to proceed, FALSE otherwise. - */ - protected function isFloodBlocked() { - $config = $this->config('user.flood'); - $limit = $config->get('user_limit'); - $interval = $config->get('user_window'); - - return $this->flood->isAllowed('user.http_login', $limit, $interval); - } - - /** * Logs out a user. * * @return \Drupal\rest\ResourceResponse @@ -282,4 +265,65 @@ protected function getRequestFormat(Request $request) { return $format; } + /** + * Enforces flood control for the current login request. + * + * @param \Symfony\Component\HttpFoundation\Request $request + * The current request. + * @param string $username + * The user name sent for login credentials. + */ + 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'))) { + throw new AccessDeniedHttpException('Access is blocked because of IP based flood prevention.', NULL, 403); + } + + 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 ($flood_config->get('uid_only')) { + $error_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 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.'); + } + else { + $error_message = $this->t('Too many failed login attempts from your IP address. This IP address is temporarily blocked.'); + } + throw new AccessDeniedHttpException($error_message, NULL, 403); + } + } + } + + /** + * Gets the login identifier for user login flood control. + * + * @param \Symfony\Component\HttpFoundation\Request $request + * The current request. + * @param string $username + * The username supplied in login credentials. + * + * @return string + * The login identifier or if the user does not exist an empty string. + */ + protected function getLoginFloodIdentifier(Request $request, $username) { + $flood_config = $this->config('user.flood'); + $accounts = $this->userStorage->loadByProperties(['name' => $username, 'status' => 1]); + if ($account = reset($accounts)) { + 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 = $account->id(); + return $identifier; + } + 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 = $account->id() . '-' . $request->getClientIp(); + return $identifier; + } + } + return ''; + } + } diff --git a/core/modules/user/tests/src/Functional/UserLoginHttpTest.php b/core/modules/user/tests/src/Functional/UserLoginHttpTest.php index a60d82b..7484b8c 100644 --- a/core/modules/user/tests/src/Functional/UserLoginHttpTest.php +++ b/core/modules/user/tests/src/Functional/UserLoginHttpTest.php @@ -124,7 +124,7 @@ public function testLogin() { $this->assertResponseWithMessage($response, 400, 'Sorry, unrecognized username or password.', $format); $response = $this->loginRequest($name, 'wrong-pass', $format); - $this->assertResponseWithMessage($response, 400, 'Blocked.', $format); + $this->assertResponseWithMessage($response, 403, 'Too many failed login attempts from your IP address. This IP address is temporarily blocked.', $format); // After testing the flood control we can increase the limit. $this->config('user.flood') @@ -167,14 +167,7 @@ public function testLogin() { $this->assertEquals(200, $response->getStatusCode()); $this->assertEquals(UserAuthenticationController::LOGGED_IN, (string) $response->getBody()); - $user_logout_url = Url::fromRoute('user.logout.http')->setRouteParameter('_format', $format)->setAbsolute(); - $response = $client->post($user_logout_url->toString(), [ - 'headers' => [ - 'Accept' => "application/$format", - ], - 'http_errors' => FALSE, - 'cookies' => $this->cookies, - ]); + $response = $this->logoutRequest($format); $this->assertEquals(204, $response->getStatusCode()); $response = $client->post($user_login_status_url->toString(), ['cookies' => $this->cookies]); @@ -216,7 +209,7 @@ protected function decode($data, $format) { } /** - * Get a value for a given key from the response. + * Gets a value for a given key from the response. * * @param \Psr\Http\Message\ResponseInterface $response * The response object. @@ -239,14 +232,17 @@ protected function getResultValue(ResponseInterface $response, $key, $format) { } /** - * Reset all flood entries. + * Resets all flood entries. */ protected function resetFlood() { \Drupal::database()->delete(DatabaseBackend::TABLE_NAME)->execute(); } /** - * Test the global login flood control. + * Tests the global login flood control. + * + * @see \Drupal\basic_auth\Tests\Authentication\BasicAuthTest::testGlobalLoginFloodControl + * @see \Drupal\user\Tests\UserLoginTest::testGlobalLoginFloodControl */ public function testGlobalLoginFloodControl() { $this->config('user.flood') @@ -257,17 +253,17 @@ public function testGlobalLoginFloodControl() { $user = $this->drupalCreateUser(array()); $incorrect_user = clone $user; - $incorrect_user->pass_raw .= 'incorrect'; + $incorrect_user->passRaw .= 'incorrect'; // Try 2 failed logins. for ($i = 0; $i < 2; $i++) { - $response = $this->loginRequest($incorrect_user->getUsername(), $incorrect_user->pass_raw); + $response = $this->loginRequest($incorrect_user->getUsername(), $incorrect_user->passRaw); $this->assertEquals('400', $response->getStatusCode()); } // IP limit has reached to its limit. Even valid user credentials will fail. - $response = $this->loginRequest($user->getUsername(), $user->pass_raw); - $this->assertResponseWithMessage($response, '403', 'Access is blocked because of IP based flood prevention.', 'json'); + $response = $this->loginRequest($user->getUsername(), $user->passRaw); + $this->assertResponseWithMessage($response, '403', 'Access is blocked because of IP based flood prevention.'); } /** @@ -310,9 +306,90 @@ protected function assertResponse(ResponseInterface $response, $expected_code, $ * @param string $format * The format that the response is encoded in. */ - protected function assertResponseWithMessage(ResponseInterface $response, $expected_code, $expected_message, $format) { + protected function assertResponseWithMessage(ResponseInterface $response, $expected_code, $expected_message, $format = 'json') { $this->assertEquals($expected_code, $response->getStatusCode()); $this->assertEquals($expected_message, $this->getResultValue($response, 'message', $format)); } + /** + * Test the per-user login flood control. + * + * @see \Drupal\user\Tests\UserLoginTest::testPerUserLoginFloodControl + * @see \Drupal\basic_auth\Tests\Authentication\BasicAuthTest::testPerUserLoginFloodControl + */ + public function testPerUserLoginFloodControl() { + 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. + ->set('ip_limit', 4000) + ->set('user_limit', 3) + ->set('uid_only', $uid_only_setting) + ->save(); + + $user1 = $this->drupalCreateUser(array()); + $incorrect_user1 = clone $user1; + $incorrect_user1->passRaw .= 'incorrect'; + + $user2 = $this->drupalCreateUser(array()); + + // Try 2 failed logins. + for ($i = 0; $i < 2; $i++) { + $response = $this->loginRequest($incorrect_user1->getUsername(), $incorrect_user1->passRaw); + $this->assertResponseWithMessage($response, 400, 'Sorry, unrecognized username or password.'); + } + + // A successful login will reset the per-user flood control count. + $this->loginRequest($user1->getUsername(), $user1->passRaw); + $this->logoutRequest(); + + // Try 3 failed logins for user 1, they will not trigger flood control. + for ($i = 0; $i < 3; $i++) { + $response = $this->loginRequest($incorrect_user1->getUsername(), $incorrect_user1->passRaw); + $this->assertResponseWithMessage($response, 400, 'Sorry, unrecognized username or password.'); + } + + // Try one successful attempt for user 2, it should not trigger any + // flood control. + $this->drupalLogin($user2); + $this->drupalLogout(); + + // Try one more attempt for user 1, it should be rejected, even if the + // correct password has been used. + $response = $this->loginRequest($user1->getUsername(), $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.'; + } + else { + $excepted_message = 'Too many failed login attempts from your IP address. This IP address is temporarily blocked.'; + } + $this->assertResponseWithMessage($response, 403, $excepted_message); + } + + } + + /** + * Executes a logout HTTP request. + * + * @param string $format + * The format to use to make the request. + * + * @return \Psr\Http\Message\ResponseInterface The HTTP response. + * The HTTP response. + */ + protected function logoutRequest($format = 'json') { + $client = \Drupal::httpClient(); + $user_logout_url = Url::fromRoute('user.logout.http') + ->setRouteParameter('_format', $format) + ->setAbsolute(); + $response = $client->post($user_logout_url->toString(), [ + 'headers' => [ + 'Accept' => "application/$format", + ], + 'http_errors' => FALSE, + 'cookies' => $this->cookies, + ]); + return $response; + } + }