[BUGFIX] Respect sub-requests in HTTP referrer redirect URL evaluation

With #99920 the HTTP referrer evaluation has been extended to not overwrite the evaluated HTTP referrer on failed logins. The fix however broke the HTTP referrer evaluation, when the login plugin is placed on a page which is configured as 403 error page. In this case, the page is called via sub-request and a possible available HTTP referrer from the initiating request is used as redirect url. This patch extends the HTTP referrer evaluation, so the URL of the initiating request is used as HTTP referrer variable, if the plugin is called via sub-request. This ensures, that the user is redirected to the URL which the 403 error handler intercepted. In order to do so, the `PageContentErrorHandler` is extended to pass the original request as request attribute `originalRequest` to the sub-request. Additionally, the evaluation of the referrer URL has been moved to `RedirectHandler` and all scenarios have been covered with tests. It has to be noted, that in TYPO3 11.5 the fix will only work, if the "Subrequest page errors" feature toggle is enabled. Resolves: #100715 Releases: main, 12.4, 11.5 Signed-off-by: Torben Hansen <derhansen@gmail.com> Change-Id: Ibcfdf5093eac72f1796d15f40ef9426d0597d7f3 Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/79785 Tested-by: Benni Mack <benni@typo3.org> Tested-by: core-ci <typo3@b13.com> Reviewed-by: Benni Mack <benni@typo3.org>
TYPO3 · Jul 6, 2023 · 9d3317e · 9d3317e
1 parent cc1909a
commit 9d3317e
Show file tree

Hide file tree

Showing 4 changed files with 122 additions and 48 deletions.
diff --git a/typo3/sysext/core/Classes/Error/PageErrorHandler/PageContentErrorHandler.php b/typo3/sysext/core/Classes/Error/PageErrorHandler/PageContentErrorHandler.php
@@ -79,9 +79,9 @@ public function handlePageError(ServerRequestInterface $request, string $message
                     $this->statusCode
                 );
             }
-            // Create a subrequest and do not take any special query parameters into account
+            // Create a sub-request and do not take any special query parameters into account
             $subRequest = $request->withQueryParams([])->withUri(new Uri($resolvedUrl))->withMethod('GET');
-            $subResponse = $this->stashEnvironment(fn (): ResponseInterface => $this->sendSubRequest($subRequest, $urlParams['pageuid']));
+            $subResponse = $this->stashEnvironment(fn (): ResponseInterface => $this->sendSubRequest($subRequest, $urlParams['pageuid'], $request));
 
             if ($subResponse->getStatusCode() >= 300) {
                 throw new \RuntimeException(sprintf('Error handler could not fetch error page "%s", status code: %s', $resolvedUrl, $subResponse->getStatusCode()), 1544172839);
@@ -115,14 +115,16 @@ protected function stashEnvironment(callable $fetcher): ResponseInterface
      *
      * The $pageId is used to ensure the correct site is accessed.
      */
-    protected function sendSubRequest(ServerRequestInterface $request, int $pageId): ResponseInterface
+    protected function sendSubRequest(ServerRequestInterface $request, int $pageId, ServerRequestInterface $originalRequest): ResponseInterface
     {
         $site = $request->getAttribute('site');
         if (!$site instanceof Site) {
             $site = $this->siteFinder->getSiteByPageId($pageId);
             $request = $request->withAttribute('site', $site);
         }
 
+        $request = $request->withAttribute('originalRequest', $originalRequest);
+
         return $this->application->handle($request);
     }
 

diff --git a/typo3/sysext/felogin/Classes/Controller/LoginController.php b/typo3/sysext/felogin/Classes/Controller/LoginController.php
@@ -33,9 +33,7 @@
 use TYPO3\CMS\FrontendLogin\Event\LogoutConfirmedEvent;
 use TYPO3\CMS\FrontendLogin\Event\ModifyLoginFormViewEvent;
 use TYPO3\CMS\FrontendLogin\Redirect\RedirectHandler;
-use TYPO3\CMS\FrontendLogin\Redirect\RedirectMode;
 use TYPO3\CMS\FrontendLogin\Service\UserService;
-use TYPO3\CMS\FrontendLogin\Validation\RedirectUrlValidator;
 
 /**
  * Used for plugin login
@@ -56,7 +54,6 @@ class LoginController extends ActionController
     public function __construct(
         protected RedirectHandler $redirectHandler,
         protected UserService $userService,
-        protected RedirectUrlValidator $redirectUrlValidator,
         protected Context $context,
         protected readonly PageRepository $pageRepository
     ) {
@@ -111,7 +108,7 @@ public function loginAction(): ResponseInterface
                 'permaloginStatus' => $this->getPermaloginStatus(),
                 'redirectURL' => $this->redirectHandler->getLoginFormRedirectUrl($this->request, $this->configuration, $this->isRedirectDisabled()),
                 'redirectReferrer' => $this->request->hasArgument('redirectReferrer') ? (string)$this->request->getArgument('redirectReferrer') : '',
-                'referer' => $this->getReferrerForLoginForm(),
+                'referer' => $this->redirectHandler->getReferrerForLoginForm($this->request, $this->settings),
                 'noRedirect' => $this->isRedirectDisabled(),
                 'requestToken' => RequestToken::create('core/user-auth/fe')
                     ->withMergedParams(['pid' => implode(',', $storagePageIds)]),
@@ -170,37 +167,6 @@ public function logoutAction(int $redirectPageLogout = 0): ResponseInterface
         return $this->htmlResponse();
     }
 
-    /**
-     * Determines the `referer` variable used in the login form for loginMode=referer depending on the
-     * following evaluation order:
-     *
-     * - HTTP POST parameter `referer`
-     * - HTTP GET parameter `referer`
-     * - HTTP_REFERER
-     *
-     * The evaluated `referer` is only returned, if it is considered as valid.
-     */
-    protected function getReferrerForLoginForm(): string
-    {
-        // Early return, if redirectMode is not configured to respect the referrer
-        if (!$this->isReferrerRedirectEnabled()) {
-            return '';
-        }
-
-        $referrer = (string)(
-            $this->request->getParsedBody()['referer'] ??
-            $this->request->getQueryParams()['referer'] ??
-            $this->request->getServerParams()['HTTP_REFERER'] ??
-            ''
-        );
-
-        if ($this->redirectUrlValidator->isValid($this->request, $referrer)) {
-            return $referrer;
-        }
-
-        return '';
-    }
-
     /**
      * Handles the redirect when $this->redirectUrl is not empty
      */
@@ -250,16 +216,6 @@ protected function isPermaloginDisabled(int $permaLogin): bool
                || $GLOBALS['TYPO3_CONF_VARS']['FE']['lifetime'] === 0;
     }
 
