From bdce324de165f9fcc00221d1e38a5557c5ff99be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johnny=20G=C3=A9rard?= Date: Sat, 2 Nov 2024 13:38:48 +0100 Subject: [PATCH 01/27] build: install ajv --- server/package-lock.json | 44 ++++++++++++++++++++++++++++++++++++++++ server/package.json | 1 + 2 files changed, 45 insertions(+) diff --git a/server/package-lock.json b/server/package-lock.json index 1233fca..7a4ec55 100644 --- a/server/package-lock.json +++ b/server/package-lock.json @@ -5,6 +5,7 @@ "packages": { "": { "dependencies": { + "ajv": "^8.17.1", "argon2": "^0.41.1", "cookie-parser": "^1.4.7", "cors": "^2.8.5", @@ -552,6 +553,22 @@ "node": ">= 0.6" } }, + "node_modules/ajv": { + "version": "8.17.1", + "resolved": "https://registry.npmjs.org/ajv/-/ajv-8.17.1.tgz", + "integrity": "sha512-B/gBuNg5SiMTrPkC+A2+cW0RszwxYmn6VYxB/inlBStS5nx6xHIt/ehKRhIMhqusl7a8LjQoZnjCs5vhwxOQ1g==", + "license": "MIT", + "dependencies": { + "fast-deep-equal": "^3.1.3", + "fast-uri": "^3.0.1", + "json-schema-traverse": "^1.0.0", + "require-from-string": "^2.0.2" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/epoberezkin" + } + }, "node_modules/argon2": { "version": "0.41.1", "resolved": "https://registry.npmjs.org/argon2/-/argon2-0.41.1.tgz", @@ -867,6 +884,18 @@ "node": ">= 0.6" } }, + "node_modules/fast-deep-equal": { + "version": "3.1.3", + "resolved": "https://registry.npmjs.org/fast-deep-equal/-/fast-deep-equal-3.1.3.tgz", + "integrity": "sha512-f3qQ9oQy9j2AhBe/H9VC91wLmKBCCU/gDOnKNAYG5hswO7BLKj09Hc5HYNz9cGI++xlpDCIgDaitVs03ATR84Q==", + "license": "MIT" + }, + "node_modules/fast-uri": { + "version": "3.0.3", + "resolved": "https://registry.npmjs.org/fast-uri/-/fast-uri-3.0.3.tgz", + "integrity": "sha512-aLrHthzCjH5He4Z2H9YZ+v6Ujb9ocRuW6ZzkJQOrTxleEijANq4v1TsaPaVG1PZcuurEzrLcWRyYBYXD5cEiaw==", + "license": "BSD-3-Clause" + }, "node_modules/finalhandler": { "version": "1.3.1", "resolved": "https://registry.npmjs.org/finalhandler/-/finalhandler-1.3.1.tgz", @@ -1043,6 +1072,12 @@ "node": ">= 0.10" } }, + "node_modules/json-schema-traverse": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/json-schema-traverse/-/json-schema-traverse-1.0.0.tgz", + "integrity": "sha512-NM8/P9n3XjXhIZn1lLhkFaACTOURQXjWhV4BA/RnOv8xvgqtqpAX9IO4mRQxSx1Rlo4tqzeqb0sOlruaOy3dug==", + "license": "MIT" + }, "node_modules/media-typer": { "version": "0.3.0", "resolved": "https://registry.npmjs.org/media-typer/-/media-typer-0.3.0.tgz", @@ -1353,6 +1388,15 @@ "@redis/time-series": "1.1.0" } }, + "node_modules/require-from-string": { + "version": "2.0.2", + "resolved": "https://registry.npmjs.org/require-from-string/-/require-from-string-2.0.2.tgz", + "integrity": "sha512-Xf0nWe6RseziFMu+Ap9biiUbmplq6S9/p+7w7YXP/JBHhrUDDUhwa+vANyubuqfZWTveU//DYVGsDG7RKL/vEw==", + "license": "MIT", + "engines": { + "node": ">=0.10.0" + } + }, "node_modules/safe-buffer": { "version": "5.2.1", "resolved": "https://registry.npmjs.org/safe-buffer/-/safe-buffer-5.2.1.tgz", diff --git a/server/package.json b/server/package.json index 1444e25..9bc301e 100644 --- a/server/package.json +++ b/server/package.json @@ -1,5 +1,6 @@ { "dependencies": { + "ajv": "^8.17.1", "argon2": "^0.41.1", "cookie-parser": "^1.4.7", "cors": "^2.8.5", From a031827967cf02fe4e053805a1d05c77ead6f5fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johnny=20G=C3=A9rard?= Date: Sat, 2 Nov 2024 14:23:45 +0100 Subject: [PATCH 02/27] feat: add ajv/index.ts --- server/src/validation/ajv/index.ts | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 server/src/validation/ajv/index.ts diff --git a/server/src/validation/ajv/index.ts b/server/src/validation/ajv/index.ts new file mode 100644 index 0000000..1cc4d33 --- /dev/null +++ b/server/src/validation/ajv/index.ts @@ -0,0 +1,3 @@ +import { Ajv } from "ajv/dist/jtd.js"; + +export const ajv = new Ajv(); From 6cabe92c2868f0e9a161e8f021042ffc02348fb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johnny=20G=C3=A9rard?= Date: Sat, 2 Nov 2024 14:24:05 +0100 Subject: [PATCH 03/27] test: add ajv/index.test.ts --- server/src/validation/ajv/index.test.ts | 50 +++++++++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 server/src/validation/ajv/index.test.ts diff --git a/server/src/validation/ajv/index.test.ts b/server/src/validation/ajv/index.test.ts new file mode 100644 index 0000000..86ecb9e --- /dev/null +++ b/server/src/validation/ajv/index.test.ts @@ -0,0 +1,50 @@ +import { faker } from "@faker-js/faker"; +import { JTDParser } from "ajv/dist/types/index.js"; +import { JTDSchemaType } from "ajv/dist/types/jtd-schema.js"; +import assert from "node:assert/strict"; +import { before, suite, test } from "node:test"; +import { ajv } from "./index.js"; + +type Person = { + fullName: string; + isFamous: boolean; +}; + +const schema: JTDSchemaType = { + properties: { + fullName: { type: "string" }, + isFamous: { type: "boolean" }, + }, +}; + +suite.only("ajv", () => { + let parser: JTDParser; + + before(() => { + parser = ajv.compileParser(schema); + }); + + test("parses valid data", () => { + const person: Person = { + fullName: faker.person.fullName(), + isFamous: faker.datatype.boolean(), + }; + + const result = parser(JSON.stringify(person)); + + assert.deepEqual(result, person); + assert(!parser.message); + }); + + test("parses invalid data", () => { + const invalidPerson = { + fullName: faker.person.fullName(), + isFamous: faker.datatype.boolean() ? "yes" : "no", + }; + + const result = parser(JSON.stringify(invalidPerson)); + + assert(!result); + assert(parser.message); + }); +}); From 66f372a99d63bedb561b9948a3e31915e2d35f5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johnny=20G=C3=A9rard?= Date: Sat, 2 Nov 2024 14:26:14 +0100 Subject: [PATCH 04/27] refactor: rename parser to parse --- server/src/validation/ajv/index.test.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/server/src/validation/ajv/index.test.ts b/server/src/validation/ajv/index.test.ts index 86ecb9e..90b9f6c 100644 --- a/server/src/validation/ajv/index.test.ts +++ b/server/src/validation/ajv/index.test.ts @@ -18,10 +18,10 @@ const schema: JTDSchemaType = { }; suite.only("ajv", () => { - let parser: JTDParser; + let parse: JTDParser; before(() => { - parser = ajv.compileParser(schema); + parse = ajv.compileParser(schema); }); test("parses valid data", () => { @@ -30,10 +30,10 @@ suite.only("ajv", () => { isFamous: faker.datatype.boolean(), }; - const result = parser(JSON.stringify(person)); + const result = parse(JSON.stringify(person)); assert.deepEqual(result, person); - assert(!parser.message); + assert(!parse.message); }); test("parses invalid data", () => { @@ -42,9 +42,9 @@ suite.only("ajv", () => { isFamous: faker.datatype.boolean() ? "yes" : "no", }; - const result = parser(JSON.stringify(invalidPerson)); + const result = parse(JSON.stringify(invalidPerson)); assert(!result); - assert(parser.message); + assert(parse.message); }); }); From b12b3ccbefec9efa108532d0470aba2ef6dd09e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johnny=20G=C3=A9rard?= Date: Sat, 2 Nov 2024 14:26:42 +0100 Subject: [PATCH 05/27] feat: add credentials.ts --- server/src/validation/ajv/credentials.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 server/src/validation/ajv/credentials.ts diff --git a/server/src/validation/ajv/credentials.ts b/server/src/validation/ajv/credentials.ts new file mode 100644 index 0000000..6e9e3f9 --- /dev/null +++ b/server/src/validation/ajv/credentials.ts @@ -0,0 +1,17 @@ +import type { JTDSchemaType } from "ajv/dist/types/jtd-schema.js"; +import { ajv } from "./index.js"; + +export type Credentials = { + username: string; + password: string; +}; + +const schema: JTDSchemaType = { + properties: { + username: { type: "string" }, + password: { type: "string" }, + }, + additionalProperties: false, +}; + +export const parseCredentials = ajv.compileParser(schema); From fb9eaec829f5752a9a2c1e8f3be5ccc397f4c153 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johnny=20G=C3=A9rard?= Date: Sat, 2 Nov 2024 14:38:21 +0100 Subject: [PATCH 06/27] test: remove only call --- server/src/validation/ajv/index.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/validation/ajv/index.test.ts b/server/src/validation/ajv/index.test.ts index 90b9f6c..e215a28 100644 --- a/server/src/validation/ajv/index.test.ts +++ b/server/src/validation/ajv/index.test.ts @@ -17,7 +17,7 @@ const schema: JTDSchemaType = { }, }; -suite.only("ajv", () => { +suite("ajv", () => { let parse: JTDParser; before(() => { From 07b6885d11548697561217d8a4293a23a711a3e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johnny=20G=C3=A9rard?= Date: Sat, 2 Nov 2024 14:44:52 +0100 Subject: [PATCH 07/27] refactor: use Credentials type --- client/e2e/log-in-user.function.ts | 6 ++---- client/e2e/register-user.function.ts | 6 ++---- client/e2e/tests/session-revocation.spec.ts | 3 ++- server/src/test-helpers/faker-extensions.ts | 6 ++---- 4 files changed, 8 insertions(+), 13 deletions(-) diff --git a/client/e2e/log-in-user.function.ts b/client/e2e/log-in-user.function.ts index 8dc3bcb..06812c9 100644 --- a/client/e2e/log-in-user.function.ts +++ b/client/e2e/log-in-user.function.ts @@ -1,5 +1,6 @@ import { expect, Page } from "@playwright/test"; import { CREATED } from "_server/constants/http-status-code"; +import { Credentials } from "_server/validation/ajv/credentials"; /** * Fill out and submit the login form while verifying the response status. @@ -9,10 +10,7 @@ import { CREATED } from "_server/constants/http-status-code"; */ export const logInUser = async ( page: Page, - credentials: { - username: string; - password: string; - }, + credentials: Credentials, expectedStatus = CREATED, ): Promise => { await page.goto("/sign-in"); diff --git a/client/e2e/register-user.function.ts b/client/e2e/register-user.function.ts index 23921b1..c9af718 100644 --- a/client/e2e/register-user.function.ts +++ b/client/e2e/register-user.function.ts @@ -1,6 +1,7 @@ import { faker } from "@faker-js/faker"; import { expect, Page } from "@playwright/test"; import { CREATED } from "_server/constants/http-status-code"; +import { Credentials } from "_server/validation/ajv/credentials"; /** * Fill out and submit the registration form while verifying the response status. @@ -10,10 +11,7 @@ import { CREATED } from "_server/constants/http-status-code"; */ export const registerUser = async ( page: Page, - credentials?: Partial<{ - username: string; - password: string; - }>, + credentials?: Partial, expectedStatus = CREATED, ): Promise => { await page.goto("/register"); diff --git a/client/e2e/tests/session-revocation.spec.ts b/client/e2e/tests/session-revocation.spec.ts index 3aa34d0..e17f3cd 100644 --- a/client/e2e/tests/session-revocation.spec.ts +++ b/client/e2e/tests/session-revocation.spec.ts @@ -1,11 +1,12 @@ import { expect, test } from "@playwright/test"; import { getFakeCredentials } from "_server/test-helpers/faker-extensions"; +import { Credentials } from "_server/validation/ajv/credentials"; import { UserMessage } from "../../src/app/types/user-message.enum"; import { logInUser } from "../log-in-user.function"; import { registerUser } from "../register-user.function"; test.describe("Session revocation", () => { - let credentials: { username: string; password: string }; + let credentials: Credentials; test.beforeEach(async ({ browser, page }) => { const context = await browser.newContext(); diff --git a/server/src/test-helpers/faker-extensions.ts b/server/src/test-helpers/faker-extensions.ts index b54f631..a7fd16d 100644 --- a/server/src/test-helpers/faker-extensions.ts +++ b/server/src/test-helpers/faker-extensions.ts @@ -2,14 +2,12 @@ import { faker } from "@faker-js/faker"; import { Buffer } from "node:buffer"; import { PASSWORD_MAX_LENGTH } from "../constants/password.js"; import { ServerSession } from "../types/server-session.js"; +import { Credentials } from "../validation/ajv/credentials.js"; /** * Generate fake credentials (username and password) */ -export const getFakeCredentials = (): { - username: string; - password: string; -} => ({ +export const getFakeCredentials = (): Credentials => ({ username: faker.internet.userName(), password: faker.internet.password(), }); From 722d7018bd53f476d9bf7051d552a6ff6c2fff92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johnny=20G=C3=A9rard?= Date: Sat, 2 Nov 2024 14:47:53 +0100 Subject: [PATCH 08/27] refactor: move Credentials type to credentials.ts --- client/e2e/log-in-user.function.ts | 2 +- client/e2e/register-user.function.ts | 2 +- client/e2e/tests/session-revocation.spec.ts | 2 +- server/src/test-helpers/faker-extensions.ts | 2 +- server/src/types/credentials.ts | 4 ++++ server/src/validation/ajv/credentials.ts | 6 +----- 6 files changed, 9 insertions(+), 9 deletions(-) create mode 100644 server/src/types/credentials.ts diff --git a/client/e2e/log-in-user.function.ts b/client/e2e/log-in-user.function.ts index 06812c9..a6180ae 100644 --- a/client/e2e/log-in-user.function.ts +++ b/client/e2e/log-in-user.function.ts @@ -1,6 +1,6 @@ import { expect, Page } from "@playwright/test"; import { CREATED } from "_server/constants/http-status-code"; -import { Credentials } from "_server/validation/ajv/credentials"; +import { Credentials } from "_server/types/credentials"; /** * Fill out and submit the login form while verifying the response status. diff --git a/client/e2e/register-user.function.ts b/client/e2e/register-user.function.ts index c9af718..debf6b1 100644 --- a/client/e2e/register-user.function.ts +++ b/client/e2e/register-user.function.ts @@ -1,7 +1,7 @@ import { faker } from "@faker-js/faker"; import { expect, Page } from "@playwright/test"; import { CREATED } from "_server/constants/http-status-code"; -import { Credentials } from "_server/validation/ajv/credentials"; +import { Credentials } from "_server/types/credentials"; /** * Fill out and submit the registration form while verifying the response status. diff --git a/client/e2e/tests/session-revocation.spec.ts b/client/e2e/tests/session-revocation.spec.ts index e17f3cd..e68fabe 100644 --- a/client/e2e/tests/session-revocation.spec.ts +++ b/client/e2e/tests/session-revocation.spec.ts @@ -1,6 +1,6 @@ import { expect, test } from "@playwright/test"; import { getFakeCredentials } from "_server/test-helpers/faker-extensions"; -import { Credentials } from "_server/validation/ajv/credentials"; +import { Credentials } from "_server/types/credentials"; import { UserMessage } from "../../src/app/types/user-message.enum"; import { logInUser } from "../log-in-user.function"; import { registerUser } from "../register-user.function"; diff --git a/server/src/test-helpers/faker-extensions.ts b/server/src/test-helpers/faker-extensions.ts index a7fd16d..32c84cf 100644 --- a/server/src/test-helpers/faker-extensions.ts +++ b/server/src/test-helpers/faker-extensions.ts @@ -1,8 +1,8 @@ import { faker } from "@faker-js/faker"; import { Buffer } from "node:buffer"; import { PASSWORD_MAX_LENGTH } from "../constants/password.js"; +import { Credentials } from "../types/credentials.js"; import { ServerSession } from "../types/server-session.js"; -import { Credentials } from "../validation/ajv/credentials.js"; /** * Generate fake credentials (username and password) diff --git a/server/src/types/credentials.ts b/server/src/types/credentials.ts new file mode 100644 index 0000000..af97f64 --- /dev/null +++ b/server/src/types/credentials.ts @@ -0,0 +1,4 @@ +export type Credentials = { + username: string; + password: string; +}; diff --git a/server/src/validation/ajv/credentials.ts b/server/src/validation/ajv/credentials.ts index 6e9e3f9..5e5ce50 100644 --- a/server/src/validation/ajv/credentials.ts +++ b/server/src/validation/ajv/credentials.ts @@ -1,11 +1,7 @@ import type { JTDSchemaType } from "ajv/dist/types/jtd-schema.js"; +import { Credentials } from "../../types/credentials.js"; import { ajv } from "./index.js"; -export type Credentials = { - username: string; - password: string; -}; - const schema: JTDSchemaType = { properties: { username: { type: "string" }, From 1e5b1b0297d987e6076a8a84a211f6f1e5a863f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johnny=20G=C3=A9rard?= Date: Sat, 2 Nov 2024 15:36:59 +0100 Subject: [PATCH 09/27] feat: add ApiError.PARSE_ERROR --- server/src/types/api-error.enum.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/server/src/types/api-error.enum.ts b/server/src/types/api-error.enum.ts index 91dc4fe..4096796 100644 --- a/server/src/types/api-error.enum.ts +++ b/server/src/types/api-error.enum.ts @@ -1,5 +1,6 @@ export const enum ApiError { BAD_CREDENTIALS = "Sorry, these credentials are incorrect. Please try again.", + PARSE_ERROR = "Invalid request. Please check your input and try again.", UNAUTHENTICATED = "Your session has expired. Please sign in again.", WRONG_PASSWORD = "Sorry, this password is incorrect. Please try again.", } From 41d8110324a1f78af0bdd5141c202c4402682e25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johnny=20G=C3=A9rard?= Date: Sat, 2 Nov 2024 15:46:22 +0100 Subject: [PATCH 10/27] feat: use express.text instead of express.json --- server/src/server.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/server/src/server.ts b/server/src/server.ts index 26631c8..8154fdc 100644 --- a/server/src/server.ts +++ b/server/src/server.ts @@ -20,7 +20,12 @@ app.set("trust proxy", 1); const middleware = [ cookieParser(), - express.json(), // Parse JSON requests + // ajv specialized JSON parsers are used instead of `express.json` + // If the request has a JSON payload, `express.text` will parse it as a raw string. + // @see https://ajv.js.org/guide/getting-started.html#parsing-and-serializing-json + express.text({ + type: "application/json", + }), ]; if (isProduction) middleware.unshift(cors); // Enable CORS From 193fe546645c965a8de9c61dac037ae95f9cb77a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johnny=20G=C3=A9rard?= Date: Sat, 2 Nov 2024 15:47:29 +0100 Subject: [PATCH 11/27] feat: update create-session.ts --- server/src/controllers/create-session.ts | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/server/src/controllers/create-session.ts b/server/src/controllers/create-session.ts index 1f4f413..04b526e 100644 --- a/server/src/controllers/create-session.ts +++ b/server/src/controllers/create-session.ts @@ -3,6 +3,7 @@ import { generateCSRFToken } from "../auth/csrf.js"; import { verifyPassword } from "../auth/password-hashing.js"; import { BAD_REQUEST, + CONTENT_TOO_LARGE, CREATED, UNAUTHORIZED, } from "../constants/http-status-code.js"; @@ -12,22 +13,25 @@ import { sessionStore } from "../session/redis-session-store.js"; import { generateSessionCookie } from "../session/session-cookie.js"; import { ApiError } from "../types/api-error.enum.js"; import { ServerSession } from "../types/server-session.js"; -import { - usernameHasValidType, - usernameHasValidValue, -} from "../validation/username.js"; +import { parseCredentials } from "../validation/ajv/credentials.js"; +import { USERNAME_MAX_LENGTH } from "../validation/username.js"; export const createSession: RequestHandler = async (req, res, next) => { try { - const { username, password } = req.body; + const credentials = parseCredentials(req.body); - if (!usernameHasValidType(username) || !usernameHasValidValue(username)) { - res.status(BAD_REQUEST).json("Invalid username"); + if (!credentials) { + res.status(BAD_REQUEST).json(ApiError.PARSE_ERROR); return; } - if (typeof password !== "string" || password.length > PASSWORD_MAX_LENGTH) { - res.status(BAD_REQUEST).json("Invalid password"); + const { username, password } = credentials; + + if ( + username.length > USERNAME_MAX_LENGTH || + password.length > PASSWORD_MAX_LENGTH + ) { + res.status(CONTENT_TOO_LARGE).end(); return; } From 828c54f2b57004c73b0485681a3e25dfd6e1afb9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johnny=20G=C3=A9rard?= Date: Sat, 2 Nov 2024 16:37:33 +0100 Subject: [PATCH 12/27] feat: add LEAKED_PASSWORD and WEAK_PASSWORD to ApiError --- server/src/types/api-error.enum.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/server/src/types/api-error.enum.ts b/server/src/types/api-error.enum.ts index 4096796..e529356 100644 --- a/server/src/types/api-error.enum.ts +++ b/server/src/types/api-error.enum.ts @@ -1,6 +1,8 @@ export const enum ApiError { BAD_CREDENTIALS = "Sorry, these credentials are incorrect. Please try again.", + LEAKED_PASSWORD = "This password has been leaked in a data breach. Please choose a different one.", PARSE_ERROR = "Invalid request. Please check your input and try again.", UNAUTHENTICATED = "Your session has expired. Please sign in again.", + WEAK_PASSWORD = "This password is too weak. Please choose a stronger one.", WRONG_PASSWORD = "Sorry, this password is incorrect. Please try again.", } From 28093a7b7d1845f1ccd0c3021fd1bbccac04a59b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johnny=20G=C3=A9rard?= Date: Sat, 2 Nov 2024 17:07:53 +0100 Subject: [PATCH 13/27] feat: replace some API errors with VALIDATION_MISMATCH --- server/src/controllers/create-session.ts | 2 +- server/src/types/api-error.enum.ts | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/server/src/controllers/create-session.ts b/server/src/controllers/create-session.ts index 04b526e..79af749 100644 --- a/server/src/controllers/create-session.ts +++ b/server/src/controllers/create-session.ts @@ -21,7 +21,7 @@ export const createSession: RequestHandler = async (req, res, next) => { const credentials = parseCredentials(req.body); if (!credentials) { - res.status(BAD_REQUEST).json(ApiError.PARSE_ERROR); + res.status(BAD_REQUEST).json(ApiError.VALIDATION_MISMATCH); return; } diff --git a/server/src/types/api-error.enum.ts b/server/src/types/api-error.enum.ts index e529356..b30acc4 100644 --- a/server/src/types/api-error.enum.ts +++ b/server/src/types/api-error.enum.ts @@ -1,8 +1,7 @@ export const enum ApiError { BAD_CREDENTIALS = "Sorry, these credentials are incorrect. Please try again.", LEAKED_PASSWORD = "This password has been leaked in a data breach. Please choose a different one.", - PARSE_ERROR = "Invalid request. Please check your input and try again.", UNAUTHENTICATED = "Your session has expired. Please sign in again.", - WEAK_PASSWORD = "This password is too weak. Please choose a stronger one.", + VALIDATION_MISMATCH = "Invalid request. Please check your input and try again.", WRONG_PASSWORD = "Sorry, this password is incorrect. Please try again.", } From 388f34f5b07a17d8136cd092819df1328afa441b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johnny=20G=C3=A9rard?= Date: Sat, 2 Nov 2024 17:10:05 +0100 Subject: [PATCH 14/27] refactor: simplify username validation --- .../username-validator.directive.ts | 8 ++------ server/src/validation/username.ts | 20 ++++--------------- 2 files changed, 6 insertions(+), 22 deletions(-) diff --git a/client/src/app/directives/username-validator.directive.ts b/client/src/app/directives/username-validator.directive.ts index fb4fa46..1838c9e 100644 --- a/client/src/app/directives/username-validator.directive.ts +++ b/client/src/app/directives/username-validator.directive.ts @@ -5,10 +5,7 @@ import { ValidationErrors, Validator, } from "@angular/forms"; -import { - usernameHasValidCharacters, - usernameHasValidType, -} from "_server/validation/username"; +import { usernameHasValidCharacters } from "_server/validation/username"; @Directive({ selector: "[appUsernameValidator]", @@ -25,8 +22,7 @@ export class UsernameValidatorDirective implements Validator { validate(control: AbstractControl): ValidationErrors | null { const username = control.value; - return usernameHasValidType(username) && - !usernameHasValidCharacters(username) + return typeof username === "string" && !usernameHasValidCharacters(username) ? { pattern: "Invalid characters" } : null; } diff --git a/server/src/validation/username.ts b/server/src/validation/username.ts index bf47aae..a34a4ab 100644 --- a/server/src/validation/username.ts +++ b/server/src/validation/username.ts @@ -1,17 +1,6 @@ export const USERNAME_MIN_LENGTH = 1; export const USERNAME_MAX_LENGTH = 100; -export const usernameHasValidType = (username: unknown): username is string => { - return typeof username === "string"; -}; - -export const usernameHasValidLength = (username: string): boolean => { - return ( - username.length >= USERNAME_MIN_LENGTH && - username.length <= USERNAME_MAX_LENGTH - ); -}; - /** * Check if the username has valid characters. * @@ -23,8 +12,7 @@ export const usernameHasValidCharacters = (username: string): boolean => { return /^\P{C}*$/u.test(username); }; -export const usernameHasValidValue = (username: string): boolean => { - return ( - usernameHasValidLength(username) && usernameHasValidCharacters(username) - ); -}; +export const isValidUsername = (username: string): boolean => + username.length >= USERNAME_MIN_LENGTH && + username.length <= USERNAME_MAX_LENGTH && + usernameHasValidCharacters(username); From 955cd824af6ef815bc3ae7b9c08155fff42c946a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johnny=20G=C3=A9rard?= Date: Sat, 2 Nov 2024 17:19:11 +0100 Subject: [PATCH 15/27] refactor: rename usernameHasValidCharacters to hasValidUsernameCharacters --- client/src/app/directives/username-validator.directive.ts | 4 ++-- server/src/validation/username.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/client/src/app/directives/username-validator.directive.ts b/client/src/app/directives/username-validator.directive.ts index 1838c9e..2c708d2 100644 --- a/client/src/app/directives/username-validator.directive.ts +++ b/client/src/app/directives/username-validator.directive.ts @@ -5,7 +5,7 @@ import { ValidationErrors, Validator, } from "@angular/forms"; -import { usernameHasValidCharacters } from "_server/validation/username"; +import { hasValidUsernameCharacters } from "_server/validation/username"; @Directive({ selector: "[appUsernameValidator]", @@ -22,7 +22,7 @@ export class UsernameValidatorDirective implements Validator { validate(control: AbstractControl): ValidationErrors | null { const username = control.value; - return typeof username === "string" && !usernameHasValidCharacters(username) + return typeof username === "string" && !hasValidUsernameCharacters(username) ? { pattern: "Invalid characters" } : null; } diff --git a/server/src/validation/username.ts b/server/src/validation/username.ts index a34a4ab..b7fbcde 100644 --- a/server/src/validation/username.ts +++ b/server/src/validation/username.ts @@ -8,11 +8,11 @@ export const USERNAME_MAX_LENGTH = 100; * category. * @see https://unicode.org/reports/tr18/#General_Category_Property */ -export const usernameHasValidCharacters = (username: string): boolean => { +export const hasValidUsernameCharacters = (username: string): boolean => { return /^\P{C}*$/u.test(username); }; export const isValidUsername = (username: string): boolean => username.length >= USERNAME_MIN_LENGTH && username.length <= USERNAME_MAX_LENGTH && - usernameHasValidCharacters(username); + hasValidUsernameCharacters(username); From 17048657ed84fee71a5a2e60003b6f17aa7709c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johnny=20G=C3=A9rard?= Date: Sat, 2 Nov 2024 17:43:44 +0100 Subject: [PATCH 16/27] refactor: rename isPasswordStrong to isValidPassword and incorporate length check --- server/src/controllers/update-password.ts | 4 ++-- server/src/validation/password.test.ts | 8 ++++---- server/src/validation/password.ts | 11 ++++++++--- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/server/src/controllers/update-password.ts b/server/src/controllers/update-password.ts index 147ea71..7218ebf 100644 --- a/server/src/controllers/update-password.ts +++ b/server/src/controllers/update-password.ts @@ -9,7 +9,7 @@ import { } from "../constants/http-status-code.js"; import { PASSWORD_MAX_LENGTH } from "../constants/password.js"; import { users } from "../database/mongo-client.js"; -import { isPasswordStrong } from "../validation/password.js"; +import { isValidPassword } from "../validation/password.js"; import { USERNAME_MAX_LENGTH } from "../validation/username.js"; export const updatePassword: RequestHandler = async (req, res, next) => { @@ -36,7 +36,7 @@ export const updatePassword: RequestHandler = async (req, res, next) => { const isPasswordExposedPromise = isPasswordExposed(newPassword); const digestPromise = hashPassword(newPassword); - const isPasswordStrongPromise = isPasswordStrong( + const isPasswordStrongPromise = isValidPassword( newPassword, oldPassword, username, diff --git a/server/src/validation/password.test.ts b/server/src/validation/password.test.ts index cd587aa..226944f 100644 --- a/server/src/validation/password.test.ts +++ b/server/src/validation/password.test.ts @@ -2,13 +2,13 @@ import { faker } from "@faker-js/faker"; import assert from "node:assert/strict"; import { suite, test } from "node:test"; import { PASSWORD_MAX_LENGTH } from "../constants/password.js"; -import { appDictionary, isPasswordStrong } from "./password.js"; +import { appDictionary, isValidPassword } from "./password.js"; -suite("The isPasswordStrong function", () => { +suite("The isValidPassword function", () => { test("returns true for a strong password", async () => { const username = faker.internet.userName(); const password = faker.internet.password({ length: 20 }); - const isStrong = await isPasswordStrong(password, username); + const isStrong = await isValidPassword(password, username); assert(isStrong); }); @@ -18,7 +18,7 @@ suite("The isPasswordStrong function", () => { // Reusing the username in the password is vulnerable to dictionary attacks // while adding LUDS characters is not enough to make it strong const password = username + "aB1@"; - const isStrong = await isPasswordStrong(password, username); + const isStrong = await isValidPassword(password, username); assert(!isStrong); }); diff --git a/server/src/validation/password.ts b/server/src/validation/password.ts index c65772f..ffd4f63 100644 --- a/server/src/validation/password.ts +++ b/server/src/validation/password.ts @@ -1,7 +1,7 @@ import { readFile } from "node:fs/promises"; import { Piscina } from "piscina"; import { APP_NAME } from "../constants/app.js"; -import { ZXCVBN_MIN_SCORE } from "../constants/password.js"; +import { PASSWORD_MAX_LENGTH, ZXCVBN_MIN_SCORE } from "../constants/password.js"; /** * Application-specific vocabulary for password strength validation @@ -13,16 +13,21 @@ export const appDictionary = text.split("\n"); appDictionary.push(APP_NAME); /** - * Check if the password is strong enough. + * Validate the password strength. + * + * To prevent DoS attacks, passwords whose length exceeds the limit are treated + * as invalid. * @param password - Plaintext password * @param userInputs - Any strings that could be used in a dictionary attack * @returns `true` if the password is strong enough, `false` otherwise * @see https://github.com/dropbox/zxcvbn?tab=readme-ov-file#readme */ -export const isPasswordStrong = async ( +export const isValidPassword = async ( password: string, ...userInputs: string[] ): Promise => { + if (password.length > PASSWORD_MAX_LENGTH) return false; + const piscina = new Piscina({ filename: new URL("zxcvbn.worker.js", import.meta.url).href, }); From 632c2bbfed80b8a98c02178e0d9ad5531ee024b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johnny=20G=C3=A9rard?= Date: Sat, 2 Nov 2024 17:44:45 +0100 Subject: [PATCH 17/27] feat: update create-account.ts --- server/src/controllers/create-account.ts | 31 +++++++++++++----------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/server/src/controllers/create-account.ts b/server/src/controllers/create-account.ts index baf5557..5469926 100644 --- a/server/src/controllers/create-account.ts +++ b/server/src/controllers/create-account.ts @@ -11,12 +11,11 @@ import { users } from "../database/mongo-client.js"; import { User } from "../models/user.js"; import { sessionStore } from "../session/redis-session-store.js"; import { generateSessionCookie } from "../session/session-cookie.js"; +import { ApiError } from "../types/api-error.enum.js"; import { ServerSession } from "../types/server-session.js"; -import { isPasswordStrong } from "../validation/password.js"; -import { - usernameHasValidType, - usernameHasValidValue, -} from "../validation/username.js"; +import { parseCredentials } from "../validation/ajv/credentials.js"; +import { isValidPassword } from "../validation/password.js"; +import { isValidUsername } from "../validation/username.js"; const isUsernameTaken = async (username: string): Promise => { const user = await users.findOne({ username }, { projection: { _id: 1 } }); @@ -25,10 +24,17 @@ const isUsernameTaken = async (username: string): Promise => { export const createAccount: RequestHandler = async (req, res, next) => { try { - const { username, password } = req.body; + const credentials = parseCredentials(req.body); + + if (!credentials) { + res.status(BAD_REQUEST).json(ApiError.VALIDATION_MISMATCH); + return; + } + + const { username, password } = credentials; - if (!usernameHasValidType(username) || !usernameHasValidValue(username)) { - res.status(BAD_REQUEST).json("Invalid username"); + if (!isValidUsername(username)) { + res.status(BAD_REQUEST).json(ApiError.VALIDATION_MISMATCH); return; } @@ -38,16 +44,13 @@ export const createAccount: RequestHandler = async (req, res, next) => { return; } - if ( - typeof password !== "string" || - !(await isPasswordStrong(password, username)) - ) { - res.status(BAD_REQUEST).json("Invalid password"); + if (!(await isValidPassword(password, username))) { + res.status(BAD_REQUEST).json(ApiError.VALIDATION_MISMATCH); return; } if (await isPasswordExposed(password)) { - res.status(BAD_REQUEST).json("Your password was leaked in a data breach"); + res.status(BAD_REQUEST).json(ApiError.LEAKED_PASSWORD); return; } From 3c175cb1379ffcb0a6e80fe94a2c502eac680d44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johnny=20G=C3=A9rard?= Date: Sat, 2 Nov 2024 17:51:15 +0100 Subject: [PATCH 18/27] refactor: rename exposed to leaked --- server/src/auth/pwned-passwords-api.test.ts | 14 +++++++------- server/src/auth/pwned-passwords-api.ts | 6 +++--- server/src/controllers/create-account.ts | 4 ++-- server/src/controllers/update-password.ts | 6 +++--- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/server/src/auth/pwned-passwords-api.test.ts b/server/src/auth/pwned-passwords-api.test.ts index 954e05d..fd27a74 100644 --- a/server/src/auth/pwned-passwords-api.test.ts +++ b/server/src/auth/pwned-passwords-api.test.ts @@ -2,18 +2,18 @@ import { faker } from "@faker-js/faker"; import assert from "node:assert/strict"; import { suite, test } from "node:test"; import { getLeakedPassword } from "../test-helpers/leaked-passwords.js"; -import { isPasswordExposed } from "./pwned-passwords-api.js"; +import { isLeakedPassword } from "./pwned-passwords-api.js"; -suite("The isPasswordExposed function", () => { +suite("The isLeakedPassword function", () => { test("returns true for a leaked password", async () => { const password = getLeakedPassword(); - const isExposed = await isPasswordExposed(password); - assert(isExposed, `Password "${password}" is not exposed`); + const isLeaked = await isLeakedPassword(password); + assert(isLeaked, `Password "${password}" is not leaked`); }); - test("returns false for a secure password", async () => { + test("returns false for a non-leaked password", async () => { const password = faker.internet.password(); - const isExposed = await isPasswordExposed(password); - assert(!isExposed, `Password "${password}" is exposed`); + const isLeaked = await isLeakedPassword(password); + assert(!isLeaked, `Password "${password}" is leaked`); }); }); diff --git a/server/src/auth/pwned-passwords-api.ts b/server/src/auth/pwned-passwords-api.ts index 09f76db..6b7f135 100644 --- a/server/src/auth/pwned-passwords-api.ts +++ b/server/src/auth/pwned-passwords-api.ts @@ -8,11 +8,11 @@ import { OK } from "../constants/http-status-code.js"; * This function queries the Pwned Passwords API using the k-Anonymity model * (only a partial digest of the hashed password is sent). * @param password - Plaintext password - * @returns `false` if the password is not exposed or the API server did not - * reply in time with the 200 status code, `true` if the password is exposed. + * @returns `false` if the password is not leaked or the API server did not + * reply in time with the 200 status code, `true` if the password is leaked. * @see https://haveibeenpwned.com/API/v3#PwnedPasswords */ -export const isPasswordExposed = async (password: string): Promise => { +export const isLeakedPassword = async (password: string): Promise => { try { const digest = hash("sha1", password); const partialDigest = digest.slice(0, 5); diff --git a/server/src/controllers/create-account.ts b/server/src/controllers/create-account.ts index 5469926..08476e4 100644 --- a/server/src/controllers/create-account.ts +++ b/server/src/controllers/create-account.ts @@ -1,7 +1,7 @@ import type { RequestHandler } from "express"; import { generateCSRFToken } from "../auth/csrf.js"; import { hashPassword } from "../auth/password-hashing.js"; -import { isPasswordExposed } from "../auth/pwned-passwords-api.js"; +import { isLeakedPassword } from "../auth/pwned-passwords-api.js"; import { BAD_REQUEST, CONFLICT, @@ -49,7 +49,7 @@ export const createAccount: RequestHandler = async (req, res, next) => { return; } - if (await isPasswordExposed(password)) { + if (await isLeakedPassword(password)) { res.status(BAD_REQUEST).json(ApiError.LEAKED_PASSWORD); return; } diff --git a/server/src/controllers/update-password.ts b/server/src/controllers/update-password.ts index 7218ebf..4e34f52 100644 --- a/server/src/controllers/update-password.ts +++ b/server/src/controllers/update-password.ts @@ -1,7 +1,7 @@ import type { RequestHandler } from "express"; import { ObjectId } from "mongodb"; import { hashPassword } from "../auth/password-hashing.js"; -import { isPasswordExposed } from "../auth/pwned-passwords-api.js"; +import { isLeakedPassword } from "../auth/pwned-passwords-api.js"; import { BAD_REQUEST, INTERNAL_SERVER_ERROR, @@ -34,7 +34,7 @@ export const updatePassword: RequestHandler = async (req, res, next) => { return; } - const isPasswordExposedPromise = isPasswordExposed(newPassword); + const isLeakedPasswordPromise = isLeakedPassword(newPassword); const digestPromise = hashPassword(newPassword); const isPasswordStrongPromise = isValidPassword( newPassword, @@ -47,7 +47,7 @@ export const updatePassword: RequestHandler = async (req, res, next) => { return; } - if (await isPasswordExposedPromise) { + if (await isLeakedPasswordPromise) { res.status(BAD_REQUEST).json("Your password was leaked in a data breach"); return; } From 2ba7809f104463ebc22ef5fd1eb9a705bcb5acce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johnny=20G=C3=A9rard?= Date: Sat, 2 Nov 2024 17:53:21 +0100 Subject: [PATCH 19/27] refactor: rename pwned-passwords-api to is-leaked-password --- .../{pwned-passwords-api.test.ts => is-leaked-password.test.ts} | 2 +- .../src/auth/{pwned-passwords-api.ts => is-leaked-password.ts} | 0 server/src/controllers/create-account.ts | 2 +- server/src/controllers/update-password.ts | 2 +- 4 files changed, 3 insertions(+), 3 deletions(-) rename server/src/auth/{pwned-passwords-api.test.ts => is-leaked-password.test.ts} (92%) rename server/src/auth/{pwned-passwords-api.ts => is-leaked-password.ts} (100%) diff --git a/server/src/auth/pwned-passwords-api.test.ts b/server/src/auth/is-leaked-password.test.ts similarity index 92% rename from server/src/auth/pwned-passwords-api.test.ts rename to server/src/auth/is-leaked-password.test.ts index fd27a74..a79d5ff 100644 --- a/server/src/auth/pwned-passwords-api.test.ts +++ b/server/src/auth/is-leaked-password.test.ts @@ -2,7 +2,7 @@ import { faker } from "@faker-js/faker"; import assert from "node:assert/strict"; import { suite, test } from "node:test"; import { getLeakedPassword } from "../test-helpers/leaked-passwords.js"; -import { isLeakedPassword } from "./pwned-passwords-api.js"; +import { isLeakedPassword } from "./is-leaked-password.js"; suite("The isLeakedPassword function", () => { test("returns true for a leaked password", async () => { diff --git a/server/src/auth/pwned-passwords-api.ts b/server/src/auth/is-leaked-password.ts similarity index 100% rename from server/src/auth/pwned-passwords-api.ts rename to server/src/auth/is-leaked-password.ts diff --git a/server/src/controllers/create-account.ts b/server/src/controllers/create-account.ts index 08476e4..a580763 100644 --- a/server/src/controllers/create-account.ts +++ b/server/src/controllers/create-account.ts @@ -1,7 +1,7 @@ import type { RequestHandler } from "express"; import { generateCSRFToken } from "../auth/csrf.js"; +import { isLeakedPassword } from "../auth/is-leaked-password.js"; import { hashPassword } from "../auth/password-hashing.js"; -import { isLeakedPassword } from "../auth/pwned-passwords-api.js"; import { BAD_REQUEST, CONFLICT, diff --git a/server/src/controllers/update-password.ts b/server/src/controllers/update-password.ts index 4e34f52..65ef4ad 100644 --- a/server/src/controllers/update-password.ts +++ b/server/src/controllers/update-password.ts @@ -1,7 +1,7 @@ import type { RequestHandler } from "express"; import { ObjectId } from "mongodb"; +import { isLeakedPassword } from "../auth/is-leaked-password.js"; import { hashPassword } from "../auth/password-hashing.js"; -import { isLeakedPassword } from "../auth/pwned-passwords-api.js"; import { BAD_REQUEST, INTERNAL_SERVER_ERROR, From da9a6da82e120e97deb6de9af6230ac8064e147d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johnny=20G=C3=A9rard?= Date: Sat, 2 Nov 2024 18:15:04 +0100 Subject: [PATCH 20/27] refactor: use form.invalid property --- .../src/app/components/register-form/register-form.component.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/src/app/components/register-form/register-form.component.ts b/client/src/app/components/register-form/register-form.component.ts index 0c01a43..fa6392f 100644 --- a/client/src/app/components/register-form/register-form.component.ts +++ b/client/src/app/components/register-form/register-form.component.ts @@ -136,7 +136,7 @@ export class RegisterFormComponent { } onSubmit(): void { - if (!this.form.valid || this.isLoading()) return; + if (this.form.invalid || this.isLoading()) return; if (this.#passwordStrength.isWorkerBusy()) { this.#shouldResubmit = true; return; From a126d98a6d8e91d6fc0ab05a3399b0b56119f0b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johnny=20G=C3=A9rard?= Date: Sat, 2 Nov 2024 22:10:35 +0100 Subject: [PATCH 21/27] feat: add parsePassword in password.ts --- server/src/validation/ajv/password.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 server/src/validation/ajv/password.ts diff --git a/server/src/validation/ajv/password.ts b/server/src/validation/ajv/password.ts new file mode 100644 index 0000000..bf82cd0 --- /dev/null +++ b/server/src/validation/ajv/password.ts @@ -0,0 +1,11 @@ +import { JTDSchemaType } from "ajv/dist/types/jtd-schema.js"; +import { ajv } from "./index.js"; + +const schema: JTDSchemaType<{ password: string }> = { + properties: { + password: { type: "string" }, + }, + additionalProperties: false, +}; + +export const parsePassword = ajv.compileParser(schema); From 8f25830c60c868faf7ce7ea2c5b3aa76949424f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johnny=20G=C3=A9rard?= Date: Sun, 3 Nov 2024 00:17:15 +0100 Subject: [PATCH 22/27] refactor: integrate max length checks in parsers --- server/src/controllers/create-session.ts | 11 ----------- server/src/validation/ajv/credentials.ts | 16 +++++++++++++++- server/src/validation/ajv/password.ts | 14 ++++++++++++-- 3 files changed, 27 insertions(+), 14 deletions(-) diff --git a/server/src/controllers/create-session.ts b/server/src/controllers/create-session.ts index 79af749..fbc1ea2 100644 --- a/server/src/controllers/create-session.ts +++ b/server/src/controllers/create-session.ts @@ -3,18 +3,15 @@ import { generateCSRFToken } from "../auth/csrf.js"; import { verifyPassword } from "../auth/password-hashing.js"; import { BAD_REQUEST, - CONTENT_TOO_LARGE, CREATED, UNAUTHORIZED, } from "../constants/http-status-code.js"; -import { PASSWORD_MAX_LENGTH } from "../constants/password.js"; import { users } from "../database/mongo-client.js"; import { sessionStore } from "../session/redis-session-store.js"; import { generateSessionCookie } from "../session/session-cookie.js"; import { ApiError } from "../types/api-error.enum.js"; import { ServerSession } from "../types/server-session.js"; import { parseCredentials } from "../validation/ajv/credentials.js"; -import { USERNAME_MAX_LENGTH } from "../validation/username.js"; export const createSession: RequestHandler = async (req, res, next) => { try { @@ -27,14 +24,6 @@ export const createSession: RequestHandler = async (req, res, next) => { const { username, password } = credentials; - if ( - username.length > USERNAME_MAX_LENGTH || - password.length > PASSWORD_MAX_LENGTH - ) { - res.status(CONTENT_TOO_LARGE).end(); - return; - } - // Retrieve user from database const user = await users.findOne( { username }, diff --git a/server/src/validation/ajv/credentials.ts b/server/src/validation/ajv/credentials.ts index 5e5ce50..95caacd 100644 --- a/server/src/validation/ajv/credentials.ts +++ b/server/src/validation/ajv/credentials.ts @@ -1,5 +1,7 @@ import type { JTDSchemaType } from "ajv/dist/types/jtd-schema.js"; +import { PASSWORD_MAX_LENGTH } from "../../constants/password.js"; import { Credentials } from "../../types/credentials.js"; +import { USERNAME_MAX_LENGTH } from "../username.js"; import { ajv } from "./index.js"; const schema: JTDSchemaType = { @@ -10,4 +12,16 @@ const schema: JTDSchemaType = { additionalProperties: false, }; -export const parseCredentials = ajv.compileParser(schema); +const parse = ajv.compileParser(schema); + +export const parseCredentials = (json: string): Credentials | undefined => { + const credentials = parse(json); + + if ( + credentials && + credentials.username.length <= USERNAME_MAX_LENGTH && + credentials.password.length <= PASSWORD_MAX_LENGTH + ) { + return credentials; + } +}; diff --git a/server/src/validation/ajv/password.ts b/server/src/validation/ajv/password.ts index bf82cd0..c980398 100644 --- a/server/src/validation/ajv/password.ts +++ b/server/src/validation/ajv/password.ts @@ -1,11 +1,21 @@ import { JTDSchemaType } from "ajv/dist/types/jtd-schema.js"; +import { PASSWORD_MAX_LENGTH } from "../../constants/password.js"; import { ajv } from "./index.js"; -const schema: JTDSchemaType<{ password: string }> = { +type Password = { password: string }; + +const schema: JTDSchemaType = { properties: { password: { type: "string" }, }, additionalProperties: false, }; -export const parsePassword = ajv.compileParser(schema); +const parse = ajv.compileParser(schema); + +export const parsePassword = (json: string): Password | undefined => { + const credentials = parse(json); + + if (credentials && credentials.password.length <= PASSWORD_MAX_LENGTH) + return credentials; +}; From 746971d6472e0ab5fb75d9ad474725d75fb9c3fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johnny=20G=C3=A9rard?= Date: Sun, 3 Nov 2024 00:25:07 +0100 Subject: [PATCH 23/27] feat: use parsePassword in deleteAccount controller --- server/src/controllers/delete-account.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/server/src/controllers/delete-account.ts b/server/src/controllers/delete-account.ts index bc533d7..e729439 100644 --- a/server/src/controllers/delete-account.ts +++ b/server/src/controllers/delete-account.ts @@ -6,21 +6,22 @@ import { NO_CONTENT, UNAUTHORIZED, } from "../constants/http-status-code.js"; -import { PASSWORD_MAX_LENGTH } from "../constants/password.js"; import { users } from "../database/mongo-client.js"; import { ApiError } from "../types/api-error.enum.js"; +import { parsePassword } from "../validation/ajv/password.js"; export const deleteAccount: RequestHandler = async (req, res, next) => { try { const userId = req.user!.id; const _id = new ObjectId(userId); - const { password } = req.body; + const credentials = parsePassword(req.body); - if (typeof password !== "string" || password.length > PASSWORD_MAX_LENGTH) { - res.status(BAD_REQUEST).json("Invalid password"); + if (!credentials) { + res.status(BAD_REQUEST).json(ApiError.VALIDATION_MISMATCH); return; } + const { password } = credentials; const user = await users.findOne({ _id }, { projection: { password: 1 } }); if (!user) { From 82c6786f4a6bde7e11d52afad5c1d264d6ed7a7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johnny=20G=C3=A9rard?= Date: Sun, 3 Nov 2024 01:41:53 +0100 Subject: [PATCH 24/27] feat: add ApiError.DATABASE_ERROR --- server/src/types/api-error.enum.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/server/src/types/api-error.enum.ts b/server/src/types/api-error.enum.ts index b30acc4..e13a93c 100644 --- a/server/src/types/api-error.enum.ts +++ b/server/src/types/api-error.enum.ts @@ -1,5 +1,6 @@ export const enum ApiError { BAD_CREDENTIALS = "Sorry, these credentials are incorrect. Please try again.", + DATABASE_ERROR = "Your request failed due to a database error. Please try again later.", LEAKED_PASSWORD = "This password has been leaked in a data breach. Please choose a different one.", UNAUTHENTICATED = "Your session has expired. Please sign in again.", VALIDATION_MISMATCH = "Invalid request. Please check your input and try again.", From 68eee5bbb5d8ac268e06940ad72a8ccd268d2d78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johnny=20G=C3=A9rard?= Date: Sun, 3 Nov 2024 01:42:55 +0100 Subject: [PATCH 25/27] feat: add NewPassword type and parseNewPassword function --- server/src/types/new-password.ts | 4 ++++ server/src/validation/ajv/new-password.ts | 26 +++++++++++++++++++++++ 2 files changed, 30 insertions(+) create mode 100644 server/src/types/new-password.ts create mode 100644 server/src/validation/ajv/new-password.ts diff --git a/server/src/types/new-password.ts b/server/src/types/new-password.ts new file mode 100644 index 0000000..d38ff29 --- /dev/null +++ b/server/src/types/new-password.ts @@ -0,0 +1,4 @@ +export type NewPassword = { + oldPassword: string; + newPassword: string; +}; diff --git a/server/src/validation/ajv/new-password.ts b/server/src/validation/ajv/new-password.ts new file mode 100644 index 0000000..8c139b0 --- /dev/null +++ b/server/src/validation/ajv/new-password.ts @@ -0,0 +1,26 @@ +import { JTDSchemaType } from "ajv/dist/types/jtd-schema.js"; +import { PASSWORD_MAX_LENGTH } from "../../constants/password.js"; +import { NewPassword } from "../../types/new-password.js"; +import { ajv } from "./index.js"; + +const schema: JTDSchemaType = { + properties: { + oldPassword: { type: "string" }, + newPassword: { type: "string" }, + }, + additionalProperties: false, +}; + +const parse = ajv.compileParser(schema); + +export const parseNewPassword = (json: string): NewPassword | undefined => { + const credentials = parse(json); + + if ( + credentials && + credentials.oldPassword.length <= PASSWORD_MAX_LENGTH && + credentials.newPassword.length <= PASSWORD_MAX_LENGTH + ) { + return credentials; + } +}; From c380944f3af221a5cc244d43f845b469c15066a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johnny=20G=C3=A9rard?= Date: Sun, 3 Nov 2024 01:43:15 +0100 Subject: [PATCH 26/27] feat: improve updatePassword controller --- server/src/controllers/update-password.ts | 68 ++++++++++++----------- 1 file changed, 36 insertions(+), 32 deletions(-) diff --git a/server/src/controllers/update-password.ts b/server/src/controllers/update-password.ts index 65ef4ad..bfab35d 100644 --- a/server/src/controllers/update-password.ts +++ b/server/src/controllers/update-password.ts @@ -1,64 +1,68 @@ import type { RequestHandler } from "express"; import { ObjectId } from "mongodb"; import { isLeakedPassword } from "../auth/is-leaked-password.js"; -import { hashPassword } from "../auth/password-hashing.js"; +import { hashPassword, verifyPassword } from "../auth/password-hashing.js"; import { BAD_REQUEST, INTERNAL_SERVER_ERROR, NO_CONTENT, + UNAUTHORIZED, } from "../constants/http-status-code.js"; -import { PASSWORD_MAX_LENGTH } from "../constants/password.js"; import { users } from "../database/mongo-client.js"; +import { ApiError } from "../types/api-error.enum.js"; +import { parseNewPassword } from "../validation/ajv/new-password.js"; import { isValidPassword } from "../validation/password.js"; -import { USERNAME_MAX_LENGTH } from "../validation/username.js"; export const updatePassword: RequestHandler = async (req, res, next) => { try { - const { oldPassword, newPassword, username } = req.body; + const _id = new ObjectId(req.user!.id); + const credentials = parseNewPassword(req.body); - if ( - typeof oldPassword !== "string" || - typeof newPassword !== "string" || - typeof username !== "string" - ) { - res.status(BAD_REQUEST).json("Invalid payload structure"); + if (!credentials) { + res.status(BAD_REQUEST).json(ApiError.VALIDATION_MISMATCH); return; } - if ( - newPassword.length > PASSWORD_MAX_LENGTH || - oldPassword.length > PASSWORD_MAX_LENGTH || - username.length > USERNAME_MAX_LENGTH - ) { - res.status(BAD_REQUEST).json("Maximum input size exceeded"); + const { oldPassword, newPassword } = credentials; + + const [digest, isLeaked, isValid, user] = await Promise.all([ + hashPassword(newPassword), + isLeakedPassword(newPassword), + isValidPassword(newPassword, oldPassword), + users.findOne({ _id }, { projection: { password: 1 } }), + ]); + + if (!isValid) { + res.status(BAD_REQUEST).json(ApiError.VALIDATION_MISMATCH); return; } - const isLeakedPasswordPromise = isLeakedPassword(newPassword); - const digestPromise = hashPassword(newPassword); - const isPasswordStrongPromise = isValidPassword( - newPassword, - oldPassword, - username, - ); + if (isLeaked) { + res.status(BAD_REQUEST).json(ApiError.LEAKED_PASSWORD); + return; + } - if (!(await isPasswordStrongPromise)) { - res.status(BAD_REQUEST).json("Weak password"); + if (!user) { + console.error(`User not found (ID: ${req.user!.id})`); + res.status(INTERNAL_SERVER_ERROR).json(ApiError.DATABASE_ERROR); return; } - if (await isLeakedPasswordPromise) { - res.status(BAD_REQUEST).json("Your password was leaked in a data breach"); + const matches = await verifyPassword(user.password, oldPassword); + + if (!matches) { + res.status(UNAUTHORIZED).json(ApiError.WRONG_PASSWORD); return; } - const result = await users.updateOne( - { _id: new ObjectId(req.user!.id) }, - { $set: { password: await digestPromise } }, + const updateResult = await users.updateOne( + { _id }, + { $set: { password: digest } }, ); - if (result.modifiedCount !== 1) { - res.status(INTERNAL_SERVER_ERROR).json("Failed to update password"); + if (updateResult.modifiedCount !== 1) { + console.error("Failed to update password", updateResult); + res.status(INTERNAL_SERVER_ERROR).json(ApiError.DATABASE_ERROR); return; } From 19a032104384ed897692294f1b9f2ebcc537d588 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johnny=20G=C3=A9rard?= Date: Sun, 3 Nov 2024 02:12:39 +0100 Subject: [PATCH 27/27] test: increase default timeout --- client/playwright.config.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/playwright.config.ts b/client/playwright.config.ts index 73ca62e..cec62d1 100644 --- a/client/playwright.config.ts +++ b/client/playwright.config.ts @@ -11,7 +11,7 @@ import ms from "ms"; * See https://playwright.dev/docs/test-configuration. */ export default defineConfig({ - timeout: ms("10 seconds"), + timeout: ms("20 seconds"), globalTimeout: ms("5 minutes"), testDir: "./e2e", /* Run tests in files in parallel */