diff --git a/phpstan.neon b/phpstan.neon index 7806d08bd..997364948 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -174,3 +174,12 @@ parameters: identifier: return.type count: 1 path: src/Component/src/Metadata/Resource/Factory/CachedResourceClassListFactory.php + + # ResourceMappingDriverChain intentionally extends @final MappingDriverChain + # Required for compatibility with Gedmo/DoctrineExtensions which calls instanceof MappingDriverChain + # See: src/Bundle/Doctrine/ResourceMappingDriverChain.php class docblock + # See: \Gedmo\Mapping\ExtensionMetadataFactory::getDriver() + - message: '#^Class Sylius\\Bundle\\ResourceBundle\\Doctrine\\ResourceMappingDriverChain extends @final class Doctrine\\Persistence\\Mapping\\Driver\\MappingDriverChain\.$#' + identifier: class.extendsFinalByPhpDoc + count: 1 + path: src/Bundle/Doctrine/ResourceMappingDriverChain.php diff --git a/src/Bundle/Controller/ResourceController.php b/src/Bundle/Controller/ResourceController.php index ba1ecd7a6..2f04a91c4 100644 --- a/src/Bundle/Controller/ResourceController.php +++ b/src/Bundle/Controller/ResourceController.php @@ -90,6 +90,7 @@ public function __construct( ?StateMachineInterface $stateMachine, ResourceUpdateHandlerInterface $resourceUpdateHandler, ResourceDeleteHandlerInterface $resourceDeleteHandler, + protected readonly string $csrfParameter = '_csrf_token', ) { $this->metadata = $metadata; $this->requestConfigurationFactory = $requestConfigurationFactory; @@ -333,7 +334,7 @@ public function deleteAction(Request $request): Response $this->isGrantedOr403($configuration, ResourceActions::DELETE); $resource = $this->findOr404($configuration); - if ($configuration->isCsrfProtectionEnabled() && !$this->isCsrfTokenValid((string) $resource->getId(), (string) $request->request->get('_csrf_token'))) { + if ($configuration->isCsrfProtectionEnabled() && !$this->isCsrfTokenValid((string) $resource->getId(), (string) $request->request->get($this->csrfParameter))) { throw new HttpException(Response::HTTP_FORBIDDEN, 'Invalid csrf token.'); } @@ -392,7 +393,7 @@ public function bulkDeleteAction(Request $request): Response if ( $configuration->isCsrfProtectionEnabled() && - !$this->isCsrfTokenValid(ResourceActions::BULK_DELETE, (string) $request->request->get('_csrf_token')) + !$this->isCsrfTokenValid(ResourceActions::BULK_DELETE, (string) $request->request->get($this->csrfParameter)) ) { throw new HttpException(Response::HTTP_FORBIDDEN, 'Invalid csrf token.'); } @@ -455,7 +456,7 @@ public function applyStateMachineTransitionAction(Request $request): Response $this->isGrantedOr403($configuration, ResourceActions::UPDATE); $resource = $this->findOr404($configuration); - if ($configuration->isCsrfProtectionEnabled() && !$this->isCsrfTokenValid((string) $resource->getId(), $this->getFromRequest($request, '_csrf_token'))) { + if ($configuration->isCsrfProtectionEnabled() && !$this->isCsrfTokenValid((string) $resource->getId(), $this->getFromRequest($request, $this->csrfParameter))) { throw new HttpException(Response::HTTP_FORBIDDEN, 'Invalid CSRF token.'); } diff --git a/src/Bundle/DependencyInjection/Configuration.php b/src/Bundle/DependencyInjection/Configuration.php index ad9ed8516..fb6f75fce 100644 --- a/src/Bundle/DependencyInjection/Configuration.php +++ b/src/Bundle/DependencyInjection/Configuration.php @@ -144,6 +144,8 @@ private function addSettingsSection(ArrayNodeDefinition $node): void ->booleanNode('filterable')->defaultFalse()->end() ->variableNode('criteria')->defaultNull()->end() ->scalarNode('state_machine_component')->defaultNull()->end() + ->scalarNode('csrf_parameter')->defaultValue('_csrf_token')->cannotBeEmpty() + ->end() ->end() ->end() ->end() diff --git a/src/Bundle/DependencyInjection/Driver/AbstractDriver.php b/src/Bundle/DependencyInjection/Driver/AbstractDriver.php index aafd46fff..db1fec110 100644 --- a/src/Bundle/DependencyInjection/Driver/AbstractDriver.php +++ b/src/Bundle/DependencyInjection/Driver/AbstractDriver.php @@ -84,6 +84,7 @@ protected function addController(ContainerBuilder $container, MetadataInterface new Reference($metadata->getServiceId('controller_state_machine'), ContainerInterface::NULL_ON_INVALID_REFERENCE), new Reference('sylius.resource_controller.resource_update_handler'), new Reference('sylius.resource_controller.resource_delete_handler'), + '%sylius.resource.csrf_parameter%', ]) ->addMethodCall('setContainer', [new Reference('service_container')]) ->addTag('controller.service_arguments') diff --git a/src/Bundle/DependencyInjection/SyliusResourceExtension.php b/src/Bundle/DependencyInjection/SyliusResourceExtension.php index f881956b0..875d8150e 100644 --- a/src/Bundle/DependencyInjection/SyliusResourceExtension.php +++ b/src/Bundle/DependencyInjection/SyliusResourceExtension.php @@ -74,6 +74,7 @@ public function load(array $configs, ContainerBuilder $container): void $container->setParameter('sylius.resource.mapping', $config['mapping']); $container->setParameter('sylius.resource.settings', $config['settings']); + $container->setParameter('sylius.resource.csrf_parameter', $config['settings']['csrf_parameter']); $routingPathBcLayer = $config['routing_path_bc_layer'] ?? null; diff --git a/src/Bundle/Resources/config/services/twig.php b/src/Bundle/Resources/config/services/twig.php index 5b582d6fd..0d2cbbe4d 100644 --- a/src/Bundle/Resources/config/services/twig.php +++ b/src/Bundle/Resources/config/services/twig.php @@ -14,6 +14,7 @@ namespace Symfony\Component\DependencyInjection\Loader\Configurator; use Sylius\Bundle\ResourceBundle\Twig\Context\LegacyContextFactory; +use Sylius\Bundle\ResourceBundle\Twig\CsrfParameterExtension; use Sylius\Resource\Twig\Context\Factory\ContextFactory; use Sylius\Resource\Twig\Context\Factory\ContextFactoryInterface; use Sylius\Resource\Twig\Context\Factory\DefaultContextFactory; @@ -22,6 +23,10 @@ return static function (ContainerConfigurator $container) { $services = $container->services(); + $services->set('sylius.twig.extension.csrf_parameter', CsrfParameterExtension::class) + ->args(['%sylius.resource.csrf_parameter%']) + ->tag('twig.extension'); + $services->set('sylius.twig.context.factory', ContextFactory::class) ->args([tagged_locator('sylius.twig_context_factory')]); diff --git a/src/Bundle/Twig/CsrfParameterExtension.php b/src/Bundle/Twig/CsrfParameterExtension.php new file mode 100644 index 000000000..386b95b85 --- /dev/null +++ b/src/Bundle/Twig/CsrfParameterExtension.php @@ -0,0 +1,36 @@ +getCsrfParameter(...)), + ]; + } + + public function getCsrfParameter(): string + { + return $this->csrfParameter; + } +} diff --git a/tests/Bundle/Controller/ResourceControllerTest.php b/tests/Bundle/Controller/ResourceControllerTest.php index 676f468a5..999ae286d 100644 --- a/tests/Bundle/Controller/ResourceControllerTest.php +++ b/tests/Bundle/Controller/ResourceControllerTest.php @@ -2898,6 +2898,176 @@ public function testDoesNotApplyStateMachineTransitionResourceAndThrowsHttpExcep $this->resourceController->applyStateMachineTransitionAction($request); } + public function testValidatesCsrfWithDefaultParameterName(): void + { + /** @var RequestConfiguration|MockObject $configurationMock */ + $configurationMock = $this->createMock(RequestConfiguration::class); + /** @var ResourceInterface|MockObject $resourceMock */ + $resourceMock = $this->createMock(ResourceInterface::class); + /** @var CsrfTokenManagerInterface|MockObject $csrfTokenManagerMock */ + $csrfTokenManagerMock = $this->createMock(CsrfTokenManagerInterface::class); + /** @var ResourceControllerEvent|MockObject $eventMock */ + $eventMock = $this->createMock(ResourceControllerEvent::class); + /** @var ResourceControllerEvent|MockObject $postEventMock */ + $postEventMock = $this->createMock(ResourceControllerEvent::class); + /** @var Response|MockObject $redirectResponseMock */ + $redirectResponseMock = $this->createMock(Response::class); + + $request = new Request(request: ['_csrf_token' => 'valid-token-here']); + + $this->requestConfigurationFactoryMock->expects($this->once())->method('create')->with($this->metadataMock, $request)->willReturn($configurationMock); + + $configurationMock->expects($this->once())->method('hasPermission')->willReturn(true); + $configurationMock->expects($this->once())->method('getPermission')->with(ResourceActions::DELETE)->willReturn('sylius.product.delete'); + $configurationMock->method('isHtmlRequest')->willReturn(true); + $configurationMock->expects($this->once())->method('isCsrfProtectionEnabled')->willReturn(true); + + $this->containerMock->expects($this->once())->method('has')->with('security.csrf.token_manager')->willReturn(true); + $this->containerMock->expects($this->once())->method('get')->with('security.csrf.token_manager')->willReturn($csrfTokenManagerMock); + + $csrfTokenManagerMock->expects($this->once())->method('isTokenValid')->with(new CsrfToken('1', 'valid-token-here'))->willReturn(true); + + $this->authorizationCheckerMock->expects($this->once())->method('isGranted')->with($configurationMock, 'sylius.product.delete')->willReturn(true); + $this->singleResourceProviderMock->expects($this->once())->method('get')->with($configurationMock, $this->repositoryMock)->willReturn($resourceMock); + + $resourceMock->expects($this->once())->method('getId')->willReturn(1); + + $this->eventDispatcherMock->expects($this->once())->method('dispatchPreEvent')->with(ResourceActions::DELETE, $configurationMock, $resourceMock)->willReturn($eventMock); + $eventMock->method('isStopped')->willReturn(false); + + $this->resourceDeleteHandlerMock->expects($this->once())->method('handle')->with($resourceMock, $this->repositoryMock); + $this->eventDispatcherMock->expects($this->once())->method('dispatchPostEvent')->with(ResourceActions::DELETE, $configurationMock, $resourceMock)->willReturn($postEventMock); + + $postEventMock->expects($this->once())->method('getResponse')->willReturn(null); + + $this->flashHelperMock->expects($this->once())->method('addSuccessFlash')->with($configurationMock, ResourceActions::DELETE, $resourceMock); + $this->redirectHandlerMock->expects($this->once())->method('redirectToIndex')->with($configurationMock, $resourceMock)->willReturn($redirectResponseMock); + + $this->assertSame($redirectResponseMock, $this->resourceController->deleteAction($request)); + } + + public function testValidatesCsrfWithCustomParameterName(): void + { + $controller = $this->buildControllerWithCsrfParameter('_my_csrf_field'); + + /** @var RequestConfiguration|MockObject $configurationMock */ + $configurationMock = $this->createMock(RequestConfiguration::class); + /** @var ResourceInterface|MockObject $resourceMock */ + $resourceMock = $this->createMock(ResourceInterface::class); + /** @var CsrfTokenManagerInterface|MockObject $csrfTokenManagerMock */ + $csrfTokenManagerMock = $this->createMock(CsrfTokenManagerInterface::class); + /** @var ResourceControllerEvent|MockObject $eventMock */ + $eventMock = $this->createMock(ResourceControllerEvent::class); + /** @var ResourceControllerEvent|MockObject $postEventMock */ + $postEventMock = $this->createMock(ResourceControllerEvent::class); + /** @var Response|MockObject $redirectResponseMock */ + $redirectResponseMock = $this->createMock(Response::class); + + $request = new Request(request: ['_my_csrf_field' => 'valid-token-here']); + + $this->requestConfigurationFactoryMock->expects($this->once())->method('create')->with($this->metadataMock, $request)->willReturn($configurationMock); + + $configurationMock->expects($this->once())->method('hasPermission')->willReturn(true); + $configurationMock->expects($this->once())->method('getPermission')->with(ResourceActions::DELETE)->willReturn('sylius.product.delete'); + $configurationMock->method('isHtmlRequest')->willReturn(true); + $configurationMock->expects($this->once())->method('isCsrfProtectionEnabled')->willReturn(true); + + $this->containerMock->expects($this->once())->method('has')->with('security.csrf.token_manager')->willReturn(true); + $this->containerMock->expects($this->once())->method('get')->with('security.csrf.token_manager')->willReturn($csrfTokenManagerMock); + + $csrfTokenManagerMock->expects($this->once())->method('isTokenValid')->with(new CsrfToken('1', 'valid-token-here'))->willReturn(true); + + $this->authorizationCheckerMock->expects($this->once())->method('isGranted')->with($configurationMock, 'sylius.product.delete')->willReturn(true); + $this->singleResourceProviderMock->expects($this->once())->method('get')->with($configurationMock, $this->repositoryMock)->willReturn($resourceMock); + + $resourceMock->expects($this->once())->method('getId')->willReturn(1); + + $this->eventDispatcherMock->expects($this->once())->method('dispatchPreEvent')->with(ResourceActions::DELETE, $configurationMock, $resourceMock)->willReturn($eventMock); + $eventMock->method('isStopped')->willReturn(false); + + $this->resourceDeleteHandlerMock->expects($this->once())->method('handle')->with($resourceMock, $this->repositoryMock); + $this->eventDispatcherMock->expects($this->once())->method('dispatchPostEvent')->with(ResourceActions::DELETE, $configurationMock, $resourceMock)->willReturn($postEventMock); + + $postEventMock->expects($this->once())->method('getResponse')->willReturn(null); + + $this->flashHelperMock->expects($this->once())->method('addSuccessFlash')->with($configurationMock, ResourceActions::DELETE, $resourceMock); + $this->redirectHandlerMock->expects($this->once())->method('redirectToIndex')->with($configurationMock, $resourceMock)->willReturn($redirectResponseMock); + + $this->assertSame($redirectResponseMock, $controller->deleteAction($request)); + } + + public function testRejectsWhenCsrfTokenIsInWrongField(): void + { + $controller = $this->buildControllerWithCsrfParameter('_my_csrf_field'); + + /** @var RequestConfiguration|MockObject $configurationMock */ + $configurationMock = $this->createMock(RequestConfiguration::class); + /** @var ResourceInterface|MockObject $resourceMock */ + $resourceMock = $this->createMock(ResourceInterface::class); + /** @var CsrfTokenManagerInterface|MockObject $csrfTokenManagerMock */ + $csrfTokenManagerMock = $this->createMock(CsrfTokenManagerInterface::class); + + $request = new Request(request: ['_csrf_token' => 'valid-token-here']); + + $this->requestConfigurationFactoryMock->expects($this->once())->method('create')->with($this->metadataMock, $request)->willReturn($configurationMock); + + $configurationMock->expects($this->once())->method('hasPermission')->willReturn(true); + $configurationMock->expects($this->once())->method('getPermission')->with(ResourceActions::DELETE)->willReturn('sylius.product.delete'); + $configurationMock->expects($this->once())->method('isCsrfProtectionEnabled')->willReturn(true); + + $this->containerMock->expects($this->once())->method('has')->with('security.csrf.token_manager')->willReturn(true); + $this->containerMock->expects($this->once())->method('get')->with('security.csrf.token_manager')->willReturn($csrfTokenManagerMock); + + $csrfTokenManagerMock->expects($this->once())->method('isTokenValid')->with(new CsrfToken('1', ''))->willReturn(false); + + $this->authorizationCheckerMock->expects($this->once())->method('isGranted')->with($configurationMock, 'sylius.product.delete')->willReturn(true); + $this->singleResourceProviderMock->expects($this->once())->method('get')->with($configurationMock, $this->repositoryMock)->willReturn($resourceMock); + + $resourceMock->expects($this->once())->method('getId')->willReturn(1); + + $this->resourceDeleteHandlerMock->expects($this->never())->method('handle'); + $this->eventDispatcherMock->expects($this->never())->method('dispatchPostEvent'); + $this->flashHelperMock->expects($this->never())->method('addSuccessFlash'); + + $this->expectException(HttpException::class); + $this->expectExceptionMessage('Invalid csrf token.'); + + try { + $controller->deleteAction($request); + } catch (HttpException $exception) { + $this->assertSame(Response::HTTP_FORBIDDEN, $exception->getStatusCode()); + + throw $exception; + } + } + + private function buildControllerWithCsrfParameter(string $csrfParameter): ResourceController + { + $controller = new ResourceController( + $this->metadataMock, + $this->requestConfigurationFactoryMock, + $this->viewHandlerMock, + $this->repositoryMock, + $this->factoryMock, + $this->newResourceFactoryMock, + $this->managerMock, + $this->singleResourceProviderMock, + $this->resourcesCollectionProviderMock, + $this->resourceFormFactoryMock, + $this->redirectHandlerMock, + $this->flashHelperMock, + $this->authorizationCheckerMock, + $this->eventDispatcherMock, + $this->stateMachineMock, + $this->resourceUpdateHandlerMock, + $this->resourceDeleteHandlerMock, + $csrfParameter, + ); + $controller->setContainer($this->containerMock); + + return $controller; + } + private function markAsSkippedIfFosRestBundleIsNotAvailable(): void { if (!class_exists(FOSRestBundle::class)) { diff --git a/tests/Bundle/DependencyInjection/SyliusResourceExtensionTest.php b/tests/Bundle/DependencyInjection/SyliusResourceExtensionTest.php index 35000c994..7a7f58aee 100644 --- a/tests/Bundle/DependencyInjection/SyliusResourceExtensionTest.php +++ b/tests/Bundle/DependencyInjection/SyliusResourceExtensionTest.php @@ -276,6 +276,24 @@ public function testItRegistersCustomPathSegmentNameGenerator(): void $this->assertSame('sylius.metadata.path_segment_name_generator.underscore', $this->container->getAlias('sylius.path_segment_name_generator')->__toString()); } + public function testItRegistersDefaultCsrfParameter(): void + { + $this->load(); + + $this->assertContainerBuilderHasParameter('sylius.resource.csrf_parameter', '_csrf_token'); + } + + public function testItRegistersCustomCsrfParameter(): void + { + $this->load([ + 'settings' => [ + 'csrf_parameter' => '_custom_csrf_token', + ], + ]); + + $this->assertContainerBuilderHasParameter('sylius.resource.csrf_parameter', '_custom_csrf_token'); + } + protected function getContainerExtensions(): array { $this->setParameter('kernel.bundles', []);