Skip to content

Commit

Permalink
Merge pull request #73 from Psifi-Solutions/chore/update-with-v3-changes
Browse files Browse the repository at this point in the history
Chore/update with v3 changes
  • Loading branch information
psibean authored Sep 21, 2024
2 parents 5ede81e + 9045ad5 commit 3a016c5
Show file tree
Hide file tree
Showing 10 changed files with 229 additions and 30 deletions.
14 changes: 14 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,20 @@

All notable changes to this project will be documented in this file. See [standard-version](https://github.com/conventional-changelog/standard-version) for commit guidelines.

## [3.0.7](https://github.com/Psifi-Solutions/csrf-csrf/compare/v3.0.6...v3.0.7) (2024-09-21)

* Marked >= 3.0.7 as security supported version

### Features

* support optional stateless association of token with session ([710d2f6](https://github.com/Psifi-Solutions/csrf-csrf/commit/710d2f6082f1ac8ab884b10913b1b86195f86bd2))

Added the `getSessionIdentifier` parameter to the `csrf-csrf` configuration. By providing the `getSessionIdentifier` callback, generated tokens will only be valid for the original session identifier they were generated for.

For example: (req) => req.session.id

The token will now be signed with the session id included, this means a generated CSRF token will only be valid for the session it was generated for. This also means that if you rotate your sessions (which you should) you will also need to generate a new CSRF token for the session after rotating it.

### [3.0.6](https://github.com/Psifi-Solutions/csrf-csrf/compare/v3.0.5...v3.0.6) (2024-05-17)

* No changes, just a bump to fix broken release
Expand Down
20 changes: 20 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ When creating your doubleCsrf, you have a few options available for configuratio
```js
const doubleCsrfUtilities = doubleCsrf({
getSecret: () => "Secret", // A function that optionally takes the request and returns a secret
getSessionIdentifier: (req) => "", // A function that should return the session identifier for a given request
cookieName: "__Host-psifi.x-csrf-token", // The name of the cookie to be used, recommend using Host prefix.
cookieOptions: {
sameSite = "lax", // Recommend you make this strict if posible
Expand All @@ -213,6 +214,25 @@ const doubleCsrfUtilities = doubleCsrf({
<p>In case multiple are provided, the first one will be used for hashing. For validation, all secrets will be tried, preferring the first one in the array. Having multiple valid secrets can be useful when you need to rotate secrets, but you don't want to invalidate the previous secret (which might still be used by some users) right away.</p>
</p>

<h3>getSessionIdentifier</h3>

```ts
(req: Request) => string;
```

<p>
<b>Optional</b><br />
<b>Default:</b> <code>() => ""</code><br />
</p>

<p>A function that takes in the request and returns the unique session identifier for that request. For example:</p>

```ts
(req: Request) => req.session.id;
```

<p>This will ensure that CSRF tokens are signed with the unique identifier included, this means tokens will only be valid for the session that they were requested by and generated for.</p>

<h3>cookieName</h3>

```ts
Expand Down
4 changes: 2 additions & 2 deletions SECURITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@

| Version | Supported |
| -------- | ------------------ |
| >= 2.3.0 | :white_check_mark: |
| < 2.3.0 | :x: |
| >= 3.0.7 | :white_check_mark: |
| < 3.0.7 | :x: |

## Reporting a Vulnerability

Expand Down
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "csrf-csrf",
"version": "3.0.6",
"version": "3.0.7",
"description": "A utility package to help implement stateless CSRF protection using the Double Submit Cookie Pattern in express.",
"type": "module",
"main": "./lib/cjs/index.cjs",
Expand Down
35 changes: 23 additions & 12 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export * from "./types";

export function doubleCsrf({
getSecret,
getSessionIdentifier = () => "",
cookieName = "__Host-psifi.x-csrf-token",
cookieOptions: {
sameSite = "lax",
Expand Down Expand Up @@ -70,7 +71,14 @@ export function doubleCsrf({
// generate a new token based on validateOnReuse.
if (typeof csrfCookie === "string" && !overwrite) {
const [csrfToken, csrfTokenHash] = csrfCookie.split("|");
if (validateTokenAndHashPair(csrfToken, csrfTokenHash, possibleSecrets)) {
if (
validateTokenAndHashPair({
csrfToken,
csrfTokenHash,
possibleSecrets,
sessionIdentifier: getSessionIdentifier(req),
})
) {
// If the pair is valid, reuse it
return { csrfToken, csrfTokenHash };
} else if (validateOnReuse) {
Expand All @@ -84,7 +92,7 @@ export function doubleCsrf({
// the 'newest' or preferred secret is the first one in the array
const secret = possibleSecrets[0];
const csrfTokenHash = createHash("sha256")
.update(`${csrfToken}${secret}`)
.update(`${getSessionIdentifier(req)}${csrfToken}${secret}`)
.digest("hex");

return { csrfToken, csrfTokenHash };
Expand Down Expand Up @@ -121,18 +129,20 @@ export function doubleCsrf({
: (req: Request) => req.cookies[cookieName] as string;

// given a secret array, iterates over it and checks whether one of the secrets makes the token and hash pair valid
const validateTokenAndHashPair: CsrfTokenAndHashPairValidator = (
token,
hash,
const validateTokenAndHashPair: CsrfTokenAndHashPairValidator = ({
csrfToken,
csrfTokenHash,
possibleSecrets,
) => {
if (typeof token !== "string" || typeof hash !== "string") return false;
sessionIdentifier,
}) => {
if (typeof csrfToken !== "string" || typeof csrfTokenHash !== "string")
return false;

for (const secret of possibleSecrets) {
const expectedHash = createHash("sha256")
.update(`${token}${secret}`)
.update(`${sessionIdentifier}${csrfToken}${secret}`)
.digest("hex");
if (hash === expectedHash) return true;
if (csrfTokenHash === expectedHash) return true;
}

return false;
Expand All @@ -156,11 +166,12 @@ export function doubleCsrf({

return (
csrfToken === csrfTokenFromRequest &&
validateTokenAndHashPair(
csrfTokenFromRequest,
validateTokenAndHashPair({
csrfToken: csrfTokenFromRequest,
csrfTokenHash,
possibleSecrets,
)
sessionIdentifier: getSessionIdentifier(req),
})
);
};

Expand Down
111 changes: 111 additions & 0 deletions src/tests/getSessionIdentifier.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
/* eslint-disable @typescript-eslint/no-unsafe-member-access */
import { assert, expect } from "chai";
import { doubleCsrf } from "../index.js";
import {
generateMocksWithToken,
next,
RequestWithSessionId,
} from "./utils/mock.js";
import {
getSingleSecret,
attachResponseValuesToRequest,
} from "./utils/helpers.js";

describe("csrf-csrf with getSessionIdentifier", () => {
const cookieName = "xsrf-protection";
const sessionIdentifier = "asdf68236tr3g34fgds9fgsd9g23grb3";

const {
invalidCsrfTokenError,
generateToken,
validateRequest,
doubleCsrfProtection,
} = doubleCsrf({
cookieName,
getSecret: getSingleSecret,
getSessionIdentifier: (req) =>
(req as RequestWithSessionId).session.id ?? "",
});

it("should have a valid CSRF token for the session it was generated for", () => {
const { mockRequest, mockResponse } = generateMocksWithToken({
cookieName,
generateToken,
validateRequest,
signed: false,
sessionIdentifier,
});

expect(() => {
doubleCsrfProtection(mockRequest, mockResponse, next);
}, "CSRF protection should be valid").not.to.throw(invalidCsrfTokenError);
});

it("should not be a valid CSRF token for a session it was not generated for", () => {
const { mockRequest, mockResponse } = generateMocksWithToken({
cookieName,
generateToken,
validateRequest,
signed: false,
sessionIdentifier,
});

(mockRequest as RequestWithSessionId).session.id = "sdf9342dfa245r13tgvrf";

expect(() => {
doubleCsrfProtection(mockRequest, mockResponse, next);
}, "CSRF protection should be invalid").to.throw(invalidCsrfTokenError);
});

it("should throw when validateOnReuse is true and session has been rotated", () => {
const { mockRequest, mockResponse } = generateMocksWithToken({
cookieName,
generateToken,
validateRequest,
signed: false,
sessionIdentifier,
});

(mockRequest as RequestWithSessionId).session.id = "sdf9342dfa245r13tgvrf";

assert.isFalse(validateRequest(mockRequest));
expect(() =>
generateToken(mockRequest, mockResponse, {
overwrite: false,
validateOnReuse: true,
}),
).to.throw(invalidCsrfTokenError);
});

it("should generate a new valid token after session has been rotated", () => {
const { csrfToken, mockRequest, mockResponse } = generateMocksWithToken({
cookieName,
generateToken,
validateRequest,
signed: false,
sessionIdentifier,
});

(mockRequest as RequestWithSessionId).session.id = "sdf9342dfa245r13tgvrf";
console.log("generating a new token");
const newCsrfToken = generateToken(mockRequest, mockResponse, {
overwrite: true,
});
console.log("new token generated");
assert.notEqual(
newCsrfToken,
csrfToken,
"New token and original token should not match",
);
attachResponseValuesToRequest({
request: mockRequest,
response: mockResponse,
bodyResponseToken: newCsrfToken,
cookieName,
});
assert.isTrue(validateRequest(mockRequest));
expect(() =>
doubleCsrfProtection(mockRequest, mockResponse, next),
).not.to.throw();
});
});
9 changes: 5 additions & 4 deletions src/tests/utils/helpers.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { Request, Response } from "express";
import { HEADER_KEY } from "./constants";

const SECRET_1 = "secrets must be unique and must not";
const SECRET_2 = "be used elsewhere, nor be sentences";
Expand Down Expand Up @@ -75,14 +76,14 @@ export const attachResponseValuesToRequest = ({
request,
response,
bodyResponseToken,
cookieName,
headerKey,
cookieName = "__Host-psifi.x-csrf-token",
headerKey = HEADER_KEY,
}: {
request: Request;
response: Response;
bodyResponseToken: string;
cookieName: string;
headerKey: string;
cookieName?: string;
headerKey?: string;
}) => {
const { cookieValue } = getCookieValueFromResponse(response);

Expand Down
34 changes: 28 additions & 6 deletions src/tests/utils/mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { COOKIE_SECRET, HEADER_KEY } from "./constants.js";
import { getCookieFromRequest, getCookieValueFromResponse } from "./helpers.js";

// Create some request and response mocks
export const generateMocks = () => {
export const generateMocks = (sessionIdentifier?: string) => {
const mockRequest = {
headers: {
cookie: "",
Expand All @@ -18,6 +18,9 @@ export const generateMocks = () => {
secret: COOKIE_SECRET,
} as unknown as Request;

if (sessionIdentifier) {
(mockRequest as RequestWithSessionId).session = { id: sessionIdentifier };
}
// Internally mock the headers as a map.
const mockResponseHeaders = new Map<string, string | string[]>();
mockResponseHeaders.set("set-cookie", [] as string[]);
Expand All @@ -41,9 +44,16 @@ export const generateMocks = () => {
: value;
const data: string = serializeCookie(name, parsesValue, options);
const previous = mockResponse.getHeader("set-cookie") || [];
const header = Array.isArray(previous)
? previous.concat(data)
: [previous, data];
let header;
if (Array.isArray(previous)) {
header = previous
.filter((header) => !header.startsWith(name))
.concat(data);
} else if (typeof previous === "string" && previous.startsWith(name)) {
header = [data];
} else {
header = [previous, data];
}

mockResponse.setHeader("set-cookie", header as string[]);
return mockResponse;
Expand All @@ -56,6 +66,12 @@ export const generateMocks = () => {
};
};

export type RequestWithSessionId = Request & {
session: {
id?: string;
};
};

// Mock the next callback and allow for error throwing.
// eslint-disable-next-line @typescript-eslint/no-explicit-any
export const next = (err: any) => {
Expand All @@ -69,6 +85,7 @@ export type GenerateMocksWithTokenOptions = {
signed: boolean;
generateToken: CsrfTokenCreator;
validateRequest: CsrfRequestValidator;
sessionIdentifier?: string;
};

// Generate the request and response mocks.
Expand All @@ -78,8 +95,10 @@ export const generateMocksWithToken = ({
signed,
generateToken,
validateRequest,
sessionIdentifier,
}: GenerateMocksWithTokenOptions) => {
const { mockRequest, mockResponse, mockResponseHeaders } = generateMocks();
const { mockRequest, mockResponse, mockResponseHeaders } =
generateMocks(sessionIdentifier);

const csrfToken = generateToken(mockRequest, mockResponse);
const { setCookie, cookieValue } = getCookieValueFromResponse(mockResponse);
Expand All @@ -102,7 +121,10 @@ export const generateMocksWithToken = ({
mockRequest.headers[HEADER_KEY] = csrfToken;

// Once a token has been generated, the request should be setup as valid
assert.isTrue(validateRequest(mockRequest));
assert.isTrue(
validateRequest(mockRequest),
"mockRequest should be valid after being setup with a token",
);
return {
csrfToken,
cookieValue,
Expand Down
Loading

0 comments on commit 3a016c5

Please sign in to comment.