diff --git a/core/core.services.yml b/core/core.services.yml index 2f04124..1e060ce 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -424,6 +424,9 @@ services: tags: - { name: service_collector, tag: route_enhancer, call: addRouteEnhancer } router: + class: Drupal\Core\Routing\AccessAwareRouter + arguments: ['@router.chain', '@access_manager', '@current_user'] + router.insecure: class: Symfony\Cmf\Component\Routing\ChainRouter calls: - [setContext, ['@router.request_context']] @@ -477,7 +480,7 @@ services: arguments: ['@config.factory'] path.validator: class: Drupal\Core\Path\PathValidator - arguments: ['@router', '@router.route_provider', '@request_stack', '@access_manager', '@current_user'] + arguments: ['@router', '@router.route_provider', '@request_stack'] # The argument to the hashing service defined in services.yml, to the # constructor of PhpassHashedPassword is the log2 number of iterations for @@ -620,13 +623,6 @@ services: arguments: ['@router.route_provider', '@url_generator', '@paramconverter_manager', '@access_arguments_resolver', '@request_stack'] calls: - [setContainer, ['@service_container']] - access_subscriber: - class: Drupal\Core\EventSubscriber\AccessSubscriber - arguments: ['@access_manager', '@current_user'] - calls: - - [setCurrentUser, ['@?current_user']] - tags: - - { name: event_subscriber } access_route_subscriber: class: Drupal\Core\EventSubscriber\AccessRouteSubscriber tags: diff --git a/core/lib/Drupal/Core/EventSubscriber/AccessSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/AccessSubscriber.php deleted file mode 100644 index 02a65ed..0000000 --- a/core/lib/Drupal/Core/EventSubscriber/AccessSubscriber.php +++ /dev/null @@ -1,113 +0,0 @@ -accessManager = $access_manager; - $this->currentUser = $current_user; - } - - /** - * Verifies that the current user can access the requested path. - * - * @param \Symfony\Component\HttpKernel\Event\GetResponseEvent $event - * The Event to process. - * - * @throws \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException - * Thrown when the access got denied. - */ - public function onKernelRequestAccessCheck(GetResponseEvent $event) { - $request = $event->getRequest(); - - // The controller is being handled by the HTTP kernel, so add an attribute - // to tell us this is the controller request. - $request->attributes->set('_controller_request', TRUE); - - if (!$request->attributes->has(RouteObjectInterface::ROUTE_OBJECT)) { - // If no Route is available it is likely a static resource and access is - // handled elsewhere. - return; - } - - // Wrap this in a try/catch to ensure the '_controller_request' attribute - // can always be removed. - try { - $access = $this->accessManager->check($request->attributes->get(RouteObjectInterface::ROUTE_OBJECT), $request, $this->currentUser); - } - catch (\Exception $e) { - $request->attributes->remove('_controller_request'); - throw $e; - } - - $request->attributes->remove('_controller_request'); - - if (!$access) { - throw new AccessDeniedHttpException(); - } - } - - /** - * Sets the current user. - * - * @param \Drupal\Core\Session\AccountInterface|null $current_user - * The current user service. - */ - public function setCurrentUser(AccountInterface $current_user = NULL) { - $this->currentUser = $current_user; - } - - /** - * Registers the methods in this class that should be listeners. - * - * @return array - * An array of event listener definitions. - */ - static function getSubscribedEvents() { - $events[KernelEvents::REQUEST][] = array('onKernelRequestAccessCheck', 30); - - return $events; - } - -} diff --git a/core/lib/Drupal/Core/Path/PathValidator.php b/core/lib/Drupal/Core/Path/PathValidator.php index a90148b..0c1580b 100644 --- a/core/lib/Drupal/Core/Path/PathValidator.php +++ b/core/lib/Drupal/Core/Path/PathValidator.php @@ -8,12 +8,11 @@ namespace Drupal\Core\Path; use Drupal\Component\Utility\UrlHelper; -use Drupal\Core\Access\AccessManagerInterface; use Drupal\Core\ParamConverter\ParamNotConvertedException; use Drupal\Core\Routing\RequestHelper; use Drupal\Core\Routing\RouteProviderInterface; -use Drupal\Core\Session\AccountInterface; use Symfony\Component\HttpFoundation\RequestStack; +use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; use Symfony\Component\Routing\Matcher\RequestMatcherInterface; /** @@ -43,20 +42,6 @@ class PathValidator implements PathValidatorInterface { protected $requestStack; /** - * The access manager. - * - * @var \Drupal\Core\Access\AccessManagerInterface - */ - protected $accessManager; - - /** - * The user account. - * - * @var \Drupal\Core\Session\AccountInterface - */ - protected $account; - - /** * Creates a new PathValidator. * * @param \Symfony\Component\Routing\Matcher\RequestMatcherInterface $request_matcher @@ -70,12 +55,10 @@ class PathValidator implements PathValidatorInterface { * @param \Drupal\Core\Session\AccountInterface $account * The user account. */ - public function __construct(RequestMatcherInterface $request_matcher, RouteProviderInterface $route_provider, RequestStack $request_stack, AccessManagerInterface $access_manager, AccountInterface $account) { + public function __construct(RequestMatcherInterface $request_matcher, RouteProviderInterface $route_provider, RequestStack $request_stack) { $this->requestMatcher = $request_matcher; $this->routeProvider = $route_provider; $this->requestStack = $request_stack; - $this->accessManager = $access_manager; - $this->account = $account; } /** @@ -93,25 +76,23 @@ public function isValid($path) { return FALSE; } + // We can not use $this->requestMatcher->match() beccause we need to set + // the _menu_admin attribute to indicate a menu administrator is running + // the menu access check. $request = RequestHelper::duplicate($this->requestStack->getCurrentRequest(), '/' . $path); $request->attributes->set('_system_path', $path); - - // We indicate that a menu administrator is running the menu access check. $request->attributes->set('_menu_admin', TRUE); - // Attempt to match this path to provide a fully built request to the - // access checker. try { - $request->attributes->add($this->requestMatcher->matchRequest($request)); + $this->requestMatcher->matchRequest($request); } catch (ParamNotConvertedException $e) { return FALSE; } - - // Consult the access manager. - $routes = $collection->all(); - $route = reset($routes); - return $this->accessManager->check($route, $request, $this->account); + catch (AccessDeniedHttpException $e) { + return FALSE; + } + return TRUE; } } diff --git a/core/lib/Drupal/Core/Routing/AccessAwareRouter.php b/core/lib/Drupal/Core/Routing/AccessAwareRouter.php new file mode 100644 index 0000000..51adade --- /dev/null +++ b/core/lib/Drupal/Core/Routing/AccessAwareRouter.php @@ -0,0 +1,128 @@ +chainRouter = $router; + $this->accessManager = $access_manager; + $this->account = $account; + } + + /** + * {@inheritdoc} + */ + public function setContext(RequestContext $context) { + $this->chainRouter->setContext($context); + } + + /** + * {@inheritdoc} + */ + public function getContext() { + return $this->chainRouter->getContext(); + } + + /** + * {@inheritdoc} + */ + public function matchRequest(Request $request) { + $parameters = $this->chainRouter->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(); + } + + /** + * @param $request + * The request to access check. + */ + protected function checkAccess(Request $request) { + // The controller is being handled by the HTTP kernel, so add an attribute + // to tell us this is the controller request. + $request->attributes->set('_controller_request', TRUE); + $e = FALSE; + try { + if (!$this->accessManager->check($request->attributes->get(RouteObjectInterface::ROUTE_OBJECT), $request, $this->account)) { + $e = new AccessDeniedHttpException(); + } + } + catch (\Exception $e) { + } + $request->attributes->remove('_controller_request'); + if ($e) { + throw $e; + } + } + + /** + * {@inheritdoc} + */ + public function getRouteCollection() { + return $this->chainRouter->getRouteCollection(); + } + + /** + * {@inheritdoc} + */ + public function generate($name, $parameters = array(), $referenceType = self::ABSOLUTE_PATH) { + return $this->chainRouter->generate($name, $parameters, $referenceType); + } + + /** + * {@inheritdoc} + */ + public function match($pathinfo) { + return $this->matchRequest(Request::create($pathinfo)); + } + + /** + * Set the current user. + * + * @param AccountInterface $account + * The current user. + */ + public function setCurrentUser(AccountInterface $account) { + $this->account = $account; + } + +} + diff --git a/core/lib/Drupal/Core/Routing/AccessAwareRouterInterface.php b/core/lib/Drupal/Core/Routing/AccessAwareRouterInterface.php new file mode 100644 index 0000000..2dbfe73 --- /dev/null +++ b/core/lib/Drupal/Core/Routing/AccessAwareRouterInterface.php @@ -0,0 +1,31 @@ +match('/' . $path); + $result = \Drupal::service('router.insecure')->match('/' . $path); } catch (ResourceNotFoundException $e) { throw new MatchingRouteNotFoundException(sprintf('No matching route could be found for the path "%s"', $path), 0, $e); diff --git a/core/modules/config_translation/src/Controller/ConfigTranslationController.php b/core/modules/config_translation/src/Controller/ConfigTranslationController.php index cb7f6b7..789f92e 100644 --- a/core/modules/config_translation/src/Controller/ConfigTranslationController.php +++ b/core/modules/config_translation/src/Controller/ConfigTranslationController.php @@ -167,15 +167,7 @@ public function itemPage(Request $request, $plugin_id) { // Check access for the path/route for editing, so we can decide to // include a link to edit or not. - $route_request = $this->getRequestForPath($request, $mapper->getBasePath()); - $edit_access = FALSE; - if (!empty($route_request)) { - $route_name = $route_request->attributes->get(RouteObjectInterface::ROUTE_NAME); - // Note that the parameters don't really matter here since we're - // passing in the request which already has the upcast attributes. - $parameters = array(); - $edit_access = $this->accessManager->checkNamedRoute($route_name, $parameters, $this->account, $route_request); - } + $edit_access = $this->accessManager->checkNamedRoute($mapper->getBaseRouteName(), $request->attributes->get('_raw_variables')->all(), $this->account); // Build list of operations. $operations = array(); @@ -228,35 +220,4 @@ public function itemPage(Request $request, $plugin_id) { return $page; } - /** - * Matches a path in the router. - * - * @param \Symfony\Component\HttpFoundation\Request $request - * Page request object. - * @param string $path - * Path to look up. - * - * @return \Symfony\Component\HttpFoundation\Request|null - * A populated request object or NULL if the patch could not be matched. - */ - protected function getRequestForPath(Request $request, $path) { - // @todo Use RequestHelper::duplicate once https://drupal.org/node/2090293 - // is fixed. - $route_request = Request::create($request->getBaseUrl() . '/' . $path); - // Find the system path by resolving aliases, language prefix, etc. - $processed = $this->pathProcessor->processInbound($path, $route_request); - $route_request->attributes->set('_system_path', $processed); - // Attempt to match this path to provide a fully built request. - try { - $route_request->attributes->add($this->router->matchRequest($route_request)); - return $route_request; - } - catch (ParamNotConvertedException $e) { - return NULL; - } - catch (ResourceNotFoundException $e) { - return NULL; - } - } - } diff --git a/core/modules/simpletest/src/WebTestBase.php b/core/modules/simpletest/src/WebTestBase.php index 89ff9db..bbd7fd1 100644 --- a/core/modules/simpletest/src/WebTestBase.php +++ b/core/modules/simpletest/src/WebTestBase.php @@ -706,7 +706,7 @@ protected function drupalLogin(AccountInterface $account) { $this->container->get('current_user')->setAccount($account); // @todo Temporary workaround for not being able to use synchronized // services in non dumped container. - $this->container->get('access_subscriber')->setCurrentUser($account); + $this->container->get('router')->setCurrentUser($account); } } diff --git a/core/modules/system/src/PathBasedBreadcrumbBuilder.php b/core/modules/system/src/PathBasedBreadcrumbBuilder.php index c2765aa..e071571 100644 --- a/core/modules/system/src/PathBasedBreadcrumbBuilder.php +++ b/core/modules/system/src/PathBasedBreadcrumbBuilder.php @@ -20,6 +20,7 @@ use Drupal\Component\Utility\Unicode; use Symfony\Component\HttpFoundation\Request; use Symfony\Cmf\Component\Routing\RouteObjectInterface; +use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; use Symfony\Component\Routing\Matcher\RequestMatcherInterface; use Symfony\Component\Routing\RequestContext; use Symfony\Component\Routing\Exception\MethodNotAllowedException; @@ -201,13 +202,12 @@ protected function getRequestForPath($path, array $exclude) { return $request; } catch (ParamNotConvertedException $e) { - return NULL; } catch (ResourceNotFoundException $e) { - return NULL; } catch (MethodNotAllowedException $e) { - return NULL; + } + catch (AccessDeniedHttpException $e) { } } diff --git a/core/modules/user/src/Plugin/LanguageNegotiation/LanguageNegotiationUserAdmin.php b/core/modules/user/src/Plugin/LanguageNegotiation/LanguageNegotiationUserAdmin.php index 4697dfa..7e17832 100644 --- a/core/modules/user/src/Plugin/LanguageNegotiation/LanguageNegotiationUserAdmin.php +++ b/core/modules/user/src/Plugin/LanguageNegotiation/LanguageNegotiationUserAdmin.php @@ -14,6 +14,7 @@ use Symfony\Cmf\Component\Routing\RouteObjectInterface; use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; use Symfony\Component\Routing\Exception\ResourceNotFoundException; use Symfony\Component\Routing\Matcher\UrlMatcherInterface; @@ -110,7 +111,7 @@ public function getLangcode(Request $request = NULL) { * @return bool * TRUE if the path is administrative, FALSE otherwise. */ - public function isAdminPath(Request $request) { + protected function isAdminPath(Request $request) { $result = FALSE; if ($request && $this->adminContext) { // If called from an event subscriber, the request may not have the route @@ -127,6 +128,9 @@ public function isAdminPath(Request $request) { catch (ResourceNotFoundException $e) { return FALSE; } + catch (AccessDeniedHttpException $e) { + return FALSE; + } $route_object = $attributes[RouteObjectInterface::ROUTE_OBJECT]; } $result = $this->adminContext->isAdminRoute($route_object); diff --git a/core/tests/Drupal/Tests/Core/EventSubscriber/AccessSubscriberTest.php b/core/tests/Drupal/Tests/Core/EventSubscriber/AccessSubscriberTest.php deleted file mode 100644 index aa902bb..0000000 --- a/core/tests/Drupal/Tests/Core/EventSubscriber/AccessSubscriberTest.php +++ /dev/null @@ -1,156 +0,0 @@ -event = $this->getMockBuilder('Symfony\Component\HttpKernel\Event\GetResponseEvent') - ->disableOriginalConstructor() - ->getMock(); - - $this->request = $this->getMockBuilder('Symfony\Component\HttpFoundation\Request') - ->disableOriginalConstructor() - ->getMock(); - - $this->parameterBag = $this->getMockBuilder('Symfony\Component\HttpFoundation\ParameterBag') - ->disableOriginalConstructor() - ->getMock(); - - $this->route = $this->getMockBuilder('Symfony\Component\Routing\Route') - ->disableOriginalConstructor() - ->getMock(); - - $this->request->attributes = $this->parameterBag; - - $this->event->expects($this->any()) - ->method('getRequest') - ->will($this->returnValue($this->request)); - - $this->accessManager = $this->getMock('Drupal\Core\Access\AccessManagerInterface'); - - $this->currentUser = $this->getMockBuilder('Drupal\Core\Session\AccountInterface') - ->disableOriginalConstructor() - ->getMock(); - } - - /** - * Tests access denied throws an exception. - * - * @expectedException \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException - */ - public function testAccessSubscriberThrowsAccessDeniedException() { - - $this->parameterBag->expects($this->any()) - ->method('has') - ->with(RouteObjectInterface::ROUTE_OBJECT) - ->will($this->returnValue(TRUE)); - - $this->parameterBag->expects($this->any()) - ->method('get') - ->with(RouteObjectInterface::ROUTE_OBJECT) - ->will($this->returnValue($this->route)); - - $this->accessManager->expects($this->any()) - ->method('check') - ->with($this->anything()) - ->will($this->returnValue(FALSE)); - - $subscriber = new AccessSubscriber($this->accessManager, $this->currentUser); - $subscriber->onKernelRequestAccessCheck($this->event); - } - - /** - * Tests that the AccessSubscriber only acts on requests with route object. - */ - public function testAccessSubscriberOnlyChecksForRequestsWithRouteObject() { - $this->parameterBag->expects($this->any()) - ->method('has') - ->with(RouteObjectInterface::ROUTE_OBJECT) - ->will($this->returnValue(FALSE)); - - $this->accessManager->expects($this->never())->method('check'); - - $subscriber = new AccessSubscriber($this->accessManager, $this->currentUser); - $subscriber->onKernelRequestAccessCheck($this->event); - } - - /** - * Tests that if access is granted, AccessSubscriber will not throw an exception. - */ - public function testAccessSubscriberDoesNotAlterRequestIfAccessManagerGrantsAccess() { - $this->parameterBag->expects($this->once()) - ->method('has') - ->with(RouteObjectInterface::ROUTE_OBJECT) - ->will($this->returnValue(TRUE)); - - $this->parameterBag->expects($this->once()) - ->method('get') - ->with(RouteObjectInterface::ROUTE_OBJECT) - ->will($this->returnValue($this->route)); - - $this->accessManager->expects($this->once()) - ->method('check') - ->with($this->equalTo($this->route)) - ->will($this->returnValue(TRUE)); - - $subscriber = new AccessSubscriber($this->accessManager, $this->currentUser); - // We're testing that no exception is thrown in this case. There is no - // return. - $subscriber->onKernelRequestAccessCheck($this->event); - } - -} diff --git a/core/tests/Drupal/Tests/Core/Routing/AccessAwareRouterTest.php b/core/tests/Drupal/Tests/Core/Routing/AccessAwareRouterTest.php new file mode 100644 index 0000000..736e144 --- /dev/null +++ b/core/tests/Drupal/Tests/Core/Routing/AccessAwareRouterTest.php @@ -0,0 +1,86 @@ +route = new Route('test'); + $this->chainRouter = $this->getMockBuilder('Symfony\Cmf\Component\Routing\ChainRouter') + ->disableOriginalConstructor() + ->getMock(); + $this->chainRouter->expects($this->once()) + ->method('matchRequest') + ->will($this->returnValue(array(RouteObjectInterface::ROUTE_OBJECT => $this->route))); + $this->accessManager = $this->getMock('Drupal\Core\Access\AccessManagerInterface'); + $this->currentUser = $this->getMock('Drupal\Core\Session\AccountInterface'); + $this->router = new AccessAwareRouter($this->chainRouter, $this->accessManager, $this->currentUser); + } + + /** + * Tests the matchrRequest() function for access allowed. + */ + public function testMatchRequestAllowed() { + $this->accessManager->expects($this->once()) + ->method('check') + ->will($this->returnValue(TRUE)); + $this->router->matchRequest(new Request()); + } + + /** + * Tests the matchrRequest() function for access denied. + * + * @expectedException \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException + */ + public function testMatchRequestDenied() { + $this->accessManager->expects($this->once()) + ->method('check') + ->will($this->returnValue(FALSE)); + $this->router->matchRequest(new Request()); + } + +} diff --git a/core/tests/Drupal/Tests/Core/UrlTest.php b/core/tests/Drupal/Tests/Core/UrlTest.php index 324f9e9..2737c9c 100644 --- a/core/tests/Drupal/Tests/Core/UrlTest.php +++ b/core/tests/Drupal/Tests/Core/UrlTest.php @@ -75,15 +75,15 @@ public function testCreateFromPath() { $this->router->expects($this->any()) ->method('match') ->will($this->returnValueMap(array( - array('/node', array( + array('/node', FALSE, array( RouteObjectInterface::ROUTE_NAME => 'view.frontpage.page_1', '_raw_variables' => new ParameterBag(), )), - array('/node/1', array( + array('/node/1', FALSE, array( RouteObjectInterface::ROUTE_NAME => 'node_view', '_raw_variables' => new ParameterBag(array('node' => '1')), )), - array('/node/2/edit', array( + array('/node/2/edit', FALSE, array( RouteObjectInterface::ROUTE_NAME => 'node_edit', '_raw_variables' => new ParameterBag(array('node' => '2')), )),