diff --git a/README.md b/README.md index d9656d7..1f8872c 100644 --- a/README.md +++ b/README.md @@ -95,7 +95,7 @@ To access additional functionality, users need to create an account. Logged-in u - Add games to favorites - Access user-specific functionality -> **Important:** Passwords are currently stored in plain text. Do not use a real or reused password when creating an account for testing. Use a temporary password that is not used anywhere else. +> **Important:** Passwords are now hashed, but you should still not use real or reused passwords when creating an account for testing. Use a temporary password that is not used anywhere else. --- @@ -397,4 +397,4 @@ Planned improvements include: This README reflects the current state of the project for the third interim delivery. -At this stage, GamingGoat is intended to function as a first complete prototype. The main focus is on testing and improving core functionality such as searching, browsing, sorting, filtering, accessibility, and the interaction between the frontend, backend, and database. \ No newline at end of file +At this stage, GamingGoat is intended to function as a first complete prototype. The main focus is on testing and improving core functionality such as searching, browsing, sorting, filtering, accessibility, and the interaction between the frontend, backend, and database. diff --git a/T04-Project-2/backend/package.json b/T04-Project-2/backend/package.json index 38d8240..4301613 100644 --- a/T04-Project-2/backend/package.json +++ b/T04-Project-2/backend/package.json @@ -4,14 +4,15 @@ "description": "", "main": "index.js", "scripts": { - "test": "echo \"Error: no test specified\" && exit 2", + "test": "node --import tsx --test tests/*.test.ts", "dev": "tsx watch src/index.ts", "start": "tsx src/index.ts", "import-rawg": "tsx src/scripts/importRawg.ts", "db-seed": "tsx src/scripts/seed.ts", "db-ensure-schema": "tsx src/db/ensureSchema.ts", "db-push": "npm run db-ensure-schema && drizzle-kit push", - "db-studio": "drizzle-kit studio" + "db-studio": "drizzle-kit studio", + "db-hash-passwords": "tsx src/scripts/hashExistingPasswords.ts" }, "keywords": [], "type": "commonjs", diff --git a/T04-Project-2/backend/src/auth/password.ts b/T04-Project-2/backend/src/auth/password.ts new file mode 100644 index 0000000..f647493 --- /dev/null +++ b/T04-Project-2/backend/src/auth/password.ts @@ -0,0 +1,36 @@ +import { randomBytes, scrypt, timingSafeEqual } from "node:crypto"; + +// OWASP scrypt baseline: N=2^17, r=8, p=1. Use an independent salt per password. +const PREFIX = "scrypt$131072$8$1$"; +const KEY_LENGTH = 64; +const OPTIONS = { N: 131072, r: 8, p: 1, maxmem: 256 * 1024 * 1024 }; +const HASH_PATTERN = /^scrypt\$131072\$8\$1\$[a-f0-9]{32}\$[a-f0-9]{128}$/; + +function deriveKey(password: string, salt: Buffer): Promise { + return new Promise((resolve, reject) => { + scrypt(password, salt, KEY_LENGTH, OPTIONS, (error, key) => { + if (error) reject(error); + else resolve(key); + }); + }); +} + +export function isPasswordHash(value: string): boolean { + return HASH_PATTERN.test(value); +} + +export async function hashPassword(password: string): Promise { + const salt = randomBytes(16); + const key = await deriveKey(password, salt); + return `${PREFIX}${salt.toString("hex")}$${key.toString("hex")}`; +} + +export async function verifyPassword(password: string, storedHash: string): Promise { + // Never accept plaintext or malformed/unsupported hashes, or trust stored cost parameters. + if (!isPasswordHash(storedHash)) return false; + const parts = storedHash.split("$"); + const salt = Buffer.from(parts[4]!, "hex"); + const expectedKey = Buffer.from(parts[5]!, "hex"); + const actualKey = await deriveKey(password, salt); + return timingSafeEqual(actualKey, expectedKey); +} diff --git a/T04-Project-2/backend/src/db/schema.ts b/T04-Project-2/backend/src/db/schema.ts index 0b625fe..d17ad08 100644 --- a/T04-Project-2/backend/src/db/schema.ts +++ b/T04-Project-2/backend/src/db/schema.ts @@ -53,7 +53,7 @@ export const platforms = pgTable("platforms", { export const users = pgTable("users", { id: serial("id").primaryKey(), username: text("username").notNull().unique(), - password: text("password").notNull(), // Currently not hashed or validated + password: text("password").notNull(), // Salted scrypt hash; never store plaintext // profile_picture: text("profile_picture"), }); diff --git a/T04-Project-2/backend/src/graphql/resolvers.ts b/T04-Project-2/backend/src/graphql/resolvers.ts index dc577e1..2aed673 100644 --- a/T04-Project-2/backend/src/graphql/resolvers.ts +++ b/T04-Project-2/backend/src/graphql/resolvers.ts @@ -1,3 +1,4 @@ +import { hashPassword, verifyPassword } from "../auth/password"; import { db } from "../db"; import { games, @@ -250,7 +251,7 @@ export const resolvers = { try { const [user] = await db .insert(users) - .values({ username, password }) + .values({ username, password: await hashPassword(password) }) .returning({ id: users.id, username: users.username, @@ -273,15 +274,14 @@ export const resolvers = { .select({ id: users.id, username: users.username, + password: users.password, }) .from(users) - .where(and( - eq(users.username, username), - eq(users.password, password), - )) + .where(eq(users.username, username)) .limit(1); - return user ?? null; + if (!user || !await verifyPassword(password, user.password)) return null; + return { id: user.id, username: user.username }; }, createReview: async ( _parent: unknown, diff --git a/T04-Project-2/backend/src/scripts/hashExistingPasswords.ts b/T04-Project-2/backend/src/scripts/hashExistingPasswords.ts new file mode 100644 index 0000000..b640d10 --- /dev/null +++ b/T04-Project-2/backend/src/scripts/hashExistingPasswords.ts @@ -0,0 +1,60 @@ +import "dotenv/config"; +import { and, asc, eq, gt } from "drizzle-orm"; +import { hashPassword, isPasswordHash } from "../auth/password"; +import { db, pool } from "../db"; +import { users } from "../db/schema"; + +async function hashExistingPasswords() { + let lastId = 0; + let updated = 0; + try { + while (true) { + const batch = await db.select({ id: users.id, password: users.password }) + .from(users).where(gt(users.id, lastId)).orderBy(asc(users.id)).limit(100); + if (batch.length === 0) break; + for (const user of batch) { + lastId = user.id; + if (isPasswordHash(user.password)) continue; + const password = await hashPassword(user.password); + // Avoid overwriting a password changed since the batch was read. + const changed = await db.update(users).set({ password }) + .where(and(eq(users.id, user.id), eq(users.password, user.password))) + .returning({ id: users.id }); + updated += changed.length; + } + } + console.log(`Hashed passwords for ${updated} existing users.`); + } finally { + await pool.end(); + } +} + +hashExistingPasswords().catch((error: unknown) => { + // Never log raw database errors: they may include legacy plaintext passwords. + const hints: Record = { + ECONNREFUSED: "Database connection refused. Start PostgreSQL (docker compose up -d from the project root) and check the host/port in DATABASE_URL.", + ENOTFOUND: "Database hostname was not found. Check the host in DATABASE_URL.", + EAI_AGAIN: "Database hostname lookup failed temporarily. Check the connection and retry.", + ETIMEDOUT: "Database connection timed out. Check that PostgreSQL is reachable.", + "28P01": "Database authentication failed. Check the credentials in DATABASE_URL.", + "3D000": "The configured database does not exist. Check DATABASE_URL and database setup.", + "42P01": "The users table is missing. Check DATABASE_URL and run npm run db-push if this database needs its schema created.", + "42703": "A required database column is missing. Check the schema with npm run db-push.", + "42501": "The database account lacks permission to read or update users.", + }; + function findHint(value: unknown): string | undefined { + if (!value || typeof value !== "object") return; + const failure = value as { code?: unknown; cause?: unknown; errors?: unknown[] }; + if (typeof failure.code === "string" && hints[failure.code]) return hints[failure.code]; + const causeHint = findHint(failure.cause); + if (causeHint) return causeHint; + if (Array.isArray(failure.errors)) { + for (const nested of failure.errors) { + const hint = findHint(nested); + if (hint) return hint; + } + } + } + console.error(`Password migration failed. ${findHint(error) ?? "Check database access and rerun the command."}`); + process.exitCode = 1; +}); diff --git a/T04-Project-2/backend/src/scripts/seed.ts b/T04-Project-2/backend/src/scripts/seed.ts index 8985ec6..4f3013f 100644 --- a/T04-Project-2/backend/src/scripts/seed.ts +++ b/T04-Project-2/backend/src/scripts/seed.ts @@ -1,3 +1,4 @@ +import { hashPassword } from "../auth/password"; import "dotenv/config"; import process from "node:process"; import { inArray } from "drizzle-orm"; @@ -80,6 +81,10 @@ function randomSubset(items: T[]): T[] { async function seedDatabase() { try { const seedUserValues = createSeedUserValues(); + // Hash sequentially to bound memory usage during seeding. + for (const user of seedUserValues) { + user.password = await hashPassword(user.password); + } const gameRows = await db .select({ id: games.id }) .from(games); diff --git a/T04-Project-2/backend/tests/auth.test.ts b/T04-Project-2/backend/tests/auth.test.ts new file mode 100644 index 0000000..aedd936 --- /dev/null +++ b/T04-Project-2/backend/tests/auth.test.ts @@ -0,0 +1,39 @@ +import assert from "node:assert/strict"; +import { afterEach, mock, test } from "node:test"; +import { hashPassword, isPasswordHash, verifyPassword } from "../src/auth/password"; + +// Resolvers are tested with a stubbed database; no connection is opened. +process.env.DATABASE_URL = "postgres://unused:unused@127.0.0.1:1/unused"; +const modules = Promise.all([import("../src/db/index.js"), import("../src/graphql/resolvers.js")]); +afterEach(() => mock.restoreAll()); + +test("registration stores a hash and returns no password", async () => { + const [{ db }, { resolvers }] = await modules; + let stored: { username: string; password: string } | undefined; + const insert = { + values(value: { username: string; password: string }) { + stored = value; + return { returning: async () => [{ id: 1, username: value.username }] }; + }, + }; + mock.method(db, "insert", () => insert); + const result = await resolvers.Mutation.createUser(null, { username: "alice", password: "secret" }); + assert.deepEqual(result, { id: 1, username: "alice" }); + assert.ok(stored && isPasswordHash(stored.password)); + assert.equal(await verifyPassword("secret", stored.password), true); +}); + +test("login checks the hash, rejects incorrect credentials, and omits the hash", async () => { + const [{ db }, { resolvers }] = await modules; + let rows = [{ id: 1, username: "alice", password: await hashPassword("secret") }]; + mock.method(db, "select", () => ({ + from: () => ({ where: () => ({ limit: async () => rows }) }), + })); + assert.deepEqual(await resolvers.Mutation.login(null, { username: "alice", password: "secret" }), + { id: 1, username: "alice" }); + assert.equal(await resolvers.Mutation.login(null, { username: "alice", password: "wrong" }), null); + rows = [{ id: 1, username: "alice", password: "secret" }]; + assert.equal(await resolvers.Mutation.login(null, { username: "alice", password: "secret" }), null); + rows = []; + assert.equal(await resolvers.Mutation.login(null, { username: "missing", password: "secret" }), null); +}); diff --git a/T04-Project-2/backend/tests/password.test.ts b/T04-Project-2/backend/tests/password.test.ts new file mode 100644 index 0000000..9e74946 --- /dev/null +++ b/T04-Project-2/backend/tests/password.test.ts @@ -0,0 +1,25 @@ +import assert from "node:assert/strict"; +import { test } from "node:test"; +import { hashPassword, isPasswordHash, verifyPassword } from "../src/auth/password"; + +test("hashes use independent salts and verify the exact password", async () => { + const password = " Løsenord 🔒 with spaces "; + const first = await hashPassword(password); + const second = await hashPassword(password); + assert.ok(isPasswordHash(first)); + assert.notEqual(first, second); + assert.ok(!first.includes(password)); + assert.equal(await verifyPassword(password, first), true); + assert.equal(await verifyPassword(password, second), true); + assert.equal(await verifyPassword("wrong", first), false); + assert.equal(await verifyPassword(password.trim(), first), false); + assert.equal(await verifyPassword(first, first), false); +}); + +test("plaintext, malformed hashes, and unsupported parameters are rejected", async () => { + for (const stored of ["", "secret", "scrypt$", "scrypt$131072$8$1$00$00", + `scrypt$999999999$8$1$${"a".repeat(32)}$${"b".repeat(128)}`, + `scrypt$131072$8$1$${"z".repeat(32)}$${"b".repeat(128)}`]) { + assert.equal(await verifyPassword("secret", stored), false); + } +}); diff --git a/T04-Project-2/docs/howToRunTheProject.md b/T04-Project-2/docs/howToRunTheProject.md index 13574e4..c265753 100644 --- a/T04-Project-2/docs/howToRunTheProject.md +++ b/T04-Project-2/docs/howToRunTheProject.md @@ -91,6 +91,28 @@ http://localhost:4000/graphql --- +## Password storage and existing accounts + +Passwords are stored as salted scrypt hashes using Node's built-in crypto module +(N=131072, r=8, p=1), including users created by `npm run db-seed`. +See the [OWASP password storage guidance](https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#scrypt). +The existing `password` text column stores the complete hash; no schema change is needed. + +Before starting the updated backend against an existing database, stop the backend +and run the following from `backend` with `DATABASE_URL` pointing to that database: + +```sh +npm run db-hash-passwords +``` + +This converts existing plaintext passwords without changing users' passwords. +It skips hashes already generated by this application and can be rerun after an +interruption. Restart the backend once it succeeds. Login rejects plaintext +password records, so existing users cannot log in until conversion is complete. +Previously created database backups may still contain plaintext passwords. + +--- + ## Optional database tools To inspect the database using Drizzle Studio, run the following from the backend directory: