Nest: Middleware Exclude behaviour not working

[Maraket]

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,

1. The filter appears to be applied to all paths, completely ignoring the exclude request
2. 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)

[alfirin]

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]

I also tried this:

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

Also not working

[Maraket]

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

[alfirin]

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.

[chase0010]

same problem

configure(consumer: MiddlewareConsumer) {
    consumer
      .apply(LoggerMiddleware, AuthMiddleware)
      .exclude({ path: 'login', method: RequestMethod.GET })
      .forRoutes({
        path: '*',
        method: RequestMethod.ALL,
      });
  }

[kamilmysliwiec]

As @Maraket explained, docs should be updated to describe the behavior of exclude() method more precisely.

[hsearcy]

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.

[phanhoang06]

the same my issue. I hope the next release will fix it

[kamilmysliwiec]

I have added somewhat more about exclude() to the docs, shouldn't be so confusedness anymore.

[alfirin]

Any news on this issue?

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

            .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.

[kamilmysliwiec]

@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

[alfirin]

Thanks for your reply @kamilmysliwiec .
It's exactly what I did a few months ago.

Something like that:

excludedRoutes.filter(
    excludedRoute => {
        const reg = pathToRegexp(excludedRoute.path);

        const isUrlEquals = reg.exec(req.baseUrl) !== null;
        return (
            isUrlEquals &&
            ('' + excludedRoute.method === '' + RequestMethod[req.method] ||
                req.method === RequestMethod.ALL)
        );
});

But for that, I had to use the pathToRegexp npm package...
Which is I think should be managed by nest instead of doing that in each middleware...

[batmanhit]

@alfirin , can you please post the full source code for your workaround?
That would be quite helpful.
Thanks.

[icewind]

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

[kamilmysliwiec]

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.

[ServisionFrontend]

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。

[NenadJovicic]

@kamilmysliwiec if you update docs, at least, provide an example how to exclude group of routes inside middleware
because, for v4, someone provided nice example with .with() method and some params to middleware method
but because .with() is removed, now there is no good example how to have generic way to exclude some routes

I agree with all people commenting, because it is really annoying every time after adding new Controller, you have to define it in forRoutes

[kamilmysliwiec]

@NenadJovicic https://github.com/nestjs/nest/issues/1378

[NenadJovicic]

and what with that? @kamilmysliwiec
there is no example how to exclude routes without adding every single controller to .forRoutes() method
and in v6, there is also no more .with() method, and they have changed middleware to have use() method, and not that resolve() with arguments

[kamilmysliwiec]

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

[alfirin]

@kamilmysliwiec , this is not the goal of this discussion.

This discussion is how to do it a proper way not a bypass road.

[prakashchokalingam]

I have a similar issue,

export class GraphqlModule { public configure(consumer: MiddlewareConsumer) { consumer.apply(authenticate('jwt')).exclude( { path: `${activeGraphqlPath}/playground`, method: RequestMethod.GET }, ).forRoutes( { path: activeGraphqlPath, method: RequestMethod.ALL }, ); } }

It adds authentication middleware to playground endpoint. I don't need authentication for graphql playground endpoint. Need help.

[ryanmr]

I think we can acknowledge now, that this is _not_ a bug. It's just a poor experience, being shown the wildcard feature and then exclusion feature, and the two together not working in a predictable fashion.

export class AppModule implements NestModule {
  configure(consumer: MiddlewareConsumer) {
    consumer
      .apply(TokenAuthMiddleware)
      .exclude('/health')
      .forRoutes('*');
  }
}

This thread documents this pattern thoroughly. It's understandable if Nest shouldn't handle this case in the core libraries, but it should be acknowledged in the documentation in a subsequent section that this is _not possible_ as is.

I have the following code in my TokenAuthMiddleware, skip is simple guard at the top of the middleware proper:

/**
 * Skip these permitted routes from middleware application.
 *
 * @param next express function to skip to the next route or middleware
 */
function skipPermittedRoutes(req: Request, next: NextFunction) {
  const permitted = ['/'];

  return permitted.includes(req.originalUrl);
  /*
   * This is required here because consumer.apply().exclude
   * does not exclude wildcard matches at this time.
   * .exclude('/') // does not work
   * .forRoutes('*')
   */
}

Hopefully this helps.

[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.