feat: Add groups and group permissions (#1204)

* WIP - got one API test to pass yay

* adds group update endpoint

* added group policies

* adds groups.list API

* adds groups.info

* remove comment

* WIP

* tests for delete

* adds group membership list

* adds tests for groups list

* add and remove user endpoints for group

* ask some questions

* fix up some issues around primary keys

* remove export from group permissions

Co-Authored-By: Tom Moor <tom.moor@gmail.com>

* remove random file

* only create events on actual updates, add tests to ensure

* adds uniqueness validation to group name

* throw validation errors on model and let it pass through the controller

* fix linting

* WIP

* WIP

* WIP

* WIP

* WIP basic edit and delete

* basic CRUD for groups and memberships in place

* got member counts working

* add member count and limit the number of users sent over teh wire to 6

* factor avatar with AvatarWithPresence into its own class

* wip

* WIP avatars in group lists

* WIP collection groups

* add and remove group endpoints

* wip add collection groups

* wip get group adding to collections to work

* wip get updating collection group memberships to work

* wip get new group modal working

* add tests for collection index

* include collection groups in the withmemberships scope

* tie permissions to group memberships

* remove unused import

* Update app/components/GroupListItem.js

update title copy

Co-Authored-By: Tom Moor <tom.moor@gmail.com>

* Update server/migrations/20191211044318-create-groups.js

Co-Authored-By: Tom Moor <tom.moor@gmail.com>

* Update server/api/groups.js

Co-Authored-By: Tom Moor <tom.moor@gmail.com>

* Update server/api/groups.js

Co-Authored-By: Tom Moor <tom.moor@gmail.com>

* Update app/menus/CollectionMenu.js

Co-Authored-By: Tom Moor <tom.moor@gmail.com>

* Update server/models/Group.js

Co-Authored-By: Tom Moor <tom.moor@gmail.com>

* minor fixes

* Update app/scenes/CollectionMembers/AddGroupsToCollection.js

Co-Authored-By: Tom Moor <tom.moor@gmail.com>

* Update app/menus/GroupMenu.js

Co-Authored-By: Tom Moor <tom.moor@gmail.com>

* Update app/menus/GroupMenu.js

Co-Authored-By: Tom Moor <tom.moor@gmail.com>

* Update app/menus/GroupMenu.js

Co-Authored-By: Tom Moor <tom.moor@gmail.com>

* Update app/scenes/Collection.js

Co-Authored-By: Tom Moor <tom.moor@gmail.com>

* Update app/scenes/CollectionMembers/CollectionMembers.js

Co-Authored-By: Tom Moor <tom.moor@gmail.com>

* Update app/scenes/GroupNew.js

Co-Authored-By: Tom Moor <tom.moor@gmail.com>

* Update app/scenes/GroupNew.js

Co-Authored-By: Tom Moor <tom.moor@gmail.com>

* Update app/scenes/Settings/Groups.js

Co-Authored-By: Tom Moor <tom.moor@gmail.com>

* Update server/api/documents.js

Co-Authored-By: Tom Moor <tom.moor@gmail.com>

* Update app/scenes/CollectionMembers/components/CollectionGroupMemberListItem.js

Co-Authored-By: Tom Moor <tom.moor@gmail.com>

* address comments

* WIP - getting websocket stuff up and running

* socket event for group deletion

* wrapped up cascading deletes

* lint

* flow

* fix: UI feedback

* fix: Facepile size

* fix: Lots of missing await's

* Allow clicking facepile on group list item to open members

* remove unused route push, grammar

* fix: Remove bad analytics events
feat: Add group events to audit log

* collection. -> collections.

* Add groups to entity websocket events (sync create/update/delete) between clients

* fix: Users should not be able to see groups they are not a member of

* fix: Not caching errors in UI when changing group memberships

* fix: Hide unusable UI

* test

* fix: Tweak language

* feat: Automatically open 'add member' modal after creating group

Co-authored-by: Tom Moor <tom.moor@gmail.com>

server/models/ApiKey.js

@@ -12,6 +12,7 @@ const ApiKey = sequelize.define(
     },
     name: DataTypes.STRING,
     secret: { type: DataTypes.STRING, unique: true },
+    // TODO: remove this, as it's redundant with associate below
     userId: {
       type: DataTypes.UUID,
       allowNull: false,

server/models/Collection.js

@@ -1,5 +1,5 @@
 // @flow
-import { find, remove } from 'lodash';
+import { find, concat, remove, uniq } from 'lodash';
 import slug from 'slug';
 import randomstring from 'randomstring';
 import { DataTypes, sequelize } from '../sequelize';
@@ -59,11 +59,21 @@ Collection.associate = models => {
     foreignKey: 'collectionId',
     onDelete: 'cascade',
   });
+  Collection.hasMany(models.CollectionGroup, {
+    as: 'collectionGroupMemberships',
+    foreignKey: 'collectionId',
+    onDelete: 'cascade',
+  });
   Collection.belongsToMany(models.User, {
     as: 'users',
     through: models.CollectionUser,
     foreignKey: 'collectionId',
   });
+  Collection.belongsToMany(models.Group, {
+    as: 'groups',
+    through: models.CollectionGroup,
+    foreignKey: 'collectionId',
+  });
   Collection.belongsTo(models.User, {
     as: 'user',
     foreignKey: 'creatorId',
@@ -79,8 +89,66 @@ Collection.associate = models => {
         where: { userId },
         required: false,
       },
+      {
+        model: models.CollectionGroup,
+        as: 'collectionGroupMemberships',
+        required: false,
+
+        // use of "separate" property: sequelize breaks when there are
+        // nested "includes" with alternating values for "required"
+        // see https://github.com/sequelize/sequelize/issues/9869
+        separate: true,
+
+        // include for groups that are members of this collection,
+        // of which userId is a member of, resulting in:
+        // CollectionGroup [inner join] Group [inner join] GroupUser [where] userId
+        include: {
+          model: models.Group,
+          as: 'group',
+          required: true,
+          include: {
+            model: models.GroupUser,
+            as: 'groupMemberships',
+            required: true,
+            where: { userId },
+          },
+        },
+      },
     ],
   }));
