Commit 9c2b19cd authored by alexpott's avatar alexpott

Issue #2431281 by dawehner: Drop support for _access_mode routes and always assume ALL

parent ca35085f
......@@ -126,7 +126,6 @@ public function check(RouteMatchInterface $route_match, AccountInterface $accoun
}
$route = $route_match->getRouteObject();
$checks = $route->getOption('_access_checks') ?: array();
$conjunction = $route->getOption('_access_mode') ?: static::ACCESS_MODE_ALL;
// Filter out checks which require the incoming request.
if (!isset($request)) {
......@@ -136,65 +135,18 @@ public function check(RouteMatchInterface $route_match, AccountInterface $accoun
$result = AccessResult::neutral();
if (!empty($checks)) {
$arguments_resolver = $this->argumentsResolverFactory->getArgumentsResolver($route_match, $account, $request);
if ($conjunction == static::ACCESS_MODE_ALL) {
$result = $this->checkAll($checks, $arguments_resolver);
if (!$checks) {
return AccessResult::neutral();
}
else {
$result = $this->checkAny($checks, $arguments_resolver);
$result = AccessResult::allowed();
foreach ($checks as $service_id) {
$result = $result->andIf($this->performCheck($service_id, $arguments_resolver));
}
}
return $return_as_object ? $result : $result->isAllowed();
}
/**
* Checks access so that every checker should allow access.
*
* @param array $checks
* Contains the list of checks on the route definition.
* @param \Drupal\Component\Utility\ArgumentsResolverInterface $arguments_resolver
* The parametrized arguments resolver instance.
*
* @return \Drupal\Core\Access\AccessResultInterface
* The access result.
*
* @see \Drupal\Core\Access\AccessResultInterface::andIf()
*/
protected function checkAll(array $checks, ArgumentsResolverInterface $arguments_resolver) {
// Without checks no opinion can be formed.
if (!$checks) {
return AccessResult::neutral();
}
$result = AccessResult::allowed();
foreach ($checks as $service_id) {
$result = $result->andIf($this->performCheck($service_id, $arguments_resolver));
}
return $result;
}
/**
* Checks access so that at least one checker should allow access.
*
* @param array $checks
* Contains the list of checks on the route definition.
* @param \Drupal\Component\Utility\ArgumentsResolverInterface $arguments_resolver
* The parametrized arguments resolver instance.
*
* @return \Drupal\Core\Access\AccessResultInterface
* The access result.
*
* @see \Drupal\Core\Access\AccessResultInterface::orIf()
*/
protected function checkAny(array $checks, ArgumentsResolverInterface $arguments_resolver) {
// No opinion by default.
$result = AccessResult::neutral();
foreach ($checks as $service_id) {
$result = $result->orIf($this->performCheck($service_id, $arguments_resolver));
}
return $result;
}
/**
* Performs the specified access check.
*
......
......@@ -16,21 +16,6 @@
*/
interface AccessManagerInterface {
/**
* All access checkers must return an AccessResultInterface object where
* ::isAllowed() is TRUE.
*
* self::ACCESS_MODE_ALL is the default behavior.
*/
const ACCESS_MODE_ALL = 'ALL';
/**
* At least one access checker must return an AccessResultInterface object
* where ::isAllowed() is TRUE and none may return one where ::isForbidden()
* is TRUE.
*/
const ACCESS_MODE_ANY = 'ANY';
/**
* Checks a named route with parameters against applicable access check services.
*
......
......@@ -57,7 +57,6 @@ entity.node.book_remove_form:
_form: '\Drupal\book\Form\BookRemoveForm'
_title: 'Remove from outline'
options:
_access_mode: 'ALL'
_node_operation_route: TRUE
requirements:
_permission: 'administer book outlines'
......
......@@ -74,6 +74,10 @@ public function access(Route $route, RouteMatchInterface $route_match, AccountIn
/* @var \Drupal\Core\Entity\ContentEntityInterface $entity */
if ($entity = $route_match->getParameter($entity_type_id)) {
if ($account->hasPermission('translate any entity')) {
return AccessResult::allowed()->cachePerRole();
}
$operation = $route->getRequirement('_access_content_translation_manage');
/* @var \Drupal\content_translation\ContentTranslationHandlerInterface $handler */
......
......@@ -94,11 +94,9 @@ protected function alterRoutes(RouteCollection $collection) {
),
array(
'_permission' => 'translate any entity',
'_access_content_translation_manage' => 'create',
),
array(
'_access_mode' => AccessManagerInterface::ACCESS_MODE_ANY,
'parameters' => array(
'source' => array(
'type' => 'language',
......@@ -124,11 +122,9 @@ protected function alterRoutes(RouteCollection $collection) {
'entity_type_id' => $entity_type_id,
),
array(
'_permission' => 'translate any entity',
'_access_content_translation_manage' => 'update',
),
array(
'_access_mode' => AccessManagerInterface::ACCESS_MODE_ANY,
'parameters' => array(
'language' => array(
'type' => 'language',
......@@ -151,7 +147,6 @@ protected function alterRoutes(RouteCollection $collection) {
'entity_type_id' => $entity_type_id,
),
array(
'_permission' => 'translate any entity',
'_access_content_translation_manage' => 'delete',
),
array(
......@@ -163,7 +158,6 @@ protected function alterRoutes(RouteCollection $collection) {
'type' => 'entity:' . $entity_type_id,
),
),
'_access_mode' => AccessManagerInterface::ACCESS_MODE_ANY,
'_admin_route' => $is_admin,
)
);
......
......@@ -11,10 +11,8 @@ node.add_page:
_title: 'Add content'
_controller: '\Drupal\node\Controller\NodeController::addPage'
options:
_access_mode: 'ANY'
_node_operation_route: TRUE
requirements:
_permission: 'administer content types'
_node_add_access: 'node'
node.add:
......
......@@ -50,6 +50,9 @@ public function __construct(EntityManagerInterface $entity_manager) {
public function access(AccountInterface $account, NodeTypeInterface $node_type = NULL) {
$access_control_handler = $this->entityManager->getAccessControlHandler('node');
// If checking whether a node of a particular type may be created.
if ($account->hasPermission('administer content types')) {
return AccessResult::allowed()->cachePerRole();
}
if ($node_type) {
return $access_control_handler->createAccess($node_type->id(), $account, [], TRUE);
}
......
......@@ -19,7 +19,6 @@ quickedit.field_form:
defaults:
_controller: '\Drupal\quickedit\QuickEditController::fieldForm'
options:
_access_mode: 'ALL'
_theme: ajax_base_page
parameters:
entity:
......
......@@ -206,12 +206,6 @@ protected function getBaseRoute($canonical_path, $method) {
// The HTTP method is a requirement for this route.
'_method' => $method,
'_permission' => "restful $lower_method $this->pluginId",
), array(
// All access restrictions on this route must grant access because the
// permission AND the CSRF protection added in
// \Drupal\rest\Routing\ResourceRoutes::alterRoutes() must be taken into
// account.
'_access_mode' => AccessManagerInterface::ACCESS_MODE_ALL,
));
return $route;
}
......
......@@ -42,10 +42,5 @@ public function testPermissionAccess() {
$this->assertResponse(200);
$this->assertNoRaw('Access denied');
$this->assertRaw('test7text', 'The correct string was returned because the route was successful.');
$this->drupalGet('router_test/test9');
$this->assertResponse(200);
$this->assertNoRaw('Access denied');
$this->assertRaw('test8', 'The correct string was returned because the route was successful.');
}
}
......@@ -46,16 +46,6 @@ router_test.8:
defaults:
_controller: '\Drupal\router_test\TestControllers::test8'
router_test.9:
path: '/router_test/test9'
options:
_access_mode: 'ANY'
defaults:
_controller: '\Drupal\router_test\TestControllers::test8'
requirements:
_permission: 'access test7'
_access_router_test: 'TRUE'
router_test.10:
path: '/router_test/test10'
defaults:
......
......@@ -11,8 +11,6 @@ tracker.users_recent_content:
defaults:
_controller: '\Drupal\tracker\Controller\TrackerUserRecent::getContent'
_title: 'My recent content'
options:
_access_mode: 'ALL'
requirements:
_permission: 'access content'
_access_tracker_own_information: 'TRUE'
......@@ -22,8 +20,6 @@ tracker.user_tab:
defaults:
_controller: '\Drupal\tracker\Controller\TrackerUserTab::getContent'
_title_callback: '\Drupal\tracker\Controller\TrackerUserTab::getTitle'
options:
_access_mode: 'ALL'
requirements:
_permission: 'access content'
_entity_access: 'user.view'
......
......@@ -27,8 +27,6 @@ update.report_install:
defaults:
_form: '\Drupal\update\Form\UpdateManagerInstall'
_title: 'Install'
options:
_access_mode: 'ALL'
requirements:
_permission: 'administer software updates'
_access_update_manager: 'TRUE'
......@@ -38,8 +36,6 @@ update.report_update:
defaults:
_form: '\Drupal\update\Form\UpdateManagerUpdate'
_title: 'Update'
options:
_access_mode: 'ALL'
requirements:
_permission: 'administer software updates'
_access_update_manager: 'TRUE'
......@@ -49,8 +45,6 @@ update.module_install:
defaults:
_form: '\Drupal\update\Form\UpdateManagerInstall'
_title: 'Install new module'
options:
_access_mode: 'ALL'
requirements:
_permission: 'administer software updates'
_access_update_manager: 'TRUE'
......@@ -60,8 +54,6 @@ update.module_update:
defaults:
_form: '\Drupal\update\Form\UpdateManagerUpdate'
_title: 'Update'
options:
_access_mode: 'ALL'
requirements:
_permission: 'administer software updates'
_access_update_manager: 'TRUE'
......@@ -71,8 +63,6 @@ update.theme_install:
defaults:
_form: '\Drupal\update\Form\UpdateManagerInstall'
_title: 'Install new theme'
options:
_access_mode: 'ALL'
requirements:
_permission: 'administer software updates'
_access_update_manager: 'TRUE'
......@@ -82,8 +72,6 @@ update.theme_update:
defaults:
_form: '\Drupal\update\Form\UpdateManagerUpdate'
_title: 'Update'
options:
_access_mode: 'ALL'
requirements:
_permission: 'administer software updates'
_access_update_manager: 'TRUE'
......@@ -93,8 +81,6 @@ update.confirmation_page:
defaults:
_form: '\Drupal\update\Form\UpdateReady'
_title: 'Ready to update'
options:
_access_mode: 'ALL'
requirements:
_permission: 'administer software updates'
_access_update_manager: 'TRUE'
......@@ -253,131 +253,41 @@ public function providerTestCheckConjunctions() {
$access_configurations = array();
$access_configurations[] = array(
'conjunction' => AccessManagerInterface::ACCESS_MODE_ALL,
'name' => 'test_route_4',
'condition_one' => 'TRUE',
'condition_two' => 'FALSE',
'expected' => $access_kill,
);
$access_configurations[] = array(
'conjunction' => NULL,
'name' => 'test_route_4',
'condition_one' => 'TRUE',
'condition_two' => 'FALSE',
'expected' => $access_kill,
);
$access_configurations[] = array(
'conjunction' => AccessManagerInterface::ACCESS_MODE_ALL,
'name' => 'test_route_5',
'condition_one' => 'TRUE',
'condition_two' => 'NULL',
'expected' => $access_deny,
);
$access_configurations[] = array(
'conjunction' => NULL,
'name' => 'test_route_5',
'condition_one' => 'TRUE',
'condition_two' => 'NULL',
'expected' => $access_deny,
);
$access_configurations[] = array(
'conjunction' => AccessManagerInterface::ACCESS_MODE_ALL,
'name' => 'test_route_6',
'condition_one' => 'FALSE',
'condition_two' => 'NULL',
'expected' => $access_kill,
);
$access_configurations[] = array(
'conjunction' => NULL,
'name' => 'test_route_6',
'condition_one' => 'FALSE',
'condition_two' => 'NULL',
'expected' => $access_kill,
);
$access_configurations[] = array(
'conjunction' => AccessManagerInterface::ACCESS_MODE_ALL,
'name' => 'test_route_7',
'condition_one' => 'TRUE',
'condition_two' => 'TRUE',
'expected' => $access_allow,
);
$access_configurations[] = array(
'conjunction' => NULL,
'name' => 'test_route_7',
'condition_one' => 'TRUE',
'condition_two' => 'TRUE',
'expected' => $access_allow,
);
$access_configurations[] = array(
'conjunction' => AccessManagerInterface::ACCESS_MODE_ALL,
'name' => 'test_route_8',
'condition_one' => 'FALSE',
'condition_two' => 'FALSE',
'expected' => $access_kill,
);
$access_configurations[] = array(
'conjunction' => NULL,
'name' => 'test_route_8',
'condition_one' => 'FALSE',
'condition_two' => 'FALSE',
'expected' => $access_kill,
);
$access_configurations[] = array(
'conjunction' => AccessManagerInterface::ACCESS_MODE_ALL,
'name' => 'test_route_9',
'condition_one' => 'NULL',
'condition_two' => 'NULL',
'expected' => $access_deny,
);
$access_configurations[] = array(
'conjunction' => NULL,
'name' => 'test_route_9',
'condition_one' => 'NULL',
'condition_two' => 'NULL',
'expected' => $access_deny,
);
$access_configurations[] = array(
'conjunction' => AccessManagerInterface::ACCESS_MODE_ANY,
'name' => 'test_route_10',
'condition_one' => 'TRUE',
'condition_two' => 'FALSE',
'expected' => $access_kill,
);
$access_configurations[] = array(
'conjunction' => AccessManagerInterface::ACCESS_MODE_ANY,
'name' => 'test_route_11',
'condition_one' => 'TRUE',
'condition_two' => 'NULL',
'expected' => $access_allow,
);
$access_configurations[] = array(
'conjunction' => AccessManagerInterface::ACCESS_MODE_ANY,
'name' => 'test_route_12',
'condition_one' => 'FALSE',
'condition_two' => 'NULL',
'expected' => $access_kill,
);
$access_configurations[] = array(
'conjunction' => AccessManagerInterface::ACCESS_MODE_ANY,
'name' => 'test_route_13',
'condition_one' => 'TRUE',
'condition_two' => 'TRUE',
'expected' => $access_allow,
);
$access_configurations[] = array(
'conjunction' => AccessManagerInterface::ACCESS_MODE_ANY,
'name' => 'test_route_14',
'condition_one' => 'FALSE',
'condition_two' => 'FALSE',
'expected' => $access_kill,
);
$access_configurations[] = array(
'conjunction' => AccessManagerInterface::ACCESS_MODE_ANY,
'name' => 'test_route_15',
'condition_one' => 'NULL',
'condition_two' => 'NULL',
'expected' => $access_deny,
);
return $access_configurations;
}
......@@ -387,7 +297,7 @@ public function providerTestCheckConjunctions() {
*
* @dataProvider providerTestCheckConjunctions
*/
public function testCheckConjunctions($conjunction, $name, $condition_one, $condition_two, $expected_access) {
public function testCheckConjunctions($name, $condition_one, $condition_two, $expected_access) {
$this->setupAccessChecker();
$access_check = new DefinedTestAccessCheck();
$this->container->register('test_access_defined', $access_check);
......@@ -399,8 +309,7 @@ public function testCheckConjunctions($conjunction, $name, $condition_one, $cond
'_access' => $condition_one,
'_test_access' => $condition_two,
);
$options = $conjunction ? array('_access_mode' => $conjunction) : array();
$route = new Route($name, array(), $requirements, $options);
$route = new Route($name, array(), $requirements);
$route_collection->add($name, $route);
$this->checkProvider->setChecks($route_collection);
......@@ -565,7 +474,7 @@ public function testCheckNamedRouteWithNonExistingRoute() {
*
* @expectedException \Drupal\Core\Access\AccessException
*/
public function testCheckException($return_value, $access_mode) {
public function testCheckException($return_value) {
$route_provider = $this->getMock('Drupal\Core\Routing\RouteProviderInterface');
// Setup a test route for each access configuration.
......@@ -573,7 +482,6 @@ public function testCheckException($return_value, $access_mode) {
'_test_incorrect_value' => 'TRUE',
);
$options = array(
'_access_mode' => $access_mode,
'_access_checks' => array(
'test_incorrect_value',
),
......@@ -614,16 +522,10 @@ public function testCheckException($return_value, $access_mode) {
*/
public function providerCheckException() {
return array(
array(array(), AccessManagerInterface::ACCESS_MODE_ALL),
array(array(), AccessManagerInterface::ACCESS_MODE_ANY),
array(array(1), AccessManagerInterface::ACCESS_MODE_ALL),
array(array(1), AccessManagerInterface::ACCESS_MODE_ANY),
array('string', AccessManagerInterface::ACCESS_MODE_ALL),
array('string', AccessManagerInterface::ACCESS_MODE_ANY),
array(0, AccessManagerInterface::ACCESS_MODE_ALL),
array(0, AccessManagerInterface::ACCESS_MODE_ANY),
array(1, AccessManagerInterface::ACCESS_MODE_ALL),
array(1, AccessManagerInterface::ACCESS_MODE_ANY),
array(array(1)),
array('string'),
array(0),
array(1),
);
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment