[websocket] Add a config to reject initial connection #750
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ebsocket init requests
Sonna
reviewed
Jun 18, 2019
| @@ -12,7 +12,7 @@ import ( | |||
|
|
|||
| "github.com/99designs/gqlgen/graphql" | |||
| "github.com/gorilla/websocket" | |||
| "github.com/hashicorp/golang-lru" | |||
| lru "github.com/hashicorp/golang-lru" | |||
There was a problem hiding this comment.
My IDE linter does this automatically 😬
Sonna
reviewed
Jun 18, 2019
mathewbyrne
reviewed
Jun 18, 2019
mathewbyrne
reviewed
Jun 18, 2019
eddeee888
commented
Jun 18, 2019
| @@ -30,6 +30,8 @@ type params struct { | |||
| Variables map[string]interface{} `json:"variables"` | |||
| } | |||
|
|
|||
| type websocketInitFunc func(ctx context.Context, initPayload InitPayload) error | |||
There was a problem hiding this comment.
Updated the interface to return error instead of bool
eddeee888
commented
Jun 18, 2019
| @@ -94,6 +94,14 @@ func (c *wsConnection) init() bool { | |||
| } | |||
| } | |||
|
|
|||
| if c.cfg.websocketInitFunc != nil { | |||
| if err := c.cfg.websocketInitFunc(c.ctx, c.initPayload); err != nil { | |||
| c.sendConnectionError(err.Error()) | |||
There was a problem hiding this comment.
The server now has the option to return an error. The error body is then sent to the client
vektah
approved these changes
Jun 18, 2019
cgxxv
pushed a commit
to cgxxv/gqlgen
that referenced
this pull request
Mar 25, 2022
…heck [websocket] Add a config to reject initial connection
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
Currently, we are allowing clients to always connect to the server unless the message is invalid. Current implementation here
This is not safe as a malicious user can connect to the WebSocket and perform queries/mutations. An example of such message:
We would have to check in every query/mutation/subscription to make sure that a user can perform such action (related PR: #348).
Proposal
This PR adds a new function to handler config called
websocketOnInitFunc. This can be used by the server to check the init payload to decide whether to establish the connection or reject it. If this function is not given, the connection is always established.Example workflow
This is an example workflow, different apps may have different approaches:
4a. If they are the same, create the connection
4b. If they are not the same, close the connection and send back a
invalid init payloadmessageAccepted connection:
Rejected connection:
I have: