Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Render the Contao login page even if the current route is not a Contao page #7077

Merged
merged 14 commits into from
May 23, 2024
36 changes: 27 additions & 9 deletions core-bundle/src/EventListener/PrettyErrorScreenListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
use Contao\PageModel;
use Contao\StringUtil;
use Symfony\Component\HttpFoundation\AcceptHeader;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\Event\ExceptionEvent;
use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException;
Expand All @@ -30,6 +31,7 @@
use Symfony\Component\HttpKernel\Exception\ServiceUnavailableHttpException;
use Symfony\Component\HttpKernel\Exception\UnauthorizedHttpException;
use Symfony\Component\HttpKernel\HttpKernelInterface;
use Symfony\Component\Routing\Matcher\RequestMatcherInterface;
use Symfony\Component\Security\Core\Exception\AuthenticationCredentialsNotFoundException;
use Symfony\Component\Security\Core\Security;
use Twig\Environment;
Expand All @@ -46,15 +48,17 @@ class PrettyErrorScreenListener
private Security $security;
private PageRegistry $pageRegistry;
private HttpKernelInterface $httpKernel;
private RequestMatcherInterface $requestMatcher;

public function __construct(bool $prettyErrorScreens, Environment $twig, ContaoFramework $framework, Security $security, PageRegistry $pageRegistry, HttpKernelInterface $httpKernel)
public function __construct(bool $prettyErrorScreens, Environment $twig, ContaoFramework $framework, Security $security, PageRegistry $pageRegistry, HttpKernelInterface $httpKernel, RequestMatcherInterface $requestMatcher)
{
$this->prettyErrorScreens = $prettyErrorScreens;
$this->twig = $twig;
$this->framework = $framework;
$this->security = $security;
$this->pageRegistry = $pageRegistry;
$this->httpKernel = $httpKernel;
$this->requestMatcher = $requestMatcher;
}

