refactor

plugins/search-postgres/server/PostgresSearchProvider.ts

@@ -664,7 +664,14 @@ export default class PostgresSearchProvider extends BaseSearchProvider {
     }
     if (collectionIds.length) {
       where[Op.or].push({
-        [Op.and]: [{ collectionId: collectionIds }, { isPrivate: false }],
+        [Op.and]: [
+          { collectionId: collectionIds },
+          // Exclude restricted documents the user cannot access. Team-level
+          // searches fail closed and never include restricted documents.
+          model instanceof User
+            ? Document.restrictionsWhere(model)
+            : { isPrivate: false },
+        ],
       });
     }
 

server/models/Collection.ts

@@ -1125,74 +1125,49 @@ class Collection extends ParanoidModel<
   });
 
   /**
-   * Filter restricted (private) nodes from a navigation tree for a given user.
-   * Prunes entire subtrees rooted at private nodes the user cannot access.
+   * Filter restricted (private) documents from a navigation tree for a given
+   * user. Prunes entire subtrees rooted at restricted documents the user
+   * cannot access. Restriction state is read from the database rather than
+   * the cached tree so stale structure data cannot expose documents. Admins
+   * can access all restricted documents, so the tree is returned unfiltered.
    *
    * @param nodes - the navigation tree to filter.
-   * @param userId - the user requesting the tree.
+   * @param user - the user requesting the tree.
+   * @param collectionId - the collection the nodes belong to.
    * @returns filtered navigation tree.
    */
   static async filterRestrictedNodes(
     nodes: NavigationNode[],
-    userId: string
+    user: User,
+    collectionId: string
   ): Promise<NavigationNode[]> {
-    const restrictedIds: string[] = [];
-    const collectRestricted = (items: NavigationNode[]) => {
-      for (const node of items) {
-        if (node.isPrivate) {
-          restrictedIds.push(node.id);
-        }
-        collectRestricted(node.children);
-      }
-    };
-    collectRestricted(nodes);
-
-    if (restrictedIds.length === 0) {
+    if (user.isAdmin) {
       return nodes;
     }
 
-    const [userMemberships, groupMemberships] = await Promise.all([
-      UserMembership.findAll({
-        attributes: ["documentId"],
-        where: {
-          userId,
-          documentId: { [Op.in]: restrictedIds },
-        },
-      }),
-      GroupMembership.findAll({
-        attributes: ["documentId"],
-        where: {
-          documentId: { [Op.in]: restrictedIds },
-        },
-        include: [
-          {
-            model: Group,
-            required: true,
-            include: [
-              {
-                model: GroupUser,
-                required: true,
-                where: { userId },
-              },
-            ],
+    // A single query resolves membership access inside the database and
+    // returns only the IDs that must be pruned from the tree.
+    const inaccessibleIds = new Set(
+      (
+        await Document.unscoped().findAll({
+          attributes: ["id"],
+          where: {
+            collectionId,
+            isPrivate: true,
+            id: { [Op.notIn]: Document.restrictedDocumentIdsQuery(user) },
           },
-        ],
-      }),
-    ]);
+        })
+      ).map((document) => document.id)
+    );
 
-    const accessibleRestrictedIds = new Set([
-      ...userMemberships
-        .map((m) => m.documentId)
-        .filter((id): id is string => id !== null),
-      ...groupMemberships
-        .map((m) => m.documentId)
-        .filter((id): id is string => id !== null),
-    ]);
+    if (inaccessibleIds.size === 0) {
+      return nodes;
+    }
 
     const filterNodes = (items: NavigationNode[]): NavigationNode[] => {
       const result: NavigationNode[] = [];
       for (const node of items) {
-        if (node.isPrivate && !accessibleRestrictedIds.has(node.id)) {
+        if (inaccessibleIds.has(node.id)) {
           continue;
         }
         result.push({

server/models/Document.ts

@@ -947,8 +947,8 @@ class Document extends ArchivableModel<
 
       // Fail closed if isPrivate cannot be determined — callers that restrict
       // attributes must explicitly include isPrivate to opt into collection
-      // inheritance.
-      if (doc.isPrivate !== false) {
+      // inheritance. Admins can access all restricted documents.
+      if (doc.isPrivate !== false && !user?.isAdmin) {
         return false;
       }
 
@@ -960,6 +960,58 @@ class Document extends ArchivableModel<
     });
   }
 
+  /**
+   * Returns a SQL subquery selecting the IDs of restricted documents that the
+   * given user can access through a direct or group membership. Use with
+   * `Op.in` so the IDs are resolved inside the database rather than
+   * materialized into a potentially large list.
+   *
+   * @param user The user to check memberships for.
+   * @returns A literal subquery for use in a where clause.
+   */
+  static restrictedDocumentIdsQuery(user: User) {
+    const userId = this.sequelize!.escape(user.id);
+    return Sequelize.literal(`(
+      SELECT user_permissions."documentId"
+      FROM user_permissions
+      JOIN documents ON documents.id = user_permissions."documentId"
+      WHERE user_permissions."userId" = ${userId}
+        AND documents."isPrivate" = true
+      UNION
+      SELECT group_permissions."documentId"
+      FROM group_permissions
+      JOIN documents ON documents.id = group_permissions."documentId"
+      JOIN groups ON groups.id = group_permissions."groupId"
+        AND groups."deletedAt" IS NULL
+      JOIN group_users ON group_users."groupId" = group_permissions."groupId"
+        AND group_users."userId" = ${userId}
+      WHERE group_permissions."deletedAt" IS NULL
+        AND documents."isPrivate" = true
+    )`);
+  }
+
+  /**
+   * Returns a where clause fragment that excludes restricted documents the
+   * given user cannot access. Compose into any document query with AND to
+   * enforce restricted document access. Admins can access all restricted
+   * documents, in which case an empty clause is returned.
+   *
+   * @param user The user requesting documents.
+   * @returns A where clause fragment.
+   */
+  static restrictionsWhere(user: User): WhereOptions<Document> {
+    if (user.isAdmin) {
+      return {};
+    }
+
+    return {
+      [Op.or]: [
+        { isPrivate: false },
+        { id: { [Op.in]: this.restrictedDocumentIdsQuery(user) } },
+      ],
+    };
+  }
+
   // instance methods
 
   /**

server/queues/tasks/ExportTask.ts

@@ -136,13 +136,20 @@ export default abstract class ExportTask extends BaseTask<Props> {
         document.id
       );
 
-      if (!documentStructure) {
+      if (!documentStructure || !document.collectionId) {
         throw new Error("Document not found in collection tree");
       }
 
+      // Exclude restricted documents the user cannot access from the export
+      const children = await Collection.filterRestrictedNodes(
+        documentStructure.children ?? [],
+        user,
+        document.collectionId
+      );
+
       return this.exportDocument(
         document,
-        documentStructure.children ?? [],
+        children,
         fileOperation.options?.includeAttachments ?? true
       );
     }
@@ -193,6 +200,15 @@ export default abstract class ExportTask extends BaseTask<Props> {
       }
     );
 
+    // Exclude restricted documents the user cannot access from the export
+    for (const collection of collections) {
+      collection.documentStructure = await Collection.filterRestrictedNodes(
+        collection.documentStructure ?? [],
+        user,
+        collection.id
+      );
+    }
+
     return this.exportCollections(collections, fileOperation);
   }
 

server/routes/api/collections/collections.ts

@@ -142,13 +142,12 @@ router.post(
 
     const documentStructure = await collection.getCachedDocumentStructure();
 
-    // Filter restricted subtrees for non-admin users
-    const filteredStructure = user.isAdmin
-      ? documentStructure || []
-      : await Collection.filterRestrictedNodes(
-          documentStructure || [],
-          user.id
-        );
+    // Filter restricted subtrees the user cannot access
+    const filteredStructure = await Collection.filterRestrictedNodes(
+      documentStructure || [],
+      user,
+      collection.id
+    );
 
     ctx.body = {
       data: filteredStructure,

server/routes/api/documents/documents.test.ts

@@ -1252,6 +1252,77 @@ describe("#documents.list", () => {
     expect(body.data[0].id).toEqual(restrictedDoc.id);
   });
 
+  it("should return restricted documents for group members", async () => {
+    const team = await buildTeam();
+    const owner = await buildUser({ teamId: team.id });
+    const user = await buildUser({ teamId: team.id });
+    const collection = await buildCollection({
+      teamId: team.id,
+      permission: CollectionPermission.ReadWrite,
+    });
+    const document = await buildDocument({
+      userId: owner.id,
+      teamId: team.id,
+      collectionId: collection.id,
+      isPrivate: true,
+    });
+    const group = await buildGroup({ teamId: team.id, createdById: owner.id });
+    await group.$add("user", user, {
+      through: {
+        createdById: owner.id,
+      },
+    });
+    await GroupMembership.create({
+      groupId: group.id,
+      documentId: document.id,
+      permission: DocumentPermission.Read,
+      createdById: owner.id,
+    });
+    const res = await server.post("/api/documents.list", user);
+    const body = await res.json();
+    expect(res.status).toEqual(200);
+    expect(body.data.map((doc: { id: string }) => doc.id)).toContain(
+      document.id
+    );
+  });
+
+  it("should return children of a document shared from a private collection", async () => {
+    const team = await buildTeam();
+    const owner = await buildUser({ teamId: team.id });
+    const user = await buildUser({ teamId: team.id });
+    const collection = await buildCollection({
+      userId: owner.id,
+      teamId: team.id,
+      permission: null,
+    });
+    const parent = await buildDocument({
+      userId: owner.id,
+      teamId: team.id,
+      collectionId: collection.id,
+    });
+    const child = await buildDocument({
+      userId: owner.id,
+      teamId: team.id,
+      collectionId: collection.id,
+      parentDocumentId: parent.id,
+    });
+    await UserMembership.create({
+      documentId: parent.id,
+      userId: user.id,
+      permission: DocumentPermission.Read,
+      createdById: owner.id,
+    });
+    const res = await server.post("/api/documents.list", user, {
+      body: {
+        parentDocumentId: parent.id,
+      },
+    });
+    const body = await res.json();
+    expect(res.status).toEqual(200);
+    expect(body.data.length).toEqual(1);
+    expect(body.data[0].id).toEqual(child.id);
+  });
+
   it("should require authentication", async () => {
     const res = await server.post("/api/documents.list");
     const body = await res.json();
@@ -1578,6 +1649,59 @@ describe("#documents.search", () => {
     expect(body.data[0].document.title).toEqual("Much test support");
   });
 
+  it("should return restricted documents for admins", async () => {
+    const team = await buildTeam();
+    const owner = await buildUser({ teamId: team.id });
+    const admin = await buildAdmin({ teamId: team.id });
+    const collection = await buildCollection({
+      userId: owner.id,
+      teamId: team.id,
+      permission: CollectionPermission.ReadWrite,
+    });
+    const document = await buildDocument({
+      title: "restricted searchable",
+      userId: owner.id,
+      teamId: team.id,
+      collectionId: collection.id,
+      isPrivate: true,
+    });
+    const res = await server.post("/api/documents.search", admin, {
+      body: {
+        query: "restricted searchable",
+      },
+    });
+    const body = await res.json();
+    expect(res.status).toEqual(200);
+    expect(body.data.length).toEqual(1);
+    expect(body.data[0].document.id).toEqual(document.id);
+  });
+
+  it("should not return restricted documents for non-members", async () => {
+    const team = await buildTeam();
+    const owner = await buildUser({ teamId: team.id });
+    const user = await buildUser({ teamId: team.id });
+    const collection = await buildCollection({
+      userId: owner.id,
+      teamId: team.id,
+      permission: CollectionPermission.ReadWrite,
+    });
+    await buildDocument({
+      title: "restricted searchable",
+      userId: owner.id,
+      teamId: team.id,
+      collectionId: collection.id,
+      isPrivate: true,
+    });
+    const res = await server.post("/api/documents.search", user, {
+      body: {
+        query: "restricted searchable",
+      },
+    });
+    const body = await res.json();
+    expect(res.status).toEqual(200);
+    expect(body.data.length).toEqual(0);
+  });
+
   it("should return results using shareId", async () => {
     const subdomain = faker.internet.domainWord();
     const team = await buildTeam({ subdomain });

server/routes/api/documents/documents.ts

@@ -146,6 +146,7 @@ router.post(
     }
 
     let documentIds: string[] = [];
+    let collectionAccess: WhereOptions<Document> | undefined;
 
     // if a specific collection is passed then we need to check auth to view it
     if (collectionId) {
@@ -167,41 +168,26 @@ router.post(
         where[Op.and].push({ id: documentIds });
       }
 
-      // Filter restricted documents for non-admin users
-      if (!user.isAdmin) {
-        const restrictedDocIds = await accessibleRestrictedDocIds(user.id);
-        where[Op.and].push({
-          [Op.or]: [
-            { isPrivate: false },
-            ...(restrictedDocIds.length
-              ? [{ [Op.and]: [{ isPrivate: true }, { id: restrictedDocIds }] }]
-              : []),
-          ],
-        });
-      }
+      // Exclude restricted documents the user cannot access
+      where[Op.and].push(Document.restrictionsWhere(user));
     } else if (!backlinkDocumentId) {
       // if it's not a backlink request, filter by all collections the user has access to
       const collectionIds = await user.collectionIds();
 
       if (user.isAdmin) {
-        // Admins can see all documents in their accessible collections
-        where[Op.and].push({ collectionId: collectionIds });
+        collectionAccess = { collectionId: collectionIds };
       } else {
-        // Non-admin users: collection access only for non-restricted docs.
-        // Restricted docs with direct membership are included via subquery.
-        const restrictedDocIds = await accessibleRestrictedDocIds(user.id);
-
-        where[Op.and].push({
+        // Restricted documents the user can access through a direct or group
+        // membership are included regardless of collection access.
+        collectionAccess = {
           [Op.or]: [
-            {
-              [Op.and]: [{ collectionId: collectionIds }, { isPrivate: false }],
-            },
-            ...(restrictedDocIds.length
-              ? [{ [Op.and]: [{ isPrivate: true }, { id: restrictedDocIds }] }]
-              : []),
+            { collectionId: collectionIds },
+            { id: { [Op.in]: Document.restrictedDocumentIdsQuery(user) } },
           ],
-        });
+        };
+        where[Op.and].push(Document.restrictionsWhere(user));
       }
+      where[Op.and].push(collectionAccess);
     }
 
     if (parentDocumentId) {
@@ -235,7 +221,10 @@ router.post(
       ]);
 
       if (groupMembership || membership) {
-        remove(where[Op.and], (cond) => has(cond, "collectionId"));
+        remove(
+          where[Op.and],
+          (cond) => cond === collectionAccess || has(cond, "collectionId")
+        );
       }
 
       where[Op.and].push({ parentDocumentId });
@@ -824,11 +813,12 @@ router.post(
       });
       documentTree = collection?.getDocumentTree(document.id) ?? undefined;
 
-      // Filter restricted subtrees for non-admin users
-      if (documentTree && !user.isAdmin) {
+      // Filter restricted subtrees the user cannot access
+      if (documentTree) {
         const [filtered] = await Collection.filterRestrictedNodes(
           [documentTree],
-          user.id
+          user,
+          document.collectionId
         );
         documentTree = filtered;
       }
@@ -2204,63 +2194,4 @@ function getAPIVersion(ctx: APIContext) {
   );
 }
 
-/**
- * Find IDs of restricted documents the given user can access via direct or group membership.
- *
- * @param userId the user to check memberships for.
- * @return restricted document IDs accessible to the user.
- */
-async function accessibleRestrictedDocIds(userId: string): Promise<string[]> {
-  const [userMemberships, groupMemberships] = await Promise.all([
-    UserMembership.findAll({
-      attributes: ["documentId"],
-      where: { userId, documentId: { [Op.ne]: null } },
-      include: [
-        {
-          model: Document.unscoped(),
-          required: true,
-          attributes: [],
-          where: { isPrivate: true },
-        },
-      ],
-    }),
-    GroupMembership.findAll({
-      attributes: ["documentId"],
-      where: { documentId: { [Op.ne]: null } },
-      include: [
-        {
-          model: Document.unscoped(),
-          required: true,
-          attributes: [],
-          where: { isPrivate: true },
-        },
-        {
-          model: Group,
-          required: true,
-          include: [
-            {
-              model: GroupUser,
-              required: true,
-              where: { userId },
-            },
-          ],
-        },
-      ],
-    }),
-  ]);
-
-  const ids = new Set<string>();
-  for (const m of userMemberships) {
-    if (m.documentId) {
-      ids.add(m.documentId);
-    }
-  }
-  for (const m of groupMemberships) {
-    if (m.documentId) {
-      ids.add(m.documentId);
-    }
-  }
-  return [...ids];
-}
-
 export default router;