diff --git a/core/lib/Drupal/Core/Access/AccessException.php b/core/lib/Drupal/Core/Access/AccessException.php new file mode 100644 index 0000000..a44738e --- /dev/null +++ b/core/lib/Drupal/Core/Access/AccessException.php @@ -0,0 +1,14 @@ +checks[$service_id]->access($route, $request, $account); + + if (!in_array($service_access, array(AccessInterface::ALLOW, AccessInterface::DENY, AccessInterface::KILL), TRUE)) { + throw new AccessException('Access services can only return AccessInterface::ALLOW, AccessInterface::DENY, or AccessInterface::KILL constants.'); + } + if ($service_access === AccessInterface::ALLOW) { $access = TRUE; } @@ -299,6 +304,11 @@ protected function checkAny(array $checks, $route, $request, AccountInterface $a } $service_access = $this->checks[$service_id]->access($route, $request, $account); + + if (!in_array($service_access, array(AccessInterface::ALLOW, AccessInterface::DENY, AccessInterface::KILL), TRUE)) { + throw new AccessException('Access services can only return AccessInterface::ALLOW, AccessInterface::DENY, or AccessInterface::KILL constants.'); + } + if ($service_access === AccessInterface::ALLOW) { $access = TRUE; } diff --git a/core/modules/field_ui/lib/Drupal/field_ui/Access/ViewModeAccessCheck.php b/core/modules/field_ui/lib/Drupal/field_ui/Access/ViewModeAccessCheck.php index 2fe350b..95d78fa 100644 --- a/core/modules/field_ui/lib/Drupal/field_ui/Access/ViewModeAccessCheck.php +++ b/core/modules/field_ui/lib/Drupal/field_ui/Access/ViewModeAccessCheck.php @@ -47,6 +47,8 @@ public function access(Route $route, Request $request, AccountInterface $account return $account->hasPermission($permission) ? static::ALLOW : static::DENY; } } + + return static::DENY; } } diff --git a/core/modules/node/lib/Drupal/node/Access/NodeRevisionAccessCheck.php b/core/modules/node/lib/Drupal/node/Access/NodeRevisionAccessCheck.php index 096335f..02ab949 100644 --- a/core/modules/node/lib/Drupal/node/Access/NodeRevisionAccessCheck.php +++ b/core/modules/node/lib/Drupal/node/Access/NodeRevisionAccessCheck.php @@ -136,7 +136,7 @@ public function checkAccess(NodeInterface $node, AccountInterface $account, $op if (!isset($this->access[$cid])) { // Perform basic permission checks first. if (!user_access($map[$op], $account) && !user_access($type_map[$op], $account) && !user_access('administer nodes', $account)) { - return $this->access[$cid] = FALSE; + $this->access[$cid] = FALSE; } // There should be at least two revisions. If the vid of the given node diff --git a/core/modules/tracker/lib/Drupal/tracker/Access/ViewOwnTrackerAccessCheck.php b/core/modules/tracker/lib/Drupal/tracker/Access/ViewOwnTrackerAccessCheck.php index 18498f9..11f42c1 100644 --- a/core/modules/tracker/lib/Drupal/tracker/Access/ViewOwnTrackerAccessCheck.php +++ b/core/modules/tracker/lib/Drupal/tracker/Access/ViewOwnTrackerAccessCheck.php @@ -30,7 +30,7 @@ public function appliesTo() { public function access(Route $route, Request $request, AccountInterface $account) { // The user object from the User ID in the path. $user = $request->attributes->get('user'); - return $user && $account->isAuthenticated() && ($user->id() == $account->id()); + return ($user && $account->isAuthenticated() && ($user->id() == $account->id())) ? static::ALLOW : static::DENY; } } diff --git a/core/tests/Drupal/Tests/Core/Access/AccessManagerTest.php b/core/tests/Drupal/Tests/Core/Access/AccessManagerTest.php index c7ae50a..740191c 100644 --- a/core/tests/Drupal/Tests/Core/Access/AccessManagerTest.php +++ b/core/tests/Drupal/Tests/Core/Access/AccessManagerTest.php @@ -11,10 +11,8 @@ use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Access\AccessManager; use Drupal\Core\Access\DefaultAccessCheck; -use Drupal\system\Tests\Routing\MockRouteProvider; use Drupal\Tests\UnitTestCase; use Drupal\router_test\Access\DefinedTestAccessCheck; -use Symfony\Cmf\Component\Routing\RouteObjectInterface; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Routing\Exception\RouteNotFoundException; @@ -469,6 +467,70 @@ public function testCheckNamedRouteWithNonExistingRoute() { } /** + * Tests that an access checker throws an exception for not allowed values. + * + * @dataProvider providerCheckException + * + * @expectedException \Drupal\Core\Access\AccessException + */ + public function testCheckException($return_value, $access_mode) { + $route_provider = $this->getMock('Drupal\Core\Routing\RouteProviderInterface'); + + // Setup a test route for each access configuration. + $requirements = array( + '_test_incorrect_value' => 'TRUE', + ); + $options = array( + '_access_mode' => $access_mode, + '_access_checks' => array( + 'test_incorrect_value', + ), + ); + $route = new Route('', array(), $requirements, $options); + + $route_provider->expects($this->any()) + ->method('getRouteByName') + ->will($this->returnValue($route)); + + $request = new Request(); + + $container = new ContainerBuilder(); + + // Register a service that will return an incorrect value. + $access_check = $this->getMock('Drupal\Core\Routing\Access\AccessInterface'); + $access_check->expects($this->any()) + ->method('access') + ->will($this->returnValue($return_value)); + $container->set('test_incorrect_value', $access_check); + + $access_manager = new AccessManager($route_provider, $this->urlGenerator, $this->paramConverter); + $access_manager->setContainer($container); + $access_manager->addCheckService('test_incorrect_value'); + + $access_manager->checkNamedRoute('test_incorrect_value', array(), $this->account, $request); + } + + /** + * Data provider for testCheckException. + * + * @return array + */ + public function providerCheckException() { + return array( + array(array(), 'ALL'), + array(array(), 'ANY'), + array(array(1), 'ALL'), + array(array(1), 'ANY'), + array('string', 'ALL'), + array('string', 'ANY'), + array(0, 'ALL'), + array(0, 'ANY'), + array(1, 'ALL'), + array(1, 'ANY'), + ); + } + + /** * Converts AccessCheckInterface constants to a string. * * @param mixed $constant