From b480adc8074c6743cf467d7afbd8f8c05892c92a Mon Sep 17 00:00:00 2001 From: David Mehren Date: Sat, 28 Aug 2021 19:03:15 +0200 Subject: [PATCH] Public API: Introduce RequestUser decorator This introduces the `RequestUser` decorator to extract the `User` from a request. It reduces code duplication across the public API and allows us to drop the override of the `Request` type from express. Signed-off-by: David Mehren --- src/api/public/me/me.controller.ts | 63 ++++----------- src/api/public/media/media.controller.ts | 20 ++--- src/api/public/notes/notes.controller.ts | 99 +++++++----------------- src/api/utils/request-user.decorator.ts | 32 ++++++++ types/express/index.d.ts | 13 ---- 5 files changed, 81 insertions(+), 146 deletions(-) create mode 100644 src/api/utils/request-user.decorator.ts delete mode 100644 types/express/index.d.ts diff --git a/src/api/public/me/me.controller.ts b/src/api/public/me/me.controller.ts index 9fcb1e36f..3c9770a8a 100644 --- a/src/api/public/me/me.controller.ts +++ b/src/api/public/me/me.controller.ts @@ -9,11 +9,9 @@ import { Delete, Get, HttpCode, - InternalServerErrorException, NotFoundException, Param, Put, - Req, UseGuards, } from '@nestjs/common'; import { @@ -24,7 +22,6 @@ import { ApiTags, ApiUnauthorizedResponse, } from '@nestjs/swagger'; -import { Request } from 'express'; import { TokenAuthGuard } from '../../../auth/token-auth.guard'; import { NotInDBError } from '../../../errors/errors'; @@ -37,12 +34,14 @@ import { MediaService } from '../../../media/media.service'; import { NoteMetadataDto } from '../../../notes/note-metadata.dto'; import { NotesService } from '../../../notes/notes.service'; import { UserInfoDto } from '../../../users/user-info.dto'; +import { User } from '../../../users/user.entity'; import { UsersService } from '../../../users/users.service'; import { notFoundDescription, successfullyDeletedDescription, unauthorizedDescription, } from '../../utils/descriptions'; +import { RequestUser } from '../../utils/request-user.decorator'; @ApiTags('me') @ApiSecurity('token') @@ -65,14 +64,8 @@ export class MeController { type: UserInfoDto, }) @ApiUnauthorizedResponse({ description: unauthorizedDescription }) - async getMe(@Req() req: Request): Promise { - if (!req.user) { - // We should never reach this, as the TokenAuthGuard handles missing user info - throw new InternalServerErrorException('Request did not specify user'); - } - return this.usersService.toUserDto( - await this.usersService.getUserByUsername(req.user.userName), - ); + getMe(@RequestUser() user: User): UserInfoDto { + return this.usersService.toUserDto(user); } @UseGuards(TokenAuthGuard) @@ -83,12 +76,8 @@ export class MeController { type: HistoryEntryDto, }) @ApiUnauthorizedResponse({ description: unauthorizedDescription }) - async getUserHistory(@Req() req: Request): Promise { - if (!req.user) { - // We should never reach this, as the TokenAuthGuard handles missing user info - throw new InternalServerErrorException('Request did not specify user'); - } - const foundEntries = await this.historyService.getEntriesByUser(req.user); + async getUserHistory(@RequestUser() user: User): Promise { + const foundEntries = await this.historyService.getEntriesByUser(user); return await Promise.all( foundEntries.map((entry) => this.historyService.toHistoryEntryDto(entry)), ); @@ -103,17 +92,13 @@ export class MeController { @ApiUnauthorizedResponse({ description: unauthorizedDescription }) @ApiNotFoundResponse({ description: notFoundDescription }) async getHistoryEntry( - @Req() req: Request, + @RequestUser() user: User, @Param('note') note: string, ): Promise { - if (!req.user) { - // We should never reach this, as the TokenAuthGuard handles missing user info - throw new InternalServerErrorException('Request did not specify user'); - } try { const foundEntry = await this.historyService.getEntryByNoteIdOrAlias( note, - req.user, + user, ); return this.historyService.toHistoryEntryDto(foundEntry); } catch (e) { @@ -133,20 +118,16 @@ export class MeController { @ApiUnauthorizedResponse({ description: unauthorizedDescription }) @ApiNotFoundResponse({ description: notFoundDescription }) async updateHistoryEntry( - @Req() req: Request, + @RequestUser() user: User, @Param('note') note: string, @Body() entryUpdateDto: HistoryEntryUpdateDto, ): Promise { - if (!req.user) { - // We should never reach this, as the TokenAuthGuard handles missing user info - throw new InternalServerErrorException('Request did not specify user'); - } // ToDo: Check if user is allowed to pin this history entry try { return this.historyService.toHistoryEntryDto( await this.historyService.updateHistoryEntry( note, - req.user, + user, entryUpdateDto, ), ); @@ -165,16 +146,12 @@ export class MeController { @ApiUnauthorizedResponse({ description: unauthorizedDescription }) @ApiNotFoundResponse({ description: notFoundDescription }) async deleteHistoryEntry( - @Req() req: Request, + @RequestUser() user: User, @Param('note') note: string, ): Promise { - if (!req.user) { - // We should never reach this, as the TokenAuthGuard handles missing user info - throw new InternalServerErrorException('Request did not specify user'); - } // ToDo: Check if user is allowed to delete note try { - await this.historyService.deleteHistoryEntry(note, req.user); + await this.historyService.deleteHistoryEntry(note, user); } catch (e) { if (e instanceof NotInDBError) { throw new NotFoundException(e.message); @@ -191,12 +168,8 @@ export class MeController { type: NoteMetadataDto, }) @ApiUnauthorizedResponse({ description: unauthorizedDescription }) - async getMyNotes(@Req() req: Request): Promise { - if (!req.user) { - // We should never reach this, as the TokenAuthGuard handles missing user info - throw new InternalServerErrorException('Request did not specify user'); - } - const notes = this.notesService.getUserNotes(req.user); + async getMyNotes(@RequestUser() user: User): Promise { + const notes = this.notesService.getUserNotes(user); return await Promise.all( (await notes).map((note) => this.notesService.toNoteMetadataDto(note)), ); @@ -210,12 +183,8 @@ export class MeController { type: MediaUploadDto, }) @ApiUnauthorizedResponse({ description: unauthorizedDescription }) - async getMyMedia(@Req() req: Request): Promise { - if (!req.user) { - // We should never reach this, as the TokenAuthGuard handles missing user info - throw new InternalServerErrorException('Request did not specify user'); - } - const media = await this.mediaService.listUploadsByUser(req.user); + async getMyMedia(@RequestUser() user: User): Promise { + const media = await this.mediaService.listUploadsByUser(user); return media.map((media) => this.mediaService.toMediaUploadDto(media)); } } diff --git a/src/api/public/media/media.controller.ts b/src/api/public/media/media.controller.ts index fdaf7fe57..e9d5545c6 100644 --- a/src/api/public/media/media.controller.ts +++ b/src/api/public/media/media.controller.ts @@ -13,7 +13,6 @@ import { NotFoundException, Param, Post, - Req, UnauthorizedException, UploadedFile, UseGuards, @@ -31,7 +30,6 @@ import { ApiTags, ApiUnauthorizedResponse, } from '@nestjs/swagger'; -import { Request } from 'express'; import { TokenAuthGuard } from '../../../auth/token-auth.guard'; import { @@ -44,12 +42,14 @@ 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 { User } from '../../../users/user.entity'; import { forbiddenDescription, successfullyDeletedDescription, unauthorizedDescription, } from '../../utils/descriptions'; import { FullApi } from '../../utils/fullapi-decorator'; +import { RequestUser } from '../../utils/request-user.decorator'; @ApiTags('media') @ApiSecurity('token') @@ -89,15 +89,11 @@ export class MediaController { @UseInterceptors(FileInterceptor('file')) @HttpCode(201) async uploadMedia( - @Req() req: Request, + @RequestUser() user: User, @UploadedFile() file: MulterFile, @Headers('HedgeDoc-Note') noteId: string, ): Promise { - if (!req.user) { - // We should never reach this, as the TokenAuthGuard handles missing user info - throw new InternalServerErrorException('Request did not specify user'); - } - const username = req.user.userName; + const username = user.userName; this.logger.debug( `Recieved filename '${file.originalname}' for note '${noteId}' from user '${username}'`, 'uploadMedia', @@ -128,14 +124,10 @@ export class MediaController { @ApiNoContentResponse({ description: successfullyDeletedDescription }) @FullApi async deleteMedia( - @Req() req: Request, + @RequestUser() user: User, @Param('filename') filename: string, ): Promise { - if (!req.user) { - // We should never reach this, as the TokenAuthGuard handles missing user info - throw new InternalServerErrorException('Request did not specify user'); - } - const username = req.user.userName; + const username = user.userName; try { this.logger.debug( `Deleting '${filename}' for user '${username}'`, diff --git a/src/api/public/notes/notes.controller.ts b/src/api/public/notes/notes.controller.ts index 331acbe7b..d683549fd 100644 --- a/src/api/public/notes/notes.controller.ts +++ b/src/api/public/notes/notes.controller.ts @@ -11,12 +11,10 @@ import { Get, Header, HttpCode, - InternalServerErrorException, NotFoundException, Param, Post, Put, - Req, UnauthorizedException, UseGuards, } from '@nestjs/common'; @@ -30,7 +28,6 @@ import { ApiTags, ApiUnauthorizedResponse, } from '@nestjs/swagger'; -import { Request } from 'express'; import { TokenAuthGuard } from '../../../auth/token-auth.guard'; import { @@ -56,6 +53,7 @@ import { PermissionsService } from '../../../permissions/permissions.service'; import { RevisionMetadataDto } from '../../../revisions/revision-metadata.dto'; import { RevisionDto } from '../../../revisions/revision.dto'; import { RevisionsService } from '../../../revisions/revisions.service'; +import { User } from '../../../users/user.entity'; import { forbiddenDescription, successfullyDeletedDescription, @@ -63,6 +61,7 @@ import { } from '../../utils/descriptions'; import { FullApi } from '../../utils/fullapi-decorator'; import { MarkdownBody } from '../../utils/markdownbody-decorator'; +import { RequestUser } from '../../utils/request-user.decorator'; @ApiTags('notes') @ApiSecurity('token') @@ -85,20 +84,16 @@ export class NotesController { @ApiUnauthorizedResponse({ description: unauthorizedDescription }) @ApiForbiddenResponse({ description: forbiddenDescription }) async createNote( - @Req() req: Request, + @RequestUser() user: User, @MarkdownBody() text: string, ): Promise { - if (!req.user) { - // We should never reach this, as the TokenAuthGuard handles missing user info - throw new InternalServerErrorException('Request did not specify user'); - } // ToDo: provide user for createNoteDto - if (!this.permissionsService.mayCreate(req.user)) { + if (!this.permissionsService.mayCreate(user)) { throw new UnauthorizedException('Creating note denied!'); } this.logger.debug('Got raw markdown:\n' + text); return await this.noteService.toNoteDto( - await this.noteService.createNote(text, undefined, req.user), + await this.noteService.createNote(text, undefined, user), ); } @@ -110,13 +105,9 @@ export class NotesController { }) @FullApi async getNote( - @Req() req: Request, + @RequestUser() user: User, @Param('noteIdOrAlias') noteIdOrAlias: string, ): Promise { - if (!req.user) { - // We should never reach this, as the TokenAuthGuard handles missing user info - throw new InternalServerErrorException('Request did not specify user'); - } let note: Note; try { note = await this.noteService.getNoteByIdOrAlias(noteIdOrAlias); @@ -129,10 +120,10 @@ export class NotesController { } throw e; } - if (!this.permissionsService.mayRead(req.user, note)) { + if (!this.permissionsService.mayRead(user, note)) { throw new UnauthorizedException('Reading note denied!'); } - await this.historyService.createOrUpdateHistoryEntry(note, req.user); + await this.historyService.createOrUpdateHistoryEntry(note, user); return await this.noteService.toNoteDto(note); } @@ -146,21 +137,17 @@ export class NotesController { @ApiUnauthorizedResponse({ description: unauthorizedDescription }) @ApiForbiddenResponse({ description: forbiddenDescription }) async createNamedNote( - @Req() req: Request, + @RequestUser() user: User, @Param('noteAlias') noteAlias: string, @MarkdownBody() text: string, ): Promise { - if (!req.user) { - // We should never reach this, as the TokenAuthGuard handles missing user info - throw new InternalServerErrorException('Request did not specify user'); - } - if (!this.permissionsService.mayCreate(req.user)) { + if (!this.permissionsService.mayCreate(user)) { throw new UnauthorizedException('Creating note denied!'); } this.logger.debug('Got raw markdown:\n' + text, 'createNamedNote'); try { return await this.noteService.toNoteDto( - await this.noteService.createNote(text, noteAlias, req.user), + await this.noteService.createNote(text, noteAlias, user), ); } catch (e) { if (e instanceof AlreadyInDBError) { @@ -179,17 +166,13 @@ export class NotesController { @ApiNoContentResponse({ description: successfullyDeletedDescription }) @FullApi async deleteNote( - @Req() req: Request, + @RequestUser() user: User, @Param('noteIdOrAlias') noteIdOrAlias: string, @Body() noteMediaDeletionDto: NoteMediaDeletionDto, ): Promise { - if (!req.user) { - // We should never reach this, as the TokenAuthGuard handles missing user info - throw new InternalServerErrorException('Request did not specify user'); - } try { const note = await this.noteService.getNoteByIdOrAlias(noteIdOrAlias); - if (!this.permissionsService.isOwner(req.user, note)) { + if (!this.permissionsService.isOwner(user, note)) { throw new UnauthorizedException('Deleting note denied!'); } const mediaUploads = await this.mediaService.listUploadsByNote(note); @@ -223,17 +206,13 @@ export class NotesController { }) @FullApi async updateNote( - @Req() req: Request, + @RequestUser() user: User, @Param('noteIdOrAlias') noteIdOrAlias: string, @MarkdownBody() text: string, ): Promise { - if (!req.user) { - // We should never reach this, as the TokenAuthGuard handles missing user info - throw new InternalServerErrorException('Request did not specify user'); - } try { const note = await this.noteService.getNoteByIdOrAlias(noteIdOrAlias); - if (!this.permissionsService.mayWrite(req.user, note)) { + if (!this.permissionsService.mayWrite(user, note)) { throw new UnauthorizedException('Updating note denied!'); } this.logger.debug('Got raw markdown:\n' + text, 'updateNote'); @@ -260,16 +239,12 @@ export class NotesController { @FullApi @Header('content-type', 'text/markdown') async getNoteContent( - @Req() req: Request, + @RequestUser() user: User, @Param('noteIdOrAlias') noteIdOrAlias: string, ): Promise { - if (!req.user) { - // We should never reach this, as the TokenAuthGuard handles missing user info - throw new InternalServerErrorException('Request did not specify user'); - } try { const note = await this.noteService.getNoteByIdOrAlias(noteIdOrAlias); - if (!this.permissionsService.mayRead(req.user, note)) { + if (!this.permissionsService.mayRead(user, note)) { throw new UnauthorizedException('Reading note denied!'); } return await this.noteService.getNoteContent(note); @@ -292,16 +267,12 @@ export class NotesController { }) @FullApi async getNoteMetadata( - @Req() req: Request, + @RequestUser() user: User, @Param('noteIdOrAlias') noteIdOrAlias: string, ): Promise { - if (!req.user) { - // We should never reach this, as the TokenAuthGuard handles missing user info - throw new InternalServerErrorException('Request did not specify user'); - } try { const note = await this.noteService.getNoteByIdOrAlias(noteIdOrAlias); - if (!this.permissionsService.mayRead(req.user, note)) { + if (!this.permissionsService.mayRead(user, note)) { throw new UnauthorizedException('Reading note denied!'); } return await this.noteService.toNoteMetadataDto(note); @@ -327,17 +298,13 @@ export class NotesController { }) @FullApi async updateNotePermissions( - @Req() req: Request, + @RequestUser() user: User, @Param('noteIdOrAlias') noteIdOrAlias: string, @Body() updateDto: NotePermissionsUpdateDto, ): Promise { - if (!req.user) { - // We should never reach this, as the TokenAuthGuard handles missing user info - throw new InternalServerErrorException('Request did not specify user'); - } try { const note = await this.noteService.getNoteByIdOrAlias(noteIdOrAlias); - if (!this.permissionsService.isOwner(req.user, note)) { + if (!this.permissionsService.isOwner(user, note)) { throw new UnauthorizedException('Updating note denied!'); } return this.noteService.toNotePermissionsDto( @@ -363,16 +330,12 @@ export class NotesController { }) @FullApi async getNoteRevisions( - @Req() req: Request, + @RequestUser() user: User, @Param('noteIdOrAlias') noteIdOrAlias: string, ): Promise { - if (!req.user) { - // We should never reach this, as the TokenAuthGuard handles missing user info - throw new InternalServerErrorException('Request did not specify user'); - } try { const note = await this.noteService.getNoteByIdOrAlias(noteIdOrAlias); - if (!this.permissionsService.mayRead(req.user, note)) { + if (!this.permissionsService.mayRead(user, note)) { throw new UnauthorizedException('Reading note denied!'); } const revisions = await this.revisionsService.getAllRevisions(note); @@ -400,17 +363,13 @@ export class NotesController { }) @FullApi async getNoteRevision( - @Req() req: Request, + @RequestUser() user: User, @Param('noteIdOrAlias') noteIdOrAlias: string, @Param('revisionId') revisionId: number, ): Promise { - if (!req.user) { - // We should never reach this, as the TokenAuthGuard handles missing user info - throw new InternalServerErrorException('Request did not specify user'); - } try { const note = await this.noteService.getNoteByIdOrAlias(noteIdOrAlias); - if (!this.permissionsService.mayRead(req.user, note)) { + if (!this.permissionsService.mayRead(user, note)) { throw new UnauthorizedException('Reading note denied!'); } return this.revisionsService.toRevisionDto( @@ -436,16 +395,12 @@ export class NotesController { }) @ApiUnauthorizedResponse({ description: unauthorizedDescription }) async getNotesMedia( - @Req() req: Request, + @RequestUser() user: User, @Param('noteIdOrAlias') noteIdOrAlias: string, ): Promise { - if (!req.user) { - // We should never reach this, as the TokenAuthGuard handles missing user info - throw new InternalServerErrorException('Request did not specify user'); - } try { const note = await this.noteService.getNoteByIdOrAlias(noteIdOrAlias); - if (!this.permissionsService.mayRead(req.user, note)) { + if (!this.permissionsService.mayRead(user, note)) { throw new UnauthorizedException('Reading note denied!'); } const media = await this.mediaService.listUploadsByNote(note); diff --git a/src/api/utils/request-user.decorator.ts b/src/api/utils/request-user.decorator.ts new file mode 100644 index 000000000..02284262e --- /dev/null +++ b/src/api/utils/request-user.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 { User } from '../../users/user.entity'; + +/** + * Extracts the {@link User} object from a request + * + * Will throw an {@link InternalServerErrorException} if no user is present + */ +// eslint-disable-next-line @typescript-eslint/naming-convention +export const RequestUser = createParamDecorator( + (data: unknown, ctx: ExecutionContext) => { + const request: Request & { user: User } = ctx.switchToHttp().getRequest(); + if (!request.user) { + // We should have a user here, otherwise something is wrong + throw new InternalServerErrorException( + 'Request is missing a user object', + ); + } + return request.user; + }, +); diff --git a/types/express/index.d.ts b/types/express/index.d.ts deleted file mode 100644 index 13bbf4862..000000000 --- a/types/express/index.d.ts +++ /dev/null @@ -1,13 +0,0 @@ -/* - * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) - * - * SPDX-License-Identifier: AGPL-3.0-only - */ - -import { User} from '../../src/users/user.entity'; - -declare module 'express' { - export interface Request { - user?: User; - } -}