Use zod for validation on create endpoint (#1391)

* add @liveinstantly/dash-mpd-parser and zod dependencies * add createRoomSchema for room creation validation * use zod validations in createRoom rather than manual checks. * actually get rid of duplicate checks * try to use infer * add zod back to package.json * fix lint * add zod-validation-error dependency * use z.infer for OttApiRequestRoomCreate * move zod schema to ott common and fix lint * fix import * add error messages * added zoderror handling to errorhandler * add zod error to error handler * fix error message * updated zod schema to account for regex and reserved room names. * removed duplicate checks. * update unit tests in create endpoint * update zod schema * add zod dependency to ott-common * remove duplicate checks --------- Co-authored-by: Carson McManus <[email protected]>
dyc3 · Mar 6, 2024 · 1137ce7 · 1137ce7
1 parent 4187c67
commit 1137ce7
Show file tree

Hide file tree

Showing 7 changed files with 73 additions and 113 deletions.
diff --git a/common/models/rest-api.ts b/common/models/rest-api.ts
@@ -3,6 +3,8 @@ import { ServerMessageEvent } from "./messages";
 import { BehaviorOption, QueueMode, RoomSettings, RoomUserInfo, Visibility } from "./types";
 import { QueueItem, Video, VideoId } from "./video";
 import { Category } from "sponsorblock-api";
+import { createRoomSchema } from "./zod-schemas";
+import { z } from "zod";
 
 export type OttResponseBody<T = unknown, E extends OttApiError = OttApiError> =
 	| OttSuccessResponseBody<T>
@@ -33,12 +35,7 @@ export interface OttApiResponseRoomGenerate {
 }
 
 /** Endpoint: `/api/room/create` */
-export interface OttApiRequestRoomCreate {
-	title?: string;
-	name: string;
-	isTemporary?: boolean;
-	visibility?: Visibility;
-}
+export type OttApiRequestRoomCreate = z.infer<typeof createRoomSchema>;
 
 /** Endpoint: `/api/room/create` */
 export interface OttApiResponseRoomCreate {}

diff --git a/common/models/zod-schemas.ts b/common/models/zod-schemas.ts
@@ -0,0 +1,20 @@
+import { ROOM_NAME_REGEX } from "ott-common/constants";
+import { Visibility, QueueMode } from "ott-common/models/types";
+import { z } from "zod";
+
+// These strings are not allowed to be used as room names.
+const RESERVED_ROOM_NAMES = ["list", "create", "generate"];
+
+export const createRoomSchema = z.object({
+	name: z
+		.string()
+		.min(3, "too short, must be atleast 3 characters")
+		.max(32, "too long, must be at most 32 characters")
+		.regex(ROOM_NAME_REGEX)
+		.refine(name => !RESERVED_ROOM_NAMES.includes(name), { message: "not allowed (reserved)" }),
+	title: z.string().max(255, "too long, must be at most 255 characters").optional(),
+	description: z.string().optional(),
+	isTemporary: z.boolean().optional().default(true),
+	visibility: z.nativeEnum(Visibility).default(Visibility.Public).optional(),
+	queueMode: z.nativeEnum(QueueMode).optional(),
+});
diff --git a/common/package.json b/common/package.json
@@ -29,7 +29,8 @@
     "lodash": "^4.17.21",
     "sponsorblock-api": "^0.2.2",
     "ts-essentials": "^9.3.0",
-    "typescript": "5.3.3"
+    "typescript": "5.3.3",
+    "zod": "^3.22.4"
   },
   "devDependencies": {
     "@types/lodash": "^4.14.170",
@@ -38,4 +39,4 @@
     "eslint-plugin-vitest": "0.3.22",
     "vitest": "^1.2.2"
   }
-}
+}
diff --git a/server/api/room.ts b/server/api/room.ts
@@ -32,13 +32,13 @@ import { getApiKey } from "../admin";
 import { v4 as uuidv4 } from "uuid";
 import { counterHttpErrors } from "../metrics";
 import { conf } from "../ott-config";
+import { createRoomSchema } from "ott-common/models/zod-schemas";
+import { ZodError } from "zod";
+import { fromZodError } from "zod-validation-error";
 
 const router = express.Router();
 const log = getLogger("api/room");
 
-// These strings are not allowed to be used as room names.
-const RESERVED_ROOM_NAMES = ["list", "create", "generate"];
-
 const VALID_ROOM_VISIBILITY = [Visibility.Public, Visibility.Unlisted, Visibility.Private];
 
 const VALID_ROOM_QUEUE_MODE = [QueueMode.Manual, QueueMode.Vote, QueueMode.Loop, QueueMode.Dj];
