search

emartech / escher-js

Library for HTTP request signing (JavaScript implementation)
MIT License
12 stars 11 forks source link

Prevent caching of date in constructor #7

Closed wolfika closed 7 years ago

wolfika commented 7 years ago

This change allows Escher to sign and validate signature against the current date, unless a date is provided explicitly to the constructor. The date is resolved when accessing the date property using an ES5 property getter. If a date is specified when creating the Escher object, the explicitly given date is saved in an internal config variable.

refs #5

sonicoder86 commented 7 years ago

LGTM

peter-vasarhelyi commented 7 years ago

LGTM, but would be happy to see a corresponding test

boogie commented 7 years ago

I think this is not solving the issue, also the original code was much more clean and doing the same.

The problem is not that we have to prevent caching. We must have a "constant" date during calculating the signature at different parts of the code, but it should be different (the actual date) on every call of the public methods. The tests are passed, because all of them have a fixed date. An "end-to-end" test might solve the problem of testing: creating two signatures with the same input (but different dates), and checking if they are different, but both of them are valid.

Also, the solution should be thread safe, so I think we have to pass the date through every method where we are using it.

felin-arch commented 7 years ago

I agree w/ @boogie. Merging this may cause problems. We included this issue in our current sprint and plan to solve it by 2017.10.10.

@wolfika, thank you for putting in the effort to provide a PR. It gave us a nudge to do something about this issue. I am closing this PR for now.