From 096b35e08e9501939d56371001c2dbaa89c3ec22 Mon Sep 17 00:00:00 2001
From: Nan Yu <nan@getoutline.com>
Date: Mon, 28 Mar 2022 10:18:59 -0700
Subject: [PATCH] chore: change the way that share permissions are checked on
 child documents to use the parentId field of documents rather than the
 collection structure (#3294)

---
 server/models/Collection.test.ts | 50 --------------------------------
 server/models/Collection.ts      | 27 -----------------
 server/routes/api/documents.ts   | 10 ++++---
 3 files changed, 6 insertions(+), 81 deletions(-)

diff --git a/server/models/Collection.test.ts b/server/models/Collection.test.ts
index 48630167f..03999f7ef 100644
--- a/server/models/Collection.test.ts
+++ b/server/models/Collection.test.ts
@@ -85,56 +85,6 @@ describe("getDocumentTree", () => {
   });
 });
 
-describe("isChildDocument", () => {
-  test("should return false with unexpected data", async () => {
-    const document = await buildDocument();
-    const collection = await buildCollection({
-      documentStructure: [document.toJSON()],
-    });
-    expect(collection.isChildDocument(document.id, document.id)).toEqual(false);
-    expect(collection.isChildDocument(document.id, undefined)).toEqual(false);
-    expect(collection.isChildDocument(undefined, document.id)).toEqual(false);
-  });
-
-  test("should return false if sibling", async () => {
-    const one = await buildDocument();
-    const document = await buildDocument();
-    const collection = await buildCollection({
-      documentStructure: [one.toJSON(), document.toJSON()],
-    });
-    expect(collection.isChildDocument(one.id, document.id)).toEqual(false);
-    expect(collection.isChildDocument(document.id, one.id)).toEqual(false);
-  });
-
-  test("should return true if direct child of parent", async () => {
-    const parent = await buildDocument();
-    const document = await buildDocument();
-    const collection = await buildCollection({
-      documentStructure: [
-        { ...parent.toJSON(), children: [document.toJSON()] },
-      ],
-    });
-    expect(collection.isChildDocument(parent.id, document.id)).toEqual(true);
-    expect(collection.isChildDocument(document.id, parent.id)).toEqual(false);
-  });
-
-  test("should return true if nested child of parent", async () => {
-    const parent = await buildDocument();
-    const nested = await buildDocument();
-    const document = await buildDocument();
-    const collection = await buildCollection({
-      documentStructure: [
-        {
-          ...parent.toJSON(),
-          children: [{ ...nested.toJSON(), children: [document.toJSON()] }],
-        },
-      ],
-    });
-    expect(collection.isChildDocument(parent.id, document.id)).toEqual(true);
-    expect(collection.isChildDocument(document.id, parent.id)).toEqual(false);
-  });
-});
-
 describe("#addDocumentToStructure", () => {
   test("should add as last element without index", async () => {
     const { collection } = await seed();
diff --git a/server/models/Collection.ts b/server/models/Collection.ts
index 17acd5d6e..258e1420a 100644
--- a/server/models/Collection.ts
+++ b/server/models/Collection.ts
@@ -506,33 +506,6 @@ class Collection extends ParanoidModel {
     return result;
   };
 
-  isChildDocument = function (
-    parentDocumentId?: string,
-    documentId?: string
-  ): boolean {
-    let result = false;
-
-    const loopChildren = (documents: NavigationNode[], input: string[]) => {
-      if (result) {
-        return;
-      }
-
-      documents.forEach((document) => {
-        const parents = [...input];
-
-        if (document.id === documentId && parentDocumentId) {
-          result = parents.includes(parentDocumentId);
-        } else {
-          parents.push(document.id);
-          loopChildren(document.children, parents);
-        }
-      });
-    };
-
-    loopChildren(this.documentStructure, []);
-    return result;
-  };
-
   /**
    * Update document's title and url in the documentStructure
    */
diff --git a/server/routes/api/documents.ts b/server/routes/api/documents.ts
index abab72050..3f6e019ea 100644
--- a/server/routes/api/documents.ts
+++ b/server/routes/api/documents.ts
@@ -542,10 +542,12 @@ async function loadDocument({
     // shared then includeChildDocuments must be enabled and the document must
     // still be nested within the shared document
     if (share.document.id !== document.id) {
-      if (
-        !share.includeChildDocuments ||
-        !collection.isChildDocument(share.document.id, document.id)
-      ) {
+      if (!share.includeChildDocuments) {
+        throw AuthorizationError();
+      }
+
+      const childDocumentIds = await share.document.getChildDocumentIds();
+      if (!childDocumentIds.includes(document.id)) {
         throw AuthorizationError();
       }
     }