diff --git a/server/commands/documentMover.test.ts b/server/commands/documentMover.test.ts index 9d1a102a2..64af61f6c 100644 --- a/server/commands/documentMover.test.ts +++ b/server/commands/documentMover.test.ts @@ -1,12 +1,9 @@ -import Attachment from "@server/models/Attachment"; import { buildDocument, - buildAttachment, buildCollection, buildUser, } from "@server/test/factories"; import { flushdb, seed } from "@server/test/support"; -import parseAttachmentIds from "@server/utils/parseAttachmentIds"; import documentMover from "./documentMover"; beforeEach(() => flushdb()); @@ -110,45 +107,4 @@ describe("documentMover", () => { expect(response.documents[0].collection.id).toEqual(newCollection.id); expect(response.documents[1].collection.id).toEqual(newCollection.id); }); - - it("should move attachments in children to another collection", async () => { - const { document, user, collection } = await seed(); - const newCollection = await buildCollection({ - teamId: collection.teamId, - }); - const attachment = await buildAttachment({ - teamId: user.teamId, - userId: user.id, - }); - const newDocument = await buildDocument({ - parentDocumentId: document.id, - collectionId: collection.id, - teamId: collection.teamId, - userId: collection.createdById, - title: "Child document", - text: `content ![attachment](${attachment.redirectUrl})`, - }); - - await collection.addDocumentToStructure(newDocument); - await documentMover({ - user, - document, - collectionId: newCollection.id, - parentDocumentId: undefined, - index: 0, - ip, - }); - - // check document ids where updated - await newDocument.reload(); - expect(newDocument.collectionId).toBe(newCollection.id); - - // check new attachment was created pointint to same key - const attachmentIds = parseAttachmentIds(newDocument.text); - const newAttachment = await Attachment.findByPk(attachmentIds[0]); - expect(newAttachment?.documentId).toBe(newDocument.id); - expect(newAttachment?.key).toBe(attachment.key); - await document.reload(); - expect(document.collectionId).toBe(newCollection.id); - }); }); diff --git a/server/commands/documentMover.ts b/server/commands/documentMover.ts index 1c4ba0f9a..bd2ca461b 100644 --- a/server/commands/documentMover.ts +++ b/server/commands/documentMover.ts @@ -2,52 +2,9 @@ import invariant from "invariant"; import { Transaction } from "sequelize"; import { sequelize } from "@server/database/sequelize"; import { APM } from "@server/logging/tracing"; -import { - User, - Document, - Attachment, - Collection, - Pin, - Event, -} from "@server/models"; -import parseAttachmentIds from "@server/utils/parseAttachmentIds"; +import { User, Document, Collection, Pin, Event } from "@server/models"; import pinDestroyer from "./pinDestroyer"; -async function copyAttachments( - document: Document, - options?: { transaction?: Transaction } -) { - let text = document.text; - const documentId = document.id; - // find any image attachments that are in this documents text - const attachmentIds = parseAttachmentIds(text); - - for (const id of attachmentIds) { - const existing = await Attachment.findOne({ - where: { - teamId: document.teamId, - id, - }, - }); - - // if the image attachment was originally uploaded to another document - // (this can happen in various ways, copy/paste, or duplicate for example) - // then create a new attachment pointed to this doc and update the reference - // in the text so that it gets the moved documents permissions - if (existing && existing.documentId !== documentId) { - // @ts-expect-error dataValues exists - const { id, ...rest } = existing.dataValues; - const attachment = await Attachment.create( - { ...rest, documentId }, - options - ); - text = text.replace(existing.redirectUrl, attachment.redirectUrl); - } - } - - return text; -} - type Props = { user: User; document: Document; @@ -127,9 +84,6 @@ async function documentMover({ await collection?.save({ transaction, }); - document.text = await copyAttachments(document, { - transaction, - }); } // add to new collection (may be the same) @@ -171,9 +125,6 @@ async function documentMover({ await Promise.all( childDocuments.map(async (child) => { await loopChildren(child.id); - child.text = await copyAttachments(child, { - transaction, - }); child.collectionId = collectionId; await child.save(); diff --git a/server/routes/api/attachments.ts b/server/routes/api/attachments.ts index 63090ef85..f33cef994 100644 --- a/server/routes/api/attachments.ts +++ b/server/routes/api/attachments.ts @@ -1,7 +1,11 @@ import Router from "koa-router"; import { v4 as uuidv4 } from "uuid"; import { bytesToHumanReadable } from "@shared/utils/files"; -import { NotFoundError, ValidationError } from "@server/errors"; +import { + AuthorizationError, + NotFoundError, + ValidationError, +} from "@server/errors"; import auth from "@server/middlewares/authentication"; import { Attachment, Document, Event } from "@server/models"; import { authorize } from "@server/policies"; @@ -141,12 +145,8 @@ router.post("attachments.redirect", auth(), async (ctx) => { } if (attachment.isPrivate) { - if (attachment.documentId) { - const document = await Document.findByPk(attachment.documentId, { - userId: user.id, - paranoid: false, - }); - authorize(user, "read", document); + if (attachment.teamId !== user.teamId) { + throw AuthorizationError(); } const accessUrl = await getSignedUrl(attachment.key);