chore: Serialize domain policies on team (#1970)

* domain policies exposed on team, consistency

* fix: Remove usage of isAdmin in frontend

* test

app/components/Sidebar/Main.js

@@ -173,7 +173,7 @@ function MainSidebar() {
                 exact={false}
                 label={t("Settings")}
               />
-              {can.invite && (
+              {can.inviteUser && (
                 <SidebarLink
                   to="/settings/people"
                   onClick={handleInviteModalOpen}
@@ -183,7 +183,7 @@ function MainSidebar() {
               )}
             </Section>
           </Scrollable>
-          {can.invite && (
+          {can.inviteUser && (
             <Modal
               title={t("Invite people")}
               onRequestClose={handleInviteModalClose}

app/menus/UserMenu.js

@@ -14,9 +14,10 @@ type Props = {|
 |};
 
 function UserMenu({ user }: Props) {
-  const { users } = useStores();
+  const { users, policies } = useStores();
   const { t } = useTranslation();
   const menu = useMenuState({ modal: true });
+  const can = policies.abilities(user.id);
 
   const handlePromote = React.useCallback(
     (ev: SyntheticEvent<>) => {
@@ -98,14 +99,14 @@ function UserMenu({ user }: Props) {
                 userName: user.name,
               }),
               onClick: handleDemote,
-              visible: user.isAdmin,
+              visible: can.demote,
             },
             {
               title: t("Make {{ userName }} an admin…", {
                 userName: user.name,
               }),
               onClick: handlePromote,
-              visible: !user.isAdmin && !user.isSuspended,
+              visible: can.promote,
             },
             {
               type: "separator",

app/scenes/Settings/People.js

@@ -87,7 +87,7 @@ class People extends React.Component<Props> {
             {team.signinMethods} but haven’t signed in yet.
           </Trans>
         </HelpText>
-        {can.invite && (
+        {can.inviteUser && (
           <Button
             type="button"
             data-on="click"
@@ -116,7 +116,7 @@ class People extends React.Component<Props> {
           <Tab to="/settings/people/all" exact>
             {t("Everyone")} <Bubble count={counts.all - counts.invited} />
           </Tab>
-          {can.invite && (
+          {can.inviteUser && (
             <>
               <Separator />
               <Tab to="/settings/people/invited" exact>
@@ -137,7 +137,7 @@ class People extends React.Component<Props> {
             />
           )}
         />
-        {can.invite && (
+        {can.inviteUser && (
           <Modal
             title={t("Invite people")}
             onRequestClose={this.handleInviteModalClose}

app/scenes/Settings/Shares.js

@@ -1,10 +1,7 @@
 // @flow
-import { observer, inject } from "mobx-react";
+import { observer } from "mobx-react";
 import * as React from "react";
 import { Link } from "react-router-dom";
-import AuthStore from "stores/AuthStore";
-import SharesStore from "stores/SharesStore";
-
 import CenteredContent from "components/CenteredContent";
 import Empty from "components/Empty";
 import HelpText from "components/HelpText";
@@ -12,23 +9,19 @@ import List from "components/List";
 import PageTitle from "components/PageTitle";
 import Subheading from "components/Subheading";
 import ShareListItem from "./components/ShareListItem";
+import useCurrentTeam from "hooks/useCurrentTeam";
+import useStores from "hooks/useStores";
 
-type Props = {
+function Shares() {
-  shares: SharesStore,
+  const team = useCurrentTeam();
-  auth: AuthStore,
+  const { shares, auth, policies } = useStores();
-};
-
-@observer
-class Shares extends React.Component<Props> {
-  componentDidMount() {
-    this.props.shares.fetchPage({ limit: 100 });
-  }
-
-  render() {
-    const { shares, auth } = this.props;
-    const { user } = auth;
   const canShareDocuments = auth.team && auth.team.sharing;
   const hasSharedDocuments = shares.orderedData.length > 0;
+  const can = policies.abilities(team.id);
+
+  React.useEffect(() => {
+    shares.fetchPage({ limit: 100 });
+  }, [shares]);
 
   return (
     <CenteredContent>
@@ -39,7 +32,7 @@ class Shares extends React.Component<Props> {
         public link can access a read-only version of the document until the
         link has been revoked.
       </HelpText>
-        {user && user.isAdmin && (
+      {can.manage && (
         <HelpText>
           {!canShareDocuments && (
             <strong>Sharing is currently disabled.</strong>
@@ -61,6 +54,5 @@ class Shares extends React.Component<Props> {
     </CenteredContent>
   );
 }
-}
 
-export default inject("shares", "auth")(Shares);
+export default observer(Shares);

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

@@ -14,6 +14,20 @@ Object {
     "name": "User 1",
   },
   "ok": true,
+  "policies": Array [
+    Object {
+      "abilities": Object {
+        "activate": true,
+        "delete": true,
+        "demote": false,
+        "promote": true,
+        "read": true,
+        "suspend": true,
+        "update": false,
+      },
+      "id": "46fde1d4-0050-428f-9f0b-0bf77f4bdf61",
+    },
+  ],
   "status": 200,
 }
 `;
@@ -50,6 +64,20 @@ Object {
     "name": "User 1",
   },
   "ok": true,
+  "policies": Array [
+    Object {
+      "abilities": Object {
+        "activate": true,
+        "delete": true,
+        "demote": false,
+        "promote": true,
+        "read": true,
+        "suspend": true,
+        "update": false,
+      },
+      "id": "46fde1d4-0050-428f-9f0b-0bf77f4bdf61",
+    },
+  ],
   "status": 200,
 }
 `;
@@ -86,6 +114,20 @@ Object {
     "name": "User 1",
   },
   "ok": true,
+  "policies": Array [
+    Object {
+      "abilities": Object {
+        "activate": true,
+        "delete": true,
+        "demote": true,
+        "promote": false,
+        "read": true,
+        "suspend": true,
+        "update": false,
+      },
+      "id": "46fde1d4-0050-428f-9f0b-0bf77f4bdf61",
+    },
+  ],
   "status": 200,
 }
 `;
@@ -131,6 +173,20 @@ Object {
     "name": "User 1",
   },
   "ok": true,
+  "policies": Array [
+    Object {
+      "abilities": Object {
+        "activate": true,
+        "delete": true,
+        "demote": false,
+        "promote": false,
+        "read": true,
+        "suspend": true,
+        "update": false,
+      },
+      "id": "46fde1d4-0050-428f-9f0b-0bf77f4bdf61",
+    },
+  ],
   "status": 200,
 }
 `;

server/api/apiKeys.js

@@ -15,7 +15,7 @@ router.post("apiKeys.create", auth(), async (ctx) => {
   ctx.assertPresent(name, "name is required");
 
   const user = ctx.state.user;
-  authorize(user, "create", ApiKey);
+  authorize(user, "createApiKey", user.team);
 
   const key = await ApiKey.create({
     name,

server/api/attachments.js

@@ -26,6 +26,8 @@ router.post("attachments.create", auth(), async (ctx) => {
   ctx.assertPresent(size, "size is required");
 
   const { user } = ctx.state;
+  authorize(user, "createAttachment", user.team);
+
   const s3Key = uuid.v4();
   const acl =
     ctx.body.public === undefined

server/api/collections.js

@@ -53,7 +53,7 @@ router.post("collections.create", auth(), async (ctx) => {
   }
 
   const user = ctx.state.user;
-  authorize(user, "create", Collection);
+  authorize(user, "createCollection", user.team);
 
   const collections = await Collection.findAll({
     where: { teamId: user.teamId, deletedAt: null },
@@ -139,7 +139,7 @@ router.post("collections.import", auth(), async (ctx) => {
   ctx.assertUuid(attachmentId, "attachmentId is required");
 
   const user = ctx.state.user;
-  authorize(user, "import", Collection);
+  authorize(user, "importCollection", user.team);
 
   const attachment = await Attachment.findByPk(attachmentId);
   authorize(user, "read", attachment);

server/api/documents.js

@@ -1165,7 +1165,7 @@ router.post("documents.import", auth(), async (ctx) => {
   if (index) ctx.assertPositiveInteger(index, "index must be an integer (>=0)");
 
   const user = ctx.state.user;
-  authorize(user, "create", Document);
+  authorize(user, "createDocument", user.team);
 
   const collection = await Collection.scope({
     method: ["withMembership", user.id],
@@ -1234,7 +1234,7 @@ router.post("documents.create", auth(), async (ctx) => {
   if (index) ctx.assertPositiveInteger(index, "index must be an integer (>=0)");
 
   const user = ctx.state.user;
-  authorize(user, "create", Document);
+  authorize(user, "createDocument", user.team);
 
   const collection = await Collection.scope({
     method: ["withMembership", user.id],

server/api/events.js

@@ -2,7 +2,7 @@
 import Router from "koa-router";
 import Sequelize from "sequelize";
 import auth from "../middlewares/authentication";
-import { Event, Team, User, Collection } from "../models";
+import { Event, User, Collection } from "../models";
 import policy from "../policies";
 import { presentEvent } from "../presenters";
 import pagination from "./middlewares/pagination";
@@ -60,7 +60,7 @@ router.post("events.list", auth(), pagination(), async (ctx) => {
   }
 
   if (auditLog) {
-    authorize(user, "auditLog", Team);
+    authorize(user, "manage", user.team);
     where.name = Event.AUDIT_EVENTS;
   }
 

server/api/groups.js

@@ -76,7 +76,7 @@ router.post("groups.create", auth(), async (ctx) => {
 
   const user = ctx.state.user;
 
-  authorize(user, "create", Group);
+  authorize(user, "createGroup", user.team);
   let group = await Group.create({
     name,
     teamId: user.teamId,

server/api/notificationSettings.js

@@ -14,7 +14,7 @@ router.post("notificationSettings.create", auth(), async (ctx) => {
   ctx.assertPresent(event, "event is required");
 
   const user = ctx.state.user;
-  authorize(user, "create", NotificationSetting);
+  authorize(user, "createNotificationSetting", user.team);
 
   const [setting] = await NotificationSetting.findOrCreate({
     where: {

server/api/users.js

@@ -5,7 +5,7 @@ import userSuspender from "../commands/userSuspender";
 import auth from "../middlewares/authentication";
 import { Event, User, Team } from "../models";
 import policy from "../policies";
-import { presentUser } from "../presenters";
+import { presentUser, presentPolicies } from "../presenters";
 import { Op } from "../sequelize";
 import pagination from "./middlewares/pagination";
 
@@ -52,6 +52,7 @@ router.post("users.list", auth(), pagination(), async (ctx) => {
     data: users.map((listUser) =>
       presentUser(listUser, { includeDetails: user.isAdmin })
     ),
+    policies: presentPolicies(user, users),
   };
 });
 
@@ -67,8 +68,11 @@ router.post("users.count", auth(), async (ctx) => {
 });
 
 router.post("users.info", auth(), async (ctx) => {
+  const { user } = ctx.state;
+
   ctx.body = {
-    data: presentUser(ctx.state.user),
+    data: presentUser(user),
+    policies: presentPolicies(user, [user]),
   };
 });
 
@@ -100,17 +104,18 @@ router.post("users.update", auth(), async (ctx) => {
 router.post("users.promote", auth(), async (ctx) => {
   const userId = ctx.body.id;
   const teamId = ctx.state.user.teamId;
+  const actor = ctx.state.user;
   ctx.assertPresent(userId, "id is required");
 
   const user = await User.findByPk(userId);
-  authorize(ctx.state.user, "promote", user);
+  authorize(actor, "promote", user);
 
   const team = await Team.findByPk(teamId);
   await team.addAdmin(user);
 
   await Event.create({
     name: "users.promote",
-    actorId: ctx.state.user.id,
+    actorId: actor.id,
     userId,
     teamId,
     data: { name: user.name },
@@ -119,23 +124,25 @@ router.post("users.promote", auth(), async (ctx) => {
 
   ctx.body = {
     data: presentUser(user, { includeDetails: true }),
+    policies: presentPolicies(actor, [user]),
   };
 });
 
 router.post("users.demote", auth(), async (ctx) => {
   const userId = ctx.body.id;
   const teamId = ctx.state.user.teamId;
+  const actor = ctx.state.user;
   ctx.assertPresent(userId, "id is required");
 
   const user = await User.findByPk(userId);
-  authorize(ctx.state.user, "demote", user);
+  authorize(actor, "demote", user);
 
   const team = await Team.findByPk(teamId);
   await team.removeAdmin(user);
 
   await Event.create({
     name: "users.demote",
-    actorId: ctx.state.user.id,
+    actorId: actor.id,
     userId,
     teamId,
     data: { name: user.name },
@@ -144,42 +151,45 @@ router.post("users.demote", auth(), async (ctx) => {
 
   ctx.body = {
     data: presentUser(user, { includeDetails: true }),
+    policies: presentPolicies(actor, [user]),
   };
 });
 
 router.post("users.suspend", auth(), async (ctx) => {
   const userId = ctx.body.id;
+  const actor = ctx.state.user;
   ctx.assertPresent(userId, "id is required");
 
   const user = await User.findByPk(userId);
-  authorize(ctx.state.user, "suspend", user);
+  authorize(actor, "suspend", user);
 
   await userSuspender({
     user,
-    actorId: ctx.state.user.id,
+    actorId: actor.id,
     ip: ctx.request.ip,
   });
 
   ctx.body = {
     data: presentUser(user, { includeDetails: true }),
+    policies: presentPolicies(actor, [user]),
   };
 });
 
 router.post("users.activate", auth(), async (ctx) => {
-  const admin = ctx.state.user;
   const userId = ctx.body.id;
   const teamId = ctx.state.user.teamId;
+  const actor = ctx.state.user;
   ctx.assertPresent(userId, "id is required");
 
   const user = await User.findByPk(userId);
-  authorize(ctx.state.user, "activate", user);
+  authorize(actor, "activate", user);
 
   const team = await Team.findByPk(teamId);
-  await team.activateUser(user, admin);
+  await team.activateUser(user, actor);
 
   await Event.create({
     name: "users.activate",
-    actorId: ctx.state.user.id,
+    actorId: actor.id,
     userId,
     teamId,
     data: { name: user.name },
@@ -188,6 +198,7 @@ router.post("users.activate", auth(), async (ctx) => {
 
   ctx.body = {
     data: presentUser(user, { includeDetails: true }),
+    policies: presentPolicies(actor, [user]),
   };
 });
 

server/models/ApiKey.js

@@ -3,7 +3,7 @@ import randomstring from "randomstring";
 import { DataTypes, sequelize } from "../sequelize";
 
 const ApiKey = sequelize.define(
-  "apiKeys",
+  "apiKey",
   {
     id: {
       type: DataTypes.UUID,
@@ -12,17 +12,8 @@ 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,
-      references: {
-        model: "users",
-      },
-    },
   },
   {
-    tableName: "apiKeys",
     paranoid: true,
     hooks: {
       beforeValidate: (key) => {

server/policies/apiKey.js

@@ -1,10 +1,13 @@
 // @flow
-import { ApiKey, User } from "../models";
+import { ApiKey, User, Team } from "../models";
 import policy from "./policy";
 
 const { allow } = policy;
 
-allow(User, "create", ApiKey);
+allow(User, "createApiKey", Team, (user, team) => {
+  if (!team || user.teamId !== team.id) return false;
+  return true;
+});
 
 allow(
   User,

server/policies/attachment.js

@@ -1,10 +1,13 @@
 // @flow
-import { Attachment, User } from "../models";
+import { Attachment, User, Team } from "../models";
 import policy from "./policy";
 
 const { allow } = policy;
 
-allow(User, "create", Attachment);
+allow(User, "createAttachment", Team, (user, team) => {
+  if (!team || user.teamId !== team.id) return false;
+  return true;
+});
 
 allow(User, ["read", "delete"], Attachment, (actor, attachment) => {
   if (!attachment || attachment.teamId !== actor.teamId) return false;

server/policies/collection.js

@@ -2,14 +2,18 @@
 import invariant from "invariant";
 import { concat, some } from "lodash";
 import { AdminRequiredError } from "../errors";
-import { Collection, User } from "../models";
+import { Collection, User, Team } from "../models";
 import policy from "./policy";
 
 const { allow } = policy;
 
-allow(User, "create", Collection);
+allow(User, "createCollection", Team, (user, team) => {
+  if (!team || user.teamId !== team.id) return false;
+  return true;
+});
 
-allow(User, "import", Collection, (actor) => {
+allow(User, "importCollection", Team, (actor, team) => {
+  if (!team || actor.teamId !== team.id) return false;
   if (actor.isAdmin) return true;
   throw new AdminRequiredError();
 });

server/policies/document.js

@@ -1,11 +1,14 @@
 // @flow
 import invariant from "invariant";
-import { Document, Revision, User } from "../models";
+import { Document, Revision, User, Team } from "../models";
 import policy from "./policy";
 
 const { allow, cannot } = policy;
 
-allow(User, "create", Document);
+allow(User, "createDocument", Team, (user, team) => {
+  if (!team || user.teamId !== team.id) return false;
+  return true;
+});
 
 allow(User, ["read", "download"], Document, (user, document) => {
   // existence of collection option is not required here to account for share tokens

server/policies/group.js

@@ -1,22 +1,17 @@
 // @flow
 import { AdminRequiredError } from "../errors";
-import { Group, User } from "../models";
+import { Group, User, Team } from "../models";
 import policy from "./policy";
 
 const { allow } = policy;
 
-allow(User, ["create"], Group, (actor) => {
+allow(User, "createGroup", Team, (actor, team) => {
+  if (!team || actor.teamId !== team.id) return false;
   if (actor.isAdmin) return true;
   throw new AdminRequiredError();
 });
 
-allow(User, ["update", "delete"], Group, (actor, group) => {
+allow(User, "read", Group, (actor, group) => {
-  if (!group || actor.teamId !== group.teamId) return false;
-  if (actor.isAdmin) return true;
-  throw new AdminRequiredError();
-});
-
-allow(User, ["read"], Group, (actor, group) => {
   if (!group || actor.teamId !== group.teamId) return false;
   if (actor.isAdmin) return true;
   if (group.groupMemberships.filter((gm) => gm.userId === actor.id).length) {
@@ -24,3 +19,9 @@ allow(User, ["read"], Group, (actor, group) => {
   }
   return false;
 });
+
+allow(User, ["update", "delete"], Group, (actor, group) => {
+  if (!group || actor.teamId !== group.teamId) return false;
+  if (actor.isAdmin) return true;
+  throw new AdminRequiredError();
+});

server/policies/index.test.js

@@ -1,5 +1,5 @@
-/* eslint-disable flowtype/require-valid-file-annotation */
+// @flow
-import { buildUser } from "../test/factories";
+import { buildUser, buildTeam } from "../test/factories";
 import { flushdb } from "../test/support";
 import { serialize } from "./index";
 
@@ -11,3 +11,11 @@ it("should serialize policy", async () => {
   expect(response.update).toEqual(true);
   expect(response.delete).toEqual(true);
 });
+
+it("should serialize domain policies on Team", async () => {
+  const team = await buildTeam();
+  const user = await buildUser({ teamId: team.id });
+  const response = serialize(user, team);
+  expect(response.createDocument).toEqual(true);
+  expect(response.inviteUser).toEqual(false);
+});

server/policies/integration.js

@@ -1,11 +1,15 @@
 // @flow
 import { AdminRequiredError } from "../errors";
-import { Integration, User } from "../models";
+import { Integration, User, Team } from "../models";
 import policy from "./policy";
 
 const { allow } = policy;
 
-allow(User, "create", Integration);
+allow(User, "createIntegration", Team, (actor, team) => {
+  if (!team || actor.teamId !== team.id) return false;
+  if (actor.isAdmin) return true;
+  throw new AdminRequiredError();
+});
 
 allow(
   User,

server/policies/notificationSetting.js

@@ -1,10 +1,13 @@
 // @flow
-import { NotificationSetting, User } from "../models";
+import { NotificationSetting, Team, User } from "../models";
 import policy from "./policy";
 
 const { allow } = policy;
 
-allow(User, "create", NotificationSetting);
+allow(User, "createNotificationSetting", Team, (user, team) => {
+  if (!team || user.teamId !== team.id) return false;
+  return true;
+});
 
 allow(
   User,

server/policies/team.js

@@ -1,5 +1,4 @@
 // @flow
-import { AdminRequiredError } from "../errors";
 import { Team, User } from "../models";
 import policy from "./policy";
 
@@ -12,24 +11,7 @@ allow(User, "share", Team, (user, team) => {
   return team.sharing;
 });
 
-allow(User, "auditLog", Team, (user) => {
+allow(User, ["update", "export", "manage"], Team, (user, team) => {
-  if (user.isAdmin) return true;
-  return false;
-});
-
-allow(User, "invite", Team, (user) => {
-  if (user.isAdmin) return true;
-  return false;
-});
-
-// ??? policy for creating new groups, I don't know how to do this other than on the team level
-allow(User, "group", Team, (user) => {
-  if (user.isAdmin) return true;
-  throw new AdminRequiredError();
-});
-
-allow(User, ["update", "export"], Team, (user, team) => {
   if (!team || user.teamId !== team.id) return false;
-  if (user.isAdmin) return true;
+  return user.isAdmin;
-  throw new AdminRequiredError();
 });

server/policies/team.test.js

@@ -0,0 +1,34 @@
+// @flow
+import { buildUser, buildTeam, buildAdmin } from "../test/factories";
+import { flushdb } from "../test/support";
+import { serialize } from "./index";
+
+beforeEach(() => flushdb());
+
+it("should allow reading only", async () => {
+  const team = await buildTeam();
+  const user = await buildUser({ teamId: team.id });
+  const abilities = serialize(user, team);
+  expect(abilities.read).toEqual(true);
+  expect(abilities.manage).toEqual(false);
+
+  expect(abilities.createAttachment).toEqual(true);
+  expect(abilities.createCollection).toEqual(true);
+  expect(abilities.createDocument).toEqual(true);
+  expect(abilities.createGroup).toEqual(false);
+  expect(abilities.createIntegration).toEqual(false);
+});
+
+it("should allow admins to manage", async () => {
+  const team = await buildTeam();
+  const admin = await buildAdmin({ teamId: team.id });
+  const abilities = serialize(admin, team);
+  expect(abilities.read).toEqual(true);
+  expect(abilities.manage).toEqual(true);
+
+  expect(abilities.createAttachment).toEqual(true);
+  expect(abilities.createCollection).toEqual(true);
+  expect(abilities.createDocument).toEqual(true);
+  expect(abilities.createGroup).toEqual(true);
+  expect(abilities.createIntegration).toEqual(true);
+});

server/policies/user.js

@@ -1,6 +1,6 @@
 // @flow
 import { AdminRequiredError } from "../errors";
-import { User } from "../models";
+import { User, Team } from "../models";
 import policy from "./policy";
 
 const { allow } = policy;
@@ -12,8 +12,10 @@ allow(
   (actor, user) => user && user.teamId === actor.teamId
 );
 
-allow(User, "invite", User, (actor) => {
+allow(User, "inviteUser", Team, (actor, team) => {
-  return true;
+  if (!team || actor.teamId !== team.id) return false;
+  if (actor.isAdmin) return true;
+  throw new AdminRequiredError();
 });
 
 allow(User, "update", User, (actor, user) => {
@@ -29,13 +31,22 @@ allow(User, "delete", User, (actor, user) => {
   throw new AdminRequiredError();
 });
 
-allow(
+allow(User, ["activate", "suspend"], User, (actor, user) => {
-  User,
-  ["promote", "demote", "activate", "suspend"],
-  User,
-  (actor, user) => {
   if (!user || user.teamId !== actor.teamId) return false;
   if (actor.isAdmin) return true;
   throw new AdminRequiredError();
-  }
+});
-);
+
+allow(User, "promote", User, (actor, user) => {
+  if (!user || user.teamId !== actor.teamId) return false;
+  if (user.isAdmin || user.isSuspended) return false;
+  if (actor.isAdmin) return true;
+  throw new AdminRequiredError();
+});
+
+allow(User, "demote", User, (actor, user) => {
+  if (!user || user.teamId !== actor.teamId) return false;
+  if (!user.isAdmin || user.isSuspended) return false;
+  if (actor.isAdmin) return true;
+  throw new AdminRequiredError();
+});

server/test/factories.js

@@ -95,6 +95,10 @@ export async function buildUser(overrides: Object = {}) {
   );
 }
 
+export async function buildAdmin(overrides: Object = {}) {
+  return buildUser({ ...overrides, isAdmin: true });
+}
+
 export async function buildInvite(overrides: Object = {}) {
   if (!overrides.teamId) {
     const team = await buildTeam();