diff --git a/core/lib/Drupal/Core/Entity/ContentEntityBase.php b/core/lib/Drupal/Core/Entity/ContentEntityBase.php index 6094bdf..8cd767c 100644 --- a/core/lib/Drupal/Core/Entity/ContentEntityBase.php +++ b/core/lib/Drupal/Core/Entity/ContentEntityBase.php @@ -1108,7 +1108,9 @@ public function createDuplicate() { $duplicate = clone $this; $entity_type = $this->getEntityType(); - $duplicate->{$entity_type->getKey('id')}->value = NULL; + if ($entity_type->hasKey('id')) { + $duplicate->{$entity_type->getKey('id')}->value = NULL; + } $duplicate->enforceIsNew(); // Check if the entity type supports UUIDs and generate a new one if so. diff --git a/core/lib/Drupal/Core/Entity/EntityAccessControlHandler.php b/core/lib/Drupal/Core/Entity/EntityAccessControlHandler.php index c67c2ed..ff8da23 100644 --- a/core/lib/Drupal/Core/Entity/EntityAccessControlHandler.php +++ b/core/lib/Drupal/Core/Entity/EntityAccessControlHandler.php @@ -316,13 +316,17 @@ public function fieldAccess($operation, FieldDefinitionInterface $field_definiti $default = $items ? $items->defaultAccess($operation, $account) : AccessResult::allowed(); // Explicitly disallow changing the entity ID and entity UUID. - if ($operation === 'edit') { + $entity = $items ? $items->getEntity() : NULL; + if ($operation === 'edit' && $entity) { if ($field_definition->getName() === $this->entityType->getKey('id')) { - return $return_as_object ? AccessResult::forbidden('The entity ID cannot be changed') : FALSE; + // String IDs can be set when creating the entity. + if (!($entity->isNew() && $field_definition->getType() === 'string')) { + return $return_as_object ? AccessResult::forbidden('The entity ID cannot be changed')->addCacheableDependency($entity) : FALSE; + } } elseif ($field_definition->getName() === $this->entityType->getKey('uuid')) { // UUIDs can be set when creating an entity. - if ($items && ($entity = $items->getEntity()) && !$entity->isNew()) { + if (!$entity->isNew()) { return $return_as_object ? AccessResult::forbidden('The entity UUID cannot be changed')->addCacheableDependency($entity) : FALSE; } } diff --git a/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php index 4c7947d..6039d2c 100644 --- a/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php @@ -87,6 +87,14 @@ protected static $patchProtectedFieldNames; /** + * The fields that need a different (random) value for each new entity created + * by a POST request. + * + * @var string[] + */ + protected static $uniqueFieldNames = []; + + /** * Optionally specify which field is the 'label' field. Some entities specify * a 'label_callback', but not a 'label' entity key. For example: User. * @@ -126,6 +134,13 @@ protected $entity; /** + * Another entity of the same type used for testing. + * + * @var \Drupal\Core\Entity\EntityInterface + */ + protected $anotherEntity; + + /** * The entity storage. * * @var \Drupal\Core\Entity\EntityStorageInterface @@ -222,6 +237,22 @@ public function setUp() { abstract protected function createEntity(); /** + * Creates another entity to be tested. + * + * @return \Drupal\Core\Entity\EntityInterface + * Another entity based on $this->entity. + */ + protected function createAnotherEntity() { + $entity = $this->entity->createDuplicate(); + $label_key = $entity->getEntityType()->getKey('label'); + if ($label_key) { + $entity->set($label_key, $entity->label() . '_dupe'); + } + $entity->save(); + return $entity; + } + + /** * Returns the expected normalization of the entity. * * @see ::createEntity() @@ -254,6 +285,46 @@ protected function getNormalizedPatchEntity() { } /** + * Gets the second normalized POST entity. + * + * Entity types can have non-sequential IDs, and in that case the second + * entity created for POST testing needs to be able to specify a different ID. + * + * @see ::testPost + * @see ::getNormalizedPostEntity + * + * @return array + * An array structure as returned by ::getNormalizedPostEntity(). + */ + protected function getSecondNormalizedPostEntity() { + // Return the values of the "parent" method by default. + return $this->getNormalizedPostEntity(); + } + + /** + * Gets the normalized POST entity with random values for its unique fields. + * + * @see ::testPost + * @see ::getNormalizedPostEntity + * + * @return array + * An array structure as returned by ::getNormalizedPostEntity(). + */ + protected function getModifiedEntityForPostTesting() { + $normalized_entity = $this->getNormalizedPostEntity(); + + // Ensure that all the unique fields of the entity type get a new random + // value. + foreach (static::$uniqueFieldNames as $field_name) { + $field_definition = $this->entity->getFieldDefinition($field_name); + $field_type_class = $field_definition->getItemDefinition()->getClass(); + $normalized_entity[$field_name] = $field_type_class::generateSampleValue($field_definition); + } + + return $normalized_entity; + } + + /** * {@inheritdoc} */ protected function getExpectedUnauthorizedAccessMessage($method) { @@ -712,7 +783,7 @@ public function testPost() { $unparseable_request_body = '!{>}<'; $parseable_valid_request_body = $this->serializer->encode($this->getNormalizedPostEntity(), static::$format); $parseable_valid_request_body_2 = $this->serializer->encode($this->getNormalizedPostEntity(), static::$format); - $parseable_invalid_request_body = $this->serializer->encode($this->makeNormalizationInvalid($this->getNormalizedPostEntity()), static::$format); + $parseable_invalid_request_body = $this->serializer->encode($this->makeNormalizationInvalid($this->getNormalizedPostEntity(), 'label'), static::$format); $parseable_invalid_request_body_2 = $this->serializer->encode($this->getNormalizedPostEntity() + ['uuid' => [$this->randomMachineName(129)]], static::$format); $parseable_invalid_request_body_3 = $this->serializer->encode($this->getNormalizedPostEntity() + ['field_rest_test' => [['value' => $this->randomString()]]], static::$format); @@ -871,8 +942,9 @@ public function testPost() { } $response = $this->request('POST', $url, $request_options); $this->assertResourceResponse(201, FALSE, $response); + $created_entity = $this->entityStorage->load(static::$secondCreatedEntityId); if ($has_canonical_url) { - $location = $this->entityStorage->load(static::$secondCreatedEntityId)->toUrl('canonical')->setAbsolute(TRUE)->toString(); + $location = $created_entity->toUrl('canonical')->setAbsolute(TRUE)->toString(); $this->assertSame([$location], $response->getHeader('Location')); } else { @@ -880,6 +952,32 @@ public function testPost() { } $this->assertFalse($response->hasHeader('X-Drupal-Cache')); + if ($this->entity->getEntityType()->getStorageClass() !== ContentEntityNullStorage::class && $this->entity->getEntityType()->hasKey('uuid')) { + // 500 when creating an entity with a duplicate UUID. + $normalized_entity = $this->getModifiedEntityForPostTesting(); + $normalized_entity[$created_entity->getEntityType()->getKey('uuid')] = [['value' => $created_entity->uuid()]]; + $normalized_entity[$label_field] = [['value' => $this->randomMachineName()]]; + $request_options[RequestOptions::BODY] = $this->serializer->encode($normalized_entity, static::$format); + + $response = $this->request('POST', $url, $request_options); + $this->assertSame(500, $response->getStatusCode()); + $this->assertContains('Internal Server Error', (string) $response->getBody()); + + // 201 when successfully creating an entity with a new UUID. + $normalized_entity = $this->getModifiedEntityForPostTesting(); + $new_uuid = \Drupal::service('uuid')->generate(); + $normalized_entity[$created_entity->getEntityType()->getKey('uuid')] = [['value' => $new_uuid]]; + $normalized_entity[$label_field] = [['value' => $this->randomMachineName()]]; + $request_options[RequestOptions::BODY] = $this->serializer->encode($normalized_entity, static::$format); + + $response = $this->request('POST', $url, $request_options); + $this->assertResourceResponse(201, FALSE, $response); + $entities = $this->entityStorage->loadByProperties([$created_entity->getEntityType()->getKey('uuid') => $new_uuid]); + $new_entity = reset($entities); + $this->assertNotNull($new_entity); + $new_entity->delete(); + } + // BC: old default POST URLs have their path updated by the inbound path // processor \Drupal\rest\PathProcessor\PathProcessorEntityResourceBC to the // new URL, which is derived from the 'create' link template if an entity @@ -902,6 +1000,9 @@ public function testPatch() { return; } + // Patch testing requires that another entity of the same type exists. + $this->anotherEntity = $this->createAnotherEntity(); + $this->initAuthentication(); $has_canonical_url = $this->entity->hasLinkTemplate('canonical'); @@ -909,7 +1010,7 @@ public function testPatch() { $unparseable_request_body = '!{>}<'; $parseable_valid_request_body = $this->serializer->encode($this->getNormalizedPatchEntity(), static::$format); $parseable_valid_request_body_2 = $this->serializer->encode($this->getNormalizedPatchEntity(), static::$format); - $parseable_invalid_request_body = $this->serializer->encode($this->makeNormalizationInvalid($this->getNormalizedPatchEntity()), static::$format); + $parseable_invalid_request_body = $this->serializer->encode($this->makeNormalizationInvalid($this->getNormalizedPatchEntity(), 'label'), static::$format); $parseable_invalid_request_body_2 = $this->serializer->encode($this->getNormalizedPatchEntity() + ['field_rest_test' => [['value' => $this->randomString()]]], static::$format); // The 'field_rest_test' field does not allow 'view' access, so does not end // up in the normalization. Even when we explicitly add it the normalization @@ -1006,6 +1107,18 @@ public function testPatch() { $response = $this->request('PATCH', $url, $request_options); $this->assertResourceErrorResponse(403, "Access denied on updating field 'field_rest_test'.", $response); + // DX: 403 when entity trying to update an entity's ID field. + $request_options[RequestOptions::BODY] = $this->serializer->encode($this->makeNormalizationInvalid($this->getNormalizedPatchEntity(), 'id'), static::$format);; + $response = $this->request('PATCH', $url, $request_options); + $this->assertResourceErrorResponse(403, "Access denied on updating field '{$this->entity->getEntityType()->getKey('id')}'.", $response); + + if ($this->entity->getEntityType()->hasKey('uuid')) { + // DX: 403 when entity trying to update an entity's UUID field. + $request_options[RequestOptions::BODY] = $this->serializer->encode($this->makeNormalizationInvalid($this->getNormalizedPatchEntity(), 'uuid'), static::$format);; + $response = $this->request('PATCH', $url, $request_options); + $this->assertResourceErrorResponse(403, "Access denied on updating field '{$this->entity->getEntityType()->getKey('uuid')}'.", $response); + } + $request_options[RequestOptions::BODY] = $parseable_invalid_request_body_3; // DX: 403 when entity contains field without 'edit' nor 'view' access, even @@ -1308,15 +1421,27 @@ protected static function getModifiedEntityForPatchTesting(EntityInterface $enti * * @param array $normalization * An entity normalization. + * @param string $entity_key + * The entity key whose normalization to make invalid. * * @return array * The updated entity normalization, now invalid. */ - protected function makeNormalizationInvalid(array $normalization) { - // Add a second label to this entity to make it invalid. - $label_field = $this->entity->getEntityType()->hasKey('label') ? $this->entity->getEntityType()->getKey('label') : static::$labelFieldName; - $normalization[$label_field][1]['value'] = 'Second Title'; - + protected function makeNormalizationInvalid(array $normalization, $entity_key) { + $entity_type = $this->entity->getEntityType(); + switch ($entity_key) { + case 'label': + // Add a second label to this entity to make it invalid. + $label_field = $entity_type->hasKey('label') ? $entity_type->getKey('label') : static::$labelFieldName; + $normalization[$label_field][1]['value'] = 'Second Title'; + break; + case 'id': + $normalization[$entity_type->getKey('id')][0]['value'] = $this->anotherEntity->id(); + break; + case 'uuid': + $normalization[$entity_type->getKey('uuid')][0]['value'] = $this->anotherEntity->uuid(); + break; + } return $normalization; } diff --git a/core/modules/rest/tests/src/Functional/EntityResource/Feed/FeedResourceTestBase.php b/core/modules/rest/tests/src/Functional/EntityResource/Feed/FeedResourceTestBase.php index 1e79395..a89bde1 100644 --- a/core/modules/rest/tests/src/Functional/EntityResource/Feed/FeedResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/EntityResource/Feed/FeedResourceTestBase.php @@ -3,10 +3,10 @@ namespace Drupal\Tests\rest\Functional\EntityResource\Feed; use Drupal\Tests\rest\Functional\BcTimestampNormalizerUnixTestTrait; -use Drupal\Tests\rest\Functional\EntityResource\EntityTest\EntityTestResourceTestBase; +use Drupal\Tests\rest\Functional\EntityResource\EntityResourceTestBase; use Drupal\aggregator\Entity\Feed; -abstract class FeedResourceTestBase extends EntityTestResourceTestBase { +abstract class FeedResourceTestBase extends EntityResourceTestBase { use BcTimestampNormalizerUnixTestTrait; @@ -23,6 +23,16 @@ /** * {@inheritdoc} */ + protected static $patchProtectedFieldNames = []; + + /** + * {@inheritdoc} + */ + protected static $uniqueFieldNames = ['url']; + + /** + * {@inheritdoc} + */ protected function setUpAuthorization($method) { switch ($method) { case 'GET': diff --git a/core/modules/rest/tests/src/Functional/EntityResource/Item/ItemResourceTestBase.php b/core/modules/rest/tests/src/Functional/EntityResource/Item/ItemResourceTestBase.php index f217b97..c7bd57a 100644 --- a/core/modules/rest/tests/src/Functional/EntityResource/Item/ItemResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/EntityResource/Item/ItemResourceTestBase.php @@ -81,6 +81,20 @@ protected function createEntity() { /** * {@inheritdoc} */ + protected function createAnotherEntity() { + $entity = $this->entity->createDuplicate(); + $entity->setLink('https://www.example.org/'); + $label_key = $entity->getEntityType()->getKey('label'); + if ($label_key) { + $entity->set($label_key, $entity->label() . '_dupe'); + } + $entity->save(); + return $entity; + } + + /** + * {@inheritdoc} + */ protected function getExpectedNormalizedEntity() { $feed = Feed::load($this->entity->getFeedId()); 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 d25bf2f..7974cba 100644 --- a/core/modules/rest/tests/src/Functional/EntityResource/User/UserResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/EntityResource/User/UserResourceTestBase.php @@ -82,6 +82,17 @@ protected function createEntity() { /** * {@inheritdoc} */ + protected function createAnotherEntity() { + /** @var \Drupal\user\UserInterface $user */ + $user = $this->entity->createDuplicate(); + $user->setUsername($user->label() . '_dupe'); + $user->save(); + return $user; + } + + /** + * {@inheritdoc} + */ protected function getExpectedNormalizedEntity() { return [ 'uid' => [ @@ -244,6 +255,49 @@ protected function assertRpcLogin($username, $password) { } /** + * Tests PATCHing security-sensitive base fields to change other users. + */ + public function testPatchSecurityOtherUser() { + // The anonymous user is never allowed to modify other users. + if (!static::$auth) { + $this->markTestSkipped(); + } + + $this->initAuthentication(); + $this->provisionEntityResource(); + + /** @var \Drupal\user\UserInterface $user */ + $user = $this->account; + $original_normalization = array_diff_key($this->serializer->normalize($user, static::$format), ['changed' => TRUE]); + + // Since this test must be performed by the user that is being modified, + // we cannot use $this->getUrl(). + $url = $user->toUrl()->setOption('query', ['_format' => static::$format]); + $request_options = [ + RequestOptions::HEADERS => ['Content-Type' => static::$mimeType], + ]; + $request_options = array_merge_recursive($request_options, $this->getAuthenticationRequestOptions('PATCH')); + + $normalization = $original_normalization; + $normalization['mail'] = [['value' => 'new-email@example.com']]; + $request_options[RequestOptions::BODY] = $this->serializer->encode($normalization, static::$format); + + // Try changing user 1's email. + $user1 = [ + 'mail' => [['value' => 'another_email_address@example.com']], + 'uid' => [['value' => 1]], + 'name' => [['value' => 'another_user_name']], + 'pass' => [['existing' => $this->account->passRaw]], + 'uuid' => [['value' => '2e9403a4-d8af-4096-a116-624710140be0']], + ] + $original_normalization; + $request_options[RequestOptions::BODY] = $this->serializer->encode($user1, static::$format); + $response = $this->request('PATCH', $url, $request_options); + // Ensure the email address has not changed. + $this->assertEquals('admin@example.com', $this->entityStorage->loadUnchanged(1)->getEmail()); + $this->assertResourceErrorResponse(403, "Access denied on updating field 'uid'.", $response); + } + + /** * {@inheritdoc} */ protected function getExpectedUnauthorizedAccessMessage($method) { diff --git a/core/modules/rest/tests/src/Functional/EntityResource/User/UserXmlBasicAuthTest.php b/core/modules/rest/tests/src/Functional/EntityResource/User/UserXmlBasicAuthTest.php index dbf74b1..2281d33 100644 --- a/core/modules/rest/tests/src/Functional/EntityResource/User/UserXmlBasicAuthTest.php +++ b/core/modules/rest/tests/src/Functional/EntityResource/User/UserXmlBasicAuthTest.php @@ -41,4 +41,12 @@ public function testPatchDxForSecuritySensitiveBaseFields() { $this->markTestSkipped(); } + /** + * {@inheritdoc} + */ + public function testPatchSecurityOtherUser() { + // Deserialization of the XML format is not supported. + $this->markTestSkipped(); + } + } diff --git a/core/modules/rest/tests/src/Functional/EntityResource/User/UserXmlCookieTest.php b/core/modules/rest/tests/src/Functional/EntityResource/User/UserXmlCookieTest.php index d1ef16e..3e01ce0 100644 --- a/core/modules/rest/tests/src/Functional/EntityResource/User/UserXmlCookieTest.php +++ b/core/modules/rest/tests/src/Functional/EntityResource/User/UserXmlCookieTest.php @@ -36,4 +36,12 @@ public function testPatchDxForSecuritySensitiveBaseFields() { $this->markTestSkipped(); } + /** + * {@inheritdoc} + */ + public function testPatchSecurityOtherUser() { + // Deserialization of the XML format is not supported. + $this->markTestSkipped(); + } + } diff --git a/core/modules/views/src/Plugin/views/query/Sql.php b/core/modules/views/src/Plugin/views/query/Sql.php index 09a754b..1f87417 100644 --- a/core/modules/views/src/Plugin/views/query/Sql.php +++ b/core/modules/views/src/Plugin/views/query/Sql.php @@ -220,6 +220,26 @@ public function &getTableQueue() { } /** + * Returns a reference to the table queue array for this query. + * + * Because this method returns by reference, alter hooks may edit the tables + * array directly to make their changes. If just adding tables, however, the + * use of the addTable() method is preferred. + * + * Note that this method must be called by reference as well: + * + * @code + * $tables =& $query->getTableQueue(); + * @endcode + * + * @return array + * A reference to the table queue array structure. + */ + public function &getTableQueue() { + return $this->tableQueue; + } + + /** * Set the view to be distinct (per base field). * * @param bool $value diff --git a/core/tests/Drupal/KernelTests/Core/Entity/EntityAccessControlHandlerTest.php b/core/tests/Drupal/KernelTests/Core/Entity/EntityAccessControlHandlerTest.php index b3af27f..8545efe 100644 --- a/core/tests/Drupal/KernelTests/Core/Entity/EntityAccessControlHandlerTest.php +++ b/core/tests/Drupal/KernelTests/Core/Entity/EntityAccessControlHandlerTest.php @@ -8,6 +8,7 @@ use Drupal\Core\Entity\EntityAccessControlHandler; use Drupal\Core\Session\AnonymousUserSession; use Drupal\entity_test\Entity\EntityTest; +use Drupal\entity_test\Entity\EntityTestStringId; use Drupal\entity_test\Entity\EntityTestDefaultAccess; use Drupal\entity_test\Entity\EntityTestNoUuid; use Drupal\entity_test\Entity\EntityTestLabel; @@ -18,6 +19,7 @@ /** * Tests the entity access control handler. * + * @coversDefaultClass \Drupal\Core\Entity\EntityAccessControlHandler * @group Entity */ class EntityAccessControlHandlerTest extends EntityLanguageTestBase { @@ -30,6 +32,7 @@ public function setUp() { $this->installEntitySchema('entity_test_no_uuid'); $this->installEntitySchema('entity_test_rev'); + $this->installEntitySchema('entity_test_string_id'); } /** @@ -293,4 +296,73 @@ public function testHooks() { $this->assertEqual($state->get('entity_test_entity_test_access'), TRUE); } + /** + * Tests the default access handling for the ID and UUID fields. + * + * @covers ::fieldAccess + * @dataProvider providerTestFieldAccess + */ + public function testFieldAccess($entity_class, array $entity_create_values, $expected_id_create_access) { + // Set up a non-admin user that is allowed to create and update test + // entities. + \Drupal::currentUser()->setAccount($this->createUser(['uid' => 2], ['administer entity_test content'])); + + // Create the entity to test field access with. + $entity = $entity_class::create($entity_create_values); + + // On newly-created entities, field access must allow setting the UUID + // field. + $this->assertTrue($entity->get('uuid')->access('edit')); + $this->assertTrue($entity->get('uuid')->access('edit', NULL, TRUE)->isAllowed()); + // On newly-created entities, field access will not allow setting the ID + // field if the ID is of type serial. It will allow access if it is of type + // string. + $this->assertEquals($expected_id_create_access, $entity->get('id')->access('edit')); + $this->assertEquals($expected_id_create_access, $entity->get('id')->access('edit', NULL, TRUE)->isAllowed()); + + // Save the entity and check that we can not update the ID or UUID fields + // anymore. + $entity->save(); + + // If the ID has been set as part of the create ensure it has been set + // correctly. + if (isset($entity_create_values['id'])) { + $this->assertSame($entity_create_values['id'], $entity->id()); + } + // The UUID is hard-coded by the data provider. + $this->assertSame('60e3a179-79ed-4653-ad52-5e614c8e8fbe', $entity->uuid()); + $this->assertFalse($entity->get('uuid')->access('edit')); + $access_result = $entity->get('uuid')->access('edit', NULL, TRUE); + $this->assertTrue($access_result->isForbidden()); + $this->assertEquals('The entity UUID cannot be changed', $access_result->getReason()); + + // Ensure the ID is still not allowed to be edited. + $this->assertFalse($entity->get('id')->access('edit')); + $access_result = $entity->get('id')->access('edit', NULL, TRUE); + $this->assertTrue($access_result->isForbidden()); + $this->assertEquals('The entity ID cannot be changed', $access_result->getReason()); + } + + public function providerTestFieldAccess() { + return [ + 'serial ID entity' => [ + EntityTest::class, + [ + 'name' => 'A test entity', + 'uuid' => '60e3a179-79ed-4653-ad52-5e614c8e8fbe', + ], + FALSE + ], + 'string ID entity' => [ + EntityTestStringId::class, + [ + 'id' => 'a_test_entity', + 'name' => 'A test entity', + 'uuid' => '60e3a179-79ed-4653-ad52-5e614c8e8fbe', + ], + TRUE + ], + ]; + } + }