Search code examples
nestjstypeorm

How to simplify Nest.js controller implementation with JWT support


I'm creating controller which is protected by JWTAuthGuard. Is there any way to make the controller smaller? (decorators, interceptors etc).

My current code:

import {User} from "../../user/entities/user.entity";

interface UserJwtPayload {
    user: {
        id: number,
        email: string
    },
}

class CreateDeviceLocationDto {
    lat: number
    long: number
    description: string
    user: User
}


@UseGuards(JwtAuthGuard)
@Controller('device-locations')
export class DeviceLocationsController {
    constructor(private readonly userService: UserService, private readonly deviceLocationsService: DeviceLocationsService) {
    }

    @Post()
    async create(@Body() createDeviceLocationDto: CreateDeviceLocationDto, @Req() request: Request & UserJwtPayload) {
        const user: User = await this.userService.findOne(request.user.id);

        createDeviceLocationDto.user = user;

        return this.deviceLocationsService.create(createDeviceLocationDto);
    }
}

@Injectable()
export class DeviceLocationsService {
    constructor(
        @InjectRepository(DeviceLocation)
        private readonly devicesLocationRepository: Repository<DeviceLocation>
    ) {
    }

    create(createDeviceLocationDto: CreateDeviceLocationDto) {
        const location = this.devicesLocationRepository.create(createDeviceLocationDto);
        return this.devicesLocationRepository.save(location);
    }
}
  1. It is annoying to add @Req() request: Request & UserJwtPayload in every method.
  2. Is there any way to save model to database (I user TypeORM) without loading the user every time? (const user: User = await this.userService.findOne(request.user.id);)? I need to pass user id only which is already available from JWT token.

Thanks!

The above code works. I just want to simplify it, if possible.


Solution

  • Sooo... (I might have skipped some imports for brevity, hope that's fine).

    First of all:

    Inject the request to the controller constructor - to make it globally available to controller methods, without using parameter decorators:

    import { Controller, Get, Inject } from '@nestjs/common';
    import { REQUEST } from '@nestjs/core';
    import type { Request } from 'express';
    
    
    @UseGuards(JwtAuthGuard)
    @Controller()
    export class DeviceLocationsController {
      constructor(@Inject(REQUEST) private request: Request) {}
    }
    

    More on that: https://docs.nestjs.com/fundamentals/injection-scopes#request-provider

    Second of all - which version of NestJS do you use?

    Afaik, in Nest 10, express.Request has user definition out-of-the box. Just remember to import this type from express.

    If you don't have user there, define it in a d.ts file (eg src/request.d.ts):

    export {}
    
    declare global {
      namespace Express {
        export interface Request {
          user?: IUserInRequest;
        }
      }
    }
    

    Third of all:

    If we talk about simplicity and cleaner code, this part:

    const user: User = await this.userService.findOne(request.user.id);
    createDeviceLocationDto.user = user;
    return this.deviceLocationsService.create(createDeviceLocationDto);
    

    can be easily changed to:

    return this.deviceLocationService.create(createDeviceLocationDto, user);
    

    where deviceLocationService has a binding for service that can resolve user. But! Does it really need to have full user, or maybe just user's identifier?

    Remember that in order to get a JWT token, user must be found in the database, so whenever JWT token is valid, it means the user was allowed to receive that token, and there's no need to refetch their data. That's the whole idea of JWT tokens - to limit number of DB calls to absolute minimum.

    If other user properties beside default stored in request are needed, then just make them available in the request.user by updating canActivate method in JwtAuthGuard to something like:

    const userInPayload = await this.jwtService.verifyAsync(token, {
        secret: 'wlazlkoteknaplotek',
    });
    
    const userData  = await this.userService.findOneOrThrow(userInPayload);
    
    // id: 123, language: is-IS, etc.
    
    request.user = payload; // to make language available in request.user
    

    If you'll still hit the typing issues, extend Request in d.ts as specified above :)


    **Extra comments :) **

    1. Don't update your DTO in controller. Instead of that, pass DTO and other stuff to service separately. This will make the code (1) easier to debug, (2) better typed - you don't need to make separate types for DTO without user that is used by controller method, and DTO with user that is used by service.
    2. Speaking of that, you have a type for DTO that includes user: User. This is wrong, because there is no User in the request body, so create(@Body() createDeviceLocationDto: CreateDeviceLocationDto) comes with a type-unsafety problem.
    3. About storing user to database - there is absolutely no reason to pass full user info to the user repository. No matter if you use relational, or NoSQL database, you need user ID only. So that async call to database to find user only to pass the full user to another repository, where it will either way probably be reverted to user ID, it's an overkill for resources, and for the code organization :) That's all you need:
    const { user } = this.request;
    return this.deviceService.create(dto, user);