From 5fabcffb44a189d00aada1cb05a58922ab6d7742 Mon Sep 17 00:00:00 2001
From: catch <catch@35733.no-reply.drupal.org>
Date: Fri, 13 Sep 2019 12:21:53 +0100
Subject: [PATCH] Issue #3057175 by Wim Leers, gabesullice, Berdir:
Implementation of user name in JSON:API can result in overwriting data
---
.../jsonapi/src/Controller/EntityResource.php | 15 +++++
.../src/JsonApiResource/ResourceObject.php | 22 +++++--
.../Normalizer/ContentEntityDenormalizer.php | 8 +++
.../ResourceType/ResourceTypeRepository.php | 9 +++
.../jsonapi_test_user.info.yml | 4 ++
.../jsonapi_test_user.module | 17 ++++++
.../tests/src/Functional/ResourceTestBase.php | 40 +++++++------
.../jsonapi/tests/src/Functional/UserTest.php | 58 ++++++++++++++++++-
.../JsonApiDocumentTopLevelNormalizerTest.php | 4 +-
9 files changed, 147 insertions(+), 30 deletions(-)
create mode 100644 core/modules/jsonapi/tests/modules/jsonapi_test_user/jsonapi_test_user.info.yml
create mode 100644 core/modules/jsonapi/tests/modules/jsonapi_test_user/jsonapi_test_user.module
diff --git a/core/modules/jsonapi/src/Controller/EntityResource.php b/core/modules/jsonapi/src/Controller/EntityResource.php
index 8c0e027aa32d..ca76ab31adf4 100644
--- a/core/modules/jsonapi/src/Controller/EntityResource.php
+++ b/core/modules/jsonapi/src/Controller/EntityResource.php
@@ -242,6 +242,13 @@ public function createIndividual(ResourceType $resource_type, Request $request)
}, array_keys($document['data'][$data_member_name])), function ($internal_field_name) use ($resource_type) {
return $resource_type->hasField($internal_field_name);
});
+ // User resource objects contain a read-only attribute that is not a
+ // real field on the user entity type.
+ // @see \Drupal\jsonapi\JsonApiResource\ResourceObject::extractContentEntityFields()
+ // @todo: eliminate this special casing in https://www.drupal.org/project/drupal/issues/3079254.
+ if ($resource_type->getEntityTypeId() === 'user') {
+ $valid_names = array_diff($valid_names, [$resource_type->getPublicName('display_name')]);
+ }
foreach ($valid_names as $field_name) {
$field_access = $parsed_entity->get($field_name)->access('edit', NULL, TRUE);
if (!$field_access->isAllowed()) {
@@ -317,6 +324,14 @@ public function patchIndividual(ResourceType $resource_type, EntityInterface $en
$data += ['attributes' => [], 'relationships' => []];
$field_names = array_merge(array_keys($data['attributes']), array_keys($data['relationships']));
+ // User resource objects contain a read-only attribute that is not a real
+ // field on the user entity type.
+ // @see \Drupal\jsonapi\JsonApiResource\ResourceObject::extractContentEntityFields()
+ // @todo: eliminate this special casing in https://www.drupal.org/project/drupal/issues/3079254.
+ if ($entity->getEntityTypeId() === 'user') {
+ $field_names = array_diff($field_names, [$resource_type->getPublicName('display_name')]);
+ }
+
array_reduce($field_names, function (EntityInterface $destination, $field_name) use ($resource_type, $parsed_entity) {
$this->updateEntityField($resource_type, $parsed_entity, $destination, $field_name);
return $destination;
diff --git a/core/modules/jsonapi/src/JsonApiResource/ResourceObject.php b/core/modules/jsonapi/src/JsonApiResource/ResourceObject.php
index 0540248c9b9c..2505d006c20a 100644
--- a/core/modules/jsonapi/src/JsonApiResource/ResourceObject.php
+++ b/core/modules/jsonapi/src/JsonApiResource/ResourceObject.php
@@ -15,6 +15,7 @@
use Drupal\jsonapi\ResourceType\ResourceType;
use Drupal\jsonapi\Revisions\VersionByRel;
use Drupal\jsonapi\Routing\Routes;
+use Drupal\user\UserInterface;
/**
* Represents a JSON:API resource object.
@@ -281,11 +282,15 @@ protected static function extractContentEntityFields(ResourceType $resource_type
[$resource_type, 'isFieldEnabled']
);
- // Special handling for user entities.
- // @todo Improve in https://www.drupal.org/project/drupal/issues/3057175.
+ // Special handling for user entities that allows a JSON:API user agent to
+ // access the display name of a user. For example, this is useful when
+ // displaying the name of a node's author.
+ // @todo: eliminate this special casing in https://www.drupal.org/project/drupal/issues/3079254.
$entity_type = $entity->getEntityType();
- if ($entity_type->id() == 'user') {
- $fields[static::getLabelFieldName($entity)]->value = $entity->label();
+ if ($entity_type->id() == 'user' && $resource_type->isFieldEnabled('display_name')) {
+ assert($entity instanceof UserInterface);
+ $display_name = $resource_type->getPublicName('display_name');
+ $output[$display_name] = $entity->getDisplayName();
}
// Return a sub-array of $output containing the keys in $enabled_fields.
@@ -294,6 +299,7 @@ protected static function extractContentEntityFields(ResourceType $resource_type
$public_field_name = $resource_type->getPublicName($field_name);
$output[$public_field_name] = $field_value;
}
+
return $output;
}
@@ -308,9 +314,13 @@ protected static function extractContentEntityFields(ResourceType $resource_type
*/
protected static function getLabelFieldName(EntityInterface $entity) {
$label_field_name = $entity->getEntityType()->getKey('label');
- // @todo Remove this work-around after https://www.drupal.org/project/drupal/issues/2450793 lands.
+ // Special handling for user entities that allows a JSON:API user agent to
+ // access the display name of a user. This is useful when displaying the
+ // name of a node's author.
+ // @see \Drupal\jsonapi\JsonApiResource\ResourceObject::extractContentEntityFields()
+ // @todo: eliminate this special casing in https://www.drupal.org/project/drupal/issues/3079254.
if ($entity->getEntityTypeId() === 'user') {
- $label_field_name = 'name';
+ $label_field_name = 'display_name';
}
return $label_field_name;
}
diff --git a/core/modules/jsonapi/src/Normalizer/ContentEntityDenormalizer.php b/core/modules/jsonapi/src/Normalizer/ContentEntityDenormalizer.php
index cd821d497564..287f2cb14050 100644
--- a/core/modules/jsonapi/src/Normalizer/ContentEntityDenormalizer.php
+++ b/core/modules/jsonapi/src/Normalizer/ContentEntityDenormalizer.php
@@ -47,6 +47,14 @@ protected function prepareInput(array $data, ResourceType $resource_type, $forma
$bundle_key = $entity_type_definition->getKey('bundle');
$uuid_key = $entity_type_definition->getKey('uuid');
+ // User resource objects contain a read-only attribute that is not a real
+ // field on the user entity type.
+ // @see \Drupal\jsonapi\JsonApiResource\ResourceObject::extractContentEntityFields()
+ // @todo: eliminate this special casing in https://www.drupal.org/project/drupal/issues/3079254.
+ if ($entity_type_id === 'user') {
+ $data = array_diff_key($data, array_flip([$resource_type->getPublicName('display_name')]));
+ }
+
// Translate the public fields into the entity fields.
foreach ($data as $public_field_name => $field_value) {
$internal_name = $resource_type->getInternalName($public_field_name);
diff --git a/core/modules/jsonapi/src/ResourceType/ResourceTypeRepository.php b/core/modules/jsonapi/src/ResourceType/ResourceTypeRepository.php
index 48563444f53e..ba609359dda9 100644
--- a/core/modules/jsonapi/src/ResourceType/ResourceTypeRepository.php
+++ b/core/modules/jsonapi/src/ResourceType/ResourceTypeRepository.php
@@ -244,6 +244,15 @@ protected static function getFieldMapping(array $field_names, EntityTypeInterfac
$mapping[$field_name] = TRUE;
}
+ // Special handling for user entities that allows a JSON:API user agent to
+ // access the display name of a user. This is useful when displaying the
+ // name of a node's author.
+ // @see \Drupal\jsonapi\JsonApiResource\ResourceObject::extractContentEntityFields()
+ // @todo: eliminate this special casing in https://www.drupal.org/project/drupal/issues/3079254.
+ if ($entity_type->id() === 'user') {
+ $mapping['display_name'] = TRUE;
+ }
+
return $mapping;
}
diff --git a/core/modules/jsonapi/tests/modules/jsonapi_test_user/jsonapi_test_user.info.yml b/core/modules/jsonapi/tests/modules/jsonapi_test_user/jsonapi_test_user.info.yml
new file mode 100644
index 000000000000..179eda4da52d
--- /dev/null
+++ b/core/modules/jsonapi/tests/modules/jsonapi_test_user/jsonapi_test_user.info.yml
@@ -0,0 +1,4 @@
+name: 'JSON:API user tests'
+type: module
+package: Testing
+core: 8.x
diff --git a/core/modules/jsonapi/tests/modules/jsonapi_test_user/jsonapi_test_user.module b/core/modules/jsonapi/tests/modules/jsonapi_test_user/jsonapi_test_user.module
new file mode 100644
index 000000000000..4056bd64d269
--- /dev/null
+++ b/core/modules/jsonapi/tests/modules/jsonapi_test_user/jsonapi_test_user.module
@@ -0,0 +1,17 @@
+<?php
+
+/**
+ * @file
+ * Support module for JSON:API user hooks testing.
+ */
+
+use Drupal\Core\Session\AccountInterface;
+
+/**
+ * Implements hook_user_format_name_alter().
+ */
+function jsonapi_test_user_user_format_name_alter(&$name, AccountInterface $account) {
+ if ($account->isAnonymous()) {
+ $name = 'User ' . $account->id();
+ }
+}
diff --git a/core/modules/jsonapi/tests/src/Functional/ResourceTestBase.php b/core/modules/jsonapi/tests/src/Functional/ResourceTestBase.php
index 6257adb4d82e..006e2b414718 100644
--- a/core/modules/jsonapi/tests/src/Functional/ResourceTestBase.php
+++ b/core/modules/jsonapi/tests/src/Functional/ResourceTestBase.php
@@ -136,14 +136,11 @@ abstract class ResourceTestBase extends BrowserTestBase {
protected static $secondCreatedEntityId = 3;
/**
- * Optionally specify which field is the 'label' field.
- *
- * Some entities specify a 'label_callback', but not a 'label' entity key.
- * For example: User.
+ * Specify which field is the 'label' field for testing a POST edge case.
*
* @var string|null
*
- * @see ::getInvalidNormalizedEntityToCreate()
+ * @see ::testPostIndividual()
*/
protected static $labelFieldName = NULL;
@@ -1883,7 +1880,9 @@ public function testPostIndividual() {
$parseable_valid_request_body = Json::encode($this->getPostDocument());
/* $parseable_valid_request_body_2 = Json::encode($this->getNormalizedPostEntity()); */
$parseable_invalid_request_body_missing_type = Json::encode($this->removeResourceTypeFromDocument($this->getPostDocument(), 'type'));
- $parseable_invalid_request_body = Json::encode($this->makeNormalizationInvalid($this->getPostDocument(), 'label'));
+ if ($this->entity->getEntityType()->hasKey('label')) {
+ $parseable_invalid_request_body = Json::encode($this->makeNormalizationInvalid($this->getPostDocument(), 'label'));
+ }
$parseable_invalid_request_body_2 = Json::encode(NestedArray::mergeDeep(['data' => ['id' => $this->randomMachineName(129)]], $this->getPostDocument()));
$parseable_invalid_request_body_3 = Json::encode(NestedArray::mergeDeep(['data' => ['attributes' => ['field_rest_test' => $this->randomString()]]], $this->getPostDocument()));
$parseable_invalid_request_body_4 = Json::encode(NestedArray::mergeDeep(['data' => ['attributes' => ['field_nonexistent' => $this->randomString()]]], $this->getPostDocument()));
@@ -1939,13 +1938,14 @@ public function testPostIndividual() {
$response = $this->request('POST', $url, $request_options);
$this->assertResourceErrorResponse(400, 'Resource object must include a "type".', $url, $response, FALSE);
- $request_options[RequestOptions::BODY] = $parseable_invalid_request_body;
-
- // DX: 422 when invalid entity: multiple values sent for single-value field.
- $response = $this->request('POST', $url, $request_options);
- $label_field = $this->entity->getEntityType()->hasKey('label') ? $this->entity->getEntityType()->getKey('label') : static::$labelFieldName;
- $label_field_capitalized = $this->entity->getFieldDefinition($label_field)->getLabel();
- $this->assertResourceErrorResponse(422, "$label_field: $label_field_capitalized: this field cannot hold more than 1 values.", NULL, $response, '/data/attributes/' . $label_field);
+ if ($this->entity->getEntityType()->hasKey('label')) {
+ $request_options[RequestOptions::BODY] = $parseable_invalid_request_body;
+ // DX: 422 when invalid entity: multiple values sent for single-value field.
+ $response = $this->request('POST', $url, $request_options);
+ $label_field = $this->entity->getEntityType()->getKey('label');
+ $label_field_capitalized = $this->entity->getFieldDefinition($label_field)->getLabel();
+ $this->assertResourceErrorResponse(422, "$label_field: $label_field_capitalized: this field cannot hold more than 1 values.", NULL, $response, '/data/attributes/' . $label_field);
+ }
$request_options[RequestOptions::BODY] = $parseable_invalid_request_body_2;
@@ -2050,6 +2050,7 @@ public function testPostIndividual() {
// 500 when creating an entity with a duplicate UUID.
$doc = $this->getModifiedEntityForPostTesting();
$doc['data']['id'] = $uuid;
+ $label_field = $this->entity->getEntityType()->hasKey('label') ? $this->entity->getEntityType()->getKey('label') : static::$labelFieldName;
$doc['data']['attributes'][$label_field] = [['value' => $this->randomMachineName()]];
$request_options[RequestOptions::BODY] = Json::encode($doc);
@@ -2148,13 +2149,14 @@ public function testPatchIndividual() {
$response = $this->request('PATCH', $url, $request_options);
$this->assertResourceErrorResponse(400, 'Syntax error', $url, $response, FALSE);
- $request_options[RequestOptions::BODY] = $parseable_invalid_request_body;
-
// DX: 422 when invalid entity: multiple values sent for single-value field.
- $response = $this->request('PATCH', $url, $request_options);
- $label_field = $this->entity->getEntityType()->hasKey('label') ? $this->entity->getEntityType()->getKey('label') : static::$labelFieldName;
- $label_field_capitalized = $this->entity->getFieldDefinition($label_field)->getLabel();
- $this->assertResourceErrorResponse(422, "$label_field: $label_field_capitalized: this field cannot hold more than 1 values.", NULL, $response, '/data/attributes/' . $label_field);
+ if ($this->entity->getEntityType()->hasKey('label')) {
+ $request_options[RequestOptions::BODY] = $parseable_invalid_request_body;
+ $response = $this->request('PATCH', $url, $request_options);
+ $label_field = $this->entity->getEntityType()->getKey('label');
+ $label_field_capitalized = $this->entity->getFieldDefinition($label_field)->getLabel();
+ $this->assertResourceErrorResponse(422, "$label_field: $label_field_capitalized: this field cannot hold more than 1 values.", NULL, $response, '/data/attributes/' . $label_field);
+ }
$request_options[RequestOptions::BODY] = $parseable_invalid_request_body_2;
diff --git a/core/modules/jsonapi/tests/src/Functional/UserTest.php b/core/modules/jsonapi/tests/src/Functional/UserTest.php
index 2277714de8ed..e9aa7b6bf319 100644
--- a/core/modules/jsonapi/tests/src/Functional/UserTest.php
+++ b/core/modules/jsonapi/tests/src/Functional/UserTest.php
@@ -22,7 +22,7 @@ class UserTest extends ResourceTestBase {
/**
* {@inheritdoc}
*/
- public static $modules = ['user'];
+ public static $modules = ['user', 'jsonapi_test_user'];
/**
* {@inheritdoc}
@@ -56,7 +56,7 @@ class UserTest extends ResourceTestBase {
/**
* {@inheritdoc}
*/
- protected static $labelFieldName = 'name';
+ protected static $labelFieldName = 'display_name';
/**
* {@inheritdoc}
@@ -138,6 +138,7 @@ protected function getExpectedDocument() {
'self' => ['href' => $self_url],
],
'attributes' => [
+ 'display_name' => 'Llama',
'created' => '1973-11-29T21:33:09+00:00',
'changed' => (new \DateTime())->setTimestamp($this->entity->getChangedTime())->setTimezone(new \DateTimeZone('UTC'))->format(\DateTime::RFC3339),
'default_langcode' => TRUE,
@@ -441,7 +442,7 @@ public function testCollectionContainsAnonymousUser() {
$this->assertCount(4, $doc['data']);
$this->assertSame(User::load(0)->uuid(), $doc['data'][0]['id']);
- $this->assertSame('Anonymous', $doc['data'][0]['attributes']['name']);
+ $this->assertSame('User 0', $doc['data'][0]['attributes']['display_name']);
}
/**
@@ -551,4 +552,55 @@ public function testCollectionFilterAccess() {
$this->assertSame($user_b->uuid(), $doc['data'][0]['id']);
}
+ /**
+ * Tests users with altered display names.
+ */
+ public function testResaveAccountName() {
+ $this->config('jsonapi.settings')->set('read_only', FALSE)->save(TRUE);
+ $this->setUpAuthorization('PATCH');
+
+ $original_name = $this->entity->get('name')->value;
+
+ $url = Url::fromRoute('jsonapi.user--user.individual', ['entity' => $this->entity->uuid()]);
+ $request_options = [];
+ $request_options[RequestOptions::HEADERS]['Accept'] = 'application/vnd.api+json';
+ $request_options = NestedArray::mergeDeep($request_options, $this->getAuthenticationRequestOptions());
+
+ $response = $this->request('GET', $url, $request_options);
+
+ // Send the unchanged data back.
+ $request_options[RequestOptions::BODY] = (string) $response->getBody();
+ $request_options[RequestOptions::HEADERS]['Content-Type'] = 'application/vnd.api+json';
+ $response = $this->request('PATCH', $url, $request_options);
+ $this->assertEquals(200, $response->getStatusCode());
+
+ // Load the user entity again, make sure the name was not changed.
+ $this->entityStorage->resetCache();
+ $updated_user = $this->entityStorage->load($this->entity->id());
+ $this->assertEquals($original_name, $updated_user->get('name')->value);
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ protected function getModifiedEntityForPostTesting() {
+ $modified = parent::getModifiedEntityForPostTesting();
+ $modified['data']['attributes']['name'] = $this->randomMachineName();
+ return $modified;
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ protected function makeNormalizationInvalid(array $document, $entity_key) {
+ if ($entity_key === 'label') {
+ $document['data']['attributes']['name'] = [
+ 0 => $document['data']['attributes']['name'],
+ 1 => 'Second Title',
+ ];
+ return $document;
+ }
+ return parent::makeNormalizationInvalid($document, $entity_key);
+ }
+
}
diff --git a/core/modules/jsonapi/tests/src/Kernel/Normalizer/JsonApiDocumentTopLevelNormalizerTest.php b/core/modules/jsonapi/tests/src/Kernel/Normalizer/JsonApiDocumentTopLevelNormalizerTest.php
index b4ba1b3b1dc7..99bd557e4177 100644
--- a/core/modules/jsonapi/tests/src/Kernel/Normalizer/JsonApiDocumentTopLevelNormalizerTest.php
+++ b/core/modules/jsonapi/tests/src/Kernel/Normalizer/JsonApiDocumentTopLevelNormalizerTest.php
@@ -230,7 +230,7 @@ public function testNormalize() {
'field_image',
],
'user--user' => [
- 'name',
+ 'display_name',
],
],
'include' => [
@@ -279,7 +279,7 @@ public function testNormalize() {
$this->assertTrue(empty($normalized['meta']['omitted']));
$this->assertSame($this->user->uuid(), $normalized['included'][0]['id']);
$this->assertSame('user--user', $normalized['included'][0]['type']);
- $this->assertSame('user1', $normalized['included'][0]['attributes']['name']);
+ $this->assertSame('user1', $normalized['included'][0]['attributes']['display_name']);
$this->assertCount(1, $normalized['included'][0]['attributes']);
$this->assertSame($this->term1->uuid(), $normalized['included'][1]['id']);
$this->assertSame('taxonomy_term--tags', $normalized['included'][1]['type']);
--
GitLab