From 8bedf88b63df0c08fa3fd589cdbac6ea03205b2d Mon Sep 17 00:00:00 2001 From: Alex Pott <alex.a.pott@googlemail.com> Date: Mon, 8 May 2017 11:09:57 +0100 Subject: [PATCH] Issue #2779931 by Sam152, alexpott, timmillwood, Wim Leers, catch, dawehner: Add storage exception that enforces unique content_entity_type_id and content_entity_id on the content moderation state content entity, and add access control handler to forbid all access --- ...entModerationStateAccessControlHandler.php | 37 +++++ .../ContentModerationStateStorageSchema.php | 19 ++- .../src/Entity/ContentModerationState.php | 6 +- .../src/EntityOperations.php | 5 +- ...oderationStateAccessControlHandlerTest.php | 54 +++++++ ...ontentModerationStateStorageSchemaTest.php | 142 ++++++++++++++++++ 6 files changed, 252 insertions(+), 11 deletions(-) create mode 100644 core/modules/content_moderation/src/ContentModerationStateAccessControlHandler.php create mode 100644 core/modules/content_moderation/tests/src/Kernel/ContentModerationStateAccessControlHandlerTest.php create mode 100644 core/modules/content_moderation/tests/src/Kernel/ContentModerationStateStorageSchemaTest.php diff --git a/core/modules/content_moderation/src/ContentModerationStateAccessControlHandler.php b/core/modules/content_moderation/src/ContentModerationStateAccessControlHandler.php new file mode 100644 index 000000000000..6dd5c903652a --- /dev/null +++ b/core/modules/content_moderation/src/ContentModerationStateAccessControlHandler.php @@ -0,0 +1,37 @@ +<?php + +namespace Drupal\content_moderation; + +use Drupal\Core\Access\AccessResult; +use Drupal\Core\Session\AccountInterface; +use Drupal\Core\Entity\EntityAccessControlHandler; +use Drupal\Core\Entity\EntityInterface; + +/** + * The access control handler for the content_moderation_state entity type. + * + * @see \Drupal\content_moderation\Entity\ContentModerationState + */ +class ContentModerationStateAccessControlHandler extends EntityAccessControlHandler { + + /** + * {@inheritdoc} + */ + public function checkAccess(EntityInterface $entity, $operation, AccountInterface $account) { + // ContentModerationState is an internal entity type. Access is denied for + // viewing, updating, and deleting. In order to update an entity's + // moderation state use its moderation_state field. + return AccessResult::forbidden('ContentModerationState is an internal entity type.'); + } + + /** + * {@inheritdoc} + */ + protected function checkCreateAccess(AccountInterface $account, array $context, $entity_bundle = NULL) { + // ContentModerationState is an internal entity type. Access is denied for + // creating. In order to update an entity's moderation state use its + // moderation_state field. + return AccessResult::forbidden('ContentModerationState is an internal entity type.'); + } + +} diff --git a/core/modules/content_moderation/src/ContentModerationStateStorageSchema.php b/core/modules/content_moderation/src/ContentModerationStateStorageSchema.php index cb87cc19ab80..869a10bfac94 100644 --- a/core/modules/content_moderation/src/ContentModerationStateStorageSchema.php +++ b/core/modules/content_moderation/src/ContentModerationStateStorageSchema.php @@ -16,11 +16,20 @@ class ContentModerationStateStorageSchema extends SqlContentEntityStorageSchema protected function getEntitySchema(ContentEntityTypeInterface $entity_type, $reset = FALSE) { $schema = parent::getEntitySchema($entity_type, $reset); - // Creates an index to ensure that the lookup in - // \Drupal\content_moderation\Plugin\Field\ModerationStateFieldItemList::getModerationState() - // is performant. - $schema['content_moderation_state_field_data']['indexes'] += [ - 'content_moderation_state__lookup' => ['content_entity_type_id', 'content_entity_id', 'content_entity_revision_id'], + // Creates unique keys to guarantee the integrity of the entity and to make + // the lookup in ModerationStateFieldItemList::getModerationState() fast. + $unique_keys = [ + 'content_entity_type_id', + 'content_entity_id', + 'content_entity_revision_id', + 'workflow', + 'langcode', + ]; + $schema['content_moderation_state_field_data']['unique keys'] += [ + 'content_moderation_state__lookup' => $unique_keys, + ]; + $schema['content_moderation_state_field_revision']['unique keys'] += [ + 'content_moderation_state__lookup' => $unique_keys, ]; return $schema; diff --git a/core/modules/content_moderation/src/Entity/ContentModerationState.php b/core/modules/content_moderation/src/Entity/ContentModerationState.php index cb1bc1fe9fd5..5b1c250679c0 100644 --- a/core/modules/content_moderation/src/Entity/ContentModerationState.php +++ b/core/modules/content_moderation/src/Entity/ContentModerationState.php @@ -24,6 +24,7 @@ * handlers = { * "storage_schema" = "Drupal\content_moderation\ContentModerationStateStorageSchema", * "views_data" = "\Drupal\views\EntityViewsData", + * "access" = "Drupal\content_moderation\ContentModerationStateAccessControlHandler", * }, * base_table = "content_moderation_state", * revision_table = "content_moderation_state_revision", @@ -74,6 +75,7 @@ public static function baseFieldDefinitions(EntityTypeInterface $entity_type) { ->setLabel(t('Content entity type ID')) ->setDescription(t('The ID of the content entity type this moderation state is for.')) ->setRequired(TRUE) + ->setSetting('max_length', EntityTypeInterface::ID_MAX_LENGTH) ->setRevisionable(TRUE); $fields['content_entity_id'] = BaseFieldDefinition::create('integer') @@ -82,10 +84,6 @@ public static function baseFieldDefinitions(EntityTypeInterface $entity_type) { ->setRequired(TRUE) ->setRevisionable(TRUE); - // @todo https://www.drupal.org/node/2779931 Add constraint that enforces - // unique content_entity_type_id, content_entity_id and - // content_entity_revision_id. - $fields['content_entity_revision_id'] = BaseFieldDefinition::create('integer') ->setLabel(t('Content entity revision ID')) ->setDescription(t('The revision ID of the content entity this moderation state is for.')) diff --git a/core/modules/content_moderation/src/EntityOperations.php b/core/modules/content_moderation/src/EntityOperations.php index d33f9ed30ae6..3e34d141ec93 100644 --- a/core/modules/content_moderation/src/EntityOperations.php +++ b/core/modules/content_moderation/src/EntityOperations.php @@ -182,8 +182,9 @@ protected function updateOrCreateFromEntity(EntityInterface $entity) { ]); $content_moderation_state->workflow->target_id = $workflow->id(); } - else { - // Create a new revision. + elseif ($content_moderation_state->content_entity_revision_id->value != $entity_revision_id) { + // If a new revision of the content has been created, add a new content + // moderation state revision. $content_moderation_state->setNewRevision(TRUE); } diff --git a/core/modules/content_moderation/tests/src/Kernel/ContentModerationStateAccessControlHandlerTest.php b/core/modules/content_moderation/tests/src/Kernel/ContentModerationStateAccessControlHandlerTest.php new file mode 100644 index 000000000000..4d10672adaa8 --- /dev/null +++ b/core/modules/content_moderation/tests/src/Kernel/ContentModerationStateAccessControlHandlerTest.php @@ -0,0 +1,54 @@ +<?php + +namespace Drupal\Tests\content_moderation\Kernel; + +use Drupal\content_moderation\Entity\ContentModerationState; +use Drupal\KernelTests\KernelTestBase; + +/** + * @coversDefaultClass \Drupal\content_moderation\ContentModerationStateAccessControlHandler + * @group content_moderation + */ +class ContentModerationStateAccessControlHandlerTest extends KernelTestBase { + + /** + * Modules to enable. + * + * @var array + */ + public static $modules = [ + 'content_moderation', + 'workflows', + 'user', + ]; + + /** + * The content_moderation_state access control handler. + * + * @var \Drupal\Core\Entity\EntityAccessControlHandlerInterface + */ + protected $accessControlHandler; + + /** + * {@inheritdoc} + */ + protected function setUp() { + parent::setUp(); + $this->installEntitySchema('content_moderation_state'); + $this->installEntitySchema('user'); + $this->accessControlHandler = $this->container->get('entity_type.manager')->getAccessControlHandler('content_moderation_state'); + } + + /** + * @covers ::checkAccess + * @covers ::checkCreateAccess + */ + public function testHandler() { + $entity = ContentModerationState::create([]); + $this->assertFalse($this->accessControlHandler->access($entity, 'view')); + $this->assertFalse($this->accessControlHandler->access($entity, 'update')); + $this->assertFalse($this->accessControlHandler->access($entity, 'delete')); + $this->assertFalse($this->accessControlHandler->createAccess()); + } + +} diff --git a/core/modules/content_moderation/tests/src/Kernel/ContentModerationStateStorageSchemaTest.php b/core/modules/content_moderation/tests/src/Kernel/ContentModerationStateStorageSchemaTest.php new file mode 100644 index 000000000000..af59651410f9 --- /dev/null +++ b/core/modules/content_moderation/tests/src/Kernel/ContentModerationStateStorageSchemaTest.php @@ -0,0 +1,142 @@ +<?php + +namespace Drupal\Tests\content_moderation\Kernel; + +use Drupal\content_moderation\Entity\ContentModerationState; +use Drupal\KernelTests\KernelTestBase; +use Drupal\node\Entity\Node; +use Drupal\node\Entity\NodeType; +use Drupal\workflows\Entity\Workflow; + +/** + * Test the ContentModerationState storage schema. + * + * @coversDefaultClass \Drupal\content_moderation\ContentModerationStateStorageSchema + * @group content_moderation + */ +class ContentModerationStateStorageSchemaTest extends KernelTestBase { + + /** + * {@inheritdoc} + */ + public static $modules = [ + 'node', + 'content_moderation', + 'user', + 'system', + 'text', + 'workflows', + 'entity_test', + ]; + + /** + * {@inheritdoc} + */ + protected function setUp() { + parent::setUp(); + + $this->installSchema('node', 'node_access'); + $this->installEntitySchema('node'); + $this->installEntitySchema('entity_test'); + $this->installEntitySchema('user'); + $this->installEntitySchema('content_moderation_state'); + $this->installConfig('content_moderation'); + + NodeType::create([ + 'type' => 'example', + ])->save(); + $workflow = Workflow::load('editorial'); + $workflow->getTypePlugin()->addEntityTypeAndBundle('node', 'example'); + $workflow->save(); + } + + /** + * Test the ContentModerationState unique keys. + * + * @covers ::getEntitySchema + */ + public function testUniqueKeys() { + // Create a node which will create a new ContentModerationState entity. + $node = Node::create([ + 'title' => 'Test title', + 'type' => 'example', + 'moderation_state' => 'draft', + ]); + $node->save(); + + // Ensure an exception when all values match. + $this->assertStorageException([ + 'content_entity_type_id' => $node->getEntityTypeId(), + 'content_entity_id' => $node->id(), + 'content_entity_revision_id' => $node->getRevisionId(), + ], TRUE); + + // No exception for the same values, with a different langcode. + $this->assertStorageException([ + 'content_entity_type_id' => $node->getEntityTypeId(), + 'content_entity_id' => $node->id(), + 'content_entity_revision_id' => $node->getRevisionId(), + 'langcode' => 'de', + ], FALSE); + + // A different workflow should not trigger an exception. + $this->assertStorageException([ + 'content_entity_type_id' => $node->getEntityTypeId(), + 'content_entity_id' => $node->id(), + 'content_entity_revision_id' => $node->getRevisionId(), + 'workflow' => 'foo', + ], FALSE); + + // Different entity types should not trigger an exception. + $this->assertStorageException([ + 'content_entity_type_id' => 'entity_test', + 'content_entity_id' => $node->id(), + 'content_entity_revision_id' => $node->getRevisionId(), + ], FALSE); + + // Different entity and revision IDs should not trigger an exception. + $this->assertStorageException([ + 'content_entity_type_id' => $node->getEntityTypeId(), + 'content_entity_id' => 9999, + 'content_entity_revision_id' => 9999, + ], FALSE); + + // Creating a version of the entity with a previously used, but not current + // revision ID should trigger an exception. + $old_revision_id = $node->getRevisionId(); + $node->setNewRevision(TRUE); + $node->title = 'Updated title'; + $node->moderation_state = 'published'; + $node->save(); + $this->assertStorageException([ + 'content_entity_type_id' => $node->getEntityTypeId(), + 'content_entity_id' => $node->id(), + 'content_entity_revision_id' => $old_revision_id, + ], TRUE); + } + + /** + * Assert if a storage exception is triggered when saving a given entity. + * + * @param array $values + * An array of entity values. + * @param bool $has_exception + * If an exception should be triggered when saving the entity. + */ + protected function assertStorageException(array $values, $has_exception) { + $defaults = [ + 'moderation_state' => 'draft', + 'workflow' => 'editorial', + ]; + $entity = ContentModerationState::create($values + $defaults); + $exception_triggered = FALSE; + try { + ContentModerationState::updateOrCreateFromEntity($entity); + } + catch (\Exception $e) { + $exception_triggered = TRUE; + } + $this->assertEquals($has_exception, $exception_triggered); + } + +} -- GitLab