Policies refactor, guest roles (#6732)

app/actions/definitions/documents.tsx

@@ -432,7 +432,8 @@ export const copyDocumentAsMarkdown = createAction({
   name: ({ t }) => t("Copy as Markdown"),
   section: DocumentSection,
   keywords: "clipboard",
-  visible: ({ activeDocumentId }) => !!activeDocumentId,
+  visible: ({ activeDocumentId, stores }) =>
+    !!activeDocumentId && stores.policies.abilities(activeDocumentId).download,
   perform: ({ stores, activeDocumentId, t }) => {
     const document = activeDocumentId
       ? stores.documents.get(activeDocumentId)
@@ -856,7 +857,7 @@ export const openDocumentHistory = createAction({
   icon: <HistoryIcon />,
   visible: ({ activeDocumentId, stores }) => {
     const can = stores.policies.abilities(activeDocumentId ?? "");
-    return !!activeDocumentId && can.read && !can.restore;
+    return !!activeDocumentId && can.listRevisions;
   },
   perform: ({ activeDocumentId, stores }) => {
     if (!activeDocumentId) {
@@ -883,7 +884,7 @@ export const openDocumentInsights = createAction({
 
     return (
       !!activeDocumentId &&
-      can.read &&
+      can.listViews &&
       !document?.isTemplate &&
       !document?.isDeleted
     );

app/components/AuthenticatedLayout.tsx

@@ -47,7 +47,8 @@ const AuthenticatedLayout: React.FC = ({ children }: Props) => {
   const { ui, auth } = useStores();
   const location = useLocation();
   const layoutRef = React.useRef<HTMLDivElement>(null);
-  const can = usePolicy(ui.activeCollectionId);
+  const can = usePolicy(ui.activeDocumentId);
+  const canCollection = usePolicy(ui.activeCollectionId);
   const team = useCurrentTeam();
   const documentContext = useLocalStore<DocumentContextValue>(() => ({
     editor: null,
@@ -69,7 +70,7 @@ const AuthenticatedLayout: React.FC = ({ children }: Props) => {
       return;
     }
     const { activeCollectionId } = ui;
-    if (!activeCollectionId || !can.createDocument) {
+    if (!activeCollectionId || !canCollection.createDocument) {
       return;
     }
     history.push(newDocumentPath(activeCollectionId));
@@ -88,15 +89,18 @@ const AuthenticatedLayout: React.FC = ({ children }: Props) => {
     </Fade>
   );
 
-  const showHistory = !!matchPath(location.pathname, {
-    path: matchDocumentHistory,
-  });
-  const showInsights = !!matchPath(location.pathname, {
-    path: matchDocumentInsights,
-  });
+  const showHistory =
+    !!matchPath(location.pathname, {
+      path: matchDocumentHistory,
+    }) && can.listRevisions;
+  const showInsights =
+    !!matchPath(location.pathname, {
+      path: matchDocumentInsights,
+    }) && can.listViews;
   const showComments =
     !showInsights &&
     !showHistory &&
+    can.comment &&
     ui.activeDocumentId &&
     ui.commentsExpanded.includes(ui.activeDocumentId) &&
     team.getPreference(TeamPreference.Commenting);

app/hooks/useSettingsConfig.ts

@@ -25,6 +25,7 @@ import isCloudHosted from "~/utils/isCloudHosted";
 import lazy from "~/utils/lazyWithRetry";
 import { settingsPath } from "~/utils/routeHelpers";
 import useCurrentTeam from "./useCurrentTeam";
+import useCurrentUser from "./useCurrentUser";
 import usePolicy from "./usePolicy";
 
 const ApiKeys = lazy(() => import("~/scenes/Settings/ApiKeys"));
@@ -54,6 +55,7 @@ export type ConfigItem = {
 };
 
 const useSettingsConfig = () => {
+  const user = useCurrentUser();
   const team = useCurrentTeam();
   const can = usePolicy(team);
   const { t } = useTranslation();
@@ -122,7 +124,7 @@ const useSettingsConfig = () => {
         name: t("Members"),
         path: settingsPath("members"),
         component: Members,
-        enabled: true,
+        enabled: can.listUsers,
         group: t("Workspace"),
         icon: UserIcon,
       },
@@ -130,7 +132,7 @@ const useSettingsConfig = () => {
         name: t("Groups"),
         path: settingsPath("groups"),
         component: Groups,
-        enabled: true,
+        enabled: can.listGroups,
         group: t("Workspace"),
         icon: GroupIcon,
       },
@@ -138,7 +140,7 @@ const useSettingsConfig = () => {
         name: t("Templates"),
         path: settingsPath("templates"),
         component: Templates,
-        enabled: true,
+        enabled: can.update,
         group: t("Workspace"),
         icon: ShapesIcon,
       },
@@ -146,7 +148,7 @@ const useSettingsConfig = () => {
         name: t("Shared Links"),
         path: settingsPath("shares"),
         component: Shares,
-        enabled: true,
+        enabled: can.listShares,
         group: t("Workspace"),
         icon: GlobeIcon,
       },
@@ -211,7 +213,7 @@ const useSettingsConfig = () => {
         enabled:
           enabledInDeployment &&
           hasSettings &&
-          (plugin.config.adminOnly === false || can.update),
+          (plugin.config.roles?.includes(user.role) || can.update),
         icon: plugin.icon,
       } as ConfigItem;
 

app/menus/DocumentMenu.tsx

@@ -312,7 +312,7 @@ function DocumentMenu({
             actionToMenuItem(permanentlyDeleteDocument, context),
           ]}
         />
-        {(showDisplayOptions || showToggleEmbeds) && (
+        {(showDisplayOptions || showToggleEmbeds) && can.update && (
           <>
             <Separator />
             <DisplayOptions>
@@ -332,7 +332,7 @@ function DocumentMenu({
                   />
                 </Style>
               )}
-              {showDisplayOptions && !isMobile && can.update && (
+              {showDisplayOptions && !isMobile && (
                 <Style>
                   <ToggleMenuItem
                     width={26}

app/models/User.ts

@@ -79,6 +79,13 @@ class User extends ParanoidModel {
     return this.role === UserRole.Admin;
   }
 
+  /**
+   * Whether the user is a member (editor).
+   */
+  get isMember(): boolean {
+    return this.role === UserRole.Member;
+  }
+
   /**
    * Whether the user is a viewer.
    */
@@ -86,6 +93,13 @@ class User extends ParanoidModel {
     return this.role === UserRole.Viewer;
   }
 
+  /**
+   * Whether the user is a guest.
+   */
+  get isGuest(): boolean {
+    return this.role === UserRole.Guest;
+  }
+
   /**
    * Whether the user has been recently active. Recently is currently defined
    * as within the last 5 minutes.

app/models/base/Model.ts

@@ -119,7 +119,6 @@ export default abstract class Model {
       try {
         this[key] = data[key];
       } catch (error) {
-        // Temporary as isViewer and isAdmin properties changed to getters
         Logger.warn(`Error setting ${key} on model`, error);
       }
     }

app/scenes/Document/components/DocumentMeta.tsx

@@ -55,7 +55,10 @@ function TitleDocumentMeta({ to, document, revision, ...rest }: Props) {
           </CommentLink>
         </>
       )}
-      {totalViewers && !document.isDraft && !document.isTemplate ? (
+      {totalViewers &&
+      can.listViews &&
+      !document.isDraft &&
+      !document.isTemplate ? (
         <Wrapper>
           &nbsp;•&nbsp;
           <Link

app/scenes/Document/components/Header.tsx

@@ -248,7 +248,9 @@ function DocumentHeader({
             {!isPublishing && isSaving && user?.separateEditMode && (
               <Status>{t("Saving")}…</Status>
             )}
-            {!isDeleted && !isRevision && <Collaborators document={document} />}
+            {!isDeleted && !isRevision && can.listViews && (
+              <Collaborators document={document} />
+            )}
             {(isEditing || !user?.separateEditMode) && !isTemplate && isNew && (
               <Action>
                 <TemplatesMenu

app/scenes/Settings/components/PeopleTable.tsx

@@ -62,6 +62,8 @@ function PeopleTable({ canManage, ...rest }: Props) {
                 <Badge primary>{t("Admin")}</Badge>
               ) : row.original.isViewer ? (
                 <Badge>{t("Viewer")}</Badge>
+              ) : row.original.isGuest ? (
+                <Badge yellow>{t("Guest")}</Badge>
               ) : (
                 <Badge>{t("Editor")}</Badge>
               )}

app/utils/PluginLoader.ts

@@ -1,11 +1,12 @@
 import React from "react";
+import { UserRole } from "@shared/types";
 
 interface Plugin {
   id: string;
   config: {
     name: string;
     description: string;
-    adminOnly?: boolean;
+    roles?: UserRole[];
     deployments?: string[];
   };
   settings: React.FC;

plugins/slack/plugin.json

@@ -2,6 +2,6 @@
   "id": "slack",
   "name": "Slack",
   "priority": 40,
-  "adminOnly": false,
+  "roles": ["admin", "member"],
   "description": "Adds a Slack authentication provider, support for the /outline slash command, and link unfurling."
 }

server/models/Share.ts

@@ -59,6 +59,13 @@ import Length from "./validators/Length";
             }),
             as: "collection",
           },
+          {
+            association: "memberships",
+            where: {
+              userId,
+            },
+            required: false,
+          },
         ],
       },
       {

server/models/User.ts

@@ -238,18 +238,41 @@ class User extends ParanoidModel<
     return !!this.suspendedAt || !!this.team?.isSuspended;
   }
 
+  /**
+   * Whether the user has been invited but not yet signed in.
+   */
   get isInvited() {
     return !this.lastActiveAt;
   }
 
+  /**
+   * Whether the user is an admin.
+   */
   get isAdmin() {
     return this.role === UserRole.Admin;
   }
 
+  /**
+   * Whether the user is a member (editor).
+   */
+  get isMember() {
+    return this.role === UserRole.Member;
+  }
+
+  /**
+   * Whether the user is a viewer.
+   */
   get isViewer() {
     return this.role === UserRole.Viewer;
   }
 
+  /**
+   * Whether the user is a guest.
+   */
+  get isGuest() {
+    return this.role === UserRole.Guest;
+  }
+
   get color() {
     return stringToColor(this.id);
   }
@@ -404,9 +427,10 @@ class User extends ParanoidModel<
     return collectionStubs
       .filter(
         (c) =>
-          Object.values(CollectionPermission).includes(
+          (Object.values(CollectionPermission).includes(
             c.permission as CollectionPermission
-          ) ||
+          ) &&
+            !this.isGuest) ||
           c.memberships.length > 0 ||
           c.collectionGroupMemberships.length > 0
       )

server/models/base/ParanoidModel.ts

@@ -8,6 +8,15 @@ class ParanoidModel<
 > extends IdModel<TModelAttributes, TCreationAttributes> {
   @DeletedAt
   deletedAt: Date | null;
+
+  /**
+   * Whether the model has been deleted.
+   *
+   * @returns True if the model has been deleted
+   */
+  get isDeleted() {
+    return !!this.deletedAt;
+  }
 }
 
 export default ParanoidModel;

server/policies/apiKey.ts

@@ -1,19 +1,15 @@
 import { ApiKey, User, Team } from "@server/models";
 import { allow } from "./cancan";
+import { and, isOwner, isTeamModel, isTeamMutable } from "./utils";
 
-allow(User, "createApiKey", Team, (user, team) => {
-  if (!team || user.isViewer || user.teamId !== team.id) {
-    return false;
-  }
-  return true;
-});
+allow(User, "createApiKey", Team, (actor, team) =>
+  and(
+    //
+    isTeamModel(actor, team),
+    isTeamMutable(actor),
+    !actor.isViewer,
+    !actor.isGuest
+  )
+);
 
-allow(User, ["read", "update", "delete"], ApiKey, (user, apiKey) => {
-  if (!apiKey) {
-    return false;
-  }
-  if (user.isViewer) {
-    return false;
-  }
-  return user && user.id === apiKey.userId;
-});
+allow(User, ["read", "update", "delete"], ApiKey, isOwner);

server/policies/attachment.ts

@@ -1,38 +1,12 @@
 import { Attachment, User, Team } from "@server/models";
 import { allow } from "./cancan";
+import { and, isOwner, isTeamModel, or } from "./utils";
 
-allow(User, "createAttachment", Team, (user, team) => {
-  if (!team || user.isViewer || user.teamId !== team.id) {
-    return false;
-  }
-  return true;
-});
+allow(User, "createAttachment", Team, isTeamModel);
 
-allow(User, "read", Attachment, (actor, attachment) => {
-  if (!attachment || !actor || attachment.teamId !== actor.teamId) {
-    return false;
-  }
-  if (actor.isAdmin) {
-    return true;
-  }
-  if (actor.id === attachment.userId) {
-    return true;
-  }
-  return false;
-});
-
-allow(User, "delete", Attachment, (actor, attachment) => {
-  if (actor.isViewer) {
-    return false;
-  }
-  if (!attachment || attachment.teamId !== actor.teamId) {
-    return false;
-  }
-  if (actor.isAdmin) {
-    return true;
-  }
-  if (actor.id === attachment.userId) {
-    return true;
-  }
-  return false;
-});
+allow(User, ["read", "update", "delete"], Attachment, (actor, attachment) =>
+  and(
+    isTeamModel(actor, attachment),
+    or(actor.isAdmin, isOwner(actor, attachment))
+  )
+);

server/policies/authenticationProvider.ts

@@ -1,40 +1,9 @@
 import { AuthenticationProvider, User, Team } from "@server/models";
-import { AdminRequiredError } from "../errors";
 import { allow } from "./cancan";
+import { isTeamAdmin, isTeamModel } from "./utils";
 
-allow(User, "createAuthenticationProvider", Team, (actor, team) => {
-  if (!team || actor.teamId !== team.id) {
-    return false;
-  }
-  if (actor.isAdmin) {
-    return true;
-  }
+allow(User, "createAuthenticationProvider", Team, isTeamAdmin);
 
-  throw AdminRequiredError();
-});
+allow(User, "read", AuthenticationProvider, isTeamModel);
 
-allow(
-  User,
-  "read",
-  AuthenticationProvider,
-
-  (actor, authenticationProvider) =>
-    actor && actor.teamId === authenticationProvider?.teamId
-);
-
-allow(
-  User,
-  ["update", "delete"],
-  AuthenticationProvider,
-
-  (actor, authenticationProvider) => {
-    if (actor.teamId !== authenticationProvider?.teamId) {
-      return false;
-    }
-    if (actor.isAdmin) {
-      return true;
-    }
-
-    throw AdminRequiredError();
-  }
-);
+allow(User, ["update", "delete"], AuthenticationProvider, isTeamAdmin);

server/policies/collection.test.ts

@@ -24,6 +24,7 @@ describe("admin", () => {
     }).findByPk(collection.id);
     const abilities = serialize(user, reloaded);
     expect(abilities.readDocument).toEqual(false);
+    expect(abilities.updateDocument).toEqual(false);
     expect(abilities.createDocument).toEqual(false);
     expect(abilities.share).toEqual(false);
     expect(abilities.read).toEqual(false);
@@ -41,6 +42,7 @@ describe("admin", () => {
     });
     const abilities = serialize(user, collection);
     expect(abilities.readDocument).toEqual(true);
+    expect(abilities.updateDocument).toEqual(true);
     expect(abilities.createDocument).toEqual(true);
     expect(abilities.share).toEqual(true);
     expect(abilities.read).toEqual(true);
@@ -72,8 +74,8 @@ describe("member", () => {
       const abilities = serialize(user, reloaded);
       expect(abilities.read).toEqual(true);
       expect(abilities.readDocument).toEqual(true);
-      expect(abilities.createDocument).toEqual(true);
-      expect(abilities.share).toEqual(true);
+      // expect(abilities.createDocument).toEqual(true);
+      // expect(abilities.share).toEqual(true);
       expect(abilities.update).toEqual(true);
     });
   });
@@ -336,3 +338,53 @@ describe("viewer", () => {
     });
   });
 });
+
+describe("guest", () => {
+  describe("read_write permission", () => {
+    it("should allow no permissions for guest", async () => {
+      const team = await buildTeam();
+      const user = await buildUser({
+        role: UserRole.Guest,
+        teamId: team.id,
+      });
+      const collection = await buildCollection({
+        teamId: team.id,
+        permission: CollectionPermission.ReadWrite,
+      });
+      const abilities = serialize(user, collection);
+      expect(abilities.read).toEqual(false);
+      expect(abilities.readDocument).toEqual(false);
+      expect(abilities.createDocument).toEqual(false);
+      expect(abilities.update).toEqual(false);
+      expect(abilities.share).toEqual(false);
+    });
+  });
+
+  it("should allow override with team member membership permission", async () => {
+    const team = await buildTeam();
+    const user = await buildUser({
+      role: UserRole.Guest,
+      teamId: team.id,
+    });
+    const collection = await buildCollection({
+      teamId: team.id,
+      permission: null,
+    });
+    await UserMembership.create({
+      createdById: user.id,
+      collectionId: collection.id,
+      userId: user.id,
+      permission: CollectionPermission.Read,
+    });
+    // reload to get membership
+    const reloaded = await Collection.scope({
+      method: ["withMembership", user.id],
+    }).findByPk(collection.id);
+    const abilities = serialize(user, reloaded);
+    expect(abilities.read).toEqual(true);
+    expect(abilities.readDocument).toEqual(true);
+    expect(abilities.createDocument).toEqual(false);
+    expect(abilities.share).toEqual(false);
+    expect(abilities.update).toEqual(false);
+  });
+});

server/policies/collection.ts

@@ -2,70 +2,72 @@ import invariant from "invariant";
 import some from "lodash/some";
 import { CollectionPermission, DocumentPermission } from "@shared/types";
 import { Collection, User, Team } from "@server/models";
-import { AdminRequiredError } from "../errors";
-import { allow } from "./cancan";
+import { allow, _can as can } from "./cancan";
+import { and, isTeamAdmin, isTeamModel, isTeamMutable, or } from "./utils";
+
+allow(User, "createCollection", Team, (actor, team) =>
+  and(
+    isTeamModel(actor, team),
+    isTeamMutable(actor),
+    !actor.isGuest,
+    !actor.isViewer,
+    or(actor.isAdmin, !!team?.memberCollectionCreate)
+  )
+);
+
+allow(User, "importCollection", Team, (actor, team) =>
+  and(
+    //
+    isTeamAdmin(actor, team),
+    isTeamMutable(actor)
+  )
+);
+
+allow(User, "move", Collection, (actor, collection) =>
+  and(
+    //
+    isTeamAdmin(actor, collection),
+    isTeamMutable(actor),
+    !collection?.deletedAt
+  )
+);
+
+allow(
+  User,
+  ["read", "readDocument", "star", "unstar"],
+  Collection,
+  (user, collection) => {
+    if (!collection || user.teamId !== collection.teamId) {
+      return false;
+    }
+
+    if (collection.isPrivate || user.isGuest) {
+      return includesMembership(
+        collection,
+        Object.values(CollectionPermission)
+      );
+    }
 
-allow(User, "createCollection", Team, (user, team) => {
-  if (!team || user.isViewer || user.teamId !== team.id) {
-    return false;
-  }
-  if (user.isAdmin || team.memberCollectionCreate) {
     return true;
   }
-  return false;
-});
+);
 
-allow(User, "importCollection", Team, (actor, team) => {
-  if (!team || actor.teamId !== team.id) {
-    return false;
-  }
-  if (actor.isAdmin) {
-    return true;
-  }
-
-  throw AdminRequiredError();
-});
-
-allow(User, "move", Collection, (user, collection) => {
-  if (!collection || user.teamId !== collection.teamId) {
-    return false;
-  }
-  if (collection.deletedAt) {
-    return false;
-  }
-  if (user.isAdmin) {
-    return true;
-  }
-
-  throw AdminRequiredError();
-});
-
-allow(User, "read", Collection, (user, collection) => {
-  if (!collection || user.teamId !== collection.teamId) {
-    return false;
-  }
-
-  if (collection.isPrivate) {
-    return includesMembership(collection, Object.values(CollectionPermission));
-  }
-
-  return true;
-});
-
-allow(User, ["star", "unstar"], Collection, (user, collection) => {
-  if (!collection || user.teamId !== collection.teamId) {
-    return false;
-  }
-
-  if (collection.isPrivate) {
-    return includesMembership(collection, Object.values(CollectionPermission));
-  }
-
-  return true;
-});
+allow(User, "export", Collection, (actor, collection) =>
+  and(
+    //
+    can(actor, "read", collection),
+    !actor.isViewer,
+    !actor.isGuest
+  )
+);
 
 allow(User, "share", Collection, (user, collection) => {
-  if (!collection || user.teamId !== collection.teamId) {
+  if (
+    !collection ||
+    user.isGuest ||
+    user.teamId !== collection.teamId ||
+    !isTeamMutable(user)
+  ) {
     return false;
   }
   if (!collection.sharing) {
@@ -88,24 +90,16 @@ allow(User, "share", Collection, (user, collection) => {
   return true;
 });
 
-allow(User, ["readDocument", "export"], Collection, (user, collection) => {
-  if (!collection || user.teamId !== collection.teamId) {
-    return false;
-  }
-
-  if (collection.isPrivate) {
-    return includesMembership(collection, Object.values(CollectionPermission));
-  }
-
-  return true;
-});
-
 allow(
   User,
   ["updateDocument", "createDocument", "deleteDocument"],
   Collection,
   (user, collection) => {
-    if (!collection || user.teamId !== collection.teamId) {
+    if (
+      !collection ||
+      user.teamId !== collection.teamId ||
+      !isTeamMutable(user)
+    ) {
       return false;
     }
 
@@ -115,7 +109,8 @@ allow(
 
     if (
       collection.permission !== CollectionPermission.ReadWrite ||
-      user.isViewer
+      user.isViewer ||
+      user.isGuest
     ) {
       return includesMembership(collection, [
         CollectionPermission.ReadWrite,
@@ -128,7 +123,7 @@ allow(
 );
 
 allow(User, ["update", "delete"], Collection, (user, collection) => {
-  if (!collection || user.teamId !== collection.teamId) {
+  if (!collection || user.isGuest || user.teamId !== collection.teamId) {
     return false;
   }
   if (user.isAdmin) {
@@ -139,12 +134,16 @@ allow(User, ["update", "delete"], Collection, (user, collection) => {
 });
 
 function includesMembership(
-  collection: Collection,
+  collection: Collection | null,
   permissions: (CollectionPermission | DocumentPermission)[]
 ) {
+  if (!collection) {
+    return false;
+  }
+
   invariant(
     collection.memberships,
-    "collection memberships should be preloaded, did you forget withMembership scope?"
+    "Development: collection memberships not preloaded, did you forget `withMembership` scope?"
   );
   return some(
     [...collection.memberships, ...collection.collectionGroupMemberships],

server/policies/comment.ts

@@ -1,27 +1,16 @@
 import { Comment, User, Team } from "@server/models";
 import { allow } from "./cancan";
+import { and, isTeamModel, or } from "./utils";
 
-allow(User, "createComment", Team, (user, team) => {
-  if (!team || user.teamId !== team.id) {
-    return false;
-  }
-  return true;
-});
+allow(User, "createComment", Team, isTeamModel);
 
-allow(User, "read", Comment, (user, comment) => {
-  if (!comment) {
-    return false;
-  }
-  return user.teamId === comment.createdBy.teamId;
-});
+allow(User, "read", Comment, (actor, comment) =>
+  isTeamModel(actor, comment?.createdBy)
+);
 
-allow(User, ["update", "delete"], Comment, (user, comment) => {
-  if (!comment) {
-    return false;
-  }
-  if (user.teamId !== comment.createdBy.teamId) {
-    return false;
-  }
-
-  return user.isAdmin || user?.id === comment.createdById;
-});
+allow(User, ["update", "delete"], Comment, (actor, comment) =>
+  and(
+    isTeamModel(actor, comment?.createdBy),
+    or(actor.isAdmin, actor?.id === comment?.createdById)
+  )
+);

server/policies/document.test.ts

@@ -64,6 +64,36 @@ describe("read_write collection", () => {
     expect(abilities.unsubscribe).toEqual(true);
     expect(abilities.comment).toEqual(true);
   });
+
+  it("should allow no permissions for guest", async () => {
+    const team = await buildTeam();
+    const user = await buildUser({
+      teamId: team.id,
+      role: UserRole.Guest,
+    });
+    const collection = await buildCollection({
+      teamId: team.id,
+      permission: CollectionPermission.ReadWrite,
+    });
+    const doc = await buildDocument({
+      teamId: team.id,
+      collectionId: collection.id,
+    });
+    // reload to get membership
+    const document = await Document.findByPk(doc.id, { userId: user.id });
+    const abilities = serialize(user, document);
+    expect(abilities.read).toEqual(false);
+    expect(abilities.download).toEqual(false);
+    expect(abilities.update).toEqual(false);
+    expect(abilities.createChildDocument).toEqual(false);
+    expect(abilities.archive).toEqual(false);
+    expect(abilities.delete).toEqual(false);
+    expect(abilities.share).toEqual(false);
+    expect(abilities.move).toEqual(false);
+    expect(abilities.subscribe).toEqual(false);
+    expect(abilities.unsubscribe).toEqual(false);
+    expect(abilities.comment).toEqual(false);
+  });
 });
 
 describe("read collection", () => {
@@ -93,6 +123,36 @@ describe("read collection", () => {
     expect(abilities.unsubscribe).toEqual(true);
     expect(abilities.comment).toEqual(true);
   });
+
+  it("should allow no permissions for guest", async () => {
+    const team = await buildTeam();
+    const user = await buildUser({
+      teamId: team.id,
+      role: UserRole.Guest,
+    });
+    const collection = await buildCollection({
+      teamId: team.id,
+      permission: CollectionPermission.Read,
+    });
+    const doc = await buildDocument({
+      teamId: team.id,
+      collectionId: collection.id,
+    });
+    // reload to get membership
+    const document = await Document.findByPk(doc.id, { userId: user.id });
+    const abilities = serialize(user, document);
+    expect(abilities.read).toEqual(false);
+    expect(abilities.download).toEqual(false);
+    expect(abilities.update).toEqual(false);
+    expect(abilities.createChildDocument).toEqual(false);
+    expect(abilities.archive).toEqual(false);
+    expect(abilities.delete).toEqual(false);
+    expect(abilities.share).toEqual(false);
+    expect(abilities.move).toEqual(false);
+    expect(abilities.subscribe).toEqual(false);
+    expect(abilities.unsubscribe).toEqual(false);
+    expect(abilities.comment).toEqual(false);
+  });
 });
 
 describe("private collection", () => {
@@ -120,6 +180,34 @@ describe("private collection", () => {
     expect(abilities.unsubscribe).toEqual(false);
     expect(abilities.comment).toEqual(false);
   });
+
+  it("should allow no permissions for guest", async () => {
+    const team = await buildTeam();
+    const user = await buildUser({
+      teamId: team.id,
+      role: UserRole.Guest,
+    });
+    const collection = await buildCollection({
+      teamId: team.id,
+      permission: null,
+    });
+    const document = await buildDocument({
+      teamId: team.id,
+      collectionId: collection.id,
+    });
+    const abilities = serialize(user, document);
+    expect(abilities.read).toEqual(false);
+    expect(abilities.download).toEqual(false);
+    expect(abilities.update).toEqual(false);
+    expect(abilities.createChildDocument).toEqual(false);
+    expect(abilities.archive).toEqual(false);
+    expect(abilities.delete).toEqual(false);
+    expect(abilities.share).toEqual(false);
+    expect(abilities.move).toEqual(false);
+    expect(abilities.subscribe).toEqual(false);
+    expect(abilities.unsubscribe).toEqual(false);
+    expect(abilities.comment).toEqual(false);
+  });
 });
 
 describe("no collection", () => {
@@ -143,6 +231,29 @@ describe("no collection", () => {
     expect(abilities.comment).toEqual(false);
   });
 
+  it("should allow no permissions for guest", async () => {
+    const team = await buildTeam();
+    const user = await buildUser({
+      teamId: team.id,
+      role: UserRole.Guest,
+    });
+    const document = await buildDraftDocument({
+      teamId: team.id,
+    });
+    const abilities = serialize(user, document);
+    expect(abilities.read).toEqual(false);
+    expect(abilities.download).toEqual(false);
+    expect(abilities.update).toEqual(false);
+    expect(abilities.createChildDocument).toEqual(false);
+    expect(abilities.archive).toEqual(false);
+    expect(abilities.delete).toEqual(false);
+    expect(abilities.share).toEqual(false);
+    expect(abilities.move).toEqual(false);
+    expect(abilities.subscribe).toEqual(false);
+    expect(abilities.unsubscribe).toEqual(false);
+    expect(abilities.comment).toEqual(false);
+  });
+
   it("should allow edit permissions for creator", async () => {
     const team = await buildTeam();
     const user = await buildUser({ teamId: team.id });
@@ -161,8 +272,8 @@ describe("no collection", () => {
     expect(abilities.delete).toEqual(true);
     expect(abilities.share).toEqual(true);
     expect(abilities.move).toEqual(true);
-    expect(abilities.subscribe).toEqual(false);
-    expect(abilities.unsubscribe).toEqual(false);
+    expect(abilities.subscribe).toEqual(true);
+    expect(abilities.unsubscribe).toEqual(true);
     expect(abilities.comment).toEqual(true);
   });
 });

server/policies/document.ts

@@ -7,446 +7,201 @@ import {
 } from "@shared/types";
 import { Document, Revision, User, Team } from "@server/models";
 import { allow, _cannot as cannot, _can as can } from "./cancan";
-
-allow(User, "createDocument", Team, (user, team) => {
-  if (!team || user.isViewer || user.teamId !== team.id) {
-    return false;
-  }
-  return true;
-});
-
-allow(User, "read", Document, (user, document) => {
-  if (!document) {
-    return false;
-  }
-
-  if (
-    includesMembership(document, [
-      DocumentPermission.Read,
-      DocumentPermission.ReadWrite,
-    ])
-  ) {
-    return true;
-  }
-
-  // existence of collection option is not required here to account for share tokens
-  if (
-    document.collection &&
-    cannot(user, "readDocument", document.collection)
-  ) {
-    return false;
-  }
-
-  if (document.isDraft) {
-    return user.id === document.createdById;
-  }
-
-  return user.teamId === document.teamId;
-});
-
-allow(User, "download", Document, (user, document) => {
-  if (!document) {
-    return false;
-  }
-
-  if (
-    user.isViewer &&
-    !user.team.getPreference(TeamPreference.ViewersCanExport)
-  ) {
-    return false;
-  }
-
-  if (
-    includesMembership(document, [
-      DocumentPermission.Read,
-      DocumentPermission.ReadWrite,
-    ])
-  ) {
-    return true;
-  }
-
-  // existence of collection option is not required here to account for share tokens
-  if (
-    document.collection &&
-    cannot(user, "readDocument", document.collection)
-  ) {
-    return false;
-  }
-
-  if (document.isDraft) {
-    return user.id === document.createdById;
-  }
-
-  return user.teamId === document.teamId;
-});
-
-allow(User, "comment", Document, (user, document) => {
-  if (!document || !document.isActive || document.template) {
-    return false;
-  }
-
-  if (
-    includesMembership(document, [
-      DocumentPermission.Read,
-      DocumentPermission.ReadWrite,
-    ])
-  ) {
-    return true;
-  }
-
-  if (document.collectionId) {
-    invariant(
-      document.collection,
-      "collection is missing, did you forget to include in the query scope?"
-    );
-    if (can(user, "readDocument", document.collection)) {
-      return true;
-    }
-  }
-
-  return user.id === document.createdById;
-});
-
-allow(User, ["star", "unstar"], Document, (user, document) => {
-  if (!document || !document.isActive || document.template) {
-    return false;
-  }
-
-  if (
-    includesMembership(document, [
-      DocumentPermission.Read,
-      DocumentPermission.ReadWrite,
-    ])
-  ) {
-    return true;
-  }
-
-  if (document.collectionId) {
-    invariant(
-      document.collection,
-      "collection is missing, did you forget to include in the query scope?"
-    );
-    if (cannot(user, "readDocument", document.collection)) {
-      return false;
-    }
-  }
-
-  return user.teamId === document.teamId;
-});
-
-allow(User, "share", Document, (user, document) => {
-  if (
-    !document ||
-    document.archivedAt ||
-    document.deletedAt ||
-    document.template
-  ) {
-    return false;
-  }
-
-  if (document.collectionId) {
-    invariant(
-      document.collection,
-      "collection is missing, did you forget to include in the query scope?"
-    );
-    if (cannot(user, "share", document.collection)) {
-      return false;
-    }
-  }
-
-  if (document.isDraft) {
-    return user.id === document.createdById;
-  }
-
-  return user.teamId === document.teamId;
-});
-
-allow(User, "update", Document, (user, document) => {
-  if (!document || !document.isActive) {
-    return false;
-  }
-
-  if (includesMembership(document, [DocumentPermission.ReadWrite])) {
-    return true;
-  }
-
-  if (document.collectionId) {
-    invariant(
-      document.collection,
-      "collection is missing, did you forget to include in the query scope?"
-    );
-    if (cannot(user, "updateDocument", document.collection)) {
-      return false;
-    }
-  }
-
-  if (document.isDraft) {
-    return user.id === document.createdById;
-  }
-
-  return user.teamId === document.teamId;
-});
-
-allow(User, "publish", Document, (user, document) => {
-  if (!document || !document.isActive || !document.isDraft) {
-    return false;
-  }
-
-  if (document.collectionId) {
-    invariant(
-      document.collection,
-      "collection is missing, did you forget to include in the query scope?"
-    );
-    if (can(user, "updateDocument", document.collection)) {
-      return true;
-    }
-  }
-
-  return user.id === document.createdById;
-});
-
-allow(User, ["manageUsers", "duplicate"], Document, (user, document) => {
-  if (!document || !document.isActive) {
-    return false;
-  }
-
-  if (document.collectionId) {
-    invariant(
-      document.collection,
-      "collection is missing, did you forget to include in the query scope?"
-    );
-    if (can(user, "updateDocument", document.collection)) {
-      return true;
-    }
-  }
-
-  return user.id === document.createdById;
-});
-
-allow(User, "updateInsights", Document, (user, document) => {
-  if (!document || !document.isActive) {
-    return false;
-  }
-
-  if (document.collectionId) {
-    invariant(
-      document.collection,
-      "collection is missing, did you forget to include in the query scope?"
-    );
-    if (can(user, "update", document.collection)) {
-      return true;
-    }
-  }
-
-  return user.id === document.createdById;
-});
-
-allow(User, "createChildDocument", Document, (user, document) => {
-  if (
-    !document ||
-    !document.isActive ||
-    document.isDraft ||
-    document.template
-  ) {
-    return false;
-  }
-
-  invariant(
-    document.collection,
-    "collection is missing, did you forget to include in the query scope?"
-  );
-  if (can(user, "updateDocument", document.collection)) {
-    return true;
-  }
-  return user.id === document.createdById;
-});
-
-allow(User, "move", Document, (user, document) => {
-  if (!document || !document.isActive) {
-    return false;
-  }
-  if (document.collection && can(user, "updateDocument", document.collection)) {
-    return true;
-  }
-  return user.id === document.createdById;
-});
-
-allow(User, "pin", Document, (user, document) => {
-  if (
-    !document ||
-    !document.isActive ||
-    document.isDraft ||
-    document.template
-  ) {
-    return false;
-  }
-  invariant(
-    document.collection,
-    "collection is missing, did you forget to include in the query scope?"
-  );
-  if (can(user, "update", document.collection)) {
-    return true;
-  }
-  return user.id === document.createdById;
-});
-
-allow(User, "unpin", Document, (user, document) => {
-  if (!document || document.isDraft || document.template) {
-    return false;
-  }
-  invariant(
-    document.collection,
-    "collection is missing, did you forget to include in the query scope?"
-  );
-  if (can(user, "update", document.collection)) {
-    return true;
-  }
-  return user.id === document.createdById;
-});
-
-allow(User, ["subscribe", "unsubscribe"], Document, (user, document) => {
-  if (
-    !document ||
-    !document.isActive ||
-    document.isDraft ||
-    document.template
-  ) {
-    return false;
-  }
-
-  if (
-    includesMembership(document, [
-      DocumentPermission.Read,
-      DocumentPermission.ReadWrite,
-    ])
-  ) {
-    return true;
-  }
-
-  invariant(
-    document.collection,
-    "collection is missing, did you forget to include in the query scope?"
-  );
-  if (can(user, "readDocument", document.collection)) {
-    return true;
-  }
-
-  return user.id === document.createdById;
-});
-
-allow(User, "pinToHome", Document, (user, document) => {
-  if (
-    !document ||
-    !document.isActive ||
-    document.isDraft ||
-    document.template
-  ) {
-    return false;
-  }
-
-  return user.teamId === document.teamId && user.isAdmin;
-});
-
-allow(User, "delete", Document, (user, document) => {
-  if (!document || document.deletedAt || user.isViewer) {
-    return false;
-  }
-
-  // allow deleting document without a collection
-  if (
-    document.collection &&
-    cannot(user, "deleteDocument", document.collection)
-  ) {
-    return false;
-  }
-
-  // unpublished drafts can always be deleted by their owner
-  if (document.isDraft) {
-    return user.id === document.createdById;
-  }
-
-  return user.teamId === document.teamId;
-});
-
-allow(User, "permanentDelete", Document, (user, document) => {
-  if (!document || !document.deletedAt || user.isViewer) {
-    return false;
-  }
-
-  // allow deleting document without a collection
-  if (
-    document.collection &&
-    cannot(user, "updateDocument", document.collection)
-  ) {
-    return false;
-  }
-
-  // unpublished drafts can always be deleted by their owner
-  if (document.isDraft && user.id === document.createdById) {
-    return true;
-  }
-
-  return user.teamId === document.teamId && user.isAdmin;
-});
-
-allow(User, "restore", Document, (user, document) => {
-  if (!document || !document.deletedAt) {
-    return false;
-  }
-
-  if (
-    document.collection &&
-    cannot(user, "updateDocument", document.collection)
-  ) {
-    return false;
-  }
-
-  // unpublished drafts can always be restored by their owner
-  if (document.isDraft && user.id === document.createdById) {
-    return true;
-  }
-
-  return user.teamId === document.teamId;
-});
-
-allow(User, "archive", Document, (user, document) => {
-  if (
-    !document ||
-    !document.isActive ||
-    document.isDraft ||
-    document.template
-  ) {
-    return false;
-  }
-  invariant(
-    document.collection,
-    "collection is missing, did you forget to include in the query scope?"
-  );
-  if (cannot(user, "updateDocument", document.collection)) {
-    return false;
-  }
-  return user.teamId === document.teamId;
-});
-
-allow(User, "unarchive", Document, (user, document) => {
-  if (!document || !document.archivedAt || document.deletedAt) {
-    return false;
-  }
-
-  invariant(
-    document.collection,
-    "collection is missing, did you forget to include in the query scope?"
-  );
-  if (cannot(user, "updateDocument", document.collection)) {
-    return false;
-  }
-
-  if (document.isDraft) {
-    return user.id === document.createdById;
-  }
-
-  return user.teamId === document.teamId;
-});
+import { and, isTeamAdmin, isTeamModel, isTeamMutable, or } from "./utils";
+
+allow(User, "createDocument", Team, (actor, document) =>
+  and(
+    //
+    !actor.isGuest,
+    !actor.isViewer,
+    isTeamModel(actor, document),
+    isTeamMutable(actor)
+  )
+);
+
+allow(User, "read", Document, (actor, document) =>
+  and(
+    isTeamModel(actor, document),
+    or(
+      includesMembership(document, [
+        DocumentPermission.Read,
+        DocumentPermission.ReadWrite,
+      ]),
+      and(!!document?.isDraft, actor.id === document?.createdById),
+      can(actor, "readDocument", document?.collection)
+    )
+  )
+);
+
+allow(User, ["listRevisions", "listViews"], Document, (actor, document) =>
+  and(
+    //
+    can(actor, "read", document),
+    !actor.isGuest
+  )
+);
+
+allow(User, "download", Document, (actor, document) =>
+  and(
+    can(actor, "read", document),
+    or(
+      and(!actor.isGuest, !actor.isViewer),
+      !!actor.team.getPreference(TeamPreference.ViewersCanExport)
+    )
+  )
+);
+
+allow(User, "comment", Document, (actor, document) =>
+  and(
+    //
+    can(actor, "read", document),
+    isTeamMutable(actor),
+    !!document?.isActive,
+    !document?.template
+  )
+);
+
+allow(
+  User,
+  ["star", "unstar", "subscribe", "unsubscribe"],
+  Document,
+  (actor, document) =>
+    and(
+      //
+      can(actor, "read", document),
+      !document?.template
+    )
+);
+
+allow(User, "share", Document, (actor, document) =>
+  and(
+    can(actor, "read", document),
+    isTeamMutable(actor),
+    !!document?.isActive,
+    !document?.template,
+    !actor.isGuest,
+    or(!document?.collection, can(actor, "share", document?.collection))
+  )
+);
+
+allow(User, "update", Document, (actor, document) =>
+  and(
+    can(actor, "read", document),
+    isTeamMutable(actor),
+    !!document?.isActive,
+    or(
+      includesMembership(document, [DocumentPermission.ReadWrite]),
+      or(
+        can(actor, "updateDocument", document?.collection),
+        and(!!document?.isDraft && actor.id === document?.createdById)
+      )
+    )
+  )
+);
+
+allow(User, "publish", Document, (actor, document) =>
+  and(
+    //
+    can(actor, "update", document),
+    !!document?.isDraft
+  )
+);
+
+allow(User, ["move", "duplicate", "manageUsers"], Document, (actor, document) =>
+  and(
+    !actor.isGuest,
+    can(actor, "update", document),
+    or(
+      can(actor, "updateDocument", document?.collection),
+      and(!!document?.isDraft && actor.id === document?.createdById)
+    )
+  )
+);
+
+allow(User, "createChildDocument", Document, (actor, document) =>
+  and(
+    can(actor, "update", document),
+    !document?.isDraft,
+    !document?.template,
+    !actor.isGuest
+  )
+);
+
+allow(User, ["pin", "unpin"], Document, (actor, document) =>
+  and(
+    can(actor, "update", document),
+    can(actor, "update", document?.collection),
+    !document?.isDraft,
+    !document?.template,
+    !actor.isGuest
+  )
+);
+
+allow(User, "pinToHome", Document, (actor, document) =>
+  and(
+    //
+    isTeamAdmin(actor, document),
+    isTeamMutable(actor)
+  )
+);
+
+allow(User, "delete", Document, (actor, document) =>
+  and(
+    isTeamModel(actor, document),
+    isTeamMutable(actor),
+    !actor.isGuest,
+    !document?.isDeleted,
+    or(can(actor, "update", document), !document?.collection)
+  )
+);
+
+allow(User, ["restore", "permanentDelete"], Document, (actor, document) =>
+  and(
+    isTeamModel(actor, document),
+    !actor.isGuest,
+    !!document?.isDeleted,
+    or(
+      includesMembership(document, [DocumentPermission.ReadWrite]),
+      or(
+        can(actor, "updateDocument", document?.collection),
+        and(!!document?.isDraft && actor.id === document?.createdById)
+      ),
+      !document?.collection
+    )
+  )
+);
+
+allow(User, "archive", Document, (actor, document) =>
+  and(
+    !actor.isGuest,
+    !document?.template,
+    !document?.isDraft,
+    !!document?.isActive,
+    can(actor, "update", document),
+    can(actor, "updateDocument", document?.collection)
+  )
+);
+
+allow(User, "unarchive", Document, (actor, document) =>
+  and(
+    !actor.isGuest,
+    !document?.template,
+    !document?.isDraft,
+    !document?.isDeleted,
+    !!document?.archivedAt,
+    and(
+      can(actor, "read", document),
+      or(
+        includesMembership(document, [DocumentPermission.ReadWrite]),
+        or(
+          can(actor, "updateDocument", document?.collection),
+          and(!!document?.isDraft && actor.id === document?.createdById)
+        )
+      )
+    ),
+    can(actor, "updateDocument", document?.collection)
+  )
+);
 
 allow(
   Document,
@@ -456,7 +211,13 @@ allow(
 );
 
 allow(User, "unpublish", Document, (user, document) => {
-  if (!document || !document.isActive || document.isDraft || user.isViewer) {
+  if (
+    !document ||
+    user.isGuest ||
+    user.isViewer ||
+    !document.isActive ||
+    document.isDraft
+  ) {
     return false;
   }
   invariant(
@@ -470,9 +231,13 @@ allow(User, "unpublish", Document, (user, document) => {
 });
 
 function includesMembership(
-  document: Document,
+  document: Document | null,
   permissions: (DocumentPermission | CollectionPermission)[]
 ) {
+  if (!document) {
+    return false;
+  }
+
   invariant(
     document.memberships,
     "document memberships should be preloaded, did you forget withMembership scope?"

server/policies/fileOperation.ts

@@ -1,35 +1,25 @@
 import { FileOperationState, FileOperationType } from "@shared/types";
 import { User, Team, FileOperation } from "@server/models";
 import { allow } from "./cancan";
+import { and, isTeamAdmin, isTeamMutable, or } from "./utils";
 
 allow(
   User,
   ["createFileOperation", "createImport", "createExport"],
   Team,
-  (user, team) => {
-    if (!team || user.isViewer || user.teamId !== team.id) {
-      return false;
-    }
-    return user.isAdmin;
-  }
+  // Note: Not checking for isTeamMutable here because we want to allow exporting data in read-only.
+  isTeamAdmin
 );
 
-allow(User, "read", FileOperation, (user, fileOperation) => {
-  if (!fileOperation || user.isViewer || user.teamId !== fileOperation.teamId) {
-    return false;
-  }
-  return user.isAdmin;
-});
+allow(User, "read", FileOperation, isTeamAdmin);
 
-allow(User, "delete", FileOperation, (user, fileOperation) => {
-  if (!fileOperation || user.isViewer || user.teamId !== fileOperation.teamId) {
-    return false;
-  }
-  if (
-    fileOperation.type === FileOperationType.Export &&
-    fileOperation.state !== FileOperationState.Complete
-  ) {
-    return false;
-  }
-  return user.isAdmin;
-});
+allow(User, "delete", FileOperation, (actor, fileOperation) =>
+  and(
+    isTeamAdmin(actor, fileOperation),
+    isTeamMutable(actor),
+    or(
+      fileOperation?.type !== FileOperationType.Export,
+      fileOperation?.state === FileOperationState.Complete
+    )
+  )
+);

server/policies/group.ts

@@ -1,34 +1,35 @@
 import { Group, User, Team } from "@server/models";
-import { AdminRequiredError } from "../errors";
 import { allow } from "./cancan";
+import { and, isTeamAdmin, isTeamModel, isTeamMutable } from "./utils";
 
-allow(User, "createGroup", Team, (actor, team) => {
-  if (!team || actor.isViewer || actor.teamId !== team.id) {
-    return false;
-  }
-  if (actor.isAdmin) {
-    return true;
-  }
+allow(User, "createGroup", Team, (actor, team) =>
+  and(
+    //
+    isTeamAdmin(actor, team),
+    isTeamMutable(actor)
+  )
+);
 
-  throw AdminRequiredError();
-});
+allow(User, "listGroups", Team, (actor, team) =>
+  and(
+    //
+    isTeamModel(actor, team),
+    !actor.isGuest
+  )
+);
 
-allow(User, "read", Group, (actor, group) => {
-  // for the time being, we're going to let everyone on the team see every group
-  // we may need to make this more granular in the future
-  if (!group || actor.teamId !== group.teamId) {
-    return false;
-  }
-  return true;
-});
+allow(User, "read", Group, (actor, team) =>
+  and(
+    //
+    isTeamModel(actor, team),
+    !actor.isGuest
+  )
+);
 
-allow(User, ["update", "delete"], Group, (actor, group) => {
-  if (!group || actor.isViewer || actor.teamId !== group.teamId) {
-    return false;
-  }
-  if (actor.isAdmin) {
-    return true;
-  }
-
-  throw AdminRequiredError();
-});
+allow(User, ["update", "delete"], Group, (actor, team) =>
+  and(
+    //
+    isTeamAdmin(actor, team),
+    isTeamMutable(actor)
+  )
+);

server/policies/integration.ts

@@ -1,42 +1,31 @@
 import { IntegrationType } from "@shared/types";
 import { Integration, User, Team } from "@server/models";
-import { AdminRequiredError } from "../errors";
 import { allow } from "./cancan";
+import {
+  and,
+  isOwner,
+  isTeamAdmin,
+  isTeamModel,
+  isTeamMutable,
+  or,
+} from "./utils";
 
-allow(User, "createIntegration", Team, (actor, team) => {
-  if (!team || actor.isViewer || actor.teamId !== team.id) {
-    return false;
-  }
-  if (actor.isAdmin) {
-    return true;
-  }
-
-  throw AdminRequiredError();
-});
-
-allow(
-  User,
-  "read",
-  Integration,
-  (user, integration) => user.teamId === integration?.teamId
+allow(User, "createIntegration", Team, (actor, team) =>
+  and(isTeamAdmin(actor, team), isTeamMutable(actor))
 );
 
-allow(User, ["update", "delete"], Integration, (user, integration) => {
-  if (!integration || user.teamId !== integration.teamId) {
-    return false;
-  }
-  if (
-    integration.userId === user.id &&
-    integration.type === IntegrationType.LinkedAccount
-  ) {
-    return true;
-  }
-  if (user.isViewer) {
-    return false;
-  }
-  if (user.isAdmin) {
-    return true;
-  }
+allow(User, "read", Integration, isTeamModel);
 
-  throw AdminRequiredError();
-});
+allow(User, ["update", "delete"], Integration, (actor, integration) =>
+  and(
+    isTeamModel(actor, integration),
+    isTeamMutable(actor),
+    !actor.isGuest,
+    !actor.isViewer,
+    or(
+      actor.isAdmin,
+      isOwner(actor, integration) &&
+        integration.type === IntegrationType.LinkedAccount
+    )
+  )
+);

server/policies/notification.ts

@@ -1,9 +1,5 @@
 import { Notification, User } from "@server/models";
 import { allow } from "./cancan";
+import { isOwner } from "./utils";
 
-allow(User, ["read", "update"], Notification, (user, notification) => {
-  if (!notification) {
-    return false;
-  }
-  return user?.id === notification.userId;
-});
+allow(User, ["read", "update"], Notification, isOwner);

server/policies/pins.ts

@@ -1,9 +1,5 @@
 import { User, Pin } from "@server/models";
 import { allow } from "./cancan";
+import { isTeamAdmin } from "./utils";
 
-allow(
-  User,
-  ["update", "delete"],
-  Pin,
-  (user, pin) => user.teamId === pin?.teamId && user.isAdmin
-);
+allow(User, ["update", "delete"], Pin, isTeamAdmin);

server/policies/searchQuery.ts

@@ -1,9 +1,5 @@
 import { SearchQuery, User } from "@server/models";
 import { allow } from "./cancan";
+import { isOwner } from "./utils";
 
-allow(
-  User,
-  ["read", "delete"],
-  SearchQuery,
-  (user, searchQuery) => user && user.id === searchQuery?.userId
-);
+allow(User, ["read", "delete"], SearchQuery, isOwner);

server/policies/share.ts

@@ -1,41 +1,46 @@
-import { Share, User } from "@server/models";
-import { AdminRequiredError } from "../errors";
-import { allow, _cannot as cannot } from "./cancan";
+import { Share, Team, User } from "@server/models";
+import { allow, _can as can } from "./cancan";
+import { and, isOwner, isTeamModel, isTeamMutable, or } from "./utils";
 
-allow(User, "read", Share, (user, share) => user.teamId === share?.teamId);
+allow(User, "createShare", Team, (actor, team) =>
+  and(
+    //
+    isTeamModel(actor, team),
+    isTeamMutable(actor),
+    !actor.isGuest
+  )
+);
 
-allow(User, "update", Share, (user, share) => {
-  if (!share) {
-    return false;
-  }
-  if (user.isViewer) {
-    return false;
-  }
+allow(User, "listShares", Team, (actor, team) =>
+  and(
+    //
+    isTeamModel(actor, team),
+    !actor.isGuest
+  )
+);
 
-  // only the user who can share the document publicly can update the share.
-  if (cannot(user, "share", share.document)) {
-    return false;
-  }
+allow(User, "read", Share, (actor, share) =>
+  and(
+    //
+    isTeamModel(actor, share),
+    !actor.isGuest
+  )
+);
 
-  return user.teamId === share.teamId;
-});
+allow(User, "update", Share, (actor, share) =>
+  and(
+    isTeamModel(actor, share),
+    !actor.isGuest,
+    !actor.isViewer,
+    can(actor, "share", share?.document)
+  )
+);
 
-allow(User, "revoke", Share, (user, share) => {
-  if (!share) {
-    return false;
-  }
-  if (user.isViewer) {
-    return false;
-  }
-  if (user.teamId !== share.teamId) {
-    return false;
-  }
-  if (user.id === share.userId) {
-    return true;
-  }
-  if (user.isAdmin) {
-    return true;
-  }
-
-  throw AdminRequiredError();
-});
+allow(User, "revoke", Share, (actor, share) =>
+  and(
+    isTeamModel(actor, share),
+    !actor.isGuest,
+    !actor.isViewer,
+    or(actor.isAdmin, isOwner(actor, share))
+  )
+);

server/policies/star.ts

@@ -1,9 +1,5 @@
 import { User, Star } from "@server/models";
 import { allow } from "./cancan";
+import { isOwner } from "./utils";
 
-allow(
-  User,
-  ["update", "delete"],
-  Star,
-  (user, star) => user.id === star?.userId
-);
+allow(User, ["read", "update", "delete"], Star, isOwner);

server/policies/subscription.ts

@@ -1,21 +1,5 @@
 import { Subscription, User } from "@server/models";
 import { allow } from "./cancan";
+import { isOwner } from "./utils";
 
-allow(
-  User,
-  ["read", "update", "delete"],
-  Subscription,
-  (user, subscription) => {
-    if (!subscription) {
-      return false;
-    }
-
-    // If `user` is an admin, early exit with allow.
-    if (user.isAdmin) {
-      return true;
-    }
-
-    // User should be able to read their subscriptions.
-    return user.id === subscription.userId;
-  }
-);
+allow(User, ["read", "update", "delete"], Subscription, isOwner);

server/policies/team.ts

@@ -1,41 +1,33 @@
-import env from "@server/env";
-import { IncorrectEditionError } from "@server/errors";
 import { Team, User } from "@server/models";
 import { allow } from "./cancan";
+import { and, isCloudHosted, isTeamAdmin, isTeamModel } from "./utils";
 
-allow(User, "read", Team, (user, team) => user.teamId === team?.id);
+allow(User, "read", Team, isTeamModel);
 
-allow(User, "share", Team, (user, team) => {
-  if (!team || user.isViewer || user.teamId !== team.id) {
-    return false;
-  }
-  return team.sharing;
-});
+allow(User, "share", Team, (actor, team) =>
+  and(
+    isTeamModel(actor, team),
+    !actor.isGuest,
+    !actor.isViewer,
+    !!team?.sharing
+  )
+);
 
-allow(User, "createTeam", Team, () => {
-  if (!env.isCloudHosted) {
-    throw IncorrectEditionError(
-      "Functionality is not available in this edition"
-    );
-  }
-  return true;
-});
+allow(User, "createTeam", Team, (actor) =>
+  and(
+    //
+    isCloudHosted(),
+    !actor.isGuest,
+    !actor.isViewer
+  )
+);
 
-allow(User, "update", Team, (user, team) => {
-  if (!team || user.isViewer || user.teamId !== team.id) {
-    return false;
-  }
-  return user.isAdmin;
-});
+allow(User, "update", Team, isTeamAdmin);
 
-allow(User, ["delete", "audit"], Team, (user, team) => {
-  if (!env.isCloudHosted) {
-    throw IncorrectEditionError(
-      "Functionality is not available in this edition"
-    );
-  }
-  if (!team || user.isViewer || user.teamId !== team.id) {
-    return false;
-  }
-  return user.isAdmin;
-});
+allow(User, ["delete", "audit"], Team, (actor, team) =>
+  and(
+    //
+    isCloudHosted(),
+    isTeamAdmin(actor, team)
+  )
+);

server/policies/user.ts

@@ -1,114 +1,59 @@
 import { TeamPreference } from "@shared/types";
 import { User, Team } from "@server/models";
-import { AdminRequiredError } from "../errors";
 import { allow } from "./cancan";
+import { and, isTeamAdmin, isTeamModel, isTeamMutable, or } from "./utils";
 
-allow(
-  User,
-  "read",
-  User,
-  (actor, user) => user && user.teamId === actor.teamId
+allow(User, "read", User, isTeamModel);
+
+allow(User, "listUsers", Team, (actor, team) =>
+  and(
+    //
+    isTeamModel(actor, team),
+    !actor.isGuest
+  )
 );
 
-allow(User, "inviteUser", Team, (actor, team) => {
-  if (!team || actor.teamId !== team.id || actor.isViewer) {
-    return false;
-  }
-  if (actor.isAdmin || team.getPreference(TeamPreference.MembersCanInvite)) {
-    return true;
-  }
+allow(User, "inviteUser", Team, (actor, team) =>
+  and(
+    isTeamModel(actor, team),
+    isTeamMutable(actor),
+    !actor.isGuest,
+    !actor.isViewer,
+    actor.isAdmin || !!team?.getPreference(TeamPreference.MembersCanInvite)
+  )
+);
 
-  throw AdminRequiredError();
-});
+allow(User, ["update", "delete", "readDetails"], User, (actor, user) =>
+  or(
+    //
+    isTeamAdmin(actor, user),
+    actor.id === user?.id
+  )
+);
 
-allow(User, "update", User, (actor, user) => {
-  if (!user || user.teamId !== actor.teamId) {
-    return false;
-  }
-  if (user.id === actor.id) {
-    return true;
-  }
+allow(User, ["activate", "suspend"], User, isTeamAdmin);
 
-  if (actor.isAdmin) {
-    return true;
-  }
+allow(User, "promote", User, (actor, user) =>
+  and(
+    //
+    isTeamAdmin(actor, user),
+    !user?.isAdmin,
+    !user?.isSuspended
+  )
+);
 
-  return false;
-});
+allow(User, "demote", User, (actor, user) =>
+  and(
+    //
+    isTeamAdmin(actor, user),
+    !user?.isSuspended
+  )
+);
 
-allow(User, "delete", User, (actor, user) => {
-  if (!user || user.teamId !== actor.teamId) {
-    return false;
-  }
-  if (user.id === actor.id) {
-    return true;
-  }
-  if (actor.isAdmin) {
-    return true;
-  }
-
-  throw AdminRequiredError();
-});
-
-allow(User, ["activate", "suspend"], User, (actor, user) => {
-  if (!user || user.teamId !== actor.teamId) {
-    return false;
-  }
-  if (actor.isAdmin) {
-    return true;
-  }
-
-  throw AdminRequiredError();
-});
-
-allow(User, "readDetails", User, (actor, user) => {
-  if (!user || user.teamId !== actor.teamId) {
-    return false;
-  }
-  if (user === actor) {
-    return true;
-  }
-  return actor.isAdmin;
-});
-
-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 AdminRequiredError();
-});
-
-allow(User, "resendInvite", User, (actor, user) => {
-  if (!user || user.teamId !== actor.teamId) {
-    return false;
-  }
-  if (!user.isInvited) {
-    return false;
-  }
-  if (actor.isAdmin) {
-    return true;
-  }
-
-  throw AdminRequiredError();
-});
-
-allow(User, "demote", User, (actor, user) => {
-  if (!user || user.teamId !== actor.teamId) {
-    return false;
-  }
-  if (user.isSuspended) {
-    return false;
-  }
-  if (actor.isAdmin) {
-    return true;
-  }
-
-  throw AdminRequiredError();
-});
+allow(User, "resendInvite", User, (actor, user) =>
+  and(
+    //
+    isTeamAdmin(actor, user),
+    !!user?.isInvited
+  )
+);

server/policies/userMembership.ts

@@ -1,9 +1,11 @@
 import { User, UserMembership } from "@server/models";
 import { allow } from "./cancan";
+import { isOwner, or } from "./utils";
 
-allow(
-  User,
-  ["update", "delete"],
-  UserMembership,
-  (user, membership) => user.id === membership?.userId || user.isAdmin
+allow(User, ["update", "delete"], UserMembership, (actor, membership) =>
+  or(
+    //
+    isOwner(actor, membership),
+    actor.isAdmin
+  )
 );

server/policies/utils.ts

@@ -0,0 +1,91 @@
+import env from "@server/env";
+import { IncorrectEditionError } from "@server/errors";
+import { User, Team } from "@server/models";
+import Model from "@server/models/base/Model";
+
+export function and(...args: boolean[]) {
+  return args.every(Boolean);
+}
+
+export function or(...args: boolean[]) {
+  return args.some(Boolean);
+}
+
+/**
+ * Check if the actor is present in the same team as the model.
+ *
+ * @param actor The actor to check
+ * @param model The model to check
+ * @returns True if the actor is in the same team as the model
+ */
+export function isTeamModel(
+  actor: User,
+  model: Model | null | undefined
+): model is Model {
+  if (!model) {
+    return false;
+  }
+  if (model instanceof Team) {
+    return actor.teamId === model.id;
+  }
+  if ("teamId" in model) {
+    return actor.teamId === model.teamId;
+  }
+  return false;
+}
+
+/**
+ * Check if the actor is the owner of the model.
+ *
+ * @param actor The actor to check
+ * @param model The model to check
+ * @returns True if the actor is the owner of the model
+ */
+export function isOwner(
+  actor: User,
+  model: Model | null | undefined
+): model is Model {
+  if (!model) {
+    return false;
+  }
+  if ("userId" in model) {
+    return actor.id === model.userId;
+  }
+  return false;
+}
+
+/**
+ * Check if the actor is an admin of the team.
+ *
+ * @param actor The actor to check
+ * @param mode The model to check
+ * @returns True if the actor is an admin of the team the model belongs to
+ */
+export function isTeamAdmin(
+  actor: User,
+  model: Model | null | undefined
+): model is Model {
+  return and(isTeamModel(actor, model), actor.isAdmin);
+}
+
+/**
+ * Check the actors team is mutable, meaning the team models can be modified.
+ *
+ * @param actor The actor to check
+ * @returns True if the actor's team is mutable
+ */
+export function isTeamMutable(_actor: User, _model?: Model | null) {
+  return true;
+}
+
+/**
+ * Check if this instance is running in the cloud-hosted environment.
+ */
+export function isCloudHosted() {
+  if (!env.isCloudHosted) {
+    throw IncorrectEditionError(
+      "Functionality is not available in this edition"
+    );
+  }
+  return true;
+}

server/policies/webhookSubscription.ts

@@ -1,35 +1,15 @@
 import { User, Team, WebhookSubscription } from "@server/models";
 import { allow } from "./cancan";
+import { and, isTeamAdmin, isTeamMutable } from "./utils";
 
-allow(User, "listWebhookSubscription", Team, (user, team) => {
-  if (!team || user.isViewer || user.teamId !== team.id) {
-    return false;
-  }
-
-  return user.isAdmin;
-});
-
-allow(User, "createWebhookSubscription", Team, (user, team) => {
-  if (!team || user.isViewer || user.teamId !== team.id) {
-    return false;
-  }
-
-  return user.isAdmin;
-});
-
-allow(
-  User,
-  ["read", "update", "delete"],
-  WebhookSubscription,
-  (user, webhook): boolean => {
-    if (!user || !webhook) {
-      return false;
-    }
-
-    if (!user.isAdmin) {
-      return false;
-    }
-
-    return user.teamId === webhook.teamId;
-  }
+allow(User, "createWebhookSubscription", Team, (actor, team) =>
+  and(
+    //
+    isTeamAdmin(actor, team),
+    isTeamMutable(actor)
+  )
 );
+
+allow(User, "listWebhookSubscription", Team, isTeamAdmin);
+
+allow(User, ["read", "update", "delete"], WebhookSubscription, isTeamAdmin);

server/routes/api/documents/documents.test.ts

@@ -2564,30 +2564,6 @@ describe("#documents.restore", () => {
     expect(body.data.archivedAt).toEqual(null);
   });
 
-  it("should not add restored templates to collection structure", async () => {
-    const user = await buildUser();
-    const collection = await buildCollection({
-      teamId: user.teamId,
-    });
-    const template = await buildDocument({
-      teamId: user.teamId,
-      collectionId: collection.id,
-      template: true,
-    });
-    await template.archive(user.id);
-    const res = await server.post("/api/documents.restore", {
-      body: {
-        token: user.getJwtToken(),
-        id: template.id,
-      },
-    });
-    const body = await res.json();
-    expect(res.status).toEqual(200);
-    expect(body.data.archivedAt).toEqual(null);
-    await collection.reload();
-    expect(collection.documentStructure).toEqual(null);
-  });
-
   it("should restore archived when previous parent is archived", async () => {
     const user = await buildUser();
     const document = await buildDocument({

server/routes/api/groups/__snapshots__/groups.test.ts.snap

@@ -2,8 +2,8 @@
 
 exports[`#groups.add_user should require admin 1`] = `
 {
-  "error": "admin_required",
-  "message": "An admin role is required to access this resource",
+  "error": "authorization_error",
+  "message": "Authorization error",
   "ok": false,
   "status": 403,
 }
@@ -56,8 +56,8 @@ exports[`#groups.memberships should require authentication 1`] = `
 
 exports[`#groups.remove_user should require admin 1`] = `
 {
-  "error": "admin_required",
-  "message": "An admin role is required to access this resource",
+  "error": "authorization_error",
+  "message": "Authorization error",
   "ok": false,
   "status": 403,
 }

server/routes/api/groups/groups.ts

@@ -27,6 +27,7 @@ router.post(
   async (ctx: APIContext<T.GroupsListReq>) => {
     const { direction, sort, userId, name } = ctx.input.body;
     const { user } = ctx.state.auth;
+    authorize(user, "listGroups", user.team);
 
     let where: WhereOptions<Group> = {
       teamId: user.teamId,

server/routes/api/revisions/revisions.ts

@@ -32,14 +32,14 @@ router.post(
       const document = await Document.findByPk(revision.documentId, {
         userId: user.id,
       });
-      authorize(user, "read", document);
+      authorize(user, "listRevisions", document);
       after = revision;
       before = await revision.before();
     } else if (documentId) {
       const document = await Document.findByPk(documentId, {
         userId: user.id,
       });
-      authorize(user, "read", document);
+      authorize(user, "listRevisions", document);
       after = Revision.buildFromDocument(document);
       after.id = RevisionHelper.latestId(document.id);
       after.user = document.updatedBy;
@@ -75,7 +75,7 @@ router.post(
     const document = await Document.findByPk(revision.documentId, {
       userId: user.id,
     });
-    authorize(user, "read", document);
+    authorize(user, "listRevisions", document);
 
     let before;
     if (compareToId) {
@@ -126,7 +126,7 @@ router.post(
     const document = await Document.findByPk(documentId, {
       userId: user.id,
     });
-    authorize(user, "read", document);
+    authorize(user, "listRevisions", document);
 
     const revisions = await Revision.findAll({
       where: {

server/routes/api/shares/shares.ts

@@ -99,6 +99,8 @@ router.post(
   async (ctx: APIContext<T.SharesListReq>) => {
     const { sort, direction } = ctx.input.body;
     const { user } = ctx.state.auth;
+    authorize(user, "listShares", user.team);
+
     const where: WhereOptions<Share> = {
       teamId: user.teamId,
       userId: user.id,
@@ -169,6 +171,8 @@ router.post(
     const { documentId, published, urlId, includeChildDocuments } =
       ctx.input.body;
     const { user } = ctx.state.auth;
+    authorize(user, "createShare", user.team);
+
     const document = await Document.findByPk(documentId, {
       userId: user.id,
     });

server/routes/api/users/__snapshots__/users.test.ts.snap

@@ -2,8 +2,8 @@
 
 exports[`#users.activate should require admin 1`] = `
 {
-  "error": "admin_required",
-  "message": "An admin role is required to access this resource",
+  "error": "authorization_error",
+  "message": "Authorization error",
   "ok": false,
   "status": 403,
 }
@@ -29,8 +29,8 @@ exports[`#users.demote should not allow demoting self 1`] = `
 
 exports[`#users.demote should require admin 1`] = `
 {
-  "error": "admin_required",
-  "message": "An admin role is required to access this resource",
+  "error": "authorization_error",
+  "message": "Authorization error",
   "ok": false,
   "status": 403,
 }
@@ -38,8 +38,8 @@ exports[`#users.demote should require admin 1`] = `
 
 exports[`#users.promote should require admin 1`] = `
 {
-  "error": "admin_required",
-  "message": "An admin role is required to access this resource",
+  "error": "authorization_error",
+  "message": "Authorization error",
   "ok": false,
   "status": 403,
 }
@@ -56,8 +56,8 @@ exports[`#users.suspend should not allow suspending the user themselves 1`] = `
 
 exports[`#users.suspend should require admin 1`] = `
 {
-  "error": "admin_required",
-  "message": "An admin role is required to access this resource",
+  "error": "authorization_error",
+  "message": "Authorization error",
   "ok": false,
   "status": 403,
 }

server/routes/api/users/users.test.ts

@@ -182,7 +182,7 @@ describe("#users.list", () => {
     expect(res.status).toEqual(200);
     expect(body.data.length).toEqual(2);
     expect(body.data[0].email).toEqual(undefined);
-    expect(body.data[1].email).toEqual(undefined);
+    expect(body.data[1].email).toEqual(user.email);
   });
 });
 

server/routes/api/views/views.ts

@@ -23,7 +23,7 @@ router.post(
     const document = await Document.findByPk(documentId, {
       userId: user.id,
     });
-    authorize(user, "read", document);
+    authorize(user, "listViews", document);
 
     if (!document.insightsEnabled) {
       throw ValidationError("Insights are not enabled for this document");

shared/i18n/locales/en_US/translation.json

@@ -798,6 +798,7 @@
   "Where do I find the file?": "Where do I find the file?",
   "In Notion, click <em>Settings & Members</em> in the left sidebar and open Settings. Look for the Export section, and click <em>Export all workspace content</em>. Choose <em>HTML</em> as the format for the best data compatability.": "In Notion, click <em>Settings & Members</em> in the left sidebar and open Settings. Look for the Export section, and click <em>Export all workspace content</em>. Choose <em>HTML</em> as the format for the best data compatability.",
   "Last active": "Last active",
+  "Guest": "Guest",
   "Shared": "Shared",
   "by {{ name }}": "by {{ name }}",
   "Last accessed": "Last accessed",

shared/types.ts

@@ -2,6 +2,7 @@ export enum UserRole {
   Admin = "admin",
   Member = "member",
   Viewer = "viewer",
+  Guest = "guest",
 }
 
 export type DateFilter = "day" | "week" | "month" | "year";