expressjs / csurf

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

how to handle csurf function not being available on req object when a session store goes down #60

Open kellyrmilligan opened 9 years ago

kellyrmilligan commented 9 years ago

Wondering your thoughts on this. recently, my redis provider went down for a sec, and sent my app into a downward spiral of death. i've since added in better error handling for when the session isn't available, and also a check to see if first the req.csrfToken() function exists, so the app won't crash. is that pretty much the extent of it?

if (req.csrfToken) {
        state.csrf = req.csrfToken();
      } else {
        next(new Error('no csrftoken function'));
      }

and for sessions:

app.use(function (req, res, next) {
    if (!req.session) {
      return next(new Error('session not defined'));
    }
    next();
  });
dougwilson commented 9 years ago

That's an interesting question, actually. It seems currently if your store is offline, then this module will pass the misconfigured csrf error down your pipeline to just 500. So, clearly if you are getting a POST and need to validate a token or add a token to a view, then you can't do much but 500, but otherwise, you could carry on, but doing a global app.use(csurf()) would prevent that from happening...

Your currently solution sounds fine to me, but I think we need to do something here to make this ess of a footgun.

dougwilson commented 9 years ago

What we can do is if the request doesn't qualify as one that needs validation (i.e. is a GET and not like a POST), then we can delay the error until the req.csrfToken() call. This would mean that even if you are using a global csurf(), requests that have nothing to do with CSRF tokens will still work out-of-the-box.