-    /**
-     * Returns, if redirect based on the referrer is enabled
-     */
-    protected function isReferrerRedirectEnabled(): bool
-    {
-        $referrerRedirectModes = [RedirectMode::REFERRER, RedirectMode::REFERRER_DOMAINS];
-        $configuredRedirectModes = GeneralUtility::trimExplode(',', $this->settings['redirectMode'] ?? '');
-        return count(array_intersect($configuredRedirectModes, $referrerRedirectModes)) > 0;
-    }
-
     /**
      * Redirect to overview on login successful and setting showLogoutFormAfterLogin disabled
      */

diff --git a/typo3/sysext/felogin/Classes/Redirect/RedirectHandler.php b/typo3/sysext/felogin/Classes/Redirect/RedirectHandler.php
@@ -19,6 +19,7 @@
 
 use TYPO3\CMS\Core\Authentication\LoginType;
 use TYPO3\CMS\Core\Context\Context;
+use TYPO3\CMS\Core\Utility\GeneralUtility;
 use TYPO3\CMS\Extbase\Mvc\RequestInterface;
 use TYPO3\CMS\FrontendLogin\Configuration\RedirectConfiguration;
 use TYPO3\CMS\FrontendLogin\Validation\RedirectUrlValidator;
@@ -180,6 +181,53 @@ public function getLoginFormRedirectUrl(
         return '';
     }
 
+    /**
+     * Determines the `referer` variable used in the login form for loginMode=referer depending on the
+     * following evaluation order:
+     *
+     * - HTTP POST parameter `referer`
+     * - HTTP GET parameter `referer`
+     * - HTTP_REFERER
+     * - URL of initiating request in case plugin has been called via sub-request
+     *
+     * The evaluated `referer` is only returned, if it is considered valid.
+     */
+    public function getReferrerForLoginForm(RequestInterface $request, array $settings): string
+    {
+        // Early return, if redirectMode is not configured to respect the referrer
+        if (!$this->isReferrerRedirectEnabled($settings)) {
+            return '';
+        }
+
+        $referrer = (string)(
+            $request->getParsedBody()['referer'] ??
+            $request->getQueryParams()['referer'] ??
+            $request->getServerParams()['HTTP_REFERER'] ??
+            ''
+        );
+
+        // If the current request was initiated via sub-request, we use the URI of the original request as referrer
+        if ($originalRequest = $request->getAttribute('originalRequest', false)) {
+            $referrer = (string)$originalRequest->getUri();
+        }
+
+        if ($this->redirectUrlValidator->isValid($request, $referrer)) {
+            return $referrer;
+        }
+
+        return '';
+    }
+
+    /**
+     * Returns whether redirect based on the referrer is enabled
+     */
+    protected function isReferrerRedirectEnabled(array $settings): bool
+    {
+        $referrerRedirectModes = [RedirectMode::REFERRER, RedirectMode::REFERRER_DOMAINS];
+        $configuredRedirectModes = GeneralUtility::trimExplode(',', $settings['redirectMode'] ?? '');
+        return count(array_intersect($configuredRedirectModes, $referrerRedirectModes)) > 0;
+    }
+
     /**
      * Returns the redirect Url that should be used in logout form
      */