/**
Expand Down Expand Up @@ -146,14 +150,7 @@ private function renderErrorScreenByType(int $type, ExceptionEvent $event): void
$this->framework->initialize(true);

$request = $event->getRequest();
$pageModel = $request->attributes->get('pageModel');

if (!$pageModel instanceof PageModel) {
return;
}

$pageAdapter = $this->framework->getAdapter(PageModel::class);
$errorPage = $pageAdapter->findFirstPublishedByTypeAndPid('error_'.$type, $pageModel->loadDetails()->rootId);
$errorPage = $this->findErrorPage($type, $request);

if (null === $errorPage) {
return;
Expand Down Expand Up @@ -250,4 +247,25 @@ private function getStatusCodeForException(\Throwable $exception): int

return 500;
}

private function findErrorPage(int $type, Request $request): PageModel|null
{
$pageModel = $request->attributes->get('pageModel');

if (!$pageModel instanceof PageModel) {
$rootRequest = Request::create('http://'.$request->getHost());
$rootRequest->headers->set('Accept-Language', $rootRequest->headers->get('Accept-Language'));
$parameters = $this->requestMatcher->matchRequest($request);
fritzmg marked this conversation as resolved.
Show resolved Hide resolved

if (($parameters['pageModel'] ?? null) instanceof PageModel) {
$pageModel = $parameters['pageModel'];
} else {
return null;
}
}

$pageAdapter = $this->framework->getAdapter(PageModel::class);

return $pageAdapter->findFirstPublishedByTypeAndPid('error_'.$type, $pageModel->loadDetails()->rootId);
}
}
1 change: 1 addition & 0 deletions core-bundle/src/Resources/config/listener.yml
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,7 @@ services:
- '@security.helper'
- '@contao.routing.page_registry'
- '@http_kernel'
- '@router'
tags:
# The priority must be higher than the one of the Twig exception listener (defaults to -128)
- { name: kernel.event_listener, priority: -96 }
Expand Down
101 changes: 81 additions & 20 deletions core-bundle/tests/EventListener/PrettyErrorScreenListenerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
use Contao\CoreBundle\Exception\PageNotFoundException;
use Contao\CoreBundle\Exception\ResponseException;
use Contao\CoreBundle\Exception\ServiceUnavailableException;
use Contao\CoreBundle\Framework\ContaoFramework;
use Contao\CoreBundle\Routing\Page\PageRegistry;
use Contao\CoreBundle\Routing\Page\PageRoute;
use Contao\CoreBundle\Tests\TestCase;
Expand All @@ -37,6 +38,7 @@
use Symfony\Component\HttpKernel\Exception\UnauthorizedHttpException;
use Symfony\Component\HttpKernel\HttpKernelInterface;
use Symfony\Component\HttpKernel\KernelInterface;
use Symfony\Component\Routing\Matcher\RequestMatcherInterface;
use Symfony\Component\Security\Core\Exception\AuthenticationCredentialsNotFoundException;
use Symfony\Component\Security\Core\Security;
use Twig\Environment;
Expand All @@ -49,7 +51,7 @@
$exception = new InternalServerErrorHttpException('', new InternalServerErrorException());
$event = $this->getResponseEvent($exception);

$listener = $this->getListener(true, true);
$listener = $this->getListener(true);
$listener($event);

$this->assertTrue($event->hasResponse());
Expand All @@ -62,6 +64,7 @@
$framework = $this->mockContaoFramework();
$pageRegistry = $this->createMock(PageRegistry::class);
$httpKernel = $this->createMock(HttpKernelInterface::class);
$requestMatcher = $this->createMock(RequestMatcherInterface::class);

$security = $this->createMock(Security::class);
$security
Expand All @@ -73,7 +76,7 @@
$exception = new InternalServerErrorHttpException('', new InternalServerErrorException());
$event = $this->getResponseEvent($exception);

$listener = new PrettyErrorScreenListener(true, $twig, $framework, $security, $pageRegistry, $httpKernel);
$listener = new PrettyErrorScreenListener(true, $twig, $framework, $security, $pageRegistry, $httpKernel, $requestMatcher);
$listener($event);

$this->assertTrue($event->hasResponse());
Expand All @@ -86,6 +89,7 @@
$framework = $this->mockContaoFramework();
$pageRegistry = $this->createMock(PageRegistry::class);
$httpKernel = $this->createMock(HttpKernelInterface::class);
$requestMatcher = $this->createMock(RequestMatcherInterface::class);

$security = $this->createMock(Security::class);
$security
Expand All @@ -96,7 +100,7 @@
$exception = new InternalServerErrorHttpException('', new InternalServerErrorException());
$event = $this->getResponseEvent($exception);

$listener = new PrettyErrorScreenListener(true, $twig, $framework, $security, $pageRegistry, $httpKernel);
$listener = new PrettyErrorScreenListener(true, $twig, $framework, $security, $pageRegistry, $httpKernel, $requestMatcher);
$listener($event);

$this->assertTrue($event->hasResponse());
Expand Down Expand Up @@ -126,7 +130,38 @@

$event = $this->getResponseEvent($exception, $request);

$listener = $this->getListener(false, false, null, $errorPage, $httpKernel);
$listener = $this->getListener(false, null, $errorPage, $httpKernel);
$listener($event);

$this->assertTrue($event->hasResponse());
$this->assertSame($type, $event->getResponse()->getStatusCode());
}

/**
* @dataProvider getErrorTypes
*/
public function testFindsPageModelIfRequestHasNone(int $type, \Exception $exception): void
{
$errorPage = $this->mockPageWithProperties([
'pid' => 1,
'type' => 'error_'.$type,
'rootLanguage' => '',
]);

$rootPage = $this->mockPageWithProperties(['rootId' => 1]);

$request = $this->getRequest('frontend');

$httpKernel = $this->createMock(HttpKernelInterface::class);
$httpKernel
->expects($this->once())
->method('handle')
->willReturn(new Response('foo', $type))
;

$event = $this->getResponseEvent($exception, $request);

$listener = $this->getListener(false, null, $errorPage, $httpKernel, $rootPage);
$listener($event);

$this->assertTrue($event->hasResponse());
Expand Down Expand Up @@ -163,7 +198,7 @@
$exception = new UnauthorizedHttpException('', '', new InsufficientAuthenticationException());
$event = $this->getResponseEvent($exception, $request);

$listener = $this->getListener(false, false, null, $errorPage, $httpKernel);
$listener = $this->getListener(false, null, $errorPage, $httpKernel);
$listener($event);

$this->assertTrue($event->hasResponse());
Expand Down Expand Up @@ -192,7 +227,7 @@
$exception = new AccessDeniedHttpException('', new AccessDeniedException());
$event = $this->getResponseEvent($exception, $request);

$listener = $this->getListener(false, false, null, $errorPage, $httpKernel);
$listener = $this->getListener(false, null, $errorPage, $httpKernel);
$listener($event);

$this->assertTrue($event->hasResponse());
Expand Down Expand Up @@ -222,7 +257,7 @@
$exception = new AccessDeniedHttpException('', new AccessDeniedException());
$event = $this->getResponseEvent($exception, $request);

$listener = $this->getListener(false, false, null, $errorPage, $httpKernel);
$listener = $this->getListener(false, null, $errorPage, $httpKernel);
$listener($event);

$this->assertFalse($event->hasResponse());
Expand All @@ -247,9 +282,11 @@
$event = $this->getResponseEvent($exception, $this->getRequest('frontend'));

$twig = $this->createMock(Environment::class);
$framework = $this->mockContaoFramework();
$adapters = [PageModel::class => $this->mockAdapter(['findFirstPublishedRootByHostAndLanguage'])];
$framework = $this->mockContaoFramework($adapters);
$pageRegistry = $this->createMock(PageRegistry::class);
$httpKernel = $this->createMock(HttpKernelInterface::class);
$requestMatcher = $this->createMock(RequestMatcherInterface::class);

$security = $this->createMock(Security::class);
$security
Expand All @@ -258,7 +295,7 @@
->willReturn(false)
;

$listener = new PrettyErrorScreenListener(false, $twig, $framework, $security, $pageRegistry, $httpKernel);
$listener = new PrettyErrorScreenListener(false, $twig, $framework, $security, $pageRegistry, $httpKernel, $requestMatcher);
$listener($event);

$this->assertFalse($event->hasResponse());
Expand All @@ -270,6 +307,7 @@
$framework = $this->mockContaoFramework();
$pageRegistry = $this->createMock(PageRegistry::class);
$httpKernel = $this->createMock(HttpKernelInterface::class);
$requestMatcher = $this->createMock(RequestMatcherInterface::class);

$security = $this->createMock(Security::class);
$security
Expand All @@ -281,7 +319,7 @@
$exception = new ServiceUnavailableHttpException(null, '', new ServiceUnavailableException(''));
$event = $this->getResponseEvent($exception, null, true);

$listener = new PrettyErrorScreenListener(true, $twig, $framework, $security, $pageRegistry, $httpKernel);
$listener = new PrettyErrorScreenListener(true, $twig, $framework, $security, $pageRegistry, $httpKernel, $requestMatcher);
$listener($event);

$this->assertFalse($event->hasResponse());
Expand All @@ -291,7 +329,7 @@
{
$event = $this->getResponseEvent(new ConflictHttpException(), $this->getRequest('frontend'));

$listener = $this->getListener(false, true);
$listener = $this->getListener(false);
$listener($event);

$this->assertTrue($event->hasResponse());
Expand All @@ -318,7 +356,7 @@
)
;

$listener = $this->getListener(false, true, $twig);
$listener = $this->getListener(false, $twig);
$listener($event);

$this->assertTrue($event->hasResponse());
Expand All @@ -331,6 +369,7 @@
$framework = $this->mockContaoFramework();
$pageRegistry = $this->createMock(PageRegistry::class);
$httpKernel = $this->createMock(HttpKernelInterface::class);
$requestMatcher = $this->createMock(RequestMatcherInterface::class);

$security = $this->createMock(Security::class);
$security
Expand All @@ -342,7 +381,7 @@
$exception = new InternalServerErrorHttpException('', new InsecureInstallationException());
$event = $this->getResponseEvent($exception, $this->getRequest('frontend', 'json'));

$listener = new PrettyErrorScreenListener(true, $twig, $framework, $security, $pageRegistry, $httpKernel);
$listener = new PrettyErrorScreenListener(true, $twig, $framework, $security, $pageRegistry, $httpKernel, $requestMatcher);
$listener($event);

$this->assertFalse($event->hasResponse());
Expand All @@ -357,6 +396,7 @@
$framework = $this->mockContaoFramework();
$pageRegistry = $this->createMock(PageRegistry::class);
$httpKernel = $this->createMock(HttpKernelInterface::class);
$requestMatcher = $this->createMock(RequestMatcherInterface::class);

$security = $this->createMock(Security::class);
$security
Expand All @@ -368,7 +408,7 @@
$exception = new InternalServerErrorHttpException('', new InsecureInstallationException());
$event = $this->getResponseEvent($exception, $this->getRequest('backend', 'html', 'application/json'));

$listener = new PrettyErrorScreenListener(true, $twig, $framework, $security, $pageRegistry, $httpKernel);
$listener = new PrettyErrorScreenListener(true, $twig, $framework, $security, $pageRegistry, $httpKernel, $requestMatcher);
$listener($event);

$this->assertFalse($event->hasResponse());
Expand Down Expand Up @@ -397,16 +437,31 @@
$this->assertSame(500, $event->getResponse()->getStatusCode());
}

