core/lib/Drupal/Core/Entity/EntityCreateAccessCheck.php | 2 +- core/modules/block/src/BlockAccessControlHandler.php | 5 ++++- core/modules/comment/src/CommentAccessControlHandler.php | 9 +++++++-- core/modules/media/src/MediaAccessControlHandler.php | 6 +++++- .../src/MenuLinkContentAccessControlHandler.php | 4 +++- .../rest/src/Plugin/rest/resource/EntityResource.php | 7 ++++++- .../modules/config_test_rest/config_test_rest.module | 7 ++++++- .../EntityResource/Block/BlockResourceTestBase.php | 2 +- .../EntityResource/Comment/CommentResourceTestBase.php | 6 ++++-- .../ConfigTest/ConfigTestResourceTestBase.php | 16 ++++++++++++++++ .../EntityResource/Media/MediaResourceTestBase.php | 2 +- .../MenuLinkContent/MenuLinkContentResourceTestBase.php | 2 +- .../ShortcutSet/ShortcutSetResourceTestBase.php | 16 ++++++++++++++++ .../EntityResource/User/UserResourceTestBase.php | 2 +- .../shortcut/src/ShortcutSetAccessControlHandler.php | 2 +- core/modules/user/src/UserAccessControlHandler.php | 7 ++++++- 16 files changed, 79 insertions(+), 16 deletions(-) diff --git a/core/lib/Drupal/Core/Entity/EntityCreateAccessCheck.php b/core/lib/Drupal/Core/Entity/EntityCreateAccessCheck.php index 9ceac52..b3c12a8 100644 --- a/core/lib/Drupal/Core/Entity/EntityCreateAccessCheck.php +++ b/core/lib/Drupal/Core/Entity/EntityCreateAccessCheck.php @@ -62,7 +62,7 @@ public function access(Route $route, RouteMatchInterface $route_match, AccountIn } // If we were unable to replace all placeholders, deny access. if (strpos($bundle, '{') !== FALSE) { - return AccessResult::neutral(); + return AccessResult::neutral(sprintf("Could not find '%s' request argument, therefore cannot check create access.", $bundle)); } } return $this->entityManager->getAccessControlHandler($entity_type)->createAccess($bundle, $account, [], TRUE); diff --git a/core/modules/block/src/BlockAccessControlHandler.php b/core/modules/block/src/BlockAccessControlHandler.php index 35af61e..0ce8384 100644 --- a/core/modules/block/src/BlockAccessControlHandler.php +++ b/core/modules/block/src/BlockAccessControlHandler.php @@ -128,7 +128,10 @@ protected function checkAccess(EntityInterface $entity, $operation, AccountInter } } else { - $access = AccessResult::forbidden(); + $reason = count($conditions) > 1 + ? "One of the block visibility conditions ('%s') denied access." + : "The block visibility condition '%s' denied access."; + $access = AccessResult::forbidden(sprintf($reason, implode("', '", array_keys($conditions)))); } $this->mergeCacheabilityFromConditions($access, $conditions); diff --git a/core/modules/comment/src/CommentAccessControlHandler.php b/core/modules/comment/src/CommentAccessControlHandler.php index bcb0fd7..1c152d3 100644 --- a/core/modules/comment/src/CommentAccessControlHandler.php +++ b/core/modules/comment/src/CommentAccessControlHandler.php @@ -3,6 +3,7 @@ namespace Drupal\comment; use Drupal\Core\Access\AccessResult; +use Drupal\Core\Access\AccessResultReasonInterface; use Drupal\Core\Entity\EntityAccessControlHandler; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Field\FieldDefinitionInterface; @@ -45,11 +46,15 @@ protected function checkAccess(EntityInterface $entity, $operation, AccountInter return $access_result; case 'update': - return AccessResult::allowedIf($account->id() && $account->id() == $entity->getOwnerId() && $entity->isPublished() && $account->hasPermission('edit own comments'))->cachePerPermissions()->cachePerUser()->addCacheableDependency($entity); + $access_result = AccessResult::allowedIf($account->id() && $account->id() == $entity->getOwnerId() && $entity->isPublished() && $account->hasPermission('edit own comments'))->cachePerPermissions()->cachePerUser()->addCacheableDependency($entity); + if (!$access_result->isAllowed() && $access_result instanceof AccessResultReasonInterface) { + $access_result->setReason("The 'edit own comments' permission is required and the comment must be published."); + } + return $access_result; default: // No opinion. - return AccessResult::neutral()->cachePerPermissions(); + return AccessResult::neutral("The 'administer comments' permission is required.")->cachePerPermissions(); } } diff --git a/core/modules/media/src/MediaAccessControlHandler.php b/core/modules/media/src/MediaAccessControlHandler.php index 3a59c33..a8779e7 100644 --- a/core/modules/media/src/MediaAccessControlHandler.php +++ b/core/modules/media/src/MediaAccessControlHandler.php @@ -36,10 +36,14 @@ protected function checkAccess(EntityInterface $entity, $operation, AccountInter if ($account->hasPermission('update any media')) { return AccessResult::allowed()->cachePerPermissions(); } - return AccessResult::allowedIf($account->hasPermission('update media') && $is_owner) + $access_result = AccessResult::allowedIf($account->hasPermission('update media') && $is_owner) ->cachePerPermissions() ->cachePerUser() ->addCacheableDependency($entity); + if (!$access_result->isAllowed() && $access_result instanceof AccessResultReasonInterface) { + $access_result->setReason("As a non-owner of this media item, the 'update any media' permission is required; as an owner of this media, the 'update media' permission is required."); + } + return $access_result; case 'delete': if ($account->hasPermission('delete any media')) { diff --git a/core/modules/menu_link_content/src/MenuLinkContentAccessControlHandler.php b/core/modules/menu_link_content/src/MenuLinkContentAccessControlHandler.php index eadf045..9e6d814 100644 --- a/core/modules/menu_link_content/src/MenuLinkContentAccessControlHandler.php +++ b/core/modules/menu_link_content/src/MenuLinkContentAccessControlHandler.php @@ -4,6 +4,7 @@ use Drupal\Core\Access\AccessResult; use Drupal\Core\Access\AccessManagerInterface; +use Drupal\Core\Access\AccessResultReasonInterface; use Drupal\Core\Entity\EntityAccessControlHandler; use Drupal\Core\Entity\EntityHandlerInterface; use Drupal\Core\Entity\EntityInterface; @@ -72,7 +73,8 @@ protected function checkAccess(EntityInterface $entity, $operation, AccountInter } case 'delete': - return AccessResult::allowedIf(!$entity->isNew() && $account->hasPermission('administer menu'))->cachePerPermissions()->addCacheableDependency($entity); + return AccessResult::allowedIfHasPermission($account, 'administer menu') + ->andIf(AccessResult::allowedIf(!$entity->isNew())->addCacheableDependency($entity)); } } diff --git a/core/modules/rest/src/Plugin/rest/resource/EntityResource.php b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php index a945b83..be77872 100644 --- a/core/modules/rest/src/Plugin/rest/resource/EntityResource.php +++ b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php @@ -346,7 +346,12 @@ protected function getBaseRoute($canonical_path, $method) { $route->setRequirement('_entity_access', $this->entityType->id() . '.view'); break; case 'POST': - $route->setRequirement('_entity_create_access', $this->entityType->id()); + if ($this->entityType->getBundleEntityType()) { + $route->setRequirement('_entity_create_access', $this->entityType->id() . ':{' . $this->entityType->getBundleEntityType() . '}'); + } + else { + $route->setRequirement('_entity_create_access', $this->entityType->id()); + } break; case 'PATCH': $route->setRequirement('_entity_access', $this->entityType->id() . '.update'); diff --git a/core/modules/rest/tests/modules/config_test_rest/config_test_rest.module b/core/modules/rest/tests/modules/config_test_rest/config_test_rest.module index fcd9979..aa9faba 100644 --- a/core/modules/rest/tests/modules/config_test_rest/config_test_rest.module +++ b/core/modules/rest/tests/modules/config_test_rest/config_test_rest.module @@ -6,6 +6,7 @@ */ use Drupal\Core\Access\AccessResult; +use Drupal\Core\Access\AccessResultReasonInterface; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Session\AccountInterface; @@ -26,5 +27,9 @@ function config_test_rest_config_test_access(EntityInterface $entity, $operation // Add permission, so that EntityResourceTestBase's scenarios can test access // being denied. By default, all access is always allowed for the config_test // config entity. - return AccessResult::forbiddenIf(!$account->hasPermission('view config_test'))->cachePerPermissions(); + $access_result = AccessResult::forbiddenIf(!$account->hasPermission('view config_test'))->cachePerPermissions(); + if (!$access_result->isAllowed() && $access_result instanceof AccessResultReasonInterface) { + $access_result->setReason("The 'view config_test' permission is required."); + } + return $access_result; } diff --git a/core/modules/rest/tests/src/Functional/EntityResource/Block/BlockResourceTestBase.php b/core/modules/rest/tests/src/Functional/EntityResource/Block/BlockResourceTestBase.php index d86f9b1..2045e7f 100644 --- a/core/modules/rest/tests/src/Functional/EntityResource/Block/BlockResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/EntityResource/Block/BlockResourceTestBase.php @@ -135,7 +135,7 @@ protected function getExpectedUnauthorizedAccessMessage($method) { switch ($method) { case 'GET': - return "You are not authorized to view this block entity."; + return "The block visibility condition 'user_role' denied access."; default: return parent::getExpectedUnauthorizedAccessMessage($method); } diff --git a/core/modules/rest/tests/src/Functional/EntityResource/Comment/CommentResourceTestBase.php b/core/modules/rest/tests/src/Functional/EntityResource/Comment/CommentResourceTestBase.php index bade2a7..b22b9dd 100644 --- a/core/modules/rest/tests/src/Functional/EntityResource/Comment/CommentResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/EntityResource/Comment/CommentResourceTestBase.php @@ -317,10 +317,12 @@ protected function getExpectedUnauthorizedAccessMessage($method) { switch ($method) { case 'GET'; return "The 'access comments' permission is required and the comment must be published."; + case 'PATCH': + return "The 'edit own comments' permission is required and the comment must be published."; case 'POST'; return "The 'post comments' permission is required."; - default: - return parent::getExpectedUnauthorizedAccessMessage($method); + case 'DELETE': + return "The 'administer comments' permission is required."; } } diff --git a/core/modules/rest/tests/src/Functional/EntityResource/ConfigTest/ConfigTestResourceTestBase.php b/core/modules/rest/tests/src/Functional/EntityResource/ConfigTest/ConfigTestResourceTestBase.php index 9fe073b..e3e3099 100644 --- a/core/modules/rest/tests/src/Functional/EntityResource/ConfigTest/ConfigTestResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/EntityResource/ConfigTest/ConfigTestResourceTestBase.php @@ -70,4 +70,20 @@ protected function getNormalizedPostEntity() { // @todo Update in https://www.drupal.org/node/2300677. } + /** + * {@inheritdoc} + */ + protected function getExpectedUnauthorizedAccessMessage($method) { + if ($this->config('rest.settings')->get('bc_entity_resource_permissions')) { + return parent::getExpectedUnauthorizedAccessMessage($method); + } + + switch ($method) { + case 'GET': + return "The 'view config_test' permission is required."; + default: + return parent::getExpectedUnauthorizedAccessMessage($method); + } + } + } diff --git a/core/modules/rest/tests/src/Functional/EntityResource/Media/MediaResourceTestBase.php b/core/modules/rest/tests/src/Functional/EntityResource/Media/MediaResourceTestBase.php index 179f2d6..f32fa59 100644 --- a/core/modules/rest/tests/src/Functional/EntityResource/Media/MediaResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/EntityResource/Media/MediaResourceTestBase.php @@ -244,7 +244,7 @@ protected function getExpectedUnauthorizedAccessMessage($method) { return "The 'view media' permission is required and the media item must be published."; case 'PATCH': - return 'You are not authorized to update this media entity of bundle camelids.'; + return "As a non-owner of this media item, the 'update any media' permission is required; as an owner of this media, the 'update media' permission is required."; case 'DELETE': return "As a non-owner of this media item, the 'delete any media' permission is required; as an owner of this media, the 'delete media' permission is required."; diff --git a/core/modules/rest/tests/src/Functional/EntityResource/MenuLinkContent/MenuLinkContentResourceTestBase.php b/core/modules/rest/tests/src/Functional/EntityResource/MenuLinkContent/MenuLinkContentResourceTestBase.php index 0b1f967..5ad4ad3 100644 --- a/core/modules/rest/tests/src/Functional/EntityResource/MenuLinkContent/MenuLinkContentResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/EntityResource/MenuLinkContent/MenuLinkContentResourceTestBase.php @@ -185,7 +185,7 @@ protected function getExpectedUnauthorizedAccessMessage($method) { switch ($method) { case 'DELETE': - return "You are not authorized to delete this menu_link_content entity."; + return "The 'administer menu' permission is required."; default: return parent::getExpectedUnauthorizedAccessMessage($method); } diff --git a/core/modules/rest/tests/src/Functional/EntityResource/ShortcutSet/ShortcutSetResourceTestBase.php b/core/modules/rest/tests/src/Functional/EntityResource/ShortcutSet/ShortcutSetResourceTestBase.php index 97ae1a6..e8ce93f 100644 --- a/core/modules/rest/tests/src/Functional/EntityResource/ShortcutSet/ShortcutSetResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/EntityResource/ShortcutSet/ShortcutSetResourceTestBase.php @@ -85,4 +85,20 @@ protected function getNormalizedPostEntity() { // @todo Update in https://www.drupal.org/node/2300677. } + /** + * {@inheritdoc} + */ + protected function getExpectedUnauthorizedAccessMessage($method) { + if ($this->config('rest.settings')->get('bc_entity_resource_permissions')) { + return parent::getExpectedUnauthorizedAccessMessage($method); + } + + switch ($method) { + case 'GET': + return "The 'access shortcuts' permission is required."; + default: + return parent::getExpectedUnauthorizedAccessMessage($method); + } + } + } diff --git a/core/modules/rest/tests/src/Functional/EntityResource/User/UserResourceTestBase.php b/core/modules/rest/tests/src/Functional/EntityResource/User/UserResourceTestBase.php index 3db06b3..ddf6814 100644 --- a/core/modules/rest/tests/src/Functional/EntityResource/User/UserResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/EntityResource/User/UserResourceTestBase.php @@ -255,7 +255,7 @@ protected function getExpectedUnauthorizedAccessMessage($method) { case 'GET': return "The 'access user profiles' permission is required and the user must be active."; case 'PATCH': - return "You are not authorized to update this user entity."; + return "Users can only update their own account, unless they have the 'administer users' permission."; case 'DELETE': return "The 'cancel account' permission is required."; default: diff --git a/core/modules/shortcut/src/ShortcutSetAccessControlHandler.php b/core/modules/shortcut/src/ShortcutSetAccessControlHandler.php index 3a55f74..87be25d 100644 --- a/core/modules/shortcut/src/ShortcutSetAccessControlHandler.php +++ b/core/modules/shortcut/src/ShortcutSetAccessControlHandler.php @@ -20,7 +20,7 @@ class ShortcutSetAccessControlHandler extends EntityAccessControlHandler { protected function checkAccess(EntityInterface $entity, $operation, AccountInterface $account) { switch ($operation) { case 'view': - return AccessResult::allowedIf($account->hasPermission('access shortcuts'))->cachePerPermissions(); + return AccessResult::allowedIfHasPermission($account, 'access shortcuts'); case 'update': if ($account->hasPermission('administer shortcuts')) { return AccessResult::allowed()->cachePerPermissions(); diff --git a/core/modules/user/src/UserAccessControlHandler.php b/core/modules/user/src/UserAccessControlHandler.php index f2f23db..4020efb 100644 --- a/core/modules/user/src/UserAccessControlHandler.php +++ b/core/modules/user/src/UserAccessControlHandler.php @@ -4,6 +4,7 @@ use Drupal\Core\Access\AccessResult; use Drupal\Core\Access\AccessResultNeutral; +use Drupal\Core\Access\AccessResultReasonInterface; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Entity\EntityAccessControlHandler; use Drupal\Core\Field\FieldDefinitionInterface; @@ -64,7 +65,11 @@ protected function checkAccess(EntityInterface $entity, $operation, AccountInter case 'update': // Users can always edit their own account. - return AccessResult::allowedIf($account->id() == $entity->id())->cachePerUser(); + $access_result = AccessResult::allowedIf($account->id() == $entity->id())->cachePerUser(); + if (!$access_result->isAllowed() && $access_result instanceof AccessResultReasonInterface) { + $access_result->setReason("Users can only update their own account, unless they have the 'administer users' permission."); + } + return $access_result; case 'delete': // Users with 'cancel account' permission can cancel their own account.