silverstripe / silverstripe-s3

Silverstripe module to store assets in S3 rather than on the local filesystem (SS4/SS5 only)
BSD 3-Clause "New" or "Revised" License
20 stars 25 forks source link

Heroku Bucketeer blocking file write - AccessControlListNotSupported #61

Closed zaidaldabbagh closed 11 months ago

zaidaldabbagh commented 11 months ago

Hi all,

Firstly, apologies about the long blurb haha.

I've been trying to make Heroku's Bucketeer (S3 Add-on) to work with silverstripe-s3 without much success.

I've followed all of the steps necessary below (See below). Is there something that I've may have missed?

Bucketeer Configs

In Heroku, I've ensured that I have the following env vars configured:

AWS_ACCESS_KEY_ID: X
AWS_BUCKET_NAME: X
AWS_REGION: X
AWS_SECRET_ACCESS_KEY: X

Here's my policy.json:

Note: I had to remove the wrapping "Policy" field which is included in this module's README, without doing so AWS CLI throws error: An error occurred (MalformedPolicy) when calling the PutBucketPolicy operation: Unknown field Policy, perhaps you need to update the README.

{
  "Version": "2012-10-17",
  "Statement": [
    {
      "Sid": "AddPerm",
      "Effect": "Allow",
      "Principal": "*",
      "Action": "s3:GetObject",
      "Resource": "arn:aws:s3:::<AWS_BUCKET_NAME>/public/*"
    }
  ]
}

I then updated the access rules on the bucket:

aws s3api put-public-access-block --bucket <AWS_BUCKET_NAME> --public-access-block-configuration BlockPublicAcls=FALSE,IgnorePublicAcls=FALSE,BlockPublicPolicy=FALSE,RestrictPublicBuckets=FALSE

Then uploaded the policy.json:

aws s3api put-bucket-policy --bucket <AWS_BUCKET_NAME> --policy file://policy.json

Then, verified I had the correct policy set:

aws s3api get-public-access-block --bucket <AWS_BUCKET_NAME>
{
    "PublicAccessBlockConfiguration": {
        "BlockPublicAcls": false,
        "IgnorePublicAcls": false,
        "BlockPublicPolicy": false,
        "RestrictPublicBuckets": false
    }
}

However, when I try and publish a FIle /admin/assets it fails with the error below:

<Error>
  <Code>AccessControlListNotSupported</Code>
  <Message>The bucket does not allow ACLs</Message>
</Error>

SilverStripe S3 - Module Configs

I've configured s3.yml like so ...

---
Only:
envvarset: AWS_BUCKET_NAME
After:
  - '#silverstripes3-flysystem'
---
SilverStripe\Core\Injector\Injector:
  Aws\S3\S3Client:
    constructor:
      configuration:
        region: '`AWS_REGION`'
        version: latest
        credentials:
          key: '`AWS_ACCESS_KEY_ID`'
          secret: '`AWS_SECRET_ACCESS_KEY`'

I also tried re-adding the Bucketeer resource a couple of times without success. I've raised this issue with the Bucketeer developers too, and they're also currently investigating this.

I just wanted to make sure it wasn't something that had I missed with this module's config.

obj63mc commented 11 months ago

You need to configure bucket to allow ACLs and that the object writer can control the ACL on the the file.

Not sure how you do this via Bucketeer or the AWS CLI but in the AWS admin console this shows what you need to check.

zaidaldabbagh commented 11 months ago

Thanks @obj63mc I did try this without success ... So I went back to the Bucketeer Documentation, and it stated that "AWS is deprecating ACLs in favor of bucket policies to control access. New buckets have policy and ACL changes disabled by default." — Which applies to my case, as I've been creating new Bucketeer instances which have this enforced.

So based on this, I've gone back to trying to make this work with Bucket Policies for controlling access, so that the S3 bucket uses the defined bucket policy for access control without needing to rely on ACLs.

Here's my policy.json:

{
  "Version": "2012-10-17",
  "Statement": [
    {
      "Effect": "Allow",
      "Principal": "*",
      "Action": "s3:GetObject",
      "Resource": "arn:aws:s3:::<AWS_BUCKET_NAME>/public/*"
    }
  ]
}

Updated put-public-access-block command to allow bucket policies for access control, and to disable public access blocks:

aws s3api put-public-access-block --bucket <AWS_BUCKET_NAME> --public-access-block-configuration BlockPublicAcls=false,IgnorePublicAcls=false,BlockPublicPolicy=false,RestrictPublicBuckets=false

Finally, used the put-bucket-policy command to apply your new policy to the bucket:

aws s3api put-bucket-policy --bucket <AWS_BUCKET_NAME> --policy file://policy.json

Unfortunately, this is still failing with the same error:

<Error>
  <Code>AccessControlListNotSupported</Code>
  <Message>The bucket does not allow ACLs</Message>
