Global Guards are not triggered when route is registered with httpService.get('/route`)

lucassith commented 9 months ago

Is there an existing issue for this?

[X] I have searched the existing issues

Current behavior

Having route registered via Express' HttpServer with .use('/route'), when entering given route, Guard is not being checked.

Guard are registered in main.ts (app.useGlobalGuards(new TestGuard())) and app.module.ts (as Provider's APP_GUARD) and no matter what, I can't make them work globally.

I find this a major problem since it creates a security holes you can't really expect.

Minimum reproduction code

https://codesandbox.io/p/devbox/nestjs-forked-c37nw6

Steps to reproduce

npm install
npm start

Open /test route and refresh multiple times. Accessing this route you can see in the response that the timestamp changes, but TestGuard is not called. Open / route - everytime you enter the page the Guard is called.

Expected behavior

Since NestJS utilizes .use call in multiple projects https://github.com/nestjs/graphql/blob/c2e408f37c2842ba9c9df90bbd8cd80bd0cb3779/packages/apollo/lib/drivers/apollo-base.driver.ts#L139 it creates possible attack vectors for unauthorized access even though you have global guards registered.

Package

[ ] I don't know. Or some 3rd-party package
[X] code>@nestjs/common</code
[X] code>@nestjs/core</code
[ ] code>@nestjs/microservices</code
[X] code>@nestjs/platform-express</code
[ ] code>@nestjs/platform-fastify</code
[ ] code>@nestjs/platform-socket.io</code
[ ] code>@nestjs/platform-ws</code
[ ] code>@nestjs/testing</code
[ ] code>@nestjs/websockets</code
[ ] Other (see below)

Other package

No response

NestJS version

10.3.1

Packages versions

[System Information]
OS Version     : Linux 6.1.43
NodeJS Version : v20.9.0
NPM Version    : 9.8.1 

[Nest CLI]
Nest CLI Version : 10.3.0 

[Nest Platform Information]
platform-express version : 10.3.1
schematics version       : 10.1.0
testing version          : 10.1.3
common version           : 10.3.1
core version             : 10.3.1
cli version              : 10.3.0

Node.js version

20.9.0

In which operating systems have you tested?

[ ] macOS
[ ] Windows
[X] Linux

Other

Discord Chat

lucassith commented 9 months ago

Please ignore my dummy code, I get that registering routes this way:

  const server = app.getHttpAdapter();
  server.use('/test', (req, res) => {
    res.send(`test ${new Date().toISOString()}`);
  });

is wrong.

But please note that NestJS packges are using this way to register GraphQLModule in @nestjs/graphql and it allows everyone to query your entire database without any authorization, even though you have global guards registered.

https://github.com/nestjs/graphql/blob/c2e408f37c2842ba9c9df90bbd8cd80bd0cb3779/packages/apollo/lib/drivers/apollo-base.driver.ts#L139

micalevisk commented 9 months ago

to me, that's not wrong. It's a good way to slowly migrate an express/fastify app to nestjs, I guess

but yeah, not sure if that is a missing feature or a bug

kamilmysliwiec commented 9 months ago

This is the expected behavior. Any routes registered directly with the HttpServerAdapter are not wrapped with guards, interceptors, pipes, etc.

nestjs / nest