From 98d7bd0b3a047438173aa5a334054ad40429706c Mon Sep 17 00:00:00 2001 From: Abhishek Date: Tue, 27 Oct 2026 06:42:00 +0000 Subject: [PATCH] fix: return 400 naming the missing field when registering with incomplete input --- .../src/controllers/authController.test.ts | 64 +++++++++++++++++++ .../src/controllers/authController.ts | 7 +- .../src/utils/registerInput.test.ts | 54 ++++++++++++++++ apps/http-server/src/utils/registerInput.ts | 49 ++++++++++++++ 4 files changed, 173 insertions(+), 1 deletion(-) create mode 100644 apps/http-server/src/controllers/authController.test.ts create mode 100644 apps/http-server/src/utils/registerInput.test.ts create mode 100644 apps/http-server/src/utils/registerInput.ts diff --git a/apps/http-server/src/controllers/authController.test.ts b/apps/http-server/src/controllers/authController.test.ts new file mode 100644 index 0000000..626f584 --- /dev/null +++ b/apps/http-server/src/controllers/authController.test.ts @@ -0,0 +1,64 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { Request, Response } from "express"; + +const { findUnique, create, hash } = vi.hoisted(() => ({ + findUnique: vi.fn(), + create: vi.fn(), + hash: vi.fn(), +})); + +vi.mock("@repo/db", () => ({ prisma: { user: { findUnique, create } } })); +vi.mock("bcrypt", () => ({ default: { hash, compare: vi.fn() } })); + +import { userRegisterController } from "./authController"; + +const run = async (body: unknown) => { + const req = { body } as unknown as Request; + const res = { status: vi.fn().mockReturnThis(), json: vi.fn() } as unknown as Response; + await userRegisterController(req, res); + return res; +}; + +const valid = { name: "Ada", email: "ada@example.com", password: "secret12" }; + +describe("userRegisterController", () => { + beforeEach(() => { + vi.clearAllMocks(); + hash.mockResolvedValue("hashed"); + findUnique.mockResolvedValue(null); + create.mockResolvedValue({ id: "user-1", name: "Ada", email: "ada@example.com" }); + }); + + it.each([ + ["no password", { name: "Ada", email: "ada@example.com" }, "password is required"], + ["an empty password", { ...valid, password: "" }, "password is required"], + ["a blank name", { ...valid, name: " " }, "name is required"], + ["a numeric password", { ...valid, password: 12345678 }, "password must be a string"], + ["no body at all", undefined, "name, email and password are required"], + ])("answers 400 for %s and never reaches the database or bcrypt", 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(hash).not.toHaveBeenCalled(); + expect(create).not.toHaveBeenCalled(); + }); + + it("still registers a valid user with a 201 and a token", async () => { + const res = await run(valid); + + expect(hash).toHaveBeenCalledWith("secret12", 10); + expect(create).toHaveBeenCalledWith({ data: { name: "Ada", password: "hashed", email: "ada@example.com" } }); + expect(res.status).toHaveBeenCalledWith(201); + }); + + it("still answers 400 for an email that is already registered", async () => { + findUnique.mockResolvedValue({ id: "user-1" }); + const res = await run(valid); + + expect(res.status).toHaveBeenCalledWith(400); + expect(res.json).toHaveBeenCalledWith({ success: false, error: "Email is already existed.." }); + expect(create).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/http-server/src/controllers/authController.ts b/apps/http-server/src/controllers/authController.ts index 86aaab0..72c0f04 100644 --- a/apps/http-server/src/controllers/authController.ts +++ b/apps/http-server/src/controllers/authController.ts @@ -4,13 +4,18 @@ import { prisma } from "@repo/db" import bcrypt from 'bcrypt' import jwt from "jsonwebtoken" import { errorResponse } from "../utils/error" +import { parseRegisterInput } from "../utils/registerInput" export const userRegisterController = async (req: Request, res: Response) => { try { - const { name, email, password } = req.body; + // Nothing below runs (findUnique, bcrypt) until all three fields are real strings. + const parsed = parseRegisterInput(req.body); + if (!parsed.ok) return errorResponse(res, 400, parsed.error); + + const { name, email, password } = parsed.data; // is user existed already diff --git a/apps/http-server/src/utils/registerInput.test.ts b/apps/http-server/src/utils/registerInput.test.ts new file mode 100644 index 0000000..d35b340 --- /dev/null +++ b/apps/http-server/src/utils/registerInput.test.ts @@ -0,0 +1,54 @@ +import { describe, expect, it } from 'vitest'; +import { parseRegisterInput } from './registerInput'; + +const valid = { name: 'Ada', email: 'ada@example.com', password: 'secret12' }; + +const errorFor = (body: unknown) => { + const result = parseRegisterInput(body); + return result.ok ? null : result.error; +}; + +describe('parseRegisterInput', () => { + it('accepts a normal registration and returns the values as sent', () => { + const result = parseRegisterInput(valid); + expect(result).toEqual({ ok: true, data: valid }); + }); + + it('does not trim what it accepts', () => { + const body = { name: ' Ada ', email: 'ada@example.com', password: ' secret ' }; + expect(parseRegisterInput(body)).toEqual({ ok: true, data: body }); + }); + + it('names a missing field', () => { + expect(errorFor({ name: 'Ada', email: 'ada@example.com' })).toBe('password is required'); + expect(errorFor({ email: 'ada@example.com', password: 'secret12' })).toBe('name is required'); + expect(errorFor({ name: 'Ada', password: 'secret12' })).toBe('email is required'); + }); + + it('names every missing field', () => { + expect(errorFor({ email: 'a@b.c' })).toBe('name and password are required'); + expect(errorFor({})).toBe('name, email and password are required'); + }); + + it.each([[''], [' '], ['\t\n'], [null]])('treats %j as not filled in', (value) => { + expect(errorFor({ ...valid, password: value })).toBe('password is required'); + expect(errorFor({ ...valid, name: value })).toBe('name is required'); + expect(errorFor({ ...valid, email: value })).toBe('email is required'); + }); + + it.each([[12345678], [true], [['secret12']], [{ a: 1 }]])('rejects a password of type %j', (value) => { + expect(errorFor({ ...valid, password: value })).toBe('password must be a string'); + }); + + it('rejects an email that is not a string', () => { + expect(errorFor({ ...valid, email: { $ne: 'x' } })).toBe('email must be a string'); + }); + + it('reports a missing field and a wrong type together', () => { + expect(errorFor({ email: 7, password: 'secret12' })).toBe('name is required; email must be a string'); + }); + + it.each([[undefined], [null], ['text'], [42], [[]], [[1, 2]]])('treats the body %j as all fields missing', (body) => { + expect(errorFor(body)).toBe('name, email and password are required'); + }); +}); diff --git a/apps/http-server/src/utils/registerInput.ts b/apps/http-server/src/utils/registerInput.ts new file mode 100644 index 0000000..643d201 --- /dev/null +++ b/apps/http-server/src/utils/registerInput.ts @@ -0,0 +1,49 @@ +import { z } from 'zod'; + +const isRequired = 'is required'; +const mustBeString = 'must be a string'; + +// Missing, null and blank all read as "not filled in"; any other type is a +// different mistake and gets its own message. +const requiredString = z + .string({ error: (issue) => (issue.input === undefined || issue.input === null ? isRequired : mustBeString) }) + .refine((value) => value.trim().length > 0, isRequired); + +export const registerSchema = z.object({ + name: requiredString, + email: requiredString, + password: requiredString, +}); + +export type RegisterInput = z.infer; + +// "password is required", "name and password are required", +// "email must be a string", ... +const sentence = (fields: string[], message: string) => { + const names = + fields.length > 1 ? `${fields.slice(0, -1).join(', ')} and ${fields[fields.length - 1]}` : fields[0]; + return `${names} ${message === isRequired && fields.length > 1 ? 'are required' : message}`; +}; + +// Checks the body of POST /auth/register. The values are returned as sent +// (not trimmed), so a valid registration behaves exactly as before. +export function parseRegisterInput(body: unknown): + | { ok: true; data: RegisterInput } + | { 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 = registerSchema.safeParse(input); + if (result.success) return { ok: true, data: result.data }; + + const byMessage = new Map(); + for (const issue of result.error.issues) { + const field = String(issue.path[0]); + const fields = byMessage.get(issue.message) ?? []; + if (!fields.includes(field)) fields.push(field); + byMessage.set(issue.message, fields); + } + + const error = [...byMessage].map(([message, fields]) => sentence(fields, message)).join('; '); + return { ok: false, error }; +}