Skip to content
This repository has been archived by the owner on Nov 20, 2018. It is now read-only.

2 - In S3 mode, provide AWS ETag to the uploadSuccess callback when available #1118

Closed
paulmelnikow opened this issue Feb 3, 2014 · 13 comments

Comments

@paulmelnikow
Copy link

Hi there, and thanks for the great library!

I'm working on drf-to-s3, a server component for direct-to-S3 uploads based on Django REST Framework and designed for Fine Uploader.

My goal is to make it as difficult as possible for a malicious user to substitute her/his own content for another user's upload.

I'm handling this by prefixing the upload key with the username, and refusing to sign policy documents outside the user's namespace. This works okay, but requires integrators to configure objectProperties.key, and also requires that the front end have access to that username. It also makes anonymous uploads tricky to handle.

A more elegant solution would be to compare the object's ETag to the ETag provided at the end of the upload, to validate that no one has tampered with the contents.

Since I have users upload to a temporary bucket and then move the file to a storage bucket, I can use the x-amz-copy-source-if-match header to a copy request, which atomically will validate the etag and copy the file.

With Fine Uploader's recommended CORS policy, AWS sets the ETag header on the POST response, so there's shouldn't be a need to calculate anything on the client.

Basically I ask that, if the ETag is present in the POST response, you would please include it with the form data in the uploadSuccess callback.

@rnicholus
Copy link
Member

It looks like the ETag header is only returned in the response to multipart encoded POST requests that Fine Uploader S3 sends. These requests are only sent for non-chunked uploads. For chunked uploads, ideally, S3 would return an ETag in response to a Complete Multipart Upload request, which is sent by Fine Uploader S3 after the last part has been uploaded, asking S3 to combine these parts into a completed object. No such header exists on this response though. So, we can certainly include the ETag in the upload success POST, when available.

Regarding this comment:

I'm handling this by prefixing the upload key with the username, and refusing to sign policy documents outside the user's namespace. This works okay, but requires integrators to configure objectProperties.key, and also requires that the front end have access to that username. It also makes anonymous uploads tricky to handle.

Note that you don't need to include any key-determination logic client-side, if you don't want to. The objectProperties.key option accepts a promissory function as a return value. This is most appropriate if you want to delegate key name determination to your server, via an ajax request.

@rnicholus
Copy link
Member

It should be quite simple to do this for non-chunked uploads. In the develop branch, more of the code used to send this upload success request has been generalized during a refactor attached to the Fine Uploader Azure development. The parameters passed to the upload success endpoint are determined for the S3 module in the override of the general _getEndpointSpecificParams function. Currently, only the file ID is passed in. The general code that calls this function will need to additionally pass the associated XMLHttpRequest object so the S3 module can extract the ETag header from the response and include it as a parameter with the upload success POST.

Seems like, at most, 2 SPs.

@rnicholus rnicholus added this to the 4.3 milestone Feb 3, 2014
@rnicholus rnicholus self-assigned this Feb 3, 2014
@paulmelnikow
Copy link
Author

Sounds good, thanks! That's unfortunate about chunked uploads.

Regarding this:

Note that you don't need to include any key-determination logic client-side, if you don't want to. The objectProperties.key option accepts a promissory function as a return value. This is most appropriate if you want to delegate key name determination to your server, via an ajax request.

That's a great point, thank you. I like letting the server specify the key.

It reminds me of another enhancement I'd really like, which is that the client pull the key from the signed policy. I'll open a separate issue.

@rnicholus
Copy link
Member

Note that the client will not be able to extract any data from the signed policy without access to your secret key.

@paulmelnikow
Copy link
Author

I though the policy was just base64-encoded – am I getting that wrong?

@rnicholus
Copy link
Member

Yes, this could potentially be extracted from the base64-encoded policy, but not the signed policy. Also, decoding the base64 encoded policy in IE9 and older will require us to pull in Crypto.js, since these versions of IE don't implement atob.

@rnicholus
Copy link
Member

This has been implemented in the develop branch as part of 4.3.0-10. As previously mentioned, an etag property will be included with the upload success POST request for all non-chunked uploads in all browsers.

@paulmelnikow
Copy link
Author

Thank you! This is great!

@paulmelnikow
Copy link
Author

Hi there,

This is working well when <ExposeHeader>ETag</ExposeHeader> is set on the bucket's CORS policy.

When that value is not set the upload completes normally, but this message appears in the console:

Refused to get unsafe header "ETag" s3.jquery.fineuploader.js:6995
qq.extend._getEndpointSpecificParams s3.jquery.fineuploader.js:6995
qq.nonTraditionalBasePrivateApi._onComplete s3.jquery.fineuploader.js:6626
qq.uiPrivateApi._onComplete s3.jquery.fineuploader.js:4744
qq.extend._onComplete s3.jquery.fineuploader.js:8870
options.onComplete s3.jquery.fineuploader.js:2041
uploadCompleted s3.jquery.fineuploader.js:8392
(anonymous function) s3.jquery.fineuploader.js:8327

@rnicholus
Copy link
Member

That's correct. That ExposeHeader rule must be part of your CORS
configuration if you want the ETag passed to your signature handler for
XHR-based uploads. It's also required if you are using chunking with the
S3 uploader.

On Thu, Feb 13, 2014 at 11:05 AM, Paul Melnikow notifications@github.comwrote:

Hi there,

This is working well when ETag is set on the
bucket's CORS policy.

When that header is not set the upload completes normally, but this
message appears in the console:

Refused to get unsafe header "ETag" s3.jquery.fineuploader.js:6995
qq.extend._getEndpointSpecificParams s3.jquery.fineuploader.js:6995
qq.nonTraditionalBasePrivateApi._onComplete s3.jquery.fineuploader.js:6626
qq.uiPrivateApi._onComplete s3.jquery.fineuploader.js:4744
qq.extend._onComplete s3.jquery.fineuploader.js:8870
options.onComplete s3.jquery.fineuploader.js:2041
uploadCompleted s3.jquery.fineuploader.js:8392
(anonymous function) s3.jquery.fineuploader.js:8327

Reply to this email directly or view it on GitHubhttps://github.com//issues/1118#issuecomment-35000457
.

@paulmelnikow
Copy link
Author

If you don't need the ETag, is the console message expected?

@rnicholus
Copy link
Member

Yes. There is no configuration option in place to tell Fine Uploader whether your server is interested in the ETag or not. It attempts to grab the ETag in the response from AWS for all XHR-based uploads with the addition of this feature. The CORS spec infers that the ETag header is not a "simple" one, and therefore can only be accessed by the client in the context of a cross-origin request if the server explicitly allows it. When you add the ExposeHeader rule to your bucket's CORS XML configuration, this tells S3 to include an Access-Control-Expose-Headers header in its response, with the value of "ETag". Only then can Fine Uploader read the value of this response header.

We probably need to update the documentation to mention that this CORS rule is now needed for all XHR uploads, even if chunking is turned off. I suspect the vast majority of users already have chunking turned on if they are using Fine Uploader S3 (there is very little reason not to). So, while this was another unintended breaking change, it's a small one.

@paulmelnikow
Copy link
Author

Understood, thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants