nest: Middleware Exclude behaviour not working

I’m submitting a…


[ ] Regression 
[x] Bug report
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead post your question on Stack Overflow.

Current behavior

Use the express-jwt Express middleware for authentication of users accessing an api path, while excluding login / register paths

import { Injectable, NestMiddleware, MiddlewareFunction } from '@nestjs/common';
import * as path from 'path';
import * as fs from 'fs';
import * as jwt from 'express-jwt';

@Injectable()
export class JwtMiddleware implements NestMiddleware {

    resolve(): MiddlewareFunction {
        return jwt({
            secret: fs.readFileSync(path.resolve(process.env.PUBLIC_KEY)),
        });
    }
}


import { Module, NestModule, MiddlewareConsumer, RequestMethod  } from '@nestjs/common';
import { MongooseModule } from '@nestjs/mongoose';
import { CredentialsSchema, TradieController, TradieService } from '.';
import { TradieSchema } from '../users/tradie';
import { UserSchema } from '../users/user';
import { JwtMiddleware } from './jwt.middleware';
import { UserController, UserService } from './user';

@Module({
  imports: [
    MongooseModule.forFeature([
      { name: 'user_credentials', schema: CredentialsSchema },
      { name: 'user_profiles', schema: UserSchema },
    ]),
  ],
  controllers: [ UserController ],
  providers: [ UserService ],
})
export class SecurityModule implements NestModule {

  configure(consumer: MiddlewareConsumer): void {
     consumer.apply(JwtMiddleware);
      .exclude(
        { path: 'user/login', method: RequestMethod.ALL },
        { path: 'user/register', method: RequestMethod.ALL },
      ).forRoutes(
          { path: '/**', method: RequestMethod.ALL },
      );

Expected behavior

2 Issues arise with the sample above,

The filter appears to be applied to all paths, completely ignoring the exclude request
Express-JWT seems to not work (I tested the same version configured with vanilla express)

Minimal reproduction of the problem with instructions

Setup new NestJs project
Create Middleware.
Configure module to use middleware, with excludes parameters
Create controller to test middleware is called or not

What is the motivation / use case for changing the behavior?

Being able to exclude the need for authentication for publicly available paths to an API such as login / register or any other publicly available enpoint.

Environment


Nest version: 5.1.0

 
For Tooling issues:
- Node version: 8.11.3  
- Platform:  Linux 4.13.0-45-generic

Others:
IDE: Visual Studio Code (Though have used command line to run tsc and node to test if was issue caused by the ide)

About this issue

Original URL
State: closed
Created 6 years ago
Reactions: 5
Comments: 24 (6 by maintainers)

Most upvoted comments

@alfirin this is not an issue.

Please note that exclude() method won’t work with your functional middleware. In addition, this function doesn’t exclude paths from more generic routes (e.g. wildcards). In such case, you should rather put your paths-restriction logic directly to the middleware and, for example, compare a request’s URL.

https://docs.nestjs.com/middleware

+11

kamilmysliwiec on Sep 11, 2018

Consequently, LoggerMiddleware will be bounded to all routes defined inside CatsController except these two passed to the exclude() function. Please note that exclude() method won’t work with your functional middleware. In addition, this function doesn’t exclude paths from more generic routes (e.g. wildcards). In such case, you should rather put your paths-restriction logic directly to the middleware and, for example, compare a request’s URL.

^ docs. It is not a bug. This is how essentially router works in both express and fastify which are used underneath. If you want to exclude something from string path - use regexp, if you want to exclude something from function middleware - create another function that wraps your middleware and returns a function enhanced with paths excluding logic which is pretty simple AND the last use case - “exclude paths from Controller” is a Nest-specific feature and thus exclude() method has been provided. It works as expected, it is filling the gap in all available use-cases.

kamilmysliwiec on Oct 10, 2018

Of course it is not an issue! As we all know, documented bugs become features. But this behaviour is weird. And writing something like In addition, this function doesn't exclude paths from more generic routes (e.g. wildcards) in the docs doesn’t make it any better.

The most obvious usage of methods like exclude is in combination with wildcard routes. I have 10 controllers and only one controller and route that should be available without auth. So now I have two options: decorate every controller with guard decorator or count them all in middleware consumer. This is astonishing how this common pattern is ignored in the framework

icewind on Oct 9, 2018

All these examples would work well: https://stackoverflow.com/questions/27117337/exclude-route-from-express-middleware + use @Inject() to pass options to middleware (in case you use a class middleware, not function middleware) as shown here https://github.com/nestjs/nest/issues/1378

kamilmysliwiec on Jul 1, 2019

Will full exclude() functionality as mentioned above, namely implementing it for the path: * forRoutes syntax, be implemented at some point? It seems like a fairly common use case that someone would want to include all routes in a middleware and exclude a couple in this manner.

hsearcy on Aug 7, 2018

I think I have figured it out, its a documentation issue, more then an actual functional issue. The way exclude works, it is for an instance where you want to exclude particular routes in a controller you pass to it. So if you were to do:

consumer.apply(SomeMiddleware)
.exclude({path:'somePath', method: RequestMethod.GET})
.forRoutes(SomeControllerWithRoutes);

This would exclude ‘somepath’ under that controller.

I think personally it should work as the above people have documented, whereby it checks exclusions first, or utilizes express-unless to somehow manage it, but I will have to look in it further

Maraket on Jul 12, 2018

Any news on this issue?

For me it doesn’t correct the expected behavior. The expected behavior is something like this:

consumer
            .apply(AuthMiddleware)
            .exclude({ path: '/status', method: RequestMethod.ALL })
            .forRoutes({ path: '/*', method: RequestMethod.ALL });

We want to have one declaration for all the routes except the ones provided inside the exclude method.

Thanks a lot guys.

alfirin on Sep 11, 2018

I have exactly the same problem.

The routes passed in exclude function are not took in account. Example:

configure(consumer: MiddlewareConsumer): void {
        consumer
            .apply(AuthMiddleware)
            .exclude('/status')
            .forRoutes('*');
    }

In the example above the AuthMiddleware is also applied to the /status route

Nestjs version: 5.1.0

alfirin on Jul 10, 2018

I also have Same problem，If you think this is the normal way to use it, you should implement the normal way to use it without making people feel doubtful about it。

ServisionFrontend on May 30, 2019

Thanks for the reply @Maraket .

But what if we want a middleware apply to all the routes by default except the ones provided by the exclude function.

With that above developers could create all the Controllers they want without adding them to the consumer.apply function and have the Middleware apply by default.

alfirin on Jul 12, 2018