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 <git@herrmehren.de>

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<UserInfoDto> {
+  getMe(@RequestUser() user: User): UserInfoDto {
-    if (!req.user) {
+    return this.usersService.toUserDto(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),
-    );
   }
 
   @UseGuards(TokenAuthGuard)
@@ -83,12 +76,8 @@ export class MeController {
     type: HistoryEntryDto,
   })
   @ApiUnauthorizedResponse({ description: unauthorizedDescription })
-  async getUserHistory(@Req() req: Request): Promise<HistoryEntryDto[]> {
+  async getUserHistory(@RequestUser() user: User): Promise<HistoryEntryDto[]> {
-    if (!req.user) {
+    const foundEntries = await this.historyService.getEntriesByUser(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);
     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<HistoryEntryDto> {
-    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<HistoryEntryDto> {
-    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<void> {
-    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<NoteMetadataDto[]> {
+  async getMyNotes(@RequestUser() user: User): Promise<NoteMetadataDto[]> {
-    if (!req.user) {
+    const notes = this.notesService.getUserNotes(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);
     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<MediaUploadDto[]> {
+  async getMyMedia(@RequestUser() user: User): Promise<MediaUploadDto[]> {
-    if (!req.user) {
+    const media = await this.mediaService.listUploadsByUser(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);
     return media.map((media) => this.mediaService.toMediaUploadDto(media));
   }
 }

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<MediaUploadUrlDto> {
-    if (!req.user) {
+    const username = user.userName;
-      // 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;
     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<void> {
-    if (!req.user) {
+    const username = user.userName;
-      // 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;
     try {
       this.logger.debug(
         `Deleting '${filename}' for user '${username}'`,

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<NoteDto> {
-    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<NoteDto> {
-    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<NoteDto> {
-    if (!req.user) {
+    if (!this.permissionsService.mayCreate(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)) {
       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<void> {
-    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<NoteDto> {
-    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<string> {
-    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<NoteMetadataDto> {
-    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<NotePermissionsDto> {
-    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<RevisionMetadataDto[]> {
-    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<RevisionDto> {
-    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<MediaUploadDto[]> {
-    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);

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;
+  },
+);

types/express/index.d.ts

@@ -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;
-  }
-}