diff --git a/core/lib/Drupal/Core/Access/AccessCheckInterface.php b/core/lib/Drupal/Core/Access/AccessCheckInterface.php new file mode 100644 index 0000000..cd382c9 --- /dev/null +++ b/core/lib/Drupal/Core/Access/AccessCheckInterface.php @@ -0,0 +1,41 @@ +request = $request; + } + /** + * Registers a new AccessCheck by service ID. + * + * @param string $service_id + * The ID of the service in the Container that provides a check. + */ + public function addCheckService($service_id) { + $this->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; + } + + /** + * Checks a route against applicable access check services. + * + * Determines whether the route is accessible or not. + * + * @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') ?: array(); + + // No checks == deny by default. + foreach ($checks as $service_id) { + if (empty($this->checks[$service_id])) { + $this->loadCheck($service_id); + } + + $access = $this->checks[$service_id]->access($route, $this->request); + 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..487aa53 --- /dev/null +++ b/core/lib/Drupal/Core/Access/DefaultAccessCheck.php @@ -0,0 +1,32 @@ +getRequirement('_access'); + } + + /** + * Implements Drupal\Core\Access\AccessCheckInterface::access(). + */ + public function access(Route $route, Request $request) { + return $route->getRequirement('_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..9502f8c --- /dev/null +++ b/core/lib/Drupal/Core/Access/PermissionAccessCheck.php @@ -0,0 +1,35 @@ +hasDefault('_permission'); + } + + /** + * Implements Drupal\Core\Access\AccessCheckInterface::access(). + */ + public function access(Route $route, Request $request) { + $permission = $route->getRequirement('_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 48684e5..477ae55 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 Drupal\Core\DependencyInjection\Compiler\RegisterSerializationClassesPass; @@ -105,8 +106,18 @@ 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') + ->addArgument(new Reference('request')) + ->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') @@ -142,6 +153,9 @@ public function build(ContainerBuilder $container) { $container->addCompilerPass(new RegisterKernelListenersPass(), PassConfig::TYPE_AFTER_REMOVING); // Add a compiler pass for adding Normalizers and Encoders to Serializer. $container->addCompilerPass(new RegisterSerializationClassesPass()); + // 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..a69e35a --- /dev/null +++ b/core/lib/Drupal/Core/DependencyInjection/Compiler/RegisterAccessChecksPass.php @@ -0,0 +1,29 @@ +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..d97f392 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\Access\AccessManager; +use Drupal\Core\Routing\RouteBuildEvent; /** * 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\RouteBuildEvent $event + * The event to process. + */ + public function onRoutingRouteAlterSetAccessCheck(RouteBuildEvent $event) { + $this->accessManager->setChecks($event->getRouteCollection()); } /** @@ -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::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/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/modules/rest/lib/Drupal/rest/EventSubscriber/RouteSubscriber.php b/core/modules/rest/lib/Drupal/rest/EventSubscriber/RouteSubscriber.php index f76d535..83fdd63 100644 --- a/core/modules/rest/lib/Drupal/rest/EventSubscriber/RouteSubscriber.php +++ b/core/modules/rest/lib/Drupal/rest/EventSubscriber/RouteSubscriber.php @@ -62,6 +62,7 @@ public function dynamicRoutes(RouteBuildEvent $event) { // @todo Switch to ->addCollection() once http://drupal.org/node/1819018 is resolved. foreach ($plugin->routes() as $name => $route) { + $route->setRequirement('_access', 'TRUE'); $collection->add("rest.$name", $route); } } diff --git a/core/modules/system/lib/Drupal/system/Access/CronAccessCheck.php b/core/modules/system/lib/Drupal/system/Access/CronAccessCheck.php new file mode 100644 index 0000000..bfff5d5 --- /dev/null +++ b/core/modules/system/lib/Drupal/system/Access/CronAccessCheck.php @@ -0,0 +1,41 @@ +getRequirement('_type') == 'cron'; + } + + /** + * Implements Drupal\system\Access\AccessCheckInterface::access(). + */ + public function access(Route $route, Request $request) { + $key = $request->attributes->get('key'); + if ($key != state()->get('system.cron_key')) { + watchdog('cron', 'Cron could not run because an invalid key was used.', array(), WATCHDOG_NOTICE); + return FALSE; + } + elseif (config('system.maintenance')->get('enabled')) { + watchdog('cron', 'Cron could not run because the site is in maintenance mode.', array(), WATCHDOG_NOTICE); + return FALSE; + } + return TRUE; + } +} diff --git a/core/modules/system/lib/Drupal/system/CronController.php b/core/modules/system/lib/Drupal/system/CronController.php new file mode 100644 index 0000000..1996015 --- /dev/null +++ b/core/modules/system/lib/Drupal/system/CronController.php @@ -0,0 +1,31 @@ +register('access_check.cron', 'Drupal\system\Access\CronAccessCheck') + ->addTag('access_check'); + } +} 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..5af6b1b --- /dev/null +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/RouterPermissionTest.php @@ -0,0 +1,63 @@ + '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/test7'); + $this->assertResponse(403, "Access denied for a route where we don't have a permission"); + } + + /** + * Confirms that a router path defaults to access denied. + * + * Unspecified access controls on a route result in an access denied response. + */ + public function testDefaultAccessDenied() { + + $this->drupalGet('router_test/test8'); + $this->assertResponse(403, 'Access denied by default if no access specified'); + } + + /** + * Confirms that our default controller logic works properly. + */ + public function testPermissionAccessPassed() { + + $user = $this->drupalCreateUser(array('access test7')); + + $this->drupalGet('router_test/test7'); + $this->assertRaw('test7', 'The correct string was returned because the route was successful.'); + + } +} diff --git a/core/modules/system/system.module b/core/modules/system/system.module index 6401361..5258008 100644 --- a/core/modules/system/system.module +++ b/core/modules/system/system.module @@ -564,13 +564,6 @@ function system_element_info() { * Implements hook_menu(). */ function system_menu() { - $items['cron/%'] = array( - 'title' => 'Run cron', - 'page callback' => 'system_cron_page', - 'access callback' => 'system_cron_access', - 'access arguments' => array(1), - 'type' => MENU_CALLBACK, - ); $items['system/files'] = array( 'title' => 'File download', 'page callback' => 'file_download', @@ -1077,41 +1070,6 @@ function system_menu() { } /** - * Page callback; Execute cron tasks. - * - * @see system_cron_access(). - */ - -function system_cron_page() { - drupal_page_is_cacheable(FALSE); - drupal_cron_run(); - - // HTTP 204 is "No content", meaning "I did what you asked and we're done." - return new Response('', 204); -} - -/** - * Access callback for system_cron(). - * - * @param string $key - * A hash to validate the page request origin. - * - * @see system_cron_page(). - */ -function system_cron_access($key) { - if ($key != state()->get('system.cron_key')) { - watchdog('cron', 'Cron could not run because an invalid key was used.', array(), WATCHDOG_NOTICE); - return FALSE; - } - elseif (config('system.maintenance')->get('enabled')) { - watchdog('cron', 'Cron could not run because the site is in maintenance mode.', array(), WATCHDOG_NOTICE); - return FALSE; - } - - return TRUE; -} - -/** * Theme callback for the default batch page. */ function _system_batch_theme() { diff --git a/core/modules/system/system.routing.yml b/core/modules/system/system.routing.yml new file mode 100644 index 0000000..05b92ce --- /dev/null +++ b/core/modules/system/system.routing.yml @@ -0,0 +1,6 @@ +cron: + pattern: '/cron/{key}' + defaults: + _controller: '\Drupal\system\CronController::run' + requirements: + _type: 'cron' diff --git a/core/modules/system/tests/modules/router_test/lib/Drupal/router_test/RouteTestSubscriber.php b/core/modules/system/tests/modules/router_test/lib/Drupal/router_test/RouteTestSubscriber.php index a67e83f..4243f81 100644 --- a/core/modules/system/tests/modules/router_test/lib/Drupal/router_test/RouteTestSubscriber.php +++ b/core/modules/system/tests/modules/router_test/lib/Drupal/router_test/RouteTestSubscriber.php @@ -35,6 +35,8 @@ public function dynamicRoutes(RouteBuildEvent $event) { $collection = $event->getRouteCollection(); $route = new Route('/router_test/test5', array( '_content' => '\Drupal\router_test\TestControllers::test5' + ), array( + '_access' => 'TRUE' )); $collection->add('router_test_5', $route); } 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 bcf18b7..e78c11b 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 @@ -34,4 +34,16 @@ public function test5() { return "test5"; } + public function test6() { + return new Response('test6'); + } + + public function test7() { + return new Response('test7'); + } + + public function test8() { + return new Response('test8'); + } + } 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..ffbe070 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 test7 route'), + 'description' => t('Test permission only.'), + ), + ); +} diff --git a/core/modules/system/tests/modules/router_test/router_test.routing.yml b/core/modules/system/tests/modules/router_test/router_test.routing.yml index cc177d3..0b1741e 100644 --- a/core/modules/system/tests/modules/router_test/router_test.routing.yml +++ b/core/modules/system/tests/modules/router_test/router_test.routing.yml @@ -2,24 +2,46 @@ router_test_1: pattern: '/router_test/test1' defaults: _controller: '\Drupal\router_test\TestControllers::test1' + requirements: + _access: 'TRUE' router_test_2: pattern: '/router_test/test2' defaults: _content: '\Drupal\router_test\TestControllers::test2' + requirements: + _access: 'TRUE' router_test_3: pattern: '/router_test/test3/{value}' defaults: _content: '\Drupal\router_test\TestControllers::test3' + requirements: + _access: 'TRUE' router_test_4: pattern: '/router_test/test4/{value}' defaults: _content: '\Drupal\router_test\TestControllers::test4' value: 'narf' + requirements: + _access: 'TRUE' router_test_6: pattern: '/router_test/test6' defaults: _controller: '\Drupal\router_test\TestControllers::test1' + requirements: + _access: 'TRUE' + +router_test_7: + pattern: 'router_test/test7' + defaults: + _controller: '\Drupal\router_test\TestControllers::test7' + requirements: + _permission: 'access test7' + +router_test_8: + pattern: 'router_test/test8' + defaults: + _controller: '\Drupal\router_test\TestControllers::test8'