refactor: add server side validation schema for attachments (#4606)

* refactor: schema for attachments.create

* refactor: schema for attachments.delete

* refactor: remove deprecated "public" request param

server/routes/api/attachments/attachments.test.ts

@@ -22,20 +22,6 @@ describe("#attachments.create", () => {
   });
 
   describe("member", () => {
-    it("should allow simple image upload for public attachments", async () => {
-      const user = await buildUser();
-      const res = await server.post("/api/attachments.create", {
-        body: {
-          name: "test.png",
-          contentType: "image/png",
-          size: 1000,
-          public: true,
-          token: user.getJwtToken(),
-        },
-      });
-      expect(res.status).toEqual(200);
-    });
-
     it("should allow upload using avatar preset", async () => {
       const user = await buildUser();
       const res = await server.post("/api/attachments.create", {
@@ -89,20 +75,6 @@ describe("#attachments.create", () => {
       expect(attachment!.expiresAt).toBeTruthy();
     });
 
-    it("should not allow file upload for public attachments", async () => {
-      const user = await buildUser();
-      const res = await server.post("/api/attachments.create", {
-        body: {
-          name: "test.pdf",
-          contentType: "application/pdf",
-          size: 1000,
-          public: true,
-          token: user.getJwtToken(),
-        },
-      });
-      expect(res.status).toEqual(400);
-    });
-
     it("should not allow attachment creation for other documents", async () => {
       const user = await buildUser();
       const document = await buildDocument();
@@ -136,20 +108,6 @@ describe("#attachments.create", () => {
   });
 
   describe("viewer", () => {
-    it("should allow simple image upload for public attachments", async () => {
-      const user = await buildViewer();
-      const res = await server.post("/api/attachments.create", {
-        body: {
-          name: "test.png",
-          contentType: "image/png",
-          size: 1000,
-          public: true,
-          token: user.getJwtToken(),
-        },
-      });
-      expect(res.status).toEqual(200);
-    });
-
     it("should allow attachment creation for documents in collections with edit access", async () => {
       const user = await buildViewer();
       const collection = await buildCollection({

server/routes/api/attachments/attachments.ts

@@ -5,46 +5,32 @@ import { bytesToHumanReadable } from "@shared/utils/files";
 import { AttachmentValidation } from "@shared/validations";
 import { AuthorizationError, ValidationError } from "@server/errors";
 import auth from "@server/middlewares/authentication";
-import {
-  transaction,
-  TransactionContext,
-} from "@server/middlewares/transaction";
+import { transaction } from "@server/middlewares/transaction";
+import validate from "@server/middlewares/validate";
 import { Attachment, Document, Event } from "@server/models";
 import AttachmentHelper from "@server/models/helpers/AttachmentHelper";
 import { authorize } from "@server/policies";
 import { presentAttachment } from "@server/presenters";
-import { ContextWithState } from "@server/types";
+import { APIContext, ContextWithState } from "@server/types";
 import { getPresignedPost, publicS3Endpoint } from "@server/utils/s3";
-import { assertIn, assertPresent, assertUuid } from "@server/validation";
+import { assertIn, assertUuid } from "@server/validation";
+import * as T from "./schema";
 
 const router = new Router();
 
 router.post(
   "attachments.create",
   auth(),
+  validate(T.AttachmentsCreateSchema),
   transaction(),
-  async (ctx: TransactionContext) => {
-    const {
-      name,
-      documentId,
-      contentType = "application/octet-stream",
-      size,
-      // 'public' is now deprecated and can be removed on December 1 2022.
-      public: isPublicDeprecated,
-      preset = isPublicDeprecated
-        ? AttachmentPreset.Avatar
-        : AttachmentPreset.DocumentAttachment,
-    } = ctx.request.body;
+  async (ctx: APIContext<T.AttachmentCreateReq>) => {
+    const { name, documentId, contentType, size, preset } = ctx.input;
     const { user, transaction } = ctx.state;
 
-    assertPresent(name, "name is required");
-    assertPresent(size, "size is required");
-
     // All user types can upload an avatar so no additional authorization is needed.
     if (preset === AttachmentPreset.Avatar) {
       assertIn(contentType, AttachmentValidation.avatarContentTypes);
     } else if (preset === AttachmentPreset.DocumentAttachment && documentId) {
-      assertUuid(documentId, "documentId must be a uuid");
       const document = await Document.findByPk(documentId, {
         userId: user.id,
       });
@@ -121,34 +107,38 @@ router.post(
   }
 );
 
-router.post("attachments.delete", auth(), async (ctx) => {
-  const { id } = ctx.request.body;
-  assertUuid(id, "id is required");
-  const { user } = ctx.state;
-  const attachment = await Attachment.findByPk(id, {
-    rejectOnEmpty: true,
-  });
-
-  if (attachment.documentId) {
-    const document = await Document.findByPk(attachment.documentId, {
-      userId: user.id,
+router.post(
+  "attachments.delete",
+  auth(),
+  validate(T.AttachmentDeleteSchema),
+  async (ctx: APIContext<T.AttachmentDeleteReq>) => {
+    const { id } = ctx.input;
+    const { user } = ctx.state;
+    const attachment = await Attachment.findByPk(id, {
+      rejectOnEmpty: true,
     });
-    authorize(user, "update", document);
+
+    if (attachment.documentId) {
+      const document = await Document.findByPk(attachment.documentId, {
+        userId: user.id,
+      });
+      authorize(user, "update", document);
+    }
+
+    authorize(user, "delete", attachment);
+    await attachment.destroy();
+    await Event.create({
+      name: "attachments.delete",
+      teamId: user.teamId,
+      actorId: user.id,
+      ip: ctx.request.ip,
+    });
+
+    ctx.body = {
+      success: true,
+    };
   }
-
-  authorize(user, "delete", attachment);
-  await attachment.destroy();
-  await Event.create({
-    name: "attachments.delete",
-    teamId: user.teamId,
-    actorId: user.id,
-    ip: ctx.request.ip,
-  });
-
-  ctx.body = {
-    success: true,
-  };
-});
+);
 
 const handleAttachmentsRedirect = async (ctx: ContextWithState) => {
   const id = ctx.request.body?.id ?? ctx.request.query?.id;

server/routes/api/attachments/index.ts

@@ -0,0 +1 @@
+export { default } from "./attachments";

server/routes/api/attachments/schema.ts

@@ -0,0 +1,28 @@
+import { z } from "zod";
+import { AttachmentPreset } from "@shared/types";
+
+export const AttachmentsCreateSchema = z.object({
+  /** Attachment name */
+  name: z.string(),
+
+  /** Id of the document to which the Attachment belongs */
+  documentId: z.string().uuid().optional(),
+
+  /** File size of the Attachment */
+  size: z.number(),
+
+  /** Content-Type of the Attachment */
+  contentType: z.string().optional().default("application/octet-stream"),
+
+  /** Attachment type */
+  preset: z.nativeEnum(AttachmentPreset),
+});
+
+export type AttachmentCreateReq = z.infer<typeof AttachmentsCreateSchema>;
+
+export const AttachmentDeleteSchema = z.object({
+  /** Id of the attachment to be deleted */
+  id: z.string().uuid(),
+});
+
+export type AttachmentDeleteReq = z.infer<typeof AttachmentDeleteSchema>;