odysseyscience / react-s3-uploader

React component that renders an <input type="file"/> and automatically uploads to an S3 bucket
MIT License
826 stars 240 forks source link

x-amz-acl Headers not signed #106

Open kusold opened 7 years ago

kusold commented 7 years ago

I had an issue where all of my requests were coming back with this:

<Error>
  <Code>
    AccessDenied
  </Code>
  <Message>
    There were headers present in the request which were not signed
  </Message>
  <HeadersNotSigned>
    x-amz-acl
  </HeadersNotSigned>
  <RequestId>
    MyRequestId
  </RequestId>
  <HostId>
    BASE64EncodedHostId=
  </HostId>
</Error>

My presigner uses the official ruby aws SDK:

def presign_foo(name)
    signer = Aws::S3::Presigner.new
    signer.presigned_url(:put_object, bucket: Rails.application.config.s3_root, key: "something_private/#{name}", acl: 'public-read')
end

And it outputs this string: https://foo-dev.s3.amazonaws.com/something_private/my_filename?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=MyCredentials%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20170118T190154Z&X-Amz-Expires=900&X-Amz-SignedHeaders=host&x-amz-acl=public-read&X-Amz-Signature=MySignature

And here is my react component. getSignedPeopleTargeting calls a route that ends up returning the value from presign_foo.

<S3Uploader
    getSignedUrl={this.props.getSignedPeopleTargeting}
/>

Eventually I tracked this down to a line in the s3-uploader that adds the header: https://github.com/odysseyscience/react-s3-uploader/blob/7a26ebc3d0cbfbf3bfb9bec2d0cb28bf147e8b95/s3upload.js#L160

I believe that this header is not needed as I can control the acl in my presign function (tested by making it private). Because all the presigner examples set the acl, can this line be removed? Alternatively, can the if statement be changed to: typeof this.uploadRequestHeaders === 'undefined'?

kusold commented 7 years ago

Further research leads me to believe that the official ruby SDK might be handling x-aml-acl differently then other libraries because it hoists the property to a query parameter ( https://github.com/aws/aws-sdk-ruby/issues/874 ). There is currently a feature request here: https://github.com/aws/aws-sdk-ruby/commit/17a8b419cc85733a99a8edd5fea06e29dd0caa3f

tmorton commented 7 years ago

For fellow googlers, here's a quick workaround. Pass the uploadRequestHeaders prop with {}, and you'll get no headers attached to the request.

speedy250 commented 7 years ago

For Python devs using Boto3, there is a sample to generate presigned urls in the documentation, but note that the ClientMethod correlates to methods in the S3.Client library. You'll want to use the put_object string, and include the ACL parameter in the Params object. Hope that saves someone a bunch of time.

rodcisal commented 6 years ago

@tmorton dude thanks for doing my life easier

iamrahulroy commented 5 years ago

@tmorton what if I need to send something in the header? The moment I pass an option to uploadRequestHeaders, it starts throwing 403 forbidden error. Any workaround?

tmorton commented 5 years ago

@iamrahulroy I haven't looked at this in a while. I'm guessing you will need to fork this project, and remove the x-amz-acl header, or sign it.

IrfanMurtaza93 commented 3 years ago

For fellow googlers, here's a quick workaround. Pass the uploadRequestHeaders prop with {}, and you'll get no headers attached to the request.

I guess in this way it won't be publicly accessible. Right?