.../EntityResource/HalEntityNormalizationTrait.php | 18 ------ .../src/Plugin/rest/resource/EntityResource.php | 67 ++-------------------- .../EntityResource/EntityResourceTestBase.php | 49 +++++++--------- 3 files changed, 26 insertions(+), 108 deletions(-) diff --git a/core/modules/hal/tests/src/Functional/EntityResource/HalEntityNormalizationTrait.php b/core/modules/hal/tests/src/Functional/EntityResource/HalEntityNormalizationTrait.php index b64de67..eb01944 100644 --- a/core/modules/hal/tests/src/Functional/EntityResource/HalEntityNormalizationTrait.php +++ b/core/modules/hal/tests/src/Functional/EntityResource/HalEntityNormalizationTrait.php @@ -73,24 +73,6 @@ protected function applyHalFieldNormalization(array $normalization) { /** * {@inheritdoc} */ - protected function removeFieldsFromNormalization(array $normalization, $field_names) { - $normalization = parent::removeFieldsFromNormalization($normalization, $field_names); - foreach ($field_names as $field_name) { - $relation_url = Url::fromUri('base:rest/relation/' . static::$entityTypeId . '/' . $this->entity->bundle() . '/' . $field_name) - ->setAbsolute(TRUE) - ->toString(); - $normalization['_links'] = array_diff_key($normalization['_links'], [$relation_url => TRUE]); - if (isset($normalization['_embedded'])) { - $normalization['_embedded'] = array_diff_key($normalization['_embedded'], [$relation_url => TRUE]); - } - } - - return array_diff_key($normalization, array_flip($field_names)); - } - - /** - * {@inheritdoc} - */ protected function assertNormalizationEdgeCases($method, Url $url, array $request_options) { // \Drupal\hal\Normalizer\EntityNormalizer::denormalize(): entity // types with bundles MUST send their bundle field to be denormalizable. diff --git a/core/modules/rest/src/Plugin/rest/resource/EntityResource.php b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php index 5d9849d..1d61994 100644 --- a/core/modules/rest/src/Plugin/rest/resource/EntityResource.php +++ b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php @@ -202,41 +202,6 @@ public function post(EntityInterface $entity = NULL) { } /** - * Gets the values from the field item list casted to the correct type. - * - * Values are casted to the correct type so we can determine whether or not - * something has changed. REST formats such as JSON support typed data but - * Drupal's database API will return values as strings. Currently, only - * primitive data types know how to cast their values to the correct type. - * - * @param \Drupal\Core\Field\FieldItemListInterface $field_item_list - * The field item list to retrieve its data from. - * - * @return mixed[][] - * The values from the field item list casted to the correct type. The array - * of values returned is a multidimensional array keyed by delta and the - * property name. - */ - protected function getCastedValueFromFieldItemList(FieldItemListInterface $field_item_list) { - $value = $field_item_list->getValue(); - - foreach ($value as $delta => $field_item_value) { - /** @var \Drupal\Core\Field\FieldItemInterface $field_item */ - $field_item = $field_item_list->get($delta); - $properties = $field_item->getProperties(TRUE); - // Foreach field value we check whether we know the underlying property. - // If we exists we try to cast the value. - foreach ($field_item_value as $property_name => $property_value) { - if (isset($properties[$property_name]) && ($property = $field_item->get($property_name)) && $property instanceof PrimitiveInterface) { - $value[$delta][$property_name] = $property->getCastedValue(); - } - } - } - - return $value; - } - - /** * Responds to entity PATCH requests. * * @param \Drupal\Core\Entity\EntityInterface $original_entity @@ -262,35 +227,15 @@ public function patch(EntityInterface $original_entity, EntityInterface $entity throw new AccessDeniedHttpException($entity_access->getReason() ?: $this->generateFallbackAccessDeniedMessage($entity, 'update')); } - // Overwrite the received properties. - $entity_keys = $entity->getEntityType()->getKeys(); + // Overwrite the received fields. foreach ($entity->_restSubmittedFields as $field_name) { $field = $entity->get($field_name); + $original_field = $original_entity->get($field_name); - // Entity key fields need special treatment: together they uniquely - // identify the entity. Therefore it does not make sense to modify any of - // them. However, rather than throwing an error, we just ignore them as - // long as their specified values match their current values. - if (in_array($field_name, $entity_keys, TRUE)) { - // @todo Work around the wrong assumption that entity keys need special - // treatment, when only read-only fields need it. - // This will be fixed in https://www.drupal.org/node/2824851. - if ($entity->getEntityTypeId() == 'comment' && $field_name == 'status' && !$original_entity->get($field_name)->access('edit')) { - throw new AccessDeniedHttpException("Access denied on updating field '$field_name'."); - } - - // Unchanged values for entity keys don't need access checking. - if ($this->getCastedValueFromFieldItemList($original_entity->get($field_name)) === $this->getCastedValueFromFieldItemList($entity->get($field_name))) { - continue; - } - // It is not possible to set the language to NULL as it is automatically - // re-initialized. As it must not be empty, skip it if it is. - elseif (isset($entity_keys['langcode']) && $field_name === $entity_keys['langcode'] && $field->isEmpty()) { - continue; - } - } - - if (!$original_entity->get($field_name)->access('edit')) { + // Check access for all received fields, but only if the are being + // changed. The bundle of an entity, for example, must be provided for + // denormalization to succeed, but it may not be changed. + if (!$original_field->equals($field) && !$original_field->access('edit')) { throw new AccessDeniedHttpException("Access denied on updating field '$field_name'."); } $original_entity->set($field_name, $field->getValue()); diff --git a/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php index 8638adf..adde7a6 100644 --- a/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php @@ -1020,22 +1020,30 @@ public function testPatch() { $this->assertResourceErrorResponse(403, "Access denied on updating field 'field_rest_test'.", $response); - // DX: 403 when sending PATCH request with read-only fields. - // First send all fields (the "maximum normalization"). Assert the expected - // error message for the first PATCH-protected field. Remove that field from - // the normalization, send another request, assert the next PATCH-protected - // field error message. And so on. - $max_normalization = $this->getNormalizedPatchEntity() + $this->serializer->normalize($this->entity, static::$format); + // DX: 403 when sending PATCH request with updated read-only fields. + // Clone the entity, modifies all PATCH-protected fields. Then performs a + $modified_entity = clone $this->entity; + $original_values = []; + foreach (static::$patchProtectedFieldNames as $field_name) { + $field = $modified_entity->get($field_name); + $original_values[$field_name] = $field->value; + $field->generateSampleItems(); + } + // Send PATCH request by serializing the cloned entity, assert the error + // response, change the cloned entity field that caused the error response + // back to its original value, repeat. for ($i = 0; $i < count(static::$patchProtectedFieldNames); $i++) { - $max_normalization = $this->removeFieldsFromNormalization($max_normalization, array_slice(static::$patchProtectedFieldNames, 0, $i)); - $request_options[RequestOptions::BODY] = $this->serializer->serialize($max_normalization, static::$format); + $patch_protected_field_name = static::$patchProtectedFieldNames[$i]; + $request_options[RequestOptions::BODY] = $this->serializer->serialize($modified_entity, static::$format); $response = $this->request('PATCH', $url, $request_options); - $this->assertResourceErrorResponse(403, "Access denied on updating field '" . static::$patchProtectedFieldNames[$i] . "'.", $response); + $this->assertResourceErrorResponse(403, "Access denied on updating field '" . $patch_protected_field_name . "'.", $response); + $modified_entity->get($patch_protected_field_name)->setValue($original_values[$patch_protected_field_name]); } - // 200 for well-formed request that sends the maximum number of fields. - $max_normalization = $this->removeFieldsFromNormalization($max_normalization, static::$patchProtectedFieldNames); - $request_options[RequestOptions::BODY] = $this->serializer->serialize($max_normalization, static::$format); + // 200 for well-formed PATCH request that sends all fields (even including + // read-only ones, but with unchanged values). + $valid_request_body = $this->getNormalizedPatchEntity() + $this->serializer->normalize($this->entity, static::$format); + $request_options[RequestOptions::BODY] = $this->serializer->serialize($valid_request_body, static::$format); $response = $this->request('PATCH', $url, $request_options); $this->assertResourceResponse(200, FALSE, $response); @@ -1275,23 +1283,6 @@ protected function makeNormalizationInvalid(array $normalization) { } /** - * Removes fields from a normalization. - * - * @param array $normalization - * An entity normalization. - * @param string[] $field_names - * The field names to remove from the entity normalization. - * - * @return array - * The updated entity normalization. - * - * @see ::testPatch - */ - protected function removeFieldsFromNormalization(array $normalization, $field_names) { - return array_diff_key($normalization, array_flip($field_names)); - } - - /** * Asserts a 406 response… or in some cases a 403 response, because weirdness. * * Asserting a 406 response should be easy, but it's not, due to bugs.