Merge branch 'develop' of github.com:outline/outline into feat/mass-import

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, {
-    userId: user.id,
-  });
-  authorize(user, "update", document);
+  if (!attachment) {
+    throw new NotFoundError();
+  }
 
+  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/api/index.js

@@ -31,13 +31,13 @@ const api = new Koa();
 const router = new Router();
 
 // middlewares
+api.use(errorHandling());
 api.use(
   bodyParser({
     multipart: true,
     formidable: { maxFieldsSize: 10 * 1024 * 1024 },
   })
 );
-api.use(errorHandling());
 api.use(methodOverride());
 api.use(validation());
 api.use(apiWrapper());