Unverified Commit 55ffe4fa authored by alexpott's avatar alexpott

Issue #2821077 by Wim Leers, dawehner, tedbow, amateescu, tstoeckler,...

Issue #2821077 by Wim Leers, dawehner, tedbow, amateescu, tstoeckler, alexpott: PATCHing entities validates the entire entity, also unmodified fields, so unmodified fields can throw validation errors

(cherry picked from commit 73769ef5)
parent f83f2147
......@@ -229,6 +229,8 @@ public function patch(EntityInterface $original_entity, EntityInterface $entity
}
// Overwrite the received fields.
// @todo Remove $changed_fields in https://www.drupal.org/project/drupal/issues/2862574.
$changed_fields = [];
foreach ($entity->_restSubmittedFields as $field_name) {
$field = $entity->get($field_name);
// It is not possible to set the language to NULL as it is automatically
......@@ -238,12 +240,18 @@ public function patch(EntityInterface $original_entity, EntityInterface $entity
continue;
}
if ($this->checkPatchFieldAccess($original_entity->get($field_name), $field)) {
$changed_fields[] = $field_name;
$original_entity->set($field_name, $field->getValue());
}
}
// If no fields are changed, we can send a response immediately!
if (empty($changed_fields)) {
return new ModifiedResourceResponse($original_entity, 200);
}
// Validate the received data before saving.
$this->validate($original_entity);
$this->validate($original_entity, $changed_fields);
try {
$original_entity->save();
$this->logger->notice('Updated entity %type with ID %id.', ['%type' => $original_entity->getEntityTypeId(), '%id' => $original_entity->id()]);
......@@ -275,13 +283,6 @@ public function patch(EntityInterface $original_entity, EntityInterface $entity
* @internal
*/
protected function checkPatchFieldAccess(FieldItemListInterface $original_field, FieldItemListInterface $received_field) {
// If the user is allowed to edit the field, it is always safe to set the
// received value. We may be setting an unchanged value, but that is ok.
$field_edit_access = $original_field->access('edit', NULL, TRUE);
if ($field_edit_access->isAllowed()) {
return TRUE;
}
// The user might not have access to edit the field, but still needs to
// submit the current field value as part of the PATCH request. For
// example, the entity keys required by denormalizers. Therefore, if the
......@@ -294,6 +295,13 @@ protected function checkPatchFieldAccess(FieldItemListInterface $original_field,
return FALSE;
}
// If the user is allowed to edit the field, it is always safe to set the
// received value. We may be setting an unchanged value, but that is ok.
$field_edit_access = $original_field->access('edit', NULL, TRUE);
if ($field_edit_access->isAllowed()) {
return TRUE;
}
// It's helpful and safe to let the user know when they are not allowed to
// update a field.
$field_name = $received_field->getName();
......
......@@ -14,16 +14,23 @@
trait EntityResourceValidationTrait {
/**
* Verifies that the whole entity does not violate any validation constraints.
* Verifies that an entity does not violate any validation constraints.
*
* The validation errors will be filtered to not include fields to which the
* current user does not have access and if $fields_to_validate is provided
* will only include fields in that array.
*
* @param \Drupal\Core\Entity\EntityInterface $entity
* The entity to validate.
* @param string[] $fields_to_validate
* (optional) An array of field names. If specified, filters the violations
* list to include only this set of fields.
*
* @throws \Symfony\Component\HttpKernel\Exception\UnprocessableEntityHttpException
* If validation errors are found.
*/
protected function validate(EntityInterface $entity) {
// @todo Remove when https://www.drupal.org/node/2164373 is committed.
protected function validate(EntityInterface $entity, array $fields_to_validate = []) {
// @todo Update this check in https://www.drupal.org/node/2300677.
if (!$entity instanceof FieldableEntityInterface) {
return;
}
......@@ -33,6 +40,11 @@ protected function validate(EntityInterface $entity) {
// changes.
$violations->filterByFieldAccess();
if ($fields_to_validate) {
// Filter violations by explicitly provided array of field names.
$violations->filterByFields(array_diff(array_keys($entity->getFieldDefinitions()), $fields_to_validate));
}
if ($violations->count() > 0) {
$message = "Unprocessable Entity: validation failed.\n";
foreach ($violations as $violation) {
......
......@@ -5,6 +5,8 @@
* Contains hook implementations for testing REST module.
*/
use Drupal\Core\Entity\EntityTypeInterface;
use Drupal\Core\Field\BaseFieldDefinition;
use Drupal\Core\Field\FieldDefinitionInterface;
use Drupal\Core\Session\AccountInterface;
use Drupal\Core\Field\FieldItemListInterface;
......@@ -40,6 +42,30 @@ function rest_test_entity_field_access($operation, FieldDefinitionInterface $fie
}
}
// @see \Drupal\Tests\rest\Functional\EntityResource\EntityResourceTestBase::testGet()
// @see \Drupal\Tests\rest\Functional\EntityResource\EntityResourceTestBase::testPatch()
if ($field_definition->getName() === 'rest_test_validation') {
switch ($operation) {
case 'view':
// Never ever allow this field to be viewed: this lets
// EntityResourceTestBase::testGet() test in a "vanilla" way.
return AccessResult::forbidden();
}
}
// No opinion.
return AccessResult::neutral();
}
/**
* Implements hook_entity_base_field_info().
*/
function rest_test_entity_base_field_info(EntityTypeInterface $entity_type) {
$fields = [];
$fields['rest_test_validation'] = BaseFieldDefinition::create('string')
->setLabel(t('REST test validation field'))
->setDescription(t('A text field with some special validations attached used for testing purposes'))
->addConstraint('rest_test_validation');
return $fields;
}
<?php
namespace Drupal\rest_test\Plugin\Validation\Constraint;
use Symfony\Component\Validator\Constraint;
/**
* Adds some validations for a REST test field.
*
* @Constraint(
* id = "rest_test_validation",
* label = @Translation("REST test validation", context = "Validation")
* )
*
* @see \Drupal\Core\TypedData\OptionsProviderInterface
*/
class RestTestConstraint extends Constraint {
public $message = 'REST test validation failed';
}
<?php
namespace Drupal\rest_test\Plugin\Validation\Constraint;
use Drupal\Core\Field\FieldItemListInterface;
use Symfony\Component\Validator\Constraint;
use Symfony\Component\Validator\ConstraintValidator;
/**
* Validator for \Drupal\rest_test\Plugin\Validation\Constraint\RestTestConstraint.
*/
class RestTestConstraintValidator extends ConstraintValidator {
/**
* {@inheritdoc}
*/
public function validate($value, Constraint $constraint) {
if ($value instanceof FieldItemListInterface) {
$value = $value->getValue();
if (!empty($value[0]['value']) && $value[0]['value'] === 'ALWAYS_FAIL') {
$this->context->addViolation($constraint->message);
}
}
}
}
......@@ -236,6 +236,7 @@ public function setUp() {
// Set a default value on the fields.
$this->entity->set('field_rest_test', ['value' => 'All the faith he had had had had no effect on the outcome of his life.']);
$this->entity->set('field_rest_test_multivalue', [['value' => 'One'], ['value' => 'Two']]);
$this->entity->set('rest_test_validation', ['value' => 'allowed value']);
$this->entity->save();
}
}
......@@ -667,6 +668,7 @@ public function testGet() {
// ::formatExpectedTimestampValue() to generate the timestamp value. This
// will take into account the above config setting.
$expected = $this->getExpectedNormalizedEntity();
// Config entities are not affected.
// @see \Drupal\serialization\Normalizer\ConfigEntityNormalizer::normalize()
static::recursiveKSort($expected);
......@@ -1144,6 +1146,39 @@ public function testPatch() {
$modified_entity->get($patch_protected_field_name)->setValue($original_values[$patch_protected_field_name]);
}
if ($this->entity instanceof FieldableEntityInterface) {
// Change the rest_test_validation field to prove that then its validation
// does run.
$override = [
'rest_test_validation' => [
[
'value' => 'ALWAYS_FAIL',
],
],
];
$valid_request_body = $override + $this->getNormalizedPatchEntity() + $this->serializer->normalize($modified_entity, static::$format);
$request_options[RequestOptions::BODY] = $this->serializer->serialize($valid_request_body, static::$format);
$response = $this->request('PATCH', $url, $request_options);
$this->assertResourceErrorResponse(422, "Unprocessable Entity: validation failed.\nrest_test_validation: REST test validation failed\n", $response);
// Set the rest_test_validation field to always fail validation, which
// allows asserting that not modifying that field does not trigger
// validation errors.
$this->entity->set('rest_test_validation', 'ALWAYS_FAIL');
$this->entity->save();
// Information disclosure prevented: when a malicious user correctly
// guesses the current invalid value of a field, ensure a 200 is not sent
// because this would disclose to the attacker what the current value is.
// @see rest_test_entity_field_access()
$response = $this->request('PATCH', $url, $request_options);
$this->assertResourceErrorResponse(422, "Unprocessable Entity: validation failed.\nrest_test_validation: REST test validation failed\n", $response);
// All requests after the above one will not include this field (neither
// its current value nor any other), and therefore all subsequent test
// assertions should not trigger a validation error.
}
// 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);
......
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