diff --git a/core/core.services.yml b/core/core.services.yml index 1ee07fe..ed35589 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -1099,7 +1099,7 @@ services: class: Drupal\Core\Access\CsrfAccessCheck tags: - { name: access_check, applies_to: _csrf_token, needs_incoming_request: TRUE } - arguments: ['@csrf_token'] + arguments: ['@csrf_token', '@session_configuration'] maintenance_mode: class: Drupal\Core\Site\MaintenanceMode arguments: ['@state', '@current_user'] diff --git a/core/lib/Drupal/Core/Access/CsrfAccessCheck.php b/core/lib/Drupal/Core/Access/CsrfAccessCheck.php index c72b116..0ca862e 100644 --- a/core/lib/Drupal/Core/Access/CsrfAccessCheck.php +++ b/core/lib/Drupal/Core/Access/CsrfAccessCheck.php @@ -4,15 +4,23 @@ use Drupal\Core\Routing\Access\AccessInterface as RoutingAccessInterface; use Drupal\Core\Routing\RouteMatchInterface; +use Drupal\Core\Session\AccountInterface; +use Drupal\Core\Session\SessionConfigurationInterface; use Symfony\Component\Routing\Route; use Symfony\Component\HttpFoundation\Request; /** * Allows access to routes to be controlled by a '_csrf_token' parameter. * - * To use this check, add a "token" GET parameter to URLs of which the value is - * a token generated by \Drupal::csrfToken()->get() using the same value as the - * "_csrf_token" parameter in the route. + * To use this check you can either provide the token via the URL or the http + * header. + * + * To use this check via the URL, add a "token" GET parameter to URLs of which + * the value is a token generated by \Drupal::csrfToken()->get() using the same + * value as the "_csrf_token" parameter in the route. + * + * To use this token via the header, provide the token the 'X-CSRF-Token' http + * header. */ class CsrfAccessCheck implements RoutingAccessInterface { @@ -24,13 +32,23 @@ class CsrfAccessCheck implements RoutingAccessInterface { protected $csrfToken; /** + * The session configuration. + * + * @var \Drupal\Core\Session\SessionConfigurationInterface + */ + protected $sessionConfiguration; + + /** * Constructs a CsrfAccessCheck object. * * @param \Drupal\Core\Access\CsrfTokenGenerator $csrf_token * The CSRF token generator. + * @param \Drupal\Core\Session\SessionConfigurationInterface $session_configuration + * The session configuration. */ - public function __construct(CsrfTokenGenerator $csrf_token) { + public function __construct(CsrfTokenGenerator $csrf_token, SessionConfigurationInterface $session_configuration) { $this->csrfToken = $csrf_token; + $this->sessionConfiguration = $session_configuration; } /** @@ -42,26 +60,91 @@ public function __construct(CsrfTokenGenerator $csrf_token) { * The request object. * @param \Drupal\Core\Routing\RouteMatchInterface $route_match * The route match object. + * @param \Drupal\Core\Session\AccountInterface $account + * The currently logged in account. * * @return \Drupal\Core\Access\AccessResultInterface * The access result. */ - public function access(Route $route, Request $request, RouteMatchInterface $route_match) { - $parameters = $route_match->getRawParameters(); - $path = ltrim($route->getPath(), '/'); - // Replace the path parameters with values from the parameters array. - foreach ($parameters as $param => $value) { - $path = str_replace("{{$param}}", $value, $path); - } - - if ($this->csrfToken->validate($request->query->get('token'), $path)) { - $result = AccessResult::allowed(); + public function access(Route $route, Request $request, RouteMatchInterface $route_match, AccountInterface $account) { + $result = AccessResult::forbidden(); + $header_access = $this->headerAccess($request, $account); + if (!$header_access->isNeutral()) { + $result = $header_access; } else { - $result = AccessResult::forbidden(); + $path_access = $this->pathAccess($route, $request, $route_match); + if (!$path_access->isNeutral()) { + $result = $path_access; + } } // Not cacheable because the CSRF token is highly dynamic. return $result->setCacheMaxAge(0); } + /** + * Checks for a valid csrf token via the path. + * + * @param \Symfony\Component\Routing\Route $route + * The route to check against. + * @param \Symfony\Component\HttpFoundation\Request $request + * The request object. + * @param \Drupal\Core\Routing\RouteMatchInterface $route_match + * The route match object. + * + * @return \Drupal\Core\Access\AccessResult + * The access result. + */ + protected function pathAccess(Route $route, Request $request, RouteMatchInterface $route_match) { + if ($request->query->has('token')) { + $parameters = $route_match->getRawParameters(); + $path = ltrim($route->getPath(), '/'); + // Replace the path parameters with values from the parameters array. + foreach ($parameters as $param => $value) { + $path = str_replace("{{$param}}", $value, $path); + } + if ($this->csrfToken->validate($request->query->get('token'), $path)) { + return AccessResult::allowed(); + } + else { + return AccessResult::forbidden(); + } + } + return AccessResult::neutral(); + } + + /** + * Checks access based on token provide in 'X-CSRF-Token' header. + * + * @param \Symfony\Component\HttpFoundation\Request $request + * The request object. + * @param \Drupal\Core\Session\AccountInterface $account + * The currently logged in account. + * + * @return \Drupal\Core\Access\AccessResultInterface + * The access result. + */ + private function headerAccess(Request $request, AccountInterface $account) { + $method = $request->getMethod(); + + // This check only applies if + // 1. this is a write operation + // 2. the user was successfully authenticated and + // 3. the request comes with a session cookie. + if (!in_array($method, array('GET', 'HEAD', 'OPTIONS', 'TRACE')) + && $account->isAuthenticated() + && $this->sessionConfiguration->hasSession($request) + ) { + if ($request->headers->has('X-CSRF-Token')) { + $csrf_token = $request->headers->get('X-CSRF-Token'); + if ($this->csrfToken->validate($csrf_token, 'rest')) { + return AccessResult::allowed(); + } + return AccessResult::forbidden(); + } + } + // If no token provided or not a write method then do not allow or forbid. + return AccessResult::neutral(); + } + } diff --git a/core/modules/user/tests/src/Functional/UserLoginHttpTest.php b/core/modules/user/tests/src/Functional/UserLoginHttpTest.php index 8bd1e5a..32c6d28 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']); + $csrf_token = $result_data['csrf_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, $csrf_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['csrf_token']); // Try 3 failed logins for user 1, they will not trigger flood control. for ($i = 0; $i < 3; $i++) { @@ -334,23 +332,89 @@ public function testPerUserLoginFloodControl() { * * @param string $format * The format to use to make the request. + * @param string $csrf_token + * The csrf token. * * @return \Psr\Http\Message\ResponseInterface The HTTP response. * The HTTP response. */ - protected function logoutRequest($format = 'json') { + protected function logoutRequest($format = 'json', $csrf_token = NULL) { + /** @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(), [ + $post_options = [ 'headers' => [ 'Accept' => "application/$format", ], 'http_errors' => FALSE, 'cookies' => $this->cookies, - ]); + ]; + + if ($csrf_token) { + $post_options['headers']['X-CSRF-Token'] = $csrf_token; + } + + $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'); + + $csrf_token = $result_data['csrf_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', FALSE); + $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', $csrf_token); + $this->assertEquals(204, $response->getStatusCode()); + $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}' diff --git a/core/tests/Drupal/Tests/Core/Access/CsrfAccessCheckTest.php b/core/tests/Drupal/Tests/Core/Access/CsrfAccessCheckTest.php index 3ed4e84..c4a1dd3 100644 --- a/core/tests/Drupal/Tests/Core/Access/CsrfAccessCheckTest.php +++ b/core/tests/Drupal/Tests/Core/Access/CsrfAccessCheckTest.php @@ -31,18 +31,30 @@ class CsrfAccessCheckTest extends UnitTestCase { /** * The mock route match. * - * @var \Drupal\Core\RouteMatch\RouteMatchInterface|\PHPUnit_Framework_MockObject_MockObject + * @var \Drupal\Core\Routing\RouteMatchInterface|\PHPUnit_Framework_MockObject_MockObject */ protected $routeMatch; + /** + * The mock account proxy. + * + * @var \Drupal\Core\Session\AccountProxy|\PHPUnit_Framework_MockObject_MockObject + */ + protected $account; protected function setUp() { $this->csrfToken = $this->getMockBuilder('Drupal\Core\Access\CsrfTokenGenerator') ->disableOriginalConstructor() ->getMock(); + $session_config = $this->getMockBuilder('Drupal\Core\Session\SessionConfiguration') + ->disableOriginalConstructor() + ->getMock(); + $this->routeMatch = $this->getMock('Drupal\Core\Routing\RouteMatchInterface'); - $this->accessCheck = new CsrfAccessCheck($this->csrfToken); + $this->account = $this->getMock('Drupal\Core\Session\AccountProxy'); + + $this->accessCheck = new CsrfAccessCheck($this->csrfToken, $session_config); } /** @@ -61,7 +73,7 @@ public function testAccessTokenPass() { $route = new Route('/test-path/{node}', array(), array('_csrf_token' => 'TRUE')); $request = Request::create('/test-path/42?token=test_query'); - $this->assertEquals(AccessResult::allowed()->setCacheMaxAge(0), $this->accessCheck->access($route, $request, $this->routeMatch)); + $this->assertEquals(AccessResult::allowed()->setCacheMaxAge(0), $this->accessCheck->access($route, $request, $this->routeMatch, $this->account)); } /** @@ -80,7 +92,7 @@ public function testAccessTokenFail() { $route = new Route('/test-path', array(), array('_csrf_token' => 'TRUE')); $request = Request::create('/test-path?token=test_query'); - $this->assertEquals(AccessResult::forbidden()->setCacheMaxAge(0), $this->accessCheck->access($route, $request, $this->routeMatch)); + $this->assertEquals(AccessResult::forbidden()->setCacheMaxAge(0), $this->accessCheck->access($route, $request, $this->routeMatch, $this->account)); } }