$entity->access('view|download') for unattached public files returns FALSE

Updated the comment and fixed the return value.

Added test.

1. +++ b/core/modules/file/src/Tests/AccessTest.php
   @@ -0,0 +1,46 @@
   +class AccessTest extends FileManagedTestBase {
   

   Can you try to use FileManagedUnitTestBase?

2. +++ b/core/modules/file/src/Tests/AccessTest.php
   @@ -0,0 +1,46 @@
   +  function testFileAccess(){
   

   coding style: missing space.

3. +++ b/core/modules/file/src/Tests/AccessTest.php
   @@ -0,0 +1,46 @@
   +
   +    // Create anonymous user to check file access.
   +    $account = $this->createUser()->getAnonymousUser();
   +    $access = $file->access('view', $account);
   +    $this->assertTrue($access, 'Public file is accessible to anonymous user');
   

   Test so far looks good.

   But since tests should always also have negative cases, let's add a second file that uses private:// and make sure that users don't have access to that by default.

Left FileManagedTestBase because of the issue with private files. everything else is updated.

1. +++ b/core/modules/file/src/FileAccessControlHandler.php
   @@ -23,6 +23,10 @@ class FileAccessControlHandler extends EntityAccessControlHandler {
    
   

   Nit: extraneous whitespace

2. +++ b/core/modules/file/src/FileAccessControlHandler.php
   @@ -23,6 +23,10 @@ class FileAccessControlHandler extends EntityAccessControlHandler {
   +    if (($operation == 'view') && (file_uri_scheme($entity->getFileUri()) == 'public')) {
   +      return AccessResult::allowed();
   +    }
        if ($operation == 'download' || $operation == 'view') {
          $references = $this->getFileReferences($entity);
   

   Nit: I think this should be if/elseif in stead of two if statements.

3. +++ b/core/modules/file/src/Tests/AccessTest.php
   @@ -0,0 +1,71 @@
   +    $access = $file->access('view', $account);
   +
   +    $this->assertTrue($access, 'Public file is accessible to different user');
   

   Let's put this on one line.
   Also:
   /s/different/authenticated

4. +++ b/core/modules/file/src/Tests/AccessTest.php
   @@ -0,0 +1,71 @@
   +    $access = $file->access('view', $account);
   +
   +    $this->assertTrue($access, 'Public file is accessible to anonymous user');
   

   Also put this on one line

5. +++ b/core/modules/file/src/Tests/AccessTest.php
   @@ -0,0 +1,71 @@
   +    $access = $file->access('view', $account);
   +
   +
   +    $this->assertFalse($access, 'Public file is not accessible to different user');
   

   Ditto

6. +++ b/core/modules/file/src/Tests/AccessTest.php
   @@ -0,0 +1,71 @@
   +    $access = $file->access('view', $account);
   +
   +    $this->assertFalse($access, 'Public file is not accessible to anonymous user');
   

   Ditto

7. +++ b/core/modules/file/src/Tests/AccessTest.php
   @@ -0,0 +1,71 @@
   \ No newline at end of file
   

   Missing new line

#9.1 not sure i get that?
#9.2 using if else now.
#9.3 added authenticated in addition to different because we are checking if the file is accessible to a different user then the one created the file. Thoughts?
everything else should be resolved.

1. +++ b/core/modules/file/src/FileAccessControlHandler.php
   @@ -23,7 +23,11 @@ class FileAccessControlHandler extends EntityAccessControlHandler {
      protected function checkAccess(EntityInterface $entity, $operation, $langcode, AccountInterface $account) {
    
   -    if ($operation == 'download' || $operation == 'view') {
   +    // Always allow access to file in public file system.
   

   The second line (26 of FileAccessControlHandler) is a blank line. Hence the Extraneous whitespace comment.

2. +++ b/core/modules/file/src/Tests/AccessTest.php
   @@ -0,0 +1,68 @@
   +    // Create authenticated user to check file access.
   +    $account = $this->createUser(array('access site reports'));
   +
   +    $this->assertTrue($file->access('view', $account), 'Public file is accessible to different authenticated user');
   

   As you can see, the previous comment describes adding an authenticated user to check file access. Therefor the asserTrue description should be "Public file is accessible to authenticated user". "different" should be removed.

Other than that, good work. Thanks!

Hope this addresses all the issues.

Sorry I missed these earlier.

1. +++ b/core/modules/file/src/Tests/AccessTest.php
   @@ -0,0 +1,69 @@
   +/**
   + * Access file tests.
   + *
   

   Let's document this a bit more clearly. i.e. "Tests access to managed files"

2. +++ b/core/modules/file/src/Tests/AccessTest.php
   @@ -0,0 +1,69 @@
   +class AccessTest extends FileManagedTestBase {
   

   Let's call this class FileManagedAccessTest, since that's what it is.

3. +++ b/core/modules/file/src/Tests/AccessTest.php
   @@ -0,0 +1,69 @@
   +    $this->assertFalse($file->access('view', $account), 'Private file is not accessible to different authenticated user');
   

Remove "different"

1. +++ b/core/modules/file/src/Tests/FileManagedAccessTest.php
   @@ -0,0 +1,69 @@
   + * @file
   + * Contains \Drupal\file\Tests\AccessTest.
   

   Classname should be updated in documentation aswel

   </li>
     <li>
   +++ b/core/modules/file/src/Tests/FileManagedAccessTest.php
   @@ -0,0 +1,69 @@
   +   * Tests if public file is always accessible.
   

   This test also tests if private files are never accessible.

While we are at it, shouldn't we also test if private files are accessible if and when they should be?

We are just adding test coverage for the new code. We already have file access tests for private files (lots of them).

Ups, forgot to change in docs.
Left tests for now as they are.

+++ b/core/modules/file/src/FileAccessControlHandler.php
@@ -22,8 +22,11 @@ class FileAccessControlHandler extends EntityAccessControlHandler {
+    if (($operation == 'view') && (file_uri_scheme($entity->getFileUri()) == 'public')) {

Do we want to include "download" here too?

Looks good to me.

This issue addresses a major bug and is allowed per https://www.drupal.org/core/beta-changes. Committed 7b91c7f and pushed to 8.0.x. Thanks!