From 3c2e7b5b63961861cf6ff6132b2113e3b989f943 Mon Sep 17 00:00:00 2001 From: Apoorv Mishra Date: Mon, 3 Jul 2023 08:43:45 +0530 Subject: [PATCH] Request validation for `/api/subscriptions.*` (#5476) * chore: req validation for subscriptions.list * chore: req validation for subscriptions.info * chore: req validation for subscriptions.create * chore: req validation for subscriptions.delete * fix: reuse validations --- server/routes/api/subscriptions/index.ts | 1 + server/routes/api/subscriptions/schema.ts | 44 ++++++++++ .../{ => subscriptions}/subscriptions.test.ts | 8 +- .../api/{ => subscriptions}/subscriptions.ts | 87 ++++++++----------- 4 files changed, 84 insertions(+), 56 deletions(-) create mode 100644 server/routes/api/subscriptions/index.ts create mode 100644 server/routes/api/subscriptions/schema.ts rename server/routes/api/{ => subscriptions}/subscriptions.test.ts (98%) rename server/routes/api/{ => subscriptions}/subscriptions.ts (60%) diff --git a/server/routes/api/subscriptions/index.ts b/server/routes/api/subscriptions/index.ts new file mode 100644 index 000000000..bcee84183 --- /dev/null +++ b/server/routes/api/subscriptions/index.ts @@ -0,0 +1 @@ +export { default } from "./subscriptions"; diff --git a/server/routes/api/subscriptions/schema.ts b/server/routes/api/subscriptions/schema.ts new file mode 100644 index 000000000..204e42427 --- /dev/null +++ b/server/routes/api/subscriptions/schema.ts @@ -0,0 +1,44 @@ +import { z } from "zod"; +import { ValidateDocumentId } from "@server/validation"; +import BaseSchema from "../BaseSchema"; + +export const SubscriptionsListSchema = BaseSchema.extend({ + body: z.object({ + documentId: z.string().refine(ValidateDocumentId.isValid, { + message: ValidateDocumentId.message, + }), + event: z.literal("documents.update"), + }), +}); + +export type SubscriptionsListReq = z.infer; + +export const SubscriptionsInfoSchema = BaseSchema.extend({ + body: z.object({ + documentId: z.string().refine(ValidateDocumentId.isValid, { + message: ValidateDocumentId.message, + }), + event: z.literal("documents.update"), + }), +}); + +export type SubscriptionsInfoReq = z.infer; + +export const SubscriptionsCreateSchema = BaseSchema.extend({ + body: z.object({ + documentId: z.string().refine(ValidateDocumentId.isValid, { + message: ValidateDocumentId.message, + }), + event: z.literal("documents.update"), + }), +}); + +export type SubscriptionsCreateReq = z.infer; + +export const SubscriptionsDeleteSchema = BaseSchema.extend({ + body: z.object({ + id: z.string().uuid(), + }), +}); + +export type SubscriptionsDeleteReq = z.infer; diff --git a/server/routes/api/subscriptions.test.ts b/server/routes/api/subscriptions/subscriptions.test.ts similarity index 98% rename from server/routes/api/subscriptions.test.ts rename to server/routes/api/subscriptions/subscriptions.test.ts index c4bfb42b4..68d76eeff 100644 --- a/server/routes/api/subscriptions.test.ts +++ b/server/routes/api/subscriptions/subscriptions.test.ts @@ -140,7 +140,7 @@ describe("#subscriptions.create", () => { expect(body.ok).toEqual(false); expect(body.error).toEqual("validation_error"); expect(body.message).toEqual( - "Not a valid subscription event for documents" + `event: Invalid literal value, expected "documents.update"` ); }); }); @@ -323,7 +323,7 @@ describe("#subscriptions.info", () => { expect(response0.ok).toEqual(false); expect(response0.error).toEqual("validation_error"); expect(response0.message).toEqual( - "Not a valid subscription event for documents" + `event: Invalid literal value, expected "documents.update"` ); // `viewer` wants info about `subscriber`'s @@ -343,7 +343,7 @@ describe("#subscriptions.info", () => { expect(response1.ok).toEqual(false); expect(response1.error).toEqual("validation_error"); expect(response1.message).toEqual( - "Not a valid subscription event for documents" + `event: Invalid literal value, expected "documents.update"` ); }); }); @@ -515,7 +515,7 @@ describe("#subscriptions.list", () => { expect(body.ok).toEqual(false); expect(body.error).toEqual("validation_error"); expect(body.message).toEqual( - "Not a valid subscription event for documents" + `event: Invalid literal value, expected "documents.update"` ); }); diff --git a/server/routes/api/subscriptions.ts b/server/routes/api/subscriptions/subscriptions.ts similarity index 60% rename from server/routes/api/subscriptions.ts rename to server/routes/api/subscriptions/subscriptions.ts index d8bfc00f8..50da69e57 100644 --- a/server/routes/api/subscriptions.ts +++ b/server/routes/api/subscriptions/subscriptions.ts @@ -3,12 +3,13 @@ import subscriptionCreator from "@server/commands/subscriptionCreator"; import subscriptionDestroyer from "@server/commands/subscriptionDestroyer"; import auth from "@server/middlewares/authentication"; import { transaction } from "@server/middlewares/transaction"; +import validate from "@server/middlewares/validate"; import { Subscription, Document } from "@server/models"; import { authorize } from "@server/policies"; import { presentSubscription } from "@server/presenters"; import { APIContext } from "@server/types"; -import { assertIn, assertUuid } from "@server/validation"; -import pagination from "./middlewares/pagination"; +import pagination from "../middlewares/pagination"; +import * as T from "./schema"; const router = new Router(); @@ -16,17 +17,10 @@ router.post( "subscriptions.list", auth(), pagination(), - async (ctx: APIContext) => { + validate(T.SubscriptionsListSchema), + async (ctx: APIContext) => { const { user } = ctx.state.auth; - const { documentId, event } = ctx.request.body; - - assertUuid(documentId, "documentId is required"); - - assertIn( - event, - ["documents.update"], - `Not a valid subscription event for documents` - ); + const { documentId, event } = ctx.input.body; const document = await Document.findByPk(documentId, { userId: user.id }); @@ -50,53 +44,43 @@ router.post( } ); -router.post("subscriptions.info", auth(), async (ctx: APIContext) => { - const { user } = ctx.state.auth; - const { documentId, event } = ctx.request.body; +router.post( + "subscriptions.info", + auth(), + validate(T.SubscriptionsInfoSchema), + async (ctx: APIContext) => { + const { user } = ctx.state.auth; + const { documentId, event } = ctx.input.body; - assertUuid(documentId, "documentId is required"); + const document = await Document.findByPk(documentId, { userId: user.id }); - assertIn( - event, - ["documents.update"], - "Not a valid subscription event for documents" - ); + authorize(user, "read", document); - const document = await Document.findByPk(documentId, { userId: user.id }); + // There can be only one subscription with these props. + const subscription = await Subscription.findOne({ + where: { + userId: user.id, + documentId: document.id, + event, + }, + rejectOnEmpty: true, + }); - authorize(user, "read", document); - - // There can be only one subscription with these props. - const subscription = await Subscription.findOne({ - where: { - userId: user.id, - documentId: document.id, - event, - }, - rejectOnEmpty: true, - }); - - ctx.body = { - data: presentSubscription(subscription), - }; -}); + ctx.body = { + data: presentSubscription(subscription), + }; + } +); router.post( "subscriptions.create", auth(), + validate(T.SubscriptionsCreateSchema), transaction(), - async (ctx: APIContext) => { + async (ctx: APIContext) => { const { auth, transaction } = ctx.state; const { user } = auth; - const { documentId, event } = ctx.request.body; - - assertUuid(documentId, "documentId is required"); - - assertIn( - event, - ["documents.update"], - "Not a valid subscription event for documents" - ); + const { documentId, event } = ctx.input.body; const document = await Document.findByPk(documentId, { userId: user.id, @@ -122,13 +106,12 @@ router.post( router.post( "subscriptions.delete", auth(), + validate(T.SubscriptionsDeleteSchema), transaction(), - async (ctx: APIContext) => { + async (ctx: APIContext) => { const { auth, transaction } = ctx.state; const { user } = auth; - const { id } = ctx.request.body; - - assertUuid(id, "id is required"); + const { id } = ctx.input.body; const subscription = await Subscription.findByPk(id, { rejectOnEmpty: true,