+  Collection.addScope('withAllMemberships', {
+    include: [
+      {
+        model: models.CollectionUser,
+        as: 'memberships',
+        required: false,
+      },
+      {
+        model: models.CollectionGroup,
+        as: 'collectionGroupMemberships',
+        required: false,
+
+        // use of "separate" property: sequelize breaks when there are
+        // nested "includes" with alternating values for "required"
+        // see https://github.com/sequelize/sequelize/issues/9869
+        separate: true,
+
+        // include for groups that are members of this collection,
+        // of which userId is a member of, resulting in:
+        // CollectionGroup [inner join] Group [inner join] GroupUser [where] userId
+        include: {
+          model: models.Group,
+          as: 'group',
+          required: true,
+          include: {
+            model: models.GroupUser,
+            as: 'groupMemberships',
+            required: true,
+          },
+        },
+      },
+    ],
+  });
 };
 
 Collection.addHook('afterDestroy', async (model: Collection) => {
@@ -107,6 +175,26 @@ Collection.addHook('afterCreate', (model: Collection, options) => {
   }
 });
 
+// Class methods
+
+// get all the membership relationshps a user could have with the collection
+Collection.membershipUserIds = async (collectionId: string) => {
+  const collection = await Collection.scope('withAllMemberships').findByPk(
+    collectionId
+  );
+
+  const groupMemberships = collection.collectionGroupMemberships
+    .map(cgm => cgm.group.groupMemberships)
+    .flat();
+
+  const membershipUserIds = concat(
+    groupMemberships,
+    collection.memberships
+  ).map(membership => membership.userId);
+
+  return uniq(membershipUserIds);
+};
+
 // Instance methods
 
 Collection.prototype.addDocumentToStructure = async function(

server/models/Collection.test.js

@@ -1,6 +1,12 @@
 /* eslint-disable flowtype/require-valid-file-annotation */
 import { flushdb, seed } from '../test/support';
 import { Collection, Document } from '../models';
+import {
+  buildUser,
+  buildGroup,
+  buildCollection,
+  buildTeam,
+} from '../test/factories';
 import uuid from 'uuid';
 
 beforeEach(flushdb);
@@ -229,3 +235,44 @@ describe('#removeDocument', () => {
     expect(collectionDocuments.count).toBe(1);
   });
 });
