bug: bind game-server sockets to their own match and drop events after failed auth - #451
Merged
Merged
Conversation
…r failed auth A failed login on /ws/matches only called close(), which waits up to 30s for the closing handshake while ws keeps dispatching messages, and events sent while the password lookup was still running were never checked at all. Failed auth now terminates the socket, and the event handler refuses any socket that has not authenticated. An authenticated server could also post events for any match. Events are now accepted only from the match's server_id, and a match_map_id in the payload has to belong to that match. The lookup is cached per connection for 5s. Once a match ends its server_id is cleared, so the last server seen hosting it can still flush late events for an hour. Refused events are acknowledged so the plugin stops resending them.
- techTimeout names its map as map_id, which the binding never checked, so a server could still zero another match's timeouts. map_id is now held to the same rule as match_map_id. - captain updated the player's row in every lineup they were in, across all matches. It is now scoped to this match's two lineups. - After a match ends, its last host may only send mapStatus, chat and player-disconnected, and only for 10 minutes. Before, it could send any event, including surrender, for an hour. - Events sent while the password lookup is still running now wait for it instead of being dropped and resent 10s later out of order.
- A match now counts as ended from its status, not from server_id being cleared. The controller clears server_id only after other side effects, and until then the host could still send surrender or score into a Finished match. - After a match ends, mapStatus is accepted only for WaitingForTV, UploadingDemo and Finished. Any other status could reopen or re-award a map, such as the unplayed third map of a 2-0 Bo3. - The auth lookup gives up after 10s and terminates the socket, so a stalled Hasura cannot make an unauthenticated socket hold its messages in memory.
This file contains hidden or 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
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.
Closes two holes in the game-server websocket (
/ws/matches): events from a socket that failed auth were still processed, and an authenticated server could post events for any match.terminate(). Events wait for the auth lookup (10s timeout) and are dropped unless the socket authenticated.match_map_id/map_idin it must belong to that match. Refusals are acked and logged. The binding is cached per connection for 5s.captainclaim only updates this match's lineups.Merge/deploy: standalone; merge before api#448 goes live.
Tests: terminate on failure, the auth wait and timeout, host binding, map ownership, the post-match rules and the captain scope each fail without their code. Manual QA: play a match to the end on a dedicated server (including a surrender and the demo upload) and confirm the api logs no "game server event refused" warnings. Then reassign a live match to another server and confirm the new server's events are accepted.