benjreinhart / react-native-aws3

Pure JavaScript React Native library for uploading to AWS S3
MIT License
399 stars 151 forks source link

presigning would be smarter to support #32

Open sebringj opened 7 years ago

sebringj commented 7 years ago

Currently, I don't need to put my credentials onto any phone because I use presigning from a network request that gives temporary permission to the person uploading with a namespaced bucket. That is a smarter approach in the long run.

Thanks for the code, will just modify it to use signing then do a PR if you want it, take it.

baptwaels commented 7 years ago

It would be great ! 👍 Thanks

benjreinhart commented 7 years ago

Yes, it is a better approach, at least in terms of security. I have supplied my thoughts on this in prior closed issues, mainly that I use IAM roles to create a user with as few privileges as possible so that any credential leak would have as limited impact as possible.

Specifically, I have a role that has only PutObject/PutObjectAcl to one sub directory in my S3 bucket. As far as I can tell, this means absolute worse case scenario is that the credentials could be used to overwrite existing files in that particular folder if they knew the file names (file names in my bucket have timestamps, UUIDs or both). Though it's possible I'm missing something else.

So given that, I'm not particularly concerned at the moment. That being said, I do agree that in general, this is still probably not a very safe stance to take on the matter. I built it this way so that I didn't have to deal with the complexity of making requests to a server first to generate the policies and handling any errors that could arise from those network requests. In other words, I chose simplicity/perf over potential security issues when I original built this.

All that being said, I agree this should be supported. I'm pretty sure one of the main reasons for this library having as many forks as it does is because of people who wanted this functionality. I haven't done this yet because I haven't been in React Native world lately and it'll take me some time to get this setup and tested to make sure it'll work correctly. I also wanted to have tests for this library before I kept making more changes, which I finally got around to doing this past weekend. So hopefully I'll have some time to get this rolled out shortly.

sebringj commented 7 years ago

I ended up making something that does this and it is pretty simple. https://github.com/sebringj/siggy but your code was helpful seeing the specific AJAX post setting for "upload" that I was missing to get progress.

benjreinhart commented 7 years ago

Cool, thanks.

This library currently only uses the browser based upload approach but I would like to switch over to the standard AWS Signature v4 signing which is more flexible. I have a branch on this repo from a while back that was attempting to get that working but ran out of time before I got it fully working. That would allow us to get/put/delete objects rather than only upload. When I have that working, I can also extract the policy generation module which is already pretty standalone within this repo, and then it could be used on the server with node.

sebringj commented 7 years ago

thanks, that sounds good. I do use this in my React Native app zipstory (ios only right now) and I just upload videos and images directly to S3 via the device after presigning so it avoids server congestion. You can just set headers and works regardless of if browser or not but will be better to do updated way as you suggest.

broskoski commented 7 years ago

+1

nedmac101 commented 6 years ago

+1