Drupal
/
drupal
mirror of https://git.drupalcode.org/project/drupal.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Issue #3395404 by acbramley, larowlan, smustgrave, Berdir, jannakha, xjm: Information disclosure access bypass for revision log fields when the JSON:API module is enabled

merge-requests/5548/head
Dave Long 2023-11-25 11:27:22 +00:00
parent 8d47e6f036
commit 184f22eef0
No known key found for this signature in database
GPG Key ID: ED52AE211E142771
15 changed files with 212 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
core/lib/Drupal/Core/Entity/EntityAccessControlHandler.php
View File

@ -389,6 +389,17 @@ class EntityAccessControlHandler extends EntityHandlerBase implements EntityAcce
* The access result.
*/
protected function checkFieldAccess($operation, FieldDefinitionInterface $field_definition, AccountInterface $account, FieldItemListInterface $items = NULL) {
if (!$items instanceof FieldItemListInterface || $operation !== 'view') {
return AccessResult::allowed();
}
$entity = $items->getEntity();
$isRevisionLogField = $this->entityType instanceof ContentEntityTypeInterface && $field_definition->getName() === $this->entityType->getRevisionMetadataKey('revision_log_message');
if ($entity && $isRevisionLogField) {
// The revision log should only be visible to those who can view the
// revisions OR edit the entity.
return $entity->access('view revision', $account, TRUE)
->orIf($entity->access('update', $account, TRUE));
}
return AccessResult::allowed();
}

2
core/modules/block_content/src/BlockContentAccessControlHandler.php
View File

@ -68,7 +68,7 @@ class BlockContentAccessControlHandler extends EntityAccessControlHandler implem
'update' => AccessResult::allowedIfHasPermission($account, 'edit any ' . $bundle . ' block content'),
'delete' => AccessResult::allowedIfHasPermission($account, 'delete any ' . $bundle . ' block content'),
// Revisions.
'view all revisions' => AccessResult::allowedIfHasPermission($account, 'view any ' . $bundle . ' block content history'),
'view revision', 'view all revisions' => AccessResult::allowedIfHasPermission($account, 'view any ' . $bundle . ' block content history'),
'revert' => AccessResult::allowedIfHasPermission($account, 'revert any ' . $bundle . ' block content revisions')
->orIf($forbidIfNotReusable()),
'delete revision' => AccessResult::allowedIfHasPermission($account, 'delete any ' . $bundle . ' block content revisions')

25
core/modules/block_content/tests/src/Kernel/BlockContentAccessHandlerTest.php
View File

@ -11,6 +11,7 @@ use Drupal\Core\Access\AccessResultForbidden;
use Drupal\Core\Access\AccessResultNeutral;
use Drupal\Core\Access\AccessResultReasonInterface;
use Drupal\KernelTests\KernelTestBase;
use Drupal\Tests\user\Traits\UserCreationTrait;
use Drupal\user\Entity\Role;
use Drupal\user\Entity\User;
@ -23,6 +24,8 @@ use Drupal\user\Entity\User;
*/
class BlockContentAccessHandlerTest extends KernelTestBase {
use UserCreationTrait;
/**
* {@inheritdoc}
*/
@ -592,4 +595,26 @@ class BlockContentAccessHandlerTest extends KernelTestBase {
return $cases;
}
/**
* Tests revision log access.
*/
public function testRevisionLogAccess(): void {
$admin = $this->createUser([
'administer block content',
'access content',
]);
$editor = $this->createUser([
'access content',
'access block library',
'view any square block content history',
]);
$viewer = $this->createUser([
'access content',
]);
$this->assertTrue($this->blockEntity->get('revision_log')->access('view', $admin));
$this->assertTrue($this->blockEntity->get('revision_log')->access('view', $editor));
$this->assertFalse($this->blockEntity->get('revision_log')->access('view', $viewer));
}
}

1
core/modules/jsonapi/tests/src/Functional/BlockContentTest.php
View File

