fix: add CompleteRequest type to have better type checks for HTTP-Request attribute injection.

Signed-off-by: Yannick Bungers <git@innay.de>
Signed-off-by: Tilman Vatteroth <git@tilmanvatteroth.de>
This commit is contained in:
Yannick Bungers 2023-03-25 16:26:52 +01:00 committed by Yannick Bungers
parent 0263c09ce1
commit d369132519
10 changed files with 34 additions and 38 deletions

View file

@ -9,12 +9,11 @@ import {
Injectable, Injectable,
NestInterceptor, NestInterceptor,
} from '@nestjs/common'; } from '@nestjs/common';
import { Request } from 'express';
import { Observable } from 'rxjs'; import { Observable } from 'rxjs';
import { Note } from '../../notes/note.entity'; import { Note } from '../../notes/note.entity';
import { NotesService } from '../../notes/notes.service'; 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 * Saves the note identified by the `noteIdOrAlias` URL parameter
@ -28,9 +27,7 @@ export class GetNoteInterceptor implements NestInterceptor {
context: ExecutionContext, context: ExecutionContext,
next: CallHandler, next: CallHandler,
): Promise<Observable<T>> { ): Promise<Observable<T>> {
const request: Request & { user: User; note: Note } = context const request: CompleteRequest = context.switchToHttp().getRequest();
.switchToHttp()
.getRequest();
const noteIdOrAlias = request.params['noteIdOrAlias']; const noteIdOrAlias = request.params['noteIdOrAlias'];
request.note = await getNote(this.noteService, noteIdOrAlias); request.note = await getNote(this.noteService, noteIdOrAlias);
return next.handle(); return next.handle();

View file

@ -9,11 +9,10 @@ import {
Injectable, Injectable,
NestInterceptor, NestInterceptor,
} from '@nestjs/common'; } from '@nestjs/common';
import { Request } from 'express';
import { Observable } from 'rxjs'; import { Observable } from 'rxjs';
import { Note } from '../../notes/note.entity';
import { NotesService } from '../../notes/notes.service'; import { NotesService } from '../../notes/notes.service';
import { CompleteRequest } from './request.type';
/** /**
* Saves the note identified by the `HedgeDoc-Note` header * Saves the note identified by the `HedgeDoc-Note` header
@ -27,9 +26,7 @@ export class NoteHeaderInterceptor implements NestInterceptor {
context: ExecutionContext, context: ExecutionContext,
next: CallHandler, next: CallHandler,
): Promise<Observable<T>> { ): Promise<Observable<T>> {
const request: Request & { const request: CompleteRequest = context.switchToHttp().getRequest();
note: Note;
} = context.switchToHttp().getRequest();
const noteId: string = request.headers['hedgedoc-note'] as string; const noteId: string = request.headers['hedgedoc-note'] as string;
request.note = await this.noteService.getNoteByIdOrAlias(noteId); request.note = await this.noteService.getNoteByIdOrAlias(noteId);
return next.handle(); return next.handle();

View file

@ -5,14 +5,13 @@
*/ */
import { CanActivate, ExecutionContext, Injectable } from '@nestjs/common'; import { CanActivate, ExecutionContext, Injectable } from '@nestjs/common';
import { Reflector } from '@nestjs/core'; import { Reflector } from '@nestjs/core';
import { Request } from 'express';
import { ConsoleLoggerService } from '../../logger/console-logger.service'; import { ConsoleLoggerService } from '../../logger/console-logger.service';
import { NotesService } from '../../notes/notes.service'; import { NotesService } from '../../notes/notes.service';
import { Permission } from '../../permissions/permissions.enum'; import { Permission } from '../../permissions/permissions.enum';
import { PermissionsService } from '../../permissions/permissions.service'; import { PermissionsService } from '../../permissions/permissions.service';
import { User } from '../../users/user.entity';
import { getNote } from './get-note.interceptor'; 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. * 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; return false;
} }
const request: Request & { user: User } = context const request: CompleteRequest = context.switchToHttp().getRequest();
.switchToHttp() const user = request.user ?? null;
.getRequest();
const user = request.user;
// handle CREATE permissions, as this does not need any note // handle CREATE permissions, as this does not need any note
if (permissions[0] === Permission.CREATE) { if (permissions[0] === Permission.CREATE) {
return this.permissionsService.mayCreate(user); return this.permissionsService.mayCreate(user);

View file

@ -8,9 +8,8 @@ import {
ExecutionContext, ExecutionContext,
InternalServerErrorException, InternalServerErrorException,
} from '@nestjs/common'; } 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 * 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 // eslint-disable-next-line @typescript-eslint/naming-convention
export const RequestNote = createParamDecorator( export const RequestNote = createParamDecorator(
(data: unknown, ctx: ExecutionContext) => { (data: unknown, ctx: ExecutionContext) => {
const request: Request & { note: Note } = ctx.switchToHttp().getRequest(); const request: CompleteRequest = ctx.switchToHttp().getRequest();
if (!request.note) { if (!request.note) {
// We should have a note here, otherwise something is wrong // We should have a note here, otherwise something is wrong
throw new InternalServerErrorException( throw new InternalServerErrorException(

View file

@ -8,9 +8,8 @@ import {
ExecutionContext, ExecutionContext,
UnauthorizedException, UnauthorizedException,
} from '@nestjs/common'; } from '@nestjs/common';
import { Request } from 'express';
import { User } from '../../users/user.entity'; import { CompleteRequest } from './request.type';
type RequestUserParameter = { type RequestUserParameter = {
guestsAllowed: boolean; guestsAllowed: boolean;
@ -29,9 +28,7 @@ export const RequestUser = createParamDecorator(
data: RequestUserParameter = { guestsAllowed: false }, data: RequestUserParameter = { guestsAllowed: false },
ctx: ExecutionContext, ctx: ExecutionContext,
) => { ) => {
const request: Request & { user: User | null } = ctx const request: CompleteRequest = ctx.switchToHttp().getRequest();
.switchToHttp()
.getRequest();
if (!request.user) { if (!request.user) {
if (data.guestsAllowed) { if (data.guestsAllowed) {
return null; return null;

View file

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

View file

@ -8,9 +8,8 @@ import {
ExecutionContext, ExecutionContext,
InternalServerErrorException, InternalServerErrorException,
} from '@nestjs/common'; } 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 * 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 // eslint-disable-next-line @typescript-eslint/naming-convention
export const SessionAuthProvider = createParamDecorator( export const SessionAuthProvider = createParamDecorator(
(data: unknown, ctx: ExecutionContext) => { (data: unknown, ctx: ExecutionContext) => {
const request: Request & { const request: CompleteRequest = ctx.switchToHttp().getRequest();
session: SessionState;
} = ctx.switchToHttp().getRequest();
if (!request.session?.authProvider) { if (!request.session?.authProvider) {
// We should have an auth provider here, otherwise something is wrong // We should have an auth provider here, otherwise something is wrong
throw new InternalServerErrorException( throw new InternalServerErrorException(

View file

@ -4,8 +4,8 @@
* SPDX-License-Identifier: AGPL-3.0-only * SPDX-License-Identifier: AGPL-3.0-only
*/ */
import { ExecutionContext, Injectable } from '@nestjs/common'; import { ExecutionContext, Injectable } from '@nestjs/common';
import { Request } from 'express';
import { CompleteRequest } from '../api/utils/request.type';
import { User } from '../users/user.entity'; import { User } from '../users/user.entity';
import { UsersService } from '../users/users.service'; import { UsersService } from '../users/users.service';
@ -16,7 +16,7 @@ export class MockAuthGuard {
constructor(private usersService: UsersService) {} constructor(private usersService: UsersService) {}
async canActivate(context: ExecutionContext): Promise<boolean> { async canActivate(context: ExecutionContext): Promise<boolean> {
const req: Request = context.switchToHttp().getRequest(); const req: CompleteRequest = context.switchToHttp().getRequest();
if (!this.user) { if (!this.user) {
// this assures that we can create the user 'hardcoded', if we need them before any calls are made or // 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 // create them on the fly when the first call to the api is made

View file

@ -11,12 +11,11 @@ import {
UnauthorizedException, UnauthorizedException,
} from '@nestjs/common'; } from '@nestjs/common';
import { CompleteRequest } from '../api/utils/request.type';
import { GuestAccess } from '../config/guest_access.enum'; import { GuestAccess } from '../config/guest_access.enum';
import noteConfiguration, { NoteConfig } from '../config/note.config'; import noteConfiguration, { NoteConfig } from '../config/note.config';
import { NotInDBError } from '../errors/errors'; import { NotInDBError } from '../errors/errors';
import { ConsoleLoggerService } from '../logger/console-logger.service'; 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'; import { UsersService } from '../users/users.service';
/** /**
@ -39,10 +38,7 @@ export class SessionGuard implements CanActivate {
} }
async canActivate(context: ExecutionContext): Promise<boolean> { async canActivate(context: ExecutionContext): Promise<boolean> {
const request: Request & { const request: CompleteRequest = context.switchToHttp().getRequest();
session?: SessionState;
user?: User;
} = context.switchToHttp().getRequest();
const username = request.session?.username; const username = request.session?.username;
if (!username) { if (!username) {
if (this.noteConfig.guestAccess !== GuestAccess.DENY && request.session) { if (this.noteConfig.guestAccess !== GuestAccess.DENY && request.session) {

View file

@ -79,7 +79,7 @@ export class PermissionsService {
* @return if the user is allowed to create notes * @return if the user is allowed to create notes
*/ */
public mayCreate(user: User | null): boolean { 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<boolean> { async isOwner(user: User | null, note: Note): Promise<boolean> {