Issue #2843139 by vaplas, Wim Leers, harings_rob, catch, Berdir, alexpott:...

Issue #2843139 by vaplas, Wim Leers, harings_rob, catch, Berdir, alexpott: EntityResource: Provide comprehensive test coverage for File entity, and tighten access control handler

core/modules/field/src/Tests/String/StringFieldTest.php

@@ -32,7 +32,7 @@ class StringFieldTest extends WebTestBase {
   protected function setUp() {
     parent::setUp();
 
-    $this->webUser = $this->drupalCreateUser(['view test entity', 'administer entity_test content']);
+    $this->webUser = $this->drupalCreateUser(['view test entity', 'administer entity_test content', 'access content']);
     $this->drupalLogin($this->webUser);
   }
 

core/modules/file/src/FileAccessControlHandler.php

@@ -22,8 +22,12 @@ protected function checkAccess(EntityInterface $entity, $operation, AccountInter
     /** @var \Drupal\file\FileInterface $entity */
     if ($operation == 'download' || $operation == 'view') {
       if (\Drupal::service('file_system')->uriScheme($entity->getFileUri()) === 'public') {
-        // Always allow access to file in public file system.
-        return AccessResult::allowed();
+        if ($operation === 'download') {
+          return AccessResult::allowed();
+        }
+        else {
+          return AccessResult::allowedIfHasPermission($account, 'access content');
+        }
       }
       elseif ($references = $this->getFileReferences($entity)) {
         foreach ($references as $field_name => $entity_map) {

core/modules/file/tests/src/Functional/FileManagedAccessTest.php

@@ -3,6 +3,7 @@
 namespace Drupal\Tests\file\Functional;
 
 use Drupal\file\Entity\File;
+use Drupal\user\Entity\Role;
 
 /**
  * Tests access to managed files.
@@ -11,6 +12,19 @@
  */
 class FileManagedAccessTest extends FileManagedTestBase {
 
+  /**
+   * {@inheritdoc}
+   */
+  protected function setUp() {
+    parent::setUp();
+
+    // Give anonymous users permission to access content, so they can view and
+    // download public files.
+    $anonymous_role = Role::load(Role::ANONYMOUS_ID);
+    $anonymous_role->grantPermission('access content');
+    $anonymous_role->save();
+  }
+
   /**
    * Tests if public file is always accessible.
    */
@@ -29,7 +43,7 @@ public function testFileAccess() {
     $file->save();
 
     // Create authenticated user to check file access.
-    $account = $this->createUser(['access site reports']);
+    $account = $this->createUser(['access site reports', 'access content']);
 
     $this->assertTrue($file->access('view', $account), 'Public file is viewable to authenticated user');
     $this->assertTrue($file->access('download', $account), 'Public file is downloadable to authenticated user');
@@ -54,7 +68,7 @@ public function testFileAccess() {
     $file->save();
 
     // Create authenticated user to check file access.
-    $account = $this->createUser(['access site reports']);
+    $account = $this->createUser(['access site reports', 'access content']);
 
     $this->assertFalse($file->access('view', $account), 'Private file is not viewable to authenticated user');
     $this->assertFalse($file->access('download', $account), 'Private file is not downloadable to authenticated user');

core/modules/file/tests/src/Kernel/FileItemTest.php

@@ -10,6 +10,7 @@
 use Drupal\Tests\field\Kernel\FieldKernelTestBase;
 use Drupal\field\Entity\FieldStorageConfig;
 use Drupal\file\Entity\File;
+use Drupal\user\Entity\Role;
 
 /**
  * Tests using entity fields of the file field type.
@@ -42,6 +43,14 @@ class FileItemTest extends FieldKernelTestBase {
   protected function setUp() {
     parent::setUp();
 
+    $this->installEntitySchema('user');
+    $this->installConfig(['user']);
+    // Give anonymous users permission to access content, so they can view and
+    // download public files.
+    $anonymous_role = Role::load(Role::ANONYMOUS_ID);
+    $anonymous_role->grantPermission('access content');
+    $anonymous_role->save();
+
     $this->installEntitySchema('file');
     $this->installSchema('file', ['file_usage']);
 

core/modules/hal/tests/src/Functional/EntityResource/File/FileHalJsonAnonTest.php

+<?php
+
+namespace Drupal\Tests\hal\Functional\EntityResource\File;
+
+use Drupal\Tests\hal\Functional\EntityResource\HalEntityNormalizationTrait;
+use Drupal\Tests\rest\Functional\AnonResourceTestTrait;
+use Drupal\Tests\rest\Functional\EntityResource\File\FileResourceTestBase;
+
+/**
+ * @group hal
+ */
+class FileHalJsonAnonTest extends FileResourceTestBase {
+
+  use HalEntityNormalizationTrait;
+  use AnonResourceTestTrait;
+
+  /**
+   * {@inheritdoc}
+   */
+  public static $modules = ['hal'];
+
+  /**
+   * {@inheritdoc}
+   */
+  protected static $format = 'hal_json';
+
+  /**
+   * {@inheritdoc}
+   */
+  protected static $mimeType = 'application/hal+json';
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function getExpectedNormalizedEntity() {
+    $default_normalization = parent::getExpectedNormalizedEntity();
+
+    $normalization = $this->applyHalFieldNormalization($default_normalization);
+
+    $url = file_create_url($this->entity->getFileUri());
+    $normalization['uri'][0]['value'] = $url;
+    $uid = $this->author->id();
+
+    return $normalization + [
+      '_embedded' => [
+        $this->baseUrl . '/rest/relation/file/file/uid' => [
+          [
+            '_links' => [
+              'self' => [
+                'href' => $this->baseUrl . "/user/$uid?_format=hal_json",
+              ],
+              'type' => [
+                'href' => $this->baseUrl . '/rest/type/user/user',
+              ],
+            ],
+            'uuid' => [
+              [
+                'value' => $this->author->uuid(),
+              ],
+            ],
+          ],
+        ],
+      ],
+      '_links' => [
+        'self' => [
+          'href' => $url,
+        ],
+        'type' => [
+          'href' => $this->baseUrl . '/rest/type/file/file',
+        ],
+        $this->baseUrl . '/rest/relation/file/file/uid' => [
+          [
+            'href' => $this->baseUrl . "/user/$uid?_format=hal_json",
+          ],
+        ],
+      ],
+    ];
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function getNormalizedPostEntity() {
+    return parent::getNormalizedPostEntity() + [
+      '_links' => [
+        'type' => [
+          'href' => $this->baseUrl . '/rest/type/file/file',
+        ],
+      ],
+    ];
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function getExpectedCacheContexts() {
+    return [
+      'url.site',
+      'user.permissions',
+    ];
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function testPatch() {
+    // @todo https://www.drupal.org/node/1927648
+    $this->markTestSkipped();
+  }
+
+}

core/modules/hal/tests/src/Functional/EntityResource/File/FileHalJsonBasicAuthTest.php

+<?php
+
+namespace Drupal\Tests\hal\Functional\EntityResource\File;
+
+use Drupal\Tests\rest\Functional\BasicAuthResourceTestTrait;
+
+/**
+ * @group hal
+ */
+class FileHalJsonBasicAuthTest extends FileHalJsonAnonTest {
+
+  use BasicAuthResourceTestTrait;
+
+  /**
+   * {@inheritdoc}
+   */
+  public static $modules = ['basic_auth'];
+
+  /**
+   * {@inheritdoc}
+   */
+  protected static $auth = 'basic_auth';
+
+}

core/modules/hal/tests/src/Functional/EntityResource/File/FileHalJsonCookieTest.php

+<?php
+
+namespace Drupal\Tests\hal\Functional\EntityResource\File;
+
+use Drupal\Tests\rest\Functional\CookieResourceTestTrait;
+
+/**
+ * @group hal
+ */
+class FileHalJsonCookieTest extends FileHalJsonAnonTest {
+
+  use CookieResourceTestTrait;
+
+  /**
+   * {@inheritdoc}
+   */
+  protected static $auth = 'cookie';
+
+}

core/modules/image/tests/src/Kernel/ImageItemTest.php

@@ -11,6 +11,7 @@
 use Drupal\Tests\field\Kernel\FieldKernelTestBase;
 use Drupal\field\Entity\FieldStorageConfig;
 use Drupal\file\Entity\File;
+use Drupal\user\Entity\Role;
 
 /**
  * Tests using entity fields of the image field type.
@@ -41,6 +42,14 @@ class ImageItemTest extends FieldKernelTestBase {
   protected function setUp() {
     parent::setUp();
 
+    $this->installEntitySchema('user');
+    $this->installConfig(['user']);
+    // Give anonymous users permission to access content, so that we can view
+    // and download public file.
+    $anonymous_role = Role::load(Role::ANONYMOUS_ID);
+    $anonymous_role->grantPermission('access content');
+    $anonymous_role->save();
+
     $this->installEntitySchema('file');
     $this->installSchema('file', ['file_usage']);
 

core/modules/rest/tests/src/Functional/EntityResource/EntityResourceRestTestCoverageTest.php

@@ -124,15 +124,6 @@ public function testEntityTypeRestTestCoverage() {
     }
     $all = count($this->definitions);
     $good = $all - count($problems);
-    // @todo Remove this in https://www.drupal.org/node/2843139. Having this
-    // work-around in here until then means we can ensure we don't add more
-    // entity types without adding REST test coverage.
-    if ($problems === ['file: File (Drupal\file\Entity\File), default normalization (expected tests: FileJsonAnonTest, FileJsonBasicAuthTest, FileJsonCookieTest, FileXmlAnonTest, FileXmlBasicAuthTest, FileXmlCookieTest)', 'file: File (Drupal\file\Entity\File), hal normalization (expected tests: FileHalJsonAnonTest, FileHalJsonBasicAuthTest, FileHalJsonCookieTest)']) {
-      $problems = [];
-    }
-    elseif ($problems === []) {
-      $this->fail('Drupal\file\Entity\File now supports REST test coverage. The work-around for it to pass until that test coverage was added should now be removed.');
-    }
     $this->assertSame([], $problems, $this->getLlamaMessage($good, $all));
   }
 

core/modules/rest/tests/src/Functional/EntityResource/File/FileJsonAnonTest.php

+<?php
+
+namespace Drupal\Tests\rest\Functional\EntityResource\File;
+
+use Drupal\Tests\rest\Functional\AnonResourceTestTrait;
+
+/**
+ * @group rest
+ */
+class FileJsonAnonTest extends FileResourceTestBase {
+
+  use AnonResourceTestTrait;
+
+  /**
+   * {@inheritdoc}
+   */
+  protected static $format = 'json';
+
+  /**
+   * {@inheritdoc}
+   */
+  protected static $mimeType = 'application/json';
+
+}

core/modules/rest/tests/src/Functional/EntityResource/File/FileJsonBasicAuthTest.php

+<?php
+
+namespace Drupal\Tests\rest\Functional\EntityResource\File;
+
+use Drupal\Tests\rest\Functional\BasicAuthResourceTestTrait;
+
+/**
+ * @group rest
+ */
+class FileJsonBasicAuthTest extends FileResourceTestBase {
+
+  use BasicAuthResourceTestTrait;
+
+  /**
+   * {@inheritdoc}
+   */
+  public static $modules = ['basic_auth'];
+
+  /**
+   * {@inheritdoc}
+   */
+  protected static $format = 'json';
+
+  /**
+   * {@inheritdoc}
+   */
+  protected static $mimeType = 'application/json';
+
+  /**
+   * {@inheritdoc}
+   */
+  protected static $auth = 'basic_auth';
+
+}

core/modules/rest/tests/src/Functional/EntityResource/File/FileJsonCookieTest.php

+<?php
+
+namespace Drupal\Tests\rest\Functional\EntityResource\File;
+
+use Drupal\Tests\rest\Functional\CookieResourceTestTrait;
+
+/**
+ * @group rest
+ */
+class FileJsonCookieTest extends FileResourceTestBase {
+
+  use CookieResourceTestTrait;
+
+  /**
+   * {@inheritdoc}
+   */
+  protected static $format = 'json';
+
+  /**
+   * {@inheritdoc}
+   */
+  protected static $mimeType = 'application/json';
+
+  /**
+   * {@inheritdoc}
+   */
+  protected static $auth = 'cookie';
+
+}

core/modules/rest/tests/src/Functional/EntityResource/File/FileResourceTestBase.php

+<?php
+
+namespace Drupal\Tests\rest\Functional\EntityResource\File;
+
+use Drupal\file\Entity\File;
+use Drupal\Tests\rest\Functional\BcTimestampNormalizerUnixTestTrait;
+use Drupal\Tests\rest\Functional\EntityResource\EntityResourceTestBase;
+use Drupal\user\Entity\User;
+
+abstract class FileResourceTestBase extends EntityResourceTestBase {
+
+  use BcTimestampNormalizerUnixTestTrait;
+
+  /**
+   * {@inheritdoc}
+   */
+  public static $modules = ['file', 'user'];
+
+  /**
+   * {@inheritdoc}
+   */
+  protected static $entityTypeId = 'file';
+
+  /**
+   * @var \Drupal\file\FileInterface
+   */
+  protected $entity;
+
+  /**
+   * {@inheritdoc}
+   */
+  protected static $patchProtectedFieldNames = [
+    'uri',
+    'filemime',
+    'filesize',
+    'status',
+    'changed',
+  ];
+
+  /**
+   * @var \Drupal\user\UserInterface
+   */
+  protected $author;
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setUpAuthorization($method) {
+    switch ($method) {
+      case 'GET':
+        $this->grantPermissionsToTestedRole(['access content']);
+        break;
+
+      case 'PATCH':
+      case 'DELETE':
+        // \Drupal\file\FileAccessControlHandler::checkAccess() grants 'update'
+        // and 'delete' access only to the user that owns the file. So there is
+        // no permission to grant: instead, the file owner must be changed from
+        // its default (user 1) to the current user.
+        $this->makeCurrentUserFileOwner();
+        break;
+    }
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function grantPermissionsToTestedRole(array $permissions) {
+    // testPatch() and testDelete() test the 'bc_entity_resource_permissions' BC
+    // layer; also call makeCurrentUserFileOwner() then.
+    if ($permissions === ['restful patch entity:file'] || $permissions === ['restful delete entity:file']) {
+      $this->makeCurrentUserFileOwner();
+    }
+    parent::grantPermissionsToTestedRole($permissions);
+  }
+
+  /**
+   * Makes the current user the file owner.
+   */
+  protected function makeCurrentUserFileOwner() {
+    $account = static::$auth ? User::load(2) : User::load(0);
+    $this->entity->setOwnerId($account->id());
+    $this->entity->setOwner($account);
+    $this->entity->save();
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function createEntity() {
+    $this->author = User::load(1);
+
+    $file = File::create();
+    $file->setOwnerId($this->author->id());
+    $file->setFilename('drupal.txt');
+    $file->setMimeType('text/plain');
+    $file->setFileUri('public://drupal.txt');
+    $file->set('status', FILE_STATUS_PERMANENT);
+    $file->save();
+
+    file_put_contents($file->getFileUri(), 'Drupal');
+
+    return $file;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function getExpectedNormalizedEntity() {
+    return [
+      'changed' => [
+        $this->formatExpectedTimestampItemValues($this->entity->getChangedTime()),
+      ],
+      'created' => [
+        $this->formatExpectedTimestampItemValues((int) $this->entity->getCreatedTime()),
+      ],
+      'fid' => [
+        [
+          'value' => 1,
+        ],
+      ],
+      'filemime' => [
+        [
+          'value' => 'text/plain',
+        ],
+      ],
+      'filename' => [
+        [
+          'value' => 'drupal.txt',
+        ],
+      ],
+      'filesize' => [
+        [
+          'value' => (int) $this->entity->getSize(),
+        ],
+      ],
+      'langcode' => [
+        [
+          'value' => 'en',
+        ],
+      ],
+      'status' => [
+        [
+          'value' => TRUE,
+        ],
+      ],
+      'uid' => [
+        [
+          'target_id' => (int) $this->author->id(),
+          'target_type' => 'user',
+          'target_uuid' => $this->author->uuid(),
+          'url' => base_path() . 'user/' . $this->author->id(),
+        ],
+      ],
+      'uri' => [
+        [
+          'value' => 'public://drupal.txt',
+        ],
+      ],
+      'uuid' => [
+        [
+          'value' => $this->entity->uuid(),
+        ],
+      ],
+    ];
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function getNormalizedPostEntity() {
+    return [
+      'uid' => [
+        [
+          'target_id' => $this->author->id(),
+        ],
+      ],
+      'filename' => [
+        [
+          'value' => 'drupal.txt',
+        ],
+      ],
+    ];
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function getNormalizedPatchEntity() {
+    return array_diff_key($this->getNormalizedPostEntity(), ['uid' => TRUE]);
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function getExpectedCacheContexts() {
+    return [
+      'user.permissions',
+    ];
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function testPost() {
+    // @todo https://www.drupal.org/node/1927648
+    $this->markTestSkipped();
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function getExpectedUnauthorizedAccessMessage($method) {
+    if ($this->config('rest.settings')->get('bc_entity_resource_permissions')) {
+      return parent::getExpectedUnauthorizedAccessMessage($method);
+    }
+
+    if ($method === 'GET') {
+      return "The 'access content' permission is required.";
+    }
+    if ($method === 'PATCH') {
+      return 'You are not authorized to update this file entity.';
+    }
+    return parent::getExpectedUnauthorizedAccessMessage($method);
+  }
+
+}

core/modules/rest/tests/src/Functional/EntityResource/File/FileXmlAnonTest.php

+<?php
+
+namespace Drupal\Tests\rest\Functional\EntityResource\File;
+