diff --git a/plugins/storage/server/api/files.test.ts b/plugins/storage/server/api/files.test.ts index de7b5c877..3964221c5 100644 --- a/plugins/storage/server/api/files.test.ts +++ b/plugins/storage/server/api/files.test.ts @@ -24,30 +24,68 @@ describe("#files.create", () => { expect(res.status).toEqual(400); }); - it("should fail with status 404 if existing file is requested with key", async () => { + it("should fail with status 401 if associated attachment does not belong to user", async () => { const user = await buildUser(); const fileName = "images.docx"; - const key = path.join("uploads", user.id, uuidV4(), fileName); + const attachment = await buildAttachment( + { + teamId: user.teamId, + contentType: + "application/vnd.openxmlformats-officedocument.wordprocessingml.document", + }, + fileName + ); + + const content = await readFile( + path.resolve(__dirname, "..", "test", "fixtures", fileName) + ); + const form = new FormData(); + form.append("key", attachment.key); + form.append("file", content, fileName); + form.append("token", user.getJwtToken()); + + const res = await server.post(`/api/files.create`, { + headers: form.getHeaders(), + body: form, + }); + expect(res.status).toEqual(403); + }); + + it("should fail with status 401 if file exists on disk", async () => { + const user = await buildUser(); + const fileName = "images.docx"; + const attachment = await buildAttachment( + { + userId: user.id, + teamId: user.teamId, + contentType: + "application/vnd.openxmlformats-officedocument.wordprocessingml.document", + }, + fileName + ); ensureDirSync( - path.dirname(path.join(env.FILE_STORAGE_LOCAL_ROOT_DIR, key)) + path.dirname(path.join(env.FILE_STORAGE_LOCAL_ROOT_DIR, attachment.key)) ); copyFileSync( path.resolve(__dirname, "..", "test", "fixtures", fileName), - path.join(env.FILE_STORAGE_LOCAL_ROOT_DIR, key) + path.join(env.FILE_STORAGE_LOCAL_ROOT_DIR, attachment.key) ); - const res = await server.get(`/api/files.get?key=${key}`); - expect(res.status).toEqual(404); - }); + const content = await readFile( + path.resolve(__dirname, "..", "test", "fixtures", fileName) + ); + const form = new FormData(); + form.append("key", attachment.key); + form.append("file", content, fileName); + form.append("token", user.getJwtToken()); - it("should fail with status 404 if non-existing file is requested with key", async () => { - const user = await buildUser(); - const fileName = "images.docx"; - const key = path.join("uploads", user.id, uuidV4(), fileName); - const res = await server.get(`/api/files.get?key=${key}`); - expect(res.status).toEqual(404); + const res = await server.post(`/api/files.create`, { + headers: form.getHeaders(), + body: form, + }); + expect(res.status).toEqual(400); }); it("should succeed with status 200 ok and create a file", async () => { @@ -86,6 +124,32 @@ describe("#files.create", () => { }); describe("#files.get", () => { + it("should fail with status 404 if existing file is requested with key", async () => { + const user = await buildUser(); + const fileName = "images.docx"; + const key = path.join("uploads", user.id, uuidV4(), fileName); + + ensureDirSync( + path.dirname(path.join(env.FILE_STORAGE_LOCAL_ROOT_DIR, key)) + ); + + copyFileSync( + path.resolve(__dirname, "..", "test", "fixtures", fileName), + path.join(env.FILE_STORAGE_LOCAL_ROOT_DIR, key) + ); + + const res = await server.get(`/api/files.get?key=${key}`); + expect(res.status).toEqual(404); + }); + + it("should fail with status 404 if non-existing file is requested with key", async () => { + const user = await buildUser(); + const fileName = "images.docx"; + const key = path.join("uploads", user.id, uuidV4(), fileName); + const res = await server.get(`/api/files.get?key=${key}`); + expect(res.status).toEqual(404); + }); + it("should fail with status 400 bad request if key is invalid", async () => { const res = await server.get(`/api/files.get?key=public/foo/bar/baz.png`); expect(res.status).toEqual(400); diff --git a/plugins/storage/server/api/files.ts b/plugins/storage/server/api/files.ts index de59497e3..eccdfed20 100644 --- a/plugins/storage/server/api/files.ts +++ b/plugins/storage/server/api/files.ts @@ -2,7 +2,11 @@ import JWT from "jsonwebtoken"; import Router from "koa-router"; import mime from "mime-types"; import env from "@server/env"; -import { AuthenticationError, ValidationError } from "@server/errors"; +import { + AuthenticationError, + AuthorizationError, + ValidationError, +} from "@server/errors"; import auth from "@server/middlewares/authentication"; import multipart from "@server/middlewares/multipart"; import { rateLimiter } from "@server/middlewares/rateLimiter"; @@ -37,11 +41,11 @@ router.post( rejectOnEmpty: true, }); - if (attachment.isPrivate) { - authorize(actor, "createAttachment", actor.team); + if (attachment?.userId !== actor.id) { + throw AuthorizationError("Invalid key"); } - await attachment.overwriteFile(file); + await attachment.writeFile(file); ctx.body = { success: true, diff --git a/server/models/Attachment.ts b/server/models/Attachment.ts index 25e37eb08..c0d1a19da 100644 --- a/server/models/Attachment.ts +++ b/server/models/Attachment.ts @@ -116,12 +116,12 @@ class Attachment extends IdModel { /** * Store the given file in storage at the location specified by the attachment key. - * If the attachment already exists, it will be overwritten. + * If the attachment already exists, an error will be thrown. * * @param file The file to store * @returns A promise resolving to the attachment */ - async overwriteFile(file: File) { + async writeFile(file: File) { return FileStorage.store({ body: createReadStream(file.filepath), contentLength: file.size, diff --git a/server/storage/files/LocalStorage.ts b/server/storage/files/LocalStorage.ts index 92edca3d6..500dcd8e3 100644 --- a/server/storage/files/LocalStorage.ts +++ b/server/storage/files/LocalStorage.ts @@ -1,19 +1,20 @@ import { Blob } from "buffer"; -import { - ReadStream, - closeSync, - createReadStream, - createWriteStream, - existsSync, - openSync, -} from "fs"; import { mkdir, unlink } from "fs/promises"; import path from "path"; import { Readable } from "stream"; +import { + ReadStream, + close, + pathExists, + createReadStream, + createWriteStream, + open, +} from "fs-extra"; import invariant from "invariant"; import JWT from "jsonwebtoken"; import safeResolvePath from "resolve-path"; import env from "@server/env"; +import { ValidationError } from "@server/errors"; import Logger from "@server/logging/Logger"; import BaseStorage from "./BaseStorage"; @@ -53,13 +54,15 @@ export default class LocalStorage extends BaseStorage { key: string; acl?: string; }) => { - const subdir = key.split("/").slice(0, -1).join("/"); - if (!existsSync(path.join(env.FILE_STORAGE_LOCAL_ROOT_DIR, subdir))) { - await mkdir(path.join(env.FILE_STORAGE_LOCAL_ROOT_DIR, subdir), { - recursive: true, - }); + const exists = await pathExists(this.getFilePath(key)); + if (exists) { + throw ValidationError(`File already exists at ${key}`); } + await mkdir(this.getFilePath(path.dirname(key)), { + recursive: true, + }); + let src: NodeJS.ReadableStream; if (body instanceof ReadStream) { src = body; @@ -70,7 +73,9 @@ export default class LocalStorage extends BaseStorage { } const filePath = this.getFilePath(key); - closeSync(openSync(filePath, "w")); + + // Create the file on disk first + await open(filePath, "w").then(close); return new Promise((resolve, reject) => { const dest = createWriteStream(filePath)