Commit ba7a18d4 authored by alexpott's avatar alexpott
Browse files

Issue #2678674 by Wim Leers, benjy, mr.baileys, dawehner, xjm, mlhess: Access...

Issue #2678674 by Wim Leers, benjy, mr.baileys, dawehner, xjm, mlhess: Access bypass to unpublished custom error pages
parent 688e42fd
...@@ -1175,7 +1175,7 @@ services: ...@@ -1175,7 +1175,7 @@ services:
class: Drupal\Core\EventSubscriber\CustomPageExceptionHtmlSubscriber class: Drupal\Core\EventSubscriber\CustomPageExceptionHtmlSubscriber
tags: tags:
- { name: event_subscriber } - { 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: exception.fast_404_html:
class: Drupal\Core\EventSubscriber\Fast404ExceptionHtmlSubscriber class: Drupal\Core\EventSubscriber\Fast404ExceptionHtmlSubscriber
tags: tags:
......
...@@ -7,8 +7,12 @@ ...@@ -7,8 +7,12 @@
namespace Drupal\Core\EventSubscriber; namespace Drupal\Core\EventSubscriber;
use Drupal\Core\Access\AccessManagerInterface;
use Drupal\Core\Cache\RefinableCacheableDependencyInterface;
use Drupal\Core\Config\ConfigFactoryInterface; use Drupal\Core\Config\ConfigFactoryInterface;
use Drupal\Core\Routing\AccessAwareRouterInterface;
use Drupal\Core\Routing\RedirectDestinationInterface; use Drupal\Core\Routing\RedirectDestinationInterface;
use Drupal\Core\Url;
use Psr\Log\LoggerInterface; use Psr\Log\LoggerInterface;
use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent; use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent;
...@@ -27,6 +31,13 @@ class CustomPageExceptionHtmlSubscriber extends DefaultExceptionHtmlSubscriber { ...@@ -27,6 +31,13 @@ class CustomPageExceptionHtmlSubscriber extends DefaultExceptionHtmlSubscriber {
*/ */
protected $configFactory; protected $configFactory;
/**
* The access manager.
*
* @var \Drupal\Core\Access\AccessManagerInterface
*/
protected $accessManager;
/** /**
* Constructs a new CustomPageExceptionHtmlSubscriber. * Constructs a new CustomPageExceptionHtmlSubscriber.
* *
...@@ -40,10 +51,13 @@ class CustomPageExceptionHtmlSubscriber extends DefaultExceptionHtmlSubscriber { ...@@ -40,10 +51,13 @@ class CustomPageExceptionHtmlSubscriber extends DefaultExceptionHtmlSubscriber {
* The redirect destination service. * The redirect destination service.
* @param \Symfony\Component\Routing\Matcher\UrlMatcherInterface $access_unaware_router * @param \Symfony\Component\Routing\Matcher\UrlMatcherInterface $access_unaware_router
* A router implementation which does not check access. * 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); parent::__construct($http_kernel, $logger, $redirect_destination, $access_unaware_router);
$this->configFactory = $config_factory; $this->configFactory = $config_factory;
$this->accessManager = $access_manager;
} }
/** /**
...@@ -59,7 +73,7 @@ protected static function getPriority() { ...@@ -59,7 +73,7 @@ protected static function getPriority() {
public function on403(GetResponseForExceptionEvent $event) { public function on403(GetResponseForExceptionEvent $event) {
$custom_403_path = $this->configFactory->get('system.site')->get('page.403'); $custom_403_path = $this->configFactory->get('system.site')->get('page.403');
if (!empty($custom_403_path)) { 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) { ...@@ -69,8 +83,45 @@ public function on403(GetResponseForExceptionEvent $event) {
public function on404(GetResponseForExceptionEvent $event) { public function on404(GetResponseForExceptionEvent $event) {
$custom_404_path = $this->configFactory->get('system.site')->get('page.404'); $custom_404_path = $this->configFactory->get('system.site')->get('page.404');
if (!empty($custom_404_path)) { 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);
}
} }
...@@ -118,7 +118,7 @@ public function on404(GetResponseForExceptionEvent $event) { ...@@ -118,7 +118,7 @@ public function on404(GetResponseForExceptionEvent $event) {
* Makes a subrequest to retrieve the default error page. * Makes a subrequest to retrieve the default error page.
* *
* @param \Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent $event * @param \Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent $event
* The event to process * The event to process.
* @param string $url * @param string $url
* The path/url to which to make a subrequest for this error message. * The path/url to which to make a subrequest for this error message.
* @param int $status_code * @param int $status_code
......
...@@ -23,7 +23,7 @@ class AccessDeniedTest extends WebTestBase { ...@@ -23,7 +23,7 @@ class AccessDeniedTest extends WebTestBase {
* *
* @var array * @var array
*/ */
public static $modules = ['block']; public static $modules = ['block', 'node', 'system_test'];
protected $adminUser; protected $adminUser;
...@@ -34,6 +34,8 @@ protected function setUp() { ...@@ -34,6 +34,8 @@ protected function setUp() {
// Create an administrative user. // Create an administrative user.
$this->adminUser = $this->drupalCreateUser(['access administration pages', 'administer site configuration', 'link to any page', 'administer blocks']); $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::ANONYMOUS_ID, array('access user profiles'));
user_role_grant_permissions(RoleInterface::AUTHENTICATED_ID, array('access user profiles')); user_role_grant_permissions(RoleInterface::AUTHENTICATED_ID, array('access user profiles'));
...@@ -109,4 +111,27 @@ function testAccessDenied() { ...@@ -109,4 +111,27 @@ function testAccessDenied() {
// Check that we're still on the same page. // Check that we're still on the same page.
$this->assertText(t('Basic site settings')); $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');
}
} }
...@@ -17,6 +17,14 @@ ...@@ -17,6 +17,14 @@
* @group system * @group system
*/ */
class PageNotFoundTest extends WebTestBase { class PageNotFoundTest extends WebTestBase {
/**
* Modules to enable.
*
* @var array
*/
public static $modules = ['system_test'];
protected $adminUser; protected $adminUser;
protected function setUp() { protected function setUp() {
...@@ -24,6 +32,8 @@ protected function setUp() { ...@@ -24,6 +32,8 @@ protected function setUp() {
// Create an administrative user. // Create an administrative user.
$this->adminUser = $this->drupalCreateUser(array('administer site configuration', 'link to any page')); $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::ANONYMOUS_ID, array('access user profiles'));
user_role_grant_permissions(RoleInterface::AUTHENTICATED_ID, array('access user profiles')); user_role_grant_permissions(RoleInterface::AUTHENTICATED_ID, array('access user profiles'));
...@@ -50,4 +60,28 @@ function testPageNotFound() { ...@@ -50,4 +60,28 @@ function testPageNotFound() {
$this->drupalGet($this->randomMachineName(10)); $this->drupalGet($this->randomMachineName(10));
$this->assertText($this->adminUser->getUsername(), 'Found the custom 404 page'); $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');
}
} }
...@@ -159,3 +159,19 @@ system_test.date: ...@@ -159,3 +159,19 @@ system_test.date:
no_cache: 'TRUE' no_cache: 'TRUE'
requirements: requirements:
_access: 'TRUE' _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'
...@@ -8,7 +8,12 @@ ...@@ -8,7 +8,12 @@
namespace Drupal\Tests\Core\EventSubscriber; namespace Drupal\Tests\Core\EventSubscriber;
use Drupal\Component\Utility\UrlHelper; use Drupal\Component\Utility\UrlHelper;
use Drupal\Core\Access\AccessResult;
use Drupal\Core\DependencyInjection\ContainerBuilder;
use Drupal\Core\EventSubscriber\CustomPageExceptionHtmlSubscriber; use Drupal\Core\EventSubscriber\CustomPageExceptionHtmlSubscriber;
use Drupal\Core\Render\HtmlResponse;
use Drupal\Core\Routing\AccessAwareRouterInterface;
use Drupal\Core\Url;
use Drupal\Tests\UnitTestCase; use Drupal\Tests\UnitTestCase;
use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpFoundation\Response;
...@@ -69,6 +74,13 @@ class CustomPageExceptionHtmlSubscriberTest extends UnitTestCase { ...@@ -69,6 +74,13 @@ class CustomPageExceptionHtmlSubscriberTest extends UnitTestCase {
*/ */
protected $accessUnawareRouter; protected $accessUnawareRouter;
/**
* The access manager.
*
* @var \Drupal\Core\Access\AccessManagerInterface
*/
protected $accessManager;
/** /**
* {@inheritdoc} * {@inheritdoc}
*/ */
...@@ -87,8 +99,20 @@ protected function setUp() { ...@@ -87,8 +99,20 @@ protected function setUp() {
->willReturn([ ->willReturn([
'_controller' => 'mocked', '_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 // You can't create an exception in PHP without throwing it. Store the
// current error_log, and disable it temporarily. // current error_log, and disable it temporarily.
...@@ -109,7 +133,7 @@ public function testHandleWithPostRequest() { ...@@ -109,7 +133,7 @@ public function testHandleWithPostRequest() {
$request = Request::create('/test', 'POST', array('name' => 'druplicon', 'pass' => '12345')); $request = Request::create('/test', 'POST', array('name' => 'druplicon', 'pass' => '12345'));
$this->kernel->expects($this->once())->method('handle')->will($this->returnCallback(function (Request $request) { $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')); $event = new GetResponseForExceptionEvent($this->kernel, $request, 'foo', new NotFoundHttpException('foo'));
...@@ -119,6 +143,7 @@ public function testHandleWithPostRequest() { ...@@ -119,6 +143,7 @@ public function testHandleWithPostRequest() {
$response = $event->getResponse(); $response = $event->getResponse();
$result = $response->getContent() . " " . UrlHelper::buildQuery($request->request->all()); $result = $response->getContent() . " " . UrlHelper::buildQuery($request->request->all());
$this->assertEquals('POST name=druplicon&pass=12345', $result); $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() { ...@@ -126,6 +151,7 @@ public function testHandleWithPostRequest() {
*/ */
public function testHandleWithGetRequest() { public function testHandleWithGetRequest() {
$request = Request::create('/test', 'GET', array('name' => 'druplicon', 'pass' => '12345')); $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) { $this->kernel->expects($this->once())->method('handle')->will($this->returnCallback(function (Request $request) {
return new Response($request->getMethod() . ' ' . UrlHelper::buildQuery($request->query->all())); return new Response($request->getMethod() . ' ' . UrlHelper::buildQuery($request->query->all()));
...@@ -137,6 +163,7 @@ public function testHandleWithGetRequest() { ...@@ -137,6 +163,7 @@ public function testHandleWithGetRequest() {
$response = $event->getResponse(); $response = $event->getResponse();
$result = $response->getContent() . " " . UrlHelper::buildQuery($request->request->all()); $result = $response->getContent() . " " . UrlHelper::buildQuery($request->request->all());
$this->assertEquals('GET name=druplicon&pass=12345&destination=test&_exception_statuscode=404 ', $result); $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));
} }
} }
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment