Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suggestion: Exclude routes from MiddlewareBuilder #17 #790

Closed
wbhob opened this issue Jun 18, 2018 · 8 comments

Comments

@wbhob
Copy link
Contributor

commented Jun 18, 2018

I'm submitting a...


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

Current behavior

Nest users have expressed a desire to exclude specific routes from being applied by the middleware consumer. Right now, paths must be hard-coded to exclude routes from being applied. Example: if you want to screen for a JWT in a header for all routes EXCEPT '/auth', there is no way to do so without hard-coding all paths.

Expected behavior

A common solution in #17 was to implement a MiddlewaresConsumer.exclude() method, wherein you can specifically exclude a route from consuming a middleware.

Example (with v4 syntax, not updated for v5):

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

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

This would reduce developer friction, and give them more control over where to apply middlewares.

Environment


Nest version: N/A

Others:

@wbhob wbhob changed the title Re-open #17 exclude() method for MiddlewareConsumer #17 Jun 18, 2018

@wbhob wbhob changed the title exclude() method for MiddlewareConsumer #17 Suggestion: Exclude routes from MiddlewareBuilder #17 Jun 18, 2018

@kamilmysliwiec

This comment has been minimized.

Copy link
Member

commented Jun 18, 2018

100% agree 👍

@alfirin

This comment has been minimized.

Copy link

commented Jun 27, 2018

Same here, definitely needed that !

@adrien2p

This comment has been minimized.

Copy link

commented Jun 27, 2018

@alfirin the builder has already been updated and add now the RouteInfo that allow us as the previous version to configure the forRoutes by adding the path/method and also to exclude.

@kamilmysliwiec

This comment has been minimized.

Copy link
Member

commented Jul 5, 2018

Added in v5.1.0

@alfirin

This comment has been minimized.

Copy link

commented Sep 11, 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.

@rlewan

This comment has been minimized.

Copy link

commented Oct 9, 2018

Agreed with @alfirin on this. We have the same use case and it doesn't seem to work correctly:

    consumer
      .apply(AuthenticationMiddleware)
      .exclude('health')
      .forRoutes('*')
$ curl --verbose localhost:8080/health
...
< HTTP/1.1 401 Unauthorized
...
@kamilmysliwiec

This comment has been minimized.

Copy link
Member

commented Oct 10, 2018

For your cases, you should rather use either regexp path or with() method to pass custom routes (for example, health) to middleware and exclude paths manually.

@NenadJovicic

This comment has been minimized.

Copy link

commented Jul 1, 2019

On v6 of nest, it still does not work. I have same example, and there is no more with() method on MiddlewareConsumer and no real example how to provide some params to new Middlewares with use() method

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.