diff --git a/README.md b/README.md index e8b4bd5..a52d91d 100644 --- a/README.md +++ b/README.md @@ -55,7 +55,7 @@ Do follow fastify/csrf-protection recommendations for secret security.
  • - Do keep secure and signed as true in production. + Do keep secure as true in production.
  • Do make sure you do not compromise your security by not following best practices. @@ -79,9 +79,9 @@ This section will guide you through using the default setup, which does sufficiently implement the Double Submit Cookie Pattern. If you'd like to customise the configuration, see the configuration section.

    - You will need to be using cookie-parser and the middleware should be registered before Double CSRF. This utility will set a cookie containing a hash of the csrf token and provide the non-hashed csrf token so you can include it within your response. + You will need to be using cookie-parser and the middleware should be registered before Double CSRF. In case you want to use signed CSRF cookies, you will need to provide cookie-parser with a unique secret for cookie signing. This utility will set a cookie containing both the csrf token and a hash of the csrf token and provide the non-hashed csrf token so you can include it within your response.

    -

    Requires TypeScript >= 3.8

    +

    If you're using TypeScript, requires TypeScript >= 3.8

    ``` npm install cookie-parser csrf-csrf @@ -98,7 +98,7 @@ const { doubleCsrf } = require("csrf-csrf"); ```js const { invalidCsrfTokenError, // This is just for convenience if you plan on making your own middleware. - generateToken, // Use this in your routes to provide a CSRF hash cookie and token. + generateToken, // Use this in your routes to provide a CSRF hash + token cookie and token. validateRequest, // Also a convenience if you plan on making your own middleware. doubleCsrfProtection, // This is the default CSRF protection middleware. } = doubleCsrf(doubleCsrfOptions); @@ -108,7 +108,7 @@ const { This will extract the default utilities, you can configure these and re-export them from your own module. You should only transmit your token to the frontend as part of a response payload, do not include the token in response headers or in a cookie, and do not transmit the token hash by any other means.

    - To create a route which generates a CSRF token and hash cookie: + To create a route which generates a CSRF token and a cookie containing ´${token|tokenHash}´:

    ```js @@ -179,6 +179,31 @@ const doubleCsrfUtilities = doubleCsrf({

    If you plan on using express-session then please ensure your cookie-parser middleware is registered after express-session, as express session parses it's own cookies and may cionflict.

    +

    generateToken

    + +```ts +(response: Response, request: Request, overwrite?: boolean) => string; +``` + +

    By default if a csrf-csrf cookie already exists on an incoming request, generateToken will not overwrite it, it will simply return the existing token. If you wish to force a token generation, you can use the third parameter:

    + +``` +generateToken(res, req, true); // This will force a new token to be generated, and a new cookie to be set, even if one already exists +``` + +

    Instead of importing and using generateToken, you can also use req.csrfToken any time after the doubleCsrfProtection middleware has executed on your incoming request.

    + +``` +req.csrfToken(); // same as generateToken(res, req) and generateToken(res, req, false); +req.csrfToken(true); // same as generateToken(res, req, true); +``` + +

    The generateToken function serves the purpose of establishing a CSRF (Cross-Site Request Forgery) protection mechanism by generating a token and an associated cookie. This function also provides the option to utilize a third parameter called overwrite. By default, this parameter is set to false.

    +

    It returns a CSRF token and attaches a cookie to the response object. The cookie content is `${token}|${tokenHash}`.

    +

    You should only transmit your token to the frontend as part of a response payload, do not include the token in response headers or in a cookie, and do not transmit the token hash by any other means.

    +

    When overwrite is set to false, the function behaves in a way that preserves the existing CSRF cookie and its corresponding token and hash. In other words, if a valid CSRF cookie is already present in the incoming request, the function will reuse this cookie along with its associated token.

    +

    On the other hand, if overwrite is set to true, 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 synchronized effectively (you would need to write your own synchronization logic).

    +

    getSecret

    ```ts diff --git a/src/index.ts b/src/index.ts index 97eaa78..8c4f02c 100644 --- a/src/index.ts +++ b/src/index.ts @@ -42,13 +42,22 @@ export type RequestMethod = | "PATCH"; export type CsrfIgnoredMethods = RequestMethod[]; export type CsrfRequestValidator = (req: Request) => boolean; +export type CsrfTokenAndHashPairValidator = ( + token: string, + hash: string, + secret: string +) => boolean; export type CsrfCookieSetter = ( res: Response, name: string, value: string, options: DoubleCsrfCookieOptions ) => void; -export type CsrfTokenCreator = (res: Response, req: Request) => string; +export type CsrfTokenCreator = ( + res: Response, + req: Request, + ovewrite?: boolean +) => string; export interface DoubleCsrfConfig { getSecret: CsrfSecretRetriever; @@ -91,7 +100,22 @@ export function doubleCsrf({ code: "EBADCSRFTOKEN", }); - const generateTokenAndHash = (req: Request) => { + const generateTokenAndHash = (req: Request, overwrite = false) => { + const csrfCookie = getCsrfCookieFromRequest(req); + // if ovewrite is set, then even if there is already a csrf cookie, do not reuse it + // if csrfCookie is present, it means that there is already a session, so we extract + // the hash/token from it, validate it and reuse the token. This makes possible having + // multiple tabs open at the same time + if (typeof csrfCookie === "string" && !overwrite) { + const [csrfToken, csrfTokenHash] = csrfCookie.split("|"); + const csrfSecret = getSecret(req); + if (!validateTokenAndHashPair(csrfToken, csrfTokenHash, csrfSecret)) { + // if the pair is not valid, then the cookie has been modified by a third party + throw invalidCsrfTokenError; + } + return { csrfToken, csrfTokenHash }; + } + // else, generate the token and hash from scratch const csrfToken = randomBytes(size).toString("hex"); const secret = getSecret(req); const csrfTokenHash = createHash("sha256") @@ -105,36 +129,58 @@ export function doubleCsrf({ // This should be used in routes or middleware to provide users with a token. // The value returned from this should ONLY be sent to the client via a response payload. // Do NOT send the csrfToken as a cookie, embed it in your HTML response, or as JSON. - const generateToken = (res: Response, req: Request) => { - const { csrfToken, csrfTokenHash } = generateTokenAndHash(req); - res.cookie(cookieName, csrfTokenHash, { ...cookieOptions, httpOnly: true }); + + const generateToken: CsrfTokenCreator = ( + res: Response, + req: Request, + overwrite?: boolean + ) => { + const { csrfToken, csrfTokenHash } = generateTokenAndHash(req, overwrite); + const cookieContent = `${csrfToken}|${csrfTokenHash}`; + res.cookie(cookieName, cookieContent, { ...cookieOptions, httpOnly: true }); return csrfToken; }; - const getTokenHashFromRequest = remainingCOokieOptions.signed + const getCsrfCookieFromRequest = remainingCOokieOptions.signed ? (req: Request) => req.signedCookies[cookieName] as string : (req: Request) => req.cookies[cookieName] as string; + // validates if a token and its hash matches, given the secret that was originally included in the hash + const validateTokenAndHashPair: CsrfTokenAndHashPairValidator = ( + token, + hash, + secret + ) => { + if (typeof token !== "string" || typeof hash !== "string") return false; + + const expectedHash = createHash("sha256") + .update(`${token}${secret}`) + .digest("hex"); + + return expectedHash === hash; + }; + const validateRequest: CsrfRequestValidator = (req) => { // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-member-access - const csrfTokenHash = getTokenHashFromRequest(req); + const csrfCookie = getCsrfCookieFromRequest(req); + if (typeof csrfCookie !== "string") return false; - // This is the csrfTokenHash previously set on the response cookie via generateToken - if (typeof csrfTokenHash !== "string") return false; + // cookie has the form {token}|{hash} + const [csrfToken, csrfTokenHash] = csrfCookie.split("|"); // csrf token from the request const csrfTokenFromRequest = getTokenFromRequest(req) as string; - // Hash the token with the provided secret and it should match the previous hash from the cookie - const expectedCsrfTokenHash = createHash("sha256") - .update(`${csrfTokenFromRequest}${getSecret(req)}`) - .digest("hex"); + const csrfSecret = getSecret(req); - return csrfTokenHash === expectedCsrfTokenHash; + return ( + csrfToken === csrfTokenFromRequest && + validateTokenAndHashPair(csrfTokenFromRequest, csrfTokenHash, csrfSecret) + ); }; const doubleCsrfProtection: doubleCsrfProtection = (req, res, next) => { - req.csrfToken = () => generateToken(res, req); + req.csrfToken = (overwrite?: boolean) => generateToken(res, req, overwrite); if (ignoredMethodsSet.has(req.method as RequestMethod)) { next(); } else if (validateRequest(req)) { diff --git a/src/tests/testsuite.ts b/src/tests/testsuite.ts index 410b863..608e2ab 100644 --- a/src/tests/testsuite.ts +++ b/src/tests/testsuite.ts @@ -6,7 +6,11 @@ import { serialize as serializeCookie } from "cookie"; import { sign } from "cookie-signature"; import { generateMocks, generateMocksWithToken, next } from "./utils/mock.js"; import { HEADER_KEY, TEST_TOKEN } from "./utils/constants.js"; -import { getCookieFromRequest, switchSecret } from "./utils/helpers.js"; +import { + getCookieFromRequest, + getCookieFromResponse, + switchSecret, +} from "./utils/helpers.js"; type CreateTestsuite = ( name: string, @@ -48,17 +52,19 @@ export const createTestSuite: CreateTestsuite = (name, doubleCsrfOptions) => { }); describe("generateToken", () => { - it("should attach a hashed token to the request and return a token", () => { - const { mockRequest, hashedToken, setCookie } = + it("should attach both a token and its hash to the response and return a token", () => { + const { mockRequest, decodedCookieValue, setCookie } = generateMocksWithTokenIntenral(); - - const cookieHash = signed - ? `s:${sign(hashedToken as string, mockRequest.secret as string)}` - : hashedToken; + const cookieValue = signed + ? `s:${sign( + decodedCookieValue as string, + mockRequest.secret as string + )}` + : decodedCookieValue; const expectedSetCookieValue = serializeCookie( cookieName, - cookieHash as string, + cookieValue as string, { path, httpOnly: true, @@ -68,6 +74,72 @@ export const createTestSuite: CreateTestsuite = (name, doubleCsrfOptions) => { ); assert.equal(setCookie, expectedSetCookieValue); }); + + it("should reuse a csrf token if a csrf cookie is already present, and overwrite is set to false", () => { + const { + mockRequest, + mockResponse, + csrfToken, + cookieValue: oldCookieValue, + } = generateMocksWithTokenIntenral(); + + // reset the mock response to have no cookies (in reality this would just be a new instance of Response) + mockResponse.setHeader("set-cookie", []); + + // overwrite is false by default + const generatedToken = generateToken(mockResponse, mockRequest); + const newCookieValue = getCookieFromResponse(mockResponse); + + assert.equal(generatedToken, csrfToken); + assert.equal(newCookieValue, oldCookieValue); + }); + + it("should generate a new token even if a csrf cookie is already present, if overwrite is set to true", () => { + const { + mockRequest, + mockResponse, + csrfToken, + cookieValue: oldCookieValue, + } = generateMocksWithTokenIntenral(); + + // reset the mock response to have no cookies (in reality this would just be a new instance of Response) + mockResponse.setHeader("set-cookie", []); + + const generatedToken = generateToken(mockResponse, mockRequest, true); + const newCookieValue = getCookieFromResponse(mockResponse); + + assert.notEqual(newCookieValue, oldCookieValue); + assert.notEqual(generatedToken, csrfToken); + }); + + it("should throw if csrf cookie is present, it is invalid (wrong token + hash pair, or not a correct value) and overwrite is false", () => { + const { mockRequest, mockResponse, decodedCookieValue } = + generateMocksWithTokenIntenral(); + // modify the cookie to make the token/hash pair invalid + signed + ? (mockRequest.signedCookies[cookieName] = `s:${sign( + (decodedCookieValue as string).split("|")[0] + "|invalid-hash", + mockRequest.secret as string + )}`) + : (mockRequest.cookies[cookieName] = + (decodedCookieValue as string).split("|")[0] + "|invalid-hash"); + + expect(() => generateToken(mockResponse, mockRequest)).to.throw( + invalidCsrfTokenError.message + ); + + // just an invalid value in the cookie + signed + ? (mockRequest.signedCookies[cookieName] = `s:${sign( + "invalid-value", + mockRequest.secret as string + )}`) + : (mockRequest.cookies[cookieName] = "invalid-value"); + + expect(() => generateToken(mockResponse, mockRequest)).to.throw( + invalidCsrfTokenError.message + ); + }); }); describe("validateRequest", () => { @@ -77,10 +149,11 @@ export const createTestSuite: CreateTestsuite = (name, doubleCsrfOptions) => { }); it("should return false when a token is generated but not received in request", () => { - const { mockRequest, hashedToken } = generateMocksWithTokenIntenral(); + const { mockRequest, decodedCookieValue } = + generateMocksWithTokenIntenral(); assert.equal( getCookieFromRequest(cookieName, signed, mockRequest), - hashedToken + decodedCookieValue ); // Wipe token diff --git a/src/tests/utils/helpers.ts b/src/tests/utils/helpers.ts index 1aab3f2..234294b 100644 --- a/src/tests/utils/helpers.ts +++ b/src/tests/utils/helpers.ts @@ -16,7 +16,7 @@ export const { getSecret, switchSecret } = (() => { /** * Parses the response 'Set-Cookie' header. * @param res The response object - * @returns The set-cookie header string and the csrf token hash value + * @returns The set-cookie header string and the cookie value containing both the csrf token and its hash */ export const getCookieValueFromResponse = (res: Response) => { const setCookie = res.getHeader("set-cookie") as string | string[]; @@ -42,3 +42,17 @@ export const getCookieFromRequest = ( ) => // eslint-disable-next-line @typescript-eslint/no-unsafe-return, @typescript-eslint/no-unsafe-member-access signed ? req.signedCookies[cookieName] : req.cookies[cookieName]; + +// as of now, we only have one cookie, so we can just return the first one +export const getCookieFromResponse = (res: Response) => { + const setCookie = res.getHeader("set-cookie") as string | string[]; + const setCookieString: string = Array.isArray(setCookie) + ? setCookie[0] + : setCookie; + const cookieValue = setCookieString.substring( + setCookieString.indexOf("=") + 1, + setCookieString.indexOf(";") + ); + + return cookieValue; +}; diff --git a/src/tests/utils/mock.ts b/src/tests/utils/mock.ts index 30d7cab..1d44775 100644 --- a/src/tests/utils/mock.ts +++ b/src/tests/utils/mock.ts @@ -84,29 +84,29 @@ export const generateMocksWithToken = ({ const csrfToken = generateToken(mockResponse, mockRequest); const { setCookie, cookieValue } = getCookieValueFromResponse(mockResponse); mockRequest.headers.cookie = `${cookieName}=${cookieValue};`; - const hashedToken = signed + const decodedCookieValue = signed ? signedCookie( parse(mockRequest.headers.cookie)[cookieName], mockRequest.secret as string ) - : cookieValue; + : // signedCookie already decodes the value, but we need it if it's not signed. + decodeURIComponent(cookieValue); // Have to delete the cookies object otherwise cookieParser will skip it's parsing. delete mockRequest["cookies"]; cookieParserMiddleware(mockRequest, mockResponse, next); assert.equal( getCookieFromRequest(cookieName, signed, mockRequest), - hashedToken + decodedCookieValue ); mockRequest.headers[HEADER_KEY] = csrfToken; - // Once a token has ben generated, the request should be setup as valid + // Once a token has been generated, the request should be setup as valid assert.isTrue(validateRequest(mockRequest)); - return { csrfToken, cookieValue, - hashedToken, + decodedCookieValue, mockRequest, mockResponse, mockResponseHeaders,