From 29a653aaeb0eccd28159bef1e5dab1d8ddf9b470 Mon Sep 17 00:00:00 2001
From: Tom Moor <tom@getoutline.com>
Date: Tue, 25 Jun 2024 08:28:32 -0400
Subject: [PATCH] fix: Admins cannot query permissions on private collections
 (#7145)

* fix: Admins have permission to see existence of all collections (in settings)

* fix: Current user filtered from suggestions. As an admin managing other collections this is limiting

* test
---
 .../Sharing/components/Suggestions.tsx          |  2 +-
 server/policies/collection.test.ts              |  2 +-
 server/policies/collection.ts                   | 17 ++++++++++++++++-
 server/policies/document.ts                     |  2 +-
 4 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/app/components/Sharing/components/Suggestions.tsx b/app/components/Sharing/components/Suggestions.tsx
index e330ff5cc..3b83feef7 100644
--- a/app/components/Sharing/components/Suggestions.tsx
+++ b/app/components/Sharing/components/Suggestions.tsx
@@ -100,7 +100,7 @@ export const Suggestions = observer(
           : collection
           ? users.notInCollection(collection.id, query)
           : users.orderedData
-      ).filter((u) => u.id !== user.id && !u.isSuspended);
+      ).filter((u) => !u.isSuspended);
 
       if (isEmail(query)) {
         filtered.push(getSuggestionForEmail(query));
diff --git a/server/policies/collection.test.ts b/server/policies/collection.test.ts
index 543488d93..2bff92e70 100644
--- a/server/policies/collection.test.ts
+++ b/server/policies/collection.test.ts
@@ -27,7 +27,7 @@ describe("admin", () => {
     expect(abilities.updateDocument).toEqual(false);
     expect(abilities.createDocument).toEqual(false);
     expect(abilities.share).toEqual(false);
-    expect(abilities.read).toEqual(false);
+    expect(abilities.read).toEqual(true);
     expect(abilities.update).toEqual(true);
   });
 
diff --git a/server/policies/collection.ts b/server/policies/collection.ts
index bd0db9c9a..fe02124e7 100644
--- a/server/policies/collection.ts
+++ b/server/policies/collection.ts
@@ -32,9 +32,24 @@ allow(User, "move", Collection, (actor, collection) =>
   )
 );
 
+allow(User, "read", Collection, (user, collection) => {
+  if (!collection || user.teamId !== collection.teamId) {
+    return false;
+  }
+  if (user.isAdmin) {
+    return true;
+  }
+
+  if (collection.isPrivate || user.isGuest) {
+    return includesMembership(collection, Object.values(CollectionPermission));
+  }
+
+  return true;
+});
+
 allow(
   User,
-  ["read", "readDocument", "star", "unstar"],
+  ["readDocument", "star", "unstar"],
   Collection,
   (user, collection) => {
     if (!collection || user.teamId !== collection.teamId) {
diff --git a/server/policies/document.ts b/server/policies/document.ts
index d53f5fae5..c3bfd1bc8 100644
--- a/server/policies/document.ts
+++ b/server/policies/document.ts
@@ -138,7 +138,7 @@ allow(User, "createChildDocument", Document, (actor, document) =>
     can(actor, "update", document),
     or(
       includesMembership(document, [DocumentPermission.Admin]),
-      can(actor, "read", document?.collection)
+      can(actor, "readDocument", document?.collection)
     ),
     !document?.isDraft,
     !document?.template