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

Feature/ws init func #275

Closed
wants to merge 9 commits into from
Closed

Conversation

chedom
Copy link
Contributor

@chedom chedom commented Aug 20, 2021

Add possibility to check initial payload to see whether to accept the websocket connection.
Example: authenticate user from websocket connection and save session of user in context.

@devsergiy
Copy link
Member

Hi @chedom
could you have a look at this commit 7375417

Does it solve a problem for you?

@chedom
Copy link
Contributor Author

chedom commented Aug 31, 2021

Hi @chedom
could you have a look at this commit 7375417

Does it solve a problem for you?

Hi @spetrunin.
No, it doesn't solve the problem for me.
I added means to authenticate WebSocket connection(it must be done once) but your hook is called every time when client sends new operation (starts new subscription or query/mutation through websocket).
Example:

|------------|--------------|---------|-----> t
A            B              C         D

Where:
A - client initiates websocket connection (sends ConnectionInit message)
B - client sends Subscription1 (sends ConnectionStart message)
C - client sends Subscription2 (sends ConnectionStart message)
D - client sends mutation through websocket (client preferred to use WS as maint transport layer for all operations) (sends ConnectionStart message)

WebsocketInit func will be called only at point A (gets token from initial payload, resolves user session and stores it in connection context, example from gqlgen).
WebsocketBeforeStartHook will be called at points B, C, D.

I have checked how you use WebsocketBeforeStartHook in tyk.
I would propose you to migrate from the hook to operation middleware #284. It allows you to move logic from websocket hook and http middlewares(this and this) to single operation middleware.

@chedom
Copy link
Contributor Author

chedom commented Nov 9, 2021

@jensneuse, @spetrunin what do you think of PR ?

@argoyle
Copy link
Contributor

argoyle commented Sep 2, 2022

I have the same problem. What's needed to get this PR going again and hopefully merged?

@jensneuse
Copy link
Member

I have the same problem. What's needed to get this PR going again and hopefully merged?

Fork the PR branch. Merge master. I'll have a look to merge.

@devsergiy
Copy link
Member

closed with #406

@devsergiy devsergiy closed this Oct 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants