.../src/Plugin/rest/resource/EntityResource.php | 62 ++++++++++++++++------ 1 file changed, 45 insertions(+), 17 deletions(-) diff --git a/core/modules/rest/src/Plugin/rest/resource/EntityResource.php b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php index 5fa7aae..cb3e07f 100644 --- a/core/modules/rest/src/Plugin/rest/resource/EntityResource.php +++ b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php @@ -11,6 +11,7 @@ use Drupal\Core\Config\ConfigFactoryInterface; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Entity\EntityStorageException; +use Drupal\Core\Field\FieldItemListInterface; use Drupal\Core\Http\Exception\CacheableAccessDeniedHttpException; use Drupal\rest\Plugin\ResourceBase; use Drupal\rest\ResourceResponse; @@ -230,7 +231,6 @@ public function patch(EntityInterface $original_entity, EntityInterface $entity // Overwrite the received fields. foreach ($entity->_restSubmittedFields as $field_name) { $field = $entity->get($field_name); - $original_field = $original_entity->get($field_name); // The User entity type's 'pass' field is a very special case: even though // it is not allowed to be viewed nor edited, to be able to modify @@ -241,24 +241,9 @@ public function patch(EntityInterface $original_entity, EntityInterface $entity continue; } - // 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. - if ($original_field->access('edit')) { + if ($this->checkPatchFieldAccess($field_name, $original_entity->get($field_name), $field)) { $original_entity->set($field_name, $field->getValue()); } - // If the user is not allowed to edit the field, throw an exception (that - // will be transformed into a helpful 403 response). But only when: - // - the user IS NOT allowed to view the field, regardless of whether the - // value matches or not, to avoid information disclosure. - // (Otherwise the user may try PATCHing with value after value, until - // they send the current value for the field, and then they won't get a - // 403 response anymore, which indicates that the value they sent in the - // PATCH request body matches the current value.) - // - the user IS allowed to view the field, but the value does not match; - // because they are not allowed to edit the field. - elseif (!$original_field->access('view') || !$original_field->equals($field)) { - throw new AccessDeniedHttpException("Access denied on updating field '$field_name'."); - } } // Validate the received data before saving. @@ -276,6 +261,49 @@ public function patch(EntityInterface $original_entity, EntityInterface $entity } /** + * Checks whether the given field should be PATCHed. + * + * @param $field_name + * The name of the field. + * @param \Drupal\Core\Field\FieldItemListInterface $original_field + * The original (stored) value for the field. + * @param \Drupal\Core\Field\FieldItemListInterface $received_field + * The received value for the field. + * + * @return bool + * Whether the field should be PATCHed or not. + * + * @throws \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException + * Thrown when the user sending the request is not allowed to update the + * field. Only thrown when the user could not abuse this information to + * determine the stored value. + * + * @internal + */ + protected function checkPatchFieldAccess($field_name, 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. + if ($original_field->access('edit')) { + return TRUE; + } + // If the user is not allowed to edit the field, throw an exception (that + // will be transformed into a helpful 403 response). But only when: + // - the user IS NOT allowed to view the field, regardless of whether the + // value matches or not, to avoid information disclosure. + // (Otherwise the user may try PATCHing with value after value, until + // they send the current value for the field, and then they won't get a + // 403 response anymore, which indicates that the value they sent in the + // PATCH request body matches the current value.) + // - the user IS allowed to view the field, but the value does not match; + // because they are not allowed to edit the field. + elseif (!$original_field->access('view') || !$original_field->equals($received_field)) { + throw new AccessDeniedHttpException("Access denied on updating field '$field_name'."); + } + + return FALSE; + } + + /** * Responds to entity DELETE requests. * * @param \Drupal\Core\Entity\EntityInterface $entity