+
+describe('#membershipUserIds', () => {
+  test('should return collection and group memberships', async () => {
+    const team = await buildTeam();
+    const teamId = team.id;
+
+    // Make 6 users
+    const users = await Promise.all(
+      Array(6)
+        .fill()
+        .map(() => {
+          return buildUser({ teamId });
+        })
+    );
+
+    const collection = await buildCollection({
+      userId: users[0].id,
+      private: true,
+      teamId,
+    });
+
+    const group1 = await buildGroup({ teamId });
+    const group2 = await buildGroup({ teamId });
+
+    const createdById = users[0].id;
+
+    await group1.addUser(users[0], { through: { createdById } });
+    await group1.addUser(users[1], { through: { createdById } });
+    await group2.addUser(users[2], { through: { createdById } });
+    await group2.addUser(users[3], { through: { createdById } });
+
+    await collection.addUser(users[4], { through: { createdById } });
+    await collection.addUser(users[5], { through: { createdById } });
+
+    await collection.addGroup(group1, { through: { createdById } });
+    await collection.addGroup(group2, { through: { createdById } });
+
+    const membershipUserIds = await Collection.membershipUserIds(collection.id);
+    expect(membershipUserIds.length).toBe(6);
+  });
+});

server/models/CollectionGroup.js

@@ -0,0 +1,38 @@
+// @flow
+import { DataTypes, sequelize } from '../sequelize';
+
+const CollectionGroup = sequelize.define(
+  'collection_group',
+  {
+    permission: {
+      type: DataTypes.STRING,
+      defaultValue: 'read_write',
+      validate: {
+        isIn: [['read', 'read_write', 'maintainer']],
+      },
+    },
+  },
+  {
+    timestamps: true,
+    paranoid: true,
+  }
+);
+
+CollectionGroup.associate = models => {
+  CollectionGroup.belongsTo(models.Collection, {
+    as: 'collection',
+    foreignKey: 'collectionId',
+    primary: true,
+  });
+  CollectionGroup.belongsTo(models.Group, {
+    as: 'group',
+    foreignKey: 'groupId',
+    primary: true,
+  });
+  CollectionGroup.belongsTo(models.User, {
+    as: 'createdBy',
+    foreignKey: 'createdById',
+  });
+};
+
+export default CollectionGroup;

server/models/Document.js

