From ec5e3120d204ce11cd23451cc8d3c31f04663f04 Mon Sep 17 00:00:00 2001
From: Nan Yu <nan@getoutline.com>
Date: Mon, 15 Nov 2021 16:05:58 -0800
Subject: [PATCH] fix: visible groups (#2729)

* updated readme to give some light testing instructions
* updated tests to accept new behavior for group memberships
* use test factories in more places
* add debug logs for mailer events in development
---
 README.md                           |  7 +++++++
 server/mailer.js                    |  8 +++++++-
 server/policies/group.js            |  8 +++-----
 server/routes/api/documents.test.js | 12 ++++++++----
 server/routes/api/groups.js         |  7 -------
 server/routes/api/groups.test.js    |  7 +++++--
 6 files changed, 30 insertions(+), 19 deletions(-)

diff --git a/README.md b/README.md
index 2b4f5da1..018a15e9 100644
--- a/README.md
+++ b/README.md
@@ -101,6 +101,13 @@ For contributing features and fixes you can quickly get an environment running u
    1. Ensure that the bot token scope contains at least `users:read`
 1. Run `make up`. This will download dependencies, build and launch a development version of Outline
 
+### Testing
+
+The `Makefile` has other useful scripts, including some test automation.
+
+1. To run the entire test suite, run `make test`
+1. During development, it's often useful, to re-run some tests every time a file is changed. Use `make watch` to start the test daemon and follow the instructions in the console
+
 # Contributing
 
 Outline is built and maintained by a small team – we'd love your help to fix bugs and add features!
diff --git a/server/mailer.js b/server/mailer.js
index eeedb5ac..6474f6d3 100644
--- a/server/mailer.js
+++ b/server/mailer.js
@@ -204,12 +204,18 @@ export class Mailer {
   };
 
   signin = async (opts: { to: string, token: string, teamUrl: string }) => {
+    const signInLink = signinEmailText(opts);
+
+    if (process.env.NODE_ENV === "development") {
+      Logger.debug("email", `Sign-In link: ${signInLink}`);
+    }
+
     this.sendMail({
       to: opts.to,
       title: "Magic signin link",
       previewText: "Here’s your link to signin to Outline.",
       html: <SigninEmail {...opts} />,
-      text: signinEmailText(opts),
+      text: signInLink,
     });
   };
 
diff --git a/server/policies/group.js b/server/policies/group.js
index 11b30cf1..43536eac 100644
--- a/server/policies/group.js
+++ b/server/policies/group.js
@@ -12,12 +12,10 @@ allow(User, "createGroup", Team, (actor, team) => {
 });
 
 allow(User, "read", Group, (actor, group) => {
+  // for the time being, we're going to let everyone on the team see every group
+  // we may need to make this more granular in the future
   if (!group || actor.teamId !== group.teamId) return false;
-  if (actor.isAdmin) return true;
-  if (group.groupMemberships.filter((gm) => gm.userId === actor.id).length) {
-    return true;
-  }
-  return false;
+  return true;
 });
 
 allow(User, ["update", "delete"], Group, (actor, group) => {
diff --git a/server/routes/api/documents.test.js b/server/routes/api/documents.test.js
index 561d6298..75450f54 100644
--- a/server/routes/api/documents.test.js
+++ b/server/routes/api/documents.test.js
@@ -1059,7 +1059,7 @@ describe("#documents.search", () => {
   });
 
   it("should strip junk from search term", async () => {
-    const { user } = await seed();
+    const user = await buildUser();
     const firstResult = await buildDocument({
       title: "search term",
       text: "this is some random text of the document body",
@@ -1137,7 +1137,7 @@ describe("#documents.search", () => {
   });
 
   it("should not return draft documents created by other users", async () => {
-    const { user } = await seed();
+    const user = await buildUser();
     await buildDocument({
       title: "search term",
       text: "search term",
@@ -1176,7 +1176,7 @@ describe("#documents.search", () => {
   });
 
   it("should return archived documents if chosen", async () => {
-    const { user } = await seed();
+    const user = await buildUser();
     const document = await buildDocument({
       title: "search term",
       text: "search term",
@@ -1230,7 +1230,11 @@ describe("#documents.search", () => {
   });
 
   it("should return documents for a specific private collection", async () => {
-    const { user, collection } = await seed();
+    const user = await buildUser();
+    const collection = await buildCollection({
+      teamId: user.teamId,
+    });
+
     collection.permission = null;
     await collection.save();
 
diff --git a/server/routes/api/groups.js b/server/routes/api/groups.js
index e8e064ad..fbd7b9af 100644
--- a/server/routes/api/groups.js
+++ b/server/routes/api/groups.js
@@ -33,13 +33,6 @@ router.post("groups.list", auth(), pagination(), async (ctx) => {
     limit: ctx.state.pagination.limit,
   });
 
-  if (!user.isAdmin) {
-    groups = groups.filter(
-      (group) =>
-        group.groupMemberships.filter((gm) => gm.userId === user.id).length
-    );
-  }
-
   ctx.body = {
     pagination: ctx.state.pagination,
     data: {
diff --git a/server/routes/api/groups.test.js b/server/routes/api/groups.test.js
index 4902a7eb..80ec80a0 100644
--- a/server/routes/api/groups.test.js
+++ b/server/routes/api/groups.test.js
@@ -203,7 +203,7 @@ describe("#groups.info", () => {
     expect(body.data.id).toEqual(group.id);
   });
 
-  it("should not return group if non-member, non-admin", async () => {
+  it("should still return group if non-member, non-admin", async () => {
     const user = await buildUser();
     const group = await buildGroup({ teamId: user.teamId });
 
@@ -211,7 +211,10 @@ describe("#groups.info", () => {
       body: { token: user.getJwtToken(), id: group.id },
     });
 
-    expect(res.status).toEqual(403);
+    const body = await res.json();
+
+    expect(res.status).toEqual(200);
+    expect(body.data.id).toEqual(group.id);
   });
 
   it("should require authentication", async () => {