Add decorator after @Get

[silvelo]

Its posible add my own decorator after @Get decorator? Because when I do this res seems to be not linked with express response

  @Get()
  @Decorator()
  public async getAll( @Res() res: Response) {
    res.status(HttpStatus.OK).send();

Update: Watching this I think it can be posible

[cojack]

@silvelo show the code

[silvelo]

My Decorator:

import { ROLE } from "./../shared/enums";
import { ForbiddenException } from './../shared/exceptions';

export function Roles(roles?: ROLE[]) {
  return (
    target: any,
    propertyKey: string,
    descriptor: TypedPropertyDescriptor<any>
  ): TypedPropertyDescriptor<any> => {
    const originalMethod = descriptor.value;
    descriptor.value = function(...args: any[]) {
      const user = args[0].session;
      let isAuthorized = true;
      if (roles) {
        isAuthorized = roles.some(rol => {
          return user.roles.some(userRol => {
            return userRol === rol;
          });
        });
      }
      if (!isAuthorized) {
        throw new ForbiddenException('No enogugh rol');
      }
      const result = originalMethod.apply(this);
      return result;
    };
    return descriptor;
  };
}

And my function:

@Get()
@Roles()
  public async getAll( @Res() res: Response) {
    res.status(HttpStatus.OK).send();
}

I want implement a decorator for manage roles, but I want to put this in controller not in middleware.

Repository

[cojack]

@silvelo imo this kind of action should be done in middleware not from decorator

[thomrick]

Hey @silvelo ! At first I join @cojack it seems that you want to use a middleware. But if you want to make decorators combinations you may look at decorator mixin. There is an example here https://medium.com/@dmyl/mixins-as-class-decorators-in-typescript-angular2-8e09f1bc1f02 But I think there are a plenty of examples that you can find ;-)

[silvelo]

@ThomRick yeah, at the first time i used middlewares but that looks very confuse, because the we had a lot of combines and that result add 2 or 4 middlewares per module, because we have 5 roles. For that we thought to do at this way using decorators and put in controllers, this bring two improve:

• A clean code because in own case we repeat the middleware a lot of times, for each module.
• Make more easy reas the code.

Thanks for quickly reply.

[thomrick]

Hey @silvelo ! I understand that problem cause I had to face it in a previous professional project but it was written in Java :-) We used a method decorator by passing each role in an array we want to let pass through like you want to in your case. That's better than a middleware because it's customizable by request. So I think you should look at the mixin.

I'll try to make an example with mixin and why not tell to @kamilmysliwiec to integrate mixin in Nest to add custom decorators ;-)

[silvelo]

@ThomRick this will be great.

Thanks for all.

Update: Perhaps we should reopen this issue as a feature or create new issue

[FranciZ]

@silvelo @ThomRick I'm facing a similar dilemma. I want to have transparent route handling and plug authentication and role checking per route in the controller in most cases. Not sure what the advantage is of having this logic separated in another file (Module) and paths redefined in two places.

Something like the UseMiddlewares decorator as below would be great.

  @Delete('v1/user/:userId')
  @UseMiddlewares(new AuthMiddleware(['ADMIN']))
  public async deleteUser(@Req() req: IRequest, @Res() res: Response, @Param() reqParams: VDeleteUser): Promise<any> {
    const userId = reqParams.userId;
    const removedUser = await this.userService.remove(userId);
    if (removedUser === null) return this.responseService.error({
      res,
      code: HttpStatus.NOT_FOUND,
      data: { _id: userId }
    });
    this.responseService.success({ res, data: { _id: userId } });
  }

[silvelo]

@FranciZ looks good. Use middleware decorator in controller.

[FranciZ]

I started working on an implementation but am not yet sure how decorators plug into DI so that they are applied to the instance and not only to the class. Any pointers would be appreciated here.

[cojack]

@FranciZ maybe instead of passing middlewares just right into controller file (because it will grow and grow...), setup them into module, fe:

import { Module, MiddlewaresConsumer } from '@nestjs/common';

@Module({
    controllers: [ UsersController ], // your controller
    components: [ UsersService ],
    exports: [ UsersService ]
})
export class UsersModule {
    configure(consumer: MiddlewaresConsumer) {
        const roles = ['ADMIN'];
        const options = {};
        consumer.apply(AuthMiddleware)
            .with(roles, options)
            .forRoutes({path: 'v1/user/:userId', method: RequestMethod.DELETE});
    }
}

Module file is a great place for kind of this actions. Please fallow this link https://docs.nestjs.com/quick-start/middlewares.html to get more examples.

[FranciZ]

@cojack Thanks, but I'm aware of this. I'd still rather have the option to apply middleware in the controller since it'll be much more transparent.

I see usefulness in defining middleware in the module if it's for all the routes of a controller or something more general. But for things like role specific access control and authentication it's much more transparent to add it as a decorator in controller imo.

Having to define paths and methods affected in two different places is also something I'd rather avoid.

[silvelo]

@cojack In my project I have this module and works fine

export class UsersModule implements NestModule {
    public configure(consumer: MiddlewaresConsumer) {
        consumer
            .apply(AuthMiddleware)
            .forRoutes(
            { path: '/users', method: RequestMethod.GET },
            { path: '/users/:id', method: RequestMethod.DELETE },
            { path: '/users/:id/favorites', method: RequestMethod.PATCH },
            { path: '/users/:id/state', method: RequestMethod.PATCH },
            { path: '/users/:id/role', method: RequestMethod.PATCH })
            .apply(RolesMiddleware)
            .with([ROLE.ADMIN])
            .forRoutes(
            { path: '/users', method: RequestMethod.GET },
            { path: '/users/:id/state', method: RequestMethod.PATCH },
            { path: '/users/:id/role', method: RequestMethod.PATCH })
            .apply(RolesMiddleware)
            .with([ROLE.USER])
            .forRoutes({ path: '/users/:id/favorites', method: RequestMethod.PATCH })
            .apply(RolesMiddleware)
            .with([])
            .forRoutes({ path: '/users/:id', method: RequestMethod.DELETE });
    }
}

But this is so confuse and grow a lot, much than the other way, because now I have to definied my routes here and in my controller, this make more easiest that I make a mistake.

If my module will be, the middleware in module was great but this is no the case.

export class UsersModule implements NestModule {
    public configure(consumer: MiddlewaresConsumer) {
        consumer
            .apply(AuthMiddleware)
            .forRoutes(UsersControllers)
           .apply(RolesMiddleware)
            .with([ROLE.USER])
           .forRoutes(UsersControllers)
    }
}

@FranciZ Said correctly in the comment above.

[FranciZ]

Is there any chance this might get some attention? As it is, I find this (middlewares) the weakest point of the framework. I'm having a really hard time to justify repeating myself in the Module and in the Controller for something that is essentially related and belongs in the same place.

Is the only fear a bloated controller or is there some other reason why this would not be suitable?

[Masterrg]

I would love to see such a feature too. We are currently working on a system that doesn't take the user role into account but a specific access right.

As we got a lot of routes already it isn't really handy to configure and maintain every single path within a module. It would be a lot easier to deal with, if we could add it as a @Decorator directly to the specific route within the controller.

[adrien2p]

Hey ! I think you talking about guard that will add in v4 release

[Masterrg]

@adrien2p thanks for the hint, just found it in the v4 commits, thank you very much! :)

[chanlito]

Can't wait 😊 for v4, I posted this same problem about defining middlewares in modules on Gitter.

[lock[bot]]

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.