diff --git a/core/core.services.yml b/core/core.services.yml index bcf528b10b..b98a491b7d 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -855,7 +855,11 @@ services: arguments: ['@url_generator', '@module_handler', '@renderer'] router: class: Drupal\Core\Routing\AccessAwareRouter - arguments: ['@router.no_access_checks', '@access_manager', '@current_user'] + arguments: ['@access_manager', '@current_user'] + parent: router.no_access_checks + tags: + - { name: service_collector, tag: non_lazy_route_enhancer, call: addRouteEnhancer } + - { name: service_collector, tag: non_lazy_route_filter, call: addRouteFilter } router.dynamic: class: Symfony\Cmf\Component\Routing\DynamicRouter arguments: ['@router.request_context', '@router.matcher', '@url_generator'] diff --git a/core/lib/Drupal/Core/Routing/AccessAwareRouter.php b/core/lib/Drupal/Core/Routing/AccessAwareRouter.php index d8487c6ef9..7014ba1081 100644 --- a/core/lib/Drupal/Core/Routing/AccessAwareRouter.php +++ b/core/lib/Drupal/Core/Routing/AccessAwareRouter.php @@ -4,25 +4,17 @@ 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\Matcher\RequestMatcherInterface; -use Symfony\Component\Routing\RequestContext as SymfonyRequestContext; -use Symfony\Component\Routing\RequestContextAwareInterface; -use Symfony\Component\Routing\RouterInterface; +use Symfony\Component\Routing\Generator\UrlGeneratorInterface as BaseUrlGeneratorInterface; /** - * A router class for Drupal with access check and upcasting. + * Extends the router to provide access checking. */ -class AccessAwareRouter implements AccessAwareRouterInterface { - - /** - * The router doing the actual routing. - * - * @var \Symfony\Component\Routing\Matcher\RequestMatcherInterface - */ - protected $router; +class AccessAwareRouter extends Router implements AccessAwareRouterInterface { /** * The access manager. @@ -34,52 +26,30 @@ class AccessAwareRouter implements AccessAwareRouterInterface { /** * The account to use in access checks. * - * @var \Drupal\Core\Session\AccountInterface; + * @var \Drupal\Core\Session\AccountInterface */ protected $account; /** - * Constructs a router for Drupal with access check and upcasting. + * Constructs a new AccessAwareRouter. * - * @param \Symfony\Component\Routing\Matcher\RequestMatcherInterface $router - * The router doing the actual routing. + * @param \Symfony\Cmf\Component\Routing\RouteProviderInterface $route_provider + * The route provider. + * @param \Drupal\Core\Path\CurrentPathStack $current_path + * 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 account to use in access checks. */ - public function __construct(RequestMatcherInterface $router, AccessManagerInterface $access_manager, AccountInterface $account) { - $this->router = $router; + public function __construct(BaseRouteProviderInterface $route_provider, CurrentPathStack $current_path, BaseUrlGeneratorInterface $url_generator, AccessManagerInterface $access_manager, AccountInterface $account) { + parent::__construct($route_provider, $current_path, $url_generator); $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} * @@ -87,7 +57,8 @@ public function getContext() { * Thrown when access checking failed. */ public function matchRequest(Request $request) { - $parameters = $this->router->matchRequest($request); + $parameters = parent::matchRequest($request); + $request->attributes->add($parameters); $this->checkAccess($request); // We can not return $parameters because the access check can change the @@ -96,7 +67,7 @@ public function matchRequest(Request $request) { } /** - * Apply access check service to the route and parameters in the request. + * Applies access check service to the route and parameters in the request. * * @param \Symfony\Component\HttpFoundation\Request $request * The request to access check. @@ -115,32 +86,4 @@ protected function checkAccess(Request $request) { } } - /** - * {@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); - } - } - - /** - * {@inheritdoc} - * - * @throws \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException - * Thrown when access checking failed. - */ - public function match($pathinfo) { - return $this->matchRequest(Request::create($pathinfo)); - } - } diff --git a/core/tests/Drupal/Tests/Core/Routing/AccessAwareRouterTest.php b/core/tests/Drupal/Tests/Core/Routing/AccessAwareRouterTest.php index 53405f115b..890a1d1158 100644 --- a/core/tests/Drupal/Tests/Core/Routing/AccessAwareRouterTest.php +++ b/core/tests/Drupal/Tests/Core/Routing/AccessAwareRouterTest.php @@ -2,14 +2,23 @@ namespace Drupal\Tests\Core\Routing; +use Drupal\Core\Access\AccessManagerInterface; use Drupal\Core\Access\AccessResult; +use Drupal\Core\Path\CurrentPathStack; use Drupal\Core\Routing\AccessAwareRouter; use Drupal\Core\Routing\AccessAwareRouterInterface; +use Drupal\Core\Routing\Router; +use Drupal\Core\Session\AccountInterface; use Drupal\Tests\UnitTestCase; +use Prophecy\Argument; use Symfony\Cmf\Component\Routing\RouteObjectInterface; +use Symfony\Cmf\Component\Routing\RouteProviderInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; +use Symfony\Component\Routing\Generator\UrlGeneratorInterface; +use Symfony\Component\Routing\RequestContext; use Symfony\Component\Routing\Route; +use Symfony\Component\Routing\RouteCollection; /** * @coversDefaultClass \Drupal\Core\Routing\AccessAwareRouter @@ -18,66 +27,93 @@ class AccessAwareRouterTest extends UnitTestCase { /** + * The mocked router provider. + * + * @var \Prophecy\Prophecy\ObjectProphecy|\Symfony\Cmf\Component\Routing\RouteProviderInterface + */ + protected $routeProvider; + + /** + * The mocked current path stack. + * + * @var \Prophecy\Prophecy\ObjectProphecy|\Drupal\Core\Path\CurrentPathStack + */ + protected $currentPathStack; + + /** + * The mocked url generator. + * + * @var \Prophecy\Prophecy\ObjectProphecy|\Symfony\Component\Routing\Generator\UrlGeneratorInterface + */ + protected $urlGenerator; + + /** + * The mocked access manager. + * + * @var \Prophecy\Prophecy\ObjectProphecy|\Drupal\Core\Access\AccessManagerInterface + */ + protected $accessManager; + + /** + * The mocked current account. + * + * @var \Prophecy\Prophecy\ObjectProphecy|\Drupal\Core\Session\AccountInterface + */ + protected $account; + + /** + * The route. + * * @var \Symfony\Component\Routing\Route */ protected $route; - /** - * @var \Symfony\Cmf\Component\Routing\ChainRouter|\PHPUnit_Framework_MockObject_MockObject - */ - protected $chainRouter; - - /** - * @var \Drupal\Core\Access\AccessManagerInterface|\PHPUnit_Framework_MockObject_MockObject - */ - protected $accessManager; - - /** - * @var \Drupal\Core\Session\AccountInterface||\PHPUnit_Framework_MockObject_MockObject - */ - protected $currentUser; - - /** - * @var \Drupal\Core\Routing\AccessAwareRouter - */ - protected $router; - /** * {@inheritdoc} */ protected function setUp() { parent::setUp(); + $this->route = new Route('test'); - $this->accessManager = $this->getMock('Drupal\Core\Access\AccessManagerInterface'); - $this->currentUser = $this->getMock('Drupal\Core\Session\AccountInterface'); + $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); } - /** - * 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); + protected function setupRouter(Request $request, 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 AccessAwareRouter($this->routeProvider->reveal(), $this->currentPathStack->reveal(), $this->urlGenerator->reveal(), $this->accessManager->reveal(), $this->account->reveal()); } /** * Tests the matchRequest() function for access allowed. + * + * @covers ::matchRequest + * @covers ::checkAccess */ public function testMatchRequestAllowed() { - $this->setupRouter(); - $request = new Request(); + $request = Request::create('/test'); $access_result = AccessResult::allowed(); - $this->accessManager->expects($this->once()) - ->method('checkRequest') - ->with($request) - ->willReturn($access_result); - $parameters = $this->router->matchRequest($request); + + $router = $this->setupRouter($request, $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, ]; @@ -87,54 +123,37 @@ public function testMatchRequestAllowed() { /** * Tests the matchRequest() function for access denied. + * + * @covers ::matchRequest + * @covers ::checkAccess */ public function testMatchRequestDenied() { - $this->setupRouter(); - $request = new Request(); + $request = Request::create('/test'); $access_result = AccessResult::forbidden(); - $this->accessManager->expects($this->once()) - ->method('checkRequest') - ->with($request) - ->willReturn($access_result); + + $router = $this->setupRouter($request, $access_result); + $router->setContext((new RequestContext())->fromRequest($request)); + $this->setExpectedException(AccessDeniedHttpException::class); - $this->router->matchRequest($request); + $router->matchRequest($request); } /** * Tests the matchRequest() function for access denied with reason message. + * + * @covers ::matchRequest + * @covers ::checkAccess */ public function testCheckAccessResultWithReason() { - $this->setupRouter(); - $request = new Request(); + $request = Request::create('/test'); $reason = $this->getRandomGenerator()->string(); $access_result = AccessResult::forbidden($reason); - $this->accessManager->expects($this->once()) - ->method('checkRequest') - ->with($request) - ->willReturn($access_result); + + $router = $this->setupRouter($request, $access_result); + $router->setContext((new RequestContext())->fromRequest($request)); + $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); + $router->matchRequest($request); } }