search

zoomoid / strapi-provider-upload-aws-s3-advanced

A partial fork of https://github.com/strapi/strapi to extend the S3 Upload Provider to support path prefixes inside a bucket
MIT License
26 stars 23 forks source link

Adding ACL to params is not working #5

Closed tahaelsherif closed 2 years ago

tahaelsherif commented 2 years ago

I tried to use ACL : "public-read" but it's still private when I check it in S3 storage

image

zoomoid commented 2 years ago

Can you provide me with the version you are currently using?

tahaelsherif commented 2 years ago

Can you provide me with the version you are currently using?

4.0.3

zoomoid commented 2 years ago

This is caused by https://github.com/zoomoid/strapi-provider-upload-aws-s3-advanced/blob/ac754d9516169a072de06cc49b40240784a3c143/lib/index.js#L20

and by extension https://github.com/strapi/strapi/blob/ce09d38972ff48db3fbfd0ab80b58f3b5bf91a0c/packages/core/upload/server/services/upload.js#L135.

While this plugin uses customParams in the creation of the PutObjectCommand, nothing is ever passed downstream to the upload function from strapi. Your configuration in the screenshot above is only ever used in the creation of the S3Client, thus the only supported configuration keys are those: https://docs.aws.amazon.com/AWSJavaScriptSDK/v3/latest/clients/client-s3/interfaces/s3clientconfig.html.

I'm happy to extend the plugin to make this configurable from the config object in the closure. Originally, the configuration was copied from the first-party strapi-provider-upload-aws-s3, where the ACL property is pre-defined the same way you did - with the exception that the first-party plugin uses the older v2 version of the aws-sdk, whereas this plugin uses the newer v3.

When I first tried porting this plugin to aws-sdk >= 3.0.0, I experienced some issues regarding using ACL: "public-read", where something went wrong, and I simply never tried again to enable this (in fact there's still a TODO left in the code). At some point, I will try out re-enabling this. For now, you can probably bypass this by setting a bucket policy to "AllowPublicRead" on the objects in question.

tahaelsherif commented 2 years ago

Can you please show me exactly where should I add the AllowPublicRead ?

This is caused by

https://github.com/zoomoid/strapi-provider-upload-aws-s3-advanced/blob/ac754d9516169a072de06cc49b40240784a3c143/lib/index.js#L20

and by extension https://github.com/strapi/strapi/blob/ce09d38972ff48db3fbfd0ab80b58f3b5bf91a0c/packages/core/upload/server/services/upload.js#L135.

While this plugin uses customParams in the creation of the PutObjectCommand, nothing is ever passed downstream to the upload function from strapi. Your configuration in the screenshot above is only ever used in the creation of the S3Client, thus the only supported configuration keys are those: https://docs.aws.amazon.com/AWSJavaScriptSDK/v3/latest/clients/client-s3/interfaces/s3clientconfig.html.

I'm happy to extend the plugin to make this configurable from the config object in the closure. Originally, the configuration was copied from the first-party strapi-provider-upload-aws-s3, where the ACL property is pre-defined the same way you did - with the exception that the first-party plugin uses the older v2 version of the aws-sdk, whereas this plugin uses the newer v3.

When I first tried porting this plugin to aws-sdk >= 3.0.0, I experienced some issues regarding using ACL: "public-read", where something went wrong, and I simply never tried again to enable this (in fact there's still a TODO left in the code). At some point, I will try out re-enabling this. For now, you can probably bypass this by setting a bucket policy to "AllowPublicRead" on the objects in question.

nihey commented 2 years ago

Hey everyone,

I've managed to workaround this ACL issue using some deep @aws-sdk/client-s3 investigation. I thought this might be useful to you there, using the code below, all uploads will be public-read by default:

// config/plugins.js
const { NodeHttpHandler } = require('@aws-sdk/node-http-handler')

const customHttpHandler = new NodeHttpHandler()

module.exports = ({ env }) => ({
  upload: {
    config: {
      provider: 'strapi-provider-upload-aws-s3-advanced',
      providerOptions: {
        accessKeyId: env('STORAGE_ID'),
        secretAccessKey: env('STORAGE_KEY'),
        region: env('STORAGE_REGION'),
        params: {
          bucket: env('STORAGE_BUCKET'),
        },
        requestHandler: {
          ...customHttpHandler,
          handle: (request, options) => {
            // This sets all uploaded files as "public-read" by default
            request.headers['x-amz-acl'] = 'public-read'
            return customHttpHandler.handle(request, options)
          },
        },
      },
    },
  },
})
tahaelsherif commented 2 years ago

Hey everyone,

I've managed to workaround this ACL issue using some deep @aws-sdk/client-s3 investigation. I thought this might be useful to you there, using the code below, all uploads will be public-read by default:

// config/plugins.js
const { NodeHttpHandler } = require('@aws-sdk/node-http-handler')

const customHttpHandler = new NodeHttpHandler()

module.exports = ({ env }) => ({
  upload: {
    config: {
      provider: 'strapi-provider-upload-aws-s3-advanced',
      providerOptions: {
        accessKeyId: env('STORAGE_ID'),
        secretAccessKey: env('STORAGE_KEY'),
        region: env('STORAGE_REGION'),
        params: {
          bucket: env('STORAGE_BUCKET'),
        },
        requestHandler: {
          ...customHttpHandler,
          handle: (request, options) => {
            // This sets all uploaded files as "public-read" by default
            request.headers['x-amz-acl'] = 'public-read'
            return customHttpHandler.handle(request, options)
          },
        },
      },
    },
  },
})

It's not working

image

nihey commented 2 years ago

@tahaelsherif I'm using another Object Storage provider that does not seem to require this request re-signing as AWS (We're using Linode Storage).

You'll probably have to re-sign the AWS Request before sending it to customHttpHandler.handle.

Could you try this changed code?

const { NodeHttpHandler } = require('@aws-sdk/node-http-handler')
const { SignatureV4 } = require('@aws-sdk/signature-v4')
const { Sha256 } = require('@aws-crypto/sha256-js')

const customHttpHandler = new NodeHttpHandler()

module.exports = ({ env }) => ({
  upload: {
    config: {
      provider: 'strapi-provider-upload-aws-s3-advanced',
      providerOptions: {
        accessKeyId: env('STORAGE_ID'),
        secretAccessKey: env('STORAGE_KEY'),
        region: env('STORAGE_REGION'),
        params: {
          bucket: env('STORAGE_BUCKET'),
        },
        requestHandler: {
          ...customHttpHandler,
          handle: async (request, options) => {
            const signer = new SignatureV4({
              service: 's3',
              region: env('STORAGE_REGION'),
              sha256: Sha256,
              credentials: {
                accessKeyId: env('STORAGE_ID'),
                secretAccessKey: env('STORAGE_KEY'),
              },
            })
            const acl = 'public-read'
            request.headers['x-amz-acl'] = acl
            const signedRequest = await signer.sign(request)

            return customHttpHandler.handle(signedRequest, options)
          },
        },
      },
    },
  },
})
tahaelsherif commented 2 years ago

@tahaelsherif I'm using another Object Storage provider that does not seem to require this request re-signing as AWS (We're using Linode Storage).

