diff --git a/CHANGELOG-1.0.md b/CHANGELOG-1.0.md index 3068bbefed6..3d0fa276463 100644 --- a/CHANGELOG-1.0.md +++ b/CHANGELOG-1.0.md @@ -1,5 +1,15 @@ # CHANGELOG FOR `1.0.X` +## v1.0.18 (2018-07-10) + +#### TL;DR + +- Fixing the application after not-so-perfect security issue fix in the last release + +#### Details + +- [See the diff since the last patch release](https://github.com/Sylius/Sylius/compare/v1.0.17...v1.0.18) + ## v1.0.17 (2018-07-08) #### TL;DR diff --git a/README.md b/README.md index 1c833181c97..f32c5b59ae9 100644 --- a/README.md +++ b/README.md @@ -37,6 +37,12 @@ Contributing Would like to help us and build the most developer-friendly eCommerce platform? Start from reading our [Contributing Guide](http://docs.sylius.com/en/latest/contributing/index.html)! +Security issues +--------------- + +If you think that you have found a security issue in Sylius, please do not use the issue tracker and do not post it publicly. +Instead, all security issues must be sent to `security@sylius.com`. + License ------- diff --git a/UPGRADE-1.0.md b/UPGRADE-1.0.md index 8f0b16ec14a..9ee0dfdade4 100644 --- a/UPGRADE-1.0.md +++ b/UPGRADE-1.0.md @@ -1,6 +1,11 @@ +# UPGRADE FROM `v1.0.17` TO `v1.0.18` + +* **BC BREAK**: `OrderShowMenuBuilder` constructor now requires the fourth argument being + `Symfony\Component\Security\Csrf\CsrfTokenManagerInterface` instance due to security reasons. + # UPGRADE FROM `v1.0.16` TO `v1.0.17` -* `Sylius\Bundle\ResourceBundle\Controller::applyStateMachineTransitionAction` method now includes CSRF token checks due +* **BC BREAK**: `Sylius\Bundle\ResourceBundle\Controller::applyStateMachineTransitionAction` method now includes CSRF token checks due to security reasons. If you used it for REST API, these checks can be disabled by adding `csrf_protection: false` to your routing configuration. diff --git a/src/Sylius/Bundle/AdminApiBundle/Resources/config/routing/order.yml b/src/Sylius/Bundle/AdminApiBundle/Resources/config/routing/order.yml index 1ad33a0f02e..969dc8e3cf2 100644 --- a/src/Sylius/Bundle/AdminApiBundle/Resources/config/routing/order.yml +++ b/src/Sylius/Bundle/AdminApiBundle/Resources/config/routing/order.yml @@ -20,6 +20,7 @@ sylius_admin_api_order_cancel: state_machine: graph: sylius_order transition: cancel + csrf_protection: false return_content: false sylius_admin_api_order_shipment_ship: diff --git a/src/Sylius/Bundle/AdminBundle/Menu/OrderShowMenuBuilder.php b/src/Sylius/Bundle/AdminBundle/Menu/OrderShowMenuBuilder.php index f0eac1c6a27..9d7aa4a32a7 100644 --- a/src/Sylius/Bundle/AdminBundle/Menu/OrderShowMenuBuilder.php +++ b/src/Sylius/Bundle/AdminBundle/Menu/OrderShowMenuBuilder.php @@ -19,6 +19,7 @@ use Sylius\Bundle\AdminBundle\Event\OrderShowMenuBuilderEvent; use Sylius\Component\Order\OrderTransitions; use Symfony\Component\EventDispatcher\EventDispatcherInterface; +use Symfony\Component\Security\Csrf\CsrfTokenManagerInterface; final class OrderShowMenuBuilder { @@ -40,18 +41,20 @@ final class OrderShowMenuBuilder private $stateMachineFactory; /** - * @param FactoryInterface $factory - * @param EventDispatcherInterface $eventDispatcher - * @param StateMachineFactoryInterface $stateMachineFactory, + * @var CsrfTokenManagerInterface */ + private $csrfTokenManager; + public function __construct( FactoryInterface $factory, EventDispatcherInterface $eventDispatcher, - StateMachineFactoryInterface $stateMachineFactory + StateMachineFactoryInterface $stateMachineFactory, + CsrfTokenManagerInterface $csrfTokenManager ) { $this->factory = $factory; $this->eventDispatcher = $eventDispatcher; $this->stateMachineFactory = $stateMachineFactory; + $this->csrfTokenManager = $csrfTokenManager; } /** @@ -84,7 +87,10 @@ public function createMenu(array $options): ItemInterface $menu ->addChild('cancel', [ 'route' => 'sylius_admin_order_cancel', - 'routeParameters' => ['id' => $order->getId()], + 'routeParameters' => [ + 'id' => $order->getId(), + '_csrf_token' => $this->csrfTokenManager->getToken((string) $order->getId())->getValue(), + ], ]) ->setAttribute('type', 'transition') ->setLabel('sylius.ui.cancel') diff --git a/src/Sylius/Bundle/AdminBundle/Resources/config/services/menu.xml b/src/Sylius/Bundle/AdminBundle/Resources/config/services/menu.xml index 4dfa61de08e..b440ff879f4 100644 --- a/src/Sylius/Bundle/AdminBundle/Resources/config/services/menu.xml +++ b/src/Sylius/Bundle/AdminBundle/Resources/config/services/menu.xml @@ -29,6 +29,7 @@ + diff --git a/src/Sylius/Bundle/AdminBundle/spec/Menu/OrderShowMenuBuilderSpec.php b/src/Sylius/Bundle/AdminBundle/spec/Menu/OrderShowMenuBuilderSpec.php deleted file mode 100644 index dd5d9762eae..00000000000 --- a/src/Sylius/Bundle/AdminBundle/spec/Menu/OrderShowMenuBuilderSpec.php +++ /dev/null @@ -1,128 +0,0 @@ -beConstructedWith($factory, $eventDispatcher, $stateMachineFactory); - } - - function it_creates_an_order_show_menu( - FactoryInterface $factory, - EventDispatcherInterface $eventDispatcher, - StateMachineFactoryInterface $stateMachineFactory, - ItemInterface $menu, - StateMachineInterface $stateMachine, - OrderInterface $order - ): void { - $factory->createItem('root')->willReturn($menu); - - $order->getId()->willReturn(7); - - $menu - ->addChild('order_history', [ - 'route' => 'sylius_admin_order_history', - 'routeParameters' => ['id' => 7], - ]) - ->shouldBeCalled() - ->willReturn($menu) - ; - $menu->setAttribute('type', 'link')->shouldBeCalled()->willReturn($menu); - $menu->setLabel('sylius.ui.history')->shouldBeCalled()->willReturn($menu); - $menu->setLabelAttribute('icon', 'history')->shouldBeCalled()->willReturn($menu); - - $stateMachineFactory->get($order, OrderTransitions::GRAPH)->willReturn($stateMachine); - $stateMachine->can(OrderTransitions::TRANSITION_CANCEL)->willReturn(true); - - $menu - ->addChild('cancel', [ - 'route' => 'sylius_admin_order_cancel', - 'routeParameters' => ['id' => 7], - ]) - ->shouldBeCalled() - ->willReturn($menu) - ; - $menu->setAttribute('type', 'transition')->shouldBeCalled()->willReturn($menu); - $menu->setLabel('sylius.ui.cancel')->shouldBeCalled()->willReturn($menu); - $menu->setLabelAttribute('icon', 'ban')->shouldBeCalled()->willReturn($menu); - $menu->setLabelAttribute('color', 'yellow')->shouldBeCalled()->willReturn($menu); - - $eventDispatcher - ->dispatch('sylius.menu.admin.order.show', Argument::type(MenuBuilderEvent::class)) - ->shouldBeCalled() - ; - - $this->createMenu(['order' => $order])->shouldReturn($menu); - } - - function it_creates_an_order_show_menu_when_cancel_transition_is_impossible( - FactoryInterface $factory, - EventDispatcherInterface $eventDispatcher, - StateMachineFactoryInterface $stateMachineFactory, - ItemInterface $menu, - StateMachineInterface $stateMachine, - OrderInterface $order - ): void { - $factory->createItem('root')->willReturn($menu); - - $order->getId()->willReturn(7); - - $menu - ->addChild('order_history', [ - 'route' => 'sylius_admin_order_history', - 'routeParameters' => ['id' => 7], - ]) - ->shouldBeCalled() - ->willReturn($menu) - ; - $menu->setAttribute('type', 'link')->shouldBeCalled()->willReturn($menu); - $menu->setLabel('sylius.ui.history')->shouldBeCalled()->willReturn($menu); - $menu->setLabelAttribute('icon', 'history')->shouldBeCalled()->willReturn($menu); - - $stateMachineFactory->get($order, OrderTransitions::GRAPH)->willReturn($stateMachine); - $stateMachine->can(OrderTransitions::TRANSITION_CANCEL)->willReturn(false); - - $eventDispatcher - ->dispatch('sylius.menu.admin.order.show', Argument::type(MenuBuilderEvent::class)) - ->shouldBeCalled() - ; - - $this->createMenu(['order' => $order])->shouldReturn($menu); - } - - function it_returns_an_empty_order_show_menu_when_there_is_no_order_in_options( - FactoryInterface $factory, - ItemInterface $menu - ): void { - $factory->createItem('root')->willReturn($menu); - - $this->createMenu([])->shouldReturn($menu); - } -} diff --git a/src/Sylius/Bundle/CoreBundle/Application/Kernel.php b/src/Sylius/Bundle/CoreBundle/Application/Kernel.php index 4407e35984c..5be29403ba5 100644 --- a/src/Sylius/Bundle/CoreBundle/Application/Kernel.php +++ b/src/Sylius/Bundle/CoreBundle/Application/Kernel.php @@ -31,11 +31,11 @@ class Kernel extends HttpKernel { - public const VERSION = '1.0.17'; - public const VERSION_ID = '10017'; + public const VERSION = '1.0.18'; + public const VERSION_ID = '10018'; public const MAJOR_VERSION = '1'; public const MINOR_VERSION = '0'; - public const RELEASE_VERSION = '17'; + public const RELEASE_VERSION = '18'; public const EXTRA_VERSION = ''; /** diff --git a/src/Sylius/Bundle/ResourceBundle/Controller/ResourceController.php b/src/Sylius/Bundle/ResourceBundle/Controller/ResourceController.php index 8ca359dc6e3..8713dff7298 100644 --- a/src/Sylius/Bundle/ResourceBundle/Controller/ResourceController.php +++ b/src/Sylius/Bundle/ResourceBundle/Controller/ResourceController.php @@ -469,7 +469,7 @@ public function applyStateMachineTransitionAction(Request $request): Response $this->isGrantedOr403($configuration, ResourceActions::UPDATE); $resource = $this->findOr404($configuration); - if ($configuration->isCsrfProtectionEnabled() && !$this->isCsrfTokenValid($resource->getId(), $request->request->get('_csrf_token'))) { + if ($configuration->isCsrfProtectionEnabled() && !$this->isCsrfTokenValid((string) $resource->getId(), $request->get('_csrf_token'))) { throw new HttpException(Response::HTTP_FORBIDDEN, 'Invalid CSRF token.'); } diff --git a/src/Sylius/Bundle/ResourceBundle/spec/Controller/ResourceControllerSpec.php b/src/Sylius/Bundle/ResourceBundle/spec/Controller/ResourceControllerSpec.php index 0f0e85f30d2..ce1ab562dc1 100644 --- a/src/Sylius/Bundle/ResourceBundle/spec/Controller/ResourceControllerSpec.php +++ b/src/Sylius/Bundle/ResourceBundle/spec/Controller/ResourceControllerSpec.php @@ -1822,6 +1822,8 @@ function it_does_not_apply_state_machine_transition_on_resource_if_not_applicabl ResourceInterface $resource, FlashHelperInterface $flashHelper, EventDispatcherInterface $eventDispatcher, + CsrfTokenManagerInterface $csrfTokenManager, + ContainerInterface $container, ResourceControllerEvent $event, Request $request ): void { @@ -1831,10 +1833,18 @@ function it_does_not_apply_state_machine_transition_on_resource_if_not_applicabl $requestConfigurationFactory->create($metadata, $request)->willReturn($configuration); $configuration->hasPermission()->willReturn(true); $configuration->getPermission(ResourceActions::UPDATE)->willReturn('sylius.product.update'); + $configuration->isCsrfProtectionEnabled()->willReturn(true); + $request->get('_csrf_token')->willReturn('xyz'); + + $container->has('security.csrf.token_manager')->willReturn(true); + $container->get('security.csrf.token_manager')->willReturn($csrfTokenManager); + $csrfTokenManager->isTokenValid(new CsrfToken('1', 'xyz'))->willReturn(true); $authorizationChecker->isGranted($configuration, 'sylius.product.update')->willReturn(true); $singleResourceProvider->get($configuration, $repository)->willReturn($resource); + $resource->getId()->willReturn('1'); + $configuration->isHtmlRequest()->willReturn(true); $eventDispatcher->dispatchPreEvent(ResourceActions::UPDATE, $configuration, $resource)->willReturn($event); @@ -1865,6 +1875,8 @@ function it_applies_state_machine_transition_to_resource_and_redirects_for_html_ FlashHelperInterface $flashHelper, AuthorizationCheckerInterface $authorizationChecker, EventDispatcherInterface $eventDispatcher, + CsrfTokenManagerInterface $csrfTokenManager, + ContainerInterface $container, StateMachineInterface $stateMachine, ResourceUpdateHandlerInterface $resourceUpdateHandler, RequestConfiguration $configuration, @@ -1880,10 +1892,18 @@ function it_applies_state_machine_transition_to_resource_and_redirects_for_html_ $requestConfigurationFactory->create($metadata, $request)->willReturn($configuration); $configuration->hasPermission()->willReturn(true); $configuration->getPermission(ResourceActions::UPDATE)->willReturn('sylius.product.update'); + $configuration->isCsrfProtectionEnabled()->willReturn(true); + $request->get('_csrf_token')->willReturn('xyz'); + + $container->has('security.csrf.token_manager')->willReturn(true); + $container->get('security.csrf.token_manager')->willReturn($csrfTokenManager); + $csrfTokenManager->isTokenValid(new CsrfToken('1', 'xyz'))->willReturn(true); $authorizationChecker->isGranted($configuration, 'sylius.product.update')->willReturn(true); $singleResourceProvider->get($configuration, $repository)->willReturn($resource); + $resource->getId()->willReturn('1'); + $configuration->isHtmlRequest()->willReturn(true); $eventDispatcher->dispatchPreEvent(ResourceActions::UPDATE, $configuration, $resource)->willReturn($event); @@ -1909,10 +1929,11 @@ function it_uses_response_from_post_apply_state_machine_transition_event_if_defi RepositoryInterface $repository, ObjectManager $manager, SingleResourceProviderInterface $singleResourceProvider, - RedirectHandlerInterface $redirectHandler, FlashHelperInterface $flashHelper, AuthorizationCheckerInterface $authorizationChecker, EventDispatcherInterface $eventDispatcher, + CsrfTokenManagerInterface $csrfTokenManager, + ContainerInterface $container, StateMachineInterface $stateMachine, ResourceUpdateHandlerInterface $resourceUpdateHandler, RequestConfiguration $configuration, @@ -1928,10 +1949,18 @@ function it_uses_response_from_post_apply_state_machine_transition_event_if_defi $requestConfigurationFactory->create($metadata, $request)->willReturn($configuration); $configuration->hasPermission()->willReturn(true); $configuration->getPermission(ResourceActions::UPDATE)->willReturn('sylius.product.update'); + $configuration->isCsrfProtectionEnabled()->willReturn(true); + $request->get('_csrf_token')->willReturn('xyz'); + + $container->has('security.csrf.token_manager')->willReturn(true); + $container->get('security.csrf.token_manager')->willReturn($csrfTokenManager); + $csrfTokenManager->isTokenValid(new CsrfToken('1', 'xyz'))->willReturn(true); $authorizationChecker->isGranted($configuration, 'sylius.product.update')->willReturn(true); $singleResourceProvider->get($configuration, $repository)->willReturn($resource); + $resource->getId()->willReturn('1'); + $configuration->isHtmlRequest()->willReturn(true); $eventDispatcher->dispatchPreEvent(ResourceActions::UPDATE, $configuration, $resource)->willReturn($event); @@ -1963,6 +1992,8 @@ function it_does_not_apply_state_machine_transition_on_resource_and_redirects_fo RedirectHandlerInterface $redirectHandler, FlashHelperInterface $flashHelper, EventDispatcherInterface $eventDispatcher, + CsrfTokenManagerInterface $csrfTokenManager, + ContainerInterface $container, ResourceControllerEvent $event, Request $request, Response $redirectResponse @@ -1973,10 +2004,18 @@ function it_does_not_apply_state_machine_transition_on_resource_and_redirects_fo $requestConfigurationFactory->create($metadata, $request)->willReturn($configuration); $configuration->hasPermission()->willReturn(true); $configuration->getPermission(ResourceActions::UPDATE)->willReturn('sylius.product.update'); + $configuration->isCsrfProtectionEnabled()->willReturn(true); + $request->get('_csrf_token')->willReturn('xyz'); + + $container->has('security.csrf.token_manager')->willReturn(true); + $container->get('security.csrf.token_manager')->willReturn($csrfTokenManager); + $csrfTokenManager->isTokenValid(new CsrfToken('1', 'xyz'))->willReturn(true); $authorizationChecker->isGranted($configuration, 'sylius.product.update')->willReturn(true); $singleResourceProvider->get($configuration, $repository)->willReturn($resource); + $resource->getId()->willReturn('1'); + $configuration->isHtmlRequest()->willReturn(true); $eventDispatcher->dispatchPreEvent(ResourceActions::UPDATE, $configuration, $resource)->willReturn($event); @@ -2008,6 +2047,8 @@ function it_does_not_apply_state_machine_transition_on_resource_and_return_event ResourceInterface $resource, FlashHelperInterface $flashHelper, EventDispatcherInterface $eventDispatcher, + CsrfTokenManagerInterface $csrfTokenManager, + ContainerInterface $container, ResourceControllerEvent $event, Request $request, Response $response @@ -2018,10 +2059,18 @@ function it_does_not_apply_state_machine_transition_on_resource_and_return_event $requestConfigurationFactory->create($metadata, $request)->willReturn($configuration); $configuration->hasPermission()->willReturn(true); $configuration->getPermission(ResourceActions::UPDATE)->willReturn('sylius.product.update'); + $configuration->isCsrfProtectionEnabled()->willReturn(true); + $request->get('_csrf_token')->willReturn('xyz'); + + $container->has('security.csrf.token_manager')->willReturn(true); + $container->get('security.csrf.token_manager')->willReturn($csrfTokenManager); + $csrfTokenManager->isTokenValid(new CsrfToken('1', 'xyz'))->willReturn(true); $authorizationChecker->isGranted($configuration, 'sylius.product.update')->willReturn(true); $singleResourceProvider->get($configuration, $repository)->willReturn($resource); + $resource->getId()->willReturn('1'); + $configuration->isHtmlRequest()->willReturn(true); $eventDispatcher->dispatchPreEvent(ResourceActions::UPDATE, $configuration, $resource)->willReturn($event); @@ -2066,6 +2115,7 @@ function it_applies_state_machine_transition_on_resource_and_returns_200_for_non $configuration->getParameters()->willReturn($parameterBag); $configuration->hasPermission()->willReturn(true); $configuration->getPermission(ResourceActions::UPDATE)->willReturn('sylius.product.update'); + $configuration->isCsrfProtectionEnabled()->willReturn(false); $parameterBag->get('return_content', true)->willReturn(true); @@ -2114,6 +2164,7 @@ function it_applies_state_machine_transition_on_resource_and_returns_204_for_non $configuration->getParameters()->willReturn($parameterBag); $configuration->hasPermission()->willReturn(true); $configuration->getPermission(ResourceActions::UPDATE)->willReturn('sylius.product.update'); + $configuration->isCsrfProtectionEnabled()->willReturn(false); $parameterBag->get('return_content', true)->willReturn(false); @@ -2158,6 +2209,7 @@ function it_does_not_apply_state_machine_transition_resource_and_throws_http_exc $requestConfigurationFactory->create($metadata, $request)->willReturn($configuration); $configuration->hasPermission()->willReturn(true); $configuration->getPermission(ResourceActions::UPDATE)->willReturn('sylius.product.update'); + $configuration->isCsrfProtectionEnabled()->willReturn(false); $authorizationChecker->isGranted($configuration, 'sylius.product.update')->willReturn(true); $singleResourceProvider->get($configuration, $repository)->willReturn($resource);