diff --git a/core/core.services.yml b/core/core.services.yml index 7b0e996..9a16989 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -1100,7 +1100,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', '@current_user'] maintenance_mode: class: Drupal\Core\Site\MaintenanceMode arguments: ['@state', '@current_user'] @@ -1249,7 +1249,7 @@ services: class: Drupal\Core\Access\RouteProcessorCsrf tags: - { name: route_processor_outbound } - arguments: ['@csrf_token'] + arguments: ['@csrf_token', '@current_user'] transliteration: class: Drupal\Core\Transliteration\PhpTransliteration arguments: [null, '@module_handler'] diff --git a/core/lib/Drupal/Core/Access/CsrfAccessCheck.php b/core/lib/Drupal/Core/Access/CsrfAccessCheck.php index c72b116..7ed8c9b 100644 --- a/core/lib/Drupal/Core/Access/CsrfAccessCheck.php +++ b/core/lib/Drupal/Core/Access/CsrfAccessCheck.php @@ -4,6 +4,7 @@ use Drupal\Core\Routing\Access\AccessInterface as RoutingAccessInterface; use Drupal\Core\Routing\RouteMatchInterface; +use Drupal\Core\Session\AccountInterface; use Symfony\Component\Routing\Route; use Symfony\Component\HttpFoundation\Request; @@ -24,13 +25,19 @@ class CsrfAccessCheck implements RoutingAccessInterface { protected $csrfToken; /** + * @var \Drupal\Core\Session\AccountInterface + */ + protected $account; + + /** * Constructs a CsrfAccessCheck object. * * @param \Drupal\Core\Access\CsrfTokenGenerator $csrf_token * The CSRF token generator. */ - public function __construct(CsrfTokenGenerator $csrf_token) { + public function __construct(CsrfTokenGenerator $csrf_token, AccountInterface $account) { $this->csrfToken = $csrf_token; + $this->account = $account; } /** @@ -54,7 +61,7 @@ public function access(Route $route, Request $request, RouteMatchInterface $rout $path = str_replace("{{$param}}", $value, $path); } - if ($this->csrfToken->validate($request->query->get('token'), $path)) { + if ($this->account->isAnonymous() || $this->csrfToken->validate($request->query->get('token'), $path)) { $result = AccessResult::allowed(); } else { diff --git a/core/lib/Drupal/Core/Access/RouteProcessorCsrf.php b/core/lib/Drupal/Core/Access/RouteProcessorCsrf.php index ce0a261..2d18864 100644 --- a/core/lib/Drupal/Core/Access/RouteProcessorCsrf.php +++ b/core/lib/Drupal/Core/Access/RouteProcessorCsrf.php @@ -4,6 +4,7 @@ use Drupal\Core\Render\BubbleableMetadata; use Drupal\Core\RouteProcessor\OutboundRouteProcessorInterface; +use Drupal\Core\Session\AccountInterface; use Symfony\Component\Routing\Route; /** @@ -19,20 +20,26 @@ class RouteProcessorCsrf implements OutboundRouteProcessorInterface { protected $csrfToken; /** + * @var \Drupal\Core\Session\AccountInterface + */ + protected $account; + + /** * Constructs a RouteProcessorCsrf object. * * @param \Drupal\Core\Access\CsrfTokenGenerator $csrf_token * The CSRF token generator. */ - function __construct(CsrfTokenGenerator $csrf_token) { + function __construct(CsrfTokenGenerator $csrf_token, AccountInterface $account) { $this->csrfToken = $csrf_token; + $this->account = $account; } /** * {@inheritdoc} */ public function processOutbound($route_name, Route $route, array &$parameters, BubbleableMetadata $bubbleable_metadata = NULL) { - if ($route->hasRequirement('_csrf_token')) { + if ($route->hasRequirement('_csrf_token') && $this->account->isAuthenticated()) { $path = ltrim($route->getPath(), '/'); // Replace the path parameters with values from the parameters array. foreach ($parameters as $param => $value) { diff --git a/core/modules/menu_link_content/tests/src/Kernel/MenuLinkContentCacheabilityBubblingTest.php b/core/modules/menu_link_content/tests/src/Kernel/MenuLinkContentCacheabilityBubblingTest.php index c7689d9..dd3479a 100644 --- a/core/modules/menu_link_content/tests/src/Kernel/MenuLinkContentCacheabilityBubblingTest.php +++ b/core/modules/menu_link_content/tests/src/Kernel/MenuLinkContentCacheabilityBubblingTest.php @@ -6,9 +6,11 @@ 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; @@ -44,6 +46,10 @@ protected function setUp() { * Tests bubbleable metadata of menu links' outbound route/path processing. */ public function testOutboundPathAndRouteProcessing() { + $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(); diff --git a/core/modules/system/src/Tests/Common/UrlTest.php b/core/modules/system/src/Tests/Common/UrlTest.php index 062e3e4..1955755 100644 --- a/core/modules/system/src/Tests/Common/UrlTest.php +++ b/core/modules/system/src/Tests/Common/UrlTest.php @@ -8,6 +8,7 @@ use Drupal\Core\Render\RenderContext; use Drupal\Core\Url; use Drupal\simpletest\WebTestBase; +use Drupal\user\Entity\User; /** * Confirm that \Drupal\Core\Url, @@ -43,6 +44,7 @@ function testLinkXSS() { * Tests that #type=link bubbles outbound route/path processors' metadata. */ function testLinkBubbleableMetadata() { + \Drupal::currentUser()->setAccount(User::load(1)); $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 --git a/core/tests/Drupal/Tests/Core/Access/CsrfAccessCheckTest.php b/core/tests/Drupal/Tests/Core/Access/CsrfAccessCheckTest.php index 3ed4e84..b0bf9b0 100644 --- a/core/tests/Drupal/Tests/Core/Access/CsrfAccessCheckTest.php +++ b/core/tests/Drupal/Tests/Core/Access/CsrfAccessCheckTest.php @@ -31,24 +31,34 @@ 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; + /** + * @var \Drupal\Core\Session\AccountInterface|\PHPUnit_Framework_MockObject_MockObject + */ + protected $account; + protected function setUp() { $this->csrfToken = $this->getMockBuilder('Drupal\Core\Access\CsrfTokenGenerator') ->disableOriginalConstructor() ->getMock(); $this->routeMatch = $this->getMock('Drupal\Core\Routing\RouteMatchInterface'); + $this->account = $this->getMock('Drupal\Core\Session\AccountInterface'); - $this->accessCheck = new CsrfAccessCheck($this->csrfToken); + $this->accessCheck = new CsrfAccessCheck($this->csrfToken, $this->account); } /** * Tests the access() method with a valid token. */ public function testAccessTokenPass() { + $this->account->expects($this->once()) + ->method('isAnonymous') + ->willReturn(FALSE); + $this->csrfToken->expects($this->once()) ->method('validate') ->with('test_query', 'test-path/42') @@ -65,6 +75,24 @@ public function testAccessTokenPass() { } /** + * Tests the access() method for anonymous users. + */ + public function testAccessTokenAnonymousPass() { + $this->account->expects($this->once()) + ->method('isAnonymous') + ->willReturn(TRUE); + $this->routeMatch->expects($this->once()) + ->method('getRawParameters') + ->will($this->returnValue(array('node' => 42))); + + $route = new Route('/test-path/{node}', array(), array('_csrf_token' => 'TRUE')); + $request = Request::create('/test-path/42?token=test_query'); + $this->csrfToken->expects($this->never()) + ->method('validate'); + $this->assertEquals(AccessResult::allowed()->setCacheMaxAge(0), $this->accessCheck->access($route, $request, $this->routeMatch)); + } + + /** * Tests the access() method with an invalid token. */ public function testAccessTokenFail() { diff --git a/core/tests/Drupal/Tests/Core/Access/RouteProcessorCsrfTest.php b/core/tests/Drupal/Tests/Core/Access/RouteProcessorCsrfTest.php index d1f8f9e..9a70384 100644 --- a/core/tests/Drupal/Tests/Core/Access/RouteProcessorCsrfTest.php +++ b/core/tests/Drupal/Tests/Core/Access/RouteProcessorCsrfTest.php @@ -3,6 +3,7 @@ namespace Drupal\Tests\Core\Access; use Drupal\Core\Render\BubbleableMetadata; +use Drupal\Core\Session\AccountInterface; use Drupal\Tests\UnitTestCase; use Drupal\Core\Access\RouteProcessorCsrf; use Symfony\Component\Routing\Route; @@ -27,12 +28,18 @@ class RouteProcessorCsrfTest extends UnitTestCase { */ protected $processor; + /** + * @var \Drupal\Core\Session\AccountInterface|\PHPUnit_Framework_MockObject_MockObject + */ + protected $account; + protected function setUp() { $this->csrfToken = $this->getMockBuilder('Drupal\Core\Access\CsrfTokenGenerator') ->disableOriginalConstructor() ->getMock(); + $this->account = $this->getMock(AccountInterface::class); - $this->processor = new RouteProcessorCsrf($this->csrfToken); + $this->processor = new RouteProcessorCsrf($this->csrfToken, $this->account); } /** @@ -58,6 +65,9 @@ public function testProcessOutboundNoRequirement() { * Tests the processOutbound() method with a _csrf_token route requirement. */ public function testProcessOutbound() { + $this->account->expects($this->once()) + ->method('isAuthenticated') + ->willReturn(TRUE); $route = new Route('/test-path', array(), array('_csrf_token' => 'TRUE')); $parameters = array(); @@ -77,9 +87,30 @@ public function testProcessOutbound() { } /** + * Tests the processOutbound() method for anonymous users. + */ + public function testProcessOutboundForAnonymous() { + $this->account->expects($this->once()) + ->method('isAuthenticated') + ->willReturn(FALSE); + $this->csrfToken->expects($this->never()) + ->method('get'); + $route = new Route('/test-path', array(), array('_csrf_token' => 'TRUE')); + $parameters = array(); + + $bubbleable_metadata = new BubbleableMetadata(); + $this->processor->processOutbound('test', $route, $parameters, $bubbleable_metadata); + $this->assertEmpty($parameters); + $this->assertEquals((new BubbleableMetadata()), $bubbleable_metadata); + } + + /** * Tests the processOutbound() method with a dynamic path and one replacement. */ public function testProcessOutboundDynamicOne() { + $this->account->expects($this->once()) + ->method('isAuthenticated') + ->willReturn(TRUE); $route = new Route('/test-path/{slug}', array(), array('_csrf_token' => 'TRUE')); $parameters = array('slug' => 100); @@ -99,6 +130,9 @@ public function testProcessOutboundDynamicOne() { * Tests the processOutbound() method with two parameter replacements. */ public function testProcessOutboundDynamicTwo() { + $this->account->expects($this->once()) + ->method('isAuthenticated') + ->willReturn(TRUE); $route = new Route('{slug_1}/test-path/{slug_2}', array(), array('_csrf_token' => 'TRUE')); $parameters = array('slug_1' => 100, 'slug_2' => 'test');