diff --git a/core/lib/Drupal/Core/Access/AccessCheckInterface.php b/core/lib/Drupal/Core/Access/AccessCheckInterface.php new file mode 100644 index 0000000..0ce0abb --- /dev/null +++ b/core/lib/Drupal/Core/Access/AccessCheckInterface.php @@ -0,0 +1,40 @@ +checkIds[] = $service_id; + } + + /** + * For each route, saves a list of applicable access checks to the route. + * + * @param RouteCollection $routes + * A collection of routes to apply checks to. + */ + public function setChecks(RouteCollection $routes) { + foreach ($routes as $route) { + $checks = $this->applies($route); + if (!empty($checks)) { + $route->setOption('_access_checks', $checks); + } + } + } + + /** + * Determine which registered access checks apply to a route. + * + * @param Symfony\Component\Routing\Route $route + * The route to get list of access checks for. + * + * @return array + * An array of service ids for the access checks that apply to passed + * route. + */ + protected function applies(Route $route) { + $checks = array(); + + foreach ($this->checkIds as $service_id) { + if (empty($this->checks[$service_id])) { + $this->loadCheck($service_id); + } + + if ($this->checks[$service_id]->applies($route)) { + $checks[] = $service_id; + } + } + + return $checks; + } + + /** + * Check route against applicable access check services to determine whether + * it is accessible. + * + * @param Symfony\Component\Routing\Route $route + * The route to check access to. + * + * @throws Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException + * If any access check denies access or none explicitly approve. + */ + public function check(Route $route) { + $access = FALSE; + $checks = $route->getOption('_access_checks'); + + // No checks == deny by default. + if (!empty($checks)) { + foreach ($checks as $service_id) { + if (empty($this->checks[$service_id])) { + $this->loadCheck($service_id); + } + + $access = $this->checks[$service_id]->access($route); + if ($access === FALSE) { + // A check has denied access, no need to continue checking. + break; + } + } + } + + // Access has been denied or not explicily approved. + if (!$access) { + throw new AccessDeniedHttpException(); + } + } + + /** + * Lazy-loads access check services. + * + * @param string $service_id + * The service id of the access check service to load. + */ + protected function loadCheck($service_id) { + if (!in_array($service_id, $this->checkIds)) { + throw new \InvalidArgumentException(sprintf('No check has been registered for %s', $service_id)); + } + + $this->checks[$service_id] = $this->container->get($service_id); + } + +} diff --git a/core/lib/Drupal/Core/Access/DefaultAccessCheck.php b/core/lib/Drupal/Core/Access/DefaultAccessCheck.php new file mode 100644 index 0000000..4dc53f7 --- /dev/null +++ b/core/lib/Drupal/Core/Access/DefaultAccessCheck.php @@ -0,0 +1,44 @@ +hasDefault('_access'); + } + + /** + * Checks for access to route. + * + * @param Symfony\Component\Routing\Route $route + * The route to check against. + * + * @return mixed + * TRUE if access is allowed. + * FALSE if not. + * NULL if no opinion. + */ + public function access(Route $route) { + return $route->getDefault('_access'); + } +} diff --git a/core/lib/Drupal/Core/Access/PermissionAccessCheck.php b/core/lib/Drupal/Core/Access/PermissionAccessCheck.php new file mode 100644 index 0000000..89e5fbd --- /dev/null +++ b/core/lib/Drupal/Core/Access/PermissionAccessCheck.php @@ -0,0 +1,49 @@ +hasDefault('_permission'); + } + + /** + * Checks for access to route. + * + * @param Symfony\Component\Routing\Route $route + * The route to check against. + * + * @return mixed + * TRUE if access is allowed. + * FALSE if not. + * NULL if no opinion. + */ + public function access(Route $route) { + $permission = $route->getDefault('_permission'); + // @todo Replace user_access() with a correctly injected and session-using + // alternative. + // If user_access() fails, return NULL to give other checks a chance. + return user_access($permission) ? TRUE : NULL; + } +} diff --git a/core/lib/Drupal/Core/CoreBundle.php b/core/lib/Drupal/Core/CoreBundle.php index 76164f4..64a0422 100644 --- a/core/lib/Drupal/Core/CoreBundle.php +++ b/core/lib/Drupal/Core/CoreBundle.php @@ -8,6 +8,7 @@ namespace Drupal\Core; use Drupal\Core\DependencyInjection\Compiler\RegisterKernelListenersPass; +use Drupal\Core\DependencyInjection\Compiler\RegisterAccessChecksPass; use Drupal\Core\DependencyInjection\Compiler\RegisterMatchersPass; use Drupal\Core\DependencyInjection\Compiler\RegisterNestedMatchersPass; use Symfony\Component\DependencyInjection\Definition; @@ -70,6 +71,7 @@ public function build(ContainerBuilder $container) { $container->register('router.dumper', 'Drupal\Core\Routing\MatcherDumper') ->addArgument(new Reference('database')); $container->register('router.builder', 'Drupal\Core\Routing\RouteBuilder') + ->addArgument(new Reference('dispatcher')) ->addArgument(new Reference('router.dumper')) ->addArgument(new Reference('lock')) ->addArgument(new Reference('dispatcher')); @@ -104,8 +106,17 @@ public function build(ContainerBuilder $container) { $container->register('view_subscriber', 'Drupal\Core\EventSubscriber\ViewSubscriber') ->addArgument(new Reference('content_negotiation')) ->addTag('event_subscriber'); + $container->register('legacy_access_subscriber', 'Drupal\Core\EventSubscriber\LegacyAccessSubscriber') + ->addTag('event_subscriber'); + $container->register('access_manager', 'Drupal\Core\Access\AccessManager') + ->addMethodCall('setContainer', array(new Reference('service_container'))); $container->register('access_subscriber', 'Drupal\Core\EventSubscriber\AccessSubscriber') + ->addArgument(new Reference('access_manager')) ->addTag('event_subscriber'); + $container->register('access_check.default', 'Drupal\Core\Access\DefaultAccessCheck') + ->addTag('access_check'); + $container->register('access_check.permission', 'Drupal\Core\Access\PermissionAccessCheck') + ->addTag('access_check'); $container->register('maintenance_mode_subscriber', 'Drupal\Core\EventSubscriber\MaintenanceModeSubscriber') ->addTag('event_subscriber'); $container->register('path_subscriber', 'Drupal\Core\EventSubscriber\PathSubscriber') @@ -132,6 +143,8 @@ public function build(ContainerBuilder $container) { $container->addCompilerPass(new RegisterNestedMatchersPass()); // Add a compiler pass for registering event subscribers. $container->addCompilerPass(new RegisterKernelListenersPass(), PassConfig::TYPE_AFTER_REMOVING); + + $container->addCompilerPass(new RegisterAccessChecksPass()); } } diff --git a/core/lib/Drupal/Core/DependencyInjection/Compiler/RegisterAccessChecksPass.php b/core/lib/Drupal/Core/DependencyInjection/Compiler/RegisterAccessChecksPass.php new file mode 100644 index 0000000..0ae3585 --- /dev/null +++ b/core/lib/Drupal/Core/DependencyInjection/Compiler/RegisterAccessChecksPass.php @@ -0,0 +1,22 @@ +hasDefinition('access_manager')) { + return; + } + $access_manager = $container->getDefinition('access_manager'); + foreach ($container->findTaggedServiceIds('access_check') as $id => $attributes) { + $access_manager->addMethodCall('AddCheckService', array($id)); + } + } +} diff --git a/core/lib/Drupal/Core/EventSubscriber/AccessSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/AccessSubscriber.php index 4f1dc75..8d50414 100644 --- a/core/lib/Drupal/Core/EventSubscriber/AccessSubscriber.php +++ b/core/lib/Drupal/Core/EventSubscriber/AccessSubscriber.php @@ -11,6 +11,9 @@ use Symfony\Component\HttpKernel\Event\GetResponseEvent; use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; use Symfony\Component\EventDispatcher\EventSubscriberInterface; +use Drupal\Core\Routing\RoutingEvents; +use Drupal\Core\Routing\Event\RoutesEvent; +use Drupal\Core\Access\AccessManager; /** * Access subscriber for controller requests. @@ -18,21 +21,41 @@ class AccessSubscriber implements EventSubscriberInterface { /** - * Verifies that the current user can access the requested path. + * Constructs a new AccessCheckManager. * - * @todo This is a total hack to keep our current access system working. It - * should be replaced with something robust and injected at some point. + * @param AccessCheckManager $access_check_manager + * The access check manager that will be responsible for applying + * AccessCheckers against routes. + */ + public function __construct(AccessManager $access_manager) { + $this->accessManager = $access_manager; + } + + /** + * Verifies that the current user can access the requested path. * * @param Symfony\Component\HttpKernel\Event\GetResponseEvent $event * The Event to process. */ public function onKernelRequestAccessCheck(GetResponseEvent $event) { + $request = $event->getRequest(); + if (!$request->attributes->has('_route')) { + // If no Route is available it is likely a static resource and access is + // handled elsewhere. + return; + } - $router_item = $event->getRequest()->attributes->get('drupal_menu_item'); + $this->accessManager->check($request->attributes->get('_route')); + } - if (isset($router_item['access']) && !$router_item['access']) { - throw new AccessDeniedHttpException(); - } + /** + * Apply access checks to routes. + * + * @param Drupal\Core\Routing\Event\RoutesEvent $event + * The event to process. + */ + public function onRoutingRouteAlterSetAccessCheck(RoutesEvent $event) { + $this->accessManager->setChecks($event->getRoutes()); } /** @@ -43,6 +66,8 @@ public function onKernelRequestAccessCheck(GetResponseEvent $event) { */ static function getSubscribedEvents() { $events[KernelEvents::REQUEST][] = array('onKernelRequestAccessCheck', 30); + // Setting very low priority to ensure access checks are run after alters. + $events[RoutingEvents::ROUTE_ALTER][] = array('onRoutingRouteAlterSetAccessCheck', 0); return $events; } diff --git a/core/lib/Drupal/Core/EventSubscriber/LegacyAccessSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/LegacyAccessSubscriber.php new file mode 100644 index 0000000..707de62 --- /dev/null +++ b/core/lib/Drupal/Core/EventSubscriber/LegacyAccessSubscriber.php @@ -0,0 +1,49 @@ +getRequest()->attributes->get('drupal_menu_item'); + + if (isset($router_item['access']) && !$router_item['access']) { + throw new AccessDeniedHttpException(); + } + } + + /** + * 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/Routing/Event/RoutesEvent.php b/core/lib/Drupal/Core/Routing/Event/RoutesEvent.php new file mode 100644 index 0000000..3dff57e --- /dev/null +++ b/core/lib/Drupal/Core/Routing/Event/RoutesEvent.php @@ -0,0 +1,37 @@ +routes = $routes; + } + + /** + * Returns the routes that are about to be saved. + * + * @return Symfony\Component\Routing\RouteCollection + */ + public function getRoutes() { + return $this->routes; + } +} diff --git a/core/lib/Drupal/Core/Routing/FirstEntryFinalMatcher.php b/core/lib/Drupal/Core/Routing/FirstEntryFinalMatcher.php index 45d0888..cc1adde 100644 --- a/core/lib/Drupal/Core/Routing/FirstEntryFinalMatcher.php +++ b/core/lib/Drupal/Core/Routing/FirstEntryFinalMatcher.php @@ -53,7 +53,8 @@ public function matchRequest(Request $request) { preg_match($compiled->getRegex(), $path, $matches); - return array_merge($this->mergeDefaults($matches, $route->getDefaults()), array('_route' => $name)); + $route->setOption('_name', $name); + return array_merge($this->mergeDefaults($matches, $route->getDefaults()), array('_route' => $route)); } } diff --git a/core/lib/Drupal/Core/Routing/RouteBuilder.php b/core/lib/Drupal/Core/Routing/RouteBuilder.php index d69e5e3..67ff034 100644 --- a/core/lib/Drupal/Core/Routing/RouteBuilder.php +++ b/core/lib/Drupal/Core/Routing/RouteBuilder.php @@ -9,8 +9,11 @@ use Symfony\Component\Routing\Matcher\Dumper\MatcherDumperInterface; use Symfony\Component\EventDispatcher\EventDispatcherInterface; +use Symfony\Component\Routing\RouteCompilerInterface; +use Symfony\Component\Routing\Route; use Symfony\Component\Yaml\Parser; use Symfony\Component\Routing\RouteCollection; +use Drupal\Core\Routing\Event\RoutesEvent; use Drupal\Core\Lock\LockBackendInterface; @@ -46,6 +49,8 @@ class RouteBuilder { /** * Construcs the RouteBuilder using the passed MatcherDumperInterface. * + * @param \Symfony\Component\EventDispatcher\EventDispatcherInterface $dispatcher + * The dispatcher used to dispatch events. * @param \Symfony\Component\Routing\Matcher\Dumper\MatcherDumperInterface $dumper * The matcher dumper used to store the route information. * @param \Drupal\Core\Lock\LockBackendInterface $lock @@ -53,7 +58,8 @@ class RouteBuilder { * @param \Symfony\Component\EventDispatcherEventDispatcherInterface * The event dispatcher to notify of routes. */ - public function __construct(MatcherDumperInterface $dumper, LockBackendInterface $lock, EventDispatcherInterface $dispatcher) { + public function __construct(EventDispatcherInterface $dispatcher, MatcherDumperInterface $dumper, LockBackendInterface $lock) { + $this->dispatcher = $dispatcher; $this->dumper = $dumper; $this->lock = $lock; $this->dispatcher = $dispatcher; diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/FirstEntryFinalMatcherTest.php b/core/modules/system/lib/Drupal/system/Tests/Routing/FirstEntryFinalMatcherTest.php index a288b9e..c44a492 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Routing/FirstEntryFinalMatcherTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/FirstEntryFinalMatcherTest.php @@ -61,7 +61,7 @@ public function testFinalMatcherStatic() { $matcher->setCollection($collection); $attributes = $matcher->matchRequest($request); - $this->assertEqual($attributes['_route'], 'route_a', 'The correct matching route was found.'); + $this->assertEqual($attributes['_route']->getOption('_name'), 'route_a', 'The correct matching route was found.'); $this->assertEqual($attributes['_controller'], 'foo', 'The correct controller was found.'); } @@ -82,7 +82,7 @@ public function testFinalMatcherPattern() { $matcher->setCollection($collection); $attributes = $matcher->matchRequest($request); - $this->assertEqual($attributes['_route'], 'route_a', 'The correct matching route was found.'); + $this->assertEqual($attributes['_route']->getOption('_name'), 'route_a', 'The correct matching route was found.'); $this->assertEqual($attributes['_controller'], 'foo', 'The correct controller was found.'); $this->assertEqual($attributes['value'], 'narf', 'Required placeholder value found.'); } @@ -105,7 +105,7 @@ public function testFinalMatcherPatternDefalts() { $matcher->setCollection($collection); $attributes = $matcher->matchRequest($request); - $this->assertEqual($attributes['_route'], 'route_a', 'The correct matching route was found.'); + $this->assertEqual($attributes['_route']->getOption('_name'), 'route_a', 'The correct matching route was found.'); $this->assertEqual($attributes['_controller'], 'foo', 'The correct controller was found.'); $this->assertEqual($attributes['value'], 'poink', 'Optional placeholder value used default.'); } diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/HttpMethodMatcherTest.php b/core/modules/system/lib/Drupal/system/Tests/Routing/HttpMethodMatcherTest.php index c98da2e..8055743 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Routing/HttpMethodMatcherTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/HttpMethodMatcherTest.php @@ -44,7 +44,7 @@ function __construct($test_id = NULL) { $this->fixtures = new RoutingFixtures(); } - + /** * Confirms that the HttpMethod matcher matches properly. */ @@ -78,7 +78,7 @@ public function testNestedMatcher() { $attributes = $matcher->matchRequest($request); - $this->assertEqual($attributes['_route'], 'route_a', 'The correct matching route was found.'); + $this->assertEqual($attributes['_route']->getOption('_name'), 'route_a', 'The correct matching route was found.'); } /** diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/NestedMatcherTest.php b/core/modules/system/lib/Drupal/system/Tests/Routing/NestedMatcherTest.php index 444785c..de29538 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Routing/NestedMatcherTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/NestedMatcherTest.php @@ -60,6 +60,6 @@ public function testNestedMatcher() { $attributes = $matcher->matchRequest($request); - $this->assertEqual($attributes['_route'], 'route_a', 'The correct matching route was found.'); + $this->assertEqual($attributes['_route']->getOption('_name'), 'route_a', 'The correct matching route was found.'); } } diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/RouterPermissionTest.php b/core/modules/system/lib/Drupal/system/Tests/Routing/RouterPermissionTest.php new file mode 100644 index 0000000..3050e4f --- /dev/null +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/RouterPermissionTest.php @@ -0,0 +1,56 @@ + 'Router Permission tests', + 'description' => 'Function Tests for the routing permission system.', + 'group' => 'Routing', + ); + } + + /** + * Confirms that the router can get to a controller. + */ + public function testPermissionAccessDenied() { + + $this->drupalGet('router_test/test5'); + $this->assertResponse(403, 'Access denied for a route where we don\'t have a permission'); + } + + /** + * Confirms that our default controller logic works properly. + */ + public function testPermissionAccessPassed() { + + $user = $this->drupalCreateUser(array('access test5')); + + $this->drupalGet('router_test/test5'); + $this->assertRaw('test5', 'The correct string was returned because the route was successful.'); + + } +} diff --git a/core/modules/system/tests/modules/router_test/lib/Drupal/router_test/TestControllers.php b/core/modules/system/tests/modules/router_test/lib/Drupal/router_test/TestControllers.php index fa92fd8..ca6605a 100644 --- a/core/modules/system/tests/modules/router_test/lib/Drupal/router_test/TestControllers.php +++ b/core/modules/system/tests/modules/router_test/lib/Drupal/router_test/TestControllers.php @@ -30,4 +30,8 @@ public function test4($value) { return $value; } + public function test5() { + return new Response('test5'); + } + } diff --git a/core/modules/system/tests/modules/router_test/router_test.module b/core/modules/system/tests/modules/router_test/router_test.module index b3d9bbc..bbaedd0 100644 --- a/core/modules/system/tests/modules/router_test/router_test.module +++ b/core/modules/system/tests/modules/router_test/router_test.module @@ -1 +1,13 @@ array( + 'title' => t('Access test5 route'), + 'description' => t('Test permission only.'), + ), + ); +} diff --git a/core/update.php b/core/update.php index 968e8f4..83c6c10 100644 --- a/core/update.php +++ b/core/update.php @@ -458,6 +458,7 @@ function update_check_requirements($skip_warnings = FALSE) { $container->register('router.dumper', '\Drupal\Core\Routing\MatcherDumper') ->addArgument(new Reference('database')); $container->register('router.builder', 'Drupal\Core\Routing\RouteBuilder') + ->addArgument(new Reference('dispatcher')) ->addArgument(new Reference('router.dumper')) ->addArgument(new Reference('lock'));