diff --git a/core/core.services.yml b/core/core.services.yml index 21a3e590ba..c607fb0000 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -854,8 +854,13 @@ services: class: Drupal\Core\Utility\LinkGenerator arguments: ['@url_generator', '@module_handler', '@renderer'] router: - class: Drupal\Core\Routing\AccessAwareRouter - arguments: ['@router.no_access_checks', '@access_manager', '@current_user'] + class: \Drupal\Core\Routing\Router + arguments: ['@router.route_provider', '@path.current', '@url_generator', '@access_manager', '@current_user', TRUE] + tags: + - { name: service_collector, tag: non_lazy_route_enhancer, call: addRouteEnhancer } + - { name: service_collector, tag: non_lazy_route_filter, call: addRouteFilter } + calls: + - [setContext, ['@router.request_context']] router.dynamic: class: Symfony\Cmf\Component\Routing\DynamicRouter arguments: ['@router.request_context', '@router.matcher', '@url_generator'] @@ -864,7 +869,7 @@ services: deprecated: The "%service_id%" service is deprecated. You should use the 'router.no_access_checks' service instead. router.no_access_checks: class: \Drupal\Core\Routing\Router - arguments: ['@router.route_provider', '@path.current', '@url_generator'] + arguments: ['@router.route_provider', '@path.current', '@url_generator', '@access_manager', '@current_user', FALSE] tags: - { name: service_collector, tag: non_lazy_route_enhancer, call: addRouteEnhancer } - { name: service_collector, tag: non_lazy_route_filter, call: addRouteFilter } diff --git a/core/lib/Drupal/Core/Routing/AccessAwareRouter.php b/core/lib/Drupal/Core/Routing/AccessAwareRouter.php index d8487c6ef9..689f4efc02 100644 --- a/core/lib/Drupal/Core/Routing/AccessAwareRouter.php +++ b/core/lib/Drupal/Core/Routing/AccessAwareRouter.php @@ -4,143 +4,28 @@ use Drupal\Core\Access\AccessManagerInterface; use Drupal\Core\Access\AccessResultReasonInterface; +use Drupal\Core\Path\CurrentPathStack; use Drupal\Core\Session\AccountInterface; +use Symfony\Cmf\Component\Routing\RouteProviderInterface as BaseRouteProviderInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; +use Symfony\Component\Routing\Generator\UrlGeneratorInterface as BaseUrlGeneratorInterface; use Symfony\Component\Routing\Matcher\RequestMatcherInterface; use Symfony\Component\Routing\RequestContext as SymfonyRequestContext; use Symfony\Component\Routing\RequestContextAwareInterface; use Symfony\Component\Routing\RouterInterface; /** - * A router class for Drupal with access check and upcasting. + * @deprecated in Drupal 8.3.x and will be removed before Drupal 9.0.0. + * Use Drupal\Core\Routing\Router with $check_access in the constructor instead. */ -class AccessAwareRouter implements AccessAwareRouterInterface { - - /** - * The router doing the actual routing. - * - * @var \Symfony\Component\Routing\Matcher\RequestMatcherInterface - */ - protected $router; - - /** - * The access manager. - * - * @var \Drupal\Core\Access\AccessManagerInterface - */ - protected $accessManager; - - /** - * The account to use in access checks. - * - * @var \Drupal\Core\Session\AccountInterface; - */ - protected $account; - - /** - * Constructs a router for Drupal with access check and upcasting. - * - * @param \Symfony\Component\Routing\Matcher\RequestMatcherInterface $router - * The router doing the actual routing. - * @param \Drupal\Core\Access\AccessManagerInterface $access_manager - * The access manager. - * @param \Drupal\Core\Session\AccountInterface $account - * The account to use in access checks. - */ - public function __construct(RequestMatcherInterface $router, AccessManagerInterface $access_manager, AccountInterface $account) { - $this->router = $router; - $this->accessManager = $access_manager; - $this->account = $account; - } - - /** - * {@inheritdoc} - */ - public function __call($name, $arguments) { - // Ensure to call every other function to the router. - return call_user_func_array([$this->router, $name], $arguments); - } - - /** - * {@inheritdoc} - */ - public function setContext(SymfonyRequestContext $context) { - if ($this->router instanceof RequestContextAwareInterface) { - $this->router->setContext($context); - } - } - - /** - * {@inheritdoc} - */ - public function getContext() { - if ($this->router instanceof RequestContextAwareInterface) { - return $this->router->getContext(); - } - } - - /** - * {@inheritdoc} - * - * @throws \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException - * Thrown when access checking failed. - */ - public function matchRequest(Request $request) { - $parameters = $this->router->matchRequest($request); - $request->attributes->add($parameters); - $this->checkAccess($request); - // We can not return $parameters because the access check can change the - // request attributes. - return $request->attributes->all(); - } - - /** - * Apply access check service to the route and parameters in the request. - * - * @param \Symfony\Component\HttpFoundation\Request $request - * The request to access check. - */ - protected function checkAccess(Request $request) { - // The cacheability (if any) of this request's access check result must be - // applied to the response. - $access_result = $this->accessManager->checkRequest($request, $this->account, TRUE); - // Allow a master request to set the access result for a subrequest: if an - // access result attribute is already set, don't overwrite it. - if (!$request->attributes->has(AccessAwareRouterInterface::ACCESS_RESULT)) { - $request->attributes->set(AccessAwareRouterInterface::ACCESS_RESULT, $access_result); - } - if (!$access_result->isAllowed()) { - throw new AccessDeniedHttpException($access_result instanceof AccessResultReasonInterface ? $access_result->getReason() : NULL); - } - } - - /** - * {@inheritdoc} - */ - public function getRouteCollection() { - if ($this->router instanceof RouterInterface) { - return $this->router->getRouteCollection(); - } - } - - /** - * {@inheritdoc} - */ - public function generate($name, $parameters = [], $referenceType = self::ABSOLUTE_PATH) { - if ($this->router instanceof UrlGeneratorInterface) { - return $this->router->generate($name, $parameters, $referenceType); - } - } +class AccessAwareRouter extends Router { /** * {@inheritdoc} - * - * @throws \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException - * Thrown when access checking failed. */ - public function match($pathinfo) { - return $this->matchRequest(Request::create($pathinfo)); + public function __construct(RouteProviderInterface $route_provider, CurrentPathStack $current_path, UrlGeneratorInterface $url_generator, AccessManagerInterface $access_manager, AccountInterface $account, $check_access = TRUE) { + parent::__construct($route_provider, $current_path, $url_generator, $access_manager, $account, $check_access); } } diff --git a/core/lib/Drupal/Core/Routing/Router.php b/core/lib/Drupal/Core/Routing/Router.php index e949c5efb8..6fb0b23c07 100644 --- a/core/lib/Drupal/Core/Routing/Router.php +++ b/core/lib/Drupal/Core/Routing/Router.php @@ -2,12 +2,16 @@ namespace Drupal\Core\Routing; +use Drupal\Core\Access\AccessManagerInterface; +use Drupal\Core\Access\AccessResultReasonInterface; use Drupal\Core\Path\CurrentPathStack; +use Drupal\Core\Session\AccountInterface; use Symfony\Cmf\Component\Routing\Enhancer\RouteEnhancerInterface as BaseRouteEnhancerInterface; use Symfony\Cmf\Component\Routing\LazyRouteCollection; use Symfony\Cmf\Component\Routing\NestedMatcher\RouteFilterInterface as BaseRouteFilterInterface; use Symfony\Cmf\Component\Routing\RouteProviderInterface as BaseRouteProviderInterface; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; use Symfony\Component\Routing\Exception\MethodNotAllowedException; use Symfony\Component\Routing\Exception\ResourceNotFoundException; use Symfony\Component\Routing\Generator\UrlGeneratorInterface as BaseUrlGeneratorInterface; @@ -31,6 +35,7 @@ * regex. See ::matchCollection(). * 4. Enhance the list of route attributes, for example loading entity objects. * See ::applyRouteEnhancers(). + * 5. Finally it checks access. * * This implementation uses ideas of the following routers: * - \Symfony\Cmf\Component\Routing\DynamicRouter @@ -41,7 +46,7 @@ * @see \Drupal\Core\Routing\UrlMatcher * @see \Symfony\Cmf\Component\Routing\NestedMatcher\NestedMatcher */ -class Router extends UrlMatcher implements RequestMatcherInterface, RouterInterface { +class Router extends UrlMatcher implements RequestMatcherInterface, RouterInterface, AccessAwareRouterInterface { /** * The route provider responsible for the first-pass match. @@ -86,6 +91,27 @@ class Router extends UrlMatcher implements RequestMatcherInterface, RouterInterf protected $urlGenerator; /** + * The access manager. + * + * @var \Drupal\Core\Access\AccessManagerInterface + */ + protected $accessManager; + + /** + * The current user. + * + * @var \Drupal\Core\Session\AccountInterface + */ + protected $account; + + /** + * Whether to check access. + * + * @var bool + */ + protected $checkAccess; + + /** * Constructs a new Router. * * @param \Symfony\Cmf\Component\Routing\RouteProviderInterface $route_provider @@ -94,11 +120,20 @@ class Router extends UrlMatcher implements RequestMatcherInterface, RouterInterf * The current path stack. * @param \Symfony\Component\Routing\Generator\UrlGeneratorInterface $url_generator * The URL generator. + * @param \Drupal\Core\Access\AccessManagerInterface $access_manager + * The access manager. + * @param \Drupal\Core\Session\AccountInterface $account + * The current user. + * @param bool $check_access + * Whether to check access. */ - public function __construct(BaseRouteProviderInterface $route_provider, CurrentPathStack $current_path, BaseUrlGeneratorInterface $url_generator) { + public function __construct(BaseRouteProviderInterface $route_provider, CurrentPathStack $current_path, BaseUrlGeneratorInterface $url_generator, AccessManagerInterface $access_manager, AccountInterface $account, $check_access = TRUE) { parent::__construct($current_path); $this->routeProvider = $route_provider; $this->urlGenerator = $url_generator; + $this->accessManager = $access_manager; + $this->account = $account; + $this->checkAccess = $check_access; } /** @@ -151,7 +186,15 @@ public function matchRequest(Request $request) { $collection = $this->applyRouteFilters($collection, $request); if ($ret = $this->matchCollection(rawurldecode($this->currentPath->getPath($request)), $collection)) { - return $this->applyRouteEnhancers($ret, $request); + $parameters = $this->applyRouteEnhancers($ret, $request); + $request->attributes->add($parameters); + + // Checking access here. ::checkAccess might throw an exception, when + // access is not granted. + if ($this->checkAccess) { + $this->checkAccess($request); + } + return $request->attributes->all(); } throw 0 < count($this->allow) @@ -374,6 +417,29 @@ protected function sortFilters() { } /** + * Apply access check service to the route and parameters in the request. + * + * @param \Symfony\Component\HttpFoundation\Request $request + * The request to access check. + * + * @throws \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException + * Thrown when access was not allowed. + */ + protected function checkAccess(Request $request) { + // The cacheability (if any) of this request's access check result must be + // applied to the response. + $access_result = $this->accessManager->checkRequest($request, $this->account, TRUE); + // Allow a master request to set the access result for a subrequest: if an + // access result attribute is already set, don't overwrite it. + if (!$request->attributes->has(AccessAwareRouterInterface::ACCESS_RESULT)) { + $request->attributes->set(AccessAwareRouterInterface::ACCESS_RESULT, $access_result); + } + if (!$access_result->isAllowed()) { + throw new AccessDeniedHttpException($access_result instanceof AccessResultReasonInterface ? $access_result->getReason() : NULL); + } + } + + /** * {@inheritdoc} */ public function getRouteCollection() { diff --git a/core/tests/Drupal/Tests/Core/Routing/AccessAwareRouterTest.php b/core/tests/Drupal/Tests/Core/Routing/AccessAwareRouterTest.php deleted file mode 100644 index 53405f115b..0000000000 --- a/core/tests/Drupal/Tests/Core/Routing/AccessAwareRouterTest.php +++ /dev/null @@ -1,140 +0,0 @@ -route = new Route('test'); - $this->accessManager = $this->getMock('Drupal\Core\Access\AccessManagerInterface'); - $this->currentUser = $this->getMock('Drupal\Core\Session\AccountInterface'); - } - - /** - * Sets up a chain router with matchRequest. - */ - protected function setupRouter() { - $this->chainRouter = $this->getMockBuilder('Symfony\Cmf\Component\Routing\ChainRouter') - ->disableOriginalConstructor() - ->getMock(); - $this->chainRouter->expects($this->once()) - ->method('matchRequest') - ->will($this->returnValue([RouteObjectInterface::ROUTE_OBJECT => $this->route])); - $this->router = new AccessAwareRouter($this->chainRouter, $this->accessManager, $this->currentUser); - } - - /** - * Tests the matchRequest() function for access allowed. - */ - public function testMatchRequestAllowed() { - $this->setupRouter(); - $request = new Request(); - $access_result = AccessResult::allowed(); - $this->accessManager->expects($this->once()) - ->method('checkRequest') - ->with($request) - ->willReturn($access_result); - $parameters = $this->router->matchRequest($request); - $expected = [ - RouteObjectInterface::ROUTE_OBJECT => $this->route, - AccessAwareRouterInterface::ACCESS_RESULT => $access_result, - ]; - $this->assertSame($expected, $request->attributes->all()); - $this->assertSame($expected, $parameters); - } - - /** - * Tests the matchRequest() function for access denied. - */ - public function testMatchRequestDenied() { - $this->setupRouter(); - $request = new Request(); - $access_result = AccessResult::forbidden(); - $this->accessManager->expects($this->once()) - ->method('checkRequest') - ->with($request) - ->willReturn($access_result); - $this->setExpectedException(AccessDeniedHttpException::class); - $this->router->matchRequest($request); - } - - /** - * Tests the matchRequest() function for access denied with reason message. - */ - public function testCheckAccessResultWithReason() { - $this->setupRouter(); - $request = new Request(); - $reason = $this->getRandomGenerator()->string(); - $access_result = AccessResult::forbidden($reason); - $this->accessManager->expects($this->once()) - ->method('checkRequest') - ->with($request) - ->willReturn($access_result); - $this->setExpectedException(AccessDeniedHttpException::class, $reason); - $this->router->matchRequest($request); - } - - /** - * Ensure that methods are passed to the wrapped router. - * - * @covers ::__call - */ - public function testCall() { - $mock_router = $this->getMock('Symfony\Component\Routing\RouterInterface'); - - $this->chainRouter = $this->getMockBuilder('Symfony\Cmf\Component\Routing\ChainRouter') - ->disableOriginalConstructor() - ->setMethods(['add']) - ->getMock(); - $this->chainRouter->expects($this->once()) - ->method('add') - ->with($mock_router) - ->willReturnSelf(); - $this->router = new AccessAwareRouter($this->chainRouter, $this->accessManager, $this->currentUser); - - $this->router->add($mock_router); - } - -} diff --git a/core/tests/Drupal/Tests/Core/Routing/RouterTest.php b/core/tests/Drupal/Tests/Core/Routing/RouterTest.php new file mode 100644 index 0000000000..ba0349ab6a --- /dev/null +++ b/core/tests/Drupal/Tests/Core/Routing/RouterTest.php @@ -0,0 +1,159 @@ +route = new Route('test'); + $this->routeProvider = $this->prophesize(RouteProviderInterface::class); + $this->currentPathStack = $this->prophesize(CurrentPathStack::class); + $this->urlGenerator = $this->prophesize(UrlGeneratorInterface::class); + $this->accessManager = $this->prophesize(AccessManagerInterface::class); + $this->account = $this->prophesize(AccountInterface::class); + } + + protected function setupRouter(Request $request, $check_access, AccessResult $access_result) { + $this->accessManager->checkRequest(Argument::type(Request::class), Argument::type(AccountInterface::class), TRUE) + ->shouldBeCalled() + ->willReturn($access_result); + + $route_collection = new RouteCollection(); + $route_collection->add('test', $this->route); + $this->routeProvider->getRouteCollectionForRequest($request) + ->willReturn($route_collection); + + $this->currentPathStack->getPath(Argument::any()) + ->willReturn('/test'); + + return new Router($this->routeProvider->reveal(), $this->currentPathStack->reveal(), $this->urlGenerator->reveal(), $this->accessManager->reveal(), $this->account->reveal(), $check_access); + } + + /** + * Tests the matchRequest() function for access allowed. + * + * @covers ::matchRequest + * @covers ::checkAccess + */ + public function testMatchRequestAllowed() { + $request = Request::create('/test'); + $access_result = AccessResult::allowed(); + + $router = $this->setupRouter($request, TRUE, $access_result); + $router->setContext((new RequestContext())->fromRequest($request)); + + $parameters = $router->matchRequest($request); + $expected = [ + RouteObjectInterface::ROUTE_NAME => 'test', + RouteObjectInterface::ROUTE_OBJECT => $this->route, + AccessAwareRouterInterface::ACCESS_RESULT => $access_result, + ]; + $this->assertSame($expected, $request->attributes->all()); + $this->assertSame($expected, $parameters); + } + + /** + * Tests the matchRequest() function for access denied. + * + * @covers ::matchRequest + * @covers ::checkAccess + */ + public function testMatchRequestDenied() { + $request = Request::create('/test'); + $access_result = AccessResult::forbidden(); + + $router = $this->setupRouter($request, TRUE, $access_result); + $router->setContext((new RequestContext())->fromRequest($request)); + + $this->setExpectedException(AccessDeniedHttpException::class); + $router->matchRequest($request); + } + + /** + * Tests the matchRequest() function for access denied with reason message. + * + * @covers ::matchRequest + * @covers ::checkAccess + */ + public function testCheckAccessResultWithReason() { + $request = Request::create('/test'); + $reason = $this->getRandomGenerator()->string(); + $access_result = AccessResult::forbidden($reason); + + $router = $this->setupRouter($request, TRUE, $access_result); + $router->setContext((new RequestContext())->fromRequest($request)); + + $this->setExpectedException(AccessDeniedHttpException::class, $reason); + $router->matchRequest($request); + } + + +}