From 0bec781695563212cb86b79dae8e7801315c9e62 Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Mon, 16 Oct 2023 21:04:13 -0400 Subject: [PATCH] fix: Improve validation of edge cases with documents.move endpoint. closes #6015 --- server/models/Document.ts | 28 +++++++++++++++ server/routes/api/documents/documents.test.ts | 35 +++++++++++++++++-- 2 files changed, 61 insertions(+), 2 deletions(-) diff --git a/server/models/Document.ts b/server/models/Document.ts index 0f1302279..3576bcffc 100644 --- a/server/models/Document.ts +++ b/server/models/Document.ts @@ -38,6 +38,7 @@ import getTasks from "@shared/utils/getTasks"; import slugify from "@shared/utils/slugify"; import { SLUG_URL_REGEX } from "@shared/utils/urlHelpers"; import { DocumentValidation } from "@shared/validations"; +import { ValidationError } from "@server/errors"; import Backlink from "./Backlink"; import Collection from "./Collection"; import FileOperation from "./FileOperation"; @@ -354,6 +355,7 @@ class Document extends ParanoidModel { model.collaboratorIds = []; } + // ensure the last modifying user is a collaborator model.collaboratorIds = uniq( model.collaboratorIds.concat(model.lastModifiedById) ); @@ -362,6 +364,32 @@ class Document extends ParanoidModel { model.revisionCount += 1; } + @BeforeUpdate + static async checkParentDocument(model: Document, options: SaveOptions) { + if ( + model.previous("parentDocumentId") === model.parentDocumentId || + !model.parentDocumentId + ) { + return; + } + + if (model.parentDocumentId === model.id) { + throw ValidationError( + "infinite loop detected, cannot nest a document inside itself" + ); + } + + const childDocumentIds = await model.findAllChildDocumentIds( + undefined, + options + ); + if (childDocumentIds.includes(model.parentDocumentId)) { + throw ValidationError( + "infinite loop detected, cannot nest a document inside itself" + ); + } + } + // associations @BelongsTo(() => FileOperation, "importId") diff --git a/server/routes/api/documents/documents.test.ts b/server/routes/api/documents/documents.test.ts index 6a5c14e2d..4bf9f8c0c 100644 --- a/server/routes/api/documents/documents.test.ts +++ b/server/routes/api/documents/documents.test.ts @@ -2004,11 +2004,10 @@ describe("#documents.move", () => { userId: user.id, teamId: user.teamId, }); - const collection = await buildCollection(); const res = await server.post("/api/documents.move", { body: { id: document.id, - collectionId: collection.id, + collectionId: document.collectionId, parentDocumentId: document.id, token: user.getJwtToken(), }, @@ -2020,6 +2019,38 @@ describe("#documents.move", () => { ); }); + it("should fail if attempting to nest doc within a child of itself", async () => { + const user = await buildUser(); + const collection = await buildCollection({ + userId: user.id, + teamId: user.teamId, + }); + const document = await buildDocument({ + userId: user.id, + teamId: user.teamId, + collectionId: collection.id, + }); + const childDocument = await buildDocument({ + userId: user.id, + teamId: user.teamId, + collectionId: document.collectionId, + parentDocumentId: document.id, + }); + const res = await server.post("/api/documents.move", { + body: { + id: document.id, + collectionId: document.collectionId, + parentDocumentId: childDocument.id, + token: user.getJwtToken(), + }, + }); + const body = await res.json(); + expect(res.status).toEqual(400); + expect(body.message).toEqual( + "infinite loop detected, cannot nest a document inside itself" + ); + }); + it("should fail if attempting to nest doc within a draft", async () => { const user = await buildUser(); const collection = await buildCollection({