diff --git a/src/api/public/media/media.controller.ts b/src/api/public/media/media.controller.ts index 27f960c23..1472088b0 100644 --- a/src/api/public/media/media.controller.ts +++ b/src/api/public/media/media.controller.ts @@ -132,7 +132,23 @@ export class MediaController { ): Promise { const username = req.user.userName; try { - await this.mediaService.deleteFile(filename, username); + this.logger.debug( + `Deleting '${filename}' for user '${username}'`, + 'deleteFile', + ); + const mediaUpload = await this.mediaService.findUploadByFilename( + filename, + ); + if (mediaUpload.user.userName !== username) { + this.logger.warn( + `${username} tried to delete '${filename}', but is not the owner`, + 'deleteFile', + ); + throw new PermissionError( + `File '${filename}' is not owned by '${username}'`, + ); + } + await this.mediaService.deleteFile(mediaUpload); } catch (e) { if (e instanceof PermissionError) { throw new UnauthorizedException(e.message); diff --git a/src/media/media.service.spec.ts b/src/media/media.service.spec.ts index 9ee8481ab..e7acf22b0 100644 --- a/src/media/media.service.spec.ts +++ b/src/media/media.service.spec.ts @@ -24,7 +24,7 @@ import { BackendData, MediaUpload } from './media-upload.entity'; import { MediaService } from './media.service'; import { Repository } from 'typeorm'; import { promises as fs } from 'fs'; -import { ClientError, NotInDBError, PermissionError } from '../errors/errors'; +import { ClientError, NotInDBError } from '../errors/errors'; import { NoteGroupPermission } from '../permissions/note-group-permission.entity'; import { NoteUserPermission } from '../permissions/note-user-permission.entity'; import { Group } from '../groups/group.entity'; @@ -145,7 +145,6 @@ describe('MediaService', () => { describe('deleteFile', () => { it('works', async () => { - const testFileName = 'testFilename'; const mockMediaUploadEntry = { id: 'testMediaUpload', backendData: 'testBackendData', @@ -153,12 +152,9 @@ describe('MediaService', () => { userName: 'hardcoded', } as User, } as MediaUpload; - jest - .spyOn(mediaRepo, 'findOne') - .mockResolvedValueOnce(mockMediaUploadEntry); jest.spyOn(service.mediaBackend, 'deleteFile').mockImplementationOnce( async (fileName: string, backendData: BackendData): Promise => { - expect(fileName).toEqual(testFileName); + expect(fileName).toEqual(mockMediaUploadEntry.id); expect(backendData).toEqual(mockMediaUploadEntry.backendData); }, ); @@ -168,24 +164,7 @@ describe('MediaService', () => { expect(entry).toEqual(mockMediaUploadEntry); return entry; }); - await service.deleteFile(testFileName, 'hardcoded'); - }); - - it('fails: the mediaUpload is not owned by user', async () => { - const testFileName = 'testFilename'; - const mockMediaUploadEntry = { - id: 'testMediaUpload', - backendData: 'testBackendData', - user: { - userName: 'not-hardcoded', - } as User, - } as MediaUpload; - jest - .spyOn(mediaRepo, 'findOne') - .mockResolvedValueOnce(mockMediaUploadEntry); - await expect( - service.deleteFile(testFileName, 'hardcoded'), - ).rejects.toThrow(PermissionError); + await service.deleteFile(mockMediaUploadEntry); }); }); describe('findUploadByFilename', () => { diff --git a/src/media/media.service.ts b/src/media/media.service.ts index 7cb1b36d6..b0a25442d 100644 --- a/src/media/media.service.ts +++ b/src/media/media.service.ts @@ -10,7 +10,7 @@ import { InjectRepository } from '@nestjs/typeorm'; import * as FileType from 'file-type'; import { Repository } from 'typeorm'; import mediaConfiguration, { MediaConfig } from '../config/media.config'; -import { ClientError, NotInDBError, PermissionError } from '../errors/errors'; +import { ClientError, NotInDBError } from '../errors/errors'; import { ConsoleLoggerService } from '../logger/console-logger.service'; import { NotesService } from '../notes/notes.service'; import { UsersService } from '../users/users.service'; @@ -113,30 +113,12 @@ export class MediaService { /** * @async - * Try to delete the file specified by the filename with the user specified by the username. - * @param {string} filename - the name of the file to delete. - * @param {string} username - the username of the user who uploaded this file - * @return {string} the url of the saved file - * @throws {PermissionError} the user is not permitted to delete this file. - * @throws {NotInDBError} - the file entry specified is not in the database + * Try to delete the specified file. + * @param {MediaUpload} mediaUpload - the name of the file to delete. * @throws {MediaBackendError} - there was an error deleting the file */ - async deleteFile(filename: string, username: string): Promise { - this.logger.debug( - `Deleting '${filename}' for user '${username}'`, - 'deleteFile', - ); - const mediaUpload = await this.findUploadByFilename(filename); - if (mediaUpload.user.userName !== username) { - this.logger.warn( - `${username} tried to delete '${filename}', but is not the owner`, - 'deleteFile', - ); - throw new PermissionError( - `File '${filename}' is not owned by '${username}'`, - ); - } - await this.mediaBackend.deleteFile(filename, mediaUpload.backendData); + async deleteFile(mediaUpload: MediaUpload): Promise { + await this.mediaBackend.deleteFile(mediaUpload.id, mediaUpload.backendData); await this.mediaUploadRepository.remove(mediaUpload); }