Problem/Motivation

I'm trying to configure s3 bucket to store private files only. Reading the documentation I figured that we couldn't have both $settings['s3fs.upload_as_private'] and $settings['s3fs.use_s3_for_private'] set to TRUE, and on Drupal\s3fs\S3fsService->validate() it would throw the error "If you upload all files as private using public stream wrapper, you can't use private stream wrapper because all of your files will be public through CNAME".

So, to allow only private files to be uploaded, we are using $settings['s3fs.use_s3_for_public'] = FALSE; and
$settings['s3fs.use_s3_for_private'] = TRUE; as advised here https://www.drupal.org/project/s3fs/issues/2894061#comment-12179882 because we were having a similar issue trying to create an image style.

We think the issue is caused by the file metadata cache being outdated, which could be refreshed in the settings for the module, but it's failing on line 118 of Drupal\s3fs\S3fsService->validate() because we don't have $settings['s3fs.upload_as_private'] = TRUE;.

If it doesn't have the settings $settings['s3fs.upload_as_private'] = TRUE it will try to upload a public file, which doesn't make sense on our case, and consequently we can never pass that step.

I think the validate is faulty by not accommodating that option. Otherwise, please instruct me as how to configure this module to upload only private files.

CommentFileSizeAuthor
#2 validation_fail_use_s3_for_private_only-3165271-2.patch1.32 KBbbombachini

Comments

bbombachini created an issue. See original summary.

bbombachini’s picture

bbombachini
Location London, ON
commented
StatusFileSize
new validation_fail_use_s3_for_private_only-3165271-2.patch1.32 KB

I hope what I said made sense, that would be my suggestion to fix it (considering my use case and following @jansete suggestion on the other issue).

bbombachini’s picture

bbombachini
Location London, ON
commented
Issue summary: View changes
bbombachini’s picture

bbombachini
Location London, ON
commented
Status: Active » Needs review
cmlara’s picture

cmlara
he/him
commented
Status: Needs review » Needs work

It looks like I doubled up on this issue while working on #3194619: Allow PRESIGN url's to work with CNAME, VersionID and custom_GET_args I didn't see you had this open before me.

     if (Settings::get('s3fs.upload_as_private') && Settings::get('s3fs.use_s3_for_private')) {
-      $errors[] = $this->t("If you upload all files as private using public stream wrapper, you cant't use private stream wrapper because all of your files will be public through CNAME");
+      $errors[] = $this->t("If you upload all files as private using public stream wrapper, you can't use private stream wrapper because all of your files will be public through CNAME");
     }

I believe this section was actually suppose to check if s3fs.upload_as_private and s3fs.use_s3_for_public. Using upload_as_private I do not believe the file would be public via a name as the ACL rules would still block the file. The CNAME portion seems even more out of place here making me believe this section has errors. I cant see any reason we throw an error on upload_as_private and use_s3_for_private as those are exactly the files we want to be restricted. I made those changes in #3194619: Allow PRESIGN url's to work with CNAME, VersionID and custom_GET_args.

+ if (!Settings::get('s3fs.upload_as_private') && Settings::get('s3fs.use_s3_for_public')) {
I do not belive we need the condition on use_s3_for_public. without upload_as_private we want private:// and s3:// links to be able to posted with an ACL of public read. This can be seen in S3fsStream.php. I think if we modify this check we will get a false-positie for s3:// content should the bucket be hardcoded to permit no public ACL rules.

    if (
          !$this->uploadAsPrivate
      &&  StreamWrapperManager::getScheme($this->uri) !== 'private'
    ) {
      // All non-private files uploaded to S3 must be set to public-read, or
      // users' browsers will get PermissionDenied errors, and torrent URLs
      // won't work.
      $options[$this->protocol]['ACL'] = 'public-read';
    }
bbombachini’s picture

bbombachini
Location London, ON
commented

Thanks for your review @cmlara.

The issue here is that we wanted to use it only for private files. But we couldn't have $settings['s3fs.use_s3_for_private'] = TRUE; AND $settings['s3fs.upload_as_private'] = TRUE;, and as advised here https://www.drupal.org/project/s3fs/issues/2894061#comment-12179882 we've been using $settings['s3fs.use_s3_for_private'] = TRUE;.

And this would break on validation, trying to upload a public file to s3 because I didn't have $settings['s3fs.upload_as_private'] = TRUE;. Maybe if we change to something like that it would make more sense?

if (!Settings::get('s3fs.upload_as_private') && !Settings::get('s3fs.use_s3_for_private')) {

cmlara’s picture

cmlara
he/him
commented

Fully agree on wanting to use it for private files only but receiving that validation error being a bug.

Its my belief that the line 88/89 message was intended to warn users that use_s3_for_public is incompatible with upload_as_private. I think this change will do exactly what you need:

    if (Settings::get('s3fs.upload_as_private') && Settings::get('s3fs.use_s3_for_public')) {
      $errors[] = $this->t("If you upload all files as private you can't use the public stream wrapper.");

If that doesn't clear up the issues on its own than I worry I may be thinking that message is for one situation when its really for another.

  • cmlara committed e96cbc3 on 8.x-3.x 
    Issue #3165271 by bbombachini, cmlara: Validation fail for '...
cmlara’s picture

cmlara
he/him
commented
Status: Needs work » Fixed

Status: Fixed » Closed (fixed)

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