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

Global app guard doesn't apply to WebSocketGateway #8060

Closed
brahmlower opened this issue Sep 9, 2021 · 7 comments
Closed

Global app guard doesn't apply to WebSocketGateway #8060

brahmlower opened this issue Sep 9, 2021 · 7 comments
Labels
needs triage This issue has not been looked into

Comments

@brahmlower
Copy link

Bug Report

Current behavior

A global app guard is applied to regular http controllers, but is not applied to websocket gateways. This can be worked around by importing the guard in the gateway module and using a UseGuards decorator. Without declaring the UseGuards decorator, websocket connections and messages are accepted without being validated by the global guard.

Input Code

const your = (code) => here;

Expected behavior

Global app guards should also apply to websocket gateways so that connections or messages to the websocket gateway are guarded like normal http handlers.

Possible Solution

No suggestions 😬

Environment


Nest version: 8.1.1
 
For Tooling issues:
- Node version: v16.9.0
- Platform:  Alpine 3.13, macOS 11.5.1

@brahmlower brahmlower added the needs triage This issue has not been looked into label Sep 9, 2021
@jmcdo29
Copy link
Member

jmcdo29 commented Sep 9, 2021

This is mentioned in the docs

In the case of hybrid apps the useGlobalGuards() method doesn't set up guards for gateways and micro services by default (see Hybrid application for information on how to change this behavior). For "standard" (non-hybrid) microservice apps, useGlobalGuards() does mount the guards globally.

@tjhiggins
Copy link

tjhiggins commented Sep 9, 2021

@jmcdo29 We should have provided a code snippet:

@Module({
  imports: [
    ...
    EventModule,
  ],
  providers: [{
    provide: APP_GUARD,
    useClass: AuthGuard,
  }],
})
export class AppModule { }
@Module({
  providers: [
    EventGateway,
    EventService,
  ],
  exports: [EventService],
})
export class EventModule { }
@WebSocketGateway({ path: '/events', perMessageDeflate: true, inheritAppConfig: true, transports: ['websocket'] })
export class EventGateway implements OnGatewayInit, OnGatewayConnection, OnGatewayDisconnect {
...
}

The issue is that for a nested ws gateway running on path /events doesn't inherit the APP_GUARD even when setting inheritAppConfig. This feels unintuitive to me.

@jmcdo29
Copy link
Member

jmcdo29 commented Sep 9, 2021

I believe that the inheretAppConfig is only for microservices. Websockets do not bind global enhancers, period. I'll see if I can find that reference in the docs again. I know I've spoken with Kamil about this before

@tjhiggins
Copy link

@jmcdo29 Any reason to not support inheritAppConfig for WebSocketGateway? Happy to turn this into a feature request.

@jmcdo29
Copy link
Member

jmcdo29 commented Sep 9, 2021

Other than it (possibly) being difficult to manage, nothing that I'm really seeing in an old conversation. If you'd like to make a PR for this we'd love to see it.

@hardcodet
Copy link

In that case, we should consider that - according to the docs - guards for web sockets should fire WsExceptions, rather than the regular HTTP exceptions. That sounds like a lot of "if/elses" in our code ;)

@kamilmysliwiec
Copy link
Member

I believe that the inheretAppConfig is only for microservices. Websockets do not bind global enhancers, period. I'll see if I can find that reference in the docs again. I know I've spoken with Kamil about this before

That's correct. inheritAppConfig has been added to allow microservices to inherit the ApplicationConfig. There are no plans to support websocket gateways as part of this feature atm but if you're interested in contributing to that issue, PRs are always more than welcome!

@nestjs nestjs locked and limited conversation to collaborators Sep 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs triage This issue has not been looked into
Projects
None yet
Development

No branches or pull requests

5 participants