From 157a0fe278e412d7e5e701ee3ee7be561fa93606 Mon Sep 17 00:00:00 2001 From: Erik Michelson Date: Wed, 12 Jun 2024 18:45:49 +0200 Subject: [PATCH] refactor(media): store filenames, use pre-signed s3/azure URLs, UUIDs Signed-off-by: Erik Michelson --- .../src/api/private/media/media.controller.ts | 33 +++-- .../src/api/public/media/media.controller.ts | 33 +++-- backend/src/api/utils/descriptions.ts | 4 +- backend/src/api/utils/openapi.decorator.ts | 13 +- backend/src/app.module.ts | 8 +- backend/src/config/media.config.spec.ts | 8 +- backend/src/config/media.config.ts | 14 +- backend/src/config/mock/media.config.mock.ts | 2 + backend/src/config/utils.spec.ts | 14 ++ backend/src/config/utils.ts | 14 ++ .../media-redirect.controller.ts | 35 +++++ .../media-redirect/media-redirect.module.ts | 16 +++ backend/src/media/backends/azure-backend.ts | 66 ++++++--- .../src/media/backends/filesystem-backend.ts | 49 +++++-- backend/src/media/backends/imgur-backend.ts | 56 +++++--- backend/src/media/backends/s3-backend.spec.ts | 87 ++++++++--- backend/src/media/backends/s3-backend.ts | 68 +++++---- backend/src/media/backends/webdav-backend.ts | 58 +++++--- backend/src/media/media-backend.interface.ts | 28 +++- backend/src/media/media-upload.dto.ts | 18 ++- backend/src/media/media-upload.entity.ts | 48 ++++--- backend/src/media/media.service.spec.ts | 135 ++++++++++++------ backend/src/media/media.service.ts | 78 +++++++--- ...-init.ts => mariadb-1726084491570-init.ts} | 11 +- ...init.ts => postgres-1726084117959-init.ts} | 11 +- ...0-init.ts => sqlite-1726084595852-init.ts} | 19 +-- backend/test/private-api/me.e2e-spec.ts | 49 +++++-- backend/test/private-api/media.e2e-spec.ts | 48 ++++--- backend/test/private-api/notes.e2e-spec.ts | 20 ++- backend/test/public-api/me.e2e-spec.ts | 48 +++++-- backend/test/public-api/media.e2e-spec.ts | 40 +++--- backend/test/public-api/notes.e2e-spec.ts | 15 +- dev-reverse-proxy/Caddyfile | 1 + docs/content/concepts/index.md | 6 + docs/content/concepts/media.md | 23 +++ docs/content/how-to/reverse-proxy.md | 7 +- docs/content/references/config/media/s3.md | 7 +- frontend/cypress/e2e/fileUpload.spec.ts | 12 +- frontend/src/api/media/index.ts | 4 +- frontend/src/api/media/types.ts | 5 +- .../editor-pane/hooks/use-handle-upload.tsx | 6 +- .../media-browser-sidebar-menu.tsx | 2 +- .../media-entry-deletion-modal.tsx | 2 +- .../media-entry.module.css | 11 ++ .../media-entry.tsx | 15 +- frontend/src/pages/api/private/me/media.ts | 6 +- frontend/src/pages/api/private/media.ts | 5 +- 47 files changed, 869 insertions(+), 389 deletions(-) create mode 100644 backend/src/media-redirect/media-redirect.controller.ts create mode 100644 backend/src/media-redirect/media-redirect.module.ts rename backend/src/migrations/{mariadb-1725266569705-init.ts => mariadb-1726084491570-init.ts} (96%) rename backend/src/migrations/{postgres-1725266697932-init.ts => postgres-1726084117959-init.ts} (96%) rename backend/src/migrations/{sqlite-1725268109950-init.ts => sqlite-1726084595852-init.ts} (95%) create mode 100644 docs/content/concepts/media.md create mode 100644 frontend/src/components/editor-page/sidebar/specific-sidebar-entries/media-browser-sidebar-menu/media-entry.module.css diff --git a/backend/src/api/private/media/media.controller.ts b/backend/src/api/private/media/media.controller.ts index 3a90dbef6..f54f0bd23 100644 --- a/backend/src/api/private/media/media.controller.ts +++ b/backend/src/api/private/media/media.controller.ts @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2022 The HedgeDoc developers (see AUTHORS file) + * SPDX-FileCopyrightText: 2024 The HedgeDoc developers (see AUTHORS file) * * SPDX-License-Identifier: AGPL-3.0-only */ @@ -100,28 +100,33 @@ export class MediaController { 'uploadMedia', ); } - const upload = await this.mediaService.saveFile(file.buffer, user, note); + const upload = await this.mediaService.saveFile( + file.originalname, + file.buffer, + user, + note, + ); return await this.mediaService.toMediaUploadDto(upload); } - @Get(':filename') - @OpenApi(404, 500) + @Get(':uuid') + @OpenApi(200, 404, 500) async getMedia( - @Param('filename') filename: string, + @Param('uuid') uuid: string, @Res() response: Response, ): Promise { - const mediaUpload = await this.mediaService.findUploadByFilename(filename); - const targetUrl = mediaUpload.fileUrl; - response.redirect(targetUrl); + const mediaUpload = await this.mediaService.findUploadByUuid(uuid); + const dto = await this.mediaService.toMediaUploadDto(mediaUpload); + response.send(dto); } - @Delete(':filename') + @Delete(':uuid') @OpenApi(204, 403, 404, 500) async deleteMedia( @RequestUser() user: User, - @Param('filename') filename: string, + @Param('uuid') uuid: string, ): Promise { - const mediaUpload = await this.mediaService.findUploadByFilename(filename); + const mediaUpload = await this.mediaService.findUploadByUuid(uuid); if ( await this.permissionsService.checkMediaDeletePermission( user, @@ -129,18 +134,18 @@ export class MediaController { ) ) { this.logger.debug( - `Deleting '${filename}' for user '${user.username}'`, + `Deleting '${uuid}' for user '${user.username}'`, 'deleteMedia', ); await this.mediaService.deleteFile(mediaUpload); } else { this.logger.warn( - `${user.username} tried to delete '${filename}', but is not the owner of upload or connected note`, + `${user.username} tried to delete '${uuid}', but is not the owner of upload or connected note`, 'deleteMedia', ); const mediaUploadNote = await mediaUpload.note; throw new PermissionError( - `Neither file '${filename}' nor note '${ + `Neither file '${uuid}' nor note '${ mediaUploadNote?.publicId ?? 'unknown' }'is owned by '${user.username}'`, ); diff --git a/backend/src/api/public/media/media.controller.ts b/backend/src/api/public/media/media.controller.ts index f552942ef..216e2498c 100644 --- a/backend/src/api/public/media/media.controller.ts +++ b/backend/src/api/public/media/media.controller.ts @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2022 The HedgeDoc developers (see AUTHORS file) + * SPDX-FileCopyrightText: 2024 The HedgeDoc developers (see AUTHORS file) * * SPDX-License-Identifier: AGPL-3.0-only */ @@ -100,28 +100,33 @@ export class MediaController { `Received filename '${file.originalname}' for note '${note.publicId}' from user '${user.username}'`, 'uploadMedia', ); - const upload = await this.mediaService.saveFile(file.buffer, user, note); + const upload = await this.mediaService.saveFile( + file.originalname, + file.buffer, + user, + note, + ); return await this.mediaService.toMediaUploadDto(upload); } - @Get(':filename') - @OpenApi(404, 500) + @Get(':uuid') + @OpenApi(200, 404, 500) async getMedia( - @Param('filename') filename: string, + @Param('uuid') uuid: string, @Res() response: Response, ): Promise { - const mediaUpload = await this.mediaService.findUploadByFilename(filename); - const targetUrl = mediaUpload.fileUrl; - response.redirect(targetUrl); + const mediaUpload = await this.mediaService.findUploadByUuid(uuid); + const dto = await this.mediaService.toMediaUploadDto(mediaUpload); + response.send(dto); } - @Delete(':filename') + @Delete(':uuid') @OpenApi(204, 403, 404, 500) async deleteMedia( @RequestUser() user: User, - @Param('filename') filename: string, + @Param('uuid') uuid: string, ): Promise { - const mediaUpload = await this.mediaService.findUploadByFilename(filename); + const mediaUpload = await this.mediaService.findUploadByUuid(uuid); if ( await this.permissionsService.checkMediaDeletePermission( user, @@ -129,18 +134,18 @@ export class MediaController { ) ) { this.logger.debug( - `Deleting '${filename}' for user '${user.username}'`, + `Deleting '${uuid}' for user '${user.username}'`, 'deleteMedia', ); await this.mediaService.deleteFile(mediaUpload); } else { this.logger.warn( - `${user.username} tried to delete '${filename}', but is not the owner of upload or connected note`, + `${user.username} tried to delete '${uuid}', but is not the owner of upload or connected note`, 'deleteMedia', ); const mediaUploadNote = await mediaUpload.note; throw new PermissionError( - `Neither file '${filename}' nor note '${ + `Neither file '${uuid}' nor note '${ mediaUploadNote?.publicId ?? 'unknown' }'is owned by '${user.username}'`, ); diff --git a/backend/src/api/utils/descriptions.ts b/backend/src/api/utils/descriptions.ts index 1ebd95a71..65ec311ea 100644 --- a/backend/src/api/utils/descriptions.ts +++ b/backend/src/api/utils/descriptions.ts @@ -1,10 +1,12 @@ /* - * SPDX-FileCopyrightText: 2022 The HedgeDoc developers (see AUTHORS file) + * SPDX-FileCopyrightText: 2024 The HedgeDoc developers (see AUTHORS file) * * SPDX-License-Identifier: AGPL-3.0-only */ export const okDescription = 'This request was successful'; +export const foundDescription = + 'The requested resource was found at another URL'; export const createdDescription = 'The requested resource was successfully created'; export const noContentDescription = diff --git a/backend/src/api/utils/openapi.decorator.ts b/backend/src/api/utils/openapi.decorator.ts index a29b5ad6f..ceec342b8 100644 --- a/backend/src/api/utils/openapi.decorator.ts +++ b/backend/src/api/utils/openapi.decorator.ts @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2022 The HedgeDoc developers (see AUTHORS file) + * SPDX-FileCopyrightText: 2024 The HedgeDoc developers (see AUTHORS file) * * SPDX-License-Identifier: AGPL-3.0-only */ @@ -8,6 +8,7 @@ import { ApiBadRequestResponse, ApiConflictResponse, ApiCreatedResponse, + ApiFoundResponse, ApiInternalServerErrorResponse, ApiNoContentResponse, ApiNotFoundResponse, @@ -21,6 +22,7 @@ import { badRequestDescription, conflictDescription, createdDescription, + foundDescription, internalServerErrorDescription, noContentDescription, notFoundDescription, @@ -33,6 +35,7 @@ export type HttpStatusCodes = | 200 | 201 | 204 + | 302 | 400 | 401 | 403 @@ -130,6 +133,14 @@ export const OpenApi = ( HttpCode(204), ); break; + case 302: + decoratorsToApply.push( + ApiFoundResponse({ + description: description ?? foundDescription, + }), + HttpCode(302), + ); + break; case 400: decoratorsToApply.push( ApiBadRequestResponse({ diff --git a/backend/src/app.module.ts b/backend/src/app.module.ts index 207154c73..c1e2e990b 100644 --- a/backend/src/app.module.ts +++ b/backend/src/app.module.ts @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2022 The HedgeDoc developers (see AUTHORS file) + * SPDX-FileCopyrightText: 2024 The HedgeDoc developers (see AUTHORS file) * * SPDX-License-Identifier: AGPL-3.0-only */ @@ -29,6 +29,7 @@ import { HistoryModule } from './history/history.module'; import { IdentityModule } from './identity/identity.module'; import { LoggerModule } from './logger/logger.module'; import { TypeormLoggerService } from './logger/typeorm-logger.service'; +import { MediaRedirectModule } from './media-redirect/media-redirect.module'; import { MediaModule } from './media/media.module'; import { MonitoringModule } from './monitoring/monitoring.module'; import { NotesModule } from './notes/notes.module'; @@ -49,6 +50,10 @@ const routes: Routes = [ path: '/api/private', module: PrivateApiModule, }, + { + path: '/media', + module: MediaRedirectModule, + }, ]; @Module({ @@ -112,6 +117,7 @@ const routes: Routes = [ WebsocketModule, IdentityModule, SessionModule, + MediaRedirectModule, ], controllers: [], providers: [FrontendConfigService], diff --git a/backend/src/config/media.config.spec.ts b/backend/src/config/media.config.spec.ts index 540c68f33..950c014d7 100644 --- a/backend/src/config/media.config.spec.ts +++ b/backend/src/config/media.config.spec.ts @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2022 The HedgeDoc developers (see AUTHORS file) + * SPDX-FileCopyrightText: 2024 The HedgeDoc developers (see AUTHORS file) * * SPDX-License-Identifier: AGPL-3.0-only */ @@ -16,6 +16,8 @@ describe('mediaConfig', () => { const secretAccessKey = 'secretAccessKey'; const bucket = 'bucket'; const endPoint = 'https://endPoint'; + const region = 'us-east-1'; + const pathStyle = false; // Azure const azureConnectionString = 'connectionString'; const container = 'container'; @@ -54,6 +56,8 @@ describe('mediaConfig', () => { HD_MEDIA_BACKEND_S3_SECRET_KEY: secretAccessKey, HD_MEDIA_BACKEND_S3_BUCKET: bucket, HD_MEDIA_BACKEND_S3_ENDPOINT: endPoint, + HD_MEDIA_BACKEND_S3_REGION: region, + HD_MEDIA_BACKEND_S3_PATH_STYLE: pathStyle.toString(), /* eslint-enable @typescript-eslint/naming-convention */ }, { @@ -66,6 +70,8 @@ describe('mediaConfig', () => { expect(config.backend.s3.secretAccessKey).toEqual(secretAccessKey); expect(config.backend.s3.bucket).toEqual(bucket); expect(config.backend.s3.endPoint).toEqual(endPoint); + expect(config.backend.s3.region).toEqual(region); + expect(config.backend.s3.pathStyle).toEqual(pathStyle); restore(); }); diff --git a/backend/src/config/media.config.ts b/backend/src/config/media.config.ts index 3411b7904..a47ddbb41 100644 --- a/backend/src/config/media.config.ts +++ b/backend/src/config/media.config.ts @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2022 The HedgeDoc developers (see AUTHORS file) + * SPDX-FileCopyrightText: 2024 The HedgeDoc developers (see AUTHORS file) * * SPDX-License-Identifier: AGPL-3.0-only */ @@ -7,7 +7,7 @@ import { registerAs } from '@nestjs/config'; import * as Joi from 'joi'; import { BackendType } from '../media/backends/backend-type.enum'; -import { buildErrorMessage } from './utils'; +import { buildErrorMessage, parseOptionalBoolean } from './utils'; export interface MediaConfig { backend: MediaBackendConfig; @@ -23,6 +23,8 @@ export interface MediaBackendConfig { secretAccessKey: string; bucket: string; endPoint: string; + region: string; + pathStyle: boolean; }; azure: { connectionString: string; @@ -59,6 +61,10 @@ const mediaSchema = Joi.object({ endPoint: Joi.string() .uri({ scheme: /^https?/ }) .label('HD_MEDIA_BACKEND_S3_ENDPOINT'), + region: Joi.string().optional().label('HD_MEDIA_BACKEND_S3_REGION'), + pathStyle: Joi.boolean() + .default(false) + .label('HD_MEDIA_BACKEND_S3_PATH_STYLE'), }), otherwise: Joi.optional(), }), @@ -110,6 +116,10 @@ export default registerAs('mediaConfig', () => { secretAccessKey: process.env.HD_MEDIA_BACKEND_S3_SECRET_KEY, bucket: process.env.HD_MEDIA_BACKEND_S3_BUCKET, endPoint: process.env.HD_MEDIA_BACKEND_S3_ENDPOINT, + region: process.env.HD_MEDIA_BACKEND_S3_REGION, + pathStyle: parseOptionalBoolean( + process.env.HD_MEDIA_BACKEND_S3_PATH_STYLE, + ), }, azure: { connectionString: diff --git a/backend/src/config/mock/media.config.mock.ts b/backend/src/config/mock/media.config.mock.ts index d02b3749e..96ee1c824 100644 --- a/backend/src/config/mock/media.config.mock.ts +++ b/backend/src/config/mock/media.config.mock.ts @@ -22,6 +22,8 @@ export function createDefaultMockMediaConfig(): MediaConfig { secretAccessKey: '', bucket: '', endPoint: '', + pathStyle: false, + region: '', }, azure: { connectionString: '', diff --git a/backend/src/config/utils.spec.ts b/backend/src/config/utils.spec.ts index 2fdd695f5..c8d20fba3 100644 --- a/backend/src/config/utils.spec.ts +++ b/backend/src/config/utils.spec.ts @@ -8,6 +8,7 @@ import { ensureNoDuplicatesExist, findDuplicatesInArray, needToLog, + parseOptionalBoolean, parseOptionalNumber, replaceAuthErrorsWithEnvironmentVariables, toArrayConfig, @@ -141,4 +142,17 @@ describe('config utils', () => { expect(parseOptionalNumber('3.14')).toEqual(3.14); }); }); + describe('parseOptionalBoolean', () => { + it('returns undefined on undefined parameter', () => { + expect(parseOptionalBoolean(undefined)).toEqual(undefined); + }); + it('correctly parses a given string', () => { + expect(parseOptionalBoolean('true')).toEqual(true); + expect(parseOptionalBoolean('1')).toEqual(true); + expect(parseOptionalBoolean('y')).toEqual(true); + expect(parseOptionalBoolean('false')).toEqual(false); + expect(parseOptionalBoolean('0')).toEqual(false); + expect(parseOptionalBoolean('HedgeDoc')).toEqual(false); + }); + }); }); diff --git a/backend/src/config/utils.ts b/backend/src/config/utils.ts index 3ba44d93b..6d1c5a5e3 100644 --- a/backend/src/config/utils.ts +++ b/backend/src/config/utils.ts @@ -118,3 +118,17 @@ export function parseOptionalNumber(value?: string): number | undefined { } return Number(value); } + +/** + * Parses a string to a boolean. The following values are considered true: + * true, 1, y + * + * @param value The value to parse + * @returns The parsed boolean or undefined if the value is undefined + */ +export function parseOptionalBoolean(value?: string): boolean | undefined { + if (value === undefined) { + return undefined; + } + return value === 'true' || value === '1' || value === 'y'; +} diff --git a/backend/src/media-redirect/media-redirect.controller.ts b/backend/src/media-redirect/media-redirect.controller.ts new file mode 100644 index 000000000..d74de6225 --- /dev/null +++ b/backend/src/media-redirect/media-redirect.controller.ts @@ -0,0 +1,35 @@ +/* + * SPDX-FileCopyrightText: 2024 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { Controller, Get, Param, Res } from '@nestjs/common'; +import { ApiTags } from '@nestjs/swagger'; +import { Response } from 'express'; + +import { OpenApi } from '../api/utils/openapi.decorator'; +import { ConsoleLoggerService } from '../logger/console-logger.service'; +import { MediaService } from '../media/media.service'; + +@OpenApi() +@ApiTags('media-redirect') +@Controller() +export class MediaRedirectController { + constructor( + private readonly logger: ConsoleLoggerService, + private mediaService: MediaService, + ) { + this.logger.setContext(MediaRedirectController.name); + } + + @Get(':uuid') + @OpenApi(302, 404, 500) + async getMedia( + @Param('uuid') uuid: string, + @Res() response: Response, + ): Promise { + const mediaUpload = await this.mediaService.findUploadByUuid(uuid); + const url = await this.mediaService.getFileUrl(mediaUpload); + response.redirect(url); + } +} diff --git a/backend/src/media-redirect/media-redirect.module.ts b/backend/src/media-redirect/media-redirect.module.ts new file mode 100644 index 000000000..b7b589a1d --- /dev/null +++ b/backend/src/media-redirect/media-redirect.module.ts @@ -0,0 +1,16 @@ +/* + * SPDX-FileCopyrightText: 2024 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { Module } from '@nestjs/common'; + +import { LoggerModule } from '../logger/logger.module'; +import { MediaModule } from '../media/media.module'; +import { MediaRedirectController } from './media-redirect.controller'; + +@Module({ + imports: [MediaModule, LoggerModule], + controllers: [MediaRedirectController], +}) +export class MediaRedirectModule {} diff --git a/backend/src/media/backends/azure-backend.ts b/backend/src/media/backends/azure-backend.ts index 2e2174079..eca45765d 100644 --- a/backend/src/media/backends/azure-backend.ts +++ b/backend/src/media/backends/azure-backend.ts @@ -1,26 +1,30 @@ /* - * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * SPDX-FileCopyrightText: 2024 The HedgeDoc developers (see AUTHORS file) * * SPDX-License-Identifier: AGPL-3.0-only */ import { + BlobSASPermissions, BlobServiceClient, BlockBlobClient, ContainerClient, + generateBlobSASQueryParameters, + StorageSharedKeyCredential, } from '@azure/storage-blob'; import { Inject, Injectable } from '@nestjs/common'; +import { FileTypeResult } from 'file-type'; import mediaConfiguration, { MediaConfig } from '../../config/media.config'; import { MediaBackendError } from '../../errors/errors'; import { ConsoleLoggerService } from '../../logger/console-logger.service'; import { MediaBackend } from '../media-backend.interface'; -import { BackendData } from '../media-upload.entity'; import { BackendType } from './backend-type.enum'; @Injectable() export class AzureBackend implements MediaBackend { private config: MediaConfig['backend']['azure']; private client: ContainerClient; + private readonly credential: StorageSharedKeyCredential; constructor( private readonly logger: ConsoleLoggerService, @@ -28,56 +32,76 @@ export class AzureBackend implements MediaBackend { private mediaConfig: MediaConfig, ) { this.logger.setContext(AzureBackend.name); - this.config = mediaConfig.backend.azure; - if (mediaConfig.backend.use === BackendType.AZURE) { + this.config = this.mediaConfig.backend.azure; + if (this.mediaConfig.backend.use === BackendType.AZURE) { // only create the client if the backend is configured to azure const blobServiceClient = BlobServiceClient.fromConnectionString( this.config.connectionString, ); + this.credential = + blobServiceClient.credential as StorageSharedKeyCredential; this.client = blobServiceClient.getContainerClient(this.config.container); } } async saveFile( + uuid: string, buffer: Buffer, - fileName: string, - ): Promise<[string, BackendData]> { + fileType: FileTypeResult, + ): Promise { const blockBlobClient: BlockBlobClient = - this.client.getBlockBlobClient(fileName); + this.client.getBlockBlobClient(uuid); try { - await blockBlobClient.upload(buffer, buffer.length); - const url = this.getUrl(fileName); - this.logger.log(`Uploaded ${url}`, 'saveFile'); - return [url, null]; + await blockBlobClient.upload(buffer, buffer.length, { + blobHTTPHeaders: { + blobContentType: fileType.mime, + }, + }); + this.logger.log(`Uploaded file ${uuid}`, 'saveFile'); + return null; } catch (e) { this.logger.error( `error: ${(e as Error).message}`, (e as Error).stack, 'saveFile', ); - throw new MediaBackendError(`Could not save '${fileName}' on Azure`); + throw new MediaBackendError(`Could not save file '${uuid}'`); } } - async deleteFile(fileName: string, _: BackendData): Promise { + async deleteFile(uuid: string, _: unknown): Promise { const blockBlobClient: BlockBlobClient = - this.client.getBlockBlobClient(fileName); + this.client.getBlockBlobClient(uuid); try { - await blockBlobClient.delete(); - const url = this.getUrl(fileName); - this.logger.log(`Deleted ${url}`, 'deleteFile'); - return; + const response = await blockBlobClient.delete(); + if (response.errorCode !== undefined) { + throw new MediaBackendError( + `Could not delete '${uuid}': ${response.errorCode}`, + ); + } + this.logger.log(`Deleted file ${uuid}`, 'deleteFile'); } catch (e) { this.logger.error( `error: ${(e as Error).message}`, (e as Error).stack, 'deleteFile', ); - throw new MediaBackendError(`Could not delete '${fileName}' on Azure`); + throw new MediaBackendError(`Could not delete file ${uuid}`); } } - private getUrl(fileName: string): string { - return `${this.client.url}/${fileName}`; + getFileUrl(uuid: string, _: unknown): Promise { + const blockBlobClient: BlockBlobClient = + this.client.getBlockBlobClient(uuid); + const blobSAS = generateBlobSASQueryParameters( + { + containerName: this.config.container, + blobName: uuid, + permissions: BlobSASPermissions.parse('r'), + expiresOn: new Date(new Date().valueOf() + 3600 * 1000), + }, + this.credential, + ); + return Promise.resolve(`${blockBlobClient.url}?${blobSAS.toString()}`); } } diff --git a/backend/src/media/backends/filesystem-backend.ts b/backend/src/media/backends/filesystem-backend.ts index 413cef904..1eb343649 100644 --- a/backend/src/media/backends/filesystem-backend.ts +++ b/backend/src/media/backends/filesystem-backend.ts @@ -1,9 +1,10 @@ /* - * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * SPDX-FileCopyrightText: 2024 The HedgeDoc developers (see AUTHORS file) * * SPDX-License-Identifier: AGPL-3.0-only */ import { Inject, Injectable } from '@nestjs/common'; +import { FileTypeResult } from 'file-type'; import { promises as fs } from 'fs'; import { join } from 'path'; @@ -11,11 +12,10 @@ import mediaConfiguration, { MediaConfig } from '../../config/media.config'; import { MediaBackendError } from '../../errors/errors'; import { ConsoleLoggerService } from '../../logger/console-logger.service'; import { MediaBackend } from '../media-backend.interface'; -import { BackendData } from '../media-upload.entity'; @Injectable() export class FilesystemBackend implements MediaBackend { - uploadDirectory = './uploads'; + private readonly uploadDirectory; constructor( private readonly logger: ConsoleLoggerService, @@ -23,37 +23,56 @@ export class FilesystemBackend implements MediaBackend { private mediaConfig: MediaConfig, ) { this.logger.setContext(FilesystemBackend.name); - this.uploadDirectory = mediaConfig.backend.filesystem.uploadPath; + this.uploadDirectory = this.mediaConfig.backend.filesystem.uploadPath; } async saveFile( + uuid: string, buffer: Buffer, - fileName: string, - ): Promise<[string, BackendData]> { - const filePath = this.getFilePath(fileName); - this.logger.debug(`Writing file to: ${filePath}`, 'saveFile'); + fileType: FileTypeResult, + ): Promise { + const filePath = this.getFilePath(uuid, fileType.ext); + this.logger.debug(`Writing uploaded file to '${filePath}'`, 'saveFile'); await this.ensureDirectory(); try { await fs.writeFile(filePath, buffer, null); - return ['/uploads/' + fileName, null]; + return JSON.stringify({ ext: fileType.ext }); } catch (e) { this.logger.error((e as Error).message, (e as Error).stack, 'saveFile'); - throw new MediaBackendError(`Could not save '${filePath}'`); + throw new MediaBackendError(`Could not save file '${filePath}'`); } } - async deleteFile(fileName: string, _: BackendData): Promise { - const filePath = this.getFilePath(fileName); + async deleteFile(uuid: string, backendData: string): Promise { + if (!backendData) { + throw new MediaBackendError('No backend data provided'); + } + const { ext } = JSON.parse(backendData) as { ext: string }; + if (!ext) { + throw new MediaBackendError('No file extension in backend data'); + } + const filePath = this.getFilePath(uuid, ext); try { return await fs.unlink(filePath); } catch (e) { this.logger.error((e as Error).message, (e as Error).stack, 'deleteFile'); - throw new MediaBackendError(`Could not delete '${filePath}'`); + throw new MediaBackendError(`Could not delete file '${filePath}'`); } } - private getFilePath(fileName: string): string { - return join(this.uploadDirectory, fileName); + getFileUrl(uuid: string, backendData: string): Promise { + if (!backendData) { + throw new MediaBackendError('No backend data provided'); + } + const { ext } = JSON.parse(backendData) as { ext: string }; + if (!ext) { + throw new MediaBackendError('No file extension in backend data'); + } + return Promise.resolve(`/uploads/${uuid}.${ext}`); + } + + private getFilePath(fileName: string, extension: string): string { + return join(this.uploadDirectory, `${fileName}.${extension}`); } private async ensureDirectory(): Promise { diff --git a/backend/src/media/backends/imgur-backend.ts b/backend/src/media/backends/imgur-backend.ts index 342d7565e..4cfba07dc 100644 --- a/backend/src/media/backends/imgur-backend.ts +++ b/backend/src/media/backends/imgur-backend.ts @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * SPDX-FileCopyrightText: 2024 The HedgeDoc developers (see AUTHORS file) * * SPDX-License-Identifier: AGPL-3.0-only */ @@ -11,15 +11,19 @@ import mediaConfiguration, { MediaConfig } from '../../config/media.config'; import { MediaBackendError } from '../../errors/errors'; import { ConsoleLoggerService } from '../../logger/console-logger.service'; import { MediaBackend } from '../media-backend.interface'; -import { BackendData } from '../media-upload.entity'; type UploadResult = { data: { link: string; - deletehash: string; + deletehash: string | null; }; }; +interface ImgurBackendData { + url: string; + deleteHash: string | null; +} + @Injectable() export class ImgurBackend implements MediaBackend { private config: MediaConfig['backend']['imgur']; @@ -30,13 +34,10 @@ export class ImgurBackend implements MediaBackend { private mediaConfig: MediaConfig, ) { this.logger.setContext(ImgurBackend.name); - this.config = mediaConfig.backend.imgur; + this.config = this.mediaConfig.backend.imgur; } - async saveFile( - buffer: Buffer, - fileName: string, - ): Promise<[string, BackendData]> { + async saveFile(uuid: string, buffer: Buffer): Promise { const params = new URLSearchParams(); params.append('image', buffer.toString('base64')); params.append('type', 'base64'); @@ -50,36 +51,41 @@ export class ImgurBackend implements MediaBackend { .then((res) => ImgurBackend.checkStatus(res)) .then((res) => res.json())) as UploadResult; this.logger.debug(`Response: ${JSON.stringify(result)}`, 'saveFile'); - this.logger.log(`Uploaded ${fileName}`, 'saveFile'); - return [result.data.link, result.data.deletehash]; + this.logger.log(`Uploaded file ${uuid}`, 'saveFile'); + const backendData: ImgurBackendData = { + url: result.data.link, + deleteHash: result.data.deletehash, + }; + return JSON.stringify(backendData); } catch (e) { this.logger.error( `error: ${(e as Error).message}`, (e as Error).stack, 'saveFile', ); - throw new MediaBackendError(`Could not save '${fileName}' on imgur`); + throw new MediaBackendError(`Could not save file ${uuid}`); } } - async deleteFile(fileName: string, backendData: BackendData): Promise { - if (backendData === null) { + async deleteFile(uuid: string, jsonBackendData: string): Promise { + const backendData = JSON.parse(jsonBackendData) as ImgurBackendData; + if (backendData.deleteHash === null) { throw new MediaBackendError( - `We don't have any delete tokens for '${fileName}' and therefore can't delete this image on imgur`, + `We don't have any delete tokens for file ${uuid} and therefore can't delete this image`, ); } try { const result = await fetch( - `https://api.imgur.com/3/image/${backendData}`, + `https://api.imgur.com/3/image/${backendData.deleteHash}`, { - method: 'POST', + method: 'DELETE', // eslint-disable-next-line @typescript-eslint/naming-convention headers: { Authorization: `Client-ID ${this.config.clientID}` }, }, - ).then((res) => ImgurBackend.checkStatus(res)); + ); + ImgurBackend.checkStatus(result); // eslint-disable-next-line @typescript-eslint/no-base-to-string - this.logger.debug(`Response: ${result.toString()}`, 'deleteFile'); - this.logger.log(`Deleted ${fileName}`, 'deleteFile'); + this.logger.log(`Deleted file ${uuid}`, 'deleteFile'); return; } catch (e) { this.logger.error( @@ -87,10 +93,20 @@ export class ImgurBackend implements MediaBackend { (e as Error).stack, 'deleteFile', ); - throw new MediaBackendError(`Could not delete '${fileName}' on imgur`); + throw new MediaBackendError(`Could not delete file '${uuid}'`); } } + getFileUrl(uuid: string, backendData: string | null): Promise { + if (backendData === null) { + throw new MediaBackendError( + `We don't have any data for file ${uuid} and therefore can't get the url of this image`, + ); + } + const data = JSON.parse(backendData) as ImgurBackendData; + return Promise.resolve(data.url); + } + private static checkStatus(res: Response): Response { if (res.ok) { // res.status >= 200 && res.status < 300 diff --git a/backend/src/media/backends/s3-backend.spec.ts b/backend/src/media/backends/s3-backend.spec.ts index c8aa05e30..1cbfc0809 100644 --- a/backend/src/media/backends/s3-backend.spec.ts +++ b/backend/src/media/backends/s3-backend.spec.ts @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2023 The HedgeDoc developers (see AUTHORS file) + * SPDX-FileCopyrightText: 2024 The HedgeDoc developers (see AUTHORS file) * * SPDX-License-Identifier: AGPL-3.0-only */ @@ -17,6 +17,7 @@ describe('s3 backend', () => { const mockedS3AccessKeyId = 'mockedS3AccessKeyId'; const mockedS3SecretAccessKey = 'mockedS3SecretAccessKey'; const mockedS3Bucket = 'mockedS3Bucket'; + const mockedUuid = 'cbe87987-8e70-4092-a879-878e70b09245'; const mockedLoggerService = Mock.of({ setContext: jest.fn(), @@ -31,6 +32,7 @@ describe('s3 backend', () => { mockedClient = Mock.of({ putObject: jest.fn(), removeObject: jest.fn(), + presignedGetObject: jest.fn(), }); clientConstructorSpy = jest @@ -143,19 +145,21 @@ describe('s3 backend', () => { const sut = new S3Backend(mockedLoggerService, mediaConfig); const mockedBuffer = Mock.of({}); - const mockedFileName = 'mockedFileName'; - const [url, backendData] = await sut.saveFile( - mockedBuffer, - mockedFileName, - ); + await sut.saveFile(mockedUuid, mockedBuffer, { + mime: 'image/png', + ext: 'png', + }); expect(saveSpy).toHaveBeenCalledWith( mockedS3Bucket, - mockedFileName, + mockedUuid, mockedBuffer, + mockedBuffer.length, + { + // eslint-disable-next-line @typescript-eslint/naming-convention + 'Content-Type': 'image/png', + }, ); - expect(url).toBe('https://s3.example.org/mockedS3Bucket/mockedFileName'); - expect(backendData).toBeNull(); }); it("will throw a MediaBackendError if the s3 client couldn't save the file", async () => { @@ -167,15 +171,24 @@ describe('s3 backend', () => { const sut = new S3Backend(mockedLoggerService, mediaConfig); const mockedBuffer = Mock.of({}); - const mockedFileName = 'mockedFileName'; await expect(() => - sut.saveFile(mockedBuffer, mockedFileName), - ).rejects.toThrow("Could not save 'mockedFileName' on S3"); + sut.saveFile(mockedUuid, mockedBuffer, { + mime: 'image/png', + ext: 'png', + }), + ).rejects.toThrow( + 'Could not save file cbe87987-8e70-4092-a879-878e70b09245', + ); expect(saveSpy).toHaveBeenCalledWith( mockedS3Bucket, - mockedFileName, + mockedUuid, mockedBuffer, + mockedBuffer.length, + { + // eslint-disable-next-line @typescript-eslint/naming-convention + 'Content-Type': 'image/png', + }, ); }); }); @@ -185,12 +198,11 @@ describe('s3 backend', () => { const deleteSpy = jest .spyOn(mockedClient, 'removeObject') .mockImplementation(() => Promise.resolve()); - const mockedFileName = 'mockedFileName'; const sut = new S3Backend(mockedLoggerService, mediaConfig); - await sut.deleteFile(mockedFileName); + await sut.deleteFile(mockedUuid, null); - expect(deleteSpy).toHaveBeenCalledWith(mockedS3Bucket, mockedFileName); + expect(deleteSpy).toHaveBeenCalledWith(mockedS3Bucket, mockedUuid); }); it("will throw a MediaBackendError if the client couldn't delete the file", async () => { @@ -198,15 +210,50 @@ describe('s3 backend', () => { const deleteSpy = jest .spyOn(mockedClient, 'removeObject') .mockImplementation(() => Promise.reject(new Error('mocked error'))); - const mockedFileName = 'mockedFileName'; const sut = new S3Backend(mockedLoggerService, mediaConfig); - await expect(() => sut.deleteFile(mockedFileName)).rejects.toThrow( - "Could not delete 'mockedFileName' on S3", + await expect(() => sut.deleteFile(mockedUuid, null)).rejects.toThrow( + 'Could not delete file cbe87987-8e70-4092-a879-878e70b09245', ); - expect(deleteSpy).toHaveBeenCalledWith(mockedS3Bucket, mockedFileName); + expect(deleteSpy).toHaveBeenCalledWith(mockedS3Bucket, mockedUuid); + }); + }); + describe('getFileUrl', () => { + it('returns a signed url', async () => { + const mediaConfig = mockMediaConfig('https://s3.example.org'); + const fileUrlSpy = jest + .spyOn(mockedClient, 'presignedGetObject') + .mockImplementation(() => + Promise.resolve( + 'https://s3.example.org/mockedS3Bucket/cbe87987-8e70-4092-a879-878e70b09245?mockedSignature', + ), + ); + + const sut = new S3Backend(mockedLoggerService, mediaConfig); + const url = await sut.getFileUrl(mockedUuid, null); + + expect(fileUrlSpy).toHaveBeenCalledWith(mockedS3Bucket, mockedUuid); + expect(url).toBe( + 'https://s3.example.org/mockedS3Bucket/cbe87987-8e70-4092-a879-878e70b09245?mockedSignature', + ); + }); + it('throws a MediaBackendError if the client could not generate a signed url', async () => { + const mediaConfig = mockMediaConfig('https://s3.example.org'); + const fileUrlSpy = jest + .spyOn(mockedClient, 'presignedGetObject') + .mockImplementation(() => { + throw new Error('mocked error'); + }); + + const sut = new S3Backend(mockedLoggerService, mediaConfig); + + await expect(() => sut.getFileUrl(mockedUuid, null)).rejects.toThrow( + 'Could not get URL for file cbe87987-8e70-4092-a879-878e70b09245', + ); + + expect(fileUrlSpy).toHaveBeenCalledWith(mockedS3Bucket, mockedUuid); }); }); }); diff --git a/backend/src/media/backends/s3-backend.ts b/backend/src/media/backends/s3-backend.ts index eca7bfc85..03a4d1092 100644 --- a/backend/src/media/backends/s3-backend.ts +++ b/backend/src/media/backends/s3-backend.ts @@ -1,9 +1,10 @@ /* - * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * SPDX-FileCopyrightText: 2024 The HedgeDoc developers (see AUTHORS file) * * SPDX-License-Identifier: AGPL-3.0-only */ import { Inject, Injectable } from '@nestjs/common'; +import { FileTypeResult } from 'file-type'; import { Client } from 'minio'; import { URL } from 'url'; @@ -11,7 +12,6 @@ import mediaConfiguration, { MediaConfig } from '../../config/media.config'; import { MediaBackendError } from '../../errors/errors'; import { ConsoleLoggerService } from '../../logger/console-logger.service'; import { MediaBackend } from '../media-backend.interface'; -import { BackendData } from '../media-upload.entity'; import { BackendType } from './backend-type.enum'; @Injectable() @@ -19,64 +19,74 @@ export class S3Backend implements MediaBackend { private config: MediaConfig['backend']['s3']; private client: Client; + private static determinePort(url: URL): number | undefined { + const port = parseInt(url.port); + return isNaN(port) ? undefined : port; + } + constructor( private readonly logger: ConsoleLoggerService, @Inject(mediaConfiguration.KEY) private mediaConfig: MediaConfig, ) { this.logger.setContext(S3Backend.name); - if (mediaConfig.backend.use !== BackendType.S3) { + if (this.mediaConfig.backend.use !== BackendType.S3) { return; } - this.config = mediaConfig.backend.s3; + this.config = this.mediaConfig.backend.s3; const url = new URL(this.config.endPoint); const isSecure = url.protocol === 'https:'; this.client = new Client({ endPoint: url.hostname, - port: this.determinePort(url), + port: S3Backend.determinePort(url), useSSL: isSecure, accessKey: this.config.accessKeyId, secretKey: this.config.secretAccessKey, + pathStyle: this.config.pathStyle, + region: this.config.region, }); } - private determinePort(url: URL): number | undefined { - const port = parseInt(url.port); - return isNaN(port) ? undefined : port; - } - async saveFile( + uuid: string, buffer: Buffer, - fileName: string, - ): Promise<[string, BackendData]> { + fileType: FileTypeResult, + ): Promise { try { - await this.client.putObject(this.config.bucket, fileName, buffer); - this.logger.log(`Uploaded file ${fileName}`, 'saveFile'); - return [this.getUrl(fileName), null]; + await this.client.putObject( + this.config.bucket, + uuid, + buffer, + buffer.length, + { + // eslint-disable-next-line @typescript-eslint/naming-convention + 'Content-Type': fileType.mime, + }, + ); + this.logger.log(`Uploaded file ${uuid}`, 'saveFile'); + return null; } catch (e) { this.logger.error((e as Error).message, (e as Error).stack, 'saveFile'); - throw new MediaBackendError(`Could not save '${fileName}' on S3`); + throw new MediaBackendError(`Could not save file ${uuid}`); } } - async deleteFile(fileName: string): Promise { + async deleteFile(uuid: string, _: unknown): Promise { try { - await this.client.removeObject(this.config.bucket, fileName); - const url = this.getUrl(fileName); - this.logger.log(`Deleted ${url}`, 'deleteFile'); - return; + await this.client.removeObject(this.config.bucket, uuid); + this.logger.log(`Deleted uploaded file ${uuid}`, 'deleteFile'); } catch (e) { - this.logger.error((e as Error).message, (e as Error).stack, 'saveFile'); - throw new MediaBackendError(`Could not delete '${fileName}' on S3`); + this.logger.error((e as Error).message, (e as Error).stack, 'deleteFile'); + throw new MediaBackendError(`Could not delete file ${uuid}`); } } - private getUrl(fileName: string): string { - const url = new URL(this.config.endPoint); - if (!url.pathname.endsWith('/')) { - url.pathname += '/'; + async getFileUrl(uuid: string, _: unknown): Promise { + try { + return await this.client.presignedGetObject(this.config.bucket, uuid); + } catch (e) { + this.logger.error((e as Error).message, (e as Error).stack, 'getFileUrl'); + throw new MediaBackendError(`Could not get URL for file ${uuid}`); } - url.pathname += `${this.config.bucket}/${fileName}`; - return url.toString(); } } diff --git a/backend/src/media/backends/webdav-backend.ts b/backend/src/media/backends/webdav-backend.ts index 78b51722e..cc5c6d552 100644 --- a/backend/src/media/backends/webdav-backend.ts +++ b/backend/src/media/backends/webdav-backend.ts @@ -1,9 +1,10 @@ /* - * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * SPDX-FileCopyrightText: 2024 The HedgeDoc developers (see AUTHORS file) * * SPDX-License-Identifier: AGPL-3.0-only */ import { Inject, Injectable } from '@nestjs/common'; +import { FileTypeResult } from 'file-type'; import fetch, { Response } from 'node-fetch'; import { URL } from 'url'; @@ -11,14 +12,13 @@ import mediaConfiguration, { MediaConfig } from '../../config/media.config'; import { MediaBackendError } from '../../errors/errors'; import { ConsoleLoggerService } from '../../logger/console-logger.service'; import { MediaBackend } from '../media-backend.interface'; -import { BackendData } from '../media-upload.entity'; import { BackendType } from './backend-type.enum'; @Injectable() export class WebdavBackend implements MediaBackend { private config: MediaConfig['backend']['webdav']; private authHeader: string; - private baseUrl: string; + private readonly baseUrl: string; constructor( private readonly logger: ConsoleLoggerService, @@ -26,11 +26,10 @@ export class WebdavBackend implements MediaBackend { private mediaConfig: MediaConfig, ) { this.logger.setContext(WebdavBackend.name); - if (mediaConfig.backend.use === BackendType.WEBDAV) { - this.config = mediaConfig.backend.webdav; + if (this.mediaConfig.backend.use === BackendType.WEBDAV) { + this.config = this.mediaConfig.backend.webdav; const url = new URL(this.config.connectionString); - const port = url.port !== '' ? `:${url.port}` : ''; - this.baseUrl = `${url.protocol}//${url.hostname}${port}${url.pathname}`; + this.baseUrl = url.toString(); if (this.config.uploadDir && this.config.uploadDir !== '') { this.baseUrl = WebdavBackend.joinURL( this.baseUrl, @@ -61,12 +60,14 @@ export class WebdavBackend implements MediaBackend { } async saveFile( + uuid: string, buffer: Buffer, - fileName: string, - ): Promise<[string, BackendData]> { + fileType: FileTypeResult, + ): Promise { try { const contentLength = buffer.length; - await fetch(WebdavBackend.joinURL(this.baseUrl, '/', fileName), { + const remoteFileName = `${uuid}.${fileType.ext}`; + await fetch(WebdavBackend.joinURL(this.baseUrl, '/', remoteFileName), { method: 'PUT', body: buffer, headers: { @@ -77,34 +78,49 @@ export class WebdavBackend implements MediaBackend { 'If-None-Match': '*', // Don't overwrite already existing files }, }).then((res) => WebdavBackend.checkStatus(res)); - this.logger.log(`Uploaded file ${fileName}`, 'saveFile'); - return [this.getUrl(fileName), null]; + this.logger.log(`Uploaded file ${uuid}`, 'saveFile'); + return JSON.stringify({ file: remoteFileName }); } catch (e) { this.logger.error((e as Error).message, (e as Error).stack, 'saveFile'); - throw new MediaBackendError(`Could not save '${fileName}' on WebDav`); + throw new MediaBackendError(`Could not save upload '${uuid}'`); } } - async deleteFile(fileName: string, _: BackendData): Promise { + async deleteFile(uuid: string, backendData: string): Promise { + if (!backendData) { + throw new MediaBackendError('No backend data provided'); + } try { - await fetch(WebdavBackend.joinURL(this.baseUrl, '/', fileName), { + const { file } = JSON.parse(backendData) as { file: string }; + if (!file) { + throw new MediaBackendError('No file name in backend data'); + } + await fetch(WebdavBackend.joinURL(this.baseUrl, '/', file), { method: 'DELETE', headers: { // eslint-disable-next-line @typescript-eslint/naming-convention Authorization: this.authHeader, }, }).then((res) => WebdavBackend.checkStatus(res)); - const url = this.getUrl(fileName); - this.logger.log(`Deleted ${url}`, 'deleteFile'); + this.logger.log(`Deleted upload ${uuid}`, 'deleteFile'); return; } catch (e) { - this.logger.error((e as Error).message, (e as Error).stack, 'saveFile'); - throw new MediaBackendError(`Could not delete '${fileName}' on WebDav`); + this.logger.error((e as Error).message, (e as Error).stack, 'deleteFile'); + throw new MediaBackendError(`Could not delete upload '${uuid}'`); } } - private getUrl(fileName: string): string { - return WebdavBackend.joinURL(this.config.publicUrl, '/', fileName); + getFileUrl(_: string, backendData: string): Promise { + if (!backendData) { + throw new MediaBackendError('No backend data provided'); + } + const { file } = JSON.parse(backendData) as { file: string }; + if (!file) { + throw new MediaBackendError('No file name in backend data'); + } + return Promise.resolve( + WebdavBackend.joinURL(this.config.publicUrl, '/', file), + ); } private static generateBasicAuthHeader( diff --git a/backend/src/media/media-backend.interface.ts b/backend/src/media/media-backend.interface.ts index d74deddd2..def764ca4 100644 --- a/backend/src/media/media-backend.interface.ts +++ b/backend/src/media/media-backend.interface.ts @@ -1,25 +1,39 @@ /* - * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * SPDX-FileCopyrightText: 2024 The HedgeDoc developers (see AUTHORS file) * * SPDX-License-Identifier: AGPL-3.0-only */ -import { BackendData } from './media-upload.entity'; +import { FileTypeResult } from 'file-type'; export interface MediaBackend { /** * Saves a file according to backend internals. + * @param uuid Unique identifier of the uploaded file * @param buffer File data - * @param fileName Name of the file to save. Can include a file extension. + * @param fileType File type result * @throws {MediaBackendError} - there was an error saving the file - * @return Tuple of file URL and internal backend data, which should be saved. + * @return The internal backend data, which should be saved */ - saveFile(buffer: Buffer, fileName: string): Promise<[string, BackendData]>; + saveFile( + uuid: string, + buffer: Buffer, + fileType?: FileTypeResult, + ): Promise; /** * Delete a file from the backend - * @param fileName String to identify the file + * @param uuid Unique identifier of the uploaded file * @param backendData Internal backend data * @throws {MediaBackendError} - there was an error deleting the file */ - deleteFile(fileName: string, backendData: BackendData): Promise; + deleteFile(uuid: string, backendData: string | null): Promise; + + /** + * Get a publicly accessible URL of a file from the backend + * @param uuid Unique identifier of the uploaded file + * @param backendData Internal backend data + * @throws {MediaBackendError} - there was an error getting the file + * @return Public accessible URL of the file + */ + getFileUrl(uuid: string, backendData: string | null): Promise; } diff --git a/backend/src/media/media-upload.dto.ts b/backend/src/media/media-upload.dto.ts index ac7512dd8..37387ec4d 100644 --- a/backend/src/media/media-upload.dto.ts +++ b/backend/src/media/media-upload.dto.ts @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2023 The HedgeDoc developers (see AUTHORS file) + * SPDX-FileCopyrightText: 2024 The HedgeDoc developers (see AUTHORS file) * * SPDX-License-Identifier: AGPL-3.0-only */ @@ -12,12 +12,20 @@ import { Username } from '../utils/username'; export class MediaUploadDto extends BaseDto { /** - * The id of the media file. - * @example "testfile123.jpg" + * The uuid of the media file. + * @example "7697582e-0020-4188-9758-2e00207188ca" */ @IsString() @ApiProperty() - id: string; + uuid: string; + + /** + * The original filename of the media upload. + * @example "example.png" + */ + @IsString() + @ApiProperty() + fileName: string; /** * The publicId of the note to which the uploaded file is linked to. @@ -26,7 +34,7 @@ export class MediaUploadDto extends BaseDto { @IsString() @IsOptional() @ApiProperty() - notePublicId: string | null; + noteId: string | null; /** * The date when the upload objects was created. diff --git a/backend/src/media/media-upload.entity.ts b/backend/src/media/media-upload.entity.ts index eec5d85b4..bf8a91baf 100644 --- a/backend/src/media/media-upload.entity.ts +++ b/backend/src/media/media-upload.entity.ts @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * SPDX-FileCopyrightText: 2024 The HedgeDoc developers (see AUTHORS file) * * SPDX-License-Identifier: AGPL-3.0-only */ @@ -15,37 +15,49 @@ import { Note } from '../notes/note.entity'; import { User } from '../users/user.entity'; import { BackendType } from './backends/backend-type.enum'; -export type BackendData = string | null; - @Entity() export class MediaUpload { + /** The unique identifier of a media upload */ @PrimaryColumn() - id: string; + uuid: string; + /** + * The note where a media file was uploaded, required for the media browser in the note editor. + * Can be set to null after creation when the note was deleted without the associated uploads + */ @ManyToOne((_) => Note, (note) => note.mediaUploads, { nullable: true, }) note: Promise; + /** The user who uploaded the media file or {@code null} if uploaded by a guest user */ @ManyToOne((_) => User, (user) => user.mediaUploads, { nullable: true, }) user: Promise; + /** The original filename of the media upload */ + @Column() + fileName: string; + + /** The backend type where this upload is stored */ @Column({ nullable: false, }) backendType: string; - @Column() - fileUrl: string; - + /** + * Additional data, depending on the backend type, serialized as JSON. + * This can include for example required additional identifiers for retrieving the file from the backend or to + * delete the file afterward again. + */ @Column({ nullable: true, type: 'text', }) - backendData: BackendData | null; + backendData: string | null; + /** The date when the upload was created */ @CreateDateColumn() createdAt: Date; @@ -53,30 +65,30 @@ export class MediaUpload { private constructor() {} /** - * Create a new media upload enity - * @param id the id of the upload + * Create a new media upload entity + * + * @param uuid the unique identifier of the upload + * @param fileName the original filename of the uploaded file * @param note the note the upload should be associated with. This is required despite the fact the note field is optional, because it's possible to delete a note without also deleting the associated media uploads, but a note is required for the initial creation. * @param user the user that owns the upload - * @param extension which file extension the upload has * @param backendType on which type of media backend the upload is saved * @param backendData the backend data returned by the media backend - * @param fileUrl the url where the upload can be accessed */ public static create( - id: string, + uuid: string, + fileName: string, note: Note, user: User | null, - extension: string, backendType: BackendType, - fileUrl: string, + backendData: string | null, ): Omit { const upload = new MediaUpload(); - upload.id = id; + upload.uuid = uuid; + upload.fileName = fileName; upload.note = Promise.resolve(note); upload.user = Promise.resolve(user); upload.backendType = backendType; - upload.backendData = null; - upload.fileUrl = fileUrl; + upload.backendData = backendData; return upload; } } diff --git a/backend/src/media/media.service.spec.ts b/backend/src/media/media.service.spec.ts index f2b8bde46..b513e6455 100644 --- a/backend/src/media/media.service.spec.ts +++ b/backend/src/media/media.service.spec.ts @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2022 The HedgeDoc developers (see AUTHORS file) + * SPDX-FileCopyrightText: 2024 The HedgeDoc developers (see AUTHORS file) * * SPDX-License-Identifier: AGPL-3.0-only */ @@ -35,7 +35,7 @@ import { User } from '../users/user.entity'; import { UsersModule } from '../users/users.module'; import { BackendType } from './backends/backend-type.enum'; import { FilesystemBackend } from './backends/filesystem-backend'; -import { BackendData, MediaUpload } from './media-upload.entity'; +import { MediaUpload } from './media-upload.entity'; import { MediaService } from './media.service'; describe('MediaService', () => { @@ -120,14 +120,16 @@ describe('MediaService', () => { ); const user = User.create('test123', 'Test 123') as User; + const uuid = 'f7d334bb-6bb6-451b-9334-bb6bb6d51b5a'; + const filename = 'test.jpg'; const note = Note.create(user) as Note; const mediaUpload = MediaUpload.create( - 'test', + uuid, + filename, note, user, - '.jpg', BackendType.FILESYSTEM, - 'test/test', + null, ) as MediaUpload; const createQueryBuilder = { @@ -174,40 +176,40 @@ describe('MediaService', () => { it('works', async () => { const testImage = await fs.readFile('test/public-api/fixtures/test.png'); - let fileId = ''; - jest - .spyOn(mediaRepo, 'save') - .mockImplementationOnce(async (entry: MediaUpload) => { - fileId = entry.id; - return entry; - }); + let givenUuid = ''; + jest.spyOn(mediaRepo, 'save').mockImplementation(); jest .spyOn(service.mediaBackend, 'saveFile') .mockImplementationOnce( - async ( - buffer: Buffer, - fileName: string, - ): Promise<[string, BackendData]> => { + async (uuid: string, buffer: Buffer): Promise => { expect(buffer).toEqual(testImage); - return [fileName, null]; + givenUuid = uuid; + return null; }, ); - const upload = await service.saveFile(testImage, user, note); - expect(upload.fileUrl).toEqual(fileId); + jest.spyOn(mediaRepo, 'save').mockImplementationOnce(async (entry) => { + expect(entry.uuid).toEqual(givenUuid); + return entry as MediaUpload; + }); + const upload = await service.saveFile('test.jpg', testImage, user, note); + expect(upload.fileName).toEqual('test.jpg'); + expect(upload.uuid).toEqual(givenUuid); + await expect(upload.note).resolves.toEqual(note); + await expect(upload.user).resolves.toEqual(user); }); describe('fails:', () => { it('MIME type not identifiable', async () => { await expect( - service.saveFile(Buffer.alloc(1), user, note), + service.saveFile('fail.png', Buffer.alloc(1), user, note), ).rejects.toThrow(ClientError); }); it('MIME type not supported', async () => { const testText = await fs.readFile('test/public-api/fixtures/test.zip'); - await expect(service.saveFile(testText, user, note)).rejects.toThrow( - ClientError, - ); + await expect( + service.saveFile('fail.zip', testText, user, note), + ).rejects.toThrow(ClientError); }); }); }); @@ -215,7 +217,12 @@ describe('MediaService', () => { describe('deleteFile', () => { it('works', async () => { const mockMediaUploadEntry = { - id: 'testMediaUpload', + uuid: '64f260cc-e0d0-47e7-b260-cce0d097e767', + fileName: 'testFileName', + note: Promise.resolve({ + id: 123, + } as Note), + backendType: BackendType.FILESYSTEM, backendData: 'testBackendData', user: Promise.resolve({ username: 'hardcoded', @@ -224,8 +231,8 @@ describe('MediaService', () => { jest .spyOn(service.mediaBackend, 'deleteFile') .mockImplementationOnce( - async (fileName: string, backendData: BackendData): Promise => { - expect(fileName).toEqual(mockMediaUploadEntry.id); + async (uuid: string, backendData: string | null): Promise => { + expect(uuid).toEqual(mockMediaUploadEntry.uuid); expect(backendData).toEqual(mockMediaUploadEntry.backendData); }, ); @@ -238,23 +245,49 @@ describe('MediaService', () => { await service.deleteFile(mockMediaUploadEntry); }); }); + + describe('getFileUrl', () => { + it('works', async () => { + const mockMediaUploadEntry = { + uuid: '64f260cc-e0d0-47e7-b260-cce0d097e767', + fileName: 'testFileName', + note: Promise.resolve({ + id: 123, + } as Note), + backendType: BackendType.FILESYSTEM, + backendData: '{"ext": "png"}', + user: Promise.resolve({ + username: 'hardcoded', + } as User), + } as MediaUpload; + await expect(service.getFileUrl(mockMediaUploadEntry)).resolves.toEqual( + '/uploads/64f260cc-e0d0-47e7-b260-cce0d097e767.png', + ); + }); + }); + describe('findUploadByFilename', () => { it('works', async () => { const testFileName = 'testFilename'; const username = 'hardcoded'; const backendData = 'testBackendData'; const mockMediaUploadEntry = { - id: 'testMediaUpload', - backendData: backendData, + uuid: '64f260cc-e0d0-47e7-b260-cce0d097e767', + fileName: testFileName, + note: Promise.resolve({ + id: 123, + } as Note), + backendType: BackendType.FILESYSTEM, + backendData, user: Promise.resolve({ - username: username, + username, } as User), } as MediaUpload; jest .spyOn(mediaRepo, 'findOne') .mockResolvedValueOnce(mockMediaUploadEntry); const mediaUpload = await service.findUploadByFilename(testFileName); - expect((await mediaUpload.user).username).toEqual(username); + expect((await mediaUpload.user)?.username).toEqual(username); expect(mediaUpload.backendData).toEqual(backendData); }); it("fails: can't find mediaUpload", async () => { @@ -271,10 +304,15 @@ describe('MediaService', () => { const username = 'hardcoded'; it('with one upload from user', async () => { const mockMediaUploadEntry = { - id: 'testMediaUpload', - backendData: 'testBackendData', + uuid: '64f260cc-e0d0-47e7-b260-cce0d097e767', + fileName: 'testFileName', + note: Promise.resolve({ + id: 123, + } as Note), + backendType: BackendType.FILESYSTEM, + backendData: null, user: Promise.resolve({ - username: username, + username, } as User), } as MediaUpload; createQueryBuilderFunc.getMany = () => [mockMediaUploadEntry]; @@ -304,11 +342,16 @@ describe('MediaService', () => { describe('works', () => { it('with one upload to note', async () => { const mockMediaUploadEntry = { - id: 'testMediaUpload', - backendData: 'testBackendData', + uuid: '64f260cc-e0d0-47e7-b260-cce0d097e767', + fileName: 'testFileName', note: Promise.resolve({ id: 123, } as Note), + backendType: BackendType.FILESYSTEM, + backendData: null, + user: Promise.resolve({ + username: 'mockUser', + } as User), } as MediaUpload; const createQueryBuilder = { where: () => createQueryBuilder, @@ -371,19 +414,19 @@ describe('MediaService', () => { Alias.create('test', mockNote, true) as Alias, ]); const mockMediaUploadEntry = { - id: 'testMediaUpload', - backendData: 'testBackendData', - note: Promise.resolve(mockNote), + uuid: '64f260cc-e0d0-47e7-b260-cce0d097e767', + fileName: 'testFileName', + note: mockNote, + backendType: BackendType.FILESYSTEM, + backendData: null, user: Promise.resolve({ - username: 'hardcoded', + username: 'mockUser', } as User), - } as MediaUpload; - jest - .spyOn(mediaRepo, 'save') - .mockImplementationOnce(async (entry: MediaUpload) => { - expect(await entry.note).toBeNull(); - return entry; - }); + } as unknown as MediaUpload; + jest.spyOn(mediaRepo, 'save').mockImplementationOnce(async (entry) => { + expect(await entry.note).toBeNull(); + return entry as MediaUpload; + }); await service.removeNoteFromMediaUpload(mockMediaUploadEntry); expect(mediaRepo.save).toHaveBeenCalled(); }); diff --git a/backend/src/media/media.service.ts b/backend/src/media/media.service.ts index 0bac3fa67..044779da3 100644 --- a/backend/src/media/media.service.ts +++ b/backend/src/media/media.service.ts @@ -1,22 +1,20 @@ /* - * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * SPDX-FileCopyrightText: 2024 The HedgeDoc developers (see AUTHORS file) * * SPDX-License-Identifier: AGPL-3.0-only */ import { Inject, Injectable } from '@nestjs/common'; import { ModuleRef } from '@nestjs/core'; import { InjectRepository } from '@nestjs/typeorm'; -import crypto from 'crypto'; import * as FileType from 'file-type'; import { Repository } from 'typeorm'; +import { v4 as uuidV4 } from 'uuid'; import mediaConfiguration, { MediaConfig } from '../config/media.config'; import { ClientError, NotInDBError } from '../errors/errors'; import { ConsoleLoggerService } from '../logger/console-logger.service'; import { Note } from '../notes/note.entity'; -import { NotesService } from '../notes/notes.service'; import { User } from '../users/user.entity'; -import { UsersService } from '../users/users.service'; import { AzureBackend } from './backends/azure-backend'; import { BackendType } from './backends/backend-type.enum'; import { FilesystemBackend } from './backends/filesystem-backend'; @@ -36,8 +34,6 @@ export class MediaService { private readonly logger: ConsoleLoggerService, @InjectRepository(MediaUpload) private mediaUploadRepository: Repository, - private notesService: NotesService, - private usersService: UsersService, private moduleRef: ModuleRef, @Inject(mediaConfiguration.KEY) private mediaConfig: MediaConfig, @@ -68,15 +64,17 @@ export class MediaService { /** * @async * Save the given buffer to the configured MediaBackend and create a MediaUploadEntity to track where the file is, who uploaded it and to which note. + * @param {string} fileName - the original file name * @param {Buffer} fileBuffer - the buffer of the file to save. * @param {User} user - the user who uploaded this file * @param {Note} note - the note which will be associated with the new file. - * @return {string} the url of the saved file + * @return {MediaUpload} the created MediaUpload entity * @throws {ClientError} the MIME type of the file is not supported. * @throws {NotInDBError} - the note or user is not in the database * @throws {MediaBackendError} - there was an error saving the file */ async saveFile( + fileName: string, fileBuffer: Buffer, user: User | null, note: Note, @@ -99,19 +97,20 @@ export class MediaService { if (!MediaService.isAllowedMimeType(fileTypeResult.mime)) { throw new ClientError('MIME type not allowed.'); } - const randomBytes = crypto.randomBytes(16); - const id = randomBytes.toString('hex') + '.' + fileTypeResult.ext; - this.logger.debug(`Generated filename: '${id}'`, 'saveFile'); - const [url, backendData] = await this.mediaBackend.saveFile(fileBuffer, id); + const uuid = uuidV4(); // TODO replace this with uuid-v7 in a later PR + const backendData = await this.mediaBackend.saveFile( + uuid, + fileBuffer, + fileTypeResult, + ); const mediaUpload = MediaUpload.create( - id, + uuid, + fileName, note, user, - fileTypeResult.ext, this.mediaBackendType, - url, + backendData, ); - mediaUpload.backendData = backendData; return await this.mediaUploadRepository.save(mediaUpload); } @@ -122,10 +121,26 @@ export class MediaService { * @throws {MediaBackendError} - there was an error deleting the file */ async deleteFile(mediaUpload: MediaUpload): Promise { - await this.mediaBackend.deleteFile(mediaUpload.id, mediaUpload.backendData); + await this.mediaBackend.deleteFile( + mediaUpload.uuid, + mediaUpload.backendData, + ); await this.mediaUploadRepository.remove(mediaUpload); } + /** + * @async + * Get the URL of the file. + * @param {MediaUpload} mediaUpload - the file to get the URL for. + * @return {string} the URL of the file. + * @throws {MediaBackendError} - there was an error retrieving the url + */ + async getFileUrl(mediaUpload: MediaUpload): Promise { + const backendName = mediaUpload.backendType as BackendType; + const backend = this.getBackendFromType(backendName); + return await backend.getFileUrl(mediaUpload.uuid, mediaUpload.backendData); + } + /** * @async * Find a file entry by its filename. @@ -136,7 +151,7 @@ export class MediaService { */ async findUploadByFilename(filename: string): Promise { const mediaUpload = await this.mediaUploadRepository.findOne({ - where: { id: filename }, + where: { fileName: filename }, relations: ['user'], }); if (mediaUpload === null) { @@ -147,6 +162,24 @@ export class MediaService { return mediaUpload; } + /** + * @async + * Find a file entry by its UUID. + * @param {string} uuid - The UUID of the MediaUpload entity to find. + * @returns {MediaUpload} - the MediaUpload entity if found. + * @throws {NotInDBError} - the MediaUpload entity with the provided UUID is not found in the database. + */ + async findUploadByUuid(uuid: string): Promise { + const mediaUpload = await this.mediaUploadRepository.findOne({ + where: { uuid }, + relations: ['user'], + }); + if (mediaUpload === null) { + throw new NotInDBError(`MediaUpload with uuid '${uuid}' not found`); + } + return mediaUpload; + } + /** * @async * List all uploads by a specific user @@ -166,9 +199,9 @@ export class MediaService { /** * @async - * List all uploads by a specific note + * List all uploads to a specific note * @param {Note} note - the specific user - * @return {MediaUpload[]} arary of media uploads owned by the user + * @return {MediaUpload[]} array of media uploads owned by the user */ async listUploadsByNote(note: Note): Promise { const mediaUploads = await this.mediaUploadRepository @@ -188,7 +221,7 @@ export class MediaService { */ async removeNoteFromMediaUpload(mediaUpload: MediaUpload): Promise { this.logger.debug( - 'Setting note to null for mediaUpload: ' + mediaUpload.id, + 'Setting note to null for mediaUpload: ' + mediaUpload.uuid, 'removeNoteFromMediaUpload', ); mediaUpload.note = Promise.resolve(null); @@ -232,8 +265,9 @@ export class MediaService { async toMediaUploadDto(mediaUpload: MediaUpload): Promise { const user = await mediaUpload.user; return { - id: mediaUpload.id, - notePublicId: (await mediaUpload.note)?.publicId ?? null, + uuid: mediaUpload.uuid, + fileName: mediaUpload.fileName, + noteId: (await mediaUpload.note)?.publicId ?? null, createdAt: mediaUpload.createdAt, username: user?.username ?? null, }; diff --git a/backend/src/migrations/mariadb-1725266569705-init.ts b/backend/src/migrations/mariadb-1726084491570-init.ts similarity index 96% rename from backend/src/migrations/mariadb-1725266569705-init.ts rename to backend/src/migrations/mariadb-1726084491570-init.ts index 55835273b..2d0d54a13 100644 --- a/backend/src/migrations/mariadb-1725266569705-init.ts +++ b/backend/src/migrations/mariadb-1726084491570-init.ts @@ -1,19 +1,14 @@ -/* - * SPDX-FileCopyrightText: 2024 The HedgeDoc developers (see AUTHORS file) - * - * SPDX-License-Identifier: AGPL-3.0-only - */ import { MigrationInterface, QueryRunner } from 'typeorm'; -export class Init1725266569705 implements MigrationInterface { - name = 'Init1725266569705'; +export class Init1726084491570 implements MigrationInterface { + name = 'Init1726084491570'; public async up(queryRunner: QueryRunner): Promise { await queryRunner.query( `CREATE TABLE \`history_entry\` (\`id\` int NOT NULL AUTO_INCREMENT, \`pinStatus\` tinyint NOT NULL, \`updatedAt\` datetime(6) NOT NULL DEFAULT CURRENT_TIMESTAMP(6) ON UPDATE CURRENT_TIMESTAMP(6), \`userId\` int NULL, \`noteId\` int NULL, UNIQUE INDEX \`IDX_928dd947355b0837366470a916\` (\`noteId\`, \`userId\`), PRIMARY KEY (\`id\`)) ENGINE=InnoDB`, ); await queryRunner.query( - `CREATE TABLE \`media_upload\` (\`id\` varchar(255) NOT NULL, \`backendType\` varchar(255) NOT NULL, \`fileUrl\` varchar(255) NOT NULL, \`backendData\` text NULL, \`createdAt\` datetime(6) NOT NULL DEFAULT CURRENT_TIMESTAMP(6), \`noteId\` int NULL, \`userId\` int NULL, PRIMARY KEY (\`id\`)) ENGINE=InnoDB`, + `CREATE TABLE \`media_upload\` (\`uuid\` varchar(255) NOT NULL, \`fileName\` varchar(255) NOT NULL, \`backendType\` varchar(255) NOT NULL, \`backendData\` text NULL, \`createdAt\` datetime(6) NOT NULL DEFAULT CURRENT_TIMESTAMP(6), \`noteId\` int NULL, \`userId\` int NULL, PRIMARY KEY (\`uuid\`)) ENGINE=InnoDB`, ); await queryRunner.query( `CREATE TABLE \`note_group_permission\` (\`id\` int NOT NULL AUTO_INCREMENT, \`canEdit\` tinyint NOT NULL, \`groupId\` int NULL, \`noteId\` int NULL, UNIQUE INDEX \`IDX_ee1744842a9ef3ffbc05a7016a\` (\`groupId\`, \`noteId\`), PRIMARY KEY (\`id\`)) ENGINE=InnoDB`, diff --git a/backend/src/migrations/postgres-1725266697932-init.ts b/backend/src/migrations/postgres-1726084117959-init.ts similarity index 96% rename from backend/src/migrations/postgres-1725266697932-init.ts rename to backend/src/migrations/postgres-1726084117959-init.ts index c3e14897f..50c9eefcd 100644 --- a/backend/src/migrations/postgres-1725266697932-init.ts +++ b/backend/src/migrations/postgres-1726084117959-init.ts @@ -1,12 +1,7 @@ -/* - * SPDX-FileCopyrightText: 2024 The HedgeDoc developers (see AUTHORS file) - * - * SPDX-License-Identifier: AGPL-3.0-only - */ import { MigrationInterface, QueryRunner } from 'typeorm'; -export class Init1725266697932 implements MigrationInterface { - name = 'Init1725266697932'; +export class Init1726084117959 implements MigrationInterface { + name = 'Init1726084117959'; public async up(queryRunner: QueryRunner): Promise { await queryRunner.query( @@ -16,7 +11,7 @@ export class Init1725266697932 implements MigrationInterface { `CREATE UNIQUE INDEX "IDX_928dd947355b0837366470a916" ON "history_entry" ("noteId", "userId") `, ); await queryRunner.query( - `CREATE TABLE "media_upload" ("id" character varying NOT NULL, "backendType" character varying NOT NULL, "fileUrl" character varying NOT NULL, "backendData" text, "createdAt" TIMESTAMP NOT NULL DEFAULT now(), "noteId" integer, "userId" integer, CONSTRAINT "PK_b406d9cee56e253dfd3b3d52706" PRIMARY KEY ("id"))`, + `CREATE TABLE "media_upload" ("uuid" character varying NOT NULL, "fileName" character varying NOT NULL, "backendType" character varying NOT NULL, "backendData" text, "createdAt" TIMESTAMP NOT NULL DEFAULT now(), "noteId" integer, "userId" integer, CONSTRAINT "PK_573c2a4f2a8f8382f2a8758444e" PRIMARY KEY ("uuid"))`, ); await queryRunner.query( `CREATE TABLE "note_group_permission" ("id" SERIAL NOT NULL, "canEdit" boolean NOT NULL, "groupId" integer, "noteId" integer, CONSTRAINT "PK_6327989190949e6a55d02a080c3" PRIMARY KEY ("id"))`, diff --git a/backend/src/migrations/sqlite-1725268109950-init.ts b/backend/src/migrations/sqlite-1726084595852-init.ts similarity index 95% rename from backend/src/migrations/sqlite-1725268109950-init.ts rename to backend/src/migrations/sqlite-1726084595852-init.ts index da94e02e4..116d8c9e3 100644 --- a/backend/src/migrations/sqlite-1725268109950-init.ts +++ b/backend/src/migrations/sqlite-1726084595852-init.ts @@ -1,12 +1,7 @@ -/* - * SPDX-FileCopyrightText: 2024 The HedgeDoc developers (see AUTHORS file) - * - * SPDX-License-Identifier: AGPL-3.0-only - */ import { MigrationInterface, QueryRunner } from 'typeorm'; -export class Init1725268109950 implements MigrationInterface { - name = 'Init1725268109950'; +export class Init1726084595852 implements MigrationInterface { + name = 'Init1726084595852'; public async up(queryRunner: QueryRunner): Promise { await queryRunner.query( @@ -16,7 +11,7 @@ export class Init1725268109950 implements MigrationInterface { `CREATE UNIQUE INDEX "IDX_928dd947355b0837366470a916" ON "history_entry" ("noteId", "userId") `, ); await queryRunner.query( - `CREATE TABLE "media_upload" ("id" varchar PRIMARY KEY NOT NULL, "backendType" varchar NOT NULL, "fileUrl" varchar NOT NULL, "backendData" text, "createdAt" datetime NOT NULL DEFAULT (datetime('now')), "noteId" integer, "userId" integer)`, + `CREATE TABLE "media_upload" ("uuid" varchar PRIMARY KEY NOT NULL, "fileName" varchar NOT NULL, "backendType" varchar NOT NULL, "backendData" text, "createdAt" datetime NOT NULL DEFAULT (datetime('now')), "noteId" integer, "userId" integer)`, ); await queryRunner.query( `CREATE TABLE "note_group_permission" ("id" integer PRIMARY KEY AUTOINCREMENT NOT NULL, "canEdit" boolean NOT NULL, "groupId" integer, "noteId" integer)`, @@ -108,10 +103,10 @@ export class Init1725268109950 implements MigrationInterface { `CREATE UNIQUE INDEX "IDX_928dd947355b0837366470a916" ON "history_entry" ("noteId", "userId") `, ); await queryRunner.query( - `CREATE TABLE "temporary_media_upload" ("id" varchar PRIMARY KEY NOT NULL, "backendType" varchar NOT NULL, "fileUrl" varchar NOT NULL, "backendData" text, "createdAt" datetime NOT NULL DEFAULT (datetime('now')), "noteId" integer, "userId" integer, CONSTRAINT "FK_edba6d4e0f3bcf6605772f0af6b" FOREIGN KEY ("noteId") REFERENCES "note" ("id") ON DELETE NO ACTION ON UPDATE NO ACTION, CONSTRAINT "FK_73ce66b082df1df2003e305e9ac" FOREIGN KEY ("userId") REFERENCES "user" ("id") ON DELETE NO ACTION ON UPDATE NO ACTION)`, + `CREATE TABLE "temporary_media_upload" ("uuid" varchar PRIMARY KEY NOT NULL, "fileName" varchar NOT NULL, "backendType" varchar NOT NULL, "backendData" text, "createdAt" datetime NOT NULL DEFAULT (datetime('now')), "noteId" integer, "userId" integer, CONSTRAINT "FK_edba6d4e0f3bcf6605772f0af6b" FOREIGN KEY ("noteId") REFERENCES "note" ("id") ON DELETE NO ACTION ON UPDATE NO ACTION, CONSTRAINT "FK_73ce66b082df1df2003e305e9ac" FOREIGN KEY ("userId") REFERENCES "user" ("id") ON DELETE NO ACTION ON UPDATE NO ACTION)`, ); await queryRunner.query( - `INSERT INTO "temporary_media_upload"("id", "backendType", "fileUrl", "backendData", "createdAt", "noteId", "userId") SELECT "id", "backendType", "fileUrl", "backendData", "createdAt", "noteId", "userId" FROM "media_upload"`, + `INSERT INTO "temporary_media_upload"("uuid", "fileName", "backendType", "backendData", "createdAt", "noteId", "userId") SELECT "uuid", "fileName", "backendType", "backendData", "createdAt", "noteId", "userId" FROM "media_upload"`, ); await queryRunner.query(`DROP TABLE "media_upload"`); await queryRunner.query( @@ -444,10 +439,10 @@ export class Init1725268109950 implements MigrationInterface { `ALTER TABLE "media_upload" RENAME TO "temporary_media_upload"`, ); await queryRunner.query( - `CREATE TABLE "media_upload" ("id" varchar PRIMARY KEY NOT NULL, "backendType" varchar NOT NULL, "fileUrl" varchar NOT NULL, "backendData" text, "createdAt" datetime NOT NULL DEFAULT (datetime('now')), "noteId" integer, "userId" integer)`, + `CREATE TABLE "media_upload" ("uuid" varchar PRIMARY KEY NOT NULL, "fileName" varchar NOT NULL, "backendType" varchar NOT NULL, "backendData" text, "createdAt" datetime NOT NULL DEFAULT (datetime('now')), "noteId" integer, "userId" integer)`, ); await queryRunner.query( - `INSERT INTO "media_upload"("id", "backendType", "fileUrl", "backendData", "createdAt", "noteId", "userId") SELECT "id", "backendType", "fileUrl", "backendData", "createdAt", "noteId", "userId" FROM "temporary_media_upload"`, + `INSERT INTO "media_upload"("uuid", "fileName", "backendType", "backendData", "createdAt", "noteId", "userId") SELECT "uuid", "fileName", "backendType", "backendData", "createdAt", "noteId", "userId" FROM "temporary_media_upload"`, ); await queryRunner.query(`DROP TABLE "temporary_media_upload"`); await queryRunner.query(`DROP INDEX "IDX_928dd947355b0837366470a916"`); diff --git a/backend/test/private-api/me.e2e-spec.ts b/backend/test/private-api/me.e2e-spec.ts index 793e87530..36994bc5f 100644 --- a/backend/test/private-api/me.e2e-spec.ts +++ b/backend/test/private-api/me.e2e-spec.ts @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2022 The HedgeDoc developers (see AUTHORS file) + * SPDX-FileCopyrightText: 2024 The HedgeDoc developers (see AUTHORS file) * * SPDX-License-Identifier: AGPL-3.0-only */ @@ -70,16 +70,44 @@ describe('Me', () => { const testImage = await fs.readFile('test/public-api/fixtures/test.png'); const imageIds = []; imageIds.push( - (await testSetup.mediaService.saveFile(testImage, user, note1)).id, + ( + await testSetup.mediaService.saveFile( + 'test.png', + testImage, + user, + note1, + ) + ).uuid, ); imageIds.push( - (await testSetup.mediaService.saveFile(testImage, user, note1)).id, + ( + await testSetup.mediaService.saveFile( + 'test.png', + testImage, + user, + note1, + ) + ).uuid, ); imageIds.push( - (await testSetup.mediaService.saveFile(testImage, user, note2)).id, + ( + await testSetup.mediaService.saveFile( + 'test.png', + testImage, + user, + note2, + ) + ).uuid, ); imageIds.push( - (await testSetup.mediaService.saveFile(testImage, user, note2)).id, + ( + await testSetup.mediaService.saveFile( + 'test.png', + testImage, + user, + note2, + ) + ).uuid, ); const response = await agent @@ -87,10 +115,10 @@ describe('Me', () => { .expect('Content-Type', /json/) .expect(200); expect(response.body).toHaveLength(4); - expect(imageIds).toContain(response.body[0].id); - expect(imageIds).toContain(response.body[1].id); - expect(imageIds).toContain(response.body[2].id); - expect(imageIds).toContain(response.body[3].id); + expect(imageIds).toContain(response.body[0].uuid); + expect(imageIds).toContain(response.body[1].uuid); + expect(imageIds).toContain(response.body[2].uuid); + expect(imageIds).toContain(response.body[3].uuid); const mediaUploads = await testSetup.mediaService.listUploadsByUser(user); for (const upload of mediaUploads) { await testSetup.mediaService.deleteFile(upload); @@ -114,6 +142,7 @@ describe('Me', () => { it('DELETE /me', async () => { const testImage = await fs.readFile('test/public-api/fixtures/test.png'); const upload = await testSetup.mediaService.saveFile( + 'test.png', testImage, user, note1, @@ -122,7 +151,7 @@ describe('Me', () => { expect(dbUser).toBeInstanceOf(User); const mediaUploads = await testSetup.mediaService.listUploadsByUser(dbUser); expect(mediaUploads).toHaveLength(1); - expect(mediaUploads[0].id).toEqual(upload.id); + expect(mediaUploads[0].uuid).toEqual(upload.uuid); await agent.delete('/api/private/me').expect(204); await expect( testSetup.userService.getUserByUsername('hardcoded'), diff --git a/backend/test/private-api/media.e2e-spec.ts b/backend/test/private-api/media.e2e-spec.ts index 03fb5242e..ee6af4185 100644 --- a/backend/test/private-api/media.e2e-spec.ts +++ b/backend/test/private-api/media.e2e-spec.ts @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2022 The HedgeDoc developers (see AUTHORS file) + * SPDX-FileCopyrightText: 2024 The HedgeDoc developers (see AUTHORS file) * * SPDX-License-Identifier: AGPL-3.0-only */ @@ -71,17 +71,17 @@ describe('Media', () => { .set('HedgeDoc-Note', 'test_upload_media') .expect('Content-Type', /json/) .expect(201); - const fileName: string = uploadResponse.body.id; + const uuid: string = uploadResponse.body.uuid; const testImage = await fs.readFile( 'test/private-api/fixtures/test.png', ); - const path = '/api/private/media/' + fileName; + const path = '/api/private/media/' + uuid; const apiResponse = await agent.get(path); - expect(apiResponse.statusCode).toEqual(302); - const downloadResponse = await agent.get(apiResponse.header.location); + expect(apiResponse.statusCode).toEqual(200); + const downloadResponse = await agent.get(`/uploads/${uuid}.png`); expect(downloadResponse.body).toEqual(testImage); // delete the file afterwards - await fs.unlink(join(uploadPath, fileName)); + await fs.unlink(join(uploadPath, uuid + '.png')); }); it('without user', async () => { const agent = request.agent(testSetup.app.getHttpServer()); @@ -91,17 +91,17 @@ describe('Media', () => { .set('HedgeDoc-Note', 'test_upload_media') .expect('Content-Type', /json/) .expect(201); - const fileName: string = uploadResponse.body.id; + const uuid: string = uploadResponse.body.uuid; const testImage = await fs.readFile( 'test/private-api/fixtures/test.png', ); - const path = '/api/private/media/' + fileName; + const path = '/api/private/media/' + uuid; const apiResponse = await agent.get(path); - expect(apiResponse.statusCode).toEqual(302); - const downloadResponse = await agent.get(apiResponse.header.location); + expect(apiResponse.statusCode).toEqual(200); + const downloadResponse = await agent.get(`/uploads/${uuid}.png`); expect(downloadResponse.body).toEqual(testImage); // delete the file afterwards - await fs.unlink(join(uploadPath, fileName)); + await fs.unlink(join(uploadPath, uuid + '.png')); }); }); describe('fails:', () => { @@ -158,11 +158,12 @@ describe('Media', () => { ); const testImage = await fs.readFile('test/private-api/fixtures/test.png'); const upload = await testSetup.mediaService.saveFile( + 'test.png', testImage, user, testNote, ); - const filename = upload.id; + const uuid = upload.uuid; // login with a different user; const agent2 = request.agent(testSetup.app.getHttpServer()); @@ -172,15 +173,15 @@ describe('Media', () => { .expect(201); // try to delete upload with second user - await agent2.delete('/api/private/media/' + filename).expect(403); + await agent2.delete('/api/private/media/' + uuid).expect(403); - await agent.get('/uploads/' + filename).expect(200); + await agent.get(`/uploads/${uuid}.png`).expect(200); // delete upload for real - await agent.delete('/api/private/media/' + filename).expect(204); + await agent.delete('/api/private/media/' + uuid).expect(204); // Test if file is really deleted - await agent.get('/uploads/' + filename).expect(404); + await agent.get(`/uploads/${uuid}.png`).expect(404); }); it('deleting user is owner of note', async () => { // upload a file with the default test user @@ -191,11 +192,12 @@ describe('Media', () => { ); const testImage = await fs.readFile('test/private-api/fixtures/test.png'); const upload = await testSetup.mediaService.saveFile( + 'test.png', testImage, null, testNote, ); - const filename = upload.fileUrl.split('/').pop() || ''; + const uuid = upload.uuid; // login with a different user; const agent2 = request.agent(testSetup.app.getHttpServer()); @@ -207,18 +209,18 @@ describe('Media', () => { const agentGuest = request.agent(testSetup.app.getHttpServer()); // try to delete upload with second user - await agent.delete('/api/private/media/' + filename).expect(403); + await agent.delete('/api/private/media/' + uuid).expect(403); - await agent.get('/uploads/' + filename).expect(200); + await agent.get(`/uploads/${uuid}.png`).expect(200); - await agentGuest.delete('/api/private/media/' + filename).expect(401); + await agentGuest.delete('/api/private/media/' + uuid).expect(401); - await agent.get('/uploads/' + filename).expect(200); + await agent.get(`/uploads/${uuid}.png`).expect(200); // delete upload for real - await agent2.delete('/api/private/media/' + filename).expect(204); + await agent2.delete('/api/private/media/' + uuid).expect(204); // Test if file is really deleted - await agent.get('/uploads/' + filename).expect(404); + await agent.get(`/uploads/${uuid}.png`).expect(404); }); }); }); diff --git a/backend/test/private-api/notes.e2e-spec.ts b/backend/test/private-api/notes.e2e-spec.ts index c7c372fd4..25fc933a4 100644 --- a/backend/test/private-api/notes.e2e-spec.ts +++ b/backend/test/private-api/notes.e2e-spec.ts @@ -165,7 +165,12 @@ describe('Notes', () => { user1, noteId, ); - await testSetup.mediaService.saveFile(testImage, user1, note); + await testSetup.mediaService.saveFile( + 'test.png', + testImage, + user1, + note, + ); await agent .delete(`/api/private/notes/${noteId}`) .set('Content-Type', 'application/json') @@ -191,6 +196,7 @@ describe('Notes', () => { noteId, ); const upload = await testSetup.mediaService.saveFile( + 'test.png', testImage, user1, note, @@ -210,10 +216,8 @@ describe('Notes', () => { expect( await testSetup.mediaService.listUploadsByUser(user1), ).toHaveLength(1); - // Remove /upload/ from path as we just need the filename. - const fileName = upload.fileUrl.replace('/uploads/', ''); // delete the file afterwards - await fs.unlink(join(uploadPath, fileName)); + await fs.unlink(join(uploadPath, upload.uuid + '.png')); await fs.rmdir(uploadPath); }); }); @@ -406,11 +410,13 @@ describe('Notes', () => { const testImage = await fs.readFile('test/private-api/fixtures/test.png'); const upload0 = await testSetup.mediaService.saveFile( + 'test.png', testImage, user1, note1, ); const upload1 = await testSetup.mediaService.saveFile( + 'test.png', testImage, user1, note2, @@ -421,11 +427,11 @@ describe('Notes', () => { .expect('Content-Type', /json/) .expect(200); expect(responseAfter.body).toHaveLength(1); - expect(responseAfter.body[0].id).toEqual(upload0.id); - expect(responseAfter.body[0].id).not.toEqual(upload1.id); + expect(responseAfter.body[0].uuid).toEqual(upload0.uuid); + expect(responseAfter.body[0].uuid).not.toEqual(upload1.uuid); for (const upload of [upload0, upload1]) { // delete the file afterwards - await fs.unlink(join(uploadPath, upload.id)); + await fs.unlink(join(uploadPath, upload.uuid + '.png')); } await fs.rm(uploadPath, { recursive: true }); }); diff --git a/backend/test/public-api/me.e2e-spec.ts b/backend/test/public-api/me.e2e-spec.ts index 565f8e0ac..d2f5a9c12 100644 --- a/backend/test/public-api/me.e2e-spec.ts +++ b/backend/test/public-api/me.e2e-spec.ts @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2022 The HedgeDoc developers (see AUTHORS file) + * SPDX-FileCopyrightText: 2024 The HedgeDoc developers (see AUTHORS file) * * SPDX-License-Identifier: AGPL-3.0-only */ @@ -201,16 +201,44 @@ describe('Me', () => { const testImage = await fs.readFile('test/public-api/fixtures/test.png'); const imageIds = []; imageIds.push( - (await testSetup.mediaService.saveFile(testImage, user, note1)).id, + ( + await testSetup.mediaService.saveFile( + 'test.png', + testImage, + user, + note1, + ) + ).uuid, ); imageIds.push( - (await testSetup.mediaService.saveFile(testImage, user, note1)).id, + ( + await testSetup.mediaService.saveFile( + 'test.png', + testImage, + user, + note1, + ) + ).uuid, ); imageIds.push( - (await testSetup.mediaService.saveFile(testImage, user, note2)).id, + ( + await testSetup.mediaService.saveFile( + 'test.png', + testImage, + user, + note2, + ) + ).uuid, ); imageIds.push( - (await testSetup.mediaService.saveFile(testImage, user, note2)).id, + ( + await testSetup.mediaService.saveFile( + 'test.png', + testImage, + user, + note2, + ) + ).uuid, ); const response = await request(httpServer) @@ -218,13 +246,13 @@ describe('Me', () => { .expect('Content-Type', /json/) .expect(200); expect(response.body).toHaveLength(4); - expect(imageIds).toContain(response.body[0].id); - expect(imageIds).toContain(response.body[1].id); - expect(imageIds).toContain(response.body[2].id); - expect(imageIds).toContain(response.body[3].id); + expect(imageIds).toContain(response.body[0].uuid); + expect(imageIds).toContain(response.body[1].uuid); + expect(imageIds).toContain(response.body[2].uuid); + expect(imageIds).toContain(response.body[3].uuid); for (const imageId of imageIds) { // delete the file afterwards - await fs.unlink(join(uploadPath, imageId)); + await fs.unlink(join(uploadPath, imageId + '.png')); } await fs.rm(uploadPath, { recursive: true }); }); diff --git a/backend/test/public-api/media.e2e-spec.ts b/backend/test/public-api/media.e2e-spec.ts index 0634ed9ba..d34c37862 100644 --- a/backend/test/public-api/media.e2e-spec.ts +++ b/backend/test/public-api/media.e2e-spec.ts @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * SPDX-FileCopyrightText: 2024 The HedgeDoc developers (see AUTHORS file) * * SPDX-License-Identifier: AGPL-3.0-only */ @@ -49,17 +49,17 @@ describe('Media', () => { .set('HedgeDoc-Note', 'testAlias1') .expect('Content-Type', /json/) .expect(201); - const fileName = uploadResponse.body.id; - const path: string = '/api/v2/media/' + fileName; + const uuid = uploadResponse.body.uuid; + const path: string = '/api/v2/media/' + uuid; const testImage = await fs.readFile('test/public-api/fixtures/test.png'); const apiResponse = await agent .get(path) .set('Authorization', `Bearer ${testSetup.authTokens[0].secret}`); - expect(apiResponse.statusCode).toEqual(302); - const downloadResponse = await agent.get(apiResponse.header.location); + expect(apiResponse.statusCode).toEqual(200); + const downloadResponse = await agent.get(`/uploads/${uuid}.png`); expect(downloadResponse.body).toEqual(testImage); // delete the file afterwards - await fs.unlink(join(uploadPath, fileName)); + await fs.unlink(join(uploadPath, uuid + '.png')); }); describe('fails:', () => { beforeEach(async () => { @@ -114,26 +114,26 @@ describe('Media', () => { it('successfully deletes an uploaded file', async () => { const testImage = await fs.readFile('test/public-api/fixtures/test.png'); const upload = await testSetup.mediaService.saveFile( + 'test.png', testImage, testSetup.users[0], testSetup.ownedNotes[0], ); - const filename = upload.fileUrl.split('/').pop() || ''; await request(testSetup.app.getHttpServer()) - .delete('/api/v2/media/' + filename) + .delete('/api/v2/media/' + upload.uuid) .set('Authorization', `Bearer ${testSetup.authTokens[0].secret}`) .expect(204); }); it('returns an error if the user does not own the file', async () => { const testImage = await fs.readFile('test/public-api/fixtures/test.png'); const upload = await testSetup.mediaService.saveFile( + 'test.png', testImage, testSetup.users[0], testSetup.ownedNotes[0], ); - const filename = upload.fileUrl.split('/').pop() || ''; await request(testSetup.app.getHttpServer()) - .delete('/api/v2/media/' + filename) + .delete('/api/v2/media/' + upload.uuid) .set('Authorization', `Bearer ${testSetup.authTokens[1].secret}`) .expect(403); }); @@ -146,34 +146,34 @@ describe('Media', () => { ); const testImage = await fs.readFile('test/public-api/fixtures/test.png'); const upload = await testSetup.mediaService.saveFile( + 'test.png', testImage, testSetup.users[0], testNote, ); - const filename = upload.fileUrl.split('/').pop() || ''; const agent2 = request.agent(testSetup.app.getHttpServer()); // try to delete upload with second user await agent2 - .delete('/api/v2/media/' + filename) + .delete('/api/v2/media/' + upload.uuid) .set('Authorization', `Bearer ${testSetup.authTokens[1].secret}`) .expect(403); await agent2 - .get('/uploads/' + filename) + .get(`/uploads/${upload.uuid}.png`) .set('Authorization', `Bearer ${testSetup.authTokens[1].secret}`) .expect(200); // delete upload for real await agent2 - .delete('/api/v2/media/' + filename) + .delete('/api/v2/media/' + upload.uuid) .set('Authorization', `Bearer ${testSetup.authTokens[0].secret}`) .expect(204); // Test if file is really deleted await agent2 - .get('/uploads/' + filename) + .get(`/uploads/${upload.uuid}.png`) .set('Authorization', `Bearer ${testSetup.authTokens[1].secret}`) .expect(404); }); @@ -186,33 +186,33 @@ describe('Media', () => { ); const testImage = await fs.readFile('test/public-api/fixtures/test.png'); const upload = await testSetup.mediaService.saveFile( + 'test.png', testImage, testSetup.users[0], testNote, ); - const filename = upload.fileUrl.split('/').pop() || ''; const agent2 = request.agent(testSetup.app.getHttpServer()); // try to delete upload with second user await agent2 - .delete('/api/v2/media/' + filename) + .delete('/api/v2/media/' + upload.uuid) .set('Authorization', `Bearer ${testSetup.authTokens[1].secret}`) .expect(403); await agent2 - .get('/uploads/' + filename) + .get(`/uploads/${upload.uuid}.png`) .set('Authorization', `Bearer ${testSetup.authTokens[1].secret}`) .expect(200); // delete upload for real await agent2 - .delete('/api/v2/media/' + filename) + .delete('/api/v2/media/' + upload.uuid) .set('Authorization', `Bearer ${testSetup.authTokens[2].secret}`) .expect(204); // Test if file is really deleted await agent2 - .get('/uploads/' + filename) + .get(`/uploads/${upload.uuid}.png`) .set('Authorization', `Bearer ${testSetup.authTokens[1].secret}`) .expect(404); }); diff --git a/backend/test/public-api/notes.e2e-spec.ts b/backend/test/public-api/notes.e2e-spec.ts index 514b212fb..107116de0 100644 --- a/backend/test/public-api/notes.e2e-spec.ts +++ b/backend/test/public-api/notes.e2e-spec.ts @@ -158,6 +158,7 @@ describe('Notes', () => { noteId, ); await testSetup.mediaService.saveFile( + 'test.png', testImage, testSetup.users[0], note, @@ -187,6 +188,7 @@ describe('Notes', () => { noteId, ); const upload = await testSetup.mediaService.saveFile( + 'test.png', testImage, testSetup.users[0], note, @@ -207,10 +209,8 @@ describe('Notes', () => { expect( await testSetup.mediaService.listUploadsByUser(testSetup.users[0]), ).toHaveLength(1); - // Remove /upload/ from path as we just need the filename. - const fileName = upload.fileUrl.replace('/uploads/', ''); // delete the file afterwards - await fs.unlink(join(uploadPath, fileName)); + await fs.unlink(join(uploadPath, upload.uuid + '.png')); }); }); it('works with an existing alias with permissions', async () => { @@ -326,7 +326,6 @@ describe('Notes', () => { expect(metadata.body.editedBy).toEqual([]); expect(metadata.body.permissions.owner).toEqual('testuser1'); expect(metadata.body.permissions.sharedToUsers).toEqual([]); - expect(metadata.body.permissions.sharedToUsers).toEqual([]); expect(metadata.body.tags).toEqual([]); expect(typeof metadata.body.updatedAt).toEqual('string'); expect(typeof metadata.body.updateUsername).toEqual('string'); @@ -489,11 +488,13 @@ describe('Notes', () => { const testImage = await fs.readFile('test/public-api/fixtures/test.png'); const upload0 = await testSetup.mediaService.saveFile( + 'test.png', testImage, testSetup.users[0], note1, ); const upload1 = await testSetup.mediaService.saveFile( + 'test.png', testImage, testSetup.users[0], note2, @@ -505,11 +506,11 @@ describe('Notes', () => { .expect('Content-Type', /json/) .expect(200); expect(responseAfter.body).toHaveLength(1); - expect(responseAfter.body[0].id).toEqual(upload0.id); - expect(responseAfter.body[0].id).not.toEqual(upload1.id); + expect(responseAfter.body[0].uuid).toEqual(upload0.uuid); + expect(responseAfter.body[0].uuid).not.toEqual(upload1.uuid); for (const upload of [upload0, upload1]) { // delete the file afterwards - await fs.unlink(join(uploadPath, upload.id)); + await fs.unlink(join(uploadPath, upload.uuid + '.png')); } await fs.rm(uploadPath, { recursive: true }); }); diff --git a/dev-reverse-proxy/Caddyfile b/dev-reverse-proxy/Caddyfile index e43632a23..fe21c9e3a 100644 --- a/dev-reverse-proxy/Caddyfile +++ b/dev-reverse-proxy/Caddyfile @@ -16,4 +16,5 @@ reverse_proxy /realtime http://localhost:{$HD_BACKEND_PORT:3000} reverse_proxy /api/* http://localhost:{$HD_BACKEND_PORT:3000} reverse_proxy /public/* http://localhost:{$HD_BACKEND_PORT:3000} reverse_proxy /uploads/* http://localhost:{$HD_BACKEND_PORT:3000} +reverse_proxy /media/* http://localhost:{$HD_BACKEND_PORT:3000} reverse_proxy /* http://localhost:{$HD_FRONTEND_PORT:3001} diff --git a/docs/content/concepts/index.md b/docs/content/concepts/index.md index b98221ceb..55955de0c 100644 --- a/docs/content/concepts/index.md +++ b/docs/content/concepts/index.md @@ -11,6 +11,12 @@ background information and explanations. They are especially useful for contribu Notes + +
+ 📸 + Media +
+
🙎 diff --git a/docs/content/concepts/media.md b/docs/content/concepts/media.md new file mode 100644 index 000000000..362f5b8da --- /dev/null +++ b/docs/content/concepts/media.md @@ -0,0 +1,23 @@ +# Media + +!!! info "Design Document" + This is a design document, explaining the design and vision for a HedgeDoc 2 + feature. It is not a user guide and may or may not be fully implemented. + +Media is the term for uploads associated with a note in HedgeDoc. +Currently, there's only support for images. + +Media files can be saved to different storage backends like the local filesystem, S3, Azure Blob +storage, generic WebDAV shares, or imgur. +Each storage backend needs to implement an interface with three methods: + +- `saveFile(uuid, buffer, fileType)` should store a given file and may return stringified metadata + to store in the database for this upload. The metadata does not need to follow a specific format, + and will only be used inside the storage backend. +- `deleteFile(uuid, metadata)` should delete a file with the given UUID. The stored metadata can + be used for example to identify the file on the storage platform. +- `getFileUrl(uuid, metadata)` should return a URL to the file with the given UUID. The stored + metadata can be used to identify the file on the storage platform. + The returned URL may be temporary. + +Uploads are checked for their MIME type and compared to an allow-list and if not matching rejected. diff --git a/docs/content/how-to/reverse-proxy.md b/docs/content/how-to/reverse-proxy.md index f10ba9f8c..124bd2b6b 100644 --- a/docs/content/how-to/reverse-proxy.md +++ b/docs/content/how-to/reverse-proxy.md @@ -31,7 +31,7 @@ in your `docker-compose.yml`: - hedgedoc_uploads:/usr/src/app/backend/uploads labels: traefik.enable: "true" - traefik.http.routers.hedgedoc_2_backend.rule: "Host(`md.example.com`) && (PathPrefix(`/realtime`) || PathPrefix(`/api`) || PathPrefix(`/public`))" + traefik.http.routers.hedgedoc_2_backend.rule: "Host(`md.example.com`) && (PathPrefix(`/realtime`) || PathPrefix(`/api`) || PathPrefix(`/public`) || PathPrefix(`/uploads`) || PathPrefix(`/media`))" traefik.http.routers.hedgedoc_2_backend.tls: "true" traefik.http.routers.hedgedoc_2_backend.tls.certresolver: "letsencrypt" traefik.http.services.hedgedoc_2_backend.loadbalancer.server.port: "3000" @@ -113,7 +113,7 @@ Here is an example configuration for [nginx][nginx]. server { server_name md.example.com; - location ~ ^/(api|public|uploads)/ { + location ~ ^/(api|public|uploads|media)/ { proxy_pass http://127.0.0.1:3000; proxy_set_header X-Forwarded-Host $host; proxy_set_header X-Real-IP $remote_addr; @@ -173,6 +173,8 @@ Here is an example config snippet for [Apache][apache]: ProxyPassReverse /api http://127.0.0.1:3000/ ProxyPassReverse /public http://127.0.0.1:3000/ + ProxyPassReverse /uploads http://127.0.0.1:3000/ + ProxyPassReverse /media http://127.0.0.1:3000/ ProxyPassReverse /realtime http://127.0.0.1:3000/ ProxyPass / http://127.0.0.1:3001/ @@ -200,6 +202,7 @@ Here is a list of things your reverse proxy needs to do to let HedgeDoc work: - Passing `/api/*` to - Passing `/public/*` to - Passing `/uploads/*` to +- Passing `/media/*` to - Passing `/*` to - Set the `X-Forwarded-Proto` header diff --git a/docs/content/references/config/media/s3.md b/docs/content/references/config/media/s3.md index 7c0008377..5644a233c 100644 --- a/docs/content/references/config/media/s3.md +++ b/docs/content/references/config/media/s3.md @@ -7,7 +7,7 @@ Your S3 bucket must be configured to be writeable. You just add the following lines to your configuration: (with the appropriate substitution for ``, ``, -``, and `` of course) +``, ``, and `` of course) ```dotenv HD_MEDIA_BACKEND="s3" @@ -15,11 +15,16 @@ HD_MEDIA_BACKEND_S3_ACCESS_KEY="" HD_MEDIA_BACKEND_S3_SECRET_KEY="" HD_MEDIA_BACKEND_S3_BUCKET="" HD_MEDIA_BACKEND_S3_ENDPOINT="" +HD_MEDIA_BACKEND_S3_REGION="" +HD_MEDIA_BACKEND_S3_PATH_STYLE="" ``` `` should be an URL and contain the protocol, the domain and if necessary the port. For example: `https://s3.example.org` or `http://s3.example.org:9000` +`` should be set to `true` if you are using a S3-compatible storage like MinIO that +uses path-style URLs. + If you use Amazon S3, `` should contain your [Amazon Region][amazon-region]. For example: If your Amazon Region is `us-east-2`,your endpoint `` should be `https://s3.us-east-2.amazonaws.com`. diff --git a/frontend/cypress/e2e/fileUpload.spec.ts b/frontend/cypress/e2e/fileUpload.spec.ts index abe92a952..6da69b4b3 100644 --- a/frontend/cypress/e2e/fileUpload.spec.ts +++ b/frontend/cypress/e2e/fileUpload.spec.ts @@ -3,8 +3,7 @@ * * SPDX-License-Identifier: AGPL-3.0-only */ - -const imageId = 'non-existing.png' +const fakeUuid = '77fdcf1c-35fa-4a65-bdcf-1c35fa8a65d5' describe('File upload', () => { beforeEach(() => { @@ -22,7 +21,8 @@ describe('File upload', () => { { statusCode: 201, body: { - id: imageId + uuid: fakeUuid, + fileName: 'demo.png' } } ) @@ -38,7 +38,7 @@ describe('File upload', () => { }, { force: true } ) - cy.get('.cm-line').contains(`![demo.png](http://127.0.0.1:3001/api/private/media/${imageId})`) + cy.get('.cm-line').contains(`![demo.png](http://127.0.0.1:3001/media/${fakeUuid})`) }) it('via paste', () => { @@ -51,7 +51,7 @@ describe('File upload', () => { } } cy.get('.cm-content').trigger('paste', pasteEvent) - cy.get('.cm-line').contains(`![](http://127.0.0.1:3001/api/private/media/${imageId})`) + cy.get('.cm-line').contains(`![](http://127.0.0.1:3001/media/${fakeUuid})`) }) }) @@ -65,7 +65,7 @@ describe('File upload', () => { }, { action: 'drag-drop', force: true } ) - cy.get('.cm-line').contains(`![demo.png](http://127.0.0.1:3001/api/private/media/${imageId})`) + cy.get('.cm-line').contains(`![demo.png](http://127.0.0.1:3001/media/${fakeUuid})`) }) }) diff --git a/frontend/src/api/media/index.ts b/frontend/src/api/media/index.ts index 1f1b9e8c8..ee04ff99e 100644 --- a/frontend/src/api/media/index.ts +++ b/frontend/src/api/media/index.ts @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2022 The HedgeDoc developers (see AUTHORS file) + * SPDX-FileCopyrightText: 2024 The HedgeDoc developers (see AUTHORS file) * * SPDX-License-Identifier: AGPL-3.0-only */ @@ -31,7 +31,7 @@ export const getProxiedUrl = async (imageUrl: string): Promise => { +export const uploadFile = async (noteIdOrAlias: string, media: File): Promise => { const postData = new FormData() postData.append('file', media) const response = await new PostApiRequestBuilder('media') diff --git a/frontend/src/api/media/types.ts b/frontend/src/api/media/types.ts index 6b888d761..39231c866 100644 --- a/frontend/src/api/media/types.ts +++ b/frontend/src/api/media/types.ts @@ -4,10 +4,11 @@ * SPDX-License-Identifier: AGPL-3.0-only */ export interface MediaUpload { - id: string + uuid: string + fileName: string noteId: string | null createdAt: string - username: string + username: string | null } export interface ImageProxyResponse { diff --git a/frontend/src/components/editor-page/editor-pane/hooks/use-handle-upload.tsx b/frontend/src/components/editor-page/editor-pane/hooks/use-handle-upload.tsx index f6917822a..898968e1c 100644 --- a/frontend/src/components/editor-page/editor-pane/hooks/use-handle-upload.tsx +++ b/frontend/src/components/editor-page/editor-pane/hooks/use-handle-upload.tsx @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2022 The HedgeDoc developers (see AUTHORS file) + * SPDX-FileCopyrightText: 2024 The HedgeDoc developers (see AUTHORS file) * * SPDX-License-Identifier: AGPL-3.0-only */ @@ -60,8 +60,8 @@ export const useHandleUpload = (): handleUploadSignature => { return replaceSelection(cursorSelection ?? currentSelection, uploadPlaceholder, false) }) uploadFile(noteId, file) - .then(({ id }) => { - const fullUrl = `${baseUrl}api/private/media/${id}` + .then(({ uuid }) => { + const fullUrl = `${baseUrl}media/${uuid}` const replacement = `![${description ?? file.name ?? ''}](${fullUrl}${additionalUrlText ?? ''})` changeContent(({ markdownContent }) => [ replaceInContent(markdownContent, uploadPlaceholder, replacement), diff --git a/frontend/src/components/editor-page/sidebar/specific-sidebar-entries/media-browser-sidebar-menu/media-browser-sidebar-menu.tsx b/frontend/src/components/editor-page/sidebar/specific-sidebar-entries/media-browser-sidebar-menu/media-browser-sidebar-menu.tsx index ebc892838..287761351 100644 --- a/frontend/src/components/editor-page/sidebar/specific-sidebar-entries/media-browser-sidebar-menu/media-browser-sidebar-menu.tsx +++ b/frontend/src/components/editor-page/sidebar/specific-sidebar-entries/media-browser-sidebar-menu/media-browser-sidebar-menu.tsx @@ -49,7 +49,7 @@ export const MediaBrowserSidebarMenu: React.FC = ({ if (loading || error || !value) { return [] } - return value.map((entry) => ) + return value.map((entry) => ) }, [value, loading, error, setMediaEntryForDeletion]) const cancelDeletion = useCallback(() => { diff --git a/frontend/src/components/editor-page/sidebar/specific-sidebar-entries/media-browser-sidebar-menu/media-entry-deletion-modal.tsx b/frontend/src/components/editor-page/sidebar/specific-sidebar-entries/media-browser-sidebar-menu/media-entry-deletion-modal.tsx index 5e13fcd81..c7df0e97c 100644 --- a/frontend/src/components/editor-page/sidebar/specific-sidebar-entries/media-browser-sidebar-menu/media-entry-deletion-modal.tsx +++ b/frontend/src/components/editor-page/sidebar/specific-sidebar-entries/media-browser-sidebar-menu/media-entry-deletion-modal.tsx @@ -25,7 +25,7 @@ export const MediaEntryDeletionModal: React.FC = ( const { showErrorNotification, dispatchUiNotification } = useUiNotifications() const handleDelete = useCallback(() => { - deleteUploadedMedia(entry.id) + deleteUploadedMedia(entry.uuid) .then(() => { dispatchUiNotification('common.success', 'editor.mediaBrowser.mediaDeleted', {}) }) diff --git a/frontend/src/components/editor-page/sidebar/specific-sidebar-entries/media-browser-sidebar-menu/media-entry.module.css b/frontend/src/components/editor-page/sidebar/specific-sidebar-entries/media-browser-sidebar-menu/media-entry.module.css new file mode 100644 index 000000000..80529fec3 --- /dev/null +++ b/frontend/src/components/editor-page/sidebar/specific-sidebar-entries/media-browser-sidebar-menu/media-entry.module.css @@ -0,0 +1,11 @@ +/* + * SPDX-FileCopyrightText: 2024 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +.preview { + max-width: 100%; + max-height: 150px; + height: auto; + width: auto; +} diff --git a/frontend/src/components/editor-page/sidebar/specific-sidebar-entries/media-browser-sidebar-menu/media-entry.tsx b/frontend/src/components/editor-page/sidebar/specific-sidebar-entries/media-browser-sidebar-menu/media-entry.tsx index 8add73701..10fd6bbb4 100644 --- a/frontend/src/components/editor-page/sidebar/specific-sidebar-entries/media-browser-sidebar-menu/media-entry.tsx +++ b/frontend/src/components/editor-page/sidebar/specific-sidebar-entries/media-browser-sidebar-menu/media-entry.tsx @@ -11,13 +11,15 @@ import { Trash as IconTrash, FileRichtextFill as IconFileRichtextFill, Person as IconPerson, - Clock as IconClock + Clock as IconClock, + FileText as IconFileText } from 'react-bootstrap-icons' import { useIsOwner } from '../../../../../hooks/common/use-is-owner' import { useApplicationState } from '../../../../../hooks/common/use-application-state' import { UserAvatarForUsername } from '../../../../common/user-avatar/user-avatar-for-username' import { useChangeEditorContentCallback } from '../../../change-content-context/use-change-editor-content-callback' import { replaceSelection } from '../../../editor-pane/tool-bar/formatters/replace-selection' +import styles from './media-entry.module.css' export interface MediaEntryProps { entry: MediaUpload @@ -37,7 +39,7 @@ export const MediaEntry: React.FC = ({ entry, onDelete }) => { const isOwner = useIsOwner() const imageUrl = useMemo(() => { - return `${baseUrl}api/private/media/${entry.id}` + return `${baseUrl}media/${entry.uuid}` }, [entry, baseUrl]) const textCreatedTime = useMemo(() => { return new Date(entry.createdAt).toLocaleString() @@ -47,7 +49,7 @@ export const MediaEntry: React.FC = ({ entry, onDelete }) => { changeEditorContent?.(({ currentSelection }) => { return replaceSelection( { from: currentSelection.to ?? currentSelection.from }, - `![${entry.id}](${imageUrl})`, + `![${entry.fileName}](${imageUrl})`, true ) }) @@ -61,10 +63,15 @@ export const MediaEntry: React.FC = ({ entry, onDelete }) => {
{/* eslint-disable-next-line @next/next/no-img-element */} - {`Upload + {`Upload
+ + + {entry.fileName} + +
diff --git a/frontend/src/pages/api/private/me/media.ts b/frontend/src/pages/api/private/me/media.ts index 8e28689ee..fbdfc7074 100644 --- a/frontend/src/pages/api/private/me/media.ts +++ b/frontend/src/pages/api/private/me/media.ts @@ -12,13 +12,15 @@ const handler = (req: NextApiRequest, res: NextApiResponse) => { { username: 'tilman', createdAt: '2022-03-20T20:36:32Z', - id: 'dummy.png', + uuid: '5355ed83-7e12-4db0-95ed-837e124db08c', + fileName: 'dummy.png', noteId: 'features' }, { username: 'tilman', createdAt: '2022-03-20T20:36:57+0000', - id: 'dummy.png', + uuid: '656745ab-fbf9-47f1-a745-abfbf9a7f10c', + fileName: 'dummy2.png', noteId: null } ]) diff --git a/frontend/src/pages/api/private/media.ts b/frontend/src/pages/api/private/media.ts index e0aaf2956..e48473500 100644 --- a/frontend/src/pages/api/private/media.ts +++ b/frontend/src/pages/api/private/media.ts @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2022 The HedgeDoc developers (see AUTHORS file) + * SPDX-FileCopyrightText: 2024 The HedgeDoc developers (see AUTHORS file) * * SPDX-License-Identifier: AGPL-3.0-only */ @@ -20,7 +20,8 @@ const handler = async (req: NextApiRequest, res: NextApiResponse): Promise req, res, { - id: '/public/img/avatar.png', + uuid: 'e81f57cd-5866-4253-9f57-cd5866a253ca', + fileName: 'avatar.png', noteId: null, username: 'test', createdAt: '2022-02-27T21:54:23.856Z'