Added check to see if disk implements a default visibility

[jaxwilko]

If you have "block all public access" enabled for an S3 bucket, then when uploading files you need to specify 'visibility' => 'private', you can currently do this for media items by adding:

'visibility' => 'private',

To your filesystem config for s3.

However the FileUpload widget does not respect the config. This change first checks if visibility is defined in the disk's config, falling back to the default public if not.

So the ACL is defined by the visibility that is passed to put here:

storm/src/Database/Attach/File.php

Line 1011 in f4c61a9

return $this->getDisk()->put($storagePath, FileHelper::get($localPath), $this->isPublic() ? 'public' : null);

It falls through a bunch of stuff and ends up defining the object ACL here:

// vendor/league/flysystem-aws-s3-v3/AwsS3V3Adapter.php:152
$acl = $options['params']['ACL'] ?? $this->determineAcl($config);

However, if you have block all public access enabled and have Bucket owner enforced enabled (the aws recommended option), then when uploading a file you get:

<?xml version="1.0" encoding="UTF-8"?>
<Error>
    <Code>AccessControlListNotSupported</Code>
    <Message>The bucket does not allow ACLs</Message>
    <RequestId>0N5Y3Q5KNMHM8AB1</RequestId>
    <HostId>8XjTaRU5aG1X7PDae1Oyb1qCeI7SAAVenu4e4dAhhXno4sw4d0v7pqa+Lq9+cc+BvPEF5SE6O/I=</HostId>
</Error>

This is because AWS does not allow the writer to create public visible objects when block public access is enabled.

To fix this, you need to set visibility to private (although, actually setting it to anything !== public works), see:

// vendor/league/flysystem-aws-s3-v3/PortableVisibilityConverter.php:20
public function visibilityToAcl(string $visibility): string
{
    if ($visibility === Visibility::PUBLIC) {
        return self::PUBLIC_ACL;
    }

    return self::PRIVATE_ACL;
}

In theory, the PR should:

• If the object is private, pass null which will end up being private
• If the object is marked as public, then use the disk's config to pass either the default disk visibility
• If there is no configured visibility then default to public
  $this->isPublic() ? ($this->getDisk()->getConfig()['visibility'] ?? 'public') : null

There'll be no change to existing functionality if people are not configuring the disk's visibility, and if they do it'll treat it as the default instead of defaulting to public.

If somebody (like me) is blocking public access, then they need to configure their bucket correctly to manage access to protected files, but that's done via bucket policies, Winter still needs to know to generate temp urls for objects, but the actual visibility is not massively important.

[LukeTowers]

@bennothommo can you fix the code analysis issue on this PR?

[bennothommo]

@LukeTowers done

[jaxwilko]

The following might be a better fix as it moves the control into the hands of the user.

File::extend(function (File $model) {
    $model->bindEvent('model.beforeSave', function () use ($model) {
        $model->is_public = false;
    }, PHP_INT_MAX);
});

If nobody else is having this issue then I'm okay with closing it, although really I think we should change something, what we should change I'm not sure :)