Skip to content

Commit bcaf1c3

Browse files
committed
fix: when validateOnReuse was false, behaviour was incorrect
Prior to this fix, if you set validateOnReuse to false, this only prevented the error from being thrown, it did not actually prevent validation from happening. This means if you were to make a GET request without including the token in the request and you were hoping to have the existing CSRF token included in the response, it wasn't really possible, a new one would be generated every time. It should not be necessary to included the CSRF token in GET requests. Now when validateOnReuse is false, the existing CSRF token will be reused from the CSRF token cookie, without prior validation. This is a semi breaking change, but it is a correction of expected behaviour.
1 parent d5705f0 commit bcaf1c3

File tree

4 files changed

+26
-22
lines changed

4 files changed

+26
-22
lines changed

README.md

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -403,7 +403,7 @@ Used to customise the error response <code>statusCode</code>, the contained erro
403403
{
404404
cookieOptions?: CookieOptions, // allows overriding of cookieOptions
405405
overwrite?: boolean, // Set to true to force a new token to be generated
406-
validateOnReuse?: boolean, // Set to true to throw an error when overwrite is false and the current CSRF token is invalid
406+
validateOnReuse?: boolean, // Deprecated, leave as default
407407
} // optional
408408
) => string;
409409
```
@@ -414,33 +414,31 @@ Used to customise the error response <code>statusCode</code>, the contained erro
414414
generateCsrfToken(req, res, { overwrite: true }); // This will force a new token to be generated, and a new cookie to be set, even if one already exists
415415
```
416416

417-
<p>If the <code>overwrite</code> parameter is set to <em>false</em> (default), the existing token will be re-used and returned. If the current / existing CSRF token is not valid, then a new token will be generated without any error being thrown. If you want the <code>generateCsrfToken</code> to throw an error instead, provide the <code>validateOnReuse: true</code> option.</p>
417+
<p>If the <code>overwrite</code> parameter is set to <em>false</em> (default), the existing token will be re-used and returned.</p>
418418

419419
<p>If <code>overwrite</code> is <em>true</em> a new token will always be generated, even if the current one is invalid.</p>
420420

421421
```ts
422-
generateCsrfToken(req, res, { overwrite: true }); // As overwrite is true, an error will never be thrown.
423-
generateCsrfToken(req, res, { overwrite: false }); // As validateOnReuse is false (default), if the current CSRF token from the cookie is invalid, a new token will be generated without any error being thrown.
422+
generateCsrfToken(req, res, { overwrite: true }); // As overwrite is true a new CSRF token will be generated.
423+
generateCsrfToken(req, res, { overwrite: false }); // As overwrite is false, the existing CSRF token will be reused from the CSRF token cookie
424424
generateCsrfToken(req, res); // same as previous
425-
generateCsrfToken(req, res, { overwrite: false, validateOnReuse: true }); // As validateOnReuse is true, if the CSRF token from the cookie is invalid, a new token will be generated without an error being thrown.
425+
generateCsrfToken(req, res, { overwrite: false, validateOnReuse: true }); // DEPRECATED - As validateOnReuse is true, if the CSRF token from the cookie is invalid, an error will be thrown
426426
```
427427

428428
<p>Instead of importing and using <code>generateCsrfToken</code>, you can also use <code>req.csrfToken</code> any time after the <code>doubleCsrfProtection</code> middleware has executed on your incoming request.</p>
429429

430430
```ts
431431
req.csrfToken(); // same as generateCsrfToken(req, res);
432-
req.csrfToken({ overwrite: true }); // same as generateCsrfToken(req, res, { overwrite: true, validateOnReuse });
433-
req.csrfToken({ overwrite: false, validateOnReuse: false }); // same as generateCsrfToken(req, res, { overwrite: false, validateOnReuse: false });
432+
req.csrfToken({ overwrite: true }); // same as generateCsrfToken(req, res, { overwrite: true });
434433
req.csrfToken(req, res, { overwrite: false });
435-
req.csrfToken(req, res, { overwrite: false, validateOnReuse: false });
436434
```
437435

