Missing "relativePath" in "targetPath" before login

rvanginneken commented 6 years ago

Environment

Sonata packages

sonata-project/admin-bundle              3.24.0          3.24.0             The missing Symfony Admin Generator
sonata-project/block-bundle              3.5.0           3.5.0              Symfony SonataBlockBundle
sonata-project/cache                     1.1.0           2.0.0              Cache library
sonata-project/cache-bundle              2.3.1           2.3.1              This bundle provides caching services
sonata-project/core-bundle               3.6.0           3.6.0              Symfony SonataCoreBundle
sonata-project/datagrid-bundle           2.2.1           dev-master 69e58b5 Symfony SonataDatagridBundle
sonata-project/doctrine-extensions       1.0.2           1.0.2              Doctrine2 behavioral extensions
sonata-project/doctrine-orm-admin-bundle 3.1.7           3.1.7              Symfony Sonata / Integrate Doctrine ORM into the SonataAdminBundle
sonata-project/easy-extends-bundle       2.2.0           2.2.0              Symfony SonataEasyExtendsBundle
sonata-project/exporter                  1.7.1           1.7.1              Lightweight Exporter library
sonata-project/formatter-bundle          3.3.0           3.3.0              Symfony SonataFormatterBundle
sonata-project/media-bundle              3.8.0           3.8.0              Symfony SonataMediaBundle
sonata-project/notification-bundle       3.2.0           3.2.0              Symfony SonataNotificationBundle
sonata-project/page-bundle               3.x-dev da1a9e7 dev-master cdbab60 This bundle provides a Site and Page management through contain...
sonata-project/seo-bundle                2.3.0           2.3.0              Symfony SonataSeoBundle

Symfony packages

symfony/monolog-bundle     v3.1.1  v3.1.2  Symfony MonologBundle
symfony/phpunit-bridge     v2.8.28 v3.3.10 Symfony PHPUnit Bridge
symfony/polyfill-apcu      v1.6.0  v1.6.0  Symfony polyfill backporting apcu_* functions to lower PHP versions
symfony/polyfill-intl-icu  v1.6.0  v1.6.0  Symfony polyfill for intl's ICU-related data and classes
symfony/polyfill-mbstring  v1.6.0  v1.6.0  Symfony polyfill for the Mbstring extension
symfony/polyfill-php56     v1.6.0  v1.6.0  Symfony polyfill backporting some PHP 5.6+ features to lower PHP versions
symfony/polyfill-php70     v1.6.0  v1.6.0  Symfony polyfill backporting some PHP 7.0+ features to lower PHP versions
symfony/polyfill-php72     v1.6.0  v1.6.0  Symfony polyfill backporting some PHP 7.2+ features to lower PHP versions
symfony/polyfill-util      v1.6.0  v1.6.0  Symfony utilities for portability of PHP codes
symfony/security-acl       v3.0.0  v3.0.0  Symfony Security Component - ACL (Access Control List)
symfony/swiftmailer-bundle v2.6.7  v3.1.6  Symfony SwiftmailerBundle
symfony/symfony            v3.3.10 v3.3.10 The Symfony PHP framework

PHP version

PHP 7.1.11-1+0~20171027135825.10+jessie~1.gbp2e638d (cli) (built: Oct 27 2017 14:42:23) ( NTS )
Copyright (c) 1997-2017 The PHP Group
Zend Engine v3.1.0, Copyright (c) 1998-2017 Zend Technologies
    with Zend OPcache v7.1.11-1+0~20171027135825.10+jessie~1.gbp2e638d, Copyright (c) 1999-2017, by Zend Technologies
    with Xdebug v2.5.5, Copyright (c) 2002-2017, by Derick Rethans

Subject

In our project, we're working multilanguage by using the "host_with_path" setting. We had to add another firewall layer for our "regular" users and here we were met with a problem: When surfing to a page behind the firewall, the user is automatically redirected to the login page. After a succesful login, he should be redirected back to the page he wanted to go. We noticed however, that in our setup the user would be redirected to the homepage.

The reason for this is that targetPath is filled by the Request's function "getUri". This function is not aware of our "sites" and their "relative paths". So it would redirect the user to the targetPath without the relative path, which page does not exist, redirecting it again to the homepage. For example, when our initial target was localhost/nl/payment, getUri will return localhost/payment.

We've built a fix for this in our project, which I'm willing to make a PR for, but I prefer to first hear your feedback. Our fix overrides the "security.exception_listener" and all it changes is the targetPath saved to the session.

protected function setTargetPath(Request $request)
    {
        // session isn't required when using HTTP basic authentication mechanism for example
        if ($request->hasSession() && $request->isMethodSafe(false) && !$request->isXmlHttpRequest()) {
            $this->saveTargetPath($request->getSession(), $this->providerKey, $request->getUriForPath($request->getRequestUri()));
        }
    }

On a sidenote: we've noticed earlier on some problem with relative paths in the admin environment. For example: Using "show" on localhost/nl/admin/sonata/page-page/49/edit would redirect the page to localhost/nl/nl/PAGE-SLUG (note the doube /nl). But because this was of minor concern at the time, we solved that one by forcing only access to the domain without relative path through the .htaccess.

rvanginneken commented 6 years ago

@acrobat Did I get everything?