fix exception handler priority (#155)

* fix exception handler priority * revert af8bfe7, add tests for it
dachcom-digital · May 15, 2024 · 05898e2 · 05898e2
1 parent 07ec2f3
commit 05898e2
Show file tree

Hide file tree

Showing 17 changed files with 125 additions and 19 deletions.
diff --git a/UPGRADE.md b/UPGRADE.md
@@ -1,5 +1,9 @@
 # Upgrade Notes
 
+## 5.0.3
+- [BUGFIX] Fix exception handler priority to prevent authentication exception hijacking
+- [BUGFIX] Revert [#af8bfe7](https://github.com/dachcom-digital/pimcore-i18n/commit/af8bfe74488fd85ebcdb14e4300f3a9f7ddc7dbe)
+
 ## 5.0.2
 - fixed bug when no locale url mapping is available [#153](https://github.com/dachcom-digital/pimcore-i18n/pull/153)
 

diff --git a/composer.json b/composer.json
@@ -37,6 +37,7 @@
     },
     "require-dev": {
         "codeception/codeception": "^5.0",
+        "codeception/module-asserts": "^3.0",
         "codeception/module-symfony": "^3.1",
         "codeception/module-webdriver": "^4.0",
         "phpstan/phpstan": "^1.0",

diff --git a/src/EventListener/Frontend/ResponseExceptionListener.php b/src/EventListener/Frontend/ResponseExceptionListener.php
@@ -44,7 +44,7 @@ public function __construct(
     public static function getSubscribedEvents(): array
     {
         return [
-            KernelEvents::EXCEPTION => ['onKernelException', 10]
+            KernelEvents::EXCEPTION => 'onKernelException'
         ];
     }
 

diff --git a/src/Modifier/RouteModifier.php b/src/Modifier/RouteModifier.php
@@ -86,7 +86,7 @@ public function parseLocaleUrlMapping(ZoneInterface $zone, string $path, string
         //transform locale style to given url mapping - if existing
         $urlMapping = $zone->getLocaleUrlMapping();
 
-        if (!array_key_exists($locale, $urlMapping) || empty($urlMapping[$locale])) {
+        if (!array_key_exists($locale, $urlMapping)) {
             return $path;
         }
 

diff --git a/tests/FunctionalDefault.suite.dist.yml b/tests/FunctionalDefault.suite.dist.yml
@@ -10,5 +10,9 @@ modules:
                 run_installer: true
         - \Dachcom\Codeception\Support\Helper\Browser\PhpBrowser:
                 depends: \Dachcom\Codeception\Support\Helper\PimcoreCore
+        - \DachcomBundle\Test\Support\Helper\I18n:
+                depends: \Dachcom\Codeception\Support\Helper\PimcoreBackend
         - \Dachcom\Codeception\Support\Helper\PimcoreBackend
         - \Dachcom\Codeception\Support\Helper\PimcoreUser
+        - \Pimcore\Tests\Support\Helper\ClassManager
+        - \Codeception\Module\Asserts
diff --git a/tests/FunctionalDefaultCountry.suite.dist.yml b/tests/FunctionalDefaultCountry.suite.dist.yml
@@ -1,4 +1,4 @@
-actor: FunctionalI18nTester
+actor: FunctionalTester
 error_level: '%PIMCORE_PHP_ERROR_REPORTING%'
 modules:
     enabled:
@@ -10,7 +10,9 @@ modules:
                 run_installer: true
         - \Dachcom\Codeception\Support\Helper\Browser\PhpBrowser:
                 depends: \Dachcom\Codeception\Support\Helper\PimcoreCore
+        - \DachcomBundle\Test\Support\Helper\I18n:
+                depends: \Dachcom\Codeception\Support\Helper\PimcoreBackend
         - \Dachcom\Codeception\Support\Helper\PimcoreBackend
         - \Dachcom\Codeception\Support\Helper\PimcoreUser
-        - \DachcomBundle\Test\Support\Helper\I18n:
-                depends: \Dachcom\Codeception\Support\Helper\PimcoreBackend
+        - \Pimcore\Tests\Support\Helper\ClassManager
+        - \Codeception\Module\Asserts
diff --git a/tests/FunctionalDefaultCountry/FrontpageMappingCest.php b/tests/FunctionalDefaultCountry/FrontpageMappingCest.php
@@ -2,11 +2,11 @@
 
 namespace DachcomBundle\Test\FunctionalDefaultCountry;
 
-use DachcomBundle\Test\Support\FunctionalI18nTester;
+use DachcomBundle\Test\Support\FunctionalTester;
 
 class FrontpageMappingCest
 {
-    public function testFrontPageMappingWithRedirecting(FunctionalI18nTester $I): void
+    public function testFrontPageMappingWithRedirecting(FunctionalTester $I): void
     {
         $document1 = $I->haveAPageDocument('en', [], 'en');
         $document2 = $I->haveAHardLink($document1, 'en-us', [], 'en_US');
@@ -17,7 +17,7 @@ public function testFrontPageMappingWithRedirecting(FunctionalI18nTester $I): vo
         $I->see($document3->getId(), '#page-id');
     }
 
-    public function testFrontPageMappingWithoutRedirecting(FunctionalI18nTester $I): void
+    public function testFrontPageMappingWithoutRedirecting(FunctionalTester $I): void
     {
         $document1 = $I->haveAPageDocument('en', [], 'en');
         $document2 = $I->haveAHardLink($document1, 'en-us', [], 'en_US');

diff --git a/tests/FunctionalFullPageCache.suite.dist.yml b/tests/FunctionalFullPageCache.suite.dist.yml
@@ -10,5 +10,9 @@ modules:
                 run_installer: true
         - \Dachcom\Codeception\Support\Helper\Browser\PhpBrowser:
                 depends: \Dachcom\Codeception\Support\Helper\PimcoreCore
+        - \DachcomBundle\Test\Support\Helper\I18n:
+                depends: \Dachcom\Codeception\Support\Helper\PimcoreBackend
         - \Dachcom\Codeception\Support\Helper\PimcoreBackend
         - \Dachcom\Codeception\Support\Helper\PimcoreUser
+        - \Pimcore\Tests\Support\Helper\ClassManager
+        - \Codeception\Module\Asserts
diff --git a/tests/FunctionalZoneExtend.suite.dist.yml b/tests/FunctionalZoneExtend.suite.dist.yml
@@ -11,6 +11,9 @@ modules:
                 run_installer: true
         - \Dachcom\Codeception\Support\Helper\Browser\PhpBrowser:
                 depends: \Dachcom\Codeception\Support\Helper\PimcoreCore
+        - \DachcomBundle\Test\Support\Helper\I18n:
+                depends: \Dachcom\Codeception\Support\Helper\PimcoreBackend
         - \Dachcom\Codeception\Support\Helper\PimcoreBackend
         - \Dachcom\Codeception\Support\Helper\PimcoreUser
         - \Pimcore\Tests\Support\Helper\ClassManager
+        - \Codeception\Module\Asserts
diff --git a/tests/FunctionalZoneHosts.suite.dist.yml b/tests/FunctionalZoneHosts.suite.dist.yml
@@ -10,5 +10,9 @@ modules:
                 run_installer: true
         - \Dachcom\Codeception\Support\Helper\Browser\PhpBrowser:
                 depends: \Dachcom\Codeception\Support\Helper\PimcoreCore
+        - \DachcomBundle\Test\Support\Helper\I18n:
+                depends: \Dachcom\Codeception\Support\Helper\PimcoreBackend
         - \Dachcom\Codeception\Support\Helper\PimcoreBackend
         - \Dachcom\Codeception\Support\Helper\PimcoreUser
+        - \Pimcore\Tests\Support\Helper\ClassManager
+        - \Codeception\Module\Asserts
diff --git a/tests/FunctionalZoneSimple.suite.dist.yml b/tests/FunctionalZoneSimple.suite.dist.yml
@@ -10,5 +10,9 @@ modules:
                 run_installer: true
         - \Dachcom\Codeception\Support\Helper\Browser\PhpBrowser:
                 depends: \Dachcom\Codeception\Support\Helper\PimcoreCore
+        - \DachcomBundle\Test\Support\Helper\I18n:
+                depends: \Dachcom\Codeception\Support\Helper\PimcoreBackend
         - \Dachcom\Codeception\Support\Helper\PimcoreBackend
         - \Dachcom\Codeception\Support\Helper\PimcoreUser
+        - \Pimcore\Tests\Support\Helper\ClassManager
+        - \Codeception\Module\Asserts
diff --git a/tests/FunctionalZoneSimple/LinkGeneratorCest.php b/tests/FunctionalZoneSimple/LinkGeneratorCest.php
@@ -0,0 +1,66 @@
+<?php
+
+namespace DachcomBundle\Test\FunctionalZoneSimple;
+
+use DachcomBundle\Test\Support\FunctionalTester;
+
+class LinkGeneratorCest
+{
+    public function testObjectLinkGenerator(FunctionalTester $I): void
+    {
+        $site = $I->haveASite('test-domain1.test');
+
+        $classDefinition = $I->haveAPimcoreClass('TestClass');
+        $object = $I->haveAPimcoreObject($classDefinition->getName(), 'object-1');
+
+        // to prevent pimcores static route caching
+        $routeName = sprintf('test_route_%s', uniqid('', false));
+        $object->setProperty('dynamic_static_route_name', 'input', $routeName);
+
+        $I->haveAPageDocumentForSite($site, 'de', [], 'de');
+
+        $I->haveAStaticRoute($routeName, [
+            'pattern'   => '/(\w+)\\/special-route\\/(.*?)$/',
+            'reverse'   => '/{%_locale}/special-route/%object_id',
+            'action'    => 'defaultAction',
+            'variables' => '_locale,object_id',
+        ]);
+
+        $url = $I->haveAI18nGeneratedLinkForElement(
+            $object,
+            ['_locale' => 'de'],
+            ['site' => $site]
+        );
+
+        $I->assertSame($url, sprintf('http://test-domain1.test/de/special-route/%d', $object->getId()));
+    }
+
+    public function testObjectLinkGeneratorWithInvalidSiteLocale(FunctionalTester $I): void
+    {
+        $site = $I->haveASite('test-domain1.test', [], 'de');
+
+        $classDefinition = $I->haveAPimcoreClass('TestClass');
+        $object = $I->haveAPimcoreObject($classDefinition->getName(), 'object-1');
+
+        // to prevent pimcores static route caching
+        $routeName = sprintf('test_route_%s', uniqid('', false));
+        $object->setProperty('dynamic_static_route_name', 'input', $routeName);
+
+        $I->haveAPageDocumentForSite($site, 'de', [], 'de');
+
+        $I->haveAStaticRoute($routeName, [
+            'pattern'   => '/(\w+)\\/special-route\\/(.*?)$/',
+            'reverse'   => '/{%_locale}/special-route/%object_id',
+            'action'    => 'defaultAction',
+            'variables' => '_locale,object_id',
+        ]);
+
+        $url = $I->haveAI18nGeneratedLinkForElement(
+            $object,
+            ['_locale' => 'de'],
+            ['site' => $site]
+        );
+
+        $I->assertSame($url, sprintf('http://test-domain1.test/special-route/%d', $object->getId()));
+    }
+}
diff --git a/tests/Support/FunctionalI18nTester.php b/tests/Support/FunctionalI18nTester.php
diff --git a/tests/Support/Helper/I18n.php b/tests/Support/Helper/I18n.php
@@ -6,8 +6,11 @@
 use Codeception\Module;
 use Codeception\Util\Debug;
 use Dachcom\Codeception\Support\Helper\PimcoreBackend;
+use I18nBundle\Builder\RouteParameterBuilder;
 use Pimcore\Model\Document\Hardlink;
 use Pimcore\Model\Document\Page;
+use Pimcore\Model\Element\ElementInterface;
+use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
 
 class I18n extends Module implements DependsOnModule
 {
@@ -25,9 +28,6 @@ public function _inject(PimcoreBackend $pimcoreBackend): void
         $this->pimcoreBackend = $pimcoreBackend;
     }
 
-    /**
-     * Actor Function to create a FrontPage mapped Document
-     */
     public function haveAFrontPageMappedDocument(Hardlink $hardlinkDocument): Page
     {
         $document = $this->pimcoreBackend->generatePageDocument('frontpage-mapped-' . $hardlinkDocument->getKey());
@@ -51,4 +51,12 @@ public function haveAFrontPageMappedDocument(Hardlink $hardlinkDocument): Page
 
         return $document;
     }
+
+    public function haveAI18nGeneratedLinkForElement(ElementInterface $element, array $routeParameters, array $context = [], string $referenceType = UrlGeneratorInterface::ABSOLUTE_URL): string
+    {
+        $parameters = RouteParameterBuilder::buildForEntity($element, $routeParameters, $context);
+        $urlGenerator = \Pimcore::getContainer()->get('router');
+
+        return $urlGenerator->generate('', $parameters, $referenceType);
+    }
 }
diff --git a/tests/Support/Services/TestLinkGenerator.php b/tests/Support/Services/TestLinkGenerator.php
@@ -11,6 +11,10 @@ class TestLinkGenerator implements LinkGeneratorInterface, I18nLinkGeneratorInte
 {
     public function getStaticRouteName(Concrete $object): string
     {
+        if ($object->hasProperty('dynamic_static_route_name')) {
+            return $object->getProperty('dynamic_static_route_name');
+        }
+
         return 'test_route';
     }
 

diff --git a/tests/Unit/Config/ConfigurationTest.php → tests/Unit/ConfigurationTest.php b/tests/Unit/Config/ConfigurationTest.php → tests/Unit/ConfigurationTest.php
diff --git a/tests/_etc/config/bundle/config_zone_simple.yaml b/tests/_etc/config/bundle/config_zone_simple.yaml
@@ -14,3 +14,13 @@ i18n:
                 - 'test-domain1.test'
                 - 'test-domain2.test'
                 - 'test-domain3.test'
+
+services:
+    DachcomBundle\Test\Support\Services\TestAlternateListener:
+        autowire: true
+        public: false
+        tags:
+            - { name: kernel.event_subscriber }
+
+    DachcomBundle\Test\Support\Services\TestLinkGenerator:
+        public: true