From 48c7bd990a9380bda737cc964cc8809f47d1c5c4 Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Wed, 11 Mar 2026 23:55:45 -0400 Subject: [PATCH] fix: Incorrect policy on file operations (#11728) --- server/policies/fileOperation.ts | 9 +- .../api/fileOperations/fileOperations.test.ts | 119 +++++++++++++++++- .../api/fileOperations/fileOperations.ts | 6 +- 3 files changed, 127 insertions(+), 7 deletions(-) diff --git a/server/policies/fileOperation.ts b/server/policies/fileOperation.ts index 429d03b82c..fc756f732a 100644 --- a/server/policies/fileOperation.ts +++ b/server/policies/fileOperation.ts @@ -1,7 +1,7 @@ import { FileOperationState, FileOperationType } from "@shared/types"; import { User, Team, FileOperation } from "@server/models"; import { allow } from "./cancan"; -import { and, isTeamAdmin, isTeamMutable, or } from "./utils"; +import { and, isTeamAdmin, isTeamModel, isTeamMutable, or } from "./utils"; allow( User, @@ -11,7 +11,12 @@ allow( isTeamAdmin ); -allow(User, "read", FileOperation, isTeamAdmin); +allow(User, "read", FileOperation, (actor, fileOperation) => + and( + isTeamModel(actor, fileOperation), + or(isTeamAdmin(actor, fileOperation), fileOperation?.userId === actor.id) + ) +); allow(User, "delete", FileOperation, (actor, fileOperation) => and( diff --git a/server/routes/api/fileOperations/fileOperations.test.ts b/server/routes/api/fileOperations/fileOperations.test.ts index 6edd2f9234..f0b27e3e43 100644 --- a/server/routes/api/fileOperations/fileOperations.test.ts +++ b/server/routes/api/fileOperations/fileOperations.test.ts @@ -37,11 +37,28 @@ describe("#fileOperations.info", () => { expect(body.data.state).toBe(exportData.state); }); - it("should require user to be an admin", async () => { + it("should allow user to read their own export", async () => { const team = await buildTeam(); - const admin = await buildAdmin({ + const user = await buildUser({ teamId: team.id }); + const exportData = await buildFileOperation({ + type: FileOperationType.Export, teamId: team.id, + userId: user.id, }); + const res = await server.post("/api/fileOperations.info", { + body: { + id: exportData.id, + token: user.getJwtToken(), + }, + }); + const body = await res.json(); + expect(res.status).toEqual(200); + expect(body.data.id).toBe(exportData.id); + }); + + it("should not allow user to read another user's export", 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, @@ -57,6 +74,46 @@ describe("#fileOperations.info", () => { expect(res.status).toEqual(403); }); + it("should allow admin to read another admin's export", async () => { + const team = await buildTeam(); + const admin1 = await buildAdmin({ teamId: team.id }); + const admin2 = await buildAdmin({ teamId: team.id }); + const exportData = await buildFileOperation({ + type: FileOperationType.Export, + teamId: team.id, + userId: admin1.id, + }); + const res = await server.post("/api/fileOperations.info", { + body: { + id: exportData.id, + token: admin2.getJwtToken(), + }, + }); + const body = await res.json(); + expect(res.status).toEqual(200); + expect(body.data.id).toBe(exportData.id); + }); + + it("should allow admin to read an import file operation", async () => { + const team = await buildTeam(); + const admin1 = await buildAdmin({ teamId: team.id }); + const admin2 = await buildAdmin({ teamId: team.id }); + const importOp = await buildFileOperation({ + type: FileOperationType.Import, + teamId: team.id, + userId: admin1.id, + }); + const res = await server.post("/api/fileOperations.info", { + body: { + id: importOp.id, + token: admin2.getJwtToken(), + }, + }); + const body = await res.json(); + expect(res.status).toEqual(200); + expect(body.data.id).toBe(importOp.id); + }); + it("should require authorization", async () => { const team = await buildTeam(); const admin = await buildAdmin(); @@ -240,6 +297,64 @@ describe("#fileOperations.redirect", () => { expect(body.message).toEqual("export is not complete yet"); }); + it("should allow admin to redirect another admin's export", async () => { + const team = await buildTeam(); + const admin1 = await buildAdmin({ teamId: team.id }); + const admin2 = await buildAdmin({ teamId: team.id }); + const exportData = await buildFileOperation({ + state: FileOperationState.Complete, + type: FileOperationType.Export, + teamId: team.id, + userId: admin1.id, + }); + const res = await server.post("/api/fileOperations.redirect", { + body: { + token: admin2.getJwtToken(), + id: exportData.id, + }, + redirect: "manual", + }); + expect(res.status).toEqual(302); + }); + + it("should allow user to redirect their own export", async () => { + const team = await buildTeam(); + const user = await buildUser({ teamId: team.id }); + const exportData = await buildFileOperation({ + state: FileOperationState.Complete, + type: FileOperationType.Export, + teamId: team.id, + userId: user.id, + }); + const res = await server.post("/api/fileOperations.redirect", { + body: { + token: user.getJwtToken(), + id: exportData.id, + }, + redirect: "manual", + }); + expect(res.status).toEqual(302); + }); + + it("should not allow user to redirect another user's export", async () => { + const team = await buildTeam(); + const admin = await buildAdmin({ teamId: team.id }); + const user = await buildUser({ teamId: team.id }); + const exportData = await buildFileOperation({ + state: FileOperationState.Complete, + type: FileOperationType.Export, + teamId: team.id, + userId: admin.id, + }); + const res = await server.post("/api/fileOperations.redirect", { + body: { + token: user.getJwtToken(), + id: exportData.id, + }, + }); + expect(res.status).toEqual(403); + }); + it("should require authorization", async () => { const team = await buildTeam(); const user = await buildUser({ teamId: team.id }); diff --git a/server/routes/api/fileOperations/fileOperations.ts b/server/routes/api/fileOperations/fileOperations.ts index a5621177b6..67ebc15eb9 100644 --- a/server/routes/api/fileOperations/fileOperations.ts +++ b/server/routes/api/fileOperations/fileOperations.ts @@ -17,7 +17,7 @@ const router = new Router(); router.post( "fileOperations.info", - auth({ role: UserRole.Admin }), + auth(), validate(T.FileOperationsInfoSchema), async (ctx: APIContext) => { const { id } = ctx.input.body; @@ -92,13 +92,13 @@ const handleFileOperationsRedirect = async ( router.get( "fileOperations.redirect", - auth({ role: UserRole.Admin }), + auth(), validate(T.FileOperationsRedirectSchema), handleFileOperationsRedirect ); router.post( "fileOperations.redirect", - auth({ role: UserRole.Admin }), + auth(), validate(T.FileOperationsRedirectSchema), handleFileOperationsRedirect );