From cc388a3b2cf1215164d0efe65429fde2f60a5232 Mon Sep 17 00:00:00 2001 From: Alex Date: Fri, 13 Dec 2024 16:30:21 +0100 Subject: [PATCH] Generate pseudo-random URLs for public uploaded files --- apps/web/src/actions/files/upload.ts | 5 +- packages/core/src/lib/disk.ts | 2 +- .../core/src/services/files/upload.test.ts | 61 ++++++++++++++++--- packages/core/src/services/files/upload.ts | 41 ++++++++++--- 4 files changed, 92 insertions(+), 17 deletions(-) diff --git a/apps/web/src/actions/files/upload.ts b/apps/web/src/actions/files/upload.ts index c7078c42c..0e5a69dcd 100644 --- a/apps/web/src/actions/files/upload.ts +++ b/apps/web/src/actions/files/upload.ts @@ -16,7 +16,10 @@ export const uploadFileAction = authProcedure }), ) .handler(async ({ input, ctx }) => { - const result = await uploadFile(input.file, ctx.workspace) + const result = await uploadFile({ + file: input.file, + workspace: ctx.workspace, + }) return result.unwrap() }) diff --git a/packages/core/src/lib/disk.ts b/packages/core/src/lib/disk.ts index 9ccfd4437..c47057556 100644 --- a/packages/core/src/lib/disk.ts +++ b/packages/core/src/lib/disk.ts @@ -152,7 +152,7 @@ export class DiskWrapper { region: awsConfig.region, bucket: awsConfig.publicBucket, supportsACL: false, - visibility: 'private', + visibility: 'public', }) } diff --git a/packages/core/src/services/files/upload.test.ts b/packages/core/src/services/files/upload.test.ts index dcb4bfd6d..f6c72c841 100644 --- a/packages/core/src/services/files/upload.test.ts +++ b/packages/core/src/services/files/upload.test.ts @@ -3,6 +3,7 @@ import { join } from 'path' import { beforeEach, describe, expect, it, vi } from 'vitest' import { Workspace } from '../../browser' import * as constants from '../../constants' +import * as lib from '../../lib' import { BadRequestError, Result, UnprocessableEntityError } from '../../lib' import { diskFactory } from '../../lib/disk' import * as factories from '../../tests/factories' @@ -25,13 +26,14 @@ describe('uploadFile', () => { workspace = w vi.spyOn(disk, 'putFile').mockResolvedValue(Result.ok(undefined)) + vi.spyOn(lib, 'generateUUIDIdentifier').mockReturnValue('fake-uuid') }) it('not uploads empty file', async () => { const file = new File([Buffer.from('')], 'file') await expect( - uploadFile(file, workspace, disk).then((r) => r.unwrap()), + uploadFile({ file, workspace }, disk).then((r) => r.unwrap()), ).rejects.toThrowError(new BadRequestError(`File is empty`)) }) @@ -41,7 +43,7 @@ describe('uploadFile', () => { const file = new File([Buffer.from('Too large!')], 'file') await expect( - uploadFile(file, workspace, disk).then((r) => r.unwrap()), + uploadFile({ file, workspace }, disk).then((r) => r.unwrap()), ).rejects.toThrowError(new BadRequestError(`File too large`)) }) @@ -51,7 +53,7 @@ describe('uploadFile', () => { const file = new File([content], name) await expect( - uploadFile(file, workspace, disk).then((r) => r.unwrap()), + uploadFile({ file, workspace }, disk).then((r) => r.unwrap()), ).rejects.toThrowError(new BadRequestError(`Unsupported file type: .bin`)) }) @@ -65,7 +67,7 @@ describe('uploadFile', () => { const file = new File([content], name) await expect( - uploadFile(file, workspace, disk).then((r) => r.unwrap()), + uploadFile({ file, workspace }, disk).then((r) => r.unwrap()), ).rejects.toThrowError( new UnprocessableEntityError(`Failed to upload .png file`, {}), ) @@ -80,8 +82,10 @@ describe('uploadFile', () => { const file = new File([content], name) await expect( - uploadFile(file, workspace, disk).then((r) => r.unwrap()), - ).resolves.toContain(`/workspaces/${workspace.id}/files/${name}`) + uploadFile({ file, workspace }, disk).then((r) => r.unwrap()), + ).resolves.toContain( + `/workspaces/${workspace.id}/files/fake-uuid/${name}`, + ) expect(convertFile).not.toHaveBeenCalled() } }) @@ -97,9 +101,50 @@ describe('uploadFile', () => { const file = new File([content], name) await expect( - uploadFile(file, workspace, disk).then((r) => r.unwrap()), - ).resolves.toContain(`/workspaces/${workspace.id}/files/${name}`) + uploadFile({ file, workspace }, disk).then((r) => r.unwrap()), + ).resolves.toContain( + `/workspaces/${workspace.id}/files/fake-uuid/${name}`, + ) expect(convertFile).toHaveBeenCalledWith(file) } }) + + it('it uploads files with a workspace prefix', async () => { + const convertFile = vi.spyOn(convert, 'convertFile') + + const name = 'file.png' + const content = await readFile(join(IMAGES_PATH, name)) + const file = new File([content], name) + + await expect( + uploadFile({ file, workspace }, disk).then((r) => r.unwrap()), + ).resolves.toContain(`/workspaces/${workspace.id}/files/fake-uuid/${name}`) + expect(convertFile).not.toHaveBeenCalled() + }) + + it('it uploads files with a custom prefix', async () => { + const convertFile = vi.spyOn(convert, 'convertFile') + + const name = 'file.png' + const content = await readFile(join(IMAGES_PATH, name)) + const file = new File([content], name) + + await expect( + uploadFile({ file, prefix: 'custom' }, disk).then((r) => r.unwrap()), + ).resolves.toContain(`/custom/files/fake-uuid/${name}`) + expect(convertFile).not.toHaveBeenCalled() + }) + + it('it uploads files with an unknown prefix', async () => { + const convertFile = vi.spyOn(convert, 'convertFile') + + const name = 'file.png' + const content = await readFile(join(IMAGES_PATH, name)) + const file = new File([content], name) + + await expect( + uploadFile({ file }, disk).then((r) => r.unwrap()), + ).resolves.toContain(`/unknown/files/fake-uuid/${name}`) + expect(convertFile).not.toHaveBeenCalled() + }) }) diff --git a/packages/core/src/services/files/upload.ts b/packages/core/src/services/files/upload.ts index cd89ae85c..6665ecda8 100644 --- a/packages/core/src/services/files/upload.ts +++ b/packages/core/src/services/files/upload.ts @@ -4,6 +4,7 @@ import { diskFactory, DiskWrapper } from '../../lib/disk' import { BadRequestError, + generateUUIDIdentifier, Result, TypedResult, UnprocessableEntityError, @@ -13,13 +14,40 @@ import { Workspace } from '../../browser' import { MAX_UPLOAD_SIZE_IN_MB, SUPPORTED_IMAGE_TYPES } from '../../constants' import { convertFile } from './convert' +function generateKey({ + filename, + prefix, + workspace, +}: { + filename: string + prefix?: string + workspace?: Workspace +}) { + let keyPrefix = prefix + if (!keyPrefix && workspace) keyPrefix = `workspaces/${workspace.id}` + if (!keyPrefix) keyPrefix = 'unknown' + + const keyUuid = generateUUIDIdentifier() + + const keyFilename = slugify(filename, { preserveCharacters: ['.'] }) + + return `${keyPrefix}/files/${keyUuid}/${keyFilename}` +} + export async function uploadFile( - file: File, - workspace: Workspace, + { + file, + prefix, + workspace, + }: { + file: File + prefix?: string + workspace?: Workspace + }, disk: DiskWrapper = diskFactory('public'), ): Promise> { + const key = generateKey({ filename: file.name, prefix, workspace }) const extension = path.extname(file.name).toLowerCase() - const key = `workspaces/${workspace.id}/files/${slugify(file.name, { preserveCharacters: ['.'] })}` if (file.size === 0) { return Result.error(new BadRequestError(`File is empty`)) @@ -38,10 +66,9 @@ export async function uploadFile( try { await disk.putFile(key, file).then((r) => r.unwrap()) - const url = await disk.getSignedUrl(key, { - expiresIn: undefined, - contentDisposition: 'inline', - }) + // TODO: Use temporal signed URLs, with a (micro)service + // acting as a reverse proxy refreshing the signed urls + const url = await disk.getUrl(key) return Result.ok(url) } catch (error) {