.../CustomPageExceptionHtmlSubscriber.php | 48 ++++++++++++++++++---- .../system/src/Tests/System/AccessDeniedTest.php | 22 +++++++--- .../system/src/Tests/System/PageNotFoundTest.php | 22 ++++++++-- .../src/Controller/SystemTestController.php | 10 +++++ .../modules/system_test/system_test.routing.yml | 16 ++++++++ .../CustomPageExceptionHtmlSubscriberTest.php | 10 ++++- 6 files changed, 110 insertions(+), 18 deletions(-) diff --git a/core/lib/Drupal/Core/EventSubscriber/CustomPageExceptionHtmlSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/CustomPageExceptionHtmlSubscriber.php index af8f825..e306b38 100644 --- a/core/lib/Drupal/Core/EventSubscriber/CustomPageExceptionHtmlSubscriber.php +++ b/core/lib/Drupal/Core/EventSubscriber/CustomPageExceptionHtmlSubscriber.php @@ -8,7 +8,10 @@ namespace Drupal\Core\EventSubscriber; use Drupal\Core\Access\AccessManagerInterface; +use Drupal\Core\Cache\CacheableDependencyInterface; +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; @@ -71,10 +74,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)) { - $url = Url::fromUserInput($custom_403_path); - if ($url->isRouted() && $this->accessManager->checkNamedRoute($url->getRouteName(), $url->getRouteParameters())) { - $this->makeSubrequest($event, $custom_403_path, Response::HTTP_FORBIDDEN); - } + $this->makeSubrequestToCustomPath($event, $custom_403_path, Response::HTTP_FORBIDDEN); } } @@ -84,11 +84,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)) { - $url = Url::fromUserInput($custom_404_path); - if ($url->isRouted() && $this->accessManager->checkNamedRoute($url->getRouteName(), $url->getRouteParameters())) { - $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/modules/system/src/Tests/System/AccessDeniedTest.php b/core/modules/system/src/Tests/System/AccessDeniedTest.php index a0747d5..c013525 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', 'node']; + public static $modules = ['block', 'node', 'system_test']; protected $adminUser; @@ -34,12 +34,14 @@ 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')); } - function testAccessDenied() { + function atestAccessDenied() { $this->drupalGet('admin'); $this->assertText(t('Access denied'), 'Found the default 403 page'); $this->assertResponse(403); @@ -112,11 +114,21 @@ function testAccessDenied() { public function testAccessDeniedCustomPageWithAccessDenied() { // Sets up a 403 page not accessible by the anonymous user. - $this->config('system.site')->set('page.403', '/admin/index')->save(); + $this->config('system.site')->set('page.403', '/system-test/custom-4xx')->save(); - $this->drupalGet('/admin'); - $this->assertNoText('Administration'); + $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 7456856..17ad8d7 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')); @@ -53,18 +63,22 @@ function testPageNotFound() { public function testPageNotFoundCustomPageWithAccessDenied() { // Sets up a 404 page not accessible by the anonymous user. - $this->config('system.site')->set('page.404', '/admin/index')->save(); + $this->config('system.site')->set('page.404', '/system-test/custom-4xx')->save(); $this->drupalGet('/this-path-does-not-exist'); - $this->assertNoText('Administration'); + $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->rootUser); + $this->drupalLogin($this->adminUser); $this->drupalGet('/this-path-does-not-exist'); - $this->assertText('Administration'); + $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/src/Controller/SystemTestController.php b/core/modules/system/tests/modules/system_test/src/Controller/SystemTestController.php index f171efd..467426a 100644 --- a/core/modules/system/tests/modules/system_test/src/Controller/SystemTestController.php +++ b/core/modules/system/tests/modules/system_test/src/Controller/SystemTestController.php @@ -19,6 +19,7 @@ use Symfony\Component\HttpFoundation\Response; use Drupal\Core\Lock\LockBackendInterface; use Symfony\Component\DependencyInjection\ContainerInterface; +use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; /** * Controller routines for system_test routes. @@ -353,4 +354,13 @@ public function getCurrentDate() { return $response; } + /** + * Always denies access. + * + * @throws \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException + */ + public function alwaysDenied() { + throw new AccessDeniedHttpException(); + } + } 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 a933048..3ce845f 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: '\Drupal\system_test\Controller\SystemTestController::alwaysDenied' + requirements: + _access: 'TRUE' diff --git a/core/tests/Drupal/Tests/Core/EventSubscriber/CustomPageExceptionHtmlSubscriberTest.php b/core/tests/Drupal/Tests/Core/EventSubscriber/CustomPageExceptionHtmlSubscriberTest.php index 6cdfde1..e931a0c 100644 --- a/core/tests/Drupal/Tests/Core/EventSubscriber/CustomPageExceptionHtmlSubscriberTest.php +++ b/core/tests/Drupal/Tests/Core/EventSubscriber/CustomPageExceptionHtmlSubscriberTest.php @@ -8,8 +8,11 @@ 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; @@ -99,7 +102,7 @@ protected function setUp() { $this->accessManager = $this->getMock('Drupal\Core\Access\AccessManagerInterface'); $this->accessManager->expects($this->any()) ->method('checkNamedRoute') - ->willReturn(TRUE); + ->willReturn(AccessResult::allowed()->addCacheTags(['foo', 'bar'])); $this->customPageSubscriber = new CustomPageExceptionHtmlSubscriber($this->configFactory, $this->kernel, $this->logger, $this->redirectDestination, $this->accessUnawareRouter, $this->accessManager); @@ -130,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')); @@ -140,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)); } /** @@ -147,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())); @@ -158,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)); } }