Skip to content
This repository has been archived by the owner on Sep 14, 2022. It is now read-only.

Csurf-expire patch #160

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Csurf-expire patch #160

wants to merge 2 commits into from

Conversation

x24git
Copy link

@x24git x24git commented Sep 6, 2018

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

@x24git x24git force-pushed the csurf-patch branch 6 times, most recently from 990f22c to a9ad8d3 Compare September 6, 2018 05:44
@dougwilson
Copy link
Contributor

Thanks! I will review shortly.

@jcasner
Copy link

jcasner commented Feb 14, 2019

Any ETA on this? Thanks!

@dougwilson dougwilson added this to the 2.0.0 milestone Apr 22, 2019
@jayflo
Copy link

jayflo commented Aug 5, 2019

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).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants