Switch to using zod for validation for patch endpoint. (#1579)

* initial commit * updated schemas * update zod schemas * made some progress * requested changes * properly narrowed types * add strict to get it to throw errors * clean up the code a little * update tests --------- Co-authored-by: Carson McManus <[email protected]>
dyc3 · Apr 28, 2024 · e581552 · e581552
1 parent abdf353
commit e581552
Show file tree

Hide file tree

Showing 4 changed files with 86 additions and 59 deletions.
diff --git a/common/models/rest-api.ts b/common/models/rest-api.ts
@@ -10,6 +10,9 @@ import {
 	OttApiRequestRemoveFromQueueSchema,
 	OttApiRequestAccountRecoveryStartSchema,
 	OttApiRequestAccountRecoveryVerifySchema,
+	OttApiRequestPatchRoomSchema,
+	ClaimSchema,
+	RoomSettingsSchema,
 } from "./zod-schemas";
 import { z } from "zod";
 
@@ -65,11 +68,7 @@ export interface OttApiResponseGetRoom extends RoomSettings {
 	users: RoomUserInfo[];
 }
 
-export interface OttApiRequestPatchRoom extends Partial<RoomSettings> {
-	claim?: boolean;
-	/** @deprecated Use `grants` instead */
-	permissions?: Grants;
-}
+export type OttApiRequestPatchRoom = z.infer<typeof OttApiRequestPatchRoomSchema>;
 
 export interface OttApiRequestUndo {
 	event: ServerMessageEvent;
@@ -92,3 +91,7 @@ export type OttApiRequestAccountRecoveryStart = z.infer<
 export type OttApiRequestAccountRecoveryVerify = z.infer<
 	typeof OttApiRequestAccountRecoveryVerifySchema
 >;
+
+export type OttClaimRequest = z.infer<typeof ClaimSchema>;
+
+export type OttSettingsRequest = z.infer<typeof RoomSettingsSchema>;
diff --git a/common/models/zod-schemas.ts b/common/models/zod-schemas.ts
@@ -1,4 +1,5 @@
 import { ALL_VIDEO_SERVICES, ROOM_NAME_REGEX } from "ott-common/constants";
+import { BehaviorOption, Role } from "ott-common/models/types";
 import { Visibility, QueueMode } from "ott-common/models/types";
 import { z } from "zod";
 
@@ -55,3 +56,34 @@ export const OttApiRequestAccountRecoveryVerifySchema = z.object({
 	verifyKey: z.string(),
 	newPassword: z.string(),
 });
+
+const CategorySchema = z.enum([
+	"sponsor",
+	"intro",
+	"outro",
+	"interaction",
+	"selfpromo",
+	"music_offtopic",
+	"preview",
+]);
+
+const GrantSchema = z.tuple([z.nativeEnum(Role), z.number()]);
+
+export const RoomSettingsSchema = z
+	.object({
+		title: z.string().max(254).optional(),
+		description: z.string().optional(),
+		visibility: z.nativeEnum(Visibility).optional(),
+		queueMode: z.nativeEnum(QueueMode).optional(),
+		grants: z.array(GrantSchema).optional(),
+		autoSkipSegmentCategories: z.array(CategorySchema).optional(),
+		restoreQueueBehavior: z.nativeEnum(BehaviorOption).optional(),
+		enableVoteSkip: z.boolean().optional(),
+	})
+	.strict();
+
+export const ClaimSchema = z.object({
+	claim: z.boolean(),
+});
+
+export const OttApiRequestPatchRoomSchema = z.union([RoomSettingsSchema, ClaimSchema]);
diff --git a/server/api/room.ts b/server/api/room.ts
@@ -27,6 +27,8 @@ import {
 	OttApiResponseRoomCreate,
 	OttApiResponseRoomGenerate,
 	OttResponseBody,
+	OttClaimRequest,
+	OttSettingsRequest,
 } from "ott-common/models/rest-api";
 import { getApiKey } from "../admin";
 import { v4 as uuidv4 } from "uuid";
@@ -37,6 +39,7 @@ import {
 	OttApiRequestVoteSchema,
 	OttApiRequestAddToQueueSchema,
 	OttApiRequestRemoveFromQueueSchema,
+	OttApiRequestPatchRoomSchema,
 } from "ott-common/models/zod-schemas";
 import { ZodError } from "zod";
 import { fromZodError } from "zod-validation-error";
@@ -185,73 +188,60 @@ const getRoom: RequestHandler<{ name: string }, OttApiResponseGetRoom, unknown>
 	res.json(resp);
 };
 
+function isClaimRequest(request: OttApiRequestPatchRoom): request is OttClaimRequest {
+	return "claim" in request;
+}
+
 const patchRoom: RequestHandler<{ name: string }, unknown, OttApiRequestPatchRoom> = async (
 	req,
 	res
 ) => {
+	const body = OttApiRequestPatchRoomSchema.parse(req.body);
+
 	if (!req.token) {
 		throw new OttException("Missing token");
 	}
-	if (req.body.visibility && !VALID_ROOM_VISIBILITY.includes(req.body.visibility)) {
-		throw new BadApiArgumentException(
-			"visibility",
-			`must be one of ${VALID_ROOM_VISIBILITY.toString()}`
-		);
-	}
-	if (req.body.queueMode && !VALID_ROOM_QUEUE_MODE.includes(req.body.queueMode)) {
-		throw new BadApiArgumentException(
-			"queueMode",
-			`must be one of ${VALID_ROOM_QUEUE_MODE.toString()}`
-		);
-	}
-
-	if (req.body.permissions) {
-		req.body.grants = req.body.permissions;
-		delete req.body.permissions;
-	}
-
-	if (req.body.title && req.body.title.length > 255) {
-		throw new BadApiArgumentException(
-			"title",
-			"not allowed (too long, must be at most 255 characters)"
-		);
-	}
-
-	req.body.grants = new Grants(req.body.grants);
 
 	const result = await roommanager.getRoom(req.params.name);
 	if (!result.ok) {
 		throw result.value;
 	}
 	const room = result.value;
-	if (req.body.claim) {
-		if (room.owner) {
-			throw new BadApiArgumentException("claim", `Room already has owner.`);
-		}
 
-		if (req.user) {
-			log.info(`Room ${room.name} claimed by ${req.user.username} (${req.user.id})`);
-			room.owner = req.user;
-			// HACK: force the room to send the updated user info to the client
-			for (const user of room.realusers) {
-				if (user.user_id === room.owner.id) {
-					room.syncUser(room.getUserInfo(user.id));
-					break;
+	if (isClaimRequest(body)) {
+		if (body.claim) {
+			if (room.owner) {
+				throw new BadApiArgumentException("claim", `Room already has owner.`);
+			}
+
+			if (req.user) {
+				log.info(`Room ${room.name} claimed by ${req.user.username} (${req.user.id})`);
+				room.owner = req.user;
+				// HACK: force the room to send the updated user info to the client
+				for (const user of room.realusers) {
+					if (user.user_id === room.owner.id) {
+						room.syncUser(room.getUserInfo(user.id));
+						break;
+					}
 				}
+			} else {
+				res.status(401).json({
+					success: false,
+					error: {
+						message: "Must be logged in to claim room ownership.",
+					},
+				});
+				return;
 			}
-		} else {
-			res.status(401).json({
-				success: false,
-				error: {
-					message: "Must be logged in to claim room ownership.",
-				},
-			});
-			return;
 		}
 	} else {
+		const newBody = {
+			...body,
+			grants: new Grants(body.grants),
+		};
 		const roomRequest: ApplySettingsRequest = {
 			type: RoomRequestType.ApplySettingsRequest,
-			settings: req.body,
+			settings: newBody,
 		};
 
 		await room.processUnauthorizedRequest(roomRequest, { token: req.token });

diff --git a/server/tests/unit/api/room.spec.ts b/server/tests/unit/api/room.spec.ts
@@ -350,13 +350,16 @@ describe("Room API", () => {
 
 		it.each([
 			[
-				{ arg: "title", reason: "not allowed (too long, must be at most 255 characters)" },
 				{
 					title: "abababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababab",
-					isTemporary: true,
 				},
 			],
-		])("should fail to modify room for validation errors: %s", async (error, body) => {
+			[
+				{
+					autoSkipSegmentCategories: ["invalid", "intro"],
+				},
+			],
+		])("should fail to modify room for validation errors: %s", async body => {
 			let resp = await request(app)
 				.patch("/api/room/foo")
 				.auth(token, { type: "bearer" })
@@ -365,15 +368,14 @@ describe("Room API", () => {
 				.expect(400);
 			expect(resp.body.success).toEqual(false);
 			expect(resp.body.error).toMatchObject({
-				name: "BadApiArgumentException",
-				...error,
+				name: "ZodValidationError",
 			});
 		});
 
 		it.each([
 			[Array(100).fill("sponsor"), ["sponsor"]],
 			[
-				["invalidCategory1", "invalidCategory2", "intro", "intro", "outro"],
+				["intro", "intro", "outro"],
 				["intro", "outro"],
 			],
 			[
@@ -398,7 +400,7 @@ describe("Room API", () => {
 			],
 			[[], []],
 		])(
-			"should update autoSkipSegmentCategories with only unique valid auto-skip segment cateogories",
+			"should update autoSkipSegmentCategories with only unique valid auto-skip segment categories",
 			async (requestAutoSkipSegmentCategories, savedAutoSkipSegmentCategories) => {
 				let resp = await request(app)
 					.patch("/api/room/foo")