feat: Added ability to disable sharing at collection (#1875)

* feat: Added ability to disable sharing at collection

* fix: Disable all previous share links when disabling collection share
Language

* fix: Disable document sharing for read-only collection members

* wip

* test

* fix: Clear policies after updating sharing settings

* chore: Less ambiguous language

* feat: Allow setting sharing choice on collection creation

server/api/collections.js

@@ -34,6 +34,7 @@ router.post("collections.create", auth(), async (ctx) => {
     name,
     color,
     description,
+    sharing,
     icon,
     sort = Collection.DEFAULT_SORT,
   } = ctx.body;
@@ -55,6 +56,7 @@ router.post("collections.create", auth(), async (ctx) => {
     teamId: user.teamId,
     createdById: user.id,
     private: isPrivate,
+    sharing,
     sort,
   });
 
@@ -452,7 +454,7 @@ router.post("collections.export_all", auth(), async (ctx) => {
 });
 
 router.post("collections.update", auth(), async (ctx) => {
-  let { id, name, description, icon, color, sort } = ctx.body;
+  let { id, name, description, icon, color, sort, sharing } = ctx.body;
   const isPrivate = ctx.body.private;
 
   if (color) {
@@ -498,6 +500,9 @@ router.post("collections.update", auth(), async (ctx) => {
   if (isPrivate !== undefined) {
     collection.private = isPrivate;
   }
+  if (sharing !== undefined) {
+    collection.sharing = sharing;
+  }
   if (sort !== undefined) {
     collection.sort = sort;
   }

server/api/collections.test.js

@@ -876,6 +876,18 @@ describe("#collections.create", () => {
     expect(body.policies[0].abilities.export).toBeTruthy();
   });
 
+  it("should allow setting sharing to false", async () => {
+    const { user } = await seed();
+    const res = await server.post("/api/collections.create", {
+      body: { token: user.getJwtToken(), name: "Test", sharing: false },
+    });
+    const body = await res.json();
+
+    expect(res.status).toEqual(200);
+    expect(body.data.id).toBeTruthy();
+    expect(body.data.sharing).toBe(false);
+  });
+
   it("should return correct policies with private collection", async () => {
     const { user } = await seed();
     const res = await server.post("/api/collections.create", {

server/api/documents.js

@@ -488,6 +488,11 @@ async function loadDocument({ id, shareId, user }) {
       authorize(user, "read", document);
     }
 
+    const collection = await Collection.findByPk(document.collectionId);
+    if (!collection.sharing) {
+      throw new AuthorizationError();
+    }
+
     const team = await Team.findByPk(document.teamId);
     if (!team.sharing) {
       throw new AuthorizationError();

server/api/documents.test.js

@@ -112,6 +112,23 @@ describe("#documents.info", () => {
     expect(res.status).toEqual(403);
   });
 
+  it("should not return document from shareId if sharing is disabled for collection", async () => {
+    const { document, collection, user } = await seed();
+    const share = await buildShare({
+      documentId: document.id,
+      teamId: document.teamId,
+      userId: user.id,
+    });
+
+    collection.sharing = false;
+    await collection.save();
+
+    const res = await server.post("/api/documents.info", {
+      body: { shareId: share.id },
+    });
+    expect(res.status).toEqual(403);
+  });
+
   it("should not return document from revoked shareId", async () => {
     const { document, user } = await seed();
     const share = await buildShare({

server/api/shares.test.js

@@ -202,7 +202,7 @@ describe("#shares.create", () => {
     expect(body.data.id).toBe(share.id);
   });
 
-  it("should not allow creating a share record if disabled", async () => {
+  it("should not allow creating a share record if team sharing disabled", async () => {
     const { user, document, team } = await seed();
     await team.update({ sharing: false });
     const res = await server.post("/api/shares.create", {
@@ -211,6 +211,15 @@ describe("#shares.create", () => {
     expect(res.status).toEqual(403);
   });
 
+  it("should not allow creating a share record if collection sharing disabled", async () => {
+    const { user, collection, document } = await seed();
+    await collection.update({ sharing: false });
+    const res = await server.post("/api/shares.create", {
+      body: { token: user.getJwtToken(), documentId: document.id },
+    });
+    expect(res.status).toEqual(403);
+  });
+
   it("should require authentication", async () => {
     const { document } = await seed();
     const res = await server.post("/api/shares.create", {

server/migrations/20210208062816-disable-collection-sharing.js

@@ -0,0 +1,12 @@
+module.exports = {
+  up: async (queryInterface, Sequelize) => {
+    await queryInterface.addColumn('collections', 'sharing', {
+      type: Sequelize.BOOLEAN,
+      allowNull: false,
+      defaultValue: true
+    });
+  },
+  down: async (queryInterface, Sequelize) => {
+    await queryInterface.removeColumn('collections', 'sharing');
+  }
+}

server/models/Collection.js

@@ -24,6 +24,11 @@ const Collection = sequelize.define(
     private: DataTypes.BOOLEAN,
     maintainerApprovalRequired: DataTypes.BOOLEAN,
     documentStructure: DataTypes.JSONB,
+    sharing: {
+      type: DataTypes.BOOLEAN,
+      allowNull: false,
+      defaultValue: true,
+    },
     sort: {
       type: DataTypes.JSONB,
       validate: {

server/policies/collection.js

@@ -31,6 +31,29 @@ allow(User, ["read", "export"], Collection, (user, collection) => {
   return true;
 });
 
+allow(User, "share", Collection, (user, collection) => {
+  if (!collection || user.teamId !== collection.teamId) return false;
+  if (!collection.sharing) return false;
+
+  if (collection.private) {
+    invariant(
+      collection.memberships,
+      "membership should be preloaded, did you forget withMembership scope?"
+    );
+
+    const allMemberships = concat(
+      collection.memberships,
+      collection.collectionGroupMemberships
+    );
+
+    return some(allMemberships, (m) =>
+      ["read_write", "maintainer"].includes(m.permission)
+    );
+  }
+
+  return true;
+});
+
 allow(User, ["publish", "update"], Collection, (user, collection) => {
   if (!collection || user.teamId !== collection.teamId) return false;
 

server/policies/document.js

@@ -31,12 +31,22 @@ allow(User, ["star", "unstar"], Document, (user, document) => {
   return user.teamId === document.teamId;
 });
 
-allow(User, ["update", "share"], Document, (user, document) => {
+allow(User, "share", Document, (user, document) => {
   if (document.archivedAt) return false;
   if (document.deletedAt) return false;
 
-  // existence of collection option is not required here to account for share tokens
-  if (document.collection && cannot(user, "update", document.collection)) {
+  if (cannot(user, "share", document.collection)) {
+    return false;
+  }
+
+  return user.teamId === document.teamId;
+});
+
+allow(User, "update", Document, (user, document) => {
+  if (document.archivedAt) return false;
+  if (document.deletedAt) return false;
+
+  if (cannot(user, "update", document.collection)) {
     return false;
   }
 

server/presenters/collection.js

@@ -30,6 +30,7 @@ export default function present(collection: Collection) {
     icon: collection.icon,
     color: collection.color || "#4E5C6E",
     private: collection.private,
+    sharing: collection.sharing,
     createdAt: collection.createdAt,
     updatedAt: collection.updatedAt,
     deletedAt: collection.deletedAt,