From 09f5462068136dbb05addcde258f7293dd306f2e Mon Sep 17 00:00:00 2001
From: Tom Moor <tom@getoutline.com>
Date: Tue, 22 Nov 2022 19:05:08 -0800
Subject: [PATCH] Enable viewers to upload attachments for documents in
 collections where they have edit permission (#4468)

---
 server/routes/api/attachments.test.ts | 85 ++++++++++++++++++++++++++-
 server/routes/api/attachments.ts      | 17 +++---
 2 files changed, 91 insertions(+), 11 deletions(-)

diff --git a/server/routes/api/attachments.test.ts b/server/routes/api/attachments.test.ts
index 7fb88a15f..75891db5a 100644
--- a/server/routes/api/attachments.test.ts
+++ b/server/routes/api/attachments.test.ts
@@ -1,4 +1,5 @@
-import { AttachmentPreset } from "@shared/types";
+import { AttachmentPreset, CollectionPermission } from "@shared/types";
+import { CollectionUser } from "@server/models";
 import Attachment from "@server/models/Attachment";
 import {
   buildUser,
@@ -53,6 +54,23 @@ describe("#attachments.create", () => {
       expect(attachment!.expiresAt).toBeNull();
     });
 
+    it("should allow attachment creation for documents", async () => {
+      const user = await buildUser();
+      const document = await buildDocument({ teamId: user.teamId });
+
+      const res = await server.post("/api/attachments.create", {
+        body: {
+          name: "test.png",
+          contentType: "image/png",
+          size: 1000,
+          documentId: document.id,
+          preset: AttachmentPreset.DocumentAttachment,
+          token: user.getJwtToken(),
+        },
+      });
+      expect(res.status).toEqual(200);
+    });
+
     it("should create expiring attachment using import preset", async () => {
       const user = await buildUser();
       const res = await server.post("/api/attachments.create", {
@@ -85,6 +103,23 @@ describe("#attachments.create", () => {
       expect(res.status).toEqual(400);
     });
 
+    it("should not allow attachment creation for other documents", async () => {
+      const user = await buildUser();
+      const document = await buildDocument();
+
+      const res = await server.post("/api/attachments.create", {
+        body: {
+          name: "test.png",
+          contentType: "image/png",
+          size: 1000,
+          documentId: document.id,
+          preset: AttachmentPreset.DocumentAttachment,
+          token: user.getJwtToken(),
+        },
+      });
+      expect(res.status).toEqual(403);
+    });
+
     it("should not allow file upload for avatar preset", async () => {
       const user = await buildUser();
       const res = await server.post("/api/attachments.create", {
@@ -115,6 +150,54 @@ describe("#attachments.create", () => {
       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({
+        teamId: user.teamId,
+        permission: null,
+      });
+      const document = await buildDocument({
+        teamId: user.teamId,
+        collectionId: collection.id,
+      });
+
+      await CollectionUser.create({
+        createdById: user.id,
+        collectionId: collection.id,
+        userId: user.id,
+        permission: CollectionPermission.ReadWrite,
+      });
+
+      const res = await server.post("/api/attachments.create", {
+        body: {
+          name: "test.png",
+          contentType: "image/png",
+          size: 1000,
+          documentId: document.id,
+          preset: AttachmentPreset.DocumentAttachment,
+          token: user.getJwtToken(),
+        },
+      });
+      expect(res.status).toEqual(200);
+    });
+
+    it("should not allow attachment creation for documents", async () => {
+      const user = await buildViewer();
+      const document = await buildDocument({ teamId: user.teamId });
+
+      const res = await server.post("/api/attachments.create", {
+        body: {
+          name: "test.png",
+          contentType: "image/png",
+          size: 1000,
+          documentId: document.id,
+          preset: AttachmentPreset.DocumentAttachment,
+          token: user.getJwtToken(),
+        },
+      });
+      expect(res.status).toEqual(403);
+    });
+
     it("should allow upload using avatar preset", async () => {
       const user = await buildViewer();
       const res = await server.post("/api/attachments.create", {
diff --git a/server/routes/api/attachments.ts b/server/routes/api/attachments.ts
index b7aea10b8..37eac2d25 100644
--- a/server/routes/api/attachments.ts
+++ b/server/routes/api/attachments.ts
@@ -40,10 +40,15 @@ router.post(
     assertPresent(name, "name is required");
     assertPresent(size, "size is required");
 
-    // Public attachments are only used for avatars, so this is loosely coupled –
-    // all user types can upload an avatar so no additional authorization is needed.
+    // 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,
+      });
+      authorize(user, "update", document);
     } else {
       authorize(user, "createAttachment", user.team);
     }
@@ -58,14 +63,6 @@ router.post(
       );
     }
 
-    if (documentId !== undefined) {
-      assertUuid(documentId, "documentId must be a uuid");
-      const document = await Document.findByPk(documentId, {
-        userId: user.id,
-      });
-      authorize(user, "update", document);
-    }
-
     const modelId = uuidv4();
     const acl = AttachmentHelper.presetToAcl(preset);
     const key = AttachmentHelper.getKey({