diff --git a/src/api/private/me/history/history.controller.ts b/src/api/private/me/history/history.controller.ts index 53960f0cf..fdb098b4c 100644 --- a/src/api/private/me/history/history.controller.ts +++ b/src/api/private/me/history/history.controller.ts @@ -22,6 +22,8 @@ import { HistoryEntryUpdateDto } from '../../../../history/history-entry-update. import { HistoryEntryDto } from '../../../../history/history-entry.dto'; import { HistoryService } from '../../../../history/history.service'; import { ConsoleLoggerService } from '../../../../logger/console-logger.service'; +import { GetNotePipe } from '../../../../notes/get-note.pipe'; +import { Note } from '../../../../notes/note.entity'; import { UsersService } from '../../../../users/users.service'; @ApiTags('history') @@ -84,14 +86,14 @@ export class HistoryController { @Put(':note') async updateHistoryEntry( - @Param('note') noteId: string, + @Param('note', GetNotePipe) note: Note, @Body() entryUpdateDto: HistoryEntryUpdateDto, ): Promise { try { // ToDo: use actual user here const user = await this.userService.getUserByUsername('hardcoded'); const newEntry = await this.historyService.updateHistoryEntry( - noteId, + note, user, entryUpdateDto, ); @@ -105,11 +107,13 @@ export class HistoryController { } @Delete(':note') - async deleteHistoryEntry(@Param('note') noteId: string): Promise { + async deleteHistoryEntry( + @Param('note', GetNotePipe) note: Note, + ): Promise { try { // ToDo: use actual user here const user = await this.userService.getUserByUsername('hardcoded'); - await this.historyService.deleteHistoryEntry(noteId, user); + await this.historyService.deleteHistoryEntry(note, user); } catch (e) { if (e instanceof NotInDBError) { throw new NotFoundException(e.message); diff --git a/src/api/private/media/media.controller.ts b/src/api/private/media/media.controller.ts index c815cc455..8db4e26f4 100644 --- a/src/api/private/media/media.controller.ts +++ b/src/api/private/media/media.controller.ts @@ -24,12 +24,18 @@ import { ConsoleLoggerService } from '../../../logger/console-logger.service'; import { MediaUploadUrlDto } from '../../../media/media-upload-url.dto'; import { MediaService } from '../../../media/media.service'; import { MulterFile } from '../../../media/multer-file.interface'; +import { Note } from '../../../notes/note.entity'; +import { NotesService } from '../../../notes/notes.service'; +import { User } from '../../../users/user.entity'; +import { UsersService } from '../../../users/users.service'; @Controller('media') export class MediaController { constructor( private readonly logger: ConsoleLoggerService, private mediaService: MediaService, + private userService: UsersService, + private noteService: NotesService, ) { this.logger.setContext(MediaController.name); } @@ -42,17 +48,15 @@ export class MediaController { @Headers('HedgeDoc-Note') noteId: string, ): Promise { // ToDo: Get real userName - const username = 'hardcoded'; - this.logger.debug( - `Recieved filename '${file.originalname}' for note '${noteId}' from user '${username}'`, - 'uploadMedia', - ); + const user: User = await this.userService.getUserByUsername('hardcoded'); try { - const url = await this.mediaService.saveFile( - file.buffer, - username, - noteId, + // TODO: Move getting the Note object into a decorator + const note: Note = await this.noteService.getNoteByIdOrAlias(noteId); + this.logger.debug( + `Recieved filename '${file.originalname}' for note '${noteId}' from user '${user.userName}'`, + 'uploadMedia', ); + const url = await this.mediaService.saveFile(file.buffer, user, note); return this.mediaService.toMediaUploadUrlDto(url); } catch (e) { if (e instanceof ClientError || e instanceof NotInDBError) { diff --git a/src/api/private/notes/notes.controller.ts b/src/api/private/notes/notes.controller.ts index fbb500ec9..634712794 100644 --- a/src/api/private/notes/notes.controller.ts +++ b/src/api/private/notes/notes.controller.ts @@ -25,6 +25,7 @@ import { HistoryService } from '../../../history/history.service'; import { ConsoleLoggerService } from '../../../logger/console-logger.service'; import { MediaUploadDto } from '../../../media/media-upload.dto'; import { MediaService } from '../../../media/media.service'; +import { GetNotePipe } from '../../../notes/get-note.pipe'; import { NoteDto } from '../../../notes/note.dto'; import { Note } from '../../../notes/note.entity'; import { NoteMediaDeletionDto } from '../../../notes/note.media-deletion.dto'; @@ -52,37 +53,24 @@ export class NotesController { @Get(':noteIdOrAlias') async getNote( - @Param('noteIdOrAlias') noteIdOrAlias: string, + @Param('noteIdOrAlias', GetNotePipe) note: Note, ): Promise { // ToDo: use actual user here const user = await this.userService.getUserByUsername('hardcoded'); - let note: Note; - try { - note = await this.noteService.getNoteByIdOrAlias(noteIdOrAlias); - } catch (e) { - if (e instanceof NotInDBError) { - throw new NotFoundException(e.message); - } - if (e instanceof ForbiddenIdError) { - throw new BadRequestException(e.message); - } - throw e; - } if (!this.permissionsService.mayRead(user, note)) { throw new UnauthorizedException('Reading note denied!'); } - await this.historyService.createOrUpdateHistoryEntry(note, user); + await this.historyService.updateHistoryEntryTimestamp(note, user); return await this.noteService.toNoteDto(note); } @Get(':noteIdOrAlias/media') async getNotesMedia( - @Param('noteIdOrAlias') noteIdOrAlias: string, + @Param('noteIdOrAlias', GetNotePipe) note: Note, ): Promise { try { // ToDo: use actual user here const user = await this.userService.getUserByUsername('hardcoded'); - const note = await this.noteService.getNoteByIdOrAlias(noteIdOrAlias); if (!this.permissionsService.mayRead(user, note)) { throw new UnauthorizedException('Reading note denied!'); } @@ -141,13 +129,12 @@ export class NotesController { @Delete(':noteIdOrAlias') @HttpCode(204) async deleteNote( - @Param('noteIdOrAlias') noteIdOrAlias: string, + @Param('noteIdOrAlias', GetNotePipe) note: Note, @Body() noteMediaDeletionDto: NoteMediaDeletionDto, ): Promise { try { // ToDo: use actual user here const user = await this.userService.getUserByUsername('hardcoded'); - const note = await this.noteService.getNoteByIdOrAlias(noteIdOrAlias); if (!this.permissionsService.isOwner(user, note)) { throw new UnauthorizedException('Deleting note denied!'); } @@ -159,29 +146,25 @@ export class NotesController { await this.mediaService.removeNoteFromMediaUpload(mediaUpload); } } - this.logger.debug('Deleting note: ' + noteIdOrAlias, 'deleteNote'); + this.logger.debug('Deleting note: ' + note.id, 'deleteNote'); await this.noteService.deleteNote(note); - this.logger.debug('Successfully deleted ' + noteIdOrAlias, 'deleteNote'); + this.logger.debug('Successfully deleted ' + note.id, 'deleteNote'); return; } catch (e) { if (e instanceof NotInDBError) { throw new NotFoundException(e.message); } - if (e instanceof ForbiddenIdError) { - throw new BadRequestException(e.message); - } throw e; } } @Get(':noteIdOrAlias/revisions') async getNoteRevisions( - @Param('noteIdOrAlias') noteIdOrAlias: string, + @Param('noteIdOrAlias', GetNotePipe) note: Note, ): Promise { try { // ToDo: use actual user here const user = await this.userService.getUserByUsername('hardcoded'); - const note = await this.noteService.getNoteByIdOrAlias(noteIdOrAlias); if (!this.permissionsService.mayRead(user, note)) { throw new UnauthorizedException('Reading note denied!'); } @@ -195,22 +178,18 @@ export class NotesController { if (e instanceof NotInDBError) { throw new NotFoundException(e.message); } - if (e instanceof ForbiddenIdError) { - throw new BadRequestException(e.message); - } throw e; } } @Get(':noteIdOrAlias/revisions/:revisionId') async getNoteRevision( - @Param('noteIdOrAlias') noteIdOrAlias: string, + @Param('noteIdOrAlias', GetNotePipe) note: Note, @Param('revisionId') revisionId: number, ): Promise { try { // ToDo: use actual user here const user = await this.userService.getUserByUsername('hardcoded'); - const note = await this.noteService.getNoteByIdOrAlias(noteIdOrAlias); if (!this.permissionsService.mayRead(user, note)) { throw new UnauthorizedException('Reading note denied!'); } @@ -221,9 +200,6 @@ export class NotesController { if (e instanceof NotInDBError) { throw new NotFoundException(e.message); } - if (e instanceof ForbiddenIdError) { - throw new BadRequestException(e.message); - } throw e; } } diff --git a/src/api/public/me/me.controller.ts b/src/api/public/me/me.controller.ts index 3c9770a8a..fd5bcff84 100644 --- a/src/api/public/me/me.controller.ts +++ b/src/api/public/me/me.controller.ts @@ -31,7 +31,9 @@ import { HistoryService } from '../../../history/history.service'; import { ConsoleLoggerService } from '../../../logger/console-logger.service'; import { MediaUploadDto } from '../../../media/media-upload.dto'; import { MediaService } from '../../../media/media.service'; +import { GetNotePipe } from '../../../notes/get-note.pipe'; import { NoteMetadataDto } from '../../../notes/note-metadata.dto'; +import { Note } from '../../../notes/note.entity'; import { NotesService } from '../../../notes/notes.service'; import { UserInfoDto } from '../../../users/user-info.dto'; import { User } from '../../../users/user.entity'; @@ -93,13 +95,10 @@ export class MeController { @ApiNotFoundResponse({ description: notFoundDescription }) async getHistoryEntry( @RequestUser() user: User, - @Param('note') note: string, + @Param('note', GetNotePipe) note: Note, ): Promise { try { - const foundEntry = await this.historyService.getEntryByNoteIdOrAlias( - note, - user, - ); + const foundEntry = await this.historyService.getEntryByNote(note, user); return this.historyService.toHistoryEntryDto(foundEntry); } catch (e) { if (e instanceof NotInDBError) { @@ -119,7 +118,7 @@ export class MeController { @ApiNotFoundResponse({ description: notFoundDescription }) async updateHistoryEntry( @RequestUser() user: User, - @Param('note') note: string, + @Param('note', GetNotePipe) note: Note, @Body() entryUpdateDto: HistoryEntryUpdateDto, ): Promise { // ToDo: Check if user is allowed to pin this history entry @@ -147,7 +146,7 @@ export class MeController { @ApiNotFoundResponse({ description: notFoundDescription }) async deleteHistoryEntry( @RequestUser() user: User, - @Param('note') note: string, + @Param('note', GetNotePipe) note: Note, ): Promise { // ToDo: Check if user is allowed to delete note try { diff --git a/src/api/public/media/media.controller.ts b/src/api/public/media/media.controller.ts index e9d5545c6..0401be2a5 100644 --- a/src/api/public/media/media.controller.ts +++ b/src/api/public/media/media.controller.ts @@ -42,6 +42,8 @@ import { ConsoleLoggerService } from '../../../logger/console-logger.service'; import { MediaUploadUrlDto } from '../../../media/media-upload-url.dto'; import { MediaService } from '../../../media/media.service'; import { MulterFile } from '../../../media/multer-file.interface'; +import { Note } from '../../../notes/note.entity'; +import { NotesService } from '../../../notes/notes.service'; import { User } from '../../../users/user.entity'; import { forbiddenDescription, @@ -58,6 +60,7 @@ export class MediaController { constructor( private readonly logger: ConsoleLoggerService, private mediaService: MediaService, + private noteService: NotesService, ) { this.logger.setContext(MediaController.name); } @@ -93,17 +96,14 @@ export class MediaController { @UploadedFile() file: MulterFile, @Headers('HedgeDoc-Note') noteId: string, ): Promise { - const username = user.userName; - this.logger.debug( - `Recieved filename '${file.originalname}' for note '${noteId}' from user '${username}'`, - 'uploadMedia', - ); try { - const url = await this.mediaService.saveFile( - file.buffer, - username, - noteId, + // TODO: Move getting the Note object into a decorator + const note: Note = await this.noteService.getNoteByIdOrAlias(noteId); + this.logger.debug( + `Recieved filename '${file.originalname}' for note '${noteId}' from user '${user.userName}'`, + 'uploadMedia', ); + const url = await this.mediaService.saveFile(file.buffer, user, note); return this.mediaService.toMediaUploadUrlDto(url); } catch (e) { if (e instanceof ClientError || e instanceof NotInDBError) { diff --git a/src/api/public/notes/notes.controller.ts b/src/api/public/notes/notes.controller.ts index d683549fd..97691b8d2 100644 --- a/src/api/public/notes/notes.controller.ts +++ b/src/api/public/notes/notes.controller.ts @@ -34,12 +34,12 @@ import { AlreadyInDBError, ForbiddenIdError, NotInDBError, - PermissionsUpdateInconsistentError, } from '../../../errors/errors'; import { HistoryService } from '../../../history/history.service'; import { ConsoleLoggerService } from '../../../logger/console-logger.service'; import { MediaUploadDto } from '../../../media/media-upload.dto'; import { MediaService } from '../../../media/media.service'; +import { GetNotePipe } from '../../../notes/get-note.pipe'; import { NoteMetadataDto } from '../../../notes/note-metadata.dto'; import { NotePermissionsDto, @@ -106,24 +106,12 @@ export class NotesController { @FullApi async getNote( @RequestUser() user: User, - @Param('noteIdOrAlias') noteIdOrAlias: string, + @Param('noteIdOrAlias', GetNotePipe) note: Note, ): Promise { - let note: Note; - try { - note = await this.noteService.getNoteByIdOrAlias(noteIdOrAlias); - } catch (e) { - if (e instanceof NotInDBError) { - throw new NotFoundException(e.message); - } - if (e instanceof ForbiddenIdError) { - throw new BadRequestException(e.message); - } - throw e; - } if (!this.permissionsService.mayRead(user, note)) { throw new UnauthorizedException('Reading note denied!'); } - await this.historyService.createOrUpdateHistoryEntry(note, user); + await this.historyService.updateHistoryEntryTimestamp(note, user); return await this.noteService.toNoteDto(note); } @@ -167,35 +155,24 @@ export class NotesController { @FullApi async deleteNote( @RequestUser() user: User, - @Param('noteIdOrAlias') noteIdOrAlias: string, + @Param('noteIdOrAlias', GetNotePipe) note: Note, @Body() noteMediaDeletionDto: NoteMediaDeletionDto, ): Promise { - try { - const note = await this.noteService.getNoteByIdOrAlias(noteIdOrAlias); - if (!this.permissionsService.isOwner(user, note)) { - throw new UnauthorizedException('Deleting note denied!'); - } - const mediaUploads = await this.mediaService.listUploadsByNote(note); - for (const mediaUpload of mediaUploads) { - if (!noteMediaDeletionDto.keepMedia) { - await this.mediaService.deleteFile(mediaUpload); - } else { - await this.mediaService.removeNoteFromMediaUpload(mediaUpload); - } - } - this.logger.debug('Deleting note: ' + noteIdOrAlias, 'deleteNote'); - await this.noteService.deleteNote(note); - this.logger.debug('Successfully deleted ' + noteIdOrAlias, 'deleteNote'); - return; - } catch (e) { - if (e instanceof NotInDBError) { - throw new NotFoundException(e.message); - } - if (e instanceof ForbiddenIdError) { - throw new BadRequestException(e.message); - } - throw e; + if (!this.permissionsService.isOwner(user, note)) { + throw new UnauthorizedException('Deleting note denied!'); } + const mediaUploads = await this.mediaService.listUploadsByNote(note); + for (const mediaUpload of mediaUploads) { + if (!noteMediaDeletionDto.keepMedia) { + await this.mediaService.deleteFile(mediaUpload); + } else { + await this.mediaService.removeNoteFromMediaUpload(mediaUpload); + } + } + this.logger.debug('Deleting note: ' + note.id, 'deleteNote'); + await this.noteService.deleteNote(note); + this.logger.debug('Successfully deleted ' + note.id, 'deleteNote'); + return; } @UseGuards(TokenAuthGuard) @@ -207,27 +184,16 @@ export class NotesController { @FullApi async updateNote( @RequestUser() user: User, - @Param('noteIdOrAlias') noteIdOrAlias: string, + @Param('noteIdOrAlias', GetNotePipe) note: Note, @MarkdownBody() text: string, ): Promise { - try { - const note = await this.noteService.getNoteByIdOrAlias(noteIdOrAlias); - if (!this.permissionsService.mayWrite(user, note)) { - throw new UnauthorizedException('Updating note denied!'); - } - this.logger.debug('Got raw markdown:\n' + text, 'updateNote'); - return await this.noteService.toNoteDto( - await this.noteService.updateNote(note, text), - ); - } catch (e) { - if (e instanceof NotInDBError) { - throw new NotFoundException(e.message); - } - if (e instanceof ForbiddenIdError) { - throw new BadRequestException(e.message); - } - throw e; + if (!this.permissionsService.mayWrite(user, note)) { + throw new UnauthorizedException('Updating note denied!'); } + this.logger.debug('Got raw markdown:\n' + text, 'updateNote'); + return await this.noteService.toNoteDto( + await this.noteService.updateNote(note, text), + ); } @UseGuards(TokenAuthGuard) @@ -240,23 +206,12 @@ export class NotesController { @Header('content-type', 'text/markdown') async getNoteContent( @RequestUser() user: User, - @Param('noteIdOrAlias') noteIdOrAlias: string, + @Param('noteIdOrAlias', GetNotePipe) note: Note, ): Promise { - try { - const note = await this.noteService.getNoteByIdOrAlias(noteIdOrAlias); - if (!this.permissionsService.mayRead(user, note)) { - throw new UnauthorizedException('Reading note denied!'); - } - return await this.noteService.getNoteContent(note); - } catch (e) { - if (e instanceof NotInDBError) { - throw new NotFoundException(e.message); - } - if (e instanceof ForbiddenIdError) { - throw new BadRequestException(e.message); - } - throw e; + if (!this.permissionsService.mayRead(user, note)) { + throw new UnauthorizedException('Reading note denied!'); } + return await this.noteService.getNoteContent(note); } @UseGuards(TokenAuthGuard) @@ -268,26 +223,12 @@ export class NotesController { @FullApi async getNoteMetadata( @RequestUser() user: User, - @Param('noteIdOrAlias') noteIdOrAlias: string, + @Param('noteIdOrAlias', GetNotePipe) note: Note, ): Promise { - try { - const note = await this.noteService.getNoteByIdOrAlias(noteIdOrAlias); - if (!this.permissionsService.mayRead(user, note)) { - throw new UnauthorizedException('Reading note denied!'); - } - return await this.noteService.toNoteMetadataDto(note); - } catch (e) { - if (e instanceof NotInDBError) { - throw new NotFoundException(e.message); - } - if (e instanceof PermissionsUpdateInconsistentError) { - throw new BadRequestException(e.message); - } - if (e instanceof ForbiddenIdError) { - throw new BadRequestException(e.message); - } - throw e; + if (!this.permissionsService.mayRead(user, note)) { + throw new UnauthorizedException('Reading note denied!'); } + return await this.noteService.toNoteMetadataDto(note); } @UseGuards(TokenAuthGuard) @@ -299,26 +240,15 @@ export class NotesController { @FullApi async updateNotePermissions( @RequestUser() user: User, - @Param('noteIdOrAlias') noteIdOrAlias: string, + @Param('noteIdOrAlias', GetNotePipe) note: Note, @Body() updateDto: NotePermissionsUpdateDto, ): Promise { - try { - const note = await this.noteService.getNoteByIdOrAlias(noteIdOrAlias); - if (!this.permissionsService.isOwner(user, note)) { - throw new UnauthorizedException('Updating note denied!'); - } - return this.noteService.toNotePermissionsDto( - await this.noteService.updateNotePermissions(note, updateDto), - ); - } catch (e) { - if (e instanceof NotInDBError) { - throw new NotFoundException(e.message); - } - if (e instanceof ForbiddenIdError) { - throw new BadRequestException(e.message); - } - throw e; + if (!this.permissionsService.isOwner(user, note)) { + throw new UnauthorizedException('Updating note denied!'); } + return this.noteService.toNotePermissionsDto( + await this.noteService.updateNotePermissions(note, updateDto), + ); } @UseGuards(TokenAuthGuard) @@ -331,28 +261,17 @@ export class NotesController { @FullApi async getNoteRevisions( @RequestUser() user: User, - @Param('noteIdOrAlias') noteIdOrAlias: string, + @Param('noteIdOrAlias', GetNotePipe) note: Note, ): Promise { - try { - const note = await this.noteService.getNoteByIdOrAlias(noteIdOrAlias); - if (!this.permissionsService.mayRead(user, note)) { - throw new UnauthorizedException('Reading note denied!'); - } - const revisions = await this.revisionsService.getAllRevisions(note); - return await Promise.all( - revisions.map((revision) => - this.revisionsService.toRevisionMetadataDto(revision), - ), - ); - } catch (e) { - if (e instanceof NotInDBError) { - throw new NotFoundException(e.message); - } - if (e instanceof ForbiddenIdError) { - throw new BadRequestException(e.message); - } - throw e; + if (!this.permissionsService.mayRead(user, note)) { + throw new UnauthorizedException('Reading note denied!'); } + const revisions = await this.revisionsService.getAllRevisions(note); + return await Promise.all( + revisions.map((revision) => + this.revisionsService.toRevisionMetadataDto(revision), + ), + ); } @UseGuards(TokenAuthGuard) @@ -364,14 +283,13 @@ export class NotesController { @FullApi async getNoteRevision( @RequestUser() user: User, - @Param('noteIdOrAlias') noteIdOrAlias: string, + @Param('noteIdOrAlias', GetNotePipe) note: Note, @Param('revisionId') revisionId: number, ): Promise { + if (!this.permissionsService.mayRead(user, note)) { + throw new UnauthorizedException('Reading note denied!'); + } try { - const note = await this.noteService.getNoteByIdOrAlias(noteIdOrAlias); - if (!this.permissionsService.mayRead(user, note)) { - throw new UnauthorizedException('Reading note denied!'); - } return this.revisionsService.toRevisionDto( await this.revisionsService.getRevision(note, revisionId), ); @@ -379,9 +297,6 @@ export class NotesController { if (e instanceof NotInDBError) { throw new NotFoundException(e.message); } - if (e instanceof ForbiddenIdError) { - throw new BadRequestException(e.message); - } throw e; } } @@ -396,20 +311,12 @@ export class NotesController { @ApiUnauthorizedResponse({ description: unauthorizedDescription }) async getNotesMedia( @RequestUser() user: User, - @Param('noteIdOrAlias') noteIdOrAlias: string, + @Param('noteIdOrAlias', GetNotePipe) note: Note, ): Promise { - try { - const note = await this.noteService.getNoteByIdOrAlias(noteIdOrAlias); - if (!this.permissionsService.mayRead(user, note)) { - throw new UnauthorizedException('Reading note denied!'); - } - const media = await this.mediaService.listUploadsByNote(note); - return media.map((media) => this.mediaService.toMediaUploadDto(media)); - } catch (e) { - if (e instanceof NotInDBError) { - throw new NotFoundException(e.message); - } - throw e; + if (!this.permissionsService.mayRead(user, note)) { + throw new UnauthorizedException('Reading note denied!'); } + const media = await this.mediaService.listUploadsByNote(note); + return media.map((media) => this.mediaService.toMediaUploadDto(media)); } } diff --git a/src/history/history.service.spec.ts b/src/history/history.service.spec.ts index 955eec642..ddfa763cb 100644 --- a/src/history/history.service.spec.ts +++ b/src/history/history.service.spec.ts @@ -135,31 +135,7 @@ describe('HistoryService', () => { }); }); - describe('getEntryByNoteIdOrAlias', () => { - const user = {} as User; - const alias = 'alias'; - describe('works', () => { - it('with history entry', async () => { - const note = Note.create(user, alias); - const historyEntry = HistoryEntry.create(user, note); - jest.spyOn(historyRepo, 'findOne').mockResolvedValueOnce(historyEntry); - jest.spyOn(noteRepo, 'findOne').mockResolvedValueOnce(note); - expect(await service.getEntryByNoteIdOrAlias(alias, user)).toEqual( - historyEntry, - ); - }); - }); - describe('fails', () => { - it('with an non-existing note', async () => { - jest.spyOn(noteRepo, 'findOne').mockResolvedValueOnce(undefined); - await expect( - service.getEntryByNoteIdOrAlias(alias, {} as User), - ).rejects.toThrow(NotInDBError); - }); - }); - }); - - describe('createOrUpdateHistoryEntry', () => { + describe('updateHistoryEntryTimestamp', () => { describe('works', () => { const user = {} as User; const alias = 'alias'; @@ -171,7 +147,7 @@ describe('HistoryService', () => { .mockImplementation( async (entry: HistoryEntry): Promise => entry, ); - const createHistoryEntry = await service.createOrUpdateHistoryEntry( + const createHistoryEntry = await service.updateHistoryEntryTimestamp( Note.create(user, alias), user, ); @@ -188,7 +164,7 @@ describe('HistoryService', () => { .mockImplementation( async (entry: HistoryEntry): Promise => entry, ); - const createHistoryEntry = await service.createOrUpdateHistoryEntry( + const createHistoryEntry = await service.updateHistoryEntryTimestamp( Note.create(user, alias), user, ); @@ -218,7 +194,7 @@ describe('HistoryService', () => { async (entry: HistoryEntry): Promise => entry, ); const updatedHistoryEntry = await service.updateHistoryEntry( - alias, + note, user, { pinStatus: true, @@ -237,7 +213,7 @@ describe('HistoryService', () => { jest.spyOn(historyRepo, 'findOne').mockResolvedValueOnce(undefined); jest.spyOn(noteRepo, 'findOne').mockResolvedValueOnce(note); await expect( - service.updateHistoryEntry(alias, user, { + service.updateHistoryEntry(note, user, { pinStatus: true, }), ).rejects.toThrow(NotInDBError); @@ -311,7 +287,7 @@ describe('HistoryService', () => { return entry; }, ); - await service.deleteHistoryEntry(alias, user); + await service.deleteHistoryEntry(note, user); }); }); describe('fails', () => { @@ -321,16 +297,10 @@ describe('HistoryService', () => { const note = Note.create(user, alias); jest.spyOn(historyRepo, 'findOne').mockResolvedValueOnce(undefined); jest.spyOn(noteRepo, 'findOne').mockResolvedValueOnce(note); - await expect(service.deleteHistoryEntry(alias, user)).rejects.toThrow( + await expect(service.deleteHistoryEntry(note, user)).rejects.toThrow( NotInDBError, ); }); - it('without a note', async () => { - jest.spyOn(noteRepo, 'findOne').mockResolvedValueOnce(undefined); - await expect( - service.getEntryByNoteIdOrAlias(alias, {} as User), - ).rejects.toThrow(NotInDBError); - }); }); }); diff --git a/src/history/history.service.ts b/src/history/history.service.ts index 13a290ad5..f61b8e93b 100644 --- a/src/history/history.service.ts +++ b/src/history/history.service.ts @@ -45,22 +45,6 @@ export class HistoryService { }); } - /** - * @async - * Get a history entry by the user and note, which is specified via id or alias - * @param {string} noteIdOrAlias - the id or alias specifying the note - * @param {User} user - the user that the note belongs to - * @throws {NotInDBError} the specified note does not exist - * @return {HistoryEntry} the requested history entry - */ - async getEntryByNoteIdOrAlias( - noteIdOrAlias: string, - user: User, - ): Promise { - const note = await this.notesService.getNoteByIdOrAlias(noteIdOrAlias); - return await this.getEntryByNote(note, user); - } - /** * @async * Get a history entry by the user and note @@ -68,7 +52,7 @@ export class HistoryService { * @param {User} user - the user that the history entry belongs to * @return {HistoryEntry} the requested history entry */ - private async getEntryByNote(note: Note, user: User): Promise { + async getEntryByNote(note: Note, user: User): Promise { const entry = await this.historyEntryRepository.findOne({ where: { note: note, @@ -86,12 +70,13 @@ export class HistoryService { /** * @async - * Create or update a history entry by the user and note. If the entry is merely updated the updatedAt date is set to the current date. + * Updates the updatedAt timestamp of a HistoryEntry. + * If no history entry exists, it will be created. * @param {Note} note - the note that the history entry belongs to * @param {User} user - the user that the history entry belongs to * @return {HistoryEntry} the requested history entry */ - async createOrUpdateHistoryEntry( + async updateHistoryEntryTimestamp( note: Note, user: User, ): Promise { @@ -111,17 +96,17 @@ export class HistoryService { /** * @async * Update a history entry identified by the user and a note id or alias - * @param {string} noteIdOrAlias - the note that the history entry belongs to + * @param {Note} note - the note that the history entry belongs to * @param {User} user - the user that the history entry belongs to * @param {HistoryEntryUpdateDto} updateDto - the change that should be applied to the history entry * @return {HistoryEntry} the requested history entry */ async updateHistoryEntry( - noteIdOrAlias: string, + note: Note, user: User, updateDto: HistoryEntryUpdateDto, ): Promise { - const entry = await this.getEntryByNoteIdOrAlias(noteIdOrAlias, user); + const entry = await this.getEntryByNote(note, user); entry.pinStatus = updateDto.pinStatus; return await this.historyEntryRepository.save(entry); } @@ -129,12 +114,12 @@ export class HistoryService { /** * @async * Delete the history entry identified by the user and a note id or alias - * @param {string} noteIdOrAlias - the note that the history entry belongs to + * @param {Note} note - the note that the history entry belongs to * @param {User} user - the user that the history entry belongs to * @throws {NotInDBError} the specified history entry does not exist */ - async deleteHistoryEntry(noteIdOrAlias: string, user: User): Promise { - const entry = await this.getEntryByNoteIdOrAlias(noteIdOrAlias, user); + async deleteHistoryEntry(note: Note, user: User): Promise { + const entry = await this.getEntryByNote(note, user); await this.historyEntryRepository.remove(entry); return; } diff --git a/src/media/media.service.spec.ts b/src/media/media.service.spec.ts index 75682436f..4dec3d89a 100644 --- a/src/media/media.service.spec.ts +++ b/src/media/media.service.spec.ts @@ -98,10 +98,12 @@ describe('MediaService', () => { }); describe('saveFile', () => { + let user: User; + let note: Note; beforeEach(() => { - const user = User.create('hardcoded', 'Testy') as User; + user = User.create('hardcoded', 'Testy') as User; const alias = 'alias'; - const note = Note.create(user, alias); + note = Note.create(user, alias); jest.spyOn(userRepo, 'findOne').mockResolvedValueOnce(user); jest.spyOn(noteRepo, 'findOne').mockResolvedValueOnce(note); }); @@ -126,22 +128,22 @@ describe('MediaService', () => { return [fileName, null]; }, ); - const url = await service.saveFile(testImage, 'hardcoded', 'test'); + const url = await service.saveFile(testImage, user, note); expect(url).toEqual(fileId); }); describe('fails:', () => { it('MIME type not identifiable', async () => { await expect( - service.saveFile(Buffer.alloc(1), 'hardcoded', 'test'), + service.saveFile(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, 'hardcoded', 'test'), - ).rejects.toThrow(ClientError); + await expect(service.saveFile(testText, user, note)).rejects.toThrow( + ClientError, + ); }); }); }); diff --git a/src/media/media.service.ts b/src/media/media.service.ts index fd8cdbc83..1c523051a 100644 --- a/src/media/media.service.ts +++ b/src/media/media.service.ts @@ -69,24 +69,18 @@ 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 {Buffer} fileBuffer - the buffer of the file to save. - * @param {string} username - the username of the user who uploaded this file - * @param {string} noteId - the id or alias of the note which will be associated with the new file. + * @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 * @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( - fileBuffer: Buffer, - username: string, - noteId: string, - ): Promise { + async saveFile(fileBuffer: Buffer, user: User, note: Note): Promise { this.logger.debug( - `Saving file for note '${noteId}' and user '${username}'`, + `Saving file for note '${note.id}' and user '${user.userName}'`, 'saveFile', ); - const note = await this.notesService.getNoteByIdOrAlias(noteId); - const user = await this.usersService.getUserByUsername(username); const fileTypeResult = await FileType.fromBuffer(fileBuffer); if (!fileTypeResult) { throw new ClientError('Could not detect file type.'); diff --git a/src/notes/get-note.pipe.ts b/src/notes/get-note.pipe.ts new file mode 100644 index 000000000..c8cdd4cd5 --- /dev/null +++ b/src/notes/get-note.pipe.ts @@ -0,0 +1,43 @@ +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { + ArgumentMetadata, + BadRequestException, + Injectable, + NotFoundException, + PipeTransform, +} from '@nestjs/common'; + +import { ForbiddenIdError, NotInDBError } from '../errors/errors'; +import { ConsoleLoggerService } from '../logger/console-logger.service'; +import { Note } from './note.entity'; +import { NotesService } from './notes.service'; + +@Injectable() +export class GetNotePipe implements PipeTransform> { + constructor( + private readonly logger: ConsoleLoggerService, + private noteService: NotesService, + ) { + this.logger.setContext(GetNotePipe.name); + } + + async transform(noteIdOrAlias: string, _: ArgumentMetadata): Promise { + let note: Note; + try { + note = await this.noteService.getNoteByIdOrAlias(noteIdOrAlias); + } catch (e) { + if (e instanceof NotInDBError) { + throw new NotFoundException(e.message); + } + if (e instanceof ForbiddenIdError) { + throw new BadRequestException(e.message); + } + throw e; + } + return note; + } +} diff --git a/src/notes/notes.service.ts b/src/notes/notes.service.ts index 58dcc0de0..b0dfd0ab9 100644 --- a/src/notes/notes.service.ts +++ b/src/notes/notes.service.ts @@ -129,7 +129,7 @@ export class NotesService { * @return {Revision} the first revision of the note */ async getLatestRevision(note: Note): Promise { - return await this.revisionsService.getLatestRevision(note.id); + return await this.revisionsService.getLatestRevision(note); } /** @@ -139,7 +139,7 @@ export class NotesService { * @return {Revision} the last revision of the note */ async getFirstRevision(note: Note): Promise { - return await this.revisionsService.getFirstRevision(note.id); + return await this.revisionsService.getFirstRevision(note); } /** diff --git a/src/revisions/revisions.service.ts b/src/revisions/revisions.service.ts index 63e6402a1..c745da897 100644 --- a/src/revisions/revisions.service.ts +++ b/src/revisions/revisions.service.ts @@ -49,10 +49,10 @@ export class RevisionsService { return revision; } - async getLatestRevision(noteId: string): Promise { + async getLatestRevision(note: Note): Promise { const revision = await this.revisionRepository.findOne({ where: { - note: noteId, + note: note, }, order: { createdAt: 'DESC', @@ -60,22 +60,22 @@ export class RevisionsService { }, }); if (revision === undefined) { - throw new NotInDBError(`Revision for note ${noteId} not found.`); + throw new NotInDBError(`Revision for note ${note.id} not found.`); } return revision; } - async getFirstRevision(noteId: string): Promise { + async getFirstRevision(note: Note): Promise { const revision = await this.revisionRepository.findOne({ where: { - note: noteId, + note: note, }, order: { createdAt: 'ASC', }, }); if (revision === undefined) { - throw new NotInDBError(`Revision for note ${noteId} not found.`); + throw new NotInDBError(`Revision for note ${note.id} not found.`); } return revision; } diff --git a/test/private-api/history.e2e-spec.ts b/test/private-api/history.e2e-spec.ts index ea548c42c..1aae79ffe 100644 --- a/test/private-api/history.e2e-spec.ts +++ b/test/private-api/history.e2e-spec.ts @@ -87,7 +87,7 @@ describe('History', () => { .expect('Content-Type', /json/) .expect(200); expect(emptyResponse.body.length).toEqual(0); - const entry = await historyService.createOrUpdateHistoryEntry(note, user); + const entry = await historyService.updateHistoryEntryTimestamp(note, user); const entryDto = historyService.toHistoryEntryDto(entry); const response = await request(app.getHttpServer()) .get('/me/history') @@ -182,7 +182,7 @@ describe('History', () => { }); it('PUT /me/history/:note', async () => { - const entry = await historyService.createOrUpdateHistoryEntry(note2, user); + const entry = await historyService.updateHistoryEntryTimestamp(note2, user); expect(entry.pinStatus).toBeFalsy(); await request(app.getHttpServer()) .put(`/me/history/${entry.note.alias || 'undefined'}`) @@ -191,12 +191,12 @@ describe('History', () => { const userEntries = await historyService.getEntriesByUser(user); expect(userEntries.length).toEqual(1); expect(userEntries[0].pinStatus).toBeTruthy(); - await historyService.deleteHistoryEntry(note2.alias, user); + await historyService.deleteHistoryEntry(note2, user); }); it('DELETE /me/history/:note', async () => { - const entry = await historyService.createOrUpdateHistoryEntry(note2, user); - const entry2 = await historyService.createOrUpdateHistoryEntry(note, user); + const entry = await historyService.updateHistoryEntryTimestamp(note2, user); + const entry2 = await historyService.updateHistoryEntryTimestamp(note, user); const entryDto = historyService.toHistoryEntryDto(entry2); await request(app.getHttpServer()) .delete(`/me/history/${entry.note.alias || 'undefined'}`) diff --git a/test/private-api/me.e2e-spec.ts b/test/private-api/me.e2e-spec.ts index 3c7a2ca7e..d0d35499e 100644 --- a/test/private-api/me.e2e-spec.ts +++ b/test/private-api/me.e2e-spec.ts @@ -111,26 +111,10 @@ describe('Me', () => { expect(responseBefore.body).toHaveLength(0); const testImage = await fs.readFile('test/public-api/fixtures/test.png'); - const url0 = await mediaService.saveFile( - testImage, - 'hardcoded', - note1.publicId, - ); - const url1 = await mediaService.saveFile( - testImage, - 'hardcoded', - note1.publicId, - ); - const url2 = await mediaService.saveFile( - testImage, - 'hardcoded', - note2.alias ?? '', - ); - const url3 = await mediaService.saveFile( - testImage, - 'hardcoded', - note2.alias ?? '', - ); + const url0 = await mediaService.saveFile(testImage, user, note1); + const url1 = await mediaService.saveFile(testImage, user, note1); + const url2 = await mediaService.saveFile(testImage, user, note2); + const url3 = await mediaService.saveFile(testImage, user, note2); const response = await request(httpServer) .get('/me/media/') @@ -163,11 +147,7 @@ describe('Me', () => { it('DELETE /me', async () => { const testImage = await fs.readFile('test/public-api/fixtures/test.png'); - const url0 = await mediaService.saveFile( - testImage, - 'hardcoded', - note1.publicId, - ); + const url0 = await mediaService.saveFile(testImage, user, note1); const dbUser = await userService.getUserByUsername('hardcoded'); expect(dbUser).toBeInstanceOf(User); const mediaUploads = await mediaService.listUploadsByUser(dbUser); diff --git a/test/private-api/notes.e2e-spec.ts b/test/private-api/notes.e2e-spec.ts index 65ced873e..d04347a56 100644 --- a/test/private-api/notes.e2e-spec.ts +++ b/test/private-api/notes.e2e-spec.ts @@ -157,8 +157,8 @@ describe('Notes', () => { describe('works', () => { it('with an existing alias and keepMedia false', async () => { const noteId = 'test3'; - await notesService.createNote(content, noteId, user); - await mediaService.saveFile(testImage, user.userName, noteId); + const note = await notesService.createNote(content, noteId, user); + await mediaService.saveFile(testImage, user, note); await request(app.getHttpServer()) .delete(`/notes/${noteId}`) .set('Content-Type', 'application/json') @@ -174,12 +174,8 @@ describe('Notes', () => { }); it('with an existing alias and keepMedia true', async () => { const noteId = 'test3a'; - await notesService.createNote(content, noteId, user); - const url = await mediaService.saveFile( - testImage, - user.userName, - noteId, - ); + const note = await notesService.createNote(content, noteId, user); + const url = await mediaService.saveFile(testImage, user, note); await request(app.getHttpServer()) .delete(`/notes/${noteId}`) .set('Content-Type', 'application/json') @@ -263,8 +259,8 @@ describe('Notes', () => { it('works', async () => { const alias = 'test6'; const extraAlias = 'test7'; - await notesService.createNote(content, alias, user); - await notesService.createNote(content, extraAlias, user); + const note1 = await notesService.createNote(content, alias, user); + const note2 = await notesService.createNote(content, extraAlias, user); const httpServer = app.getHttpServer(); const response = await request(httpServer) .get(`/notes/${alias}/media/`) @@ -273,12 +269,8 @@ describe('Notes', () => { expect(response.body).toHaveLength(0); const testImage = await fs.readFile('test/private-api/fixtures/test.png'); - const url0 = await mediaService.saveFile(testImage, 'hardcoded', alias); - const url1 = await mediaService.saveFile( - testImage, - 'hardcoded', - extraAlias, - ); + const url0 = await mediaService.saveFile(testImage, user, note1); + const url1 = await mediaService.saveFile(testImage, user, note2); const responseAfter = await request(httpServer) .get(`/notes/${alias}/media/`) diff --git a/test/public-api/me.e2e-spec.ts b/test/public-api/me.e2e-spec.ts index 38ed4c5f5..18b3a1a28 100644 --- a/test/public-api/me.e2e-spec.ts +++ b/test/public-api/me.e2e-spec.ts @@ -96,10 +96,8 @@ describe('Me', () => { it(`GET /me/history`, async () => { const noteName = 'testGetNoteHistory1'; const note = await notesService.createNote('', noteName); - const createdHistoryEntry = await historyService.createOrUpdateHistoryEntry( - note, - user, - ); + const createdHistoryEntry = + await historyService.updateHistoryEntryTimestamp(note, user); const response = await request(app.getHttpServer()) .get('/me/history') .expect('Content-Type', /json/) @@ -123,7 +121,7 @@ describe('Me', () => { const noteName = 'testGetNoteHistory2'; const note = await notesService.createNote('', noteName); const createdHistoryEntry = - await historyService.createOrUpdateHistoryEntry(note, user); + await historyService.updateHistoryEntryTimestamp(note, user); const response = await request(app.getHttpServer()) .get(`/me/history/${noteName}`) .expect('Content-Type', /json/) @@ -151,7 +149,7 @@ describe('Me', () => { it('works', async () => { const noteName = 'testGetNoteHistory3'; const note = await notesService.createNote('', noteName); - await historyService.createOrUpdateHistoryEntry(note, user); + await historyService.updateHistoryEntryTimestamp(note, user); const historyEntryUpdateDto = new HistoryEntryUpdateDto(); historyEntryUpdateDto.pinStatus = true; const response = await request(app.getHttpServer()) @@ -181,7 +179,7 @@ describe('Me', () => { it('works', async () => { const noteName = 'testGetNoteHistory4'; const note = await notesService.createNote('', noteName); - await historyService.createOrUpdateHistoryEntry(note, user); + await historyService.updateHistoryEntryTimestamp(note, user); const response = await request(app.getHttpServer()) .delete(`/me/history/${noteName}`) .expect(204); @@ -243,26 +241,10 @@ describe('Me', () => { expect(response1.body).toHaveLength(0); const testImage = await fs.readFile('test/public-api/fixtures/test.png'); - const url0 = await mediaService.saveFile( - testImage, - 'hardcoded', - note1.publicId, - ); - const url1 = await mediaService.saveFile( - testImage, - 'hardcoded', - note1.publicId, - ); - const url2 = await mediaService.saveFile( - testImage, - 'hardcoded', - note2.publicId, - ); - const url3 = await mediaService.saveFile( - testImage, - 'hardcoded', - note2.publicId, - ); + const url0 = await mediaService.saveFile(testImage, user, note1); + const url1 = await mediaService.saveFile(testImage, user, note1); + const url2 = await mediaService.saveFile(testImage, user, note2); + const url3 = await mediaService.saveFile(testImage, user, note2); const response = await request(httpServer) .get('/me/media/') diff --git a/test/public-api/media.e2e-spec.ts b/test/public-api/media.e2e-spec.ts index 04a984f33..ec21f6ead 100644 --- a/test/public-api/media.e2e-spec.ts +++ b/test/public-api/media.e2e-spec.ts @@ -22,15 +22,20 @@ import { ConsoleLoggerService } from '../../src/logger/console-logger.service'; import { LoggerModule } from '../../src/logger/logger.module'; import { MediaModule } from '../../src/media/media.module'; import { MediaService } from '../../src/media/media.service'; +import { Note } from '../../src/notes/note.entity'; import { NotesModule } from '../../src/notes/notes.module'; import { NotesService } from '../../src/notes/notes.service'; import { PermissionsModule } from '../../src/permissions/permissions.module'; +import { User } from '../../src/users/user.entity'; +import { UsersService } from '../../src/users/users.service'; import { ensureDeleted } from '../utils'; describe('Media', () => { let app: NestExpressApplication; let mediaService: MediaService; let uploadPath: string; + let testNote: Note; + let user: User; beforeAll(async () => { const moduleRef = await Test.createTestingModule({ @@ -69,7 +74,12 @@ describe('Media', () => { logger.log('Switching logger', 'AppBootstrap'); app.useLogger(logger); const notesService: NotesService = moduleRef.get(NotesService); - await notesService.createNote('test content', 'test_upload_media'); + const userService = moduleRef.get(UsersService); + user = await userService.createUser('hardcoded', 'Testy'); + testNote = await notesService.createNote( + 'test content', + 'test_upload_media', + ); mediaService = moduleRef.get(MediaService); }); @@ -129,11 +139,7 @@ describe('Media', () => { it('DELETE /media/{filename}', async () => { const testImage = await fs.readFile('test/public-api/fixtures/test.png'); - const url = await mediaService.saveFile( - testImage, - 'hardcoded', - 'test_upload_media', - ); + const url = await mediaService.saveFile(testImage, user, testNote); const filename = url.split('/').pop() || ''; await request(app.getHttpServer()) .delete('/media/' + filename) diff --git a/test/public-api/notes.e2e-spec.ts b/test/public-api/notes.e2e-spec.ts index 61abb79eb..f72b0d258 100644 --- a/test/public-api/notes.e2e-spec.ts +++ b/test/public-api/notes.e2e-spec.ts @@ -113,6 +113,13 @@ describe('Notes', () => { .expect('Content-Type', /json/) .expect(404); }); + it('fails with a forbidden note id', async () => { + // check if a forbidden note correctly returns 400 + await request(app.getHttpServer()) + .get('/notes/forbiddenNoteId') + .expect('Content-Type', /json/) + .expect(400); + }); }); describe('POST /notes/{note}', () => { @@ -154,8 +161,8 @@ describe('Notes', () => { describe('works', () => { it('with an existing alias and keepMedia false', async () => { const noteId = 'test3'; - await notesService.createNote(content, noteId, user); - await mediaService.saveFile(testImage, user.userName, noteId); + const note = await notesService.createNote(content, noteId, user); + await mediaService.saveFile(testImage, user, note); await request(app.getHttpServer()) .delete(`/notes/${noteId}`) .set('Content-Type', 'application/json') @@ -170,12 +177,8 @@ describe('Notes', () => { }); it('with an existing alias and keepMedia true', async () => { const noteId = 'test3a'; - await notesService.createNote(content, noteId, user); - const url = await mediaService.saveFile( - testImage, - user.userName, - noteId, - ); + const note = await notesService.createNote(content, noteId, user); + const url = await mediaService.saveFile(testImage, user, note); await request(app.getHttpServer()) .delete(`/notes/${noteId}`) .set('Content-Type', 'application/json') @@ -395,8 +398,8 @@ describe('Notes', () => { it('works', async () => { const alias = 'test9'; const extraAlias = 'test10'; - await notesService.createNote(content, alias, user); - await notesService.createNote(content, extraAlias, user); + const note1 = await notesService.createNote(content, alias, user); + const note2 = await notesService.createNote(content, extraAlias, user); const httpServer = app.getHttpServer(); const response = await request(httpServer) .get(`/notes/${alias}/media/`) @@ -405,12 +408,8 @@ describe('Notes', () => { expect(response.body).toHaveLength(0); const testImage = await fs.readFile('test/public-api/fixtures/test.png'); - const url0 = await mediaService.saveFile(testImage, 'hardcoded', alias); - const url1 = await mediaService.saveFile( - testImage, - 'hardcoded', - extraAlias, - ); + const url0 = await mediaService.saveFile(testImage, user, note1); + const url1 = await mediaService.saveFile(testImage, user, note2); const responseAfter = await request(httpServer) .get(`/notes/${alias}/media/`)