fix: Incorrect policy on file operations (#11728)

This commit is contained in:
Tom Moor
2026-03-11 23:55:45 -04:00
committed by GitHub
parent 54f2994b13
commit 48c7bd990a
3 changed files with 127 additions and 7 deletions
+7 -2
View File
@@ -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(
@@ -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 });
@@ -17,7 +17,7 @@ const router = new Router();
router.post(
"fileOperations.info",
auth({ role: UserRole.Admin }),
auth(),
validate(T.FileOperationsInfoSchema),
async (ctx: APIContext<T.FileOperationsInfoReq>) => {
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
);