diff --git a/core/modules/jsonapi/jsonapi.services.yml b/core/modules/jsonapi/jsonapi.services.yml index 8e5f4a8fd59..56e585f9007 100644 --- a/core/modules/jsonapi/jsonapi.services.yml +++ b/core/modules/jsonapi/jsonapi.services.yml @@ -156,8 +156,6 @@ services: public: false arguments: ['@jsonapi.resource_type.repository', '@router.no_access_checks', '@current_user', '@entity.repository'] calls: - - [setNodeRevisionAccessCheck, ['@?access_check.node.revision']] # This is only injected when the service is available. - - [setMediaRevisionAccessCheck, ['@?access_check.media.revision']] # This is only injected when the service is available. # This is a temporary measure. JSON:API should not need to be aware of the Content Moderation module. - [setLatestRevisionCheck, ['@?access_check.latest_revision']] # This is only injected when the service is available. access_check.jsonapi.relationship_route_access: diff --git a/core/modules/jsonapi/src/Access/EntityAccessChecker.php b/core/modules/jsonapi/src/Access/EntityAccessChecker.php index 1a5b3ea19ce..779a34f5c1f 100644 --- a/core/modules/jsonapi/src/Access/EntityAccessChecker.php +++ b/core/modules/jsonapi/src/Access/EntityAccessChecker.php @@ -15,10 +15,6 @@ use Drupal\jsonapi\JsonApiResource\LabelOnlyResourceObject; use Drupal\jsonapi\JsonApiResource\ResourceObject; use Drupal\jsonapi\JsonApiSpec; use Drupal\jsonapi\ResourceType\ResourceTypeRepositoryInterface; -use Drupal\media\Access\MediaRevisionAccessCheck; -use Drupal\media\MediaInterface; -use Drupal\node\Access\NodeRevisionAccessCheck; -use Drupal\node\NodeInterface; use Symfony\Component\Routing\RouterInterface; /** @@ -64,24 +60,6 @@ class EntityAccessChecker { */ protected $entityRepository; - /** - * The node revision access check service. - * - * This will be NULL unless the node module is installed. - * - * @var \Drupal\node\Access\NodeRevisionAccessCheck|null - */ - protected $nodeRevisionAccessCheck = NULL; - - /** - * The media revision access check service. - * - * This will be NULL unless the media module is installed. - * - * @var \Drupal\media\Access\MediaRevisionAccessCheck|null - */ - protected $mediaRevisionAccessCheck = NULL; - /** * The latest revision check service. * @@ -112,30 +90,6 @@ class EntityAccessChecker { $this->entityRepository = $entity_repository; } - /** - * Sets the node revision access check service. - * - * This is only called when node module is installed. - * - * @param \Drupal\node\Access\NodeRevisionAccessCheck $node_revision_access_check - * The node revision access check service. - */ - public function setNodeRevisionAccessCheck(NodeRevisionAccessCheck $node_revision_access_check) { - $this->nodeRevisionAccessCheck = $node_revision_access_check; - } - - /** - * Sets the media revision access check service. - * - * This is only called when media module is installed. - * - * @param \Drupal\media\Access\MediaRevisionAccessCheck $media_revision_access_check - * The media revision access check service. - */ - public function setMediaRevisionAccessCheck(MediaRevisionAccessCheck $media_revision_access_check) { - $this->mediaRevisionAccessCheck = $media_revision_access_check; - } - /** * Sets the media revision access check service. * @@ -235,10 +189,6 @@ class EntityAccessChecker { * * @return \Drupal\Core\Access\AccessResultInterface|\Drupal\Core\Access\AccessResultReasonInterface * The access check result. - * - * @todo: remove when a generic revision access API exists in Drupal core, and - * also remove the injected "node" and "media" services. - * @see https://www.drupal.org/project/drupal/issues/2992833#comment-12818386 */ protected function checkRevisionViewAccess(EntityInterface $entity, AccountInterface $account) { assert($entity instanceof RevisionableInterface); @@ -246,13 +196,8 @@ class EntityAccessChecker { $entity_type = $entity->getEntityType(); switch ($entity_type->id()) { case 'node': - assert($entity instanceof NodeInterface); - $access = AccessResult::allowedIf($this->nodeRevisionAccessCheck->checkAccess($entity, $account, 'view'))->cachePerPermissions()->addCacheableDependency($entity); - break; - case 'media': - assert($entity instanceof MediaInterface); - $access = AccessResult::allowedIf($this->mediaRevisionAccessCheck->checkAccess($entity, $account, 'view'))->cachePerPermissions()->addCacheableDependency($entity); + $access = $entity->access('view all revisions', $account, TRUE); break; default: diff --git a/core/modules/jsonapi/tests/src/Functional/ResourceTestBase.php b/core/modules/jsonapi/tests/src/Functional/ResourceTestBase.php index d7dae50d0c1..ba22bb7db7e 100644 --- a/core/modules/jsonapi/tests/src/Functional/ResourceTestBase.php +++ b/core/modules/jsonapi/tests/src/Functional/ResourceTestBase.php @@ -503,6 +503,16 @@ abstract class ResourceTestBase extends BrowserTestBase { return Cache::mergeTags($expected_cache_tags, $this->entity->getCacheTags()); } + /** + * The expected cache tags when checking revision responses. + * + * @return string[] + * A set of cache tags. + */ + protected function getExtraRevisionCacheTags() { + return []; + } + /** * The expected cache contexts for the GET/HEAD response of the test entity. * @@ -2945,7 +2955,7 @@ abstract class ResourceTestBase extends BrowserTestBase { // object. $expected_document['data']['links']['latest-version']['href'] = $rel_latest_version_url->setAbsolute()->toString(); $expected_document['data']['links']['working-copy']['href'] = $rel_working_copy_url->setAbsolute()->toString(); - $this->assertResourceResponse(200, $expected_document, $actual_response, $expected_cache_tags, $expected_cache_contexts, FALSE, 'MISS'); + $this->assertResourceResponse(200, $expected_document, $actual_response, Cache::mergeTags($expected_cache_tags, $this->getExtraRevisionCacheTags()), $expected_cache_contexts, FALSE, 'MISS'); // Install content_moderation module. $this->assertTrue($this->container->get('module_installer')->install(['content_moderation'], TRUE), 'Installed modules.'); @@ -3109,46 +3119,47 @@ abstract class ResourceTestBase extends BrowserTestBase { $expected_document['data']['links']['latest-version']['href'] = $rel_latest_version_url->setAbsolute()->toString(); $expected_cache_tags = $this->getExpectedCacheTags(); $expected_cache_contexts = $this->getExpectedCacheContexts(); - $this->assertResourceResponse(200, $expected_document, $actual_response, $expected_cache_tags, $expected_cache_contexts, FALSE, 'MISS'); + $this->assertResourceResponse(200, $expected_document, $actual_response, Cache::mergeTags($expected_cache_tags, $this->getExtraRevisionCacheTags()), $expected_cache_contexts, FALSE, 'MISS'); // And the collection response should also have the latest revision. $actual_response = $this->request('GET', $rel_working_copy_collection_url, $request_options); $expected_response = static::getExpectedCollectionResponse([$entity], $rel_working_copy_collection_url->toString(), $request_options); $expected_collection_document = $expected_response->getResponseData(); $expected_collection_document['data'] = [$expected_document['data']]; $expected_cacheability = $expected_response->getCacheableMetadata(); - $this->assertResourceResponse(200, $expected_collection_document, $actual_response, $expected_cacheability->getCacheTags(), $expected_cacheability->getCacheContexts(), FALSE, 'MISS'); + $this->assertResourceResponse(200, $expected_collection_document, $actual_response, Cache::mergeTags($expected_cacheability->getCacheTags(), $this->getExtraRevisionCacheTags()), $expected_cacheability->getCacheContexts(), FALSE, 'MISS'); // Test relationship responses. // Fetch the prior revision's relationship URL. $test_relationship_urls = [ - [ + 'canonical' => [ NULL, $relationship_url, $related_url, ], - [ + 'original' => [ $original_revision_id, $original_revision_id_relationship_url, $original_revision_id_related_url, ], - [ + 'latest' => [ $latest_revision_id, $latest_revision_id_relationship_url, $latest_revision_id_related_url, ], - [ + 'default' => [ $default_revision_id, $rel_latest_version_relationship_url, $rel_latest_version_related_url, ], - [ + 'forward' => [ $forward_revision_id, $rel_working_copy_relationship_url, $rel_working_copy_related_url, ], ]; - foreach ($test_relationship_urls as $revision_case) { - list($revision_id, $relationship_url, $related_url) = $revision_case; + $default_revision_types = ['canonical', 'default']; + foreach ($test_relationship_urls as $relationship_type => $revision_case) { + [$revision_id, $relationship_url, $related_url] = $revision_case; // Load the revision that will be requested. $this->entityStorage->resetCache([$entity->id()]); $revision = is_null($revision_id) @@ -3161,7 +3172,9 @@ abstract class ResourceTestBase extends BrowserTestBase { $expected_document = $expected_response->getResponseData(); $expected_cacheability = $expected_response->getCacheableMetadata(); $expected_document['errors'][0]['links']['via']['href'] = $relationship_url->toString(); - $this->assertResourceResponse(403, $expected_document, $actual_response, $expected_cacheability->getCacheTags(), $expected_cacheability->getCacheContexts()); + // Only add node type check tags for non-default revisions. + $expected_cache_tags = !in_array($relationship_type, $default_revision_types, TRUE) ? Cache::mergeTags($expected_cacheability->getCacheTags(), $this->getExtraRevisionCacheTags()) : $expected_cacheability->getCacheTags(); + $this->assertResourceResponse(403, $expected_document, $actual_response, $expected_cache_tags, $expected_cacheability->getCacheContexts()); // Request the related route. $actual_response = $this->request('GET', $related_url, $request_options); // @todo: refactor self::getExpectedRelatedResponses() into a function which returns a single response. @@ -3169,11 +3182,11 @@ abstract class ResourceTestBase extends BrowserTestBase { $expected_document = $expected_response->getResponseData(); $expected_cacheability = $expected_response->getCacheableMetadata(); $expected_document['errors'][0]['links']['via']['href'] = $related_url->toString(); - $this->assertResourceResponse(403, $expected_document, $actual_response, $expected_cacheability->getCacheTags(), $expected_cacheability->getCacheContexts()); + $this->assertResourceResponse(403, $expected_document, $actual_response, $expected_cache_tags, $expected_cacheability->getCacheContexts()); } $this->grantPermissionsToTestedRole(['field_jsonapi_test_entity_ref view access']); - foreach ($test_relationship_urls as $revision_case) { - list($revision_id, $relationship_url, $related_url) = $revision_case; + foreach ($test_relationship_urls as $relationship_type => $revision_case) { + [$revision_id, $relationship_url, $related_url] = $revision_case; // Load the revision that will be requested. $this->entityStorage->resetCache([$entity->id()]); $revision = is_null($revision_id) @@ -3186,7 +3199,9 @@ abstract class ResourceTestBase extends BrowserTestBase { $expected_document = $expected_response->getResponseData(); $expected_document['links']['self']['href'] = $relationship_url->setAbsolute()->toString(); $expected_cacheability = $expected_response->getCacheableMetadata(); - $this->assertResourceResponse(200, $expected_document, $actual_response, $expected_cacheability->getCacheTags(), $expected_cacheability->getCacheContexts(), FALSE, 'MISS'); + // Only add node type check tags for non-default revisions. + $expected_cache_tags = !in_array($relationship_type, $default_revision_types, TRUE) ? Cache::mergeTags($expected_cacheability->getCacheTags(), $this->getExtraRevisionCacheTags()) : $expected_cacheability->getCacheTags(); + $this->assertResourceResponse(200, $expected_document, $actual_response, $expected_cache_tags, $expected_cacheability->getCacheContexts(), FALSE, 'MISS'); // Request the related route. $actual_response = $this->request('GET', $related_url, $request_options); $expected_response = $this->getExpectedRelatedResponse('field_jsonapi_test_entity_ref', $request_options, $revision); @@ -3195,7 +3210,8 @@ abstract class ResourceTestBase extends BrowserTestBase { $expected_document['links']['self']['href'] = $related_url->toString(); // MISS or UNCACHEABLE depends on data. It must not be HIT. $dynamic_cache = !empty(array_intersect(['user', 'session'], $expected_cacheability->getCacheContexts())) ? 'UNCACHEABLE' : 'MISS'; - $this->assertResourceResponse(200, $expected_document, $actual_response, $expected_cacheability->getCacheTags(), $expected_cacheability->getCacheContexts(), FALSE, $dynamic_cache); + $expected_cache_tags = !in_array($relationship_type, $default_revision_types, TRUE) ? Cache::mergeTags($expected_cacheability->getCacheTags(), $this->getExtraRevisionCacheTags()) : $expected_cacheability->getCacheTags(); + $this->assertResourceResponse(200, $expected_document, $actual_response, $expected_cache_tags, $expected_cacheability->getCacheContexts(), FALSE, $dynamic_cache); } $this->config('jsonapi.settings')->set('read_only', FALSE)->save(TRUE); diff --git a/core/modules/media/media.routing.yml b/core/modules/media/media.routing.yml index 4c8d8517741..5dc97324d7f 100644 --- a/core/modules/media/media.routing.yml +++ b/core/modules/media/media.routing.yml @@ -10,7 +10,7 @@ entity.media.revision: media_revision: type: entity_revision:media requirements: - _access_media_revision: 'view' + _entity_access: 'media_revision.view all revisions' media: \d+ media.oembed_iframe: diff --git a/core/modules/media/media.services.yml b/core/modules/media/media.services.yml index db7e79a2bc9..847e9e3d485 100644 --- a/core/modules/media/media.services.yml +++ b/core/modules/media/media.services.yml @@ -7,6 +7,7 @@ services: arguments: ['@entity_type.manager'] tags: - { name: access_check, applies_to: _access_media_revision } + deprecated: The "%service_id%" service is deprecated. You should use the 'access_check.entity' service instead. See https://www.drupal.org/node/3161210 media.oembed.url_resolver: class: Drupal\media\OEmbed\UrlResolver arguments: ['@media.oembed.provider_repository', '@media.oembed.resource_fetcher', '@http_client', '@module_handler', '@cache.default'] diff --git a/core/modules/media/src/Access/MediaRevisionAccessCheck.php b/core/modules/media/src/Access/MediaRevisionAccessCheck.php index b4d4daad3f6..627e03d816d 100644 --- a/core/modules/media/src/Access/MediaRevisionAccessCheck.php +++ b/core/modules/media/src/Access/MediaRevisionAccessCheck.php @@ -30,13 +30,6 @@ class MediaRevisionAccessCheck implements AccessInterface { */ protected $mediaAccess; - /** - * A static cache of access checks. - * - * @var array - */ - protected $access = []; - /** * Constructs a new MediaRevisionAccessCheck. * @@ -44,6 +37,8 @@ class MediaRevisionAccessCheck implements AccessInterface { * The entity type manager. */ public function __construct(EntityTypeManagerInterface $entity_type_manager) { + @trigger_error('MediaRevisionAccessCheck is deprecated in drupal:9.3.0 and will be removed before drupal:10.0.0. Use "_entity_access" requirement with relevant operation instead. See https://www.drupal.org/node/3161210', E_USER_DEPRECATED); + $this->mediaStorage = $entity_type_manager->getStorage('media'); $this->mediaAccess = $entity_type_manager->getAccessControlHandler('media'); } @@ -96,50 +91,7 @@ class MediaRevisionAccessCheck implements AccessInterface { return FALSE; } - // Statically cache access by revision ID, language code, user account ID, - // and operation. - $langcode = $media->language()->getId(); - $cid = $media->getRevisionId() . ':' . $langcode . ':' . $account->id() . ':' . $op; - - if (!isset($this->access[$cid])) { - // Perform basic permission checks first. - if (!$account->hasPermission('view all media revisions') && !$account->hasPermission('administer media')) { - $this->access[$cid] = FALSE; - return FALSE; - } - - if ($account->hasPermission('administer media')) { - $this->access[$cid] = TRUE; - } - else { - // First check the access to the default revision and finally, if the - // media passed in is not the default revision then access to that, too. - $this->access[$cid] = $this->mediaAccess->access($this->mediaStorage->load($media->id()), $op, $account) && ($media->isDefaultRevision() || $this->mediaAccess->access($media, $op, $account)); - } - } - - return $this->access[$cid]; - } - - /** - * Counts the number of revisions in the default language. - * - * @param \Drupal\media\MediaInterface $media - * The media item for which to count the revisions. - * - * @return int - * The number of revisions in the default language. - */ - protected function countDefaultLanguageRevisions(MediaInterface $media) { - $entity_type = $media->getEntityType(); - $count = $this->mediaStorage->getQuery() - ->accessCheck(FALSE) - ->allRevisions() - ->condition($entity_type->getKey('id'), $media->id()) - ->condition($entity_type->getKey('default_langcode'), 1) - ->count() - ->execute(); - return $count; + return $this->mediaAccess->access($media, 'view all revisions', $account); } } diff --git a/core/modules/media/src/MediaAccessControlHandler.php b/core/modules/media/src/MediaAccessControlHandler.php index a8fdea2f63e..d5240badf57 100644 --- a/core/modules/media/src/MediaAccessControlHandler.php +++ b/core/modules/media/src/MediaAccessControlHandler.php @@ -4,19 +4,59 @@ namespace Drupal\media; use Drupal\Core\Access\AccessResult; use Drupal\Core\Entity\EntityAccessControlHandler; +use Drupal\Core\Entity\EntityHandlerInterface; use Drupal\Core\Entity\EntityInterface; +use Drupal\Core\Entity\EntityTypeInterface; +use Drupal\Core\Entity\EntityTypeManagerInterface; use Drupal\Core\Session\AccountInterface; +use Symfony\Component\DependencyInjection\ContainerInterface; /** * Defines an access control handler for media items. */ -class MediaAccessControlHandler extends EntityAccessControlHandler { +class MediaAccessControlHandler extends EntityAccessControlHandler implements EntityHandlerInterface { + + /** + * The entity type manager. + * + * @var \Drupal\Core\Entity\EntityTypeManagerInterface + */ + protected $entityTypeManager; + + /** + * Constructs a MediaAccessControlHandler object. + * + * @param \Drupal\Core\Entity\EntityTypeInterface $entity_type + * The entity type definition. + * @param \Drupal\Core\Entity\EntityTypeManagerInterface|null $entity_type_manager + * The entity type manager. + */ + public function __construct(EntityTypeInterface $entity_type, EntityTypeManagerInterface $entity_type_manager = NULL) { + parent::__construct($entity_type); + if (!isset($entity_type_manager)) { + @trigger_error('Calling ' . __METHOD__ . '() without the $entity_type_manager argument is deprecated in drupal:9.3.0 and will be required in drupal:10.0.0. See https://www.drupal.org/node/3214171', E_USER_DEPRECATED); + $entity_type_manager = \Drupal::entityTypeManager(); + } + $this->entityTypeManager = $entity_type_manager; + } + + /** + * {@inheritdoc} + */ + public static function createInstance(ContainerInterface $container, EntityTypeInterface $entity_type) { + return new static( + $entity_type, + $container->get('entity_type.manager'), + ); + } /** * {@inheritdoc} */ protected function checkAccess(EntityInterface $entity, $operation, AccountInterface $account) { - if ($account->hasPermission('administer media')) { + /** @var \Drupal\media\MediaInterface $entity */ + // Allow admin permission to override all operations. + if ($account->hasPermission($this->entityType->getAdminPermission())) { return AccessResult::allowed()->cachePerPermissions(); } @@ -83,6 +123,22 @@ class MediaAccessControlHandler extends EntityAccessControlHandler { } return AccessResult::neutral("The following permissions are required: 'delete any media' OR 'delete own media' OR '$type: delete any media' OR '$type: delete own media'.")->cachePerPermissions(); + case 'view all revisions': + // Perform basic permission checks first. + if (!$account->hasPermission('view all media revisions')) { + return AccessResult::neutral("The 'view all media revisions' permission is required.")->cachePerPermissions(); + } + + // First check the access to the default revision and finally, if the + // media passed in is not the default revision then access to that, + // too. + $media_storage = $this->entityTypeManager->getStorage($entity->getEntityTypeId()); + $access = $this->access($media_storage->load($entity->id()), 'view', $account, TRUE); + if (!$entity->isDefaultRevision()) { + $access = $access->orIf($this->access($entity, 'view', $account, TRUE)); + } + return $access->cachePerPermissions()->addCacheableDependency($entity); + default: return AccessResult::neutral()->cachePerPermissions(); } diff --git a/core/modules/media/tests/src/Kernel/MediaAccessControlHandlerTest.php b/core/modules/media/tests/src/Kernel/MediaAccessControlHandlerTest.php index fd7bb341f50..59fa8af6ace 100644 --- a/core/modules/media/tests/src/Kernel/MediaAccessControlHandlerTest.php +++ b/core/modules/media/tests/src/Kernel/MediaAccessControlHandlerTest.php @@ -4,7 +4,9 @@ namespace Drupal\Tests\media\Kernel; use Drupal\Core\Access\AccessResult; use Drupal\Core\Access\AccessResultInterface; +use Drupal\Core\Entity\EntityTypeInterface; use Drupal\media\Entity\Media; +use Drupal\media\MediaAccessControlHandler; use Drupal\Tests\user\Traits\UserCreationTrait; /** @@ -549,4 +551,16 @@ class MediaAccessControlHandlerTest extends MediaKernelTestBase { return $test_data; } + /** + * Tests MediaAccessControlHandler deprecation. + * + * @group legacy + */ + public function testMediaAccessControlHandlerDeprecation() { + $this->expectDeprecation('Calling Drupal\media\MediaAccessControlHandler::__construct() without the $entity_type_manager argument is deprecated in drupal:9.3.0 and will be required in drupal:10.0.0. See https://www.drupal.org/node/3214171'); + $entity_type = $this->prophesize(EntityTypeInterface::class); + $entity_type->id()->willReturn('media'); + new MediaAccessControlHandler($entity_type->reveal()); + } + } diff --git a/core/modules/node/node.routing.yml b/core/modules/node/node.routing.yml index 7d3f2f3ea8c..a723d43a2b9 100644 --- a/core/modules/node/node.routing.yml +++ b/core/modules/node/node.routing.yml @@ -56,7 +56,7 @@ entity.node.version_history: _title: 'Revisions' _controller: '\Drupal\node\Controller\NodeController::revisionOverview' requirements: - _access_node_revision: 'view' + _entity_access: 'node.view all revisions' node: \d+ options: _node_operation_route: TRUE @@ -70,7 +70,7 @@ entity.node.revision: _controller: '\Drupal\node\Controller\NodeController::revisionShow' _title_callback: '\Drupal\node\Controller\NodeController::revisionPageTitle' requirements: - _access_node_revision: 'view' + _entity_access: 'node_revision.view revision' node: \d+ options: parameters: @@ -85,7 +85,7 @@ node.revision_revert_confirm: _form: '\Drupal\node\Form\NodeRevisionRevertForm' _title: 'Revert to earlier revision' requirements: - _access_node_revision: 'update' + _entity_access: 'node_revision.revert revision' node: \d+ options: _node_operation_route: TRUE @@ -101,7 +101,7 @@ node.revision_revert_translation_confirm: _form: '\Drupal\node\Form\NodeRevisionRevertTranslationForm' _title: 'Revert to earlier revision of a translation' requirements: - _access_node_revision: 'update' + _entity_access: 'node_revision.revert revision' node: \d+ options: _node_operation_route: TRUE @@ -117,7 +117,7 @@ node.revision_delete_confirm: _form: '\Drupal\node\Form\NodeRevisionDeleteForm' _title: 'Delete earlier revision' requirements: - _access_node_revision: 'delete' + _entity_access: 'node_revision.delete revision' node: \d+ options: _node_operation_route: TRUE diff --git a/core/modules/node/node.services.yml b/core/modules/node/node.services.yml index f4c70183dbb..65d4b900669 100644 --- a/core/modules/node/node.services.yml +++ b/core/modules/node/node.services.yml @@ -13,6 +13,7 @@ services: arguments: ['@entity_type.manager'] tags: - { name: access_check, applies_to: _access_node_revision } + deprecated: The "%service_id%" service is deprecated. You should use the 'access_check.entity' service instead. See https://www.drupal.org/node/3161210 access_check.node.add: class: Drupal\node\Access\NodeAddAccessCheck arguments: ['@entity_type.manager'] diff --git a/core/modules/node/src/Access/NodeRevisionAccessCheck.php b/core/modules/node/src/Access/NodeRevisionAccessCheck.php index 31a2f2941dc..c9181e416f0 100644 --- a/core/modules/node/src/Access/NodeRevisionAccessCheck.php +++ b/core/modules/node/src/Access/NodeRevisionAccessCheck.php @@ -23,20 +23,6 @@ class NodeRevisionAccessCheck implements AccessInterface { */ protected $nodeStorage; - /** - * The node access control handler. - * - * @var \Drupal\Core\Entity\EntityAccessControlHandlerInterface - */ - protected $nodeAccess; - - /** - * A static cache of access checks. - * - * @var array - */ - protected $access = []; - /** * Constructs a new NodeRevisionAccessCheck. * @@ -44,8 +30,9 @@ class NodeRevisionAccessCheck implements AccessInterface { * The entity type manager. */ public function __construct(EntityTypeManagerInterface $entity_type_manager) { + @trigger_error('NodeRevisionAccessCheck is deprecated in drupal:9.3.0 and will be removed before drupal:10.0.0. Use "_entity_access" requirement with relevant operation instead. See https://www.drupal.org/node/3161210', E_USER_DEPRECATED); + $this->nodeStorage = $entity_type_manager->getStorage('node'); - $this->nodeAccess = $entity_type_manager->getAccessControlHandler('node'); } /** @@ -90,53 +77,16 @@ class NodeRevisionAccessCheck implements AccessInterface { * TRUE if the operation may be performed, FALSE otherwise. */ public function checkAccess(NodeInterface $node, AccountInterface $account, $op = 'view') { - $map = [ + // Converts legacy operations for this access check to new revision + // operation found in access control handler. + $entity_operation_map = [ 'view' => 'view all revisions', - 'update' => 'revert all revisions', - 'delete' => 'delete all revisions', + 'update' => 'revert revision', + 'delete' => 'delete revision', ]; - $bundle = $node->bundle(); - $type_map = [ - 'view' => "view $bundle revisions", - 'update' => "revert $bundle revisions", - 'delete' => "delete $bundle revisions", - ]; - - if (!$node || !isset($map[$op]) || !isset($type_map[$op])) { - // If there was no node to check against, or the $op was not one of the - // supported ones, we return access denied. - return FALSE; - } - - // Statically cache access by revision ID, language code, user account ID, - // and operation. - $langcode = $node->language()->getId(); - $cid = $node->getRevisionId() . ':' . $langcode . ':' . $account->id() . ':' . $op; - - if (!isset($this->access[$cid])) { - // Perform basic permission checks first. - if (!$account->hasPermission($map[$op]) && !$account->hasPermission($type_map[$op]) && !$account->hasPermission('administer nodes')) { - $this->access[$cid] = FALSE; - return FALSE; - } - - // If this is the default revision, return access denied for revert or - // delete operations. - if ($node->isDefaultRevision() && ($op === 'update' || $op === 'delete')) { - $this->access[$cid] = FALSE; - } - elseif ($account->hasPermission('administer nodes')) { - $this->access[$cid] = TRUE; - } - else { - // First check the access to the default revision and finally, if the - // node passed in is not the default revision then check access to - // that, too. - $this->access[$cid] = $this->nodeAccess->access($this->nodeStorage->load($node->id()), $op, $account) && ($node->isDefaultRevision() || $this->nodeAccess->access($node, $op, $account)); - } - } - - return $this->access[$cid]; + return isset($entity_operation_map[$op]) ? + $node->access($entity_operation_map[$op], $account) : + FALSE; } } diff --git a/core/modules/node/src/Controller/NodeController.php b/core/modules/node/src/Controller/NodeController.php index eb39f42a78c..ed80d7ced7a 100644 --- a/core/modules/node/src/Controller/NodeController.php +++ b/core/modules/node/src/Controller/NodeController.php @@ -154,20 +154,15 @@ class NodeController extends ControllerBase implements ContainerInjectionInterfa * An array as expected by \Drupal\Core\Render\RendererInterface::render(). */ public function revisionOverview(NodeInterface $node) { - $account = $this->currentUser(); $langcode = $node->language()->getId(); $langname = $node->language()->getName(); $languages = $node->getTranslationLanguages(); $has_translations = (count($languages) > 1); $node_storage = $this->entityTypeManager()->getStorage('node'); - $type = $node->getType(); $build['#title'] = $has_translations ? $this->t('@langname revisions for %title', ['@langname' => $langname, '%title' => $node->label()]) : $this->t('Revisions for %title', ['%title' => $node->label()]); $header = [$this->t('Revision'), $this->t('Operations')]; - $revert_permission = (($account->hasPermission("revert $type revisions") || $account->hasPermission('revert all revisions') || $account->hasPermission('administer nodes')) && $node->access('update')); - $delete_permission = (($account->hasPermission("delete $type revisions") || $account->hasPermission('delete all revisions') || $account->hasPermission('administer nodes')) && $node->access('delete')); - $rows = []; $default_revision = $node->getRevisionId(); $current_revision_displayed = FALSE; @@ -231,7 +226,7 @@ class NodeController extends ControllerBase implements ContainerInjectionInterfa } else { $links = []; - if ($revert_permission) { + if ($revision->access('revert revision')) { $links['revert'] = [ 'title' => $vid < $node->getRevisionId() ? $this->t('Revert') : $this->t('Set as current revision'), 'url' => $has_translations ? @@ -240,7 +235,7 @@ class NodeController extends ControllerBase implements ContainerInjectionInterfa ]; } - if ($delete_permission) { + if ($revision->access('delete revision')) { $links['delete'] = [ 'title' => $this->t('Delete'), 'url' => Url::fromRoute('node.revision_delete_confirm', ['node' => $node->id(), 'node_revision' => $vid]), diff --git a/core/modules/node/src/NodeAccessControlHandler.php b/core/modules/node/src/NodeAccessControlHandler.php index 185942c48fe..25b38671318 100644 --- a/core/modules/node/src/NodeAccessControlHandler.php +++ b/core/modules/node/src/NodeAccessControlHandler.php @@ -6,6 +6,7 @@ use Drupal\Core\Access\AccessResult; use Drupal\Core\Cache\RefinableCacheableDependencyInterface; use Drupal\Core\Entity\EntityHandlerInterface; use Drupal\Core\Entity\EntityTypeInterface; +use Drupal\Core\Entity\EntityTypeManagerInterface; use Drupal\Core\Field\FieldDefinitionInterface; use Drupal\Core\Field\FieldItemListInterface; use Drupal\Core\Entity\EntityAccessControlHandler; @@ -28,6 +29,33 @@ class NodeAccessControlHandler extends EntityAccessControlHandler implements Nod */ protected $grantStorage; + /** + * The entity type manager. + * + * @var \Drupal\Core\Entity\EntityTypeManagerInterface + */ + protected $entityTypeManager; + + /** + * Map of revision operations. + * + * Keys contain revision operations, where values are an array containing the + * permission operation and entity operation. + * + * Permission operation is used to build the required permission, e.g. + * 'permissionOperation all revisions', 'permissionOperation type revisions'. + * + * Entity operation is used to determine access, e.g for 'delete revision' + * operation, an account must also have access to 'delete' operation on an + * entity. + */ + protected const REVISION_OPERATION_MAP = [ + 'view all revisions' => ['view', 'view'], + 'view revision' => ['view', 'view'], + 'revert revision' => ['revert', 'update'], + 'delete revision' => ['delete', 'delete'], + ]; + /** * Constructs a NodeAccessControlHandler object. * @@ -35,10 +63,17 @@ class NodeAccessControlHandler extends EntityAccessControlHandler implements Nod * The entity type definition. * @param \Drupal\node\NodeGrantDatabaseStorageInterface $grant_storage * The node grant storage. + * @param \Drupal\Core\Entity\EntityTypeManagerInterface|null $entity_type_manager + * The entity type manager. */ - public function __construct(EntityTypeInterface $entity_type, NodeGrantDatabaseStorageInterface $grant_storage) { + public function __construct(EntityTypeInterface $entity_type, NodeGrantDatabaseStorageInterface $grant_storage, EntityTypeManagerInterface $entity_type_manager = NULL) { parent::__construct($entity_type); $this->grantStorage = $grant_storage; + if (!isset($entity_type_manager)) { + @trigger_error('Calling ' . __METHOD__ . '() without the $entity_type_manager argument is deprecated in drupal:9.3.0 and will be required in drupal:10.0.0. See https://www.drupal.org/node/3214171', E_USER_DEPRECATED); + $entity_type_manager = \Drupal::entityTypeManager(); + } + $this->entityTypeManager = $entity_type_manager; } /** @@ -47,7 +82,8 @@ class NodeAccessControlHandler extends EntityAccessControlHandler implements Nod public static function createInstance(ContainerInterface $container, EntityTypeInterface $entity_type) { return new static( $entity_type, - $container->get('node.grant_storage') + $container->get('node.grant_storage'), + $container->get('entity_type.manager') ); } @@ -57,7 +93,8 @@ class NodeAccessControlHandler extends EntityAccessControlHandler implements Nod public function access(EntityInterface $entity, $operation, AccountInterface $account = NULL, $return_as_object = FALSE) { $account = $this->prepareUser($account); - if ($account->hasPermission('bypass node access')) { + // Only bypass if not a revision operation, to retain compatibility. + if ($account->hasPermission('bypass node access') && !isset(static::REVISION_OPERATION_MAP[$operation])) { $result = AccessResult::allowed()->cachePerPermissions(); return $return_as_object ? $result : $result->isAllowed(); } @@ -104,6 +141,45 @@ class NodeAccessControlHandler extends EntityAccessControlHandler implements Nod return AccessResult::allowed()->cachePerPermissions()->cachePerUser()->addCacheableDependency($node); } + [$revision_permission_operation, $entity_operation] = static::REVISION_OPERATION_MAP[$operation] ?? [ + NULL, + NULL, + ]; + + // Revision operations. + if ($revision_permission_operation) { + $bundle = $node->bundle(); + // If user doesn't have any of these then quit. + if (!$account->hasPermission("$revision_permission_operation all revisions") && !$account->hasPermission("$revision_permission_operation $bundle revisions") && !$account->hasPermission('administer nodes')) { + return AccessResult::neutral()->cachePerPermissions(); + } + + // If the user has the view all revisions permission and this is the view + // all revisions operation then we can allow access. + if ($operation === 'view all revisions') { + return AccessResult::allowed()->cachePerPermissions(); + } + + // If this is the default revision, return access denied for revert or + // delete operations. + if ($node->isDefaultRevision() && ($operation === 'revert revision' || $operation === 'delete revision')) { + return AccessResult::forbidden()->addCacheableDependency($node); + } + elseif ($account->hasPermission('administer nodes')) { + return AccessResult::allowed()->cachePerPermissions(); + } + + // First check the access to the default revision and finally, if the + // node passed in is not the default revision then check access to + // that, too. + $node_storage = $this->entityTypeManager->getStorage($node->getEntityTypeId()); + $access = $this->access($node_storage->load($node->id()), 'view', $account, TRUE); + if (!$node->isDefaultRevision()) { + $access = $access->orIf($this->access($node, 'view', $account, TRUE)); + } + return $access->cachePerPermissions()->addCacheableDependency($node); + } + // Evaluate node grants. $access_result = $this->grantStorage->access($node, $operation, $account); if ($operation === 'view' && $access_result instanceof RefinableCacheableDependencyInterface) { diff --git a/core/modules/node/tests/src/Functional/NodeRevisionPermissionsTest.php b/core/modules/node/tests/src/Functional/NodeRevisionPermissionsTest.php index 3211d94020a..18f7b761bff 100644 --- a/core/modules/node/tests/src/Functional/NodeRevisionPermissionsTest.php +++ b/core/modules/node/tests/src/Functional/NodeRevisionPermissionsTest.php @@ -8,6 +8,7 @@ use Drupal\Tests\Traits\Core\GeneratePermutationsTrait; * Tests user permissions for node revisions. * * @group node + * @group legacy */ class NodeRevisionPermissionsTest extends NodeTestBase { @@ -79,6 +80,7 @@ class NodeRevisionPermissionsTest extends NodeTestBase { * Tests general revision access permissions. */ public function testNodeRevisionAccessAnyType() { + $this->expectDeprecation('NodeRevisionAccessCheck is deprecated in drupal:9.3.0 and will be removed before drupal:10.0.0. Use "_entity_access" requirement with relevant operation instead. See https://www.drupal.org/node/3161210'); // Create three users, one with each revision permission. foreach ($this->map as $op => $permission) { // Create the user. @@ -145,6 +147,7 @@ class NodeRevisionPermissionsTest extends NodeTestBase { * Tests revision access permissions for a specific content type. */ public function testNodeRevisionAccessPerType() { + $this->expectDeprecation('NodeRevisionAccessCheck is deprecated in drupal:9.3.0 and will be removed before drupal:10.0.0. Use "_entity_access" requirement with relevant operation instead. See https://www.drupal.org/node/3161210'); // Create three users, one with each revision permission. foreach ($this->typeMap as $op => $permission) { // Create the user. diff --git a/core/modules/node/tests/src/Unit/NodeOperationAccessTest.php b/core/modules/node/tests/src/Unit/NodeOperationAccessTest.php new file mode 100644 index 00000000000..694590fec44 --- /dev/null +++ b/core/modules/node/tests/src/Unit/NodeOperationAccessTest.php @@ -0,0 +1,294 @@ +getMockBuilder(CacheContextsManager::class) + ->disableOriginalConstructor() + ->getMock(); + $cacheContextsManager->method('assertValidTokens')->willReturn(TRUE); + $container = new ContainerBuilder(); + $container->set('cache_contexts_manager', $cacheContextsManager); + \Drupal::setContainer($container); + } + + /** + * Tests revision operations. + * + * @param string $operation + * A revision operation. + * @param array $hasPermissionMap + * A map of permissions, to whether they should be granted. + * @param bool|null $assertAccess + * Whether the access is allowed or denied. + * @param bool|null $isDefaultRevision + * Whether the node should be default revision, or NULL if not to expect it + * to be called. + * + * @dataProvider providerTestRevisionOperations + */ + public function testRevisionOperations($operation, array $hasPermissionMap, $assertAccess, $isDefaultRevision = NULL) { + $account = $this->createMock(AccountInterface::class); + $account->method('hasPermission') + ->willReturnMap($hasPermissionMap); + + $entityType = $this->createMock(EntityTypeInterface::class); + $grants = $this->createMock(NodeGrantDatabaseStorageInterface::class); + $grants->expects($this->any()) + ->method('access') + ->willReturn(AccessResult::neutral()); + + $language = $this->createMock(LanguageInterface::class); + $language->expects($this->any()) + ->method('getId') + ->will($this->returnValue('de')); + + $nid = 333; + /** @var \Drupal\node\NodeInterface|\PHPUnit\Framework\MockObject\MockObject $node */ + $node = $this->createMock(NodeInterface::class); + $node->expects($this->any()) + ->method('language') + ->willReturn($language); + $node->expects($this->any()) + ->method('id') + ->willReturn($nid); + $node->expects($this->any()) + ->method('getCacheContexts') + ->willReturn([]); + $node->expects($this->any()) + ->method('getCacheTags') + ->willReturn([]); + $node->expects($this->any()) + ->method('getCacheMaxAge') + ->willReturn(-1); + $node->expects($this->any()) + ->method('getEntityTypeId') + ->willReturn('node'); + + if (isset($isDefaultRevision)) { + $node->expects($this->atLeastOnce()) + ->method('isDefaultRevision') + ->willReturn($isDefaultRevision); + } + + $nodeStorage = $this->createMock(NodeStorageInterface::class); + $nodeStorage->expects($this->any()) + ->method('load') + ->with($nid) + ->willReturn($node); + $entityTypeManager = $this->createMock(EntityTypeManagerInterface::class); + $entityTypeManager->expects($this->any()) + ->method('getStorage') + ->with('node') + ->willReturn($nodeStorage); + + $moduleHandler = $this->createMock(ModuleHandlerInterface::class); + $moduleHandler->expects($this->any()) + ->method('invokeAll') + ->willReturn([]); + $accessControl = new NodeAccessControlHandler($entityType, $grants, $entityTypeManager); + $accessControl->setModuleHandler($moduleHandler); + + $nodeType = $this->createMock(RevisionableEntityBundleInterface::class); + $typeProperty = new \stdClass(); + $typeProperty->entity = $nodeType; + $node->type = $typeProperty; + + $access = $accessControl->access($node, $operation, $account, FALSE); + $this->assertEquals($assertAccess, $access); + } + + /** + * Data provider for revisionOperationsProvider. + * + * @return array + * Data for testing. + */ + public function providerTestRevisionOperations() { + $data = []; + + // Tests 'bypass node access' never works on revision operations. + $data['bypass, view all revisions'] = [ + 'view all revisions', + [ + ['access content', TRUE], + ['bypass node access', TRUE], + ], + FALSE, + ]; + $data['bypass, view revision'] = [ + 'view revision', + [ + ['access content', TRUE], + ['bypass node access', TRUE], + ], + FALSE, + ]; + $data['bypass, revert'] = [ + 'revert revision', + [ + ['access content', TRUE], + ['bypass node access', TRUE], + ], + FALSE, + ]; + $data['bypass, delete revision'] = [ + 'delete revision', + [ + ['access content', TRUE], + ['bypass node access', TRUE], + ], + FALSE, + ]; + + $data['view all revisions'] = [ + 'view all revisions', + [ + ['access content', TRUE], + ['view all revisions', TRUE], + ], + TRUE, + ]; + $data['view all revisions with view access'] = [ + 'view all revisions', + [ + ['access content', TRUE], + ['view all revisions', TRUE], + // Bypass for 'view' operation. + ['bypass node access', TRUE], + ], + TRUE, + ]; + + $data['view revision, without view access'] = [ + 'view revision', + [ + ['access content', TRUE], + ['view all revisions', TRUE], + ], + FALSE, + ]; + + $data['view revision, with view access'] = [ + 'view revision', + [ + ['access content', TRUE], + ['view all revisions', TRUE], + // Bypass for 'view' operation. + ['bypass node access', TRUE], + ], + TRUE, + ]; + + // Cannot revert if no update access. + $data['revert, without update access, non default'] = [ + 'revert revision', + [ + ['access content', TRUE], + ['revert all revisions', TRUE], + ], + FALSE, + FALSE, + ]; + + // Can revert if has update access. + $data['revert, with update access, non default'] = [ + 'revert revision', + [ + ['access content', TRUE], + ['revert all revisions', TRUE], + // Bypass for 'update' operation. + ['bypass node access', TRUE], + ], + TRUE, + FALSE, + ]; + + // Can never revert default revision. + $data['revert, with update access, default revision'] = [ + 'revert revision', + [ + ['access content', TRUE], + ['revert all revisions', TRUE], + // Bypass for 'update' operation. + ['bypass node access', TRUE], + ], + FALSE, + TRUE, + ]; + + // Cannot delete non default revision if no delete access. + $data['delete revision, without delete access, non default'] = [ + 'delete revision', + [ + ['access content', TRUE], + ['delete all revisions', TRUE], + ], + FALSE, + FALSE, + ]; + + // Can delete non default revision if delete access. + $data['delete revision, with delete access, non default'] = [ + 'delete revision', + [ + ['access content', TRUE], + ['delete all revisions', TRUE], + // Bypass for 'delete' operation. + ['bypass node access', TRUE], + ], + TRUE, + FALSE, + ]; + + return $data; + } + + /** + * Tests NodeAccessControlHandler deprecation. + * + * @group legacy + */ + public function testNodeAccessControlHandlerDeprecation() { + $entity_type = $this->prophesize(EntityTypeInterface::class); + $entity_type->id()->willReturn(mt_rand(1, 128)); + $node_grant_storage = $this->prophesize(NodeGrantDatabaseStorageInterface::class); + $entity_type_manager = $this->prophesize(EntityTypeManagerInterface::class); + $container = $this->prophesize(ContainerInterface::class); + $container->get('entity_type.manager')->willReturn($entity_type_manager->reveal()); + \Drupal::setContainer($container->reveal()); + $this->expectDeprecation('Calling Drupal\node\NodeAccessControlHandler::__construct() without the $entity_type_manager argument is deprecated in drupal:9.3.0 and will be required in drupal:10.0.0. See https://www.drupal.org/node/3214171'); + new NodeAccessControlHandler($entity_type->reveal(), $node_grant_storage->reveal()); + } + +}