diff -u b/core/core.services.yml b/core/core.services.yml --- b/core/core.services.yml +++ b/core/core.services.yml @@ -1100,7 +1100,7 @@ class: Drupal\Core\Access\CsrfAccessCheck tags: - { name: access_check, applies_to: _csrf_token, needs_incoming_request: TRUE } - arguments: ['@csrf_token', '@current_user'] + arguments: ['@csrf_token', '@session_configuration'] maintenance_mode: class: Drupal\Core\Site\MaintenanceMode arguments: ['@state', '@current_user'] @@ -1249,7 +1249,7 @@ class: Drupal\Core\Access\RouteProcessorCsrf tags: - { name: route_processor_outbound } - arguments: ['@csrf_token', '@current_user'] + arguments: ['@csrf_token', '@session_configuration', '@request_stack'] transliteration: class: Drupal\Core\Transliteration\PhpTransliteration arguments: [null, '@module_handler'] diff -u b/core/lib/Drupal/Core/Access/CsrfAccessCheck.php b/core/lib/Drupal/Core/Access/CsrfAccessCheck.php --- b/core/lib/Drupal/Core/Access/CsrfAccessCheck.php +++ b/core/lib/Drupal/Core/Access/CsrfAccessCheck.php @@ -5,6 +5,7 @@ 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; @@ -25,9 +26,9 @@ protected $csrfToken; /** - * @var \Drupal\Core\Session\AccountInterface + * @var \Drupal\Core\Session\SessionConfigurationInterface */ - protected $account; + protected $sessionConfiguration; /** * Constructs a CsrfAccessCheck object. @@ -35,9 +36,9 @@ * @param \Drupal\Core\Access\CsrfTokenGenerator $csrf_token * The CSRF token generator. */ - public function __construct(CsrfTokenGenerator $csrf_token, AccountInterface $account) { + public function __construct(CsrfTokenGenerator $csrf_token, SessionConfigurationInterface $session_configuration) { $this->csrfToken = $csrf_token; - $this->account = $account; + $this->sessionConfiguration = $session_configuration; } /** @@ -63,7 +64,7 @@ // Per https://www.drupal.org/node/2319205 anonymous users do not need // CSRF checking. - if ($this->account->isAnonymous() || $this->csrfToken->validate($request->query->get('token'), $path)) { + if (!$this->sessionConfiguration->hasSession($request) || $this->csrfToken->validate($request->query->get('token'), $path)) { $result = AccessResult::allowed(); } else { diff -u b/core/lib/Drupal/Core/Access/RouteProcessorCsrf.php b/core/lib/Drupal/Core/Access/RouteProcessorCsrf.php --- b/core/lib/Drupal/Core/Access/RouteProcessorCsrf.php +++ b/core/lib/Drupal/Core/Access/RouteProcessorCsrf.php @@ -4,7 +4,8 @@ use Drupal\Core\Render\BubbleableMetadata; use Drupal\Core\RouteProcessor\OutboundRouteProcessorInterface; -use Drupal\Core\Session\AccountInterface; +use Drupal\Core\Session\SessionConfigurationInterface; +use Symfony\Component\HttpFoundation\RequestStack; use Symfony\Component\Routing\Route; /** @@ -20,9 +21,14 @@ protected $csrfToken; /** - * @var \Drupal\Core\Session\AccountInterface + * @var \Drupal\Core\Session\SessionConfigurationInterface */ - protected $account; + protected $sessionConfiguration; + + /** + * @var \Symfony\Component\HttpFoundation\RequestStack + */ + protected $requestStack; /** * Constructs a RouteProcessorCsrf object. @@ -30,9 +36,10 @@ * @param \Drupal\Core\Access\CsrfTokenGenerator $csrf_token * The CSRF token generator. */ - function __construct(CsrfTokenGenerator $csrf_token, AccountInterface $account) { + function __construct(CsrfTokenGenerator $csrf_token, SessionConfigurationInterface $session_configuration, RequestStack $request_stack) { $this->csrfToken = $csrf_token; - $this->account = $account; + $this->sessionConfiguration = $session_configuration; + $this->requestStack = $request_stack; } /** @@ -41,7 +48,7 @@ public function processOutbound($route_name, Route $route, array &$parameters, BubbleableMetadata $bubbleable_metadata = NULL) { // Per https://www.drupal.org/node/2319205 anonymous users do not need // CSRF checking. - if ($route->hasRequirement('_csrf_token') && $this->account->isAuthenticated()) { + if ($route->hasRequirement('_csrf_token') && $this->sessionConfiguration->hasSession($this->requestStack->getCurrentRequest())) { $path = ltrim($route->getPath(), '/'); // Replace the path parameters with values from the parameters array. foreach ($parameters as $param => $value) { diff -u b/core/modules/menu_link_content/tests/src/Kernel/MenuLinkContentCacheabilityBubblingTest.php b/core/modules/menu_link_content/tests/src/Kernel/MenuLinkContentCacheabilityBubblingTest.php --- b/core/modules/menu_link_content/tests/src/Kernel/MenuLinkContentCacheabilityBubblingTest.php +++ b/core/modules/menu_link_content/tests/src/Kernel/MenuLinkContentCacheabilityBubblingTest.php @@ -6,11 +6,9 @@ use Drupal\Core\Language\LanguageInterface; use Drupal\Core\Menu\MenuTreeParameters; use Drupal\Core\Render\BubbleableMetadata; -use Drupal\Core\Session\AccountInterface; use Drupal\menu_link_content\Entity\MenuLinkContent; use Drupal\KernelTests\KernelTestBase; use Drupal\user\Entity\User; -use Prophecy\Argument; use Symfony\Cmf\Component\Routing\RouteObjectInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Routing\Route; @@ -46,12 +44,6 @@ * Tests bubbleable metadata of menu links' outbound route/path processing. */ public function testOutboundPathAndRouteProcessing() { - // Set an authenticated user to make CSRF protected links work. - // @see \Drupal\Core\Access\RouteProcessorCSRF::processOutbound() - $account = $this->prophesize(AccountInterface::class); - $account->isAuthenticated()->willReturn(TRUE); - $account->hasPermission(Argument::any())->wilLReturn(TRUE); - \Drupal::currentUser()->setAccount($account->reveal()); \Drupal::service('router.builder')->rebuild(); $request_stack = \Drupal::requestStack(); @@ -61,13 +53,14 @@ $request = Request::create('/'); $request->attributes->set(RouteObjectInterface::ROUTE_NAME, ''); $request->attributes->set(RouteObjectInterface::ROUTE_OBJECT, new Route('/')); + // Fake a started session. + $request->cookies->add(['SESS' . substr(hash('sha256', $this->getDatabasePrefix()), 0, 32) => '']); $request_stack->push($request); $request_context->fromRequest($request); $menu_tree = \Drupal::menuTree(); $renderer = \Drupal::service('renderer'); - $default_menu_cacheability = (new BubbleableMetadata()) ->setCacheMaxAge(Cache::PERMANENT) ->setCacheTags(['config:system.menu.tools']) diff -u b/core/modules/system/src/Tests/Common/UrlTest.php b/core/modules/system/src/Tests/Common/UrlTest.php --- b/core/modules/system/src/Tests/Common/UrlTest.php +++ b/core/modules/system/src/Tests/Common/UrlTest.php @@ -8,7 +8,6 @@ use Drupal\Core\Render\RenderContext; use Drupal\Core\Url; use Drupal\simpletest\WebTestBase; -use Drupal\user\Entity\User; /** * Confirm that \Drupal\Core\Url, @@ -44,9 +43,9 @@ * Tests that #type=link bubbles outbound route/path processors' metadata. */ function testLinkBubbleableMetadata() { - // Set an authenticated user to make CSRF protected links work. - // @see \Drupal\Core\Access\RouteProcessorCSRF::processOutbound() - \Drupal::currentUser()->setAccount(User::load(1)); + // Fake a started session. + \Drupal::request()->cookies->add(['SESS' . substr(hash('sha256', $this->getDatabasePrefix()), 0, 32) => '']); + $cases = [ ['Regular link', 'internal:/user', [], ['contexts' => [], 'tags' => [], 'max-age' => Cache::PERMANENT], []], ['Regular link, absolute', 'internal:/user', ['absolute' => TRUE], ['contexts' => ['url.site'], 'tags' => [], 'max-age' => Cache::PERMANENT], []], diff -u b/core/tests/Drupal/Tests/Core/Access/CsrfAccessCheckTest.php b/core/tests/Drupal/Tests/Core/Access/CsrfAccessCheckTest.php --- b/core/tests/Drupal/Tests/Core/Access/CsrfAccessCheckTest.php +++ b/core/tests/Drupal/Tests/Core/Access/CsrfAccessCheckTest.php @@ -29,16 +29,9 @@ protected $accessCheck; /** - * The mock route match. - * - * @var \Drupal\Core\Routing\RouteMatchInterface|\PHPUnit_Framework_MockObject_MockObject + * @var \Drupal\Core\Session\SessionConfigurationInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $routeMatch; - - /** - * @var \Drupal\Core\Session\AccountInterface|\PHPUnit_Framework_MockObject_MockObject - */ - protected $account; + protected $sessionConfiguration; protected function setUp() { $this->csrfToken = $this->getMockBuilder('Drupal\Core\Access\CsrfTokenGenerator') @@ -46,18 +39,17 @@ ->getMock(); $this->routeMatch = $this->getMock('Drupal\Core\Routing\RouteMatchInterface'); - $this->account = $this->getMock('Drupal\Core\Session\AccountInterface'); - - $this->accessCheck = new CsrfAccessCheck($this->csrfToken, $this->account); + $this->sessionConfiguration = $this->getMock('Drupal\Core\Session\SessionConfigurationInterface'); + $this->accessCheck = new CsrfAccessCheck($this->csrfToken, $this->sessionConfiguration); } /** * Tests the access() method with a valid token. */ public function testAccessTokenPass() { - $this->account->expects($this->once()) - ->method('isAnonymous') - ->willReturn(FALSE); + $this->sessionConfiguration->expects($this->once()) + ->method('hasSession') + ->willReturn(TRUE); $this->csrfToken->expects($this->once()) ->method('validate') @@ -78,9 +70,9 @@ * Tests the access() method for anonymous users. */ public function testAccessTokenAnonymousPass() { - $this->account->expects($this->once()) - ->method('isAnonymous') - ->willReturn(TRUE); + $this->sessionConfiguration->expects($this->once()) + ->method('hasSession') + ->willReturn(FALSE); $this->routeMatch->expects($this->once()) ->method('getRawParameters') ->will($this->returnValue(array('node' => 42))); @@ -96,6 +88,9 @@ * Tests the access() method with an invalid token. */ public function testAccessTokenFail() { + $this->sessionConfiguration->expects($this->once()) + ->method('hasSession') + ->willReturn(TRUE); $this->csrfToken->expects($this->once()) ->method('validate') ->with('test_query', 'test-path') diff -u b/core/tests/Drupal/Tests/Core/Access/RouteProcessorCsrfTest.php b/core/tests/Drupal/Tests/Core/Access/RouteProcessorCsrfTest.php --- b/core/tests/Drupal/Tests/Core/Access/RouteProcessorCsrfTest.php +++ b/core/tests/Drupal/Tests/Core/Access/RouteProcessorCsrfTest.php @@ -28,23 +28,32 @@ protected $processor; /** - * @var \Drupal\Core\Session\AccountInterface|\PHPUnit_Framework_MockObject_MockObject + * @var \Drupal\Core\Session\SessionConfigurationInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $account; + protected $sessionConfiguration; protected function setUp() { $this->csrfToken = $this->getMockBuilder('Drupal\Core\Access\CsrfTokenGenerator') ->disableOriginalConstructor() ->getMock(); - $this->account = $this->getMock('Drupal\Core\Session\AccountInterface'); + $this->sessionConfiguration = $this->getMock('Drupal\Core\Session\SessionConfigurationInterface'); + $request_stack = $this->getMock('Symfony\Component\HttpFoundation\RequestStack'); + // The number this is called differs between tests and is completely + // irrelevant, the sessionConfiguration mock object will have the exact + // number of calls. + $request_stack->expects($this->atMost(1)) + ->method('getCurrentRequest') + ->willReturn($this->getMock('Symfony\Component\HttpFoundation\Request')); - $this->processor = new RouteProcessorCsrf($this->csrfToken, $this->account); + $this->processor = new RouteProcessorCsrf($this->csrfToken, $this->sessionConfiguration, $request_stack); } /** * Tests the processOutbound() method with no _csrf_token route requirement. */ public function testProcessOutboundNoRequirement() { + $this->sessionConfiguration->expects($this->never()) + ->method('hasSession'); $this->csrfToken->expects($this->never()) ->method('get'); @@ -64,8 +73,8 @@ * Tests the processOutbound() method with a _csrf_token route requirement. */ public function testProcessOutbound() { - $this->account->expects($this->once()) - ->method('isAuthenticated') + $this->sessionConfiguration->expects($this->once()) + ->method('hasSession') ->willReturn(TRUE); $route = new Route('/test-path', array(), array('_csrf_token' => 'TRUE')); $parameters = array(); @@ -89,8 +98,8 @@ * Tests the processOutbound() method for anonymous users. */ public function testProcessOutboundForAnonymous() { - $this->account->expects($this->once()) - ->method('isAuthenticated') + $this->sessionConfiguration->expects($this->once()) + ->method('hasSession') ->willReturn(FALSE); $this->csrfToken->expects($this->never()) ->method('get'); @@ -107,8 +116,8 @@ * Tests the processOutbound() method with a dynamic path and one replacement. */ public function testProcessOutboundDynamicOne() { - $this->account->expects($this->once()) - ->method('isAuthenticated') + $this->sessionConfiguration->expects($this->once()) + ->method('hasSession') ->willReturn(TRUE); $route = new Route('/test-path/{slug}', array(), array('_csrf_token' => 'TRUE')); $parameters = array('slug' => 100); @@ -129,8 +138,8 @@ * Tests the processOutbound() method with two parameter replacements. */ public function testProcessOutboundDynamicTwo() { - $this->account->expects($this->once()) - ->method('isAuthenticated') + $this->sessionConfiguration->expects($this->once()) + ->method('hasSession') ->willReturn(TRUE); $route = new Route('{slug_1}/test-path/{slug_2}', array(), array('_csrf_token' => 'TRUE')); $parameters = array('slug_1' => 100, 'slug_2' => 'test');