From 5b2cb036e898c69905156b080c121aac49e7f91c Mon Sep 17 00:00:00 2001
From: Nathaniel Catchpole <catch@35733.no-reply.drupal.org>
Date: Wed, 8 Feb 2017 14:14:09 +0000
Subject: [PATCH] Issue #2847687 by jp.stacey, dawehner: EntityAccessCheck
mandates instanceof EntityInterface when it only needs AccessibleInterface
---
.../Drupal/Core/Entity/EntityAccessCheck.php | 20 +++++---
.../Core/Entity/EntityAccessCheckTest.php | 50 +++++++++++++++++--
2 files changed, 58 insertions(+), 12 deletions(-)
diff --git a/core/lib/Drupal/Core/Entity/EntityAccessCheck.php b/core/lib/Drupal/Core/Entity/EntityAccessCheck.php
index bc8bd5973925..8ca8cf66cded 100644
--- a/core/lib/Drupal/Core/Entity/EntityAccessCheck.php
+++ b/core/lib/Drupal/Core/Entity/EntityAccessCheck.php
@@ -16,28 +16,32 @@ class EntityAccessCheck implements AccessInterface {
/**
* Checks access to the entity operation on the given route.
*
- * The value of the '_entity_access' key must be in the pattern
- * 'entity_slug_name.operation.' For example, this will check a node for
- * 'update' access:
+ * The route's '_entity_access' requirement must follow the pattern
+ * 'entity_stub_name.operation', where available operations are:
+ * 'view', 'update', 'create', and 'delete'.
+ *
+ * For example, this route configuration invokes a permissions check for
+ * 'update' access to entities of type 'node':
* @code
* pattern: '/foo/{node}/bar'
* requirements:
* _entity_access: 'node.update'
* @endcode
- * And this will check a dynamic entity type:
+ * And this will check 'delete' access to a dynamic entity type:
* @code
* example.route:
* path: foo/{entity_type}/{example}
* requirements:
- * _entity_access: example.update
+ * _entity_access: example.delete
* options:
* parameters:
* example:
* type: entity:{entity_type}
* @endcode
- * @see \Drupal\Core\ParamConverter\EntityConverter
+ * The route match parameter corresponding to the stub name is checked to
+ * see if it is entity-like i.e. implements EntityInterface.
*
- * Available operations are 'view', 'update', 'create', and 'delete'.
+ * @see \Drupal\Core\ParamConverter\EntityConverter
*
* @param \Symfony\Component\Routing\Route $route
* The route to check against.
@@ -53,7 +57,7 @@ public function access(Route $route, RouteMatchInterface $route_match, AccountIn
// Split the entity type and the operation.
$requirement = $route->getRequirement('_entity_access');
list($entity_type, $operation) = explode('.', $requirement);
- // If there is valid entity of the given entity type, check its access.
+ // If $entity_type parameter is a valid entity, call its own access check.
$parameters = $route_match->getParameters();
if ($parameters->has($entity_type)) {
$entity = $parameters->get($entity_type);
diff --git a/core/tests/Drupal/Tests/Core/Entity/EntityAccessCheckTest.php b/core/tests/Drupal/Tests/Core/Entity/EntityAccessCheckTest.php
index f99160bbfb20..e08c37ac1479 100644
--- a/core/tests/Drupal/Tests/Core/Entity/EntityAccessCheckTest.php
+++ b/core/tests/Drupal/Tests/Core/Entity/EntityAccessCheckTest.php
@@ -9,8 +9,10 @@
use Drupal\node\NodeInterface;
use Symfony\Component\HttpFoundation\ParameterBag;
use Symfony\Component\Routing\Route;
+use Drupal\Core\Access\AccessibleInterface;
use Drupal\Core\Access\AccessResult;
use Drupal\Core\Entity\EntityAccessCheck;
+use Drupal\Core\Entity\EntityInterface;
use Drupal\Tests\UnitTestCase;
/**
@@ -70,13 +72,53 @@ public function testAccessWithTypePlaceholder() {
$node = $node->reveal();
/** @var \Drupal\Core\Routing\RouteMatchInterface|\Prophecy\Prophecy\ObjectProphecy $route_match */
- $route_match = $this->prophesize(RouteMatchInterface::class);
- $route_match->getRawParameters()->willReturn(new ParameterBag(['entity_type' => 'node', 'var_name' => 1]));
- $route_match->getParameters()->willReturn(new ParameterBag(['entity_type' => 'node', 'var_name' => $node]));
- $route_match = $route_match->reveal();
+ $route_match = $this->createRouteMatchForObject($node);
$access_check = new EntityAccessCheck();
$this->assertEquals(AccessResult::allowed(), $access_check->access($route, $route_match, $account));
}
+ /**
+ * @covers ::access
+ */
+ public function testAccessWithDifferentRouteParameters() {
+ $route = new Route(
+ '/foo/{var_name}',
+ [],
+ ['_entity_access' => 'var_name.update'],
+ ['parameters' => ['var_name' => ['type' => 'entity:node']]]
+ );
+ /** @var \Drupal\Core\Session\AccountInterface $account */
+ $account = $this->prophesize(AccountInterface::class)->reveal();
+ $access_check = new EntityAccessCheck();
+
+ // Confirm an EntityInterface route parameter's ::access() is called.
+ /** @var \Drupal\Core\Entity\EntityInterface|\Prophecy\Prophecy\ObjectProphecy $node */
+ $node = $this->prophesize(EntityInterface::class);
+ $node->access('update', $account, TRUE)->willReturn(AccessResult::allowed());
+ $route_match = $this->createRouteMatchForObject($node->reveal());
+ $this->assertEquals(AccessResult::allowed(), $access_check->access($route, $route_match, $account));
+
+ // AccessibleInterface is not entity-like: ::access() should not be called.
+ /** @var \Drupal\Core\Access\AccessibleInterface|\Prophecy\Prophecy\ObjectProphecy $node */
+ $node = $this->prophesize(AccessibleInterface::class);
+ $node->access('update', $account, TRUE)->willReturn(AccessResult::allowed());
+ $route_match = $this->createRouteMatchForObject($node->reveal());
+ $this->assertEquals(AccessResult::neutral(), $access_check->access($route, $route_match, $account));
+ }
+
+ /**
+ * Wrap any object with a route match, and return that.
+ *
+ * @param \stdClass $object
+ * Any object, including prophesized mocks based on interfaces.
+ * @return RouteMatchInterface
+ * A prophesized RouteMatchInterface.
+ */
+ private function createRouteMatchForObject(\stdClass $object) {
+ $route_match = $this->prophesize(RouteMatchInterface::class);
+ $route_match->getRawParameters()->willReturn(new ParameterBag(['entity_type' => 'node', 'var_name' => 1]));
+ $route_match->getParameters()->willReturn(new ParameterBag(['entity_type' => 'node', 'var_name' => $object]));
+ return $route_match->reveal();
+ }
}
--
GitLab