fix: Cannot duplicate document from read-only collection (#13198)

* fix: Cannot duplicate document from read-only collection

* Improve validation

server/policies/document.ts

@@ -122,15 +122,14 @@ allow(User, "manageUsers", Document, (actor, document) =>
   )
 );
 
+// Note that read access to the source document is sufficient – the destination
+// collection is chosen separately and authorized at that point.
 allow(User, "duplicate", Document, (actor, document) =>
   and(
-    can(actor, "update", document),
-    or(
-      includesMembership(document, [DocumentPermission.Admin]),
-      and(isTeamAdmin(actor, document), can(actor, "read", document)),
-      can(actor, "updateDocument", document?.collection),
-      !!document?.isDraft && actor.id === document?.createdById
-    )
+    !!document?.isActive,
+    isTeamMutable(actor),
+    can(actor, "read", document),
+    can(actor, "createDocument", actor.team)
   )
 );
 

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

@@ -5829,6 +5829,207 @@ describe("#documents.duplicate", () => {
     expect(body.data.documents[0].fullWidth).toBe(true);
     expect(body.data.documents[1].fullWidth).toBe(true);
   });
+
+  it("should allow duplicating from a read-only collection into a writable one", async () => {
+    const user = await buildUser();
+    const source = await buildCollection({
+      teamId: user.teamId,
+      permission: CollectionPermission.Read,
+    });
+    const destination = await buildCollection({
+      teamId: user.teamId,
+      permission: CollectionPermission.ReadWrite,
+    });
+    const document = await buildDocument({
+      teamId: user.teamId,
+      collectionId: source.id,
+    });
+
+    const res = await server.post("/api/documents.duplicate", user, {
+      body: {
+        id: document.id,
+        collectionId: destination.id,
+      },
+    });
+    const body = await res.json();
+
+    expect(res.status).toEqual(200);
+    expect(body.data.documents).toHaveLength(1);
+    expect(body.data.documents[0].collectionId).toEqual(destination.id);
+  });
+
+  it("should allow duplicating from a read-only collection under a writable parent", async () => {
+    const user = await buildUser();
+    const source = await buildCollection({
+      teamId: user.teamId,
+      permission: CollectionPermission.Read,
+    });
+    const destination = await buildCollection({
+      teamId: user.teamId,
+      permission: CollectionPermission.ReadWrite,
+    });
+    const document = await buildDocument({
+      teamId: user.teamId,
+      collectionId: source.id,
+    });
+    const parent = await buildDocument({
+      teamId: user.teamId,
+      collectionId: destination.id,
+    });
+
+    const res = await server.post("/api/documents.duplicate", user, {
+      body: {
+        id: document.id,
+        collectionId: destination.id,
+        parentDocumentId: parent.id,
+      },
+    });
+    const body = await res.json();
+
+    expect(res.status).toEqual(200);
+    expect(body.data.documents).toHaveLength(1);
+    expect(body.data.documents[0].parentDocumentId).toEqual(parent.id);
+    expect(body.data.documents[0].collectionId).toEqual(destination.id);
+  });
+
+  it("should not allow a collectionId that disagrees with the parent document", async () => {
+    const user = await buildUser();
+    const source = await buildCollection({
+      teamId: user.teamId,
+      permission: CollectionPermission.Read,
+    });
+    const destination = await buildCollection({
+      teamId: user.teamId,
+      permission: CollectionPermission.ReadWrite,
+    });
+    const document = await buildDocument({
+      teamId: user.teamId,
+      collectionId: source.id,
+    });
+    const parent = await buildDocument({
+      teamId: user.teamId,
+      collectionId: destination.id,
+    });
+
+    const res = await server.post("/api/documents.duplicate", user, {
+      body: {
+        id: document.id,
+        // deliberately mismatched with the parent's collection
+        collectionId: source.id,
+        parentDocumentId: parent.id,
+      },
+    });
+    const body = await res.json();
+
+    expect(res.status).toEqual(400);
+    expect(body.message).toEqual(
+      "collectionId must match the collection of the parent document"
+    );
+  });
+
+  it("should place the copy in the parent's collection when no collectionId is given", async () => {
+    const user = await buildUser();
+    const source = await buildCollection({
+      teamId: user.teamId,
+      permission: CollectionPermission.Read,
+    });
+    const destination = await buildCollection({
+      teamId: user.teamId,
+      permission: CollectionPermission.ReadWrite,
+    });
+    const document = await buildDocument({
+      teamId: user.teamId,
+      collectionId: source.id,
+    });
+    const parent = await buildDocument({
+      teamId: user.teamId,
+      collectionId: destination.id,
+    });
+
+    const res = await server.post("/api/documents.duplicate", user, {
+      body: {
+        id: document.id,
+        parentDocumentId: parent.id,
+      },
+    });
+    const body = await res.json();
+
+    expect(res.status).toEqual(200);
+    expect(body.data.documents).toHaveLength(1);
+    expect(body.data.documents[0].collectionId).toEqual(destination.id);
+    expect(body.data.documents[0].parentDocumentId).toEqual(parent.id);
+  });
+
+  it("should not allow duplicating under a parent in a read-only collection", async () => {
+    const user = await buildUser();
+    const source = await buildCollection({
+      teamId: user.teamId,
+      permission: CollectionPermission.ReadWrite,
+    });
+    const destination = await buildCollection({
+      teamId: user.teamId,
+      permission: CollectionPermission.Read,
+    });
+    const document = await buildDocument({
+      teamId: user.teamId,
+      collectionId: source.id,
+    });
+    const parent = await buildDocument({
+      teamId: user.teamId,
+      collectionId: destination.id,
+    });
+
+    const res = await server.post("/api/documents.duplicate", user, {
+      body: {
+        id: document.id,
+        parentDocumentId: parent.id,
+      },
+    });
+
+    expect(res.status).toEqual(403);
+  });
+
+  it("should not allow duplicating into a read-only collection", async () => {
+    const user = await buildUser();
+    const collection = await buildCollection({
+      teamId: user.teamId,
+      permission: CollectionPermission.Read,
+    });
+    const document = await buildDocument({
+      teamId: user.teamId,
+      collectionId: collection.id,
+    });
+
+    const res = await server.post("/api/documents.duplicate", user, {
+      body: {
+        id: document.id,
+        collectionId: collection.id,
+      },
+    });
+
+    expect(res.status).toEqual(403);
+  });
+
+  it("should not allow a viewer to duplicate", async () => {
+    const user = await buildViewer();
+    const collection = await buildCollection({
+      teamId: user.teamId,
+      permission: CollectionPermission.ReadWrite,
+    });
+    const document = await buildDocument({
+      teamId: user.teamId,
+      collectionId: collection.id,
+    });
+
+    const res = await server.post("/api/documents.duplicate", user, {
+      body: {
+        id: document.id,
+        collectionId: collection.id,
+      },
+    });
+
+    expect(res.status).toEqual(403);
+  });
 });
 
 describe("#documents.empty_trash", () => {

server/routes/api/documents/documents.ts

@@ -1340,18 +1340,9 @@ router.post(
       userId: user.id,
       transaction,
     });
-    authorize(user, "read", document);
+    authorize(user, "duplicate", document);
 
-    const collection = collectionId
-      ? await Collection.findByPk(collectionId, {
-          userId: user.id,
-          transaction,
-        })
-      : document?.collection;
-
-    if (collection) {
-      authorize(user, "updateDocument", collection);
-    }
+    let collection: Collection | null | undefined;
 
     if (parentDocumentId) {
       const parent = await Document.findByPk(parentDocumentId, {
@@ -1363,6 +1354,34 @@ router.post(
       if (!parent.publishedAt) {
         throw InvalidRequestError("Cannot duplicate document inside a draft");
       }
+
+      if (collectionId && collectionId !== parent.collectionId) {
+        throw InvalidRequestError(
+          "collectionId must match the collection of the parent document"
+        );
+      }
+
+      // The copy is nested under the parent, so it belongs to the parent's
+      // collection.
+      collection = parent.collectionId
+        ? await Collection.findByPk(parent.collectionId, {
+            userId: user.id,
+            transaction,
+          })
+        : undefined;
+    } else {
+      collection = collectionId
+        ? await Collection.findByPk(collectionId, {
+            userId: user.id,
+            transaction,
+          })
+        : document?.collection;
+
+      // The copy is created in the destination collection, so create permission
+      // is required there rather than on the source.
+      if (collection) {
+        authorize(user, "createDocument", collection);
+      }
     }
 
     const response = await documentDuplicator(ctx, {