@ -162,7 +162,6 @@ class BlockContentTest extends ResourceTestBase {
],
'changed' => (new \DateTime())->setTimestamp($this->entity->getChangedTime())->setTimezone(new \DateTimeZone('UTC'))->format(\DateTime::RFC3339),
'info' => 'Llama',
'revision_log' => NULL,
'revision_created' => (new \DateTime())->setTimestamp($this->entity->getRevisionCreationTime())->setTimezone(new \DateTimeZone('UTC'))->format(\DateTime::RFC3339),
'revision_translation_affected' => TRUE,
'status' => FALSE,

1
core/modules/jsonapi/tests/src/Functional/NodeTest.php
View File

@ -177,7 +177,6 @@ class NodeTest extends ResourceTestBase {
'langcode' => 'en',
],
'promote' => TRUE,
'revision_log' => NULL,
'revision_timestamp' => '1973-11-29T21:33:09+00:00',
// @todo Attempt to remove this in https://www.drupal.org/project/drupal/issues/2933518.
'revision_translation_affected' => TRUE,

50
core/modules/jsonapi/tests/src/Functional/ResourceTestBase.php
View File

@ -14,6 +14,7 @@ use Drupal\Core\Cache\CacheRedirect;
use Drupal\Core\Config\Entity\ConfigEntityInterface;
use Drupal\Core\Entity\ContentEntityInterface;
use Drupal\Core\Entity\ContentEntityNullStorage;
use Drupal\Core\Entity\ContentEntityTypeInterface;
use Drupal\Core\Entity\EntityInterface;
use Drupal\Core\Entity\EntityPublishedInterface;
use Drupal\Core\Entity\FieldableEntityInterface;
@ -29,13 +30,13 @@ use Drupal\Core\Url;
use Drupal\field\Entity\FieldConfig;
use Drupal\field\Entity\FieldStorageConfig;
use Drupal\jsonapi\CacheableResourceResponse;
use Drupal\jsonapi\JsonApiResource\JsonApiDocumentTopLevel;
use Drupal\jsonapi\JsonApiResource\Link;
use Drupal\jsonapi\JsonApiResource\LinkCollection;
use Drupal\jsonapi\JsonApiResource\NullIncludedData;
use Drupal\jsonapi\JsonApiResource\Link;
use Drupal\jsonapi\JsonApiResource\ResourceObject;
use Drupal\jsonapi\JsonApiResource\ResourceObjectData;
use Drupal\jsonapi\Normalizer\HttpExceptionNormalizer;
use Drupal\jsonapi\JsonApiResource\JsonApiDocumentTopLevel;
use Drupal\jsonapi\ResourceResponse;
use Drupal\path\Plugin\Field\FieldType\PathItem;
use Drupal\Tests\BrowserTestBase;
@ -2878,7 +2879,8 @@ abstract class ResourceTestBase extends BrowserTestBase {
// the default revision. This is always the latest revision when
// content_moderation is not installed.
$actual_response = $this->request('GET', $url, $request_options);
$expected_document = $this->getExpectedDocument();
$expected_document = $this->alterExpectedDocumentForRevision($this->getExpectedDocument());
// The resource object should always links to the specific revision it
// represents.
$expected_document['data']['links']['self']['href'] = $latest_revision_id_url->setAbsolute()->toString();
@ -2941,6 +2943,8 @@ abstract class ResourceTestBase extends BrowserTestBase {
$workflow->getTypePlugin()->addEntityTypeAndBundle(static::$entityTypeId, $this->entity->bundle());
$workflow->save();
$this->grantPermissionsToTestedRole(['use editorial transition publish']);
// Ensure the test entity has content_moderation fields attached to it.
/** @var \Drupal\Core\Entity\FieldableEntityInterface|\Drupal\Core\Entity\TranslatableRevisionableInterface $entity */
$entity = $this->entityStorage->load($entity->id());
@ -2971,6 +2975,8 @@ abstract class ResourceTestBase extends BrowserTestBase {
// should be no links.
unset($expected_document['data']['links']['latest-version']);
unset($expected_document['data']['links']['working-copy']);
$expected_document = $this->alterExpectedDocumentForRevision($expected_document);
$expected_cache_tags = array_unique([...$expected_cache_tags, ...$workflow->getCacheTags()]);
$this->assertResourceResponse(200, $expected_document, $actual_response, $expected_cache_tags, $expected_cache_contexts, FALSE, 'MISS');
// Fetch the collection URL using the `latest-version` version argument.
$actual_response = $this->request('GET', $rel_latest_version_collection_url, $request_options);
@ -3095,6 +3101,7 @@ 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();
$expected_cache_tags = array_unique([...$expected_cache_tags, ...$workflow->getCacheTags()]);
$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);
@ -3498,4 +3505,41 @@ abstract class ResourceTestBase extends BrowserTestBase {
return $this->entityStorage->loadUnchanged($id);
}
/**
* Alters the expected JSON:API document for revisions.
*
* Default revision tests assume a non-privileged user is performing the GET
* request and as such the expected document may not include the revision log
* or other fields that require elevated permissions. This method is an
* extension point where child classes can modify the expected document to
* take into account these changes.
*
* @param array $expected_document
* Expected document for the default revision.
*
* @return array[]
* Modified document for a revision or user with access to edit a revision
* AND/OR view revision information.
*/
protected function alterExpectedDocumentForRevision(array $expected_document): array {
$entity_type = $this->entity->getEntityType();
if ($entity_type instanceof ContentEntityTypeInterface &&
($field_name = $entity_type->getRevisionMetadataKey('revision_log_message'))) {
// The default entity access control handler assumes that permissions do not
// change during the lifetime of a request and caches access results.
// However, we're changing permissions during a test run and need fresh
// results, so reset the cache.
\Drupal::entityTypeManager()->getAccessControlHandler($this->entity->getEntityTypeId())->resetCache();
$revisionLogAccess = $this->entity->access('view revision', $this->account, TRUE)
->orIf($this->entity->access('update', $this->account, TRUE));
if ($revisionLogAccess->isAllowed()) {
$expected_document['data']['attributes'][$field_name] = NULL;
return $expected_document;
}
unset($expected_document['data']['attributes'][$field_name]);
}
return $expected_document;
}
}

1
core/modules/jsonapi/tests/src/Functional/TermTest.php
View File

@ -281,7 +281,6 @@ class TermTest extends ResourceTestBase {
'status' => TRUE,
'drupal_internal__revision_id' => 1,
'revision_created' => (new \DateTime())->setTimestamp($this->entity->getRevisionCreationTime())->setTimezone(new \DateTimeZone('UTC'))->format(\DateTime::RFC3339),
'revision_log_message' => NULL,
// @todo Attempt to remove this in https://www.drupal.org/project/drupal/issues/2933518.
'revision_translation_affected' => TRUE,
],

4
core/modules/jsonapi/tests/src/Kernel/Normalizer/JsonApiDocumentTopLevelNormalizerTest.php
View File

@ -342,7 +342,7 @@ class JsonApiDocumentTopLevelNormalizerTest extends JsonapiKernelTestBase {
$this->assertSame($this->term1->uuid(), $normalized['included'][1]['id']);
$this->assertSame('taxonomy_term--tags', $normalized['included'][1]['type']);
$this->assertSame($this->term1->label(), $normalized['included'][1]['attributes']['name']);
$this->assertCount(12, $normalized['included'][1]['attributes']);
$this->assertCount(11, $normalized['included'][1]['attributes']);
$this->assertTrue(!isset($normalized['included'][1]['attributes']['created']));
// Make sure that the cache tags for the includes and the requested entities
// are bubbling as expected.
@ -388,7 +388,7 @@ class JsonApiDocumentTopLevelNormalizerTest extends JsonapiKernelTestBase {
$this->assertArrayNotHasKey('meta', $normalized);
$this->assertEquals($this->user->uuid(), $normalized['included'][0]['id']);
$this->assertCount(1, $normalized['included'][0]['attributes']);
$this->assertCount(12, $normalized['included'][1]['attributes']);
$this->assertCount(11, $normalized['included'][1]['attributes']);
// Make sure that the cache tags for the includes and the requested entities
// are bubbling as expected.
$this->assertEqualsCanonicalizing(

36
core/modules/media/tests/src/Kernel/MediaAccessControlHandlerTest.php
View File

@ -742,4 +742,40 @@ class MediaAccessControlHandlerTest extends MediaKernelTestBase {
return $test_data;
}
/**
* Tests access to the revision log field.
*/
public function testRevisionLogFieldAccess(): void {
$admin = $this->createUser([
'administer media',
'view media',
]);
$editor = $this->createUser([
'view all media revisions',
'view media',
]);
$viewer = $this->createUser([
'view media',
]);
$media_type = $this->createMediaType('test', [
'id' => 'test',
]);
$entity = Media::create([
'status' => TRUE,
'bundle' => $media_type->id(),
]);
$entity->save();
$this->assertTrue($entity->get('revision_log_message')->access('view', $admin));
$this->assertTrue($entity->get('revision_log_message')->access('view', $editor));
// revision_log_message field access can be granted with the "view revision"
// operation. "view revision" access is granted if the user is allowed to
// view the default revision of the media entity.
$this->assertTrue($entity->get('revision_log_message')->access('view', $viewer));
$entity->setUnpublished()->save();
\Drupal::entityTypeManager()->getAccessControlHandler('media')->resetCache();
$this->assertFalse($entity->get('revision_log_message')->access('view', $viewer));
}
}

17
core/modules/node/tests/src/Functional/Rest/NodeJsonBasicAuthTest.php
View File

@ -36,4 +36,21 @@ class NodeJsonBasicAuthTest extends NodeResourceTestBase {
*/
protected static $auth = 'basic_auth';
/**
* {@inheritdoc}
*/
protected function setUpAuthorization($method) {
parent::setUpAuthorization($method);
$this->grantPermissionsToTestedRole(['view camelids revisions']);
}
/**
* {@inheritdoc}
*/
protected function getExpectedNormalizedEntity() {
$entity = parent::getExpectedNormalizedEntity();
$entity['revision_log'] = [];
return $entity;
}
}

1
core/modules/node/tests/src/Functional/Rest/NodeResourceTestBase.php
View File

@ -182,7 +182,6 @@ abstract class NodeResourceTestBase extends EntityResourceTestBase {
'url' => base_path() . 'user/' . $author->id(),
],
],
'revision_log' => [],
'path' => [
[
'alias' => '/llama',

41
core/modules/node/tests/src/Kernel/NodeFieldAccessTest.php
View File

@ -64,11 +64,21 @@ class NodeFieldAccessTest extends EntityKernelTestBase {
// An administrator user. No user exists yet, ensure that the first user
// does not have UID 1.
$content_admin_user = $this->createUser(['administer nodes'], NULL, FALSE, ['uid' => 2]);
$content_admin_user = $this->createUser(['administer nodes', 'access content'], values: ['uid' => 2]);
// Two different editor users.
$page_creator_user = $this->createUser(['create page content', 'edit own page content', 'delete own page content']);
$page_manager_user = $this->createUser(['create page content', 'edit any page content', 'delete any page content']);
$page_creator_user = $this->createUser([
'create page content',
'edit own page content',
'delete own page content',
'access content',
]);
$page_manager_user = $this->createUser([
'create page content',
'edit any page content',
'delete any page content',
'access content',
]);
// An unprivileged user.
$page_unrelated_user = $this->createUser(['access content']);
@ -88,15 +98,21 @@ class NodeFieldAccessTest extends EntityKernelTestBase {
'uid' => $page_creator_user->id(),
'type' => 'page',
]);
$node1->save();
$node2 = Node::create([
'title' => $this->randomMachineName(8),
'uid' => $page_manager_user->id(),
'type' => 'article',
'revision_log' => 'Updated to requirements',
]);
$node2->save();
$node3 = Node::create([
'title' => $this->randomMachineName(8),
'type' => 'page',
]);
$node3->save();
foreach ($this->administrativeFields as $field) {
@ -144,8 +160,27 @@ class NodeFieldAccessTest extends EntityKernelTestBase {
// Check the revision_log field on node 2 which has revisions enabled.
$may_update = $node2->revision_log->access('edit', $content_admin_user);
$this->assertTrue($may_update, 'A user with permission "administer nodes" can edit the revision_log field when revisions are enabled.');
$may_update = $node2->revision_log->access('edit', $page_creator_user);
$this->assertTrue($may_update, 'A user without permission "administer nodes" can edit the revision_log field when revisions are enabled.');
$may_view = $node2->revision_log->access('view', $content_admin_user);
$this->assertTrue($may_view, 'A user without permission "administer nodes" cannot view the revision_log field when revisions are enabled.');
// Page manager only has permissions to 'page', not 'article' content type.
$may_view = $node2->revision_log->access('view', $page_manager_user);
$this->assertFalse($may_view, 'A user without permission to the content type cannot view the revision_log field when revisions are enabled.');
$article_revision_manager_user = $this->createUser(['access content', 'view article revisions']);
$may_view = $node2->revision_log->access('view', $article_revision_manager_user);
$this->assertTrue($may_view, 'A user without permission "view article revisions" cannot view the revision_log field when revisions are enabled on article.');
$revision_manager_user = $this->createUser(['access content', 'view all revisions']);
$may_view = $node2->revision_log->access('view', $revision_manager_user);
$this->assertTrue($may_view, 'A user without permission "view all revisions" cannot view the revision_log field when revisions are enabled.');
$may_view = $node2->revision_log->access('view', $page_unrelated_user);
$this->assertFalse($may_view, 'A user with only permission "access content" cannot view the revision_log field when revisions are enabled.');
}
}

8
core/modules/rest/tests/src/Functional/EntityResource/ModeratedNode/ModeratedNodeResourceTestBase.php
View File

@ -2,6 +2,7 @@
namespace Drupal\Tests\rest\Functional\EntityResource\ModeratedNode;
use Drupal\Core\Cache\Cache;
use Drupal\Tests\content_moderation\Traits\ContentModerationTestTrait;
use Drupal\Tests\node\Functional\Rest\NodeResourceTestBase;
@ -71,4 +72,11 @@ abstract class ModeratedNodeResourceTestBase extends NodeResourceTestBase {
]);
}
/**
* {@inheritdoc}
*/
protected function getExpectedCacheTags() {
return Cache::mergeTags(parent::getExpectedCacheTags(), ['config:workflows.workflow.editorial']);
}
}

1
core/modules/taxonomy/tests/src/Functional/Rest/TermResourceTestBase.php
View File

@ -220,7 +220,6 @@ abstract class TermResourceTestBase extends EntityResourceTestBase {
],
],
'revision_user' => [],
'revision_log_message' => [],
'revision_translation_affected' => [
[
'value' => TRUE,

28
core/modules/taxonomy/tests/src/Kernel/TermKernelTest.php
View File

@ -5,6 +5,7 @@ namespace Drupal\Tests\taxonomy\Kernel;
use Drupal\taxonomy\Entity\Term;
use Drupal\KernelTests\KernelTestBase;
use Drupal\Tests\taxonomy\Traits\TaxonomyTestTrait;
use Drupal\Tests\user\Traits\UserCreationTrait;
/**
* Kernel tests for taxonomy term functions.
@ -14,11 +15,12 @@ use Drupal\Tests\taxonomy\Traits\TaxonomyTestTrait;
class TermKernelTest extends KernelTestBase {
use TaxonomyTestTrait;
use UserCreationTrait;
/**
* {@inheritdoc}
*/
protected static $modules = ['filter', 'taxonomy', 'text', 'user'];
protected static $modules = ['filter', 'taxonomy', 'text', 'user', 'system'];
/**
* {@inheritdoc}
@ -27,6 +29,7 @@ class TermKernelTest extends KernelTestBase {
parent::setUp();
$this->installConfig(['filter']);
$this->installEntitySchema('taxonomy_term');
$this->installEntitySchema('user');
}
/**
@ -196,4 +199,27 @@ class TermKernelTest extends KernelTestBase {
$storage->updateTermHierarchy($term);
}
/**
* Tests revision log access.
*/
public function testRevisionLogAccess(): void {
$vocabulary = $this->createVocabulary();
$entity = $this->createTerm($vocabulary, ['status' => TRUE]);
$admin = $this->createUser([
'administer taxonomy',
'access content',
]);
$editor = $this->createUser([
'edit terms in ' . $vocabulary->id(),
'access content',
]);
$viewer = $this->createUser([
'access content',
]);
$this->assertTrue($entity->get('revision_log_message')->access('view', $admin));
$this->assertTrue($entity->get('revision_log_message')->access('view', $editor));
$this->assertFalse($entity->get('revision_log_message')->access('view', $viewer));
}
}