search

leocavalcante / siler

⚡ Flat-files and plain-old PHP functions rockin'on as a set of general purpose high-level abstractions.
https://siler.leocavalcante.dev
MIT License
1.12k stars 90 forks source link

Missing authorization logic in graphql subscriptions manager? #346

Open sergey-solo opened 4 years ago

sergey-solo commented 4 years ago

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 commented 4 years ago

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

sergey-solo commented 4 years ago

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

leocavalcante commented 4 years ago

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 :)