Collection Permissions (#829)

see https://github.com/outline/outline/issues/668

server/api/__snapshots__/collections.test.js.snap

@@ -1,5 +1,13 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`#collections.add_user should require user in team 1`] = `
+Object {
+  "error": "authorization_error",
+  "message": "Authorization error",
+  "ok": false,
+}
+`;
+
 exports[`#collections.create should require authentication 1`] = `
 Object {
   "error": "authentication_required",
@@ -53,3 +61,20 @@ Object {
   "status": 401,
 }
 `;
+
+exports[`#collections.remove_user should require user in team 1`] = `
+Object {
+  "error": "authorization_error",
+  "message": "Authorization error",
+  "ok": false,
+}
+`;
+
+exports[`#collections.users should require authentication 1`] = `
+Object {
+  "error": "authentication_required",
+  "message": "Authentication required",
+  "ok": false,
+  "status": 401,
+}
+`;

server/api/apiKeys.js

@@ -48,7 +48,7 @@ router.post('apiKeys.list', auth(), pagination(), async ctx => {
 
 router.post('apiKeys.delete', auth(), async ctx => {
   const { id } = ctx.body;
-  ctx.assertPresent(id, 'id is required');
+  ctx.assertUuid(id, 'id is required');
 
   const user = ctx.state.user;
   const key = await ApiKey.findById(id);

server/api/collections.js

@@ -1,11 +1,10 @@
 // @flow
 import Router from 'koa-router';
-
 import auth from '../middlewares/authentication';
 import pagination from './middlewares/pagination';
-import { presentCollection } from '../presenters';
-import { Collection, Team } from '../models';
-import { ValidationError } from '../errors';
+import { presentCollection, presentUser } from '../presenters';
+import { Collection, CollectionUser, Team, User } from '../models';
+import { ValidationError, InvalidRequestError } from '../errors';
 import { exportCollection, exportCollections } from '../logistics';
 import policy from '../policies';
 
@@ -14,6 +13,8 @@ const router = new Router();
 
 router.post('collections.create', auth(), async ctx => {
   const { name, color, description, type } = ctx.body;
+  const isPrivate = ctx.body.private;
+
   ctx.assertPresent(name, 'name is required');
   if (color)
     ctx.assertHexColor(color, 'Invalid hex value (please use format #FFFFFF)');
@@ -28,6 +29,7 @@ router.post('collections.create', auth(), async ctx => {
     type: type || 'atlas',
     teamId: user.teamId,
     creatorId: user.id,
+    private: isPrivate,
   });
 
   ctx.body = {
@@ -37,9 +39,9 @@ router.post('collections.create', auth(), async ctx => {
 
 router.post('collections.info', auth(), async ctx => {
   const { id } = ctx.body;
-  ctx.assertPresent(id, 'id is required');
+  ctx.assertUuid(id, 'id is required');
 
-  const collection = await Collection.scope('withRecentDocuments').findById(id);
+  const collection = await Collection.findById(id);
   authorize(ctx.state.user, 'read', collection);
 
   ctx.body = {
@@ -47,9 +49,76 @@ router.post('collections.info', auth(), async ctx => {
   };
 });
 
+router.post('collections.add_user', auth(), async ctx => {
+  const { id, userId, permission = 'read_write' } = ctx.body;
+  ctx.assertUuid(id, 'id is required');
+  ctx.assertUuid(userId, 'userId is required');
+
+  const collection = await Collection.findById(id);
+  authorize(ctx.state.user, 'update', collection);
+
+  if (!collection.private) {
+    throw new InvalidRequestError('Collection must be private to add users');
+  }
+
+  const user = await User.findById(userId);
+  authorize(ctx.state.user, 'read', user);
+
+  await CollectionUser.create({
+    collectionId: id,
+    userId,
+    permission,
+    createdById: ctx.state.user.id,
+  });
+
+  ctx.body = {
+    success: true,
+  };
+});
+
+router.post('collections.remove_user', auth(), async ctx => {
+  const { id, userId } = ctx.body;
+  ctx.assertUuid(id, 'id is required');
+  ctx.assertUuid(userId, 'userId is required');
+
+  const collection = await Collection.findById(id);
+  authorize(ctx.state.user, 'update', collection);
+
+  if (!collection.private) {
+    throw new InvalidRequestError('Collection must be private to remove users');
+  }
+
+  const user = await User.findById(userId);
+  authorize(ctx.state.user, 'read', user);
+
+  await collection.removeUser(user);
+
+  ctx.body = {
+    success: true,
+  };
+});
+
+router.post('collections.users', auth(), async ctx => {
+  const { id } = ctx.body;
+  ctx.assertUuid(id, 'id is required');
+
+  const collection = await Collection.findById(id);
+  authorize(ctx.state.user, 'read', collection);
+
+  const users = await collection.getUsers();
+
+  const data = await Promise.all(
+    users.map(async user => await presentUser(ctx, user))
+  );
+
+  ctx.body = {
+    data,
+  };
+});
+
 router.post('collections.export', auth(), async ctx => {
   const { id } = ctx.body;
-  ctx.assertPresent(id, 'id is required');
+  ctx.assertUuid(id, 'id is required');
 
   const user = ctx.state.user;
   const collection = await Collection.findById(id);
@@ -78,16 +147,33 @@ router.post('collections.exportAll', auth(), async ctx => {
 
 router.post('collections.update', auth(), async ctx => {
   const { id, name, description, color } = ctx.body;
+  const isPrivate = ctx.body.private;
+
   ctx.assertPresent(name, 'name is required');
   if (color)
     ctx.assertHexColor(color, 'Invalid hex value (please use format #FFFFFF)');
 
+  const user = ctx.state.user;
   const collection = await Collection.findById(id);
-  authorize(ctx.state.user, 'update', collection);
+  authorize(user, 'update', collection);
+
+  if (isPrivate && !collection.private) {
+    await CollectionUser.findOrCreate({
+      where: {
+        collectionId: collection.id,
+        userId: user.id,
+      },
+      defaults: {
+        permission: 'read_write',
+        createdById: user.id,
+      },
+    });
+  }
 
   collection.name = name;
   collection.description = description;
   collection.color = color;
+  collection.private = isPrivate;
   await collection.save();
 
   ctx.body = {
@@ -97,9 +183,12 @@ router.post('collections.update', auth(), async ctx => {
 
 router.post('collections.list', auth(), pagination(), async ctx => {
   const user = ctx.state.user;
-  const collections = await Collection.findAll({
+
+  const collectionIds = await user.collectionIds();
+  let collections = await Collection.findAll({
     where: {
       teamId: user.teamId,
+      id: collectionIds,
     },
     order: [['updatedAt', 'DESC']],
     offset: ctx.state.pagination.offset,
@@ -120,7 +209,7 @@ router.post('collections.list', auth(), pagination(), async ctx => {
 
 router.post('collections.delete', auth(), async ctx => {
   const { id } = ctx.body;
-  ctx.assertPresent(id, 'id is required');
+  ctx.assertUuid(id, 'id is required');
 
   const collection = await Collection.findById(id);
   authorize(ctx.state.user, 'delete', collection);

server/api/collections.test.js

@@ -2,7 +2,7 @@
 import TestServer from 'fetch-test-server';
 import app from '..';
 import { flushdb, seed } from '../test/support';
-import { buildUser } from '../test/factories';
+import { buildUser, buildCollection } from '../test/factories';
 import { Collection } from '../models';
 const server = new TestServer(app.callback());
 
@@ -29,9 +29,54 @@ describe('#collections.list', async () => {
     expect(body.data.length).toEqual(1);
     expect(body.data[0].id).toEqual(collection.id);
   });
+
+  it('should not return private collections not a member of', async () => {
+    const { user, collection } = await seed();
+    await buildCollection({
+      private: true,
+      teamId: user.teamId,
+    });
+    const res = await server.post('/api/collections.list', {
+      body: { token: user.getJwtToken() },
+    });
+    const body = await res.json();
+
+    expect(res.status).toEqual(200);
+    expect(body.data.length).toEqual(1);
+    expect(body.data[0].id).toEqual(collection.id);
+  });
+
+  it('should return private collections member of', async () => {
+    const { user } = await seed();
+    await buildCollection({
+      private: true,
+      teamId: user.teamId,
+      userId: user.id,
+    });
+    const res = await server.post('/api/collections.list', {
+      body: { token: user.getJwtToken() },
+    });
+    const body = await res.json();
+
+    expect(res.status).toEqual(200);
+    expect(body.data.length).toEqual(2);
+  });
 });
 
 describe('#collections.export', async () => {
+  it('should require user to be a member', async () => {
+    const { user } = await seed();
+    const collection = await buildCollection({
+      private: true,
+      teamId: user.teamId,
+    });
+    const res = await server.post('/api/collections.export', {
+      body: { token: user.getJwtToken(), id: collection.id },
+    });
+
+    expect(res.status).toEqual(403);
+  });
+
   it('should require authentication', async () => {
     const res = await server.post('/api/collections.export');
     const body = await res.json();
@@ -77,6 +122,170 @@ describe('#collections.exportAll', async () => {
   });
 });
 
+describe('#collections.add_user', async () => {
+  it('should add user to collection', async () => {
+    const user = await buildUser();
+    const collection = await buildCollection({
+      teamId: user.teamId,
+      userId: user.id,
+      private: true,
+    });
+    const anotherUser = await buildUser({ teamId: user.teamId });
+    const res = await server.post('/api/collections.add_user', {
+      body: {
+        token: user.getJwtToken(),
+        id: collection.id,
+        userId: anotherUser.id,
+      },
+    });
+
+    const users = await collection.getUsers();
+    expect(res.status).toEqual(200);
+    expect(users.length).toEqual(2);
+  });
+
+  it('should require user in team', async () => {
+    const user = await buildUser();
+    const collection = await buildCollection({
+      teamId: user.teamId,
+      private: true,
+    });
+    const anotherUser = await buildUser();
+    const res = await server.post('/api/collections.add_user', {
+      body: {
+        token: user.getJwtToken(),
+        id: collection.id,
+        userId: anotherUser.id,
+      },
+    });
+    const body = await res.json();
+
+    expect(res.status).toEqual(403);
+    expect(body).toMatchSnapshot();
+  });
+
+  it('should require authentication', async () => {
+    const res = await server.post('/api/collections.add_user');
+
+    expect(res.status).toEqual(401);
+  });
+
+  it('should require authorization', async () => {
+    const { collection } = await seed();
+    const user = await buildUser();
+    const anotherUser = await buildUser({ teamId: user.teamId });
+
+    const res = await server.post('/api/collections.add_user', {
+      body: {
+        token: user.getJwtToken(),
+        id: collection.id,
+        userId: anotherUser.id,
+      },
+    });
+    expect(res.status).toEqual(403);
+  });
+});
+
+describe('#collections.remove_user', async () => {
+  it('should remove user from collection', async () => {
+    const user = await buildUser();
+    const collection = await buildCollection({
+      teamId: user.teamId,
+      userId: user.id,
+      private: true,
+    });
+    const anotherUser = await buildUser({ teamId: user.teamId });
+
+    await server.post('/api/collections.add_user', {
+      body: {
+        token: user.getJwtToken(),
+        id: collection.id,
+        userId: anotherUser.id,
+      },
+    });
+
+    const res = await server.post('/api/collections.remove_user', {
+      body: {
+        token: user.getJwtToken(),
+        id: collection.id,
+        userId: anotherUser.id,
+      },
+    });
+
+    const users = await collection.getUsers();
+    expect(res.status).toEqual(200);
+    expect(users.length).toEqual(1);
+  });
+
+  it('should require user in team', async () => {
+    const user = await buildUser();
+    const collection = await buildCollection({
+      teamId: user.teamId,
+      private: true,
+    });
+    const anotherUser = await buildUser();
+    const res = await server.post('/api/collections.remove_user', {
+      body: {
+        token: user.getJwtToken(),
+        id: collection.id,
+        userId: anotherUser.id,
+      },
+    });
+    const body = await res.json();
+
+    expect(res.status).toEqual(403);
+    expect(body).toMatchSnapshot();
+  });
+
+  it('should require authentication', async () => {
+    const res = await server.post('/api/collections.remove_user');
+
+    expect(res.status).toEqual(401);
+  });
+
+  it('should require authorization', async () => {
+    const { collection } = await seed();
+    const user = await buildUser();
+    const anotherUser = await buildUser({ teamId: user.teamId });
+
+    const res = await server.post('/api/collections.remove_user', {
+      body: {
+        token: user.getJwtToken(),
+        id: collection.id,
+        userId: anotherUser.id,
+      },
+    });
+    expect(res.status).toEqual(403);
+  });
+});
+
+describe('#collections.users', async () => {
+  it('should return members in private collection', async () => {
+    const { collection, user } = await seed();
+    const res = await server.post('/api/collections.users', {
+      body: { token: user.getJwtToken(), id: collection.id },
+    });
+    expect(res.status).toEqual(200);
+  });
+
+  it('should require authentication', async () => {
+    const res = await server.post('/api/collections.users');
+    const body = await res.json();
+
+    expect(res.status).toEqual(401);
+    expect(body).toMatchSnapshot();
+  });
+
+  it('should require authorization', async () => {
+    const { collection } = await seed();
+    const user = await buildUser();
+    const res = await server.post('/api/collections.users', {
+      body: { token: user.getJwtToken(), id: collection.id },
+    });
+    expect(res.status).toEqual(403);
+  });
+});
+
 describe('#collections.info', async () => {
   it('should return collection', async () => {
     const { user, collection } = await seed();
@@ -89,6 +298,17 @@ describe('#collections.info', async () => {
     expect(body.data.id).toEqual(collection.id);
   });
 
+  it('should require user member of collection', async () => {
+    const { user, collection } = await seed();
+    collection.private = true;
+    await collection.save();
+
+    const res = await server.post('/api/collections.info', {
+      body: { token: user.getJwtToken(), id: collection.id },
+    });
+    expect(res.status).toEqual(403);
+  });
+
   it('should require authentication', async () => {
     const res = await server.post('/api/collections.info');
     const body = await res.json();

server/api/documents.js

@@ -14,14 +14,39 @@ const { authorize, cannot } = policy;
 const router = new Router();
 
 router.post('documents.list', auth(), pagination(), async ctx => {
-  let { sort = 'updatedAt', direction, collection, user } = ctx.body;
+  const { sort = 'updatedAt' } = ctx.body;
+  const collectionId = ctx.body.collection;
+  const createdById = ctx.body.user;
+  let direction = ctx.body.direction;
   if (direction !== 'ASC') direction = 'DESC';
 
-  let where = { teamId: ctx.state.user.teamId };
-  if (collection) where = { ...where, collectionId: collection };
-  if (user) where = { ...where, createdById: user };
+  // always filter by the current team
+  const user = ctx.state.user;
+  let where = { teamId: user.teamId };
 
-  const starredScope = { method: ['withStarred', ctx.state.user.id] };
+  // if a specific user is passed then add to filters. If the user doesn't
+  // exist in the team then nothing will be returned, so no need to check auth
+  if (createdById) {
+    ctx.assertUuid(createdById, 'user must be a UUID');
+    where = { ...where, createdById };
+  }
+
+  // if a specific collection is passed then we need to check auth to view it
+  if (collectionId) {
+    ctx.assertUuid(collectionId, 'collection must be a UUID');
+
+    where = { ...where, collectionId };
+    const collection = await Collection.findById(collectionId);
+    authorize(user, 'read', collection);
+
+    // otherwise, filter by all collections the user has access to
+  } else {
+    const collectionIds = await user.collectionIds();
+    where = { ...where, collectionId: collectionIds };
+  }
+
+  // add the users starred state to the response by default
+  const starredScope = { method: ['withStarred', user.id] };
   const documents = await Document.scope('defaultScope', starredScope).findAll({
     where,
     order: [[sort, direction]],
@@ -40,16 +65,21 @@ router.post('documents.list', auth(), pagination(), async ctx => {
 });
 
 router.post('documents.pinned', auth(), pagination(), async ctx => {
-  let { sort = 'updatedAt', direction, collection } = ctx.body;
+  const { sort = 'updatedAt' } = ctx.body;
+  const collectionId = ctx.body.collection;
+  let direction = ctx.body.direction;
   if (direction !== 'ASC') direction = 'DESC';
-  ctx.assertPresent(collection, 'collection is required');
+  ctx.assertUuid(collectionId, 'collection is required');
 
   const user = ctx.state.user;
+  const collection = await Collection.findById(collectionId);
+  authorize(user, 'read', collection);
+
   const starredScope = { method: ['withStarred', user.id] };
   const documents = await Document.scope('defaultScope', starredScope).findAll({
     where: {
       teamId: user.teamId,
-      collectionId: collection,
+      collectionId,
       pinnedById: {
         // $FlowFixMe
         [Op.ne]: null,
@@ -75,6 +105,8 @@ router.post('documents.viewed', auth(), pagination(), async ctx => {
   if (direction !== 'ASC') direction = 'DESC';
 
   const user = ctx.state.user;
+  const collectionIds = await user.collectionIds();
+
   const views = await View.findAll({
     where: { userId: user.id },
     order: [[sort, direction]],
@@ -82,6 +114,9 @@ router.post('documents.viewed', auth(), pagination(), async ctx => {
       {
         model: Document,
         required: true,
+        where: {
+          collectionId: collectionIds,
+        },
         include: [
           {
             model: Star,
@@ -111,13 +146,28 @@ router.post('documents.starred', auth(), pagination(), async ctx => {
   if (direction !== 'ASC') direction = 'DESC';
 
   const user = ctx.state.user;
+  const collectionIds = await user.collectionIds();
+
   const views = await Star.findAll({
-    where: { userId: user.id },
+    where: {
+      userId: user.id,
+    },
     order: [[sort, direction]],
     include: [
       {
         model: Document,
-        include: [{ model: Star, as: 'starred', where: { userId: user.id } }],
+        where: {
+          collectionId: collectionIds,
+        },
+        include: [
+          {
+            model: Star,
+            as: 'starred',
+            where: {
+              userId: user.id,
+            },
+          },
+        ],
       },
     ],
     offset: ctx.state.pagination.offset,
@@ -139,9 +189,15 @@ router.post('documents.drafts', auth(), pagination(), async ctx => {
   if (direction !== 'ASC') direction = 'DESC';
 
   const user = ctx.state.user;
+  const collectionIds = await user.collectionIds();
+
   const documents = await Document.findAll({
-    // $FlowFixMe
-    where: { userId: user.id, publishedAt: { [Op.eq]: null } },
+    where: {
+      userId: user.id,
+      collectionId: collectionIds,
+      // $FlowFixMe
+      publishedAt: { [Op.eq]: null },
+    },
     order: [[sort, direction]],
     offset: ctx.state.pagination.offset,
     limit: ctx.state.pagination.limit,
@@ -199,6 +255,7 @@ router.post('documents.revision', auth(), async ctx => {
   let { id, revisionId } = ctx.body;
   ctx.assertPresent(id, 'id is required');
   ctx.assertPresent(revisionId, 'revisionId is required');
+
   const document = await Document.findById(id);
   authorize(ctx.state.user, 'read', document);
 
@@ -346,7 +403,6 @@ router.post('documents.unstar', auth(), async ctx => {
 router.post('documents.create', auth(), async ctx => {
   const { title, text, publish, parentDocument, index } = ctx.body;
   const collectionId = ctx.body.collection;
-  ctx.assertPresent(collectionId, 'collection is required');
   ctx.assertUuid(collectionId, 'collection must be an uuid');
   ctx.assertPresent(title, 'title is required');
   ctx.assertPresent(text, 'text is required');

server/api/documents.test.js

@@ -3,7 +3,12 @@ import TestServer from 'fetch-test-server';
 import app from '..';
 import { Document, View, Star, Revision } from '../models';
 import { flushdb, seed } from '../test/support';
-import { buildShare, buildUser, buildDocument } from '../test/factories';
+import {
+  buildShare,
+  buildCollection,
+  buildUser,
+  buildDocument,
+} from '../test/factories';
 
 const server = new TestServer(app.callback());
 
@@ -22,6 +27,18 @@ describe('#documents.info', async () => {
     expect(body.data.id).toEqual(document.id);
   });
 
+  it('should not return published document in collection not a member of', async () => {
+    const { user, document, collection } = await seed();
+    collection.private = true;
+    await collection.save();
+
+    const res = await server.post('/api/documents.info', {
+      body: { token: user.getJwtToken(), id: document.id },
+    });
+
+    expect(res.status).toEqual(403);
+  });
+
   it('should return drafts', async () => {
     const { user, document } = await seed();
     document.publishedAt = null;
@@ -36,7 +53,7 @@ describe('#documents.info', async () => {
     expect(body.data.id).toEqual(document.id);
   });
 
-  it('should return redacted document from shareId without token', async () => {
+  it('should return document from shareId without token', async () => {
     const { document } = await seed();
     const share = await buildShare({
       documentId: document.id,
@@ -141,6 +158,20 @@ describe('#documents.list', async () => {
     expect(body.data.length).toEqual(1);
   });
 
+  it('should not return documents in private collections not a member of', async () => {
+    const { user, collection } = await seed();
+    collection.private = true;
+    await collection.save();
+
+    const res = await server.post('/api/documents.list', {
+      body: { token: user.getJwtToken() },
+    });
+    const body = await res.json();
+
+    expect(res.status).toEqual(200);
+    expect(body.data.length).toEqual(0);
+  });
+
   it('should allow changing sort direction', async () => {
     const { user, document } = await seed();
     const res = await server.post('/api/documents.list', {
@@ -189,6 +220,23 @@ describe('#documents.drafts', async () => {
     expect(res.status).toEqual(200);
     expect(body.data.length).toEqual(1);
   });
+
+  it('should not return documents in private collections not a member of', async () => {
+    const { user, document, collection } = await seed();
+    document.publishedAt = null;
+    await document.save();
+
+    collection.private = true;
+    await collection.save();
+
+    const res = await server.post('/api/documents.drafts', {
+      body: { token: user.getJwtToken() },
+    });
+    const body = await res.json();
+
+    expect(res.status).toEqual(200);
+    expect(body.data.length).toEqual(0);
+  });
 });
 
 describe('#documents.revision', async () => {
@@ -208,6 +256,18 @@ describe('#documents.revision', async () => {
     expect(body.data[0].title).toEqual(document.title);
   });
 
+  it('should not return revisions for document in collection not a member of', async () => {
+    const { user, document, collection } = await seed();
+    collection.private = true;
+    await collection.save();
+
+    const res = await server.post('/api/documents.revisions', {
+      body: { token: user.getJwtToken(), id: document.id },
+    });
+
+    expect(res.status).toEqual(403);
+  });
+
   it('should require authorization', async () => {
     const { document } = await seed();
     const user = await buildUser();
@@ -296,6 +356,26 @@ describe('#documents.search', async () => {
     expect(body.data.length).toEqual(0);
   });
 
+  it('should not return documents in private collections not a member of', async () => {
+    const { user } = await seed();
+    const collection = await buildCollection({ private: true });
+
+    await buildDocument({
+      title: 'search term',
+      text: 'search term',
+      publishedAt: null,
+      teamId: user.teamId,
+      collectionId: collection.id,
+    });
+    const res = await server.post('/api/documents.search', {
+      body: { token: user.getJwtToken(), query: 'search term' },
+    });
+    const body = await res.json();
+
+    expect(res.status).toEqual(200);
+    expect(body.data.length).toEqual(0);
+  });
+
   it('should require authentication', async () => {
     const res = await server.post('/api/documents.search');
     const body = await res.json();
@@ -345,6 +425,21 @@ describe('#documents.viewed', async () => {
     expect(body.data.length).toEqual(0);
   });
 
+  it('should not return recently viewed documents in collection not a member of', async () => {
+    const { user, document, collection } = await seed();
+    await View.increment({ documentId: document.id, userId: user.id });
+    collection.private = true;
+    await collection.save();
+
+    const res = await server.post('/api/documents.viewed', {
+      body: { token: user.getJwtToken() },
+    });
+    const body = await res.json();
+
+    expect(res.status).toEqual(200);
+    expect(body.data.length).toEqual(0);
+  });
+
   it('should require authentication', async () => {
     const res = await server.post('/api/documents.viewed');
     const body = await res.json();

server/api/integrations.js

@@ -33,7 +33,7 @@ router.post('integrations.list', auth(), pagination(), async ctx => {
 
 router.post('integrations.delete', auth(), async ctx => {
   const { id } = ctx.body;
-  ctx.assertPresent(id, 'id is required');
+  ctx.assertUuid(id, 'id is required');
 
   const integration = await Integration.findById(id);
   authorize(ctx.state.user, 'delete', integration);

server/api/notificationSettings.js

@@ -44,7 +44,7 @@ router.post('notificationSettings.list', auth(), async ctx => {
 
 router.post('notificationSettings.delete', auth(), async ctx => {
   const { id } = ctx.body;
-  ctx.assertPresent(id, 'id is required');
+  ctx.assertUuid(id, 'id is required');
 
   const user = ctx.state.user;
   const setting = await NotificationSetting.findById(id);
@@ -59,7 +59,7 @@ router.post('notificationSettings.delete', auth(), async ctx => {
 
 router.post('notificationSettings.unsubscribe', async ctx => {
   const { id, token } = ctx.body;
-  ctx.assertPresent(id, 'id is required');
+  ctx.assertUuid(id, 'id is required');
   ctx.assertPresent(token, 'token is required');
 
   const setting = await NotificationSetting.findById(id);

server/api/shares.js

@@ -80,7 +80,7 @@ router.post('shares.create', auth(), async ctx => {
 
 router.post('shares.revoke', auth(), async ctx => {
   const { id } = ctx.body;
-  ctx.assertPresent(id, 'id is required');
+  ctx.assertUuid(id, 'id is required');
 
   const user = ctx.state.user;
   const share = await Share.findById(id);

server/api/views.js

@@ -10,7 +10,7 @@ const router = new Router();
 
 router.post('views.list', auth(), async ctx => {
   const { id } = ctx.body;
-  ctx.assertPresent(id, 'id is required');
+  ctx.assertUuid(id, 'id is required');
 
   const user = ctx.state.user;
   const document = await Document.findById(id);
@@ -40,7 +40,7 @@ router.post('views.list', auth(), async ctx => {
 
 router.post('views.create', auth(), async ctx => {
   const { id } = ctx.body;
-  ctx.assertPresent(id, 'id is required');
+  ctx.assertUuid(id, 'id is required');
 
   const user = ctx.state.user;
   const document = await Document.findById(id);

server/middlewares/validation.js

@@ -25,7 +25,7 @@ export default function validation() {
     };
 
     ctx.assertUuid = (value, message) => {
-      if (!validator.isUUID(value)) {
+      if (!validator.isUUID(value.toString())) {
         throw new ValidationError(message);
       }
     };

server/migrations/20181227001547-collection-permissions.js

@@ -0,0 +1,53 @@
+module.exports = {
+  up: async (queryInterface, Sequelize) => {
+    await queryInterface.createTable('collection_users', {
+      collectionId: {
+        type: Sequelize.UUID,
+        allowNull: false,
+        references: {
+          model: 'collections',
+        },
+      },
+      userId: {
+        type: Sequelize.UUID,
+        allowNull: false,
+        references: {
+          model: 'users',
+        },
+      },
+      permission: {
+        type: Sequelize.STRING,
+        allowNull: false
+      },
+      createdById: {
+        type: Sequelize.UUID,
+        allowNull: false,
+        references: {
+          model: 'users',
+        },
+      },
+      createdAt: {
+        type: Sequelize.DATE,
+        allowNull: false,
+      },
+      updatedAt: {
+        type: Sequelize.DATE,
+        allowNull: false,
+      }
+    });
+    await queryInterface.addColumn('collections', 'private', {
+      type: Sequelize.BOOLEAN,
+      allowNull: false,
+      defaultValue: false
+    });
+
+    await queryInterface.addIndex('collection_users', ['collectionId', 'userId']);
+  },
+
+  down: async (queryInterface, Sequelize) => {
+    await queryInterface.dropTable('collection_users');
+    await queryInterface.removeColumn('collections', 'private');
+
+    await queryInterface.removeIndex('collection_users', ['collectionId', 'userId']);
+  },
+};

server/models/Collection.js

@@ -6,6 +6,7 @@ import { DataTypes, sequelize } from '../sequelize';
 import { asyncLock } from '../redis';
 import events from '../events';
 import Document from './Document';
+import CollectionUser from './CollectionUser';
 import Event from './Event';
 import { welcomeMessage } from '../utils/onboarding';
 
@@ -26,6 +27,7 @@ const Collection = sequelize.define(
     name: DataTypes.STRING,
     description: DataTypes.STRING,
     color: DataTypes.STRING,
+    private: DataTypes.BOOLEAN,
     type: {
       type: DataTypes.STRING,
       validate: { isIn: allowedCollectionTypes },
@@ -85,6 +87,11 @@ Collection.associate = models => {
     foreignKey: 'collectionId',
     onDelete: 'cascade',
   });
+  Collection.belongsToMany(models.User, {
+    as: 'users',
+    through: models.CollectionUser,
+    foreignKey: 'collectionId',
+  });
   Collection.belongsTo(models.User, {
     as: 'user',
     foreignKey: 'creatorId',
@@ -92,16 +99,20 @@ Collection.associate = models => {
   Collection.belongsTo(models.Team, {
     as: 'team',
   });
-  Collection.addScope('withRecentDocuments', {
-    include: [
-      {
-        as: 'documents',
-        limit: 10,
-        model: models.Document,
-        order: [['updatedAt', 'DESC']],
-      },
-    ],
-  });
+  Collection.addScope(
+    'defaultScope',
+    {
+      include: [
+        {
+          model: models.User,
+          as: 'users',
+          through: 'collection_users',
+          paranoid: false,
+        },
+      ],
+    },
+    { override: true }
+  );
 };
 
 Collection.addHook('afterDestroy', async model => {
@@ -112,8 +123,6 @@ Collection.addHook('afterDestroy', async model => {
   });
 });
 
-// Hooks
-
 Collection.addHook('afterCreate', model =>
   events.add({ name: 'collections.create', model })
 );
@@ -126,6 +135,22 @@ Collection.addHook('afterUpdate', model =>
   events.add({ name: 'collections.update', model })
 );
 
+Collection.addHook('afterCreate', (model, options) => {
+  if (model.private) {
+    return CollectionUser.findOrCreate({
+      where: {
+        collectionId: model.id,
+        userId: model.creatorId,
+      },
+      defaults: {
+        permission: 'read_write',
+        createdById: model.creatorId,
+      },
+      transaction: options.transaction,
+    });
+  }
+});
+
 // Instance methods
 
 Collection.prototype.addDocumentToStructure = async function(

server/models/CollectionUser.js

@@ -0,0 +1,34 @@
+// @flow
+import { DataTypes, sequelize } from '../sequelize';
+
+const CollectionUser = sequelize.define(
+  'collection_user',
+  {
+    permission: {
+      type: DataTypes.STRING,
+      validate: {
+        isIn: [['read', 'read_write']],
+      },
+    },
+  },
+  {
+    timestamps: true,
+  }
+);
+
+CollectionUser.associate = models => {
+  CollectionUser.belongsTo(models.Collection, {
+    as: 'collection',
+    foreignKey: 'collectionId',
+  });
+  CollectionUser.belongsTo(models.User, {
+    as: 'user',
+    foreignKey: 'userId',
+  });
+  CollectionUser.belongsTo(models.User, {
+    as: 'createdBy',
+    foreignKey: 'createdById',
+  });
+};
+
+export default CollectionUser;

server/models/Document.js

@@ -220,7 +220,7 @@ Document.searchForUser = async (
       ts_headline('english', "text", plainto_tsquery('english', :query), 'MaxFragments=1, MinWords=20, MaxWords=30') as "searchContext"
     FROM documents
     WHERE "searchVector" @@ plainto_tsquery('english', :query) AND
-      "teamId" = '${user.teamId}'::uuid AND
+      "collectionId" IN(:collectionIds) AND
       "deletedAt" IS NULL AND
       ("publishedAt" IS NOT NULL OR "createdById" = '${user.id}')
     ORDER BY 
@@ -230,20 +230,24 @@ Document.searchForUser = async (
     OFFSET :offset;
   `;
 
+  const collectionIds = await user.collectionIds();
   const results = await sequelize.query(sql, {
     type: sequelize.QueryTypes.SELECT,
     replacements: {
       query,
       limit,
       offset,
+      collectionIds,
     },
   });
 
-  // Second query to get associated document data
+  // Final query to get associated document data
   const documents = await Document.scope({
     method: ['withViews', user.id],
   }).findAll({
-    where: { id: map(results, 'id') },
+    where: {
+      id: map(results, 'id'),
+    },
     include: [
       { model: Collection, as: 'collection' },
       { model: User, as: 'createdBy', paranoid: false },

server/models/User.js

@@ -6,7 +6,7 @@ import subMinutes from 'date-fns/sub_minutes';
 import { DataTypes, sequelize, encryptedFields } from '../sequelize';
 import { publicS3Endpoint, uploadToS3FromUrl } from '../utils/s3';
 import { sendEmail } from '../mailer';
-import { Star, NotificationSetting, ApiKey } from '.';
+import { Star, Collection, NotificationSetting, ApiKey } from '.';
 
 const User = sequelize.define(
   'user',
@@ -54,6 +54,25 @@ User.associate = models => {
 };
 
 // Instance methods
+User.prototype.collectionIds = async function() {
+  let models = await Collection.findAll({
+    attributes: ['id', 'private'],
+    where: { teamId: this.teamId },
+    include: [
+      {
+        model: User,
+        through: 'collection_users',
+        as: 'users',
+        where: { id: this.id },
+        required: false,
+      },
+    ],
+  });
+
+  // Filter collections that are private and don't have an association
+  return models.filter(c => !c.private || c.users.length).map(c => c.id);
+};
+
 User.prototype.updateActiveAt = function(ip) {
   const fiveMinutesAgo = subMinutes(new Date(), 5);
 

server/models/index.js

@@ -2,6 +2,7 @@
 import ApiKey from './ApiKey';
 import Authentication from './Authentication';
 import Collection from './Collection';
+import CollectionUser from './CollectionUser';
 import Document from './Document';
 import Event from './Event';
 import Integration from './Integration';
@@ -18,6 +19,7 @@ const models = {
   ApiKey,
   Authentication,
   Collection,
+  CollectionUser,
   Document,
   Event,
   Integration,
@@ -42,6 +44,7 @@ export {
   ApiKey,
   Authentication,
   Collection,
+  CollectionUser,
   Document,
   Event,
   Integration,

server/pages/developers/Api.js

@@ -152,11 +152,12 @@ export default function Pricing() {
 
           <Method method="collections.update" label="Update a collection">
             <Description>
-              This method allows you to modify already created document.
+              This method allows you to modify an already created collection.
             </Description>
             <Arguments>
               <Argument id="id" description="Collection ID" required />
               <Argument id="name" description="Name for the collection" />
+              <Argument id="private" description="Boolean" />
               <Argument
                 id="color"
                 description="Collection color in hex form (e.g. #E1E1E1)"
@@ -164,6 +165,45 @@ export default function Pricing() {
             </Arguments>
           </Method>
 
+          <Method method="collections.add_user" label="Add a collection member">
+            <Description>
+              This method allows you to add a user to a private collection.
+            </Description>
+            <Arguments>
+              <Argument id="id" description="Collection ID" required />
+              <Argument
+                id="userId"
+                description="User ID to add to the collection"
+              />
+            </Arguments>
+          </Method>
+
+          <Method
+            method="collections.remove_user"
+            label="Remove a collection member"
+          >
+            <Description>
+              This method allows you to remove a user from a private collection.
+            </Description>
+            <Arguments>
+              <Argument id="id" description="Collection ID" required />
+              <Argument
+                id="userId"
+                description="User ID to remove from the collection"
+              />
+            </Arguments>
+          </Method>
+
+          <Method method="collections.users" label="List collection members">
+            <Description>
+              This method allows you to list users with access to a private
+              collection.
+            </Description>
+            <Arguments>
+              <Argument id="id" description="Collection ID" required />
+            </Arguments>
+          </Method>
+
           <Method method="collections.delete" label="Delete a collection">
             <Description>
               Delete a collection and all of its documents. This action can’t be

server/policies/collection.js

@@ -1,5 +1,6 @@
 // @flow
 import policy from './policy';
+import { map } from 'lodash';
 import { Collection, User } from '../models';
 import { AdminRequiredError } from '../errors';
 
@@ -11,12 +12,27 @@ allow(
   User,
   ['read', 'publish', 'update', 'export'],
   Collection,
-  (user, collection) => collection && user.teamId === collection.teamId
+  (user, collection) => {
+    if (!collection || user.teamId !== collection.teamId) return false;
+
+    if (
+      collection.private &&
+      !map(collection.users, u => u.id).includes(user.id)
+    )
+      return false;
+
+    return true;
+  }
 );
 
 allow(User, 'delete', Collection, (user, collection) => {
   if (!collection || user.teamId !== collection.teamId) return false;
-  if (user.id === collection.creatorId) return true;
+
+  if (collection.private && !map(collection.users, u => u.id).includes(user.id))
+    return false;
+
   if (user.isAdmin) return true;
+  if (user.id === collection.creatorId) return true;
+
   throw new AdminRequiredError();
 });

server/policies/document.js

@@ -2,7 +2,7 @@
 import policy from './policy';
 import { Document, Revision, User } from '../models';
 
-const { allow } = policy;
+const { allow, authorize } = policy;
 
 allow(User, 'create', Document);
 
@@ -10,7 +10,13 @@ allow(
   User,
   ['read', 'update', 'delete', 'share'],
   Document,
-  (user, document) => user.teamId === document.teamId
+  (user, document) => {
+    if (document.collection) {
+      authorize(user, 'read', document.collection);
+    }
+
+    return user.teamId === document.teamId;
+  }
 );
 
 allow(

server/presenters/collection.js

@@ -1,6 +1,5 @@
 // @flow
 import { Collection } from '../models';
-import presentDocument from './document';
 import naturalSort from '../../shared/utils/naturalSort';
 
 type Document = {
@@ -29,9 +28,9 @@ async function present(ctx: Object, collection: Collection) {
     description: collection.description,
     color: collection.color || '#4E5C6E',
     type: collection.type,
+    private: collection.private,
     createdAt: collection.createdAt,
     updatedAt: collection.updatedAt,
-    recentDocuments: undefined,
     documents: undefined,
   };
 
@@ -40,14 +39,6 @@ async function present(ctx: Object, collection: Collection) {
     data.documents = sortDocuments(collection.documentStructure);
   }
 
-  if (collection.documents) {
-    data.recentDocuments = await Promise.all(
-      collection.documents.map(
-        async document => await presentDocument(ctx, document)
-      )
-    );
-  }
-
   return data;
 }
 

server/presenters/document.js

@@ -1,12 +1,9 @@
 // @flow
-import _ from 'lodash';
-import Sequelize from 'sequelize';
+import { takeRight } from 'lodash';
 import { User, Document } from '../models';
 import presentUser from './user';
 import presentCollection from './collection';
 
-const Op = Sequelize.Op;
-
 type Options = {
   isPublic?: boolean,
 };
@@ -43,7 +40,6 @@ async function present(ctx: Object, document: Document, options: ?Options) {
     revision: document.revisionCount,
     pinned: undefined,
     collectionId: undefined,
-    collaboratorCount: undefined,
     collection: undefined,
     views: undefined,
   };
@@ -67,14 +63,9 @@ async function present(ctx: Object, document: Document, options: ?Options) {
     // This could be further optimized by using ctx.cache
     data.collaborators = await User.findAll({
       where: {
-        id: {
-          // $FlowFixMe
-          [Op.in]: _.takeRight(document.collaboratorIds, 10) || [],
-        },
+        id: takeRight(document.collaboratorIds, 10) || [],
       },
     }).map(user => presentUser(ctx, user));
-
-    data.collaboratorCount = document.collaboratorIds.length;
   }
 
   return data;

server/services/notifications.js

@@ -77,6 +77,7 @@ export default class Notifications {
       ],
     });
     if (!collection) return;
+    if (collection.private) return;
 
     const notificationSettings = await NotificationSetting.findAll({
       where: {