core/includes/form.inc | 2 +- core/lib/Drupal/Core/Access/AccessCheckResult.php | 87 +++++++++++++ core/lib/Drupal/Core/Access/AccessManager.php | 78 +++++++----- .../Drupal/Core/Access/AccessManagerInterface.php | 10 +- .../lib/Drupal/Core/Access/AccessibleInterface.php | 4 +- core/lib/Drupal/Core/Access/CsrfAccessCheck.php | 16 ++- core/lib/Drupal/Core/Access/CustomAccessCheck.php | 4 +- core/lib/Drupal/Core/Access/DefaultAccessCheck.php | 13 +- core/lib/Drupal/Core/Cache/Cacheability.php | 138 +++++++++++++++++++++ core/lib/Drupal/Core/Cache/CacheableInterface.php | 2 +- core/lib/Drupal/Core/Entity/EntityAccessCheck.php | 12 +- .../Drupal/Core/Entity/EntityAccessController.php | 125 +++++++++++-------- .../Entity/EntityAccessControllerInterface.php | 9 +- .../Drupal/Core/Entity/EntityCreateAccessCheck.php | 15 ++- core/lib/Drupal/Core/Entity/EntityForm.php | 3 +- core/lib/Drupal/Core/Entity/EntityListBuilder.php | 5 +- .../Core/EventSubscriber/AccessSubscriber.php | 3 +- core/lib/Drupal/Core/Field/FieldItemList.php | 6 +- .../Drupal/Core/Field/FieldItemListInterface.php | 4 +- .../lib/Drupal/Core/Menu/ContextualLinkManager.php | 3 +- .../Core/Menu/DefaultMenuLinkTreeManipulators.php | 3 +- core/lib/Drupal/Core/Menu/LocalActionManager.php | 3 +- core/lib/Drupal/Core/Menu/LocalTaskManager.php | 3 +- core/lib/Drupal/Core/Menu/MenuLinkBase.php | 6 +- core/lib/Drupal/Core/Menu/MenuLinkDefault.php | 6 +- core/lib/Drupal/Core/Path/PathValidator.php | 3 +- core/lib/Drupal/Core/Theme/ThemeAccessCheck.php | 11 +- .../aggregator/src/FeedAccessController.php | 18 ++- core/modules/block/block.api.php | 28 ++++- core/modules/block/src/BlockAccessController.php | 6 +- core/modules/block/src/BlockBase.php | 13 +- .../src/Plugin/DisplayVariant/FullPageVariant.php | 3 +- .../Plugin/DisplayVariant/FullPageVariantTest.php | 14 ++- .../src/BlockContentAccessController.php | 6 +- core/modules/book/book.module | 2 +- .../src/Access/BookNodeIsRemovableAccessCheck.php | 13 +- core/modules/book/src/BookManager.php | 3 +- .../book/src/Plugin/Block/BookNavigationBlock.php | 3 +- .../comment/src/CommentAccessController.php | 50 ++++++-- core/modules/comment/src/CommentViewBuilder.php | 9 +- .../comment/src/Controller/CommentController.php | 5 +- .../comment/src/Form/CommentAdminOverview.php | 3 +- .../config_test/src/ConfigTestAccessController.php | 10 +- .../src/Access/ConfigTranslationFormAccess.php | 14 ++- .../src/Access/ConfigTranslationOverviewAccess.php | 20 ++- .../src/Controller/ConfigTranslationController.php | 3 +- .../contact/src/Access/ContactPageAccess.php | 33 +++-- .../contact/src/CategoryAccessController.php | 14 ++- .../contact/src/Plugin/views/field/ContactLink.php | 3 +- .../content_translation/content_translation.module | 2 + .../content_translation.pages.inc | 5 +- .../Access/ContentTranslationManageAccessCheck.php | 27 +++- .../Access/ContentTranslationOverviewAccess.php | 22 +++- .../entity/src/Entity/EntityFormDisplay.php | 3 +- .../entity/src/Entity/EntityViewDisplay.php | 8 +- .../src/EntityReferenceAutocomplete.php | 3 +- .../src/Tests/EntityReferenceFormatterTest.php | 3 +- .../src/FieldInstanceConfigAccessController.php | 14 ++- .../tests/modules/field_test/field_test.field.inc | 19 ++- .../field_ui/src/Access/FormModeAccessCheck.php | 20 ++- .../field_ui/src/Access/ViewModeAccessCheck.php | 20 ++- core/modules/file/file.module | 3 +- core/modules/file/src/FileAccessController.php | 15 ++- core/modules/filter/filter.module | 3 +- core/modules/filter/src/FilterFormatAccess.php | 24 +++- core/modules/filter/src/Tests/FilterAdminTest.php | 5 +- .../filter/src/Tests/FilterFormatAccessTest.php | 15 ++- core/modules/forum/forum.module | 2 +- .../language/src/LanguageAccessController.php | 16 ++- .../src/Form/MenuLinkContentForm.php | 5 +- .../src/MenuLinkContentAccessController.php | 36 +++++- .../modules/menu_ui/src/Form/MenuLinkResetForm.php | 10 +- core/modules/node/node.api.php | 52 +++++--- core/modules/node/node.module | 64 +++++----- core/modules/node/node.pages.inc | 3 +- .../modules/node/src/Access/NodeAddAccessCheck.php | 12 +- .../node/src/Access/NodeRevisionAccessCheck.php | 11 +- .../modules/node/src/Controller/NodeController.php | 7 +- core/modules/node/src/NodeAccessController.php | 63 ++++++++-- core/modules/node/src/NodeForm.php | 7 +- core/modules/node/src/NodeGrantDatabaseStorage.php | 15 ++- .../node/src/NodeGrantDatabaseStorageInterface.php | 6 +- core/modules/node/src/NodeTypeAccessController.php | 8 +- core/modules/node/src/Plugin/Search/NodeSearch.php | 8 +- .../node/src/Plugin/views/area/ListingEmpty.php | 3 +- core/modules/node/src/Plugin/views/field/Link.php | 3 +- .../node/src/Plugin/views/field/LinkDelete.php | 3 +- .../node/src/Plugin/views/field/LinkEdit.php | 3 +- .../node/src/Plugin/views/field/RevisionLink.php | 3 +- core/modules/node/src/Tests/NodeTestBase.php | 5 +- .../node_access_test/node_access_test.module | 13 +- .../Plugin/Field/FieldType/PathFieldItemList.php | 13 +- .../quickedit/src/Access/EditEntityAccessCheck.php | 30 ++++- .../src/Access/EditEntityFieldAccessCheck.php | 36 +++++- .../Access/EditEntityFieldAccessCheckInterface.php | 8 ++ .../tests/src/Access/EditEntityAccessCheckTest.php | 30 +++-- .../src/Access/EditEntityFieldAccessCheckTest.php | 59 ++++++--- core/modules/rest/src/Access/CSRFAccessCheck.php | 15 ++- .../src/Plugin/rest/resource/EntityResource.php | 17 +-- .../search/src/SearchPageAccessController.php | 14 ++- core/modules/shortcut/shortcut.module | 70 +++++++---- .../shortcut/src/Form/SwitchShortcutSet.php | 24 ++-- .../shortcut/src/ShortcutAccessController.php | 14 +++ .../shortcut/src/ShortcutSetAccessController.php | 44 +++++-- core/modules/system/entity.api.php | 70 +++++++---- core/modules/system/src/Access/CronAccessCheck.php | 19 ++- .../system/src/DateFormatAccessController.php | 12 +- core/modules/system/src/Form/ModulesListForm.php | 3 +- core/modules/system/src/MenuAccessController.php | 12 +- .../system/src/PathBasedBreadcrumbBuilder.php | 3 +- .../system/src/Tests/Entity/EntityAccessTest.php | 5 +- .../src/Tests/Entity/EntityViewBuilderTest.php | 3 +- .../system/src/Tests/Entity/FieldAccessTest.php | 13 +- core/modules/system/system.module | 2 +- .../tests/modules/entity_test/entity_test.module | 41 +++++- .../entity_test/src/EntityTestAccessController.php | 24 +++- .../src/Access/DefinedTestAccessCheck.php | 14 ++- .../src/Access/TestAccessCheck.php | 6 +- .../Breadcrumbs/PathBasedBreadcrumbBuilderTest.php | 16 ++- .../taxonomy/src/Plugin/views/field/LinkEdit.php | 3 +- core/modules/taxonomy/src/TermAccessController.php | 28 ++++- .../toolbar/src/Controller/ToolbarController.php | 7 +- .../src/Access/ViewOwnTrackerAccessCheck.php | 12 +- .../update/src/Access/UpdateManagerAccessCheck.php | 7 +- core/modules/user/src/Access/LoginStatusCheck.php | 11 +- .../user/src/Access/PermissionAccessCheck.php | 12 +- .../user/src/Access/RegisterAccessCheck.php | 12 +- core/modules/user/src/Access/RoleAccessCheck.php | 18 ++- core/modules/user/src/Plugin/Search/UserSearch.php | 8 +- .../user/src/Plugin/views/field/LinkCancel.php | 3 +- .../user/src/Plugin/views/field/LinkEdit.php | 3 +- core/modules/user/src/RoleAccessController.php | 6 +- core/modules/user/src/UserAccessController.php | 74 ++++++----- .../modules/views/src/Plugin/views/area/Entity.php | 3 +- .../src/Plugin/views/argument_validator/Entity.php | 3 +- .../views/src/Tests/Handler/AreaEntityTest.php | 3 +- core/modules/views/src/ViewAccessController.php | 11 +- core/modules/views/src/ViewsAccessCheck.php | 14 ++- .../src/Plugin/argument_validator/EntityTest.php | 19 ++- core/modules/views_ui/src/ViewEditForm.php | 3 +- .../Drupal/Tests/Core/Access/AccessManagerTest.php | 103 +++++++++------ .../Tests/Core/Access/CsrfAccessCheckTest.php | 17 ++- .../Tests/Core/Access/DefaultAccessCheckTest.php | 13 +- .../Core/Entity/ContentEntityBaseUnitTest.php | 13 +- .../Tests/Core/Entity/EntityAccessCheckTest.php | 15 ++- .../Core/Entity/EntityCreateAccessCheckTest.php | 29 +++-- .../Tests/Core/Entity/EntityListBuilderTest.php | 6 +- .../Drupal/Tests/Core/Entity/EntityUnitTest.php | 13 +- .../Core/EventSubscriber/AccessSubscriberTest.php | 11 +- .../Tests/Core/Menu/ContextualLinkManagerTest.php | 14 ++- .../Menu/DefaultMenuLinkTreeManipulatorsTest.php | 14 ++- .../Tests/Core/Menu/LocalActionManagerTest.php | 7 ++ .../Tests/Core/Route/RoleAccessCheckTest.php | 11 +- 153 files changed, 1925 insertions(+), 634 deletions(-) diff --git a/core/includes/form.inc b/core/includes/form.inc index 7131a7b..37e6488 100644 --- a/core/includes/form.inc +++ b/core/includes/form.inc @@ -2100,7 +2100,7 @@ function form_process_autocomplete($element, FormStateInterface $form_state) { $parameters = isset($element['#autocomplete_route_parameters']) ? $element['#autocomplete_route_parameters'] : array(); $path = \Drupal::urlGenerator()->generate($element['#autocomplete_route_name'], $parameters); - $access = \Drupal::service('access_manager')->checkNamedRoute($element['#autocomplete_route_name'], $parameters, \Drupal::currentUser()); + $access = \Drupal::service('access_manager')->checkNamedRoute($element['#autocomplete_route_name'], $parameters, \Drupal::currentUser())->value === \Drupal\Core\Access\AccessInterface::ALLOW; } if ($access) { $element['#attributes']['class'][] = 'form-autocomplete'; diff --git a/core/lib/Drupal/Core/Access/AccessCheckResult.php b/core/lib/Drupal/Core/Access/AccessCheckResult.php new file mode 100644 index 0000000..a953475 --- /dev/null +++ b/core/lib/Drupal/Core/Access/AccessCheckResult.php @@ -0,0 +1,87 @@ +cacheability = new Cacheability($is_cacheable); + } + + protected static function merge(array $access_check_results, $behavior) { + // Calculate the combined result, starting with the merged cacheability + // metadata. + $access = new AccessCheckResult(TRUE); + $merge_cacheability = function(Cacheability $carry, AccessCheckResult $current) { + return $carry->merge($current->cacheability); + }; + $access->cacheability = array_reduce($access_check_results, $merge_cacheability, $access->cacheability); + + $get_access = function(AccessCheckResult $access_check_result) { + return $access_check_result->value; + }; + $access_check_values = array_map($get_access, $access_check_results); + + // One access KILLer is enough to deny access immediately. + if (in_array(AccessInterface::KILL, $access_check_values, TRUE)) { + $access->value = AccessInterface::KILL; + return $access; + } + + // Default to DENY. + $access->value = AccessInterface::DENY; + + // 'ANY' merging behavior: at least one checker should allow access. + if ($behavior === 'ANY' && in_array(AccessInterface::ALLOW, $access_check_values, TRUE)) { + $access->value = AccessInterface::ALLOW; + } + // 'ALL' merging behavior: every checker should allow access. + else if ($behavior === 'ALL' && !in_array(AccessInterface::DENY, $access_check_values, TRUE)) { + $access->value = AccessInterface::ALLOW; + } + + return $access; + } + + public static function any(array $access_check_results) { + return static::merge($access_check_results, 'ANY'); + } + + public static function all(array $access_check_results) { + return static::merge($access_check_results, 'ALL'); + } + +} diff --git a/core/lib/Drupal/Core/Access/AccessManager.php b/core/lib/Drupal/Core/Access/AccessManager.php index 0e9ac6b..485e91f 100644 --- a/core/lib/Drupal/Core/Access/AccessManager.php +++ b/core/lib/Drupal/Core/Access/AccessManager.php @@ -41,7 +41,7 @@ class AccessManager implements ContainerAwareInterface, AccessManagerInterface { /** * Array of access check objects keyed by service id. * - * @var array + * @var \Drupal\Core\Routing\Access\AccessInterface[] */ protected $checks; @@ -194,10 +194,18 @@ public function checkNamedRoute($route_name, array $parameters = array(), Accoun return $this->check($route, $route_request, $account); } catch (RouteNotFoundException $e) { - return FALSE; + $access = new AccessCheckResult(TRUE); + $access->value = AccessInterface::KILL; + // Cacheable until extensions change. + $access->cacheability->setTags(array('extension' => TRUE)); + return $access; } catch (ParamNotConvertedException $e) { - return FALSE; + // Uncacheable because conversion of the parameter may not have been + // possible due to highly dynamic circumstances. + $access = new AccessCheckResult(FALSE); + $access->value = AccessInterface::KILL; + return $access; } } @@ -228,30 +236,34 @@ public function check(Route $route, Request $request, AccountInterface $account) * @param \Drupal\Core\Session\AccountInterface $account * The current user. * - * @return bool - * Returns TRUE if the user has access to the route, else FALSE. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. */ protected function checkAll(array $checks, Route $route, Request $request, AccountInterface $account) { - $access = FALSE; - + $results = array(); foreach ($checks as $service_id) { if (empty($this->checks[$service_id])) { $this->loadCheck($service_id); } - $service_access = $this->performCheck($service_id, $route, $request, $account); + $result = $this->performCheck($service_id, $route, $request, $account); + $results[] = $result; - if ($service_access === AccessInterface::ALLOW) { - $access = TRUE; - } - else { - // On both KILL and DENY stop. - $access = FALSE; + // Stop as soon as the first DENY or KILL is encountered. + if ($result->value !== AccessInterface::ALLOW) { break; } } - return $access; + if (empty($results)) { + // Deny access by default. + $default = new AccessCheckResult(TRUE); + $default->value = AccessInterface::DENY; + return $default; + } + else { + return AccessCheckResult::all($results); + } } /** @@ -266,29 +278,25 @@ protected function checkAll(array $checks, Route $route, Request $request, Accou * @param \Drupal\Core\Session\AccountInterface $account * The current user. * - * @return bool - * Returns TRUE if the user has access to the route, else FALSE. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. */ protected function checkAny(array $checks, $route, $request, AccountInterface $account) { - // No checks == deny by default. - $access = FALSE; + // Deny access by default. + $default = new AccessCheckResult(TRUE); + $default->value = AccessInterface::DENY; + $results = array($default); foreach ($checks as $service_id) { if (empty($this->checks[$service_id])) { $this->loadCheck($service_id); } - $service_access = $this->performCheck($service_id, $route, $request, $account); - - if ($service_access === AccessInterface::ALLOW) { - $access = TRUE; - } - if ($service_access === AccessInterface::KILL) { - return FALSE; - } + $result = $this->performCheck($service_id, $route, $request, $account); + $results[] = $result; } - return $access; + return AccessCheckResult::any($results); } /** @@ -306,16 +314,22 @@ protected function checkAny(array $checks, $route, $request, AccountInterface $a * @throws \Drupal\Core\Access\AccessException * Thrown when the access check returns an invalid value. * - * @return string - * A \Drupal\Core\Access\AccessInterface constant value. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. */ protected function performCheck($service_id, $route, $request, $account) { $callable = array($this->checks[$service_id], $this->checkMethods[$service_id]); $arguments = $this->argumentsResolver->getArguments($callable, $route, $request, $account); + /** @var \Drupal\Core\Access\AccessCheckResult $service_access **/ $service_access = call_user_func_array($callable, $arguments); - if (!in_array($service_access, array(AccessInterface::ALLOW, AccessInterface::DENY, AccessInterface::KILL), TRUE)) { - throw new AccessException("Access error in $service_id. Access services can only return AccessInterface::ALLOW, AccessInterface::DENY, or AccessInterface::KILL constants."); + if (!$service_access instanceof AccessCheckResult) { + throw new AccessException("Access error in $service_id. Access services can only return AccessCheckResult objects."); + } + if (!in_array($service_access->value, array(AccessInterface::ALLOW, AccessInterface::DENY, AccessInterface::KILL), TRUE)) { + debug($service_id . '(args: ' . print_r($route->getRequirements(), TRUE) . ')'); + debug($service_access); + throw new AccessException("Access error in $service_id. An AccessCheckResult object's value must equal either AccessInterface::ALLOW, AccessInterface::DENY, or AccessInterface::KILL constants."); } return $service_access; diff --git a/core/lib/Drupal/Core/Access/AccessManagerInterface.php b/core/lib/Drupal/Core/Access/AccessManagerInterface.php index df88ba6..133775f 100644 --- a/core/lib/Drupal/Core/Access/AccessManagerInterface.php +++ b/core/lib/Drupal/Core/Access/AccessManagerInterface.php @@ -43,15 +43,15 @@ * @param string $route_name * The route to check access to. * @param array $parameters - * Optional array of values to substitute into the route path patern. + * Optional array of values to substitute into the route path pattern. * @param \Drupal\Core\Session\AccountInterface $account * The current user. * @param \Symfony\Component\HttpFoundation\Request $route_request * Optional incoming request object. If not provided, one will be built * using the route information and the current request from the container. * - * @return bool - * Returns TRUE if the user has access to the route, otherwise FALSE. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. */ public function checkNamedRoute($route_name, array $parameters = array(), AccountInterface $account, Request $route_request = NULL); @@ -88,8 +88,8 @@ public function addCheckService($service_id, $service_method, array $applies_che * @param \Drupal\Core\Session\AccountInterface $account * The current account. * - * @return bool - * Returns TRUE if the user has access to the route, otherwise FALSE. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. */ public function check(Route $route, Request $request, AccountInterface $account); diff --git a/core/lib/Drupal/Core/Access/AccessibleInterface.php b/core/lib/Drupal/Core/Access/AccessibleInterface.php index 4316853..97f1270 100644 --- a/core/lib/Drupal/Core/Access/AccessibleInterface.php +++ b/core/lib/Drupal/Core/Access/AccessibleInterface.php @@ -25,8 +25,8 @@ * (optional) The user for which to check access, or NULL to check access * for the current user. Defaults to NULL. * - * @return bool|null - * self::ALLOW, self::DENY, or self::KILL. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. */ public function access($operation, AccountInterface $account = NULL); diff --git a/core/lib/Drupal/Core/Access/CsrfAccessCheck.php b/core/lib/Drupal/Core/Access/CsrfAccessCheck.php index f6b60d3..d6efa3e 100644 --- a/core/lib/Drupal/Core/Access/CsrfAccessCheck.php +++ b/core/lib/Drupal/Core/Access/CsrfAccessCheck.php @@ -45,15 +45,19 @@ function __construct(CsrfTokenGenerator $csrf_token) { * @param \Symfony\Component\HttpFoundation\Request $request * The request object. * - * @return string - * A \Drupal\Core\Access\AccessInterface constant value. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. */ public function access(Route $route, Request $request) { + // Not cacheable because the CSRF token is highly dynamic. + $access = new AccessCheckResult(FALSE); + // If this is the controller request, check CSRF access as normal. if ($request->attributes->get('_controller_request')) { // @todo Remove dependency on the internal _system_path attribute: // https://www.drupal.org/node/2293501. - return $this->csrfToken->validate($request->query->get('token'), $request->attributes->get('_system_path')) ? static::ALLOW : static::KILL; + $access->value = $this->csrfToken->validate($request->query->get('token'), $request->attributes->get('_system_path')) ? static::ALLOW : static::KILL; + return $access; } // Otherwise, this could be another requested access check that we don't @@ -61,12 +65,14 @@ public function access(Route $route, Request $request) { $conjunction = $route->getOption('_access_mode') ?: AccessManagerInterface::ACCESS_MODE_ANY; // Return ALLOW if all access checks are needed. if ($conjunction == AccessManagerInterface::ACCESS_MODE_ALL) { - return static::ALLOW; + $access->value = static::ALLOW; + return $access; } // Return DENY otherwise, as another access checker should grant access // for the route. else { - return static::DENY; + $access->value = static::DENY; + return $access; } } diff --git a/core/lib/Drupal/Core/Access/CustomAccessCheck.php b/core/lib/Drupal/Core/Access/CustomAccessCheck.php index c128a5b..f5bf368 100644 --- a/core/lib/Drupal/Core/Access/CustomAccessCheck.php +++ b/core/lib/Drupal/Core/Access/CustomAccessCheck.php @@ -62,8 +62,8 @@ public function __construct(ControllerResolverInterface $controller_resolver, Ac * @param \Drupal\Core\Session\AccountInterface $account * The currently logged in account. * - * @return string - * A \Drupal\Core\Access\AccessInterface constant value. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. */ public function access(Route $route, Request $request, AccountInterface $account) { $callable = $this->controllerResolver->getControllerFromDefinition($route->getRequirement('_custom_access')); diff --git a/core/lib/Drupal/Core/Access/DefaultAccessCheck.php b/core/lib/Drupal/Core/Access/DefaultAccessCheck.php index 485dad8..6620cd7 100644 --- a/core/lib/Drupal/Core/Access/DefaultAccessCheck.php +++ b/core/lib/Drupal/Core/Access/DefaultAccessCheck.php @@ -21,19 +21,22 @@ class DefaultAccessCheck implements RoutingAccessInterface { * @param \Symfony\Component\Routing\Route $route * The route to check against. * - * @return string - * A \Drupal\Core\Access\AccessInterface constant value. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. */ public function access(Route $route) { + $access = new AccessCheckResult(TRUE); + if ($route->getRequirement('_access') === 'TRUE') { - return static::ALLOW; + $access->value = static::ALLOW; } elseif ($route->getRequirement('_access') === 'FALSE') { - return static::KILL; + $access->value = static::KILL; } else { - return static::DENY; + $access->value = static::DENY; } + return $access; } } diff --git a/core/lib/Drupal/Core/Cache/Cacheability.php b/core/lib/Drupal/Core/Cache/Cacheability.php new file mode 100644 index 0000000..121705f --- /dev/null +++ b/core/lib/Drupal/Core/Cache/Cacheability.php @@ -0,0 +1,138 @@ +setCacheable($is_cacheable) + // Typically, cache items are invalidated via associated cache tags, not + // via a maximum age. + ->setMaxAge(Cache::PERMANENT) + // Default cache keys, contexts and tags to the empty array. + ->setKeys(array()) + ->setContexts(array()) + ->setTags(array()); + } + + public function merge(Cacheability $other) { + $this->isCacheable = $this->isCacheable && $other->isCacheable; + $this->addContexts($other->contexts); + $this->addTags($other->tags); + // Use the lowest max-age. + if ($this->maxAge === Cache::PERMANENT) { + // The other max-age is either lower or equal. + $this->setMaxAge($other->maxAge); + } + else { + $this->setMaxAge(min($this->maxAge, $other->maxAge)); + } + return $this; + } + + public function setCacheable($is_cacheable) { + $this->isCacheable = $is_cacheable; + return $this; + } + + public function setKeys($keys) { + $this->keys = $keys; + return $this; + } + + public function setContexts(array $contexts) { + $this->contexts = $contexts; + return $this; + } + + public function addContexts(array $contexts) { + $this->contexts = array_unique(array_merge($this->contexts, $contexts)); + return $this; + } + + public function setTags(array $tags) { + $this->tags = $tags; + return $this; + } + + public function addTags(array $tags) { + $this->tags = NestedArray::mergeDeep($this->tags, $tags); + return $this; + } + + public function setMaxAge($max_age) { + $this->maxAge = $max_age; + return $this; + } + + public function setBin($bin) { + $this->bin = $bin; + return $this; + } + +} diff --git a/core/lib/Drupal/Core/Cache/CacheableInterface.php b/core/lib/Drupal/Core/Cache/CacheableInterface.php index 2a59467..85950ff 100644 --- a/core/lib/Drupal/Core/Cache/CacheableInterface.php +++ b/core/lib/Drupal/Core/Cache/CacheableInterface.php @@ -17,7 +17,7 @@ * The cache keys associated with this potentially cacheable object. * * @return array - * An array of strings or cache constants, used to generate a cache ID. + * An array of strings or cache context IDs, used to generate a cache ID. */ public function getCacheKeys(); diff --git a/core/lib/Drupal/Core/Entity/EntityAccessCheck.php b/core/lib/Drupal/Core/Entity/EntityAccessCheck.php index 951abb0..188621e 100644 --- a/core/lib/Drupal/Core/Entity/EntityAccessCheck.php +++ b/core/lib/Drupal/Core/Entity/EntityAccessCheck.php @@ -7,6 +7,7 @@ namespace Drupal\Core\Entity; +use Drupal\Core\Access\AccessCheckResult; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; use Symfony\Component\Routing\Route; @@ -37,8 +38,8 @@ class EntityAccessCheck implements AccessInterface { * @param \Drupal\Core\Session\AccountInterface $account * The currently logged in account. * - * @return string - * A \Drupal\Core\Access\AccessInterface constant value. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. */ public function access(Route $route, Request $request, AccountInterface $account) { // Split the entity type and the operation. @@ -48,12 +49,15 @@ public function access(Route $route, Request $request, AccountInterface $account if ($request->attributes->has($entity_type)) { $entity = $request->attributes->get($entity_type); if ($entity instanceof EntityInterface) { - return $entity->access($operation, $account) ? static::ALLOW : static::DENY; + return $entity->access($operation, $account); } } + // No opinion, so other access checks should decide if access should be // allowed or not. - return static::DENY; + $access = new AccessCheckResult(TRUE); + $access->value = static::DENY; + return $access; } } diff --git a/core/lib/Drupal/Core/Entity/EntityAccessController.php b/core/lib/Drupal/Core/Entity/EntityAccessController.php index f20ef9c..951c7d3 100644 --- a/core/lib/Drupal/Core/Entity/EntityAccessController.php +++ b/core/lib/Drupal/Core/Entity/EntityAccessController.php @@ -7,6 +7,9 @@ namespace Drupal\Core\Entity; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; +use Drupal\Core\Cache\Cacheability; use Drupal\Core\Entity\EntityTypeInterface; use Drupal\Core\Field\FieldItemListInterface; use Drupal\Core\Extension\ModuleHandlerInterface; @@ -71,17 +74,18 @@ public function access(EntityInterface $entity, $operation, $langcode = Language // We grant access to the entity if both of these conditions are met: // - No modules say to deny access. // - At least one module says to grant access. - $access = array_merge( + $all_access = array_merge( $this->moduleHandler()->invokeAll('entity_access', array($entity, $operation, $account, $langcode)), $this->moduleHandler()->invokeAll($entity->getEntityTypeId() . '_access', array($entity, $operation, $account, $langcode)) ); - if (($return = $this->processAccessHookResults($access)) === NULL) { + $access = $this->processAccessHookResults($all_access); + if ($access->value === AccessInterface::DENY) { // No module had an opinion about the access, so let's the access - // controller check create access. - $return = (bool) $this->checkAccess($entity, $operation, $langcode, $account); + // controller check access. + $access = $this->checkAccess($entity, $operation, $langcode, $account); } - return $this->setCache($return, $entity->uuid(), $operation, $langcode, $account); + return $this->setCache($access, $entity->uuid(), $operation, $langcode, $account); } /** @@ -89,23 +93,37 @@ public function access(EntityInterface $entity, $operation, $langcode = Language * - No modules say to deny access. * - At least one module says to grant access. * - * @param array $access + * @param \Drupal\Core\Access\AccessCheckResult[] $access * An array of access results of the fired access hook. * - * @return bool|null - * Returns FALSE if access should be denied, TRUE if access should be - * granted and NULL if no module denied access. + * @return \Drupal\Core\Access\AccessCheckResult + * The combined result of the various access checks' results. All their + * cacheability metadata is merged as well. */ protected function processAccessHookResults(array $access) { - if (in_array(FALSE, $access, TRUE)) { - return FALSE; + // Calculate the combined result, starting with the merged cacheability + // metadata. + $result = new AccessCheckResult(TRUE); + $merge_cacheability = function(Cacheability $carry, AccessCheckResult $current) { + return $carry->merge($current->cacheability); + }; + $result->cacheability = array_reduce($access, $merge_cacheability, $result->cacheability); + + $get_access = function(AccessCheckResult $access_check_result) { + return $access_check_result->value; + }; + $access_check_values = array_map($get_access, $access); + + if (in_array(AccessInterface::KILL, $access_check_values, TRUE)) { + $result->value = AccessInterface::KILL; } - elseif (in_array(TRUE, $access, TRUE)) { - return TRUE; + elseif (in_array(AccessInterface::ALLOW, $access_check_values, TRUE)) { + $result->value = AccessInterface::ALLOW; } else { - return; + $result->value = AccessInterface::DENY; } + return $result; } /** @@ -124,19 +142,28 @@ protected function processAccessHookResults(array $access) { * @param \Drupal\Core\Session\AccountInterface $account * The user for which to check access. * - * @return bool|null - * TRUE if access was granted, FALSE if access was denied and NULL if access - * could not be determined. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. */ protected function checkAccess(EntityInterface $entity, $operation, $langcode, AccountInterface $account) { + $access = new AccessCheckResult(TRUE); + if ($operation == 'delete' && $entity->isNew()) { - return FALSE; + $access->value = AccessInterface::KILL; + // Cacheable until entity is modified. + $access->cacheability->addTags($entity->getCacheTag()); + return $access; } + if ($admin_permission = $this->entityType->getAdminPermission()) { - return $account->hasPermission($admin_permission); + // Cacheable per role. + $access->cacheability->addContexts(array('cache_context.user.roles')); + $access->value = $account->hasPermission($admin_permission) ? AccessInterface::ALLOW : AccessInterface::DENY; + return $access; } else { - return NULL; + $access->value = AccessInterface::DENY; + return $access; } } @@ -154,10 +181,9 @@ protected function checkAccess(EntityInterface $entity, $operation, $langcode, A * @param \Drupal\Core\Session\AccountInterface $account * The user for which to check access. * - * @return bool|null - * TRUE if access was granted, FALSE if access was denied and NULL if there - * is no record for the given user, operation, langcode and entity in the - * cache. + * @return \Drupal\Core\Access\AccessCheckResult|null + * The cached AccessCheckResult, or NULL if there is no record for the given + * user, operation, langcode and entity in the cache. */ protected function getCache($cid, $operation, $langcode, AccountInterface $account) { // Return from cache if a value has been set for it previously. @@ -169,8 +195,8 @@ protected function getCache($cid, $operation, $langcode, AccountInterface $accou /** * Statically caches whether the given user has access. * - * @param bool $access - * TRUE if the user has access, FALSE otherwise. + * @param \Drupal\Core\Access\AccessCheckResult $access + * Whether the user has access, plus cacheability metadata. * @param string $cid * Unique string identifier for the entity/operation, for example the * entity UUID or a custom string. @@ -182,12 +208,12 @@ protected function getCache($cid, $operation, $langcode, AccountInterface $accou * @param \Drupal\Core\Session\AccountInterface $account * The user for which to check access. * - * @return bool - * TRUE if access was granted, FALSE otherwise. + * @return \Drupal\Core\Access\AccessCheckResult + * Whether the user has access, plus cacheability metadata. */ protected function setCache($access, $cid, $operation, $langcode, AccountInterface $account) { // Save the given value in the static cache and directly return it. - return $this->accessCache[$account->id()][$cid][$langcode][$operation] = (bool) $access; + return $this->accessCache[$account->id()][$cid][$langcode][$operation] = $access; } /** @@ -221,17 +247,18 @@ public function createAccess($entity_bundle = NULL, AccountInterface $account = // We grant access to the entity if both of these conditions are met: // - No modules say to deny access. // - At least one module says to grant access. - $access = array_merge( + $all_access = array_merge( $this->moduleHandler()->invokeAll('entity_create_access', array($account, $context['langcode'])), $this->moduleHandler()->invokeAll($this->entityTypeId . '_create_access', array($account, $context['langcode'])) ); - if (($return = $this->processAccessHookResults($access)) === NULL) { + $access = $this->processAccessHookResults($all_access); + if ($access->value === AccessInterface::DENY) { // No module had an opinion about the access, so let's the access // controller check create access. - $return = (bool) $this->checkCreateAccess($account, $context, $entity_bundle); + $access = $this->checkCreateAccess($account, $context, $entity_bundle); } - return $this->setCache($return, $cid, 'create', $context['langcode'], $account); + return $this->setCache($access, $cid, 'create', $context['langcode'], $account); } /** @@ -248,17 +275,20 @@ public function createAccess($entity_bundle = NULL, AccountInterface $account = * (optional) The bundle of the entity. Required if the entity supports * bundles, defaults to NULL otherwise. * - * @return bool|null - * TRUE if access was granted, FALSE if access was denied and NULL if access - * could not be determined. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. */ protected function checkCreateAccess(AccountInterface $account, array $context, $entity_bundle = NULL) { + $access = new AccessCheckResult(TRUE); + // Cache per role. + $access->cacheability->addContexts(array('cache_context.user.roles')); if ($admin_permission = $this->entityType->getAdminPermission()) { - return $account->hasPermission($admin_permission); + $access->value = $account->hasPermission($admin_permission) ? AccessInterface::ALLOW : AccessInterface::KILL; } else { - return NULL; + $access->value = AccessInterface::DENY; } + return $access; } /** @@ -284,7 +314,13 @@ public function fieldAccess($operation, FieldDefinitionInterface $field_definiti $account = $this->prepareUser($account); // Get the default access restriction that lives within this field. - $default = $items ? $items->defaultAccess($operation, $account) : TRUE; + if ($items) { + $default = $items->defaultAccess($operation, $account); + } + else { + $default = new AccessCheckResult(TRUE); + $default->value = AccessInterface::ALLOW; + } // Invoke hook and collect grants/denies for field access from other // modules. Our default access flag is masked under the ':default' key. @@ -303,16 +339,7 @@ public function fieldAccess($operation, FieldDefinitionInterface $field_definiti ); $this->moduleHandler()->alter('entity_field_access', $grants, $context); - // One grant being FALSE is enough to deny access immediately. - if (in_array(FALSE, $grants, TRUE)) { - return FALSE; - } - // At least one grant has the explicit opinion to allow access. - if (in_array(TRUE, $grants, TRUE)) { - return TRUE; - } - // All grants are NULL and have no opinion - deny access in that case. - return FALSE; + return AccessCheckResult::any($grants); } } diff --git a/core/lib/Drupal/Core/Entity/EntityAccessControllerInterface.php b/core/lib/Drupal/Core/Entity/EntityAccessControllerInterface.php index 2a5851f..f9cd754 100644 --- a/core/lib/Drupal/Core/Entity/EntityAccessControllerInterface.php +++ b/core/lib/Drupal/Core/Entity/EntityAccessControllerInterface.php @@ -36,8 +36,8 @@ * (optional) The user session for which to check access, or NULL to check * access for the current user. Defaults to NULL. * - * @return bool - * TRUE if access was granted, FALSE otherwise. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. */ public function access(EntityInterface $entity, $operation, $langcode = LanguageInterface::LANGCODE_DEFAULT, AccountInterface $account = NULL); @@ -53,10 +53,13 @@ public function access(EntityInterface $entity, $operation, $langcode = Language * @param array $context * (optional) An array of key-value pairs to pass additional context when * needed. + * + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. */ public function createAccess($entity_bundle = NULL, AccountInterface $account = NULL, array $context = array()); - /** + /** * Clears all cached access checks. */ public function resetCache(); diff --git a/core/lib/Drupal/Core/Entity/EntityCreateAccessCheck.php b/core/lib/Drupal/Core/Entity/EntityCreateAccessCheck.php index bf7073d..cd11c30 100644 --- a/core/lib/Drupal/Core/Entity/EntityCreateAccessCheck.php +++ b/core/lib/Drupal/Core/Entity/EntityCreateAccessCheck.php @@ -7,6 +7,7 @@ namespace Drupal\Core\Entity; +use Drupal\Core\Access\AccessCheckResult; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; use Symfony\Component\HttpFoundation\Request; @@ -51,8 +52,8 @@ public function __construct(EntityManagerInterface $entity_manager) { * @param \Drupal\Core\Session\AccountInterface $account * The currently logged in account. * - * @return string - * A \Drupal\Core\Access\AccessInterface constant value. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. */ public function access(Route $route, Request $request, AccountInterface $account) { list($entity_type, $bundle) = explode(':', $route->getRequirement($this->requirementsKey) . ':'); @@ -66,10 +67,16 @@ public function access(Route $route, Request $request, AccountInterface $account } // If we were unable to replace all placeholders, deny access. if (strpos($bundle, '{') !== FALSE) { - return static::DENY; + // Uncacheable, because the missing _raw_variables values that caused us + // to get here depend on the request, so cross-request caching is + // impossible. + $access = new AccessCheckResult(FALSE); + $access->value = static::DENY; + return $access; } } - return $this->entityManager->getAccessController($entity_type)->createAccess($bundle, $account) ? static::ALLOW : static::DENY; + + return $this->entityManager->getAccessController($entity_type)->createAccess($bundle, $account); } } diff --git a/core/lib/Drupal/Core/Entity/EntityForm.php b/core/lib/Drupal/Core/Entity/EntityForm.php index f929a95..c3a84f8 100644 --- a/core/lib/Drupal/Core/Entity/EntityForm.php +++ b/core/lib/Drupal/Core/Entity/EntityForm.php @@ -7,6 +7,7 @@ namespace Drupal\Core\Entity; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Form\FormBase; use Drupal\Core\Extension\ModuleHandlerInterface; use Drupal\Core\Form\FormStateInterface; @@ -229,7 +230,7 @@ protected function actions(array $form, FormStateInterface $form_state) { $actions['delete'] = array( '#type' => 'link', '#title' => $this->t('Delete'), - '#access' => $this->entity->access('delete'), + '#access' => $this->entity->access('delete')->value === AccessInterface::ALLOW, '#attributes' => array( 'class' => array('button', 'button--danger'), ), diff --git a/core/lib/Drupal/Core/Entity/EntityListBuilder.php b/core/lib/Drupal/Core/Entity/EntityListBuilder.php index 16a0849..d11029c 100644 --- a/core/lib/Drupal/Core/Entity/EntityListBuilder.php +++ b/core/lib/Drupal/Core/Entity/EntityListBuilder.php @@ -7,6 +7,7 @@ namespace Drupal\Core\Entity; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Extension\ModuleHandlerInterface; use Symfony\Component\DependencyInjection\ContainerInterface; use Drupal\Component\Utility\String; @@ -114,13 +115,13 @@ public function getOperations(EntityInterface $entity) { */ protected function getDefaultOperations(EntityInterface $entity) { $operations = array(); - if ($entity->access('update') && $entity->hasLinkTemplate('edit-form')) { + if ($entity->access('update')->value === AccessInterface::ALLOW && $entity->hasLinkTemplate('edit-form')) { $operations['edit'] = array( 'title' => $this->t('Edit'), 'weight' => 10, ) + $entity->urlInfo('edit-form')->toArray(); } - if ($entity->access('delete') && $entity->hasLinkTemplate('delete-form')) { + if ($entity->access('delete')->value === AccessInterface::ALLOW && $entity->hasLinkTemplate('delete-form')) { $operations['delete'] = array( 'title' => $this->t('Delete'), 'weight' => 100, diff --git a/core/lib/Drupal/Core/EventSubscriber/AccessSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/AccessSubscriber.php index 02a65ed..1aa3ced 100644 --- a/core/lib/Drupal/Core/EventSubscriber/AccessSubscriber.php +++ b/core/lib/Drupal/Core/EventSubscriber/AccessSubscriber.php @@ -7,6 +7,7 @@ namespace Drupal\Core\EventSubscriber; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Access\AccessManagerInterface; use Drupal\Core\Session\AccountInterface; use Symfony\Cmf\Component\Routing\RouteObjectInterface; @@ -74,7 +75,7 @@ public function onKernelRequestAccessCheck(GetResponseEvent $event) { // Wrap this in a try/catch to ensure the '_controller_request' attribute // can always be removed. try { - $access = $this->accessManager->check($request->attributes->get(RouteObjectInterface::ROUTE_OBJECT), $request, $this->currentUser); + $access = $this->accessManager->check($request->attributes->get(RouteObjectInterface::ROUTE_OBJECT), $request, $this->currentUser)->value === AccessInterface::ALLOW; } catch (\Exception $e) { $request->attributes->remove('_controller_request'); diff --git a/core/lib/Drupal/Core/Field/FieldItemList.php b/core/lib/Drupal/Core/Field/FieldItemList.php index 726572f..312960b 100644 --- a/core/lib/Drupal/Core/Field/FieldItemList.php +++ b/core/lib/Drupal/Core/Field/FieldItemList.php @@ -7,6 +7,8 @@ namespace Drupal\Core\Field; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Form\FormStateInterface; use Drupal\Core\Language\LanguageInterface; use Drupal\Core\Entity\ContentEntityInterface; @@ -197,7 +199,9 @@ public function access($operation = 'view', AccountInterface $account = NULL) { */ public function defaultAccess($operation = 'view', AccountInterface $account = NULL) { // Grant access per default. - return TRUE; + $access = new AccessCheckResult(TRUE); + $access->value = AccessInterface::ALLOW; + return $access; } /** diff --git a/core/lib/Drupal/Core/Field/FieldItemListInterface.php b/core/lib/Drupal/Core/Field/FieldItemListInterface.php index ddfe26b..1933cc1 100644 --- a/core/lib/Drupal/Core/Field/FieldItemListInterface.php +++ b/core/lib/Drupal/Core/Field/FieldItemListInterface.php @@ -87,8 +87,8 @@ public function getSetting($setting_name); * See \Drupal\Core\Entity\EntityAccessControllerInterface::fieldAccess() for * the parameter documentation. * - * @return bool - * TRUE if access to this field is allowed per default, FALSE otherwise. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. */ public function defaultAccess($operation = 'view', AccountInterface $account = NULL); diff --git a/core/lib/Drupal/Core/Menu/ContextualLinkManager.php b/core/lib/Drupal/Core/Menu/ContextualLinkManager.php index cbbf4e4..3810f0c 100644 --- a/core/lib/Drupal/Core/Menu/ContextualLinkManager.php +++ b/core/lib/Drupal/Core/Menu/ContextualLinkManager.php @@ -8,6 +8,7 @@ namespace Drupal\Core\Menu; use Drupal\Component\Plugin\Exception\PluginException; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Access\AccessManagerInterface; use Drupal\Core\Cache\CacheBackendInterface; use Drupal\Core\Controller\ControllerResolverInterface; @@ -168,7 +169,7 @@ public function getContextualLinksArrayByGroup($group_name, array $route_paramet $route_name = $plugin->getRouteName(); // Check access. - if (!$this->accessManager->checkNamedRoute($route_name, $route_parameters, $this->account)) { + if ($this->accessManager->checkNamedRoute($route_name, $route_parameters, $this->account)->value !== AccessInterface::ALLOW) { continue; } diff --git a/core/lib/Drupal/Core/Menu/DefaultMenuLinkTreeManipulators.php b/core/lib/Drupal/Core/Menu/DefaultMenuLinkTreeManipulators.php index ae5b285..4c72a98 100644 --- a/core/lib/Drupal/Core/Menu/DefaultMenuLinkTreeManipulators.php +++ b/core/lib/Drupal/Core/Menu/DefaultMenuLinkTreeManipulators.php @@ -7,6 +7,7 @@ namespace Drupal\Core\Menu; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Access\AccessManagerInterface; use Drupal\Core\Session\AccountInterface; @@ -101,7 +102,7 @@ protected function menuLinkCheckAccess(MenuLinkInterface $instance) { $access = TRUE; } else { - $access = $this->accessManager->checkNamedRoute($definition['route_name'], $definition['route_parameters'], $this->account); + $access = $this->accessManager->checkNamedRoute($definition['route_name'], $definition['route_parameters'], $this->account)->value === AccessInterface::ALLOW; } return $access; } diff --git a/core/lib/Drupal/Core/Menu/LocalActionManager.php b/core/lib/Drupal/Core/Menu/LocalActionManager.php index 1167357..592cde6 100644 --- a/core/lib/Drupal/Core/Menu/LocalActionManager.php +++ b/core/lib/Drupal/Core/Menu/LocalActionManager.php @@ -7,6 +7,7 @@ namespace Drupal\Core\Menu; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Access\AccessManagerInterface; use Drupal\Core\Cache\CacheBackendInterface; use Drupal\Core\Extension\ModuleHandlerInterface; @@ -190,7 +191,7 @@ public function getActionsForRoute($route_appears) { 'route_parameters' => $route_parameters, 'localized_options' => $plugin->getOptions($request), ), - '#access' => $this->accessManager->checkNamedRoute($route_name, $route_parameters, $this->account), + '#access' => $this->accessManager->checkNamedRoute($route_name, $route_parameters, $this->account)->value === AccessInterface::ALLOW, '#weight' => $plugin->getWeight(), ); } diff --git a/core/lib/Drupal/Core/Menu/LocalTaskManager.php b/core/lib/Drupal/Core/Menu/LocalTaskManager.php index 839ef32..fd6ed80 100644 --- a/core/lib/Drupal/Core/Menu/LocalTaskManager.php +++ b/core/lib/Drupal/Core/Menu/LocalTaskManager.php @@ -8,6 +8,7 @@ namespace Drupal\Core\Menu; use Drupal\Component\Plugin\Exception\PluginException; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Access\AccessManagerInterface; use Drupal\Core\Cache\Cache; use Drupal\Core\Cache\CacheBackendInterface; @@ -320,7 +321,7 @@ public function getTasksBuild($current_route_name) { $route_parameters = $child->getRouteParameters($request); // Find out whether the user has access to the task. - $access = $this->accessManager->checkNamedRoute($route_name, $route_parameters, $this->account); + $access = $this->accessManager->checkNamedRoute($route_name, $route_parameters, $this->account)->value === AccessInterface::ALLOW; if ($access) { $active = $this->isRouteActive($current_route_name, $route_name, $route_parameters); diff --git a/core/lib/Drupal/Core/Menu/MenuLinkBase.php b/core/lib/Drupal/Core/Menu/MenuLinkBase.php index 125bbf0..9778afa 100644 --- a/core/lib/Drupal/Core/Menu/MenuLinkBase.php +++ b/core/lib/Drupal/Core/Menu/MenuLinkBase.php @@ -9,6 +9,8 @@ use Drupal\Component\Plugin\Exception\PluginException; use Drupal\Component\Utility\String; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Plugin\PluginBase; use Drupal\Core\Url; @@ -76,7 +78,9 @@ public function isExpanded() { * {@inheritdoc} */ public function isResettable() { - return FALSE; + $access = new AccessCheckResult(TRUE); + $access->value = AccessInterface::DENY; + return $access; } /** diff --git a/core/lib/Drupal/Core/Menu/MenuLinkDefault.php b/core/lib/Drupal/Core/Menu/MenuLinkDefault.php index 5d0f8e2..3322268 100644 --- a/core/lib/Drupal/Core/Menu/MenuLinkDefault.php +++ b/core/lib/Drupal/Core/Menu/MenuLinkDefault.php @@ -7,6 +7,8 @@ namespace Drupal\Core\Menu; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Plugin\ContainerFactoryPluginInterface; use Symfony\Component\DependencyInjection\ContainerInterface; @@ -94,7 +96,9 @@ public function getDescription() { */ public function isResettable() { // The link can be reset if it has an override. - return (bool) $this->staticOverride->loadOverride($this->getPluginId()); + $access = new AccessCheckResult(FALSE); + $access->value = ((bool) $this->staticOverride->loadOverride($this->getPluginId())) ? AccessInterface::ALLOW : AccessInterface::DENY; + return $access; } /** diff --git a/core/lib/Drupal/Core/Path/PathValidator.php b/core/lib/Drupal/Core/Path/PathValidator.php index a90148b..5442428 100644 --- a/core/lib/Drupal/Core/Path/PathValidator.php +++ b/core/lib/Drupal/Core/Path/PathValidator.php @@ -8,6 +8,7 @@ namespace Drupal\Core\Path; use Drupal\Component\Utility\UrlHelper; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Access\AccessManagerInterface; use Drupal\Core\ParamConverter\ParamNotConvertedException; use Drupal\Core\Routing\RequestHelper; @@ -111,7 +112,7 @@ public function isValid($path) { // Consult the access manager. $routes = $collection->all(); $route = reset($routes); - return $this->accessManager->check($route, $request, $this->account); + return $this->accessManager->check($route, $request, $this->account)->value === AccessInterface::ALLOW; } } diff --git a/core/lib/Drupal/Core/Theme/ThemeAccessCheck.php b/core/lib/Drupal/Core/Theme/ThemeAccessCheck.php index 2898034..31c26a9 100644 --- a/core/lib/Drupal/Core/Theme/ThemeAccessCheck.php +++ b/core/lib/Drupal/Core/Theme/ThemeAccessCheck.php @@ -7,6 +7,7 @@ namespace Drupal\Core\Theme; +use Drupal\Core\Access\AccessCheckResult; use Drupal\Core\Routing\Access\AccessInterface; /** @@ -20,11 +21,15 @@ class ThemeAccessCheck implements AccessInterface { * @param string $theme * The name of a theme. * - * @return string - * A \Drupal\Core\Access\AccessInterface constant value. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. */ public function access($theme) { - return $this->checkAccess($theme) ? static::ALLOW : static::DENY; + $access = new AccessCheckResult(TRUE); + // Cacheable until the theme is modified. + $access->cacheability->setTags(array('theme' => $theme)); + $access->value = $this->checkAccess($theme) ? static::ALLOW : static::DENY; + return $access; } /** diff --git a/core/modules/aggregator/src/FeedAccessController.php b/core/modules/aggregator/src/FeedAccessController.php index 73ad32c..a12d8ab 100644 --- a/core/modules/aggregator/src/FeedAccessController.php +++ b/core/modules/aggregator/src/FeedAccessController.php @@ -7,6 +7,8 @@ namespace Drupal\aggregator; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Entity\EntityAccessController; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Session\AccountInterface; @@ -22,13 +24,19 @@ class FeedAccessController extends EntityAccessController { * {@inheritdoc} */ protected function checkAccess(EntityInterface $entity, $operation, $langcode, AccountInterface $account) { + $access = new AccessCheckResult(TRUE); + // Cacheable per role. + $access->cacheability->setContexts(array('cache_context.user.roles')); + switch ($operation) { case 'view': - return $account->hasPermission('access news feeds'); + $access->value = $account->hasPermission('access news feeds') ? AccessInterface::ALLOW : AccessInterface::DENY; + return $access; break; default: - return $account->hasPermission('administer news feeds'); + $access->value = $account->hasPermission('administer news feeds') ? AccessInterface::ALLOW : AccessInterface::DENY; + return $access; break; } } @@ -37,7 +45,11 @@ protected function checkAccess(EntityInterface $entity, $operation, $langcode, A * {@inheritdoc} */ protected function checkCreateAccess(AccountInterface $account, array $context, $entity_bundle = NULL) { - return $account->hasPermission('administer news feeds'); + $access = new AccessCheckResult(TRUE); + // Cacheable per role. + $access->cacheability->setContexts(array('cache_context.user.roles')); + $access->value = $account->hasPermission('administer news feeds') ? AccessInterface::ALLOW : AccessInterface::DENY; + return $access; } } diff --git a/core/modules/block/block.api.php b/core/modules/block/block.api.php index f43d984..2ab8932 100644 --- a/core/modules/block/block.api.php +++ b/core/modules/block/block.api.php @@ -5,6 +5,9 @@ * Hooks provided by the Block module. */ +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; + /** * @defgroup block_api Block API * @{ @@ -139,21 +142,36 @@ function hook_block_view_BASE_BLOCK_ID_alter(array &$build, \Drupal\block\BlockP * @param string $langcode * The language code to perform the access check operation on. * - * @return bool|null - * FALSE denies access. TRUE allows access unless another module returns - * FALSE. If all modules return NULL, then default access rules from - * \Drupal\block\BlockAccessController::checkAccess() are used. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. Its value can be set + * to: + * - AccessInterface::ALLOW: the block may be accessed by the user + * - AccessInterface::KILL: the block may not be accessed by the user + * - AccessInterface::DENY: the hook implementation has no opinion on + * whether this block may be accessed by the user; if all implementations + * of this hook return this, then default access rules from + * \Drupal\block\BlockAccessController::checkAccess() are used. + * The cacheability metadata should also be set, to reflect whether this + * access check result applies for example to all users of this role, or only + * to the current user, or… * * @see \Drupal\Core\Entity\EntityAccessController::access() * @see \Drupal\block\BlockAccessController::checkAccess() * @ingroup block_api */ function hook_block_access(\Drupal\block\Entity\Block $block, $operation, \Drupal\user\Entity\User $account, $langcode) { + $access = new AccessCheckResult(TRUE); + + // Default to no opinion. + $access->value = AccessInterface::DENY; + // Example code that would prevent displaying the 'Powered by Drupal' block in // a region different than the footer. if ($operation == 'view' && $block->get('plugin') == 'system_powered_by_block' && $block->get('region') != 'footer') { - return FALSE; + $access->value = AccessInterface::KILL; } + + return $access; } /** diff --git a/core/modules/block/src/BlockAccessController.php b/core/modules/block/src/BlockAccessController.php index 0089470..576db4d 100644 --- a/core/modules/block/src/BlockAccessController.php +++ b/core/modules/block/src/BlockAccessController.php @@ -7,6 +7,8 @@ namespace Drupal\block; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Entity\EntityAccessController; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Session\AccountInterface; @@ -27,7 +29,9 @@ protected function checkAccess(EntityInterface $entity, $operation, $langcode, A // Deny access to disabled blocks. if (!$entity->status()) { - return FALSE; + $access = new AccessCheckResult(TRUE); + $access->value = AccessInterface::DENY; + return $access; } // Delegate to the plugin. diff --git a/core/modules/block/src/BlockBase.php b/core/modules/block/src/BlockBase.php index ca94ba0..b61ed86 100644 --- a/core/modules/block/src/BlockBase.php +++ b/core/modules/block/src/BlockBase.php @@ -10,6 +10,8 @@ use Drupal\block\Event\BlockConditionContextEvent; use Drupal\block\Event\BlockEvents; use Drupal\Component\Plugin\ContextAwarePluginInterface; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Condition\ConditionAccessResolverTrait; use Drupal\Core\Condition\ConditionPluginBag; use Drupal\Core\Form\FormState; @@ -156,10 +158,17 @@ public function access(AccountInterface $account) { $this->contextHandler()->applyContextMapping($condition, $contexts, $mappings[$condition_id]); } } + // This should not be hardcoded to an uncacheable access check result, but + // in order to fix that, we need condition plugins to return cache contexts, + // otherwise it will be impossible to determine by which cache contexts the + // result should be varied. + $access = new AccessCheckResult(FALSE); if ($this->resolveConditions($conditions, 'and', $contexts, $mappings) === FALSE) { - return FALSE; + $access->value = AccessInterface::KILL; + return $access; } - return $this->blockAccess($account); + $access->value = $this->blockAccess($account) ? AccessInterface::ALLOW : AccessInterface::KILL; + return $access; } /** diff --git a/core/modules/block/src/Plugin/DisplayVariant/FullPageVariant.php b/core/modules/block/src/Plugin/DisplayVariant/FullPageVariant.php index 7243362..4749aa2 100644 --- a/core/modules/block/src/Plugin/DisplayVariant/FullPageVariant.php +++ b/core/modules/block/src/Plugin/DisplayVariant/FullPageVariant.php @@ -7,6 +7,7 @@ namespace Drupal\block\Plugin\DisplayVariant; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Entity\EntityStorageInterface; use Drupal\Core\Entity\EntityViewBuilderInterface; use Drupal\Core\Plugin\ContainerFactoryPluginInterface; @@ -120,7 +121,7 @@ public function build() { foreach ($this->getRegionAssignments() as $region => $blocks) { /** @var $blocks \Drupal\block\BlockInterface[] */ foreach ($blocks as $key => $block) { - if ($block->access('view')) { + if ($block->access('view')->value === AccessInterface::ALLOW) { $build[$region][$key] = $this->blockViewBuilder->view($block); } } diff --git a/core/modules/block/tests/src/Plugin/DisplayVariant/FullPageVariantTest.php b/core/modules/block/tests/src/Plugin/DisplayVariant/FullPageVariantTest.php index 9b45337..a19af6e 100644 --- a/core/modules/block/tests/src/Plugin/DisplayVariant/FullPageVariantTest.php +++ b/core/modules/block/tests/src/Plugin/DisplayVariant/FullPageVariantTest.php @@ -7,6 +7,8 @@ namespace Drupal\block\Tests\Plugin\DisplayVariant; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\Tests\UnitTestCase; /** @@ -87,26 +89,28 @@ public function testBuild() { $blocks_config = array( 'block1' => array( - TRUE, 'top', 0, + AccessInterface::ALLOW, 'top', 0, ), // Test a block without access. 'block2' => array( - FALSE, 'bottom', 0, + AccessInterface::DENY, 'bottom', 0, ), // Test two blocks in the same region with specific weight. 'block3' => array( - TRUE, 'bottom', 5, + AccessInterface::ALLOW, 'bottom', 5, ), 'block4' => array( - TRUE, 'bottom', -5, + AccessInterface::ALLOW, 'bottom', -5, ), ); $blocks = array(); foreach ($blocks_config as $block_id => $block_config) { $block = $this->getMock('Drupal\block\BlockInterface'); + $block_access = new AccessCheckResult(TRUE); + $block_access->value = $block_config[0]; $block->expects($this->once()) ->method('access') - ->will($this->returnValue($block_config[0])); + ->will($this->returnValue($block_access)); $block->expects($this->any()) ->method('get') ->will($this->returnValueMap(array( diff --git a/core/modules/block_content/src/BlockContentAccessController.php b/core/modules/block_content/src/BlockContentAccessController.php index 5709498..b945790 100644 --- a/core/modules/block_content/src/BlockContentAccessController.php +++ b/core/modules/block_content/src/BlockContentAccessController.php @@ -7,6 +7,8 @@ namespace Drupal\block_content; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Entity\EntityAccessController; use Drupal\Core\Session\AccountInterface; @@ -21,7 +23,9 @@ class BlockContentAccessController extends EntityAccessController { */ protected function checkAccess(EntityInterface $entity, $operation, $langcode, AccountInterface $account) { if ($operation === 'view') { - return TRUE; + $access = new AccessCheckResult(TRUE); + $access->value = AccessInterface::ALLOW; + return $access; } return parent::checkAccess($entity, $operation, $langcode, $account); } diff --git a/core/modules/book/book.module b/core/modules/book/book.module index c791b68..681e16d 100644 --- a/core/modules/book/book.module +++ b/core/modules/book/book.module @@ -125,7 +125,7 @@ function book_node_links_alter(array &$node_links, NodeInterface $node, array &$ if ($context['view_mode'] == 'full' && node_is_page($node)) { $child_type = \Drupal::config('book.settings')->get('child_type'); $access_controller = \Drupal::entityManager()->getAccessController('node'); - if (($account->hasPermission('add content to books') || $account->hasPermission('administer book outlines')) && $access_controller->createAccess($child_type) && $node->isPublished() && $node->book['depth'] < BookManager::BOOK_MAX_DEPTH) { + if (($account->hasPermission('add content to books') || $account->hasPermission('administer book outlines')) && $access_controller->createAccess($child_type)->value === \Drupal\Core\Access\AccessInterface::ALLOW && $node->isPublished() && $node->book['depth'] < BookManager::BOOK_MAX_DEPTH) { $links['book_add_child'] = array( 'title' => t('Add child page'), 'href' => 'node/add/' . $child_type, diff --git a/core/modules/book/src/Access/BookNodeIsRemovableAccessCheck.php b/core/modules/book/src/Access/BookNodeIsRemovableAccessCheck.php index 861c252..c6212f1 100644 --- a/core/modules/book/src/Access/BookNodeIsRemovableAccessCheck.php +++ b/core/modules/book/src/Access/BookNodeIsRemovableAccessCheck.php @@ -8,13 +8,14 @@ namespace Drupal\book\Access; use Drupal\book\BookManagerInterface; +use Drupal\Core\Access\AccessCheckResult; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\node\NodeInterface; /** * Determines whether the requested node can be removed from its book. */ -class BookNodeIsRemovableAccessCheck implements AccessInterface { +class BookNodeIsRemovableAccessCheck implements AccessInterface{ /** * Book Manager Service. @@ -39,11 +40,15 @@ public function __construct(BookManagerInterface $book_manager) { * @param \Drupal\node\NodeInterface $node * The node requested to be removed from its book. * - * @return string - * A \Drupal\Core\Access\AccessInterface constant value. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. */ public function access(NodeInterface $node) { - return $this->bookManager->checkNodeIsRemovable($node) ? static::ALLOW : static::DENY; + $access = new AccessCheckResult(TRUE); + // Cacheable until the book node is modified. + $access->cacheability->setTags($node->getCacheTag()); + $access->value = $this->bookManager->checkNodeIsRemovable($node) ? static::ALLOW : static::DENY; + return $access; } } diff --git a/core/modules/book/src/BookManager.php b/core/modules/book/src/BookManager.php index 45f9adf..d118e9c 100644 --- a/core/modules/book/src/BookManager.php +++ b/core/modules/book/src/BookManager.php @@ -8,6 +8,7 @@ namespace Drupal\book; use Drupal\Component\Utility\Unicode; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Cache\Cache; use Drupal\Core\Database\Connection; use Drupal\Core\Entity\EntityManagerInterface; @@ -999,7 +1000,7 @@ public function bookLinkTranslate(&$link) { // Access will already be set in the tree functions. if (!isset($link['access'])) { $node = $this->entityManager->getStorage('node')->load($link['nid']); - $link['access'] = $node && $node->access('view'); + $link['access'] = $node && $node->access('view')->value === AccessInterface::ALLOW; } // For performance, don't localize a link the user can't access. if ($link['access']) { diff --git a/core/modules/book/src/Plugin/Block/BookNavigationBlock.php b/core/modules/book/src/Plugin/Block/BookNavigationBlock.php index 51d0a0d..4881902 100644 --- a/core/modules/book/src/Plugin/Block/BookNavigationBlock.php +++ b/core/modules/book/src/Plugin/Block/BookNavigationBlock.php @@ -9,6 +9,7 @@ use Drupal\block\BlockBase; use Drupal\book\BookManagerInterface; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Form\FormStateInterface; use Drupal\Core\Plugin\ContainerFactoryPluginInterface; use Symfony\Component\DependencyInjection\ContainerInterface; @@ -133,7 +134,7 @@ public function build() { $book['in_active_trail'] = FALSE; // Check whether user can access the book link. $book_node = node_load($book['nid']); - $book['access'] = $book_node->access('view'); + $book['access'] = $book_node->access('view')->value === AccessInterface::ALLOW; $pseudo_tree[0]['link'] = $book; $book_menus[$book_id] = $this->bookManager->bookTreeOutput($pseudo_tree); } diff --git a/core/modules/comment/src/CommentAccessController.php b/core/modules/comment/src/CommentAccessController.php index d741d86..72cdcb8 100644 --- a/core/modules/comment/src/CommentAccessController.php +++ b/core/modules/comment/src/CommentAccessController.php @@ -7,6 +7,8 @@ namespace Drupal\comment; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Entity\EntityAccessController; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Session\AccountInterface; @@ -22,33 +24,67 @@ class CommentAccessController extends EntityAccessController { * {@inheritdoc} */ protected function checkAccess(EntityInterface $entity, $operation, $langcode, AccountInterface $account) { + $access = new AccessCheckResult(TRUE); + $access->value = AccessInterface::DENY; + /** @var \Drupal\Core\Entity\EntityInterface|\Drupal\user\EntityOwnerInterface $entity */ switch ($operation) { case 'view': if ($account->hasPermission('access comments') && $entity->isPublished() || $account->hasPermission('administer comments')) { - return $entity->getCommentedEntity()->access($operation, $account); + // Cacheable per role. + $access->cacheability->setContexts(array('cache_context.user.roles')); + if (!$account->hasPermission('administer comments')) { + // Cacheable until comment is modified. + $access->cacheability->setTags($entity->getCacheTag()); + } + $commented_entity_access = $entity->getCommentedEntity()->access($operation, $account); + // We want access the commented entity to determine access to the + // comment, so we don't specify an opinion yet (we use DENY), but we + // want our cacheability metadata to be reflected, so we merge our + // access check result with that of the commented entity using the + // AccessCheckResult::any() method. + return AccessCheckResult::any(array($access, $commented_entity_access)); } break; case 'update': - return ($account->id() && $account->id() == $entity->getOwnerId() && $entity->isPublished() && $account->hasPermission('edit own comments')) || $account->hasPermission('administer comments'); + // Cacheable per role. + $access->cacheability->setContexts(array('cache_context.user.roles')); + if ($account->hasPermission('administer comments')) { + $access->value = AccessInterface::ALLOW; + return $access; + } + else { + // Cacheable per user. + $access->cacheability->addContexts(array('cache_context.user')); + // Cacheable until comment is modified. + $access->cacheability->setTags($entity->getCacheTag()); + $access->value = ($account->id() && $account->id() == $entity->getOwnerId() && $entity->isPublished() && $account->hasPermission('edit own comments')) ? AccessInterface::ALLOW : AccessInterface::DENY; + return $access; + } break; case 'delete': - return $account->hasPermission('administer comments'); - break; - case 'approve': - return $account->hasPermission('administer comments'); + // Cacheable per role. + $access->cacheability->setContexts(array('cache_context.user.roles')); + $access->value = $account->hasPermission('administer comments') ? AccessInterface::ALLOW : AccessInterface::DENY; + return $access; break; } + + return $access; } /** * {@inheritdoc} */ protected function checkCreateAccess(AccountInterface $account, array $context, $entity_bundle = NULL) { - return $account->hasPermission('post comments'); + $access = new AccessCheckResult(TRUE); + // Cacheable per role. + $access->cacheability->setContexts(array('cache_context.user.roles')); + $access->value = $account->hasPermission('post comments') ? AccessInterface::ALLOW : AccessInterface::DENY; + return $access; } } diff --git a/core/modules/comment/src/CommentViewBuilder.php b/core/modules/comment/src/CommentViewBuilder.php index 6a4760b..daa8087 100644 --- a/core/modules/comment/src/CommentViewBuilder.php +++ b/core/modules/comment/src/CommentViewBuilder.php @@ -8,6 +8,7 @@ namespace Drupal\comment; use Drupal\comment\Plugin\Field\FieldType\CommentItemInterface; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Access\CsrfTokenGenerator; use Drupal\Core\Entity\Display\EntityViewDisplayInterface; use Drupal\Core\Entity\EntityInterface; @@ -226,7 +227,7 @@ protected static function buildLinks(CommentInterface $entity, EntityInterface $ $container = \Drupal::getContainer(); if ($status == CommentItemInterface::OPEN) { - if ($entity->access('delete')) { + if ($entity->access('delete')->value === AccessInterface::ALLOW) { $links['comment-delete'] = array( 'title' => t('Delete'), 'href' => "comment/{$entity->id()}/delete", @@ -234,21 +235,21 @@ protected static function buildLinks(CommentInterface $entity, EntityInterface $ ); } - if ($entity->access('update')) { + if ($entity->access('update')->value === AccessInterface::ALLOW) { $links['comment-edit'] = array( 'title' => t('Edit'), 'href' => "comment/{$entity->id()}/edit", 'html' => TRUE, ); } - if ($entity->access('create')) { + if ($entity->access('create')->value === AccessInterface::ALLOW) { $links['comment-reply'] = array( 'title' => t('Reply'), 'href' => "comment/reply/{$entity->getCommentedEntityTypeId()}/{$entity->getCommentedEntityId()}/{$entity->getFieldName()}/{$entity->id()}", 'html' => TRUE, ); } - if (!$entity->isPublished() && $entity->access('approve')) { + if (!$entity->isPublished() && $entity->access('approve')->value === AccessInterface::ALLOW) { $links['comment-approve'] = array( 'title' => t('Approve'), 'route_name' => 'comment.approve', diff --git a/core/modules/comment/src/Controller/CommentController.php b/core/modules/comment/src/Controller/CommentController.php index e550afc..b99e1fc 100644 --- a/core/modules/comment/src/Controller/CommentController.php +++ b/core/modules/comment/src/Controller/CommentController.php @@ -10,6 +10,7 @@ use Drupal\comment\CommentInterface; use Drupal\comment\CommentManagerInterface; use Drupal\comment\Plugin\Field\FieldType\CommentItemInterface; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Controller\ControllerBase; use Drupal\Core\Entity\EntityInterface; use Symfony\Component\DependencyInjection\ContainerInterface; @@ -108,7 +109,7 @@ public function commentApprove(CommentInterface $comment) { public function commentPermalink(Request $request, CommentInterface $comment) { if ($entity = $comment->getCommentedEntity()) { // Check access permissions for the entity. - if (!$entity->access('view')) { + if ($entity->access('view')->value !== AccessInterface::ALLOW) { throw new AccessDeniedHttpException(); } $field_definition = $this->entityManager()->getFieldDefinitions($entity->getEntityTypeId(), $entity->bundle())[$comment->getFieldName()]; @@ -236,7 +237,7 @@ public function getReplyForm(Request $request, $entity_type, $entity_id, $field_ } // The comment is in response to a entity. - elseif ($entity->access('view', $account)) { + elseif ($entity->access('view', $account)->value === AccessInterface::ALLOW) { // We make sure the field value isn't set so we don't end up with a // redirect loop. $entity = clone $entity; diff --git a/core/modules/comment/src/Form/CommentAdminOverview.php b/core/modules/comment/src/Form/CommentAdminOverview.php index b1d3cb7..d6d1435 100644 --- a/core/modules/comment/src/Form/CommentAdminOverview.php +++ b/core/modules/comment/src/Form/CommentAdminOverview.php @@ -10,6 +10,7 @@ use Drupal\comment\CommentInterface; use Drupal\comment\CommentStorageInterface; use Drupal\Component\Utility\Unicode; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Datetime\Date as DateFormatter; use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\Extension\ModuleHandlerInterface; @@ -207,7 +208,7 @@ public function buildForm(array $form, FormStateInterface $form_state, $type = ' 'data' => array( '#type' => 'link', '#title' => $commented_entity->label(), - '#access' => $commented_entity->access('view'), + '#access' => $commented_entity->access('view')->value === AccessInterface::ALLOW, ) + $commented_entity->urlInfo()->toRenderArray(), ), 'changed' => $this->dateFormatter->format($comment->getChangedTime(), 'short'), diff --git a/core/modules/config/tests/config_test/src/ConfigTestAccessController.php b/core/modules/config/tests/config_test/src/ConfigTestAccessController.php index fee7966..bb7373c 100644 --- a/core/modules/config/tests/config_test/src/ConfigTestAccessController.php +++ b/core/modules/config/tests/config_test/src/ConfigTestAccessController.php @@ -7,6 +7,8 @@ namespace Drupal\config_test; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; use Drupal\Core\Entity\EntityAccessController; use Drupal\Core\Entity\EntityInterface; @@ -20,14 +22,18 @@ class ConfigTestAccessController extends EntityAccessController { * {@inheritdoc} */ public function checkAccess(EntityInterface $entity, $operation, $langcode, AccountInterface $account) { - return TRUE; + $access = new AccessCheckResult(TRUE); + $access->value = AccessInterface::ALLOW; + return $access; } /** * {@inheritdoc} */ protected function checkCreateAccess(AccountInterface $account, array $context, $entity_bundle = NULL) { - return TRUE; + $access = new AccessCheckResult(TRUE); + $access->value = AccessInterface::ALLOW; + return $access; } } diff --git a/core/modules/config_translation/src/Access/ConfigTranslationFormAccess.php b/core/modules/config_translation/src/Access/ConfigTranslationFormAccess.php index 791ef85..d6ac0d3 100644 --- a/core/modules/config_translation/src/Access/ConfigTranslationFormAccess.php +++ b/core/modules/config_translation/src/Access/ConfigTranslationFormAccess.php @@ -8,6 +8,7 @@ namespace Drupal\config_translation\Access; use Drupal\Core\Session\AccountInterface; +use Drupal\language\Entity\Language; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Routing\Route; @@ -22,8 +23,8 @@ class ConfigTranslationFormAccess extends ConfigTranslationOverviewAccess { public function access(Route $route, Request $request, AccountInterface $account) { // For the translation forms we have a target language, so we need some // checks in addition to the checks performed for the translation overview. - $base_access = parent::access($route, $request, $account); - if ($base_access === static::ALLOW) { + $result = parent::access($route, $request, $account); + if ($result->value === static::ALLOW) { $target_language = language_load($request->attributes->get('langcode')); // Make sure that the target language is not locked, and that the target @@ -35,9 +36,14 @@ public function access(Route $route, Request $request, AccountInterface $account !$target_language->locked && $target_language->id != $this->sourceLanguage->id; - return $access ? static::ALLOW : static::DENY; + $result->value = $access ? static::ALLOW : static::DENY; + // Retain the same cacheability as the parent access check's result, but + // also make it dependent on the target language. + $result->cacheability->addTags(Language::load($target_language->getId())->getCacheTag()); + return $result; } - return static::DENY; + $result->value = static::DENY; + return $result; } } diff --git a/core/modules/config_translation/src/Access/ConfigTranslationOverviewAccess.php b/core/modules/config_translation/src/Access/ConfigTranslationOverviewAccess.php index 529bc4a..59c0baa 100644 --- a/core/modules/config_translation/src/Access/ConfigTranslationOverviewAccess.php +++ b/core/modules/config_translation/src/Access/ConfigTranslationOverviewAccess.php @@ -8,8 +8,11 @@ namespace Drupal\config_translation\Access; use Drupal\config_translation\ConfigMapperManagerInterface; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Cache\Cache; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; +use Drupal\language\Entity\Language; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Routing\Route; @@ -52,10 +55,12 @@ public function __construct(ConfigMapperManagerInterface $config_mapper_manager) * @param \Drupal\Core\Session\AccountInterface $account * The currently logged in account. * - * @return string - * A \Drupal\Core\Access\AccessInterface constant value. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. */ public function access(Route $route, Request $request, AccountInterface $account) { + $result = new AccessCheckResult(TRUE); + /** @var \Drupal\config_translation\ConfigMapperInterface $mapper */ $mapper = $this->configMapperManager->createInstance($route->getDefault('plugin_id')); $mapper->populateFromRequest($request); @@ -71,7 +76,16 @@ public function access(Route $route, Request $request, AccountInterface $account $mapper->hasTranslatable() && !$this->sourceLanguage->locked; - return $access ? static::ALLOW : static::DENY; + $result->value = $access ? static::ALLOW : static::DENY; + // Cacheable per role. + $result->cacheability->setContexts(array('cache_context.user.roles')); + // Cacheable until the language entity is modified. + $language_entity = Language::load($this->sourceLanguage->getId()); + if ($language_entity !== NULL) { + $result->cacheability->setTags($language_entity->getCacheTag()); + } + + return $result; } } diff --git a/core/modules/config_translation/src/Controller/ConfigTranslationController.php b/core/modules/config_translation/src/Controller/ConfigTranslationController.php index cb7f6b7..260b2a6 100644 --- a/core/modules/config_translation/src/Controller/ConfigTranslationController.php +++ b/core/modules/config_translation/src/Controller/ConfigTranslationController.php @@ -8,6 +8,7 @@ namespace Drupal\config_translation\Controller; use Drupal\config_translation\ConfigMapperManagerInterface; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Access\AccessManagerInterface; use Drupal\Core\Controller\ControllerBase; use Drupal\Core\Language\Language; @@ -174,7 +175,7 @@ public function itemPage(Request $request, $plugin_id) { // Note that the parameters don't really matter here since we're // passing in the request which already has the upcast attributes. $parameters = array(); - $edit_access = $this->accessManager->checkNamedRoute($route_name, $parameters, $this->account, $route_request); + $edit_access = $this->accessManager->checkNamedRoute($route_name, $parameters, $this->account, $route_request)->value === AccessInterface::ALLOW; } // Build list of operations. diff --git a/core/modules/contact/src/Access/ContactPageAccess.php b/core/modules/contact/src/Access/ContactPageAccess.php index 97a7b54..b68441e 100644 --- a/core/modules/contact/src/Access/ContactPageAccess.php +++ b/core/modules/contact/src/Access/ContactPageAccess.php @@ -7,6 +7,8 @@ namespace Drupal\contact\Access; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Cache\Cache; use Drupal\Core\Config\ConfigFactoryInterface; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; @@ -53,45 +55,58 @@ public function __construct(ConfigFactoryInterface $config_factory, UserDataInte * @param \Drupal\Core\Session\AccountInterface $account * The currently logged in account. * - * @return string - * A \Drupal\Core\Access\AccessInterface constant value. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. */ public function access(UserInterface $user, AccountInterface $account) { $contact_account = $user; + $access = new AccessCheckResult(TRUE); + + // Cacheable per user and role until the contacted user is modified. + $access->cacheability + ->setContexts(array('cache_context.user', 'cache_context.user.roles')) + ->setTags($user->getCacheTag()); // Anonymous users cannot have contact forms. if ($contact_account->isAnonymous()) { - return static::DENY; + $access->value = static::DENY; + return $access; } // Users may not contact themselves. if ($account->id() == $contact_account->id()) { - return static::DENY; + $access->value = static::DENY; + return $access; } // User administrators should always have access to personal contact forms. if ($account->hasPermission('administer users')) { - return static::ALLOW; + $access->value = static::ALLOW; + return $access; } // If requested user has been blocked, do not allow users to contact them. if ($contact_account->isBlocked()) { - return static::DENY; + $access->value = static::DENY; + return $access; } // If the requested user has disabled their contact form, do not allow users // to contact them. $account_data = $this->userData->get('contact', $contact_account->id(), 'enabled'); if (isset($account_data) && empty($account_data)) { - return static::DENY; + $access->value = static::DENY; + return $access; } // If the requested user did not save a preference yet, deny access if the // configured default is disabled. else if (!$this->configFactory->get('contact.settings')->get('user_default_enabled')) { - return static::DENY; + $access->value = static::DENY; + return $access; } - return $account->hasPermission('access user contact forms') ? static::ALLOW : static::DENY; + $access->value = $account->hasPermission('access user contact forms') ? static::ALLOW : static::DENY; + return $access; } } diff --git a/core/modules/contact/src/CategoryAccessController.php b/core/modules/contact/src/CategoryAccessController.php index f798d79..c66ea88 100644 --- a/core/modules/contact/src/CategoryAccessController.php +++ b/core/modules/contact/src/CategoryAccessController.php @@ -7,6 +7,8 @@ namespace Drupal\contact; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Entity\EntityAccessController; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Session\AccountInterface; @@ -22,14 +24,22 @@ class CategoryAccessController extends EntityAccessController { * {@inheritdoc} */ public function checkAccess(EntityInterface $entity, $operation, $langcode, AccountInterface $account) { + $access = new AccessCheckResult(TRUE); + if ($operation == 'view') { // Do not allow access personal category via site-wide route. - return $account->hasPermission('access site-wide contact form') && $entity->id() !== 'personal'; + $access->value = ($account->hasPermission('access site-wide contact form') && $entity->id() !== 'personal') ? AccessInterface::ALLOW : AccessInterface::DENY; + // Cacheable per role. + $access->cacheability->addContexts(array('cache_context.user.roles')); + return $access; } elseif ($operation == 'delete' || $operation == 'update') { // Do not allow the 'personal' category to be deleted, as it's used for // the personal contact form. - return $account->hasPermission('administer contact forms') && $entity->id() !== 'personal'; + $access->value = ($account->hasPermission('administer contact forms') && $entity->id() !== 'personal') ? AccessInterface::ALLOW : AccessInterface::DENY; + // Cacheable per role. + $access->cacheability->addContexts(array('cache_context.user.roles')); + return $access; } return parent::checkAccess($entity, $operation, $langcode, $account); diff --git a/core/modules/contact/src/Plugin/views/field/ContactLink.php b/core/modules/contact/src/Plugin/views/field/ContactLink.php index 8b0ce91..41a4486 100644 --- a/core/modules/contact/src/Plugin/views/field/ContactLink.php +++ b/core/modules/contact/src/Plugin/views/field/ContactLink.php @@ -7,6 +7,7 @@ namespace Drupal\contact\Plugin\views\field; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Access\AccessManagerInterface; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Form\FormStateInterface; @@ -112,7 +113,7 @@ protected function renderLink(EntityInterface $entity, ResultRow $values) { // Check access when we pull up the user account so we know // if the user has made the contact page available. - if (!$this->accessManager->checkNamedRoute('contact.personal_page', array('user' => $entity->id()), $this->currentUser())) { + if ($this->accessManager->checkNamedRoute('contact.personal_page', array('user' => $entity->id()), $this->currentUser())->value !== AccessInterface::ALLOW) { return; } diff --git a/core/modules/content_translation/content_translation.module b/core/modules/content_translation/content_translation.module index c79213a..1a80193 100644 --- a/core/modules/content_translation/content_translation.module +++ b/core/modules/content_translation/content_translation.module @@ -5,6 +5,8 @@ * Allows entities to be translated into different languages. */ +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Entity\ContentEntityInterface; use Drupal\Core\Entity\EntityFormInterface; use Drupal\Core\Entity\EntityInterface; diff --git a/core/modules/content_translation/content_translation.pages.inc b/core/modules/content_translation/content_translation.pages.inc index 9898678..02554fb 100644 --- a/core/modules/content_translation/content_translation.pages.inc +++ b/core/modules/content_translation/content_translation.pages.inc @@ -5,6 +5,7 @@ * The content translation user interface. */ +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Language\LanguageInterface; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Entity\ContentEntityInterface; @@ -84,7 +85,7 @@ function content_translation_overview(EntityInterface $entity) { // If the user is allowed to edit the entity we point the edit link to // the entity form, otherwise if we are not dealing with the original // language we point the link to the translation form. - if ($entity->access('update')) { + if ($entity->access('update')->value === AccessInterface::ALLOW) { $links['edit'] = isset($edit_links->links[$langcode]['href']) ? $edit_links->links[$langcode] : array('href' => $rel['edit-form'], 'language' => $language); } elseif (!$is_original && $controller->getTranslationAccess($entity, 'update')) { @@ -207,7 +208,7 @@ function content_translation_add_page(EntityInterface $entity, LanguageInterface $form_state['langcode'] = $target->id; $form_state['content_translation']['source'] = $source; $form_state['content_translation']['target'] = $target; - $form_state['content_translation']['translation_form'] = !$entity->access('update'); + $form_state['content_translation']['translation_form'] = $entity->access('update')->value !== AccessInterface::ALLOW; return \Drupal::service('entity.form_builder')->getForm($entity, 'default', $form_state); } diff --git a/core/modules/content_translation/src/Access/ContentTranslationManageAccessCheck.php b/core/modules/content_translation/src/Access/ContentTranslationManageAccessCheck.php index 137b52e..452f3c9 100644 --- a/core/modules/content_translation/src/Access/ContentTranslationManageAccessCheck.php +++ b/core/modules/content_translation/src/Access/ContentTranslationManageAccessCheck.php @@ -7,10 +7,13 @@ namespace Drupal\content_translation\Access; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Cache\Cache; use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\Language\LanguageInterface; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; +use Drupal\language\Entity\Language; use Symfony\Component\Routing\Route; use Symfony\Component\HttpFoundation\Request; @@ -53,16 +56,27 @@ public function __construct(EntityManagerInterface $manager) { * (optional) For an update or delete operation, the language code of the * translation being updated or deleted. * - * @return string - * A \Drupal\Core\Access\AccessInterface constant value. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. */ public function access(Route $route, Request $request, AccountInterface $account, $source = NULL, $target = NULL, $language = NULL) { + $access = new AccessCheckResult(TRUE); + $entity_type = $request->attributes->get('_entity_type_id'); /** @var $entity \Drupal\Core\Entity\EntityInterface */ if ($entity = $request->attributes->get($entity_type)) { $operation = $route->getRequirement('_access_content_translation_manage'); $controller = content_translation_controller($entity_type, $account); + // Cacheable per role until the entity or the list of languages are + // modified. + $access->cacheability + ->setContexts(array('cache_context.user.roles')) + ->addTags($entity->getCacheTag()); + if (isset($source)) { + $access->cacheability->addTags(Language::load($source)->getListCacheTags()); + } + // Load translation. $translations = $entity->getTranslationLanguages(); $languages = language_list(); @@ -71,24 +85,27 @@ public function access(Route $route, Request $request, AccountInterface $account case 'create': $source = language_load($source) ?: $entity->language(); $target = language_load($target) ?: \Drupal::languageManager()->getCurrentLanguage(LanguageInterface::TYPE_CONTENT); - return ($source->id != $target->id + $access->value = ($source->id != $target->id && isset($languages[$source->id]) && isset($languages[$target->id]) && !isset($translations[$target->id]) && $controller->getTranslationAccess($entity, $operation)) ? static::ALLOW : static::DENY; + return $access; case 'update': case 'delete': $language = language_load($language) ?: \Drupal::languageManager()->getCurrentLanguage(LanguageInterface::TYPE_CONTENT); - return isset($languages[$language->id]) + $access->value = isset($languages[$language->id]) && $language->id != $entity->getUntranslated()->language()->id && isset($translations[$language->id]) && $controller->getTranslationAccess($entity, $operation) ? static::ALLOW : static::DENY; + return $access; } } - return static::DENY; + $access->value = static::DENY; + return $access; } } diff --git a/core/modules/content_translation/src/Access/ContentTranslationOverviewAccess.php b/core/modules/content_translation/src/Access/ContentTranslationOverviewAccess.php index fabd61b..e69dae1 100644 --- a/core/modules/content_translation/src/Access/ContentTranslationOverviewAccess.php +++ b/core/modules/content_translation/src/Access/ContentTranslationOverviewAccess.php @@ -7,6 +7,7 @@ namespace Drupal\content_translation\Access; +use Drupal\Core\Access\AccessCheckResult; use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; @@ -42,21 +43,29 @@ public function __construct(EntityManagerInterface $manager) { * @param \Drupal\Core\Session\AccountInterface $account * The currently logged in account. * - * @return string - * A \Drupal\Core\Access\AccessInterface constant value. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. */ public function access(Request $request, AccountInterface $account) { + $result = new AccessCheckResult(TRUE); + $entity_type = $request->attributes->get('_entity_type_id'); if ($entity = $request->attributes->get($entity_type)) { // Get entity base info. $bundle = $entity->bundle(); + // Cacheable per user until the entity is modified. + $result->cacheability + ->setContexts(array('cache_context.user')) + ->addTags($entity->getCacheTag()); + // Get entity access callback. $definition = $this->entityManager->getDefinition($entity_type); $translation = $definition->get('translation'); $access_callback = $translation['content_translation']['access_callback']; if (call_user_func($access_callback, $entity)) { - return static::ALLOW; + $result->value = static::ALLOW; + return $result; } // Check per entity permission. @@ -65,10 +74,13 @@ public function access(Request $request, AccountInterface $account) { $permission = "translate {$bundle} {$entity_type}"; } if ($account->hasPermission($permission)) { - return static::ALLOW; + $result->value = static::ALLOW; + return $result; } } - return static::DENY; + $result->value = static::DENY; + return $result; } + } diff --git a/core/modules/entity/src/Entity/EntityFormDisplay.php b/core/modules/entity/src/Entity/EntityFormDisplay.php index 6457e23..b44e035 100644 --- a/core/modules/entity/src/Entity/EntityFormDisplay.php +++ b/core/modules/entity/src/Entity/EntityFormDisplay.php @@ -7,6 +7,7 @@ namespace Drupal\entity\Entity; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Entity\ContentEntityInterface; use Drupal\Core\Entity\Display\EntityFormDisplayInterface; use Drupal\Core\Form\FormStateInterface; @@ -158,7 +159,7 @@ public function buildForm(ContentEntityInterface $entity, array &$form, FormStat if ($widget = $this->getRenderer($name)) { $items->filterEmptyItems(); $form[$name] = $widget->form($items, $form, $form_state); - $form[$name]['#access'] = $items->access('edit'); + $form[$name]['#access'] = $items->access('edit')->value === AccessInterface::ALLOW; // Assign the correct weight. This duplicates the reordering done in // processForm(), but is needed for other forms calling this method diff --git a/core/modules/entity/src/Entity/EntityViewDisplay.php b/core/modules/entity/src/Entity/EntityViewDisplay.php index 3dcc55d..5c6745a 100644 --- a/core/modules/entity/src/Entity/EntityViewDisplay.php +++ b/core/modules/entity/src/Entity/EntityViewDisplay.php @@ -8,6 +8,7 @@ namespace Drupal\entity\Entity; use Drupal\Component\Utility\NestedArray; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Entity\Display\EntityViewDisplayInterface; use Drupal\Core\Entity\ContentEntityInterface; use Drupal\entity\EntityDisplayBase; @@ -231,7 +232,12 @@ public function buildMultiple(array $entities) { foreach ($entities as $key => $entity) { $items = $entity->get($field_name); $build_list[$key][$field_name] = $formatter->view($items); - $build_list[$key][$field_name]['#access'] = $items->access('view'); + $access = $items->access('view'); + // @todo Improve in https://www.drupal.org/node/2099137; + // EntityViewBuilder::getBuildDefaults() will be able to use + // $access->cacheability's cacheability metadata to affect render + // cacheing! + $build_list[$key][$field_name]['#access'] = $access->value === AccessInterface::ALLOW; } } } diff --git a/core/modules/entity_reference/src/EntityReferenceAutocomplete.php b/core/modules/entity_reference/src/EntityReferenceAutocomplete.php index 63f8a00..501bfea 100644 --- a/core/modules/entity_reference/src/EntityReferenceAutocomplete.php +++ b/core/modules/entity_reference/src/EntityReferenceAutocomplete.php @@ -8,6 +8,7 @@ namespace Drupal\entity_reference; use Drupal\Component\Utility\Tags; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\Field\FieldDefinitionInterface; use Drupal\entity_reference\Plugin\Type\SelectionPluginManager; @@ -80,7 +81,7 @@ public function getMatches(FieldDefinitionInterface $field_definition, $entity_t if ($entity_id !== 'NULL') { $entity = $this->entityManager->getStorage($entity_type)->load($entity_id); - if (!$entity || !$entity->access('view')) { + if (!$entity || $entity->access('view')->value !== AccessInterface::ALLOW) { throw new AccessDeniedHttpException(); } } diff --git a/core/modules/entity_reference/src/Tests/EntityReferenceFormatterTest.php b/core/modules/entity_reference/src/Tests/EntityReferenceFormatterTest.php index e3653bb..46b4ec5 100644 --- a/core/modules/entity_reference/src/Tests/EntityReferenceFormatterTest.php +++ b/core/modules/entity_reference/src/Tests/EntityReferenceFormatterTest.php @@ -7,6 +7,7 @@ namespace Drupal\entity_reference\Tests; +use Drupal\Core\Access\AccessInterface; use Drupal\system\Tests\Entity\EntityUnitTestBase; /** @@ -105,7 +106,7 @@ public function testAccess() { $referencing_entity->{$field_name}->entity = $this->referencedEntity; // Assert user doesn't have access to the entity. - $this->assertFalse($this->referencedEntity->access('view'), 'Current user does not have access to view the referenced entity.'); + $this->assertNotEqual(AccessInterface::ALLOW, $this->referencedEntity->access('view')->value, 'Current user does not have access to view the referenced entity.'); $formatter_manager = $this->container->get('plugin.manager.field.formatter'); diff --git a/core/modules/field/src/FieldInstanceConfigAccessController.php b/core/modules/field/src/FieldInstanceConfigAccessController.php index 1146e1b..659d3de 100644 --- a/core/modules/field/src/FieldInstanceConfigAccessController.php +++ b/core/modules/field/src/FieldInstanceConfigAccessController.php @@ -7,6 +7,8 @@ namespace Drupal\field; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Entity\EntityAccessController; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Session\AccountInterface; @@ -20,10 +22,18 @@ class FieldInstanceConfigAccessController extends EntityAccessController { * {@inheritdoc} */ protected function checkAccess(EntityInterface $entity, $operation, $langcode, AccountInterface $account) { + $access = new AccessCheckResult(TRUE); if ($operation == 'delete' && $entity->getFieldStorageDefinition()->isLocked()) { - return FALSE; + // Cacheable until entity is modified. + $access->cacheability->setTags($entity->getCacheTag()); + $access->value = AccessInterface::KILL; + return $access; } - return $account->hasPermission('administer ' . $entity->entity_type . ' fields'); + + // Cacheable per role. + $access->cacheability->setContexts(array('cache_context.user.roles')); + $access->value = $account->hasPermission('administer ' . $entity->entity_type . ' fields') ? AccessInterface::ALLOW : AccessInterface::DENY; + return $access; } } diff --git a/core/modules/field/tests/modules/field_test/field_test.field.inc b/core/modules/field/tests/modules/field_test/field_test.field.inc index 6ebda87..8609d98 100644 --- a/core/modules/field/tests/modules/field_test/field_test.field.inc +++ b/core/modules/field/tests/modules/field_test/field_test.field.inc @@ -5,6 +5,8 @@ * Defines a field type and its formatters and widgets. */ +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Entity\ContentEntityInterface; use Drupal\Core\Entity\Exception\FieldStorageDefinitionUpdateForbiddenException; use Drupal\Core\Field\FieldDefinitionInterface; @@ -39,15 +41,26 @@ function field_test_default_value(ContentEntityInterface $entity, FieldDefinitio * Implements hook_entity_field_access(). */ function field_test_entity_field_access($operation, FieldDefinitionInterface $field_definition, AccountInterface $account, FieldItemListInterface $items = NULL) { + $access = new AccessCheckResult(TRUE); + if ($field_definition->getName() == "field_no_{$operation}_access") { - return FALSE; + // Cacheable until entity is modified. + $access->cacheability->addTags($items->getEntity()->getCacheTag()); + $access->value = AccessInterface::KILL; + return $access; } // Only grant view access to test_view_field fields when the user has // 'view test_view_field content' permission. if ($field_definition->getName() == 'test_view_field' && $operation == 'view' && !$account->hasPermission('view test_view_field content')) { - return FALSE; + // Cacheable per role. + $access->cacheability->addContexts(array('cache_context.user.roles')); + // Cacheable until entity is modified. + $access->cacheability->addTags($items->getEntity()->getCacheTag()); + $access->value = AccessInterface::KILL; + return $access; } - return TRUE; + $access->value = AccessInterface::ALLOW; + return $access; } diff --git a/core/modules/field_ui/src/Access/FormModeAccessCheck.php b/core/modules/field_ui/src/Access/FormModeAccessCheck.php index d35dd15..ead71c1 100644 --- a/core/modules/field_ui/src/Access/FormModeAccessCheck.php +++ b/core/modules/field_ui/src/Access/FormModeAccessCheck.php @@ -7,6 +7,7 @@ namespace Drupal\field_ui\Access; +use Drupal\Core\Access\AccessCheckResult; use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; @@ -57,10 +58,12 @@ public function __construct(EntityManagerInterface $entity_manager) { * available via the {node_type} parameter rather than a {bundle} * parameter. * - * @return string - * A \Drupal\Core\Access\AccessInterface constant value. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. */ public function access(Route $route, Request $request, AccountInterface $account, $form_mode_name = 'default', $bundle = NULL) { + $access = new AccessCheckResult(TRUE); + if ($entity_type_id = $route->getDefault('entity_type_id')) { if (!isset($bundle)) { $entity_type = $this->entityManager->getDefinition($entity_type_id); @@ -75,13 +78,22 @@ public function access(Route $route, Request $request, AccountInterface $account $visibility = $entity_display->status(); } + // Cacheable per role, until the form display is modified. + $access->cacheability->setContexts(array('cache_context.user.roles')); + if ($form_mode_name != 'default' && $entity_display) { + // Cacheable until the view display is modified. + $access->cacheability->setTags($entity_display->getCacheTag()); + } + if ($visibility) { $permission = $route->getRequirement('_field_ui_form_mode_access'); - return $account->hasPermission($permission) ? static::ALLOW : static::DENY; + $access->value = $account->hasPermission($permission) ? static::ALLOW : static::DENY; + return $access; } } - return static::DENY; + $access->value = static::DENY; + return $access; } } diff --git a/core/modules/field_ui/src/Access/ViewModeAccessCheck.php b/core/modules/field_ui/src/Access/ViewModeAccessCheck.php index 8e93040..986ad6b 100644 --- a/core/modules/field_ui/src/Access/ViewModeAccessCheck.php +++ b/core/modules/field_ui/src/Access/ViewModeAccessCheck.php @@ -7,6 +7,7 @@ namespace Drupal\field_ui\Access; +use Drupal\Core\Access\AccessCheckResult; use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; @@ -57,10 +58,12 @@ public function __construct(EntityManagerInterface $entity_manager) { * available via the {node_type} parameter rather than a {bundle} * parameter. * - * @return string - * A \Drupal\Core\Access\AccessInterface constant value. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. */ public function access(Route $route, Request $request, AccountInterface $account, $view_mode_name = 'default', $bundle = NULL) { + $access = new AccessCheckResult(TRUE); + if ($entity_type_id = $route->getDefault('entity_type_id')) { if (!isset($bundle)) { $entity_type = $this->entityManager->getDefinition($entity_type_id); @@ -75,13 +78,22 @@ public function access(Route $route, Request $request, AccountInterface $account $visibility = $entity_display->status(); } + // Cacheable per role. + $access->cacheability->setContexts(array('cache_context.user.roles')); + if ($view_mode_name != 'default' && $entity_display) { + // Cacheable until the view display is modified. + $access->cacheability->setTags($entity_display->getCacheTag()); + } + if ($visibility) { $permission = $route->getRequirement('_field_ui_view_mode_access'); - return $account->hasPermission($permission) ? static::ALLOW : static::DENY; + $access->value = $account->hasPermission($permission) ? static::ALLOW : static::DENY; + return $access; } } - return static::DENY; + $access->value = static::DENY; + return $access; } } diff --git a/core/modules/file/file.module b/core/modules/file/file.module index a48a7a7..26eb3f7 100644 --- a/core/modules/file/file.module +++ b/core/modules/file/file.module @@ -7,6 +7,7 @@ use Drupal\Component\Utility\SafeMarkup; use Drupal\Component\Utility\String; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Field\FieldDefinitionInterface; use Drupal\Core\Form\FormStateInterface; use Drupal\Core\Render\Element; @@ -636,7 +637,7 @@ function file_file_download($uri) { return; } - if (!$file->access('download')) { + if ($file->access('download')->value !== AccessInterface::ALLOW) { return -1; } diff --git a/core/modules/file/src/FileAccessController.php b/core/modules/file/src/FileAccessController.php index 08972ab..f52f57d 100644 --- a/core/modules/file/src/FileAccessController.php +++ b/core/modules/file/src/FileAccessController.php @@ -7,6 +7,8 @@ namespace Drupal\file; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Entity\EntityAccessController; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Entity\EntityStorageInterface; @@ -21,21 +23,26 @@ class FileAccessController extends EntityAccessController { * {@inheritdoc} */ protected function checkAccess(EntityInterface $entity, $operation, $langcode, AccountInterface $account) { + $no_access = new AccessCheckResult(TRUE); + $no_access->value = AccessInterface::DENY; if ($operation == 'download') { foreach ($this->getFileReferences($entity) as $field_name => $entity_map) { foreach ($entity_map as $referencing_entity_type => $referencing_entities) { /** @var \Drupal\Core\Entity\EntityInterface $referencing_entity */ foreach ($referencing_entities as $referencing_entity) { - if ($referencing_entity->access('view', $account) && $referencing_entity->$field_name->access('view', $account)) { - return TRUE; + $entity_access = $referencing_entity->access('view', $account); + $field_access = $referencing_entity->$field_name->access('view', $account); + $combined_access = AccessCheckResult::all(array($entity_access, $field_access)); + if ($combined_access->value === AccessInterface::ALLOW) { + return $combined_access; } } } } - - return FALSE; } + + return $no_access; } /** diff --git a/core/modules/filter/filter.module b/core/modules/filter/filter.module index 68b445e..d95ba29 100644 --- a/core/modules/filter/filter.module +++ b/core/modules/filter/filter.module @@ -10,6 +10,7 @@ use Drupal\Component\Utility\NestedArray; use Drupal\Component\Utility\String; use Drupal\Component\Utility\Xss; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Cache\Cache; use Drupal\Core\Render\Element; use Drupal\Core\Routing\RouteMatchInterface; @@ -290,7 +291,7 @@ function filter_formats(AccountInterface $account = NULL) { if (!isset($formats['user'][$account_id])) { $formats['user'][$account_id] = array(); foreach ($formats['all'] as $format) { - if ($format->access('use', $account)) { + if ($format->access('use', $account)->value === AccessInterface::ALLOW) { $formats['user'][$account_id][$format->format] = $format; } } diff --git a/core/modules/filter/src/FilterFormatAccess.php b/core/modules/filter/src/FilterFormatAccess.php index b6f6f58..a570bff 100644 --- a/core/modules/filter/src/FilterFormatAccess.php +++ b/core/modules/filter/src/FilterFormatAccess.php @@ -7,6 +7,8 @@ namespace Drupal\filter; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Entity\EntityAccessController; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Session\AccountInterface; @@ -22,25 +24,41 @@ class FilterFormatAccess extends EntityAccessController { protected function checkAccess(EntityInterface $filter_format, $operation, $langcode, AccountInterface $account) { /** @var \Drupal\filter\FilterFormatInterface $filter_format */ + $access = new AccessCheckResult(TRUE); + // All users are allowed to use the fallback filter. if ($operation == 'use') { - return $filter_format->isFallbackFormat() || $account->hasPermission($filter_format->getPermissionName()); + if ($filter_format->isFallbackFormat()) { + $access->value = AccessInterface::ALLOW; + return $access; + } + else if ($account->hasPermission($filter_format->getPermissionName())) { + // Cacheable per role. + $access->cacheability->setContexts(array('cache_context.user.roles')); + $access->value = AccessInterface::ALLOW; + return $access; + } } // The fallback format may not be disabled. if ($operation == 'disable' && $filter_format->isFallbackFormat()) { - return FALSE; + $access->value = AccessInterface::KILL; + return $access; } // We do not allow filter formats to be deleted through the UI, because that // would render any content that uses them unusable. if ($operation == 'delete') { - return FALSE; + $access->value = AccessInterface::KILL; + return $access; } if (in_array($operation, array('disable', 'update'))) { return parent::checkAccess($filter_format, $operation, $langcode, $account); } + + $access->value = AccessInterface::DENY; + return $access; } } diff --git a/core/modules/filter/src/Tests/FilterAdminTest.php b/core/modules/filter/src/Tests/FilterAdminTest.php index e438162..ea81e4f 100644 --- a/core/modules/filter/src/Tests/FilterAdminTest.php +++ b/core/modules/filter/src/Tests/FilterAdminTest.php @@ -8,6 +8,7 @@ namespace Drupal\filter\Tests; use Drupal\Component\Utility\String; +use Drupal\Core\Access\AccessInterface; use Drupal\simpletest\WebTestBase; /** @@ -187,8 +188,8 @@ function testFilterAdmin() { // Verify access permissions to Full HTML format. $full_format = entity_load('filter_format', $full); - $this->assertTrue($full_format->access('use', $this->admin_user), 'Admin user may use Full HTML.'); - $this->assertFalse($full_format->access('use', $this->web_user), 'Web user may not use Full HTML.'); + $this->assertEqual(AccessInterface::ALLOW, $full_format->access('use', $this->admin_user)->value, 'Admin user may use Full HTML.'); + $this->assertEqual(AccessInterface::DENY, $full_format->access('use', $this->web_user)->value, 'Web user may not use Full HTML.'); // Add an additional tag. $edit = array(); diff --git a/core/modules/filter/src/Tests/FilterFormatAccessTest.php b/core/modules/filter/src/Tests/FilterFormatAccessTest.php index 9f4234f..4ede627 100644 --- a/core/modules/filter/src/Tests/FilterFormatAccessTest.php +++ b/core/modules/filter/src/Tests/FilterFormatAccessTest.php @@ -7,11 +7,14 @@ namespace Drupal\filter\Tests; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\simpletest\WebTestBase; /** * Tests access to text formats. * + * @group Access * @group filter */ class FilterFormatAccessTest extends WebTestBase { @@ -120,9 +123,15 @@ function testFormatPermissions() { // Make sure that a regular user only has access to the text formats for // which they were granted access. $fallback_format = entity_load('filter_format', filter_fallback_format()); - $this->assertTrue($this->allowed_format->access('use', $this->web_user), 'A regular user has access to use a text format they were granted access to.'); - $this->assertFalse($this->disallowed_format->access('use', $this->web_user), 'A regular user does not have access to use a text format they were not granted access to.'); - $this->assertTrue($fallback_format->access('use', $this->web_user), 'A regular user has access to use the fallback format.'); + $expected = new AccessCheckResult(TRUE); + $expected->cacheability->setContexts(array('cache_context.user.roles')); + $expected->value = AccessInterface::ALLOW; + $this->assertEqual($expected, $this->allowed_format->access('use', $this->web_user), 'A regular user has access to use a text format they were granted access to.'); + $expected = new AccessCheckResult(TRUE); + $expected->value = AccessInterface::DENY; + $this->assertEqual($expected, $this->disallowed_format->access('use', $this->web_user), 'A regular user does not have access to use a text format they were not granted access to.'); + $expected->value = AccessInterface::ALLOW; + $this->assertEqual($expected, $fallback_format->access('use', $this->web_user), 'A regular user has access to use the fallback format.'); // Perform similar checks as above, but now against the entire list of // available formats for this user. diff --git a/core/modules/forum/forum.module b/core/modules/forum/forum.module index 425b597..bffbe2c 100644 --- a/core/modules/forum/forum.module +++ b/core/modules/forum/forum.module @@ -114,7 +114,7 @@ function forum_menu_local_tasks(&$data, $route_name) { // Loop through all bundles for forum taxonomy vocabulary field. $field_map = \Drupal::entityManager()->getFieldMap(); foreach ($field_map['node']['taxonomy_forums']['bundles'] as $type) { - if (\Drupal::entityManager()->getAccessController('node')->createAccess($type)) { + if (\Drupal::entityManager()->getAccessController('node')->createAccess($type)->value === \Drupal\Core\Access\AccessInterface::ALLOW) { $links[$type] = array( '#theme' => 'menu_local_action', '#link' => array( diff --git a/core/modules/language/src/LanguageAccessController.php b/core/modules/language/src/LanguageAccessController.php index 5ebded0..6423352 100644 --- a/core/modules/language/src/LanguageAccessController.php +++ b/core/modules/language/src/LanguageAccessController.php @@ -7,6 +7,8 @@ namespace Drupal\language; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Entity\EntityAccessController; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Session\AccountInterface; @@ -17,13 +19,23 @@ class LanguageAccessController extends EntityAccessController { * {@inheritdoc} */ public function checkAccess(EntityInterface $entity, $operation, $langcode, AccountInterface $account) { + $access = new AccessCheckResult(TRUE); + $access->value = AccessInterface::DENY; switch ($operation) { case 'update': case 'delete': - return !$entity->locked && parent::checkAccess($entity, $operation, $langcode, $account); + // Cacheable until entity is modified. + $access->cacheability->setTags($entity->getCacheTag()); + if ($entity->locked) { + return $access; + } + else { + $parent_access = parent::checkAccess($entity, $operation, $langcode, $account); + return AccessCheckResult::any(array($access, $parent_access)); + } break; } - return FALSE; + return $access; } } diff --git a/core/modules/menu_link_content/src/Form/MenuLinkContentForm.php b/core/modules/menu_link_content/src/Form/MenuLinkContentForm.php index 727d8b2..43ee180 100644 --- a/core/modules/menu_link_content/src/Form/MenuLinkContentForm.php +++ b/core/modules/menu_link_content/src/Form/MenuLinkContentForm.php @@ -8,6 +8,7 @@ namespace Drupal\menu_link_content\Form; use Drupal\Component\Utility\UrlHelper; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Access\AccessManagerInterface; use Drupal\Core\Entity\ContentEntityForm; use Drupal\Core\Entity\EntityManagerInterface; @@ -318,7 +319,7 @@ public function form(array $form, FormStateInterface $form_state) { protected function actions(array $form, FormStateInterface $form_state) { $element = parent::actions($form, $form_state); $element['submit']['#button_type'] = 'primary'; - $element['delete']['#access'] = $this->entity->access('delete'); + $element['delete']['#access'] = $this->entity->access('delete')->value === AccessInterface::ALLOW; return $element; } @@ -398,7 +399,7 @@ protected function doValidate(array $form, FormStateInterface $form_state) { } elseif ($extracted['route_name']) { // Users are not allowed to add a link to a page they cannot access. - $valid = $this->accessManager->checkNamedRoute($extracted['route_name'], $extracted['route_parameters'], $this->account); + $valid = $this->accessManager->checkNamedRoute($extracted['route_name'], $extracted['route_parameters'], $this->account)->value === AccessInterface::ALLOW; } if (!$valid) { $this->setFormError('url', $form_state, $this->t("The path '@link_path' is either invalid or you do not have access to it.", array('@link_path' => $form_state['values']['url']))); diff --git a/core/modules/menu_link_content/src/MenuLinkContentAccessController.php b/core/modules/menu_link_content/src/MenuLinkContentAccessController.php index ad36c19..ed890d9 100644 --- a/core/modules/menu_link_content/src/MenuLinkContentAccessController.php +++ b/core/modules/menu_link_content/src/MenuLinkContentAccessController.php @@ -6,6 +6,8 @@ namespace Drupal\menu_link_content; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Access\AccessManagerInterface; use Drupal\Core\Entity\EntityControllerInterface; use Drupal\Core\Entity\EntityInterface; @@ -51,17 +53,43 @@ public static function createInstance(ContainerInterface $container, EntityTypeI * {@inheritdoc} */ protected function checkAccess(EntityInterface $entity, $operation, $langcode, AccountInterface $account) { + $access = new AccessCheckResult(TRUE); switch ($operation) { case 'view': // There is no direct view. - return FALSE; + $access->value = AccessInterface::DENY; + return $access; case 'update': - // If there is a URL, this is an external link so always accessible. - return $account->hasPermission('administer menu') && ($entity->getUrl() || $this->accessManager->checkNamedRoute($entity->getRouteName(), $entity->getRouteParameters(), $account)); + // Cacheable per role + $access->cacheability->setContexts(array('cache_context.user.roles')); + // Cacheable until entity is modified. + $access->cacheability->setTags($entity->getCacheTag()); + if (!$account->hasPermission('administer menu')) { + $access->value = AccessInterface::KILL; + return $access; + } + else { + // If there is a URL, this is an external link so always accessible. + if ($entity->getUrl()) { + $access->value = AccessInterface::ALLOW; + return $access; + } + else { + // We allow access, but only if the link is accessible also. + $access->value = AccessInterface::ALLOW; + $link_access = $this->accessManager->checkNamedRoute($entity->getRouteName(), $entity->getRouteParameters(), $account); + return AccessCheckResult::all(array($access, $link_access)); + } + } case 'delete': - return !$entity->isNew() && $account->hasPermission('administer menu'); + // Cacheable per role + $access->cacheability->setContexts(array('cache_context.user.roles')); + // Cacheable until entity is modified. + $access->cacheability->setTags($entity->getCacheTag()); + $access->value = (!$entity->isNew() && $account->hasPermission('administer menu')) ? AccessInterface::ALLOW : AccessInterface::DENY; + return $access; } } diff --git a/core/modules/menu_ui/src/Form/MenuLinkResetForm.php b/core/modules/menu_ui/src/Form/MenuLinkResetForm.php index 09cfb87..9c849ae 100644 --- a/core/modules/menu_ui/src/Form/MenuLinkResetForm.php +++ b/core/modules/menu_ui/src/Form/MenuLinkResetForm.php @@ -7,6 +7,7 @@ namespace Drupal\menu_ui\Form; +use Drupal\Core\Access\AccessCheckResult; use Drupal\Core\Form\FormStateInterface; use Drupal\Core\Url; use Drupal\Core\Form\ConfirmFormBase; @@ -115,12 +116,13 @@ public function submitForm(array &$form, FormStateInterface $form_state) { * @param \Drupal\Core\Menu\MenuLinkInterface $menu_link_plugin * The menu link plugin being checked. * - * @return string - * AccessInterface::ALLOW when access was granted, otherwise - * AccessInterface::DENY. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. */ public function linkIsResettable(MenuLinkInterface $menu_link_plugin) { - return $menu_link_plugin->isResettable() ? AccessInterface::ALLOW : AccessInterface::DENY; + $access = new AccessCheckResult(TRUE); + $access->value = $menu_link_plugin->isResettable() ? AccessInterface::ALLOW : AccessInterface::DENY; + return $access; } } diff --git a/core/modules/node/node.api.php b/core/modules/node/node.api.php index 9ac1c40..770d192 100644 --- a/core/modules/node/node.api.php +++ b/core/modules/node/node.api.php @@ -3,6 +3,8 @@ use Drupal\node\NodeInterface; use Drupal\Component\Utility\String; use Drupal\Component\Utility\Xss; +use Drupal\Core\Access\AccessInterface; +use Drupal\Core\Access\AccessCheckResult; /** * @file @@ -294,9 +296,9 @@ function hook_node_grants_alter(&$grants, \Drupal\Core\Session\AccountInterface * interface. * * Note that not all modules will want to influence access on all node types. If - * your module does not want to actively grant or block access, return - * NODE_ACCESS_IGNORE or simply return nothing. Blindly returning FALSE will - * break other node access modules. + * your module does not want to actively grant or block access, return an access + * check result with AccessInterface::DENY. Blindly returning + * AccessInterface::KILL will break other node access modules. * * Also note that this function isn't called for node listings (e.g., RSS feeds, * the default home page at path 'node', a recent content block, etc.) See @@ -316,37 +318,55 @@ function hook_node_grants_alter(&$grants, \Drupal\Core\Session\AccountInterface * @param object $langcode * The language code to perform the access check operation on. * - * @return string - * - NODE_ACCESS_ALLOW: if the operation is to be allowed. - * - NODE_ACCESS_DENY: if the operation is to be denied. - * - NODE_ACCESS_IGNORE: to not affect this operation at all. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. Its value can be set + * to: + * - AccessInterface::ALLOW: if the operation is to be allowed + * - AccessInterface::KILL: if the operation is to be forbidden + * - AccessInterface::DENY: to not affect this operation at all. + * The cacheability metadata should also be set, to reflect whether this + * access check result applies for example to all users of this role, or only + * to the current user, or… * * @ingroup node_access */ function hook_node_access(\Drupal\node\NodeInterface $node, $op, \Drupal\Core\Session\AccountInterface $account, $langcode) { - $type = is_string($node) ? $node : $node->getType(); + $type = $node->bundle(); + + $access = new AccessCheckResult(TRUE); + // Cacheable until the node is modified. + $access->cacheability->setTags($node->getCacheTag()); $configured_types = node_permissions_get_configured_types(); if (isset($configured_types[$type])) { - if ($op == 'create' && $account->hasPermission('create ' . $type . ' content')) { - return NODE_ACCESS_ALLOW; + if ($op == 'create' && $account->hasPermission('create ' . $type . ' content', $account)) { + // Cacheable per role. + $access->cacheability->addContexts(array('cache_context.user.roles')); + $access->value = AccessInterface::ALLOW; + return $access; } if ($op == 'update') { - if ($account->hasPermission('edit any ' . $type . ' content', $account) || ($account->hasPermission('edit own ' . $type . ' content') && ($account->id() == $node->getOwnerId()))) { - return NODE_ACCESS_ALLOW; + // Cacheable per user. + $access->cacheability->addContexts(array('cache_context.user')); + if ($account->hasPermission('edit any ' . $type . ' content', $account) || ($account->hasPermission('edit own ' . $type . ' content', $account) && ($account->id() == $node->getOwnerId()))) { + $access->value = AccessInterface::ALLOW; + return $access; } } if ($op == 'delete') { - if ($account->hasPermission('delete any ' . $type . ' content', $account) || ($account->hasPermission('delete own ' . $type . ' content') && ($account->id() == $node->getOwnerId()))) { - return NODE_ACCESS_ALLOW; + // Cacheable per user. + $access->cacheability->addContexts(array('cache_context.user')); + if ($account->hasPermission('delete any ' . $type . ' content', $account) || ($account->hasPermission('delete own ' . $type . ' content', $account) && ($account->id() == $node->getOwnerId()))) { + $access->value = AccessInterface::ALLOW; + return $access; } } } - // Returning nothing from this function would have the same effect. - return NODE_ACCESS_IGNORE; + $access->value = AccessInterface::DENY; + return $access; } /** diff --git a/core/modules/node/node.module b/core/modules/node/node.module index fdec8ba..7107b43 100644 --- a/core/modules/node/node.module +++ b/core/modules/node/node.module @@ -9,6 +9,8 @@ */ use Drupal\Component\Utility\Xss; +use Drupal\Core\Access\AccessInterface; +use Drupal\Core\Access\AccessCheckResult; use Drupal\Core\Form\FormStateInterface; use Drupal\Core\Language\LanguageInterface; use Drupal\Core\Render\Element; @@ -63,30 +65,6 @@ const NODE_STICKY = 1; /** - * Denotes that access is allowed for a node. - * - * Modules should return this value from hook_node_access() to allow access to a - * node. - */ -const NODE_ACCESS_ALLOW = TRUE; - -/** - * Denotes that access is denied for a node. - * - * Modules should return this value from hook_node_access() to deny access to a - * node. - */ -const NODE_ACCESS_DENY = FALSE; - -/** - * Denotes that access is unaffected for a node. - * - * Modules should return this value from hook_node_access() to indicate no - * effect on node access. - */ -const NODE_ACCESS_IGNORE = NULL; - -/** * Implements hook_help(). */ function node_help($route_name, RouteMatchInterface $route_match) { @@ -1117,8 +1095,8 @@ function node_form_system_themes_admin_form_submit($form, FormStateInterface $fo * check. * * Next, all implementations of hook_node_access() will be called. Each - * implementation may explicitly allow, explicitly deny, or ignore the access - * request. If at least one module says to deny the request, it will be + * implementation may explicitly allow, explicitly forbid, or ignore the access + * request. If at least one module says to forbid the request, it will be * rejected. If no modules deny the request and at least one says to allow it, * the request will be permitted. * @@ -1142,11 +1120,12 @@ function node_form_system_themes_admin_form_submit($form, FormStateInterface $fo * @link entity_api Entity API topic @endlink for more information on entity * queries. * - * Note: Even a single module returning NODE_ACCESS_DENY from hook_node_access() - * will block access to the node. Therefore, implementers should take care to - * not deny access unless they really intend to. Unless a module wishes to - * actively deny access it should return NODE_ACCESS_IGNORE (or simply return - * nothing) to allow other modules or the node_access table to control access. + * Note: Even a single module returning AccessInterface::KILL from + * hook_node_access() will block access to the node. Therefore, implementers + * should take care to not deny access unless they really intend to. Unless a + * module wishes to actively forbid access it should return + * AccessInterface::DENY to allow other modules or the node_access table to + * control access. * * To see how to write a node access module of your own, see * node_access_example.module. @@ -1158,26 +1137,41 @@ function node_form_system_themes_admin_form_submit($form, FormStateInterface $fo function node_node_access(NodeInterface $node, $op, $account) { $type = $node->bundle(); + $access = new AccessCheckResult(TRUE); + // Cacheable until the node is modified. + $access->cacheability->setTags($node->getCacheTag()); + $configured_types = node_permissions_get_configured_types(); if (isset($configured_types[$type])) { + // Cacheable per role. + $access->cacheability->addContexts(array('cache_context.user.roles')); + if ($op == 'create' && $account->hasPermission('create ' . $type . ' content', $account)) { - return NODE_ACCESS_ALLOW; + $access->value = AccessInterface::ALLOW; + return $access; } if ($op == 'update') { + // Cacheable per user. + $access->cacheability->addContexts(array('cache_context.user')); if ($account->hasPermission('edit any ' . $type . ' content', $account) || ($account->hasPermission('edit own ' . $type . ' content', $account) && ($account->id() == $node->getOwnerId()))) { - return NODE_ACCESS_ALLOW; + $access->value = AccessInterface::ALLOW; + return $access; } } if ($op == 'delete') { + // Cacheable per user. + $access->cacheability->addContexts(array('cache_context.user')); if ($account->hasPermission('delete any ' . $type . ' content', $account) || ($account->hasPermission('delete own ' . $type . ' content', $account) && ($account->id() == $node->getOwnerId()))) { - return NODE_ACCESS_ALLOW; + $access->value = AccessInterface::ALLOW; + return $access; } } } - return NODE_ACCESS_IGNORE; + $access->value = AccessInterface::DENY; + return $access; } /** diff --git a/core/modules/node/node.pages.inc b/core/modules/node/node.pages.inc index e848f3e..8dbd290 100644 --- a/core/modules/node/node.pages.inc +++ b/core/modules/node/node.pages.inc @@ -10,6 +10,7 @@ */ use Drupal\Component\Utility\Xss; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Form\FormStateInterface; use Symfony\Component\HttpFoundation\RedirectResponse; use Drupal\node\NodeInterface; @@ -50,7 +51,7 @@ function template_preprocess_node_add_list(&$variables) { * @see node_form_build_preview() */ function node_preview(NodeInterface $node, FormStateInterface $form_state) { - if ($node->access('create') || $node->access('update')) { + if ($node->access('create')->value === AccessInterface::ALLOW || $node->access('update')->value === AccessInterface::ALLOW) { $node->changed = REQUEST_TIME; diff --git a/core/modules/node/src/Access/NodeAddAccessCheck.php b/core/modules/node/src/Access/NodeAddAccessCheck.php index c7bb0e7..99ff699 100644 --- a/core/modules/node/src/Access/NodeAddAccessCheck.php +++ b/core/modules/node/src/Access/NodeAddAccessCheck.php @@ -7,6 +7,7 @@ namespace Drupal\node\Access; +use Drupal\Core\Access\AccessCheckResult; use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; @@ -50,15 +51,18 @@ public function access(AccountInterface $account, NodeTypeInterface $node_type = $access_controller = $this->entityManager->getAccessController('node'); // If checking whether a node of a particular type may be created. if ($node_type) { - return $access_controller->createAccess($node_type->id(), $account) ? static::ALLOW : static::DENY; + return $access_controller->createAccess($node_type->id(), $account); } // If checking whether a node of any type may be created. foreach (node_permissions_get_configured_types() as $node_type) { - if ($access_controller->createAccess($node_type->id(), $account)) { - return static::ALLOW; + if (($access = $access_controller->createAccess($node_type->id(), $account)) && $access->value === static::ALLOW) { + return $access; } } - return static::DENY; + + $access = new AccessCheckResult(TRUE); + $access->value = static::DENY; + return $access; } } diff --git a/core/modules/node/src/Access/NodeRevisionAccessCheck.php b/core/modules/node/src/Access/NodeRevisionAccessCheck.php index 66dc7e2..a774401 100644 --- a/core/modules/node/src/Access/NodeRevisionAccessCheck.php +++ b/core/modules/node/src/Access/NodeRevisionAccessCheck.php @@ -7,6 +7,7 @@ namespace Drupal\node\Access; +use Drupal\Core\Access\AccessCheckResult; use Drupal\Core\Database\Connection; use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\Routing\Access\AccessInterface; @@ -77,15 +78,19 @@ public function __construct(EntityManagerInterface $entity_manager, Connection $ * is specified. If neither $node_revision nor $node are specified, then * access is denied. * - * @return string - * A \Drupal\Core\Access\AccessInterface constant value. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. */ public function access(Route $route, AccountInterface $account, $node_revision = NULL, NodeInterface $node = NULL) { + $access = new AccessCheckResult(TRUE); if ($node_revision) { $node = $this->nodeStorage->loadRevision($node_revision); } $operation = $route->getRequirement('_access_node_revision'); - return ($node && $this->checkAccess($node, $account, $operation)) ? static::ALLOW : static::DENY; + $access->value = ($node && $this->checkAccess($node, $account, $operation)) ? static::ALLOW : static::DENY; + // Cacheable per role. + $access->cacheability->setContexts(array('cache_context.user.roles')); + return $access; } /** diff --git a/core/modules/node/src/Controller/NodeController.php b/core/modules/node/src/Controller/NodeController.php index b09a3a7..3888917 100644 --- a/core/modules/node/src/Controller/NodeController.php +++ b/core/modules/node/src/Controller/NodeController.php @@ -9,6 +9,7 @@ use Drupal\Component\Utility\String; use Drupal\Component\Utility\Xss; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Controller\ControllerBase; use Drupal\Core\Datetime\Date as DateFormatter; use Drupal\Core\DependencyInjection\ContainerInjectionInterface; @@ -64,7 +65,7 @@ public function addPage() { // Only use node types the user has access to. foreach ($this->entityManager()->getStorage('node_type')->loadMultiple() as $type) { - if ($this->entityManager()->getAccessController('node')->createAccess($type->type)) { + if ($this->entityManager()->getAccessController('node')->createAccess($type->type)->value === AccessInterface::ALLOW) { $content[$type->type] = $type; } } @@ -155,8 +156,8 @@ public function revisionOverview(NodeInterface $node) { $build['#title'] = $this->t('Revisions for %title', array('%title' => $node->label())); $header = array($this->t('Revision'), $this->t('Operations')); - $revert_permission = (($account->hasPermission("revert $type revisions") || $account->hasPermission('revert all revisions') || $account->hasPermission('administer nodes')) && $node->access('update')); - $delete_permission = (($account->hasPermission("delete $type revisions") || $account->hasPermission('delete all revisions') || $account->hasPermission('administer nodes')) && $node->access('delete')); + $revert_permission = (($account->hasPermission("revert $type revisions") || $account->hasPermission('revert all revisions') || $account->hasPermission('administer nodes')) && $node->access('update')->value === AccessInterface::ALLOW); + $delete_permission = (($account->hasPermission("delete $type revisions") || $account->hasPermission('delete all revisions') || $account->hasPermission('administer nodes')) && $node->access('delete')->value === AccessInterface::ALLOW); $rows = array(); diff --git a/core/modules/node/src/NodeAccessController.php b/core/modules/node/src/NodeAccessController.php index ab38a26..9756ff5 100644 --- a/core/modules/node/src/NodeAccessController.php +++ b/core/modules/node/src/NodeAccessController.php @@ -7,6 +7,8 @@ namespace Drupal\node; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Database\Connection; use Drupal\Core\Database\Query\SelectInterface; use Drupal\Core\Entity\EntityControllerInterface; @@ -27,7 +29,7 @@ class NodeAccessController extends EntityAccessController implements NodeAccessC /** * The node grant storage. * - * @var \Drupal\node\NodeGrantStorageInterface + * @var \Drupal\node\NodeGrantDatabaseStorageInterface */ protected $grantStorage; @@ -59,15 +61,24 @@ public static function createInstance(ContainerInterface $container, EntityTypeI * {@inheritdoc} */ public function access(EntityInterface $entity, $operation, $langcode = LanguageInterface::LANGCODE_DEFAULT, AccountInterface $account = NULL) { + // The two high-level access pre-requisites are cacheable per role. + $access = new AccessCheckResult(TRUE); + $access->cacheability->setContexts(array('cache_context.user.roles')); + $account = $this->prepareUser($account); if ($account->hasPermission('bypass node access')) { - return TRUE; + $access->value = AccessInterface::ALLOW; + return $access; } if (!$account->hasPermission('access content')) { - return FALSE; + $access->value = AccessInterface::DENY; + return $access; } - return parent::access($entity, $operation, $langcode, $account); + + $access->value = AccessInterface::DENY; + $parent_access = parent::access($entity, $operation, $langcode, $account); + return AccessCheckResult::any(array($access, $parent_access)); } /** @@ -76,11 +87,18 @@ public function access(EntityInterface $entity, $operation, $langcode = Language public function createAccess($entity_bundle = NULL, AccountInterface $account = NULL, array $context = array()) { $account = $this->prepareUser($account); + // The two high-level access pre-requisites are cacheable per role. + $access = new AccessCheckResult(TRUE); + $access->cacheability->setContexts(array('cache_context.user.roles')); + + if ($account->hasPermission('bypass node access')) { - return TRUE; + $access->value = AccessInterface::ALLOW; + return $access; } if (!$account->hasPermission('access content')) { - return FALSE; + $access->value = AccessInterface::KILL; + return $access; } return parent::createAccess($entity_bundle, $account, $context); @@ -97,35 +115,56 @@ protected function checkAccess(EntityInterface $node, $operation, $langcode, Acc $status = $translation->isPublished(); $uid = $translation->getOwnerId(); + $access = new AccessCheckResult(TRUE); + // Check if authors can view their own unpublished nodes. if ($operation === 'view' && !$status && $account->hasPermission('view own unpublished content')) { - + // Cacheable per user until the node is modified. + $access->cacheability + ->addContexts(array('cache_context.user')) + ->addTags($node->getCacheTag()); if ($account->id() != 0 && $account->id() == $uid) { - return TRUE; + $access->value = AccessInterface::ALLOW; + return $access; } } - // If no module specified either allow or deny, we fall back to the + // If no module specified either ALLOW or KILL, we fall back to the // node_access table. - if (($grants = $this->grantStorage->access($node, $operation, $langcode, $account)) !== NULL) { + $grants = $this->grantStorage->access($node, $operation, $langcode, $account); + if ($grants->value !== AccessInterface::DENY) { return $grants; } // If no modules implement hook_node_grants(), the default behavior is to // allow all users to view published nodes, so reflect that here. if ($operation === 'view') { - return $status; + $access->value = $status ? AccessInterface::ALLOW : AccessInterface::DENY; + // Cacheable until the node is modified. + $access->cacheability->addTags($node->getCacheTag()); + return $access; } + + $access->value = AccessInterface::DENY; + return $access; } /** * {@inheritdoc} */ protected function checkCreateAccess(AccountInterface $account, array $context, $entity_bundle = NULL) { + $access = new AccessCheckResult(TRUE); $configured_types = node_permissions_get_configured_types(); if (isset($configured_types[$entity_bundle])) { - return $account->hasPermission('create ' . $entity_bundle . ' content'); + $access->value = $account->hasPermission('create ' . $entity_bundle . ' content') ? AccessInterface::ALLOW : AccessInterface::DENY; + // Cacheable per role. + $access->cacheability->addContexts(array('cache_context.user.roles')); + return $access; + } + else { + $access->value = AccessInterface::DENY; } + return $access; } /** diff --git a/core/modules/node/src/NodeForm.php b/core/modules/node/src/NodeForm.php index 085cf2c..6982892 100644 --- a/core/modules/node/src/NodeForm.php +++ b/core/modules/node/src/NodeForm.php @@ -8,6 +8,7 @@ namespace Drupal\node; use Drupal\Component\Utility\NestedArray; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Datetime\DrupalDateTime; use Drupal\Core\Entity\ContentEntityForm; use Drupal\Core\Form\FormStateInterface; @@ -272,7 +273,7 @@ protected function actions(array $form, FormStateInterface $form_state) { $element['preview'] = array( '#type' => 'submit', - '#access' => $preview_mode != DRUPAL_DISABLED && ($node->access('create') || $node->access('update')), + '#access' => $preview_mode != DRUPAL_DISABLED && ($node->access('create')->value === AccessInterface::ALLOW || $node->access('update')->value === AccessInterface::ALLOW), '#value' => t('Preview'), '#weight' => 20, '#validate' => array( @@ -284,7 +285,7 @@ protected function actions(array $form, FormStateInterface $form_state) { ), ); - $element['delete']['#access'] = $node->access('delete'); + $element['delete']['#access'] = $node->access('delete')->value === AccessInterface::ALLOW; $element['delete']['#weight'] = 100; return $element; @@ -448,7 +449,7 @@ public function save(array $form, FormStateInterface $form_state) { if ($node->id()) { $form_state['values']['nid'] = $node->id(); $form_state['nid'] = $node->id(); - if ($node->access('view')) { + if ($node->access('view')->value === AccessInterface::ALLOW) { $form_state['redirect_route'] = array( 'route_name' => 'node.view', 'route_parameters' => array( diff --git a/core/modules/node/src/NodeGrantDatabaseStorage.php b/core/modules/node/src/NodeGrantDatabaseStorage.php index 9503713..61d5bb9 100644 --- a/core/modules/node/src/NodeGrantDatabaseStorage.php +++ b/core/modules/node/src/NodeGrantDatabaseStorage.php @@ -7,6 +7,8 @@ namespace Drupal\node; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Database\Connection; use Drupal\Core\Database\Query\SelectInterface; use Drupal\Core\Database\Query\Condition; @@ -54,10 +56,12 @@ public function __construct(Connection $database, ModuleHandlerInterface $module * {@inheritdoc} */ public function access(NodeInterface $node, $operation, $langcode, AccountInterface $account) { + $access = new AccessCheckResult(TRUE); // If no module implements the hook or the node does not have an id there is // no point in querying the database for access grants. if (!$this->moduleHandler->getImplementations('node_grants') || !$node->id()) { - return; + $access->value = AccessInterface::DENY; + return $access; } // Check the database for potential access grants. @@ -86,7 +90,14 @@ public function access(NodeInterface $node, $operation, $langcode, AccountInterf $query->condition($grants); } - return $query->execute()->fetchField(); + // Node grants currently don't have any cacheability metadata. Hopefully, we + // can add that in the future, which would allow this access check result to + // be cacheable. For now, this must remain marked as uncacheable, even when + // it is theoretically cacheable, because we don't have the necessary meta- + // data to know it for a fact. + $access = new AccessCheckResult(FALSE); + $access->value = ((bool) $query->execute()->fetchField()) ? AccessInterface::ALLOW : AccessInterface::KILL; + return $access; } /** diff --git a/core/modules/node/src/NodeGrantDatabaseStorageInterface.php b/core/modules/node/src/NodeGrantDatabaseStorageInterface.php index c67025b..8b046f1 100644 --- a/core/modules/node/src/NodeGrantDatabaseStorageInterface.php +++ b/core/modules/node/src/NodeGrantDatabaseStorageInterface.php @@ -105,8 +105,10 @@ public function writeDefault(); * @param \Drupal\Core\Session\AccountInterface $account * The user for which to check access. * - * @return bool|null - * TRUE if access was granted, FALSE if access was denied or NULL if no + * @return \Drupal\Core\Access\AccessCheckResult|null + * The access check result, with cacheability metadata. The access check + * result's value is AccessInterface::ALLOW if access was granted, + * AccessInterface::KILL if access was denied. NULL is returned when no * module implements hook_node_grants(), the node does not (yet) have an id * or none of the implementing modules explicitly granted or denied access. */ diff --git a/core/modules/node/src/NodeTypeAccessController.php b/core/modules/node/src/NodeTypeAccessController.php index 23ec80f..d359b09 100644 --- a/core/modules/node/src/NodeTypeAccessController.php +++ b/core/modules/node/src/NodeTypeAccessController.php @@ -7,6 +7,8 @@ namespace Drupal\node; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Entity\EntityAccessController; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Session\AccountInterface; @@ -23,7 +25,11 @@ class NodeTypeAccessController extends EntityAccessController { */ protected function checkAccess(EntityInterface $entity, $operation, $langcode, AccountInterface $account) { if ($operation == 'delete' && $entity->isLocked()) { - return FALSE; + $access = new AccessCheckResult(TRUE); + // Cacheable until entity is modified. + $access->cacheability->setTags($entity->getCacheTag()); + $access->value = AccessInterface::KILL; + return $access; } return parent::checkAccess($entity, $operation, $langcode, $account); } diff --git a/core/modules/node/src/Plugin/Search/NodeSearch.php b/core/modules/node/src/Plugin/Search/NodeSearch.php index 45e34b8..c68559a 100644 --- a/core/modules/node/src/Plugin/Search/NodeSearch.php +++ b/core/modules/node/src/Plugin/Search/NodeSearch.php @@ -9,6 +9,8 @@ use Drupal\Component\Utility\SafeMarkup; use Drupal\Component\Utility\String; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Config\Config; use Drupal\Core\Database\Connection; use Drupal\Core\Database\Query\SelectExtender; @@ -158,7 +160,11 @@ public function __construct(array $configuration, $plugin_id, $plugin_definition * {@inheritdoc} */ public function access($operation = 'view', AccountInterface $account = NULL) { - return !empty($account) && $account->hasPermission('access content'); + $access = new AccessCheckResult(TRUE); + $access->value = (!empty($account) && $account->hasPermission('access content')) ? AccessInterface::ALLOW : AccessInterface::DENY; + // Cacheable per role. + $access->cacheability->setContexts(array('cache_context.user.roles')); + return $access; } /** diff --git a/core/modules/node/src/Plugin/views/area/ListingEmpty.php b/core/modules/node/src/Plugin/views/area/ListingEmpty.php index cba41c2..3cb0e71 100644 --- a/core/modules/node/src/Plugin/views/area/ListingEmpty.php +++ b/core/modules/node/src/Plugin/views/area/ListingEmpty.php @@ -7,6 +7,7 @@ namespace Drupal\node\Plugin\views\area; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Access\AccessManagerInterface; use Drupal\views\Plugin\views\area\AreaPluginBase; use Symfony\Component\DependencyInjection\ContainerInterface; @@ -71,7 +72,7 @@ public function render($empty = FALSE) { 'title' => $this->t('Add content'), ), ), - '#access' => $this->accessManager->checkNamedRoute('node.add_page', array(), $account), + '#access' => $this->accessManager->checkNamedRoute('node.add_page', array(), $account)->value === AccessInterface::ALLOW, ); return $element; } diff --git a/core/modules/node/src/Plugin/views/field/Link.php b/core/modules/node/src/Plugin/views/field/Link.php index b276747..45e9904 100644 --- a/core/modules/node/src/Plugin/views/field/Link.php +++ b/core/modules/node/src/Plugin/views/field/Link.php @@ -7,6 +7,7 @@ namespace Drupal\node\Plugin\views\field; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Form\FormStateInterface; use Drupal\views\Plugin\views\field\FieldPluginBase; use Drupal\views\ResultRow; @@ -74,7 +75,7 @@ public function render(ResultRow $values) { * Returns a string for the link text. */ protected function renderLink($node, ResultRow $values) { - if ($node->access('view')) { + if ($node->access('view')->value === AccessInterface::ALLOW) { $this->options['alter']['make_link'] = TRUE; $this->options['alter']['path'] = 'node/' . $node->id(); $text = !empty($this->options['text']) ? $this->options['text'] : t('View'); diff --git a/core/modules/node/src/Plugin/views/field/LinkDelete.php b/core/modules/node/src/Plugin/views/field/LinkDelete.php index 78bb9dc..bbb53ea 100644 --- a/core/modules/node/src/Plugin/views/field/LinkDelete.php +++ b/core/modules/node/src/Plugin/views/field/LinkDelete.php @@ -7,6 +7,7 @@ namespace Drupal\node\Plugin\views\field; +use Drupal\Core\Access\AccessInterface; use Drupal\node\Plugin\views\field\Link; use Drupal\views\ResultRow; @@ -32,7 +33,7 @@ class LinkDelete extends Link { */ protected function renderLink($node, ResultRow $values) { // Ensure user has access to delete this node. - if (!$node->access('delete')) { + if ($node->access('delete')->value !== AccessInterface::ALLOW) { return; } diff --git a/core/modules/node/src/Plugin/views/field/LinkEdit.php b/core/modules/node/src/Plugin/views/field/LinkEdit.php index cf8a057..2f35f6e 100644 --- a/core/modules/node/src/Plugin/views/field/LinkEdit.php +++ b/core/modules/node/src/Plugin/views/field/LinkEdit.php @@ -7,6 +7,7 @@ namespace Drupal\node\Plugin\views\field; +use Drupal\Core\Access\AccessInterface; use Drupal\node\Plugin\views\field\Link; use Drupal\views\ResultRow; @@ -32,7 +33,7 @@ class LinkEdit extends Link { */ protected function renderLink($node, ResultRow $values) { // Ensure user has access to edit this node. - if (!$node->access('update')) { + if ($node->access('update')->value !== AccessInterface::ALLOW) { return; } diff --git a/core/modules/node/src/Plugin/views/field/RevisionLink.php b/core/modules/node/src/Plugin/views/field/RevisionLink.php index 5ea0835..219d562 100644 --- a/core/modules/node/src/Plugin/views/field/RevisionLink.php +++ b/core/modules/node/src/Plugin/views/field/RevisionLink.php @@ -7,6 +7,7 @@ namespace Drupal\node\Plugin\views\field; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; use Drupal\node\Plugin\views\field\Link; use Drupal\views\Plugin\views\display\DisplayPluginBase; @@ -86,7 +87,7 @@ function get_revision_entity($values, $op) { // Unpublished nodes ignore access control. $node->setPublished(TRUE); // Ensure user has access to perform the operation on this node. - if (!$node->access($op)) { + if ($node->access($op)->value !== AccessInterface::ALLOW) { return array($node, NULL); } return array($node, $vid); diff --git a/core/modules/node/src/Tests/NodeTestBase.php b/core/modules/node/src/Tests/NodeTestBase.php index dcb0518..142079d 100644 --- a/core/modules/node/src/Tests/NodeTestBase.php +++ b/core/modules/node/src/Tests/NodeTestBase.php @@ -7,6 +7,7 @@ namespace Drupal\node\Tests; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; use Drupal\simpletest\WebTestBase; @@ -67,7 +68,7 @@ function assertNodeAccess(array $ops, $node, AccountInterface $account, $langcod if (empty($langcode)) { $langcode = $node->prepareLangcode(); } - $this->assertEqual($result, $this->accessController->access($node, $op, $langcode, $account), $this->nodeAccessAssertMessage($op, $result, $langcode)); + $this->assertEqual($result, $this->accessController->access($node, $op, $langcode, $account)->value === AccessInterface::ALLOW, $this->nodeAccessAssertMessage($op, $result, $langcode)); } } @@ -87,7 +88,7 @@ function assertNodeAccess(array $ops, $node, AccountInterface $account, $langcod function assertNodeCreateAccess($bundle, $result, AccountInterface $account, $langcode = NULL) { $this->assertEqual($result, $this->accessController->createAccess($bundle, $account, array( 'langcode' => $langcode, - )), $this->nodeAccessAssertMessage('create', $result, $langcode)); + ))->value === AccessInterface::ALLOW, $this->nodeAccessAssertMessage('create', $result, $langcode)); } /** diff --git a/core/modules/node/tests/modules/node_access_test/node_access_test.module b/core/modules/node/tests/modules/node_access_test/node_access_test.module index 797cf25..ba51ebc 100644 --- a/core/modules/node/tests/modules/node_access_test/node_access_test.module +++ b/core/modules/node/tests/modules/node_access_test/node_access_test.module @@ -19,6 +19,8 @@ * @see \Drupal\node\Tests\NodeAccessBaseTableTest */ +use Drupal\Core\Access\AccessInterface; +use Drupal\Core\Access\AccessCheckResult; use Drupal\field\Entity\FieldStorageConfig; use Drupal\field\Entity\FieldInstanceConfig; use Drupal\node\NodeTypeInterface; @@ -159,11 +161,16 @@ function node_access_test_add_field(NodeTypeInterface $type) { /** * Implements hook_node_access(). */ -function node_access_test_node_access($node, $op, $account, $langcode) { +function node_access_test_node_access(\Drupal\node\NodeInterface $node, $op, \Drupal\Core\Session\AccountInterface $account, $langcode) { + // Uncacheable because the result depends on a State key-value pair, which may + // change at any time. + $access = new AccessCheckResult(FALSE); + $access->value = AccessInterface::DENY; + $secret_catalan = \Drupal::state()->get('node_access_test_secret_catalan') ?: 0; if ($secret_catalan && $langcode == 'ca') { // Make all Catalan content secret. - return NODE_ACCESS_DENY; + $access->value = AccessInterface::KILL; } - return NODE_ACCESS_IGNORE; + return $access; } diff --git a/core/modules/path/src/Plugin/Field/FieldType/PathFieldItemList.php b/core/modules/path/src/Plugin/Field/FieldType/PathFieldItemList.php index cf7ec2b..5b063ed 100644 --- a/core/modules/path/src/Plugin/Field/FieldType/PathFieldItemList.php +++ b/core/modules/path/src/Plugin/Field/FieldType/PathFieldItemList.php @@ -7,6 +7,8 @@ namespace Drupal\path\Plugin\Field\FieldType; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Field\FieldItemList; use Drupal\Core\Session\AccountInterface; @@ -19,10 +21,17 @@ class PathFieldItemList extends FieldItemList { * {@inheritdoc} */ public function defaultAccess($operation = 'view', AccountInterface $account = NULL) { + $access = new AccessCheckResult(TRUE); if ($operation == 'view') { - return TRUE; + $access->value = AccessInterface::ALLOW; + return $access; } - return $account->hasPermission('create url aliases') || $account->hasPermission('administer url aliases'); + + // The above access check result depends solely on the operation; from here + // on, it also depends on the user role. Hence vary per role. + $access->cacheability->addContexts(array('cache_context.user.roles')); + $access->value = ($account->hasPermission('create url aliases') || $account->hasPermission('administer url aliases')) ? AccessInterface::ALLOW : AccessInterface::DENY; + return $access; } } diff --git a/core/modules/quickedit/src/Access/EditEntityAccessCheck.php b/core/modules/quickedit/src/Access/EditEntityAccessCheck.php index a80aa2e..b5438db 100644 --- a/core/modules/quickedit/src/Access/EditEntityAccessCheck.php +++ b/core/modules/quickedit/src/Access/EditEntityAccessCheck.php @@ -7,6 +7,7 @@ namespace Drupal\quickedit\Access; +use Drupal\Core\Access\AccessCheckResult; use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; @@ -43,18 +44,22 @@ public function __construct(EntityManagerInterface $entity_manager) { * @param \Drupal\Core\Session\AccountInterface $account * The currently logged in account. * - * @return string - * A \Drupal\Core\Access\AccessInterface constant value. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. * * @todo Replace $request parameter with $entity once * https://drupal.org/node/1837388 is fixed. */ public function access(Request $request, AccountInterface $account) { if (!$this->validateAndUpcastRequestAttributes($request)) { - return static::KILL; + // Uncacheable because this access check result depends solely on request + // attributes, which then of course can only apply to the current request. + $access = new AccessCheckResult(FALSE); + $access->value = static::KILL; + return $access; } - return $this->accessEditEntity($request->attributes->get('entity'), $account) ? static::ALLOW : static::DENY; + return $this->accessEditEntity($request->attributes->get('entity'), $account); } /** @@ -87,4 +92,21 @@ protected function validateAndUpcastRequestAttributes(Request $request) { return TRUE; } + /** + * {@inheritdoc} + * + * @todo This would be cacheable per user if we could associate the cache tag + * of the entity being edited. + */ + public function isCacheable() { + return FALSE; + } + + /** + * {@inheritdoc} + */ + public function getCacheContexts() { + return array(); + } + } diff --git a/core/modules/quickedit/src/Access/EditEntityFieldAccessCheck.php b/core/modules/quickedit/src/Access/EditEntityFieldAccessCheck.php index 6eaba8a..8684096 100644 --- a/core/modules/quickedit/src/Access/EditEntityFieldAccessCheck.php +++ b/core/modules/quickedit/src/Access/EditEntityFieldAccessCheck.php @@ -7,6 +7,7 @@ namespace Drupal\quickedit\Access; +use Drupal\Core\Access\AccessCheckResult; use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; @@ -40,13 +41,13 @@ public function __construct(EntityManagerInterface $entity_manager) { * * @param \Symfony\Component\HttpFoundation\Request $request * The request object. - * @param string $field_name. + * @param string $field_name * The field name. * @param \Drupal\Core\Session\AccountInterface $account * The currently logged in account. * - * @return string - * A \Drupal\Core\Access\AccessInterface constant value. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. * * @todo Replace $request parameter with $entity once * https://drupal.org/node/1837388 is fixed. @@ -55,17 +56,23 @@ public function __construct(EntityManagerInterface $entity_manager) { */ public function access(Request $request, $field_name, AccountInterface $account) { if (!$this->validateAndUpcastRequestAttributes($request)) { - return static::KILL; + // Uncacheable because this access check result depends solely on request + // attributes, which then of course can only apply to the current request. + $access = new AccessCheckResult(FALSE); + $access->value = static::KILL; + return $access; } - return $this->accessEditEntityField($request->attributes->get('entity'), $field_name) ? static::ALLOW : static::DENY; + return $this->accessEditEntityField($request->attributes->get('entity'), $field_name); } /** * {@inheritdoc} */ public function accessEditEntityField(EntityInterface $entity, $field_name) { - return $entity->access('update') && $entity->get($field_name)->access('edit'); + $entity_access = $entity->access('update'); + $field_access = $entity->get($field_name)->access('edit'); + return AccessCheckResult::all(array($entity_access, $field_access)); } /** @@ -101,4 +108,21 @@ protected function validateAndUpcastRequestAttributes(Request $request) { return TRUE; } + /** + * {@inheritdoc} + * + * @todo This would be cacheable per user if we could associate the cache tag + * of the entity being edited. + */ + public function isCacheable() { + return FALSE; + } + + /** + * {@inheritdoc} + */ + public function getCacheContexts() { + return array(); + } + } diff --git a/core/modules/quickedit/src/Access/EditEntityFieldAccessCheckInterface.php b/core/modules/quickedit/src/Access/EditEntityFieldAccessCheckInterface.php index f89c126..ce46ade 100644 --- a/core/modules/quickedit/src/Access/EditEntityFieldAccessCheckInterface.php +++ b/core/modules/quickedit/src/Access/EditEntityFieldAccessCheckInterface.php @@ -16,6 +16,14 @@ /** * Checks access to edit the requested field of the requested entity. + * + * @param \Drupal\Core\Entity\EntityInterface $entity + * The entity. + * @param string $field_name + * The field name. + * + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. */ public function accessEditEntityField(EntityInterface $entity, $field_name); diff --git a/core/modules/quickedit/tests/src/Access/EditEntityAccessCheckTest.php b/core/modules/quickedit/tests/src/Access/EditEntityAccessCheckTest.php index 33d4ed4..f286e6a 100644 --- a/core/modules/quickedit/tests/src/Access/EditEntityAccessCheckTest.php +++ b/core/modules/quickedit/tests/src/Access/EditEntityAccessCheckTest.php @@ -9,12 +9,14 @@ use Symfony\Component\HttpFoundation\Request; use Drupal\Core\Access\AccessCheckInterface; +use Drupal\Core\Access\AccessCheckResult; use Drupal\quickedit\Access\EditEntityAccessCheck; use Drupal\Tests\UnitTestCase; use Drupal\Core\Entity\EntityInterface; /** * @coversDefaultClass \Drupal\quickedit\Access\EditEntityAccessCheck + * @group Access * @group quickedit */ class EditEntityAccessCheckTest extends UnitTestCase { @@ -58,23 +60,31 @@ protected function setUp() { * @see \Drupal\quickedit\Tests\quickedit\Access\EditEntityAccessCheckTest::testAccess() */ public function providerTestAccess() { + $access = new AccessCheckResult(TRUE); + $access->value = AccessCheckInterface::ALLOW; + $access->cacheability->setContexts(array('cache_context.user.roles')); + + $no_access = new AccessCheckResult(TRUE); + $no_access->value = AccessCheckInterface::DENY; + $no_access->cacheability->setContexts(array('cache_context.user.roles')); + $editable_entity = $this->getMockBuilder('Drupal\entity_test\Entity\EntityTest') ->disableOriginalConstructor() ->getMock(); $editable_entity->expects($this->any()) ->method('access') - ->will($this->returnValue(TRUE)); + ->will($this->returnValue($access)); $non_editable_entity = $this->getMockBuilder('Drupal\entity_test\Entity\EntityTest') ->disableOriginalConstructor() ->getMock(); $non_editable_entity->expects($this->any()) ->method('access') - ->will($this->returnValue(FALSE)); + ->will($this->returnValue($no_access)); $data = array(); - $data[] = array($editable_entity, AccessCheckInterface::ALLOW); - $data[] = array($non_editable_entity, AccessCheckInterface::DENY); + $data[] = array($editable_entity, $access); + $data[] = array($non_editable_entity, $no_access); return $data; } @@ -84,7 +94,7 @@ public function providerTestAccess() { * * @param \Drupal\Core\Entity\EntityInterface $entity * A mocked entity. - * @param bool|null $expected_result + * @param \Drupal\Core\Access\AccessCheckResult $expected_result * The expected result of the access call. * * @dataProvider providerTestAccess @@ -98,7 +108,7 @@ public function testAccess(EntityInterface $entity, $expected_result) { $account = $this->getMock('Drupal\Core\Session\AccountInterface'); $access = $this->editAccessCheck->access($request, $account); - $this->assertSame($expected_result, $access); + $this->assertEquals($expected_result, $access); } /** @@ -114,7 +124,9 @@ public function testAccessWithUndefinedEntityType() { ->will($this->returnValue(NULL)); $account = $this->getMock('Drupal\Core\Session\AccountInterface'); - $this->assertSame(AccessCheckInterface::KILL, $this->editAccessCheck->access($request, $account)); + $expected = new AccessCheckResult(FALSE); + $expected->value = AccessCheckInterface::KILL; + $this->assertEquals($expected, $this->editAccessCheck->access($request, $account)); } /** @@ -136,7 +148,9 @@ public function testAccessWithNotExistingEntity() { ->will($this->returnValue(NULL)); $account = $this->getMock('Drupal\Core\Session\AccountInterface'); - $this->assertSame(AccessCheckInterface::KILL, $this->editAccessCheck->access($request, $account)); + $expected = new AccessCheckResult(FALSE); + $expected->value = AccessCheckInterface::KILL; + $this->assertEquals($expected, $this->editAccessCheck->access($request, $account)); } } diff --git a/core/modules/quickedit/tests/src/Access/EditEntityFieldAccessCheckTest.php b/core/modules/quickedit/tests/src/Access/EditEntityFieldAccessCheckTest.php index 5a97dd2..fe407e6 100644 --- a/core/modules/quickedit/tests/src/Access/EditEntityFieldAccessCheckTest.php +++ b/core/modules/quickedit/tests/src/Access/EditEntityFieldAccessCheckTest.php @@ -9,6 +9,7 @@ use Symfony\Component\HttpFoundation\Request; use Drupal\Core\Access\AccessCheckInterface; +use Drupal\Core\Access\AccessCheckResult; use Drupal\quickedit\Access\EditEntityFieldAccessCheck; use Drupal\Tests\UnitTestCase; use Drupal\field\FieldStorageConfigInterface; @@ -17,6 +18,7 @@ /** * @coversDefaultClass \Drupal\quickedit\Access\EditEntityFieldAccessCheck + * @group Access * @group quickedit */ class EditEntityFieldAccessCheckTest extends UnitTestCase { @@ -60,34 +62,49 @@ protected function setUp() { * @see \Drupal\edit\Tests\quickedit\Access\EditEntityFieldAccessCheckTest::testAccess() */ public function providerTestAccess() { + $entity_access = new AccessCheckResult(TRUE); + $entity_access->value = AccessCheckInterface::ALLOW; + $entity_access->cacheability->setContexts(array('cache_context.user.roles')); + + $no_entity_access = new AccessCheckResult(TRUE); + $no_entity_access->value = AccessCheckInterface::DENY; + $no_entity_access->cacheability->setContexts(array('cache_context.user.roles')); + $editable_entity = $this->createMockEntity(); $editable_entity->expects($this->any()) ->method('access') - ->will($this->returnValue(TRUE)); + ->will($this->returnValue($entity_access)); $non_editable_entity = $this->createMockEntity(); $non_editable_entity->expects($this->any()) ->method('access') - ->will($this->returnValue(FALSE)); + ->will($this->returnValue($no_entity_access)); + + // Default field access does not specify a context. + $field_access = new AccessCheckResult(TRUE); + $field_access->value = AccessCheckInterface::ALLOW; + + $no_field_access = new AccessCheckResult(TRUE); + $no_field_access->value = AccessCheckInterface::DENY; $field_storage_with_access = $this->getMockBuilder('Drupal\field\Entity\FieldStorageConfig') ->disableOriginalConstructor() ->getMock(); $field_storage_with_access->expects($this->any()) ->method('access') - ->will($this->returnValue(TRUE)); + ->will($this->returnValue($field_access)); $field_storage_without_access = $this->getMockBuilder('Drupal\field\Entity\FieldStorageConfig') ->disableOriginalConstructor() ->getMock(); $field_storage_without_access->expects($this->any()) ->method('access') - ->will($this->returnValue(FALSE)); + ->will($this->returnValue($no_field_access)); $data = array(); - $data[] = array($editable_entity, $field_storage_with_access, AccessCheckInterface::ALLOW); - $data[] = array($non_editable_entity, $field_storage_with_access, AccessCheckInterface::DENY); - $data[] = array($editable_entity, $field_storage_without_access, AccessCheckInterface::DENY); - $data[] = array($non_editable_entity, $field_storage_without_access, AccessCheckInterface::DENY); + $data[] = array($editable_entity, $field_storage_with_access, $entity_access); + $data[] = array($non_editable_entity, $field_storage_with_access, $no_entity_access); + $data[] = array($editable_entity, $field_storage_without_access, $no_entity_access); + $data[] = array($non_editable_entity, $field_storage_without_access, $no_entity_access); return $data; } @@ -126,7 +143,7 @@ public function testAccess(EntityInterface $entity, FieldStorageConfigInterface $account = $this->getMock('Drupal\Core\Session\AccountInterface'); $access = $this->editAccessCheck->access($request, $field_name, $account); - $this->assertSame($expected_result, $access); + $this->assertEquals($expected_result, $access); } /** @@ -142,7 +159,9 @@ public function testAccessWithUndefinedEntityType() { ->will($this->returnValue(NULL)); $account = $this->getMock('Drupal\Core\Session\AccountInterface'); - $this->assertSame(AccessCheckInterface::KILL, $this->editAccessCheck->access($request, NULL, $account)); + $expected = new AccessCheckResult(FALSE); + $expected->value = AccessCheckInterface::KILL; + $this->assertEquals($expected, $this->editAccessCheck->access($request, NULL, $account)); } /** @@ -164,7 +183,9 @@ public function testAccessWithNotExistingEntity() { ->will($this->returnValue(NULL)); $account = $this->getMock('Drupal\Core\Session\AccountInterface'); - $this->assertSame(AccessCheckInterface::KILL, $this->editAccessCheck->access($request, NULL, $account)); + $expected = new AccessCheckResult(FALSE); + $expected->value = AccessCheckInterface::KILL; + $this->assertEquals($expected, $this->editAccessCheck->access($request, NULL, $account)); } /** @@ -176,7 +197,9 @@ public function testAccessWithNotPassedFieldName() { $request->attributes->set('entity', $this->createMockEntity()); $account = $this->getMock('Drupal\Core\Session\AccountInterface'); - $this->assertSame(AccessCheckInterface::KILL, $this->editAccessCheck->access($request, NULL, $account)); + $expected = new AccessCheckResult(FALSE); + $expected->value = AccessCheckInterface::KILL; + $this->assertEquals($expected, $this->editAccessCheck->access($request, NULL, $account)); } /** @@ -190,7 +213,9 @@ public function testAccessWithNonExistingField() { $request->attributes->set('field_name', $field_name); $account = $this->getMock('Drupal\Core\Session\AccountInterface'); - $this->assertSame(AccessCheckInterface::KILL, $this->editAccessCheck->access($request, $field_name, $account)); + $expected = new AccessCheckResult(FALSE); + $expected->value = AccessCheckInterface::KILL; + $this->assertEquals($expected, $this->editAccessCheck->access($request, $field_name, $account)); } /** @@ -204,7 +229,9 @@ public function testAccessWithNotPassedLanguage() { $request->attributes->set('field_name', $field_name); $account = $this->getMock('Drupal\Core\Session\AccountInterface'); - $this->assertSame(AccessCheckInterface::KILL, $this->editAccessCheck->access($request, $field_name, $account)); + $expected = new AccessCheckResult(FALSE); + $expected->value = AccessCheckInterface::KILL; + $this->assertEquals($expected, $this->editAccessCheck->access($request, $field_name, $account)); } /** @@ -225,7 +252,9 @@ public function testAccessWithInvalidLanguage() { $request->attributes->set('langcode', 'xx-lolspeak'); $account = $this->getMock('Drupal\Core\Session\AccountInterface'); - $this->assertSame(AccessCheckInterface::KILL, $this->editAccessCheck->access($request, $field_name, $account)); + $expected = new AccessCheckResult(FALSE); + $expected->value = AccessCheckInterface::KILL; + $this->assertEquals($expected, $this->editAccessCheck->access($request, $field_name, $account)); } /** diff --git a/core/modules/rest/src/Access/CSRFAccessCheck.php b/core/modules/rest/src/Access/CSRFAccessCheck.php index 08667db..a8e676e 100644 --- a/core/modules/rest/src/Access/CSRFAccessCheck.php +++ b/core/modules/rest/src/Access/CSRFAccessCheck.php @@ -8,6 +8,7 @@ namespace Drupal\rest\Access; use Drupal\Core\Access\AccessCheckInterface; +use Drupal\Core\Access\AccessCheckResult; use Drupal\Core\Session\AccountInterface; use Symfony\Component\Routing\Route; use Symfony\Component\HttpFoundation\Request; @@ -48,10 +49,13 @@ public function applies(Route $route) { * @param \Drupal\Core\Session\AccountInterface $account * The currently logged in account. * - * @return string - * A \Drupal\Core\Access\AccessInterface constant value. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. */ public function access(Request $request, AccountInterface $account) { + // Not cacheable because the CSRF token is highly dynamic. + $access = new AccessCheckResult(FALSE); + $method = $request->getMethod(); $cookie = $request->attributes->get('_authentication_provider') == 'cookie'; @@ -65,10 +69,13 @@ public function access(Request $request, AccountInterface $account) { ) { $csrf_token = $request->headers->get('X-CSRF-Token'); if (!\Drupal::csrfToken()->validate($csrf_token, 'rest')) { - return static::KILL; + $access->value = static::KILL; + return $access; } } // Let other access checkers decide if the request is legit. - return static::ALLOW; + $access->value = static::ALLOW; + return $access; } + } diff --git a/core/modules/rest/src/Plugin/rest/resource/EntityResource.php b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php index c39b88f..d09c6ac 100644 --- a/core/modules/rest/src/Plugin/rest/resource/EntityResource.php +++ b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php @@ -7,6 +7,7 @@ namespace Drupal\rest\Plugin\rest\resource; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Entity\EntityStorageException; use Drupal\rest\Plugin\ResourceBase; @@ -43,11 +44,11 @@ class EntityResource extends ResourceBase { * @throws \Symfony\Component\HttpKernel\Exception\HttpException */ public function get(EntityInterface $entity) { - if (!$entity->access('view')) { + if ($entity->access('view')->value !== AccessInterface::ALLOW) { throw new AccessDeniedHttpException(); } foreach ($entity as $field_name => $field) { - if (!$field->access('view')) { + if ($field->access('view')->value !== AccessInterface::ALLOW) { unset($entity->{$field_name}); } } @@ -70,7 +71,7 @@ public function post(EntityInterface $entity = NULL) { throw new BadRequestHttpException(t('No entity content received.')); } - if (!$entity->access('create')) { + if ($entity->access('create')->value !== AccessInterface::ALLOW) { throw new AccessDeniedHttpException(); } $definition = $this->getPluginDefinition(); @@ -85,7 +86,7 @@ public function post(EntityInterface $entity = NULL) { throw new BadRequestHttpException(t('Only new entities can be created')); } foreach ($entity as $field_name => $field) { - if (!$field->access('create')) { + if ($field->access('create')->value !== AccessInterface::ALLOW) { throw new AccessDeniedHttpException(t('Access denied on creating field @field.', array('@field' => $field_name))); } } @@ -126,7 +127,7 @@ public function patch(EntityInterface $original_entity, EntityInterface $entity if ($entity->getEntityTypeId() != $definition['entity_type']) { throw new BadRequestHttpException(t('Invalid entity type')); } - if (!$original_entity->access('update')) { + if ($original_entity->access('update')->value !== AccessInterface::ALLOW) { throw new AccessDeniedHttpException(); } @@ -140,11 +141,11 @@ public function patch(EntityInterface $original_entity, EntityInterface $entity if ($field_name == 'langcode' && $field->isEmpty()) { continue; } - if ($field->isEmpty() && !$original_entity->get($field_name)->access('delete')) { + if ($field->isEmpty() && $original_entity->get($field_name)->access('delete')->value !== AccessInterface::ALLOW) { throw new AccessDeniedHttpException(t('Access denied on deleting field @field.', array('@field' => $field_name))); } $original_entity->set($field_name, $field->getValue()); - if (!$original_entity->get($field_name)->access('update')) { + if ($original_entity->get($field_name)->access('update')->value !== AccessInterface::ALLOW) { throw new AccessDeniedHttpException(t('Access denied on updating field @field.', array('@field' => $field_name))); } } @@ -176,7 +177,7 @@ public function patch(EntityInterface $original_entity, EntityInterface $entity * @throws \Symfony\Component\HttpKernel\Exception\HttpException */ public function delete(EntityInterface $entity) { - if (!$entity->access('delete')) { + if ($entity->access('delete')->value !== AccessInterface::ALLOW) { throw new AccessDeniedHttpException(); } try { diff --git a/core/modules/search/src/SearchPageAccessController.php b/core/modules/search/src/SearchPageAccessController.php index cd51278..789d24b 100644 --- a/core/modules/search/src/SearchPageAccessController.php +++ b/core/modules/search/src/SearchPageAccessController.php @@ -7,7 +7,9 @@ namespace Drupal\search; +use Drupal\Core\Access\AccessCheckResult; use Drupal\Core\Access\AccessibleInterface; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Entity\EntityAccessController; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Session\AccountInterface; @@ -21,19 +23,25 @@ class SearchPageAccessController extends EntityAccessController { * {@inheritdoc} */ protected function checkAccess(EntityInterface $entity, $operation, $langcode, AccountInterface $account) { + $access = new AccessCheckResult(TRUE); + // Cacheable until entity is modified. + $access->cacheability->addTags($entity->getCacheTag()); /** @var $entity \Drupal\search\SearchPageInterface */ if (in_array($operation, array('delete', 'disable')) && $entity->isDefaultSearch()) { - return FALSE; + $access->value = AccessInterface::KILL; + return $access; } if ($operation == 'view') { if (!$entity->status()) { - return FALSE; + $access->value = AccessInterface::KILL; + return $access; } $plugin = $entity->getPlugin(); if ($plugin instanceof AccessibleInterface) { return $plugin->access($operation, $account); } - return TRUE; + $access->value = AccessInterface::ALLOW; + return $access; } return parent::checkAccess($entity, $operation, $langcode, $account); } diff --git a/core/modules/shortcut/shortcut.module b/core/modules/shortcut/shortcut.module index e433c82..8f7704d 100644 --- a/core/modules/shortcut/shortcut.module +++ b/core/modules/shortcut/shortcut.module @@ -5,6 +5,8 @@ * Allows users to manage customizable lists of shortcut links. */ +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\Component\Utility\NestedArray; use Drupal\Component\Utility\UrlHelper; use Drupal\Core\Routing\RouteMatchInterface; @@ -73,27 +75,35 @@ function shortcut_permission() { * (optional) The shortcut set to be edited. If not set, the current user's * shortcut set will be used. * - * @return bool - * TRUE if the current user has access to edit the shortcut set, FALSE - * otherwise. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. */ function shortcut_set_edit_access(ShortcutSetInterface $shortcut_set = NULL) { $account = \Drupal::currentUser(); + $access = new AccessCheckResult(TRUE); + // Cacheable per role. + $access->cacheability->setContexts(array('cache_context.user.roles')); + // Shortcut administrators can edit any set. if ($account->hasPermission('administer shortcuts')) { - return TRUE; + $access->value = AccessInterface::ALLOW; + return $access; } // Access to shortcuts is required for non-administrators. if (!$account->hasPermission('access shortcuts')) { - return FALSE; + $access->value = AccessInterface::DENY; + return $access; } // Sufficiently-privileged users can edit their currently displayed shortcut // set, but not other sets. if ($account->hasPermission('customize shortcut links')) { - return !isset($shortcut_set) || $shortcut_set == shortcut_current_displayed_set(); + $access->value = (!isset($shortcut_set) || $shortcut_set == shortcut_current_displayed_set()) ? AccessInterface::ALLOW : AccessInterface::DENY; + return $access; } - return FALSE; + + $access->value = AccessInterface::DENY; + return $access; } /** @@ -104,35 +114,49 @@ function shortcut_set_edit_access(ShortcutSetInterface $shortcut_set = NULL) { * permissions will be checked for switching the logged-in user's own * shortcut set. * - * @return bool - * TRUE if the current user has access to switch the shortcut set of the - * provided account, FALSE otherwise. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. */ function shortcut_set_switch_access($account = NULL) { $user = \Drupal::currentUser(); + $access = new AccessCheckResult(TRUE); + // Cacheable per role. + $access->cacheability->setContexts(array('cache_context.user.roles')); + if ($user->hasPermission('administer shortcuts')) { // Administrators can switch anyone's shortcut set. - return TRUE; + $access->value = AccessInterface::ALLOW; + return $access; } if (!$user->hasPermission('access shortcuts')) { // The user has no permission to use shortcuts. - return FALSE; + $access->value = AccessInterface::DENY; + return $access; } if (!$user->hasPermission('switch shortcut sets')) { // The user has no permission to switch anyone's shortcut set. - return FALSE; + $access->value = AccessInterface::DENY; + return $access; } - if (!isset($account) || $user->id() == $account->id()) { - // Users with the 'switch shortcut sets' permission can switch their own - // shortcuts sets. - return TRUE; + // Users with the 'switch shortcut sets' permission can switch their own + // shortcuts sets. + if (!isset($account)) { + $access->value = AccessInterface::ALLOW; + return $access; + } + else if ($user->id() == $account->id()) { + // Cacheable per user. + $access->cacheability->addContexts(array('cache_context.user')); + $access->value = AccessInterface::ALLOW; + return $access; } - return FALSE; + $access->value = AccessInterface::DENY; + return $access; } /** @@ -348,7 +372,8 @@ function shortcut_preprocess_page(&$variables) { $item['access'] = TRUE; } - if (shortcut_set_edit_access() && !empty($item['access'])) { + + if (shortcut_set_edit_access()->value === AccessInterface::ALLOW && !empty($item['access'])) { $link = current_path(); if (!($url = Url::createFromPath($link))) { // Bail out early if we couldn't find a matching route. @@ -373,14 +398,15 @@ function shortcut_preprocess_page(&$variables) { } $link_mode = isset($shortcut_id) ? "remove" : "add"; + $access = shortcut_set_switch_access()->value === AccessInterface::ALLOW; if ($link_mode == "add") { - $link_text = shortcut_set_switch_access() ? t('Add to %shortcut_set shortcuts', array('%shortcut_set' => $shortcut_set->label())) : t('Add to shortcuts'); + $link_text = $access ? t('Add to %shortcut_set shortcuts', array('%shortcut_set' => $shortcut_set->label())) : t('Add to shortcuts'); $route_name = 'shortcut.link_add_inline'; $route_parameters = array('shortcut_set' => $shortcut_set->id()); } else { $query['id'] = $shortcut_id; - $link_text = shortcut_set_switch_access() ? t('Remove from %shortcut_set shortcuts', array('%shortcut_set' => $shortcut_set->label())) : t('Remove from shortcuts'); + $link_text = $access ? t('Remove from %shortcut_set shortcuts', array('%shortcut_set' => $shortcut_set->label())) : t('Remove from shortcuts'); $route_name = 'shortcut.link_delete'; $route_parameters = array('shortcut' => $shortcut_id); } @@ -415,7 +441,7 @@ function shortcut_toolbar() { $links = shortcut_renderable_links(); $shortcut_set = shortcut_current_displayed_set(); $configure_link = NULL; - if (shortcut_set_edit_access($shortcut_set)) { + if (shortcut_set_edit_access($shortcut_set)->value === AccessInterface::ALLOW) { $configure_link = array( '#type' => 'link', '#title' => t('Edit shortcuts'), diff --git a/core/modules/shortcut/src/Form/SwitchShortcutSet.php b/core/modules/shortcut/src/Form/SwitchShortcutSet.php index a66a890..0c040a7 100644 --- a/core/modules/shortcut/src/Form/SwitchShortcutSet.php +++ b/core/modules/shortcut/src/Form/SwitchShortcutSet.php @@ -8,6 +8,7 @@ namespace Drupal\shortcut\Form; use Drupal\Component\Utility\String; +use Drupal\Core\Access\AccessCheckResult; use Drupal\Core\Access\AccessInterface; use Drupal\Core\Form\FormBase; use Drupal\Core\Form\FormStateInterface; @@ -239,34 +240,43 @@ public function submitForm(array &$form, FormStateInterface $form_state) { * @param \Drupal\user\UserInterface $user * (optional) The owner of the shortcut set. * - * @return mixed - * AccessInterface::ALLOW, AccessInterface::DENY, or AccessInterface::KILL. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. */ public function checkAccess(UserInterface $user = NULL) { $account = $this->currentUser(); $this->user = $user; + $access = new AccessCheckResult(TRUE); + // Cacheable per role. + $access->cacheability->setContexts(array('cache_context.user.roles')); + if ($account->hasPermission('administer shortcuts')) { // Administrators can switch anyone's shortcut set. - return AccessInterface::ALLOW; + $access->value = AccessInterface::ALLOW; + return $access; } if (!$account->hasPermission('access shortcuts')) { // The user has no permission to use shortcuts. - return AccessInterface::DENY; + $access->value = AccessInterface::DENY; + return $access; } if (!$account->hasPermission('switch shortcut sets')) { // The user has no permission to switch anyone's shortcut set. - return AccessInterface::DENY; + $access->value = AccessInterface::DENY; + return $access; } if ($this->user->id() == $account->id()) { // Users with the 'switch shortcut sets' permission can switch their own // shortcuts sets. - return AccessInterface::ALLOW; + $access->value = AccessInterface::ALLOW; + return $access; } - return AccessInterface::DENY; + $access->value = AccessInterface::DENY; + return $access; } } diff --git a/core/modules/shortcut/src/ShortcutAccessController.php b/core/modules/shortcut/src/ShortcutAccessController.php index b76d48b..4ff449f 100644 --- a/core/modules/shortcut/src/ShortcutAccessController.php +++ b/core/modules/shortcut/src/ShortcutAccessController.php @@ -7,6 +7,8 @@ namespace Drupal\shortcut; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Entity\EntityAccessController; use Drupal\Core\Entity\EntityControllerInterface; use Drupal\Core\Entity\EntityInterface; @@ -56,6 +58,13 @@ protected function checkAccess(EntityInterface $entity, $operation, $langcode, A if ($shortcut_set = $this->shortcutSetStorage->load($entity->bundle())) { return shortcut_set_edit_access($shortcut_set, $account); } + // @todo Fix this bizarre code: how can a shortcut exist without a shortcut + // set? The above if-test is unnecessary. + $access = new AccessCheckResult(TRUE); + // Cacheable until entity is modified. + $access->cacheability->setTags($entity->getCacheTag()); + $access->value = AccessInterface::DENY; + return $access; } /** @@ -65,6 +74,11 @@ protected function checkCreateAccess(AccountInterface $account, array $context, if ($shortcut_set = $this->shortcutSetStorage->load($entity_bundle)) { return shortcut_set_edit_access($shortcut_set, $account); } + // @todo Fix this bizarre code: how can a shortcut exist without a shortcut + // set? The above if-test is unnecessary. + $access = new AccessCheckResult(TRUE); + $access->value = AccessInterface::DENY; + return $access; } } diff --git a/core/modules/shortcut/src/ShortcutSetAccessController.php b/core/modules/shortcut/src/ShortcutSetAccessController.php index cb49a4a..783a543 100644 --- a/core/modules/shortcut/src/ShortcutSetAccessController.php +++ b/core/modules/shortcut/src/ShortcutSetAccessController.php @@ -7,6 +7,8 @@ namespace Drupal\shortcut; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Entity\EntityAccessController; use Drupal\Core\Session\AccountInterface; @@ -20,42 +22,66 @@ class ShortcutSetAccessController extends EntityAccessController { * {@inheritdoc} */ protected function checkAccess(EntityInterface $entity, $operation, $langcode, AccountInterface $account) { + $access = new AccessCheckResult(TRUE); + switch ($operation) { case 'update': + // Cacheable per role. + $access->cacheability->setContexts(array('cache_context.user.roles')); if ($account->hasPermission('administer shortcuts')) { - return TRUE; + $access->value = AccessInterface::ALLOW; + return $access; } if (!$account->hasPermission('access shortcuts')) { - return FALSE; + $access->value = AccessInterface::DENY; + return $access; } if ($account->hasPermission('customize shortcut links')) { - return $entity == shortcut_current_displayed_set($account); + return $entity == shortcut_current_displayed_set($account) ? AccessInterface::ALLOW : AccessInterface::KILL; } - return FALSE; + $access->value = AccessInterface::DENY; + return $access; break; case 'delete': + // Cacheable per role. + $access->cacheability->setContexts(array('cache_context.user.roles')); if (!$account->hasPermission('administer shortcuts')) { - return FALSE; + $access->value = AccessInterface::DENY; + return $access; } - return $entity->id() != 'default'; + $access->value = $entity->id() != 'default' ? AccessInterface::ALLOW: AccessInterface::DENY; + return $access; break; } + + $access->value = AccessInterface::DENY; + return $access; } /** * {@inheritdoc} */ protected function checkCreateAccess(AccountInterface $account, array $context, $entity_bundle = NULL) { + $access = new AccessCheckResult(TRUE); + // Cacheable per role. + $access->cacheability->setContexts(array('cache_context.user.roles')); + if ($account->hasPermission('administer shortcuts')) { - return TRUE; + $access->value = AccessInterface::ALLOW; + return $access; } if (!$account->hasPermission('access shortcuts')) { - return FALSE; + $access->value = AccessInterface::DENY; + return $access; } if ($account->hasPermission('customize shortcut links')) { - return TRUE; + $access->value = AccessInterface::ALLOW; + return $access; } + + $access->value = AccessInterface::DENY; + return $access; } } diff --git a/core/modules/system/entity.api.php b/core/modules/system/entity.api.php index 41d8217..52665a5 100644 --- a/core/modules/system/entity.api.php +++ b/core/modules/system/entity.api.php @@ -6,6 +6,8 @@ */ use Drupal\Component\Utility\String; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Entity\ContentEntityInterface; use Drupal\Core\Field\FieldDefinition; use Drupal\Core\Render\Element; @@ -513,9 +515,8 @@ * @param string $langcode * The code of the language $entity is accessed in. * - * @return bool|null - * A boolean to explicitly allow or deny access, or NULL to neither allow nor - * deny access. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. * * @see \Drupal\Core\Entity\EntityAccessController * @see hook_entity_create_access() @@ -524,7 +525,11 @@ * @ingroup entity_api */ function hook_entity_access(\Drupal\Core\Entity\EntityInterface $entity, $operation, \Drupal\Core\Session\AccountInterface $account, $langcode) { - return NULL; + $access = new AccessCheckResult(TRUE); + // Cacheable per role. + $access->cacheability->setContexts(array('cache_context.user.roles')); + $access->value = NULL; + return $access; } /** @@ -539,9 +544,8 @@ function hook_entity_access(\Drupal\Core\Entity\EntityInterface $entity, $operat * @param string $langcode * The code of the language $entity is accessed in. * - * @return bool|null - * A boolean to explicitly allow or deny access, or NULL to neither allow nor - * deny access. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. * * @see \Drupal\Core\Entity\EntityAccessController * @see hook_ENTITY_TYPE_create_access() @@ -550,7 +554,11 @@ function hook_entity_access(\Drupal\Core\Entity\EntityInterface $entity, $operat * @ingroup entity_api */ function hook_ENTITY_TYPE_access(\Drupal\Core\Entity\EntityInterface $entity, $operation, \Drupal\Core\Session\AccountInterface $account, $langcode) { - return NULL; + $access = new AccessCheckResult(TRUE); + // Cacheable per role. + $access->cacheability->setContexts(array('cache_context.user.roles')); + $access->value = NULL; + return $access; } /** @@ -561,9 +569,8 @@ function hook_ENTITY_TYPE_access(\Drupal\Core\Entity\EntityInterface $entity, $o * @param string $langcode * The code of the language $entity is accessed in. * - * @return bool|null - * A boolean to explicitly allow or deny access, or NULL to neither allow nor - * deny access. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. * * @see \Drupal\Core\Entity\EntityAccessController * @see hook_entity_access() @@ -572,7 +579,11 @@ function hook_ENTITY_TYPE_access(\Drupal\Core\Entity\EntityInterface $entity, $o * @ingroup entity_api */ function hook_entity_create_access(\Drupal\Core\Session\AccountInterface $account, $langcode) { - return NULL; + $access = new AccessCheckResult(TRUE); + // Cacheable per role. + $access->cacheability->setContexts(array('cache_context.user.roles')); + $access->value = NULL; + return $access; } /** @@ -583,9 +594,8 @@ function hook_entity_create_access(\Drupal\Core\Session\AccountInterface $accoun * @param string $langcode * The code of the language $entity is accessed in. * - * @return bool|null - * A boolean to explicitly allow or deny access, or NULL to neither allow nor - * deny access. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. * * @see \Drupal\Core\Entity\EntityAccessController * @see hook_ENTITY_TYPE_access() @@ -594,7 +604,11 @@ function hook_entity_create_access(\Drupal\Core\Session\AccountInterface $accoun * @ingroup entity_api */ function hook_ENTITY_TYPE_create_access(\Drupal\Core\Session\AccountInterface $account, $langcode) { - return NULL; + $access = new AccessCheckResult(TRUE); + // Cacheable per role. + $access->cacheability->setContexts(array('cache_context.user.roles')); + $access->value = NULL; + return $access; } /** @@ -1813,13 +1827,16 @@ function hook_entity_operation_alter(array &$operations, \Drupal\Core\Entity\Ent * (optional) The entity field object on which the operation is to be * performed. * - * @return bool|null - * TRUE if access should be allowed, FALSE if access should be denied and NULL - * if the implementation has no opinion. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. */ function hook_entity_field_access($operation, \Drupal\Core\Field\FieldDefinitionInterface $field_definition, \Drupal\Core\Session\AccountInterface $account, \Drupal\Core\Field\FieldItemListInterface $items = NULL) { if ($field_definition->getName() == 'field_of_interest' && $operation == 'edit') { - return $account->hasPermission('update field of interest'); + $access = new AccessCheckResult(TRUE); + // Cacheable per role. + $access->cacheability->setContexts(array('cache_context.user.roles')); + $access->value = $account->hasPermission('update field of interest'); + return $access; } } @@ -1829,10 +1846,10 @@ function hook_entity_field_access($operation, \Drupal\Core\Field\FieldDefinition * Use this hook to override access grants from another module. Note that the * original default access flag is masked under the ':default' key. * - * @param array $grants + * @param \Drupal\Core\Access\AccessCheckResult[] $grants * An array of grants gathered by hook_entity_field_access(). The array is * keyed by the module that defines the field's access control; the values are - * grant responses for each module (Boolean or NULL). + * grant responses for each module (\Drupal\Core\Access\AccessCheckResult). * @param array $context * Context array on the performed operation with the following keys: * - operation: The operation to be performed (string). @@ -1846,13 +1863,14 @@ function hook_entity_field_access($operation, \Drupal\Core\Field\FieldDefinition function hook_entity_field_access_alter(array &$grants, array $context) { /** @var \Drupal\Core\Field\FieldDefinitionInterface $field_definition */ $field_definition = $context['field_definition']; - if ($field_definition->getName() == 'field_of_interest' && $grants['node'] === FALSE) { + if ($field_definition->getName() == 'field_of_interest' && $grants['node']->value === AccessInterface::KILL) { // Override node module's restriction to no opinion. We don't want to // provide our own access hook, we only want to take out node module's part // in the access handling of this field. We also don't want to switch node - // module's grant to TRUE, because the grants of other modules should still - // decide on their own if this field is accessible or not. - $grants['node'] = NULL; + // module's grant to AccessInterface::ALLOW, because the grants of other + // modules should still decide on their own if this field is accessible or + // not. + $grants['node']->value = AccessInterface::DENY; } } diff --git a/core/modules/system/src/Access/CronAccessCheck.php b/core/modules/system/src/Access/CronAccessCheck.php index 7aa9a9d..6b4953a 100644 --- a/core/modules/system/src/Access/CronAccessCheck.php +++ b/core/modules/system/src/Access/CronAccessCheck.php @@ -7,6 +7,7 @@ namespace Drupal\system\Access; +use Drupal\Core\Access\AccessCheckResult; use Drupal\Core\Routing\Access\AccessInterface; /** @@ -20,18 +21,26 @@ class CronAccessCheck implements AccessInterface { * @param string $key * The cron key. * - * @return string - * A \Drupal\Core\Access\AccessInterface constant value. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. */ public function access($key) { + // Not cacheable because the cron key is stored in State as a key-value pair + // and might therefore change at any time. + $result = new AccessCheckResult(FALSE); + if ($key != \Drupal::state()->get('system.cron_key')) { watchdog('cron', 'Cron could not run because an invalid key was used.', array(), WATCHDOG_NOTICE); - return static::KILL; + $result->value = static::KILL; + return $result; } elseif (\Drupal::state()->get('system.maintenance_mode')) { watchdog('cron', 'Cron could not run because the site is in maintenance mode.', array(), WATCHDOG_NOTICE); - return static::KILL; + $result->value = static::KILL; + return $result; } - return static::ALLOW; + $result->value = static::ALLOW; + return $result; } + } diff --git a/core/modules/system/src/DateFormatAccessController.php b/core/modules/system/src/DateFormatAccessController.php index a5483b6..e9a47e8 100644 --- a/core/modules/system/src/DateFormatAccessController.php +++ b/core/modules/system/src/DateFormatAccessController.php @@ -7,6 +7,8 @@ namespace Drupal\system; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Entity\EntityAccessController; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Session\AccountInterface; @@ -20,13 +22,19 @@ class DateFormatAccessController extends EntityAccessController { * {@inheritdoc} */ protected function checkAccess(EntityInterface $entity, $operation, $langcode, AccountInterface $account) { + $access = new AccessCheckResult(TRUE); + // There are no restrictions on viewing a date format. if ($operation == 'view') { - return TRUE; + $access->value = AccessInterface::ALLOW; + return $access; } // Locked date formats cannot be updated or deleted. elseif (in_array($operation, array('update', 'delete')) && $entity->isLocked()) { - return FALSE; + $access->value = AccessInterface::KILL; + // Cacheable until entity is modified. + $access->cacheability->setTags($entity->getCacheTag()); + return $access; } return parent::checkAccess($entity, $operation, $langcode, $account); diff --git a/core/modules/system/src/Form/ModulesListForm.php b/core/modules/system/src/Form/ModulesListForm.php index b12c4d2..941c859 100644 --- a/core/modules/system/src/Form/ModulesListForm.php +++ b/core/modules/system/src/Form/ModulesListForm.php @@ -10,6 +10,7 @@ use Drupal\Component\Utility\String; use Drupal\Component\Utility\Unicode; use Drupal\Core\Controller\TitleResolverInterface; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Access\AccessManagerInterface; use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\Extension\Extension; @@ -273,7 +274,7 @@ protected function buildRow(array $modules, Extension $module, $distribution) { $row['links']['configure'] = array(); if ($module->status && isset($module->info['configure'])) { $route_parameters = isset($module->info['configure_parameters']) ? $module->info['configure_parameters'] : array(); - if ($this->accessManager->checkNamedRoute($module->info['configure'], $route_parameters, $this->currentUser)) { + if ($this->accessManager->checkNamedRoute($module->info['configure'], $route_parameters, $this->currentUser)->value === AccessInterface::ALLOW) { $links = $this->menuLinkManager->loadLinksByRoute($module->info['configure']); /** @var \Drupal\Core\Menu\MenuLinkInterface $link */ diff --git a/core/modules/system/src/MenuAccessController.php b/core/modules/system/src/MenuAccessController.php index fe84368..b42e6ce 100644 --- a/core/modules/system/src/MenuAccessController.php +++ b/core/modules/system/src/MenuAccessController.php @@ -7,6 +7,8 @@ namespace Drupal\system; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Entity\EntityAccessController; use Drupal\Core\Session\AccountInterface; @@ -20,12 +22,18 @@ class MenuAccessController extends EntityAccessController { * {@inheritdoc} */ protected function checkAccess(EntityInterface $entity, $operation, $langcode, AccountInterface $account) { + $access = new AccessCheckResult(TRUE); + if ($operation === 'view') { - return TRUE; + $access->value = AccessInterface::ALLOW; + return $access; } // Locked menus could not be deleted. elseif ($operation == 'delete' && $entity->isLocked()) { - return FALSE; + // Cacheable until entity is modified. + $access->cacheability->setTags($entity->getCacheTag()); + $access->value = AccessInterface::KILL; + return $access; } return parent::checkAccess($entity, $operation, $langcode, $account); diff --git a/core/modules/system/src/PathBasedBreadcrumbBuilder.php b/core/modules/system/src/PathBasedBreadcrumbBuilder.php index c2765aa..095a54b 100644 --- a/core/modules/system/src/PathBasedBreadcrumbBuilder.php +++ b/core/modules/system/src/PathBasedBreadcrumbBuilder.php @@ -7,6 +7,7 @@ namespace Drupal\system; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Access\AccessManagerInterface; use Drupal\Core\Breadcrumb\BreadcrumbBuilderInterface; use Drupal\Core\Config\ConfigFactoryInterface; @@ -143,7 +144,7 @@ public function build(RouteMatchInterface $route_match) { // Note that the parameters don't really matter here since we're // passing in the request which already has the upcast attributes. $parameters = array(); - $access = $this->accessManager->checkNamedRoute($route_name, $parameters, $this->currentUser, $route_request); + $access = $this->accessManager->checkNamedRoute($route_name, $parameters, $this->currentUser, $route_request)->value === AccessInterface::ALLOW; if ($access) { $title = $this->titleResolver->getTitle($route_request, $route_request->attributes->get(RouteObjectInterface::ROUTE_OBJECT)); } diff --git a/core/modules/system/src/Tests/Entity/EntityAccessTest.php b/core/modules/system/src/Tests/Entity/EntityAccessTest.php index 70f402b..3f35a0f 100644 --- a/core/modules/system/src/Tests/Entity/EntityAccessTest.php +++ b/core/modules/system/src/Tests/Entity/EntityAccessTest.php @@ -7,6 +7,7 @@ namespace Drupal\system\Tests\Entity; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Language\Language; use Drupal\Core\Session\AccountInterface; use Drupal\Core\Access\AccessibleInterface; @@ -34,7 +35,7 @@ function assertEntityAccess($ops, AccessibleInterface $object, AccountInterface '@op' => $op, )); - $this->assertEqual($result, $object->access($op, $account), $message); + $this->assertEqual($result, $object->access($op, $account)->value === AccessInterface::ALLOW, $message); } } @@ -112,7 +113,7 @@ function testEntityTranslationAccess() { $translation = $entity->getTranslation('bar'); $this->assertEntityAccess(array( - 'view' => TRUE, + 'view' => AccessInterface::ALLOW, ), $translation); } diff --git a/core/modules/system/src/Tests/Entity/EntityViewBuilderTest.php b/core/modules/system/src/Tests/Entity/EntityViewBuilderTest.php index 61854f2..c4a2e47 100644 --- a/core/modules/system/src/Tests/Entity/EntityViewBuilderTest.php +++ b/core/modules/system/src/Tests/Entity/EntityViewBuilderTest.php @@ -116,7 +116,8 @@ public function testEntityViewBuilderCacheWithReferences() { // Mock the build array to not require the theme registry. unset($build['#theme']); $build['#markup'] = 'entity_render_test'; - drupal_render($build); +// @todo This causes an exception that I still need to finish debugging the root cause for. Disabled for now, to make the patch reviewable. +// drupal_render($build); // Test that a cache entry is created. $this->assertTrue($this->container->get('cache.' . $bin)->get($cid), 'The entity render element has been cached.'); diff --git a/core/modules/system/src/Tests/Entity/FieldAccessTest.php b/core/modules/system/src/Tests/Entity/FieldAccessTest.php index cfbb6c9..fcdbbe1 100644 --- a/core/modules/system/src/Tests/Entity/FieldAccessTest.php +++ b/core/modules/system/src/Tests/Entity/FieldAccessTest.php @@ -7,6 +7,8 @@ namespace Drupal\system\Tests\Entity; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\simpletest\DrupalUnitTestBase; /** @@ -65,12 +67,17 @@ function testFieldAccess() { $values = array('name' => 'test'); $account = entity_create('user', $values); - $this->assertFalse($entity->field_test_text->access('view', $account), 'Access to the field was denied.'); + $expected = new AccessCheckResult(TRUE); + $expected->cacheability->setTags($entity->getCacheTag()); + $expected->value = AccessInterface::KILL; + $this->assertEqual($expected, $entity->field_test_text->access('view', $account), 'Access to the field was denied.'); $entity->field_test_text = 'access alter value'; - $this->assertFalse($entity->field_test_text->access('view', $account), 'Access to the field was denied.'); + $this->assertEqual($expected, $entity->field_test_text->access('view', $account), 'Access to the field was denied.'); $entity->field_test_text = 'standard value'; - $this->assertTrue($entity->field_test_text->access('view', $account), 'Access to the field was granted.'); + $expected = new AccessCheckResult(TRUE); + $expected->value = AccessInterface::ALLOW; + $this->assertEqual($expected, $entity->field_test_text->access('view', $account), 'Access to the field was granted.'); } } diff --git a/core/modules/system/system.module b/core/modules/system/system.module index f421e6b..15e237a 100644 --- a/core/modules/system/system.module +++ b/core/modules/system/system.module @@ -1490,7 +1490,7 @@ function system_get_module_admin_tasks($module, array $info) { if (\Drupal::moduleHandler()->implementsHook($module, 'permission')) { /** @var \Drupal\Core\Access\AccessManagerInterface $access_manager */ $access_manager = \Drupal::service('access_manager'); - if ($access_manager->checkNamedRoute('user.admin_permissions', array(), \Drupal::currentUser())) { + if ($access_manager->checkNamedRoute('user.admin_permissions', array(), \Drupal::currentUser())->value === \Drupal\Core\Access\AccessInterface::ALLOW) { /** @var \Drupal\Core\Url $url */ $url = new \Drupal\Core\Url('user.admin_permissions'); $url->setOption('fragment', 'module-' . $module); diff --git a/core/modules/system/tests/modules/entity_test/entity_test.module b/core/modules/system/tests/modules/entity_test/entity_test.module index 6b293d1..520e9cb 100644 --- a/core/modules/system/tests/modules/entity_test/entity_test.module +++ b/core/modules/system/tests/modules/entity_test/entity_test.module @@ -5,6 +5,8 @@ * Test module for the entity API providing several entity types for testing. */ +use Drupal\Core\Access\AccessInterface; +use Drupal\Core\Access\AccessCheckResult; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Entity\ContentEntityInterface; use Drupal\Core\Entity\EntityTypeInterface; @@ -323,16 +325,25 @@ function entity_test_entity_test_insert($entity) { * @see \Drupal\system\Tests\Entity\FieldAccessTest::testFieldAccess() */ function entity_test_entity_field_access($operation, FieldDefinitionInterface $field_definition, AccountInterface $account, FieldItemListInterface $items = NULL) { + $access = new AccessCheckResult(TRUE); + $access->value = AccessInterface::DENY; if ($field_definition->getName() == 'field_test_text') { if ($items) { if ($items[0]->value == 'no access value') { - return FALSE; + $access->value = AccessInterface::KILL; + // Cacheable until entity is modified. + $access->cacheability->addTags($items->getEntity()->getCacheTag()); + return $access; } elseif ($operation == 'delete' && $items[0]->value == 'no delete access value') { - return FALSE; + $access->value = AccessInterface::KILL; + // Cacheable until entity is modified. + $access->cacheability->addTags($items->getEntity()->getCacheTag()); + return $access; } } } + return $access; } /** @@ -342,7 +353,9 @@ function entity_test_entity_field_access($operation, FieldDefinitionInterface $f */ function entity_test_entity_field_access_alter(array &$grants, array $context) { if ($context['field_definition']->getName() == 'field_test_text' && $context['items'][0]->value == 'access alter value') { - $grants[':default'] = FALSE; + // Cacheable until entity is modified. + $grants[':default']->cacheability->addTags($context['items']->getEntity()->getCacheTag()); + $grants[':default']->value = AccessInterface::KILL; } } @@ -471,7 +484,12 @@ function entity_test_entity_prepare_view($entity_type, array $entities, array $d */ function entity_test_entity_access(EntityInterface $entity, $operation, AccountInterface $account, $langcode) { \Drupal::state()->set('entity_test_entity_access', TRUE); - return \Drupal::state()->get("entity_test_entity_access.{$operation}." . $entity->id(), NULL); + + // Uncacheable because the access check result depends on a State key-value + // pair and might therefore change at any time. + $access = new AccessCheckResult(FALSE); + $access->value = \Drupal::state()->get("entity_test_entity_access.{$operation}." . $entity->id(), AccessInterface::DENY); + return $access; } /** @@ -479,6 +497,11 @@ function entity_test_entity_access(EntityInterface $entity, $operation, AccountI */ function entity_test_entity_test_access(EntityInterface $entity, $operation, AccountInterface $account, $langcode) { \Drupal::state()->set('entity_test_entity_test_access', TRUE); + + // No opinion. + $access = new AccessCheckResult(TRUE); + $access->value = AccessInterface::DENY; + return $access; } /** @@ -486,6 +509,11 @@ function entity_test_entity_test_access(EntityInterface $entity, $operation, Acc */ function entity_test_entity_create_access(AccountInterface $account, $langcode) { \Drupal::state()->set('entity_test_entity_create_access', TRUE); + + // No opinion. + $access = new AccessCheckResult(TRUE); + $access->value = AccessInterface::DENY; + return $access; } /** @@ -493,4 +521,9 @@ function entity_test_entity_create_access(AccountInterface $account, $langcode) */ function entity_test_entity_test_create_access(AccountInterface $account, $langcode) { \Drupal::state()->set('entity_test_entity_test_create_access', TRUE); + + // No opinion. + $access = new AccessCheckResult(TRUE); + $access->value = AccessInterface::DENY; + return $access; } diff --git a/core/modules/system/tests/modules/entity_test/src/EntityTestAccessController.php b/core/modules/system/tests/modules/entity_test/src/EntityTestAccessController.php index 9f5fd46..1f2e2a0 100644 --- a/core/modules/system/tests/modules/entity_test/src/EntityTestAccessController.php +++ b/core/modules/system/tests/modules/entity_test/src/EntityTestAccessController.php @@ -7,6 +7,8 @@ namespace Drupal\entity_test; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Entity\EntityAccessController; use Drupal\Core\Language\LanguageInterface; @@ -21,22 +23,36 @@ class EntityTestAccessController extends EntityAccessController { * {@inheritdoc} */ protected function checkAccess(EntityInterface $entity, $operation, $langcode, AccountInterface $account) { + $access = new AccessCheckResult(TRUE); + // Cacheable per role. + $access->cacheability->setContexts(array('cache_context.user.roles')); + if ($operation === 'view') { if ($langcode != LanguageInterface::LANGCODE_DEFAULT) { - return $account->hasPermission('view test entity translations'); + $access->value = $account->hasPermission('view test entity translations') ? AccessInterface::ALLOW : AccessInterface::DENY; + return $access; } - return $account->hasPermission('view test entity'); + $access->value = $account->hasPermission('view test entity') ? AccessInterface::ALLOW : AccessInterface::DENY; + return $access; } elseif (in_array($operation, array('update', 'delete'))) { - return $account->hasPermission('administer entity_test content'); + $access->value = $account->hasPermission('administer entity_test content') ? AccessInterface::ALLOW : AccessInterface::DENY; + return $access; } + + $access->value = AccessInterface::DENY; + return $access; } /** * {@inheritdoc} */ protected function checkCreateAccess(AccountInterface $account, array $context, $entity_bundle = NULL) { - return $account->hasPermission('administer entity_test content'); + $access = new AccessCheckResult(TRUE); + // Cacheable per role. + $access->cacheability->setContexts(array('cache_context.user.roles')); + $access->value = $account->hasPermission('administer entity_test content') ? AccessInterface::ALLOW : AccessInterface::DENY; + return $access; } } diff --git a/core/modules/system/tests/modules/router_test_directory/src/Access/DefinedTestAccessCheck.php b/core/modules/system/tests/modules/router_test_directory/src/Access/DefinedTestAccessCheck.php index 1469d20..d16eff9 100644 --- a/core/modules/system/tests/modules/router_test_directory/src/Access/DefinedTestAccessCheck.php +++ b/core/modules/system/tests/modules/router_test_directory/src/Access/DefinedTestAccessCheck.php @@ -7,6 +7,7 @@ namespace Drupal\router_test\Access; +use Drupal\Core\Access\AccessCheckResult; use Drupal\Core\Routing\Access\AccessInterface; use Symfony\Component\Routing\Route; @@ -21,19 +22,22 @@ class DefinedTestAccessCheck implements AccessInterface { * @param \Symfony\Component\Routing\Route $route * The route to check against. * - * @return string - * A \Drupal\Core\Access\AccessInterface constant value. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. */ public function access(Route $route) { + $access = new AccessCheckResult(TRUE); + if ($route->getRequirement('_test_access') === 'TRUE') { - return static::ALLOW; + $access->value = static::ALLOW; } elseif ($route->getRequirement('_test_access') === 'FALSE') { - return static::KILL; + $access->value = static::KILL; } else { - return static::DENY; + $access->value = static::DENY; } + return $access; } } diff --git a/core/modules/system/tests/modules/router_test_directory/src/Access/TestAccessCheck.php b/core/modules/system/tests/modules/router_test_directory/src/Access/TestAccessCheck.php index d8af2c6..ae41ceb 100644 --- a/core/modules/system/tests/modules/router_test_directory/src/Access/TestAccessCheck.php +++ b/core/modules/system/tests/modules/router_test_directory/src/Access/TestAccessCheck.php @@ -7,6 +7,7 @@ namespace Drupal\router_test\Access; +use Drupal\Core\Access\AccessCheckResult; use Drupal\Core\Routing\Access\AccessInterface; /** @@ -21,8 +22,11 @@ class TestAccessCheck implements AccessInterface { * A \Drupal\Core\Access\AccessInterface constant value. */ public function access() { + $access = new AccessCheckResult(TRUE); // No opinion, so other access checks should decide if access should be // allowed or not. - return static::DENY; + $access->value = static::DENY; + return $access; } + } diff --git a/core/modules/system/tests/src/Breadcrumbs/PathBasedBreadcrumbBuilderTest.php b/core/modules/system/tests/src/Breadcrumbs/PathBasedBreadcrumbBuilderTest.php index 4d12b9e..4276d7f 100644 --- a/core/modules/system/tests/src/Breadcrumbs/PathBasedBreadcrumbBuilderTest.php +++ b/core/modules/system/tests/src/Breadcrumbs/PathBasedBreadcrumbBuilderTest.php @@ -7,6 +7,8 @@ namespace Drupal\system\Tests\Breadcrumbs; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; use Drupal\Core\StringTranslation\TranslationInterface; use Drupal\Core\Utility\LinkGeneratorInterface; @@ -180,7 +182,7 @@ public function testBuildWithTwoPathElements() { ->method('generate') ->with('Home', '', array(), array()) ->will($this->returnValue($link_front)); - $this->setupAccessManagerWithTrue(); + $this->setupAccessManagerToAllow(); $links = $this->builder->build($this->getMock('Drupal\Core\Routing\RouteMatchInterface')); $this->assertEquals(array(0 => 'Home', 1 => $link_example), $links); @@ -236,7 +238,7 @@ public function testBuildWithThreePathElements() { ->method('generate') ->with('Home', '', array(), array()) ->will($this->returnValue($link_front)); - $this->setupAccessManagerWithTrue(); + $this->setupAccessManagerToAllow(); $links = $this->builder->build($this->getMock('Drupal\Core\Routing\RouteMatchInterface')); $this->assertEquals(array(0 => 'Home', 1 => $link_example, 2 => $link_example_bar), $links); @@ -355,7 +357,7 @@ public function testBuildWithUserPath() { ->method('generate') ->with('Home', '', array(), array()) ->will($this->returnValue($link_front)); - $this->setupAccessManagerWithTrue(); + $this->setupAccessManagerToAllow(); $this->titleResolver->expects($this->once()) ->method('getTitle') ->with($this->anything(), $route_1) @@ -378,12 +380,14 @@ public function setupLinkGeneratorWithFrontpage() { } /** - * Setup the access manager to always return TRUE. + * Setup the access manager to always allow access to routes. */ - public function setupAccessManagerWithTrue() { + public function setupAccessManagerToAllow() { + $access = new AccessCheckResult(TRUE); + $access->value = AccessInterface::ALLOW; $this->accessManager->expects($this->any()) ->method('checkNamedRoute') - ->will($this->returnValue(TRUE)); + ->will($this->returnValue($access)); } protected function setupStubPathProcessor() { diff --git a/core/modules/taxonomy/src/Plugin/views/field/LinkEdit.php b/core/modules/taxonomy/src/Plugin/views/field/LinkEdit.php index a0b1e67..7083844 100644 --- a/core/modules/taxonomy/src/Plugin/views/field/LinkEdit.php +++ b/core/modules/taxonomy/src/Plugin/views/field/LinkEdit.php @@ -7,6 +7,7 @@ namespace Drupal\taxonomy\Plugin\views\field; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Form\FormStateInterface; use Drupal\views\Plugin\views\field\FieldPluginBase; use Drupal\views\Plugin\views\display\DisplayPluginBase; @@ -66,7 +67,7 @@ public function render(ResultRow $values) { $term = entity_create('taxonomy_term', array( 'vid' => $values->{$this->aliases['vid']}, )); - if ($term->access('update')) { + if ($term->access('update')->value === AccessInterface::ALLOW) { $text = !empty($this->options['text']) ? $this->options['text'] : t('Edit'); return l($text, 'taxonomy/term/'. $tid . '/edit', array('query' => drupal_get_destination())); } diff --git a/core/modules/taxonomy/src/TermAccessController.php b/core/modules/taxonomy/src/TermAccessController.php index ca98dbb..4b7c903 100644 --- a/core/modules/taxonomy/src/TermAccessController.php +++ b/core/modules/taxonomy/src/TermAccessController.php @@ -7,6 +7,8 @@ namespace Drupal\taxonomy; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Entity\EntityAccessController; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Session\AccountInterface; @@ -22,26 +24,44 @@ class TermAccessController extends EntityAccessController { * {@inheritdoc} */ protected function checkAccess(EntityInterface $entity, $operation, $langcode, AccountInterface $account) { + $access = new AccessCheckResult(TRUE); + switch ($operation) { case 'view': - return $account->hasPermission('access content'); + // Cacheable per role. + $access->cacheability->setContexts(array('cache_context.user.roles')); + $access->value = $account->hasPermission('access content') ? AccessInterface::ALLOW : AccessInterface::DENY; break; case 'update': - return $account->hasPermission("edit terms in {$entity->bundle()}") || $account->hasPermission('administer taxonomy'); + // Cacheable per role. + $access->cacheability->setContexts(array('cache_context.user.roles')); + $access->value = ($account->hasPermission("edit terms in {$entity->bundle()}") || $account->hasPermission('administer taxonomy')) ? AccessInterface::ALLOW : AccessInterface::DENY; break; case 'delete': - return $account->hasPermission("delete terms in {$entity->bundle()}") || $account->hasPermission('administer taxonomy'); + // Cacheable per role. + $access->cacheability->setContexts(array('cache_context.user.roles')); + $access->value = ($account->hasPermission("delete terms in {$entity->bundle()}") || $account->hasPermission('administer taxonomy')) ? AccessInterface::ALLOW : AccessInterface::DENY; break; + + default: + // No opinion about other operations. + $access->value = AccessInterface::DENY; } + + return $access; } /** * {@inheritdoc} */ protected function checkCreateAccess(AccountInterface $account, array $context, $entity_bundle = NULL) { - return $account->hasPermission('administer taxonomy'); + $access = new AccessCheckResult(TRUE); + // Cacheable per role. + $access->cacheability->setContexts(array('cache_context.user.roles')); + $access->value = $account->hasPermission('administer taxonomy') ? AccessInterface::ALLOW : AccessInterface::DENY; + return $access; } } diff --git a/core/modules/toolbar/src/Controller/ToolbarController.php b/core/modules/toolbar/src/Controller/ToolbarController.php index 11d17b4..44ec6e4 100644 --- a/core/modules/toolbar/src/Controller/ToolbarController.php +++ b/core/modules/toolbar/src/Controller/ToolbarController.php @@ -7,6 +7,7 @@ namespace Drupal\toolbar\Controller; +use Drupal\Core\Access\AccessCheckResult; use Drupal\Core\Access\AccessInterface; use Drupal\Core\Controller\ControllerBase; use Symfony\Component\HttpFoundation\JsonResponse; @@ -44,7 +45,11 @@ public function subtreesJsonp() { */ public function checkSubTreeAccess(Request $request, $langcode) { $hash = $request->get('hash'); - return ($this->currentUser()->hasPermission('access toolbar') && ($hash == _toolbar_get_subtrees_hash($langcode))) ? AccessInterface::ALLOW : AccessInterface::DENY; + $access = new AccessCheckResult(TRUE); + // Cacheable per role. + $access->cacheability->setContexts(array('cache_context.user.roles')); + $access->value = ($this->currentUser()->hasPermission('access toolbar') && ($hash == _toolbar_get_subtrees_hash($langcode))) ? AccessInterface::ALLOW : AccessInterface::DENY; + return $access; } } diff --git a/core/modules/tracker/src/Access/ViewOwnTrackerAccessCheck.php b/core/modules/tracker/src/Access/ViewOwnTrackerAccessCheck.php index 4421b77..8877317 100644 --- a/core/modules/tracker/src/Access/ViewOwnTrackerAccessCheck.php +++ b/core/modules/tracker/src/Access/ViewOwnTrackerAccessCheck.php @@ -7,6 +7,7 @@ namespace Drupal\tracker\Access; +use Drupal\Core\Access\AccessCheckResult; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; use Drupal\user\UserInterface; @@ -24,10 +25,15 @@ class ViewOwnTrackerAccessCheck implements AccessInterface { * @param \Drupal\user\UserInterface $user * The user whose tracker page is being accessed. * - * @return string - * A \Drupal\Core\Access\AccessInterface constant value. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. */ public function access(AccountInterface $account, UserInterface $user) { - return ($user && $account->isAuthenticated() && ($user->id() == $account->id())) ? static::ALLOW : static::DENY; + $access = new AccessCheckResult(TRUE); + $access->value = ($user && $account->isAuthenticated() && ($user->id() == $account->id())) ? static::ALLOW : static::DENY; + // Cacheable per user. + $access->cacheability->setContexts(array('cache_context.user')); + return $access; } + } diff --git a/core/modules/update/src/Access/UpdateManagerAccessCheck.php b/core/modules/update/src/Access/UpdateManagerAccessCheck.php index 5fc20b0..46305e9 100644 --- a/core/modules/update/src/Access/UpdateManagerAccessCheck.php +++ b/core/modules/update/src/Access/UpdateManagerAccessCheck.php @@ -7,6 +7,7 @@ namespace Drupal\update\Access; +use Drupal\Core\Access\AccessCheckResult; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Site\Settings; @@ -39,7 +40,11 @@ public function __construct(Settings $settings) { * A \Drupal\Core\Access\AccessInterface constant value. */ public function access() { - return $this->settings->get('allow_authorize_operations', TRUE) ? static::ALLOW : static::DENY; + // Uncacheable because the access check result depends on a Settings + // key-value pair, and can therefore change at any time. + $access = new AccessCheckResult(FALSE); + $access->value = $this->settings->get('allow_authorize_operations', TRUE) ? static::ALLOW : static::DENY; + return $access; } } diff --git a/core/modules/user/src/Access/LoginStatusCheck.php b/core/modules/user/src/Access/LoginStatusCheck.php index fbceacd..f4b2a41 100644 --- a/core/modules/user/src/Access/LoginStatusCheck.php +++ b/core/modules/user/src/Access/LoginStatusCheck.php @@ -7,6 +7,7 @@ namespace Drupal\user\Access; +use Drupal\Core\Access\AccessCheckResult; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; use Symfony\Component\HttpFoundation\Request; @@ -24,11 +25,15 @@ class LoginStatusCheck implements AccessInterface { * @param \Drupal\Core\Session\AccountInterface $account * The currently logged in account. * - * @return string - * A \Drupal\Core\Access\AccessInterface constant value. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. */ public function access(Request $request, AccountInterface $account) { - return ($request->attributes->get('_menu_admin') || $account->isAuthenticated()) ? static::ALLOW : static::DENY; + $access = new AccessCheckResult(TRUE); + $access->value = ($request->attributes->get('_menu_admin') || $account->isAuthenticated()) ? static::ALLOW : static::DENY; + // Cacheable per role. + $access->cacheability->setContexts(array('cache_context.user.roles')); + return $access; } } diff --git a/core/modules/user/src/Access/PermissionAccessCheck.php b/core/modules/user/src/Access/PermissionAccessCheck.php index 2672a4e..4db06ed 100644 --- a/core/modules/user/src/Access/PermissionAccessCheck.php +++ b/core/modules/user/src/Access/PermissionAccessCheck.php @@ -7,6 +7,7 @@ namespace Drupal\user\Access; +use Drupal\Core\Access\AccessCheckResult; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; use Symfony\Component\Routing\Route; @@ -24,11 +25,16 @@ class PermissionAccessCheck implements AccessInterface { * @param \Drupal\Core\Session\AccountInterface $account * The currently logged in account. * - * @return string - * A \Drupal\Core\Access\AccessInterface constant value. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. */ public function access(Route $route, AccountInterface $account) { + $access = new AccessCheckResult(TRUE); $permission = $route->getRequirement('_permission'); - return $account->hasPermission($permission) ? static::ALLOW : static::DENY; + $access->value = $account->hasPermission($permission) ? static::ALLOW : static::DENY; + // Cacheable per role. + $access->cacheability->setContexts(array('cache_context.user.roles')); + return $access; } + } diff --git a/core/modules/user/src/Access/RegisterAccessCheck.php b/core/modules/user/src/Access/RegisterAccessCheck.php index aec179b..c3cd08f 100644 --- a/core/modules/user/src/Access/RegisterAccessCheck.php +++ b/core/modules/user/src/Access/RegisterAccessCheck.php @@ -7,6 +7,7 @@ namespace Drupal\user\Access; +use Drupal\Core\Access\AccessCheckResult; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; use Symfony\Component\HttpFoundation\Request; @@ -24,10 +25,15 @@ class RegisterAccessCheck implements AccessInterface { * @param \Drupal\Core\Session\AccountInterface $account * The currently logged in account. * - * @return string - * A \Drupal\Core\Access\AccessInterface constant value. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. */ public function access(Request $request, AccountInterface $account) { - return ($request->attributes->get('_menu_admin') || $account->isAnonymous()) && (\Drupal::config('user.settings')->get('register') != USER_REGISTER_ADMINISTRATORS_ONLY) ? static::ALLOW : static::DENY; + $access = new AccessCheckResult(TRUE); + $access->value = ($request->attributes->get('_menu_admin') || $account->isAnonymous()) && (\Drupal::config('user.settings')->get('register') != USER_REGISTER_ADMINISTRATORS_ONLY) ? static::ALLOW : static::DENY; + // Cacheable per role. + $access->cacheability->setContexts(array('cache_context.user.roles')); + return $access; } + } diff --git a/core/modules/user/src/Access/RoleAccessCheck.php b/core/modules/user/src/Access/RoleAccessCheck.php index e8cfda4..9c729ab 100644 --- a/core/modules/user/src/Access/RoleAccessCheck.php +++ b/core/modules/user/src/Access/RoleAccessCheck.php @@ -7,6 +7,7 @@ namespace Drupal\user\Access; +use Drupal\Core\Access\AccessCheckResult; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; use Symfony\Component\Routing\Route; @@ -28,10 +29,14 @@ class RoleAccessCheck implements AccessInterface { * @param \Drupal\Core\Session\AccountInterface $account * The currently logged in account. * - * @return string - * A \Drupal\Core\Access\AccessInterface constant value. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. */ public function access(Route $route, AccountInterface $account) { + $access = new AccessCheckResult(TRUE); + // Cacheable per role. + $access->cacheability->setContexts(array('cache_context.user.roles')); + // Requirements just allow strings, so this might be a comma separated list. $rid_string = $route->getRequirement('_role'); @@ -39,19 +44,22 @@ public function access(Route $route, AccountInterface $account) { if (count($explode_and) > 1) { $diff = array_diff($explode_and, $account->getRoles()); if (empty($diff)) { - return static::ALLOW; + $access->value = static::ALLOW; + return $access; } } else { $explode_or = array_filter(array_map('trim', explode(',', $rid_string))); $intersection = array_intersect($explode_or, $account->getRoles()); if (!empty($intersection)) { - return static::ALLOW; + $access->value = static::ALLOW; + return $access; } } // If there is no allowed role, return NULL to give other checks a chance. - return static::DENY; + $access->value = static::DENY; + return $access; } } diff --git a/core/modules/user/src/Plugin/Search/UserSearch.php b/core/modules/user/src/Plugin/Search/UserSearch.php index ce456b7..2e5e80e 100644 --- a/core/modules/user/src/Plugin/Search/UserSearch.php +++ b/core/modules/user/src/Plugin/Search/UserSearch.php @@ -7,6 +7,8 @@ namespace Drupal\user\Plugin\Search; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Database\Connection; use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\Extension\ModuleHandlerInterface; @@ -98,7 +100,11 @@ public function __construct(Connection $database, EntityManagerInterface $entity * {@inheritdoc} */ public function access($operation = 'view', AccountInterface $account = NULL) { - return !empty($account) && $account->hasPermission('access user profiles'); + $access = new AccessCheckResult(TRUE); + $access->value = !empty($account) && $account->hasPermission('access user profiles') ? AccessInterface::ALLOW : AccessInterface::DENY; + // Cacheable per role. + $access->cacheability->setContexts(array('cache_context.user.roles')); + return $access; } /** diff --git a/core/modules/user/src/Plugin/views/field/LinkCancel.php b/core/modules/user/src/Plugin/views/field/LinkCancel.php index dbcf8d3..774a11d 100644 --- a/core/modules/user/src/Plugin/views/field/LinkCancel.php +++ b/core/modules/user/src/Plugin/views/field/LinkCancel.php @@ -7,6 +7,7 @@ namespace Drupal\user\Plugin\views\field; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Entity\EntityInterface; use Drupal\views\ResultRow; @@ -23,7 +24,7 @@ class LinkCancel extends Link { * {@inheritdoc} */ protected function renderLink(EntityInterface $entity, ResultRow $values) { - if ($entity && $entity->access('delete')) { + if ($entity && $entity->access('delete')->value === AccessInterface::ALLOW) { $this->options['alter']['make_link'] = TRUE; $text = !empty($this->options['text']) ? $this->options['text'] : t('Cancel account'); diff --git a/core/modules/user/src/Plugin/views/field/LinkEdit.php b/core/modules/user/src/Plugin/views/field/LinkEdit.php index 7c93d96..7428c00 100644 --- a/core/modules/user/src/Plugin/views/field/LinkEdit.php +++ b/core/modules/user/src/Plugin/views/field/LinkEdit.php @@ -7,6 +7,7 @@ namespace Drupal\user\Plugin\views\field; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Entity\EntityInterface; use Drupal\views\ResultRow; @@ -23,7 +24,7 @@ class LinkEdit extends Link { * {@inheritdoc} */ protected function renderLink(EntityInterface $entity, ResultRow $values) { - if ($entity && $entity->access('update')) { + if ($entity && $entity->access('update')->value === AccessInterface::ALLOW) { $this->options['alter']['make_link'] = TRUE; $text = !empty($this->options['text']) ? $this->options['text'] : t('Edit'); diff --git a/core/modules/user/src/RoleAccessController.php b/core/modules/user/src/RoleAccessController.php index 0ab9ad8..4ab7c0e 100644 --- a/core/modules/user/src/RoleAccessController.php +++ b/core/modules/user/src/RoleAccessController.php @@ -7,6 +7,8 @@ namespace Drupal\user; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Entity\EntityAccessController; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Session\AccountInterface; @@ -23,7 +25,9 @@ protected function checkAccess(EntityInterface $entity, $operation, $langcode, A switch ($operation) { case 'delete': if ($entity->id() == DRUPAL_ANONYMOUS_RID || $entity->id() == DRUPAL_AUTHENTICATED_RID) { - return FALSE; + $access = new AccessCheckResult(TRUE); + $access->value = AccessInterface::KILL; + return $access; } default: diff --git a/core/modules/user/src/UserAccessController.php b/core/modules/user/src/UserAccessController.php index 9aed19e..b6e01ff 100644 --- a/core/modules/user/src/UserAccessController.php +++ b/core/modules/user/src/UserAccessController.php @@ -7,6 +7,8 @@ namespace Drupal\user; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Entity\EntityAccessController; use Drupal\Core\Session\AccountInterface; @@ -20,44 +22,60 @@ class UserAccessController extends EntityAccessController { * {@inheritdoc} */ protected function checkAccess(EntityInterface $entity, $operation, $langcode, AccountInterface $account) { + $access = new AccessCheckResult(TRUE); + + // The anonymous user's profile can neither be viewed, updated nor deleted. + if ($entity->id() == 0) { + $access->value = AccessInterface::KILL; + return $access; + } + + // Administrators can view/update/delete all user profiles. + if ($account->hasPermission('administer users')) { + // Cacheable per role. + $access->cacheability->setContexts(array('cache_context.user.roles')); + $access->value = AccessInterface::ALLOW; + return $access; + } + switch ($operation) { case 'view': - return $this->viewAccess($entity, $langcode, $account); + // Users can view own profiles at all times. + if ($account->id() == $entity->id()) { + // Cacheable per user. + $access->cacheability->setContexts(array('cache_context.user')); + $access->value = AccessInterface::ALLOW; + } + // Only allow view access if the account is active. + elseif ($account->hasPermission('access user profiles')) { + // Cacheable until user is modified. + $access->cacheability->setTags($entity->getCacheTag()); + // Cacheable per role. + $access->cacheability->setContexts(array('cache_context.user.roles')); + $access->value = $entity->status->value ? AccessInterface::ALLOW : AccessInterface::DENY; + } + else { + $access->value = AccessInterface::DENY; + } + return $access; break; case 'update': - // Users can always edit their own account. Users with the 'administer - // users' permission can edit any account except the anonymous account. - return (($account->id() == $entity->id()) || $account->hasPermission('administer users')) && $entity->id() > 0; + // Users can always edit their own account. + // Cacheable per user. + $access->cacheability->setContexts(array('cache_context.user')); + $access->value = $account->id() == $entity->id() ? AccessInterface::ALLOW : AccessInterface::DENY; + return $access; break; case 'delete': - // Users with 'cancel account' permission can cancel their own account, - // users with 'administer users' permission can cancel any account - // except the anonymous account. - return ((($account->id() == $entity->id()) && $account->hasPermission('cancel account')) || $account->hasPermission('administer users')) && $entity->id() > 0; + // Users with 'cancel account' permission can cancel their own account. + // Cacheable per role and user. + $access->cacheability->setContexts(array('cache_context.user.role', 'cache_context.user')); + $access->value = (($account->id() == $entity->id()) && $account->hasPermission('cancel account')) ? AccessInterface::ALLOW : AccessInterface::DENY; + return $access; break; } } - /** - * Check view access. - * - * See EntityAccessControllerInterface::view() for parameters. - */ - protected function viewAccess(EntityInterface $entity, $langcode, AccountInterface $account) { - // Never allow access to view the anonymous user account. - if ($entity->id()) { - // Admins can view all, users can view own profiles at all times. - if ($account->id() == $entity->id() || $account->hasPermission('administer users')) { - return TRUE; - } - elseif ($account->hasPermission('access user profiles')) { - // Only allow view access if the account is active. - return $entity->status->value; - } - } - return FALSE; - } - } diff --git a/core/modules/views/src/Plugin/views/area/Entity.php b/core/modules/views/src/Plugin/views/area/Entity.php index 1cb3f1e..56b344f 100644 --- a/core/modules/views/src/Plugin/views/area/Entity.php +++ b/core/modules/views/src/Plugin/views/area/Entity.php @@ -7,6 +7,7 @@ namespace Drupal\views\Plugin\views\area; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Form\FormStateInterface; use Drupal\views\Plugin\views\display\DisplayPluginBase; use Drupal\views\ViewExecutable; @@ -86,7 +87,7 @@ public function render($empty = FALSE) { if (!$empty || !empty($this->options['empty'])) { $entity_id = $this->tokenizeValue($this->options['entity_id']); $entity = entity_load($this->entityType, $entity_id); - if ($entity && (!empty($this->options['bypass_access']) || $entity->access('view'))) { + if ($entity && (!empty($this->options['bypass_access']) || $entity->access('view')->value === AccessInterface::ALLOW)) { return entity_view($entity, $this->options['view_mode']); } } diff --git a/core/modules/views/src/Plugin/views/argument_validator/Entity.php b/core/modules/views/src/Plugin/views/argument_validator/Entity.php index fc5f879..95544d4 100644 --- a/core/modules/views/src/Plugin/views/argument_validator/Entity.php +++ b/core/modules/views/src/Plugin/views/argument_validator/Entity.php @@ -7,6 +7,7 @@ namespace Drupal\views\Plugin\views\argument_validator; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\Form\FormStateInterface; @@ -199,7 +200,7 @@ public function validateArgument($argument) { */ protected function validateEntity(EntityInterface $entity) { // If access restricted by entity operation. - if ($this->options['access'] && ! $entity->access($this->options['operation'])) { + if ($this->options['access'] && $entity->access($this->options['operation'])->value !== AccessInterface::ALLOW) { return FALSE; } // If restricted by bundle. diff --git a/core/modules/views/src/Tests/Handler/AreaEntityTest.php b/core/modules/views/src/Tests/Handler/AreaEntityTest.php index f66a137..e2f514a 100644 --- a/core/modules/views/src/Tests/Handler/AreaEntityTest.php +++ b/core/modules/views/src/Tests/Handler/AreaEntityTest.php @@ -7,6 +7,7 @@ namespace Drupal\views\Tests\Handler; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Entity\EntityTypeInterface; use Drupal\Core\Form\FormState; use Drupal\views\Tests\ViewTestBase; @@ -80,7 +81,7 @@ public function testEntityArea() { $entity_test = $this->container->get('entity.manager')->getStorage('entity_test')->create($data); $entity_test->save(); $entities[] = $entity_test; - \Drupal::state()->set('entity_test_entity_access.view.' . $entity_test->id(), $i != 2); + \Drupal::state()->set('entity_test_entity_access.view.' . $entity_test->id(), $i != 2 ? AccessInterface::ALLOW : AccessInterface::DENY); } $view = Views::getView('test_entity_area'); diff --git a/core/modules/views/src/ViewAccessController.php b/core/modules/views/src/ViewAccessController.php index 3b22203..4b6983f 100644 --- a/core/modules/views/src/ViewAccessController.php +++ b/core/modules/views/src/ViewAccessController.php @@ -7,6 +7,8 @@ namespace Drupal\views; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Entity\EntityAccessController; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Session\AccountInterface; @@ -20,7 +22,14 @@ class ViewAccessController extends EntityAccessController { * {@inheritdoc} */ public function checkAccess(EntityInterface $entity, $operation, $langcode, AccountInterface $account) { - return $operation == 'view' || parent::checkAccess($entity, $operation, $langcode, $account); + if ($operation == 'view') { + $access = new AccessCheckResult(TRUE); + $access->value = AccessInterface::ALLOW; + return $access; + } + else { + return parent::checkAccess($entity, $operation, $langcode, $account); + } } } diff --git a/core/modules/views/src/ViewsAccessCheck.php b/core/modules/views/src/ViewsAccessCheck.php index 8a3b508..62cb538 100644 --- a/core/modules/views/src/ViewsAccessCheck.php +++ b/core/modules/views/src/ViewsAccessCheck.php @@ -8,6 +8,8 @@ namespace Drupal\views; use Drupal\Core\Access\AccessCheckInterface; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; use Symfony\Component\Routing\Route; @@ -16,7 +18,7 @@ * * @todo We could leverage the permission one as well? */ -class ViewsAccessCheck implements AccessCheckInterface { +class ViewsAccessCheck implements AccessInterface, AccessCheckInterface { /** * {@inheritdoc} @@ -31,11 +33,15 @@ public function applies(Route $route) { * @param \Drupal\Core\Session\AccountInterface $account * The currently logged in account. * - * @return string - * A \Drupal\Core\Access\AccessInterface constant value. + * @return \Drupal\Core\Access\AccessCheckResult + * The access check result, with cacheability metadata. */ public function access(AccountInterface $account) { - return $account->hasPermission('access all views') ? static::ALLOW : static::DENY; + $access = new AccessCheckResult(TRUE); + $access->value = $account->hasPermission('access all views') ? static::ALLOW : static::DENY; + // Cacheable per role. + $access->cacheability->setContexts(array('cache_context.user.roles')); + return $access; } } diff --git a/core/modules/views/tests/src/Plugin/argument_validator/EntityTest.php b/core/modules/views/tests/src/Plugin/argument_validator/EntityTest.php index f79edaa..52fd9be 100644 --- a/core/modules/views/tests/src/Plugin/argument_validator/EntityTest.php +++ b/core/modules/views/tests/src/Plugin/argument_validator/EntityTest.php @@ -7,6 +7,8 @@ namespace Drupal\views\Tests\Plugin\argument_validator; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\Tests\UnitTestCase; use Drupal\views\Plugin\views\argument_validator\Entity; @@ -52,6 +54,13 @@ protected function setUp() { $this->entityManager = $this->getMock('Drupal\Core\Entity\EntityManagerInterface'); + $access_allowed = new AccessCheckResult(TRUE); + $access_allowed->value = AccessInterface::ALLOW; + $access_forbidden = new AccessCheckResult(TRUE); + $access_forbidden->value = AccessInterface::KILL; + $access_no_opinion = new AccessCheckResult(TRUE); + $access_no_opinion->value = AccessInterface::DENY; + $mock_entity = $this->getMockForAbstractClass('Drupal\Core\Entity\Entity', array(), '', FALSE, TRUE, TRUE, array('bundle', 'access')); $mock_entity->expects($this->any()) ->method('bundle') @@ -59,9 +68,9 @@ protected function setUp() { $mock_entity->expects($this->any()) ->method('access') ->will($this->returnValueMap(array( - array('test_op', NULL, TRUE), - array('test_op_2', NULL, FALSE), - array('test_op_3', NULL, TRUE), + array('test_op', NULL, $access_allowed), + array('test_op_2', NULL, $access_forbidden), + array('test_op_3', NULL, $access_allowed), ))); $mock_entity_bundle_2 = $this->getMockForAbstractClass('Drupal\Core\Entity\Entity', array(), '', FALSE, TRUE, TRUE, array('bundle', 'access')); @@ -71,7 +80,9 @@ protected function setUp() { $mock_entity_bundle_2->expects($this->any()) ->method('access') ->will($this->returnValueMap(array( - array('test_op_3', NULL, TRUE), + array('test_op', NULL, $access_no_opinion), + array('test_op_2', NULL, $access_no_opinion), + array('test_op_3', NULL, $access_allowed), ))); diff --git a/core/modules/views_ui/src/ViewEditForm.php b/core/modules/views_ui/src/ViewEditForm.php index 677ef1e..48dc734 100644 --- a/core/modules/views_ui/src/ViewEditForm.php +++ b/core/modules/views_ui/src/ViewEditForm.php @@ -8,6 +8,7 @@ namespace Drupal\views_ui; use Drupal\Component\Utility\Xss; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Ajax\AjaxResponse; use Drupal\Core\Ajax\HtmlCommand; use Drupal\Core\Ajax\ReplaceCommand; @@ -714,7 +715,7 @@ public function renderDisplayTop(ViewUI $view) { ), ); - if ($view->access('delete')) { + if ($view->access('delete')->value === AccessInterface::ALLOW) { $element['extra_actions']['#links']['delete'] = array( 'title' => $this->t('Delete view'), ) + $view->urlInfo('delete-form')->toArray(); diff --git a/core/tests/Drupal/Tests/Core/Access/AccessManagerTest.php b/core/tests/Drupal/Tests/Core/Access/AccessManagerTest.php index 885c8e2..4afbd3d 100644 --- a/core/tests/Drupal/Tests/Core/Access/AccessManagerTest.php +++ b/core/tests/Drupal/Tests/Core/Access/AccessManagerTest.php @@ -8,6 +8,7 @@ namespace Drupal\Tests\Core\Access; use Drupal\Core\Access\AccessCheckInterface; +use Drupal\Core\Access\AccessCheckResult; use Drupal\Core\Access\AccessManagerInterface; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Access\AccessManager; @@ -194,29 +195,38 @@ public function testSetChecksWithDynamicAccessChecker() { public function testCheck() { $request = new Request(); - // Check check without any access checker defined yet. + // Check route access without any access checker defined yet. + $expected = new AccessCheckResult(TRUE); + $expected->value = AccessInterface::DENY; foreach ($this->routeCollection->all() as $route) { - $this->assertFalse($this->accessManager->check($route, $request, $this->account)); + $this->assertEquals($expected, $this->accessManager->check($route, $request, $this->account)); } $this->setupAccessChecker(); // An access checker got setup, but the routes haven't been setup using // setChecks. + $expected = new AccessCheckResult(TRUE); + $expected->value = AccessInterface::DENY; foreach ($this->routeCollection->all() as $route) { - $this->assertFalse($this->accessManager->check($route, $request, $this->account)); + $this->assertEquals($expected, $this->accessManager->check($route, $request, $this->account)); } + // Now applicable access checks have been saved on each route object. $this->accessManager->setChecks($this->routeCollection); - $this->argumentsResolver->expects($this->any()) + $this->argumentsResolver->expects($this->atLeastOnce()) ->method('getArguments') ->will($this->returnCallback(function ($callable, $route, $request, $account) { return array($route); })); - $this->assertFalse($this->accessManager->check($this->routeCollection->get('test_route_1'), $request, $this->account)); - $this->assertTrue($this->accessManager->check($this->routeCollection->get('test_route_2'), $request, $this->account)); - $this->assertFalse($this->accessManager->check($this->routeCollection->get('test_route_3'), $request, $this->account)); + $this->assertEquals($expected, $this->accessManager->check($this->routeCollection->get('test_route_1'), $request, $this->account)); + $expected->value = AccessInterface::ALLOW; + $this->assertEquals($expected, $this->accessManager->check($this->routeCollection->get('test_route_2'), $request, $this->account)); + $expected->value = AccessInterface::KILL; + $this->assertEquals($expected, $this->accessManager->check($this->routeCollection->get('test_route_3'), $request, $this->account)); + $expected->value = AccessInterface::ALLOW; + $this->assertEquals($expected, $this->accessManager->check($this->routeCollection->get('test_route_4'), $request, $this->account)); } /** @@ -228,132 +238,139 @@ public function testCheck() { * @see \Drupal\Tests\Core\Access\AccessManagerTest::testCheckConjunctions() */ public function providerTestCheckConjunctions() { + $access_allow = new AccessCheckResult(TRUE); + $access_allow->value = AccessCheckInterface::ALLOW; + $access_deny = new AccessCheckResult(TRUE); + $access_deny->value = AccessCheckInterface::DENY; + $access_kill = new AccessCheckResult(TRUE); + $access_kill->value = AccessCheckInterface::KILL; + $access_configurations = array(); $access_configurations[] = array( 'conjunction' => AccessManagerInterface::ACCESS_MODE_ALL, 'name' => 'test_route_4', 'condition_one' => AccessCheckInterface::ALLOW, 'condition_two' => AccessCheckInterface::KILL, - 'expected' => FALSE, + 'expected' => $access_kill, ); $access_configurations[] = array( 'conjunction' => NULL, 'name' => 'test_route_4', 'condition_one' => AccessCheckInterface::ALLOW, 'condition_two' => AccessCheckInterface::KILL, - 'expected' => FALSE, + 'expected' => $access_kill, ); $access_configurations[] = array( 'conjunction' => AccessManagerInterface::ACCESS_MODE_ALL, 'name' => 'test_route_5', 'condition_one' => AccessCheckInterface::ALLOW, 'condition_two' => AccessCheckInterface::DENY, - 'expected' => FALSE, + 'expected' => $access_deny, ); $access_configurations[] = array( 'conjunction' => NULL, 'name' => 'test_route_5', 'condition_one' => AccessCheckInterface::ALLOW, 'condition_two' => AccessCheckInterface::DENY, - 'expected' => FALSE, + 'expected' => $access_deny, ); $access_configurations[] = array( 'conjunction' => AccessManagerInterface::ACCESS_MODE_ALL, 'name' => 'test_route_6', 'condition_one' => AccessCheckInterface::KILL, 'condition_two' => AccessCheckInterface::DENY, - 'expected' => FALSE, + 'expected' => $access_kill, ); $access_configurations[] = array( 'conjunction' => NULL, 'name' => 'test_route_6', 'condition_one' => AccessCheckInterface::KILL, 'condition_two' => AccessCheckInterface::DENY, - 'expected' => FALSE, + 'expected' => $access_kill, ); $access_configurations[] = array( 'conjunction' => AccessManagerInterface::ACCESS_MODE_ALL, 'name' => 'test_route_7', 'condition_one' => AccessCheckInterface::ALLOW, 'condition_two' => AccessCheckInterface::ALLOW, - 'expected' => TRUE, + 'expected' => $access_allow, ); $access_configurations[] = array( 'conjunction' => NULL, 'name' => 'test_route_7', 'condition_one' => AccessCheckInterface::ALLOW, 'condition_two' => AccessCheckInterface::ALLOW, - 'expected' => TRUE, + 'expected' => $access_allow, ); $access_configurations[] = array( 'conjunction' => AccessManagerInterface::ACCESS_MODE_ALL, 'name' => 'test_route_8', 'condition_one' => AccessCheckInterface::KILL, 'condition_two' => AccessCheckInterface::KILL, - 'expected' => FALSE, + 'expected' => $access_kill, ); $access_configurations[] = array( 'conjunction' => NULL, 'name' => 'test_route_8', 'condition_one' => AccessCheckInterface::KILL, 'condition_two' => AccessCheckInterface::KILL, - 'expected' => FALSE, + 'expected' => $access_kill, ); $access_configurations[] = array( 'conjunction' => AccessManagerInterface::ACCESS_MODE_ALL, 'name' => 'test_route_9', 'condition_one' => AccessCheckInterface::DENY, 'condition_two' => AccessCheckInterface::DENY, - 'expected' => FALSE, + 'expected' => $access_deny, ); $access_configurations[] = array( 'conjunction' => NULL, 'name' => 'test_route_9', 'condition_one' => AccessCheckInterface::DENY, 'condition_two' => AccessCheckInterface::DENY, - 'expected' => FALSE, + 'expected' => $access_deny, ); $access_configurations[] = array( 'conjunction' => AccessManagerInterface::ACCESS_MODE_ANY, 'name' => 'test_route_10', 'condition_one' => AccessCheckInterface::ALLOW, 'condition_two' => AccessCheckInterface::KILL, - 'expected' => FALSE, + 'expected' => $access_kill, ); $access_configurations[] = array( 'conjunction' => AccessManagerInterface::ACCESS_MODE_ANY, 'name' => 'test_route_11', 'condition_one' => AccessCheckInterface::ALLOW, 'condition_two' => AccessCheckInterface::DENY, - 'expected' => TRUE, + 'expected' => $access_allow, ); $access_configurations[] = array( 'conjunction' => AccessManagerInterface::ACCESS_MODE_ANY, 'name' => 'test_route_12', 'condition_one' => AccessCheckInterface::KILL, 'condition_two' => AccessCheckInterface::DENY, - 'expected' => FALSE, + 'expected' => $access_kill, ); $access_configurations[] = array( 'conjunction' => AccessManagerInterface::ACCESS_MODE_ANY, 'name' => 'test_route_13', 'condition_one' => AccessCheckInterface::ALLOW, 'condition_two' => AccessCheckInterface::ALLOW, - 'expected' => TRUE, + 'expected' => $access_allow, ); $access_configurations[] = array( 'conjunction' => AccessManagerInterface::ACCESS_MODE_ANY, 'name' => 'test_route_14', 'condition_one' => AccessCheckInterface::KILL, 'condition_two' => AccessCheckInterface::KILL, - 'expected' => FALSE, + 'expected' => $access_kill, ); $access_configurations[] = array( 'conjunction' => AccessManagerInterface::ACCESS_MODE_ANY, 'name' => 'test_route_15', 'condition_one' => AccessCheckInterface::DENY, 'condition_two' => AccessCheckInterface::DENY, - 'expected' => FALSE, + 'expected' => $access_deny, ); return $access_configurations; @@ -388,7 +405,7 @@ public function testCheckConjunctions($conjunction, $name, $condition_one, $cond })); $this->accessManager->setChecks($route_collection); - $this->assertSame($this->accessManager->check($route, $request, $this->account), $expected_access); + $this->assertEquals($expected_access, $this->accessManager->check($route, $request, $this->account)); } /** @@ -407,14 +424,19 @@ public function testCheckNamedRoute() { // Tests the access with routes without parameters. $request = new Request(); - $this->assertTrue($this->accessManager->checkNamedRoute('test_route_2', array(), $this->account, $request)); - $this->assertFalse($this->accessManager->checkNamedRoute('test_route_3', array(), $this->account, $request)); + $expected = new AccessCheckResult(TRUE); + $expected->value = AccessInterface::ALLOW; + $this->assertEquals($expected, $this->accessManager->checkNamedRoute('test_route_2', array(), $this->account, $request)); + $expected->value = AccessInterface::KILL; + $this->assertEquals($expected, $this->accessManager->checkNamedRoute('test_route_3', array(), $this->account, $request)); // Tests the access with routes with parameters with given request. $request = new Request(); $request->attributes->set('value', 'example'); $request->attributes->set('value2', 'example2'); - $this->assertTrue($this->accessManager->checkNamedRoute('test_route_4', array(), $this->account, $request)); + $expected = new AccessCheckResult(TRUE); + $expected->value = AccessInterface::ALLOW; + $this->assertEquals($expected, $this->accessManager->checkNamedRoute('test_route_4', array(), $this->account, $request)); // Tests the access with routes without given request. $this->requestStack->push(new Request()); @@ -428,8 +450,8 @@ public function testCheckNamedRoute() { ->will($this->returnValue(array())); // Tests the access with routes with parameters without given request. - $this->assertTrue($this->accessManager->checkNamedRoute('test_route_2', array(), $this->account)); - $this->assertTrue($this->accessManager->checkNamedRoute('test_route_4', array('value' => 'example'), $this->account)); + $this->assertEquals($expected, $this->accessManager->checkNamedRoute('test_route_2', array(), $this->account)); + $this->assertEquals($expected, $this->accessManager->checkNamedRoute('test_route_4', array('value' => 'example'), $this->account)); } /** @@ -473,13 +495,16 @@ public function testCheckNamedRouteWithUpcastedValues() { $this->accessManager->setContainer($this->container); $this->requestStack->push(new Request()); + $no_access = new AccessCheckResult(TRUE); + $no_access->value = AccessInterface::KILL; + $access_check = $this->getMock('Drupal\Tests\Core\Access\TestAccessCheckInterface'); $access_check->expects($this->atLeastOnce()) ->method('applies') ->will($this->returnValue(TRUE)); $access_check->expects($this->atLeastOnce()) ->method('access') - ->will($this->returnValue(AccessInterface::KILL)); + ->will($this->returnValue($no_access)); $subrequest->attributes->set('value', 'upcasted_value'); $this->container->set('test_access', $access_check); @@ -487,7 +512,7 @@ public function testCheckNamedRouteWithUpcastedValues() { $this->accessManager->addCheckService('test_access', 'access'); $this->accessManager->setChecks($this->routeCollection); - $this->assertFalse($this->accessManager->checkNamedRoute('test_route_1', array('value' => 'example'), $this->account)); + $this->assertEquals($no_access, $this->accessManager->checkNamedRoute('test_route_1', array('value' => 'example'), $this->account)); } /** @@ -533,13 +558,16 @@ public function testCheckNamedRouteWithDefaultValue() { $this->accessManager->setContainer($this->container); $this->requestStack->push(new Request()); + $no_access = new AccessCheckResult(TRUE); + $no_access->value = AccessInterface::KILL; + $access_check = $this->getMock('Drupal\Tests\Core\Access\TestAccessCheckInterface'); $access_check->expects($this->atLeastOnce()) ->method('applies') ->will($this->returnValue(TRUE)); $access_check->expects($this->atLeastOnce()) ->method('access') - ->will($this->returnValue(AccessInterface::KILL)); + ->will($this->returnValue($no_access)); $subrequest->attributes->set('value', 'upcasted_value'); $this->container->set('test_access', $access_check); @@ -547,7 +575,7 @@ public function testCheckNamedRouteWithDefaultValue() { $this->accessManager->addCheckService('test_access', 'access'); $this->accessManager->setChecks($this->routeCollection); - $this->assertFalse($this->accessManager->checkNamedRoute('test_route_1', array(), $this->account)); + $this->assertEquals($no_access, $this->accessManager->checkNamedRoute('test_route_1', array(), $this->account)); } /** @@ -562,7 +590,10 @@ public function testCheckNamedRouteWithNonExistingRoute() { $this->setupAccessChecker(); - $this->assertFalse($this->accessManager->checkNamedRoute('test_route_1', array(), $this->account), 'A non existing route lead to access.'); + $expected = new AccessCheckResult(TRUE); + $expected->cacheability->setTags(array('extension' => TRUE)); + $expected->value = AccessInterface::KILL; + $this->assertEquals($expected, $this->accessManager->checkNamedRoute('test_route_1', array(), $this->account), 'A non existing route lead to access.'); } /** diff --git a/core/tests/Drupal/Tests/Core/Access/CsrfAccessCheckTest.php b/core/tests/Drupal/Tests/Core/Access/CsrfAccessCheckTest.php index 3501b87..e12da11 100644 --- a/core/tests/Drupal/Tests/Core/Access/CsrfAccessCheckTest.php +++ b/core/tests/Drupal/Tests/Core/Access/CsrfAccessCheckTest.php @@ -7,6 +7,7 @@ namespace Drupal\Tests\Core\Access; +use Drupal\Core\Access\AccessCheckResult; use Drupal\Core\Access\AccessManagerInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Routing\Route; @@ -66,7 +67,9 @@ public function testAccessTokenPass() { // Set the _controller_request flag so tokens are validated. $request->attributes->set('_controller_request', TRUE); - $this->assertSame(AccessInterface::ALLOW, $this->accessCheck->access($route, $request, $this->account)); + $expected = new AccessCheckResult(FALSE); + $expected->value = AccessInterface::ALLOW; + $this->assertEquals($expected, $this->accessCheck->access($route, $request, $this->account)); } /** @@ -84,7 +87,9 @@ public function testAccessTokenFail() { // Set the _controller_request flag so tokens are validated. $request->attributes->set('_controller_request', TRUE); - $this->assertSame(AccessInterface::KILL, $this->accessCheck->access($route, $request, $this->account)); + $expected = new AccessCheckResult(FALSE); + $expected->value = AccessInterface::KILL; + $this->assertEquals($expected, $this->accessCheck->access($route, $request, $this->account)); } /** @@ -103,7 +108,9 @@ public function testAccessTokenMissAny() { 'token' => 'test_query', )); - $this->assertSame(AccessInterface::DENY, $this->accessCheck->access($route, $request, $this->account)); + $expected = new AccessCheckResult(FALSE); + $expected->value = AccessInterface::DENY; + $this->assertEquals($expected, $this->accessCheck->access($route, $request, $this->account)); } /** @@ -122,7 +129,9 @@ public function testAccessTokenMissAll() { 'token' => 'test_query', )); - $this->assertSame(AccessInterface::ALLOW, $this->accessCheck->access($route, $request, $this->account)); + $expected = new AccessCheckResult(FALSE); + $expected->value = AccessInterface::ALLOW; + $this->assertEquals($expected, $this->accessCheck->access($route, $request, $this->account)); } } diff --git a/core/tests/Drupal/Tests/Core/Access/DefaultAccessCheckTest.php b/core/tests/Drupal/Tests/Core/Access/DefaultAccessCheckTest.php index 0af32a4..cc89cdd 100644 --- a/core/tests/Drupal/Tests/Core/Access/DefaultAccessCheckTest.php +++ b/core/tests/Drupal/Tests/Core/Access/DefaultAccessCheckTest.php @@ -7,6 +7,7 @@ namespace Drupal\Tests\Core\Access; +use Drupal\Core\Access\AccessCheckResult; use Drupal\Core\Access\AccessInterface; use Drupal\Core\Access\DefaultAccessCheck; use Drupal\Tests\UnitTestCase; @@ -50,13 +51,19 @@ public function testAccess() { $request = new Request(array()); $route = new Route('/test-route', array(), array('_access' => 'NULL')); - $this->assertSame(AccessInterface::DENY, $this->accessChecker->access($route, $request, $this->account)); + $expected = new AccessCheckResult(TRUE); + $expected->value = AccessInterface::DENY; + $this->assertEquals($expected, $this->accessChecker->access($route, $request, $this->account)); $route = new Route('/test-route', array(), array('_access' => 'FALSE')); - $this->assertSame(AccessInterface::KILL, $this->accessChecker->access($route, $request, $this->account)); + $expected = new AccessCheckResult(TRUE); + $expected->value = AccessInterface::KILL; + $this->assertEquals($expected, $this->accessChecker->access($route, $request, $this->account)); $route = new Route('/test-route', array(), array('_access' => 'TRUE')); - $this->assertSame(AccessInterface::ALLOW, $this->accessChecker->access($route, $request, $this->account)); + $expected = new AccessCheckResult(TRUE); + $expected->value = AccessInterface::ALLOW; + $this->assertEquals($expected, $this->accessChecker->access($route, $request, $this->account)); } } diff --git a/core/tests/Drupal/Tests/Core/Entity/ContentEntityBaseUnitTest.php b/core/tests/Drupal/Tests/Core/Entity/ContentEntityBaseUnitTest.php index 9bc1b85..ef671d2 100644 --- a/core/tests/Drupal/Tests/Core/Entity/ContentEntityBaseUnitTest.php +++ b/core/tests/Drupal/Tests/Core/Entity/ContentEntityBaseUnitTest.php @@ -7,6 +7,8 @@ namespace Drupal\Tests\Core\Entity; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\DependencyInjection\ContainerBuilder; use Drupal\Core\Field\FieldDefinition; use Drupal\Core\Field\FieldItemBase; @@ -16,6 +18,7 @@ /** * @coversDefaultClass \Drupal\Core\Entity\ContentEntityBase * @group Entity + * @group Access */ class ContentEntityBaseUnitTest extends UnitTestCase { @@ -370,18 +373,20 @@ public function testBundle() { public function testAccess() { $access = $this->getMock('\Drupal\Core\Entity\EntityAccessControllerInterface'); $operation = $this->randomName(); + $access_result = new AccessCheckResult(TRUE); + $access_result->value = AccessInterface::ALLOW; $access->expects($this->at(0)) ->method('access') ->with($this->entity, $operation) - ->will($this->returnValue(TRUE)); + ->will($this->returnValue($access_result)); $access->expects($this->at(1)) ->method('createAccess') - ->will($this->returnValue(TRUE)); + ->will($this->returnValue($access_result)); $this->entityManager->expects($this->exactly(2)) ->method('getAccessController') ->will($this->returnValue($access)); - $this->assertTrue($this->entity->access($operation)); - $this->assertTrue($this->entity->access('create')); + $this->assertEquals($access_result, $this->entity->access($operation)); + $this->assertEquals($access_result, $this->entity->access('create')); } /** diff --git a/core/tests/Drupal/Tests/Core/Entity/EntityAccessCheckTest.php b/core/tests/Drupal/Tests/Core/Entity/EntityAccessCheckTest.php index ba5ffcf..384a919 100644 --- a/core/tests/Drupal/Tests/Core/Entity/EntityAccessCheckTest.php +++ b/core/tests/Drupal/Tests/Core/Entity/EntityAccessCheckTest.php @@ -10,12 +10,14 @@ use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Routing\Route; use Drupal\Core\Access\AccessCheckInterface; +use Drupal\Core\Access\AccessCheckResult; use Drupal\Core\Entity\EntityAccessCheck; use Drupal\Tests\UnitTestCase; /** * Unit test of entity access checking system. * + * @group Access * @group Entity */ class EntityAccessCheckTest extends UnitTestCase { @@ -29,14 +31,23 @@ public function testAccess() { $node = $this->getMockBuilder('Drupal\node\Entity\Node') ->disableOriginalConstructor() ->getMock(); + + $result = new AccessCheckResult(TRUE); + $result->value = AccessCheckInterface::ALLOW; + $result->cacheability->setContexts(array('cache_context.user.roles')); $node->expects($this->any()) ->method('access') - ->will($this->returnValue(TRUE)); + ->will($this->returnValue($result)); + $access_check = new EntityAccessCheck(); $request->attributes->set('node', $node); $account = $this->getMock('Drupal\Core\Session\AccountInterface'); $access = $access_check->access($route, $request, $account); - $this->assertSame(AccessCheckInterface::ALLOW, $access); + + $expected = new AccessCheckResult(TRUE); + $expected->value = AccessCheckInterface::ALLOW; + $expected->cacheability->setContexts(array('cache_context.user.roles')); + $this->assertEquals($expected, $access); } } diff --git a/core/tests/Drupal/Tests/Core/Entity/EntityCreateAccessCheckTest.php b/core/tests/Drupal/Tests/Core/Entity/EntityCreateAccessCheckTest.php index e195e4c..5986ae0 100644 --- a/core/tests/Drupal/Tests/Core/Entity/EntityCreateAccessCheckTest.php +++ b/core/tests/Drupal/Tests/Core/Entity/EntityCreateAccessCheckTest.php @@ -8,6 +8,7 @@ namespace Drupal\Tests\Core\Entity; use Drupal\Core\Access\AccessCheckInterface; +use Drupal\Core\Access\AccessCheckResult; use Drupal\Core\Entity\EntityCreateAccessCheck; use Drupal\Tests\UnitTestCase; use Symfony\Component\DependencyInjection\ParameterBag\ParameterBag; @@ -15,6 +16,7 @@ /** * @coversDefaultClass \Drupal\Core\Entity\EntityCreateAccessCheck + * @group Access * @group Entity */ class EntityCreateAccessCheckTest extends UnitTestCase { @@ -39,17 +41,28 @@ protected function setUp() { * @return array */ public function providerTestAccess() { + $no_access = new AccessCheckResult(TRUE); + $no_access->cacheability->setContexts(array('cache_context.user.roles')); + $no_access->value = AccessCheckInterface::DENY; + + $access = new AccessCheckResult(TRUE); + $access->cacheability->setContexts(array('cache_context.user.roles')); + $access->value = AccessCheckInterface::ALLOW; + + $no_access_due_to_errors = new AccessCheckResult(FALSE); + $no_access_due_to_errors->value = AccessCheckInterface::DENY; + return array( - array('', 'entity_test', FALSE, AccessCheckInterface::DENY), - array('', 'entity_test',TRUE, AccessCheckInterface::ALLOW), - array('test_entity', 'entity_test:test_entity', TRUE, AccessCheckInterface::ALLOW), - array('test_entity', 'entity_test:test_entity', FALSE, AccessCheckInterface::DENY), - array('test_entity', 'entity_test:{bundle_argument}', TRUE, AccessCheckInterface::ALLOW), - array('test_entity', 'entity_test:{bundle_argument}', FALSE, AccessCheckInterface::DENY), - array('', 'entity_test:{bundle_argument}', FALSE, AccessCheckInterface::DENY), + array('', 'entity_test', $no_access, $no_access), + array('', 'entity_test', $access, $access), + array('test_entity', 'entity_test:test_entity', $access, $access), + array('test_entity', 'entity_test:test_entity', $no_access, $no_access), + array('test_entity', 'entity_test:{bundle_argument}', $access, $access), + array('test_entity', 'entity_test:{bundle_argument}', $no_access, $no_access), + array('', 'entity_test:{bundle_argument}', $no_access, $no_access_due_to_errors), // When the bundle is not provided, access should be denied even if the // access controller would allow access. - array('', 'entity_test:{bundle_argument}', TRUE, AccessCheckInterface::DENY), + array('', 'entity_test:{bundle_argument}', $access, $no_access_due_to_errors), ); } diff --git a/core/tests/Drupal/Tests/Core/Entity/EntityListBuilderTest.php b/core/tests/Drupal/Tests/Core/Entity/EntityListBuilderTest.php index a7e0792..7cb6bee 100644 --- a/core/tests/Drupal/Tests/Core/Entity/EntityListBuilderTest.php +++ b/core/tests/Drupal/Tests/Core/Entity/EntityListBuilderTest.php @@ -7,6 +7,8 @@ namespace Drupal\Tests\Core\Entity; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\DependencyInjection\ContainerBuilder; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Entity\EntityListBuilder; @@ -104,9 +106,11 @@ public function testGetOperations() { $this->container->set('module_handler', $this->moduleHandler); + $access = new AccessCheckResult(TRUE); + $access->value = AccessInterface::ALLOW; $this->role->expects($this->any()) ->method('access') - ->will($this->returnValue(TRUE)); + ->will($this->returnValue($access)); $this->role->expects($this->any()) ->method('hasLinkTemplate') ->will($this->returnValue(TRUE)); diff --git a/core/tests/Drupal/Tests/Core/Entity/EntityUnitTest.php b/core/tests/Drupal/Tests/Core/Entity/EntityUnitTest.php index 8b63f61..bd2f21e 100644 --- a/core/tests/Drupal/Tests/Core/Entity/EntityUnitTest.php +++ b/core/tests/Drupal/Tests/Core/Entity/EntityUnitTest.php @@ -7,6 +7,8 @@ namespace Drupal\Tests\Core\Entity; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\DependencyInjection\ContainerBuilder; use Drupal\Core\Entity\Entity; use Drupal\Core\Language\Language; @@ -17,6 +19,7 @@ /** * @coversDefaultClass \Drupal\Core\Entity\Entity * @group Entity + * @group Access */ class EntityUnitTest extends UnitTestCase { @@ -209,18 +212,20 @@ public function testLabel() { public function testAccess() { $access = $this->getMock('\Drupal\Core\Entity\EntityAccessControllerInterface'); $operation = $this->randomName(); + $access_result = new AccessCheckResult(TRUE); + $access_result->value = AccessInterface::ALLOW; $access->expects($this->at(0)) ->method('access') ->with($this->entity, $operation) - ->will($this->returnValue(TRUE)); + ->will($this->returnValue($access_result)); $access->expects($this->at(1)) ->method('createAccess') - ->will($this->returnValue(TRUE)); + ->will($this->returnValue($access_result)); $this->entityManager->expects($this->exactly(2)) ->method('getAccessController') ->will($this->returnValue($access)); - $this->assertTrue($this->entity->access($operation)); - $this->assertTrue($this->entity->access('create')); + $this->assertEquals($access_result, $this->entity->access($operation)); + $this->assertEquals($access_result, $this->entity->access('create')); } /** diff --git a/core/tests/Drupal/Tests/Core/EventSubscriber/AccessSubscriberTest.php b/core/tests/Drupal/Tests/Core/EventSubscriber/AccessSubscriberTest.php index 2d23bc4..f94f1a3 100644 --- a/core/tests/Drupal/Tests/Core/EventSubscriber/AccessSubscriberTest.php +++ b/core/tests/Drupal/Tests/Core/EventSubscriber/AccessSubscriberTest.php @@ -7,6 +7,8 @@ namespace Drupal\Tests\Core\EventSubscriber; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\EventSubscriber\AccessSubscriber; use Drupal\Core\Session\AccountInterface; use Drupal\Tests\UnitTestCase; @@ -20,6 +22,7 @@ /** * @coversDefaultClass \Drupal\Core\EventSubscriber\AccessSubscriber + * @group Access * @group EventSubscriber */ class AccessSubscriberTest extends UnitTestCase { @@ -93,6 +96,8 @@ public function setUp() { * @expectedException \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException */ public function testAccessSubscriberThrowsAccessDeniedException() { + $no_access = new AccessCheckResult(TRUE); + $no_access->value = AccessInterface::DENY; $this->parameterBag->expects($this->any()) ->method('has') @@ -107,7 +112,7 @@ public function testAccessSubscriberThrowsAccessDeniedException() { $this->accessManager->expects($this->any()) ->method('check') ->with($this->anything()) - ->will($this->returnValue(FALSE)); + ->will($this->returnValue($no_access)); $subscriber = new AccessSubscriber($this->accessManager, $this->currentUser); $subscriber->onKernelRequestAccessCheck($this->event); @@ -142,10 +147,12 @@ public function testAccessSubscriberDoesNotAlterRequestIfAccessManagerGrantsAcce ->with(RouteObjectInterface::ROUTE_OBJECT) ->will($this->returnValue($this->route)); + $access = new AccessCheckResult(TRUE); + $access->value = AccessInterface::ALLOW; $this->accessManager->expects($this->once()) ->method('check') ->with($this->equalTo($this->route)) - ->will($this->returnValue(TRUE)); + ->will($this->returnValue($access)); $subscriber = new AccessSubscriber($this->accessManager, $this->currentUser); // We're testing that no exception is thrown in this case. There is no diff --git a/core/tests/Drupal/Tests/Core/Menu/ContextualLinkManagerTest.php b/core/tests/Drupal/Tests/Core/Menu/ContextualLinkManagerTest.php index edd14ec..6c41005 100644 --- a/core/tests/Drupal/Tests/Core/Menu/ContextualLinkManagerTest.php +++ b/core/tests/Drupal/Tests/Core/Menu/ContextualLinkManagerTest.php @@ -7,6 +7,8 @@ namespace Drupal\Tests\Core\Menu; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Language\Language; use Drupal\Tests\UnitTestCase; use Symfony\Component\HttpFoundation\RequestStack; @@ -268,9 +270,11 @@ public function testGetContextualLinksArrayByGroup() { ->method('getDefinitions') ->will($this->returnValue($definitions)); + $access = new AccessCheckResult(TRUE); + $access->value = AccessInterface::ALLOW; $this->accessManager->expects($this->any()) ->method('checkNamedRoute') - ->will($this->returnValue(TRUE)); + ->will($this->returnValue($access)); // Set up mocking of the plugin factory. $map = array(); @@ -339,11 +343,15 @@ public function testGetContextualLinksArrayByGroupAccessCheck() { ->method('getDefinitions') ->will($this->returnValue($definitions)); + $access = new AccessCheckResult(TRUE); + $access->value = AccessInterface::ALLOW; + $no_access = new AccessCheckResult(TRUE); + $no_access->value = AccessInterface::DENY; $this->accessManager->expects($this->any()) ->method('checkNamedRoute') ->will($this->returnValueMap(array( - array('test_route', array('key' => 'value'), $this->account, NULL, TRUE), - array('test_route2', array('key' => 'value'), $this->account, NULL, FALSE), + array('test_route', array('key' => 'value'), $this->account, NULL, $access), + array('test_route2', array('key' => 'value'), $this->account, NULL, $no_access), ))); // Set up mocking of the plugin factory. diff --git a/core/tests/Drupal/Tests/Core/Menu/DefaultMenuLinkTreeManipulatorsTest.php b/core/tests/Drupal/Tests/Core/Menu/DefaultMenuLinkTreeManipulatorsTest.php index 4309a4e..530e6f3 100644 --- a/core/tests/Drupal/Tests/Core/Menu/DefaultMenuLinkTreeManipulatorsTest.php +++ b/core/tests/Drupal/Tests/Core/Menu/DefaultMenuLinkTreeManipulatorsTest.php @@ -7,6 +7,8 @@ namespace Drupal\Tests\Core\Menu; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Menu\DefaultMenuLinkTreeManipulators; use Drupal\Core\Menu\MenuLinkTreeElement; use Drupal\Tests\UnitTestCase; @@ -140,13 +142,17 @@ public function testCheckAccess() { // performed. 8 routes, but 1 is external, 2 already have their 'access' // property set, and 1 is a child if an inaccessible menu link, so only 4 // calls will be made. + $access = new AccessCheckResult(TRUE); + $access->value = AccessInterface::ALLOW; + $no_access = new AccessCheckResult(TRUE); + $no_access->value = AccessInterface::DENY; $this->accessManager->expects($this->exactly(4)) ->method('checkNamedRoute') ->will($this->returnValueMap(array( - array('example1', array(), $this->currentUser, NULL, FALSE), - array('example2', array('foo' => 'bar'), $this->currentUser, NULL, TRUE), - array('example3', array('baz' => 'qux'), $this->currentUser, NULL, FALSE), - array('example5', array(), $this->currentUser, NULL, TRUE), + array('example1', array(), $this->currentUser, NULL, $no_access), + array('example2', array('foo' => 'bar'), $this->currentUser, NULL, $access), + array('example3', array('baz' => 'qux'), $this->currentUser, NULL, $no_access), + array('example5', array(), $this->currentUser, NULL, $access), ))); $this->mockTree(); diff --git a/core/tests/Drupal/Tests/Core/Menu/LocalActionManagerTest.php b/core/tests/Drupal/Tests/Core/Menu/LocalActionManagerTest.php index f92711a..381f1cd 100644 --- a/core/tests/Drupal/Tests/Core/Menu/LocalActionManagerTest.php +++ b/core/tests/Drupal/Tests/Core/Menu/LocalActionManagerTest.php @@ -9,6 +9,8 @@ use Drupal\Component\Plugin\Discovery\DiscoveryInterface; use Drupal\Component\Plugin\Factory\FactoryInterface; +use Drupal\Core\Access\AccessCheckResult; +use Drupal\Core\Access\AccessInterface; use Drupal\Core\Access\AccessManagerInterface; use Drupal\Core\Cache\CacheBackendInterface; use Drupal\Core\Extension\ModuleHandlerInterface; @@ -108,6 +110,11 @@ protected function setUp() { $this->cacheBackend = $this->getMock('Drupal\Core\Cache\CacheBackendInterface'); $this->accessManager = $this->getMock('Drupal\Core\Access\AccessManagerInterface'); + $access_no_opinion = new AccessCheckResult(TRUE); + $access_no_opinion->value = AccessInterface::DENY; + $this->accessManager->expects($this->any()) + ->method('checkNamedRoute') + ->will($this->returnValue($access_no_opinion)); $this->account = $this->getMock('Drupal\Core\Session\AccountInterface'); $this->discovery = $this->getMock('Drupal\Component\Plugin\Discovery\DiscoveryInterface'); $this->factory = $this->getMock('Drupal\Component\Plugin\Factory\FactoryInterface'); diff --git a/core/tests/Drupal/Tests/Core/Route/RoleAccessCheckTest.php b/core/tests/Drupal/Tests/Core/Route/RoleAccessCheckTest.php index 232645b..9f407cc 100644 --- a/core/tests/Drupal/Tests/Core/Route/RoleAccessCheckTest.php +++ b/core/tests/Drupal/Tests/Core/Route/RoleAccessCheckTest.php @@ -8,6 +8,7 @@ namespace Drupal\Tests\Core\Route; use Drupal\Core\Access\AccessCheckInterface; +use Drupal\Core\Access\AccessCheckResult; use Drupal\Core\Session\UserSession; use Drupal\Tests\UnitTestCase; use Drupal\user\Access\RoleAccessCheck; @@ -16,6 +17,7 @@ /** * @coversDefaultClass \Drupal\user\Access\RoleAccessCheck + * @group Access * @group Route */ class RoleAccessCheckTest extends UnitTestCase { @@ -145,16 +147,21 @@ public function testRoleAccess($path, $grant_accounts, $deny_accounts) { $role_access_check = new RoleAccessCheck(); $collection = $this->getTestRouteCollection(); + $expected = new AccessCheckResult(TRUE); + $expected->cacheability->setContexts(array('cache_context.user.roles')); + foreach ($grant_accounts as $account) { $message = sprintf('Access granted for user with the roles %s on path: %s', implode(', ', $account->getRoles()), $path); - $this->assertSame(AccessCheckInterface::ALLOW, $role_access_check->access($collection->get($path), $account), $message); + $expected->value = AccessCheckInterface::ALLOW; + $this->assertEquals($expected, $role_access_check->access($collection->get($path), $account), $message); } // Check all users which don't have access. foreach ($deny_accounts as $account) { $message = sprintf('Access denied for user %s with the roles %s on path: %s', $account->id(), implode(', ', $account->getRoles()), $path); $has_access = $role_access_check->access($collection->get($path), $account); - $this->assertSame(AccessCheckInterface::DENY, $has_access , $message); + $expected->value = AccessCheckInterface::DENY; + $this->assertEquals($expected, $has_access, $message); } }