mirror of
https://github.com/openfrontio/OpenFrontIO.git
synced 2026-06-27 03:04:39 +00:00
fix: don't re-challenge Turnstile on lobby reconnect (#4420)
## 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 <noreply@anthropic.com>
This commit is contained in:
@@ -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<GameConfig> | undefined,
|
||||
|
||||
@@ -79,6 +79,11 @@ export class GameServer {
|
||||
private allClients: Map<ClientID, Client> = new Map();
|
||||
// Map persistentID to clientID for reconnection lookup
|
||||
private persistentIdToClientId: Map<string, ClientID> = 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<string> = new Set();
|
||||
private clientsDisconnectedStatus: Map<ClientID, boolean> = 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);
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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<string, ((...args: any[]) => 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<typeof makeMockWs>,
|
||||
): 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);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user