diff --git a/core/lib/Drupal/Core/Access/AccessCheckInterface.php b/core/lib/Drupal/Core/Access/AccessCheckInterface.php index 0280330c19627751d3596d1bfc904bfc829fd929..172e09503ebbf20c937ee80d8abdb65de87e8fdd 100644 --- a/core/lib/Drupal/Core/Access/AccessCheckInterface.php +++ b/core/lib/Drupal/Core/Access/AccessCheckInterface.php @@ -15,6 +15,31 @@ */ interface AccessCheckInterface { + /** + * Grant access. + * + * A checker should return this value to indicate that it grants access to a + * route. + */ + const ALLOW = TRUE; + + /** + * Deny access. + * + * A checker should return this value to indicate it does not grant access to + * a route. + */ + const DENY = NULL; + + /** + * Block access. + * + * A checker should return this value to indicate that it wants to completely + * block access to this route, regardless of any other access checkers. Most + * checkers should prefer DENY. + */ + const KILL = FALSE; + /** * Declares whether the access check applies to a specific route or not. * diff --git a/core/lib/Drupal/Core/Access/AccessManager.php b/core/lib/Drupal/Core/Access/AccessManager.php index 94dba60e3652e149a6c151643e4fd4dff47aa1cb..d5fd0473f10051c7f7d2c9eb17f1f5eb4d1ecad7 100644 --- a/core/lib/Drupal/Core/Access/AccessManager.php +++ b/core/lib/Drupal/Core/Access/AccessManager.php @@ -14,6 +14,8 @@ /** * Attaches access check services to routes and runs them on request. + * + * @see \Drupal\Tests\Core\Access\AccessManagerTest */ class AccessManager extends ContainerAware { @@ -22,7 +24,7 @@ class AccessManager extends ContainerAware { * * @var array */ - protected $checkIds; + protected $checkIds = array(); /** * Array of access check objects keyed by service id. @@ -89,32 +91,92 @@ protected function applies(Route $route) { * * @param \Symfony\Component\Routing\Route $route * The route to check access to. - * @param \Symfony\Commponent\HttpFoundation\Request $request + * @param \Symfony\Component\HttpFoundation\Request $request * The incoming request object. * + * @return bool + * Returns TRUE if the user has access to the route, otherwise FALSE. + * * @throws \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException * If any access check denies access or none explicitly approve. */ public function check(Route $route, Request $request) { - $access = FALSE; $checks = $route->getOption('_access_checks') ?: array(); - // No checks == deny by default. + $conjunction = $route->getOption('_access_mode') ?: 'ANY'; + + if ($conjunction == 'ALL') { + return $this->checkAll($checks, $route, $request); + } + else { + return $this->checkAny($checks, $route, $request); + } + } + + /** + * Checks access so that every checker should allow access. + * + * @param array $checks + * Contains the list of checks on the route definition. + * @param \Symfony\Component\Routing\Route $route + * The route to check access to. + * @param \Symfony\Component\HttpFoundation\Request $request + * The incoming request object. + * + * @return bool + * Returns TRUE if the user has access to the route, else FALSE. + */ + protected function checkAll(array $checks, Route $route, Request $request) { + $access = FALSE; + foreach ($checks as $service_id) { if (empty($this->checks[$service_id])) { $this->loadCheck($service_id); } $service_access = $this->checks[$service_id]->access($route, $request); - if ($service_access === FALSE) { - // A check has denied access, no need to continue checking. + if ($service_access === AccessCheckInterface::ALLOW) { + $access = TRUE; + } + else { + // On both KILL and DENY stop. $access = FALSE; break; } - elseif ($service_access === TRUE) { - // A check has explicitly granted access, so we need to remember that. + } + + return $access; + } + + /** + * Checks access so that at least one checker should allow access. + * + * @param array $checks + * Contains the list of checks on the route definition. + * @param \Symfony\Component\Routing\Route $route + * The route to check access to. + * @param \Symfony\Component\HttpFoundation\Request $request + * The incoming request object. + * + * @return bool + * Returns TRUE if the user has access to the route, else FALSE. + */ + protected function checkAny(array $checks, $route, $request) { + // No checks == deny by default. + $access = FALSE; + + foreach ($checks as $service_id) { + if (empty($this->checks[$service_id])) { + $this->loadCheck($service_id); + } + + $service_access = $this->checks[$service_id]->access($route, $request); + if ($service_access === AccessCheckinterface::ALLOW) { $access = TRUE; } + if ($service_access === AccessCheckInterface::KILL) { + return FALSE; + } } return $access; diff --git a/core/lib/Drupal/Core/Access/DefaultAccessCheck.php b/core/lib/Drupal/Core/Access/DefaultAccessCheck.php index 074dfd0baa01a970e4476ac0a911d7459eaa51ae..647bae5c5c7496c65b4ca2d47dfa047ed7bf8628 100644 --- a/core/lib/Drupal/Core/Access/DefaultAccessCheck.php +++ b/core/lib/Drupal/Core/Access/DefaultAccessCheck.php @@ -23,9 +23,18 @@ public function applies(Route $route) { } /** - * Implements AccessCheckInterface::access(). + * {@inheritdoc} */ public function access(Route $route, Request $request) { - return $route->getRequirement('_access') === 'TRUE'; + if ($route->getRequirement('_access') === 'TRUE') { + return static::ALLOW; + } + elseif ($route->getRequirement('_access') === 'FALSE') { + return static::KILL; + } + else { + return static::DENY; + } } + } diff --git a/core/modules/system/tests/modules/router_test/lib/Drupal/router_test/Access/DefinedTestAccessCheck.php b/core/modules/system/tests/modules/router_test/lib/Drupal/router_test/Access/DefinedTestAccessCheck.php new file mode 100644 index 0000000000000000000000000000000000000000..f2cc4d999b5fa9f3501a021869f973ffe08306a9 --- /dev/null +++ b/core/modules/system/tests/modules/router_test/lib/Drupal/router_test/Access/DefinedTestAccessCheck.php @@ -0,0 +1,41 @@ +<?php + +/** + * @file + * Contains \Drupal\router_test\Access\DefinedTestAccessCheck. + */ + +namespace Drupal\router_test\Access; + +use Drupal\Core\Access\AccessCheckInterface; +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\Routing\Route; + +/** + * Defines an access checker similar to DefaultAccessCheck + */ +class DefinedTestAccessCheck implements AccessCheckInterface { + + /** + * {@inheritdoc} + */ + public function applies(Route $route) { + return array_key_exists('_test_access', $route->getRequirements()); + } + + /** + * {@inheritdoc} + */ + public function access(Route $route, Request $request) { + if ($route->getRequirement('_test_access') === 'TRUE') { + return static::ALLOW; + } + elseif ($route->getRequirement('_test_access') === 'FALSE') { + return static::KILL; + } + else { + return static::DENY; + } + } + +} diff --git a/core/modules/user/lib/Drupal/user/Access/PermissionAccessCheck.php b/core/modules/user/lib/Drupal/user/Access/PermissionAccessCheck.php index 931818915677999da471d36679997a82f40dea2b..519ba86f760efee1f7ad3d4ca9eaef9c90cc16b5 100644 --- a/core/modules/user/lib/Drupal/user/Access/PermissionAccessCheck.php +++ b/core/modules/user/lib/Drupal/user/Access/PermissionAccessCheck.php @@ -31,6 +31,6 @@ public function access(Route $route, Request $request) { // @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; + return user_access($permission) ? static::ALLOW : static::DENY; } } diff --git a/core/modules/user/lib/Drupal/user/Access/RoleAccessCheck.php b/core/modules/user/lib/Drupal/user/Access/RoleAccessCheck.php index cb44b96bdcfa56614e3fdfa248febc1b7c0145ed..7940bf3aab1956b25f4ae470932a4f907ba1f966 100644 --- a/core/modules/user/lib/Drupal/user/Access/RoleAccessCheck.php +++ b/core/modules/user/lib/Drupal/user/Access/RoleAccessCheck.php @@ -42,19 +42,19 @@ public function access(Route $route, Request $request) { if (count($explode_and) > 1) { $diff = array_diff($explode_and, $account->roles); if (empty($diff)) { - return TRUE; + return static::ALLOW; } } else { $explode_or = array_filter(array_map('trim', explode(',', $rid_string))); $intersection = array_intersect($explode_or, $account->roles); if (!empty($intersection)) { - return TRUE; + return static::ALLOW; } } // If there is no allowed role, return NULL to give other checks a chance. - return NULL; + return static::DENY; } } diff --git a/core/tests/Drupal/Tests/Core/Access/AccessManagerTest.php b/core/tests/Drupal/Tests/Core/Access/AccessManagerTest.php new file mode 100644 index 0000000000000000000000000000000000000000..2212eef16a6829d2060024651d14298151d3e57d --- /dev/null +++ b/core/tests/Drupal/Tests/Core/Access/AccessManagerTest.php @@ -0,0 +1,277 @@ +<?php + +/** + * @file + * Contains \Drupal\Tests\Core\Access\AccessManagerTest. + */ + +namespace Drupal\Tests\Core\Access; + +use Drupal\Core\Access\AccessCheckInterface; +use Drupal\Core\Access\AccessManager; +use Drupal\Core\Access\DefaultAccessCheck; +use Drupal\Tests\UnitTestCase; +use Drupal\router_test\Access\DefinedTestAccessCheck; +use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\Routing\Route; +use Symfony\Component\Routing\RouteCollection; + +/** + * Tests the access manager. + * + * @see \Drupal\Core\Access\AccessManager + */ +class AccessManagerTest extends UnitTestCase { + + /** + * The dependency injection container. + * + * @var \Symfony\Component\DependencyInjection\ContainerBuilder + */ + protected $container; + + /** + * The collection of routes, which are tested. + * + * @var \Symfony\Component\Routing\RouteCollection + */ + protected $routeCollection; + + /** + * The access manager to test. + * + * @var \Drupal\Core\Access\AccessManager + */ + protected $accessManager; + + public static function getInfo() { + return array( + 'name' => 'Access manager tests', + 'description' => 'Test for the AccessManager object.', + 'group' => 'Routing', + ); + } + + /** + * {@inheritdoc} + */ + protected function setUp() { + parent::setUp(); + + $this->container = new ContainerBuilder(); + $this->accessManager = new AccessManager(); + $this->accessManager->setContainer($this->container); + + $this->routeCollection = new RouteCollection(); + $this->routeCollection->add('test_route_1', new Route('/test-route-1')); + $this->routeCollection->add('test_route_2', new Route('/test-route-2', array(), array('_access' => 'TRUE'))); + $this->routeCollection->add('test_route_3', new Route('/test-route-3', array(), array('_access' => 'FALSE'))); + } + + /** + * Tests \Drupal\Core\Access\AccessManager::setChecks(). + */ + public function testSetChecks() { + // Check setChecks without any access checker defined yet. + $this->accessManager->setChecks($this->routeCollection); + + foreach ($this->routeCollection->all() as $route) { + $this->assertNull($route->getOption('_access_checks')); + } + + + $this->setupAccessChecker(); + + $this->accessManager->setChecks($this->routeCollection); + + $this->assertEquals($this->routeCollection->get('test_route_1')->getOption('_access_checks'), NULL); + $this->assertEquals($this->routeCollection->get('test_route_2')->getOption('_access_checks'), array('test_access_default')); + $this->assertEquals($this->routeCollection->get('test_route_3')->getOption('_access_checks'), array('test_access_default')); + } + + /** + * Tests \Drupal\Core\Access\AccessManager::check(). + */ + public function testCheck() { + $request = new Request(); + + // Check check without any access checker defined yet. + foreach ($this->routeCollection->all() as $route) { + $this->assertFalse($this->accessManager->check($route, $request)); + } + + $this->setupAccessChecker(); + + // An access checker got setup, but the routes haven't been setup using + // setChecks. + foreach ($this->routeCollection->all() as $route) { + $this->assertFalse($this->accessManager->check($route, $request)); + } + + $this->accessManager->setChecks($this->routeCollection); + + $this->assertFalse($this->accessManager->check($this->routeCollection->get('test_route_1'), $request)); + $this->assertTrue($this->accessManager->check($this->routeCollection->get('test_route_2'), $request)); + $this->assertFalse($this->accessManager->check($this->routeCollection->get('test_route_3'), $request)); + } + + /** + * Provides data for the conjunction test. + * + * @return array + * An array of data for check conjunctions. + * + * @see \Drupal\Tests\Core\Access\AccessManagerTest::testCheckConjunctions() + */ + public function providerTestCheckConjunctions() { + $access_configurations = array(); + $access_configurations[] = array( + 'conjunction' => 'ALL', + 'name' => 'test_route_4', + 'condition_one' => AccessCheckInterface::ALLOW, + 'condition_two' => AccessCheckInterface::KILL, + 'expected' => FALSE, + ); + $access_configurations[] = array( + 'conjunction' => 'ALL', + 'name' => 'test_route_5', + 'condition_one' => AccessCheckInterface::ALLOW, + 'condition_two' => AccessCheckInterface::DENY, + 'expected' => FALSE, + ); + $access_configurations[] = array( + 'conjunction' => 'ALL', + 'name' => 'test_route_6', + 'condition_one' => AccessCheckInterface::KILL, + 'condition_two' => AccessCheckInterface::DENY, + 'expected' => FALSE, + ); + $access_configurations[] = array( + 'conjunction' => 'ALL', + 'name' => 'test_route_7', + 'condition_one' => AccessCheckInterface::ALLOW, + 'condition_two' => AccessCheckInterface::ALLOW, + 'expected' => TRUE, + ); + $access_configurations[] = array( + 'conjunction' => 'ALL', + 'name' => 'test_route_8', + 'condition_one' => AccessCheckInterface::KILL, + 'condition_two' => AccessCheckInterface::KILL, + 'expected' => FALSE, + ); + $access_configurations[] = array( + 'conjunction' => 'ALL', + 'name' => 'test_route_9', + 'condition_one' => AccessCheckInterface::DENY, + 'condition_two' => AccessCheckInterface::DENY, + 'expected' => FALSE, + ); + $access_configurations[] = array( + 'conjunction' => 'ANY', + 'name' => 'test_route_10', + 'condition_one' => AccessCheckInterface::ALLOW, + 'condition_two' => AccessCheckInterface::KILL, + 'expected' => FALSE, + ); + $access_configurations[] = array( + 'conjunction' => 'ANY', + 'name' => 'test_route_11', + 'condition_one' => AccessCheckInterface::ALLOW, + 'condition_two' => AccessCheckInterface::DENY, + 'expected' => TRUE, + ); + $access_configurations[] = array( + 'conjunction' => 'ANY', + 'name' => 'test_route_12', + 'condition_one' => AccessCheckInterface::KILL, + 'condition_two' => AccessCheckInterface::DENY, + 'expected' => FALSE, + ); + $access_configurations[] = array( + 'conjunction' => 'ANY', + 'name' => 'test_route_13', + 'condition_one' => AccessCheckInterface::ALLOW, + 'condition_two' => AccessCheckInterface::ALLOW, + 'expected' => TRUE, + ); + $access_configurations[] = array( + 'conjunction' => 'ANY', + 'name' => 'test_route_14', + 'condition_one' => AccessCheckInterface::KILL, + 'condition_two' => AccessCheckInterface::KILL, + 'expected' => FALSE, + ); + $access_configurations[] = array( + 'conjunction' => 'ANY', + 'name' => 'test_route_15', + 'condition_one' => AccessCheckInterface::DENY, + 'condition_two' => AccessCheckInterface::DENY, + 'expected' => FALSE, + ); + + return $access_configurations; + } + + /** + * Test \Drupal\Core\Access\AccessManager::check() with conjunctions. + * + * @dataProvider providerTestCheckConjunctions + */ + public function testCheckConjunctions($conjunction, $name, $condition_one, $condition_two, $expected_access) { + $this->setupAccessChecker(); + $access_check = new DefinedTestAccessCheck(); + $this->container->register('test_access_defined', $access_check); + $this->accessManager->addCheckService('test_access_defined'); + + $request = new Request(); + + $route_collection = new RouteCollection(); + // Setup a test route for each access configuration. + $requirements = array( + '_access' => static::convertAccessCheckInterfaceToString($condition_one), + '_test_access' => static::convertAccessCheckInterfaceToString($condition_two), + ); + $options = array('_access_mode' => $conjunction); + $route = new Route($name, array(), $requirements, $options); + $route_collection->add($name, $route); + + $this->accessManager->setChecks($route_collection); + $this->assertSame($this->accessManager->check($route, $request), $expected_access); + } + + /** + * Converts AccessCheckInterface constants to a string. + * + * @param mixed $constant + * The access constant which is tested, so either + * AccessCheckInterface::ALLOW, AccessCheckInterface::DENY OR + * AccessCheckInterface::KILL. + * + * @return string + * The corresponding string used in route requirements, so 'TRUE', 'FALSE' + * or 'NULL'. + */ + protected static function convertAccessCheckInterfaceToString($constant) { + if ($constant === AccessCheckInterface::ALLOW) { + return 'TRUE'; + } + if ($constant === AccessCheckInterface::DENY) { + return 'NULL'; + } + if ($constant === AccessCheckInterface::KILL) { + return 'FALSE'; + } + } + + /** + * Adds a default access check service to the container and the access manager. + */ + protected function setupAccessChecker() { + $access_check = new DefaultAccessCheck(); + $this->container->register('test_access_default', $access_check); + $this->accessManager->addCheckService('test_access_default'); + } + +} diff --git a/core/tests/Drupal/Tests/Core/Access/DefaultAccessCheckTest.php b/core/tests/Drupal/Tests/Core/Access/DefaultAccessCheckTest.php index d96825c004cfaafe3dde6ad650adc5b4e950fe6b..bb96686443060277a08630d908e0eed3f3d60dd7 100644 --- a/core/tests/Drupal/Tests/Core/Access/DefaultAccessCheckTest.php +++ b/core/tests/Drupal/Tests/Core/Access/DefaultAccessCheckTest.php @@ -62,13 +62,15 @@ public function testApplies() { * Test the access method. */ public function testAccess() { - $route = new Route('/test-route'); $request = new Request(array()); - $route->addRequirements(array('_access' => 'FALSE')); + $route = new Route('/test-route', array(), array('_access' => 'NULL')); + $this->assertNull($this->accessChecker->access($route, $request)); + + $route = new Route('/test-route', array(), array('_access' => 'FALSE')); $this->assertFalse($this->accessChecker->access($route, $request)); - $route->addRequirements(array('_access' => 'TRUE')); + $route = new Route('/test-route', array(), array('_access' => 'TRUE')); $this->assertTrue($this->accessChecker->access($route, $request)); } diff --git a/core/tests/Drupal/Tests/Core/Route/RoleAccessCheckTest.php b/core/tests/Drupal/Tests/Core/Route/RoleAccessCheckTest.php index b072412ade3d79c06fdb2559439148ff203bfa1c..b6e5ab53a1a98d1c21c8c03b069c5b79dc3cd072 100644 --- a/core/tests/Drupal/Tests/Core/Route/RoleAccessCheckTest.php +++ b/core/tests/Drupal/Tests/Core/Route/RoleAccessCheckTest.php @@ -7,6 +7,7 @@ namespace Drupal\Tests\Core\Route; +use Drupal\Core\Access\AccessCheckInterface; use Drupal\Core\DependencyInjection\ContainerBuilder; use Drupal\Tests\UnitTestCase; use Drupal\user\Access\RoleAccessCheck; @@ -163,7 +164,7 @@ public function testRoleAccess($path, $grant_accounts, $deny_accounts) { $subrequest = Request::create($path, 'GET'); $message = sprintf('Access granted for user with the roles %s on path: %s', implode(', ', $account->roles), $path); - $this->assertTrue($role_access_check->access($collection->get($path), $subrequest), $message); + $this->assertSame(AccessCheckInterface::ALLOW, $role_access_check->access($collection->get($path), $subrequest), $message); } // Check all users which don't have access. @@ -173,7 +174,7 @@ public function testRoleAccess($path, $grant_accounts, $deny_accounts) { $subrequest = Request::create($path, 'GET'); $message = sprintf('Access denied for user %s with the roles %s on path: %s', $account->uid, implode(', ', $account->roles), $path); $has_access = $role_access_check->access($collection->get($path), $subrequest); - $this->assertEmpty($has_access , $message); + $this->assertSame(AccessCheckInterface::DENY, $has_access , $message); } }