From 22ba4d0f4824b60cd98c37f538e7e36306702a46 Mon Sep 17 00:00:00 2001
From: Saumya Pandey <sp160899@gmail.com>
Date: Thu, 26 Aug 2021 09:35:59 +0530
Subject: [PATCH] fix: prevent access to docs in trash from deleted private
 collections (#2431)

* Check for collection in deleted document

* Add tests

* Use update policy

* Set paranoid to false when fetching deleted doc

* Update policy
---
 server/api/documents.js      |  1 +
 server/api/documents.test.js | 68 ++++++++++++++++++++++++++++++++++++
 server/models/Document.js    |  5 +--
 server/policies/document.js  |  5 +++
 4 files changed, 77 insertions(+), 2 deletions(-)

diff --git a/server/api/documents.js b/server/api/documents.js
index 4a4906a9..bda3fe6c 100644
--- a/server/api/documents.js
+++ b/server/api/documents.js
@@ -585,6 +585,7 @@ async function loadDocument({
     }
 
     if (document.deletedAt) {
+      // don't send data if user cannot restore deleted doc
       authorize(user, "restore", document);
     } else {
       authorize(user, "read", document);
diff --git a/server/api/documents.test.js b/server/api/documents.test.js
index c10c4b7c..4e2d098b 100644
--- a/server/api/documents.test.js
+++ b/server/api/documents.test.js
@@ -98,6 +98,74 @@ describe("#documents.info", () => {
     expect(share.lastAccessedAt).toBeTruthy();
   });
 
+  it("should not return document of a deleted collection, when the user was absent in the collection", async () => {
+    const user = await buildUser();
+    const user2 = await buildUser({
+      teamId: user.teamId,
+    });
+    const collection = await buildCollection({
+      permission: null,
+      teamId: user.teamId,
+      createdById: user.id,
+    });
+
+    const doc = await buildDocument({
+      collectionId: collection.id,
+      teamId: user.teamId,
+      userId: user.id,
+    });
+
+    await server.post("/api/collections.delete", {
+      body: {
+        id: collection.id,
+        token: user.getJwtToken(),
+      },
+    });
+
+    const res = await server.post("/api/documents.info", {
+      body: {
+        id: doc.id,
+        token: user2.getJwtToken(),
+      },
+    });
+
+    expect(res.status).toEqual(403);
+  });
+
+  it("should return document of a deleted collection, when the user was present in the collection", async () => {
+    const user = await buildUser();
+    const collection = await buildCollection({
+      permission: null,
+      teamId: user.teamId,
+      createdById: user.id,
+    });
+
+    const doc = await buildDocument({
+      collectionId: collection.id,
+      teamId: user.teamId,
+      userId: user.id,
+    });
+
+    await server.post("/api/collections.delete", {
+      body: {
+        id: collection.id,
+        token: user.getJwtToken(),
+      },
+    });
+
+    const res = await server.post("/api/documents.info", {
+      body: {
+        id: doc.id,
+        token: user.getJwtToken(),
+      },
+    });
+
+    const body = await res.json();
+
+    expect(res.status).toEqual(200);
+    expect(body.data.id).toEqual(doc.id);
+  });
+
   describe("apiVersion=2", () => {
     it("should return sharedTree from shareId", async () => {
       const { document, collection, user } = await seed();
diff --git a/server/models/Document.js b/server/models/Document.js
index 52fb1c52..52b9f9f3 100644
--- a/server/models/Document.js
+++ b/server/models/Document.js
@@ -163,7 +163,7 @@ Document.associate = (models) => {
       },
     },
   });
-  Document.addScope("withCollection", (userId) => {
+  Document.addScope("withCollection", (userId, paranoid = true) => {
     if (userId) {
       return {
         include: [
@@ -172,6 +172,7 @@ Document.associate = (models) => {
               method: ["withMembership", userId],
             }),
             as: "collection",
+            paranoid,
           },
         ],
       };
@@ -221,7 +222,7 @@ Document.findByPk = async function (id, options = {}) {
   const scope = this.scope(
     "withUnpublished",
     {
-      method: ["withCollection", options.userId],
+      method: ["withCollection", options.userId, options.paranoid],
     },
     {
       method: ["withViews", options.userId],
diff --git a/server/policies/document.js b/server/policies/document.js
index 519fdbb3..535fc013 100644
--- a/server/policies/document.js
+++ b/server/policies/document.js
@@ -135,6 +135,11 @@ allow(User, "permanentDelete", Document, (user, document) => {
 allow(User, "restore", Document, (user, document) => {
   if (user.isViewer) return false;
   if (!document.deletedAt) return false;
+
+  if (document.collection && cannot(user, "update", document.collection)) {
+    return false;
+  }
+
   return user.teamId === document.teamId;
 });