</Error>
obj63mc commented 11 months ago

If you are not able to set the bucket to allow ACLs on a per object basis, not sure this will work for you then to use bucketeer. All we are doing with this is simply wrapping the publish/write of the file using flysystem's adapter. https://flysystem.thephpleague.com/v1/docs/adapter/aws-s3-v3/. All of SilverStripe's filesystem stuff is simply a wrapper using flysystem. We don't really control the actual integration with the aws sdk. May want to check their repo to see if there are any bugs/comments on how to work around that. https://github.com/thephpleague/flysystem-aws-s3-v3

obj63mc commented 11 months ago

Looking at https://github.com/thephpleague/flysystem-aws-s3-v3/issues/170 - you should be able to resolve this in your config for the aws s3 client in your yml files

Could be something like this as an example -

---
Only:
  envvarset: AWS_BUCKET_NAME
After:
  - '#assetsflysystem'
---
SilverStripe\Core\Injector\Injector:
  Aws\S3\S3Client:
    constructor:
      configuration:
        region: '`AWS_REGION`'
        version: latest
        credentials:
          key: '`AWS_ACCESS_KEY_ID`'
          secret: '`AWS_SECRET_ACCESS_KEY`'
  League\Flysystem\Filesystem.public:
    class: League\Flysystem\Filesystem
    constructor:
      FilesystemAdapter: '%$SilverStripe\Assets\Flysystem\PublicAdapter'
      FilesystemConfig:
        visibility: noPredefinedVisibility

Now not sure if that is exactly what you will need but you can change the config of the flysystem adapter to control what S3 is using and what the default configuration is on the actual s3 client itself.

zaidaldabbagh commented 11 months ago

Thnx @obj63mc I've amended my yaml configs with your proposed. Also, the Bucketeer developers have changed the settings on my Bucket (For testing purposes) so that the Object Writer remains to be the Bucket Owner. However, now I'm getting a different error 😅 any ideas? Anything that I can amend in yaml configs?

Monolog\ErrorHandler-&gt;handleError(2, Trying to access array offset on value of type bool, /app/vendor/league/flysystem-cached-adapter/src/Storage/AbstractCache.php, 130)

Also tried passing this in config:

---
Only:
  envvarset: AWS_BUCKET_NAME
After:
  - '#assetsflysystem'
---
SilverStripe\Core\Injector\Injector:
  Aws\S3\S3Client:
    constructor:
      configuration:
        region: '`AWS_REGION`'
        version: latest
        credentials:
          key: '`AWS_ACCESS_KEY_ID`'
          secret: '`AWS_SECRET_ACCESS_KEY`'
  League\Flysystem\Filesystem.public:
    class: League\Flysystem\Filesystem
    constructor:
      FilesystemAdapter: '%$SilverStripe\Assets\Flysystem\PublicAdapter'
      FilesystemConfig:
        visibility: noPredefinedVisibility
        ACL: 'bucket-owner-full-control'
obj63mc commented 11 months ago

Can't say what that error would be. You would need to look at that source file and try to figure out what is going on. https://github.com/thephpleague/flysystem-cached-adapter/blob/master/src/Storage/AbstractCache.php

Since it is erroring when dealing with the cache you may just need to clear the sites cache with a flush.

zaidaldabbagh commented 11 months ago

@obj63mc ok it's finally working 🎉 hahaha So this is what had to be changed ... As mentioned above, the Heroku Bucketeer devs temporarily set the Bucket policy so that [Object Writer remains to be the Bucket Owner] ... They're going to investigate that option so that it's configurable by devs. Ideally, it would be set by default, but not sure how they're going to manage that.

Also, after reviewing this AWS guide — Disabling ACLs for all new buckets and enforcing Object Ownership, I had to change my policy.json to the following:

{
  "Version": "2012-10-17",
  "Statement": [
     {
        "Sid": "Only allow writes to my bucket with bucket owner full control",
        "Effect": "Allow",
        "Principal": "*",
        "Action": [
           "s3:PutObject"
        ],
        "Resource": "arn:aws:s3:::<AWS_BUCKET_NAME>/public/*",
        "Condition": {
           "StringEquals": {
              "s3:x-amz-acl": "bucket-owner-full-control"
           }
        }
     }
  ]
}

Lastly, I also revised my s3.yml to the following, removing the League\Flysystem\Filesystem.public configs, here it is now:

---
Only:
  envvarset: AWS_BUCKET_NAME
After:
  - '#assetsflysystem'
---
SilverStripe\Core\Injector\Injector:
  Aws\S3\S3Client:
    constructor:
      configuration:
        region: '`AWS_REGION`'
        version: latest
        credentials:
          key: '`AWS_ACCESS_KEY_ID`'
          secret: '`AWS_SECRET_ACCESS_KEY`'

I'll add new steps for integrating with Bucketeer later on. I think we can close this ticket now.