From d3691325197a2a410110d3511d9aa003b9162daa Mon Sep 17 00:00:00 2001 From: Yannick Bungers Date: Sat, 25 Mar 2023 16:26:52 +0100 Subject: [PATCH] fix: add CompleteRequest type to have better type checks for HTTP-Request attribute injection. Signed-off-by: Yannick Bungers Signed-off-by: Tilman Vatteroth --- backend/src/api/utils/get-note.interceptor.ts | 7 ++----- backend/src/api/utils/note-header.interceptor.ts | 7 ++----- backend/src/api/utils/permissions.guard.ts | 9 +++------ backend/src/api/utils/request-note.decorator.ts | 5 ++--- backend/src/api/utils/request-user.decorator.ts | 7 ++----- backend/src/api/utils/request.type.ts | 16 ++++++++++++++++ .../api/utils/session-authprovider.decorator.ts | 7 ++----- backend/src/auth/mock-auth.guard.ts | 4 ++-- backend/src/identity/session.guard.ts | 8 ++------ backend/src/permissions/permissions.service.ts | 2 +- 10 files changed, 34 insertions(+), 38 deletions(-) create mode 100644 backend/src/api/utils/request.type.ts diff --git a/backend/src/api/utils/get-note.interceptor.ts b/backend/src/api/utils/get-note.interceptor.ts index 3dd9d8838..776564a8f 100644 --- a/backend/src/api/utils/get-note.interceptor.ts +++ b/backend/src/api/utils/get-note.interceptor.ts @@ -9,12 +9,11 @@ import { Injectable, NestInterceptor, } from '@nestjs/common'; -import { Request } from 'express'; import { Observable } from 'rxjs'; import { Note } from '../../notes/note.entity'; import { NotesService } from '../../notes/notes.service'; -import { User } from '../../users/user.entity'; +import { CompleteRequest } from './request.type'; /** * Saves the note identified by the `noteIdOrAlias` URL parameter @@ -28,9 +27,7 @@ export class GetNoteInterceptor implements NestInterceptor { context: ExecutionContext, next: CallHandler, ): Promise> { - const request: Request & { user: User; note: Note } = context - .switchToHttp() - .getRequest(); + const request: CompleteRequest = context.switchToHttp().getRequest(); const noteIdOrAlias = request.params['noteIdOrAlias']; request.note = await getNote(this.noteService, noteIdOrAlias); return next.handle(); diff --git a/backend/src/api/utils/note-header.interceptor.ts b/backend/src/api/utils/note-header.interceptor.ts index 6fc53d615..3db590622 100644 --- a/backend/src/api/utils/note-header.interceptor.ts +++ b/backend/src/api/utils/note-header.interceptor.ts @@ -9,11 +9,10 @@ import { Injectable, NestInterceptor, } from '@nestjs/common'; -import { Request } from 'express'; import { Observable } from 'rxjs'; -import { Note } from '../../notes/note.entity'; import { NotesService } from '../../notes/notes.service'; +import { CompleteRequest } from './request.type'; /** * Saves the note identified by the `HedgeDoc-Note` header @@ -27,9 +26,7 @@ export class NoteHeaderInterceptor implements NestInterceptor { context: ExecutionContext, next: CallHandler, ): Promise> { - const request: Request & { - note: Note; - } = context.switchToHttp().getRequest(); + const request: CompleteRequest = context.switchToHttp().getRequest(); const noteId: string = request.headers['hedgedoc-note'] as string; request.note = await this.noteService.getNoteByIdOrAlias(noteId); return next.handle(); diff --git a/backend/src/api/utils/permissions.guard.ts b/backend/src/api/utils/permissions.guard.ts index 96116db00..95125e317 100644 --- a/backend/src/api/utils/permissions.guard.ts +++ b/backend/src/api/utils/permissions.guard.ts @@ -5,14 +5,13 @@ */ import { CanActivate, ExecutionContext, Injectable } from '@nestjs/common'; import { Reflector } from '@nestjs/core'; -import { Request } from 'express'; import { ConsoleLoggerService } from '../../logger/console-logger.service'; 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.interceptor'; +import { CompleteRequest } from './request.type'; /** * This guards controller methods from access, if the user has not the appropriate permissions. @@ -41,10 +40,8 @@ export class PermissionsGuard implements CanActivate { ); return false; } - const request: Request & { user: User } = context - .switchToHttp() - .getRequest(); - const user = request.user; + const request: CompleteRequest = context.switchToHttp().getRequest(); + const user = request.user ?? null; // handle CREATE permissions, as this does not need any note if (permissions[0] === Permission.CREATE) { return this.permissionsService.mayCreate(user); diff --git a/backend/src/api/utils/request-note.decorator.ts b/backend/src/api/utils/request-note.decorator.ts index f10370abe..0c1597337 100644 --- a/backend/src/api/utils/request-note.decorator.ts +++ b/backend/src/api/utils/request-note.decorator.ts @@ -8,9 +8,8 @@ import { ExecutionContext, InternalServerErrorException, } from '@nestjs/common'; -import { Request } from 'express'; -import { Note } from '../../notes/note.entity'; +import { CompleteRequest } from './request.type'; /** * Extracts the {@link Note} object from a request @@ -20,7 +19,7 @@ import { Note } from '../../notes/note.entity'; // eslint-disable-next-line @typescript-eslint/naming-convention export const RequestNote = createParamDecorator( (data: unknown, ctx: ExecutionContext) => { - const request: Request & { note: Note } = ctx.switchToHttp().getRequest(); + const request: CompleteRequest = ctx.switchToHttp().getRequest(); if (!request.note) { // We should have a note here, otherwise something is wrong throw new InternalServerErrorException( diff --git a/backend/src/api/utils/request-user.decorator.ts b/backend/src/api/utils/request-user.decorator.ts index 71b96e428..5d53a2de2 100644 --- a/backend/src/api/utils/request-user.decorator.ts +++ b/backend/src/api/utils/request-user.decorator.ts @@ -8,9 +8,8 @@ import { ExecutionContext, UnauthorizedException, } from '@nestjs/common'; -import { Request } from 'express'; -import { User } from '../../users/user.entity'; +import { CompleteRequest } from './request.type'; type RequestUserParameter = { guestsAllowed: boolean; @@ -29,9 +28,7 @@ export const RequestUser = createParamDecorator( data: RequestUserParameter = { guestsAllowed: false }, ctx: ExecutionContext, ) => { - const request: Request & { user: User | null } = ctx - .switchToHttp() - .getRequest(); + const request: CompleteRequest = ctx.switchToHttp().getRequest(); if (!request.user) { if (data.guestsAllowed) { return null; diff --git a/backend/src/api/utils/request.type.ts b/backend/src/api/utils/request.type.ts new file mode 100644 index 000000000..95054d00b --- /dev/null +++ b/backend/src/api/utils/request.type.ts @@ -0,0 +1,16 @@ +/* + * SPDX-FileCopyrightText: 2023 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { Request } from 'express'; + +import { Note } from '../../notes/note.entity'; +import { SessionState } from '../../session/session.service'; +import { User } from '../../users/user.entity'; + +export type CompleteRequest = Request & { + user?: User; + note?: Note; + session?: SessionState; +}; diff --git a/backend/src/api/utils/session-authprovider.decorator.ts b/backend/src/api/utils/session-authprovider.decorator.ts index 19c56a1c6..dc9e65d34 100644 --- a/backend/src/api/utils/session-authprovider.decorator.ts +++ b/backend/src/api/utils/session-authprovider.decorator.ts @@ -8,9 +8,8 @@ import { ExecutionContext, InternalServerErrorException, } from '@nestjs/common'; -import { Request } from 'express'; -import { SessionState } from '../../session/session.service'; +import { CompleteRequest } from './request.type'; /** * Extracts the auth provider identifier from a session inside a request @@ -20,9 +19,7 @@ import { SessionState } from '../../session/session.service'; // eslint-disable-next-line @typescript-eslint/naming-convention export const SessionAuthProvider = createParamDecorator( (data: unknown, ctx: ExecutionContext) => { - const request: Request & { - session: SessionState; - } = ctx.switchToHttp().getRequest(); + const request: CompleteRequest = ctx.switchToHttp().getRequest(); if (!request.session?.authProvider) { // We should have an auth provider here, otherwise something is wrong throw new InternalServerErrorException( diff --git a/backend/src/auth/mock-auth.guard.ts b/backend/src/auth/mock-auth.guard.ts index da19d1767..289ebc5b5 100644 --- a/backend/src/auth/mock-auth.guard.ts +++ b/backend/src/auth/mock-auth.guard.ts @@ -4,8 +4,8 @@ * SPDX-License-Identifier: AGPL-3.0-only */ import { ExecutionContext, Injectable } from '@nestjs/common'; -import { Request } from 'express'; +import { CompleteRequest } from '../api/utils/request.type'; import { User } from '../users/user.entity'; import { UsersService } from '../users/users.service'; @@ -16,7 +16,7 @@ export class MockAuthGuard { constructor(private usersService: UsersService) {} async canActivate(context: ExecutionContext): Promise { - const req: Request = context.switchToHttp().getRequest(); + const req: CompleteRequest = context.switchToHttp().getRequest(); if (!this.user) { // this assures that we can create the user 'hardcoded', if we need them before any calls are made or // create them on the fly when the first call to the api is made diff --git a/backend/src/identity/session.guard.ts b/backend/src/identity/session.guard.ts index 1da0f56f6..91902b822 100644 --- a/backend/src/identity/session.guard.ts +++ b/backend/src/identity/session.guard.ts @@ -11,12 +11,11 @@ import { UnauthorizedException, } from '@nestjs/common'; +import { CompleteRequest } from '../api/utils/request.type'; import { GuestAccess } from '../config/guest_access.enum'; import noteConfiguration, { NoteConfig } from '../config/note.config'; import { NotInDBError } from '../errors/errors'; import { ConsoleLoggerService } from '../logger/console-logger.service'; -import { SessionState } from '../session/session.service'; -import { User } from '../users/user.entity'; import { UsersService } from '../users/users.service'; /** @@ -39,10 +38,7 @@ export class SessionGuard implements CanActivate { } async canActivate(context: ExecutionContext): Promise { - const request: Request & { - session?: SessionState; - user?: User; - } = context.switchToHttp().getRequest(); + const request: CompleteRequest = context.switchToHttp().getRequest(); const username = request.session?.username; if (!username) { if (this.noteConfig.guestAccess !== GuestAccess.DENY && request.session) { diff --git a/backend/src/permissions/permissions.service.ts b/backend/src/permissions/permissions.service.ts index 9e99358ea..6c4b3e34d 100644 --- a/backend/src/permissions/permissions.service.ts +++ b/backend/src/permissions/permissions.service.ts @@ -79,7 +79,7 @@ export class PermissionsService { * @return if the user is allowed to create notes */ public mayCreate(user: User | null): boolean { - return user !== null || this.noteConfig.guestAccess === GuestAccess.CREATE; + return !!user || this.noteConfig.guestAccess === GuestAccess.CREATE; } async isOwner(user: User | null, note: Note): Promise {