Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use zod for validation on create endpoint #1391

Merged
merged 23 commits into from
Mar 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 3 additions & 6 deletions common/models/rest-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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 {}
Expand Down
20 changes: 20 additions & 0 deletions common/models/zod-schemas.ts
Original file line number Diff line number Diff line change
@@ -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(),
});

Check warning on line 20 in common/models/zod-schemas.ts

View check run for this annotation

Codecov / codecov/patch

common/models/zod-schemas.ts#L2-L20

Added lines #L2 - L20 were not covered by tests
5 changes: 3 additions & 2 deletions common/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,13 @@
"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",
"@types/node": "^18.13.0",
"eslint-plugin-vitest": "0.3.22",
"vitest": "^1.2.2"
}
}
}
79 changes: 21 additions & 58 deletions server/api/room.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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({
Expand Down
4 changes: 3 additions & 1 deletion server/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Victor-M-Giraldo marked this conversation as resolved.
Show resolved Hide resolved
"zod-validation-error": "^3.0.2"
},
"devDependencies": {
"@types/convict": "^6.1.1",
Expand Down
59 changes: 13 additions & 46 deletions server/tests/unit/api/room.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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" })
Expand All @@ -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",
});
});

Expand Down
10 changes: 10 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -15397,6 +15397,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"
Expand Down
Loading