diff --git a/core/modules/user/src/Controller/UserAuthenticationController.php b/core/modules/user/src/Controller/UserAuthenticationController.php index c8644f6..569a121 100644 --- a/core/modules/user/src/Controller/UserAuthenticationController.php +++ b/core/modules/user/src/Controller/UserAuthenticationController.php @@ -6,6 +6,7 @@ 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\UserInterface; use Drupal\user\UserStorageInterface; @@ -65,6 +66,13 @@ class UserAuthenticationController extends ControllerBase implements ContainerIn protected $userAuth; /** + * The route provider. + * + * @var \Drupal\Core\Routing\RouteProviderInterface + */ + protected $routeProvider; + + /** * The serializer. * * @var \Symfony\Component\Serializer\Serializer @@ -89,18 +97,21 @@ class UserAuthenticationController extends ControllerBase implements ContainerIn * The CSRF token generator. * @param \Drupal\user\UserAuthInterface $user_auth * The user authentication. + * @param \Drupal\Core\Routing\RouteProviderInterface $route_provider + * The route provider. * @param \Symfony\Component\Serializer\Serializer $serializer * The serializer. * @param array $serializer_formats * The available serialization formats. */ - public function __construct(FloodInterface $flood, UserStorageInterface $user_storage, CsrfTokenGenerator $csrf_token, UserAuthInterface $user_auth, Serializer $serializer, array $serializer_formats) { + public function __construct(FloodInterface $flood, UserStorageInterface $user_storage, CsrfTokenGenerator $csrf_token, UserAuthInterface $user_auth, RouteProviderInterface $route_provider, Serializer $serializer, array $serializer_formats) { $this->flood = $flood; $this->userStorage = $user_storage; $this->csrfToken = $csrf_token; $this->userAuth = $user_auth; $this->serializer = $serializer; $this->serializerFormats = $serializer_formats; + $this->routeProvider = $route_provider; } /** @@ -122,6 +133,7 @@ public static function create(ContainerInterface $container) { $container->get('entity_type.manager')->getStorage('user'), $container->get('csrf_token'), $container->get('user.auth'), + $container->get('router.route_provider'), $serializer, $formats ); @@ -177,6 +189,11 @@ public function login(Request $request) { } $response_data['csrf_token'] = $this->csrfToken->get('rest'); + $logout_route = $this->routeProvider->getRouteByName('user.logout.http'); + // Trim '/' off path to match \Drupal\Core\Access\CsrfAccessCheck. + $logout_path = ltrim($logout_route->getPath(), '/'); + $response_data['logout_token'] = $this->csrfToken->get($logout_path); + $encoded_response_data = $this->serializer->encode($response_data, $format); return new Response($encoded_response_data); } diff --git a/core/modules/user/tests/src/Functional/UserLoginHttpTest.php b/core/modules/user/tests/src/Functional/UserLoginHttpTest.php index 8bd1e5a..3565441 100644 --- a/core/modules/user/tests/src/Functional/UserLoginHttpTest.php +++ b/core/modules/user/tests/src/Functional/UserLoginHttpTest.php @@ -104,11 +104,8 @@ public function testLogin() { $name = $account->getUsername(); $pass = $account->passRaw; - $user_login_status_url = Url::fromRoute('user.login_status.http'); - $user_login_status_url->setRouteParameter('_format', $format); - $user_login_status_url->setAbsolute(); - - $response = $client->get($user_login_status_url->toString()); + $login_status_url = $this->getLoginStatusUrlString($format); + $response = $client->get($login_status_url); $this->assertHttpResponse($response, 200, UserAuthenticationController::LOGGED_OUT); // Flooded. @@ -166,15 +163,15 @@ public function testLogin() { $this->assertEquals($name, $result_data['current_user']['name']); $this->assertEquals($account->id(), $result_data['current_user']['uid']); $this->assertEquals($account->getRoles(), $result_data['current_user']['roles']); + $logout_token = $result_data['logout_token']; - $response = $client->get($user_login_status_url->toString(), ['cookies' => $this->cookies]); - $this->assertEquals(200, $response->getStatusCode()); - $this->assertEquals(UserAuthenticationController::LOGGED_IN, (string) $response->getBody()); + $response = $client->get($login_status_url, ['cookies' => $this->cookies]); + $this->assertHttpResponse($response, 200, UserAuthenticationController::LOGGED_IN); - $response = $this->logoutRequest($format); + $response = $this->logoutRequest($format, $logout_token); $this->assertEquals(204, $response->getStatusCode()); - $response = $client->get($user_login_status_url->toString(), ['cookies' => $this->cookies]); + $response = $client->get($login_status_url, ['cookies' => $this->cookies]); $this->assertHttpResponse($response, 200, UserAuthenticationController::LOGGED_OUT); $this->resetFlood(); @@ -300,8 +297,9 @@ public function testPerUserLoginFloodControl() { } // A successful login will reset the per-user flood control count. - $this->loginRequest($user1->getUsername(), $user1->passRaw); - $this->logoutRequest(); + $response = $this->loginRequest($user1->getUsername(), $user1->passRaw); + $result_data = $this->serializer->decode($response->getBody(), 'json'); + $this->logoutRequest('json', $result_data['logout_token']); // Try 3 failed logins for user 1, they will not trigger flood control. for ($i = 0; $i < 3; $i++) { @@ -334,23 +332,90 @@ public function testPerUserLoginFloodControl() { * * @param string $format * The format to use to make the request. + * @param string $logout_token + * The csrf token for user logout. * * @return \Psr\Http\Message\ResponseInterface The HTTP response. * The HTTP response. */ - protected function logoutRequest($format = 'json') { + protected function logoutRequest($format = 'json', $logout_token = '') { + /** @var \GuzzleHttp\Client $client */ $client = $this->container->get('http_client'); $user_logout_url = Url::fromRoute('user.logout.http') ->setRouteParameter('_format', $format) ->setAbsolute(); - $response = $client->post($user_logout_url->toString(), [ + if ($logout_token) { + $user_logout_url->setOption('query', ['token' => $logout_token]); + } + $post_options = [ 'headers' => [ 'Accept' => "application/$format", ], 'http_errors' => FALSE, 'cookies' => $this->cookies, - ]); + ]; + + $response = $client->post($user_logout_url->toString(), $post_options); return $response; } + /** + * Test csrf protection of User Logout route. + */ + public function testLogoutCsrfProtection() { + $client = \Drupal::httpClient(); + $login_status_url = $this->getLoginStatusUrlString(); + $account = $this->drupalCreateUser(); + $name = $account->getUsername(); + $pass = $account->passRaw; + + $response = $this->loginRequest($name, $pass); + $this->assertEquals(200, $response->getStatusCode()); + $result_data = $this->serializer->decode($response->getBody(), 'json'); + + $logout_token = $result_data['logout_token']; + + // Test third party site posting to current site with logout request. + // This should not logout the current user because it lacks the CSRF + // token. + $response = $this->logoutRequest('json'); + $this->assertEquals(403, $response->getStatusCode()); + + // Ensure still logged in. + $response = $client->get($login_status_url, ['cookies' => $this->cookies]); + $this->assertHttpResponse($response, 200, UserAuthenticationController::LOGGED_IN); + + // Try with an incorrect token. + $response = $this->logoutRequest('json', 'not-the-correct-token'); + $this->assertEquals(403, $response->getStatusCode()); + + // Ensure still logged in. + $response = $client->get($login_status_url, ['cookies' => $this->cookies]); + $this->assertHttpResponse($response, 200, UserAuthenticationController::LOGGED_IN); + + // Try a logout request with correct token. + $response = $this->logoutRequest('json', $logout_token); + $this->assertEquals(204, $response->getStatusCode()); + + // Ensure actually logged out. + $response = $client->get($login_status_url, ['cookies' => $this->cookies]); + $this->assertHttpResponse($response, 200, UserAuthenticationController::LOGGED_OUT); + } + + /** + * Gets the URL string for checking login. + * + * @param string $format + * The format to use to make the request. + * + * @return string + * The URL string. + */ + protected function getLoginStatusUrlString($format = 'json') { + $user_login_status_url = Url::fromRoute('user.login_status.http'); + $user_login_status_url->setRouteParameter('_format', $format); + $user_login_status_url->setAbsolute(); + return $user_login_status_url->toString(); + } + } diff --git a/core/modules/user/user.routing.yml b/core/modules/user/user.routing.yml index 9cf1949..caea979 100644 --- a/core/modules/user/user.routing.yml +++ b/core/modules/user/user.routing.yml @@ -155,6 +155,7 @@ user.logout.http: requirements: _user_is_logged_in: 'TRUE' _format: 'json' + _csrf_token: 'TRUE' user.cancel_confirm: path: '/user/{user}/cancel/confirm/{timestamp}/{hashed_pass}'