diff --git a/lib/Drupal/Core/Access/AccessInterface.php b/lib/Drupal/Core/Access/AccessInterface.php deleted file mode 100644 index faa798cd522..00000000000 --- a/lib/Drupal/Core/Access/AccessInterface.php +++ /dev/null @@ -1,38 +0,0 @@ -routeProvider->getRouteByName($route_name, $parameters); if (empty($route_request)) { @@ -207,20 +208,25 @@ public function checkNamedRoute($route_name, array $parameters = array(), Accoun $parameters[RouteObjectInterface::ROUTE_OBJECT] = $route; $route_request->attributes->add($this->paramConverterManager->convert($parameters)); } - return $this->check($route, $route_request, $account); + return $this->check($route, $route_request, $account, $return_as_object); } catch (RouteNotFoundException $e) { - return FALSE; + // Cacheable until extensions change. + $result = AccessResult::forbidden()->addCacheTags(array('extension' => TRUE)); + return $return_as_object ? $result : $result->isAllowed(); } catch (ParamNotConvertedException $e) { - return FALSE; + // Uncacheable because conversion of the parameter may not have been + // possible due to dynamic circumstances. + $result = AccessResult::forbidden()->setCacheable(FALSE); + return $return_as_object ? $result : $result->isAllowed(); } } /** * {@inheritdoc} */ - public function check(Route $route, Request $request, AccountInterface $account = NULL) { + public function check(Route $route, Request $request, AccountInterface $account = NULL, $return_as_object = FALSE) { if (!isset($account)) { $account = $this->currentUser; } @@ -228,11 +234,12 @@ public function check(Route $route, Request $request, AccountInterface $account $conjunction = $route->getOption('_access_mode') ?: static::ACCESS_MODE_ALL; if ($conjunction == static::ACCESS_MODE_ALL) { - return $this->checkAll($checks, $route, $request, $account); + $result = $this->checkAll($checks, $route, $request, $account); } else { - return $this->checkAny($checks, $route, $request, $account); + $result = $this->checkAny($checks, $route, $request, $account); } + return $return_as_object ? $result : $result->isAllowed(); } /** @@ -247,30 +254,39 @@ 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\AccessResultInterface + * The access result. + * + * @see \Drupal\Core\Access\AccessResultInterface::andIf() */ 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 non-allowed check is encountered. + if (!$result->isAllowed()) { break; } } - return $access; + if (empty($results)) { + // No opinion. + return AccessResult::create(); + } + else { + /** @var \Drupal\Core\Access\AccessResultInterface $result */ + $result = array_shift($results); + foreach ($results as $other) { + $result->andIf($other); + } + return $result; + } } /** @@ -285,29 +301,23 @@ 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\AccessResultInterface + * The access result. + * + * @see \Drupal\Core\Access\AccessResultInterface::orIf() */ protected function checkAny(array $checks, $route, $request, AccountInterface $account) { - // No checks == deny by default. - $access = FALSE; + // No opinion by default. + $result = AccessResult::create(); 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 = $result->orIf($this->performCheck($service_id, $route, $request, $account)); } - return $access; + return $result; } /** @@ -325,16 +335,17 @@ 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\AccessResultInterface + * The access result. */ 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\AccessResultInterface $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 AccessResultInterface) { + throw new AccessException("Access error in $service_id. Access services must return an object that implements AccessResultInterface."); } return $service_access; diff --git a/lib/Drupal/Core/Access/AccessManagerInterface.php b/lib/Drupal/Core/Access/AccessManagerInterface.php index db6cfa34150..9f74dd34af4 100644 --- a/lib/Drupal/Core/Access/AccessManagerInterface.php +++ b/lib/Drupal/Core/Access/AccessManagerInterface.php @@ -18,20 +18,17 @@ interface AccessManagerInterface { /** - * All access checkers have to return AccessInterface::ALLOW. + * All access checkers must return an AccessResultInterface object where + * ::isAllowed() is TRUE. * * self::ACCESS_MODE_ALL is the default behavior. - * - * @see \Drupal\Core\Access\AccessInterface::ALLOW */ const ACCESS_MODE_ALL = 'ALL'; /** - * At least one access checker has to return AccessInterface::ALLOW - * and none should return AccessInterface::KILL. - * - * @see \Drupal\Core\Access\AccessInterface::ALLOW - * @see \Drupal\Core\Access\AccessInterface::KILL + * At least one access checker must return an AccessResultInterface object + * where ::isAllowed() is TRUE and none may return one where ::isForbidden() + * is TRUE. */ const ACCESS_MODE_ANY = 'ANY'; @@ -43,18 +40,24 @@ interface AccessManagerInterface { * @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 * (optional) Run access checks for this account. Defaults to 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. + * @param bool $return_as_object + * (optional) Defaults to FALSE. * - * @return bool - * Returns TRUE if the user has access to the route, otherwise FALSE. + * @return bool|\Drupal\Core\Access\AccessResultInterface + * The access result. Returns a boolean if $return_as_object is FALSE (this + * is the default) and otherwise an AccessResultInterface object. + * When a boolean is returned, the result of AccessInterface::isAllowed() is + * returned, i.e. TRUE means access is explicitly allowed, FALSE means + * access is either explicitly forbidden or "no opinion". */ - public function checkNamedRoute($route_name, array $parameters = array(), AccountInterface $account = NULL, Request $route_request = NULL); + public function checkNamedRoute($route_name, array $parameters = array(), AccountInterface $account = NULL, Request $route_request = NULL, $return_as_object = FALSE); /** * For each route, saves a list of applicable access checks to the route. @@ -89,10 +92,16 @@ public function addCheckService($service_id, $service_method, array $applies_che * @param \Drupal\Core\Session\AccountInterface $account * (optional) Run access checks for this account. Defaults to the current * user. + * @param bool $return_as_object + * (optional) Defaults to FALSE. * - * @return bool - * Returns TRUE if the user has access to the route, otherwise FALSE. + * @return bool|\Drupal\Core\Access\AccessResultInterface + * The access result. Returns a boolean if $return_as_object is FALSE (this + * is the default) and otherwise an AccessResultInterface object. + * When a boolean is returned, the result of AccessInterface::isAllowed() is + * returned, i.e. TRUE means access is explicitly allowed, FALSE means + * access is either explicitly forbidden or "no opinion". */ - public function check(Route $route, Request $request, AccountInterface $account = NULL); + public function check(Route $route, Request $request, AccountInterface $account = NULL, $return_as_object = FALSE); } diff --git a/lib/Drupal/Core/Access/AccessResult.php b/lib/Drupal/Core/Access/AccessResult.php new file mode 100644 index 00000000000..c87b80ff478 --- /dev/null +++ b/lib/Drupal/Core/Access/AccessResult.php @@ -0,0 +1,486 @@ +resetAccess(); + $this->setCacheable(TRUE) + ->resetCacheContexts() + ->resetCacheTags() + // Typically, cache items are invalidated via associated cache tags, not + // via a maximum age. + ->setCacheMaxAge(Cache::PERMANENT); + } + + /** + * Instantiates a new AccessResult object. + * + * This factory method exists to improve DX; it allows developers to fluently + * create access results. + * + * Defaults to a cacheable access result that neither explicitly allows nor + * explicitly forbids access. + * + * @return \Drupal\Core\Access\AccessResult + */ + public static function create() { + return new static(); + } + + /** + * Convenience method, creates an AccessResult object and calls allow(). + * + * @return \Drupal\Core\Access\AccessResult + */ + public static function allowed() { + return static::create()->allow(); + } + + /** + * Convenience method, creates an AccessResult object and calls forbid(). + * + * @return \Drupal\Core\Access\AccessResult + */ + public static function forbidden() { + return static::create()->forbid(); + } + + /** + * Convenience method, creates an AccessResult object and calls allowIf(). + * + * @param bool $condition + * The condition to evaluate. If TRUE, ::allow() will be called. + * + * @return \Drupal\Core\Access\AccessResult + */ + public static function allowedIf($condition) { + return static::create()->allowIf($condition); + } + + /** + * Convenience method, creates an AccessResult object and calls forbiddenIf(). + * + * @param bool $condition + * The condition to evaluate. If TRUE, ::forbid() will be called. + * + * @return \Drupal\Core\Access\AccessResult + */ + public static function forbiddenIf($condition) { + return static::create()->forbidIf($condition); + } + + /** + * Convenience method, creates an AccessResult object and calls allowIfHasPermission(). + * + * @param \Drupal\Core\Session\AccountInterface $account + * The account for which to check a permission. + * @param string $permission + * The permission to check for. + * + * @return \Drupal\Core\Access\AccessResult + */ + public static function allowedIfHasPermission(AccountInterface $account, $permission) { + return static::create()->allowIfHasPermission($account, $permission); + } + + /** + * {@inheritdoc} + */ + public function isAllowed() { + return $this->value === static::ALLOW; + } + + /** + * {@inheritdoc} + */ + public function isForbidden() { + return $this->value === static::KILL; + } + + /** + * Explicitly allows access. + * + * @return $this + */ + public function allow() { + $this->value = static::ALLOW; + return $this; + } + + /** + * Explicitly forbids access. + * + * @return $this + */ + public function forbid() { + $this->value = static::KILL; + return $this; + } + + /** + * Neither explicitly allows nor explicitly forbids access. + * + * @return $this + */ + public function resetAccess() { + $this->value = static::DENY; + return $this; + } + + /** + * Conditionally calls ::allow(). + * + * @param bool $condition + * The condition to evaluate. If TRUE, ::allow() will be called. + * + * @return $this + */ + public function allowIf($condition) { + if ($condition) { + $this->allow(); + } + return $this; + } + + /** + * Conditionally calls ::forbid(). + * + * @param bool $condition + * The condition to evaluate. If TRUE, ::forbid() will be called. + * + * @return $this + */ + public function forbidIf($condition) { + if ($condition) { + $this->forbid(); + } + return $this; + } + + /** + * {@inheritdoc} + * + * AccessResult objects solely return cache context tokens, no static strings. + */ + public function getCacheKeys() { + sort($this->contexts); + return $this->contexts; + } + + /** + * {@inheritdoc} + */ + public function getCacheTags() { + return $this->tags; + } + + /** + * {@inheritdoc} + * + * It's not very useful to cache individual access results, but the interface + * forces us to implement this method, so just use the default cache bin. + */ + public function getCacheBin() { + return 'default'; + } + + /** + * {@inheritdoc} + */ + public function getCacheMaxAge() { + return $this->maxAge; + } + + /** + * {@inheritdoc} + */ + public function isCacheable() { + return $this->isCacheable; + } + + /** + * Sets whether this access result is cacheable. It is cacheable by default. + * + * @param bool $is_cacheable + * Whether this access result is cacheable. + * + * @return $this + */ + public function setCacheable($is_cacheable) { + $this->isCacheable = $is_cacheable; + return $this; + } + + /** + * Adds cache contexts associated with the access result. + * + * @param string[] $contexts + * An array of cache context IDs, used to generate a cache ID. + * + * @return $this + */ + public function addCacheContexts(array $contexts) { + $this->contexts = array_unique(array_merge($this->contexts, $contexts)); + return $this; + } + + /** + * Resets cache contexts (to the empty array). + * + * @return $this + */ + public function resetCacheContexts() { + $this->contexts = array(); + return $this; + } + + /** + * Adds cache tags associated with the access result. + * + * @param array $tags + * An array of cache tags. + * + * @return $this + */ + public function addCacheTags(array $tags) { + foreach ($tags as $namespace => $values) { + if (is_array($values)) { + foreach ($values as $value) { + $this->tags[$namespace][$value] = $value; + } + ksort($this->tags[$namespace]); + } + else { + if (!isset($this->tags[$namespace])) { + $this->tags[$namespace] = $values; + } + } + } + ksort($this->tags); + return $this; + } + + /** + * Resets cache tags (to the empty array). + * + * @return $this + */ + public function resetCacheTags() { + $this->tags = array(); + return $this; + } + + /** + * Sets the maximum age for which this access result may be cached. + * + * @param int $max_age + * The maximum time in seconds that this access result may be cached. + * + * @return $this + */ + public function setCacheMaxAge($max_age) { + $this->maxAge = $max_age; + return $this; + } + + /** + * Convenience method, adds the "cache_context.user.roles" cache context. + * + * @return $this + */ + public function cachePerRole() { + $this->addCacheContexts(array('cache_context.user.roles')); + return $this; + } + + /** + * Convenience method, adds the "cache_context.user" cache context. + * + * @return $this + */ + public function cachePerUser() { + $this->addCacheContexts(array('cache_context.user')); + return $this; + } + + /** + * Convenience method, adds the entity's cache tag. + * + * @param \Drupal\Core\Entity\EntityInterface $entity + * The entity whose cache tag to set on the access result. + * + * @return $this + */ + public function cacheUntilEntityChanges(EntityInterface $entity) { + $this->addCacheTags($entity->getCacheTag()); + return $this; + } + + /** + * Convenience method, checks permission and calls ::cachePerRole(). + * + * @param \Drupal\Core\Session\AccountInterface $account + * The account for which to check a permission. + * @param string $permission + * The permission to check for. + * + * @return $this + */ + public function allowIfHasPermission(AccountInterface $account, $permission) { + $this->allowIf($account->hasPermission($permission))->cachePerRole(); + return $this; + } + + /** + * {@inheritdoc} + */ + public function orIf(AccessResultInterface $other) { + // If this AccessResult already is forbidden, then that already is the + // conclusion. We can completely disregard $other. + if ($this->isForbidden()) { + return $this; + } + // Otherwise, we make this AccessResult forbidden if the other is, or + // allowed if the other is, and we merge in the cacheability metadata if the + // other access result also has cacheability metadata. + else { + if ($other->isForbidden()) { + $this->forbid(); + } + else if ($other->isAllowed()) { + $this->allow(); + } + $this->mergeCacheabilityMetadata($other); + return $this; + } + } + + /** + * {@inheritdoc} + */ + public function andIf(AccessResultInterface $other) { + // If this AccessResult already is forbidden or is merely not explicitly + // allowed, then that already is the conclusion. We can completely disregard + // $other. + if ($this->isForbidden() || !$this->isAllowed()) { + return $this; + } + // Otherwise, we make this AccessResult forbidden if the other is, or not + // explicitly allowed if the other isn't, and we merge in the cacheability + // metadata if the other access result also has cacheability metadata. + else { + if ($other->isForbidden()) { + $this->forbid(); + } + else if (!$other->isAllowed()) { + $this->resetAccess(); + } + $this->mergeCacheabilityMetadata($other); + return $this; + } + } + + /** + * Merges the cacheability metadata of the other access result, if any. + * + * @param \Drupal\Core\Access\AccessResultInterface $other + * The other access result, whose cacheability data (if any) to merge. + */ + protected function mergeCacheabilityMetadata(AccessResultInterface $other) { + if ($other instanceof CacheableInterface) { + $this->setCacheable($other->isCacheable()); + $this->addCacheContexts($other->getCacheKeys()); + $this->addCacheTags($other->getCacheTags()); + // Use the lowest max-age. + if ($this->getCacheMaxAge() === Cache::PERMANENT) { + // The other max-age is either lower or equal. + $this->setCacheMaxAge($other->getCacheMaxAge()); + } + else { + $this->setCacheMaxAge(min($this->getCacheMaxAge(), $other->getCacheMaxAge())); + } + } + // If any of the access results don't provide cacheability metadata, then + // we cannot cache the combined access result, for we may not make + // assumptions. + else { + $this->setCacheable(FALSE); + } + } + +} diff --git a/lib/Drupal/Core/Access/AccessResultInterface.php b/lib/Drupal/Core/Access/AccessResultInterface.php new file mode 100644 index 00000000000..5e6f5bc256d --- /dev/null +++ b/lib/Drupal/Core/Access/AccessResultInterface.php @@ -0,0 +1,79 @@ +isAllowed() && !$access->isForbidden(); + * @endcode + */ +interface AccessResultInterface { + + /** + * Checks whether this access result indicates access is explicitly allowed. + * + * @return bool + */ + public function isAllowed(); + + /** + * Checks whether this access result indicates access is explicitly forbidden. + * + * @return bool + */ + public function isForbidden(); + + /** + * Combine this access result with another using OR. + * + * When OR-ing two access results, the result is: + * - isForbidden() in either ⇒ isForbidden() + * - isAllowed() in either ⇒ isAllowed() + * - neither isForbidden() nor isAllowed() => !isAllowed() && !isForbidden() + * + * @param \Drupal\Core\Access\AccessResultInterface $other + * The other access result to OR this one with. + * + * @return $this + */ + public function orIf(AccessResultInterface $other); + + /** + * Combine this access result with another using AND. + * + * When OR-ing two access results, the result is: + * - isForbidden() in either ⇒ isForbidden() + * - isAllowed() in both ⇒ isAllowed() + * - neither isForbidden() nor isAllowed() => !isAllowed() && !isForbidden() + * + * @param \Drupal\Core\Access\AccessResultInterface $other + * The other access result to AND this one with. + * + * @return $this + */ + public function andIf(AccessResultInterface $other); + +} diff --git a/lib/Drupal/Core/Access/AccessibleInterface.php b/lib/Drupal/Core/Access/AccessibleInterface.php index 4316853f26e..324beeaa3b4 100644 --- a/lib/Drupal/Core/Access/AccessibleInterface.php +++ b/lib/Drupal/Core/Access/AccessibleInterface.php @@ -14,7 +14,7 @@ * * @ingroup entity_api */ -interface AccessibleInterface extends AccessInterface { +interface AccessibleInterface { /** * Checks data value access. @@ -24,10 +24,16 @@ interface AccessibleInterface extends AccessInterface { * @param \Drupal\Core\Session\AccountInterface $account * (optional) The user for which to check access, or NULL to check access * for the current user. Defaults to NULL. + * @param bool $return_as_object + * (optional) Defaults to FALSE. * - * @return bool|null - * self::ALLOW, self::DENY, or self::KILL. + * @return bool|\Drupal\Core\Access\AccessResultInterface + * The access result. Returns a boolean if $return_as_object is FALSE (this + * is the default) and otherwise an AccessResultInterface object. + * When a boolean is returned, the result of AccessInterface::isAllowed() is + * returned, i.e. TRUE means access is explicitly allowed, FALSE means + * access is either explicitly forbidden or "no opinion". */ - public function access($operation, AccountInterface $account = NULL); + public function access($operation, AccountInterface $account = NULL, $return_as_object = FALSE); } diff --git a/lib/Drupal/Core/Access/CsrfAccessCheck.php b/lib/Drupal/Core/Access/CsrfAccessCheck.php index f6b60d3dc82..b603e5c0ef7 100644 --- a/lib/Drupal/Core/Access/CsrfAccessCheck.php +++ b/lib/Drupal/Core/Access/CsrfAccessCheck.php @@ -45,29 +45,33 @@ 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\AccessResultInterface + * The access result. */ public function access(Route $route, Request $request) { + // Not cacheable because the CSRF token is highly dynamic. + $access = AccessResult::create()->setCacheable(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; + if ($this->csrfToken->validate($request->query->get('token'), $request->attributes->get('_system_path'))) { + $access->allow(); + } + else { + $access->forbid(); + } + return $access; } // Otherwise, this could be another requested access check that we don't // want to check CSRF tokens on. $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; - } - // Return DENY otherwise, as another access checker should grant access - // for the route. - else { - return static::DENY; - } + // Allow if all access checks are needed. This sets DENY if not all access + // checks are needed, because another access checker should explicitly grant + // access for the route. + return $access->allowIf($conjunction == AccessManagerInterface::ACCESS_MODE_ALL); } } diff --git a/lib/Drupal/Core/Access/CustomAccessCheck.php b/lib/Drupal/Core/Access/CustomAccessCheck.php index c128a5b435e..c0f8c9b8efd 100644 --- a/lib/Drupal/Core/Access/CustomAccessCheck.php +++ b/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\AccessResultInterface + * The access result. */ public function access(Route $route, Request $request, AccountInterface $account) { $callable = $this->controllerResolver->getControllerFromDefinition($route->getRequirement('_custom_access')); diff --git a/lib/Drupal/Core/Access/DefaultAccessCheck.php b/lib/Drupal/Core/Access/DefaultAccessCheck.php index 485dad862cb..6e27a5141ce 100644 --- a/lib/Drupal/Core/Access/DefaultAccessCheck.php +++ b/lib/Drupal/Core/Access/DefaultAccessCheck.php @@ -21,18 +21,18 @@ 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\AccessResultInterface + * The access result. */ public function access(Route $route) { if ($route->getRequirement('_access') === 'TRUE') { - return static::ALLOW; + return AccessResult::allowed(); } elseif ($route->getRequirement('_access') === 'FALSE') { - return static::KILL; + return AccessResult::forbidden(); } else { - return static::DENY; + return AccessResult::create(); } } diff --git a/lib/Drupal/Core/Block/BlockBase.php b/lib/Drupal/Core/Block/BlockBase.php index 11890c8043b..626650269d5 100644 --- a/lib/Drupal/Core/Block/BlockBase.php +++ b/lib/Drupal/Core/Block/BlockBase.php @@ -11,6 +11,7 @@ use Drupal\block\Event\BlockConditionContextEvent; use Drupal\block\Event\BlockEvents; use Drupal\Component\Plugin\ContextAwarePluginInterface; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Condition\ConditionAccessResolverTrait; use Drupal\Core\Condition\ConditionPluginBag; use Drupal\Core\Form\FormState; @@ -165,10 +166,22 @@ 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 = AccessResult::create()->setCacheable(FALSE); if ($this->resolveConditions($conditions, 'and', $contexts, $mappings) === FALSE) { - return FALSE; + $access->forbid(); + return $access; } - return $this->blockAccess($account); + if ($this->blockAccess($account)) { + $access->allow(); + } + else { + $access->forbid(); + } + return $access; } /** diff --git a/lib/Drupal/Core/Cache/CacheableInterface.php b/lib/Drupal/Core/Cache/CacheableInterface.php index 2a59467643e..67afc2e6a8d 100644 --- a/lib/Drupal/Core/Cache/CacheableInterface.php +++ b/lib/Drupal/Core/Cache/CacheableInterface.php @@ -16,8 +16,14 @@ interface CacheableInterface { /** * The cache keys associated with this potentially cacheable object. * + * Cache keys may either be static (just strings) or tokens (placeholders + * that are converted to static keys by the @cache_contexts service, depending + * depending on the request). + * * @return array - * An array of strings or cache constants, used to generate a cache ID. + * An array of strings or cache context tokens, used to generate a cache ID. + * + * @see \Drupal\Core\Cache\CacheContexts::convertTokensToKeys() */ public function getCacheKeys(); diff --git a/lib/Drupal/Core/Entity/ContentEntityBase.php b/lib/Drupal/Core/Entity/ContentEntityBase.php index 586da85657c..e8df9c88f6e 100644 --- a/lib/Drupal/Core/Entity/ContentEntityBase.php +++ b/lib/Drupal/Core/Entity/ContentEntityBase.php @@ -549,15 +549,15 @@ public function isEmpty() { /** * {@inheritdoc} */ - public function access($operation, AccountInterface $account = NULL) { + public function access($operation, AccountInterface $account = NULL, $return_as_object = FALSE) { if ($operation == 'create') { return $this->entityManager() ->getAccessControlHandler($this->entityTypeId) - ->createAccess($this->bundle(), $account); + ->createAccess($this->bundle(), $account, [], $return_as_object); } return $this->entityManager() ->getAccessControlHandler($this->entityTypeId) - ->access($this, $operation, $this->activeLangcode, $account); + ->access($this, $operation, $this->activeLangcode, $account, $return_as_object); } /** diff --git a/lib/Drupal/Core/Entity/Entity.php b/lib/Drupal/Core/Entity/Entity.php index ba7380a27b1..94249942165 100644 --- a/lib/Drupal/Core/Entity/Entity.php +++ b/lib/Drupal/Core/Entity/Entity.php @@ -272,15 +272,15 @@ public function uriRelationships() { /** * {@inheritdoc} */ - public function access($operation, AccountInterface $account = NULL) { + public function access($operation, AccountInterface $account = NULL, $return_as_object = FALSE) { if ($operation == 'create') { return $this->entityManager() ->getAccessControlHandler($this->entityTypeId) - ->createAccess($this->bundle(), $account); + ->createAccess($this->bundle(), $account, [], $return_as_object); } - return $this->entityManager() + return $this->entityManager() ->getAccessControlHandler($this->entityTypeId) - ->access($this, $operation, LanguageInterface::LANGCODE_DEFAULT, $account); + ->access($this, $operation, LanguageInterface::LANGCODE_DEFAULT, $account, $return_as_object); } /** diff --git a/lib/Drupal/Core/Entity/EntityAccessCheck.php b/lib/Drupal/Core/Entity/EntityAccessCheck.php index 951abb0aee0..49c9e3cef91 100644 --- a/lib/Drupal/Core/Entity/EntityAccessCheck.php +++ b/lib/Drupal/Core/Entity/EntityAccessCheck.php @@ -7,6 +7,7 @@ namespace Drupal\Core\Entity; +use Drupal\Core\Access\AccessResult; 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\AccessResultInterface + * The access result. */ public function access(Route $route, Request $request, AccountInterface $account) { // Split the entity type and the operation. @@ -48,12 +49,12 @@ 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, TRUE); } } // No opinion, so other access checks should decide if access should be // allowed or not. - return static::DENY; + return AccessResult::create(); } } diff --git a/lib/Drupal/Core/Entity/EntityAccessControlHandler.php b/lib/Drupal/Core/Entity/EntityAccessControlHandler.php index 5b43673c6d0..e655880030b 100644 --- a/lib/Drupal/Core/Entity/EntityAccessControlHandler.php +++ b/lib/Drupal/Core/Entity/EntityAccessControlHandler.php @@ -7,6 +7,7 @@ namespace Drupal\Core\Entity; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Field\FieldItemListInterface; use Drupal\Core\Field\FieldDefinitionInterface; use Drupal\Core\Language\LanguageInterface; @@ -52,12 +53,12 @@ public function __construct(EntityTypeInterface $entity_type) { /** * {@inheritdoc} */ - public function access(EntityInterface $entity, $operation, $langcode = LanguageInterface::LANGCODE_DEFAULT, AccountInterface $account = NULL) { + public function access(EntityInterface $entity, $operation, $langcode = LanguageInterface::LANGCODE_DEFAULT, AccountInterface $account = NULL, $return_as_object = FALSE) { $account = $this->prepareUser($account); - if (($access = $this->getCache($entity->uuid(), $operation, $langcode, $account)) !== NULL) { + if (($return = $this->getCache($entity->uuid(), $operation, $langcode, $account)) !== NULL) { // Cache hit, no work necessary. - return $access; + return $return_as_object ? $return : $return->isAllowed(); } // Invoke hook_entity_access() and hook_ENTITY_TYPE_access(). Hook results @@ -74,12 +75,14 @@ public function access(EntityInterface $entity, $operation, $langcode = Language $this->moduleHandler()->invokeAll($entity->getEntityTypeId() . '_access', array($entity, $operation, $account, $langcode)) ); - if (($return = $this->processAccessHookResults($access)) === NULL) { + $return = $this->processAccessHookResults($access); + if (!$return->isAllowed() && !$return->isForbidden()) { // No module had an opinion about the access, so let's the access - // handler check create access. - $return = (bool) $this->checkAccess($entity, $operation, $langcode, $account); + // handler check access. + $return->orIf($this->checkAccess($entity, $operation, $langcode, $account)); } - return $this->setCache($return, $entity->uuid(), $operation, $langcode, $account); + $result = $this->setCache($return, $entity->uuid(), $operation, $langcode, $account); + return $return_as_object ? $result : $result->isAllowed(); } /** @@ -87,23 +90,27 @@ 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\AccessResultInterface[] $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\AccessResultInterface + * The combined result of the various access checks' results. All their + * cacheability metadata is merged as well. + * + * @see \Drupal\Core\Access\AccessResultInterface::orIf() */ protected function processAccessHookResults(array $access) { - if (in_array(FALSE, $access, TRUE)) { - return FALSE; + // No results means no opinion. + if (empty($access)) { + return AccessResult::create(); } - elseif (in_array(TRUE, $access, TRUE)) { - return TRUE; - } - else { - return; + + /** @var \Drupal\Core\Access\AccessResultInterface $result */ + $result = array_shift($access); + foreach ($access as $other) { + $result = $result->orIf($other); } + return $result; } /** @@ -122,19 +129,19 @@ 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\AccessResultInterface + * The access result. */ protected function checkAccess(EntityInterface $entity, $operation, $langcode, AccountInterface $account) { if ($operation == 'delete' && $entity->isNew()) { - return FALSE; + return AccessResult::forbidden()->cacheUntilEntityChanges($entity); } if ($admin_permission = $this->entityType->getAdminPermission()) { - return $account->hasPermission($admin_permission); + return AccessResult::allowedIfHasPermission($account, $this->entityType->getAdminPermission()); } else { - return NULL; + // No opinion. + return AccessResult::create(); } } @@ -152,10 +159,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\AccessResultInterface|null + * The cached AccessResult, 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. @@ -167,8 +173,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\AccessResultInterface $access + * The access result. * @param string $cid * Unique string identifier for the entity/operation, for example the * entity UUID or a custom string. @@ -180,12 +186,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\AccessResultInterface + * 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; } /** @@ -198,7 +204,7 @@ public function resetCache() { /** * {@inheritdoc} */ - public function createAccess($entity_bundle = NULL, AccountInterface $account = NULL, array $context = array()) { + public function createAccess($entity_bundle = NULL, AccountInterface $account = NULL, array $context = array(), $return_as_object = FALSE) { $account = $this->prepareUser($account); $context += array( 'langcode' => LanguageInterface::LANGCODE_DEFAULT, @@ -207,7 +213,7 @@ public function createAccess($entity_bundle = NULL, AccountInterface $account = $cid = $entity_bundle ? 'create:' . $entity_bundle : 'create'; if (($access = $this->getCache($cid, 'create', $context['langcode'], $account)) !== NULL) { // Cache hit, no work necessary. - return $access; + return $return_as_object ? $access : $access->isAllowed(); } // Invoke hook_entity_create_access() and hook_ENTITY_TYPE_create_access(). @@ -224,12 +230,14 @@ public function createAccess($entity_bundle = NULL, AccountInterface $account = $this->moduleHandler()->invokeAll($this->entityTypeId . '_create_access', array($account, $context, $entity_bundle)) ); - if (($return = $this->processAccessHookResults($access)) === NULL) { + $return = $this->processAccessHookResults($access); + if (!$return->isAllowed() && !$return->isForbidden()) { // No module had an opinion about the access, so let's the access // handler check create access. - $return = (bool) $this->checkCreateAccess($account, $context, $entity_bundle); + $return->orIf($this->checkCreateAccess($account, $context, $entity_bundle)); } - return $this->setCache($return, $cid, 'create', $context['langcode'], $account); + $result = $this->setCache($return, $cid, 'create', $context['langcode'], $account); + return $return_as_object ? $result : $result->isAllowed(); } /** @@ -246,16 +254,16 @@ 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\AccessResultInterface + * The access result. */ protected function checkCreateAccess(AccountInterface $account, array $context, $entity_bundle = NULL) { if ($admin_permission = $this->entityType->getAdminPermission()) { - return $account->hasPermission($admin_permission); + return AccessResult::allowedIfHasPermission($account, $admin_permission); } else { - return NULL; + // No opinion. + return AccessResult::create(); } } @@ -278,18 +286,18 @@ protected function prepareUser(AccountInterface $account = NULL) { /** * {@inheritdoc} */ - public function fieldAccess($operation, FieldDefinitionInterface $field_definition, AccountInterface $account = NULL, FieldItemListInterface $items = NULL) { + public function fieldAccess($operation, FieldDefinitionInterface $field_definition, AccountInterface $account = NULL, FieldItemListInterface $items = NULL, $return_as_object = FALSE) { $account = $this->prepareUser($account); // Get the default access restriction that lives within this field. - $default = $items ? $items->defaultAccess($operation, $account) : TRUE; + $default = $items ? $items->defaultAccess($operation, $account) : AccessResult::allowed(); // Get the default access restriction as specified by the access control // handler. $entity_default = $this->checkFieldAccess($operation, $field_definition, $account, $items); // Combine default access, denying access wins. - $default = $default && $entity_default; + $default = $default->andIf($entity_default); // Invoke hook and collect grants/denies for field access from other // modules. Our default access flag is masked under the ':default' key. @@ -308,16 +316,8 @@ 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; + $result = $this->processAccessHookResults($grants); + return $return_as_object ? $result : $result->isAllowed(); } /** @@ -339,7 +339,7 @@ public function fieldAccess($operation, FieldDefinitionInterface $field_definiti * TRUE if access is allowed, FALSE otherwise. */ protected function checkFieldAccess($operation, FieldDefinitionInterface $field_definition, AccountInterface $account, FieldItemListInterface $items = NULL) { - return TRUE; + return AccessResult::allowed(); } } diff --git a/lib/Drupal/Core/Entity/EntityAccessControlHandlerInterface.php b/lib/Drupal/Core/Entity/EntityAccessControlHandlerInterface.php index bea60e3d562..4a0a5b5adb1 100644 --- a/lib/Drupal/Core/Entity/EntityAccessControlHandlerInterface.php +++ b/lib/Drupal/Core/Entity/EntityAccessControlHandlerInterface.php @@ -35,11 +35,17 @@ interface EntityAccessControlHandlerInterface { * @param \Drupal\Core\Session\AccountInterface $account * (optional) The user session for which to check access, or NULL to check * access for the current user. Defaults to NULL. + * @param bool $return_as_object + * (optional) Defaults to FALSE. * - * @return bool - * TRUE if access was granted, FALSE otherwise. + * @return bool|\Drupal\Core\Access\AccessResultInterface + * The access result. Returns a boolean if $return_as_object is FALSE (this + * is the default) and otherwise an AccessResultInterface object. + * When a boolean is returned, the result of AccessInterface::isAllowed() is + * returned, i.e. TRUE means access is explicitly allowed, FALSE means + * access is either explicitly forbidden or "no opinion". */ - public function access(EntityInterface $entity, $operation, $langcode = LanguageInterface::LANGCODE_DEFAULT, AccountInterface $account = NULL); + public function access(EntityInterface $entity, $operation, $langcode = LanguageInterface::LANGCODE_DEFAULT, AccountInterface $account = NULL, $return_as_object = FALSE); /** * Checks access to create an entity. @@ -53,10 +59,19 @@ public function access(EntityInterface $entity, $operation, $langcode = Language * @param array $context * (optional) An array of key-value pairs to pass additional context when * needed. + * @param bool $return_as_object + * (optional) Defaults to FALSE. + * + * @return bool|\Drupal\Core\Access\AccessResultInterface + * The access result. Returns a boolean if $return_as_object is FALSE (this + * is the default) and otherwise an AccessResultInterface object. + * When a boolean is returned, the result of AccessInterface::isAllowed() is + * returned, i.e. TRUE means access is explicitly allowed, FALSE means + * access is either explicitly forbidden or "no opinion". */ - public function createAccess($entity_bundle = NULL, AccountInterface $account = NULL, array $context = array()); + public function createAccess($entity_bundle = NULL, AccountInterface $account = NULL, array $context = array(), $return_as_object = FALSE); - /** + /** * Clears all cached access checks. */ public function resetCache(); @@ -91,9 +106,18 @@ public function setModuleHandler(ModuleHandlerInterface $module_handler); * (optional) The field values for which to check access, or NULL if access * is checked for the field definition, without any specific value * available. Defaults to NULL. + * @param bool $return_as_object + * (optional) Defaults to FALSE. + * + * @return bool|\Drupal\Core\Access\AccessResultInterface + * The access result. Returns a boolean if $return_as_object is FALSE (this + * is the default) and otherwise an AccessResultInterface object. + * When a boolean is returned, the result of AccessInterface::isAllowed() is + * returned, i.e. TRUE means access is explicitly allowed, FALSE means + * access is either explicitly forbidden or "no opinion". * * @see \Drupal\Core\Entity\EntityAccessControlHandlerInterface::access() */ - public function fieldAccess($operation, FieldDefinitionInterface $field_definition, AccountInterface $account = NULL, FieldItemListInterface $items = NULL); + public function fieldAccess($operation, FieldDefinitionInterface $field_definition, AccountInterface $account = NULL, FieldItemListInterface $items = NULL, $return_as_object = FALSE); } diff --git a/lib/Drupal/Core/Entity/EntityCreateAccessCheck.php b/lib/Drupal/Core/Entity/EntityCreateAccessCheck.php index e3fab39f14a..daefcbeda9a 100644 --- a/lib/Drupal/Core/Entity/EntityCreateAccessCheck.php +++ b/lib/Drupal/Core/Entity/EntityCreateAccessCheck.php @@ -7,6 +7,7 @@ namespace Drupal\Core\Entity; +use Drupal\Core\Access\AccessResult; 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\AccessResultInterface + * The access result. */ public function access(Route $route, Request $request, AccountInterface $account) { list($entity_type, $bundle) = explode(':', $route->getRequirement($this->requirementsKey) . ':'); @@ -66,10 +67,10 @@ 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; + return AccessResult::create(); } } - return $this->entityManager->getAccessControlHandler($entity_type)->createAccess($bundle, $account) ? static::ALLOW : static::DENY; + return $this->entityManager->getAccessControlHandler($entity_type)->createAccess($bundle, $account, [], TRUE); } } diff --git a/lib/Drupal/Core/Field/FieldItemList.php b/lib/Drupal/Core/Field/FieldItemList.php index 6b70f96ccaf..69daf679bef 100644 --- a/lib/Drupal/Core/Field/FieldItemList.php +++ b/lib/Drupal/Core/Field/FieldItemList.php @@ -7,6 +7,7 @@ namespace Drupal\Core\Field; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Form\FormStateInterface; use Drupal\Core\Language\LanguageInterface; use Drupal\Core\Entity\ContentEntityInterface; @@ -187,9 +188,9 @@ public function __unset($property_name) { /** * {@inheritdoc} */ - public function access($operation = 'view', AccountInterface $account = NULL) { + public function access($operation = 'view', AccountInterface $account = NULL, $return_as_object = FALSE) { $access_control_handler = \Drupal::entityManager()->getAccessControlHandler($this->getEntity()->getEntityTypeId()); - return $access_control_handler->fieldAccess($operation, $this->getFieldDefinition(), $account, $this); + return $access_control_handler->fieldAccess($operation, $this->getFieldDefinition(), $account, $this, $return_as_object); } /** @@ -197,7 +198,7 @@ public function access($operation = 'view', AccountInterface $account = NULL) { */ public function defaultAccess($operation = 'view', AccountInterface $account = NULL) { // Grant access per default. - return TRUE; + return AccessResult::allowed(); } /** diff --git a/lib/Drupal/Core/Field/FieldItemListInterface.php b/lib/Drupal/Core/Field/FieldItemListInterface.php index 3ae486c557a..b4ecf5136c6 100644 --- a/lib/Drupal/Core/Field/FieldItemListInterface.php +++ b/lib/Drupal/Core/Field/FieldItemListInterface.php @@ -87,8 +87,8 @@ public function getSetting($setting_name); * See \Drupal\Core\Entity\EntityAccessControlHandlerInterface::fieldAccess() for * the parameter documentation. * - * @return bool - * TRUE if access to this field is allowed per default, FALSE otherwise. + * @return \Drupal\Core\Access\AccessResultInterface + * The access result. */ public function defaultAccess($operation = 'view', AccountInterface $account = NULL); diff --git a/lib/Drupal/Core/Menu/MenuLinkBase.php b/lib/Drupal/Core/Menu/MenuLinkBase.php index e0feb4cd51a..b09bfa92eca 100644 --- a/lib/Drupal/Core/Menu/MenuLinkBase.php +++ b/lib/Drupal/Core/Menu/MenuLinkBase.php @@ -9,6 +9,7 @@ use Drupal\Component\Plugin\Exception\PluginException; use Drupal\Component\Utility\String; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Plugin\PluginBase; use Drupal\Core\Url; @@ -76,7 +77,7 @@ public function isExpanded() { * {@inheritdoc} */ public function isResettable() { - return FALSE; + return AccessResult::forbidden(); } /** diff --git a/lib/Drupal/Core/Menu/MenuLinkDefault.php b/lib/Drupal/Core/Menu/MenuLinkDefault.php index c2241428ce5..e118824b9c7 100644 --- a/lib/Drupal/Core/Menu/MenuLinkDefault.php +++ b/lib/Drupal/Core/Menu/MenuLinkDefault.php @@ -7,6 +7,7 @@ namespace Drupal\Core\Menu; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Plugin\ContainerFactoryPluginInterface; use Symfony\Component\DependencyInjection\ContainerInterface; @@ -94,7 +95,8 @@ public function getDescription() { */ public function isResettable() { // The link can be reset if it has an override. - return (bool) $this->staticOverride->loadOverride($this->getPluginId()); + // @todo This will be cacheable after https://www.drupal.org/node/2040135. + return AccessResult::allowedIf($this->staticOverride->loadOverride($this->getPluginId()))->setCacheable(FALSE); } /** diff --git a/lib/Drupal/Core/Operations/OperationsProviderInterface.php b/lib/Drupal/Core/Operations/OperationsProviderInterface.php new file mode 100644 index 00000000000..71ec8743ac2 --- /dev/null +++ b/lib/Drupal/Core/Operations/OperationsProviderInterface.php @@ -0,0 +1,23 @@ +checkAccess($theme) ? static::ALLOW : static::DENY; + // Cacheable until the theme is modified. + return AccessResult::allowedIf($this->checkAccess($theme))->addCacheTags(array('theme' => $theme)); } /** diff --git a/modules/aggregator/src/FeedAccessControlHandler.php b/modules/aggregator/src/FeedAccessControlHandler.php index 9683ad5f68d..f147c53262c 100644 --- a/modules/aggregator/src/FeedAccessControlHandler.php +++ b/modules/aggregator/src/FeedAccessControlHandler.php @@ -7,6 +7,7 @@ namespace Drupal\aggregator; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Entity\EntityAccessControlHandler; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Session\AccountInterface; @@ -24,11 +25,11 @@ class FeedAccessControlHandler extends EntityAccessControlHandler { protected function checkAccess(EntityInterface $entity, $operation, $langcode, AccountInterface $account) { switch ($operation) { case 'view': - return $account->hasPermission('access news feeds'); + return AccessResult::allowedIfHasPermission($account, 'access news feeds'); break; default: - return $account->hasPermission('administer news feeds'); + return AccessResult::allowedIfHasPermission($account, 'administer news feeds'); break; } } @@ -37,7 +38,7 @@ 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'); + return AccessResult::allowedIfHasPermission($account, 'administer news feeds'); } } diff --git a/modules/block/block.api.php b/modules/block/block.api.php index 2d2f3ccf3ac..43f03e9db63 100644 --- a/modules/block/block.api.php +++ b/modules/block/block.api.php @@ -5,6 +5,8 @@ * Hooks provided by the Block module. */ +use Drupal\Core\Access\AccessResult; + /** * @defgroup block_api Block API * @{ @@ -139,9 +141,10 @@ function hook_block_view_BASE_BLOCK_ID_alter(array &$build, \Drupal\Core\Block\B * @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 + * @return \Drupal\Core\Access\AccessResultInterface + * The access result. If all implementations of this hook return + * AccessResultInterface objects whose value is !isAllowed() and + * !isForbidden(), then default access rules from * \Drupal\block\BlockAccessControlHandler::checkAccess() are used. * * @see \Drupal\Core\Entity\EntityAccessControlHandler::access() @@ -151,9 +154,12 @@ function hook_block_view_BASE_BLOCK_ID_alter(array &$build, \Drupal\Core\Block\B function hook_block_access(\Drupal\block\Entity\Block $block, $operation, \Drupal\user\Entity\User $account, $langcode) { // 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; + if ($operation == 'view' && $block->get('plugin') == 'system_powered_by_block') { + return AccessResult::forbiddenIf($block->get('region') != 'footer')->cacheUntilEntityChanges($block); } + + // No opinion. + return AccessResult::create(); } /** diff --git a/modules/block/src/BlockAccessControlHandler.php b/modules/block/src/BlockAccessControlHandler.php index 2768d884ae3..8bfa3656f46 100644 --- a/modules/block/src/BlockAccessControlHandler.php +++ b/modules/block/src/BlockAccessControlHandler.php @@ -7,6 +7,7 @@ namespace Drupal\block; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Entity\EntityAccessControlHandler; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Session\AccountInterface; @@ -27,13 +28,14 @@ protected function checkAccess(EntityInterface $entity, $operation, $langcode, A return parent::checkAccess($entity, $operation, $langcode, $account); } - // Deny access to disabled blocks. + // Don't grant access to disabled blocks. if (!$entity->status()) { - return FALSE; + return AccessResult::forbidden()->cacheUntilEntityChanges($entity); + } + else { + // Delegate to the plugin. + return $entity->getPlugin()->access($account)->cacheUntilEntityChanges($entity); } - - // Delegate to the plugin. - return $entity->getPlugin()->access($account); } } diff --git a/modules/block_content/src/BlockContentAccessControlHandler.php b/modules/block_content/src/BlockContentAccessControlHandler.php index 6b4de2313bb..e88643283cb 100644 --- a/modules/block_content/src/BlockContentAccessControlHandler.php +++ b/modules/block_content/src/BlockContentAccessControlHandler.php @@ -7,6 +7,7 @@ namespace Drupal\block_content; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Entity\EntityAccessControlHandler; use Drupal\Core\Session\AccountInterface; @@ -23,7 +24,7 @@ class BlockContentAccessControlHandler extends EntityAccessControlHandler { */ protected function checkAccess(EntityInterface $entity, $operation, $langcode, AccountInterface $account) { if ($operation === 'view') { - return TRUE; + return AccessResult::allowed(); } return parent::checkAccess($entity, $operation, $langcode, $account); } diff --git a/modules/book/src/Access/BookNodeIsRemovableAccessCheck.php b/modules/book/src/Access/BookNodeIsRemovableAccessCheck.php index 861c25268b5..ee17c707a84 100644 --- a/modules/book/src/Access/BookNodeIsRemovableAccessCheck.php +++ b/modules/book/src/Access/BookNodeIsRemovableAccessCheck.php @@ -8,13 +8,14 @@ namespace Drupal\book\Access; use Drupal\book\BookManagerInterface; +use Drupal\Core\Access\AccessResult; 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,11 @@ 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\AccessResultInterface + * The access result. */ public function access(NodeInterface $node) { - return $this->bookManager->checkNodeIsRemovable($node) ? static::ALLOW : static::DENY; + return AccessResult::allowedIf($this->bookManager->checkNodeIsRemovable($node))->cacheUntilEntityChanges($node); } } diff --git a/modules/comment/src/CommentAccessControlHandler.php b/modules/comment/src/CommentAccessControlHandler.php index 77367ab9336..a31f5d7a8f4 100644 --- a/modules/comment/src/CommentAccessControlHandler.php +++ b/modules/comment/src/CommentAccessControlHandler.php @@ -7,6 +7,7 @@ namespace Drupal\comment; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Entity\EntityAccessControlHandler; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Session\AccountInterface; @@ -23,24 +24,23 @@ class CommentAccessControlHandler extends EntityAccessControlHandler { */ protected function checkAccess(EntityInterface $entity, $operation, $langcode, AccountInterface $account) { /** @var \Drupal\Core\Entity\EntityInterface|\Drupal\user\EntityOwnerInterface $entity */ + + if ($account->hasPermission('administer comments')) { + $access = AccessResult::allowed()->cachePerRole(); + return ($operation != 'view') ? $access : $access->andIf($entity->getCommentedEntity()->access($operation, $account, TRUE)); + } + switch ($operation) { case 'view': - if ($account->hasPermission('access comments') && $entity->isPublished() || $account->hasPermission('administer comments')) { - return $entity->getCommentedEntity()->access($operation, $account); - } - break; + return AccessResult::allowedIf($account->hasPermission('access comments') && $entity->isPublished())->cachePerRole()->cacheUntilEntityChanges($entity) + ->andIf($entity->getCommentedEntity()->access($operation, $account, TRUE)); case 'update': - return ($account->id() && $account->id() == $entity->getOwnerId() && $entity->isPublished() && $account->hasPermission('edit own comments')) || $account->hasPermission('administer comments'); - break; - - case 'delete': - return $account->hasPermission('administer comments'); - break; + return AccessResult::allowedIf($account->id() && $account->id() == $entity->getOwnerId() && $entity->isPublished() && $account->hasPermission('edit own comments'))->cachePerRole()->cachePerUser()->cacheUntilEntityChanges($entity); - case 'approve': - return $account->hasPermission('administer comments'); - break; + default: + // No opinion. + return AccessResult::create()->cachePerRole(); } } @@ -48,7 +48,7 @@ protected function checkAccess(EntityInterface $entity, $operation, $langcode, A * {@inheritdoc} */ protected function checkCreateAccess(AccountInterface $account, array $context, $entity_bundle = NULL) { - return $account->hasPermission('post comments'); + return AccessResult::allowedIfHasPermission($account, 'post comments'); } } diff --git a/modules/comment/src/CommentViewBuilder.php b/modules/comment/src/CommentViewBuilder.php index d2d6290c389..cd36bc4efb9 100644 --- a/modules/comment/src/CommentViewBuilder.php +++ b/modules/comment/src/CommentViewBuilder.php @@ -262,7 +262,7 @@ protected static function buildLinks(CommentInterface $entity, EntityInterface $ } // Add translations link for translation-enabled comment bundles. - if (\Drupal::moduleHandler()->moduleExists('content_translation') && content_translation_translate_access($entity)) { + if (\Drupal::moduleHandler()->moduleExists('content_translation') && content_translation_translate_access($entity)->isAllowed()) { $links['comment-translations'] = array( 'title' => t('Translate'), 'href' => 'comment/' . $entity->id() . '/translations', diff --git a/modules/comment/src/Form/CommentAdminOverview.php b/modules/comment/src/Form/CommentAdminOverview.php index 8edba5168ff..723241ad9b9 100644 --- a/modules/comment/src/Form/CommentAdminOverview.php +++ b/modules/comment/src/Form/CommentAdminOverview.php @@ -221,7 +221,7 @@ public function buildForm(array $form, FormStateInterface $form_state, $type = ' 'options' => $comment_uri_options, 'query' => $destination, ); - if ($this->moduleHandler->invoke('content_translation', 'translate_access', array($comment))) { + if ($this->moduleHandler->moduleExists('content_translation') && $this->moduleHandler->invoke('content_translation', 'translate_access', array($comment))->isAllowed()) { $links['translate'] = array( 'title' => $this->t('Translate'), 'route_name' => 'content_translation.translation_overview_comment', diff --git a/modules/config/tests/config_test/src/ConfigTestAccessControlHandler.php b/modules/config/tests/config_test/src/ConfigTestAccessControlHandler.php index 0814dd8e6f2..4508ce2ea1f 100644 --- a/modules/config/tests/config_test/src/ConfigTestAccessControlHandler.php +++ b/modules/config/tests/config_test/src/ConfigTestAccessControlHandler.php @@ -7,6 +7,7 @@ namespace Drupal\config_test; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Session\AccountInterface; use Drupal\Core\Entity\EntityAccessControlHandler; use Drupal\Core\Entity\EntityInterface; @@ -22,14 +23,14 @@ class ConfigTestAccessControlHandler extends EntityAccessControlHandler { * {@inheritdoc} */ public function checkAccess(EntityInterface $entity, $operation, $langcode, AccountInterface $account) { - return TRUE; + return AccessResult::allowed(); } /** * {@inheritdoc} */ protected function checkCreateAccess(AccountInterface $account, array $context, $entity_bundle = NULL) { - return TRUE; + return AccessResult::allowed(); } } diff --git a/modules/config_translation/src/Access/ConfigTranslationFormAccess.php b/modules/config_translation/src/Access/ConfigTranslationFormAccess.php index 791ef85c9cc..da7f4247c9b 100644 --- a/modules/config_translation/src/Access/ConfigTranslationFormAccess.php +++ b/modules/config_translation/src/Access/ConfigTranslationFormAccess.php @@ -7,6 +7,7 @@ namespace Drupal\config_translation\Access; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Session\AccountInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Routing\Route; @@ -23,7 +24,7 @@ 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) { + if ($base_access->isAllowed()) { $target_language = language_load($request->attributes->get('langcode')); // Make sure that the target language is not locked, and that the target @@ -35,9 +36,9 @@ public function access(Route $route, Request $request, AccountInterface $account !$target_language->locked && $target_language->id != $this->sourceLanguage->id; - return $access ? static::ALLOW : static::DENY; + return $base_access->andIf(AccessResult::allowedIf($access)); } - return static::DENY; + return $base_access; } } diff --git a/modules/config_translation/src/Access/ConfigTranslationOverviewAccess.php b/modules/config_translation/src/Access/ConfigTranslationOverviewAccess.php index 529bc4ae0ea..407f9a159b7 100644 --- a/modules/config_translation/src/Access/ConfigTranslationOverviewAccess.php +++ b/modules/config_translation/src/Access/ConfigTranslationOverviewAccess.php @@ -8,6 +8,7 @@ namespace Drupal\config_translation\Access; use Drupal\config_translation\ConfigMapperManagerInterface; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; use Symfony\Component\HttpFoundation\Request; @@ -52,8 +53,8 @@ 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\AccessResultInterface + * The access result. */ public function access(Route $route, Request $request, AccountInterface $account) { /** @var \Drupal\config_translation\ConfigMapperInterface $mapper */ @@ -71,7 +72,7 @@ public function access(Route $route, Request $request, AccountInterface $account $mapper->hasTranslatable() && !$this->sourceLanguage->locked; - return $access ? static::ALLOW : static::DENY; + return AccessResult::allowedIf($access)->cachePerRole(); } } diff --git a/modules/contact/src/Access/ContactPageAccess.php b/modules/contact/src/Access/ContactPageAccess.php index 97a7b54fee2..0a7eae74506 100644 --- a/modules/contact/src/Access/ContactPageAccess.php +++ b/modules/contact/src/Access/ContactPageAccess.php @@ -7,6 +7,7 @@ namespace Drupal\contact\Access; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Config\ConfigFactoryInterface; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; @@ -53,45 +54,47 @@ 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\AccessResultInterface + * The access result. */ public function access(UserInterface $user, AccountInterface $account) { $contact_account = $user; // Anonymous users cannot have contact forms. if ($contact_account->isAnonymous()) { - return static::DENY; + return AccessResult::forbidden(); } // Users may not contact themselves. if ($account->id() == $contact_account->id()) { - return static::DENY; + return AccessResult::forbidden()->cachePerUser(); } // User administrators should always have access to personal contact forms. + $access = AccessResult::create()->cachePerRole(); if ($account->hasPermission('administer users')) { - return static::ALLOW; + return $access->allow(); } // If requested user has been blocked, do not allow users to contact them. + $access->cacheUntilEntityChanges($contact_account); if ($contact_account->isBlocked()) { - return 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; + 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; + return $access; } - return $account->hasPermission('access user contact forms') ? static::ALLOW : static::DENY; + return $access->allowIfHasPermission($account, 'access user contact forms'); } } diff --git a/modules/contact/src/ContactFormAccessControlHandler.php b/modules/contact/src/ContactFormAccessControlHandler.php index 8f1b65c1fb9..3af949afa25 100644 --- a/modules/contact/src/ContactFormAccessControlHandler.php +++ b/modules/contact/src/ContactFormAccessControlHandler.php @@ -7,6 +7,7 @@ namespace Drupal\contact; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Entity\EntityAccessControlHandler; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Session\AccountInterface; @@ -24,12 +25,12 @@ class ContactFormAccessControlHandler extends EntityAccessControlHandler { public function checkAccess(EntityInterface $entity, $operation, $langcode, AccountInterface $account) { if ($operation == 'view') { // Do not allow access personal form via site-wide route. - return $account->hasPermission('access site-wide contact form') && $entity->id() !== 'personal'; + return AccessResult::allowedIf($account->hasPermission('access site-wide contact form') && $entity->id() !== 'personal')->cachePerRole(); } elseif ($operation == 'delete' || $operation == 'update') { // Do not allow the 'personal' form to be deleted, as it's used for // the personal contact form. - return $account->hasPermission('administer contact forms') && $entity->id() !== 'personal'; + return AccessResult::allowedIf($account->hasPermission('administer contact forms') && $entity->id() !== 'personal')->cachePerRole(); } return parent::checkAccess($entity, $operation, $langcode, $account); diff --git a/modules/content_translation/content_translation.module b/modules/content_translation/content_translation.module index 22b7fe0a375..f1a1b602347 100644 --- a/modules/content_translation/content_translation.module +++ b/modules/content_translation/content_translation.module @@ -5,6 +5,7 @@ * Allows entities to be translated into different languages. */ +use Drupal\Core\Access\AccessResult; use Drupal\Core\Entity\ContentEntityInterface; use Drupal\Core\Entity\EntityFormInterface; use Drupal\Core\Entity\EntityInterface; @@ -218,11 +219,15 @@ function _content_translation_menu_strip_loaders($path) { * * @param \Drupal\Core\Entity\EntityInterface $entity * The entity whose translation overview should be displayed. + * + * @return \Drupal\Core\Access\AccessResultInterface + * The access result. */ function content_translation_translate_access(EntityInterface $entity) { $account = \Drupal::currentUser(); - return $entity instanceof ContentEntityInterface && empty($entity->getUntranslated()->language()->locked) && \Drupal::languageManager()->isMultilingual() && $entity->isTranslatable() && + $condition = $entity instanceof ContentEntityInterface && empty($entity->getUntranslated()->language()->locked) && \Drupal::languageManager()->isMultilingual() && $entity->isTranslatable() && ($account->hasPermission('create content translations') || $account->hasPermission('update content translations') || $account->hasPermission('delete content translations')); + return AccessResult::allowedIf($condition)->cachePerRole()->cacheUntilEntityChanges($entity); } /** @@ -346,8 +351,8 @@ function content_translation_controller($entity_type_id) { * - "delete" * - "create" * - * @return - * TRUE if the current user is allowed to view the translation. + * @return \Drupal\Core\Access\AccessResultInterface + * The access result. * * @todo Move to \Drupal\content_translation\ContentTranslationManager. */ diff --git a/modules/content_translation/src/Access/ContentTranslationManageAccessCheck.php b/modules/content_translation/src/Access/ContentTranslationManageAccessCheck.php index 5f124526f54..6c85edd01dd 100644 --- a/modules/content_translation/src/Access/ContentTranslationManageAccessCheck.php +++ b/modules/content_translation/src/Access/ContentTranslationManageAccessCheck.php @@ -7,6 +7,7 @@ namespace Drupal\content_translation\Access; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\Language\LanguageInterface; use Drupal\Core\Language\LanguageManagerInterface; @@ -66,8 +67,8 @@ public function __construct(EntityManagerInterface $manager, LanguageManagerInte * @param string $entity_type_id * (optional) The entity type ID. * - * @return string - * A \Drupal\Core\Access\AccessInterface constant value. + * @return \Drupal\Core\Access\AccessResultInterface + * The access result. */ public function access(Route $route, Request $request, AccountInterface $account, $source = NULL, $target = NULL, $language = NULL, $entity_type_id = NULL) { /* @var \Drupal\Core\Entity\ContentEntityInterface $entity */ @@ -86,24 +87,26 @@ public function access(Route $route, Request $request, AccountInterface $account case 'create': $source_language = $this->languageManager->getLanguage($source) ?: $entity->language(); $target_language = $this->languageManager->getLanguage($target) ?: $this->languageManager->getCurrentLanguage(LanguageInterface::TYPE_CONTENT); - return ($source_language->getId() != $target_language->getId() + $is_new_translation = ($source_language->getId() != $target_language->getId() && isset($languages[$source_language->getId()]) && isset($languages[$target_language->getId()]) - && !isset($translations[$target_language->getId()]) - && $handler->getTranslationAccess($entity, $operation)) - ? static::ALLOW : static::DENY; + && !isset($translations[$target_language->getId()])); + return AccessResult::allowedIf($is_new_translation)->cachePerRole()->cacheUntilEntityChanges($entity) + ->andIf($handler->getTranslationAccess($entity, $operation)); case 'update': case 'delete': $language = $this->languageManager->getLanguage($language) ?: $this->languageManager->getCurrentLanguage(LanguageInterface::TYPE_CONTENT); - return isset($languages[$language->getId()]) + $has_translation = isset($languages[$language->getId()]) && $language->getId() != $entity->getUntranslated()->language()->getId() - && isset($translations[$language->getId()]) - && $handler->getTranslationAccess($entity, $operation) - ? static::ALLOW : static::DENY; + && isset($translations[$language->getId()]); + return AccessResult::allowedIf($has_translation)->cachePerRole()->cacheUntilEntityChanges($entity) + ->andIf($handler->getTranslationAccess($entity, $operation)); } } - return static::DENY; + + // No opinion. + return AccessResult::create(); } } diff --git a/modules/content_translation/src/Access/ContentTranslationOverviewAccess.php b/modules/content_translation/src/Access/ContentTranslationOverviewAccess.php index 893b8b152fd..13e240beb9f 100644 --- a/modules/content_translation/src/Access/ContentTranslationOverviewAccess.php +++ b/modules/content_translation/src/Access/ContentTranslationOverviewAccess.php @@ -7,6 +7,7 @@ namespace Drupal\content_translation\Access; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; @@ -42,8 +43,8 @@ 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\AccessResultInterface + * The access result. */ public function access(Request $request, AccountInterface $account) { $entity_type = $request->attributes->get('entity_type_id'); @@ -56,13 +57,14 @@ public function access(Request $request, AccountInterface $account) { $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; + $access = call_user_func($access_callback, $entity); + if ($access->isAllowed()) { + return $access; } // Check "translate any entity" permission. if ($account->hasPermission('translate any entity')) { - return static::ALLOW; + return $access->allow()->cachePerRole(); } // Check per entity permission. @@ -70,11 +72,10 @@ public function access(Request $request, AccountInterface $account) { if ($definition->getPermissionGranularity() == 'bundle') { $permission = "translate {$bundle} {$entity_type}"; } - if ($account->hasPermission($permission)) { - return static::ALLOW; - } + return $access->allowIfHasPermission($account, $permission); } - return static::DENY; + // No opinion. + return AccessResult::create(); } } diff --git a/modules/content_translation/src/ContentTranslationHandler.php b/modules/content_translation/src/ContentTranslationHandler.php index dcc8746cbf9..b619036969d 100644 --- a/modules/content_translation/src/ContentTranslationHandler.php +++ b/modules/content_translation/src/ContentTranslationHandler.php @@ -7,6 +7,7 @@ namespace Drupal\content_translation; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Entity\EntityTypeInterface; use Drupal\Core\Form\FormStateInterface; @@ -70,7 +71,7 @@ public function getTranslationAccess(EntityInterface $entity, $op) { if (!$current_user->hasPermission('translate any entity') && $permission_granularity = $entity_type->getPermissionGranularity()) { $translate_permission = $current_user->hasPermission($permission_granularity == 'bundle' ? "translate {$entity->bundle()} {$entity->getEntityTypeId()}" : "translate {$entity->getEntityTypeId()}"); } - return $translate_permission && $current_user->hasPermission("$op content translations"); + return AccessResult::allowedIf($translate_permission && $current_user->hasPermission("$op content translations"))->cachePerRole(); } /** @@ -175,7 +176,7 @@ public function entityFormAlter(array &$form, FormStateInterface $form_state, En '#value' => t('Delete translation'), '#weight' => $weight, '#submit' => array(array($this, 'entityFormDeleteTranslation')), - '#access' => $this->getTranslationAccess($entity, 'delete'), + '#access' => $this->getTranslationAccess($entity, 'delete')->isAllowed(), ); } @@ -191,7 +192,7 @@ public function entityFormAlter(array &$form, FormStateInterface $form_state, En '#title' => t('Translation'), '#tree' => TRUE, '#weight' => 10, - '#access' => $this->getTranslationAccess($entity, $source_langcode ? 'create' : 'update'), + '#access' => $this->getTranslationAccess($entity, $source_langcode ? 'create' : 'update')->isAllowed(), '#multilingual' => TRUE, ); diff --git a/modules/content_translation/src/ContentTranslationHandlerInterface.php b/modules/content_translation/src/ContentTranslationHandlerInterface.php index 8d3d725ee62..5ab88581688 100644 --- a/modules/content_translation/src/ContentTranslationHandlerInterface.php +++ b/modules/content_translation/src/ContentTranslationHandlerInterface.php @@ -30,8 +30,8 @@ interface ContentTranslationHandlerInterface { * - "update" * - "delete" * - * @return boolean - * TRUE if the operation may be performed, FALSE otherwise. + * @return \Drupal\Core\Access\AccessResultInterface + * The access result. */ public function getTranslationAccess(EntityInterface $entity, $op); diff --git a/modules/content_translation/src/Controller/ContentTranslationController.php b/modules/content_translation/src/Controller/ContentTranslationController.php index 5f2c4602165..93b4887cc8c 100644 --- a/modules/content_translation/src/Controller/ContentTranslationController.php +++ b/modules/content_translation/src/Controller/ContentTranslationController.php @@ -139,7 +139,7 @@ public function overview(Request $request, $entity_type_id = NULL) { 'language' => $language, ); } - elseif (!$is_original && $handler->getTranslationAccess($entity, 'update')) { + elseif (!$is_original && $handler->getTranslationAccess($entity, 'update')->isAllowed()) { $links['edit'] = $edit_url->toArray(); } @@ -162,7 +162,7 @@ public function overview(Request $request, $entity_type_id = NULL) { } else { $source_name = isset($languages[$source]) ? $languages[$source]->name : $this->t('n/a'); - if ($handler->getTranslationAccess($entity, 'delete')) { + if ($handler->getTranslationAccess($entity, 'delete')->isAllowed()) { $links['delete'] = array( 'title' => $this->t('Delete'), ) + $delete_url->toArray(); @@ -174,7 +174,7 @@ public function overview(Request $request, $entity_type_id = NULL) { $row_title = $source_name = $this->t('n/a'); $source = $entity->language()->getId(); - if ($source != $langcode && $handler->getTranslationAccess($entity, 'create')) { + if ($source != $langcode && $handler->getTranslationAccess($entity, 'create')->isAllowed()) { if ($translatable) { $links['add'] = array( 'title' => $this->t('Add'), diff --git a/modules/content_translation/src/Plugin/views/field/TranslationLink.php b/modules/content_translation/src/Plugin/views/field/TranslationLink.php index 9d234abf34a..f800e3747a9 100644 --- a/modules/content_translation/src/Plugin/views/field/TranslationLink.php +++ b/modules/content_translation/src/Plugin/views/field/TranslationLink.php @@ -61,7 +61,7 @@ public function render(ResultRow $values) { * The actual rendered text (without the link) of this field. */ protected function renderLink(EntityInterface $entity, ResultRow $values) { - if (content_translation_translate_access($entity)) { + if (content_translation_translate_access($entity)->isAllowed()) { $text = !empty($this->options['text']) ? $this->options['text'] : t('Translate'); $this->options['alter']['make_link'] = TRUE; diff --git a/modules/content_translation/tests/src/Unit/Access/ContentTranslationManageAccessCheckTest.php b/modules/content_translation/tests/src/Unit/Access/ContentTranslationManageAccessCheckTest.php index 7bdfc981baf..23a4819f23e 100644 --- a/modules/content_translation/tests/src/Unit/Access/ContentTranslationManageAccessCheckTest.php +++ b/modules/content_translation/tests/src/Unit/Access/ContentTranslationManageAccessCheckTest.php @@ -8,7 +8,7 @@ namespace Drupal\Tests\content_translation\Unit\Access; use Drupal\content_translation\Access\ContentTranslationManageAccessCheck; -use Drupal\Core\Access\AccessInterface; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Language\Language; use Drupal\Tests\UnitTestCase; use Symfony\Component\HttpFoundation\Request; @@ -18,6 +18,7 @@ * Tests for content translation manage check. * * @coversDefaultClass \Drupal\content_translation\Access\ContentTranslationManageAccessCheck + * @group Access * @group content_translation */ class ContentTranslationManageAccessCheckTest extends UnitTestCase { @@ -32,7 +33,7 @@ public function testCreateAccess() { $translation_handler = $this->getMock('\Drupal\content_translation\ContentTranslationHandlerInterface'); $translation_handler->expects($this->once()) ->method('getTranslationAccess') - ->will($this->returnValue(TRUE)); + ->will($this->returnValue(AccessResult::allowed())); $entity_manager = $this->getMock('Drupal\Core\Entity\EntityManagerInterface'); $entity_manager->expects($this->once()) @@ -69,6 +70,9 @@ public function testCreateAccess() { ->method('getTranslationLanguages') ->with() ->will($this->returnValue(array())); + $entity->expects($this->once()) + ->method('getCacheTag') + ->will($this->returnValue(array('node' => 1337))); // Set the route requirements. $route = new Route('test_route'); @@ -88,7 +92,7 @@ public function testCreateAccess() { $language = 'en'; $entity_type_id = 'node'; - $this->assertEquals($check->access($route, $request, $account, $source, $target, $language, $entity_type_id), AccessInterface::ALLOW, "The access check matches"); + $this->assertTrue($check->access($route, $request, $account, $source, $target, $language, $entity_type_id)->isAllowed(), "The access check matches"); } } diff --git a/modules/field/src/FieldInstanceConfigAccessControlHandler.php b/modules/field/src/FieldInstanceConfigAccessControlHandler.php index 6b217834262..eb4a37fa90d 100644 --- a/modules/field/src/FieldInstanceConfigAccessControlHandler.php +++ b/modules/field/src/FieldInstanceConfigAccessControlHandler.php @@ -7,6 +7,7 @@ namespace Drupal\field; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Entity\EntityAccessControlHandler; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Session\AccountInterface; @@ -22,10 +23,16 @@ class FieldInstanceConfigAccessControlHandler extends EntityAccessControlHandler * {@inheritdoc} */ protected function checkAccess(EntityInterface $entity, $operation, $langcode, AccountInterface $account) { - if ($operation == 'delete' && $entity->getFieldStorageDefinition()->isLocked()) { - return FALSE; + if ($operation == 'delete') { + $field_storage_entity = $entity->getFieldStorageDefinition(); + if ($field_storage_entity->isLocked()) { + return AccessResult::forbidden()->cacheUntilEntityChanges($field_storage_entity); + } + else { + return AccessResult::allowedIfHasPermission($account, 'administer ' . $entity->entity_type . ' fields')->cacheUntilEntityChanges($field_storage_entity); + } } - return $account->hasPermission('administer ' . $entity->entity_type . ' fields'); + return AccessResult::allowedIfHasPermission($account, 'administer ' . $entity->entity_type . ' fields'); } } diff --git a/modules/field/src/Plugin/views/field/Field.php b/modules/field/src/Plugin/views/field/Field.php index d18ca128568..7788a5bb6d8 100644 --- a/modules/field/src/Plugin/views/field/Field.php +++ b/modules/field/src/Plugin/views/field/Field.php @@ -214,7 +214,7 @@ public function init(ViewExecutable $view, DisplayPluginBase $display, array &$o public function access(AccountInterface $account) { $base_table = $this->get_base_table(); $access_control_handler = $this->entityManager->getAccessControlHandler($this->definition['entity_tables'][$base_table]); - return $access_control_handler->fieldAccess('view', $this->getFieldDefinition(), $account); + return $access_control_handler->fieldAccess('view', $this->getFieldDefinition(), $account, NULL, TRUE); } /** diff --git a/modules/field/tests/modules/field_test/field_test.field.inc b/modules/field/tests/modules/field_test/field_test.field.inc index 6ebda87c18f..b1a55896eb1 100644 --- a/modules/field/tests/modules/field_test/field_test.field.inc +++ b/modules/field/tests/modules/field_test/field_test.field.inc @@ -5,6 +5,7 @@ * Defines a field type and its formatters and widgets. */ +use Drupal\Core\Access\AccessResult; use Drupal\Core\Entity\ContentEntityInterface; use Drupal\Core\Entity\Exception\FieldStorageDefinitionUpdateForbiddenException; use Drupal\Core\Field\FieldDefinitionInterface; @@ -40,14 +41,14 @@ function field_test_default_value(ContentEntityInterface $entity, FieldDefinitio */ function field_test_entity_field_access($operation, FieldDefinitionInterface $field_definition, AccountInterface $account, FieldItemListInterface $items = NULL) { if ($field_definition->getName() == "field_no_{$operation}_access") { - return FALSE; + return AccessResult::forbidden(); } // 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; + if ($field_definition->getName() == 'test_view_field' && $operation == 'view') { + return AccessResult::forbiddenIf(!$account->hasPermission('view test_view_field content'))->cachePerRole(); } - return TRUE; + return AccessResult::allowed(); } diff --git a/modules/field_ui/src/Access/FormModeAccessCheck.php b/modules/field_ui/src/Access/FormModeAccessCheck.php index d35dd15c2f6..1b2d95d006a 100644 --- a/modules/field_ui/src/Access/FormModeAccessCheck.php +++ b/modules/field_ui/src/Access/FormModeAccessCheck.php @@ -7,6 +7,7 @@ namespace Drupal\field_ui\Access; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; @@ -57,8 +58,8 @@ 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\AccessResultInterface + * The access result. */ public function access(Route $route, Request $request, AccountInterface $account, $form_mode_name = 'default', $bundle = NULL) { if ($entity_type_id = $route->getDefault('entity_type_id')) { @@ -75,13 +76,21 @@ public function access(Route $route, Request $request, AccountInterface $account $visibility = $entity_display->status(); } + $access = AccessResult::create(); + if ($form_mode_name != 'default' && $entity_display) { + $access->cacheUntilEntityChanges($entity_display); + } + if ($visibility) { $permission = $route->getRequirement('_field_ui_form_mode_access'); - return $account->hasPermission($permission) ? static::ALLOW : static::DENY; + $access->allowIfHasPermission($account, $permission); } + return $access; + } + else { + // No opinion. + return AccessResult::create(); } - - return static::DENY; } } diff --git a/modules/field_ui/src/Access/ViewModeAccessCheck.php b/modules/field_ui/src/Access/ViewModeAccessCheck.php index 8e930408e9b..3c9c2beba54 100644 --- a/modules/field_ui/src/Access/ViewModeAccessCheck.php +++ b/modules/field_ui/src/Access/ViewModeAccessCheck.php @@ -7,6 +7,7 @@ namespace Drupal\field_ui\Access; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; @@ -57,8 +58,8 @@ 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\AccessResultInterface + * The access result. */ public function access(Route $route, Request $request, AccountInterface $account, $view_mode_name = 'default', $bundle = NULL) { if ($entity_type_id = $route->getDefault('entity_type_id')) { @@ -75,13 +76,21 @@ public function access(Route $route, Request $request, AccountInterface $account $visibility = $entity_display->status(); } + $access = AccessResult::create(); + if ($view_mode_name != 'default' && $entity_display) { + $access->cacheUntilEntityChanges($entity_display); + } + if ($visibility) { $permission = $route->getRequirement('_field_ui_view_mode_access'); - return $account->hasPermission($permission) ? static::ALLOW : static::DENY; + $access->allowIfHasPermission($account, $permission); } + return $access; + } + else { + // No opinion. + return AccessResult::create(); } - - return static::DENY; } } diff --git a/modules/file/src/FileAccessControlHandler.php b/modules/file/src/FileAccessControlHandler.php index ee62c2277e4..cd87b193380 100644 --- a/modules/file/src/FileAccessControlHandler.php +++ b/modules/file/src/FileAccessControlHandler.php @@ -7,6 +7,7 @@ namespace Drupal\file; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Entity\EntityAccessControlHandler; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Entity\EntityStorageInterface; @@ -27,15 +28,17 @@ protected function checkAccess(EntityInterface $entity, $operation, $langcode, A 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_and_field_access = $referencing_entity->access('view', $account, TRUE)->andIf($referencing_entity->$field_name->access('view', $account, TRUE)); + if ($entity_and_field_access->isAllowed()) { + return $entity_and_field_access; } } } } - - return FALSE; } + + // No opinion. + return AccessResult::create(); } /** diff --git a/modules/filter/src/FilterFormatAccessControlHandler.php b/modules/filter/src/FilterFormatAccessControlHandler.php index 1a876b997e7..025cb870941 100644 --- a/modules/filter/src/FilterFormatAccessControlHandler.php +++ b/modules/filter/src/FilterFormatAccessControlHandler.php @@ -7,6 +7,7 @@ namespace Drupal\filter; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Entity\EntityAccessControlHandler; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Session\AccountInterface; @@ -26,23 +27,31 @@ protected function checkAccess(EntityInterface $filter_format, $operation, $lang // 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()) { + return AccessResult::allowed(); + } + else { + return AccessResult::allowedIfHasPermission($account, $filter_format->getPermissionName()); + } } // The fallback format may not be disabled. if ($operation == 'disable' && $filter_format->isFallbackFormat()) { - return FALSE; + return AccessResult::forbidden(); } // 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; + return AccessResult::forbidden(); } if (in_array($operation, array('disable', 'update'))) { return parent::checkAccess($filter_format, $operation, $langcode, $account); } + + // No opinion. + return AccessResult::create(); } } diff --git a/modules/filter/src/Tests/FilterFormatAccessTest.php b/modules/filter/src/Tests/FilterFormatAccessTest.php index 1882a265170..b3d7b5556a2 100644 --- a/modules/filter/src/Tests/FilterFormatAccessTest.php +++ b/modules/filter/src/Tests/FilterFormatAccessTest.php @@ -7,11 +7,13 @@ namespace Drupal\filter\Tests; +use Drupal\Core\Access\AccessResult; use Drupal\simpletest\WebTestBase; /** * Tests access to text formats. * + * @group Access * @group filter */ class FilterFormatAccessTest extends WebTestBase { @@ -121,8 +123,11 @@ function testFormatPermissions() { // 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->assertEqual(AccessResult::allowed()->cachePerRole(), $this->allowed_format->access('use', $this->web_user, TRUE), '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->assertEqual(AccessResult::create()->cachePerRole(), $this->disallowed_format->access('use', $this->web_user, TRUE), '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.'); + $this->assertEqual(AccessResult::allowed(), $fallback_format->access('use', $this->web_user, TRUE), '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/modules/language/src/LanguageAccessControlHandler.php b/modules/language/src/LanguageAccessControlHandler.php index 4adb778319f..e0d0aceb2e9 100644 --- a/modules/language/src/LanguageAccessControlHandler.php +++ b/modules/language/src/LanguageAccessControlHandler.php @@ -7,6 +7,7 @@ namespace Drupal\language; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Entity\EntityAccessControlHandler; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Session\AccountInterface; @@ -25,10 +26,13 @@ public function checkAccess(EntityInterface $entity, $operation, $langcode, Acco switch ($operation) { case 'update': case 'delete': - return !$entity->locked && parent::checkAccess($entity, $operation, $langcode, $account); - break; + return AccessResult::allowedIf(!$entity->locked)->cacheUntilEntityChanges($entity) + ->andIf(parent::checkAccess($entity, $operation, $langcode, $account)); + + default: + // No opinion. + return AccessResult::create(); } - return FALSE; } } diff --git a/modules/menu_link_content/src/MenuLinkContentAccessControlHandler.php b/modules/menu_link_content/src/MenuLinkContentAccessControlHandler.php index 4274a8e4f26..a7acb846b4e 100644 --- a/modules/menu_link_content/src/MenuLinkContentAccessControlHandler.php +++ b/modules/menu_link_content/src/MenuLinkContentAccessControlHandler.php @@ -6,6 +6,7 @@ namespace Drupal\menu_link_content; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Access\AccessManagerInterface; use Drupal\Core\Entity\EntityAccessControlHandler; use Drupal\Core\Entity\EntityHandlerInterface; @@ -54,14 +55,27 @@ protected function checkAccess(EntityInterface $entity, $operation, $langcode, A switch ($operation) { case 'view': // There is no direct view. - return FALSE; + return AccessResult::create(); 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)); + if (!$account->hasPermission('administer menu')) { + return AccessResult::create()->cachePerRole(); + } + else { + $access = AccessResult::create()->cachePerRole()->cacheUntilEntityChanges($entity); + // If there is a URL, this is an external link so always accessible. + if ($entity->getUrl()) { + return $access->allow(); + } + else { + // We allow access, but only if the link is accessible as well. + $link_access = $this->accessManager->checkNamedRoute($entity->getRouteName(), $entity->getRouteParameters(), $account, NULL, TRUE); + return $access->allow()->andIf($link_access); + } + } case 'delete': - return !$entity->isNew() && $account->hasPermission('administer menu'); + return AccessResult::allowedIf(!$entity->isNew() && $account->hasPermission('administer menu'))->cachePerRole()->cacheUntilEntityChanges($entity); } } diff --git a/modules/menu_ui/src/Form/MenuLinkResetForm.php b/modules/menu_ui/src/Form/MenuLinkResetForm.php index 74d0fa0b378..9c4c63b59df 100644 --- a/modules/menu_ui/src/Form/MenuLinkResetForm.php +++ b/modules/menu_ui/src/Form/MenuLinkResetForm.php @@ -7,12 +7,12 @@ namespace Drupal\menu_ui\Form; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Form\FormStateInterface; use Drupal\Core\Url; use Drupal\Core\Form\ConfirmFormBase; use Drupal\Core\Menu\MenuLinkManagerInterface; use Drupal\Core\Menu\MenuLinkInterface; -use Drupal\Core\Routing\Access\AccessInterface; use Symfony\Component\DependencyInjection\ContainerInterface; /** @@ -115,12 +115,11 @@ 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\AccessResultInterface + * The access result. */ public function linkIsResettable(MenuLinkInterface $menu_link_plugin) { - return $menu_link_plugin->isResettable() ? AccessInterface::ALLOW : AccessInterface::DENY; + return AccessResult::allowedIf($menu_link_plugin->isResettable())->setCacheable(FALSE); } } diff --git a/modules/node/node.api.php b/modules/node/node.api.php index 6fb311cfa2c..3fecffa4664 100644 --- a/modules/node/node.api.php +++ b/modules/node/node.api.php @@ -3,6 +3,7 @@ use Drupal\node\NodeInterface; use Drupal\Component\Utility\String; use Drupal\Component\Utility\Xss; +use Drupal\Core\Access\AccessResult; /** * @file @@ -294,9 +295,10 @@ 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 explicitly allow or forbid access, return an + * AccessResultInterface object with neither isAllowed() nor isForbidden() + * equaling TRUE. Blindly returning an object with isForbidden() equaling TRUE + * 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,34 +318,38 @@ 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\AccessResultInterface + * The access result. * * @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(); - if ($op == 'create' && $account->hasPermission('create ' . $type . ' content')) { - return NODE_ACCESS_ALLOW; - } + switch ($op) { + case 'create': + return AccessResult::allowedIfHasPermission($account, 'create ' . $type . ' content'); - 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; - } - } + case 'update': + if ($account->hasPermission('edit any ' . $type . ' content', $account)) { + return AccessResult::allowed()->cachePerRole(); + } + else { + return AccessResult::allowedIf($account->hasPermission('edit own ' . $type . ' content', $account) && ($account->id() == $node->getOwnerId()))->cachePerRole()->cachePerUser()->cacheUntilEntityChanges($node); + } - 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; - } - } + case 'delete': + if ($account->hasPermission('delete any ' . $type . ' content', $account)) { + return AccessResult::allowed()->cachePerRole(); + } + else { + return AccessResult::allowedIf($account->hasPermission('delete own ' . $type . ' content', $account) && ($account->id() == $node->getOwnerId()))->cachePerRole()->cachePerUser()->cacheUntilEntityChanges($node); + } - // Returning nothing from this function would have the same effect. - return NODE_ACCESS_IGNORE; + default: + // No opinion. + return AccessResult::create(); + } } /** diff --git a/modules/node/node.module b/modules/node/node.module index 030e62aeafa..e252d7e1b74 100644 --- a/modules/node/node.module +++ b/modules/node/node.module @@ -9,6 +9,7 @@ */ use Drupal\Component\Utility\Xss; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Form\FormStateInterface; use Drupal\Core\Language\LanguageInterface; use Drupal\Core\Render\Element; @@ -62,30 +63,6 @@ const NODE_NOT_STICKY = 0; */ 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(). */ @@ -1001,8 +978,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. * @@ -1026,11 +1003,13 @@ 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 an AccessResultInterface object from + * hook_node_access() whose isForbidden() method equals TRUE 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 an AccessResultInterface object whose isAllowed() nor + * isForbidden() methods return TRUE, 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. @@ -1042,23 +1021,30 @@ function node_form_system_themes_admin_form_submit($form, FormStateInterface $fo function node_node_access(NodeInterface $node, $op, $account) { $type = $node->bundle(); - if ($op == 'create' && $account->hasPermission('create ' . $type . ' content', $account)) { - return NODE_ACCESS_ALLOW; - } + switch ($op) { + case 'create': + return AccessResult::allowedIfHasPermission($account, 'create ' . $type . ' content'); - if ($op == 'update') { - if ($account->hasPermission('edit any ' . $type . ' content', $account) || ($account->hasPermission('edit own ' . $type . ' content', $account) && ($account->id() == $node->getOwnerId()))) { - return NODE_ACCESS_ALLOW; - } - } + case 'update': + if ($account->hasPermission('edit any ' . $type . ' content', $account)) { + return AccessResult::allowed()->cachePerRole(); + } + else { + return AccessResult::allowedIf($account->hasPermission('edit own ' . $type . ' content', $account) && ($account->id() == $node->getOwnerId()))->cachePerRole()->cachePerUser()->cacheUntilEntityChanges($node); + } - if ($op == 'delete') { - if ($account->hasPermission('delete any ' . $type . ' content', $account) || ($account->hasPermission('delete own ' . $type . ' content', $account) && ($account->id() == $node->getOwnerId()))) { - return NODE_ACCESS_ALLOW; - } - } + case 'delete': + if ($account->hasPermission('delete any ' . $type . ' content', $account)) { + return AccessResult::allowed()->cachePerRole(); + } + else { + return AccessResult::allowedIf($account->hasPermission('delete own ' . $type . ' content', $account) && ($account->id() == $node->getOwnerId()))->cachePerRole()->cachePerUser()->cacheUntilEntityChanges($node); + } - return NODE_ACCESS_IGNORE; + default: + // No opinion. + return AccessResult::create(); + } } /** diff --git a/modules/node/src/Access/NodeAddAccessCheck.php b/modules/node/src/Access/NodeAddAccessCheck.php index 89b1fd542c7..5d82dac135d 100644 --- a/modules/node/src/Access/NodeAddAccessCheck.php +++ b/modules/node/src/Access/NodeAddAccessCheck.php @@ -7,6 +7,7 @@ namespace Drupal\node\Access; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; @@ -50,15 +51,17 @@ public function access(AccountInterface $account, NodeTypeInterface $node_type = $access_control_handler = $this->entityManager->getAccessControlHandler('node'); // If checking whether a node of a particular type may be created. if ($node_type) { - return $access_control_handler->createAccess($node_type->id(), $account) ? static::ALLOW : static::DENY; + return $access_control_handler->createAccess($node_type->id(), $account, [], TRUE); } // If checking whether a node of any type may be created. foreach ($this->entityManager->getStorage('node_type')->loadMultiple() as $node_type) { - if ($access_control_handler->createAccess($node_type->id(), $account)) { - return static::ALLOW; + if (($access = $access_control_handler->createAccess($node_type->id(), $account, [], TRUE)) && $access->isAllowed()) { + return $access; } } - return static::DENY; + + // No opinion. + return AccessResult::create(); } } diff --git a/modules/node/src/Access/NodePreviewAccessCheck.php b/modules/node/src/Access/NodePreviewAccessCheck.php index 18bbcd185e5..a352b6b2d38 100644 --- a/modules/node/src/Access/NodePreviewAccessCheck.php +++ b/modules/node/src/Access/NodePreviewAccessCheck.php @@ -48,10 +48,10 @@ public function __construct(EntityManagerInterface $entity_manager) { public function access(AccountInterface $account, NodeInterface $node_preview) { if ($node_preview->isNew()) { $access_controller = $this->entityManager->getAccessControlHandler('node'); - return $access_controller->createAccess($node_preview->bundle(), $account) ? static::ALLOW : static::DENY; + return $access_controller->createAccess($node_preview->bundle(), $account, [], TRUE); } else { - return $node_preview->access('update', $account) ? static::ALLOW : static::DENY; + return $node_preview->access('update', $account, TRUE); } } diff --git a/modules/node/src/Access/NodeRevisionAccessCheck.php b/modules/node/src/Access/NodeRevisionAccessCheck.php index 3801e367eec..0fdc11530ed 100644 --- a/modules/node/src/Access/NodeRevisionAccessCheck.php +++ b/modules/node/src/Access/NodeRevisionAccessCheck.php @@ -7,6 +7,7 @@ namespace Drupal\node\Access; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Database\Connection; use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\Routing\Access\AccessInterface; @@ -77,15 +78,15 @@ 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\AccessResultInterface + * The access result. */ public function access(Route $route, AccountInterface $account, $node_revision = NULL, NodeInterface $node = NULL) { 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; + return AccessResult::allowedIf($node && $this->checkAccess($node, $account, $operation))->cachePerRole(); } /** diff --git a/modules/node/src/Entity/Node.php b/modules/node/src/Entity/Node.php index 03ccd913724..037475b8d0a 100644 --- a/modules/node/src/Entity/Node.php +++ b/modules/node/src/Entity/Node.php @@ -147,14 +147,14 @@ public function getType() { /** * {@inheritdoc} */ - public function access($operation = 'view', AccountInterface $account = NULL) { + public function access($operation = 'view', AccountInterface $account = NULL, $return_as_object = FALSE) { if ($operation == 'create') { - return parent::access($operation, $account); + return parent::access($operation, $account, $return_as_object); } return \Drupal::entityManager() ->getAccessControlHandler($this->entityTypeId) - ->access($this, $operation, $this->prepareLangcode(), $account); + ->access($this, $operation, $this->prepareLangcode(), $account, $return_as_object); } /** diff --git a/modules/node/src/NodeAccessControlHandler.php b/modules/node/src/NodeAccessControlHandler.php index af07c4cd14b..2268a6007a3 100644 --- a/modules/node/src/NodeAccessControlHandler.php +++ b/modules/node/src/NodeAccessControlHandler.php @@ -7,6 +7,7 @@ namespace Drupal\node; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Entity\EntityHandlerInterface; use Drupal\Core\Entity\EntityTypeInterface; use Drupal\Core\Field\FieldDefinitionInterface; @@ -27,7 +28,7 @@ class NodeAccessControlHandler extends EntityAccessControlHandler implements Nod /** * The node grant storage. * - * @var \Drupal\node\NodeGrantStorageInterface + * @var \Drupal\node\NodeGrantDatabaseStorageInterface */ protected $grantStorage; @@ -58,32 +59,38 @@ public static function createInstance(ContainerInterface $container, EntityTypeI /** * {@inheritdoc} */ - public function access(EntityInterface $entity, $operation, $langcode = LanguageInterface::LANGCODE_DEFAULT, AccountInterface $account = NULL) { + public function access(EntityInterface $entity, $operation, $langcode = LanguageInterface::LANGCODE_DEFAULT, AccountInterface $account = NULL, $return_as_object = FALSE) { $account = $this->prepareUser($account); if ($account->hasPermission('bypass node access')) { - return TRUE; + $result = AccessResult::allowed()->cachePerRole(); + return $return_as_object ? $result : $result->isAllowed(); } if (!$account->hasPermission('access content')) { - return FALSE; + $result = AccessResult::forbidden()->cachePerRole(); + return $return_as_object ? $result : $result->isAllowed(); } - return parent::access($entity, $operation, $langcode, $account); + $result = parent::access($entity, $operation, $langcode, $account, TRUE)->cachePerRole(); + return $return_as_object ? $result : $result->isAllowed(); } /** * {@inheritdoc} */ - public function createAccess($entity_bundle = NULL, AccountInterface $account = NULL, array $context = array()) { + public function createAccess($entity_bundle = NULL, AccountInterface $account = NULL, array $context = array(), $return_as_object = FALSE) { $account = $this->prepareUser($account); if ($account->hasPermission('bypass node access')) { - return TRUE; + $result = AccessResult::allowed()->cachePerRole(); + return $return_as_object ? $result : $result->isAllowed(); } if (!$account->hasPermission('access content')) { - return FALSE; + $result = AccessResult::forbidden()->cachePerRole(); + return $return_as_object ? $result : $result->isAllowed(); } - return parent::createAccess($entity_bundle, $account, $context); + $result = parent::createAccess($entity_bundle, $account, $context, TRUE)->cachePerRole(); + return $return_as_object ? $result : $result->isAllowed(); } /** @@ -98,31 +105,32 @@ protected function checkAccess(EntityInterface $node, $operation, $langcode, Acc $uid = $translation->getOwnerId(); // Check if authors can view their own unpublished nodes. - if ($operation === 'view' && !$status && $account->hasPermission('view own unpublished content')) { - - if ($account->id() != 0 && $account->id() == $uid) { - return TRUE; - } + if ($operation === 'view' && !$status && $account->hasPermission('view own unpublished content') && $account->isAuthenticated() && $account->id() == $uid) { + return AccessResult::allowed()->cachePerRole()->cachePerUser()->cacheUntilEntityChanges($node); } - // 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->isAllowed() || $grants->isForbidden()) { 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; + return AccessResult::allowedIf($status)->cacheUntilEntityChanges($node); } + + // No opinion. + return AccessResult::create(); } /** * {@inheritdoc} */ protected function checkCreateAccess(AccountInterface $account, array $context, $entity_bundle = NULL) { - return $account->hasPermission('create ' . $entity_bundle . ' content'); + return AccessResult::allowedIf($account->hasPermission('create ' . $entity_bundle . ' content'))->cachePerRole(); } /** @@ -133,22 +141,22 @@ protected function checkFieldAccess($operation, FieldDefinitionInterface $field_ // fields. $administrative_fields = array('uid', 'status', 'created', 'promote', 'sticky'); if ($operation == 'edit' && in_array($field_definition->getName(), $administrative_fields)) { - return $account->hasPermission('administer nodes'); + return AccessResult::allowedIfHasPermission($account, 'administer nodes'); } // No user can change read only fields. $read_only_fields = array('changed', 'revision_timestamp', 'revision_uid'); if ($operation == 'edit' && in_array($field_definition->getName(), $read_only_fields)) { - return FALSE; + return AccessResult::forbidden(); } // Users have access to the revision_log field either if they have // administrative permissions or if the new revision option is enabled. if ($operation == 'edit' && $field_definition->getName() == 'revision_log') { if ($account->hasPermission('administer nodes')) { - return TRUE; + return AccessResult::allowed()->cachePerRole(); } - return $items->getEntity()->type->entity->isNewRevision(); + return AccessResult::allowedIf($items->getEntity()->type->entity->isNewRevision())->cachePerRole(); } return parent::checkFieldAccess($operation, $field_definition, $account, $items); } diff --git a/modules/node/src/NodeGrantDatabaseStorage.php b/modules/node/src/NodeGrantDatabaseStorage.php index a47a7d9e61b..78b96b5ea7e 100644 --- a/modules/node/src/NodeGrantDatabaseStorage.php +++ b/modules/node/src/NodeGrantDatabaseStorage.php @@ -7,6 +7,7 @@ namespace Drupal\node; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Database\Connection; use Drupal\Core\Database\Query\SelectInterface; use Drupal\Core\Database\Query\Condition; @@ -57,7 +58,8 @@ public function access(NodeInterface $node, $operation, $langcode, AccountInterf // 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; + // No opinion. + return AccessResult::create(); } // Check the database for potential access grants. @@ -86,7 +88,17 @@ 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. + if ($query->execute()->fetchField()) { + return AccessResult::allowed()->setCacheable(FALSE); + } + else { + return AccessResult::forbidden()->setCacheable(FALSE); + } } /** diff --git a/modules/node/src/NodeGrantDatabaseStorageInterface.php b/modules/node/src/NodeGrantDatabaseStorageInterface.php index 54c31efdf13..e8653878515 100644 --- a/modules/node/src/NodeGrantDatabaseStorageInterface.php +++ b/modules/node/src/NodeGrantDatabaseStorageInterface.php @@ -105,10 +105,8 @@ 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 - * module implements hook_node_grants(), the node does not (yet) have an id - * or none of the implementing modules explicitly granted or denied access. + * @return \Drupal\Core\Access\AccessResultInterface + * The access result. */ public function access(NodeInterface $node, $operation, $langcode, AccountInterface $account); diff --git a/modules/node/src/NodeTypeAccessControlHandler.php b/modules/node/src/NodeTypeAccessControlHandler.php index e3dc594156c..b3589440891 100644 --- a/modules/node/src/NodeTypeAccessControlHandler.php +++ b/modules/node/src/NodeTypeAccessControlHandler.php @@ -7,6 +7,7 @@ namespace Drupal\node; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Entity\EntityAccessControlHandler; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Session\AccountInterface; @@ -22,8 +23,13 @@ class NodeTypeAccessControlHandler extends EntityAccessControlHandler { * {@inheritdoc} */ protected function checkAccess(EntityInterface $entity, $operation, $langcode, AccountInterface $account) { - if ($operation == 'delete' && $entity->isLocked()) { - return FALSE; + if ($operation == 'delete') { + if ($entity->isLocked()) { + return AccessResult::forbidden()->cacheUntilEntityChanges($entity); + } + else { + return parent::checkAccess($entity, $operation, $langcode, $account)->cacheUntilEntityChanges($entity); + } } return parent::checkAccess($entity, $operation, $langcode, $account); } diff --git a/modules/node/src/Plugin/Search/NodeSearch.php b/modules/node/src/Plugin/Search/NodeSearch.php index 8279d9e525e..bd51e98d468 100644 --- a/modules/node/src/Plugin/Search/NodeSearch.php +++ b/modules/node/src/Plugin/Search/NodeSearch.php @@ -9,6 +9,7 @@ use Drupal\Component\Utility\SafeMarkup; use Drupal\Component\Utility\String; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Config\Config; use Drupal\Core\Database\Connection; use Drupal\Core\Database\Query\SelectExtender; @@ -145,8 +146,9 @@ 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'); + public function access($operation = 'view', AccountInterface $account = NULL, $return_as_object = FALSE) { + $result = AccessResult::allowedIfHasPermission($account, 'access content'); + return $return_as_object ? $result : $result->isAllowed(); } /** diff --git a/modules/node/tests/modules/node_access_test/node_access_test.module b/modules/node/tests/modules/node_access_test/node_access_test.module index 797cf25fbbb..86fdbf13461 100644 --- a/modules/node/tests/modules/node_access_test/node_access_test.module +++ b/modules/node/tests/modules/node_access_test/node_access_test.module @@ -19,6 +19,7 @@ * @see \Drupal\node\Tests\NodeAccessBaseTableTest */ +use Drupal\Core\Access\AccessResult; use Drupal\field\Entity\FieldStorageConfig; use Drupal\field\Entity\FieldInstanceConfig; use Drupal\node\NodeTypeInterface; @@ -159,11 +160,12 @@ 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) { $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; + return AccessResult::forbidden()->setCacheable(FALSE); } - return NODE_ACCESS_IGNORE; + // No opinion. + return AccessResult::create()->setCacheable(FALSE); } diff --git a/modules/path/src/Plugin/Field/FieldType/PathFieldItemList.php b/modules/path/src/Plugin/Field/FieldType/PathFieldItemList.php index cf7ec2b13c5..20c3810b3a5 100644 --- a/modules/path/src/Plugin/Field/FieldType/PathFieldItemList.php +++ b/modules/path/src/Plugin/Field/FieldType/PathFieldItemList.php @@ -7,6 +7,7 @@ namespace Drupal\path\Plugin\Field\FieldType; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Field\FieldItemList; use Drupal\Core\Session\AccountInterface; @@ -20,9 +21,9 @@ class PathFieldItemList extends FieldItemList { */ public function defaultAccess($operation = 'view', AccountInterface $account = NULL) { if ($operation == 'view') { - return TRUE; + return AccessResult::allowed(); } - return $account->hasPermission('create url aliases') || $account->hasPermission('administer url aliases'); + return AccessResult::allowedIf($account->hasPermission('create url aliases') || $account->hasPermission('administer url aliases'))->cachePerRole(); } } diff --git a/modules/quickedit/src/Access/EditEntityFieldAccessCheck.php b/modules/quickedit/src/Access/EditEntityFieldAccessCheck.php index 9edb49ec69a..813ba45d0b3 100644 --- a/modules/quickedit/src/Access/EditEntityFieldAccessCheck.php +++ b/modules/quickedit/src/Access/EditEntityFieldAccessCheck.php @@ -7,6 +7,7 @@ namespace Drupal\quickedit\Access; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; use Drupal\Core\Entity\EntityInterface; @@ -28,24 +29,24 @@ class EditEntityFieldAccessCheck implements AccessInterface, EditEntityFieldAcce * @param \Drupal\Core\Session\AccountInterface $account * The currently logged in account. * - * @return string - * A \Drupal\Core\Access\AccessInterface constant value. + * @return \Drupal\Core\Access\AccessResultInterface + * The access result. * * @todo Use the $account argument: https://drupal.org/node/2266809. */ public function access(EntityInterface $entity, $field_name, $langcode, AccountInterface $account) { if (!$this->validateRequestAttributes($entity, $field_name, $langcode)) { - return static::KILL; + return AccessResult::forbidden(); } - return $this->accessEditEntityField($entity, $field_name) ? static::ALLOW : static::DENY; + return $this->accessEditEntityField($entity, $field_name); } /** * {@inheritdoc} */ public function accessEditEntityField(EntityInterface $entity, $field_name) { - return $entity->access('update') && $entity->get($field_name)->access('edit'); + return $entity->access('update', NULL, TRUE)->andIf($entity->get($field_name)->access('edit', NULL, TRUE)); } /** diff --git a/modules/quickedit/src/Access/EditEntityFieldAccessCheckInterface.php b/modules/quickedit/src/Access/EditEntityFieldAccessCheckInterface.php index f89c126f665..dc4ac70bbc0 100644 --- a/modules/quickedit/src/Access/EditEntityFieldAccessCheckInterface.php +++ b/modules/quickedit/src/Access/EditEntityFieldAccessCheckInterface.php @@ -16,6 +16,14 @@ interface EditEntityFieldAccessCheckInterface { /** * 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\AccessResultInterface + * The access result. */ public function accessEditEntityField(EntityInterface $entity, $field_name); diff --git a/modules/quickedit/tests/src/Unit/Access/EditEntityFieldAccessCheckTest.php b/modules/quickedit/tests/src/Unit/Access/EditEntityFieldAccessCheckTest.php index 4bc7a1a1ae3..7f5c4aa0b10 100644 --- a/modules/quickedit/tests/src/Unit/Access/EditEntityFieldAccessCheckTest.php +++ b/modules/quickedit/tests/src/Unit/Access/EditEntityFieldAccessCheckTest.php @@ -7,7 +7,7 @@ namespace Drupal\Tests\quickedit\Unit\Access; -use Drupal\Core\Access\AccessCheckInterface; +use Drupal\Core\Access\AccessResult; use Drupal\quickedit\Access\EditEntityFieldAccessCheck; use Drupal\Tests\UnitTestCase; use Drupal\field\FieldStorageConfigInterface; @@ -16,6 +16,7 @@ /** * @coversDefaultClass \Drupal\quickedit\Access\EditEntityFieldAccessCheck + * @group Access * @group quickedit */ class EditEntityFieldAccessCheckTest extends UnitTestCase { @@ -43,31 +44,31 @@ public function providerTestAccess() { $editable_entity = $this->createMockEntity(); $editable_entity->expects($this->any()) ->method('access') - ->will($this->returnValue(TRUE)); + ->will($this->returnValue(AccessResult::allowed()->cachePerRole())); $non_editable_entity = $this->createMockEntity(); $non_editable_entity->expects($this->any()) ->method('access') - ->will($this->returnValue(FALSE)); + ->will($this->returnValue(AccessResult::create()->cachePerRole())); $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(AccessResult::allowed())); $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(AccessResult::create())); $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, AccessResult::allowed()->cachePerRole()); + $data[] = array($non_editable_entity, $field_storage_with_access, AccessResult::create()->cachePerRole()); + $data[] = array($editable_entity, $field_storage_without_access, AccessResult::create()->cachePerRole()); + $data[] = array($non_editable_entity, $field_storage_without_access, AccessResult::create()->cachePerRole()); return $data; } @@ -98,24 +99,24 @@ public function testAccess(EntityInterface $entity, FieldStorageConfigInterface $account = $this->getMock('Drupal\Core\Session\AccountInterface'); $access = $this->editAccessCheck->access($entity_with_field, $field_name, LanguageInterface::LANGCODE_NOT_SPECIFIED, $account); - $this->assertSame($expected_result, $access); + $this->assertEquals($expected_result, $access); } /** - * Tests checking access to routes that result in AccessCheckInterface::KILL. + * Tests checking access to routes that result in AccessResult::isForbidden(). * - * @dataProvider providerTestAccessKill + * @dataProvider providerTestAccessForbidden */ - public function testAccessKill($field_name, $langcode) { + public function testAccessForbidden($field_name, $langcode) { $account = $this->getMock('Drupal\Core\Session\AccountInterface'); $entity = $this->createMockEntity(); - $this->assertSame(AccessCheckInterface::KILL, $this->editAccessCheck->access($entity, $field_name, $langcode, $account)); + $this->assertEquals(AccessResult::forbidden(), $this->editAccessCheck->access($entity, $field_name, $langcode, $account)); } /** - * Provides test data for testAccessKill. + * Provides test data for testAccessForbidden. */ - public function providerTestAccessKill() { + public function providerTestAccessForbidden() { $data = array(); // Tests the access method without a field_name. $data[] = array(NULL, LanguageInterface::LANGCODE_NOT_SPECIFIED); diff --git a/modules/rest/src/Access/CSRFAccessCheck.php b/modules/rest/src/Access/CSRFAccessCheck.php index 08667db8030..563a0a4cdec 100644 --- a/modules/rest/src/Access/CSRFAccessCheck.php +++ b/modules/rest/src/Access/CSRFAccessCheck.php @@ -8,6 +8,7 @@ namespace Drupal\rest\Access; use Drupal\Core\Access\AccessCheckInterface; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Session\AccountInterface; use Symfony\Component\Routing\Route; use Symfony\Component\HttpFoundation\Request; @@ -48,8 +49,8 @@ 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\AccessResultInterface + * The access result. */ public function access(Request $request, AccountInterface $account) { $method = $request->getMethod(); @@ -65,10 +66,10 @@ 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; + return AccessResult::forbidden()->setCacheable(FALSE); } } // Let other access checkers decide if the request is legit. - return static::ALLOW; + return AccessResult::allowed()->setCacheable(FALSE); } } diff --git a/modules/search/src/SearchPageAccessControlHandler.php b/modules/search/src/SearchPageAccessControlHandler.php index 65723b695de..a0b9a978b40 100644 --- a/modules/search/src/SearchPageAccessControlHandler.php +++ b/modules/search/src/SearchPageAccessControlHandler.php @@ -7,6 +7,7 @@ namespace Drupal\search; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Access\AccessibleInterface; use Drupal\Core\Entity\EntityAccessControlHandler; use Drupal\Core\Entity\EntityInterface; @@ -24,18 +25,23 @@ class SearchPageAccessControlHandler extends EntityAccessControlHandler { */ protected function checkAccess(EntityInterface $entity, $operation, $langcode, AccountInterface $account) { /** @var $entity \Drupal\search\SearchPageInterface */ - if (in_array($operation, array('delete', 'disable')) && $entity->isDefaultSearch()) { - return FALSE; + if (in_array($operation, array('delete', 'disable'))) { + if ($entity->isDefaultSearch()) { + return AccessResult::forbidden()->cacheUntilEntityChanges($entity); + } + else { + return parent::checkAccess($entity, $operation, $langcode, $account)->cacheUntilEntityChanges($entity); + } } if ($operation == 'view') { if (!$entity->status()) { - return FALSE; + return AccessResult::forbidden()->cacheUntilEntityChanges($entity); } $plugin = $entity->getPlugin(); if ($plugin instanceof AccessibleInterface) { - return $plugin->access($operation, $account); + return $plugin->access($operation, $account, TRUE)->cacheUntilEntityChanges($entity); } - return TRUE; + return AccessResult::allowed()->cacheUntilEntityChanges($entity); } return parent::checkAccess($entity, $operation, $langcode, $account); } diff --git a/modules/shortcut/shortcut.module b/modules/shortcut/shortcut.module index 13c827bc754..49548877744 100644 --- a/modules/shortcut/shortcut.module +++ b/modules/shortcut/shortcut.module @@ -5,6 +5,7 @@ * Allows users to manage customizable lists of shortcut links. */ +use Drupal\Core\Access\AccessResult; use Drupal\Component\Utility\NestedArray; use Drupal\Component\Utility\UrlHelper; use Drupal\Core\Routing\RouteMatchInterface; @@ -73,27 +74,21 @@ 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\AccessResultInterface + * The access result. */ function shortcut_set_edit_access(ShortcutSetInterface $shortcut_set = NULL) { $account = \Drupal::currentUser(); // Shortcut administrators can edit any set. if ($account->hasPermission('administer shortcuts')) { - return TRUE; - } - // Access to shortcuts is required for non-administrators. - if (!$account->hasPermission('access shortcuts')) { - return FALSE; + return AccessResult::allowed()->cachePerRole(); } + // 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(); - } - return FALSE; + // set, but not other sets. They must also be able to access shortcuts. + $may_edit_current_shortcut_set = $account->hasPermission('customize shortcut links') && (!isset($shortcut_set) || $shortcut_set == shortcut_current_displayed_set()) && $account->hasPermission('access shortcuts'); + return AccessResult::allowedIf($may_edit_current_shortcut_set)->cachePerRole(); } /** @@ -104,35 +99,38 @@ 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\AccessResultInterface + * The access result. */ function shortcut_set_switch_access($account = NULL) { $user = \Drupal::currentUser(); if ($user->hasPermission('administer shortcuts')) { // Administrators can switch anyone's shortcut set. - return TRUE; + return AccessResult::allowed()->cachePerRole(); } if (!$user->hasPermission('access shortcuts')) { // The user has no permission to use shortcuts. - return FALSE; + return AccessResult::create()->cachePerRole(); } if (!$user->hasPermission('switch shortcut sets')) { // The user has no permission to switch anyone's shortcut set. - return FALSE; + return AccessResult::create()->cachePerRole(); } - 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)) { + return AccessResult::allowed()->cachePerRole(); + } + else if ($user->id() == $account->id()) { + return AccessResult::allowed()->cachePerRole()->cachePerUser(); } - return FALSE; + // No opinion. + return AccessResult::create()->cachePerRole(); } /** @@ -320,7 +318,7 @@ function shortcut_preprocess_page(&$variables) { // shortcuts and if the page's actual content is being shown (for example, // we do not want to display it on "access denied" or "page not found" // pages). - if (shortcut_set_edit_access() && !\Drupal::request()->attributes->has('exception')) { + if (shortcut_set_edit_access()->isAllowed() && !\Drupal::request()->attributes->has('exception')) { $link = current_path(); if (!($url = \Drupal::pathValidator()->getUrlIfValid($link))) { // Bail out early if we couldn't find a matching route. @@ -346,13 +344,13 @@ function shortcut_preprocess_page(&$variables) { $link_mode = isset($shortcut_id) ? "remove" : "add"; 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 = shortcut_set_switch_access()->isAllowed() ? 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 = shortcut_set_switch_access()->isAllowed() ? t('Remove from %shortcut_set shortcuts', array('%shortcut_set' => $shortcut_set->label())) : t('Remove from shortcuts'); $route_name = 'entity.shortcut.link_delete_inline'; $route_parameters = array('shortcut' => $shortcut_id); } @@ -387,7 +385,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)->isAllowed()) { $configure_link = array( '#type' => 'link', '#title' => t('Edit shortcuts'), diff --git a/modules/shortcut/src/Form/SwitchShortcutSet.php b/modules/shortcut/src/Form/SwitchShortcutSet.php index c4d065a1668..496fa0765ef 100644 --- a/modules/shortcut/src/Form/SwitchShortcutSet.php +++ b/modules/shortcut/src/Form/SwitchShortcutSet.php @@ -8,7 +8,6 @@ namespace Drupal\shortcut\Form; use Drupal\Component\Utility\String; -use Drupal\Core\Access\AccessInterface; use Drupal\Core\Form\FormBase; use Drupal\Core\Form\FormStateInterface; use Drupal\Core\Routing\RouteMatchInterface; @@ -237,34 +236,11 @@ 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\AccessResultInterface + * The access result. */ public function checkAccess(UserInterface $user = NULL) { - $account = $this->currentUser(); - $this->user = $user; - - if ($account->hasPermission('administer shortcuts')) { - // Administrators can switch anyone's shortcut set. - return AccessInterface::ALLOW; - } - - if (!$account->hasPermission('access shortcuts')) { - // The user has no permission to use shortcuts. - return AccessInterface::DENY; - } - - if (!$account->hasPermission('switch shortcut sets')) { - // The user has no permission to switch anyone's shortcut set. - return AccessInterface::DENY; - } - - if ($this->user->id() == $account->id()) { - // Users with the 'switch shortcut sets' permission can switch their own - // shortcuts sets. - return AccessInterface::ALLOW; - } - return AccessInterface::DENY; + return shortcut_set_switch_access($user); } } diff --git a/modules/shortcut/src/ShortcutAccessControlHandler.php b/modules/shortcut/src/ShortcutAccessControlHandler.php index d07a2f5a280..aee649c6b24 100644 --- a/modules/shortcut/src/ShortcutAccessControlHandler.php +++ b/modules/shortcut/src/ShortcutAccessControlHandler.php @@ -7,6 +7,7 @@ namespace Drupal\shortcut; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Entity\EntityAccessControlHandler; use Drupal\Core\Entity\EntityHandlerInterface; use Drupal\Core\Entity\EntityInterface; @@ -58,6 +59,9 @@ 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. See https://www.drupal.org/node/2339903. + return AccessResult::create()->cacheUntilEntityChanges($entity); } /** @@ -67,6 +71,9 @@ 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. See https://www.drupal.org/node/2339903. + return AccessResult::create(); } } diff --git a/modules/shortcut/src/ShortcutSetAccessControlHandler.php b/modules/shortcut/src/ShortcutSetAccessControlHandler.php index b6ca7c6344a..4968f040f6e 100644 --- a/modules/shortcut/src/ShortcutSetAccessControlHandler.php +++ b/modules/shortcut/src/ShortcutSetAccessControlHandler.php @@ -7,6 +7,7 @@ namespace Drupal\shortcut; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Entity\EntityAccessControlHandler; use Drupal\Core\Session\AccountInterface; @@ -25,23 +26,19 @@ protected function checkAccess(EntityInterface $entity, $operation, $langcode, A switch ($operation) { case 'update': if ($account->hasPermission('administer shortcuts')) { - return TRUE; + return AccessResult::allowed()->cachePerRole(); } if (!$account->hasPermission('access shortcuts')) { - return FALSE; + return AccessResult::create()->cachePerRole(); } - if ($account->hasPermission('customize shortcut links')) { - return $entity == shortcut_current_displayed_set($account); - } - return FALSE; - break; + return AccessResult::allowedIf($account->hasPermission('customize shortcut links') && $entity == shortcut_current_displayed_set($account))->cachePerRole()->cacheUntilEntityChanges($entity); case 'delete': - if (!$account->hasPermission('administer shortcuts')) { - return FALSE; - } - return $entity->id() != 'default'; - break; + return AccessResult::allowedIf($account->hasPermission('administer shortcuts') && $entity->id() != 'default')->cachePerRole(); + + default: + // No opinion. + return AccessResult::create(); } } @@ -49,15 +46,8 @@ protected function checkAccess(EntityInterface $entity, $operation, $langcode, A * {@inheritdoc} */ protected function checkCreateAccess(AccountInterface $account, array $context, $entity_bundle = NULL) { - if ($account->hasPermission('administer shortcuts')) { - return TRUE; - } - if (!$account->hasPermission('access shortcuts')) { - return FALSE; - } - if ($account->hasPermission('customize shortcut links')) { - return TRUE; - } + $condition = $account->hasPermission('administer shortcuts') || ($account->hasPermission('access shortcuts') && $account->hasPermission('customize shortcut links')); + return AccessResult::allowedIf($condition)->cachePerRole(); } } diff --git a/modules/system/entity.api.php b/modules/system/entity.api.php index 137407ca903..09860a614a2 100644 --- a/modules/system/entity.api.php +++ b/modules/system/entity.api.php @@ -6,6 +6,7 @@ */ use Drupal\Component\Utility\String; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Entity\ContentEntityInterface; use Drupal\Core\Field\BaseFieldDefinition; use Drupal\Core\Render\Element; @@ -520,9 +521,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\AccessResultInterface + * The access result. * * @see \Drupal\Core\Entity\EntityAccessControlHandler * @see hook_entity_create_access() @@ -531,7 +531,8 @@ * @ingroup entity_api */ function hook_entity_access(\Drupal\Core\Entity\EntityInterface $entity, $operation, \Drupal\Core\Session\AccountInterface $account, $langcode) { - return NULL; + // No opinion. + return AccessResult::create(); } /** @@ -546,9 +547,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\AccessResultInterface + * The access result. * * @see \Drupal\Core\Entity\EntityAccessControlHandler * @see hook_ENTITY_TYPE_create_access() @@ -557,7 +557,8 @@ 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; + // No opinion. + return AccessResult::create(); } /** @@ -572,9 +573,8 @@ function hook_ENTITY_TYPE_access(\Drupal\Core\Entity\EntityInterface $entity, $o * @param string $entity_bundle * The entity bundle name. * - * @return bool|null - * A boolean to explicitly allow or deny access, or NULL to neither allow nor - * deny access. + * @return \Drupal\Core\Access\AccessResultInterface + * The access result. * * @see \Drupal\Core\Entity\EntityAccessControlHandler * @see hook_entity_access() @@ -583,7 +583,8 @@ function hook_ENTITY_TYPE_access(\Drupal\Core\Entity\EntityInterface $entity, $o * @ingroup entity_api */ function hook_entity_create_access(\Drupal\Core\Session\AccountInterface $account, array $context, $entity_bundle) { - return NULL; + // No opinion. + return AccessResult::create(); } /** @@ -598,9 +599,8 @@ function hook_entity_create_access(\Drupal\Core\Session\AccountInterface $accoun * @param string $entity_bundle * The entity bundle name. * - * @return bool|null - * A boolean to explicitly allow or deny access, or NULL to neither allow nor - * deny access. + * @return \Drupal\Core\Access\AccessResultInterface + * The access result. * * @see \Drupal\Core\Entity\EntityAccessControlHandler * @see hook_ENTITY_TYPE_access() @@ -609,7 +609,8 @@ function hook_entity_create_access(\Drupal\Core\Session\AccountInterface $accoun * @ingroup entity_api */ function hook_ENTITY_TYPE_create_access(\Drupal\Core\Session\AccountInterface $account, array $context, $entity_bundle) { - return NULL; + // No opinion. + return AccessResult::create(); } /** @@ -1828,13 +1829,12 @@ 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\AccessResultInterface + * The access result. */ 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'); + return AccessResult::allowedIfHasPermission($account, 'update field of interest'); } } @@ -1844,10 +1844,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\AccessResultInterface[] $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\AccessResult). * @param array $context * Context array on the performed operation with the following keys: * - operation: The operation to be performed (string). @@ -1861,13 +1861,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) { - // 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; + if ($field_definition->getName() == 'field_of_interest' && $grants['node']->isForbidden()) { + // Override node module's restriction to no opinion (neither allowed nor + // forbidden). 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 + // AccessResultInterface::isAllowed() , because the grants of other modules + // should still decide on their own if this field is accessible or not + $grants['node']->resetAccess(); } } diff --git a/modules/system/src/Access/CronAccessCheck.php b/modules/system/src/Access/CronAccessCheck.php index a5cbb885605..acb1401bea9 100644 --- a/modules/system/src/Access/CronAccessCheck.php +++ b/modules/system/src/Access/CronAccessCheck.php @@ -7,6 +7,7 @@ namespace Drupal\system\Access; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Routing\Access\AccessInterface; /** @@ -20,18 +21,18 @@ class CronAccessCheck implements AccessInterface { * @param string $key * The cron key. * - * @return string - * A \Drupal\Core\Access\AccessInterface constant value. + * @return \Drupal\Core\Access\AccessResultInterface + * The access result. */ public function access($key) { if ($key != \Drupal::state()->get('system.cron_key')) { \Drupal::logger('cron')->notice('Cron could not run because an invalid key was used.'); - return static::KILL; + return AccessResult::forbidden()->setCacheable(FALSE); } elseif (\Drupal::state()->get('system.maintenance_mode')) { \Drupal::logger('cron')->notice('Cron could not run because the site is in maintenance mode.'); - return static::KILL; + return AccessResult::forbidden()->setCacheable(FALSE); } - return static::ALLOW; + return AccessResult::allowed()->setCacheable(FALSE); } } diff --git a/modules/system/src/Access/DbUpdateAccessCheck.php b/modules/system/src/Access/DbUpdateAccessCheck.php index 0d37db19637..5225a189bcb 100644 --- a/modules/system/src/Access/DbUpdateAccessCheck.php +++ b/modules/system/src/Access/DbUpdateAccessCheck.php @@ -7,7 +7,8 @@ namespace Drupal\system\Access; -use Drupal\Core\Access\AccessInterface; +use Drupal\Core\Routing\Access\AccessInterface; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Session\AccountInterface; use Drupal\Core\Site\Settings; @@ -28,9 +29,14 @@ class DbUpdateAccessCheck implements AccessInterface { public function access(AccountInterface $account) { // Allow the global variable in settings.php to override the access check. if (Settings::get('update_free_access')) { - return static::ALLOW; + return AccessResult::allowed()->setCacheable(FALSE); } - return $account->hasPermission('administer software updates') ? static::ALLOW : static::KILL; + if ($account->hasPermission('administer software updates')) { + return AccessResult::allowed()->cachePerRole(); + } + else { + return AccessResult::forbidden()->cachePerRole(); + } } } diff --git a/modules/system/src/DateFormatAccessControlHandler.php b/modules/system/src/DateFormatAccessControlHandler.php index 943b678dcb0..83009fc02f6 100644 --- a/modules/system/src/DateFormatAccessControlHandler.php +++ b/modules/system/src/DateFormatAccessControlHandler.php @@ -7,6 +7,7 @@ namespace Drupal\system; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Entity\EntityAccessControlHandler; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Session\AccountInterface; @@ -24,11 +25,16 @@ class DateFormatAccessControlHandler extends EntityAccessControlHandler { protected function checkAccess(EntityInterface $entity, $operation, $langcode, AccountInterface $account) { // There are no restrictions on viewing a date format. if ($operation == 'view') { - return TRUE; + return AccessResult::allowed(); } // Locked date formats cannot be updated or deleted. - elseif (in_array($operation, array('update', 'delete')) && $entity->isLocked()) { - return FALSE; + elseif (in_array($operation, array('update', 'delete'))) { + if ($entity->isLocked()) { + return AccessResult::forbidden()->cacheUntilEntityChanges($entity); + } + else { + return parent::checkAccess($entity, $operation, $langcode, $account)->cacheUntilEntityChanges($entity); + } } return parent::checkAccess($entity, $operation, $langcode, $account); diff --git a/modules/system/src/MenuAccessControlHandler.php b/modules/system/src/MenuAccessControlHandler.php index acee9aad3bd..fba0183908e 100644 --- a/modules/system/src/MenuAccessControlHandler.php +++ b/modules/system/src/MenuAccessControlHandler.php @@ -7,6 +7,7 @@ namespace Drupal\system; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Entity\EntityAccessControlHandler; use Drupal\Core\Session\AccountInterface; @@ -23,11 +24,16 @@ class MenuAccessControlHandler extends EntityAccessControlHandler { */ protected function checkAccess(EntityInterface $entity, $operation, $langcode, AccountInterface $account) { if ($operation === 'view') { - return TRUE; + return AccessResult::allowed(); } // Locked menus could not be deleted. - elseif ($operation == 'delete' && $entity->isLocked()) { - return FALSE; + elseif ($operation == 'delete') { + if ($entity->isLocked()) { + return AccessResult::forbidden()->cacheUntilEntityChanges($entity); + } + else { + return parent::checkAccess($entity, $operation, $langcode, $account)->cacheUntilEntityChanges($entity); + } } return parent::checkAccess($entity, $operation, $langcode, $account); diff --git a/modules/system/src/Tests/Entity/EntityViewBuilderTest.php b/modules/system/src/Tests/Entity/EntityViewBuilderTest.php index 65355fed2cd..c72c8c25854 100644 --- a/modules/system/src/Tests/Entity/EntityViewBuilderTest.php +++ b/modules/system/src/Tests/Entity/EntityViewBuilderTest.php @@ -86,7 +86,9 @@ public function testEntityViewBuilderCacheWithReferences() { // Create an entity reference field and an entity that will be referenced. entity_reference_create_instance('entity_test', 'entity_test', 'reference_field', 'Reference', 'entity_test'); - entity_get_display('entity_test', 'entity_test', 'full')->setComponent('reference_field')->save(); + entity_get_display('entity_test', 'entity_test', 'full')->setComponent('reference_field', [ + 'settings' => ['link' => FALSE], + ])->save(); $entity_test_reference = $this->createTestEntity('entity_test'); $entity_test_reference->save(); diff --git a/modules/system/src/Tests/Entity/FieldAccessTest.php b/modules/system/src/Tests/Entity/FieldAccessTest.php index 30c874b1d4d..e87eb44e0d7 100644 --- a/modules/system/src/Tests/Entity/FieldAccessTest.php +++ b/modules/system/src/Tests/Entity/FieldAccessTest.php @@ -7,6 +7,7 @@ namespace Drupal\system\Tests\Entity; +use Drupal\Core\Access\AccessResult; use Drupal\simpletest\DrupalUnitTestBase; /** @@ -66,11 +67,15 @@ function testFieldAccess() { $account = entity_create('user', $values); $this->assertFalse($entity->field_test_text->access('view', $account), 'Access to the field was denied.'); + $expected = AccessResult::forbidden()->cacheUntilEntityChanges($entity); + $this->assertEqual($expected, $entity->field_test_text->access('view', $account, TRUE), '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, TRUE), '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.'); + $this->assertEqual(AccessResult::allowed(), $entity->field_test_text->access('view', $account, TRUE), 'Access to the field was granted.'); } } diff --git a/modules/system/tests/modules/entity_test/entity_test.module b/modules/system/tests/modules/entity_test/entity_test.module index b808270eb9c..835b9d5e6d1 100644 --- a/modules/system/tests/modules/entity_test/entity_test.module +++ b/modules/system/tests/modules/entity_test/entity_test.module @@ -5,6 +5,7 @@ * Test module for the entity API providing several entity types for testing. */ +use Drupal\Core\Access\AccessResult; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Entity\ContentEntityInterface; use Drupal\Core\Entity\EntityTypeInterface; @@ -337,13 +338,15 @@ function entity_test_entity_field_access($operation, FieldDefinitionInterface $f if ($field_definition->getName() == 'field_test_text') { if ($items) { if ($items[0]->value == 'no access value') { - return FALSE; + return AccessResult::forbidden()->cacheUntilEntityChanges($items->getEntity()); } elseif ($operation == 'delete' && $items[0]->value == 'no delete access value') { - return FALSE; + return AccessResult::forbidden()->cacheUntilEntityChanges($items->getEntity()); } } } + // No opinion. + return AccessResult::create(); } /** @@ -353,7 +356,7 @@ 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; + $grants[':default']->forbid()->cacheUntilEntityChanges($context['items']->getEntity()); } } @@ -486,7 +489,11 @@ 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 result depends on a State key-value pair and + // might therefore change at any time. + $condition = \Drupal::state()->get("entity_test_entity_access.{$operation}." . $entity->id(), FALSE); + return AccessResult::allowedIf($condition)->setCacheable(FALSE); } /** @@ -494,6 +501,9 @@ 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. + return AccessResult::create(); } /** @@ -501,6 +511,9 @@ function entity_test_entity_test_access(EntityInterface $entity, $operation, Acc */ function entity_test_entity_create_access(AccountInterface $account, $context, $entity_bundle) { \Drupal::state()->set('entity_test_entity_create_access', TRUE); + + // No opinion. + return AccessResult::create(); } /** @@ -508,4 +521,7 @@ function entity_test_entity_create_access(AccountInterface $account, $context, $ */ function entity_test_entity_test_create_access(AccountInterface $account, $context, $entity_bundle) { \Drupal::state()->set('entity_test_entity_test_create_access', TRUE); + + // No opinion. + return AccessResult::create(); } diff --git a/modules/system/tests/modules/entity_test/src/EntityTestAccessControlHandler.php b/modules/system/tests/modules/entity_test/src/EntityTestAccessControlHandler.php index d16f6abe980..95efd80a983 100644 --- a/modules/system/tests/modules/entity_test/src/EntityTestAccessControlHandler.php +++ b/modules/system/tests/modules/entity_test/src/EntityTestAccessControlHandler.php @@ -7,6 +7,7 @@ namespace Drupal\entity_test; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Entity\EntityAccessControlHandler; use Drupal\Core\Language\LanguageInterface; @@ -31,20 +32,24 @@ class EntityTestAccessControlHandler extends EntityAccessControlHandler { protected function checkAccess(EntityInterface $entity, $operation, $langcode, AccountInterface $account) { if ($operation === 'view') { if ($langcode != LanguageInterface::LANGCODE_DEFAULT) { - return $account->hasPermission('view test entity translations'); + return AccessResult::allowedIfHasPermission($account, 'view test entity translations'); } - return $account->hasPermission('view test entity'); + return AccessResult::allowedIfHasPermission($account, 'view test entity'); } elseif (in_array($operation, array('update', 'delete'))) { - return $account->hasPermission('administer entity_test content'); + return AccessResult::allowedIfHasPermission($account, 'administer entity_test content'); } + + // No opinion. + return AccessResult::create(); + } /** * {@inheritdoc} */ protected function checkCreateAccess(AccountInterface $account, array $context, $entity_bundle = NULL) { - return $account->hasPermission('administer entity_test content'); + return AccessResult::allowedIfHasPermission($account, 'administer entity_test content'); } } diff --git a/modules/system/tests/modules/router_test_directory/src/Access/DefinedTestAccessCheck.php b/modules/system/tests/modules/router_test_directory/src/Access/DefinedTestAccessCheck.php index 1469d20ead6..b435a398e8b 100644 --- a/modules/system/tests/modules/router_test_directory/src/Access/DefinedTestAccessCheck.php +++ b/modules/system/tests/modules/router_test_directory/src/Access/DefinedTestAccessCheck.php @@ -7,6 +7,7 @@ namespace Drupal\router_test\Access; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Routing\Access\AccessInterface; use Symfony\Component\Routing\Route; @@ -21,18 +22,18 @@ 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\AccessResultInterface + * The access result. */ public function access(Route $route) { if ($route->getRequirement('_test_access') === 'TRUE') { - return static::ALLOW; + return AccessResult::allowed(); } elseif ($route->getRequirement('_test_access') === 'FALSE') { - return static::KILL; + return AccessResult::forbidden(); } else { - return static::DENY; + return AccessResult::create(); } } diff --git a/modules/system/tests/modules/router_test_directory/src/Access/TestAccessCheck.php b/modules/system/tests/modules/router_test_directory/src/Access/TestAccessCheck.php index d8af2c66cdf..45ab4b5237f 100644 --- a/modules/system/tests/modules/router_test_directory/src/Access/TestAccessCheck.php +++ b/modules/system/tests/modules/router_test_directory/src/Access/TestAccessCheck.php @@ -7,6 +7,7 @@ namespace Drupal\router_test\Access; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Routing\Access\AccessInterface; /** @@ -23,6 +24,6 @@ class TestAccessCheck implements AccessInterface { public function access() { // No opinion, so other access checks should decide if access should be // allowed or not. - return static::DENY; + return AccessResult::create(); } } diff --git a/modules/system/tests/src/Unit/Breadcrumbs/PathBasedBreadcrumbBuilderTest.php b/modules/system/tests/src/Unit/Breadcrumbs/PathBasedBreadcrumbBuilderTest.php index 5d24f20bad2..0dc1b191589 100644 --- a/modules/system/tests/src/Unit/Breadcrumbs/PathBasedBreadcrumbBuilderTest.php +++ b/modules/system/tests/src/Unit/Breadcrumbs/PathBasedBreadcrumbBuilderTest.php @@ -8,6 +8,7 @@ namespace Drupal\Tests\system\Unit\Breadcrumbs; use Drupal\Core\Link; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Session\AccountInterface; use Drupal\Core\StringTranslation\TranslationInterface; use Drupal\Core\Url; @@ -160,7 +161,7 @@ public function testBuildWithTwoPathElements() { } })); - $this->setupAccessManagerWithTrue(); + $this->setupAccessManagerToAllow(); $links = $this->builder->build($this->getMock('Drupal\Core\Routing\RouteMatchInterface')); $this->assertEquals(array(0 => new Link('Home', new Url('')), 1 => new Link('Example', new Url('example'))), $links); @@ -200,7 +201,7 @@ public function testBuildWithThreePathElements() { } })); - $this->setupAccessManagerWithTrue(); + $this->setupAccessManagerToAllow(); $links = $this->builder->build($this->getMock('Drupal\Core\Routing\RouteMatchInterface')); $this->assertEquals(array( @@ -310,7 +311,7 @@ public function testBuildWithUserPath() { } })); - $this->setupAccessManagerWithTrue(); + $this->setupAccessManagerToAllow(); $this->titleResolver->expects($this->once()) ->method('getTitle') ->with($this->anything(), $route_1) @@ -321,12 +322,12 @@ public function testBuildWithUserPath() { } /** - * Setup the access manager to always return TRUE. + * Setup the access manager to always allow access to routes. */ - public function setupAccessManagerWithTrue() { + public function setupAccessManagerToAllow() { $this->accessManager->expects($this->any()) ->method('checkNamedRoute') - ->will($this->returnValue(TRUE)); + ->will($this->returnValue(AccessResult::allowed())); } protected function setupStubPathProcessor() { diff --git a/modules/taxonomy/src/Form/OverviewTerms.php b/modules/taxonomy/src/Form/OverviewTerms.php index 6020b617d77..981964cdae9 100644 --- a/modules/taxonomy/src/Form/OverviewTerms.php +++ b/modules/taxonomy/src/Form/OverviewTerms.php @@ -264,7 +264,7 @@ public function buildForm(array $form, FormStateInterface $form_state, Vocabular 'query' => $destination, ) + $term->urlInfo('delete-form')->toArray(), ); - if ($this->moduleHandler->moduleExists('content_translation') && content_translation_translate_access($term)) { + if ($this->moduleHandler->moduleExists('content_translation') && content_translation_translate_access($term)->isAllowed()) { $operations['translate'] = array( 'title' => $this->t('Translate'), 'query' => $destination, diff --git a/modules/taxonomy/src/TermAccessControlHandler.php b/modules/taxonomy/src/TermAccessControlHandler.php index fe38a4cba03..2ecd3a6f967 100644 --- a/modules/taxonomy/src/TermAccessControlHandler.php +++ b/modules/taxonomy/src/TermAccessControlHandler.php @@ -7,6 +7,7 @@ namespace Drupal\taxonomy; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Entity\EntityAccessControlHandler; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Session\AccountInterface; @@ -24,16 +25,20 @@ class TermAccessControlHandler extends EntityAccessControlHandler { protected function checkAccess(EntityInterface $entity, $operation, $langcode, AccountInterface $account) { switch ($operation) { case 'view': - return $account->hasPermission('access content'); + return AccessResult::allowedIfHasPermission($account, 'access content'); break; case 'update': - return $account->hasPermission("edit terms in {$entity->bundle()}") || $account->hasPermission('administer taxonomy'); + return AccessResult::allowedIf($account->hasPermission("edit terms in {$entity->bundle()}") || $account->hasPermission('administer taxonomy'))->cachePerRole(); break; case 'delete': - return $account->hasPermission("delete terms in {$entity->bundle()}") || $account->hasPermission('administer taxonomy'); + return AccessResult::allowedIf($account->hasPermission("delete terms in {$entity->bundle()}") || $account->hasPermission('administer taxonomy'))->cachePerRole(); break; + + default: + // No opinion. + return AccessResult::create(); } } @@ -41,7 +46,7 @@ protected function checkAccess(EntityInterface $entity, $operation, $langcode, A * {@inheritdoc} */ protected function checkCreateAccess(AccountInterface $account, array $context, $entity_bundle = NULL) { - return $account->hasPermission('administer taxonomy'); + return AccessResult::allowedIfHasPermission($account, 'administer taxonomy'); } } diff --git a/modules/toolbar/src/Controller/ToolbarController.php b/modules/toolbar/src/Controller/ToolbarController.php index 11d17b4c59f..b1de82db958 100644 --- a/modules/toolbar/src/Controller/ToolbarController.php +++ b/modules/toolbar/src/Controller/ToolbarController.php @@ -7,7 +7,7 @@ namespace Drupal\toolbar\Controller; -use Drupal\Core\Access\AccessInterface; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Controller\ControllerBase; use Symfony\Component\HttpFoundation\JsonResponse; use Symfony\Component\HttpFoundation\Request; @@ -38,13 +38,12 @@ public function subtreesJsonp() { * @param string $langcode * The langcode of the requested site, NULL if none given. * - * @return string - * Returns AccessInterface::ALLOW when access was granted, otherwise - * AccessInterface::DENY. + * @return \Drupal\Core\Access\AccessResultInterface + * The access result. */ 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; + return AccessResult::allowedIf($this->currentUser()->hasPermission('access toolbar') && $hash == _toolbar_get_subtrees_hash($langcode))->cachePerRole(); } } diff --git a/modules/tracker/src/Access/ViewOwnTrackerAccessCheck.php b/modules/tracker/src/Access/ViewOwnTrackerAccessCheck.php index 4421b77e89c..097627cccb5 100644 --- a/modules/tracker/src/Access/ViewOwnTrackerAccessCheck.php +++ b/modules/tracker/src/Access/ViewOwnTrackerAccessCheck.php @@ -7,6 +7,7 @@ namespace Drupal\tracker\Access; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; use Drupal\user\UserInterface; @@ -24,10 +25,10 @@ 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\AccessResultInterface + * The access result. */ public function access(AccountInterface $account, UserInterface $user) { - return ($user && $account->isAuthenticated() && ($user->id() == $account->id())) ? static::ALLOW : static::DENY; + return AccessResult::allowedIf($user && $account->isAuthenticated() && ($user->id() == $account->id()))->cachePerUser(); } } diff --git a/modules/update/src/Access/UpdateManagerAccessCheck.php b/modules/update/src/Access/UpdateManagerAccessCheck.php index 5fc20b03da2..d057ce3290b 100644 --- a/modules/update/src/Access/UpdateManagerAccessCheck.php +++ b/modules/update/src/Access/UpdateManagerAccessCheck.php @@ -7,6 +7,7 @@ namespace Drupal\update\Access; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Site\Settings; @@ -35,11 +36,13 @@ public function __construct(Settings $settings) { /** * Checks access. * - * @return string - * A \Drupal\Core\Access\AccessInterface constant value. + * @return \Drupal\Core\Access\AccessResultInterface + * The access result. */ public function access() { - return $this->settings->get('allow_authorize_operations', TRUE) ? static::ALLOW : static::DENY; + // Uncacheable because the access result depends on a Settings key-value + // pair, and can therefore change at any time. + return AccessResult::allowedIf($this->settings->get('allow_authorize_operations', TRUE))->setCacheable(FALSE); } } diff --git a/modules/user/src/Access/LoginStatusCheck.php b/modules/user/src/Access/LoginStatusCheck.php index 7539fe25645..56790cbfd02 100644 --- a/modules/user/src/Access/LoginStatusCheck.php +++ b/modules/user/src/Access/LoginStatusCheck.php @@ -7,6 +7,7 @@ namespace Drupal\user\Access; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; @@ -21,11 +22,11 @@ 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\AccessResultInterface + * The access result. */ public function access(AccountInterface $account) { - return $account->isAuthenticated() ? static::ALLOW : static::DENY; + return AccessResult::allowedIf($account->isAuthenticated())->cachePerRole(); } } diff --git a/modules/user/src/Access/PermissionAccessCheck.php b/modules/user/src/Access/PermissionAccessCheck.php index 2672a4e4591..15752de140c 100644 --- a/modules/user/src/Access/PermissionAccessCheck.php +++ b/modules/user/src/Access/PermissionAccessCheck.php @@ -7,6 +7,7 @@ namespace Drupal\user\Access; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; use Symfony\Component\Routing\Route; @@ -24,11 +25,11 @@ 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\AccessResultInterface + * The access result. */ public function access(Route $route, AccountInterface $account) { $permission = $route->getRequirement('_permission'); - return $account->hasPermission($permission) ? static::ALLOW : static::DENY; + return AccessResult::allowedIfHasPermission($account, $permission); } } diff --git a/modules/user/src/Access/RegisterAccessCheck.php b/modules/user/src/Access/RegisterAccessCheck.php index aa9433fce70..0f0a3f597ac 100644 --- a/modules/user/src/Access/RegisterAccessCheck.php +++ b/modules/user/src/Access/RegisterAccessCheck.php @@ -7,6 +7,7 @@ namespace Drupal\user\Access; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; @@ -21,10 +22,11 @@ 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\AccessResultInterface + * The access result. */ public function access(AccountInterface $account) { - return ($account->isAnonymous()) && (\Drupal::config('user.settings')->get('register') != USER_REGISTER_ADMINISTRATORS_ONLY) ? static::ALLOW : static::DENY; + // @todo cacheable per role once https://www.drupal.org/node/2040135 lands. + return AccessResult::allowedIf($account->isAnonymous() && \Drupal::config('user.settings')->get('register') != USER_REGISTER_ADMINISTRATORS_ONLY)->setCacheable(FALSE); } } diff --git a/modules/user/src/Access/RoleAccessCheck.php b/modules/user/src/Access/RoleAccessCheck.php index e8cfda4ba75..ebfaa808a40 100644 --- a/modules/user/src/Access/RoleAccessCheck.php +++ b/modules/user/src/Access/RoleAccessCheck.php @@ -7,6 +7,7 @@ namespace Drupal\user\Access; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; use Symfony\Component\Routing\Route; @@ -28,8 +29,8 @@ 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\AccessResultInterface + * The access result. */ public function access(Route $route, AccountInterface $account) { // Requirements just allow strings, so this might be a comma separated list. @@ -39,19 +40,19 @@ 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; + return AccessResult::allowed()->cachePerRole(); } } else { $explode_or = array_filter(array_map('trim', explode(',', $rid_string))); $intersection = array_intersect($explode_or, $account->getRoles()); if (!empty($intersection)) { - return static::ALLOW; + return AccessResult::allowed()->cachePerRole(); } } - // If there is no allowed role, return NULL to give other checks a chance. - return static::DENY; + // If there is no allowed role, give other access checks a chance. + return AccessResult::create()->cachePerRole(); } } diff --git a/modules/user/src/Plugin/Search/UserSearch.php b/modules/user/src/Plugin/Search/UserSearch.php index 0e8ebb035d7..b50e0d83ed7 100644 --- a/modules/user/src/Plugin/Search/UserSearch.php +++ b/modules/user/src/Plugin/Search/UserSearch.php @@ -7,6 +7,7 @@ namespace Drupal\user\Plugin\Search; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Database\Connection; use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\Extension\ModuleHandlerInterface; @@ -97,8 +98,9 @@ public function __construct(Connection $database, EntityManagerInterface $entity /** * {@inheritdoc} */ - public function access($operation = 'view', AccountInterface $account = NULL) { - return !empty($account) && $account->hasPermission('access user profiles'); + public function access($operation = 'view', AccountInterface $account = NULL, $return_as_object = FALSE) { + $result = AccessResult::allowedIf(!empty($account) && $account->hasPermission('access user profiles'))->cachePerRole(); + return $return_as_object ? $result : $result->isAllowed(); } /** diff --git a/modules/user/src/RoleAccessControlHandler.php b/modules/user/src/RoleAccessControlHandler.php index c53c2a368ed..52f590d8df6 100644 --- a/modules/user/src/RoleAccessControlHandler.php +++ b/modules/user/src/RoleAccessControlHandler.php @@ -7,6 +7,7 @@ namespace Drupal\user; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Entity\EntityAccessControlHandler; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Session\AccountInterface; @@ -25,7 +26,7 @@ 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; + return AccessResult::forbidden(); } default: diff --git a/modules/user/src/UserAccessControlHandler.php b/modules/user/src/UserAccessControlHandler.php index 81b072fa909..aab5f3bcc82 100644 --- a/modules/user/src/UserAccessControlHandler.php +++ b/modules/user/src/UserAccessControlHandler.php @@ -7,6 +7,7 @@ namespace Drupal\user; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Entity\EntityAccessControlHandler; use Drupal\Core\Session\AccountInterface; @@ -22,44 +23,41 @@ class UserAccessControlHandler extends EntityAccessControlHandler { * {@inheritdoc} */ protected function checkAccess(EntityInterface $entity, $operation, $langcode, AccountInterface $account) { + /** @var \Drupal\user\UserInterface $entity*/ + + // The anonymous user's profile can neither be viewed, updated nor deleted. + if ($entity->isAnonymous()) { + return AccessResult::forbidden(); + } + + // Administrators can view/update/delete all user profiles. + if ($account->hasPermission('administer users')) { + return AccessResult::allowed()->cachePerRole(); + } + switch ($operation) { case 'view': - return $this->viewAccess($entity, $langcode, $account); + // Only allow view access if the account is active. + if ($account->hasPermission('access user profiles') && $entity->isActive()) { + return AccessResult::allowed()->cachePerRole()->cacheUntilEntityChanges($entity); + } + // Users can view own profiles at all times. + else if ($account->id() == $entity->id()) { + return AccessResult::allowed()->cachePerUser(); + } 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; - break; + // Users can always edit their own account. + return AccessResult::allowedIf($account->id() == $entity->id())->cachePerUser(); 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; - break; + // Users with 'cancel account' permission can cancel their own account. + return AccessResult::allowedIf($account->id() == $entity->id() && $account->hasPermission('cancel account'))->cachePerRole()->cachePerUser(); } - } - /** - * Check view access. - * - * See EntityAccessControlHandlerInterface::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; + // No opinion. + return AccessResult::create(); } } diff --git a/modules/views/src/Plugin/views/argument_validator/Entity.php b/modules/views/src/Plugin/views/argument_validator/Entity.php index fc5f8796df9..bb001f5744c 100644 --- a/modules/views/src/Plugin/views/argument_validator/Entity.php +++ b/modules/views/src/Plugin/views/argument_validator/Entity.php @@ -199,7 +199,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'])) { return FALSE; } // If restricted by bundle. diff --git a/modules/views/src/ViewAccessControlHandler.php b/modules/views/src/ViewAccessControlHandler.php index 1532301ffb6..96d130e6ad4 100644 --- a/modules/views/src/ViewAccessControlHandler.php +++ b/modules/views/src/ViewAccessControlHandler.php @@ -7,6 +7,7 @@ namespace Drupal\views; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Entity\EntityAccessControlHandler; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Session\AccountInterface; @@ -22,7 +23,12 @@ class ViewAccessControlHandler extends EntityAccessControlHandler { * {@inheritdoc} */ public function checkAccess(EntityInterface $entity, $operation, $langcode, AccountInterface $account) { - return $operation == 'view' || parent::checkAccess($entity, $operation, $langcode, $account); + if ($operation == 'view') { + return AccessResult::allowed(); + } + else { + return parent::checkAccess($entity, $operation, $langcode, $account); + } } } diff --git a/modules/views/src/ViewsAccessCheck.php b/modules/views/src/ViewsAccessCheck.php index 8a3b508f6a9..62f68a68ac0 100644 --- a/modules/views/src/ViewsAccessCheck.php +++ b/modules/views/src/ViewsAccessCheck.php @@ -8,6 +8,7 @@ namespace Drupal\views; use Drupal\Core\Access\AccessCheckInterface; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Session\AccountInterface; use Symfony\Component\Routing\Route; @@ -31,11 +32,11 @@ 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\AccessResultInterface + * The access result. */ public function access(AccountInterface $account) { - return $account->hasPermission('access all views') ? static::ALLOW : static::DENY; + return AccessResult::allowedIfHasPermission($account, 'access all views'); } } diff --git a/modules/views/tests/src/Unit/Plugin/argument_validator/EntityTest.php b/modules/views/tests/src/Unit/Plugin/argument_validator/EntityTest.php index 8a27bcf0b60..250cc0e21dd 100644 --- a/modules/views/tests/src/Unit/Plugin/argument_validator/EntityTest.php +++ b/modules/views/tests/src/Unit/Plugin/argument_validator/EntityTest.php @@ -59,9 +59,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, FALSE, TRUE), + array('test_op_2', NULL, FALSE, FALSE), + array('test_op_3', NULL, FALSE, TRUE), ))); $mock_entity_bundle_2 = $this->getMockForAbstractClass('Drupal\Core\Entity\Entity', array(), '', FALSE, TRUE, TRUE, array('bundle', 'access')); @@ -71,7 +71,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, FALSE, FALSE), + array('test_op_2', NULL, FALSE, FALSE), + array('test_op_3', NULL, FALSE, TRUE), ))); diff --git a/modules/views_ui/src/ViewUI.php b/modules/views_ui/src/ViewUI.php index feea58f3fdc..7e1bc27444a 100644 --- a/modules/views_ui/src/ViewUI.php +++ b/modules/views_ui/src/ViewUI.php @@ -968,8 +968,8 @@ public function language() { /** * {@inheritdoc} */ - public function access($operation = 'view', AccountInterface $account = NULL) { - return $this->storage->access($operation, $account); + public function access($operation = 'view', AccountInterface $account = NULL, $return_as_object = FALSE) { + return $this->storage->access($operation, $account, $return_as_object); } /** diff --git a/tests/Drupal/Tests/Core/Access/AccessManagerTest.php b/tests/Drupal/Tests/Core/Access/AccessManagerTest.php index 0f45b83e6ed..4465c49e48a 100644 --- a/tests/Drupal/Tests/Core/Access/AccessManagerTest.php +++ b/tests/Drupal/Tests/Core/Access/AccessManagerTest.php @@ -8,8 +8,8 @@ namespace Drupal\Tests\Core\Access; use Drupal\Core\Access\AccessCheckInterface; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Access\AccessManagerInterface; -use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Access\AccessManager; use Drupal\Core\Access\DefaultAccessCheck; use Drupal\Tests\UnitTestCase; @@ -200,9 +200,10 @@ 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. foreach ($this->routeCollection->all() as $route) { - $this->assertFalse($this->accessManager->check($route, $request, $this->account)); + $this->assertEquals(FALSE, $this->accessManager->check($route, $request, $this->account)); + $this->assertEquals(AccessResult::create(), $this->accessManager->check($route, $request, $this->account, TRUE)); } $this->setupAccessChecker(); @@ -210,19 +211,27 @@ public function testCheck() { // An access checker got setup, but the routes haven't been setup using // setChecks. foreach ($this->routeCollection->all() as $route) { - $this->assertFalse($this->accessManager->check($route, $request, $this->account)); + $this->assertEquals(FALSE, $this->accessManager->check($route, $request, $this->account)); + $this->assertEquals(AccessResult::create(), $this->accessManager->check($route, $request, $this->account, TRUE)); } + // 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(FALSE, $this->accessManager->check($this->routeCollection->get('test_route_1'), $request, $this->account)); + $this->assertEquals(TRUE, $this->accessManager->check($this->routeCollection->get('test_route_2'), $request, $this->account)); + $this->assertEquals(FALSE, $this->accessManager->check($this->routeCollection->get('test_route_3'), $request, $this->account)); + $this->assertEquals(TRUE, $this->accessManager->check($this->routeCollection->get('test_route_4'), $request, $this->account)); + $this->assertEquals(AccessResult::create(), $this->accessManager->check($this->routeCollection->get('test_route_1'), $request, $this->account, TRUE)); + $this->assertEquals(AccessResult::allowed(), $this->accessManager->check($this->routeCollection->get('test_route_2'), $request, $this->account, TRUE)); + $this->assertEquals(AccessResult::forbidden(), $this->accessManager->check($this->routeCollection->get('test_route_3'), $request, $this->account, TRUE)); + $this->assertEquals(AccessResult::allowed(), $this->accessManager->check($this->routeCollection->get('test_route_4'), $request, $this->account, TRUE)); + } /** @@ -253,132 +262,136 @@ public function testCheckWithNullAccount() { * @see \Drupal\Tests\Core\Access\AccessManagerTest::testCheckConjunctions() */ public function providerTestCheckConjunctions() { + $access_allow = AccessResult::allowed(); + $access_deny = AccessResult::create(); + $access_kill = AccessResult::forbidden(); + $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, + 'condition_one' => 'TRUE', + 'condition_two' => 'FALSE', + 'expected' => $access_kill, ); $access_configurations[] = array( 'conjunction' => NULL, 'name' => 'test_route_4', - 'condition_one' => AccessCheckInterface::ALLOW, - 'condition_two' => AccessCheckInterface::KILL, - 'expected' => FALSE, + 'condition_one' => 'TRUE', + 'condition_two' => '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, + 'condition_one' => 'TRUE', + 'condition_two' => 'NULL', + 'expected' => $access_deny, ); $access_configurations[] = array( 'conjunction' => NULL, 'name' => 'test_route_5', - 'condition_one' => AccessCheckInterface::ALLOW, - 'condition_two' => AccessCheckInterface::DENY, - 'expected' => FALSE, + 'condition_one' => 'TRUE', + 'condition_two' => 'NULL', + '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, + 'condition_one' => 'FALSE', + 'condition_two' => 'NULL', + 'expected' => $access_kill, ); $access_configurations[] = array( 'conjunction' => NULL, 'name' => 'test_route_6', - 'condition_one' => AccessCheckInterface::KILL, - 'condition_two' => AccessCheckInterface::DENY, - 'expected' => FALSE, + 'condition_one' => 'FALSE', + 'condition_two' => 'NULL', + '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, + 'condition_one' => 'TRUE', + 'condition_two' => 'TRUE', + 'expected' => $access_allow, ); $access_configurations[] = array( 'conjunction' => NULL, 'name' => 'test_route_7', - 'condition_one' => AccessCheckInterface::ALLOW, - 'condition_two' => AccessCheckInterface::ALLOW, - 'expected' => TRUE, + 'condition_one' => 'TRUE', + 'condition_two' => '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, + 'condition_one' => 'FALSE', + 'condition_two' => 'FALSE', + 'expected' => $access_kill, ); $access_configurations[] = array( 'conjunction' => NULL, 'name' => 'test_route_8', - 'condition_one' => AccessCheckInterface::KILL, - 'condition_two' => AccessCheckInterface::KILL, - 'expected' => FALSE, + 'condition_one' => 'FALSE', + 'condition_two' => '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, + 'condition_one' => 'NULL', + 'condition_two' => 'NULL', + 'expected' => $access_deny, ); $access_configurations[] = array( 'conjunction' => NULL, 'name' => 'test_route_9', - 'condition_one' => AccessCheckInterface::DENY, - 'condition_two' => AccessCheckInterface::DENY, - 'expected' => FALSE, + 'condition_one' => 'NULL', + 'condition_two' => 'NULL', + '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, + 'condition_one' => 'TRUE', + 'condition_two' => '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, + 'condition_one' => 'TRUE', + 'condition_two' => 'NULL', + '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, + 'condition_one' => 'FALSE', + 'condition_two' => 'NULL', + '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, + 'condition_one' => 'TRUE', + 'condition_two' => '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, + 'condition_one' => 'FALSE', + 'condition_two' => '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, + 'condition_one' => 'NULL', + 'condition_two' => 'NULL', + 'expected' => $access_deny, ); return $access_configurations; @@ -400,8 +413,8 @@ public function testCheckConjunctions($conjunction, $name, $condition_one, $cond $route_collection = new RouteCollection(); // Setup a test route for each access configuration. $requirements = array( - '_access' => static::convertAccessCheckInterfaceToString($condition_one), - '_test_access' => static::convertAccessCheckInterfaceToString($condition_two), + '_access' => $condition_one, + '_test_access' => $condition_two, ); $options = $conjunction ? array('_access_mode' => $conjunction) : array(); $route = new Route($name, array(), $requirements, $options); @@ -413,7 +426,8 @@ 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->isAllowed(), $this->accessManager->check($route, $request, $this->account)); + $this->assertEquals($expected_access, $this->accessManager->check($route, $request, $this->account, TRUE)); } /** @@ -432,14 +446,17 @@ 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)); + $this->assertEquals(TRUE, $this->accessManager->checkNamedRoute('test_route_2', array(), $this->account, $request)); + $this->assertEquals(AccessResult::allowed(), $this->accessManager->checkNamedRoute('test_route_2', array(), $this->account, $request, TRUE)); + $this->assertEquals(FALSE, $this->accessManager->checkNamedRoute('test_route_3', array(), $this->account, $request)); + $this->assertEquals(AccessResult::forbidden(), $this->accessManager->checkNamedRoute('test_route_3', array(), $this->account, $request, TRUE)); // 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)); + $this->assertEquals(TRUE, $this->accessManager->checkNamedRoute('test_route_4', array(), $this->account, $request)); + $this->assertEquals(AccessResult::allowed(), $this->accessManager->checkNamedRoute('test_route_4', array(), $this->account, $request, TRUE)); // Tests the access with routes without given request. $this->requestStack->push(new Request()); @@ -448,15 +465,25 @@ public function testCheckNamedRoute() { ->method('convert') ->with(array(RouteObjectInterface::ROUTE_OBJECT => $this->routeCollection->get('test_route_2'))) ->will($this->returnValue(array())); - $this->paramConverter->expects($this->at(1)) + ->method('convert') + ->with(array(RouteObjectInterface::ROUTE_OBJECT => $this->routeCollection->get('test_route_2'))) + ->will($this->returnValue(array())); + + $this->paramConverter->expects($this->at(2)) + ->method('convert') + ->with(array('value' => 'example', RouteObjectInterface::ROUTE_OBJECT => $this->routeCollection->get('test_route_4'))) + ->will($this->returnValue(array('value' => 'example'))); + $this->paramConverter->expects($this->at(3)) ->method('convert') ->with(array('value' => 'example', RouteObjectInterface::ROUTE_OBJECT => $this->routeCollection->get('test_route_4'))) ->will($this->returnValue(array('value' => 'example'))); // 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(TRUE, $this->accessManager->checkNamedRoute('test_route_2', array(), $this->account)); + $this->assertEquals(AccessResult::allowed(), $this->accessManager->checkNamedRoute('test_route_2', array(), $this->account, NULL, TRUE)); + $this->assertEquals(TRUE, $this->accessManager->checkNamedRoute('test_route_4', array('value' => 'example'), $this->account)); + $this->assertEquals(AccessResult::allowed(), $this->accessManager->checkNamedRoute('test_route_4', array('value' => 'example'), $this->account, NULL, TRUE)); } /** @@ -484,7 +511,7 @@ public function testCheckNamedRouteWithUpcastedValues() { ->will($this->returnValueMap($map)); $this->paramConverter = $this->getMock('Drupal\Core\ParamConverter\ParamConverterManagerInterface'); - $this->paramConverter->expects($this->at(0)) + $this->paramConverter->expects($this->atLeastOnce()) ->method('convert') ->with(array('value' => 'example', RouteObjectInterface::ROUTE_OBJECT => $route)) ->will($this->returnValue(array('value' => 'upcasted_value'))); @@ -507,7 +534,7 @@ public function testCheckNamedRouteWithUpcastedValues() { ->will($this->returnValue(TRUE)); $access_check->expects($this->atLeastOnce()) ->method('access') - ->will($this->returnValue(AccessInterface::KILL)); + ->will($this->returnValue(AccessResult::forbidden())); $subrequest->attributes->set('value', 'upcasted_value'); $this->container->set('test_access', $access_check); @@ -515,7 +542,8 @@ 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(FALSE, $this->accessManager->checkNamedRoute('test_route_1', array('value' => 'example'), $this->account)); + $this->assertEquals(AccessResult::forbidden(), $this->accessManager->checkNamedRoute('test_route_1', array('value' => 'example'), $this->account, NULL, TRUE)); } /** @@ -544,7 +572,7 @@ public function testCheckNamedRouteWithDefaultValue() { ->will($this->returnValueMap($map)); $this->paramConverter = $this->getMock('Drupal\Core\ParamConverter\ParamConverterManagerInterface'); - $this->paramConverter->expects($this->at(0)) + $this->paramConverter->expects($this->atLeastOnce()) ->method('convert') ->with(array('value' => 'example', RouteObjectInterface::ROUTE_OBJECT => $route)) ->will($this->returnValue(array('value' => 'upcasted_value'))); @@ -567,7 +595,7 @@ public function testCheckNamedRouteWithDefaultValue() { ->will($this->returnValue(TRUE)); $access_check->expects($this->atLeastOnce()) ->method('access') - ->will($this->returnValue(AccessInterface::KILL)); + ->will($this->returnValue(AccessResult::forbidden())); $subrequest->attributes->set('value', 'upcasted_value'); $this->container->set('test_access', $access_check); @@ -575,7 +603,8 @@ 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(FALSE, $this->accessManager->checkNamedRoute('test_route_1', array(), $this->account)); + $this->assertEquals(AccessResult::forbidden(), $this->accessManager->checkNamedRoute('test_route_1', array(), $this->account, NULL, TRUE)); } /** @@ -590,7 +619,8 @@ public function testCheckNamedRouteWithNonExistingRoute() { $this->setupAccessChecker(); - $this->assertFalse($this->accessManager->checkNamedRoute('test_route_1', array(), $this->account), 'A non existing route lead to access.'); + $this->assertEquals(FALSE, $this->accessManager->checkNamedRoute('test_route_1', array(), $this->account), 'A non existing route lead to access.'); + $this->assertEquals(AccessResult::forbidden()->addCacheTags(array('extension' => TRUE)), $this->accessManager->checkNamedRoute('test_route_1', array(), $this->account, NULL, TRUE), 'A non existing route lead to access.'); } /** @@ -662,30 +692,6 @@ public function providerCheckException() { ); } - /** - * Converts AccessCheckInterface constants to a string. - * - * @param mixed $constant - * The access constant which is tested, so either - * AccessCheckInterface::ALLOW, AccessCheckInterface::DENY OR - * AccessCheckInterface::KILL. - * - * @return string - * The corresponding string used in route requirements, so 'TRUE', 'FALSE' - * or 'NULL'. - */ - protected static function convertAccessCheckInterfaceToString($constant) { - if ($constant === AccessCheckInterface::ALLOW) { - return 'TRUE'; - } - if ($constant === AccessCheckInterface::DENY) { - return 'NULL'; - } - if ($constant === AccessCheckInterface::KILL) { - return 'FALSE'; - } - } - /** * Adds a default access check service to the container and the access manager. */ diff --git a/tests/Drupal/Tests/Core/Access/AccessResultTest.php b/tests/Drupal/Tests/Core/Access/AccessResultTest.php new file mode 100644 index 00000000000..67d25d5be41 --- /dev/null +++ b/tests/Drupal/Tests/Core/Access/AccessResultTest.php @@ -0,0 +1,532 @@ +assertTrue($access->isCacheable()); + $this->assertSame([], $access->getCacheKeys()); + $this->assertSame([], $access->getCacheTags()); + $this->assertSame('default', $access->getCacheBin()); + $this->assertSame(Cache::PERMANENT, $access->getCacheMaxAge()); + } + + /** + * Tests the construction of an AccessResult object. + * + * @covers ::__construct + * @covers ::create + * @covers ::getCacheBin + */ + public function testConstruction() { + $verify = function (AccessResult $access) { + $this->assertFalse($access->isAllowed()); + $this->assertFalse($access->isForbidden()); + $this->assertDefaultCacheability($access); + }; + + // Verify the object when using the constructor. + $a = new AccessResult(); + $verify($a); + + // Verify the object when using the ::create() convenience method. + $b = AccessResult::create(); + $verify($b); + + $this->assertEquals($a, $b); + } + + /** + * @covers ::allow + * @covers ::allowed + * @covers ::isAllowed + * @covers ::isForbidden + */ + public function testAccessAllowed() { + $verify = function (AccessResult $access) { + $this->assertTrue($access->isAllowed()); + $this->assertFalse($access->isForbidden()); + $this->assertDefaultCacheability($access); + }; + + // Verify the object when using the ::allow() instance method. + $a = AccessResult::create()->allow(); + $verify($a); + + // Verify the object when using the ::allowed() convenience static method. + $b = AccessResult::allowed(); + $verify($b); + + $this->assertEquals($a, $b); + } + + /** + * @covers ::forbid + * @covers ::forbidden + * @covers ::isAllowed + * @covers ::isForbidden + */ + public function testAccessForbidden() { + $verify = function (AccessResult $access) { + $this->assertFalse($access->isAllowed()); + $this->assertTrue($access->isForbidden()); + $this->assertDefaultCacheability($access); + }; + + // Verify the object when using the ::forbid() instance method. + $a = AccessResult::create()->forbid(); + $verify($a); + + // Verify the object when using the ::forbidden() convenience static method. + $b = AccessResult::forbidden(); + $verify($b); + + $this->assertEquals($a, $b); + } + + /** + * @covers ::reset + * @covers ::isAllowed + * @covers ::isForbidden + */ + public function testAccessReset() { + $verify = function (AccessResult $access) { + $this->assertFalse($access->isAllowed()); + $this->assertFalse($access->isForbidden()); + $this->assertDefaultCacheability($access); + }; + + $a = AccessResult::allowed()->resetAccess(); + $verify($a); + + $b = AccessResult::forbidden()->resetAccess(); + $verify($b); + + $this->assertEquals($a, $b); + } + + /** + * @covers ::allowIf + * @covers ::allowedIf + * @covers ::isAllowed + * @covers ::isForbidden + */ + public function testAccessConditionallyAllowed() { + $verify = function (AccessResult $access, $allowed, $forbidden = FALSE) { + $this->assertSame($allowed, $access->isAllowed()); + $this->assertSame($forbidden, $access->isForbidden()); + $this->assertDefaultCacheability($access); + }; + + // Verify the object when using the ::allowIf() instance method. + $a1 = AccessResult::create()->allowIf(TRUE); + $verify($a1, TRUE); + $a2 = AccessResult::create()->allowIf(FALSE); + $verify($a2, FALSE); + + $b1 = AccessResult::allowedIf(TRUE); + $verify($b1, TRUE); + $b2 = AccessResult::allowedIf(FALSE); + $verify($b2, FALSE); + + $this->assertEquals($a1, $b1); + $this->assertEquals($a2, $b2); + + // Verify that ::allowIf() does not overwrite an existing value when the + // condition does not evaluate to TRUE. + $a1 = AccessResult::forbidden()->allowIf(TRUE); + $verify($a1, TRUE); + $a2 = AccessResult::forbidden()->allowIf(FALSE); + $verify($a2, FALSE, TRUE); + } + + /** + * @covers ::forbidIf + * @covers ::forbiddenIf + * @covers ::isAllowed + * @covers ::isForbidden + */ + public function testAccessConditionallyForbidden() { + $verify = function (AccessResult $access, $forbidden, $allowed = FALSE) { + $this->assertSame($allowed, $access->isAllowed()); + $this->assertSame($forbidden, $access->isForbidden()); + $this->assertDefaultCacheability($access); + }; + + // Verify the object when using the ::allowIf() instance method. + $a1 = AccessResult::create()->forbidIf(TRUE); + $verify($a1, TRUE); + $a2 = AccessResult::create()->forbidIf(FALSE); + $verify($a2, FALSE); + + $b1 = AccessResult::forbiddenIf(TRUE); + $verify($b1, TRUE); + $b2 = AccessResult::forbiddenIf(FALSE); + $verify($b2, FALSE); + + $this->assertEquals($a1, $b1); + $this->assertEquals($a2, $b2); + + // Verify that ::forbidIf() does not overwrite an existing value when the + // condition does not evaluate to TRUE. + $a1 = AccessResult::allowed()->forbidIf(TRUE); + $verify($a1, TRUE); + $a2 = AccessResult::allowed()->forbidIf(FALSE); + $verify($a2, FALSE, TRUE); + } + + /** + * @covers ::andIf + */ + public function testAndIf() { + $no_opinion = AccessResult::create(); + $allowed = AccessResult::allowed(); + $forbidden = AccessResult::forbidden(); + $unused_access_result_due_to_lazy_evaluation = $this->getMock('\Drupal\Core\Access\AccessResultInterface'); + $unused_access_result_due_to_lazy_evaluation->expects($this->never()) + ->method($this->anything()); + + // ALLOW && ALLOW === ALLOW. + $access = clone $allowed; + $access->andIf($allowed); + $this->assertTrue($access->isAllowed()); + $this->assertFalse($access->isForbidden()); + $this->assertDefaultCacheability($access); + + // ALLOW && DENY === DENY. + $access = clone $allowed; + $access->andIf($no_opinion); + $this->assertFalse($access->isAllowed()); + $this->assertFalse($access->isForbidden()); + $this->assertDefaultCacheability($access); + + // ALLOW && KILL === KILL. + $access = clone $allowed; + $access->andIf($forbidden); + $this->assertFalse($access->isAllowed()); + $this->assertTrue($access->isForbidden()); + $this->assertDefaultCacheability($access); + + // DENY && * === DENY. + $access = clone $no_opinion; + $access->andIf($unused_access_result_due_to_lazy_evaluation); + $this->assertFalse($access->isAllowed()); + $this->assertFalse($access->isForbidden()); + $this->assertDefaultCacheability($access); + + // KILL && * === KILL. + $access = clone $forbidden; + $access->andIf($unused_access_result_due_to_lazy_evaluation); + $this->assertFalse($access->isAllowed()); + $this->assertTrue($access->isForbidden()); + $this->assertDefaultCacheability($access); + } + + /** + * @covers ::orIf + */ + public function testOrIf() { + $no_opinion = AccessResult::create(); + $allowed = AccessResult::allowed(); + $forbidden = AccessResult::forbidden(); + $unused_access_result_due_to_lazy_evaluation = $this->getMock('\Drupal\Core\Access\AccessResultInterface'); + $unused_access_result_due_to_lazy_evaluation->expects($this->never()) + ->method($this->anything()); + + // ALLOW || ALLOW === ALLOW. + $access = clone $allowed; + $access->orIf($allowed); + $this->assertTrue($access->isAllowed()); + $this->assertFalse($access->isForbidden()); + $this->assertDefaultCacheability($access); + + // ALLOW || DENY === ALLOW. + $access = clone $allowed; + $access->orIf($no_opinion); + $this->assertTrue($access->isAllowed()); + $this->assertFalse($access->isForbidden()); + $this->assertDefaultCacheability($access); + + // ALLOW || KILL === KILL. + $access = clone $allowed; + $access->orIf($forbidden); + $this->assertFalse($access->isAllowed()); + $this->assertTrue($access->isForbidden()); + $this->assertDefaultCacheability($access); + + // DENY || DENY === DENY. + $access = clone $no_opinion; + $access->orIf($no_opinion); + $this->assertFalse($access->isAllowed()); + $this->assertFalse($access->isForbidden()); + $this->assertDefaultCacheability($access); + + // DENY || ALLOW === ALLOW. + $access = clone $no_opinion; + $access->orIf($allowed); + $this->assertTrue($access->isAllowed()); + $this->assertFalse($access->isForbidden()); + $this->assertDefaultCacheability($access); + + // DENY || KILL === KILL. + $access = clone $no_opinion; + $access->orIf($forbidden); + $this->assertFalse($access->isAllowed()); + $this->assertTrue($access->isForbidden()); + $this->assertDefaultCacheability($access); + + // KILL || * === KILL. + $access = clone $forbidden; + $access->orIf($unused_access_result_due_to_lazy_evaluation); + $this->assertFalse($access->isAllowed()); + $this->assertTrue($access->isForbidden()); + $this->assertDefaultCacheability($access); + } + + /** + * @covers ::setCacheable + * @covers ::isCacheable + */ + public function testCacheable() { + $this->assertTrue(AccessResult::create()->isCacheable()); + $this->assertTrue(AccessResult::create()->setCacheable(TRUE)->isCacheable()); + $this->assertFalse(AccessResult::create()->setCacheable(FALSE)->isCacheable()); + } + + /** + * @covers ::setCacheMaxAge + * @covers ::getCacheMaxAge + */ + public function testCacheMaxAge() { + $this->assertSame(Cache::PERMANENT, AccessResult::create()->getCacheMaxAge()); + $this->assertSame(1337, AccessResult::create()->setCacheMaxAge(1337)->getCacheMaxAge()); + } + + /** + * @covers ::addCacheContexts + * @covers ::resetCacheContexts + * @covers ::getCacheKeys + * @covers ::cachePerRole + * @covers ::cachePerUser + * @covers ::allowIfHasPermission + * @covers ::allowedIfHasPermission + */ + public function testCacheContexts() { + $verify = function (AccessResult $access, array $contexts) { + $this->assertFalse($access->isAllowed()); + $this->assertFalse($access->isForbidden()); + $this->assertTrue($access->isCacheable()); + $this->assertSame('default', $access->getCacheBin()); + $this->assertSame(Cache::PERMANENT, $access->getCacheMaxAge()); + $this->assertSame($contexts, $access->getCacheKeys()); + $this->assertSame([], $access->getCacheTags()); + }; + + $access = AccessResult::create()->addCacheContexts(['cache_context.foo']); + $verify($access, ['cache_context.foo']); + // Verify resetting works. + $access->resetCacheContexts(); + $verify($access, []); + // Verify idempotency. + $access->addCacheContexts(['cache_context.foo']) + ->addCacheContexts(['cache_context.foo']); + $verify($access, ['cache_context.foo']); + // Verify same values in different call order yields the same result. + $access->resetCacheContexts() + ->addCacheContexts(['cache_context.foo']) + ->addCacheContexts(['cache_context.bar']); + $verify($access, ['cache_context.bar', 'cache_context.foo']); + $access->resetCacheContexts() + ->addCacheContexts(['cache_context.bar']) + ->addCacheContexts(['cache_context.foo']); + $verify($access, ['cache_context.bar', 'cache_context.foo']); + + // ::cachePerRole() convenience method. + $contexts = array('cache_context.user.roles'); + $a = AccessResult::create()->addCacheContexts($contexts); + $verify($a, $contexts); + $b = AccessResult::create()->cachePerRole(); + $verify($b, $contexts); + $this->assertEquals($a, $b); + + // ::cachePerUser() convenience method. + $contexts = array('cache_context.user'); + $a = AccessResult::create()->addCacheContexts($contexts); + $verify($a, $contexts); + $b = AccessResult::create()->cachePerUser(); + $verify($b, $contexts); + $this->assertEquals($a, $b); + + // Both. + $contexts = array('cache_context.user', 'cache_context.user.roles'); + $a = AccessResult::create()->addCacheContexts($contexts); + $verify($a, $contexts); + $b = AccessResult::create()->cachePerRole()->cachePerUser(); + $verify($b, $contexts); + $c = AccessResult::create()->cachePerUser()->cachePerRole(); + $verify($c, $contexts); + $this->assertEquals($a, $b); + $this->assertEquals($a, $c); + + // ::allowIfHasPermission and ::allowedIfHasPermission convenience methods. + $account = $this->getMock('\Drupal\Core\Session\AccountInterface'); + $account->expects($this->any()) + ->method('hasPermission') + ->with('may herd llamas') + ->will($this->returnValue(FALSE)); + $contexts = array('cache_context.user.roles'); + + // Verify the object when using the ::allowIfHasPermission() convenience + // instance method. + $a = AccessResult::create()->allowIfHasPermission($account, 'may herd llamas'); + $verify($a, $contexts); + + // Verify the object when using the ::allowedIfHasPermission() convenience + // static method. + $b = AccessResult::allowedIfHasPermission($account, 'may herd llamas'); + $verify($b, $contexts); + + $this->assertEquals($a, $b); + } + + /** + * @covers ::addCacheTags + * @covers ::resetCacheTags + * @covers ::getCacheTags + * @covers ::cacheUntilEntityChanges + */ + public function testCacheTags() { + $verify = function (AccessResult $access, array $tags) { + $this->assertFalse($access->isAllowed()); + $this->assertFalse($access->isForbidden()); + $this->assertTrue($access->isCacheable()); + $this->assertSame('default', $access->getCacheBin()); + $this->assertSame(Cache::PERMANENT, $access->getCacheMaxAge()); + $this->assertSame([], $access->getCacheKeys()); + $this->assertSame($tags, $access->getCacheTags()); + }; + + $access = AccessResult::create()->addCacheTags(['foo' => ['bar']]); + $verify($access, ['foo' => ['bar' => 'bar']]); + // Verify resetting works. + $access->resetCacheTags(); + $verify($access, []); + // Verify idempotency. + $access->addCacheTags(['foo' => ['bar']]) + ->addCacheTags(['foo' => ['bar']]); + $verify($access, ['foo' => ['bar' => 'bar']]); + // Verify same values in different call order yields the same result. + $access->resetCacheTags() + ->addCacheTags(['bar' => ['baz']]) + ->addCacheTags(['bar' => ['qux']]) + ->addCacheTags(['foo' => ['bar']]) + ->addCacheTags(['foo' => ['baz']]); + $verify($access, ['bar' => ['baz' => 'baz', 'qux' => 'qux'], 'foo' => ['bar' => 'bar', 'baz' => 'baz']]); + $access->resetCacheTags() + ->addCacheTags(['foo' => ['bar']]) + ->addCacheTags(['bar' => ['qux']]) + ->addCacheTags(['foo' => ['baz']]) + ->addCacheTags(['bar' => ['baz']]); + $verify($access, ['bar' => ['baz' => 'baz', 'qux' => 'qux'], 'foo' => ['bar' => 'bar', 'baz' => 'baz']]); + // Verify tags with nested arrays and without. + $access->resetCacheTags() + // Array. + ->addCacheTags(['foo' => ['bar']]) + // String. + ->addCacheTags(['bar' => 'baz']) + // Boolean. + ->addCacheTags(['qux' => TRUE]); + $verify($access, ['bar' => 'baz', 'foo' => ['bar' => 'bar'], 'qux' => TRUE]); + + // ::cacheUntilEntityChanges() convenience method. + $node = $this->getMock('\Drupal\node\NodeInterface'); + $node->expects($this->any()) + ->method('getCacheTag') + ->will($this->returnValue(array('node' => array(20011988)))); + $tags = array('node' => array(20011988 => 20011988)); + $a = AccessResult::create()->addCacheTags($tags); + $verify($a, $tags); + $b = AccessResult::create()->cacheUntilEntityChanges($node); + $verify($b, $tags); + $this->assertEquals($a, $b); + } + + /** + * @covers ::andIf + * @covers ::orIf + * @covers ::mergeCacheabilityMetadata + */ + public function testCacheabilityMerging() { + $access_without_cacheability = $this->getMock('\Drupal\Core\Access\AccessResultInterface'); + $access_without_cacheability->expects($this->exactly(2)) + ->method('isAllowed') + ->willReturn(TRUE); + $access_without_cacheability->expects($this->exactly(2)) + ->method('isForbidden') + ->willReturn(FALSE); + $access_without_cacheability->expects($this->once()) + ->method('andIf') + ->will($this->returnSelf()); + + // andIf(); 1st has defaults, 2nd has custom tags, contexts and max-age. + $access = AccessResult::allowed() + ->andIf(AccessResult::allowed()->setCacheMaxAge(1500)->cachePerRole()->addCacheTags(['node' => [20011988]])); + $this->assertTrue($access->isAllowed()); + $this->assertFalse($access->isForbidden()); + $this->assertTrue($access->isCacheable()); + $this->assertSame(['cache_context.user.roles'], $access->getCacheKeys()); + $this->assertSame(['node' => [20011988 => 20011988]], $access->getCacheTags()); + $this->assertSame('default', $access->getCacheBin()); + $this->assertSame(1500, $access->getCacheMaxAge()); + + // andIf(); 1st has custom tags, max-age, 2nd has custom contexts and max-age. + $access = AccessResult::allowed()->cachePerUser()->setCacheMaxAge(43200) + ->andIf(AccessResult::forbidden()->addCacheTags(['node' => [14031991]])->setCacheMaxAge(86400)); + $this->assertFalse($access->isAllowed()); + $this->assertTrue($access->isForbidden()); + $this->assertTrue($access->isCacheable()); + $this->assertSame(['cache_context.user'], $access->getCacheKeys()); + $this->assertSame(['node' => [14031991 => 14031991]], $access->getCacheTags()); + $this->assertSame('default', $access->getCacheBin()); + $this->assertSame(43200, $access->getCacheMaxAge()); + + // orIf(); 1st is cacheable, 2nd isn't. + $access = AccessResult::allowed()->orIf(AccessResult::allowed()->setCacheable(FALSE)); + $this->assertTrue($access->isAllowed()); + $this->assertFalse($access->isForbidden()); + $this->assertFalse($access->isCacheable()); + + // andIf(); 1st is cacheable, 2nd isn't. + $access = AccessResult::allowed()->andIf(AccessResult::allowed()->setCacheable(FALSE)); + $this->assertTrue($access->isAllowed()); + $this->assertFalse($access->isForbidden()); + $this->assertFalse($access->isCacheable()); + + // andIf(); 1st implements CacheableInterface, 2nd doesn't. + $access = AccessResult::allowed()->andIf($access_without_cacheability); + $this->assertTrue($access->isAllowed()); + $this->assertFalse($access->isForbidden()); + $this->assertFalse($access->isCacheable()); + + // andIf(); 1st doesn't implement CacheableInterface, 2nd does. + $access = $access_without_cacheability->andIf(AccessResult::allowed()); + $this->assertTrue($access->isAllowed()); + $this->assertFalse($access->isForbidden()); + $this->assertFalse(method_exists($access, 'isCacheable')); + } + +} diff --git a/tests/Drupal/Tests/Core/Access/CsrfAccessCheckTest.php b/tests/Drupal/Tests/Core/Access/CsrfAccessCheckTest.php index 55c47ecacaf..2a2822e84a6 100644 --- a/tests/Drupal/Tests/Core/Access/CsrfAccessCheckTest.php +++ b/tests/Drupal/Tests/Core/Access/CsrfAccessCheckTest.php @@ -7,11 +7,11 @@ namespace Drupal\Tests\Core\Access; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Access\AccessManagerInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Routing\Route; use Drupal\Core\Access\CsrfAccessCheck; -use Drupal\Core\Access\AccessInterface; use Drupal\Tests\UnitTestCase; /** @@ -66,7 +66,7 @@ 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)); + $this->assertEquals(AccessResult::allowed()->setCacheable(FALSE), $this->accessCheck->access($route, $request, $this->account)); } /** @@ -84,7 +84,7 @@ 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)); + $this->assertEquals(AccessResult::forbidden()->setCacheable(FALSE), $this->accessCheck->access($route, $request, $this->account)); } /** @@ -103,7 +103,7 @@ public function testAccessTokenMissAny() { 'token' => 'test_query', )); - $this->assertSame(AccessInterface::DENY, $this->accessCheck->access($route, $request, $this->account)); + $this->assertEquals(AccessResult::create()->setCacheable(FALSE), $this->accessCheck->access($route, $request, $this->account)); } /** @@ -122,7 +122,7 @@ public function testAccessTokenMissAll() { 'token' => 'test_query', )); - $this->assertSame(AccessInterface::ALLOW, $this->accessCheck->access($route, $request, $this->account)); + $this->assertEquals(AccessResult::allowed()->setCacheable(FALSE), $this->accessCheck->access($route, $request, $this->account)); } } diff --git a/tests/Drupal/Tests/Core/Access/CustomAccessCheckTest.php b/tests/Drupal/Tests/Core/Access/CustomAccessCheckTest.php index 23f500d77f5..73cd223e365 100644 --- a/tests/Drupal/Tests/Core/Access/CustomAccessCheckTest.php +++ b/tests/Drupal/Tests/Core/Access/CustomAccessCheckTest.php @@ -7,7 +7,7 @@ namespace Drupal\Tests\Core\Access; -use Drupal\Core\Access\AccessInterface; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Access\CustomAccessCheck; use Drupal\Tests\UnitTestCase; use Symfony\Component\HttpFoundation\Request; @@ -86,13 +86,13 @@ public function testAccess() { $route = new Route('/test-route', array(), array('_custom_access' => '\Drupal\Tests\Core\Access\TestController::accessDeny')); $account = $this->getMock('Drupal\Core\Session\AccountInterface'); - $this->assertSame(AccessInterface::DENY, $this->accessChecker->access($route, $request, $account)); + $this->assertEquals(AccessResult::create(), $this->accessChecker->access($route, $request, $account)); $route = new Route('/test-route', array(), array('_custom_access' => '\Drupal\Tests\Core\Access\TestController::accessAllow')); - $this->assertSame(AccessInterface::ALLOW, $this->accessChecker->access($route, $request, $account)); + $this->assertEquals(AccessResult::allowed(), $this->accessChecker->access($route, $request, $account)); $route = new Route('/test-route', array('parameter' => 'TRUE'), array('_custom_access' => '\Drupal\Tests\Core\Access\TestController::accessParameter')); - $this->assertSame(AccessInterface::ALLOW, $this->accessChecker->access($route, $request, $account)); + $this->assertEquals(AccessResult::allowed(), $this->accessChecker->access($route, $request, $account)); } } @@ -100,20 +100,15 @@ public function testAccess() { class TestController { public function accessAllow() { - return AccessInterface::ALLOW; + return AccessResult::allowed(); } public function accessDeny() { - return AccessInterface::DENY; + return AccessResult::create(); } public function accessParameter($parameter) { - if ($parameter == 'TRUE') { - return AccessInterface::ALLOW; - } - else { - return AccessInterface::DENY; - } + return AccessResult::allowedIf($parameter == 'TRUE'); } } diff --git a/tests/Drupal/Tests/Core/Access/DefaultAccessCheckTest.php b/tests/Drupal/Tests/Core/Access/DefaultAccessCheckTest.php index 0af32a4fb44..c7bc1dffc05 100644 --- a/tests/Drupal/Tests/Core/Access/DefaultAccessCheckTest.php +++ b/tests/Drupal/Tests/Core/Access/DefaultAccessCheckTest.php @@ -7,7 +7,7 @@ namespace Drupal\Tests\Core\Access; -use Drupal\Core\Access\AccessInterface; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Access\DefaultAccessCheck; use Drupal\Tests\UnitTestCase; use Symfony\Component\HttpFoundation\Request; @@ -50,13 +50,13 @@ 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)); + $this->assertEquals(AccessResult::create(), $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)); + $this->assertEquals(AccessResult::forbidden(), $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)); + $this->assertEquals(AccessResult::allowed(), $this->accessChecker->access($route, $request, $this->account)); } } diff --git a/tests/Drupal/Tests/Core/Entity/ContentEntityBaseUnitTest.php b/tests/Drupal/Tests/Core/Entity/ContentEntityBaseUnitTest.php index 39f8d76dcb6..60d58389a71 100644 --- a/tests/Drupal/Tests/Core/Entity/ContentEntityBaseUnitTest.php +++ b/tests/Drupal/Tests/Core/Entity/ContentEntityBaseUnitTest.php @@ -7,6 +7,7 @@ namespace Drupal\Tests\Core\Entity; +use Drupal\Core\Access\AccessResult; use Drupal\Core\DependencyInjection\ContainerBuilder; use Drupal\Core\Field\BaseFieldDefinition; use Drupal\Core\Field\FieldItemBase; @@ -17,6 +18,7 @@ /** * @coversDefaultClass \Drupal\Core\Entity\ContentEntityBase * @group Entity + * @group Access */ class ContentEntityBaseUnitTest extends UnitTestCase { @@ -409,13 +411,22 @@ public function testAccess() { ->with($this->entity, $operation) ->will($this->returnValue(TRUE)); $access->expects($this->at(1)) + ->method('access') + ->with($this->entity, $operation) + ->will($this->returnValue(AccessResult::allowed())); + $access->expects($this->at(2)) ->method('createAccess') ->will($this->returnValue(TRUE)); - $this->entityManager->expects($this->exactly(2)) + $access->expects($this->at(3)) + ->method('createAccess') + ->will($this->returnValue(AccessResult::allowed())); + $this->entityManager->expects($this->exactly(4)) ->method('getAccessControlHandler') ->will($this->returnValue($access)); $this->assertTrue($this->entity->access($operation)); + $this->assertEquals(AccessResult::allowed(), $this->entity->access($operation, NULL, TRUE)); $this->assertTrue($this->entity->access('create')); + $this->assertEquals(AccessResult::allowed(), $this->entity->access('create', NULL, TRUE)); } /** diff --git a/tests/Drupal/Tests/Core/Entity/EntityAccessCheckTest.php b/tests/Drupal/Tests/Core/Entity/EntityAccessCheckTest.php index ba5ffcf07d3..32388f54d7e 100644 --- a/tests/Drupal/Tests/Core/Entity/EntityAccessCheckTest.php +++ b/tests/Drupal/Tests/Core/Entity/EntityAccessCheckTest.php @@ -9,13 +9,14 @@ use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Routing\Route; -use Drupal\Core\Access\AccessCheckInterface; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Entity\EntityAccessCheck; use Drupal\Tests\UnitTestCase; /** * Unit test of entity access checking system. * + * @group Access * @group Entity */ class EntityAccessCheckTest extends UnitTestCase { @@ -31,12 +32,12 @@ public function testAccess() { ->getMock(); $node->expects($this->any()) ->method('access') - ->will($this->returnValue(TRUE)); + ->will($this->returnValue(AccessResult::allowed()->cachePerRole())); $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); + $this->assertEquals(AccessResult::allowed()->cachePerRole(), $access); } } diff --git a/tests/Drupal/Tests/Core/Entity/EntityCreateAccessCheckTest.php b/tests/Drupal/Tests/Core/Entity/EntityCreateAccessCheckTest.php index bf2ebf3ceca..f64b04aee44 100644 --- a/tests/Drupal/Tests/Core/Entity/EntityCreateAccessCheckTest.php +++ b/tests/Drupal/Tests/Core/Entity/EntityCreateAccessCheckTest.php @@ -7,7 +7,7 @@ namespace Drupal\Tests\Core\Entity; -use Drupal\Core\Access\AccessCheckInterface; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Entity\EntityCreateAccessCheck; use Drupal\Tests\UnitTestCase; use Symfony\Component\DependencyInjection\ParameterBag\ParameterBag; @@ -16,6 +16,7 @@ /** * @coversDefaultClass \Drupal\Core\Entity\EntityCreateAccessCheck * + * @group Access * @group Entity */ class EntityCreateAccessCheckTest extends UnitTestCase { @@ -40,17 +41,21 @@ protected function setUp() { * @return array */ public function providerTestAccess() { + $no_access = AccessResult::create()->cachePerRole(); + $access = AccessResult::allowed()->cachePerRole(); + $no_access_due_to_errors = AccessResult::create(); + 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 control handler 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/tests/Drupal/Tests/Core/Entity/EntityListBuilderTest.php b/tests/Drupal/Tests/Core/Entity/EntityListBuilderTest.php index 8c373381cbf..7e2fce4aef4 100644 --- a/tests/Drupal/Tests/Core/Entity/EntityListBuilderTest.php +++ b/tests/Drupal/Tests/Core/Entity/EntityListBuilderTest.php @@ -7,6 +7,7 @@ namespace Drupal\Tests\Core\Entity; +use Drupal\Core\Access\AccessResult; use Drupal\Core\DependencyInjection\ContainerBuilder; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Entity\EntityListBuilder; @@ -106,7 +107,7 @@ public function testGetOperations() { $this->role->expects($this->any()) ->method('access') - ->will($this->returnValue(TRUE)); + ->will($this->returnValue(AccessResult::allowed())); $this->role->expects($this->any()) ->method('hasLinkTemplate') ->will($this->returnValue(TRUE)); diff --git a/tests/Drupal/Tests/Core/Entity/EntityUnitTest.php b/tests/Drupal/Tests/Core/Entity/EntityUnitTest.php index add017d1e86..49134460242 100644 --- a/tests/Drupal/Tests/Core/Entity/EntityUnitTest.php +++ b/tests/Drupal/Tests/Core/Entity/EntityUnitTest.php @@ -7,6 +7,7 @@ namespace Drupal\Tests\Core\Entity; +use Drupal\Core\Access\AccessResult; use Drupal\Core\DependencyInjection\ContainerBuilder; use Drupal\Core\Entity\Entity; use Drupal\Core\Entity\Exception\NoCorrespondingEntityClassException; @@ -18,6 +19,7 @@ /** * @coversDefaultClass \Drupal\Core\Entity\Entity * @group Entity + * @group Access */ class EntityUnitTest extends UnitTestCase { @@ -213,15 +215,15 @@ public function testAccess() { $access->expects($this->at(0)) ->method('access') ->with($this->entity, $operation) - ->will($this->returnValue(TRUE)); + ->will($this->returnValue(AccessResult::allowed())); $access->expects($this->at(1)) ->method('createAccess') - ->will($this->returnValue(TRUE)); + ->will($this->returnValue(AccessResult::allowed())); $this->entityManager->expects($this->exactly(2)) ->method('getAccessControlHandler') ->will($this->returnValue($access)); - $this->assertTrue($this->entity->access($operation)); - $this->assertTrue($this->entity->access('create')); + $this->assertEquals(AccessResult::allowed(), $this->entity->access($operation)); + $this->assertEquals(AccessResult::allowed(), $this->entity->access('create')); } /** diff --git a/tests/Drupal/Tests/Core/Menu/ContextualLinkManagerTest.php b/tests/Drupal/Tests/Core/Menu/ContextualLinkManagerTest.php index edd14ecf57c..b6d7ae824c9 100644 --- a/tests/Drupal/Tests/Core/Menu/ContextualLinkManagerTest.php +++ b/tests/Drupal/Tests/Core/Menu/ContextualLinkManagerTest.php @@ -7,6 +7,7 @@ namespace Drupal\Tests\Core\Menu; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Language\Language; use Drupal\Tests\UnitTestCase; use Symfony\Component\HttpFoundation\RequestStack; @@ -270,7 +271,7 @@ public function testGetContextualLinksArrayByGroup() { $this->accessManager->expects($this->any()) ->method('checkNamedRoute') - ->will($this->returnValue(TRUE)); + ->will($this->returnValue(AccessResult::allowed())); // Set up mocking of the plugin factory. $map = array(); @@ -342,8 +343,8 @@ public function testGetContextualLinksArrayByGroupAccessCheck() { $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, FALSE, TRUE), + array('test_route2', array('key' => 'value'), $this->account, NULL, FALSE, FALSE), ))); // Set up mocking of the plugin factory. diff --git a/tests/Drupal/Tests/Core/Menu/DefaultMenuLinkTreeManipulatorsTest.php b/tests/Drupal/Tests/Core/Menu/DefaultMenuLinkTreeManipulatorsTest.php index cbd81f50dcc..e3edf63a2c3 100644 --- a/tests/Drupal/Tests/Core/Menu/DefaultMenuLinkTreeManipulatorsTest.php +++ b/tests/Drupal/Tests/Core/Menu/DefaultMenuLinkTreeManipulatorsTest.php @@ -154,10 +154,10 @@ public function testCheckAccess() { $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, FALSE, FALSE), + array('example2', array('foo' => 'bar'), $this->currentUser, NULL, FALSE, TRUE), + array('example3', array('baz' => 'qux'), $this->currentUser, NULL, FALSE, FALSE), + array('example5', array(), $this->currentUser, NULL, FALSE, TRUE), ))); $this->mockTree(); diff --git a/tests/Drupal/Tests/Core/Menu/LocalActionManagerTest.php b/tests/Drupal/Tests/Core/Menu/LocalActionManagerTest.php index f92711ad094..a01afea11e9 100644 --- a/tests/Drupal/Tests/Core/Menu/LocalActionManagerTest.php +++ b/tests/Drupal/Tests/Core/Menu/LocalActionManagerTest.php @@ -108,6 +108,9 @@ protected function setUp() { $this->cacheBackend = $this->getMock('Drupal\Core\Cache\CacheBackendInterface'); $this->accessManager = $this->getMock('Drupal\Core\Access\AccessManagerInterface'); + $this->accessManager->expects($this->any()) + ->method('checkNamedRoute') + ->will($this->returnValue(FALSE)); $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'); @@ -197,7 +200,7 @@ public function getActionsForRouteProvider() { 'route_parameters' => array(), 'localized_options' => '', ), - '#access' => NULL, + '#access' => FALSE, '#weight' => 0, ), ), @@ -232,7 +235,7 @@ public function getActionsForRouteProvider() { 'route_parameters' => array(), 'localized_options' => '', ), - '#access' => NULL, + '#access' => FALSE, '#weight' => 0, ), ), @@ -268,7 +271,7 @@ public function getActionsForRouteProvider() { 'route_parameters' => array(), 'localized_options' => '', ), - '#access' => NULL, + '#access' => FALSE, '#weight' => 1, ), 'plugin_id_2' => array( @@ -279,7 +282,7 @@ public function getActionsForRouteProvider() { 'route_parameters' => array(), 'localized_options' => '', ), - '#access' => NULL, + '#access' => FALSE, '#weight' => 0, ), ), @@ -317,7 +320,7 @@ public function getActionsForRouteProvider() { 'route_parameters' => array('test1'), 'localized_options' => '', ), - '#access' => NULL, + '#access' => FALSE, '#weight' => 1, ), 'plugin_id_2' => array( @@ -328,7 +331,7 @@ public function getActionsForRouteProvider() { 'route_parameters' => array('test2'), 'localized_options' => '', ), - '#access' => NULL, + '#access' => FALSE, '#weight' => 0, ), ), diff --git a/tests/Drupal/Tests/Core/Route/RoleAccessCheckTest.php b/tests/Drupal/Tests/Core/Route/RoleAccessCheckTest.php index 232645b2a23..5fc71238dcf 100644 --- a/tests/Drupal/Tests/Core/Route/RoleAccessCheckTest.php +++ b/tests/Drupal/Tests/Core/Route/RoleAccessCheckTest.php @@ -7,7 +7,7 @@ namespace Drupal\Tests\Core\Route; -use Drupal\Core\Access\AccessCheckInterface; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Session\UserSession; use Drupal\Tests\UnitTestCase; use Drupal\user\Access\RoleAccessCheck; @@ -16,6 +16,7 @@ /** * @coversDefaultClass \Drupal\user\Access\RoleAccessCheck + * @group Access * @group Route */ class RoleAccessCheckTest extends UnitTestCase { @@ -147,14 +148,14 @@ public function testRoleAccess($path, $grant_accounts, $deny_accounts) { 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); + $this->assertEquals(AccessResult::allowed()->cachePerRole(), $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); + $this->assertEquals(AccessResult::create()->cachePerRole(), $has_access, $message); } }