You'll probably have to re-sign the AWS Request before sending it to customHttpHandler.handle.

Could you try this changed code?

const { NodeHttpHandler } = require('@aws-sdk/node-http-handler')
const { SignatureV4 } = require('@aws-sdk/signature-v4')
const { Sha256 } = require('@aws-crypto/sha256-js')

const customHttpHandler = new NodeHttpHandler()

module.exports = ({ env }) => ({
  upload: {
    config: {
      provider: 'strapi-provider-upload-aws-s3-advanced',
      providerOptions: {
        accessKeyId: env('STORAGE_ID'),
        secretAccessKey: env('STORAGE_KEY'),
        region: env('STORAGE_REGION'),
        params: {
          bucket: env('STORAGE_BUCKET'),
        },
        requestHandler: {
          ...customHttpHandler,
          handle: async (request, options) => {
            const signer = new SignatureV4({
              service: 's3',
              region: env('STORAGE_REGION'),
              sha256: Sha256,
              credentials: {
                accessKeyId: env('STORAGE_ID'),
                secretAccessKey: env('STORAGE_KEY'),
              },
            })
            const acl = 'public-read'
            request.headers['x-amz-acl'] = acl
            const signedRequest = await signer.sign(request)

            return customHttpHandler.handle(signedRequest, options)
          },
        },
      },
    },
  },
})

It worked thank you

boomaker commented 2 years ago

Hey everyone,

I've managed to workaround this ACL issue using some deep @aws-sdk/client-s3 investigation. I thought this might be useful to you there, using the code below, all uploads will be public-read by default:

// config/plugins.js
const { NodeHttpHandler } = require('@aws-sdk/node-http-handler')

const customHttpHandler = new NodeHttpHandler()

module.exports = ({ env }) => ({
  upload: {
    config: {
      provider: 'strapi-provider-upload-aws-s3-advanced',
      providerOptions: {
        accessKeyId: env('STORAGE_ID'),
        secretAccessKey: env('STORAGE_KEY'),
        region: env('STORAGE_REGION'),
        params: {
          bucket: env('STORAGE_BUCKET'),
        },
        requestHandler: {
          ...customHttpHandler,
          handle: (request, options) => {
            // This sets all uploaded files as "public-read" by default
            request.headers['x-amz-acl'] = 'public-read'
            return customHttpHandler.handle(request, options)
          },
        },
      },
    },
  },
})

Thanks for your contribution, it worked like a charm for me!

nchowning commented 2 years ago

@tahaelsherif I'm using another Object Storage provider that does not seem to require this request re-signing as AWS (We're using Linode Storage).

You'll probably have to re-sign the AWS Request before sending it to customHttpHandler.handle.

Could you try this changed code?

const { NodeHttpHandler } = require('@aws-sdk/node-http-handler')
const { SignatureV4 } = require('@aws-sdk/signature-v4')
const { Sha256 } = require('@aws-crypto/sha256-js')

const customHttpHandler = new NodeHttpHandler()

module.exports = ({ env }) => ({
  upload: {
    config: {
      provider: 'strapi-provider-upload-aws-s3-advanced',
      providerOptions: {
        accessKeyId: env('STORAGE_ID'),
        secretAccessKey: env('STORAGE_KEY'),
        region: env('STORAGE_REGION'),
        params: {
          bucket: env('STORAGE_BUCKET'),
        },
        requestHandler: {
          ...customHttpHandler,
          handle: async (request, options) => {
            const signer = new SignatureV4({
              service: 's3',
              region: env('STORAGE_REGION'),
              sha256: Sha256,
              credentials: {
                accessKeyId: env('STORAGE_ID'),
                secretAccessKey: env('STORAGE_KEY'),
              },
            })
            const acl = 'public-read'
            request.headers['x-amz-acl'] = acl
            const signedRequest = await signer.sign(request)

            return customHttpHandler.handle(signedRequest, options)
          },
        },
      },
    },
  },
})

This solution worked for me as well. Thanks for the help!