diff --git a/core/core.services.yml b/core/core.services.yml index 62a0f3e6923d1c6fe7beabbd4341e18e36aac8f0..7c1e473ac49f03a3790c8bd1424cd1d8712c7ad8 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -1175,7 +1175,7 @@ services: class: Drupal\Core\EventSubscriber\CustomPageExceptionHtmlSubscriber tags: - { name: event_subscriber } - arguments: ['@config.factory', '@http_kernel', '@logger.channel.php', '@redirect.destination', '@router.no_access_checks'] + arguments: ['@config.factory', '@http_kernel', '@logger.channel.php', '@redirect.destination', '@router.no_access_checks', '@access_manager'] exception.fast_404_html: class: Drupal\Core\EventSubscriber\Fast404ExceptionHtmlSubscriber tags: diff --git a/core/lib/Drupal/Core/EventSubscriber/CustomPageExceptionHtmlSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/CustomPageExceptionHtmlSubscriber.php index ce5697065cb919a5bdca789511bda59cbdb341cd..623ba09e750c1e86355aa83cd9963b78fca83859 100644 --- a/core/lib/Drupal/Core/EventSubscriber/CustomPageExceptionHtmlSubscriber.php +++ b/core/lib/Drupal/Core/EventSubscriber/CustomPageExceptionHtmlSubscriber.php @@ -7,8 +7,12 @@ namespace Drupal\Core\EventSubscriber; +use Drupal\Core\Access\AccessManagerInterface; +use Drupal\Core\Cache\RefinableCacheableDependencyInterface; use Drupal\Core\Config\ConfigFactoryInterface; +use Drupal\Core\Routing\AccessAwareRouterInterface; use Drupal\Core\Routing\RedirectDestinationInterface; +use Drupal\Core\Url; use Psr\Log\LoggerInterface; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent; @@ -27,6 +31,13 @@ class CustomPageExceptionHtmlSubscriber extends DefaultExceptionHtmlSubscriber { */ protected $configFactory; + /** + * The access manager. + * + * @var \Drupal\Core\Access\AccessManagerInterface + */ + protected $accessManager; + /** * Constructs a new CustomPageExceptionHtmlSubscriber. * @@ -40,10 +51,13 @@ class CustomPageExceptionHtmlSubscriber extends DefaultExceptionHtmlSubscriber { * The redirect destination service. * @param \Symfony\Component\Routing\Matcher\UrlMatcherInterface $access_unaware_router * A router implementation which does not check access. + * @param \Drupal\Core\Access\AccessManagerInterface $access_manager + * The access manager. */ - public function __construct(ConfigFactoryInterface $config_factory, HttpKernelInterface $http_kernel, LoggerInterface $logger, RedirectDestinationInterface $redirect_destination, UrlMatcherInterface $access_unaware_router) { + public function __construct(ConfigFactoryInterface $config_factory, HttpKernelInterface $http_kernel, LoggerInterface $logger, RedirectDestinationInterface $redirect_destination, UrlMatcherInterface $access_unaware_router, AccessManagerInterface $access_manager) { parent::__construct($http_kernel, $logger, $redirect_destination, $access_unaware_router); $this->configFactory = $config_factory; + $this->accessManager = $access_manager; } /** @@ -59,7 +73,7 @@ protected static function getPriority() { public function on403(GetResponseForExceptionEvent $event) { $custom_403_path = $this->configFactory->get('system.site')->get('page.403'); if (!empty($custom_403_path)) { - $this->makeSubrequest($event, $custom_403_path, Response::HTTP_FORBIDDEN); + $this->makeSubrequestToCustomPath($event, $custom_403_path, Response::HTTP_FORBIDDEN); } } @@ -69,8 +83,45 @@ public function on403(GetResponseForExceptionEvent $event) { public function on404(GetResponseForExceptionEvent $event) { $custom_404_path = $this->configFactory->get('system.site')->get('page.404'); if (!empty($custom_404_path)) { - $this->makeSubrequest($event, $custom_404_path, Response::HTTP_NOT_FOUND); + $this->makeSubrequestToCustomPath($event, $custom_404_path, Response::HTTP_NOT_FOUND); } } + /** + * Makes a subrequest to retrieve the custom error page. + * + * @param \Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent $event + * The event to process. + * @param string $custom_path + * The custom path to which to make a subrequest for this error message. + * @param int $status_code + * The status code for the error being handled. + */ + protected function makeSubrequestToCustomPath(GetResponseForExceptionEvent $event, $custom_path, $status_code) { + $url = Url::fromUserInput($custom_path); + if ($url->isRouted()) { + $access_result = $this->accessManager->checkNamedRoute($url->getRouteName(), $url->getRouteParameters(), NULL, TRUE); + $request = $event->getRequest(); + + // Merge the custom path's route's access result's cacheability metadata + // with the existing one (from the master request), otherwise create it. + if (!$request->attributes->has(AccessAwareRouterInterface::ACCESS_RESULT)) { + $request->attributes->set(AccessAwareRouterInterface::ACCESS_RESULT, $access_result); + } + else { + $existing_access_result = $request->attributes->get(AccessAwareRouterInterface::ACCESS_RESULT); + if ($existing_access_result instanceof RefinableCacheableDependencyInterface) { + $existing_access_result->addCacheableDependency($access_result); + } + } + + // Only perform the subrequest if the custom path is actually accessible. + if (!$access_result->isAllowed()) { + return; + } + } + + $this->makeSubrequest($event, $custom_path, $status_code); + } + } diff --git a/core/lib/Drupal/Core/EventSubscriber/DefaultExceptionHtmlSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/DefaultExceptionHtmlSubscriber.php index bd9a6f34864c61412edec39359b960b6aa20e54c..153253aa11eecc8d29906117c2e13dead5af37f0 100644 --- a/core/lib/Drupal/Core/EventSubscriber/DefaultExceptionHtmlSubscriber.php +++ b/core/lib/Drupal/Core/EventSubscriber/DefaultExceptionHtmlSubscriber.php @@ -118,7 +118,7 @@ public function on404(GetResponseForExceptionEvent $event) { * Makes a subrequest to retrieve the default error page. * * @param \Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent $event - * The event to process + * The event to process. * @param string $url * The path/url to which to make a subrequest for this error message. * @param int $status_code diff --git a/core/modules/system/src/Tests/System/AccessDeniedTest.php b/core/modules/system/src/Tests/System/AccessDeniedTest.php index 5c6c3bf03f65dbd226cfc923fc9554cbd3ab4229..db231c38bc6a137ccd89f4e20b145ef631704e86 100644 --- a/core/modules/system/src/Tests/System/AccessDeniedTest.php +++ b/core/modules/system/src/Tests/System/AccessDeniedTest.php @@ -23,7 +23,7 @@ class AccessDeniedTest extends WebTestBase { * * @var array */ - public static $modules = ['block']; + public static $modules = ['block', 'node', 'system_test']; protected $adminUser; @@ -34,6 +34,8 @@ protected function setUp() { // Create an administrative user. $this->adminUser = $this->drupalCreateUser(['access administration pages', 'administer site configuration', 'link to any page', 'administer blocks']); + $this->adminUser->roles[] = 'administrator'; + $this->adminUser->save(); user_role_grant_permissions(RoleInterface::ANONYMOUS_ID, array('access user profiles')); user_role_grant_permissions(RoleInterface::AUTHENTICATED_ID, array('access user profiles')); @@ -109,4 +111,27 @@ function testAccessDenied() { // Check that we're still on the same page. $this->assertText(t('Basic site settings')); } + + /** + * Tests that an inaccessible custom 403 page falls back to the default. + */ + public function testAccessDeniedCustomPageWithAccessDenied() { + // Sets up a 403 page not accessible by the anonymous user. + $this->config('system.site')->set('page.403', '/system-test/custom-4xx')->save(); + + $this->drupalGet('/system-test/always-denied'); + $this->assertNoText('Admin-only 4xx response'); + $this->assertText('You are not authorized to access this page.'); + $this->assertResponse(403); + // Verify the access cacheability metadata for custom 403 is bubbled. + $this->assertCacheContext('user.roles'); + + $this->drupalLogin($this->adminUser); + $this->drupalGet('/system-test/always-denied'); + $this->assertText('Admin-only 4xx response'); + $this->assertResponse(403); + // Verify the access cacheability metadata for custom 403 is bubbled. + $this->assertCacheContext('user.roles'); + } + } diff --git a/core/modules/system/src/Tests/System/PageNotFoundTest.php b/core/modules/system/src/Tests/System/PageNotFoundTest.php index b7c060973ee47ecf43dd8a55fadb7eed33fe9931..ffe985b9e3c6f088ffa67890759822b1996674a8 100644 --- a/core/modules/system/src/Tests/System/PageNotFoundTest.php +++ b/core/modules/system/src/Tests/System/PageNotFoundTest.php @@ -17,6 +17,14 @@ * @group system */ class PageNotFoundTest extends WebTestBase { + + /** + * Modules to enable. + * + * @var array + */ + public static $modules = ['system_test']; + protected $adminUser; protected function setUp() { @@ -24,6 +32,8 @@ protected function setUp() { // Create an administrative user. $this->adminUser = $this->drupalCreateUser(array('administer site configuration', 'link to any page')); + $this->adminUser->roles[] = 'administrator'; + $this->adminUser->save(); user_role_grant_permissions(RoleInterface::ANONYMOUS_ID, array('access user profiles')); user_role_grant_permissions(RoleInterface::AUTHENTICATED_ID, array('access user profiles')); @@ -50,4 +60,28 @@ function testPageNotFound() { $this->drupalGet($this->randomMachineName(10)); $this->assertText($this->adminUser->getUsername(), 'Found the custom 404 page'); } + + /** + * Tests that an inaccessible custom 404 page falls back to the default. + */ + public function testPageNotFoundCustomPageWithAccessDenied() { + // Sets up a 404 page not accessible by the anonymous user. + $this->config('system.site')->set('page.404', '/system-test/custom-4xx')->save(); + + $this->drupalGet('/this-path-does-not-exist'); + $this->assertNoText('Admin-only 4xx response'); + $this->assertText('The requested page could not be found.'); + $this->assertResponse(404); + // Verify the access cacheability metadata for custom 404 is bubbled. + $this->assertCacheContext('user.roles'); + + $this->drupalLogin($this->adminUser); + $this->drupalGet('/this-path-does-not-exist'); + $this->assertText('Admin-only 4xx response'); + $this->assertNoText('The requested page could not be found.'); + $this->assertResponse(404); + // Verify the access cacheability metadata for custom 404 is bubbled. + $this->assertCacheContext('user.roles'); + } + } diff --git a/core/modules/system/tests/modules/system_test/system_test.routing.yml b/core/modules/system/tests/modules/system_test/system_test.routing.yml index a933048daaf72ffe8960b55abf3b6d854f897762..a91e83b1e3e4ee02867848a552921499bc791068 100644 --- a/core/modules/system/tests/modules/system_test/system_test.routing.yml +++ b/core/modules/system/tests/modules/system_test/system_test.routing.yml @@ -159,3 +159,19 @@ system_test.date: no_cache: 'TRUE' requirements: _access: 'TRUE' + +system_test.custom_4xx_with_limited_access: + path: '/system-test/custom-4xx' + defaults: + _title: 'Admin-only 4xx response' + _controller: '\Drupal\system_test\Controller\SystemTestController::mainContentFallback' + requirements: + _role: 'administrator' + +system_test.always_denied: + path: '/system-test/always-denied' + defaults: + _title: 'Always denied' + _controller: 'chop' + requirements: + _access: 'FALSE' diff --git a/core/tests/Drupal/Tests/Core/EventSubscriber/CustomPageExceptionHtmlSubscriberTest.php b/core/tests/Drupal/Tests/Core/EventSubscriber/CustomPageExceptionHtmlSubscriberTest.php index cdcf5b4a6c9d1f6d7bd279791755baab56d9be39..e931a0c3adb4adcd984ac9dbe407526f55054fbc 100644 --- a/core/tests/Drupal/Tests/Core/EventSubscriber/CustomPageExceptionHtmlSubscriberTest.php +++ b/core/tests/Drupal/Tests/Core/EventSubscriber/CustomPageExceptionHtmlSubscriberTest.php @@ -8,7 +8,12 @@ namespace Drupal\Tests\Core\EventSubscriber; use Drupal\Component\Utility\UrlHelper; +use Drupal\Core\Access\AccessResult; +use Drupal\Core\DependencyInjection\ContainerBuilder; use Drupal\Core\EventSubscriber\CustomPageExceptionHtmlSubscriber; +use Drupal\Core\Render\HtmlResponse; +use Drupal\Core\Routing\AccessAwareRouterInterface; +use Drupal\Core\Url; use Drupal\Tests\UnitTestCase; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; @@ -69,6 +74,13 @@ class CustomPageExceptionHtmlSubscriberTest extends UnitTestCase { */ protected $accessUnawareRouter; + /** + * The access manager. + * + * @var \Drupal\Core\Access\AccessManagerInterface + */ + protected $accessManager; + /** * {@inheritdoc} */ @@ -87,8 +99,20 @@ protected function setUp() { ->willReturn([ '_controller' => 'mocked', ]); + $this->accessManager = $this->getMock('Drupal\Core\Access\AccessManagerInterface'); + $this->accessManager->expects($this->any()) + ->method('checkNamedRoute') + ->willReturn(AccessResult::allowed()->addCacheTags(['foo', 'bar'])); + + $this->customPageSubscriber = new CustomPageExceptionHtmlSubscriber($this->configFactory, $this->kernel, $this->logger, $this->redirectDestination, $this->accessUnawareRouter, $this->accessManager); - $this->customPageSubscriber = new CustomPageExceptionHtmlSubscriber($this->configFactory, $this->kernel, $this->logger, $this->redirectDestination, $this->accessUnawareRouter); + $path_validator = $this->getMock('Drupal\Core\Path\PathValidatorInterface'); + $path_validator->expects($this->any()) + ->method('getUrlIfValidWithoutAccessCheck') + ->willReturn(Url::fromRoute('foo', ['foo' => 'bar'])); + $container = new ContainerBuilder(); + $container->set('path.validator', $path_validator); + \Drupal::setContainer($container); // You can't create an exception in PHP without throwing it. Store the // current error_log, and disable it temporarily. @@ -109,7 +133,7 @@ public function testHandleWithPostRequest() { $request = Request::create('/test', 'POST', array('name' => 'druplicon', 'pass' => '12345')); $this->kernel->expects($this->once())->method('handle')->will($this->returnCallback(function (Request $request) { - return new Response($request->getMethod()); + return new HtmlResponse($request->getMethod()); })); $event = new GetResponseForExceptionEvent($this->kernel, $request, 'foo', new NotFoundHttpException('foo')); @@ -119,6 +143,7 @@ public function testHandleWithPostRequest() { $response = $event->getResponse(); $result = $response->getContent() . " " . UrlHelper::buildQuery($request->request->all()); $this->assertEquals('POST name=druplicon&pass=12345', $result); + $this->assertEquals(AccessResult::allowed()->addCacheTags(['foo', 'bar']), $request->attributes->get(AccessAwareRouterInterface::ACCESS_RESULT)); } /** @@ -126,6 +151,7 @@ public function testHandleWithPostRequest() { */ public function testHandleWithGetRequest() { $request = Request::create('/test', 'GET', array('name' => 'druplicon', 'pass' => '12345')); + $request->attributes->set(AccessAwareRouterInterface::ACCESS_RESULT, AccessResult::forbidden()->addCacheTags(['druplicon'])); $this->kernel->expects($this->once())->method('handle')->will($this->returnCallback(function (Request $request) { return new Response($request->getMethod() . ' ' . UrlHelper::buildQuery($request->query->all())); @@ -137,6 +163,7 @@ public function testHandleWithGetRequest() { $response = $event->getResponse(); $result = $response->getContent() . " " . UrlHelper::buildQuery($request->request->all()); $this->assertEquals('GET name=druplicon&pass=12345&destination=test&_exception_statuscode=404 ', $result); + $this->assertEquals(AccessResult::forbidden()->addCacheTags(['druplicon', 'foo', 'bar']), $request->attributes->get(AccessAwareRouterInterface::ACCESS_RESULT)); } }