Graphql: Guards on GraphQL subscriptions

[jukkahell]

I'm submitting a...

[ ] Regression 
[ ] 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

Request is not available for subscriptions. Also guards doesn't work on GraphQL subscriptions:

@Subscription("chatMessage")
@UseGuards(AuthGuard)
chatMessage() {
  return {
    subscribe: () => this.pubSub.asyncIterator("chatMessage");
  };
}

Expected behavior

We should have request object available in the subscribe context object so that we can get JWT token from the headers. Make UseGuards decorator to work if possible:

@Subscription("receiveMessage")
receiveMessage() {
  return {
    subscribe: (rootValue, args, context, info) => {
      const request = context.req; // <--- context.req should not be undefined
    }
  }
}

Minimal reproduction of the problem with instructions

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

Currently apps can't protect their subscriptions separately or have knowledge of the user making the request.

Environment

Nest version: 5.3.7
Nest GraphQL version: 5.4.0

[jukkahell]

Hope it's OK that this issue contains two things: guard decorator and the request object. Not sure if those are two separate things or if they relate to each other. I can make different issues for both if needed!

[cschroeter]

Please see here for a solution https://github.com/nestjs/graphql/issues/82#issuecomment-427664196

[jzivanov]

@cschroeter the solution you're proposing works when you want to protect all subscriptions. The authorization for specific subscription will be possible if request object is in the context.

[biels]

So I think that the solution here would be to inject the request into the context. Maybe it is possible to do it using an Interceptor, but it would be nice if @nestjs/graphql offered this feature.

[jkosonen]

This sounds pretty critical issue and makes it impossible to do anything private with subscriptions, eg. private chat. Should be addressed asap.

[cschroeter]

@zjovan what you mean with _all_ subscriptions? There is just this one connection established between client and server.

@biels Subscriptions in GraphQL are not a first class citizen in NestJs - thats true. In the Apollo docs there is an example how to implement authentication for subscriptions.

// https://www.apollographql.com/docs/graphql-subscriptions/authentication.html
   onConnect: (connectionParams, webSocket) => {
       if (connectionParams.authToken) {
            return validateToken(connectionParams.authToken)
                .then(findUser(connectionParams.authToken))
                .then((user) => {
                    return {
                        // Note: The currentUser is now available in the subscription context
                        currentUser: user,
                    };
                });
       }

       throw new Error('Missing auth token!');
    }

@jkosonen This is an feature request on an open source project. We are not talking about a security vulnerability. If you feel the need that this should be addressed "asap" no one stops you from creating a pull request.

[jkosonen]

@cschroeter I feel this is kind of a security vulnerability if you don't realize that request context in subscriptions isn't working the same way as in queries and mutations. And I see it rather as a bug than feature. And I'm pretty sure that it won't happen asap if I'll start digging in to the code base.

[jukkahell]

@cschroeter there can be multiple different subscriptions per application and you might want to guard only some of them and keep some public.

Regarding to the security side. Think about use case where you publish a new private chat message using pubsub. Do we currently have any way to filter that message from subscribers that shouldn't see it? https://www.apollographql.com/docs/graphql-subscriptions/setup.html#filter-subscriptions <-- here's a way to do the filtering. You could pass the user id from client side along with variables (not good) but in my understanding there's no way to get the subscriber's user id server side currently. I would be happy if I'm wrong :)

[radarsu]

As a workaround for now to protect single subscription I recommend sending access token as a param:
auth.resolver.ts

@RadarsuSubscription()
public method(@RadarsuArgs('token') token?: string): RpcMessage {
    // subscription fires on start, so ignore it
    return {
        subscribe: (...args: any[]) => {
            const ctx = args[1];
            const ws = args[2];
            const token = ws.token || ctx.token;
            return this.authService.subscribeMethod(token);
        },
    }
    as any;
}

auth.service.ts

public subscribeMethod(token ? : string) {
    if (!token) {
        return `Token required for method subscription.`;
    }

    try {
        this.jwtService.verify(token);
        return config.pubSub.asyncIterator(token);
    } catch (err) {
        return err.message;
    }
}

[ph55]

It is addressed in Apollo docs:
https://www.apollographql.com/docs/apollo-server/features/subscriptions.html#Context-with-Subscriptions

So you need to configure context in GraphQLModule:

GraphQLModule.forRoot({
  //...
  context: async ({ req, connection }) => {
        // subscriptions
        if (connection) {
          return { req: connection.context };
        }
        // queries and mutations
        return { req };
  }
  //...
})

Then you can access auth header via context.req.Authorization.

Another question though, why Middlewares and Guards doesn't trigger for subscriptions at all.
And should they?

[jukkahell]

connection.context seems to be an empty object for me. So I'll just continue using the workaround Artur mentioned.

[kamilmysliwiec]

Added in 6.0.0 release :)

[marcus-sa]

Been away from Github for a while because of 2FA issues, but glad to see that this finally got implemented!

[j-tegen]

I'm having issue implementing guards on my subscriptions even with the change to the context. I'm getting asyncIterator is not a function (only) when using the @UseGuards decorator:

@Subscription(returns => Message, {
  filter: (payload, variables) => {
    return payload.messageAdded.conversation.id === variables.conversationId
  },
})
@UseGuards(GqlAuthGuard)
messageAdded(@Args('conversationId') conversationId: number) {
  return pubSub.asyncIterator<Message>(SubscriptionTypes.MessageAdded)
}

Does anyone have any tips on how to implement authentication on subscriptions? Are guards still not supported for subscriptions, and in that case, is there any other way to handle this?

[m19]

I am getting the same TypeError: asyncIterator.return is not a function when the guard returns false. Have not really found a solution yet.

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