Skip to content

Commit

Permalink
fix: Add filetype validation to API (#4213)
Browse files Browse the repository at this point in the history
  • Loading branch information
DafyddLlyr authored Jan 27, 2025
1 parent de76aa6 commit b818e0c
Show file tree
Hide file tree
Showing 3 changed files with 135 additions and 30 deletions.
117 changes: 89 additions & 28 deletions api.planx.uk/modules/file/file.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,21 +39,84 @@ vi.mock("@aws-sdk/client-s3", async (importOriginal) => {
};
});

const PRIVATE_ENDPOINT = "/file/private/upload";
const PUBLIC_ENDPOINT = "/file/public/upload";

describe("File upload", () => {
beforeEach(() => {
vi.clearAllMocks();

mockPutObject = vi.fn(() => Promise.resolve());
});

describe("Private", () => {
const ENDPOINT = "/file/private/upload";
describe.each([PRIVATE_ENDPOINT, PUBLIC_ENDPOINT])(
"File type validation for %s",
(ENDPOINT) => {
it("should not upload a file with an invalid extension", async () => {
await supertest(app)
.post(ENDPOINT)
.field("filename", "")
.attach("file", Buffer.from("some data"), "some_file.xlxs")
.expect(500)
.then((res) => {
expect(mockPutObject).not.toHaveBeenCalled();
expect(res.body.error).toMatch(/Unsupported file type/);
});
});

it("should not upload a file with an invalid MIME type", async () => {
await supertest(app)
.post(ENDPOINT)
.field("filename", "")
.attach("file", Buffer.from("some data"), {
filename: "invalid_file.txt", // Invalid file type
contentType: "text/plain", // Invalid MIME type
})
.expect(500)
.then((res) => {
expect(mockPutObject).not.toHaveBeenCalled();
expect(res.body.error).toMatch(/Unsupported file type/);
});
});

it("should not upload a file a valid file type, but invalid MIME type", async () => {
await supertest(app)
.post(ENDPOINT)
.field("filename", "")
.attach("file", Buffer.from("some data"), {
filename: "invalid_file.png", // Valid file type
contentType: "text/plain", // Invalid MIME type
})
.expect(500)
.then((res) => {
expect(mockPutObject).not.toHaveBeenCalled();
expect(res.body.error).toMatch(/Unsupported file type/);
});
});

it("should not upload a file a valid MIME type, but invalid file type", async () => {
await supertest(app)
.post(ENDPOINT)
.field("filename", "")
.attach("file", Buffer.from("some data"), {
filename: "invalid_file.txt", // Invalid file type
contentType: "application/pdf", // Valid MIME type
})
.expect(500)
.then((res) => {
expect(mockPutObject).not.toHaveBeenCalled();
expect(res.body.error).toMatch(/Unsupported file type/);
});
});
},
);

describe("Private", () => {
it("should not upload without filename", async () => {
await supertest(app)
.post(ENDPOINT)
.post(PRIVATE_ENDPOINT)
.field("filename", "")
.attach("file", Buffer.from("some data"), "some_file.txt")
.attach("file", Buffer.from("some data"), "some_file.jpg")
.expect(400)
.then((res) => {
expect(mockPutObject).not.toHaveBeenCalled();
Expand All @@ -64,7 +127,7 @@ describe("File upload", () => {

it("should not upload without file", async () => {
await supertest(app)
.post(ENDPOINT)
.post(PRIVATE_ENDPOINT)
.field("filename", "some filename")
.expect(500)
.then((res) => {
Expand All @@ -78,12 +141,12 @@ describe("File upload", () => {
vi.stubEnv("AWS_S3_BUCKET", "myBucketName");

await supertest(app)
.post(ENDPOINT)
.field("filename", "some_file.txt")
.attach("file", Buffer.from("some data"), "some_file.txt")
.post(PRIVATE_ENDPOINT)
.field("filename", "some_file.jpg")
.attach("file", Buffer.from("some data"), "some_file.jpg")
.then((res) => {
expect(res.body).toEqual({
fileType: "text/plain",
fileType: "image/jpeg",
// Bucket name stripped from URL
fileUrl:
"https://api.editor.planx.dev/file/private/nanoid/modified%20key",
Expand All @@ -97,9 +160,9 @@ describe("File upload", () => {
mockPutObject = vi.fn(() => Promise.reject(new Error("S3 error!")));

await supertest(app)
.post("/file/private/upload")
.field("filename", "some_file.txt")
.attach("file", Buffer.from("some data"), "some_file.txt")
.post(PRIVATE_ENDPOINT)
.field("filename", "some_file.jpg")
.attach("file", Buffer.from("some data"), "some_file.jpg")
.expect(500)
.then((res) => {
expect(res.body.error).toMatch(/S3 error!/);
Expand All @@ -112,12 +175,12 @@ describe("File upload", () => {
vi.stubEnv("NODE_ENV", "production");

await supertest(app)
.post(ENDPOINT)
.field("filename", "some_file.txt")
.attach("file", Buffer.from("some data"), "some_file.txt")
.post(PRIVATE_ENDPOINT)
.field("filename", "some_file.jpg")
.attach("file", Buffer.from("some data"), "some_file.jpg")
.then((res) => {
expect(res.body).toEqual({
fileType: "text/plain",
fileType: "image/jpeg",
fileUrl:
"https://api.editor.planx.uk/file/private/nanoid/modified%20key",
});
Expand All @@ -128,8 +191,6 @@ describe("File upload", () => {
});

describe("Public", () => {
const ENDPOINT = "/file/public/upload";

const auth = authHeader({ role: "teamEditor" });

it("returns an error if authorization headers are not set", async () => {
Expand All @@ -145,10 +206,10 @@ describe("File upload", () => {

it("should not upload without filename", async () => {
await supertest(app)
.post(ENDPOINT)
.post(PUBLIC_ENDPOINT)
.set(auth)
.field("filename", "")
.attach("file", Buffer.from("some data"), "some_file.txt")
.attach("file", Buffer.from("some data"), "some_file.pdf")
.expect(400)
.then((res) => {
expect(mockPutObject).not.toHaveBeenCalled();
Expand All @@ -159,7 +220,7 @@ describe("File upload", () => {

it("should not upload without file", async () => {
await supertest(app)
.post(ENDPOINT)
.post(PUBLIC_ENDPOINT)
.set(auth)
.field("filename", "some filename")
.expect(500)
Expand All @@ -171,13 +232,13 @@ describe("File upload", () => {

it("should upload file", async () => {
await supertest(app)
.post(ENDPOINT)
.post(PUBLIC_ENDPOINT)
.set(auth)
.field("filename", "some_file.txt")
.attach("file", Buffer.from("some data"), "some_file.txt")
.field("filename", "some_file.pdf")
.attach("file", Buffer.from("some data"), "some_file.pdf")
.then((res) => {
expect(res.body).toEqual({
fileType: "text/plain",
fileType: "application/pdf",
fileUrl: expect.stringContaining(
"file/public/nanoid/modified%20key",
),
Expand All @@ -191,10 +252,10 @@ describe("File upload", () => {
mockPutObject = vi.fn(() => Promise.reject(new Error("S3 error!")));

await supertest(app)
.post(ENDPOINT)
.post(PUBLIC_ENDPOINT)
.set(auth)
.field("filename", "some_file.txt")
.attach("file", Buffer.from("some data"), "some_file.txt")
.field("filename", "some_file.pdf")
.attach("file", Buffer.from("some data"), "some_file.pdf")
.expect(500)
.then((res) => {
expect(res.body.error).toMatch(/S3 error!/);
Expand Down
43 changes: 43 additions & 0 deletions api.planx.uk/modules/file/middleware/useFileUpload.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import multer from "multer";
import path from "path";

/**
* 30mb to match limit set in frontend
* See editor.planx.uk/src/@planx/components/shared/PrivateFileUpload/Dropzone.tsx
*/
const FILE_SIZE_LIMIT = 30 * 1024 * 1024;

/**
* Should match MIME type restrictions in frontend
* See editor.planx.uk/src/@planx/components/shared/PrivateFileUpload/Dropzone.tsx
*/
const ALLOWED_MIME_TYPES = ["image/jpeg", "image/png", "application/pdf"];
const ALLOWED_EXTENSIONS = [".jpg", ".jpeg", ".png", ".pdf"];

const validateExtension = (filename: string): boolean => {
const extension = path.extname(filename).toLowerCase();
return ALLOWED_EXTENSIONS.includes(extension);
};

/**
* Filter out invalid files
*/
const fileFilter: multer.Options["fileFilter"] = (_req, file, callback) => {
const isValidMimeType = ALLOWED_MIME_TYPES.includes(file.mimetype);
const isValidExtension = validateExtension(file.originalname);

if (isValidMimeType && isValidExtension) {
callback(null, true);
} else {
callback(new Error("Unsupported file type"));
}
};

const multerOptions: multer.Options = {
limits: {
fileSize: FILE_SIZE_LIMIT,
},
fileFilter,
};

export const useFileUpload = multer(multerOptions).single("file");
5 changes: 3 additions & 2 deletions api.planx.uk/modules/file/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,21 @@ import {
uploadFileSchema,
} from "./controller.js";
import { validate } from "../../shared/middleware/validate.js";
import { useFileUpload } from "./middleware/useFileUpload.js";

const router = Router();

router.post(
"/file/public/upload",
multer().single("file"),
useFileUpload,
useTeamEditorAuth,
validate(uploadFileSchema),
publicUploadController,
);

router.post(
"/file/private/upload",
multer().single("file"),
useFileUpload,
validate(uploadFileSchema),
privateUploadController,
);
Expand Down

0 comments on commit b818e0c

Please sign in to comment.