From ea0588f02e984b4452ec73a6202f14713ac810a8 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Tue, 30 Nov 2021 22:20:49 +0100 Subject: [PATCH 1/3] feat: refactor get note pipe to interceptor This is necessary, because of the order of operations in nestjs, the validation pipe is not able to get the note as the noteIdOrAlias will be transformed by the get note pipe after the validation did run. Signed-off-by: Philip Molares --- ...t-note.pipe.ts => get-note.interceptor.ts} | 34 ++++++++++++------- src/api/utils/permissions.guard.ts | 4 +-- 2 files changed, 24 insertions(+), 14 deletions(-) rename src/api/utils/{get-note.pipe.ts => get-note.interceptor.ts} (52%) diff --git a/src/api/utils/get-note.pipe.ts b/src/api/utils/get-note.interceptor.ts similarity index 52% rename from src/api/utils/get-note.pipe.ts rename to src/api/utils/get-note.interceptor.ts index 8790e326a..80d98aa13 100644 --- a/src/api/utils/get-note.pipe.ts +++ b/src/api/utils/get-note.interceptor.ts @@ -4,29 +4,39 @@ * SPDX-License-Identifier: AGPL-3.0-only */ import { - ArgumentMetadata, BadRequestException, + CallHandler, + ExecutionContext, Injectable, + NestInterceptor, NotFoundException, - PipeTransform, } from '@nestjs/common'; +import { Request } from 'express'; +import { Observable } from 'rxjs'; import { ForbiddenIdError, NotInDBError } from '../../errors/errors'; -import { ConsoleLoggerService } from '../../logger/console-logger.service'; import { Note } from '../../notes/note.entity'; import { NotesService } from '../../notes/notes.service'; +import { User } from '../../users/user.entity'; +/** + * Saves the note identified by the `noteIdOrAlias` URL parameter + * under the `note` property of the request object. + */ @Injectable() -export class GetNotePipe implements PipeTransform> { - constructor( - private readonly logger: ConsoleLoggerService, - private noteService: NotesService, - ) { - this.logger.setContext(GetNotePipe.name); - } +export class GetNoteInterceptor implements NestInterceptor { + constructor(private noteService: NotesService) {} - async transform(noteIdOrAlias: string, _: ArgumentMetadata): Promise { - return await getNote(this.noteService, noteIdOrAlias); + async intercept( + context: ExecutionContext, + next: CallHandler, + ): Promise> { + const request: Request & { user: User; note: Note } = context + .switchToHttp() + .getRequest(); + const noteIdOrAlias = request.params['noteIdOrAlias']; + request.note = await getNote(this.noteService, noteIdOrAlias); + return next.handle(); } } diff --git a/src/api/utils/permissions.guard.ts b/src/api/utils/permissions.guard.ts index 5206f1c48..bdbe95ee5 100644 --- a/src/api/utils/permissions.guard.ts +++ b/src/api/utils/permissions.guard.ts @@ -12,7 +12,7 @@ import { NotesService } from '../../notes/notes.service'; import { Permission } from '../../permissions/permissions.enum'; import { PermissionsService } from '../../permissions/permissions.service'; import { User } from '../../users/user.entity'; -import { getNote } from './get-note.pipe'; +import { getNote } from './get-note.interceptor'; /** * This guards controller methods from access, if the user has not the appropriate permissions. @@ -50,7 +50,7 @@ export class PermissionsGuard implements CanActivate { return this.permissionsService.mayCreate(user); } // Get the note from the parameter noteIdOrAlias - // Attention: This gets the note an additional time if used in conjunction with GetNotePipe + // Attention: This gets the note an additional time if used in conjunction with GetNoteInterceptor const noteIdOrAlias = request.params['noteIdOrAlias']; const note = await getNote(this.noteService, noteIdOrAlias); switch (permissions[0]) { From 9e2a138a149c6d9c87e5f4bd423f8c4a7d16b016 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Tue, 30 Nov 2021 22:26:55 +0100 Subject: [PATCH 2/3] feat: add request note decorator This extracts the note inserted with the get note interceptor into the request to be used by the controller service. Signed-off-by: Philip Molares --- src/api/utils/request-note.decorator.ts | 32 +++++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 src/api/utils/request-note.decorator.ts diff --git a/src/api/utils/request-note.decorator.ts b/src/api/utils/request-note.decorator.ts new file mode 100644 index 000000000..f10370abe --- /dev/null +++ b/src/api/utils/request-note.decorator.ts @@ -0,0 +1,32 @@ +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { + createParamDecorator, + ExecutionContext, + InternalServerErrorException, +} from '@nestjs/common'; +import { Request } from 'express'; + +import { Note } from '../../notes/note.entity'; + +/** + * Extracts the {@link Note} object from a request + * + * Will throw an {@link InternalServerErrorException} if no note is present + */ +// eslint-disable-next-line @typescript-eslint/naming-convention +export const RequestNote = createParamDecorator( + (data: unknown, ctx: ExecutionContext) => { + const request: Request & { note: Note } = ctx.switchToHttp().getRequest(); + if (!request.note) { + // We should have a note here, otherwise something is wrong + throw new InternalServerErrorException( + 'Request is missing a note object', + ); + } + return request.note; + }, +); From 6fddeebc56c625751db113fb2b7f6486eb3bc8a5 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Tue, 30 Nov 2021 22:33:20 +0100 Subject: [PATCH 3/3] feat: replace GetNotePipe with GetNoteInterceptor and RequestNote Signed-off-by: Philip Molares --- .../private/me/history/history.controller.ts | 15 +++++---- src/api/private/notes/notes.controller.ts | 24 ++++++++------ src/api/public/me/me.controller.ts | 20 +++++++----- src/api/public/notes/notes.controller.ts | 31 +++++++++++++------ 4 files changed, 57 insertions(+), 33 deletions(-) diff --git a/src/api/private/me/history/history.controller.ts b/src/api/private/me/history/history.controller.ts index 3cb5f32a8..6b3ebe9b1 100644 --- a/src/api/private/me/history/history.controller.ts +++ b/src/api/private/me/history/history.controller.ts @@ -10,10 +10,10 @@ import { Delete, Get, NotFoundException, - Param, Post, Put, UseGuards, + UseInterceptors, } from '@nestjs/common'; import { ApiTags } from '@nestjs/swagger'; @@ -26,7 +26,8 @@ import { SessionGuard } from '../../../../identity/session.guard'; import { ConsoleLoggerService } from '../../../../logger/console-logger.service'; import { Note } from '../../../../notes/note.entity'; import { User } from '../../../../users/user.entity'; -import { GetNotePipe } from '../../../utils/get-note.pipe'; +import { GetNoteInterceptor } from '../../../utils/get-note.interceptor'; +import { RequestNote } from '../../../utils/request-note.decorator'; import { RequestUser } from '../../../utils/request-user.decorator'; @UseGuards(SessionGuard) @@ -82,9 +83,10 @@ export class HistoryController { } } - @Put(':note') + @Put(':noteIdOrAlias') + @UseInterceptors(GetNoteInterceptor) async updateHistoryEntry( - @Param('note', GetNotePipe) note: Note, + @RequestNote() note: Note, @RequestUser() user: User, @Body() entryUpdateDto: HistoryEntryUpdateDto, ): Promise { @@ -103,9 +105,10 @@ export class HistoryController { } } - @Delete(':note') + @Delete(':noteIdOrAlias') + @UseInterceptors(GetNoteInterceptor) async deleteHistoryEntry( - @Param('note', GetNotePipe) note: Note, + @RequestNote() note: Note, @RequestUser() user: User, ): Promise { try { diff --git a/src/api/private/notes/notes.controller.ts b/src/api/private/notes/notes.controller.ts index 4d38ff561..527f32bbe 100644 --- a/src/api/private/notes/notes.controller.ts +++ b/src/api/private/notes/notes.controller.ts @@ -14,6 +14,7 @@ import { Param, Post, UseGuards, + UseInterceptors, } from '@nestjs/common'; import { @@ -37,9 +38,10 @@ import { RevisionDto } from '../../../revisions/revision.dto'; import { RevisionsService } from '../../../revisions/revisions.service'; import { User } from '../../../users/user.entity'; import { UsersService } from '../../../users/users.service'; -import { GetNotePipe } from '../../utils/get-note.pipe'; +import { GetNoteInterceptor } from '../../utils/get-note.interceptor'; import { MarkdownBody } from '../../utils/markdownbody-decorator'; import { PermissionsGuard } from '../../utils/permissions.guard'; +import { RequestNote } from '../../utils/request-note.decorator'; import { RequestUser } from '../../utils/request-user.decorator'; @UseGuards(SessionGuard) @@ -58,10 +60,11 @@ export class NotesController { @Get(':noteIdOrAlias') @Permissions(Permission.READ) + @UseInterceptors(GetNoteInterceptor) @UseGuards(PermissionsGuard) async getNote( @RequestUser() user: User, - @Param('noteIdOrAlias', GetNotePipe) note: Note, + @RequestNote() note: Note, ): Promise { await this.historyService.updateHistoryEntryTimestamp(note, user); return await this.noteService.toNoteDto(note); @@ -69,10 +72,9 @@ export class NotesController { @Get(':noteIdOrAlias/media') @Permissions(Permission.READ) + @UseInterceptors(GetNoteInterceptor) @UseGuards(PermissionsGuard) - async getNotesMedia( - @Param('noteIdOrAlias', GetNotePipe) note: Note, - ): Promise { + async getNotesMedia(@RequestNote() note: Note): Promise { const media = await this.mediaService.listUploadsByNote(note); return media.map((media) => this.mediaService.toMediaUploadDto(media)); } @@ -119,10 +121,11 @@ export class NotesController { @Delete(':noteIdOrAlias') @HttpCode(204) @Permissions(Permission.OWNER) + @UseInterceptors(GetNoteInterceptor) @UseGuards(PermissionsGuard) async deleteNote( @RequestUser() user: User, - @Param('noteIdOrAlias', GetNotePipe) note: Note, + @RequestNote() note: Note, @Body() noteMediaDeletionDto: NoteMediaDeletionDto, ): Promise { const mediaUploads = await this.mediaService.listUploadsByNote(note); @@ -141,10 +144,11 @@ export class NotesController { @Get(':noteIdOrAlias/revisions') @Permissions(Permission.READ) + @UseInterceptors(GetNoteInterceptor) @UseGuards(PermissionsGuard) async getNoteRevisions( @RequestUser() user: User, - @Param('noteIdOrAlias', GetNotePipe) note: Note, + @RequestNote() note: Note, ): Promise { const revisions = await this.revisionsService.getAllRevisions(note); return await Promise.all( @@ -157,10 +161,11 @@ export class NotesController { @Delete(':noteIdOrAlias/revisions') @HttpCode(204) @Permissions(Permission.READ) + @UseInterceptors(GetNoteInterceptor) @UseGuards(PermissionsGuard) async purgeNoteRevisions( @RequestUser() user: User, - @Param('noteIdOrAlias', GetNotePipe) note: Note, + @RequestNote() note: Note, ): Promise { this.logger.debug( 'Purging history of note: ' + note.id, @@ -176,10 +181,11 @@ export class NotesController { @Get(':noteIdOrAlias/revisions/:revisionId') @Permissions(Permission.READ) + @UseInterceptors(GetNoteInterceptor) @UseGuards(PermissionsGuard) async getNoteRevision( @RequestUser() user: User, - @Param('noteIdOrAlias', GetNotePipe) note: Note, + @RequestNote() note: Note, @Param('revisionId') revisionId: number, ): Promise { try { diff --git a/src/api/public/me/me.controller.ts b/src/api/public/me/me.controller.ts index a64e412dd..059e681c7 100644 --- a/src/api/public/me/me.controller.ts +++ b/src/api/public/me/me.controller.ts @@ -10,9 +10,9 @@ import { Get, HttpCode, NotFoundException, - Param, Put, UseGuards, + UseInterceptors, } from '@nestjs/common'; import { ApiNoContentResponse, @@ -42,7 +42,8 @@ import { successfullyDeletedDescription, unauthorizedDescription, } from '../../utils/descriptions'; -import { GetNotePipe } from '../../utils/get-note.pipe'; +import { GetNoteInterceptor } from '../../utils/get-note.interceptor'; +import { RequestNote } from '../../utils/request-note.decorator'; import { RequestUser } from '../../utils/request-user.decorator'; @ApiTags('me') @@ -85,8 +86,9 @@ export class MeController { ); } + @UseInterceptors(GetNoteInterceptor) @UseGuards(TokenAuthGuard) - @Get('history/:note') + @Get('history/:noteIdOrAlias') @ApiOkResponse({ description: 'The history entry of the user which points to the note', type: HistoryEntryDto, @@ -95,7 +97,7 @@ export class MeController { @ApiNotFoundResponse({ description: notFoundDescription }) async getHistoryEntry( @RequestUser() user: User, - @Param('note', GetNotePipe) note: Note, + @RequestNote() note: Note, ): Promise { try { const foundEntry = await this.historyService.getEntryByNote(note, user); @@ -108,8 +110,9 @@ export class MeController { } } + @UseInterceptors(GetNoteInterceptor) @UseGuards(TokenAuthGuard) - @Put('history/:note') + @Put('history/:noteIdOrAlias') @ApiOkResponse({ description: 'The updated history entry', type: HistoryEntryDto, @@ -118,7 +121,7 @@ export class MeController { @ApiNotFoundResponse({ description: notFoundDescription }) async updateHistoryEntry( @RequestUser() user: User, - @Param('note', GetNotePipe) note: Note, + @RequestNote() note: Note, @Body() entryUpdateDto: HistoryEntryUpdateDto, ): Promise { // ToDo: Check if user is allowed to pin this history entry @@ -138,15 +141,16 @@ export class MeController { } } + @UseInterceptors(GetNoteInterceptor) @UseGuards(TokenAuthGuard) - @Delete('history/:note') + @Delete('history/:noteIdOrAlias') @HttpCode(204) @ApiNoContentResponse({ description: successfullyDeletedDescription }) @ApiUnauthorizedResponse({ description: unauthorizedDescription }) @ApiNotFoundResponse({ description: notFoundDescription }) async deleteHistoryEntry( @RequestUser() user: User, - @Param('note', GetNotePipe) note: Note, + @RequestNote() note: Note, ): Promise { // ToDo: Check if user is allowed to delete note try { diff --git a/src/api/public/notes/notes.controller.ts b/src/api/public/notes/notes.controller.ts index fc4ccdf63..d1a1b4649 100644 --- a/src/api/public/notes/notes.controller.ts +++ b/src/api/public/notes/notes.controller.ts @@ -16,6 +16,7 @@ import { Post, Put, UseGuards, + UseInterceptors, } from '@nestjs/common'; import { ApiCreatedResponse, @@ -59,9 +60,10 @@ import { unauthorizedDescription, } from '../../utils/descriptions'; import { FullApi } from '../../utils/fullapi-decorator'; -import { GetNotePipe } from '../../utils/get-note.pipe'; +import { GetNoteInterceptor } from '../../utils/get-note.interceptor'; import { MarkdownBody } from '../../utils/markdownbody-decorator'; import { PermissionsGuard } from '../../utils/permissions.guard'; +import { RequestNote } from '../../utils/request-note.decorator'; import { RequestUser } from '../../utils/request-user.decorator'; @ApiTags('notes') @@ -94,6 +96,7 @@ export class NotesController { ); } + @UseInterceptors(GetNoteInterceptor) @Permissions(Permission.READ) @UseGuards(TokenAuthGuard, PermissionsGuard) @Get(':noteIdOrAlias') @@ -104,7 +107,7 @@ export class NotesController { @FullApi async getNote( @RequestUser() user: User, - @Param('noteIdOrAlias', GetNotePipe) note: Note, + @RequestNote() note: Note, ): Promise { await this.historyService.updateHistoryEntryTimestamp(note, user); return await this.noteService.toNoteDto(note); @@ -141,6 +144,7 @@ export class NotesController { } } + @UseInterceptors(GetNoteInterceptor) @Permissions(Permission.OWNER) @UseGuards(TokenAuthGuard, PermissionsGuard) @Delete(':noteIdOrAlias') @@ -149,7 +153,7 @@ export class NotesController { @FullApi async deleteNote( @RequestUser() user: User, - @Param('noteIdOrAlias', GetNotePipe) note: Note, + @RequestNote() note: Note, @Body() noteMediaDeletionDto: NoteMediaDeletionDto, ): Promise { const mediaUploads = await this.mediaService.listUploadsByNote(note); @@ -166,6 +170,7 @@ export class NotesController { return; } + @UseInterceptors(GetNoteInterceptor) @Permissions(Permission.WRITE) @UseGuards(TokenAuthGuard, PermissionsGuard) @Put(':noteIdOrAlias') @@ -176,7 +181,7 @@ export class NotesController { @FullApi async updateNote( @RequestUser() user: User, - @Param('noteIdOrAlias', GetNotePipe) note: Note, + @RequestNote() note: Note, @MarkdownBody() text: string, ): Promise { this.logger.debug('Got raw markdown:\n' + text, 'updateNote'); @@ -185,6 +190,7 @@ export class NotesController { ); } + @UseInterceptors(GetNoteInterceptor) @Permissions(Permission.READ) @UseGuards(TokenAuthGuard, PermissionsGuard) @Get(':noteIdOrAlias/content') @@ -196,11 +202,12 @@ export class NotesController { @Header('content-type', 'text/markdown') async getNoteContent( @RequestUser() user: User, - @Param('noteIdOrAlias', GetNotePipe) note: Note, + @RequestNote() note: Note, ): Promise { return await this.noteService.getNoteContent(note); } + @UseInterceptors(GetNoteInterceptor) @Permissions(Permission.READ) @UseGuards(TokenAuthGuard, PermissionsGuard) @Get(':noteIdOrAlias/metadata') @@ -211,11 +218,12 @@ export class NotesController { @FullApi async getNoteMetadata( @RequestUser() user: User, - @Param('noteIdOrAlias', GetNotePipe) note: Note, + @RequestNote() note: Note, ): Promise { return await this.noteService.toNoteMetadataDto(note); } + @UseInterceptors(GetNoteInterceptor) @Permissions(Permission.OWNER) @UseGuards(TokenAuthGuard, PermissionsGuard) @Put(':noteIdOrAlias/metadata/permissions') @@ -226,7 +234,7 @@ export class NotesController { @FullApi async updateNotePermissions( @RequestUser() user: User, - @Param('noteIdOrAlias', GetNotePipe) note: Note, + @RequestNote() note: Note, @Body() updateDto: NotePermissionsUpdateDto, ): Promise { return this.noteService.toNotePermissionsDto( @@ -234,6 +242,7 @@ export class NotesController { ); } + @UseInterceptors(GetNoteInterceptor) @Permissions(Permission.READ) @UseGuards(TokenAuthGuard, PermissionsGuard) @Get(':noteIdOrAlias/revisions') @@ -245,7 +254,7 @@ export class NotesController { @FullApi async getNoteRevisions( @RequestUser() user: User, - @Param('noteIdOrAlias', GetNotePipe) note: Note, + @RequestNote() note: Note, ): Promise { const revisions = await this.revisionsService.getAllRevisions(note); return await Promise.all( @@ -255,6 +264,7 @@ export class NotesController { ); } + @UseInterceptors(GetNoteInterceptor) @Permissions(Permission.READ) @UseGuards(TokenAuthGuard, PermissionsGuard) @Get(':noteIdOrAlias/revisions/:revisionId') @@ -265,7 +275,7 @@ export class NotesController { @FullApi async getNoteRevision( @RequestUser() user: User, - @Param('noteIdOrAlias', GetNotePipe) note: Note, + @RequestNote() note: Note, @Param('revisionId') revisionId: number, ): Promise { try { @@ -280,6 +290,7 @@ export class NotesController { } } + @UseInterceptors(GetNoteInterceptor) @Permissions(Permission.READ) @UseGuards(TokenAuthGuard, PermissionsGuard) @Get(':noteIdOrAlias/media') @@ -291,7 +302,7 @@ export class NotesController { @ApiUnauthorizedResponse({ description: unauthorizedDescription }) async getNotesMedia( @RequestUser() user: User, - @Param('noteIdOrAlias', GetNotePipe) note: Note, + @RequestNote() note: Note, ): Promise { const media = await this.mediaService.listUploadsByNote(note); return media.map((media) => this.mediaService.toMediaUploadDto(media));