Add utilities to prevent cookie tossing and replay attacks #2
Conversation
|
Note: on another PR we might need to adress the move from |
| @@ -131,15 +174,50 @@ Tokens.prototype.verify = function verify (secret, token) { | |||
| } | |||
|
|
|||
| var index = token.indexOf('-') | |||
There was a problem hiding this comment.
Maybe in another PR, but wouldn't it be clearer to var tokenParts = token.split('-')?
There was a problem hiding this comment.
possibly, I would like to ship this and the linked change in fastify-csrf first.
Co-authored-by: James Sumners <[email protected]>
Co-authored-by: James Sumners <[email protected]>
Co-authored-by: James Sumners <[email protected]>
index.js
Outdated
| var validity = Number.isInteger(opts.validity) === true | ||
| ? opts.validity | ||
| : 0 | ||
|
|
||
| if (typeof validity !== 'number' || !isFinite(validity) || validity < 0) { | ||
| throw new TypeError('option validity must be finite number > 0') | ||
| } |
There was a problem hiding this comment.
I think this if (typeof... become redundant by the Number.isInteger, don't you?
There was a problem hiding this comment.
I think the code is wrong right now. We are casting strings to zero instead of throwing. The code was correct as it was before.
There was a problem hiding this comment.
Please explain. Number.isInteger only returns true for integer primitives.
There was a problem hiding this comment.
As the code is right now, https://github.com/fastify/csrf/pull/2/files#diff-a561630bb56b82342bc66697aee2ad96efddcbc9d150665abd6fb7ecb7c0ab2fR56 is failing. If validity is a string, we cast it as 0.
There was a problem hiding this comment.
Ah, it's the word "cast" that is confusing me. No casting is taking place; if a string is supplied then it is ignored and the default value, 0, is used instead.
When I left my change suggestion I had not reached the if (typeof ... yet. I saw the possibility of a non-integer being supplied for the option and suggested an easy change to ignore such cases.
Checklist
npm run testandnpm run benchmarkand the Code of conduct