From 088607803652d77ea80881890f044002d737c84e Mon Sep 17 00:00:00 2001 From: Ernest Warwas Date: Wed, 2 Mar 2022 11:36:57 +0100 Subject: [PATCH 01/11] listener added to finish response with X-Frame-Options sameorigin header --- .../EventListener/FinishResponseListener.php | 48 +++++++++++++++++++ .../Resources/config/services/listeners.xml | 4 ++ tests/Controller/FinishResponseTest.php | 29 +++++++++++ 3 files changed, 81 insertions(+) create mode 100644 src/Sylius/Bundle/CoreBundle/EventListener/FinishResponseListener.php create mode 100644 tests/Controller/FinishResponseTest.php diff --git a/src/Sylius/Bundle/CoreBundle/EventListener/FinishResponseListener.php b/src/Sylius/Bundle/CoreBundle/EventListener/FinishResponseListener.php new file mode 100644 index 00000000000..735ff4a88a7 --- /dev/null +++ b/src/Sylius/Bundle/CoreBundle/EventListener/FinishResponseListener.php @@ -0,0 +1,48 @@ +isMainRequest($event)) { + return; + } + + $response = $event->getResponse(); + + $response->headers->set('X-Frame-Options', 'sameorigin'); + } + + public static function getSubscribedEvents() + { + return [ + KernelEvents::RESPONSE => [['onKernelResponse']], + ]; + } + + private function isMainRequest(ResponseEvent $event): bool + { + if (\method_exists($event, 'isMainRequest')) { + return $event->isMainRequest(); + } + + return $event->isMasterRequest(); + } +} diff --git a/src/Sylius/Bundle/CoreBundle/Resources/config/services/listeners.xml b/src/Sylius/Bundle/CoreBundle/Resources/config/services/listeners.xml index ef39f9b6ba5..9b1b97520b6 100644 --- a/src/Sylius/Bundle/CoreBundle/Resources/config/services/listeners.xml +++ b/src/Sylius/Bundle/CoreBundle/Resources/config/services/listeners.xml @@ -95,6 +95,10 @@ + + + + diff --git a/tests/Controller/FinishResponseTest.php b/tests/Controller/FinishResponseTest.php new file mode 100644 index 00000000000..513780469f3 --- /dev/null +++ b/tests/Controller/FinishResponseTest.php @@ -0,0 +1,29 @@ +client->request('GET', '/'); + + $response = $this->client->getResponse(); + + $this->assertSame('sameorigin', $response->headers->get('X-Frame-Options')); + } +} From c23643109c9976aa13fa3cd355c2138f4af4b790 Mon Sep 17 00:00:00 2001 From: Ernest Warwas Date: Thu, 3 Mar 2022 11:31:00 +0100 Subject: [PATCH 02/11] suggested review changes --- ...eListener.php => XFrameOptionsSubscriber.php} | 16 ++++++++-------- .../Resources/config/services/listeners.xml | 2 +- ...ishResponseTest.php => XFrameOptionsTest.php} | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) rename src/Sylius/Bundle/CoreBundle/EventListener/{FinishResponseListener.php => XFrameOptionsSubscriber.php} (84%) rename tests/Controller/{FinishResponseTest.php => XFrameOptionsTest.php} (91%) diff --git a/src/Sylius/Bundle/CoreBundle/EventListener/FinishResponseListener.php b/src/Sylius/Bundle/CoreBundle/EventListener/XFrameOptionsSubscriber.php similarity index 84% rename from src/Sylius/Bundle/CoreBundle/EventListener/FinishResponseListener.php rename to src/Sylius/Bundle/CoreBundle/EventListener/XFrameOptionsSubscriber.php index 735ff4a88a7..8e4727682ea 100644 --- a/src/Sylius/Bundle/CoreBundle/EventListener/FinishResponseListener.php +++ b/src/Sylius/Bundle/CoreBundle/EventListener/XFrameOptionsSubscriber.php @@ -17,8 +17,15 @@ use Symfony\Component\HttpKernel\Event\ResponseEvent; use Symfony\Component\HttpKernel\KernelEvents; -final class FinishResponseListener implements EventSubscriberInterface +final class XFrameOptionsSubscriber implements EventSubscriberInterface { + public static function getSubscribedEvents(): array + { + return [ + KernelEvents::RESPONSE => 'onKernelResponse', + ]; + } + public function onKernelResponse(ResponseEvent $event): void { if (!$this->isMainRequest($event)) { @@ -30,13 +37,6 @@ public function onKernelResponse(ResponseEvent $event): void $response->headers->set('X-Frame-Options', 'sameorigin'); } - public static function getSubscribedEvents() - { - return [ - KernelEvents::RESPONSE => [['onKernelResponse']], - ]; - } - private function isMainRequest(ResponseEvent $event): bool { if (\method_exists($event, 'isMainRequest')) { diff --git a/src/Sylius/Bundle/CoreBundle/Resources/config/services/listeners.xml b/src/Sylius/Bundle/CoreBundle/Resources/config/services/listeners.xml index 9b1b97520b6..9f02b50e081 100644 --- a/src/Sylius/Bundle/CoreBundle/Resources/config/services/listeners.xml +++ b/src/Sylius/Bundle/CoreBundle/Resources/config/services/listeners.xml @@ -95,7 +95,7 @@ - + diff --git a/tests/Controller/FinishResponseTest.php b/tests/Controller/XFrameOptionsTest.php similarity index 91% rename from tests/Controller/FinishResponseTest.php rename to tests/Controller/XFrameOptionsTest.php index 513780469f3..1011230418b 100644 --- a/tests/Controller/FinishResponseTest.php +++ b/tests/Controller/XFrameOptionsTest.php @@ -15,7 +15,7 @@ use ApiTestCase\JsonApiTestCase; -final class FinishResponseTest extends JsonApiTestCase +final class XFrameOptionsTest extends JsonApiTestCase { /** @test */ public function it_sets_frame_options_header(): void From 4b6a77ae9e5e902737c656fb32b16ef17b461a09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Chru=C5=9Bciel?= Date: Fri, 17 Sep 2021 13:21:03 +0200 Subject: [PATCH 03/11] [UI] Force no-store cache directives for admin and customer account section --- .../AdminSectionCacheControlSubscriber.php | 51 ++++++++++ .../Resources/config/services/listener.xml | 5 + ...AdminSectionCacheControlSubscriberSpec.php | 96 +++++++++++++++++++ ...ccountSubSectionCacheControlSubscriber.php | 51 ++++++++++ .../ShopBundle/Resources/config/services.xml | 1 + .../Resources/config/services/listeners.xml | 5 + .../ShopCustomerAccountSubSection.php | 18 ++++ .../ShopUriBasedSectionResolver.php | 11 +++ ...ntSubSectionCacheControlSubscriberSpec.php | 93 ++++++++++++++++++ .../ShopUriBasedSectionResolverSpec.php | 28 +++++- 10 files changed, 358 insertions(+), 1 deletion(-) create mode 100644 src/Sylius/Bundle/AdminBundle/EventListener/AdminSectionCacheControlSubscriber.php create mode 100644 src/Sylius/Bundle/AdminBundle/spec/EventListener/AdminSectionCacheControlSubscriberSpec.php create mode 100644 src/Sylius/Bundle/ShopBundle/EventListener/ShopCustomerAccountSubSectionCacheControlSubscriber.php create mode 100644 src/Sylius/Bundle/ShopBundle/SectionResolver/ShopCustomerAccountSubSection.php create mode 100644 src/Sylius/Bundle/ShopBundle/spec/EventListener/ShopCustomerAccountSubSectionCacheControlSubscriberSpec.php diff --git a/src/Sylius/Bundle/AdminBundle/EventListener/AdminSectionCacheControlSubscriber.php b/src/Sylius/Bundle/AdminBundle/EventListener/AdminSectionCacheControlSubscriber.php new file mode 100644 index 00000000000..08cc687f2c3 --- /dev/null +++ b/src/Sylius/Bundle/AdminBundle/EventListener/AdminSectionCacheControlSubscriber.php @@ -0,0 +1,51 @@ +sectionProvider = $sectionProvider; + } + + public static function getSubscribedEvents(): array + { + return [ + KernelEvents::RESPONSE => 'setCacheControlDirectives', + ]; + } + + public function setCacheControlDirectives(ResponseEvent $event): void + { + if (!$this->sectionProvider->getSection() instanceof AdminSection) { + return; + } + + $response = $event->getResponse(); + + $response->headers->addCacheControlDirective('no-cache', true); + $response->headers->addCacheControlDirective('max-age', '0'); + $response->headers->addCacheControlDirective('must-revalidate', true); + $response->headers->addCacheControlDirective('no-store', true); + } +} diff --git a/src/Sylius/Bundle/AdminBundle/Resources/config/services/listener.xml b/src/Sylius/Bundle/AdminBundle/Resources/config/services/listener.xml index 8c074404ec3..3ae6fdbe3c4 100644 --- a/src/Sylius/Bundle/AdminBundle/Resources/config/services/listener.xml +++ b/src/Sylius/Bundle/AdminBundle/Resources/config/services/listener.xml @@ -25,5 +25,10 @@ + + + + + diff --git a/src/Sylius/Bundle/AdminBundle/spec/EventListener/AdminSectionCacheControlSubscriberSpec.php b/src/Sylius/Bundle/AdminBundle/spec/EventListener/AdminSectionCacheControlSubscriberSpec.php new file mode 100644 index 00000000000..adb9d92b0a9 --- /dev/null +++ b/src/Sylius/Bundle/AdminBundle/spec/EventListener/AdminSectionCacheControlSubscriberSpec.php @@ -0,0 +1,96 @@ +beConstructedWith($sectionProvider); + } + + function it_subscribes_to_kernel_response_event() + { + $this::getSubscribedEvents()->shouldReturn([KernelEvents::RESPONSE => 'setCacheControlDirectives']); + } + + function it_adds_cache_control_directives_to_admin_requests( + SectionProviderInterface $sectionProvider, + HttpKernelInterface $kernel, + Request $request, + Response $response, + ResponseHeaderBag $responseHeaderBag, + AdminSection $adminSection + ): void { + $sectionProvider->getSection()->willReturn($adminSection); + + $response->headers = $responseHeaderBag->getWrappedObject(); + + $event = new ResponseEvent( + $kernel->getWrappedObject(), + $request->getWrappedObject(), + KernelInterface::MASTER_REQUEST, + $response->getWrappedObject() + ); + + $responseHeaderBag->addCacheControlDirective('no-cache', true)->shouldBeCalled(); + $responseHeaderBag->addCacheControlDirective('max-age', '0')->shouldBeCalled(); + $responseHeaderBag->addCacheControlDirective('must-revalidate', true)->shouldBeCalled(); + $responseHeaderBag->addCacheControlDirective('no-store', true)->shouldBeCalled(); + + $this->setCacheControlDirectives($event); + } + + function it_does_nothing_if_section_is_different_then_admin( + SectionProviderInterface $sectionProvider, + HttpKernelInterface $kernel, + Request $request, + Response $response, + ResponseHeaderBag $responseHeaderBag, + SectionInterface $section + ): void { + $sectionProvider->getSection()->willReturn($section); + + $response->headers = $responseHeaderBag->getWrappedObject(); + + $event = new ResponseEvent( + $kernel->getWrappedObject(), + $request->getWrappedObject(), + KernelInterface::MASTER_REQUEST, + $response->getWrappedObject() + ); + + $responseHeaderBag->addCacheControlDirective('no-cache', true)->shouldNotBeCalled(); + $responseHeaderBag->addCacheControlDirective('max-age', '0')->shouldNotBeCalled(); + $responseHeaderBag->addCacheControlDirective('must-revalidate', true)->shouldNotBeCalled(); + $responseHeaderBag->addCacheControlDirective('no-store', true)->shouldNotBeCalled(); + + $this->setCacheControlDirectives($event); + } +} diff --git a/src/Sylius/Bundle/ShopBundle/EventListener/ShopCustomerAccountSubSectionCacheControlSubscriber.php b/src/Sylius/Bundle/ShopBundle/EventListener/ShopCustomerAccountSubSectionCacheControlSubscriber.php new file mode 100644 index 00000000000..0c01b84585a --- /dev/null +++ b/src/Sylius/Bundle/ShopBundle/EventListener/ShopCustomerAccountSubSectionCacheControlSubscriber.php @@ -0,0 +1,51 @@ +sectionProvider = $sectionProvider; + } + + public static function getSubscribedEvents(): array + { + return [ + KernelEvents::RESPONSE => 'setCacheControlDirectives', + ]; + } + + public function setCacheControlDirectives(ResponseEvent $event): void + { + if (!$this->sectionProvider->getSection() instanceof ShopCustomerAccountSubSection) { + return; + } + + $response = $event->getResponse(); + + $response->headers->addCacheControlDirective('no-cache', true); + $response->headers->addCacheControlDirective('max-age', '0'); + $response->headers->addCacheControlDirective('must-revalidate', true); + $response->headers->addCacheControlDirective('no-store', true); + } +} diff --git a/src/Sylius/Bundle/ShopBundle/Resources/config/services.xml b/src/Sylius/Bundle/ShopBundle/Resources/config/services.xml index 5b5e5d86234..476ea0be3ea 100644 --- a/src/Sylius/Bundle/ShopBundle/Resources/config/services.xml +++ b/src/Sylius/Bundle/ShopBundle/Resources/config/services.xml @@ -30,6 +30,7 @@ + account diff --git a/src/Sylius/Bundle/ShopBundle/Resources/config/services/listeners.xml b/src/Sylius/Bundle/ShopBundle/Resources/config/services/listeners.xml index 0fb75b33793..f4467d7c00b 100644 --- a/src/Sylius/Bundle/ShopBundle/Resources/config/services/listeners.xml +++ b/src/Sylius/Bundle/ShopBundle/Resources/config/services/listeners.xml @@ -25,6 +25,11 @@ + + + + + diff --git a/src/Sylius/Bundle/ShopBundle/SectionResolver/ShopCustomerAccountSubSection.php b/src/Sylius/Bundle/ShopBundle/SectionResolver/ShopCustomerAccountSubSection.php new file mode 100644 index 00000000000..c99afa65ec5 --- /dev/null +++ b/src/Sylius/Bundle/ShopBundle/SectionResolver/ShopCustomerAccountSubSection.php @@ -0,0 +1,18 @@ +shopCustomerAccountResourceUri = $shopCustomerAccountResourceUri; + } + public function getSection(string $uri): SectionInterface { + if (str_contains($uri, $this->shopCustomerAccountResourceUri)) { + return new ShopCustomerAccountSubSection(); + } + return new ShopSection(); } } diff --git a/src/Sylius/Bundle/ShopBundle/spec/EventListener/ShopCustomerAccountSubSectionCacheControlSubscriberSpec.php b/src/Sylius/Bundle/ShopBundle/spec/EventListener/ShopCustomerAccountSubSectionCacheControlSubscriberSpec.php new file mode 100644 index 00000000000..67ac0afa0d3 --- /dev/null +++ b/src/Sylius/Bundle/ShopBundle/spec/EventListener/ShopCustomerAccountSubSectionCacheControlSubscriberSpec.php @@ -0,0 +1,93 @@ +beConstructedWith($sectionProvider); + } + + function it_subscribes_to_kernel_response_event() + { + $this::getSubscribedEvents()->shouldReturn([KernelEvents::RESPONSE => 'setCacheControlDirectives']); + } + + function it_adds_cache_control_directives_to_customer_account_requests( + SectionProviderInterface $sectionProvider, + HttpKernelInterface $kernel, + Request $request, + Response $response, + ResponseHeaderBag $responseHeaderBag, + ShopCustomerAccountSubSection $customerAccountSubSection + ): void { + $sectionProvider->getSection()->willReturn($customerAccountSubSection); + + $response->headers = $responseHeaderBag->getWrappedObject(); + + $event = new ResponseEvent( + $kernel->getWrappedObject(), + $request->getWrappedObject(), + KernelInterface::MASTER_REQUEST, + $response->getWrappedObject() + ); + + $responseHeaderBag->addCacheControlDirective('no-cache', true)->shouldBeCalled(); + $responseHeaderBag->addCacheControlDirective('max-age', '0')->shouldBeCalled(); + $responseHeaderBag->addCacheControlDirective('must-revalidate', true)->shouldBeCalled(); + $responseHeaderBag->addCacheControlDirective('no-store', true)->shouldBeCalled(); + + $this->setCacheControlDirectives($event); + } + + function it_does_nothing_if_section_is_different_then_customer_account( + SectionProviderInterface $sectionProvider, + HttpKernelInterface $kernel, + Request $request, + Response $response, + ResponseHeaderBag $responseHeaderBag, + SectionInterface $section + ): void { + $sectionProvider->getSection()->willReturn($section); + + $response->headers = $responseHeaderBag->getWrappedObject(); + + $event = new ResponseEvent( + $kernel->getWrappedObject(), + $request->getWrappedObject(), + KernelInterface::MASTER_REQUEST, + $response->getWrappedObject() + ); + + $responseHeaderBag->addCacheControlDirective('no-cache', true)->shouldNotBeCalled(); + $responseHeaderBag->addCacheControlDirective('max-age', '0')->shouldNotBeCalled(); + $responseHeaderBag->addCacheControlDirective('must-revalidate', true)->shouldNotBeCalled(); + $responseHeaderBag->addCacheControlDirective('no-store', true)->shouldNotBeCalled(); + + $this->setCacheControlDirectives($event); + } +} diff --git a/src/Sylius/Bundle/ShopBundle/spec/SectionResolver/ShopUriBasedSectionResolverSpec.php b/src/Sylius/Bundle/ShopBundle/spec/SectionResolver/ShopUriBasedSectionResolverSpec.php index ff710236909..1d687b94d71 100644 --- a/src/Sylius/Bundle/ShopBundle/spec/SectionResolver/ShopUriBasedSectionResolverSpec.php +++ b/src/Sylius/Bundle/ShopBundle/spec/SectionResolver/ShopUriBasedSectionResolverSpec.php @@ -15,6 +15,7 @@ use PhpSpec\ObjectBehavior; use Sylius\Bundle\CoreBundle\SectionResolver\UriBasedSectionResolverInterface; +use Sylius\Bundle\ShopBundle\SectionResolver\ShopCustomerAccountSubSection; use Sylius\Bundle\ShopBundle\SectionResolver\ShopSection; final class ShopUriBasedSectionResolverSpec extends ObjectBehavior @@ -24,7 +25,7 @@ function it_it_uri_based_section_resolver(): void $this->shouldImplement(UriBasedSectionResolverInterface::class); } - function it_always_returns_shop(): void + function it_returns_shop_by_default(): void { $this->getSection('/api/something')->shouldBeLike(new ShopSection()); $this->getSection('/api')->shouldBeLike(new ShopSection()); @@ -33,4 +34,29 @@ function it_always_returns_shop(): void $this->getSection('/admin/asd')->shouldBeLike(new ShopSection()); $this->getSection('/en_US/api')->shouldBeLike(new ShopSection()); } + + function it_uses_account_prefix_for_customer_account_subsection_by_default(): void + { + $this->getSection('/account')->shouldBeLike(new ShopCustomerAccountSubSection()); + $this->getSection('/api/account')->shouldBeLike(new ShopCustomerAccountSubSection()); + $this->getSection('/en_US/account')->shouldBeLike(new ShopCustomerAccountSubSection()); + $this->getSection('/account/random')->shouldBeLike(new ShopCustomerAccountSubSection()); + } + + function it_may_have_account_prefix_customized(): void + { + $this->beConstructedWith('konto'); + + $this->getSection('/konto')->shouldBeLike(new ShopCustomerAccountSubSection()); + $this->getSection('/konto')->shouldBeLike(new ShopCustomerAccountSubSection()); + $this->getSection('/api/konto')->shouldBeLike(new ShopCustomerAccountSubSection()); + $this->getSection('/en_US/konto')->shouldBeLike(new ShopCustomerAccountSubSection()); + $this->getSection('/konto/random')->shouldBeLike(new ShopCustomerAccountSubSection()); + + $this->getSection('/account')->shouldBeLike(new ShopSection()); + $this->getSection('/account')->shouldBeLike(new ShopSection()); + $this->getSection('/api/account')->shouldBeLike(new ShopSection()); + $this->getSection('/en_US/account')->shouldBeLike(new ShopSection()); + $this->getSection('/account/random')->shouldBeLike(new ShopSection()); + } } From 691b700a11b6688d7296a1693efb47bbef6141cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Chru=C5=9Bciel?= Date: Tue, 21 Sep 2021 14:40:20 +0200 Subject: [PATCH 04/11] [Maintenace] Test existence of new cache headers --- ...AdminSectionCacheControlSubscriberTest.php | 43 +++++++++++++++++++ ...AdminSectionCacheControlSubscriberTest.php | 43 +++++++++++++++++++ 2 files changed, 86 insertions(+) create mode 100644 src/Sylius/Bundle/AdminBundle/Tests/EventListener/AdminSectionCacheControlSubscriberTest.php create mode 100644 src/Sylius/Bundle/ShopBundle/Tests/EventListener/AdminSectionCacheControlSubscriberTest.php diff --git a/src/Sylius/Bundle/AdminBundle/Tests/EventListener/AdminSectionCacheControlSubscriberTest.php b/src/Sylius/Bundle/AdminBundle/Tests/EventListener/AdminSectionCacheControlSubscriberTest.php new file mode 100644 index 00000000000..40201426a2d --- /dev/null +++ b/src/Sylius/Bundle/AdminBundle/Tests/EventListener/AdminSectionCacheControlSubscriberTest.php @@ -0,0 +1,43 @@ +request('GET', '/admin/'); + + $this->assertResponseHeaderSame('Cache-Control', 'max-age=0, must-revalidate, no-cache, no-store, private'); + } + + /** + * @test + */ + public function it_returns_normal_headers_otherwise(): void + { + $client = static::createClient(); + + $client->request('GET', '/en_US/'); + + $this->assertResponseHeaderSame('Cache-Control', 'max-age=0, must-revalidate, private'); + } +} diff --git a/src/Sylius/Bundle/ShopBundle/Tests/EventListener/AdminSectionCacheControlSubscriberTest.php b/src/Sylius/Bundle/ShopBundle/Tests/EventListener/AdminSectionCacheControlSubscriberTest.php new file mode 100644 index 00000000000..f1b53199059 --- /dev/null +++ b/src/Sylius/Bundle/ShopBundle/Tests/EventListener/AdminSectionCacheControlSubscriberTest.php @@ -0,0 +1,43 @@ +request('GET', '/en_US/account/'); + + $this->assertResponseHeaderSame('Cache-Control', 'max-age=0, must-revalidate, no-cache, no-store, private'); + } + + /** + * @test + */ + public function it_returns_normal_headers_otherwise(): void + { + $client = static::createClient(); + + $client->request('GET', '/en_US/'); + + $this->assertResponseHeaderSame('Cache-Control', 'max-age=0, must-revalidate, private'); + } +} From 08d0f5a1f431f9c7400f8c8c264418541c1ef56f Mon Sep 17 00:00:00 2001 From: Grzegorz Sadowski Date: Wed, 2 Mar 2022 08:53:44 +0100 Subject: [PATCH 05/11] Remove type declarations for properties due to supporting PHP 7.3 --- .../EventListener/AdminSectionCacheControlSubscriber.php | 3 ++- ...opCustomerAccountSubSectionCacheControlSubscriber.php | 3 ++- .../SectionResolver/ShopUriBasedSectionResolver.php | 9 +++++---- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/Sylius/Bundle/AdminBundle/EventListener/AdminSectionCacheControlSubscriber.php b/src/Sylius/Bundle/AdminBundle/EventListener/AdminSectionCacheControlSubscriber.php index 08cc687f2c3..c85a01fbff1 100644 --- a/src/Sylius/Bundle/AdminBundle/EventListener/AdminSectionCacheControlSubscriber.php +++ b/src/Sylius/Bundle/AdminBundle/EventListener/AdminSectionCacheControlSubscriber.php @@ -21,7 +21,8 @@ final class AdminSectionCacheControlSubscriber implements EventSubscriberInterface { - private SectionProviderInterface $sectionProvider; + /** @var SectionProviderInterface */ + private $sectionProvider; public function __construct(SectionProviderInterface $sectionProvider) { diff --git a/src/Sylius/Bundle/ShopBundle/EventListener/ShopCustomerAccountSubSectionCacheControlSubscriber.php b/src/Sylius/Bundle/ShopBundle/EventListener/ShopCustomerAccountSubSectionCacheControlSubscriber.php index 0c01b84585a..803d481c9fa 100644 --- a/src/Sylius/Bundle/ShopBundle/EventListener/ShopCustomerAccountSubSectionCacheControlSubscriber.php +++ b/src/Sylius/Bundle/ShopBundle/EventListener/ShopCustomerAccountSubSectionCacheControlSubscriber.php @@ -21,7 +21,8 @@ final class ShopCustomerAccountSubSectionCacheControlSubscriber implements EventSubscriberInterface { - private SectionProviderInterface $sectionProvider; + /** @var SectionProviderInterface */ + private $sectionProvider; public function __construct(SectionProviderInterface $sectionProvider) { diff --git a/src/Sylius/Bundle/ShopBundle/SectionResolver/ShopUriBasedSectionResolver.php b/src/Sylius/Bundle/ShopBundle/SectionResolver/ShopUriBasedSectionResolver.php index a98b5068f2c..ef6c08b2845 100644 --- a/src/Sylius/Bundle/ShopBundle/SectionResolver/ShopUriBasedSectionResolver.php +++ b/src/Sylius/Bundle/ShopBundle/SectionResolver/ShopUriBasedSectionResolver.php @@ -18,16 +18,17 @@ final class ShopUriBasedSectionResolver implements UriBasedSectionResolverInterface { - private string $shopCustomerAccountResourceUri; + /** @var string */ + private $shopCustomerAccountUri; - public function __construct(string $shopCustomerAccountResourceUri = 'account') + public function __construct(string $shopCustomerAccountUri = 'account') { - $this->shopCustomerAccountResourceUri = $shopCustomerAccountResourceUri; + $this->shopCustomerAccountUri = $shopCustomerAccountUri; } public function getSection(string $uri): SectionInterface { - if (str_contains($uri, $this->shopCustomerAccountResourceUri)) { + if (str_contains($uri, $this->shopCustomerAccountUri)) { return new ShopCustomerAccountSubSection(); } From 94366fdfece8932ac22a54244492cd0d8811a648 Mon Sep 17 00:00:00 2001 From: Grzegorz Sadowski Date: Wed, 2 Mar 2022 08:55:37 +0100 Subject: [PATCH 06/11] Minor fixes for specs and unit tests of cache control subscribers --- .../AdminSectionCacheControlSubscriberTest.php | 2 +- .../AdminSectionCacheControlSubscriberSpec.php | 7 ++----- ...ustomerAccountSubSectionCacheControlSubscriberTest.php} | 6 +++--- ...CustomerAccountSubSectionCacheControlSubscriberSpec.php | 2 +- 4 files changed, 7 insertions(+), 10 deletions(-) rename src/Sylius/Bundle/ShopBundle/Tests/EventListener/{AdminSectionCacheControlSubscriberTest.php => ShopCustomerAccountSubSectionCacheControlSubscriberTest.php} (80%) diff --git a/src/Sylius/Bundle/AdminBundle/Tests/EventListener/AdminSectionCacheControlSubscriberTest.php b/src/Sylius/Bundle/AdminBundle/Tests/EventListener/AdminSectionCacheControlSubscriberTest.php index 40201426a2d..41feff738b5 100644 --- a/src/Sylius/Bundle/AdminBundle/Tests/EventListener/AdminSectionCacheControlSubscriberTest.php +++ b/src/Sylius/Bundle/AdminBundle/Tests/EventListener/AdminSectionCacheControlSubscriberTest.php @@ -32,7 +32,7 @@ public function it_returns_proper_cache_headers_for_admin_endpoints(): void /** * @test */ - public function it_returns_normal_headers_otherwise(): void + public function it_returns_standard_headers_otherwise(): void { $client = static::createClient(); diff --git a/src/Sylius/Bundle/AdminBundle/spec/EventListener/AdminSectionCacheControlSubscriberSpec.php b/src/Sylius/Bundle/AdminBundle/spec/EventListener/AdminSectionCacheControlSubscriberSpec.php index adb9d92b0a9..f67decf3f5d 100644 --- a/src/Sylius/Bundle/AdminBundle/spec/EventListener/AdminSectionCacheControlSubscriberSpec.php +++ b/src/Sylius/Bundle/AdminBundle/spec/EventListener/AdminSectionCacheControlSubscriberSpec.php @@ -14,12 +14,9 @@ namespace spec\Sylius\Bundle\AdminBundle\EventListener; use PhpSpec\ObjectBehavior; -use Sylius\Bundle\AdminBundle\EmailManager\ShipmentEmailManagerInterface; use Sylius\Bundle\AdminBundle\SectionResolver\AdminSection; use Sylius\Bundle\CoreBundle\SectionResolver\SectionInterface; use Sylius\Bundle\CoreBundle\SectionResolver\SectionProviderInterface; -use Sylius\Component\Core\Model\ShipmentInterface; -use Symfony\Component\EventDispatcher\GenericEvent; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpFoundation\ResponseHeaderBag; @@ -35,7 +32,7 @@ function let(SectionProviderInterface $sectionProvider): void $this->beConstructedWith($sectionProvider); } - function it_subscribes_to_kernel_response_event() + function it_subscribes_to_kernel_response_event(): void { $this::getSubscribedEvents()->shouldReturn([KernelEvents::RESPONSE => 'setCacheControlDirectives']); } @@ -67,7 +64,7 @@ function it_adds_cache_control_directives_to_admin_requests( $this->setCacheControlDirectives($event); } - function it_does_nothing_if_section_is_different_then_admin( + function it_does_nothing_if_section_is_different_than_admin( SectionProviderInterface $sectionProvider, HttpKernelInterface $kernel, Request $request, diff --git a/src/Sylius/Bundle/ShopBundle/Tests/EventListener/AdminSectionCacheControlSubscriberTest.php b/src/Sylius/Bundle/ShopBundle/Tests/EventListener/ShopCustomerAccountSubSectionCacheControlSubscriberTest.php similarity index 80% rename from src/Sylius/Bundle/ShopBundle/Tests/EventListener/AdminSectionCacheControlSubscriberTest.php rename to src/Sylius/Bundle/ShopBundle/Tests/EventListener/ShopCustomerAccountSubSectionCacheControlSubscriberTest.php index f1b53199059..0dce7db7fad 100644 --- a/src/Sylius/Bundle/ShopBundle/Tests/EventListener/AdminSectionCacheControlSubscriberTest.php +++ b/src/Sylius/Bundle/ShopBundle/Tests/EventListener/ShopCustomerAccountSubSectionCacheControlSubscriberTest.php @@ -11,11 +11,11 @@ declare(strict_types=1); -namespace Sylius\Bundle\AdminBundle\Tests\EventListener; +namespace Sylius\Bundle\ShopBundle\Tests\EventListener; use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; -final class AdminSectionCacheControlSubscriberTest extends WebTestCase +final class ShopCustomerAccountSubSectionCacheControlSubscriberTest extends WebTestCase { /** * @test @@ -32,7 +32,7 @@ public function it_returns_proper_cache_headers_for_customer_account_endpoints() /** * @test */ - public function it_returns_normal_headers_otherwise(): void + public function it_returns_standard_headers_otherwise(): void { $client = static::createClient(); diff --git a/src/Sylius/Bundle/ShopBundle/spec/EventListener/ShopCustomerAccountSubSectionCacheControlSubscriberSpec.php b/src/Sylius/Bundle/ShopBundle/spec/EventListener/ShopCustomerAccountSubSectionCacheControlSubscriberSpec.php index 67ac0afa0d3..61fc6e72b81 100644 --- a/src/Sylius/Bundle/ShopBundle/spec/EventListener/ShopCustomerAccountSubSectionCacheControlSubscriberSpec.php +++ b/src/Sylius/Bundle/ShopBundle/spec/EventListener/ShopCustomerAccountSubSectionCacheControlSubscriberSpec.php @@ -64,7 +64,7 @@ function it_adds_cache_control_directives_to_customer_account_requests( $this->setCacheControlDirectives($event); } - function it_does_nothing_if_section_is_different_then_customer_account( + function it_does_nothing_if_section_is_different_than_customer_account( SectionProviderInterface $sectionProvider, HttpKernelInterface $kernel, Request $request, From 5dee3dc656b91a9b0b88034365d48bfc1ccd0063 Mon Sep 17 00:00:00 2001 From: Grzegorz Sadowski Date: Wed, 2 Mar 2022 12:53:02 +0100 Subject: [PATCH 07/11] [Behat] Add scenarios for securing access to account and dashboard after logging out --- ...sing_back_button_after_logging_out.feature | 17 ++++++++++++ ...sing_back_button_after_logging_out.feature | 17 ++++++++++++ .../Context/Ui/Admin/DashboardContext.php | 27 ++++++++++++++++++- .../Behat/Context/Ui/Admin/LoginContext.php | 8 ++++++ .../Behat/Context/Ui/Shop/AccountContext.php | 19 ++++++++++++- src/Sylius/Behat/Context/Ui/UserContext.php | 8 ++++++ src/Sylius/Behat/Page/Admin/DashboardPage.php | 5 ++++ .../Page/Admin/DashboardPageInterface.php | 2 ++ src/Sylius/Behat/Page/Shop/HomePage.php | 5 ++++ .../Behat/Page/Shop/HomePageInterface.php | 2 ++ .../config/suites/ui/account/customer.yml | 1 + .../config/suites/ui/admin/dashboard.yml | 1 + 12 files changed, 110 insertions(+), 2 deletions(-) create mode 100644 features/account/customer_account/securing_access_to_account_after_using_back_button_after_logging_out.feature create mode 100644 features/admin/securing_access_to_account_after_using_back_button_after_logging_out.feature diff --git a/features/account/customer_account/securing_access_to_account_after_using_back_button_after_logging_out.feature b/features/account/customer_account/securing_access_to_account_after_using_back_button_after_logging_out.feature new file mode 100644 index 00000000000..2861b65eff4 --- /dev/null +++ b/features/account/customer_account/securing_access_to_account_after_using_back_button_after_logging_out.feature @@ -0,0 +1,17 @@ +@customer_account +Feature: Securing access to the account after using the back button after logging out + In order to have my personal information secured + As a Customer + I want to be unable to access to the account by using the back button after logging out + + Background: + Given the store operates on a single channel in "United States" + And I am a logged in customer + And I am browsing my orders + + @ui @javascript @no-api + Scenario: Securing access to the account after using the back button after logging out + When I log out + And I go back one page in the browser + Then I should not see my orders + And I should be on the login page diff --git a/features/admin/securing_access_to_account_after_using_back_button_after_logging_out.feature b/features/admin/securing_access_to_account_after_using_back_button_after_logging_out.feature new file mode 100644 index 00000000000..8954ec9a5ff --- /dev/null +++ b/features/admin/securing_access_to_account_after_using_back_button_after_logging_out.feature @@ -0,0 +1,17 @@ +@admin_dashboard +Feature: Securing access to the administration panel after using the back button after logging out + In order to have administration panel secured + As an Administrator + I want to be unable to access to the administration panel by using the back button after logging out + + Background: + Given the store operates on a single channel in "United States" + And I am logged in as an administrator + And I am on the administration dashboard + + @ui @javascript @no-api + Scenario: Securing access to administration dashboard after using the back button after logging out + When I log out + And I go back one page in the browser + Then I should not see the administration dashboard + And I should be on the login page diff --git a/src/Sylius/Behat/Context/Ui/Admin/DashboardContext.php b/src/Sylius/Behat/Context/Ui/Admin/DashboardContext.php index 0a76bdda944..fedabec76cb 100644 --- a/src/Sylius/Behat/Context/Ui/Admin/DashboardContext.php +++ b/src/Sylius/Behat/Context/Ui/Admin/DashboardContext.php @@ -30,9 +30,10 @@ public function __construct(DashboardPageInterface $dashboardPage) } /** + * @Given I am on the administration dashboard * @When I (try to )open administration dashboard */ - public function iOpenAdministrationDashboard() + public function iOpenAdministrationDashboard(): void { try { $this->dashboardPage->open(); @@ -56,6 +57,22 @@ public function iChooseChannel($channelName) $this->dashboardPage->chooseChannel($channelName); } + /** + * @When I log out + */ + public function iLogOut(): void + { + $this->dashboardPage->logOut(); + } + + /** + * @When I go back one page in the browser + */ + public function iGoBackOnePageInTheBrowser(): void + { + $this->dashboardPage->goBackInTheBrowser(); + } + /** * @Then I should see :number new orders */ @@ -103,4 +120,12 @@ public function iShouldSeeNewOrdersInTheList($number) { Assert::same($this->dashboardPage->getNumberOfNewOrdersInTheList(), (int) $number); } + + /** + * @Then I should not see the administration dashboard + */ + public function iShouldNotSeeTheAdministrationDashboard(): void + { + Assert::false($this->dashboardPage->isOpen()); + } } diff --git a/src/Sylius/Behat/Context/Ui/Admin/LoginContext.php b/src/Sylius/Behat/Context/Ui/Admin/LoginContext.php index a8434c56c22..3064c2e141e 100644 --- a/src/Sylius/Behat/Context/Ui/Admin/LoginContext.php +++ b/src/Sylius/Behat/Context/Ui/Admin/LoginContext.php @@ -141,4 +141,12 @@ private function logInAgain($username, $password) $this->loginPage->specifyPassword($password); $this->loginPage->logIn(); } + + /** + * @Then I should be on the login page + */ + public function iShouldBeOnTheLoginPage(): void + { + Assert::true($this->loginPage->isOpen()); + } } diff --git a/src/Sylius/Behat/Context/Ui/Shop/AccountContext.php b/src/Sylius/Behat/Context/Ui/Shop/AccountContext.php index ecb99ee819f..79e4e515a51 100644 --- a/src/Sylius/Behat/Context/Ui/Shop/AccountContext.php +++ b/src/Sylius/Behat/Context/Ui/Shop/AccountContext.php @@ -263,9 +263,10 @@ public function iShouldBeNotifiedThatThePasswordShouldBeAtLeastCharactersLong() } /** + * @Given I am browsing my orders * @When I browse my orders */ - public function iBrowseMyOrders() + public function iBrowseMyOrders(): void { $this->orderIndexPage->open(); } @@ -531,4 +532,20 @@ public function iShouldNotBeLoggedIn(): void throw new \InvalidArgumentException('Dashboard has been openned, but it shouldn\'t as customer should not be logged in'); } + + /** + * @Then I should not see my orders + */ + public function iShouldNotSeeMyOrders(): void + { + Assert::false($this->orderIndexPage->isOpen()); + } + + /** + * @Then I should be on the login page + */ + public function iShouldBeOnTheLoginPage(): void + { + Assert::true($this->loginPage->isOpen()); + } } diff --git a/src/Sylius/Behat/Context/Ui/UserContext.php b/src/Sylius/Behat/Context/Ui/UserContext.php index 3da2d86ed92..252b80fdf02 100644 --- a/src/Sylius/Behat/Context/Ui/UserContext.php +++ b/src/Sylius/Behat/Context/Ui/UserContext.php @@ -69,6 +69,14 @@ public function iDeleteAccount($email) $this->customerShowPage->deleteAccount(); } + /** + * @When I go back one page in the browser + */ + public function iGoBackOnePageInTheBrowser(): void + { + $this->homePage->goBackInTheBrowser(); + } + /** * @Then the user account should be deleted */ diff --git a/src/Sylius/Behat/Page/Admin/DashboardPage.php b/src/Sylius/Behat/Page/Admin/DashboardPage.php index 00d39b41286..6bdbd6b9235 100644 --- a/src/Sylius/Behat/Page/Admin/DashboardPage.php +++ b/src/Sylius/Behat/Page/Admin/DashboardPage.php @@ -84,6 +84,11 @@ public function chooseChannel(string $channelName): void $this->getElement('channel_choosing_link', ['%channelName%' => $channelName])->click(); } + public function goBackInTheBrowser(): void + { + $this->getDriver()->back(); + } + public function getRouteName(): string { return 'sylius_admin_dashboard'; diff --git a/src/Sylius/Behat/Page/Admin/DashboardPageInterface.php b/src/Sylius/Behat/Page/Admin/DashboardPageInterface.php index e5220e82709..03615799efc 100644 --- a/src/Sylius/Behat/Page/Admin/DashboardPageInterface.php +++ b/src/Sylius/Behat/Page/Admin/DashboardPageInterface.php @@ -36,4 +36,6 @@ public function isSectionWithLabelVisible(string $name): bool; public function logOut(): void; public function chooseChannel(string $channelName): void; + + public function goBackInTheBrowser(): void; } diff --git a/src/Sylius/Behat/Page/Shop/HomePage.php b/src/Sylius/Behat/Page/Shop/HomePage.php index 8d5ecbca5eb..d87b7c2438f 100644 --- a/src/Sylius/Behat/Page/Shop/HomePage.php +++ b/src/Sylius/Behat/Page/Shop/HomePage.php @@ -103,6 +103,11 @@ function (NodeElement $element) { ); } + public function goBackInTheBrowser(): void + { + $this->getDriver()->back(); + } + protected function getDefinedElements(): array { return array_merge(parent::getDefinedElements(), [ diff --git a/src/Sylius/Behat/Page/Shop/HomePageInterface.php b/src/Sylius/Behat/Page/Shop/HomePageInterface.php index 948865257b0..19bb4ac18aa 100644 --- a/src/Sylius/Behat/Page/Shop/HomePageInterface.php +++ b/src/Sylius/Behat/Page/Shop/HomePageInterface.php @@ -38,4 +38,6 @@ public function getAvailableLocales(): array; public function switchLocale(string $localeCode): void; public function getLatestProductsNames(): array; + + public function goBackInTheBrowser(): void; } diff --git a/src/Sylius/Behat/Resources/config/suites/ui/account/customer.yml b/src/Sylius/Behat/Resources/config/suites/ui/account/customer.yml index 34f08b62f9a..e86ba712e35 100644 --- a/src/Sylius/Behat/Resources/config/suites/ui/account/customer.yml +++ b/src/Sylius/Behat/Resources/config/suites/ui/account/customer.yml @@ -44,6 +44,7 @@ default: - sylius.behat.context.ui.shop.checkout.shipping - sylius.behat.context.ui.shop.currency - sylius.behat.context.ui.shop.homepage + - sylius.behat.context.ui.user filters: tags: "@customer_account&&@ui" diff --git a/src/Sylius/Behat/Resources/config/suites/ui/admin/dashboard.yml b/src/Sylius/Behat/Resources/config/suites/ui/admin/dashboard.yml index 2b2f32caa1f..2d2d07bfc20 100644 --- a/src/Sylius/Behat/Resources/config/suites/ui/admin/dashboard.yml +++ b/src/Sylius/Behat/Resources/config/suites/ui/admin/dashboard.yml @@ -26,6 +26,7 @@ default: - sylius.behat.context.transform.shared_storage - sylius.behat.context.ui.admin.dashboard + - sylius.behat.context.ui.admin.login - sylius.behat.context.ui.admin.notification filters: From d4bf36c88426b899bba3aefb97987cd45604b093 Mon Sep 17 00:00:00 2001 From: Grzegorz Sadowski Date: Fri, 4 Mar 2022 06:45:57 +0100 Subject: [PATCH 08/11] [Behat] Extract browser element and context --- .../Context/Ui/Admin/DashboardContext.php | 8 ----- .../Behat/Context/Ui/BrowserContext.php | 36 +++++++++++++++++++ src/Sylius/Behat/Context/Ui/UserContext.php | 8 ----- src/Sylius/Behat/Element/BrowserElement.php | 24 +++++++++++++ .../Behat/Element/BrowserElementInterface.php | 19 ++++++++++ src/Sylius/Behat/Page/Admin/DashboardPage.php | 5 --- .../Page/Admin/DashboardPageInterface.php | 2 -- src/Sylius/Behat/Page/Shop/HomePage.php | 5 --- .../Behat/Page/Shop/HomePageInterface.php | 2 -- .../Resources/config/services/contexts/ui.xml | 4 +++ .../Resources/config/services/elements.xml | 3 ++ .../config/suites/ui/account/customer.yml | 1 + .../config/suites/ui/admin/dashboard.yml | 1 + 13 files changed, 88 insertions(+), 30 deletions(-) create mode 100644 src/Sylius/Behat/Context/Ui/BrowserContext.php create mode 100644 src/Sylius/Behat/Element/BrowserElement.php create mode 100644 src/Sylius/Behat/Element/BrowserElementInterface.php diff --git a/src/Sylius/Behat/Context/Ui/Admin/DashboardContext.php b/src/Sylius/Behat/Context/Ui/Admin/DashboardContext.php index fedabec76cb..52b281e79d1 100644 --- a/src/Sylius/Behat/Context/Ui/Admin/DashboardContext.php +++ b/src/Sylius/Behat/Context/Ui/Admin/DashboardContext.php @@ -65,14 +65,6 @@ public function iLogOut(): void $this->dashboardPage->logOut(); } - /** - * @When I go back one page in the browser - */ - public function iGoBackOnePageInTheBrowser(): void - { - $this->dashboardPage->goBackInTheBrowser(); - } - /** * @Then I should see :number new orders */ diff --git a/src/Sylius/Behat/Context/Ui/BrowserContext.php b/src/Sylius/Behat/Context/Ui/BrowserContext.php new file mode 100644 index 00000000000..544704d13d5 --- /dev/null +++ b/src/Sylius/Behat/Context/Ui/BrowserContext.php @@ -0,0 +1,36 @@ +browserElement = $browserElement; + } + + /** + * @When I go back one page in the browser + */ + public function iGoBackOnePageInTheBrowser(): void + { + $this->browserElement->goBack(); + } +} diff --git a/src/Sylius/Behat/Context/Ui/UserContext.php b/src/Sylius/Behat/Context/Ui/UserContext.php index 252b80fdf02..3da2d86ed92 100644 --- a/src/Sylius/Behat/Context/Ui/UserContext.php +++ b/src/Sylius/Behat/Context/Ui/UserContext.php @@ -69,14 +69,6 @@ public function iDeleteAccount($email) $this->customerShowPage->deleteAccount(); } - /** - * @When I go back one page in the browser - */ - public function iGoBackOnePageInTheBrowser(): void - { - $this->homePage->goBackInTheBrowser(); - } - /** * @Then the user account should be deleted */ diff --git a/src/Sylius/Behat/Element/BrowserElement.php b/src/Sylius/Behat/Element/BrowserElement.php new file mode 100644 index 00000000000..3743fa94fc7 --- /dev/null +++ b/src/Sylius/Behat/Element/BrowserElement.php @@ -0,0 +1,24 @@ +getDriver()->back(); + } +} diff --git a/src/Sylius/Behat/Element/BrowserElementInterface.php b/src/Sylius/Behat/Element/BrowserElementInterface.php new file mode 100644 index 00000000000..4f533b13d57 --- /dev/null +++ b/src/Sylius/Behat/Element/BrowserElementInterface.php @@ -0,0 +1,19 @@ +getElement('channel_choosing_link', ['%channelName%' => $channelName])->click(); } - public function goBackInTheBrowser(): void - { - $this->getDriver()->back(); - } - public function getRouteName(): string { return 'sylius_admin_dashboard'; diff --git a/src/Sylius/Behat/Page/Admin/DashboardPageInterface.php b/src/Sylius/Behat/Page/Admin/DashboardPageInterface.php index 03615799efc..e5220e82709 100644 --- a/src/Sylius/Behat/Page/Admin/DashboardPageInterface.php +++ b/src/Sylius/Behat/Page/Admin/DashboardPageInterface.php @@ -36,6 +36,4 @@ public function isSectionWithLabelVisible(string $name): bool; public function logOut(): void; public function chooseChannel(string $channelName): void; - - public function goBackInTheBrowser(): void; } diff --git a/src/Sylius/Behat/Page/Shop/HomePage.php b/src/Sylius/Behat/Page/Shop/HomePage.php index d87b7c2438f..8d5ecbca5eb 100644 --- a/src/Sylius/Behat/Page/Shop/HomePage.php +++ b/src/Sylius/Behat/Page/Shop/HomePage.php @@ -103,11 +103,6 @@ function (NodeElement $element) { ); } - public function goBackInTheBrowser(): void - { - $this->getDriver()->back(); - } - protected function getDefinedElements(): array { return array_merge(parent::getDefinedElements(), [ diff --git a/src/Sylius/Behat/Page/Shop/HomePageInterface.php b/src/Sylius/Behat/Page/Shop/HomePageInterface.php index 19bb4ac18aa..948865257b0 100644 --- a/src/Sylius/Behat/Page/Shop/HomePageInterface.php +++ b/src/Sylius/Behat/Page/Shop/HomePageInterface.php @@ -38,6 +38,4 @@ public function getAvailableLocales(): array; public function switchLocale(string $localeCode): void; public function getLatestProductsNames(): array; - - public function goBackInTheBrowser(): void; } diff --git a/src/Sylius/Behat/Resources/config/services/contexts/ui.xml b/src/Sylius/Behat/Resources/config/services/contexts/ui.xml index f5dc1e78188..b98059d3fb3 100644 --- a/src/Sylius/Behat/Resources/config/services/contexts/ui.xml +++ b/src/Sylius/Behat/Resources/config/services/contexts/ui.xml @@ -299,6 +299,10 @@ + + + + diff --git a/src/Sylius/Behat/Resources/config/services/elements.xml b/src/Sylius/Behat/Resources/config/services/elements.xml index 659d57b131f..36d55417c4e 100644 --- a/src/Sylius/Behat/Resources/config/services/elements.xml +++ b/src/Sylius/Behat/Resources/config/services/elements.xml @@ -20,10 +20,13 @@ + + + diff --git a/src/Sylius/Behat/Resources/config/suites/ui/account/customer.yml b/src/Sylius/Behat/Resources/config/suites/ui/account/customer.yml index e86ba712e35..12b5861363c 100644 --- a/src/Sylius/Behat/Resources/config/suites/ui/account/customer.yml +++ b/src/Sylius/Behat/Resources/config/suites/ui/account/customer.yml @@ -33,6 +33,7 @@ default: - sylius.behat.context.setup.user - sylius.behat.context.setup.zone + - sylius.behat.context.ui.browser - sylius.behat.context.ui.channel - sylius.behat.context.ui.email - sylius.behat.context.ui.shop.account diff --git a/src/Sylius/Behat/Resources/config/suites/ui/admin/dashboard.yml b/src/Sylius/Behat/Resources/config/suites/ui/admin/dashboard.yml index 2d2d07bfc20..0a3844c97fc 100644 --- a/src/Sylius/Behat/Resources/config/suites/ui/admin/dashboard.yml +++ b/src/Sylius/Behat/Resources/config/suites/ui/admin/dashboard.yml @@ -28,6 +28,7 @@ default: - sylius.behat.context.ui.admin.dashboard - sylius.behat.context.ui.admin.login - sylius.behat.context.ui.admin.notification + - sylius.behat.context.ui.browser filters: tags: "@admin_dashboard&&@ui" From afa04e392472efed81f0b91c931d2452edbbc760 Mon Sep 17 00:00:00 2001 From: Grzegorz Sadowski Date: Fri, 4 Mar 2022 06:52:38 +0100 Subject: [PATCH 09/11] Replace str_contains with strpos method to support PHP 7 --- .../ShopBundle/SectionResolver/ShopUriBasedSectionResolver.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Sylius/Bundle/ShopBundle/SectionResolver/ShopUriBasedSectionResolver.php b/src/Sylius/Bundle/ShopBundle/SectionResolver/ShopUriBasedSectionResolver.php index ef6c08b2845..ddba21afbdc 100644 --- a/src/Sylius/Bundle/ShopBundle/SectionResolver/ShopUriBasedSectionResolver.php +++ b/src/Sylius/Bundle/ShopBundle/SectionResolver/ShopUriBasedSectionResolver.php @@ -28,7 +28,7 @@ public function __construct(string $shopCustomerAccountUri = 'account') public function getSection(string $uri): SectionInterface { - if (str_contains($uri, $this->shopCustomerAccountUri)) { + if (strpos($uri, $this->shopCustomerAccountUri) !== false) { return new ShopCustomerAccountSubSection(); } From b00eb517b1863bf013ac19512dbcc6102463953e Mon Sep 17 00:00:00 2001 From: Grzegorz Sadowski Date: Mon, 7 Mar 2022 17:07:50 +0100 Subject: [PATCH 10/11] [PHPUnit] Move subscribers tests to main directory --- .../AdminSectionCacheControlSubscriberTest.php | 14 +------------- ...AccountSubSectionCacheControlSubscriberTest.php | 14 +------------- 2 files changed, 2 insertions(+), 26 deletions(-) rename {src/Sylius/Bundle/AdminBundle/Tests => tests}/EventListener/AdminSectionCacheControlSubscriberTest.php (66%) rename {src/Sylius/Bundle/ShopBundle/Tests => tests}/EventListener/ShopCustomerAccountSubSectionCacheControlSubscriberTest.php (67%) diff --git a/src/Sylius/Bundle/AdminBundle/Tests/EventListener/AdminSectionCacheControlSubscriberTest.php b/tests/EventListener/AdminSectionCacheControlSubscriberTest.php similarity index 66% rename from src/Sylius/Bundle/AdminBundle/Tests/EventListener/AdminSectionCacheControlSubscriberTest.php rename to tests/EventListener/AdminSectionCacheControlSubscriberTest.php index 41feff738b5..ae9bd10c944 100644 --- a/src/Sylius/Bundle/AdminBundle/Tests/EventListener/AdminSectionCacheControlSubscriberTest.php +++ b/tests/EventListener/AdminSectionCacheControlSubscriberTest.php @@ -11,7 +11,7 @@ declare(strict_types=1); -namespace Sylius\Bundle\AdminBundle\Tests\EventListener; +namespace Sylius\Tests\EventListener; use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; @@ -28,16 +28,4 @@ public function it_returns_proper_cache_headers_for_admin_endpoints(): void $this->assertResponseHeaderSame('Cache-Control', 'max-age=0, must-revalidate, no-cache, no-store, private'); } - - /** - * @test - */ - public function it_returns_standard_headers_otherwise(): void - { - $client = static::createClient(); - - $client->request('GET', '/en_US/'); - - $this->assertResponseHeaderSame('Cache-Control', 'max-age=0, must-revalidate, private'); - } } diff --git a/src/Sylius/Bundle/ShopBundle/Tests/EventListener/ShopCustomerAccountSubSectionCacheControlSubscriberTest.php b/tests/EventListener/ShopCustomerAccountSubSectionCacheControlSubscriberTest.php similarity index 67% rename from src/Sylius/Bundle/ShopBundle/Tests/EventListener/ShopCustomerAccountSubSectionCacheControlSubscriberTest.php rename to tests/EventListener/ShopCustomerAccountSubSectionCacheControlSubscriberTest.php index 0dce7db7fad..c3ad2f1da82 100644 --- a/src/Sylius/Bundle/ShopBundle/Tests/EventListener/ShopCustomerAccountSubSectionCacheControlSubscriberTest.php +++ b/tests/EventListener/ShopCustomerAccountSubSectionCacheControlSubscriberTest.php @@ -11,7 +11,7 @@ declare(strict_types=1); -namespace Sylius\Bundle\ShopBundle\Tests\EventListener; +namespace Sylius\Tests\EventListener; use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; @@ -28,16 +28,4 @@ public function it_returns_proper_cache_headers_for_customer_account_endpoints() $this->assertResponseHeaderSame('Cache-Control', 'max-age=0, must-revalidate, no-cache, no-store, private'); } - - /** - * @test - */ - public function it_returns_standard_headers_otherwise(): void - { - $client = static::createClient(); - - $client->request('GET', '/en_US/'); - - $this->assertResponseHeaderSame('Cache-Control', 'max-age=0, must-revalidate, private'); - } } From 46ed54bb0b7dae44e53d429a3edd68f5506c47a0 Mon Sep 17 00:00:00 2001 From: Rafikooo Date: Thu, 3 Mar 2022 15:08:07 +0100 Subject: [PATCH 11/11] [Security] XSS - SVG file upload vulnerability fixed --- composer.json | 1 + src/Sylius/Bundle/ApiBundle/composer.json | 1 + .../Component/Core/Uploader/ImageUploader.php | 26 ++++++-- src/Sylius/Component/Core/composer.json | 1 + symfony.lock | 3 + tests/Functional/ImageUploaderTest.php | 59 +++++++++++++++++++ tests/Resources/xss.svg | 9 +++ 7 files changed, 95 insertions(+), 5 deletions(-) create mode 100644 tests/Functional/ImageUploaderTest.php create mode 100644 tests/Resources/xss.svg diff --git a/composer.json b/composer.json index e750eb0d0aa..9e3de1f2245 100644 --- a/composer.json +++ b/composer.json @@ -35,6 +35,7 @@ "doctrine/migrations": "^3.0", "doctrine/orm": "^2.7", "egulias/email-validator": "^2.1", + "enshrined/svg-sanitize": "^0.15.4", "fakerphp/faker": "^1.9", "friendsofphp/proxy-manager-lts": "^1.0", "friendsofsymfony/oauth-server-bundle": ">2.0.0-alpha.0 ^2.0@dev", diff --git a/src/Sylius/Bundle/ApiBundle/composer.json b/src/Sylius/Bundle/ApiBundle/composer.json index 6a4c4fb5013..fa47c64d68a 100644 --- a/src/Sylius/Bundle/ApiBundle/composer.json +++ b/src/Sylius/Bundle/ApiBundle/composer.json @@ -26,6 +26,7 @@ "php": "^7.3", "api-platform/core": "^2.5", "doctrine/dbal": "^2.7", + "enshrined/svg-sanitize": "^0.15.4", "lexik/jwt-authentication-bundle": "^2.6", "sylius/core-bundle": "^1.7", "symfony/messenger": "^4.4 || ^5.2" diff --git a/src/Sylius/Component/Core/Uploader/ImageUploader.php b/src/Sylius/Component/Core/Uploader/ImageUploader.php index 495caf06916..21d5302ce5d 100644 --- a/src/Sylius/Component/Core/Uploader/ImageUploader.php +++ b/src/Sylius/Component/Core/Uploader/ImageUploader.php @@ -13,6 +13,7 @@ namespace Sylius\Component\Core\Uploader; +use enshrined\svgSanitize\Sanitizer; use Gaufrette\Filesystem; use Sylius\Component\Core\Generator\ImagePathGeneratorInterface; use Sylius\Component\Core\Generator\UploadedImagePathGenerator; @@ -22,12 +23,18 @@ class ImageUploader implements ImageUploaderInterface { + private const MIME_SVG_XML = 'image/svg+xml'; + private const MIME_SVG = 'image/svg'; + /** @var Filesystem */ protected $filesystem; /** @var ImagePathGeneratorInterface */ protected $imagePathGenerator; + /** @var Sanitizer */ + protected $sanitizer; + public function __construct( Filesystem $filesystem, ?ImagePathGeneratorInterface $imagePathGenerator = null @@ -41,6 +48,7 @@ public function __construct( } $this->imagePathGenerator = $imagePathGenerator ?? new UploadedImagePathGenerator(); + $this->sanitizer = new Sanitizer(); } public function upload(ImageInterface $image): void @@ -49,11 +57,13 @@ public function upload(ImageInterface $image): void return; } + /** @var File $file */ $file = $image->getFile(); - /** @var File $file */ Assert::isInstanceOf($file, File::class); + $fileContent = $this->sanitizeContent(file_get_contents($file->getPathname()), $file->getMimeType()); + if (null !== $image->getPath() && $this->has($image->getPath())) { $this->remove($image->getPath()); } @@ -64,10 +74,7 @@ public function upload(ImageInterface $image): void $image->setPath($path); - $this->filesystem->write( - $image->getPath(), - file_get_contents($image->getFile()->getPathname()) - ); + $this->filesystem->write($image->getPath(), $fileContent); } public function remove(string $path): bool @@ -79,6 +86,15 @@ public function remove(string $path): bool return false; } + protected function sanitizeContent(string $fileContent, string $mimeType): string + { + if (self::MIME_SVG_XML === $mimeType || self::MIME_SVG === $mimeType) { + $fileContent = $this->sanitizer->sanitize($fileContent); + } + + return $fileContent; + } + private function has(string $path): bool { return $this->filesystem->has($path); diff --git a/src/Sylius/Component/Core/composer.json b/src/Sylius/Component/Core/composer.json index d656d92d3eb..ac5a6bab7e3 100644 --- a/src/Sylius/Component/Core/composer.json +++ b/src/Sylius/Component/Core/composer.json @@ -27,6 +27,7 @@ ], "require": { "php": "^7.3", + "enshrined/svg-sanitize": "^0.15.4", "knplabs/gaufrette": "^0.8", "payum/payum": "^1.6", "php-http/guzzle6-adapter": "^2.0", diff --git a/symfony.lock b/symfony.lock index cf7e64da40d..9c829d7ae67 100644 --- a/symfony.lock +++ b/symfony.lock @@ -155,6 +155,9 @@ "egulias/email-validator": { "version": "2.1.19" }, + "enshrined/svg-sanitize": { + "version": "0.15.4" + }, "fakerphp/faker": { "version": "v1.12.0" }, diff --git a/tests/Functional/ImageUploaderTest.php b/tests/Functional/ImageUploaderTest.php new file mode 100644 index 00000000000..3bc4cecef14 --- /dev/null +++ b/tests/Functional/ImageUploaderTest.php @@ -0,0 +1,59 @@ +getContainer(); + + $imageUploader = self::$container->get('sylius.image_uploader'); + $fileSystem = self::$container->get('gaufrette.sylius_image_filesystem'); + + $file = new UploadedFile(__DIR__ . '/../Resources/xss.svg', 'xss.svg'); + Assert::assertStringContainsString('getContent($file)); + + $image = new ProductImage(); + $image->setFile($file); + + $imageUploader->upload($image); + + $sanitizedFile = $fileSystem->get($image->getPath()); + Assert::assertStringNotContainsString('getContent()); + } + + private function getContent(UploadedFile $file): string + { + $content = file_get_contents($file->getPathname()); + + if (false === $content) { + throw new FileException(sprintf('Could not get the content of the file "%s".', $file->getPathname())); + } + + return $content; + } +} diff --git a/tests/Resources/xss.svg b/tests/Resources/xss.svg new file mode 100644 index 00000000000..08ecd246d46 --- /dev/null +++ b/tests/Resources/xss.svg @@ -0,0 +1,9 @@ + + + + + + +