chore: Normalize permission logic between API/MCP doc creation (#12517)

server/commands/documentCreator.ts

@@ -1,9 +1,11 @@
 import type { Optional } from "utility-types";
 import { TextHelper } from "@shared/utils/TextHelper";
-import { Document, type Template } from "@server/models";
+import { Collection, Document, type Template } from "@server/models";
 import { DocumentHelper } from "@server/models/helpers/DocumentHelper";
 import { ProsemirrorHelper } from "@server/models/helpers/ProsemirrorHelper";
+import { authorize } from "@server/policies";
 import type { APIContext } from "@server/types";
+import { assertPresent } from "@server/validation";
 
 type Props = Optional<
   Pick<
@@ -35,6 +37,113 @@ type Props = Optional<
   index?: number;
 };
 
+type CreateLocation = {
+  /** The collection to place the document in, if any. */
+  collectionId?: string | null;
+  /** The parent document to nest the new document under, if any. */
+  parentDocumentId?: string | null;
+};
+
+/**
+ * Authorizes the creation of a document at the requested location and resolves
+ * the collection and parent document it will belong to. Shared by the
+ * documents.create API route and the MCP create_document tool so that both
+ * enforce identical permissions, including the team-level check that prevents
+ * viewers and guests from creating drafts with no collection.
+ *
+ * @param ctx the API context containing the acting user.
+ * @param location the requested collection and/or parent document.
+ * @returns the resolved collection and parent document, when applicable.
+ * @throws AuthorizationError when the user may not create the document.
+ */
+export async function authorizeDocumentCreate(
+  ctx: APIContext,
+  { collectionId, parentDocumentId }: CreateLocation
+): Promise<{
+  collection?: Collection | null;
+  parentDocument?: Document | null;
+}> {
+  const { user } = ctx.state.auth;
+  const { transaction } = ctx.state;
+
+  if (parentDocumentId) {
+    const parentDocument = await Document.findByPk(parentDocumentId, {
+      userId: user.id,
+      transaction,
+    });
+    const collection = parentDocument?.collectionId
+      ? await Collection.findByPk(parentDocument.collectionId, {
+          userId: user.id,
+          transaction,
+        })
+      : undefined;
+    authorize(user, "createChildDocument", parentDocument, { collection });
+    return { collection, parentDocument };
+  }
+
+  if (collectionId) {
+    const collection = await Collection.findByPk(collectionId, {
+      userId: user.id,
+      transaction,
+    });
+    authorize(user, "createDocument", collection);
+    return { collection };
+  }
+
+  authorize(user, "createDocument", user.team);
+  return {};
+}
+
+/**
+ * Authorizes publishing a document into a collection and resolves the target
+ * collection. Shared by the documents.update API route and the MCP
+ * update_document tool. Publishing places a document into a collection, so it
+ * requires create permission on the destination — separate from the update
+ * permission that governs editing a draft's content.
+ *
+ * @param ctx the API context containing the acting user.
+ * @param document the document being published.
+ * @param collectionId the destination collection, required when publishing a draft that has none.
+ * @returns the resolved destination collection.
+ * @throws AuthorizationError when the user may not publish into the collection.
+ */
+export async function authorizeDocumentPublish(
+  ctx: APIContext,
+  document: Document,
+  collectionId?: string | null
+): Promise<Collection | null | undefined> {
+  const { user } = ctx.state.auth;
+  const { transaction } = ctx.state;
+  let collection = document.collection;
+
+  if (document.isDraft) {
+    authorize(user, "publish", document);
+  }
+
+  if (!document.collectionId) {
+    assertPresent(
+      collectionId,
+      "collectionId is required to publish a draft without collection"
+    );
+    collection = await Collection.findByPk(collectionId!, {
+      userId: user.id,
+      transaction,
+    });
+  }
+
+  if (document.parentDocumentId) {
+    const parentDocument = await Document.findByPk(document.parentDocumentId, {
+      userId: user.id,
+      transaction,
+    });
+    authorize(user, "createChildDocument", parentDocument, { collection });
+  } else {
+    authorize(user, "createDocument", collection);
+  }
+
+  return collection;
+}
+
 export default async function documentCreator(
   ctx: APIContext,
   {

server/routes/api/documents/documents.ts

@@ -21,7 +21,10 @@ import {
 import { subtractDate } from "@shared/utils/date";
 import slugify from "@shared/utils/slugify";
 import { Day } from "@shared/utils/time";
-import documentCreator from "@server/commands/documentCreator";
+import documentCreator, {
+  authorizeDocumentCreate,
+  authorizeDocumentPublish,
+} from "@server/commands/documentCreator";
 import documentDuplicator from "@server/commands/documentDuplicator";
 import documentLoader from "@server/commands/documentLoader";
 import documentMover from "@server/commands/documentMover";
@@ -1324,33 +1327,7 @@ router.post(
     }
 
     if (publish) {
-      if (document.isDraft) {
-        authorize(user, "publish", document);
-      }
-
-      if (!document.collectionId) {
-        assertPresent(
-          collectionId,
-          "collectionId is required to publish a draft without collection"
-        );
-        collection = await Collection.findByPk(collectionId!, {
-          userId: user.id,
-          transaction,
-        });
-      }
-
-      if (document.parentDocumentId) {
-        const parentDocument = await Document.findByPk(
-          document.parentDocumentId,
-          {
-            userId: user.id,
-            transaction,
-          }
-        );
-        authorize(user, "createChildDocument", parentDocument, { collection });
-      } else {
-        authorize(user, "createDocument", collection);
-      }
+      await authorizeDocumentPublish(ctx, document, collectionId);
     }
 
     document = await documentUpdater(ctx, {
@@ -1704,31 +1681,10 @@ router.post(
     const { transaction } = ctx.state;
     const { user } = ctx.state.auth;
 
-    let collection;
-
-    let parentDocument;
-
-    if (parentDocumentId) {
-      parentDocument = await Document.findByPk(parentDocumentId, {
-        userId: user.id,
-      });
-
-      if (parentDocument?.collectionId) {
-        collection = await Collection.findByPk(parentDocument.collectionId, {
-          userId: user.id,
-        });
-      }
-
-      authorize(user, "createChildDocument", parentDocument, {
-        collection,
-      });
-    } else if (collectionId) {
-      collection = await Collection.findByPk(collectionId, {
-        userId: user.id,
-        transaction,
-      });
-      authorize(user, "createDocument", collection);
-    }
+    const { collection } = await authorizeDocumentCreate(ctx, {
+      collectionId,
+      parentDocumentId,
+    });
 
     let template: Template | null | undefined;
 

server/tools/documents.test.ts

@@ -1,10 +1,12 @@
 import { CollectionPermission, Scope } from "@shared/types";
 import {
   buildUser,
+  buildViewer,
   buildCollection,
   buildDocument,
   buildOAuthAuthentication,
 } from "@server/test/factories";
+import { Document } from "@server/models";
 import { getTestServer } from "@server/test/support";
 import { buildOAuthUser, callMcpTool } from "@server/test/McpHelper";
 
@@ -186,6 +188,61 @@ describe("create_document", () => {
     expect(data.document.title).toEqual("Child Document");
     expect(data.document.parentDocumentId).toEqual(parent.id);
   });
+
+  it("does not allow a viewer to create a draft", async () => {
+    const user = await buildViewer();
+    const auth = await buildOAuthAuthentication({
+      user,
+      scope: [Scope.Read, Scope.Write, Scope.Create],
+    });
+
+    const res = await callMcpTool(
+      server,
+      auth.accessToken!,
+      "create_document",
+      {
+        title: "Viewer Draft",
+        text: "Should not be created",
+        publish: false,
+      }
+    );
+
+    expect(res?.result?.isError).toBe(true);
+
+    const documents = await Document.unscoped().findAll({
+      where: { createdById: user.id },
+    });
+    expect(documents.length).toEqual(0);
+  });
+
+  it("does not allow a viewer to create a document in a collection", async () => {
+    const user = await buildViewer();
+    const collection = await buildCollection({
+      teamId: user.teamId,
+      permission: CollectionPermission.ReadWrite,
+    });
+    const auth = await buildOAuthAuthentication({
+      user,
+      scope: [Scope.Read, Scope.Write, Scope.Create],
+    });
+
+    const res = await callMcpTool(
+      server,
+      auth.accessToken!,
+      "create_document",
+      {
+        title: "Viewer Document",
+        collectionId: collection.id,
+      }
+    );
+
+    expect(res?.result?.isError).toBe(true);
+
+    const documents = await Document.unscoped().findAll({
+      where: { createdById: user.id },
+    });
+    expect(documents.length).toEqual(0);
+  });
 });
 
 describe("update_document", () => {
@@ -259,6 +316,41 @@ describe("update_document", () => {
 
     expect(res?.result?.isError).toBe(true);
   });
+
+  it("does not allow a viewer to publish their draft into a restricted collection", async () => {
+    const viewer = await buildViewer();
+    const collection = await buildCollection({
+      teamId: viewer.teamId,
+      permission: CollectionPermission.ReadWrite,
+    });
+    const draft = await buildDocument({
+      teamId: viewer.teamId,
+      userId: viewer.id,
+      collectionId: null,
+      publishedAt: null,
+    });
+    const auth = await buildOAuthAuthentication({
+      user: viewer,
+      scope: [Scope.Read, Scope.Write, Scope.Create],
+    });
+
+    const res = await callMcpTool(
+      server,
+      auth.accessToken!,
+      "update_document",
+      {
+        id: draft.id,
+        publish: true,
+        collectionId: collection.id,
+      }
+    );
+
+    expect(res?.result?.isError).toBe(true);
+
+    const reloaded = await Document.unscoped().findByPk(draft.id);
+    expect(reloaded?.collectionId).toBeNull();
+    expect(reloaded?.publishedAt).toBeNull();
+  });
 });
 
 describe("move_document", () => {

server/tools/documents.ts

@@ -1,7 +1,10 @@
 import { z } from "zod";
 import { type McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
 import { type CallToolResult } from "@modelcontextprotocol/sdk/types.js";
-import documentCreator from "@server/commands/documentCreator";
+import documentCreator, {
+  authorizeDocumentCreate,
+  authorizeDocumentPublish,
+} from "@server/commands/documentCreator";
 import documentMover from "@server/commands/documentMover";
 import documentUpdater from "@server/commands/documentUpdater";
 import { Op } from "sequelize";
@@ -339,30 +342,11 @@ export function documentTools(server: McpServer, scopes: string[]) {
           const { collectionId, parentDocumentId } = input;
           const ctx = buildAPIContext(context);
           const { user } = ctx.state.auth;
-          let collection;
-          let parentDocument;
 
-          if (parentDocumentId) {
-            parentDocument = await Document.findByPk(parentDocumentId, {
-              userId: user.id,
-            });
-
-            if (parentDocument?.collectionId) {
-              collection = await Collection.findByPk(
-                parentDocument.collectionId,
-                { userId: user.id }
-              );
-            }
-
-            authorize(user, "createChildDocument", parentDocument, {
-              collection,
-            });
-          } else if (collectionId) {
-            collection = await Collection.findByPk(collectionId, {
-              userId: user.id,
-            });
-            authorize(user, "createDocument", collection);
-          }
+          const { collection } = await authorizeDocumentCreate(ctx, {
+            collectionId,
+            parentDocumentId,
+          });
 
           const document = await documentCreator(ctx, {
             title: input.title,
@@ -619,6 +603,10 @@ export function documentTools(server: McpServer, scopes: string[]) {
           } else {
             authorize(user, "update", document);
 
+            if (input.publish) {
+              await authorizeDocumentPublish(ctx, document, input.collectionId);
+            }
+
             updated = await documentUpdater(ctx, {
               document,
               ...input,