Commit 0f28b515 authored by catch's avatar catch

Issue #2287071 by Wim Leers, alexpott, chx: Add cacheability metadata to access checks.

parent 6f0b27bf
<?php
/**
* @file
* Contains \Drupal\Core\Access\AccessInterface.
*/
namespace Drupal\Core\Access;
/**
* Provides access check results.
*/
interface AccessInterface {
/**
* Grant access.
*
* A checker should return this value to indicate that it grants access.
*/
const ALLOW = 'ALLOW';
/**
* Deny access.
*
* A checker should return this value to indicate it does not grant access.
*/
const DENY = 'DENY';
/**
* Block access.
*
* A checker should return this value to indicate that it wants to completely
* block access, regardless of any other access checkers. Most checkers
* should prefer AccessInterface::DENY.
*/
const KILL = 'KILL';
}
......@@ -9,6 +9,7 @@
use Drupal\Core\ParamConverter\ParamConverterManagerInterface;
use Drupal\Core\ParamConverter\ParamNotConvertedException;
use Drupal\Core\Routing\Access\AccessInterface;
use Drupal\Core\Routing\RequestHelper;
use Drupal\Core\Routing\RouteProviderInterface;
use Drupal\Core\Session\AccountInterface;
......@@ -42,7 +43,7 @@ class AccessManager implements ContainerAwareInterface, AccessManagerInterface {
/**
* Array of access check objects keyed by service id.
*
* @var array
* @var \Drupal\Core\Routing\Access\AccessInterface[]
*/
protected $checks;
......@@ -192,7 +193,7 @@ protected function applies(Route $route) {
/**
* {@inheritdoc}
*/
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) {
try {
$route = $this->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;
......
......@@ -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);
}
This diff is collapsed.
<?php
/**
* @file
* Contains \Drupal\Core\Access\AccessResultInterface.
*/
namespace Drupal\Core\Access;
/**
* Interface for access result value objects.
*
* IMPORTANT NOTE: You have to call isAllowed() when you want to know whether
* someone has access. Just using
* @code
* if ($access_result) {
* // The user has access!
* }
* else {
* // The user doesn't have access!
* }
* @endcode
* would never enter the else-statement and hence introduce a critical security
* issue.
*
* Note: you can check whether access is neither explicitly allowed nor
* explicitly forbidden:
*
* @code
* $no_opinion = !$access->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);
}
......@@ -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);
}
......@@ -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);
}
}
......@@ -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'));
......
......@@ -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();
}
}
......
......@@ -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;
}
/**
......
......@@ -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();
......
......@@ -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);
}
/**
......
......@@ -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);
}
/**
......
......@@ -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();
}
}
......@@ -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