Skip to content
This repository has been archived by the owner on Feb 13, 2022. It is now read-only.

Missing authorization logic in graphql subscriptions manager? #346

Open
sergey-solo opened this issue Jun 30, 2020 · 3 comments
Open

Missing authorization logic in graphql subscriptions manager? #346

sergey-solo opened this issue Jun 30, 2020 · 3 comments

Comments

@sergey-solo
Copy link

sergey-solo commented Jun 30, 2020

I might be missing something, not sure. But it looks like there is a missing puzzle piece. I mean, the method handle in the SubscriptionsManager class handles four cases:

  1. connection init
  2. start of subscription
  3. data that is passed to subscription
  4. stopping subscription

But the method handleConnectionInit despite returning message of the GQL_CONNECTION_ERROR type(if you throw an error in ON_CONNECT callback, for example), doesn't close the connection. And so you can still use the rest of the "methods": GQL_START, GQL_DATA, GQL_STOP without any problem even after the GQL_CONNECTION_ERROR was returned to the client.

My understanding is that, the GQL_CONNECTION_INIT should close the connection upon exception or otherwise mark it as authorized. And the other three "methods" should always check if connection is authorized.

Please let me know if I really miss something here.

@leocavalcante
Copy link
Owner

Yes, thinking on a user-land point-of-view it should close the connection if unauthorized/on exception. I'll check.

@sergey-solo
Copy link
Author

I could submit a PR if you want. Within a week or so. Because I need to make it work with auth anyway.

@leocavalcante
Copy link
Owner

leocavalcante commented Jun 30, 2020

Don't worry then, it was just if you already have something, in this case I'll build one. Thanks! Unless you want to, of couse, all help is welcome :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants