From 2436eebaa7559fcff866b59a06e4219f4f7fc7d3 Mon Sep 17 00:00:00 2001 From: Evan Date: Fri, 26 Jun 2026 14:54:57 -0700 Subject: [PATCH] fix: don't re-challenge Turnstile on lobby reconnect (#4420) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem A player who joins a **private** lobby and waits for the start timer can get an alert — `connection refused: Unauthorized: Turnstile token rejected` — the moment the game starts. Turnstile is only supposed to gate the *first* join, so this looks wrong. ## Root cause A websocket **reconnect during the lobby phase** re-sends the original Turnstile token via `joinGame()` (`ClientGameRunner.ts` lobby `onconnect` → `Transport.joinGame()`, line 417). Cloudflare Turnstile tokens are **single-use** and `lobbyConfig.turnstileToken` is never refreshed, so re-verifying the already-redeemed token returns `rejected` → `ws.close(1002, ...)` (`Worker.ts`). Normally the server skips Turnstile for reconnects: a `join` first tries `rejoinClient` and returns early if the player is a known member (`Worker.ts:359-366`). But on a **lobby-phase disconnect**, the close handler **deletes** the `persistentId → clientId` mapping to free the slot (`GameServer.ts`, `if (!this._hasStarted) { persistentIdToClientId.delete(...) }`). With the mapping gone, `rejoinClient` fails and the reconnect falls through to a full join + a doomed Turnstile re-check. **Why at game start:** `GameManager.tick()` calls `prestart()` immediately but schedules `start()` 2s later, so `_hasStarted` is still `false` for ~2s — exactly while the client runs its heavy terrain-decode + WebGL init, which stalls the ping loop and makes a socket drop (`1006` → `reconnect()`) likely. A reconnect in that window re-sends the spent token and gets rejected. ## Fix Decouple **"was admitted"** from the slot-mapping: - `GameServer` tracks `admittedPersistentIds` (populated on a successful `joinClient`) that **survives** lobby-phase disconnects, plus a `wasAdmitted()` accessor. - `GameManager.wasAdmitted(gameID, persistentID)` exposes it. - `Worker` skips the Turnstile check for an already-admitted player: `if (env !== Dev && !gm.wasAdmitted(gameID, persistentId))`. A reconnecting admitted player now proceeds through `joinClient` normally instead of failing on the spent token. ### Safety Only the Turnstile check is skipped. Every other gate still runs on every join: token-signature, ban, flares, clan tag, cosmetics, allowlist, maxPlayers, and **kick**. Genuine first joins are still challenged (no admission record yet). The set is per-game and excludes kicked players, and `persistentId` comes from the verified token so it can't be spoofed. ## Testing - New `tests/server/TurnstileReadmit.test.ts` (4 tests), incl. a regression that fires the real `ws.on("close")` handler and asserts `getClientIdForPersistentId` goes null **but `wasAdmitted` stays true**. - Full server suite: 126/126 pass · `tsc --noEmit` clean · eslint clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 --- src/server/GameManager.ts | 4 + src/server/GameServer.ts | 15 +++ src/server/Worker.ts | 10 +- tests/server/TurnstileReadmit.test.ts | 130 ++++++++++++++++++++++++++ 4 files changed, 158 insertions(+), 1 deletion(-) create mode 100644 tests/server/TurnstileReadmit.test.ts diff --git a/src/server/GameManager.ts b/src/server/GameManager.ts index ea69afcae..29270f4b7 100644 --- a/src/server/GameManager.ts +++ b/src/server/GameManager.ts @@ -49,6 +49,10 @@ export class GameManager { return game.rejoinClient(ws, persistentID, lastTurn, identityUpdate); } + wasAdmitted(gameID: GameID, persistentID: string): boolean { + return this.games.get(gameID)?.wasAdmitted(persistentID) ?? false; + } + createGame( id: GameID, gameConfig: Partial | undefined, diff --git a/src/server/GameServer.ts b/src/server/GameServer.ts index 59d22cac6..6a63e1080 100644 --- a/src/server/GameServer.ts +++ b/src/server/GameServer.ts @@ -79,6 +79,11 @@ export class GameServer { private allClients: Map = new Map(); // Map persistentID to clientID for reconnection lookup private persistentIdToClientId: Map = new Map(); + // persistentIDs that have passed authorization (incl. Turnstile) for this + // game at least once. Survives lobby-phase disconnects, unlike + // persistentIdToClientId (which is cleared to free up player slots). Lets a + // reconnecting player skip the single-use Turnstile re-check. + private admittedPersistentIds: Set = new Set(); private clientsDisconnectedStatus: Map = new Map(); private _hasStarted = false; private _startTime: number | null = null; @@ -406,6 +411,15 @@ export class GameServer { return clientID; } + // Whether this persistentID has already been admitted (passed Turnstile and + // other join authorization) for this game. Used to skip the single-use + // Turnstile re-check when an already-admitted player reconnects. Kicked + // players are excluded so a kick still forces them back through the gate. + public wasAdmitted(persistentID: string): boolean { + if (this.kickedPersistentIds.has(persistentID)) return false; + return this.admittedPersistentIds.has(persistentID); + } + public joinClient( client: Client, ): "joined" | "kicked" | "rejected" | "not_allowlisted" { @@ -488,6 +502,7 @@ export class GameServer { // Client connection accepted this.websockets.add(client.ws); this.persistentIdToClientId.set(client.persistentID, client.clientID); + this.admittedPersistentIds.add(client.persistentID); this.activeClients.push(client); client.lastPing = Date.now(); this.markClientDisconnected(client.clientID, false); diff --git a/src/server/Worker.ts b/src/server/Worker.ts index fc9bbf48d..9aab63de5 100644 --- a/src/server/Worker.ts +++ b/src/server/Worker.ts @@ -435,7 +435,15 @@ export async function startWorker() { return; } - if (ServerEnv.env() !== GameEnv.Dev) { + // Turnstile gates the FIRST join only. An already-admitted player who + // reconnects (e.g. a socket drop during the lobby->start transition, + // after which the server has cleared their reconnection mapping) must + // not be re-challenged: their original Turnstile token is single-use + // and was already redeemed, so re-verifying it would always fail. + if ( + ServerEnv.env() !== GameEnv.Dev && + !gm.wasAdmitted(clientMsg.gameID, persistentId) + ) { const turnstileResult = await verifyTurnstileToken( ip, clientMsg.turnstileToken, diff --git a/tests/server/TurnstileReadmit.test.ts b/tests/server/TurnstileReadmit.test.ts new file mode 100644 index 000000000..48fb7f7ce --- /dev/null +++ b/tests/server/TurnstileReadmit.test.ts @@ -0,0 +1,130 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +vi.mock("../../src/core/Schemas", async () => { + const actual = (await vi.importActual("../../src/core/Schemas")) as any; + return { + ...actual, + GameStartInfoSchema: { + safeParse: (data: any) => ({ success: true, data }), + }, + ServerPrestartMessageSchema: { + safeParse: (data: any) => ({ success: true, data }), + }, + ClientMessageSchema: { + safeParse: (data: any) => ({ success: true, data }), + }, + }; +}); + +import { GameType } from "../../src/core/game/Game"; +import { Client } from "../../src/server/Client"; +import { GameServer } from "../../src/server/GameServer"; + +// Stateful mock that records listeners so a test can fire the "close" event, +// exercising GameServer's real ws.on("close") handler. +function makeMockWs() { + const listeners: Record void)[]> = {}; + return { + on(event: string, cb: (...args: any[]) => void) { + (listeners[event] ??= []).push(cb); + }, + removeAllListeners() { + for (const k of Object.keys(listeners)) delete listeners[k]; + }, + emit(event: string, ...args: any[]) { + (listeners[event] ?? []).forEach((cb) => cb(...args)); + }, + send: vi.fn(), + close: vi.fn(), + readyState: 1, + }; +} + +function makeClient( + clientID: string, + persistentID: string, + ws: ReturnType, +): Client { + return new Client( + clientID, + persistentID, + null, + null, + undefined, + "127.0.0.1", + "TestUser", + null, + ws as any, + undefined, + undefined, + [], + ); +} + +describe("GameServer - wasAdmitted (Turnstile re-admission)", () => { + let mockLogger: any; + + beforeEach(() => { + vi.useFakeTimers(); + mockLogger = { + child: vi.fn().mockReturnThis(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }; + }); + + afterEach(() => { + vi.restoreAllMocks(); + vi.clearAllTimers(); + vi.useRealTimers(); + }); + + function makeGame() { + return new GameServer("test-game", mockLogger, Date.now(), { + gameType: GameType.Private, + } as any); + } + + it("reports unknown players as not admitted", () => { + const game = makeGame(); + expect(game.wasAdmitted("nobody")).toBe(false); + }); + + it("marks a player admitted after a successful join", () => { + const game = makeGame(); + expect(game.joinClient(makeClient("c1", "p1", makeMockWs()))).toBe( + "joined", + ); + expect(game.wasAdmitted("p1")).toBe(true); + }); + + // Core regression: a lobby-phase disconnect clears the reconnect mapping (to + // free the slot), but admission must survive so the reconnect skips the + // single-use Turnstile re-check instead of failing on the spent token. + it("keeps a player admitted after a lobby-phase disconnect clears their reconnect mapping", () => { + const game = makeGame(); + const ws = makeMockWs(); + expect(game.joinClient(makeClient("c1", "p1", ws))).toBe("joined"); + expect(game.getClientIdForPersistentId("p1")).toBe("c1"); + expect(game.wasAdmitted("p1")).toBe(true); + + // Socket drops before the game starts -> the close handler clears the + // persistentID->clientID mapping. + ws.emit("close"); + + expect(game.getClientIdForPersistentId("p1")).toBeNull(); + expect(game.wasAdmitted("p1")).toBe(true); + }); + + it("does not treat a kicked player as admitted (kick still forces the gate)", () => { + const game = makeGame(); + expect(game.joinClient(makeClient("c1", "p1", makeMockWs()))).toBe( + "joined", + ); + expect(game.wasAdmitted("p1")).toBe(true); + + game.kickClient("c1"); + expect(game.wasAdmitted("p1")).toBe(false); + }); +});