diff --git a/README.md b/README.md
index 4a3e4e5..ad1920b 100644
--- a/README.md
+++ b/README.md
@@ -410,6 +410,8 @@ Used to customise the error response statusCode, the contained erro
By default if a csrf-csrf cookie already exists on an incoming request, generateCsrfToken will not overwrite it, it will return the existing token so long as the token is valid. If you wish to force a token generation, you can use the overwrite option of the third parameter:
+The validateOnReuse parameter is a bit misleading, and is also deprecated (will be removed with the next major release). A better name for it would be throwOnReuseInvalid.
+
```ts
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
```
diff --git a/src/index.ts b/src/index.ts
index 3b3be7c..c09845d 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -71,15 +71,19 @@ export function doubleCsrf({
const possibleSecrets = getPossibleSecrets(req);
// If cookie is not present, this is a new user (no existing csrfToken)
// If ovewrite is true, always generate a new token.
- // If overwrite is false and validateOnReuse is true and there is an existing token, validate it first
- // If overwrite is false and validateOnReuse is false, just return the cookie value
+ // If the existing token is valid, return it
+ // If the existing token is not valid and validateOnReuse is true, throw an error
+ // If the existing token is not valid and validateOnReuse is false, silently ignore, return new valid token
if (cookieName in req.cookies && !overwrite) {
- if (!validateOnReuse || (validateOnReuse && validateCsrfToken(req, possibleSecrets))) {
- // If validateOnReuse is false, or if the token is valid, reuse it
+ // If the current token is valid, reuse it
+ if (validateCsrfTokenCookie(req, possibleSecrets)) {
return getCsrfTokenFromCookie(req);
}
- // This only happens if overwrite is false and validateOnReuse is true
- throw invalidCsrfTokenError;
+
+ // If the current token is invalid and validateOnReuse is true, throw here
+ if (validateOnReuse) {
+ throw invalidCsrfTokenError;
+ }
}
// otherwise, generate a completely new token
// the 'newest' or preferred secret is the first one in the array
@@ -112,7 +116,38 @@ export function doubleCsrf({
return csrfToken;
};
- const getCsrfTokenFromCookie = (req: Request) => req.cookies[cookieName] as string;
+ const getCsrfTokenFromCookie = (req: Request) => req.cookies[cookieName] ?? "";
+
+ const validateHmac = ({
+ expectedHmac,
+ req,
+ randomValue,
+ possibleSecrets,
+ }: { expectedHmac: string; possibleSecrets: Array; randomValue: string; req: Request }) => {
+ const message = constructMessage(req, randomValue);
+ for (const secret of possibleSecrets) {
+ const hmacForSecret = generateHmac(secret, message);
+ if (expectedHmac === hmacForSecret) return true;
+ }
+
+ return false;
+ };
+
+ const validateCsrfTokenCookie = (req: Request, possibleSecrets: Array) => {
+ const csrfTokenFromCookie = getCsrfTokenFromCookie(req);
+ const [expectedHmac, randomValue] = csrfTokenFromCookie.split(csrfTokenDelimiter);
+
+ if (
+ typeof expectedHmac !== "string" ||
+ expectedHmac === "" ||
+ typeof randomValue !== "string" ||
+ randomValue === ""
+ ) {
+ return false;
+ }
+
+ return validateHmac({ expectedHmac, possibleSecrets, randomValue, req });
+ };
// given an array of secrets, checks whether at least one of the secrets constructs a matching hmac
const validateCsrfToken: CsrfTokenValidator = (req, possibleSecrets) => {
@@ -131,13 +166,7 @@ export function doubleCsrf({
// The reason it's safe for us to only validate the hmac and random value from the cookie here
// is because we've already checked above whether the token in the cookie and the token provided
// by the request are the same.
- const message = constructMessage(req, randomValue);
- for (const secret of possibleSecrets) {
- const hmacForSecret = generateHmac(secret, message);
- if (receivedHmac === hmacForSecret) return true;
- }
-
- return false;
+ return validateHmac({ expectedHmac: receivedHmac, req, possibleSecrets, randomValue });
};
const validateRequest: CsrfRequestValidator = (req) => {
diff --git a/src/tests/testsuite.ts b/src/tests/testsuite.ts
index 9b59f87..26c803e 100644
--- a/src/tests/testsuite.ts
+++ b/src/tests/testsuite.ts
@@ -10,7 +10,7 @@ import {
getCookieValueFromResponse,
switchSecret,
} from "./utils/helpers.js";
-import { generateMocks, generateMocksWithToken, next } from "./utils/mock.js";
+import { generateMocks, generateMocksWithToken, next, setCsrfCookieOnRequest } from "./utils/mock.js";
type CreateTestsuite = (
name: string,
@@ -171,7 +171,7 @@ export const createTestSuite: CreateTestsuite = (name, doubleCsrfOptions) => {
expect(generatedToken).not.toBe(csrfToken);
});
- it("should return existing CSRF token for a GET request that does not include the CSRF token", () => {
+ it("should return existing CSRF token for a GET request that does not send the CSRF token", () => {
const { mockRequest, mockResponse, csrfToken } = generateMocksWithTokenInternal();
mockRequest.method = "GET";
mockRequest.headers[HEADER_KEY] = undefined;
@@ -179,6 +179,21 @@ export const createTestSuite: CreateTestsuite = (name, doubleCsrfOptions) => {
expect(reusedToken).toBe(csrfToken);
});
+
+ it("should return a new and valid CSRF token for a GET request that does not send the CSRF token or have the CSRF cookie", () => {
+ const { mockRequest, mockResponse, csrfToken } = generateMocksWithTokenInternal();
+ mockRequest.method = "GET";
+ mockRequest.headers[HEADER_KEY] = undefined;
+ mockRequest.headers.cookie = `${cookieName}=`;
+ mockRequest.cookies[cookieName] = undefined;
+
+ const newToken = generateCsrfToken(mockRequest, mockResponse);
+ setCsrfCookieOnRequest(mockRequest, mockResponse, cookieName);
+ expect(newToken).not.toBe(csrfToken);
+ expect(validateRequest(mockRequest)).toBe(false);
+ mockRequest.headers[HEADER_KEY] = newToken;
+ expect(validateRequest(mockRequest)).toBe(true);
+ });
});
describe("validateRequest", () => {
diff --git a/src/tests/utils/mock.ts b/src/tests/utils/mock.ts
index 8aee0e2..ec87d99 100644
--- a/src/tests/utils/mock.ts
+++ b/src/tests/utils/mock.ts
@@ -78,6 +78,18 @@ export type GenerateMocksWithTokenOptions = {
sessionIdentifier?: string;
};
+export const setCsrfCookieOnRequest = (req: Request, response: Response, cookieName: string) => {
+ const { setCookie, cookieValue } = getCookieValueFromResponse(response);
+ req.headers.cookie = `${cookieName}=${cookieValue};`;
+
+ (req as any).cookies = undefined;
+ cookieParserMiddleware(req, response, next);
+ return {
+ setCookie,
+ cookieValue,
+ };
+};
+
// Generate the request and response mocks.
// Set them up as if they have been pre-processed in a valid state.
export const generateMocksWithToken = ({
@@ -89,15 +101,12 @@ export const generateMocksWithToken = ({
const { mockRequest, mockResponse, mockResponseHeaders } = generateMocks(sessionIdentifier);
const csrfToken = generateCsrfToken(mockRequest, mockResponse);
- const { setCookie, cookieValue } = getCookieValueFromResponse(mockResponse);
- mockRequest.headers.cookie = `${cookieName}=${cookieValue};`;
+ const { setCookie, cookieValue } = setCsrfCookieOnRequest(mockRequest, mockResponse, cookieName);
const decodedCookieValue = decodeURIComponent(cookieValue);
// Have to delete the cookies object otherwise cookieParser will skip it's parsing.
// After removing @types/cookie-parser and relying on cookie-parser provided types
// the types prevent cookies from being undefined despite it being valid for cases
// before the middleware runs.
- (mockRequest as any).cookies = undefined;
- cookieParserMiddleware(mockRequest, mockResponse, next);
expect(getCookieFromRequest(cookieName, mockRequest)).toBe(decodedCookieValue);
mockRequest.headers[HEADER_KEY] = csrfToken;