fix: Document published notification potentially sent to users without permission to view document

server/services/notifications.js

@@ -70,6 +70,13 @@ export default class Notifications {
         return;
       }
 
+      // Check the user has access to the collection this document is in. Just
+      // because they were a collaborator once doesn't mean they still are.
+      const collectionIds = await setting.user.collectionIds();
+      if (!collectionIds.includes(document.collectionId)) {
+        return;
+      }
+
       // If this user has viewed the document since the last update was made
       // then we can avoid sending them a useless notification, yay.
       const view = await View.findOne({

server/services/notifications.test.js

@@ -1,7 +1,7 @@
 /* eslint-disable flowtype/require-valid-file-annotation */
 import mailer from "../mailer";
 import { View, NotificationSetting } from "../models";
-import { buildDocument, buildUser } from "../test/factories";
+import { buildDocument, buildCollection, buildUser } from "../test/factories";
 import { flushdb } from "../test/support";
 import NotificationsService from "./notifications";
 
@@ -12,6 +12,83 @@ const Notifications = new NotificationsService();
 beforeEach(() => flushdb());
 beforeEach(jest.resetAllMocks);
 
+describe("documents.publish", () => {
+  test("should not send a notification to author", async () => {
+    const user = await buildUser();
+    const document = await buildDocument({
+      teamId: user.teamId,
+      lastModifiedById: user.id,
+    });
+
+    await NotificationSetting.create({
+      userId: user.id,
+      teamId: user.teamId,
+      event: "documents.publish",
+    });
+
+    await Notifications.on({
+      name: "documents.publish",
+      documentId: document.id,
+      collectionId: document.collectionId,
+      teamId: document.teamId,
+      actorId: document.createdById,
+    });
+
+    expect(mailer.documentNotification).not.toHaveBeenCalled();
+  });
+
+  test("should send a notification to other users in team", async () => {
+    const user = await buildUser();
+    const document = await buildDocument({
+      teamId: user.teamId,
+    });
+
+    await NotificationSetting.create({
+      userId: user.id,
+      teamId: user.teamId,
+      event: "documents.publish",
+    });
+
+    await Notifications.on({
+      name: "documents.publish",
+      documentId: document.id,
+      collectionId: document.collectionId,
+      teamId: document.teamId,
+      actorId: document.createdById,
+    });
+
+    expect(mailer.documentNotification).toHaveBeenCalled();
+  });
+
+  test("should not send a notification to users without collection access", async () => {
+    const user = await buildUser();
+    const collection = await buildCollection({
+      teamId: user.teamId,
+      private: true,
+    });
+    const document = await buildDocument({
+      teamId: user.teamId,
+      collectionId: collection.id,
+    });
+
+    await NotificationSetting.create({
+      userId: user.id,
+      teamId: user.teamId,
+      event: "documents.publish",
+    });
+
+    await Notifications.on({
+      name: "documents.publish",
+      documentId: document.id,
+      collectionId: document.collectionId,
+      teamId: document.teamId,
+      actorId: document.createdById,
+    });
+
+    expect(mailer.documentNotification).not.toHaveBeenCalled();
+  });
+});
+
 describe("documents.update.debounced", () => {
   test("should send a notification to other collaborator", async () => {
     const document = await buildDocument();