fix: Allow deleting attachments not linked to documents when owned by user

closes #1729

server/api/attachments.js

@@ -98,11 +98,18 @@ router.post("attachments.delete", auth(), async (ctx) => {
 
   const user = ctx.state.user;
   const attachment = await Attachment.findByPk(id);
-  const document = await Document.findByPk(attachment.documentId, {
+  if (!attachment) {
-    userId: user.id,
+    throw new NotFoundError();
-  });
+  }
-  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({

server/api/attachments.test.js

@@ -43,6 +43,71 @@ describe("#attachments.delete", () => {
     expect(await Attachment.count()).toEqual(0);
   });
 
+  it("should allow deleting an attachment without a document created by user", async () => {
+    const user = await buildUser();
+    const attachment = await buildAttachment({
+      teamId: user.teamId,
+      userId: user.id,
+    });
+
+    attachment.documentId = null;
+    await attachment.save();
+
+    const res = await server.post("/api/attachments.delete", {
+      body: { token: user.getJwtToken(), id: attachment.id },
+    });
+
+    expect(res.status).toEqual(200);
+    expect(await Attachment.count()).toEqual(0);
+  });
+
+  it("should allow deleting an attachment without a document if admin", async () => {
+    const user = await buildUser({ isAdmin: true });
+    const attachment = await buildAttachment({
+      teamId: user.teamId,
+    });
+
+    attachment.documentId = null;
+    await attachment.save();
+
+    const res = await server.post("/api/attachments.delete", {
+      body: { token: user.getJwtToken(), id: attachment.id },
+    });
+
+    expect(res.status).toEqual(200);
+    expect(await Attachment.count()).toEqual(0);
+  });
+
+  it("should not allow deleting an attachment in another team", async () => {
+    const user = await buildUser({ isAdmin: true });
+    const attachment = await buildAttachment();
+
+    attachment.documentId = null;
+    await attachment.save();
+
+    const res = await server.post("/api/attachments.delete", {
+      body: { token: user.getJwtToken(), id: attachment.id },
+    });
+
+    expect(res.status).toEqual(403);
+  });
+
+  it("should not allow deleting an attachment without a document", async () => {
+    const user = await buildUser();
+    const attachment = await buildAttachment({
+      teamId: user.teamId,
+    });
+
+    attachment.documentId = null;
+    await attachment.save();
+
+    const res = await server.post("/api/attachments.delete", {
+      body: { token: user.getJwtToken(), id: attachment.id },
+    });
+
+    expect(res.status).toEqual(403);
+  });
+
   it("should not allow deleting an attachment belonging to a document user does not have access to", async () => {
     const user = await buildUser();
     const collection = await buildCollection({

server/policies/attachment.js

@@ -0,0 +1,14 @@
+// @flow
+import { Attachment, User } from "../models";
+import policy from "./policy";
+
+const { allow } = policy;
+
+allow(User, "create", Attachment);
+
+allow(User, "delete", Attachment, (actor, attachment) => {
+  if (!attachment || attachment.teamId !== actor.teamId) return false;
+  if (actor.isAdmin) return true;
+  if (actor.id === attachment.userId) return true;
+  return false;
+});

server/policies/index.js

@@ -1,7 +1,8 @@
 // @flow
-import { Team, User, Collection, Document, Group } from "../models";
+import { Attachment, Team, User, Collection, Document, Group } from "../models";
 import policy from "./policy";
 import "./apiKey";
+import "./attachment";
 import "./collection";
 import "./document";
 import "./integration";
@@ -24,7 +25,7 @@ type Policy = {
  */
 export function serialize(
   model: User,
-  target: Team | Collection | Document | Group
+  target: Attachment | Team | Collection | Document | Group
 ): Policy {
   let output = {};