Issue #3485174 by fago, arthur_lorenz, alexpott, smustgrave: Menu APIs provide invalid CSRF tokens (438a6e04) · Commits · project / drupal

core/core.services.yml

+1 −1

Original line number	Diff line number	Diff line
		@@ -1481,7 +1481,7 @@ services:
		class: Drupal\Core\Access\RouteProcessorCsrf
		tags:
		- { name: route_processor_outbound }
		arguments: ['@csrf_token']
		arguments: ['@csrf_token', '@request_stack']
		transliteration:
		class: Drupal\Core\Transliteration\PhpTransliteration
		arguments: [null, '@module_handler']

core/lib/Drupal/Core/Access/RouteProcessorCsrf.php

+12 −12

Original line number	Diff line number	Diff line
		@@ -6,6 +6,7 @@
		use Drupal\Core\Render\BubbleableMetadata;
		use Drupal\Core\Security\TrustedCallbackInterface;
		use Drupal\Core\RouteProcessor\OutboundRouteProcessorInterface;
		use Symfony\Component\HttpFoundation\RequestStack;
		use Symfony\Component\Routing\Route;

		/**
		@@ -13,21 +14,21 @@
		*/
		class RouteProcessorCsrf implements OutboundRouteProcessorInterface, TrustedCallbackInterface {

		/**
		* The CSRF token generator.
		*
		* @var \Drupal\Core\Access\CsrfTokenGenerator
		*/
		protected $csrfToken;

		/**
		* Constructs a RouteProcessorCsrf object.
		*
		* @param \Drupal\Core\Access\CsrfTokenGenerator $csrf_token
		* @param \Drupal\Core\Access\CsrfTokenGenerator $csrfToken
		* The CSRF token generator.
		* @param \Symfony\Component\HttpFoundation\RequestStack\|null $requestStack
		* The request stack.
		*/
		public function __construct(CsrfTokenGenerator $csrf_token) {
		$this->csrfToken = $csrf_token;
		public function __construct(
		protected CsrfTokenGenerator $csrfToken,
		protected ?RequestStack $requestStack = NULL,
		) {
		if ($requestStack === NULL) {
		$this->requestStack = \Drupal::service('request_stack');
		}
		}

		/**
		@@ -42,7 +43,7 @@ public function processOutbound($route_name, Route $route, array &$parameters, ?
		}
		// Adding this to the parameters means it will get merged into the query
		// string when the route is compiled.
		if (!$bubbleable_metadata) {
		if (!$bubbleable_metadata \|\| $this->requestStack->getCurrentRequest()->getRequestFormat() !== 'html') {
		$parameters['token'] = $this->csrfToken->get($path);
		}
		else {
		@@ -51,7 +52,6 @@ public function processOutbound($route_name, Route $route, array &$parameters, ?
		$placeholder_render_array = [
		'#lazy_builder' => ['route_processor_csrf:renderPlaceholderCsrfToken', [$path]],
		];

		// Instead of setting an actual CSRF token as the query string, we set
		// the placeholder, which will be replaced at the very last moment. This
		// ensures links with CSRF tokens don't break cacheability.

core/modules/system/src/Routing/MenuLinksetRoutes.php

+1 −0

Original line number	Diff line number	Diff line
		@@ -103,6 +103,7 @@ public function routes() {
		],
		[
		'_access' => 'TRUE',
		'_format' => 'json',
		],
		[
		'parameters' => [

core/tests/Drupal/Tests/Core/Access/RouteProcessorCsrfTest.php

+57 −7

Original line number	Diff line number	Diff line
		@@ -5,9 +5,12 @@
		namespace Drupal\Tests\Core\Access;

		use Drupal\Component\Utility\Crypt;
		use Drupal\Core\Access\CsrfTokenGenerator;
		use Drupal\Core\Render\BubbleableMetadata;
		use Drupal\Tests\UnitTestCase;
		use Drupal\Core\Access\RouteProcessorCsrf;
		use PHPUnit\Framework\MockObject\MockObject;
		use Symfony\Component\HttpFoundation\RequestStack;
		use Symfony\Component\Routing\Route;

		/**
		@@ -18,17 +21,18 @@ class RouteProcessorCsrfTest extends UnitTestCase {

		/**
		* The mock CSRF token generator.
		*
		* @var \Drupal\Core\Access\CsrfTokenGenerator\|\PHPUnit\Framework\MockObject\MockObject
		*/
		protected $csrfToken;
		protected CsrfTokenGenerator&MockObject $csrfToken;

		/**
		* The mock request stack.
		*/
		protected RequestStack&MockObject $requestStack;

		/**
		* The route processor.
		*
		* @var \Drupal\Core\Access\RouteProcessorCsrf
		*/
		protected $processor;
		protected RouteProcessorCsrf $processor;

		/**
		* {@inheritdoc}
		@@ -40,7 +44,20 @@ protected function setUp(): void {
		->disableOriginalConstructor()
		->getMock();

		$this->processor = new RouteProcessorCsrf($this->csrfToken);
		$this->requestStack = $this->getMockBuilder('Symfony\Component\HttpFoundation\RequestStack')
		->disableOriginalConstructor()
		->getMock();

		$request = $this->createMock('Symfony\Component\HttpFoundation\Request');
		$request->expects($this->any())
		->method('getRequestFormat')
		->willReturn('html');

		$this->requestStack->expects($this->any())
		->method('getCurrentRequest')
		->willReturn($request);

		$this->processor = new RouteProcessorCsrf($this->csrfToken, $this->requestStack);
		}

		/**
		@@ -122,4 +139,37 @@ public function testProcessOutboundDynamicTwo(): void {
		$this->assertEquals((new BubbleableMetadata())->setAttachments(['placeholders' => [$placeholder => $placeholder_render_array]]), $bubbleable_metadata);
		}

		/**
		* Tests JSON requests to get no placeholders, but real tokens.
		*/
		public function testProcessOutboundJsonFormat(): void {
		// Create a new request mock that returns 'json' format.
		$request = $this->createMock('Symfony\Component\HttpFoundation\Request');
		$request->expects($this->once())
		->method('getRequestFormat')
		->willReturn('json');
		$this->requestStack = $this->createMock('Symfony\Component\HttpFoundation\RequestStack');
		$this->requestStack->expects($this->once())
		->method('getCurrentRequest')
		->willReturn($request);

		// Mock that the CSRF token service should be called once with 'test-path'
		// and return a test token.
		$this->csrfToken->expects($this->any())
		->method('get')
		->with('test-path')
		->willReturn('real_token_value');

		$this->processor = new RouteProcessorCsrf($this->csrfToken, $this->requestStack);

		$route = new Route('/test-path', [], ['_csrf_token' => 'TRUE']);
		$parameters = [];
		// For JSON requests, the actual CSRF token should be in parameters,
		// regardless of whether cache metadata is present.
		$this->processor->processOutbound('test', $route, $parameters);
		$this->assertEquals('real_token_value', $parameters['token']);
		$this->processor->processOutbound('test', $route, $parameters, new BubbleableMetadata());
		$this->assertEquals('real_token_value', $parameters['token']);
		}

		}