-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PP-13360 csrf express middleware #1314
Conversation
33dbf7e
to
74ecebb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work, just a few thoughts
*/ | ||
const checkToken = (req, res, next) => { | ||
const csrfSecret = req[sessionKey][secretKey] | ||
const csrfToken = (req.body && req.body[tokenKey]) || (req.query && req.query[tokenKey]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it be possible to pass the csrf token in a query paramter? since the validation is only enforced on PUT or POST operations I think it should only ever be expected in the body, unless there's a case where we are passing in a query parameter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
glad you asked :D, in the case of multipart/form-data
encoded POSTs we don't have (easy) access to the body when validating the token so the token is passed as a query parameter. Basically anywhere we use a file
input type
9f1595d
to
9a153c6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work
WHAT