@@ -119,77 +119,32 @@ const createRoom: RequestHandler<
 	OttResponseBody<OttApiResponseRoomCreate>,
 	OttApiRequestRoomCreate
 > = async (req, res) => {
-	function isValidCreateRoom(body: any): body is OttApiRequestRoomCreate {
-		return !!body.name;
-	}
-	if (!isValidCreateRoom(req.body)) {
-		throw new BadApiArgumentException("name", "missing");
-	}
+	const body = createRoomSchema.parse(req.body);
 
-	if (req.body.isTemporary && !conf.get("room.enable_create_temporary")) {
+	if (body.isTemporary && !conf.get("room.enable_create_temporary")) {
 		throw new FeatureDisabledException("Temporary rooms are disabled.");
-	} else if (!req.body.isTemporary && !conf.get("room.enable_create_permanent")) {
+	} else if (!body.isTemporary && !conf.get("room.enable_create_permanent")) {
 		throw new FeatureDisabledException("Permanent rooms are disabled.");
 	}
 
-	if (!req.body.name) {
-		throw new BadApiArgumentException("name", "missing");
-	}
-	if (RESERVED_ROOM_NAMES.includes(req.body.name)) {
-		throw new BadApiArgumentException("name", "not allowed (reserved)");
-	}
-	if (req.body.name.length < 3) {
-		throw new BadApiArgumentException(
-			"name",
-			"not allowed (too short, must be at least 3 characters)"
-		);
-	}
-	if (req.body.name.length > 32) {
-		throw new BadApiArgumentException(
-			"name",
-			"not allowed (too long, must be at most 32 characters)"
-		);
-	}
-
-	if (req.body.title && req.body.title.length > 255) {
-		throw new BadApiArgumentException(
-			"title",
-			"not allowed (too long, must be at most 255 characters)"
-		);
-	}
-
-	if (!ROOM_NAME_REGEX.exec(req.body.name)) {
-		throw new BadApiArgumentException("name", "not allowed (invalid characters)");
-	}
-	if (req.body.visibility && !VALID_ROOM_VISIBILITY.includes(req.body.visibility)) {
-		throw new BadApiArgumentException(
-			"visibility",
-			`must be one of ${VALID_ROOM_VISIBILITY.toString()}`
-		);
-	}
 	let points = 50;
-	if (req.body.isTemporary === undefined) {
-		req.body.isTemporary = true;
-	}
-	if (!req.body.isTemporary) {
+
+	if (!body.isTemporary) {
 		points *= 4;
 	}
 	if (!(await consumeRateLimitPoints(res, req.ip, points))) {
 		return;
 	}
 
-	if (!req.body.visibility) {
-		req.body.visibility = Visibility.Public;
-	}
 	if (req.user) {
-		await roommanager.createRoom({ ...req.body, owner: req.user });
+		await roommanager.createRoom({ ...body, owner: req.user });
 	} else {
-		await roommanager.createRoom(req.body);
+		await roommanager.createRoom(body);
 	}
 	log.info(
-		`${req.body.isTemporary ? "Temporary" : "Permanent"} room created: name=${
-			req.body.name
-		} ip=${req.ip} user-agent=${req.headers["user-agent"]}`
+		`${body.isTemporary ? "Temporary" : "Permanent"} room created: name=${body.name} ip=${
+			req.ip
+		} user-agent=${req.headers["user-agent"]}`
 	);
 	res.status(201).json({
 		success: true,
@@ -519,6 +474,14 @@ const errorHandler: ErrorRequestHandler = (err: Error, req, res) => {
 				},
 			});
 		}
+	} else if (err instanceof ZodError) {
+		err = fromZodError(err);
+		res.status(400).json({
+			success: false,
+			error: {
+				name: err.name,
+			},
+		});
 	} else {
 		log.error(`Unhandled exception: path=${req.path} ${err.name} ${err.message} ${err.stack}`);
 		res.status(500).json({

diff --git a/server/package.json b/server/package.json
@@ -52,7 +52,9 @@
     "uuid": "^9.0.0",
     "validator": "^13.7.0",
     "winston": "^3.10.0",
-    "ws": "^7.4.6"
+    "ws": "^7.4.6",
+    "zod": "^3.22.4",
+    "zod-validation-error": "^3.0.2"
   },
   "devDependencies": {
     "@types/convict": "^6.1.1",

diff --git a/server/tests/unit/api/room.spec.ts b/server/tests/unit/api/room.spec.ts
@@ -214,63 +214,31 @@ describe("Room API", () => {
 		);
 
 		it.each([
-			[{ arg: "name", reason: "missing" }, { isTemporary: true }],
+			[{ isTemporary: true }],
+			[{ name: "list", isTemporary: true }],
+			[{ name: "create", isTemporary: true }],
+			[{ name: "generate", isTemporary: true }],
+			[{ name: "abc<", isTemporary: true }],
+			[{ name: "abc[", isTemporary: true }],
+			[{ name: "abc]", isTemporary: true }],
+			[{ name: "abc!", isTemporary: true }],
+			[{ name: "abc!", isTemporary: true }],
+			[{ name: "ab", isTemporary: true }],
 			[
-				{ arg: "name", reason: "not allowed (reserved)" },
-				{ name: "list", isTemporary: true },
-			],
-			[
-				{ arg: "name", reason: "not allowed (reserved)" },
-				{ name: "create", isTemporary: true },
-			],
-			[
-				{ arg: "name", reason: "not allowed (reserved)" },
-				{ name: "generate", isTemporary: true },
-			],
-			[
-				{ arg: "name", reason: "not allowed (invalid characters)" },
-				{ name: "abc<", isTemporary: true },
-			],
-			[
-				{ arg: "name", reason: "not allowed (invalid characters)" },
-				{ name: "abc[", isTemporary: true },
-			],
-			[
-				{ arg: "name", reason: "not allowed (invalid characters)" },
-				{ name: "abc]", isTemporary: true },
-			],
-			[
-				{ arg: "name", reason: "not allowed (invalid characters)" },
-				{ name: "abc!", isTemporary: true },
-			],
-			[
-				{ arg: "name", reason: "not allowed (invalid characters)" },
-				{ name: "abc!", isTemporary: true },
-			],
-			[
-				{ arg: "name", reason: "not allowed (too short, must be at least 3 characters)" },
-				{ name: "ab", isTemporary: true },
-			],
-			[
-				{ arg: "name", reason: "not allowed (too long, must be at most 32 characters)" },
 				{
 					name: "abababababababababababababababababababababababababababababababababab",
 					isTemporary: true,
 				},
 			],
 			[
-				{ arg: "title", reason: "not allowed (too long, must be at most 255 characters)" },
 				{
 					name: "foo",
 					title: "abababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababab",
 					isTemporary: true,
 				},
 			],
-			[
-				{ arg: "visibility", reason: "must be one of public,unlisted,private" },
-				{ name: "test1", isTemporary: true, visibility: "invalid" },
-			],
-		])("should fail to create room for validation errors: %s", async (error, body) => {
+			[{ name: "test1", isTemporary: true, visibility: "invalid" }],
+		])("should fail to create room for validation errors: %s", async body => {
 			let resp = await request(app)
 				.post("/api/room/create")
 				.auth(token, { type: "bearer" })
@@ -279,8 +247,7 @@ describe("Room API", () => {
 				.expect(400);
 			expect(resp.body.success).toEqual(false);
 			expect(resp.body.error).toMatchObject({
-				name: "BadApiArgumentException",
-				...error,
+				name: "ZodValidationError",
 			});
 		});
 

diff --git a/yarn.lock b/yarn.lock
@@ -15383,6 +15383,16 @@ yocto-queue@^1.0.0:
   resolved "https://registry.yarnpkg.com/yocto-queue/-/yocto-queue-1.0.0.tgz#7f816433fb2cbc511ec8bf7d263c3b58a1a3c251"
   integrity sha512-9bnSc/HEW2uRy67wc+T8UwauLuPJVn28jb+GtJY16iiKWyvmYJRXVT4UamsAEGQfPohgr2q4Tq0sQbQlxTfi1g==
 
+zod-validation-error@^3.0.2:
+  version "3.0.2"
+  resolved "https://registry.yarnpkg.com/zod-validation-error/-/zod-validation-error-3.0.2.tgz#35a0f15e998d7fc88a569d50a9c1dbc3fb1abaae"
+  integrity sha512-21xGaDmnU7lJZ4J63n5GXWqi+rTzGy3gDHbuZ1jP6xrK/DEQGyOqs/xW7eH96tIfCOYm+ecCuT0bfajBRKEVUw==
+
+zod@^3.22.4:
+  version "3.22.4"
+  resolved "https://registry.yarnpkg.com/zod/-/zod-3.22.4.tgz#f31c3a9386f61b1f228af56faa9255e845cf3fff"
+  integrity sha512-iC+8Io04lddc+mVqQ9AZ7OQ2MrUKGN+oIQyq1vemgt46jwCwLfhq7/pwnBnNXXXZb8VTVLKwp9EDkx+ryxIWmg==
+
 zstddec@^0.1.0:
   version "0.1.0"
   resolved "https://registry.yarnpkg.com/zstddec/-/zstddec-0.1.0.tgz#7050f3f0e0c3978562d0c566b3e5a427d2bad7ec"