Enable viewers to upload attachments for documents in collections where they have edit permission (#4468)

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", {

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({