I have a private bucket set up on AWS S3 and configured flysystem s3 as follows

/**
 * Flysystem S3 Setup
 */
$schemes = [
  's3' => [
    'driver' => 's3',
    'config' => [
      'key' => 'MY_KEY',
      'secret' => 'MY_SECRET',
      'region' => 'us-west-2',
      'bucket' => 'mybucket',
      'public' => FALSE,
      'options' => [
        'ServerSideEncryption' => 'AES256',
        'ACL' => 'private',
      ]
    ],
    'cache' => FALSE,
    'serve_js' => FALSE,
    'serve_css' => FALSE,
  ],
];
$settings['flysystem'] = $schemes;

The issue is that every time I upload a file via file field using the S3 scheme, I get the following error:
Error: The file permissions could not be set on s3://filename.pdf.

The cause of this error is that \Drupal\Core\File\FileSystem::chmod() is trying to apply the default permission mode 0664 which \Twistor\FlysystemStreamWrapper::stream_metadata() determines is public. This causes an exception to be thrown. Instead, the permission mode 0600 must be used.

This is related to https://www.drupal.org/project/flysystem_s3/issues/2772847, however, I chose to report and provide a patch for this bug separately as I have no need to implement signed URLs in my project.

CommentFileSizeAuthor
#7 interdiff_3058063_3-7.txt1.74 KBsocketwench
#7 flysystem_s3-chmod_fix-3058063-7.patch3.69 KBsocketwench
#3 interdiff_2-3.txt2.36 KBtame4tex
#3 flysystem_s3-chmod_fix-3058063-3.patch3.95 KBtame4tex
#2 flysystem_s3-chmod_fix-3058063-2.patch6.43 KBtame4tex

Comments

tame4tex created an issue. See original summary.

tame4tex’s picture

tame4tex commented
StatusFileSize
new flysystem_s3-chmod_fix-3058063-2.patch6.43 KB

Parts of this patch were taken from the patch provided on https://www.drupal.org/project/flysystem_s3/issues/2772847.

The difference with this patch is the FileSystem decorator class extends FileSystem instead of replacing it. In my opinion, this makes the class less fragile and vulnerable to breaking if FileSystem is modified. Although maybe I am missing an important reason as to why this is not a good idea so open to feedback.

The test provided is the exact same test provide in the patch from the issue listed above.

tame4tex’s picture

tame4tex commented
StatusFileSize
new flysystem_s3-chmod_fix-3058063-3.patch3.95 KB
new interdiff_2-3.txt2.36 KB

Removed the test from the patch. Given I renamed and refactored the FileSystem decorator class from the patch on https://www.drupal.org/project/flysystem_s3/issues/2772847, the test needs to also be refactored. I don't understand how it passed. Unfortunately, I don't have time to fix right now.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented

I'm not sure that it makes sense to both inject and extend the service. If we're extending the service, instead of calling $this->fileSystem->methodName() in the chmod method, we can use parent::methodName().

tame4tex’s picture

tame4tex commented

That makes sense ... I will resubmit the patch with that change in the next couple of days.

socketwench’s picture

socketwench commented

Having the same problem here.

No matter what ACL I set in the Flysystem options, the resulting items are uploaded to the bucket with public visibility. This, despite the $acl parameter passed in \Drupal\flysystem_s3\Flysystem\Adapter\S3Adapter::upload().

socketwench’s picture

socketwench commented
Status: Active » Needs review
StatusFileSize
new flysystem_s3-chmod_fix-3058063-7.patch3.69 KB
new interdiff_3058063_3-7.txt1.74 KB

Removed the injected service, used parent::.

tame4tex’s picture

tame4tex commented

Thanks @socketwench I hadn't gotten back round to fix this patch so appreciate your input. Did this patch fix your issue?

socketwench’s picture

socketwench commented

I think I might have screwed something up in that patch, since after applying it, it seemed like no uploads were working period.

SlayJay’s picture

SlayJay commented

I had the exact same issue, I applied the patch in #7 via composer and everything works fine for me.

SlayJay’s picture

SlayJay commented
Status: Needs review » Reviewed & tested by the community

Been running this for a few weeks now with zero issues, marking as RBTC

  • MiSc committed f946cfb on 8.x-1.x authored by socketwench 
    Issue #3058063 by tame4tex, socketwench: Issue with setting permissions...
misc’s picture

misc commented

Thanks for you contribution! This is now merged and in the the new beta release.

misc’s picture

misc commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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

leon kessler’s picture

leon kessler commented

I still see this error when image styles are generated.

I did a small amount of investigation, and found that when an image style is created, a call is made to $filesystem->move().
This then calls $this->chmod($destination); directly (bypassing the decorated method from this patch).

I couldn't really see an easy way around this, we could override the move() method in theFlysystemS3FileSystem class. But it's a fairly large function, and there doesn't seem to be a nice way to break it off and just insert our file permissions.

Instead I've added this into my settings.php

$settings['file_chmod_directory'] = 0700;
$settings['file_chmod_file'] = 0600;

Which fixes the issue for now.

If I work out a better way I'll create a new issue.

leon kessler’s picture

leon kessler commented

Ignore my previous comment, the issue was actually caused by patch from #2772847: Add support for private uploads / presigned URLs (which had an overridden implementation of $filesystem->move()).