diff --git a/core/lib/Drupal/Core/Access/CsrfAccessCheck.php b/core/lib/Drupal/Core/Access/CsrfAccessCheck.php index bd88a31c27cb7c6e3fbeb32843e5bb54f78a828a..f1599b24fdbbe99e86a6cf9881c258bad02a5698 100644 --- a/core/lib/Drupal/Core/Access/CsrfAccessCheck.php +++ b/core/lib/Drupal/Core/Access/CsrfAccessCheck.php @@ -24,12 +24,12 @@ */ class CsrfAccessCheck implements RoutingAccessInterface { + use RoutePathGenerationTrait; + /** * The CSRF token generator. - * - * @var \Drupal\Core\Access\CsrfTokenGenerator */ - protected $csrfToken; + protected CsrfTokenGenerator $csrfToken; /** * Constructs a CsrfAccessCheck object. @@ -55,13 +55,7 @@ public function __construct(CsrfTokenGenerator $csrf_token) { * 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); - } - + $path = $this->generateRoutePath($route, $route_match->getRawParameters()->all()); if ($this->csrfToken->validate($request->query->get('token', ''), $path)) { $result = AccessResult::allowed(); } diff --git a/core/lib/Drupal/Core/Access/RoutePathGenerationTrait.php b/core/lib/Drupal/Core/Access/RoutePathGenerationTrait.php new file mode 100644 index 0000000000000000000000000000000000000000..9ef1e8b609cd3fc2ba1fa809fcf4799314db9758 --- /dev/null +++ b/core/lib/Drupal/Core/Access/RoutePathGenerationTrait.php @@ -0,0 +1,58 @@ +<?php + +namespace Drupal\Core\Access; + +use Symfony\Component\Routing\Route; + +/** + * Provides a method for generating route paths. + */ +trait RoutePathGenerationTrait { + + /** + * Generates a route path by replacing placeholders with their values. + * + * Placeholders without corresponding values in the parameters array + * are removed from the resulting path. + * + * @param \Symfony\Component\Routing\Route $route + * The route object containing the path with placeholders. + * @param array $parameters + * An associative array of parameters to replace in the route path, + * where the keys are placeholders and the values are the replacement + * values. + * @code + * Example: + * [ + * 'parameter1' => 'value1', + * ] + * @endcode + * This will transform a route path such as + * '/route/path/{parameter1}{parameter2}' into '/route/path/value1'. + * + * @return string + * The generated path with all placeholders either replaced by their + * corresponding values or removed if no matching parameter exists. + */ + public function generateRoutePath(Route $route, array $parameters): string { + $path = ltrim($route->getPath(), '/'); + + // Replace path parameters with their corresponding values from the + // parameters array. + foreach ($parameters as $param => $value) { + if (NULL !== $value) { + $path = str_replace("{{$param}}", $value, $path); + } + } + + // Remove placeholders that were not replaced. + $path = preg_replace('/\/{[^}]+}/', '', $path); + + // Remove trailing slashes (multiple slashes may result from the removal of + // unreplaced placeholders). + $path = rtrim($path, '/'); + + return $path; + } + +} diff --git a/core/lib/Drupal/Core/Access/RouteProcessorCsrf.php b/core/lib/Drupal/Core/Access/RouteProcessorCsrf.php index 64bf0f2450c270a30a69701b079bd68870db35f8..901c30c56b64d175e176af9a1cacc6ff810d087d 100644 --- a/core/lib/Drupal/Core/Access/RouteProcessorCsrf.php +++ b/core/lib/Drupal/Core/Access/RouteProcessorCsrf.php @@ -14,6 +14,8 @@ */ class RouteProcessorCsrf implements OutboundRouteProcessorInterface, TrustedCallbackInterface { + use RoutePathGenerationTrait; + /** * Constructs a RouteProcessorCsrf object. * @@ -37,11 +39,7 @@ public function __construct( */ public function processOutbound($route_name, Route $route, array &$parameters, ?BubbleableMetadata $bubbleable_metadata = NULL) { if ($route->hasRequirement('_csrf_token')) { - $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); - } + $path = $this->generateRoutePath($route, $parameters); // Adding this to the parameters means it will get merged into the query // string when the route is compiled. if (!$bubbleable_metadata || $this->requestStack->getCurrentRequest()->getRequestFormat() !== 'html') { diff --git a/core/tests/Drupal/Tests/Core/Access/CsrfAccessCheckTest.php b/core/tests/Drupal/Tests/Core/Access/CsrfAccessCheckTest.php index 34bb870ef4819360046949a9fe828fa172ce7d13..00758a9338d60ea0c0833856872e5b0345b5293f 100644 --- a/core/tests/Drupal/Tests/Core/Access/CsrfAccessCheckTest.php +++ b/core/tests/Drupal/Tests/Core/Access/CsrfAccessCheckTest.php @@ -5,6 +5,9 @@ namespace Drupal\Tests\Core\Access; use Drupal\Core\Access\AccessResult; +use Drupal\Core\Access\CsrfTokenGenerator; +use Drupal\Core\Routing\RouteMatchInterface; +use Symfony\Component\DependencyInjection\ParameterBag\ParameterBagInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Routing\Route; use Drupal\Core\Access\CsrfAccessCheck; @@ -18,24 +21,23 @@ class CsrfAccessCheckTest extends UnitTestCase { /** * The mock CSRF token generator. - * - * @var \Drupal\Core\Access\CsrfTokenGenerator|\PHPUnit\Framework\MockObject\MockObject */ - protected $csrfToken; + protected CsrfTokenGenerator $csrfToken; /** * The access checker. - * - * @var \Drupal\Core\Access\CsrfAccessCheck */ - protected $accessCheck; + protected CsrfAccessCheck $accessCheck; /** * The mock route match. - * - * @var \Drupal\Core\Routing\RouteMatchInterface|\PHPUnit\Framework\MockObject\MockObject */ - protected $routeMatch; + protected RouteMatchInterface $routeMatch; + + /** + * The mock parameter bag. + */ + protected ParameterBagInterface $parameterBag; /** * {@inheritdoc} @@ -43,11 +45,13 @@ class CsrfAccessCheckTest extends UnitTestCase { protected function setUp(): void { parent::setUp(); - $this->csrfToken = $this->getMockBuilder('Drupal\Core\Access\CsrfTokenGenerator') + $this->csrfToken = $this->getMockBuilder(CsrfTokenGenerator::class) ->disableOriginalConstructor() ->getMock(); - $this->routeMatch = $this->createMock('Drupal\Core\Routing\RouteMatchInterface'); + $this->parameterBag = $this->createMock(ParameterBagInterface::class); + + $this->routeMatch = $this->createMock(RouteMatchInterface::class); $this->accessCheck = new CsrfAccessCheck($this->csrfToken); } @@ -61,9 +65,13 @@ public function testAccessTokenPass(): void { ->with('test_query', 'test-path/42') ->willReturn(TRUE); + $this->parameterBag + ->method('all') + ->willReturn(['node' => 42]); + $this->routeMatch->expects($this->once()) ->method('getRawParameters') - ->willReturn(['node' => 42]); + ->willReturn($this->parameterBag); $route = new Route('/test-path/{node}', [], ['_csrf_token' => 'TRUE']); $request = Request::create('/test-path/42?token=test_query'); @@ -80,9 +88,13 @@ public function testCsrfTokenInvalid(): void { ->with('test_query', 'test-path') ->willReturn(FALSE); + $this->parameterBag + ->method('all') + ->willReturn([]); + $this->routeMatch->expects($this->once()) ->method('getRawParameters') - ->willReturn([]); + ->willReturn($this->parameterBag); $route = new Route('/test-path', [], ['_csrf_token' => 'TRUE']); $request = Request::create('/test-path?token=test_query'); @@ -99,9 +111,13 @@ public function testCsrfTokenMissing(): void { ->with('', 'test-path') ->willReturn(FALSE); + $this->parameterBag + ->method('all') + ->willReturn([]); + $this->routeMatch->expects($this->once()) ->method('getRawParameters') - ->willReturn([]); + ->willReturn($this->parameterBag); $route = new Route('/test-path', [], ['_csrf_token' => 'TRUE']); $request = Request::create('/test-path'); diff --git a/core/tests/Drupal/Tests/Core/Access/RoutePathGenerationTraitTest.php b/core/tests/Drupal/Tests/Core/Access/RoutePathGenerationTraitTest.php new file mode 100644 index 0000000000000000000000000000000000000000..56c4766bca8e569a0696b28888441c6dfc5f8803 --- /dev/null +++ b/core/tests/Drupal/Tests/Core/Access/RoutePathGenerationTraitTest.php @@ -0,0 +1,132 @@ +<?php + +declare(strict_types=1); + +namespace Drupal\Tests\Core\Access; + +use Drupal\Core\Access\AccessResultAllowed; +use Drupal\Core\Access\CsrfAccessCheck; +use Drupal\Core\Access\CsrfTokenGenerator; +use Drupal\Core\Access\RouteProcessorCsrf; +use Drupal\Core\Routing\RouteMatchInterface; +use Drupal\Tests\UnitTestCase; +use Symfony\Component\DependencyInjection\ParameterBag\ParameterBagInterface; +use Symfony\Component\HttpFoundation\InputBag; +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\RequestStack; +use Symfony\Component\Routing\Route; + +/** + * @covers \Drupal\Core\Access\RoutePathGenerationTrait + * @group Access + */ +class RoutePathGenerationTraitTest extends UnitTestCase { + + /** + * The mock CSRF token generator. + */ + protected CsrfTokenGenerator $csrfToken; + + /** + * The request stack. + */ + protected RequestStack $requestStack; + + /** + * The route processor. + */ + protected RouteProcessorCsrf $processor; + + /** + * The CSRF access checker. + */ + protected CsrfAccessCheck $accessCheck; + + /** + * {@inheritdoc} + */ + protected function setUp(): void { + parent::setUp(); + $this->csrfToken = $this->getMockBuilder(CsrfTokenGenerator::class) + ->disableOriginalConstructor() + ->getMock(); + // Make CsrfTokenGenerator mock use a simple hash of the value passed as + // parameter, as it is enough for the sake of our tests. + $this->csrfToken->method('get')->willReturnCallback(function ($value) { + return hash('sha256', $value); + }); + $this->csrfToken->method('validate')->willReturnCallback(function ($token, $value) { + return $token === hash('sha256', $value); + }); + $this->requestStack = $this->createMock(RequestStack::class); + $this->processor = new RouteProcessorCsrf($this->csrfToken, $this->requestStack); + $this->accessCheck = new CsrfAccessCheck($this->csrfToken); + } + + /** + * Tests that CSRF token creation and validation is consistent. + * + * This checks that CsrfAccessCheck() and RouteProcessorCsrf() produce the + * same results. + * + * Multiple cases are provided for an optional parameter (non-empty, empty, + * null, undefined). + * + * @dataProvider providerTestCsrfTokenCompleteLifeCycle + */ + public function testCsrfTokenCompleteLifeCycle($params): void { + + // Mock a route. + $route = $this->createMock(Route::class); + $route + ->method('getPath') + ->willReturn('test/example/{param}'); + $route + ->method('hasRequirement') + ->with('_csrf_token') + ->willReturn(TRUE); + + // Process the route so the "token" param is generated. + $routeParams = $params; + $this->processor->processOutbound('test.example', $route, $routeParams); + + $requestParams = $params + ['token' => $routeParams['token']]; + + // Mock Parameter bag. + $parameterBag = $this->createMock(ParameterBagInterface::class); + $parameterBag->method('get')->willReturnCallback(function ($key, $default = NULL) use ($requestParams) { + return $requestParams[$key] ?? $default; + }); + $parameterBag->method('all')->willReturn($requestParams); + + // Get a real InputBag because it is a final class. + $inputBag = new InputBag($requestParams); + + // Mock Request. + $request = $this->createMock(Request::class); + $request->query = $inputBag; + + // Mock RouteMatch. + $routeMatch = $this->createMock(RouteMatchInterface::class); + $routeMatch->method('getRawParameters')->willReturn($parameterBag); + + // Check for allowed access. + $this->assertInstanceOf(AccessResultAllowed::class, $this->accessCheck->access($route, $request, $routeMatch)); + } + + /** + * Data provider for testCsrfTokenCompleteLifeCycle(). + * + * @return array + * An array of route parameters. + */ + public static function providerTestCsrfTokenCompleteLifeCycle(): array { + return [ + [['param' => 'value']], + [['param' => '']], + [['param' => NULL]], + [[]], + ]; + } + +}