From c2ccf48c3966608f7292c6818d1b95469aa1b3cd Mon Sep 17 00:00:00 2001 From: Erik Michelson Date: Sun, 5 Jul 2026 01:29:36 +0200 Subject: [PATCH] feat(media): permission checks for accessing media uploads Previously, media uploads were associated with one note upload but served regardless of the requesting user's permission. As we have a fine-grade permission system in place, we can use this as well for enforcing permission checks on media uploads. In order to make an upload reusable across multiple notes, this adds the capability to link one media upload to multiple notes and unlink that accordingly again. Signed-off-by: Erik Michelson --- .../src/api/private/media/media.controller.ts | 56 +++- .../src/api/private/notes/notes.controller.ts | 2 +- .../src/api/public/media/media.controller.ts | 32 ++- .../src/api/public/notes/notes.controller.ts | 2 +- .../migrations/20250312211152_initial.js | 34 ++- backend/src/database/types/knex.types.ts | 3 + .../media-redirect.controller.ts | 13 +- backend/src/media/media.module.ts | 3 +- backend/src/media/media.service.spec.ts | 119 ++++---- backend/src/media/media.service.ts | 260 ++++++++++++------ backend/src/permissions/permission.service.ts | 53 ++-- .../permissions/permissions.service.spec.ts | 64 ++--- .../private-api/private-api.media.e2e-spec.ts | 115 +++++++- .../public-api/public-api.media.e2e-spec.ts | 8 +- commons/src/dtos/media/media-upload.dto.ts | 9 +- database/src/types/media-upload.ts | 21 +- frontend/src/pages/api/private/me/media.ts | 4 +- frontend/src/pages/api/private/media.ts | 2 +- 18 files changed, 531 insertions(+), 269 deletions(-) diff --git a/backend/src/api/private/media/media.controller.ts b/backend/src/api/private/media/media.controller.ts index 685aca441..1d601c4e2 100644 --- a/backend/src/api/private/media/media.controller.ts +++ b/backend/src/api/private/media/media.controller.ts @@ -3,17 +3,18 @@ * * SPDX-License-Identifier: AGPL-3.0-only */ -import { MediaUploadSchema } from '@hedgedoc/commons'; -import { PermissionLevel } from '@hedgedoc/commons'; +import { MediaUploadSchema, PermissionLevel } from '@hedgedoc/commons'; +import { FieldNameMediaUpload } from '@hedgedoc/database'; import { - BadRequestException, Controller, Delete, Get, Param, Post, + Put, UseGuards, UseInterceptors, + BadRequestException, } from '@nestjs/common'; import { ApiBody, ApiConsumes, ApiHeader, ApiTags } from '@nestjs/swagger'; @@ -23,6 +24,7 @@ import { PermissionError } from '../../../errors/errors'; import { ConsoleLoggerService } from '../../../logger/console-logger.service'; import { MediaService } from '../../../media/media.service'; import { MulterFile } from '../../../media/multer-file.interface'; +import { NoteService } from '../../../notes/note.service'; import { PermissionService } from '../../../permissions/permission.service'; import { PermissionsGuard } from '../../../permissions/permissions.guard'; import { RequirePermission } from '../../../permissions/require-permission.decorator'; @@ -40,6 +42,7 @@ export class MediaController { constructor( private readonly logger: ConsoleLoggerService, private mediaService: MediaService, + private noteService: NoteService, private permissionsService: PermissionService, ) { this.logger.setContext(MediaController.name); @@ -80,7 +83,7 @@ export class MediaController { @FastifyFile('file') file: MulterFile | undefined, @RequestNoteId() noteId: number, @RequestUserId() userId: number, - ): Promise { + ): Promise { if (file === undefined) { throw new BadRequestException('Request does not contain a file'); } @@ -88,13 +91,46 @@ export class MediaController { `Received filename '${file.originalname}' for note '${noteId}' from user '${userId}'`, 'uploadMedia', ); - return await this.mediaService.saveFile(file.originalname, file.buffer, userId, noteId); + const uploadUuid = await this.mediaService.saveFile( + file.originalname, + file.buffer, + userId, + noteId, + ); + return await this.mediaService.getMediaUploadDtoByUuid(uploadUuid); + } + + @Put(':uuid/notes/:noteAlias') + @OpenApi(204, 403, 404, 500) + async linkMediaToNote( + @RequestUserId() userId: number, + @Param('uuid') uuid: string, + @Param('noteAlias') noteAlias: string, + ): Promise { + const mediaUpload = await this.mediaService.findUploadByUuid(uuid); + const noteIdNumber = await this.noteService.getNoteIdByAlias(noteAlias); + if (mediaUpload[FieldNameMediaUpload.userId] !== userId) { + throw new PermissionError('Only the uploader may link this media upload to a note'); + } + if ( + (await this.permissionsService.determinePermission(userId, noteIdNumber)) < + PermissionLevel.WRITE + ) { + throw new PermissionError('You do not have permission to write to this note'); + } + await this.mediaService.addNoteToMediaUpload(uuid, noteIdNumber); } @Get(':uuid') @OpenApi(200, 404, 500) - async getMedia(@Param('uuid') uuid: string): Promise { - return (await this.mediaService.getMediaUploadDtosByUuids([uuid]))[0]; + async getMedia( + @RequestUserId() userId: number, + @Param('uuid') uuid: string, + ): Promise { + if (!(await this.mediaService.canUserAccessUpload(userId, uuid))) { + throw new PermissionError('You do not have permission to access this media upload'); + } + return await this.mediaService.getMediaUploadDtoByUuid(uuid); } @Delete(':uuid') @@ -106,12 +142,10 @@ export class MediaController { ); if (!hasUserMediaDeletePermission) { this.logger.warn( - `${userId} tried to delete '${uuid}', but is not the owner of upload or connected note`, + `User ${userId} tried to delete '${uuid}', but is not the owner of upload or connected note`, 'deleteMedia', ); - throw new PermissionError( - `'${userId}' does neither own the upload '${uuid}' nor the note associacted with this upload'`, - ); + throw new PermissionError(`User does neither own the upload '${uuid}' nor a linked note`); } this.logger.debug(`Deleting '${uuid}' for user '${userId}'`, 'deleteMedia'); await this.mediaService.deleteFile(uuid); diff --git a/backend/src/api/private/notes/notes.controller.ts b/backend/src/api/private/notes/notes.controller.ts index fedd7032d..6e94faa4f 100644 --- a/backend/src/api/private/notes/notes.controller.ts +++ b/backend/src/api/private/notes/notes.controller.ts @@ -134,7 +134,7 @@ export class NotesController { if (!noteMediaDeletionDto.keepMedia) { await this.mediaService.deleteFile(mediaUpload); } else { - await this.mediaService.removeNoteFromMediaUpload(mediaUpload); + await this.mediaService.removeNoteFromMediaUpload(mediaUpload, noteId); } } await this.noteService.deleteNote(noteId); diff --git a/backend/src/api/public/media/media.controller.ts b/backend/src/api/public/media/media.controller.ts index 8c213c253..e5aa80aea 100644 --- a/backend/src/api/public/media/media.controller.ts +++ b/backend/src/api/public/media/media.controller.ts @@ -4,7 +4,6 @@ * SPDX-License-Identifier: AGPL-3.0-only */ import { MediaUploadSchema, PermissionLevel } from '@hedgedoc/commons'; -import { FieldNameMediaUpload } from '@hedgedoc/database'; import { BadRequestException, Controller, @@ -78,7 +77,7 @@ export class MediaController { @RequirePermission(PermissionLevel.WRITE) async uploadMedia( @RequestUserId() userId: number, - @FastifyFile('file') file: MulterFile, + @FastifyFile('file') file: MulterFile | undefined, @RequestNoteId() noteId: number, ): Promise { if (file === undefined) { @@ -94,31 +93,34 @@ export class MediaController { userId, noteId, ); - return (await this.mediaService.getMediaUploadDtosByUuids([uploadUuid]))[0]; + return await this.mediaService.getMediaUploadDtoByUuid(uploadUuid); } @Get(':uuid') @OpenApi(200, 404, 500) - async getMedia(@Param('uuid') uuid: string): Promise { - return (await this.mediaService.getMediaUploadDtosByUuids([uuid]))[0]; + async getMedia( + @RequestUserId() userId: number, + @Param('uuid') uuid: string, + ): Promise { + if (!(await this.mediaService.canUserAccessUpload(userId, uuid))) { + throw new PermissionError('You do not have permission to access this media upload.'); + } + return await this.mediaService.getMediaUploadDtoByUuid(uuid); } @Delete(':uuid') @OpenApi(204, 404, 500) async deleteMedia(@RequestUserId() userId: number, @Param('uuid') uuid: string): Promise { - const mediaUpload = await this.mediaService.findUploadByUuid(uuid); if (await this.permissionsService.checkMediaDeletePermission(userId, uuid)) { this.logger.debug(`Deleting '${uuid}' for user '${userId}'`, 'deleteMedia'); await this.mediaService.deleteFile(uuid); - } else { - this.logger.warn( - `${userId} tried to delete '${uuid}', but is not the owner of upload or connected note`, - 'deleteMedia', - ); - const mediaUploadNote = mediaUpload[FieldNameMediaUpload.noteId]; - throw new PermissionError( - `Neither file '${uuid}' nor note '${mediaUploadNote ?? 'unknown'}'is owned by '${userId}'`, - ); + return; } + + this.logger.warn( + `${userId} tried to delete '${uuid}', but is not the owner of upload or connected note`, + 'deleteMedia', + ); + throw new PermissionError(`Neither file '${uuid}' nor a linked note is owned by the user`); } } diff --git a/backend/src/api/public/notes/notes.controller.ts b/backend/src/api/public/notes/notes.controller.ts index 5087bce3a..229073b64 100644 --- a/backend/src/api/public/notes/notes.controller.ts +++ b/backend/src/api/public/notes/notes.controller.ts @@ -131,7 +131,7 @@ export class NotesController { if (!noteMediaDeletionDto.keepMedia) { await this.mediaService.deleteFile(mediaUpload); } else { - await this.mediaService.removeNoteFromMediaUpload(mediaUpload); + await this.mediaService.removeNoteFromMediaUpload(mediaUpload, noteId); } } this.logger.debug(`Deleting note: ${noteId}`, 'deleteNote'); diff --git a/backend/src/database/migrations/20250312211152_initial.js b/backend/src/database/migrations/20250312211152_initial.js index 24edf7910..91e5b2d52 100644 --- a/backend/src/database/migrations/20250312211152_initial.js +++ b/backend/src/database/migrations/20250312211152_initial.js @@ -32,6 +32,7 @@ const { TableGroupUser, TableIdentity, TableMediaUpload, + TableMediaUploadNote, TableNote, TableNoteGroupPermission, TableNoteUserPermission, @@ -41,6 +42,7 @@ const { TableUserPinnedNote, TableVisitedNote, FieldNameSession, + FieldNameMediaUploadNote, TableSession, } = require('@hedgedoc/database'); @@ -318,13 +320,6 @@ const up = async function (knex) { // Create media_upload table await knex.schema.createTable(TableMediaUpload, (table) => { table.uuid(FieldNameMediaUpload.uuid).primary(); - table - .integer(FieldNameMediaUpload.noteId) - .unsigned() - .nullable() - .references(FieldNameNote.id) - .inTable(TableNote) - .onDelete('SET NULL'); table .integer(FieldNameMediaUpload.userId) .unsigned() @@ -353,10 +348,32 @@ const up = async function (knex) { useTz: false, precision: 3, }); - table.index([FieldNameMediaUpload.noteId], 'idx_media_upload_note_id'); table.index([FieldNameMediaUpload.userId], 'idx_media_upload_user_id'); }); + // Create media_upload_note join table + await knex.schema.createTable(TableMediaUploadNote, (table) => { + table + .uuid(FieldNameMediaUploadNote.mediaUploadUuid) + .notNullable() + .references(FieldNameMediaUpload.uuid) + .inTable(TableMediaUpload) + .onDelete('CASCADE'); + table + .integer(FieldNameMediaUploadNote.noteId) + .unsigned() + .notNullable() + .references(FieldNameNote.id) + .inTable(TableNote) + .onDelete('CASCADE'); + table.primary([FieldNameMediaUploadNote.mediaUploadUuid, FieldNameMediaUploadNote.noteId]); + table.index( + [FieldNameMediaUploadNote.mediaUploadUuid], + 'idx_media_upload_note_media_upload_uuid', + ); + table.index([FieldNameMediaUploadNote.noteId], 'idx_media_upload_note_note_id'); + }); + // Create user_pinned_note table await knex.schema.createTable(TableUserPinnedNote, (table) => { table @@ -451,6 +468,7 @@ const down = async function (knex) { await knex.schema.dropTableIfExists(TableSession); await knex.schema.dropTableIfExists(TableVisitedNote); await knex.schema.dropTableIfExists(TableUserPinnedNote); + await knex.schema.dropTableIfExists(TableMediaUploadNote); await knex.schema.dropTableIfExists(TableMediaUpload); await knex.schema.dropTableIfExists(TableNoteGroupPermission); await knex.schema.dropTableIfExists(TableNoteUserPermission); diff --git a/backend/src/database/types/knex.types.ts b/backend/src/database/types/knex.types.ts index 0dfa111bd..f27e11e02 100644 --- a/backend/src/database/types/knex.types.ts +++ b/backend/src/database/types/knex.types.ts @@ -11,6 +11,7 @@ import { GroupUser, Identity, MediaUpload, + MediaUploadNote, Note, NoteGroupPermission, NoteUserPermission, @@ -24,6 +25,7 @@ import { TableGroupUser, TableIdentity, TableMediaUpload, + TableMediaUploadNote, TableNote, TableNoteGroupPermission, TableNoteUserPermission, @@ -62,6 +64,7 @@ declare module 'knex/types/tables.js' { MediaUpload, TypeUpdateMediaUpload >; + [TableMediaUploadNote]: MediaUploadNote; [TableNote]: KnexOriginal.CompositeTableType; [TableNoteGroupPermission]: KnexOriginal.CompositeTableType< NoteGroupPermission, diff --git a/backend/src/media-redirect/media-redirect.controller.ts b/backend/src/media-redirect/media-redirect.controller.ts index 6ce81a37d..3d82c8a97 100644 --- a/backend/src/media-redirect/media-redirect.controller.ts +++ b/backend/src/media-redirect/media-redirect.controller.ts @@ -10,6 +10,8 @@ import { FastifyReply } from 'fastify'; import { OpenApi } from '../api/utils/decorators/openapi.decorator'; import { ConsoleLoggerService } from '../logger/console-logger.service'; import { MediaService } from '../media/media.service'; +import { RequestUserId } from '../api/utils/decorators/request-user-id.decorator'; +import { PermissionError } from '../errors/errors'; @OpenApi() @ApiTags('media-redirect') @@ -24,8 +26,15 @@ export class MediaRedirectController { @Get(':uuid') @OpenApi(302, 404, 500) - async getMedia(@Param('uuid') uuid: string, @Res() response: FastifyReply): Promise { + async getMedia( + @RequestUserId() userId: number, + @Param('uuid') uuid: string, + @Res() response: FastifyReply, + ): Promise { + if (!(await this.mediaService.canUserAccessUpload(userId, uuid))) { + throw new PermissionError('You do not have permission to access this media upload.'); + } const url = await this.mediaService.getFileUrl(uuid); - response.redirect(url); + await response.redirect(url); } } diff --git a/backend/src/media/media.module.ts b/backend/src/media/media.module.ts index c68e08a8c..157a27644 100644 --- a/backend/src/media/media.module.ts +++ b/backend/src/media/media.module.ts @@ -11,9 +11,10 @@ import { ImgurBackend } from './backends/imgur-backend'; import { S3Backend } from './backends/s3-backend'; import { WebdavBackend } from './backends/webdav-backend'; import { MediaService } from './media.service'; +import { PermissionsModule } from '../permissions/permissions.module'; @Module({ - imports: [], + imports: [PermissionsModule], providers: [ MediaService, FilesystemBackend, diff --git a/backend/src/media/media.service.spec.ts b/backend/src/media/media.service.spec.ts index 43fa7bf2d..07e8fc43a 100644 --- a/backend/src/media/media.service.spec.ts +++ b/backend/src/media/media.service.spec.ts @@ -5,13 +5,11 @@ */ import { describe, it, expect, beforeAll, afterEach, jest } from '@jest/globals'; import { - FieldNameAlias, FieldNameMediaUpload, - FieldNameUser, + FieldNameMediaUploadNote, MediaBackendType, - TableAlias, TableMediaUpload, - TableUser, + TableMediaUploadNote, } from '@hedgedoc/database'; import { Provider } from '@nestjs/common'; import { ConfigModule } from '@nestjs/config'; @@ -24,10 +22,11 @@ import appConfigMock from '../config/mock/app.config.mock'; import databaseConfigMock from '../config/mock/database.config.mock'; import mediaConfigMock from '../config/mock/media.config.mock'; import { expectBindings } from '../database/mock/expect-bindings'; -import { mockDelete, mockInsert, mockSelect, mockUpdate } from '../database/mock/mock-queries'; +import { mockDelete, mockInsert, mockSelect } from '../database/mock/mock-queries'; import { mockKnexDb } from '../database/mock/provider'; import { ClientError, NotInDBError } from '../errors/errors'; import { LoggerModule } from '../logger/logger.module'; +import { PermissionService } from '../permissions/permission.service'; import { dateTimeToDB, getCurrentDateTime } from '../utils/datetime'; import { FilesystemBackend } from './backends/filesystem-backend'; import { MediaService } from './media.service'; @@ -44,8 +43,6 @@ describe('MediaService', () => { const backendData = JSON.stringify({ ext: 'png' }); const fileBuffer = Buffer.from('test'); const username = 'testuser'; - const alias = 'note-alias'; - const createdAt = '2025-11-05 20:39:25'; const createdAtIso = '2025-11-05T20:39:25.000Z'; let service: MediaService; @@ -57,7 +54,12 @@ describe('MediaService', () => { [tracker, knexProvider] = mockKnexDb(); const module: TestingModule = await Test.createTestingModule({ - providers: [MediaService, knexProvider, FilesystemBackend], + providers: [ + MediaService, + knexProvider, + FilesystemBackend, + { provide: PermissionService, useValue: { canReadNote: jest.fn() } }, + ], imports: [ LoggerModule, await ConfigModule.forRoot({ @@ -97,12 +99,17 @@ describe('MediaService', () => { FieldNameMediaUpload.backendType, FieldNameMediaUpload.createdAt, FieldNameMediaUpload.fileName, - FieldNameMediaUpload.noteId, FieldNameMediaUpload.userId, FieldNameMediaUpload.uuid, ], [{ [FieldNameMediaUpload.uuid]: uuid }], ); + mockInsert( + tracker, + TableMediaUploadNote, + [FieldNameMediaUploadNote.mediaUploadUuid, FieldNameMediaUploadNote.noteId], + [{ [FieldNameMediaUploadNote.mediaUploadUuid]: uuid }], + ); jest .spyOn(service.mediaBackend, 'saveFile') .mockImplementationOnce( @@ -121,7 +128,8 @@ describe('MediaService', () => { const result = await service.saveFile(fileName, fileBuffer, userId, noteId); expect(result).toBe(uuid); expectBindings(tracker, 'insert', [ - [backendData, backendType, dateTimeToDB(now), fileName, noteId, userId, uuid], + [backendData, backendType, dateTimeToDB(now), fileName, userId, uuid], + [uuid, noteId], ]); jest.useRealTimers(); }); @@ -185,7 +193,9 @@ describe('MediaService', () => { it('returns file url if found', async () => { mockSelect( tracker, - [FieldNameMediaUpload.backendType, FieldNameMediaUpload.backendData], + [ + FieldNameMediaUpload.backendType, + FieldNameMediaUpload.backendData], TableMediaUpload, FieldNameMediaUpload.uuid, { @@ -206,19 +216,31 @@ describe('MediaService', () => { ); const result = await service.getFileUrl(uuid); expect(result).toBe(`http://example.com/${fileName}`); - expectBindings(tracker, 'select', [[uuid]], true); + expect(tracker.history.select).toHaveLength(1); + expect(tracker.history.select[0].bindings).toEqual([uuid, 1]); }); it('throws NotInDBError if not found', async () => { mockSelect( tracker, - [FieldNameMediaUpload.backendType, FieldNameMediaUpload.backendData], + [ + FieldNameMediaUpload.backendType, + FieldNameMediaUpload.backendData, + FieldNameMediaUpload.fileName, + ], TableMediaUpload, FieldNameMediaUpload.uuid, undefined, ); - await expect(service.getFileUrl(uuid)).rejects.toThrow(NotInDBError); - expectBindings(tracker, 'select', [[uuid]], true); + await expect(service.getFileResponse(uuid)).rejects.toThrow(NotInDBError); + }); + + it('throws NotInDBError when given an invalid uuid', async () => { + jest.spyOn(uuidModule, 'validate').mockReturnValueOnce(false); + await expect(service.getFileResponse(invalidUuid)).rejects.toThrow( + new NotInDBError('Invalid media upload id provided', 'MediaService', 'getFileResponse'), + ); + expect(tracker.history.select).toHaveLength(0); }); }); @@ -256,12 +278,12 @@ describe('MediaService', () => { describe('getMediaUploadUuidsByNoteId', () => { it('returns uuids for note', async () => { - const rows = [{ [FieldNameMediaUpload.uuid]: uuid }]; + const rows = [{ [FieldNameMediaUploadNote.mediaUploadUuid]: uuid }]; mockSelect( tracker, - [FieldNameMediaUpload.uuid], - TableMediaUpload, - FieldNameMediaUpload.noteId, + [FieldNameMediaUploadNote.mediaUploadUuid], + TableMediaUploadNote, + FieldNameMediaUploadNote.noteId, rows, ); const result = await service.getMediaUploadUuidsByNoteId(noteId); @@ -270,15 +292,13 @@ describe('MediaService', () => { }); describe('removeNoteFromMediaUpload', () => { - it('updates noteId to null', async () => { - mockUpdate( - tracker, - TableMediaUpload, - [FieldNameMediaUpload.noteId], - FieldNameMediaUpload.uuid, - ); - await service.removeNoteFromMediaUpload(uuid); - expectBindings(tracker, 'update', [[null, uuid]]); + it('deletes note association', async () => { + mockDelete(tracker, TableMediaUploadNote, [ + FieldNameMediaUploadNote.mediaUploadUuid, + FieldNameMediaUploadNote.noteId, + ]); + await service.removeNoteFromMediaUpload(uuid, noteId); + expectBindings(tracker, 'delete', [[uuid, noteId]]); }); }); @@ -294,45 +314,20 @@ describe('MediaService', () => { describe('getMediaUploadDtosByUuids', () => { it('returns media upload dtos', async () => { - const rows = [ - { - [FieldNameMediaUpload.uuid]: uuid, - [FieldNameMediaUpload.fileName]: fileName, - [FieldNameMediaUpload.createdAt]: createdAt, - [FieldNameUser.username]: username, - [FieldNameAlias.alias]: alias, - }, - ]; - mockSelect( - tracker, - [ - `${TableMediaUpload}"."${FieldNameMediaUpload.uuid}`, - `${TableMediaUpload}"."${FieldNameMediaUpload.fileName}`, - `${TableMediaUpload}"."${FieldNameMediaUpload.createdAt}`, - `${TableUser}"."${FieldNameUser.username}`, - `${TableAlias}"."${FieldNameAlias.alias}`, - ], - TableMediaUpload, - FieldNameMediaUpload.uuid, - rows, - [ - { - joinTable: TableAlias, - keyLeft: FieldNameAlias.noteId, - }, - { - joinTable: TableUser, - keyLeft: FieldNameUser.id, - keyRight: FieldNameMediaUpload.userId, - }, - ], - ); + const dtoSpy = jest.spyOn(service, 'getMediaUploadDtoByUuid').mockResolvedValueOnce({ + uuid, + fileName, + linkedNoteCount: 1, + createdAt: createdAtIso, + username, + }); const result = await service.getMediaUploadDtosByUuids([uuid]); + expect(dtoSpy).toHaveBeenCalledWith(uuid); expect(result).toEqual([ { uuid, fileName, - noteAlias: alias, + linkedNoteCount: 1, createdAt: createdAtIso, username, }, diff --git a/backend/src/media/media.service.ts b/backend/src/media/media.service.ts index 32a0816ab..6adb22286 100644 --- a/backend/src/media/media.service.ts +++ b/backend/src/media/media.service.ts @@ -3,17 +3,16 @@ * * SPDX-License-Identifier: AGPL-3.0-only */ -import { MediaBackendType } from '@hedgedoc/commons'; +import { MediaBackendType, PermissionLevel } from '@hedgedoc/commons'; import { - Alias, - FieldNameAlias, FieldNameMediaUpload, + FieldNameMediaUploadNote, FieldNameNote, FieldNameUser, MediaUpload, Note, - TableAlias, TableMediaUpload, + TableMediaUploadNote, TableUser, User, } from '@hedgedoc/database'; @@ -28,6 +27,7 @@ import mediaConfiguration, { MediaConfig } from '../config/media.config'; import { MediaUploadDto } from '../dtos/media-upload.dto'; import { ClientError, NotInDBError } from '../errors/errors'; import { ConsoleLoggerService } from '../logger/console-logger.service'; +import { PermissionService } from '../permissions/permission.service'; import { dateTimeToDB, dateTimeToISOString, @@ -56,6 +56,8 @@ export class MediaService { @Inject(mediaConfiguration.KEY) private mediaConfig: MediaConfig, + + private readonly permissionService: PermissionService, ) { this.logger.setContext(MediaService.name); this.mediaBackendType = this.chooseBackendType(); @@ -105,7 +107,7 @@ export class MediaService { userId: User[FieldNameUser.id], noteId: Note[FieldNameNote.id], ): Promise { - this.logger.debug(`Saving file for note '${noteId}' and user '${userId}'`, 'saveFile'); + this.logger.debug(`Saving file for user '${userId}'`, 'saveFile'); const fileTypeResult = await FileType.fromBuffer(fileBuffer); if (!fileTypeResult) { throw new ClientError('Could not detect file type.'); @@ -115,14 +117,19 @@ export class MediaService { } const uuid = uuidV7(); const backendData = await this.mediaBackend.saveFile(uuid, fileBuffer, fileTypeResult); - await this.knex(TableMediaUpload).insert({ - [FieldNameMediaUpload.uuid]: uuid, - [FieldNameMediaUpload.fileName]: fileName, - [FieldNameMediaUpload.userId]: userId, - [FieldNameMediaUpload.noteId]: noteId, - [FieldNameMediaUpload.backendType]: this.mediaBackendType, - [FieldNameMediaUpload.backendData]: backendData, - [FieldNameMediaUpload.createdAt]: dateTimeToDB(getCurrentDateTime()), + await this.knex.transaction(async (transaction) => { + await transaction(TableMediaUpload).insert({ + [FieldNameMediaUpload.uuid]: uuid, + [FieldNameMediaUpload.fileName]: fileName, + [FieldNameMediaUpload.userId]: userId, + [FieldNameMediaUpload.backendType]: this.mediaBackendType, + [FieldNameMediaUpload.backendData]: backendData, + [FieldNameMediaUpload.createdAt]: dateTimeToDB(getCurrentDateTime()), + }); + await transaction(TableMediaUploadNote).insert({ + [FieldNameMediaUploadNote.mediaUploadUuid]: uuid, + [FieldNameMediaUploadNote.noteId]: noteId, + }); }); return uuid; } @@ -139,7 +146,7 @@ export class MediaService { .select(FieldNameMediaUpload.backendData) .where(FieldNameMediaUpload.uuid, uuid) .first(); - if (backendData == undefined) { + if (backendData === undefined) { throw new NotInDBError( `Can't find backend data for '${uuid}'`, this.logger.getContext(), @@ -151,28 +158,50 @@ export class MediaService { } /** - * Retrieves the URL to a media upload file + * Resolves a media upload into either a redirect URL (for backends that + * expose a publicly accessible, time-limited URL like Azure, S3, imgur, or + * WebDAV) or the file content + content type (for the local filesystem + * backend so the controller can stream the bytes. * - * @param uuid the uuid of the file to get the URL for - * @returns the URL of the file - * @throws MediaBackendError if there was an error retrieving the url + * @param uuid The UUID of the media upload + * @returns A discriminated union describing how the controller should respond */ - async getFileUrl(uuid: string): Promise { + async getFileResponse(uuid: string): Promise { + if (!validateUuid(uuid)) { + throw new NotInDBError( + 'Invalid media upload id provided', + this.logger.getContext(), + 'getFileResponse', + ); + } const mediaUpload = await this.knex(TableMediaUpload) - .select(FieldNameMediaUpload.backendType, FieldNameMediaUpload.backendData) + .select( + FieldNameMediaUpload.backendType, + FieldNameMediaUpload.backendData, + FieldNameMediaUpload.fileName, + ) .where(FieldNameMediaUpload.uuid, uuid) .first(); + if (mediaUpload === undefined) { throw new NotInDBError( `Can't find backend data for '${uuid}'`, this.logger.getContext(), - 'getFileUrl', + 'getFileResponse', ); } + const backendName = mediaUpload[FieldNameMediaUpload.backendType]; - const backend = this.getBackendFromType(backendName); const backendData = mediaUpload[FieldNameMediaUpload.backendData]; - return await backend.getFileUrl(uuid, backendData); + + const backend = this.getBackendFromType(backendName); + if (backendName === MediaBackendType.FILESYSTEM) { + const fileResponse = await (backend as FilesystemBackend).getFileResponse(uuid, backendData); + return { type: 'file', ...fileResponse }; + } + + const url = await backend.getFileUrl(uuid, backendData); + return { type: 'redirect', url }; } /** @@ -188,7 +217,7 @@ export class MediaService { .where(FieldNameMediaUpload.uuid, uuid) .first(); if (mediaUpload === undefined) { - throw new NotInDBError(`MediaUpload with uuid '${uuid}' not found`); + throw new NotInDBError(`MediaUpload with given uuid was not found`); } return mediaUpload; } @@ -209,7 +238,7 @@ export class MediaService { } /** - * Lists all uploads to a specific note + * Lists all uploads linked to a specific note * * @param noteId the specific user * @returns An array of media uploads owned by the user @@ -217,30 +246,135 @@ export class MediaService { async getMediaUploadUuidsByNoteId( noteId: number, ): Promise { - return await this.knex.transaction(async (transaction) => { - const results = await transaction(TableMediaUpload) - .select(FieldNameMediaUpload.uuid) - .where(FieldNameMediaUpload.noteId, noteId); - return results.map((result) => result[FieldNameMediaUpload.uuid]); + const results = await this.knex(TableMediaUploadNote) + .select(FieldNameMediaUploadNote.mediaUploadUuid) + .where(FieldNameMediaUploadNote.noteId, noteId); + return results.map((result) => result[FieldNameMediaUploadNote.mediaUploadUuid]); + } + + /** + * Removes a note association from a media upload. + * + * @param uuid The UUID of the media upload + * @param noteId The ID of the note to disassociate from the media upload + */ + async removeNoteFromMediaUpload(uuid: string, noteId: number): Promise { + this.logger.debug( + `Removing note '${noteId}' from mediaUpload: ${uuid}`, + 'removeNoteFromMediaUpload', + ); + await this.knex(TableMediaUploadNote) + .where(FieldNameMediaUploadNote.mediaUploadUuid, uuid) + .andWhere(FieldNameMediaUploadNote.noteId, noteId) + .delete(); + } + + /** + * Adds a note association to a media upload. + * + * @param uuid The UUID of the media upload + * @param noteId The ID of the note to associate with the media upload + */ + async addNoteToMediaUpload(uuid: string, noteId: number): Promise { + await this.knex(TableMediaUploadNote) + .insert({ + [FieldNameMediaUploadNote.mediaUploadUuid]: uuid, + [FieldNameMediaUploadNote.noteId]: noteId, + }) + .onConflict([FieldNameMediaUploadNote.mediaUploadUuid, FieldNameMediaUploadNote.noteId]) + .ignore(); + } + + /** + * Checks whether a user may access a media upload. + * + * @param userId The id of the user to check + * @param uuid The UUID of the media upload to check against + * @returns true if the user has access, false otherwise + */ + async canUserAccessUpload(userId: number, uuid: string): Promise { + const mediaUpload = await this.knex(TableMediaUpload) + .select(FieldNameMediaUpload.userId) + .where(FieldNameMediaUpload.uuid, uuid) + .first(); + if (mediaUpload === undefined) { + throw new NotInDBError(`MediaUpload with given uuid was not found`); + } + const linkedNoteIds = await this.getLinkedNoteIds(uuid); + + if (userId === null) { + return false; + } + + if (linkedNoteIds.length === 0) { + return mediaUpload[FieldNameMediaUpload.userId] === userId; + } + for (const noteId of linkedNoteIds) { + const linkedNotePermission = await this.permissionService.determinePermission(userId, noteId); + if (linkedNotePermission >= PermissionLevel.READ) { + return true; + } + } + + return false; + } + + /** + * Retrieves a media upload DTO by UUID. + * + * @param uuid The UUID to fetch the media upload DTO for + * @returns The {@link MediaUploadDto} + */ + async getMediaUploadDtoByUuid(uuid: string): Promise { + const mediaUpload = await this.knex(TableMediaUpload) + .join( + TableUser, + `${TableUser}.${FieldNameUser.id}`, + `${TableMediaUpload}.${FieldNameMediaUpload.userId}`, + ) + .select( + `${TableMediaUpload}.${FieldNameMediaUpload.uuid}`, + `${TableMediaUpload}.${FieldNameMediaUpload.fileName}`, + `${TableMediaUpload}.${FieldNameMediaUpload.createdAt}`, + `${TableUser}.${FieldNameUser.username}`, + ) + .where(`${TableMediaUpload}.${FieldNameMediaUpload.uuid}`, uuid) + .first(); + + if (mediaUpload === undefined) { + throw new NotInDBError(`MediaUpload with given uuid was not found`); + } + + return MediaUploadDto.create({ + uuid: mediaUpload[FieldNameMediaUpload.uuid], + fileName: mediaUpload[FieldNameMediaUpload.fileName], + linkedNoteCount: (await this.getLinkedNoteIds(uuid)).length, + createdAt: dateTimeToISOString(dbToDateTime(mediaUpload[FieldNameMediaUpload.createdAt])), + username: mediaUpload[FieldNameUser.username], }); } /** - * Sets the note of a mediaUpload to null + * Retrieves media upload DTOs by a list of their UUIDs. + * * - * @param uuid the media upload to be changed */ - async removeNoteFromMediaUpload(uuid: string): Promise { - this.logger.debug('Setting note to null for mediaUpload: ' + uuid, 'removeNoteFromMediaUpload'); - await this.knex(TableMediaUpload) - .update({ - [FieldNameMediaUpload.noteId]: null, - }) - .where(FieldNameMediaUpload.uuid, uuid); + async getMediaUploadDtosByUuids(uuids: string[]): Promise { + return await Promise.all(uuids.map(async (uuid) => await this.getMediaUploadDtoByUuid(uuid))); } /** - * Returns the backend type that is configured in the media configuration + * Retrieves an array of note IDs linked to the specified media upload UUID. + */ + private async getLinkedNoteIds(uuid: string): Promise { + const results = await this.knex(TableMediaUploadNote) + .select(FieldNameMediaUploadNote.noteId) + .where(FieldNameMediaUploadNote.mediaUploadUuid, uuid); + return results.map((result) => result[FieldNameMediaUploadNote.noteId]); + } + + /** + * Returns the backend type that is configured in the media configuration. */ private chooseBackendType(): MediaBackendType { switch (this.mediaConfig.backend.type as string) { @@ -279,50 +413,4 @@ export class MediaService { return this.moduleRef.get(WebdavBackend); } } - - /** - * Retrieves media upload DTOs by a list of their UUIDs - * - * @param uuids The UUIDs of the media uploads to retrieve - * @returns An array of MediaUploadDto objects containing the details of the media uploads - */ - async getMediaUploadDtosByUuids(uuids: string[]): Promise { - const mediaUploads = await this.knex(TableMediaUpload) - .select< - (Pick< - MediaUpload, - FieldNameMediaUpload.uuid | FieldNameMediaUpload.fileName | FieldNameMediaUpload.createdAt - > & - Pick & - Pick)[] - >( - `${TableMediaUpload}.${FieldNameMediaUpload.uuid}`, - `${TableMediaUpload}.${FieldNameMediaUpload.fileName}`, - `${TableMediaUpload}.${FieldNameMediaUpload.createdAt}`, - `${TableUser}.${FieldNameUser.username}`, - `${TableAlias}.${FieldNameAlias.alias}`, - ) - .join( - TableAlias, - `${TableAlias}.${FieldNameAlias.noteId}`, - `${TableMediaUpload}.${FieldNameMediaUpload.noteId}`, - ) - .join( - TableUser, - `${TableUser}.${FieldNameUser.id}`, - `${TableMediaUpload}.${FieldNameMediaUpload.userId}`, - ) - .whereIn(FieldNameMediaUpload.uuid, uuids) - .andWhere(FieldNameAlias.isPrimary, true); - - return mediaUploads.map((mediaUpload) => - MediaUploadDto.create({ - uuid: mediaUpload[FieldNameMediaUpload.uuid], - fileName: mediaUpload[FieldNameMediaUpload.fileName], - noteAlias: mediaUpload[FieldNameAlias.alias], - createdAt: dateTimeToISOString(dbToDateTime(mediaUpload[FieldNameMediaUpload.createdAt])), - username: mediaUpload[FieldNameUser.username], - }), - ); - } } diff --git a/backend/src/permissions/permission.service.ts b/backend/src/permissions/permission.service.ts index 24418255c..8daf722bd 100644 --- a/backend/src/permissions/permission.service.ts +++ b/backend/src/permissions/permission.service.ts @@ -7,6 +7,7 @@ import { PermissionLevel } from '@hedgedoc/commons'; import { FieldNameGroup, FieldNameMediaUpload, + FieldNameMediaUploadNote, FieldNameNote, FieldNameNoteGroupPermission, FieldNameNoteUserPermission, @@ -15,6 +16,7 @@ import { Note, TableGroup, TableMediaUpload, + TableMediaUploadNote, TableNote, TableNoteGroupPermission, TableNoteUserPermission, @@ -68,24 +70,12 @@ export class PermissionService { userId: number, mediaUploadUuid: string, ): Promise { - const dbResult = await this.knex(TableMediaUpload) - .join( - TableNote, - `${TableNote}.${FieldNameNote.id}`, - '=', - `${TableMediaUpload}.${FieldNameMediaUpload.noteId}`, - ) - .select<{ - [FieldNameMediaUpload.userId]: number; - [FieldNameNote.ownerId]: number; - }>( - `${TableMediaUpload}.${FieldNameMediaUpload.userId}`, - `${TableNote}.${FieldNameNote.ownerId}`, - ) - .where(`${TableMediaUpload}.${FieldNameMediaUpload.uuid}`, mediaUploadUuid) + const mediaUpload = await this.knex(TableMediaUpload) + .select(FieldNameMediaUpload.userId) + .where(FieldNameMediaUpload.uuid, mediaUploadUuid) .first(); - if (dbResult === undefined) { + if (mediaUpload === undefined) { throw new NotInDBError( `There is no upload with the id ${mediaUploadUuid}`, this.logger.getContext(), @@ -93,9 +83,34 @@ export class PermissionService { ); } - return ( - dbResult[FieldNameMediaUpload.userId] === userId || dbResult[FieldNameNote.ownerId] === userId - ); + if (mediaUpload[FieldNameMediaUpload.userId] === userId) { + return true; + } + + return await this.canDeleteViaAnyLinkedNote(userId, mediaUploadUuid); + } + + /** + * Determines whether the user has permission to delete via any linked note + * associated with the given media upload UUID. + * + * @param userId The id of the user requesting deletion. + * @param mediaUploadUuid The UUID of the media upload. + * @returns A promise that resolves to `true` if the user has permission to delete through one of the linked notes, otherwise `false`. + */ + private async canDeleteViaAnyLinkedNote( + userId: number, + mediaUploadUuid: string, + ): Promise { + const noteIds = await this.knex(TableMediaUploadNote) + .pluck(FieldNameMediaUploadNote.noteId) + .where(FieldNameMediaUploadNote.mediaUploadUuid, mediaUploadUuid); + for (const noteId of noteIds) { + if (await this.isOwner(userId, noteId)) { + return true; + } + } + return false; } /** diff --git a/backend/src/permissions/permissions.service.spec.ts b/backend/src/permissions/permissions.service.spec.ts index e8788ca66..31013509f 100644 --- a/backend/src/permissions/permissions.service.spec.ts +++ b/backend/src/permissions/permissions.service.spec.ts @@ -9,6 +9,7 @@ import { PermissionLevel } from '@hedgedoc/commons'; import { FieldNameGroup, FieldNameMediaUpload, + FieldNameMediaUploadNote, FieldNameNote, FieldNameNoteGroupPermission, FieldNameNoteUserPermission, @@ -16,6 +17,7 @@ import { Group, TableGroup, TableMediaUpload, + TableMediaUploadNote, TableNote, TableNoteGroupPermission, TableNoteUserPermission, @@ -109,70 +111,58 @@ describe('PermissionsService', () => { }); describe('checkMediaDeletePermission', () => { - afterEach(() => { - expectBindings(tracker, 'select', [[mockMediaUploadUuid]], true); - }); - - const buildMockSelect = (returnValues: unknown) => { + const buildMockSelect = (uploadValues: unknown, linkedNoteValues?: unknown) => { mockSelect( tracker, - [ - `${TableMediaUpload}"."${FieldNameMediaUpload.userId}`, - `${TableNote}"."${FieldNameNote.ownerId}`, - ], + [FieldNameMediaUpload.userId], TableMediaUpload, FieldNameMediaUpload.uuid, - returnValues, - [ - { - joinTable: TableNote, - keyLeft: FieldNameNote.id, - keyRight: FieldNameMediaUpload.noteId, - }, - ], + uploadValues, ); + if (linkedNoteValues !== undefined) { + mockSelect( + tracker, + [FieldNameMediaUploadNote.noteId], + TableMediaUploadNote, + FieldNameMediaUploadNote.mediaUploadUuid, + linkedNoteValues, + ); + } }; it('throws NotInDBError if dbResult is undefined', async () => { - buildMockSelect([]); + buildMockSelect(undefined); await expect( service.checkMediaDeletePermission(mockUserId1, mockMediaUploadUuid), ).rejects.toThrow(NotInDBError); - expectBindings(tracker, 'select', [[mockMediaUploadUuid]], true); }); describe('return true', () => { it('for media owner', async () => { - buildMockSelect([ - { - [FieldNameMediaUpload.userId]: mockUserId1, - [FieldNameNote.ownerId]: mockUserId2, - }, - ]); + buildMockSelect([{ [FieldNameMediaUpload.userId]: mockUserId1 }]); expect( await service.checkMediaDeletePermission(mockUserId1, mockMediaUploadUuid), ).toBeTruthy(); }); it('for note owner', async () => { - buildMockSelect([ - { - [FieldNameMediaUpload.userId]: mockUserId2, - [FieldNameNote.ownerId]: mockUserId1, - }, - ]); + buildMockSelect( + [{ [FieldNameMediaUpload.userId]: mockUserId2 }], + [{ [FieldNameMediaUploadNote.noteId]: mockNoteId }], + ); + const spyOnIsOwner = jest.spyOn(service, 'isOwner').mockResolvedValue(true); expect( await service.checkMediaDeletePermission(mockUserId1, mockMediaUploadUuid), ).toBeTruthy(); + expect(spyOnIsOwner).toHaveBeenCalledWith(mockUserId1, mockNoteId); }); }); it('returns false for a non-owner', async () => { - buildMockSelect([ - { - [FieldNameMediaUpload.userId]: mockUserId2, - [FieldNameNote.ownerId]: mockUserId2, - }, - ]); + buildMockSelect( + [{ [FieldNameMediaUpload.userId]: mockUserId2 }], + [{ [FieldNameMediaUploadNote.noteId]: mockNoteId }], + ); + jest.spyOn(service, 'isOwner').mockResolvedValue(false); expect( await service.checkMediaDeletePermission(mockUserId1, mockMediaUploadUuid), ).toBeFalsy(); diff --git a/backend/test/private-api/private-api.media.e2e-spec.ts b/backend/test/private-api/private-api.media.e2e-spec.ts index a3460dd9a..54df1bb07 100644 --- a/backend/test/private-api/private-api.media.e2e-spec.ts +++ b/backend/test/private-api/private-api.media.e2e-spec.ts @@ -12,6 +12,10 @@ import type { TestSetup } from '../test-setup'; import { noteAlias1, TestSetupBuilder } from '../test-setup'; import { ensureDeleted } from '../utils'; import { setupAgent } from './utils/setup-agent'; +import { MediaUploadDto } from '../../src/dtos/media-upload.dto'; +import { SpecialGroup } from '@hedgedoc/commons'; + +type PartialMediaUploadResponse = Pick; describe('Media', () => { let testSetup: TestSetup; @@ -25,6 +29,7 @@ describe('Media', () => { let userId: number; let testImage: Buffer; + const testFileName = 'test.png'; beforeEach(async () => { testSetup = await TestSetupBuilder.create().withUsers().withNotes().build(); @@ -58,7 +63,7 @@ describe('Media', () => { .attach('file', 'test/private-api/fixtures/test.png') .set('HedgeDoc-Note', noteAlias1) .expect(201); - uuid = uploadResponse.text; + uuid = uploadResponse.body.uuid; const apiResponse = await agentUser1.get(`${PRIVATE_API_PREFIX}/media/${uuid}`); expect(apiResponse.statusCode).toEqual(200); const downloadResponse = await agentUser1.get(`/uploads/${uuid}.png`); @@ -70,7 +75,7 @@ describe('Media', () => { .attach('file', 'test/private-api/fixtures/test.png') .set('HedgeDoc-Note', noteAlias1.toUpperCase()) .expect(201); - uuid = uploadResponse.text; + uuid = uploadResponse.body.uuid; const apiResponse = await agentUser1.get(`${PRIVATE_API_PREFIX}/media/${uuid}`); expect(apiResponse.statusCode).toEqual(200); const downloadResponse = await agentUser1.get(`/uploads/${uuid}.png`); @@ -88,7 +93,7 @@ describe('Media', () => { .attach('file', 'test/private-api/fixtures/test.png') .set('HedgeDoc-Note', noteDtoResponse.body.metadata.primaryAlias) .expect(201); - uuid = uploadResponse.text; + uuid = uploadResponse.body.uuid; const apiResponse = await agentGuestUser.get(`${PRIVATE_API_PREFIX}/media/${uuid}`); expect(apiResponse.statusCode).toEqual(200); const downloadResponse = await agentGuestUser.get(`/uploads/${uuid}.png`); @@ -106,7 +111,7 @@ describe('Media', () => { .attach('file', 'test/private-api/fixtures/test.png') .set('HedgeDoc-Note', noteDtoResponse.body.metadata.primaryAlias.toUpperCase()) .expect(201); - uuid = uploadResponse.text; + uuid = uploadResponse.body.uuid; const apiResponse = await agentGuestUser.get(`${PRIVATE_API_PREFIX}/media/${uuid}`); expect(apiResponse.statusCode).toEqual(200); const downloadResponse = await agentGuestUser.get(`/uploads/${uuid}.png`); @@ -162,10 +167,53 @@ describe('Media', () => { }); }); + describe(`GET ${PRIVATE_API_PREFIX}/media/:uuid`, () => { + let mediaUploadUuid: string; + beforeEach(async () => { + mediaUploadUuid = await testSetup.mediaService.saveFile( + testFileName, + testImage, + userId, + testSetup.ownedNoteIds[0], + ); + // Make note non-readable to guests to test access forbidden case + const specialGroupEveryoneId = await testSetup.groupService.getGroupIdByName( + SpecialGroup.EVERYONE, + ); + await testSetup.permissionsService.removeGroupPermission( + testSetup.ownedNoteIds[0], + specialGroupEveryoneId, + ); + }); + it('returns correct data for owner', async () => { + // agentUser1 is the uploader and therefore owner + const response = await agentUser1 + .get(`${PRIVATE_API_PREFIX}/media/${mediaUploadUuid}`) + .expect(200); + expect(response.headers['content-type']).toContain('application/json'); + const jsonResponse = response.body as PartialMediaUploadResponse; + expect(jsonResponse.fileName).toEqual(testFileName); + expect(jsonResponse.linkedNoteCount).toEqual(1); + }); + it('returns correct data for user with read-access to a linked note', async () => { + // agentUser2 has read-permission to the note via the logged-in special group + const response = await agentUser2 + .get(`${PRIVATE_API_PREFIX}/media/${mediaUploadUuid}`) + .expect(200); + expect(response.headers['content-type']).toContain('application/json'); + const jsonResponse = response.body as PartialMediaUploadResponse; + expect(jsonResponse.fileName).toEqual(testFileName); + expect(jsonResponse.linkedNoteCount).toEqual(1); + }); + it('rejects with PermissionError for user with no read-access to a linked note', async () => { + await agentGuestUser.get(`${PRIVATE_API_PREFIX}/media/${mediaUploadUuid}`).expect(403); + }); + }); + describe(`DELETE ${PRIVATE_API_PREFIX}/media/:filename`, () => { it('allowed if user is owner of file', async () => { const uuid = await testSetup.mediaService.saveFile( - 'test.png', + testFileName, testImage, userId, testSetup.ownedNoteIds[0], @@ -179,7 +227,7 @@ describe('Media', () => { }); it('allowed if user is owner of note', async () => { const uuid = await testSetup.mediaService.saveFile( - 'test.png', + testFileName, testImage, testSetup.userIds[1], testSetup.ownedNoteIds[0], @@ -193,7 +241,7 @@ describe('Media', () => { }); it("other user can't delete", async () => { const uuid = await testSetup.mediaService.saveFile( - 'test.png', + testFileName, testImage, testSetup.userIds[0], testSetup.ownedNoteIds[0], @@ -203,7 +251,7 @@ describe('Media', () => { }); it("guest user can't delete", async () => { const uuid = await testSetup.mediaService.saveFile( - 'test.png', + testFileName, testImage, testSetup.userIds[0], testSetup.ownedNoteIds[0], @@ -212,4 +260,55 @@ describe('Media', () => { await agentGuestUser.delete(`${PRIVATE_API_PREFIX}/media/${uuid}`).expect(403); }); }); + + describe(`PUT ${PRIVATE_API_PREFIX}/media/:uuid/notes/:noteAlias`, () => { + it('links media to a note by alias', async () => { + const targetAlias = 'media_link_target'; + const targetNoteId = await testSetup.notesService.createNote( + 'test content', + testSetup.userIds[0], + targetAlias, + ); + const uuid = await testSetup.mediaService.saveFile( + testFileName, + testImage, + testSetup.userIds[0], + testSetup.ownedNoteIds[0], + ); + + await agentUser1 + .put(`${PRIVATE_API_PREFIX}/media/${uuid}/notes/${targetAlias.toUpperCase()}`) + .expect(204); + + const mediaResponse = await agentUser1.get(`${PRIVATE_API_PREFIX}/media/${uuid}`).expect(200); + expect(mediaResponse.body.linkedNoteCount).toEqual(2); + + const linkedUploads = await testSetup.mediaService.getMediaUploadUuidsByNoteId(targetNoteId); + expect(linkedUploads).toContain(uuid); + }); + + it('rejects users who are not the uploader', async () => { + const targetAlias = 'media_link_foreign_uploader'; + await testSetup.notesService.createNote('test content', testSetup.userIds[1], targetAlias); + const uuid = await testSetup.mediaService.saveFile( + testFileName, + testImage, + testSetup.userIds[0], + testSetup.ownedNoteIds[0], + ); + + await agentUser2.put(`${PRIVATE_API_PREFIX}/media/${uuid}/notes/${targetAlias}`).expect(403); + }); + + it('returns 404 for unknown note aliases', async () => { + const uuid = await testSetup.mediaService.saveFile( + testFileName, + testImage, + testSetup.userIds[0], + testSetup.ownedNoteIds[0], + ); + + await agentUser1.put(`${PRIVATE_API_PREFIX}/media/${uuid}/notes/does_not_exist`).expect(404); + }); + }); }); diff --git a/backend/test/public-api/public-api.media.e2e-spec.ts b/backend/test/public-api/public-api.media.e2e-spec.ts index fada4809b..be5701201 100644 --- a/backend/test/public-api/public-api.media.e2e-spec.ts +++ b/backend/test/public-api/public-api.media.e2e-spec.ts @@ -140,7 +140,7 @@ describe('Media', () => { expect( isoStringToDateTime(mediaDto.createdAt).toMillis() - hardCodedNow.toMillis(), ).toBeLessThan(100); - expect(mediaDto.noteAlias).toEqual(noteAlias1); + expect(mediaDto.linkedNoteCount).toEqual(1); expect(mediaDto.fileName).toEqual(fileName); expect(mediaDto.username).toEqual(username1); jest.useRealTimers(); @@ -164,7 +164,7 @@ describe('Media', () => { // upload a file with the default test user const testNote = await testSetup.notesService.createNote( 'test content', - testSetup.userIds[2], + testSetup.userIds[0], 'test_delete_media_file', ); const upload = await testSetup.mediaService.saveFile( @@ -194,7 +194,7 @@ describe('Media', () => { // Test if file is really deleted await agent .get(`/uploads/${upload}.png`) - .set('Authorization', `Bearer ${testSetup.authTokens[1].secret}`) + .set('Authorization', `Bearer ${testSetup.authTokens[0].secret}`) .expect(404); }); it('deleting user is owner of note', async () => { @@ -231,7 +231,7 @@ describe('Media', () => { // Test if file is really deleted await agent .get(`/uploads/${upload}.png`) - .set('Authorization', `Bearer ${testSetup.authTokens[1].secret}`) + .set('Authorization', `Bearer ${testSetup.authTokens[2].secret}`) .expect(404); }); it('errors if the user does not own the file', async () => { diff --git a/commons/src/dtos/media/media-upload.dto.ts b/commons/src/dtos/media/media-upload.dto.ts index 97b2e9438..23ed2256d 100644 --- a/commons/src/dtos/media/media-upload.dto.ts +++ b/commons/src/dtos/media/media-upload.dto.ts @@ -10,10 +10,11 @@ export const MediaUploadSchema = z .object({ uuid: z.string().uuid().describe('The uuid of the media file'), fileName: z.string().describe('The original filename of the media upload'), - noteAlias: z - .string() - .nullable() - .describe('The note alias to which the uploaded file is linked to'), + linkedNoteCount: z + .number() + .int() + .nonnegative() + .describe('How many notes are linked to the upload'), createdAt: z .string() .datetime({ offset: false, local: false }) diff --git a/database/src/types/media-upload.ts b/database/src/types/media-upload.ts index 98a2876b4..25febcd2f 100644 --- a/database/src/types/media-upload.ts +++ b/database/src/types/media-upload.ts @@ -14,16 +14,12 @@ export enum MediaBackendType { /** * A media upload object represents an uploaded file. While the file itself is stored in the configured storage backend, - * the metadata is stored in the database. Uploads are attached to the {@link Note} where they were uploaded, but can be - * detached by deleting the note and setting the option to keep the media files. + * the metadata is stored in the database. Uploads can be linked to zero or more notes. */ export interface MediaUpload { /** UUID (v7) identifying the media upload. Is public and unique */ [FieldNameMediaUpload.uuid]: string - /** The id of the attached {@link Note} or null if the media upload was detached from a note */ - [FieldNameMediaUpload.noteId]: number | null - /** The id of the {@link User} who uploaded the media file */ [FieldNameMediaUpload.userId]: number @@ -42,7 +38,6 @@ export interface MediaUpload { export enum FieldNameMediaUpload { uuid = 'uuid', - noteId = 'note_id', userId = 'user_id', fileName = 'file_name', backendType = 'backend_type', @@ -52,4 +47,16 @@ export enum FieldNameMediaUpload { export const TableMediaUpload = 'media_upload' -export type TypeUpdateMediaUpload = Pick +export const TableMediaUploadNote = 'media_upload_note' + +export enum FieldNameMediaUploadNote { + mediaUploadUuid = 'media_upload_uuid', + noteId = 'note_id', +} + +export type TypeUpdateMediaUpload = never + +export interface MediaUploadNote { + [FieldNameMediaUploadNote.mediaUploadUuid]: string + [FieldNameMediaUploadNote.noteId]: number +} diff --git a/frontend/src/pages/api/private/me/media.ts b/frontend/src/pages/api/private/me/media.ts index 689bd995d..e1f2b1133 100644 --- a/frontend/src/pages/api/private/me/media.ts +++ b/frontend/src/pages/api/private/me/media.ts @@ -14,14 +14,14 @@ const handler = (req: NextApiRequest, res: NextApiResponse) => { createdAt: '2022-03-20T20:36:32Z', uuid: '5355ed83-7e12-4db0-95ed-837e124db08c', fileName: 'dummy.png', - noteAlias: 'features' + linkedNoteCount: 1 }, { username: 'tilman', createdAt: '2022-03-20T20:36:57+0000', uuid: '656745ab-fbf9-47f1-a745-abfbf9a7f10c', fileName: 'dummy2.png', - noteAlias: null + linkedNoteCount: 1 } ]) } diff --git a/frontend/src/pages/api/private/media.ts b/frontend/src/pages/api/private/media.ts index 70b37d33a..00bdcd08c 100644 --- a/frontend/src/pages/api/private/media.ts +++ b/frontend/src/pages/api/private/media.ts @@ -22,7 +22,7 @@ const handler = async (req: NextApiRequest, res: NextApiResponse): Promise { uuid: 'e81f57cd-5866-4253-9f57-cd5866a253ca', fileName: 'avatar.png', - noteAlias: null, + linkedNoteCount: 1, username: 'test', createdAt: '2022-02-27T21:54:23.856Z' },