jasonsims / aws-cloudfront-sign

Utility module for AWS CloudFront
MIT License
176 stars 80 forks source link

Default `expireTime` is short and can cause `Access Denied` due to server clockdrift #29

Closed fanktom closed 7 years ago

fanktom commented 7 years ago

Hey there,

thanks for packing this up into this neat bundle. I just spent half a day figuring out why my signed link does not work.

All looked peachy, but CloudFront would return AccedDenied. After trying out the official go implementation (it also contains the sign code, and uses a default expire time of +1 h) I knew it wasn't related to my CF setup but something else.

When I finally increased the expireMethod to 5 minutes, it instantly worked like a charm.

Hence, I propose to increase the default expireTime from 30 seconds to at least something in the minutes range. It is not seldom for some servers to exhibit +20s clock drift and other APIs (official ones from AWS) start with much higher defaults.

What do you think?

jasonsims commented 7 years ago

Yeah I agree with bumping it up. 30 seconds is pretty useless haha

jasonsims commented 7 years ago

Can you submit a PR switching it to some other reasonable default? Syncing it up with what other libraries are doing, like the go one you tried, might make sense.

fanktom commented 7 years ago

Sure, will do on the weekend and come back to you. Thanks!

fanktom commented 7 years ago

30 I increased the expireTime to 30 minutes. I think this is a sensible value based on the other SDKs. What do you think?

jasonsims commented 7 years ago

Resolved by #30. Thanks @fanktom!

VladimirPal commented 7 years ago

@jasonsims @fanktom Hi guys! You changed the code and docs, but you didn't update npm package, so i read documentation and it says expireTime 30 minutes, but in fact it's 30 seconds.

fanktom commented 7 years ago

Sorry @VladimirPal, I don't have the access rights to update npm. @jasonsims Might have forgotten to push the update online, but surely will do soon? :)