From 070ed8e6e5d6405d6dabead8a632920e817699b4 Mon Sep 17 00:00:00 2001 From: Abhishek Date: Thu, 5 Nov 2026 05:25:17 +0000 Subject: [PATCH] fix(http-server): validate join and create quiz input before prisma code, username, title and description were only checked for being truthy, so an object, array or number got through to prisma and came back as a 500. A request with no body was a 500 too. parseJoinQuiz and parseCreateQuiz in utils/quizInput.ts now check the type and length (username 30 after trim, title 120, description 2000) and the controllers answer with errorResponse 400. Closes #231 --- .../src/controllers/createQuiz.test.ts | 58 +++++++++++ .../src/controllers/quizController.ts | 6 +- .../src/controllers/sessionController.test.ts | 45 ++++++++- .../src/controllers/sessionController.ts | 10 +- apps/http-server/src/utils/quizInput.test.ts | 95 +++++++++++++++++++ apps/http-server/src/utils/quizInput.ts | 55 +++++++++++ 6 files changed, 260 insertions(+), 9 deletions(-) create mode 100644 apps/http-server/src/controllers/createQuiz.test.ts create mode 100644 apps/http-server/src/utils/quizInput.test.ts create mode 100644 apps/http-server/src/utils/quizInput.ts diff --git a/apps/http-server/src/controllers/createQuiz.test.ts b/apps/http-server/src/controllers/createQuiz.test.ts new file mode 100644 index 0000000..644cc9d --- /dev/null +++ b/apps/http-server/src/controllers/createQuiz.test.ts @@ -0,0 +1,58 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { Request, Response } from "express"; + +const { create } = vi.hoisted(() => ({ create: vi.fn() })); + +vi.mock("@repo/db", () => ({ prisma: { quiz: { create } } })); + +import { createQuizController } from "./quizController"; + +const run = async (body: unknown) => { + const req = { body, userId: "user-1" } as unknown as Request; + const res = { status: vi.fn().mockReturnThis(), json: vi.fn() } as unknown as Response; + await createQuizController(req, res); + return res; +}; + +const valid = { title: "Capitals", description: "Countries of Europe" }; + +describe("createQuizController", () => { + beforeEach(() => { + vi.clearAllMocks(); + create.mockResolvedValue({ id: "quiz-1" }); + }); + + it.each([ + ["no body at all", undefined, "Title and description are required"], + ["no title", { description: "d" }, "Title and description are required"], + ["no description", { title: "t" }, "Title and description are required"], + ["an empty title", { ...valid, title: "" }, "Title and description are required"], + ["a blank description", { ...valid, description: " " }, "Title and description are required"], + ["an object title", { ...valid, title: {} }, "title must be a string"], + ["an array title", { ...valid, title: ["Capitals"] }, "title must be a string"], + ["a numeric title", { ...valid, title: 42 }, "title must be a string"], + ["an object description", { ...valid, description: {} }, "description must be a string"], + ["an array description", { ...valid, description: ["d"] }, "description must be a string"], + ["a 121 character title", { ...valid, title: "t".repeat(121) }, "title must be at most 120 characters"], + [ + "a 2001 character description", + { ...valid, description: "d".repeat(2001) }, + "description must be at most 2000 characters", + ], + ])("answers 400 for %s and never creates the quiz", async (_label, body, message) => { + const res = await run(body); + + expect(res.status).toHaveBeenCalledWith(400); + expect(res.json).toHaveBeenCalledWith({ success: false, error: message }); + expect(create).not.toHaveBeenCalled(); + }); + + it("still creates a valid quiz with a 201", async () => { + const res = await run(valid); + + expect(create).toHaveBeenCalledWith({ + data: expect.objectContaining({ ...valid, creatorId: "user-1" }), + }); + expect(res.status).toHaveBeenCalledWith(201); + }); +}); diff --git a/apps/http-server/src/controllers/quizController.ts b/apps/http-server/src/controllers/quizController.ts index 67f45c1..89d0766 100644 --- a/apps/http-server/src/controllers/quizController.ts +++ b/apps/http-server/src/controllers/quizController.ts @@ -3,14 +3,16 @@ import { errorResponse } from '../utils/error'; import { prisma } from '@repo/db'; import { generateCode } from '../utils/code'; import { parseCreateQuestion } from '../utils/questionInput'; +import { parseCreateQuiz } from '../utils/quizInput'; import { renumberQuestions } from '../utils/questionOrder'; export const createQuizController = async (req: Request, res: Response) => { try { - const { title, description } = req.body; + const parsed = parseCreateQuiz(req.body); + if (!parsed.ok) return errorResponse(res, 400, parsed.error); - if (!title || !description) return errorResponse(res, 400, "Title and description are required"); + const { title, description } = parsed.data; const quiz = await prisma.quiz.create({ data: { diff --git a/apps/http-server/src/controllers/sessionController.test.ts b/apps/http-server/src/controllers/sessionController.test.ts index b3a076f..1c62198 100644 --- a/apps/http-server/src/controllers/sessionController.test.ts +++ b/apps/http-server/src/controllers/sessionController.test.ts @@ -1,9 +1,13 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import type { Request, Response } from "express"; -const { findUnique } = vi.hoisted(() => ({ findUnique: vi.fn() })); +const { findUnique, participantCreate, transaction } = vi.hoisted(() => ({ + findUnique: vi.fn(), + participantCreate: vi.fn(), + transaction: vi.fn(), +})); -vi.mock("@repo/db", () => ({ prisma: { quiz: { findUnique } } })); +vi.mock("@repo/db", () => ({ prisma: { quiz: { findUnique }, $transaction: transaction } })); import { joinQuizController } from "./sessionController"; @@ -24,6 +28,8 @@ describe("joinQuizController", () => { ["no username", { code: "ABC123" }], ["no code", { username: "ada" }], ["an empty username", { code: "ABC123", username: "" }], + ["a blank username", { code: "ABC123", username: " " }], + ["no body at all", undefined], ])("answers 400 with { success, error } for %s and never reads the quiz", async (_label, body) => { const res = await run(body); @@ -32,6 +38,41 @@ describe("joinQuizController", () => { expect(findUnique).not.toHaveBeenCalled(); }); + it.each([ + ["an object username", { code: "ABC123", username: {} }, "username must be a string"], + ["an array username", { code: "ABC123", username: ["ada"] }, "username must be a string"], + ["a numeric username", { code: "ABC123", username: 42 }, "username must be a string"], + ["an object code", { code: { contains: "A" }, username: "ada" }, "code must be a string"], + ["an array code", { code: ["ABC123"], username: "ada" }, "code must be a string"], + ["a 31 character username", { code: "ABC123", username: "a".repeat(31) }, "username must be at most 30 characters"], + ])("answers 400 for %s instead of reaching prisma", async (_label, body, message) => { + const res = await run(body); + + expect(res.status).toHaveBeenCalledWith(400); + expect(res.json).toHaveBeenCalledWith({ success: false, error: message }); + expect(findUnique).not.toHaveBeenCalled(); + expect(transaction).not.toHaveBeenCalled(); + }); + + it("stores the trimmed username", async () => { + findUnique.mockResolvedValue({ + id: "quiz-1", + title: "Quiz", + status: "ACTIVE", + quizSessions: [{ id: "session-1", liveVideo: false }], + }); + participantCreate.mockResolvedValue({ id: "participant-1" }); + transaction.mockImplementation((fn) => fn({ participant: { create: participantCreate } })); + + const res = await run({ code: "ABC123", username: " ada " }); + + expect(res.status).toHaveBeenCalledWith(200); + expect(findUnique).toHaveBeenCalledWith(expect.objectContaining({ where: { joinCode: "ABC123" } })); + expect(participantCreate).toHaveBeenCalledWith({ + data: { username: "ada", sessionId: "session-1", userId: null }, + }); + }); + it("answers 404 in the same shape when the quiz does not exist", async () => { findUnique.mockResolvedValue(null); const res = await run({ code: "NOPE00", username: "ada" }); diff --git a/apps/http-server/src/controllers/sessionController.ts b/apps/http-server/src/controllers/sessionController.ts index 7b93b2e..558af57 100644 --- a/apps/http-server/src/controllers/sessionController.ts +++ b/apps/http-server/src/controllers/sessionController.ts @@ -3,6 +3,7 @@ import { prisma } from "@repo/db"; import { Request, Response } from "express"; import jwt from "jsonwebtoken"; import { errorResponse } from "../utils/error"; +import { parseJoinQuiz } from "../utils/quizInput"; import { string } from "zod"; /** @@ -22,12 +23,11 @@ export const joinQuizController = async (req: Request, res: Response) => { try { - const { code, username } = req.body; - // validate input - if (!code || !username) { - return errorResponse(res, 400, "Code and username are required"); - } + const parsed = parseJoinQuiz(req.body); + if (!parsed.ok) return errorResponse(res, 400, parsed.error); + + const { code, username } = parsed.data; const quiz = await prisma.quiz.findUnique({ where: { diff --git a/apps/http-server/src/utils/quizInput.test.ts b/apps/http-server/src/utils/quizInput.test.ts new file mode 100644 index 0000000..f08da0a --- /dev/null +++ b/apps/http-server/src/utils/quizInput.test.ts @@ -0,0 +1,95 @@ +import { describe, expect, it } from 'vitest'; +import { parseCreateQuiz, parseJoinQuiz } from './quizInput'; + +const joinError = (body: unknown) => { + const result = parseJoinQuiz(body); + return result.ok ? null : result.error; +}; + +const quizError = (body: unknown) => { + const result = parseCreateQuiz(body); + return result.ok ? null : result.error; +}; + +const join = { code: 'ABC123', username: 'ada' }; +const quiz = { title: 'Capitals', description: 'Countries of Europe' }; + +describe('parseJoinQuiz', () => { + it('accepts a normal join', () => { + expect(parseJoinQuiz(join)).toEqual({ ok: true, data: join }); + }); + + it('trims the username and leaves the code as sent', () => { + expect(parseJoinQuiz({ code: ' abc ', username: ' ada lovelace ' })).toEqual({ + ok: true, + data: { code: ' abc ', username: 'ada lovelace' }, + }); + }); + + it('allows 30 characters and counts them after the trim', () => { + expect(parseJoinQuiz({ ...join, username: 'a'.repeat(30) }).ok).toBe(true); + expect(parseJoinQuiz({ ...join, username: ` ${'a'.repeat(30)} ` }).ok).toBe(true); + }); + + it('rejects a username longer than 30 characters', () => { + expect(joinError({ ...join, username: 'a'.repeat(31) })).toBe('username must be at most 30 characters'); + }); + + it.each([[{}], [['ada']], [[]], [42], [true]])('rejects a username of type %j', (username) => { + expect(joinError({ ...join, username })).toBe('username must be a string'); + }); + + it.each([[{}], [['ABC123']], [123456], [false]])('rejects a code of type %j', (code) => { + expect(joinError({ ...join, code })).toBe('code must be a string'); + }); + + it.each([[''], [' '], ['\t\n'], [null]])('treats %j as not filled in', (value) => { + expect(joinError({ ...join, username: value })).toBe('Code and username are required'); + expect(joinError({ ...join, code: value })).toBe('Code and username are required'); + }); + + it('does not report a blank username as too long', () => { + expect(joinError({ ...join, username: ' '.repeat(40) })).toBe('Code and username are required'); + }); + + it.each([[undefined], [null], ['text'], [42], [[]], [[1, 2]]])('treats the body %j as all fields missing', (body) => { + expect(joinError(body)).toBe('Code and username are required'); + }); +}); + +describe('parseCreateQuiz', () => { + it('accepts a normal quiz', () => { + expect(parseCreateQuiz(quiz)).toEqual({ ok: true, data: quiz }); + }); + + it('does not trim what it accepts', () => { + const body = { title: ' Capitals ', description: ' Europe ' }; + expect(parseCreateQuiz(body)).toEqual({ ok: true, data: body }); + }); + + it('allows a 120 character title and a 2000 character description', () => { + expect(parseCreateQuiz({ title: 't'.repeat(120), description: 'd'.repeat(2000) }).ok).toBe(true); + }); + + it('rejects a title longer than 120 characters', () => { + expect(quizError({ ...quiz, title: 't'.repeat(121) })).toBe('title must be at most 120 characters'); + }); + + it('rejects a description longer than 2000 characters', () => { + expect(quizError({ ...quiz, description: 'd'.repeat(2001) })).toBe('description must be at most 2000 characters'); + }); + + it.each([[{}], [['Capitals']], [[]], [42], [true]])('rejects a title or description of type %j', (value) => { + expect(quizError({ ...quiz, title: value })).toBe('title must be a string'); + expect(quizError({ ...quiz, description: value })).toBe('description must be a string'); + }); + + it.each([[''], [' '], ['\t\n'], [null]])('treats %j as not filled in', (value) => { + expect(quizError({ ...quiz, title: value })).toBe('Title and description are required'); + expect(quizError({ ...quiz, description: value })).toBe('Title and description are required'); + }); + + it.each([[undefined], [null], ['text'], [42], [[]], [[1, 2]]])('treats the body %j as all fields missing', (body) => { + expect(quizError(body)).toBe('Title and description are required'); + }); +}); diff --git a/apps/http-server/src/utils/quizInput.ts b/apps/http-server/src/utils/quizInput.ts new file mode 100644 index 0000000..b5ce022 --- /dev/null +++ b/apps/http-server/src/utils/quizInput.ts @@ -0,0 +1,55 @@ +import { z } from 'zod'; + +export const MAX_USERNAME_LENGTH = 30; +export const MAX_TITLE_LENGTH = 120; +export const MAX_DESCRIPTION_LENGTH = 2000; + +const joinRequired = 'Code and username are required'; +const quizRequired = 'Title and description are required'; + +// Missing, null and blank all read as "not filled in"; any other type is a +// different mistake and gets its own message. +const requiredString = (field: string, required: string) => + z + .string({ + error: (issue) => + issue.input === undefined || issue.input === null ? required : `${field} must be a string`, + }) + .refine((value) => value.trim().length > 0, required); + +const tooLong = (field: string, max: number) => `${field} must be at most ${max} characters`; + +const joinQuizSchema = z.object({ + code: requiredString('code', joinRequired), + // The limit counts what gets stored, so trim first. + username: requiredString('username', joinRequired) + .trim() + .max(MAX_USERNAME_LENGTH, tooLong('username', MAX_USERNAME_LENGTH)), +}); + +const createQuizSchema = z.object({ + title: requiredString('title', quizRequired).max(MAX_TITLE_LENGTH, tooLong('title', MAX_TITLE_LENGTH)), + description: requiredString('description', quizRequired).max( + MAX_DESCRIPTION_LENGTH, + tooLong('description', MAX_DESCRIPTION_LENGTH) + ), +}); + +// One message for the 400 response, for the first field that failed. +const parseBody = (schema: z.ZodType, body: unknown): + | { ok: true; data: T } + | { ok: false; error: string } => { + // A request with no JSON body has no req.body at all. + const input = typeof body === 'object' && body !== null && !Array.isArray(body) ? body : {}; + + const result = schema.safeParse(input); + if (result.success) return { ok: true, data: result.data }; + + return { ok: false, error: result.error.issues[0].message }; +}; + +// Checks the body of POST /session/join. username comes back trimmed. +export const parseJoinQuiz = (body: unknown) => parseBody(joinQuizSchema, body); + +// Checks the body of POST /quizzes. The values are returned as sent. +export const parseCreateQuiz = (body: unknown) => parseBody(createQuizSchema, body);