Commit 4fbf9977 authored by alexpott's avatar alexpott

Issue #2885469 by Wim Leers, amateescu, timmillwood, alexpott, tstoeckler,...

Issue #2885469 by Wim Leers, amateescu, timmillwood, alexpott, tstoeckler, xjm, remram, Berdir, dawehner, larowlan, samuel.mortenson: Regression: manually setting the ID field for newly-created content entities is not possible anymore (public follow-up to SA-2017-002)
parent 418e4991
......@@ -1123,7 +1123,9 @@ public function createDuplicate() {
$duplicate = clone $this;
$entity_type = $this->getEntityType();
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.
......
......@@ -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;
}
}
......
......@@ -86,6 +86,14 @@ abstract class EntityResourceTestBase extends ResourceTestBase {
*/
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.
......@@ -125,6 +133,13 @@ abstract class EntityResourceTestBase extends ResourceTestBase {
*/
protected $entity;
/**
* Another entity of the same type used for testing.
*
* @var \Drupal\Core\Entity\EntityInterface
*/
protected $anotherEntity;
/**
* The entity storage.
*
......@@ -221,6 +236,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.
*
......@@ -253,6 +284,29 @@ protected function getNormalizedPatchEntity() {
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}
*/
......@@ -712,7 +766,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 +925,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 +935,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 +983,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 +993,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 +1090,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 +1404,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) {
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 = $this->entity->getEntityType()->hasKey('label') ? $this->entity->getEntityType()->getKey('label') : static::$labelFieldName;
$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;
}
......
......@@ -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;
......@@ -20,6 +20,16 @@ abstract class FeedResourceTestBase extends EntityTestResourceTestBase {
*/
public static $entityTypeId = 'aggregator_feed';
/**
* {@inheritdoc}
*/
protected static $patchProtectedFieldNames = [];
/**
* {@inheritdoc}
*/
protected static $uniqueFieldNames = ['url'];
/**
* {@inheritdoc}
*/
......
......@@ -78,6 +78,20 @@ protected function createEntity() {
return $item;
}
/**
* {@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}
*/
......
......@@ -79,6 +79,17 @@ protected function createEntity() {
return $user;
}
/**
* {@inheritdoc}
*/
protected function createAnotherEntity() {
/** @var \Drupal\user\UserInterface $user */
$user = $this->entity->createDuplicate();
$user->setUsername($user->label() . '_dupe');
$user->save();
return $user;
}
/**
* {@inheritdoc}
*/
......@@ -243,6 +254,49 @@ protected function assertRpcLogin($username, $password) {
$this->assertSame(200, $response->getStatusCode());
}
/**
* 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}
*/
......
......@@ -41,4 +41,12 @@ public function testPatchDxForSecuritySensitiveBaseFields() {
$this->markTestSkipped();
}
/**
* {@inheritdoc}
*/
public function testPatchSecurityOtherUser() {
// Deserialization of the XML format is not supported.
$this->markTestSkipped();
}
}
......@@ -36,4 +36,12 @@ public function testPatchDxForSecuritySensitiveBaseFields() {
$this->markTestSkipped();
}
/**
* {@inheritdoc}
*/
public function testPatchSecurityOtherUser() {
// Deserialization of the XML format is not supported.
$this->markTestSkipped();
}
}
......@@ -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
],
];
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment