fix: Attempting to restore document in deleted collection without a collectionId override results in server error (#1777)

closes #1767

server/api/documents.js

@@ -522,18 +522,27 @@ router.post("documents.restore", auth(), async (ctx) => {
     throw new NotFoundError();
   }
 
+  // Passing collectionId allows restoring to a different collection than the
+  // document was originally within
   if (collectionId) {
     ctx.assertUuid(collectionId, "collectionId must be a uuid");
-    authorize(user, "restore", document);
-
-    const collection = await Collection.scope({
-      method: ["withMembership", user.id],
-    }).findByPk(collectionId);
-    authorize(user, "update", collection);
-
     document.collectionId = collectionId;
   }
 
+  const collection = await Collection.scope({
+    method: ["withMembership", user.id],
+  }).findByPk(document.collectionId);
+
+  // if the collectionId was provided in the request and isn't valid then it will
+  // be caught as a 403 on the authorize call below. Otherwise we're checking here
+  // that the original collection still exists and advising to pass collectionId
+  // if not.
+  if (!collectionId) {
+    ctx.assertPresent(collection, "collectionId is required");
+  }
+
+  authorize(user, "update", collection);
+
   if (document.deletedAt) {
     authorize(user, "restore", document);
 

server/api/documents.test.js

@@ -1334,7 +1334,22 @@ describe("#documents.restore", () => {
     expect(body.data.collectionId).toEqual(collection.id);
   });
 
-  it("should now allow restore of trashed documents to collection user cannot access", async () => {
+  it("should not allow restore of documents in deleted collection", async () => {
+    const { user, document, collection } = await seed();
+
+    await document.destroy(user.id);
+    await collection.destroy();
+
+    const res = await server.post("/api/documents.restore", {
+      body: {
+        token: user.getJwtToken(),
+        id: document.id,
+      },
+    });
+    expect(res.status).toEqual(400);
+  });
+
+  it("should not allow restore of trashed documents to collection user cannot access", async () => {
     const { user, document } = await seed();
     const collection = await buildCollection();