From 466759dc15f57fb2a01381612041a71da32b0362 Mon Sep 17 00:00:00 2001 From: Alex Pott Date: Mon, 2 Feb 2015 12:31:59 +0000 Subject: [PATCH] Issue #2405091 by marthinal, Berdir, RavindraSingh: Cannot create user entities - {"error":"Access denied on creating field pass"} --- .../Normalizer/ContentEntityNormalizer.php | 7 +- .../modules/hal/src/Tests/DenormalizeTest.php | 2 +- .../Plugin/rest/resource/EntityResource.php | 12 +- core/modules/rest/src/Tests/CreateTest.php | 408 ++++++++++++++---- core/modules/rest/src/Tests/RESTTestBase.php | 28 +- core/modules/user/src/Entity/User.php | 2 +- 6 files changed, 370 insertions(+), 89 deletions(-) diff --git a/core/modules/hal/src/Normalizer/ContentEntityNormalizer.php b/core/modules/hal/src/Normalizer/ContentEntityNormalizer.php index 61ea21409b7..110378feeb1 100644 --- a/core/modules/hal/src/Normalizer/ContentEntityNormalizer.php +++ b/core/modules/hal/src/Normalizer/ContentEntityNormalizer.php @@ -171,11 +171,8 @@ class ContentEntityNormalizer extends NormalizerBase { } } - // Special handling for PATCH: pass the names of the fields whose values - // should be merged. - if (isset($context['request_method']) && $context['request_method'] == 'patch') { - $entity->_restPatchFields = array_keys($data); - } + // Pass the names of the fields whose values can be merged. + $entity->_restSubmittedFields = array_keys($data); // Iterate through remaining items in data array. These should all // correspond to fields. diff --git a/core/modules/hal/src/Tests/DenormalizeTest.php b/core/modules/hal/src/Tests/DenormalizeTest.php index 799585bc6d1..f0ca0d77d40 100644 --- a/core/modules/hal/src/Tests/DenormalizeTest.php +++ b/core/modules/hal/src/Tests/DenormalizeTest.php @@ -197,6 +197,6 @@ class DenormalizeTest extends NormalizerTestBase { // Check that the one field got populated as expected. $this->assertEqual($data['field_test_text'], $denormalized->get('field_test_text')->getValue()); // Check the custom property that contains the list of fields to merge. - $this->assertEqual($denormalized->_restPatchFields, ['field_test_text']); + $this->assertEqual($denormalized->_restSubmittedFields, ['field_test_text']); } } diff --git a/core/modules/rest/src/Plugin/rest/resource/EntityResource.php b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php index ffb15e0d533..1059bd4139e 100644 --- a/core/modules/rest/src/Plugin/rest/resource/EntityResource.php +++ b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php @@ -87,9 +87,13 @@ class EntityResource extends ResourceBase { if (!$entity->isNew()) { throw new BadRequestHttpException('Only new entities can be created'); } - foreach ($entity as $field_name => $field) { - if (!$field->access('create')) { - throw new AccessDeniedHttpException(String::format('Access denied on creating field ', array('@field' => $field_name))); + + // Only check 'edit' permissions for fields that were actually + // submitted by the user. Field access makes no difference between 'create' + // and 'update', so the 'edit' operation is used here. + foreach ($entity->_restSubmittedFields as $key => $field_name) { + if (!$entity->get($field_name)->access('edit')) { + throw new AccessDeniedHttpException(String::format('Access denied on creating field @field', array('@field' => $field_name))); } } @@ -134,7 +138,7 @@ class EntityResource extends ResourceBase { // Overwrite the received properties. $langcode_key = $entity->getEntityType()->getKey('langcode'); - foreach ($entity->_restPatchFields as $field_name) { + 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 // re-initialized. As it must not be empty, skip it if it is. diff --git a/core/modules/rest/src/Tests/CreateTest.php b/core/modules/rest/src/Tests/CreateTest.php index e547c2c2bdf..9a6b66e6483 100644 --- a/core/modules/rest/src/Tests/CreateTest.php +++ b/core/modules/rest/src/Tests/CreateTest.php @@ -8,6 +8,10 @@ namespace Drupal\rest\Tests; use Drupal\Component\Serialization\Json; +use Drupal\Core\Entity\EntityInterface; +use Drupal\entity_test\Entity\EntityTest; +use Drupal\node\Entity\Node; +use Drupal\user\Entity\User; /** * Tests the creation of resources. @@ -24,103 +28,353 @@ class CreateTest extends RESTTestBase { public static $modules = array('hal', 'rest', 'entity_test'); /** - * Tests several valid and invalid create requests on all entity types. + * The 'serializer' service. + * + * @var \Symfony\Component\Serializer\Serializer */ - public function testCreate() { - $serializer = $this->container->get('serializer'); - $entity_types = array('entity_test', 'node'); - foreach ($entity_types as $entity_type) { + protected $serializer; - $this->enableService('entity:' . $entity_type, 'POST'); - // Create a user account that has the required permissions to create - // resources via the REST API. - $permissions = $this->entityPermissions($entity_type, 'create'); - $permissions[] = 'restful post entity:' . $entity_type; - $account = $this->drupalCreateUser($permissions); + protected function setUp() { + parent::setUp(); + // Get the 'serializer' service. + $this->serializer = $this->container->get('serializer'); + } + + /** + * Try to create a resource which is not REST API enabled. + */ + public function testCreateResourceRestApiNotEnabled() { + $entity_type = 'entity_test'; + // Enables the REST service for a specific entity type. + $this->enableService('entity:' . $entity_type, 'POST'); + + // Get the necessary user permissions to create the current entity type. + $permissions = $this->entityPermissions($entity_type, 'create'); + // POST method must be allowed for the current entity type. + $permissions[] = 'restful post entity:' . $entity_type; + + // Create the user. + $account = $this->drupalCreateUser($permissions); + // Populate some entity properties before create the entity. + $entity_values = $this->entityValues($entity_type); + $entity = EntityTest::create($entity_values); + + // Serialize the entity before the POST request. + $serialized = $this->serializer->serialize($entity, $this->defaultFormat, ['account' => $account]); + + // Disable all resource types. + $this->enableService(FALSE); + $this->drupalLogin($account); + + // POST request to create the current entity. GET request for CSRF token + // is included into the httpRequest() method. + $this->httpRequest('entity/entity_test', 'POST', $serialized, $this->defaultMimeType); + + // The resource is not enabled. So, we receive a 'not found' response. + $this->assertResponse(404); + $this->assertFalse(EntityTest::loadMultiple(), 'No entity has been created in the database.'); + } + + /** + * Tests several valid and invalid create requests for 'entity_test' entity type. + */ + public function testCreateEntityTest() { + $entity_type = 'entity_test'; + // Enables the REST service for 'entity_test' entity type. + $this->enableService('entity:' . $entity_type, 'POST'); + // Create two accounts that have the required permissions to create resources. + // The second one has administrative permissions. + $accounts = $this->createAccountPerEntity($entity_type); + + // Verify create requests per user. + foreach ($accounts as $key => $account) { $this->drupalLogin($account); - + // Populate some entity properties before create the entity. $entity_values = $this->entityValues($entity_type); - $entity = entity_create($entity_type, $entity_values); - $serialized = $serializer->serialize($entity, $this->defaultFormat, ['account' => $account]); + $entity = EntityTest::create($entity_values); + + // Serialize the entity before the POST request. + $serialized = $this->serializer->serialize($entity, $this->defaultFormat, ['account' => $account]); + // Create the entity over the REST API. - $this->httpRequest('entity/' . $entity_type, 'POST', $serialized, $this->defaultMimeType); - $this->assertResponse(201); - - // Get the new entity ID from the location header and try to read it from - // the database. - $location_url = $this->drupalGetHeader('location'); - $url_parts = explode('/', $location_url); - $id = end($url_parts); - $loaded_entity = entity_load($entity_type, $id); - $this->assertNotIdentical(FALSE, $loaded_entity, 'The new ' . $entity_type . ' was found in the database.'); - $this->assertEqual($entity->uuid(), $loaded_entity->uuid(), 'UUID of created entity is correct.'); - // @todo Remove the user reference field for now until deserialization for - // entity references is implemented. - unset($entity_values['user_id']); - foreach ($entity_values as $property => $value) { - $actual_value = $loaded_entity->get($property)->value; - $send_value = $entity->get($property)->value; - $this->assertEqual($send_value, $actual_value, 'Created property ' . $property . ' expected: ' . $send_value . ', actual: ' . $actual_value); - } - - $loaded_entity->delete(); + $this->assertCreateEntityOverRestApi($entity_type, $serialized); + // Get the entity ID from the location header and try to read it from the database. + $this->assertReadEntityIdFromHeaderAndDb($entity_type, $entity, $entity_values); // Try to create an entity with an access protected field. // @see entity_test_entity_field_access() - if ($entity_type == 'entity_test') { - $context = ['account' => $account]; - $normalized = $serializer->normalize($entity, $this->defaultFormat, $context); - $normalized['field_test_text'][0]['value'] = 'no access value'; - $this->httpRequest('entity/' . $entity_type, 'POST', $serializer->serialize($normalized, $this->defaultFormat, $context), $this->defaultMimeType); - $this->assertResponse(403); - $this->assertFalse(entity_load_multiple($entity_type, NULL, TRUE), 'No entity has been created in the database.'); + $normalized = $this->serializer->normalize($entity, $this->defaultFormat, ['account' => $account]); + $normalized['field_test_text'][0]['value'] = 'no access value'; + $this->httpRequest('entity/' . $entity_type, 'POST', $this->serializer->serialize($normalized, $this->defaultFormat, ['account' => $account]), $this->defaultMimeType); + $this->assertResponse(403); + $this->assertFalse(EntityTest::loadMultiple(), 'No entity has been created in the database.'); - // Try to create a field with a text format this user has no access to. - $entity->field_test_text->value = $entity_values['field_test_text'][0]['value']; - $entity->field_test_text->format = 'full_html'; - $serialized = $serializer->serialize($entity, $this->defaultFormat, $context); - $this->httpRequest('entity/' . $entity_type, 'POST', $serialized, $this->defaultMimeType); - $this->assertResponse(422); - $this->assertFalse(entity_load_multiple($entity_type, NULL, TRUE), 'No entity has been created in the database.'); + // Try to create a field with a text format this user has no access to. + $entity->field_test_text->value = $entity_values['field_test_text'][0]['value']; + $entity->field_test_text->format = 'full_html'; - // Restore the valid test value. - $entity->field_test_text->format = 'plain_text'; - $serialized = $serializer->serialize($entity, $this->defaultFormat, $context); - } + $serialized = $this->serializer->serialize($entity, $this->defaultFormat, ['account' => $account]); + $this->httpRequest('entity/' . $entity_type, 'POST', $serialized, $this->defaultMimeType); + // The value selected is not a valid choice because the format must be 'plain_txt'. + $this->assertResponse(422); + $this->assertFalse(EntityTest::loadMultiple(), 'No entity has been created in the database.'); + + // Restore the valid test value. + $entity->field_test_text->format = 'plain_text'; + $serialized = $this->serializer->serialize($entity, $this->defaultFormat, ['account' => $account]); // Try to send invalid data that cannot be correctly deserialized. - $this->httpRequest('entity/' . $entity_type, 'POST', 'kaboom!', $this->defaultMimeType); - $this->assertResponse(400); + $this->assertCreateEntityInvalidData($entity_type); // Try to send no data at all, which does not make sense on POST requests. - $this->httpRequest('entity/' . $entity_type, 'POST', NULL, $this->defaultMimeType); - $this->assertResponse(400); + $this->assertCreateEntityNoData($entity_type); + + // Try to send invalid data to trigger the entity validation constraints. Send a UUID that is too long. + $this->assertCreateEntityInvalidSerialized($entity, $entity_type); + + // Try to create an entity without proper permissions. + $this->assertCreateEntityWithoutProperPermissions($entity_type, $serialized, ['account' => $account]); + + } + + } + + /** + * Tests several valid and invalid create requests for 'node' entity type. + */ + public function testCreateNode() { + $entity_type = 'node'; + // Enables the REST service for 'node' entity type. + $this->enableService('entity:' . $entity_type, 'POST'); + // Create two accounts that have the required permissions to create resources. + // The second one has administrative permissions. + $accounts = $this->createAccountPerEntity($entity_type); + + // Verify create requests per user. + foreach ($accounts as $key => $account) { + $this->drupalLogin($account); + // Populate some entity properties before create the entity. + $entity_values = $this->entityValues($entity_type); + $entity = Node::create($entity_values); + + // Verify that user cannot create content when trying to write to fields where it is not possible. + if (!$account->hasPermission('administer nodes')) { + $serialized = $this->serializer->serialize($entity, $this->defaultFormat, ['account' => $account]); + $this->httpRequest('entity/' . $entity_type, 'POST', $serialized, $this->defaultMimeType); + $this->assertResponse(403); + // Remove fields where non-administrative users cannot write. + $entity = $this->removeNodeFieldsForNonAdminUsers($entity); + } + else { + // Changed and revision_timestamp fields can never be added. + unset($entity->changed); + unset($entity->revision_timestamp); + } + + $serialized = $this->serializer->serialize($entity, $this->defaultFormat, ['account' => $account]); + + // Create the entity over the REST API. + $this->assertCreateEntityOverRestApi($entity_type, $serialized); + + // Get the new entity ID from the location header and try to read it from the database. + $this->assertReadEntityIdFromHeaderAndDb($entity_type, $entity, $entity_values); + + // Try to send invalid data that cannot be correctly deserialized. + $this->assertCreateEntityInvalidData($entity_type); + + // Try to send no data at all, which does not make sense on POST requests. + $this->assertCreateEntityNoData($entity_type); + + // Try to send invalid data to trigger the entity validation constraints. Send a UUID that is too long. + $this->assertCreateEntityInvalidSerialized($entity, $entity_type); + + // Try to create an entity without proper permissions. + $this->assertCreateEntityWithoutProperPermissions($entity_type, $serialized); + + } + + } + + /** + * Tests several valid and invalid create requests for 'user' entity type. + */ + public function testCreateUser() { + $entity_type = 'user'; + // Enables the REST service for 'user' entity type. + $this->enableService('entity:' . $entity_type, 'POST'); + // Create two accounts that have the required permissions to create resources. + // The second one has administrative permissions. + $accounts = $this->createAccountPerEntity($entity_type); + + foreach ($accounts as $key => $account) { + $this->drupalLogin($account); + $entity_values = $this->entityValues($entity_type); + $entity = User::create($entity_values); + + // Verify that only administrative users can create users. + if (!$account->hasPermission('administer users')) { + $serialized = $this->serializer->serialize($entity, $this->defaultFormat, ['account' => $account]); + $this->httpRequest('entity/' . $entity_type, 'POST', $serialized, $this->defaultMimeType); + $this->assertResponse(403); + continue; + } + + // Changed field can never be added. + unset($entity->changed); + + $serialized = $this->serializer->serialize($entity, $this->defaultFormat, ['account' => $account]); + + // Create the entity over the REST API. + $this->assertCreateEntityOverRestApi($entity_type, $serialized); + + // Get the new entity ID from the location header and try to read it from the database. + $this->assertReadEntityIdFromHeaderAndDb($entity_type, $entity, $entity_values); + + // Try to send invalid data that cannot be correctly deserialized. + $this->assertCreateEntityInvalidData($entity_type); + + // Try to send no data at all, which does not make sense on POST requests. + $this->assertCreateEntityNoData($entity_type); // Try to send invalid data to trigger the entity validation constraints. // Send a UUID that is too long. - $entity->set('uuid', $this->randomMachineName(129)); - $invalid_serialized = $serializer->serialize($entity, $this->defaultFormat, $context); - $response = $this->httpRequest('entity/' . $entity_type, 'POST', $invalid_serialized, $this->defaultMimeType); - $this->assertResponse(422); - $error = Json::decode($response); - $this->assertEqual($error['error'], "Unprocessable Entity: validation failed.\nuuid.0.value: UUID: may not be longer than 128 characters.\n"); - - // Try to create an entity without proper permissions. - $this->drupalLogout(); - $this->httpRequest('entity/' . $entity_type, 'POST', $serialized, $this->defaultMimeType); - $this->assertResponse(403); - $this->assertFalse(entity_load_multiple($entity_type, NULL, TRUE), 'No entity has been created in the database.'); + $this->assertCreateEntityInvalidSerialized($entity, $entity_type); } - // Try to create a resource which is not REST API enabled. - $this->enableService(FALSE); - $this->drupalLogin($account); - $this->httpRequest('entity/entity_test', 'POST', $serialized, $this->defaultMimeType); - $this->assertResponse(404); - $this->assertFalse(entity_load_multiple($entity_type, NULL, TRUE), 'No entity has been created in the database.'); + } - // @todo Add a security test. It should not be possible for example to - // create a test entity on a node resource route. + /** + * Creates user accounts that have the required permissions to create resources via the REST API. + * The second one has administrative permissions. + * + * @param string $entity_type + * Entity type needed to apply user permissions. + * @return array + * An array that contains user accounts. + */ + public function createAccountPerEntity($entity_type) { + $accounts = array(); + // Get the necessary user permissions for the current $entity_type creation. + $permissions = $this->entityPermissions($entity_type, 'create'); + // POST method must be allowed for the current entity type. + $permissions[] = 'restful post entity:' . $entity_type; + // Create user without administrative permissions. + $accounts[] = $this->drupalCreateUser($permissions); + // Add administrative permissions for nodes and users. + $permissions[] = 'administer nodes'; + $permissions[] = 'administer users'; + // Create an administrative user. + $accounts[] = $this->drupalCreateUser($permissions); + + return $accounts; + } + + /** + * Creates the entity over the REST API. + * + * @param string $entity_type + * The type of the entity that should be created. + * @param string $serialized + * The body for the POST request. + */ + public function assertCreateEntityOverRestApi($entity_type, $serialized = NULL) { + $this->httpRequest('entity/' . $entity_type, 'POST', $serialized, $this->defaultMimeType); + $this->assertResponse(201); + } + + /** + * Get the new entity ID from the location header and try to read it from the database. + * + * @param string $entity_type + * Entity type we need to load the entity from DB. + * @param \Drupal\Core\Entity\EntityInterface $entity + * The entity we want to check that was inserted correctly. + * @param array $entity_values + * The values of $entity. + */ + public function assertReadEntityIdFromHeaderAndDb($entity_type, EntityInterface $entity, array $entity_values = array()) { + // Get the location from the HTTP response header. + $location_url = $this->drupalGetHeader('location'); + $url_parts = explode('/', $location_url); + $id = end($url_parts); + + // Get the entity using the ID found. + $loaded_entity = \Drupal::entityManager()->getStorage($entity_type)->load($id); + $this->assertNotIdentical(FALSE, $loaded_entity, 'The new ' . $entity_type . ' was found in the database.'); + $this->assertEqual($entity->uuid(), $loaded_entity->uuid(), 'UUID of created entity is correct.'); + + // Verify that the field values sent and received from DB are the same. + foreach ($entity_values as $property => $value) { + $actual_value = $loaded_entity->get($property)->value; + $send_value = $entity->get($property)->value; + $this->assertEqual($send_value, $actual_value, 'Created property ' . $property . ' expected: ' . $send_value . ', actual: ' . $actual_value); + } + + // Delete the entity loaded from DB. + $loaded_entity->delete(); + } + + /** + * Try to send invalid data that cannot be correctly deserialized. + * + * @param string $entity_type + * The type of the entity that should be created. + */ + public function assertCreateEntityInvalidData($entity_type) { + $this->httpRequest('entity/' . $entity_type, 'POST', 'kaboom!', $this->defaultMimeType); + $this->assertResponse(400); + } + + /** + * Try to send no data at all, which does not make sense on POST requests. + * + * @param string $entity_type + * The type of the entity that should be created. + */ + public function assertCreateEntityNoData($entity_type) { + $this->httpRequest('entity/' . $entity_type, 'POST', NULL, $this->defaultMimeType); + $this->assertResponse(400); + } + + /** + * Send an invalid UUID to trigger the entity validation. + * + * @param \Drupal\Core\Entity\EntityInterface $entity + * The entity we want to check that was inserted correctly. + * @param string $entity_type + * The type of the entity that should be created. + * @param array $context + * Options normalizers/encoders have access to. + */ + public function assertCreateEntityInvalidSerialized(EntityInterface $entity, $entity_type, array $context = array()) { + // Add a UUID that is too long. + $entity->set('uuid', $this->randomMachineName(129)); + $invalid_serialized = $this->serializer->serialize($entity, $this->defaultFormat, $context); + + $response = $this->httpRequest('entity/' . $entity_type, 'POST', $invalid_serialized, $this->defaultMimeType); + + // Unprocessable Entity as response. + $this->assertResponse(422); + + // Verify that the text of the response is correct. + $error = Json::decode($response); + $this->assertEqual($error['error'], "Unprocessable Entity: validation failed.\nuuid.0.value: UUID: may not be longer than 128 characters.\n"); + } + + /** + * Try to create an entity without proper permissions. + * + * @param string $entity_type + * The type of the entity that should be created. + * @param string $serialized + * The body for the POST request. + */ + public function assertCreateEntityWithoutProperPermissions($entity_type, $serialized = NULL) { + $this->drupalLogout(); + $this->httpRequest('entity/' . $entity_type, 'POST', $serialized, $this->defaultMimeType); + // Forbidden Error as response. + $this->assertResponse(403); + $this->assertFalse(\Drupal::entityManager()->getStorage($entity_type)->loadMultiple(), 'No entity has been created in the database.'); } } diff --git a/core/modules/rest/src/Tests/RESTTestBase.php b/core/modules/rest/src/Tests/RESTTestBase.php index 6b0597355ee..4856bde2a97 100644 --- a/core/modules/rest/src/Tests/RESTTestBase.php +++ b/core/modules/rest/src/Tests/RESTTestBase.php @@ -9,7 +9,9 @@ namespace Drupal\rest\Tests; use Drupal\Core\Session\AccountInterface; use Drupal\Core\Url; +use Drupal\node\NodeInterface; use Drupal\simpletest\WebTestBase; +use Drupal\user\UserInterface; /** * Test helper class that provides a REST client method to send HTTP requests. @@ -67,10 +69,13 @@ abstract class RESTTestBase extends WebTestBase { * A Url object or system path. * @param string $method * HTTP method, one of GET, POST, PUT or DELETE. - * @param array $body + * @param string $body * The body for POST and PUT. * @param string $mime_type * The MIME type of the transmitted content. + * + * @return string + * The content returned from the request. */ protected function httpRequest($url, $method, $body = NULL, $mime_type = NULL) { if (!isset($mime_type)) { @@ -345,4 +350,25 @@ abstract class RESTTestBase extends WebTestBase { return entity_load($this->testEntityType, $id); } + /** + * Remove node fields that can only be written by an admin user. + * + * @param \Drupal\node\NodeInterface $node + * The node to remove fields where non-administrative users cannot write. + * + * @return \Drupal\node\NodeInterface + * The node with removed fields. + */ + protected function removeNodeFieldsForNonAdminUsers(NodeInterface $node) { + $node->set('status', NULL); + $node->set('created', NULL); + $node->set('changed', NULL); + $node->set('promote', NULL); + $node->set('sticky', NULL); + $node->set('revision_timestamp', NULL); + $node->set('uid', NULL); + + return $node; + } + } diff --git a/core/modules/user/src/Entity/User.php b/core/modules/user/src/Entity/User.php index 22303e5a3fe..5cb964ef78a 100644 --- a/core/modules/user/src/Entity/User.php +++ b/core/modules/user/src/Entity/User.php @@ -40,7 +40,7 @@ use Drupal\user\UserInterface; * }, * "translation" = "Drupal\user\ProfileTranslationHandler" * }, - * admin_permission = "administer user", + * admin_permission = "administer users", * base_table = "users", * data_table = "users_field_data", * label_callback = "user_format_name",