Separates policy for file operations

app/hooks/useAuthorizedSettingsConfig.ts

@@ -149,7 +149,7 @@ const useAuthorizedSettingsConfig = () => {
         name: t("Import"),
         path: "/settings/import",
         component: Import,
-        enabled: can.manage,
+        enabled: can.createImport,
         group: t("Team"),
         icon: NewDocumentIcon,
       },
@@ -157,7 +157,7 @@ const useAuthorizedSettingsConfig = () => {
         name: t("Export"),
         path: "/settings/export",
         component: Export,
-        enabled: can.export,
+        enabled: can.createExport,
         group: t("Team"),
         icon: DownloadIcon,
       },
@@ -190,8 +190,8 @@ const useAuthorizedSettingsConfig = () => {
     [
       can.createApiKey,
       can.createWebhookSubscription,
-      can.export,
-      can.manage,
+      can.createExport,
+      can.createImport,
       can.update,
       t,
     ]

server/policies/fileOperation.ts

@@ -0,0 +1,21 @@
+import { User, Team, FileOperation } from "@server/models";
+import { allow } from "./cancan";
+
+allow(
+  User,
+  ["createFileOperation", "createImport", "createExport"],
+  Team,
+  (user, team) => {
+    if (!team || user.isViewer || user.teamId !== team.id) {
+      return false;
+    }
+    return user.isAdmin;
+  }
+);
+
+allow(User, ["read", "delete"], FileOperation, (user, fileOperation) => {
+  if (!fileOperation || user.isViewer || user.teamId !== fileOperation.teamId) {
+    return false;
+  }
+  return user.isAdmin;
+});

server/policies/index.ts

@@ -1,5 +1,6 @@
 import {
   Attachment,
+  FileOperation,
   Team,
   User,
   Collection,
@@ -12,6 +13,7 @@ import "./attachment";
 import "./authenticationProvider";
 import "./collection";
 import "./document";
+import "./fileOperation";
 import "./integration";
 import "./notificationSetting";
 import "./pins";
@@ -42,7 +44,15 @@ export const abilities = _abilities;
  */
 export function serialize(
   model: User,
-  target: Attachment | Team | Collection | Document | User | Group | null
+  target:
+    | Attachment
+    | FileOperation
+    | Team
+    | Collection
+    | Document
+    | User
+    | Group
+    | null
 ): Policy {
   const output = {};
   abilities.forEach((ability) => {

server/policies/team.ts

@@ -10,7 +10,7 @@ allow(User, "share", Team, (user, team) => {
   return team.sharing;
 });
 
-allow(User, ["update", "export", "manage"], Team, (user, team) => {
+allow(User, ["update", "manage"], Team, (user, team) => {
   if (!team || user.isViewer || user.teamId !== team.id) {
     return false;
   }

server/routes/api/collections.ts

@@ -488,7 +488,7 @@ router.post("collections.export", auth(), async (ctx) => {
   assertUuid(id, "id is required");
   const { user } = ctx.state;
   const team = await Team.findByPk(user.teamId);
-  authorize(user, "export", team);
+  authorize(user, "createExport", team);
 
   const collection = await Collection.scope({
     method: ["withMembership", user.id],
@@ -516,7 +516,7 @@ router.post("collections.export", auth(), async (ctx) => {
 router.post("collections.export_all", auth(), async (ctx) => {
   const { user } = ctx.state;
   const team = await Team.findByPk(user.teamId);
-  authorize(user, "export", team);
+  authorize(user, "createExport", team);
 
   const fileOperation = await sequelize.transaction(async (transaction) => {
     return collectionExporter({

server/routes/api/fileOperations.test.ts

@@ -17,6 +17,8 @@ import { flushdb } from "@server/test/support";
 const app = webService();
 const server = new TestServer(app.callback());
 
+jest.mock("@server/utils/s3");
+
 beforeEach(() => flushdb());
 afterAll(() => server.close());
 
@@ -35,7 +37,6 @@ describe("#fileOperations.info", () => {
       body: {
         id: exportData.id,
         token: admin.getJwtToken(),
-        type: FileOperationType.Export,
       },
     });
     const body = await res.json();
@@ -61,7 +62,26 @@ describe("#fileOperations.info", () => {
       body: {
         id: exportData.id,
         token: user.getJwtToken(),
-        type: FileOperationType.Export,
+      },
+    });
+    expect(res.status).toEqual(403);
+  });
+
+  it("should require authorization", async () => {
+    const team = await buildTeam();
+    const admin = await buildAdmin();
+    await buildUser({
+      teamId: team.id,
+    });
+    const exportData = await buildFileOperation({
+      type: FileOperationType.Export,
+      teamId: team.id,
+      userId: admin.id,
+    });
+    const res = await server.post("/api/fileOperations.info", {
+      body: {
+        id: exportData.id,
+        token: admin.getJwtToken(),
       },
     });
     expect(res.status).toEqual(403);
@@ -196,7 +216,7 @@ describe("#fileOperations.list", () => {
     expect(data.user.id).toBe(admin.id);
   });
 
-  it("should require authorization", async () => {
+  it("should require admin", async () => {
     const user = await buildUser();
     const res = await server.post("/api/fileOperations.list", {
       body: {
@@ -229,51 +249,26 @@ describe("#fileOperations.redirect", () => {
     expect(res.status).toEqual(400);
     expect(body.message).toEqual("export is not complete yet");
   });
-});
 
-describe("#fileOperations.info", () => {
-  it("should return file operation", async () => {
+  it("should require authorization", async () => {
     const team = await buildTeam();
-    const admin = await buildAdmin({
+    const user = await buildUser({
       teamId: team.id,
     });
+    const admin = await buildAdmin();
     const exportData = await buildFileOperation({
+      state: FileOperationState.Complete,
       type: FileOperationType.Export,
       teamId: team.id,
-      userId: admin.id,
+      userId: user.id,
     });
-    const res = await server.post("/api/fileOperations.info", {
+    const res = await server.post("/api/fileOperations.redirect", {
       body: {
         token: admin.getJwtToken(),
         id: exportData.id,
       },
     });
-    const body = await res.json();
-    expect(res.status).toBe(200);
-    expect(body.data.id).toBe(exportData.id);
-    expect(body.data.user.id).toBe(admin.id);
-  });
-
-  it("should require authorization", async () => {
-    const team = await buildTeam();
-    const admin = await buildAdmin({
-      teamId: team.id,
-    });
-    const user = await buildUser({
-      teamId: team.id,
-    });
-    const exportData = await buildFileOperation({
-      type: FileOperationType.Export,
-      teamId: team.id,
-      userId: admin.id,
-    });
-    const res = await server.post("/api/fileOperations.info", {
-      body: {
-        token: user.getJwtToken(),
-        id: exportData.id,
-      },
-    });
-    expect(res.status).toBe(403);
+    expect(res.status).toEqual(403);
   });
 });
 
@@ -299,4 +294,24 @@ describe("#fileOperations.delete", () => {
     expect(await Event.count()).toBe(1);
     expect(await FileOperation.count()).toBe(0);
   });
+
+  it("should require authorization", async () => {
+    const team = await buildTeam();
+    const user = await buildUser({
+      teamId: team.id,
+    });
+    const admin = await buildAdmin();
+    const exportData = await buildFileOperation({
+      type: FileOperationType.Export,
+      teamId: team.id,
+      userId: user.id,
+    });
+    const res = await server.post("/api/fileOperations.delete", {
+      body: {
+        token: admin.getJwtToken(),
+        id: exportData.id,
+      },
+    });
+    expect(res.status).toEqual(403);
+  });
 });

server/routes/api/fileOperations.ts

@@ -1,13 +1,14 @@
 import Router from "koa-router";
 import { WhereOptions } from "sequelize/types";
 import fileOperationDeleter from "@server/commands/fileOperationDeleter";
-import { NotFoundError, ValidationError } from "@server/errors";
+import { ValidationError } from "@server/errors";
 import auth from "@server/middlewares/authentication";
 import { FileOperation, Team } from "@server/models";
+import { FileOperationType } from "@server/models/FileOperation";
 import { authorize } from "@server/policies";
 import { presentFileOperation } from "@server/presenters";
 import { getSignedUrl } from "@server/utils/s3";
-import { assertPresent, assertIn, assertUuid } from "@server/validation";
+import { assertIn, assertSort, assertUuid } from "@server/validation";
 import pagination from "./middlewares/pagination";
 
 const router = new Router();
@@ -16,16 +17,11 @@ router.post("fileOperations.info", auth(), async (ctx) => {
   const { id } = ctx.body;
   assertUuid(id, "id is required");
   const { user } = ctx.state;
-  const team = await Team.findByPk(user.teamId);
   const fileOperation = await FileOperation.findByPk(id, {
     rejectOnEmpty: true,
   });
 
-  authorize(user, fileOperation.type, team);
-
-  if (!fileOperation) {
-    throw NotFoundError();
-  }
+  authorize(user, "read", fileOperation);
 
   ctx.body = {
     data: presentFileOperation(fileOperation),
@@ -35,12 +31,8 @@ router.post("fileOperations.info", auth(), async (ctx) => {
 router.post("fileOperations.list", auth(), pagination(), async (ctx) => {
   let { direction } = ctx.body;
   const { sort = "createdAt", type } = ctx.body;
-  assertPresent(type, "type is required");
-  assertIn(
-    type,
-    ["import", "export"],
-    "type must be one of 'import' or 'export'"
-  );
+  assertIn(type, Object.values(FileOperationType));
+  assertSort(sort, FileOperation);
 
   if (direction !== "ASC") {
     direction = "DESC";
@@ -51,7 +43,7 @@ router.post("fileOperations.list", auth(), pagination(), async (ctx) => {
     type,
   };
   const team = await Team.findByPk(user.teamId);
-  authorize(user, type, team);
+  authorize(user, "manage", team);
 
   const [exports, total] = await Promise.all([
     await FileOperation.findAll({
@@ -76,20 +68,16 @@ router.post("fileOperations.redirect", auth(), async (ctx) => {
   assertUuid(id, "id is required");
 
   const { user } = ctx.state;
-  const team = await Team.findByPk(user.teamId);
-  const fileOp = await FileOperation.unscoped().findByPk(id);
+  const fileOperation = await FileOperation.unscoped().findByPk(id, {
+    rejectOnEmpty: true,
+  });
+  authorize(user, "read", fileOperation);
 
-  if (!fileOp) {
-    throw NotFoundError();
+  if (fileOperation.state !== "complete") {
+    throw ValidationError(`${fileOperation.type} is not complete yet`);
   }
 
-  authorize(user, fileOp.type, team);
-
-  if (fileOp.state !== "complete") {
-    throw ValidationError(`${fileOp.type} is not complete yet`);
-  }
-
-  const accessUrl = await getSignedUrl(fileOp.key);
+  const accessUrl = await getSignedUrl(fileOperation.key);
   ctx.redirect(accessUrl);
 });
 
@@ -98,15 +86,12 @@ router.post("fileOperations.delete", auth(), async (ctx) => {
   assertUuid(id, "id is required");
 
   const { user } = ctx.state;
-  const team = await Team.findByPk(user.teamId);
-  const fileOp = await FileOperation.findByPk(id);
+  const fileOperation = await FileOperation.unscoped().findByPk(id, {
+    rejectOnEmpty: true,
+  });
+  authorize(user, "delete", fileOperation);
 
-  if (!fileOp) {
-    throw NotFoundError();
-  }
-
-  authorize(user, fileOp.type, team);
-  await fileOperationDeleter(fileOp, user, ctx.request.ip);
+  await fileOperationDeleter(fileOperation, user, ctx.request.ip);
 
   ctx.body = {
     success: true,

server/utils/__mocks__/s3.ts

@@ -1,3 +1,7 @@
 export const uploadToS3FromBuffer = jest.fn().mockReturnValue("/endpoint/key");
 
 export const publicS3Endpoint = jest.fn().mockReturnValue("http://mock");
+
+export const getSignedUrl = jest.fn().mockReturnValue("http://s3mock");
+
+export const getSignedUrlPromise = jest.fn().mockResolvedValue("http://s3mock");

server/validation.ts

@@ -22,7 +22,7 @@ export const assertIn = (
   message?: string
 ) => {
   if (!options.includes(value)) {
-    throw ValidationError(message);
+    throw ValidationError(message ?? `Must be one of ${options.join(", ")}`);
   }
 };