Ability to vary expiration time on forms
holli opened this issue · 20 comments
Currently these helpers result in a form that is valid for 5 minutes. Thats quite rough.
Case: User comes to form. Spends 10 minutes on finding 5 photos to upload. Hits upload -> suddenly the upload is invalid because form was valid only 5 minutes.
I'd need the follow to work:
<%= transloadit :upload_basics, {:auth => [:expires => 1.hour.from_now]} %>
Easiest would be to change view_helper to change the hash auth params before signing it. More clearlier fix would be to change how ruby-sdk work and change this one accordingly.
Hi holli,
thanks. Agreed, we need to make this work. We'll tackle it.
--fg
The timeout is actually set in the ruby-sdk [https://github.com/transloadit/ruby-sdk/blob/master/lib/transloadit.rb#L112]
As we got more that one request to change the default timeout it might me a good idea to raise it to something more sensible. Especially on multiupload forms it would be good to have something like 10 / 15 minutes.
We have to think about how to expose this to the rails-sdk so someone can set that in the helper and maybe as config option in the transloadit.yml.
@stouset I don't have enough time at the moment, but I'm open for discussion if you have any ideas about that.
I'll work on getting a fix out by this evening. Totally agreed that the timeout should be configurable (and perhaps an increased default).
@stouset awesome! Please feel free to bump the default timeout to 30 minutes, that should give most users enough time to pick a file while not making the signature/token too powerful.
My plan is to leave the default for the Ruby SDK as it is (since I imagine most users will issue requests immediately after signing them) while making it configurable, and let the Rails SDK override the default.
Okay. The value is configurable in the newly-pushed version of the Ruby SDK. I don't have time to fix the Rails side of things tonight, but I'll tackle it in the morning.
A fix has been pushed.
I'm doing some manual tests right now to ensure it works as intended. Before I release the change, I'd like to confirm that it works for you as well, @holli. Can you check that the duration is configurable (via config/transloadit.yml) in the latest version of transloadit-rails in git? You will also need the latest released version of the transloadit gem.
Also, is it enough for you to be able to set the timeout duration in the config file? Or do you require it to be settable from the helper as well? I'm hesitant to expose something like that all the way out to the view — it doesn't seem like it would be all that necessary to customize the timeouts per-page.
Cool. Fast response time :)
I can check the software in couple days when I'm next to the developer tools again.
In my case: I probably set duration to a very long time and have my own authentication where needed. I just need signing so that users wont alter the parameters. So your solution is enough and probably is enough for others.
I don't see any problem in setting it in helper. But I can easily think of example of need of customization:
- A upload page with selecting multiple photos alongside possibility to edit comments. Page behind custom login. I want to provide possibility to start the process, go to lunch, come back, continue and send the files. Takes easily a long time (hours).
- A single profile photo upload page. Job is done in couple minutes anyway.
It's probably not worth the hassle to do it right now, unless somebody in particular needs it.
Security-wise, what's the necessity for short-duration expiry times on upload forms? Might it just make sense to change the default to 24h? Or does that expose either the website owner or Transloadit to potential attacks?
Since the signatures do not cover the uploaded files, a user could take a signature / params combo and re-upload any amount of data for the validity duration of the token as often as he likes. So this exposes the website owner to potential costs by from an evil user.
Transloadit would handle this case by refunding the customer, but not setting the expiration times longer than needed is the best prevention method at this point. I'm open to ideas on this so.
What about using a nonce? Any form could then only be uploaded once. You'd need to remember used nonces on the Transloadit end, but could purge them after their expiry time has passed.
Even then, it seems like no approach here actually provides that much security. A user merely has to refresh the page over and over to get as many new nonces (or fresh expiry dates) as he/she wants.
Yeah, new code works for me :)
One small fix in Readme.md. value 30 * 60 won't evaluate by itself. So in the example it would be nicer to use numbers.
Readme.md:
auth:
...
duration: 1800 # 1800 = 30 * 60 = 30 minute validity period for signed upload forms
@felixge thanks for explaining. I was wondering why there was time limit at all. In the long run there might be some wiser ways to tackle that problem. Maybe @stouset's nonce suggestion.
Oops. Fixed.
What about using a nonce? Any form could then only be uploaded once. You'd need to remember used nonces on the Transloadit end, but could purge them after their expiry time has passed.
How would you generate those without collisions?
UUIDs, most likely.
UUIDs, most likely.
I guess that's doable ... but it makes the authentication schema more difficult ... . But yeah, maybe we should reconsider that at some point.
Yeah. Plus it doesn't solve the problem entirely — it just makes it more difficult to exploit. At any rate, I'm going to close this bug and push a new release of the plugin.