expressjs / csurf

CSRF token middleware
MIT License
2.3k stars 216 forks source link

Csurf-expire patch #160

Open x24git opened 5 years ago

x24git commented 5 years ago

Added in functionality that allows for the read in of the "max age" option for a cookie (if being created with cookies not sessions). If the cookie is expired, then we will reject the token. Puts up a minor defence against storing cookie and tokens and replaying them days later.

Rather than store cookies in a database, we can simply apend the expiry time to the cookie and obfuscate the value so its not completely obvious what it represents. We can then generate the XSRF token like normal. When checking the cookie(secret) we decode the time and compare it to the currect time to determine if it has expired.

Was thinking that rather than automatically enabling this feature if a user sets the MaxAge property on a cookie, it may be prudent to add a seperate options flag. I am open to any suggestions on how to improve this functionality.

Thank you for your time and consideration

dougwilson commented 5 years ago

Thanks! I will review shortly.

jcasner commented 5 years ago

Any ETA on this? Thanks!

jayflo commented 4 years ago

I've seen two problems with the csurf cookie:

  1. Lack of maxAge option
  2. Lack of "rolling" option

Having (1) certainly helps, but having a fixed maxAge can still lead to problems. If a user is on a website for a long period of time, the csrf secret cookie can expire while they're using the site...which leads to a random EBADCSRFTOKEN error.

It would be nice to have a "rolling" feature as in express-session (https://www.npmjs.com/package/express-session#rolling). This way the cookie is extended, or regenerated, every time the csurf middleware is executed. This would make it very easy to sync the csrf functionality with login sessions.

I currently handle this manually in the app code by setting the cookie maxAge on each request. But it would be nice if this were simply an option (would be little code on top of what is in the PR).