private function getListener(bool $isBackendUser = false, bool $expectLogging = false, Environment $twig = null, PageModel $errorPage = null, HttpKernelInterface $httpKernel = null): PrettyErrorScreenListener
protected function mockContaoFramework(array $adapters = []): ContaoFramework
{
if (!isset($adapters[PageModel::class])) {
$pageAdapter = $this->mockAdapter(['findFirstPublishedRootByHostAndLanguage']);
$pageAdapter
->method('findFirstPublishedRootByHostAndLanguage')
->willReturn(null)
;

$adapters[PageModel::class] = $pageAdapter;
}

return parent::mockContaoFramework($adapters);
}

private function getListener(bool $isBackendUser = false, Environment $twig = null, PageModel $errorPage = null, HttpKernelInterface $httpKernel = null, PageModel $rootPage = null): PrettyErrorScreenListener
{
$twig ??= $this->createMock(Environment::class);
$httpKernel ??= $this->createMock(HttpKernelInterface::class);
$requestMatcher ??= $this->createMock(RequestMatcherInterface::class);

Check failure on line 459 in core-bundle/tests/EventListener/PrettyErrorScreenListenerTest.php

View workflow job for this annotation

GitHub Actions / PHPStan

Variable $requestMatcher on left side of ??= is never defined.

$adapters = [];
$pageRegistry = $this->createMock(PageRegistry::class);
$pageAdapter = $this->mockAdapter(['findFirstPublishedByTypeAndPid']);

if (null !== $errorPage) {
$pageAdapter = $this->mockAdapter(['findFirstPublishedByTypeAndPid']);
$pageAdapter
->expects($this->once())
->method('findFirstPublishedByTypeAndPid')
Expand All @@ -420,11 +475,17 @@
->with($errorPage)
->willReturn(new PageRoute($errorPage))
;
}

$adapters[PageModel::class] = $pageAdapter;
if (null !== $rootPage) {
$requestMatcher
->expects($this->once())
->method('matchRequest')
->willReturn(['pageModel' => $rootPage])
;
}

$framework = $this->mockContaoFramework($adapters);
$framework = $this->mockContaoFramework([PageModel::class => $pageAdapter]);

$security = $this->createMock(Security::class);
$security
Expand All @@ -433,7 +494,7 @@
->willReturn($isBackendUser)
;

return new PrettyErrorScreenListener(true, $twig, $framework, $security, $pageRegistry, $httpKernel);
return new PrettyErrorScreenListener(true, $twig, $framework, $security, $pageRegistry, $httpKernel, $requestMatcher);
}

private function getRequest(string $scope = 'backend', string $format = 'html', string $accept = 'text/html'): Request
Expand Down