expressjs / csurf

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

please document the `signed` config option #138

Closed adon-at-work closed 6 years ago

adon-at-work commented 6 years ago

From the code, we can see that signedCookies are supported for _csrf with a secret located at req.secret https://github.com/expressjs/csurf/blob/master/index.js#L272-L281

Would be nice if this feature can be documented (and even encouraged) in README. Thanks.

dougwilson commented 6 years ago

Pull request to add documentation welcome :+1:

dougwilson commented 6 years ago

I took a look to just add it myself and it's already there in the README.

Since the request is to document it and it's already documented, I'm going to close the issue. If you would like it documented differently, that's OK and you can help us understand why you don't think it's currently documented and how it can be better (I usually suggest making a PR in this case, as usually that is the most straight forward way to get your thoughts across).

adon-at-work commented 6 years ago

I guess you're referring to the last bullet, which sends the user to http://expressjs.com/en/4x/api.html#res.cookie

When set to an object, cookie storage of the secret is enabled and the object contains options for this functionality (when set to true, the defaults for the options are used). The options may contain any of the following keys:

  • key - the name of the cookie to use to store the token secret (defaults to '_csrf').
  • path - the path of the cookie (defaults to '/').
  • any other res.cookie option can be set.

That's pretty indirect and obscure to me at least. One has to visit doc of expressjs, then click thru to cookie-parser before knowing that req.secret is the location. I'm trying to get a consent before raising a PR (i guess it's easy for anyone to do that)

p.s. path is however in this README despite it's part of the "other res.cookie option"

dougwilson commented 6 years ago

You already have consent to make a PR to add whatever documentation you think would be helpful 👍 The only ones documented here directly and the ones in which the default values differ from res.cookie (because this module contains code to override them). Since it just uses the same underlying code, it used to get out of sync constantly, so it just sends the users to a link, which is why it's that way currently, as a user make a PR suggesting that was better than a out-of-date copy-paste.

If you have thoughts / ideas on how it should be and if we're going to copy-and-paste parts of other documentation what the method will be to keep it updated over time I'm all ears 👍 the doc is the way that someone came by and suggested, and I don't care either way, haha. Whatever is most useful to folks.