Issue #2430669 by larowlan, marthinal, clemens.tolboom, jibran: Cannot create comments from REST - field access is too strict

core/modules/comment/src/CommentAccessControlHandler.php

@@ -80,15 +80,26 @@ class CommentAccessControlHandler extends EntityAccessControlHandler {
       // No user can change read-only fields.
       $read_only_fields = array(
         'hostname',
-        'uuid',
+        'changed',
         'cid',
         'thread',
+      );
+      // These fields can be edited during comment creation.
+      $create_only_fields = [
         'comment_type',
-        'pid',
+        'uuid',
         'entity_id',
         'entity_type',
         'field_name',
-      );
+        'pid',
+      ];
+      if ($items && ($entity = $items->getEntity()) && $entity->isNew() && in_array($field_definition->getName(), $create_only_fields, TRUE)) {
+        // We are creating a new comment, user can edit create only fields.
+        return AccessResult::allowedIfHasPermission($account, 'post comments')->addCacheableDependency($entity);
+      }
+      // We are editing an existing comment - create only fields are now read
+      // only.
+      $read_only_fields = array_merge($read_only_fields, $create_only_fields);
       if (in_array($field_definition->getName(), $read_only_fields, TRUE)) {
         return AccessResult::forbidden();
       }

core/modules/comment/src/Tests/CommentFieldAccessTest.php

@@ -53,15 +53,23 @@ class CommentFieldAccessTest extends EntityUnitTestBase {
   protected $readOnlyFields = array(
     'changed',
     'hostname',
-    'uuid',
     'cid',
     'thread',
-    'comment_type',
+  );
+
+  /**
+   * These fields can be edited on create only.
+   *
+   * @var array
+   */
+  protected $createOnlyFields = [
+    'uuid',
     'pid',
+    'comment_type',
     'entity_id',
     'entity_type',
     'field_name',
-  );
+  ];
 
   /**
    * These fields can only be edited by the admin or anonymous users if allowed.
@@ -252,6 +260,28 @@ class CommentFieldAccessTest extends EntityUnitTestBase {
       }
     }
 
+    // Check create-only fields.
+    foreach ($this->createOnlyFields as $field) {
+      // Check view operation.
+      foreach ($permutations as $set) {
+        $may_view = $set['comment']->{$field}->access('view', $set['user']);
+        $may_update = $set['comment']->{$field}->access('edit', $set['user']);
+        $this->assertEqual($may_view, $field != 'hostname' && ($set['user']->hasPermission('administer comments') ||
+            ($set['comment']->isPublished() && $set['user']->hasPermission('access comments'))), SafeMarkup::format('User @user !state view field !field on comment @comment', [
+          '@user' => $set['user']->getUsername(),
+          '!state' => $may_view ? 'can' : 'cannot',
+          '@comment' => $set['comment']->getSubject(),
+          '!field' => $field,
+        ]));
+        $this->assertEqual($may_update, $set['user']->hasPermission('post comments') && $set['comment']->isNew(), SafeMarkup::format('User @user !state update field !field on comment @comment', [
+          '@user' => $set['user']->getUsername(),
+          '!state' => $may_update ? 'can' : 'cannot',
+          '@comment' => $set['comment']->getSubject(),
+          '!field' => $field,
+        ]));
+      }
+    }
+
     // Check contact fields.
     foreach ($this->contactFields as $field) {
       // Check view operation.

core/modules/rest/src/Tests/CreateTest.php

@@ -7,11 +7,13 @@
 
 namespace Drupal\rest\Tests;
 
+use Drupal\comment\Tests\CommentTestTrait;
 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;
+use Drupal\comment\Entity\Comment;
 
 /**
  * Tests the creation of resources.
@@ -20,12 +22,13 @@ use Drupal\user\Entity\User;
  */
 class CreateTest extends RESTTestBase {
 
+  use CommentTestTrait;
   /**
    * Modules to install.
    *
    * @var array
    */
-  public static $modules = array('hal', 'rest', 'entity_test');
+  public static $modules = array('hal', 'rest', 'entity_test', 'comment');
 
   /**
    * The 'serializer' service.
@@ -36,6 +39,7 @@ class CreateTest extends RESTTestBase {
 
   protected function setUp() {
     parent::setUp();
+    $this->addDefaultCommentField('node', 'resttest');
     // Get the 'serializer' service.
     $this->serializer = $this->container->get('serializer');
   }
@@ -224,6 +228,52 @@ class CreateTest extends RESTTestBase {
 
   }
 
+  /**
+   * Test comment creation.
+   */
+  protected function testCreateComment() {
+    $node = Node::create([
+      'type' => 'resttest',
+      'title' => 'some node',
+    ]);
+    $node->save();
+    $entity_type = 'comment';
+    // Enable the REST service for 'comment' 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);
+    $account = end($accounts);
+
+    $this->drupalLogin($account);
+    $entity_values = $this->entityValues($entity_type);
+    $entity_values['entity_id'] = $node->id();
+
+    $entity = Comment::create($entity_values);
+
+    // 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.
+    $this->assertCreateEntityInvalidSerialized($entity, $entity_type);
+  }
+
   /**
    * Tests several valid and invalid create requests for 'user' entity type.
    */
@@ -293,6 +343,7 @@ class CreateTest extends RESTTestBase {
     // Add administrative permissions for nodes and users.
     $permissions[] = 'administer nodes';
     $permissions[] = 'administer users';
+    $permissions[] = 'administer comments';
     // Create an administrative user.
     $accounts[] = $this->drupalCreateUser($permissions);
 

core/modules/rest/src/Tests/RESTTestBase.php

@@ -213,6 +213,17 @@ abstract class RESTTestBase extends WebTestBase {
         );
       case 'user':
         return array('name' => $this->randomMachineName());
+
+      case 'comment':
+        return [
+          'subject' => $this->randomMachineName(),
+          'entity_type' => 'node',
+          'comment_type' => 'comment',
+          'comment_body' => $this->randomString(),
+          'entity_id' => 'invalid',
+          'field_name' => 'comment',
+        ];
+
       default:
         return array();
     }
@@ -313,6 +324,22 @@ abstract class RESTTestBase extends WebTestBase {
             return array('delete any resttest content');
         }
 
+      case 'comment':
+        switch ($operation) {
+          case 'view':
+            return ['access comments'];
+
+          case 'create':
+            return ['post comments', 'skip comment approval'];
+
+          case 'update':
+            return ['edit own comments'];
+
+          case 'delete':
+            return ['administer comments'];
+        }
+        break;
+
       case 'user':
         switch ($operation) {
           case 'view':