There should be a way to reorder guards. For example, if I have a RoleGuard that checks whether a user has access to a resource (following the tutorial: Role-based authentication) and I install it globally like [1], and then I have a JwtAuthGuard (following the tutorial: Implement protected route and JWT strategy guards), I don't have access to request.user inside JwtAuthGuard. This is because RolesGuard is executed before JwtAuthGuard, and the user object has not beed added to the request.
[1] Code snippet 1: Installing
RolesGuard to the app module
@Module({
providers: [
{
provide: APP_GUARD,
useClass: RolesGuard,
},
],
})
export class AppModule {}
[2] Code snippet 2: Installing
JwtAuthGuard to a controller:
@Controller('users')
@UseGuards(JwtAuthGuard)
export class UserController {
constructor(private usersService: UsersService) {}
// ...
}
This is a problem because most applications of RolesGuard would require you to know which user you are checking the role for, available via request.user. There is already a StackOverflow discussion on this issue: https://stackoverflow.com/a/50801832/1656944, which recommends this solution:
Ultimately this appears to be an ordering issue with the guards and it doesn't look like it can be easily resolved (without the framework allowing some control over the ordering). My hope was to register the RolesGuard globally but that causes it to be registered first and fire first. If I register it at the endpoint level and put it after the AuthGuard then it fires second and I get the user context I am expecting within the guard itself. It isn't perfect but it works.
@UseGuards(AuthGuard('jwt'), RolesGuard)
@Roles('admin')
The temporary solution for me is to remove the JwtAuthGuard registration from the main app module, and register it on the top of each controller:
@Controller('users')
@UseGuards(JwtAuthGuard)
export class UserController {}
However, this means each *.controller.ts that has to sit behind authorization needs this guard duplication.
The easiest thing would be to order guards in a way that I can request "execute JwtAuthGuard first, then execute ScopesGuard". This way, request.user will be available to ScopesGuard.
Presently, I can order them in this way to ensure that ScopesGuard is only executed after JwtAuthGuard, by installing them both globally:
@Module({
providers: [
{
provide: APP_GUARD,
useClass: JwtAuthGuard,
},
{
provide: APP_GUARD,
useClass: ScopesGuard,
},
],
})
export class AppModule {}
However, the above means that every route is protected by JwtAuthGuard, which is a problem since you'd want routes like /auth/login to be public. Perhaps there's a way to add a @Protected decorator, and only for those JwtAuthGuard will be executed.
One option you have with the JwtAuthGuard is to add an ability to skip if the guard runs via a short circuit from metadata. Basically the inverse of what you're thinking of with @Protected(). I have a sample decorator here and you can see it's usage here and here. This would allow you to have a globally bound guard but still skip the guard if that metadata exists.
Generally, you'll need to have that guard globally bound if you want req.user on every request, because req.user is set by passport. And as Kamil said in #767, adding the ability to let devs re-order the running order of guards could really bring a lot of mess to the framework, and make support much more difficult. You can see the current execution order in the docs
This makes a lot of sense, thanks! I used your idea and created a @Public() decorator that skips this guard. Maybe it'd be nice to have this part of the docs on the authentication tutorial where we teach users to create JwtAuthGuard?
PRs to the documentation (https://github.com/nestjs/docs.nestjs.com) are welcome! Any additions would be useful :)
Most helpful comment
One option you have with the
JwtAuthGuardis to add an ability to skip if the guard runs via a short circuit from metadata. Basically the inverse of what you're thinking of with@Protected(). I have a sample decorator here and you can see it's usage here and here. This would allow you to have a globally bound guard but still skip the guard if that metadata exists.Generally, you'll need to have that guard globally bound if you want
req.useron every request, becausereq.useris set bypassport. And as Kamil said in #767, adding the ability to let devs re-order the running order of guards could really bring a lot of mess to the framework, and make support much more difficult. You can see the current execution order in the docs