diff --git a/typo3/sysext/felogin/Tests/Unit/Redirect/RedirectHandlerTest.php b/typo3/sysext/felogin/Tests/Unit/Redirect/RedirectHandlerTest.php
@@ -29,6 +29,7 @@
 use TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController;
 use TYPO3\CMS\FrontendLogin\Configuration\RedirectConfiguration;
 use TYPO3\CMS\FrontendLogin\Redirect\RedirectHandler;
+use TYPO3\CMS\FrontendLogin\Redirect\RedirectMode;
 use TYPO3\CMS\FrontendLogin\Redirect\RedirectModeHandler;
 use TYPO3\CMS\FrontendLogin\Validation\RedirectUrlValidator;
 use TYPO3\TestingFramework\Core\Unit\UnitTestCase;
@@ -206,4 +207,71 @@ public function getLoginFormRedirectUrlReturnsExpectedValue(
         $configuration = RedirectConfiguration::fromSettings(['redirectMode' => $redirectMode]);
         self::assertEquals($expected, $this->subject->getLoginFormRedirectUrl($request, $configuration, $redirectDisabled));
     }
+
+    /**
+     * @test
+     */
+    public function getReferrerForLoginFormReturnsEmptyStringIfRedirectModeReferrerDisabled(): void
+    {
+        $serverRequest = (new ServerRequest())->withAttribute('extbase', new ExtbaseRequestParameters());
+        $request = new Request($serverRequest);
+        $settings = ['redirectMode' => RedirectMode::LOGIN];
+        self::assertEquals('', $this->subject->getReferrerForLoginForm($request, $settings));
+    }
+
+    /**
+     * @test
+     */
+    public function getReferrerForLoginFormReturnsReferrerGetParameter(): void
+    {
+        $expectedReferrer = 'https://example.com/page-referrer';
+        $serverRequest = (new ServerRequest())->withAttribute('extbase', new ExtbaseRequestParameters())
+            ->withQueryParams(['referer' => $expectedReferrer]);
+        $request = new Request($serverRequest);
+        $this->redirectUrlValidator->expects(self::once())->method('isValid')->with($request, $expectedReferrer)->willReturn(true);
+        $settings = ['redirectMode' => RedirectMode::REFERRER];
+        self::assertEquals($expectedReferrer, $this->subject->getReferrerForLoginForm($request, $settings));
+    }
+
+    /**
+     * @test
+     */
+    public function getReferrerForLoginFormReturnsReferrerPostParameter(): void
+    {
+        $expectedReferrer = 'https://example.com/page-referrer';
+        $serverRequest = (new ServerRequest())->withAttribute('extbase', new ExtbaseRequestParameters())
+            ->withParsedBody(['referer' => $expectedReferrer]);
+        $request = new Request($serverRequest);
+        $this->redirectUrlValidator->expects(self::once())->method('isValid')->with($request, $expectedReferrer)->willReturn(true);
+        $settings = ['redirectMode' => RedirectMode::REFERRER];
+        self::assertEquals($expectedReferrer, $this->subject->getReferrerForLoginForm($request, $settings));
+    }
+
+    /**
+     * @test
+     */
+    public function getReferrerForLoginFormReturnsHttpReferrerParameter(): void
+    {
+        $expectedReferrer = 'https://example.com/page-referrer';
+        $serverRequest = (new ServerRequest('/login', 'GET', 'php://input', [], ['HTTP_REFERER' => $expectedReferrer]))
+            ->withAttribute('extbase', new ExtbaseRequestParameters());
+        $request = new Request($serverRequest);
+        $this->redirectUrlValidator->expects(self::once())->method('isValid')->with($request, $expectedReferrer)->willReturn(true);
+        $settings = ['redirectMode' => RedirectMode::REFERRER];
+        self::assertEquals($expectedReferrer, $this->subject->getReferrerForLoginForm($request, $settings));
+    }
+
+    /**
+     * @test
+     */
+    public function getReferrerForLoginFormReturnsOriginalRequestUrlIfCalledBySubRequest(): void
+    {
+        $expectedReferrer = 'https://example.com/original-page';
+        $serverRequest = (new ServerRequest())->withAttribute('extbase', new ExtbaseRequestParameters())
+            ->withAttribute('originalRequest', new ServerRequest($expectedReferrer));
+        $request = new Request($serverRequest);
+        $this->redirectUrlValidator->expects(self::once())->method('isValid')->with($request, $expectedReferrer)->willReturn(true);
+        $settings = ['redirectMode' => RedirectMode::REFERRER];
+        self::assertEquals($expectedReferrer, $this->subject->getReferrerForLoginForm($request, $settings));
+    }
 }