nestjs / nest

A progressive Node.js framework for building efficient, scalable, and enterprise-grade server-side applications with TypeScript/JavaScript πŸš€
https://nestjs.com
MIT License
67.83k stars 7.65k forks source link

Reflector.createDecorator is not setting the meta data #13970

Closed abuzain432432 closed 2 months ago

abuzain432432 commented 2 months ago

Is there an existing issue for this?

Current behavior

I am using the Roles Decorator in my controller, but I don't know why this is not working. I have added a log in my code, and I am getting undefined. muuve_nestjs_api-1 | ++++++++++++++++++++++++ muuve_nestjs_api-1 | undefined muuve_nestjs_api-1 | +++++++++++++++++++++

import { RolesEnum } from '../enums/roles.enum';

import { Reflector } from '@nestjs/core';

export const Roles = Reflector.createDecorator<RolesEnum []>();

Controller

  @Roles([RolesEnum.ADMIN])
  @UseGuards(JwtAuthGuard)
  findAll() {
    return this.userService.findAll();
  }

Guard:

import {
  Injectable,
  CanActivate,
  ExecutionContext,
  ForbiddenException,
} from '@nestjs/common';
import { Reflector } from '@nestjs/core';
import { RolesEnum } from '../enums/roles.enum';

@Injectable()
export class RolesGuard implements CanActivate {
  constructor(private reflector: Reflector) {}

  private matchRoles(roles: RolesEnum[], userRole: RolesEnum): boolean {
    return roles.some((role) => role === userRole);
  }

  canActivate(context: ExecutionContext): boolean {
    const roles = this.reflector.getAllAndOverride<undefined | RolesEnum[]>(
      'roles',
      [context.getHandler(), context.getClass()],
    );
    console.log('++++++++++++++++++++++++');
    console.log(roles);
    console.log('+++++++++++++++++++++');
    if (!roles) {
      return true;
    }

    const request = context.switchToHttp().getRequest();
    const userRole = request?.user?.role as RolesEnum;

    if (!this.matchRoles(roles, userRole)) {
      throw new ForbiddenException(
        'You are not allowed to perform this action',
      );
    }
    return true;
  }
}

Minimum reproduction code

https://docs.nestjs.com/guards#setting-roles-per-handler

Steps to reproduce

Copy and paste the code into any new NestJS project and send a request to a route where the Roles decorator is used.

Expected behavior

When I use the explicit implementation with SetMetadata like this, it is working fine πŸ‘, and my routes are being protected accurately.

// roles.decorator.ts
import { SetMetadata } from '@nestjs/common';
import { RolesEnum } from '../enums/roles.enum';

/**
 * This decorator is used to define the role required to execute a handler
 * or to get the list of required roles for a decorator
 **/
export const Roles = (roles: RolesEnum[]) => SetMetadata('roles', roles);

I am very interested in knowing what is wrong with the implementation. Is there something wrong with the way I used the Roles decorator, or is there some other problem?

Package

Other package

No response

NestJS version

No response

Packages versions

{
  "name": "nest-js-basics",
  "version": "0.0.1",
  "description": "",
  "author": "",
  "private": true,
  "license": "UNLICENSED",
  "scripts": {
    "build": "nest build",
    "format": "prettier --write \"src/**/*.ts\" \"test/**/*.ts\"",
    "start": "nest start",
    "start:dev": "NODE_ENV=development nest start --watch ",
    "start:debug": "nest start --debug --watch",
    "start:prod": "node dist/main",
    "lint": "eslint \"{src,apps,libs,test}/**/*.ts\" --fix",
    "test": "jest",
    "test:watch": "jest --watch",
    "test:cov": "jest --coverage",
    "test:debug": "node --inspect-brk -r tsconfig-paths/register -r ts-node/register node_modules/.bin/jest --runInBand",
    "test:e2e": "jest --config ./test/jest-e2e.json"
  },
  "dependencies": {
    "@nestjs/bullmq": "^10.2.1",
    "@nestjs/common": "^10.0.0",
    "@nestjs/config": "^3.2.3",
    "@nestjs/core": "^10.0.0",
    "@nestjs/jwt": "^10.2.0",
    "@nestjs/mongoose": "^10.0.10",
    "@nestjs/passport": "^10.0.3",
    "@nestjs/platform-express": "^10.0.0",
    "@nestjs/platform-socket.io": "^10.4.1",
    "@nestjs/websockets": "^10.4.1",
    "@socket.io/redis-adapter": "^8.3.0",
    "bcryptjs": "^2.4.3",
    "class-transformer": "^0.5.1",
    "class-validator": "^0.14.1",
    "cookie-parser": "^1.4.6",
    "helmet": "^7.1.0",
    "joi": "^17.13.3",
    "mongoose": "^8.5.2",
    "passport": "^0.7.0",
    "passport-jwt": "^4.0.1",
    "passport-local": "^1.0.0",
    "redis": "^4.7.0",
    "reflect-metadata": "^0.2.0",
    "rxjs": "^7.8.1"
  },
  "devDependencies": {
    "@nestjs/cli": "^10.0.0",
    "@nestjs/schematics": "^10.0.0",
    "@nestjs/testing": "^10.0.0",
    "@types/cookie-parser": "^1.4.7",
    "@types/express": "^4.17.17",
    "@types/jest": "^29.5.2",
    "@types/node": "^20.3.1",
    "@types/passport-jwt": "^4.0.1",
    "@types/passport-local": "^1.0.38",
    "@types/supertest": "^6.0.0",
    "@typescript-eslint/eslint-plugin": "^6.0.0",
    "@typescript-eslint/parser": "^6.0.0",
    "eslint": "^8.42.0",
    "eslint-config-prettier": "^9.0.0",
    "eslint-plugin-prettier": "^5.0.0",
    "jest": "^29.5.0",
    "prettier": "^3.0.0",
    "source-map-support": "^0.5.21",
    "supertest": "^6.3.3",
    "ts-jest": "^29.1.0",
    "ts-loader": "^9.4.3",
    "ts-node": "^10.9.1",
    "tsconfig-paths": "^4.2.0",
    "typescript": "^5.1.3"
  },
  "jest": {
    "moduleFileExtensions": [
      "js",
      "json",
      "ts"
    ],
    "rootDir": "src",
    "testRegex": ".*\\.spec\\.ts$",
    "transform": {
      "^.+\\.(t|j)s$": "ts-jest"
    },
    "collectCoverageFrom": [
      "**/*.(t|j)s"
    ],
    "coverageDirectory": "../coverage",
    "testEnvironment": "node"
  }
}

Node.js version

20

In which operating systems have you tested?

Other

No response

abuzain432432 commented 2 months ago

Ah, I see! I figured it out, and it seems that the issue is related to how I am extracting the metadata from the context. According to the documentation, the actual Roles decorator is passed as an argument to get the metadata, but I was using a string. Correct way:

  const roles = this.reflector.getAllAndOverride<undefined | RolesEnum[]>(
      Roles,
      [context.getHandler(), context.getClass()],
    );

Incorrect way:

 const roles = this.reflector.getAllAndOverride<undefined | RolesEnum[]>(
      'roles',
      [context.getHandler(), context.getClass()],
    );