diff --git a/server/api/attachments.js b/server/api/attachments.js index 6ff93e63..a2d34d54 100644 --- a/server/api/attachments.js +++ b/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({ diff --git a/server/api/attachments.test.js b/server/api/attachments.test.js index e77c5927..3a6e47b4 100644 --- a/server/api/attachments.test.js +++ b/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({ diff --git a/server/policies/attachment.js b/server/policies/attachment.js new file mode 100644 index 00000000..d7105402 --- /dev/null +++ b/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; +}); diff --git a/server/policies/index.js b/server/policies/index.js index 7be7f01e..cb5df353 100644 --- a/server/policies/index.js +++ b/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 = {};