Add credentials warning to documentation

[franciscop]

When doing a POST or PUT request with the new-ish fetch() API:

fetch('/URL', {
  method: 'PUT',
  body: 'whatever',
  headers: { 'csrf-token': csrf }
}).then(res => res.json()).then(res => {
  console.log(res);
});

I receive a typical error of:

ERROR { ForbiddenError: invalid csrf token
...
code: 'EBADCSRFTOKEN' }

After many hours searching the web and debugging it (since it is a hard-to-debug issue if you don't know why), I found out that this is because fetch() does not send the current session. This can be solved adding the credentials: 'include' key-value pair to the fetch() request:

fetch(`/todo/${e.target.id}`, {
  method: 'PUT',
  body: done,
  credentials: 'include',
  headers: { 'csrf-token': csrf }
}).then(res => res.json()).then(res => {
  console.log(res);
});

However Google returns really bad results for this as there are many similar errors. I think more people might be having this issue and it'll be more and more common as fetch() becomes more popular, and it's not something that will be easy to debug for people new to csurf.

So I propose to add a warning to the documentation, since fetch is an standard and it doesn't work without adding this (it creates a new session).

[dougwilson]

This module just depends on req.session being there; it doesn't provide the mechanism for how req.session works or anything, and there are quite a few different ways this is populated, for example using OAuth tokens and more. If your issue was that the session wasn't being there, and to get your session module to work was to set that, then perhaps it is the module that is managing your sessions that should be adding that documentation?

[franciscop]

While that is completely true, the issue that is displayed to the user is still "Invalid CSRF token" and from an external point of view req.sessions would not be too high in the list of places where I'd start debugging.

Maybe then we could add a "common errors" / "debugging csurf" or similar section in the end of the README so people can just add their problems+solutions? Otherwise I'll just rename this issue+change the original one a bit so at least people searching for this can find it.

[dougwilson]

Sure, but then every module on npm that uses req.session should also add the same information. If that is the case, I'm just going to remove support for using req.session today.

[dougwilson]

Actually, if the scenario is what you describe, we should be able to provide a different error: that you tried to validate a CSRF token when no CSRF secret has been established yet. Thus being a different error, you can better surmise that your issue is with your session module. Does that sound like a better solution?

[franciscop]

Sure! That sounds way better, I didn't think that kind of detection was possible but now reading the code I can see it. You'd be detecting that there is no existing CSRF and that there is a provided one, so an error would be displayed, right?

Would it go just before this?

if (!secret) {

  //HERE? Check if there is value(req)?

  secret = tokens.secretSync()
  setSecret(req, res, sessionKey, secret, cookie)
}

[dougwilson]

Yep, essentially correct. Would need to do some extra refactoring to take into account the ignore methods (and ideally not invoke the value function twice).

[dougwilson]

Because if a token is incoming and a new secret is also being generated, there is certainly no way that the token will match the secret, so even trying to match is likely a waste and I figure providing a different error in that case will be better than the single error.