438-
<p>The <code>generateCsrfToken</code> function serves the purpose of establishing a CSRF protection mechanism by generating a token and an associated cookie. This function also provides the option to utilise a third parameter, which is an object that may contain: <code>overwrite</code>, <code>validateOnReuse</code>, or <code>cookieOptions</code>. By default, <code>overwrite</code> and <code>validateOnReuse</code> are both set to <em>false</em>. <code>cookieOptions</code> if not provided will just default to the options originally provided to the initialisation configuration, any options that are provided will override those initially provided.</p>
436+
<p>The <code>generateCsrfToken</code> function serves the purpose of establishing a CSRF protection mechanism by generating a token and an associated cookie. This function also provides the option to utilise a third parameter, which is an object that may contain: <code>overwrite</code>, and <code>cookieOptions</code>. By default, <code>overwrite</code> is set to false. <code>cookieOptions</code> if not provided will just default to the options originally provided to the initialisation configuration, any options that are provided will override those initially provided.</p>
439437
<p>It returns a CSRF token and attaches a cookie to the response object. The cookie content is <code>`${hmac}${csrfTokenDelimiter}${randomValue}`</code>.</p>
440438
<p>In some cases you should only transmit your token to the frontend as part of a response payload. Consult the <a href="./FAQ.md#do-i-need-csrf-csrf">"Do I need csrf-csrf?"</a> and <a href="./FAQ.md#does-httponly-have-to-be-true">"Does httpOnly have to be true?"</a> sections of the FAQ.</p>
441439
<p>When <code>overwrite</code> is set to <em>false</em>, the function behaves in a way that preserves the existing CSRF token. In other words, if a valid CSRF token is already present in the incoming request cookie, the function will reuse the existing CSRF token.</p>
442440
<p>If <code>overwrite</code> is set to <em>true</em>, the function will generate a new token and cookie each time it is invoked. This behavior can potentially lead to certain complications, particularly when multiple tabs are being used to interact with your web application. In such scenarios, the creation of new cookies with every call to the function can disrupt the proper functioning of your web app across different tabs, as the changes might not be synchronised effectively (you would need to write your own synchronisation logic).</p>
443-
<p>If overwrite is set to <em>false</em>, the function will also validate the existing cookie information. If the information is found to be invalid, a new token will be generated and returned. If you want an error to be thrown when validation fails during generation you can set the <code>validateOnReuse</code> (by default, <em>false</em>) to <em>true</em>. If it is <em>true</em> then an error will be thrown instead of a new token being generated.</p>
441+
<p>If overwrite is set to <em>false</em>, the function will return the existing CSRF token from the existing CSRF token cookie.</p>
444442

445443
<h3>invalidCsrfTokenError</h3>
446444

src/index.ts

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -69,21 +69,15 @@ export function doubleCsrf({
6969
const possibleSecrets = getPossibleSecrets(req);
7070
// If cookie is not present, this is a new user (no existing csrfToken)
7171
// If ovewrite is true, always generate a new token.
72-
// If overwrite is false and there is no existing token, generate a new token.
73-
// If overwrite is false and there is an existin token then validate the token and hash pair
74-
// the existing cookie and reuse it if it is valid. If it isn't valid, then either throw or
75-
// generate a new token based on validateOnReuse.
72+
// If overwrite is false and validateOnReuse is true and there is an existing token, validate it first
73+
// If overwrite is false and validateOnReuse is false, just return the cookie value
7674
if (cookieName in req.cookies && !overwrite) {
77-
if (validateCsrfToken(req, possibleSecrets)) {
78-
// If the token is valid, reuse it
75+
if (!validateOnReuse || (validateOnReuse && validateCsrfToken(req, possibleSecrets))) {
76+
// If validateOnReuse is false, or if the token is valid, reuse it
7977
return getCsrfTokenFromCookie(req);
8078
}
81-
82-
if (validateOnReuse) {
83-
// If the pair is invalid, but we want to validate on generation, throw an error
84-
// only if the option is set
85-
throw invalidCsrfTokenError;
86-
}
79+
// This only happens if overwrite is false and validateOnReuse is true
80+
throw invalidCsrfTokenError;
8781
}
8882
// otherwise, generate a completely new token
8983
// the 'newest' or preferred secret is the first one in the array

src/tests/testsuite.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,15 @@ export const createTestSuite: CreateTestsuite = (name, doubleCsrfOptions) => {
170170
expect(newCookieValue).not.toBe(oldCookieValue);
171171
expect(generatedToken).not.toBe(csrfToken);
172172
});
173+
174+
it("should return existing CSRF token for a GET request that does not include the CSRF token", () => {
175+
const { mockRequest, mockResponse, csrfToken } = generateMocksWithTokenInternal();
176+
mockRequest.method = "GET";
177+
mockRequest.headers[HEADER_KEY] = undefined;
178+
const reusedToken = generateCsrfToken(mockRequest, mockResponse);
179+
180+
expect(reusedToken).toBe(csrfToken);
181+
});
173182
});
174183

175184
describe("validateRequest", () => {

src/types.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,9 @@ export type CsrfErrorConfig = {
3737
export type CsrfErrorConfigOptions = Partial<CsrfErrorConfig>;
3838
export type GenerateCsrfTokenConfig = {
3939
overwrite: boolean;
40+
/**
41+
* @deprecated leave this as the default value, to be removed in the future
42+
*/
4043
validateOnReuse: boolean;
4144
cookieOptions: CsrfTokenCookieOptions;
4245
};

0 commit comments

Comments
 (0)