On this page: https://www.drupal.org/node/2541116 it states that :

"Flysystem storage schemes works like private files in drupal 8.

It hides the actual link of the to file and delivers it via drupal generated path."

I have found that to be not the case. Files are linked directly to S3 with no access control.

Are access controlled private files possible with this module? If so what config is required to achieve this?

Steps to reproduce
Configure and Enable a Flysystem S3 file system and use it to store a file, e.g. file/image field on a content type
The file will be linked to directly on the configured S3 system.

Expected outcome
If the file-system is configured as public, the direct link to the file should be provided.
If the file-system is not configured as public, the flysystem generated link (/_flysystem/{scheme}/{filepath}) should be provided.

CommentFileSizeAuthor
#12 interdiff-2793355-9-12.txt1.17 KBt-lo
#12 flysystem_s3_private_files-2793355-12.patch1.72 KBt-lo
#9 flysystem_s3_private_files-2793355-9.patch1.2 KBt-lo

Comments

maniosullivan created an issue. See original summary.

twistor’s picture

twistor commented
Status: Active » Postponed (maintainer needs more info)

What does your configuration look like?

The setting that changes things is 'public' => TRUE, setting this to FALSE, or just removing it, should do what you're looking for. If not, then that's a bug.

maniosullivan’s picture

maniosullivan commented

My config looks like this: 

$schemes = [
  'dev' => [
    'type' => 's3',
    'driver' => 's3',
    'config' => [
      'key'    => 'SECRET',      // 'key' and 'secret' do not need to be
      'secret' => 'SECRET',   // provided if using IAM roles.
      'region' => 'SECRET',
      'bucket' => 'SECRET',
    ],

    'cache' => FALSE, // Creates a metadata cache to speed up lookups.
  ],
];

$settings['flysystem'] = $schemes;

After doing some digging I have found that the file is initially uploaded as 'private'. Because the file is being added using a file field on a node the core file module jumps in with file_save_upload(). In there it performs a drupal_chmod to set the file permissions. After that the file is publicly available for anyone to see or download, as the permissions on AWS have been updated. "Everyone" has been granted Open/Download permission.

If I comment the drupal_chmod call the file is added as private, but this is obviously not practical.

maniosullivan’s picture

maniosullivan commented

I have some more information.

So in FileSystem.php on line 90 we have:

$mode = $this->settings->get('file_chmod_file', static::CHMOD_FILE);

Which gets the constant value CHMOD_FILE, which is set to 0644.

Now in stream_metadata in FlysystemStreamWrapper.php the decimal value of 644 permission has been passed in. This is then applied to the uploaded file on line 442 with:

return $this->getFilesystem()->setVisibility($this->getTarget(), $visibility);

in this instance $is_public is always set to 1, and the file permissions are always set to public.

Perhaps in here we could check the uri scheme of the file, and then backtrack to see what permission type (pubic or private) is configured for that particular scheme?

maniosullivan’s picture

maniosullivan commented
Category: Feature request » Bug report
deviantintegral’s picture

deviantintegral commented
Status: Postponed (maintainer needs more info) » Closed (duplicate)

This actually belongs over in the Flysystem S3 project, where there's a patch at #2772847: Add support for private uploads / presigned URLs.

t-lo’s picture

t-lo
Primary language English
Location Bristol, UK
commented
Project: Flysystem » Flysystem - S3
Version: 8.x-1.0-alpha4 » 8.x-1.x-dev
Component: Documentation » Code
Status: Closed (duplicate) » Active

This does belong in flysystem_s3 but is is a separate issue to #2772847

The absence of 'public' => TRUE in the driver config should lead to the URLs being provided as a Drupal generated path rather than S3.

t-lo’s picture

t-lo
Primary language English
Location Bristol, UK
commented
Assigned: Unassigned » t-lo
t-lo’s picture

t-lo
Primary language English
Location Bristol, UK
commented
Assigned: t-lo » Unassigned
Status: Active » Needs review
StatusFileSize
new flysystem_s3_private_files-2793355-9.patch1.2 KB

The attached patch adds the use of the public config option as per the main flysystem module documentation.

It should be noted this defaults to private as per the base module.

kevla’s picture

kevla commented

Is this going to be reviewed and merged in any time soon?

vijaycs85’s picture

vijaycs85
Location London, UK
commented
Status: Needs review » Needs work 
  1. +++ b/src/Flysystem/S3.php
@@ -62,6 +62,13 @@ class S3 implements FlysystemPluginInterface, ContainerFactoryPluginInterface {
+  protected $isPublic;

    Might need to be updated in README.md code snippet as well? 

  2. +++ b/src/Flysystem/S3.php
@@ -73,6 +80,7 @@ class S3 implements FlysystemPluginInterface, ContainerFactoryPluginInterface {
+    $this->isPublic = $config->get('public',FALSE);

    Missing space after comma. 

  3. +++ b/src/Flysystem/S3.php
@@ -159,6 +167,11 @@ class S3 implements FlysystemPluginInterface, ContainerFactoryPluginInterface {
+    ¶

    unnecessary space.

t-lo’s picture

t-lo
Primary language English
Location Bristol, UK
commented
Status: Needs work » Needs review
StatusFileSize
new flysystem_s3_private_files-2793355-12.patch1.72 KB
new interdiff-2793355-9-12.txt1.17 KB

Thanks for the review!

Updated patch fixing those issues

vijaycs85’s picture

vijaycs85
Location London, UK
commented
Issue tags: +Needs manual testing, +Needs issue summary update

Thanks for the quick response @T-lo. I don't have a local setup to verify it. could you update steps to reproduce on issue summary and/or test results please?

vijaycs85’s picture

vijaycs85
Location London, UK
commented
Status: Needs review » Needs work
t-lo’s picture

t-lo
Primary language English
Location Bristol, UK
commented
Issue summary: View changes
Status: Needs work » Needs review
dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Related issues: +#2772847: Add support for private uploads / presigned URLs

This is not a separate issue from #2772847: Add support for private uploads / presigned URLs, supporting private files is exactly what that existing is doing. It goes with an 'ACL' option instead of a public/private flag. This also needs to consider serving private files through image styles, which looking at the patch would errantly always get caught in the getDownloadUrl() call when it shouldn't.

Prabhu.shan’s picture

Prabhu.shan commented

Hi,

Any update on this issue ? I still see public URL of S3 rather than drupal generated code?

t-lo’s picture

t-lo
Primary language English
Location Bristol, UK
commented

@dave-reid, I'd suggest this is separate, as the issue you've linked does not deal with the Drupal generated URL, (see comment 21 and 22 https://www.drupal.org/project/flysystem_s3/issues/2772847#comment-12149031), rather signatures on the s3 url

I had to resolve the image style issue with patches from https://www.drupal.org/project/drupal/issues/1358896

misc’s picture

misc commented
Status: Needs review » Fixed

Added patch in #12 to latest dev and will be included in release later today.

  • MiSc committed 42e5b81 on 8.x-1.x authored by T-lo 
    Issue #2793355 by T-lo: Is it possible to use S3 with private managed...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.