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.07k stars 7.56k forks source link

[PROBLEM] Reflector - Guard doesn't run with reflector decorator #1478

Closed aronraja closed 5 years ago

aronraja commented 5 years ago

I'm submitting a...


[ x ] Support request

Current behavior

I have a problem with using a reflector and a guard. The problem is that the guard doesn't run on my reflector and immediately skips the guard. I am using it on @nestjs/graphql

Expected behavior

It should log the value that was passed in.

Minimal reproduction

resolver.ts

    @Query('test')
    @RateLimit('value', 'sometext')
    public async test(
    ){
        return await this.userService.test()
    }

decorator.ts export const RateLimit = (...value: string[]) => ReflectMetadata('value', value);

guard.ts

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

  canActivate(context: ExecutionContext): boolean {
    const ctx = GqlExecutionContext.create(context);
    const value= this.reflector.get<string[]>('value', context.getHandler())
    // const request: Request = ctx.getContext().req

    console.log(value)

    return false 
// It still returned true/ didn't run at all...
  }
}

Environment


Nest version: 5.4.0


For Tooling issues:
- Node version: v8.11.1  
- Platform:  Mac 

Others:

kamilmysliwiec commented 5 years ago

Could you please provide a minimal repository which reproduces your issue?

IntellectProductions commented 5 years ago

@kamilmysliwiec I'm having this same issue :( I just tried to basically do what was setup in the NestJS documentation and it doesn't even trigger my guard. I'm on the latest NestJS version too (5.7.3).

// permissions.decorator.ts

import { ReflectMetadata } from '@nestjs/common';

export const Permissions = (...permissions: string[]) => ReflectMetadata('permissions', permissions);

// permissions.guard.ts

import { ExtractJwt } from 'passport-jwt';
import { CanActivate, ExecutionContext, Injectable } from "@nestjs/common";
import { Observable } from "rxjs/internal/Observable";
import { Reflector } from "@nestjs/core";

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

  canActivate(context: ExecutionContext): boolean | Promise<boolean> | Observable<boolean> {
    // Check to see if user has permission
    console.log('hitting canActivate');
    const permissions: string[] = this.reflector.get<string[]>('permissions', context.getClass());
    console.log('list of permissions', permissions);
    if (! permissions) {
      return true;
    }

    const request = context.switchToHttp().getRequest();
    const user = request.user;
    console.log(user.permissions);
    const hasPermission = () => user.permissions.some((permission) => permissions.includes(permission.name));
    return user && user.permissions && hasPermission();
  }

}

// module

import { Module } from '@nestjs/common';
import { StaffController } from './staff.controller';
import { ItemController } from './item/item.controller';

@Module({
  controllers: [StaffController, ItemController]
})
export class StaffPanelModule {
}

// item.controller.ts

import { Controller, Get, UseGuards } from '@nestjs/common';
import { JwtAuthGuard } from "../../auth/auth.guard";
import { Permissions } from "../../auth/authority/permission/permissions.decorator";

@Controller('staff/item')
@UseGuards(JwtAuthGuard)
export class ItemController {

  @Get('/')
  @Permissions('access_staff')
  index() {
    // Make sure user is admin
    return 'hit';
  }

}

When I hit this route, it doesn't even do my console logs inside my PermissionsGuard like it does in my AuthGuard.

picheli20 commented 5 years ago

@kamilmysliwiec same here, even if you run the example on the repo (https://github.com/nestjs/nest/tree/master/sample/01-cats-app), the user is always undefined on this guard :(

    const request = context.switchToHttp().getRequest();
    const user = request.user;
SloCompTech commented 5 years ago

Same, here, is there a solution for this ? For me #2027 helped

Paul-Vandell commented 5 years ago

Hey @SloCompTech I propose a solution on -> https://github.com/nestjs/nest/issues/2027#issuecomment-527863600 It could maybe help you

lock[bot] commented 4 years ago

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.