@@ -172,16 +172,10 @@ Document.associate = models => {
       return {
         include: [
           {
-            model: models.Collection,
+            model: models.Collection.scope({
+              method: ['withMembership', userId],
+            }),
             as: 'collection',
-            include: [
-              {
-                model: models.CollectionUser,
-                as: 'memberships',
-                where: { userId },
-                required: false,
-              },
-            ],
           },
         ],
       };
@@ -269,7 +263,7 @@ Document.searchForTeam = async (
       "collectionId" IN(:collectionIds) AND
       "deletedAt" IS NULL AND
       "publishedAt" IS NOT NULL
-    ORDER BY 
+    ORDER BY
       "searchRanking" DESC,
       "updatedAt" DESC
     LIMIT :limit
@@ -356,8 +350,8 @@ Document.searchForUser = async (
       options.includeDrafts
         ? '("publishedAt" IS NOT NULL OR "createdById" = :userId)'
         : '"publishedAt" IS NOT NULL'
-    } 
-  ORDER BY 
+    }
+  ORDER BY
     "searchRanking" DESC,
     "updatedAt" DESC
   LIMIT :limit

server/models/Event.js

@@ -81,10 +81,15 @@ Event.AUDIT_EVENTS = [
   'documents.delete',
   'shares.create',
   'shares.revoke',
+  'groups.create',
+  'groups.update',
+  'groups.delete',
   'collections.create',
   'collections.update',
   'collections.add_user',
   'collections.remove_user',
+  'collections.add_group',
+  'collections.remove_group',
   'collections.delete',
 ];
 

server/models/Group.js

@@ -0,0 +1,83 @@
+// @flow
+import { Op, DataTypes, sequelize } from '../sequelize';
+import { CollectionGroup, GroupUser } from '../models';
+
+const Group = sequelize.define(
+  'group',
+  {
+    id: {
+      type: DataTypes.UUID,
+      defaultValue: DataTypes.UUIDV4,
+      primaryKey: true,
+    },
+    teamId: {
+      type: DataTypes.UUID,
+      defaultValue: DataTypes.UUIDV4,
+    },
+    name: {
+      type: DataTypes.STRING,
+      allowNull: false,
+    },
+  },
+  {
+    timestamps: true,
+    paranoid: true,
+    validate: {
+      isUniqueNameInTeam: async function() {
+        const foundItem = await Group.findOne({
+          where: {
+            teamId: this.teamId,
+            name: { [Op.iLike]: this.name },
+            id: { [Op.not]: this.id },
+          },
+        });
+        if (foundItem) {
+          throw new Error('The name of this group is already in use');
+        }
+      },
+    },
+  }
+);
+
+Group.associate = models => {
+  Group.hasMany(models.GroupUser, {
+    as: 'groupMemberships',
+    foreignKey: 'groupId',
+  });
+  Group.hasMany(models.CollectionGroup, {
+    as: 'collectionGroupMemberships',
+    foreignKey: 'groupId',
+  });
+  Group.belongsTo(models.Team, {
+    as: 'team',
+    foreignKey: 'teamId',
+  });
+  Group.belongsTo(models.User, {
+    as: 'createdBy',
+    foreignKey: 'createdById',
+  });
+  Group.belongsToMany(models.User, {
+    as: 'users',
+    through: models.GroupUser,
+    foreignKey: 'groupId',
+  });
+  Group.addScope('defaultScope', {
+    include: [
+      {
+        association: 'groupMemberships',
+        required: false,
+      },
+    ],
+    order: [['name', 'ASC']],
+  });
+};
+
+// Cascade deletes to group and collection relations
+Group.addHook('afterDestroy', async (group, options) => {
+  if (!group.deletedAt) return;
+
+  await GroupUser.destroy({ where: { groupId: group.id } });
+  await CollectionGroup.destroy({ where: { groupId: group.id } });
+});
+
+export default Group;

server/models/Group.test.js

@@ -0,0 +1,48 @@
+/* eslint-disable flowtype/require-valid-file-annotation */
+import { flushdb } from '../test/support';
+import { CollectionGroup, GroupUser } from '../models';
+import { buildUser, buildGroup, buildCollection } from '../test/factories';
+
+beforeEach(flushdb);
+beforeEach(jest.resetAllMocks);
+
+describe('afterDestroy hook', () => {
+  test('should destroy associated group and collection join relations', async () => {
+    const group = await buildGroup();
+    const teamId = group.teamId;
+
+    const user1 = await buildUser({ teamId });
+    const user2 = await buildUser({ teamId });
+
+    const collection1 = await buildCollection({
+      private: true,
+      teamId,
+    });
+    const collection2 = await buildCollection({
+      private: true,
+      teamId,
+    });
+
+    const createdById = user1.id;
+
+    await group.addUser(user1, { through: { createdById } });
+    await group.addUser(user2, { through: { createdById } });
+
+    await collection1.addGroup(group, { through: { createdById } });
+    await collection2.addGroup(group, { through: { createdById } });
+
+    let collectionGroupCount = await CollectionGroup.count();
+    let groupUserCount = await GroupUser.count();
+
+    expect(collectionGroupCount).toBe(2);
+    expect(groupUserCount).toBe(2);
+
+    await group.destroy();
+
+    collectionGroupCount = await CollectionGroup.count();
+    groupUserCount = await GroupUser.count();
+
+    expect(collectionGroupCount).toBe(0);
+    expect(groupUserCount).toBe(0);
+  });
+});

server/models/GroupUser.js

@@ -0,0 +1,33 @@
+// @flow
+import { sequelize } from '../sequelize';
+
+const GroupUser = sequelize.define(
+  'group_user',
+  {},
+  {
+    timestamps: true,
+    paranoid: true,
+  }
+);
+
+GroupUser.associate = models => {
+  GroupUser.belongsTo(models.Group, {
+    as: 'group',
+    foreignKey: 'groupId',
+    primary: true,
+  });
+  GroupUser.belongsTo(models.User, {
+    as: 'user',
+    foreignKey: 'userId',
+    primary: true,
+  });
+  GroupUser.belongsTo(models.User, {
+    as: 'createdBy',
+    foreignKey: 'createdById',
+  });
+  GroupUser.addScope('defaultScope', {
+    include: [{ association: 'user' }],
+  });
+};
+
+export default GroupUser;

server/models/Team.js

@@ -11,6 +11,7 @@ import {
   RESERVED_SUBDOMAINS,
 } from '../../shared/utils/domains';
 import parseTitle from '../../shared/utils/parseTitle';
+import { ValidationError } from '../errors';
 
 import Collection from './Collection';
 import Document from './Document';
@@ -181,13 +182,13 @@ Team.prototype.removeAdmin = async function(user: User) {
   if (res.count >= 1) {
     return user.update({ isAdmin: false });
   } else {
-    throw new Error('At least one admin is required');
+    throw new ValidationError('At least one admin is required');
   }
 };
 
 Team.prototype.suspendUser = async function(user: User, admin: User) {
   if (user.id === admin.id)
-    throw new Error('Unable to suspend the current user');
+    throw new ValidationError('Unable to suspend the current user');
   return user.update({
     suspendedById: admin.id,
     suspendedAt: new Date(),

server/models/User.js

@@ -3,6 +3,7 @@ import crypto from 'crypto';
 import uuid from 'uuid';
 import JWT from 'jsonwebtoken';
 import subMinutes from 'date-fns/sub_minutes';
+import { ValidationError } from '../errors';
 import { DataTypes, sequelize, encryptedFields } from '../sequelize';
 import { publicS3Endpoint, uploadToS3FromUrl } from '../utils/s3';
 import { sendEmail } from '../mailer';
@@ -71,22 +72,22 @@ User.associate = models => {
 
 // Instance methods
 User.prototype.collectionIds = async function(paranoid: boolean = true) {
-  let models = await Collection.findAll({
+  const collectionStubs = await Collection.scope({
+    method: ['withMembership', this.id],
+  }).findAll({
     attributes: ['id', 'private'],
     where: { teamId: this.teamId },
-    include: [
-      {
-        model: User,
-        as: 'users',
-        where: { id: this.id },
-        required: false,
-      },
-    ],
     paranoid,
   });
 
-  // Filter collections that are private and don't have an association
-  return models.filter(c => !c.private || c.users.length).map(c => c.id);
+  return collectionStubs
+    .filter(
+      c =>
+        !c.private ||
+        c.memberships.length > 0 ||
+        c.collectionGroupMemberships.length > 0
+    )
+    .map(c => c.id);
 };
 
 User.prototype.updateActiveAt = function(ip) {
@@ -186,7 +187,7 @@ const checkLastAdmin = async model => {
     const adminCount = await User.count({ where: { isAdmin: true, teamId } });
 
     if (userCount > 1 && adminCount <= 1) {
-      throw new Error(
+      throw new ValidationError(
         'Cannot delete account as only admin. Please transfer admin permissions to another user and try again.'
       );
     }

server/models/index.js

@@ -5,9 +5,12 @@ import Authentication from './Authentication';
 import Backlink from './Backlink';
 import Collection from './Collection';
 import CollectionUser from './CollectionUser';
+import CollectionGroup from './CollectionGroup';
 import Document from './Document';
 import Event from './Event';
 import Integration from './Integration';
+import Group from './Group';
+import GroupUser from './GroupUser';
 import Notification from './Notification';
 import NotificationSetting from './NotificationSetting';
 import Revision from './Revision';
@@ -23,9 +26,12 @@ const models = {
   Authentication,
   Backlink,
   Collection,
+  CollectionGroup,
   CollectionUser,
   Document,
   Event,
+  Group,
+  GroupUser,
   Integration,
   Notification,
   NotificationSetting,
@@ -50,9 +56,12 @@ export {
   Authentication,
   Backlink,
   Collection,
+  CollectionGroup,
   CollectionUser,
   Document,
   Event,
+  Group,
+  GroupUser,
   Integration,
   Notification,
   NotificationSetting,