diff --git a/src/server/GameManager.ts b/src/server/GameManager.ts index a18e002c5..72065a206 100644 --- a/src/server/GameManager.ts +++ b/src/server/GameManager.ts @@ -42,7 +42,7 @@ export class GameManager { persistentID: string, gameID: GameID, lastTurn: number = 0, - identityUpdate?: { username: string; clanTag?: string | null }, + identityUpdate?: { username: string; clanTag: string | null }, ): boolean { const game = this.games.get(gameID); if (!game) return false; diff --git a/src/server/GameServer.ts b/src/server/GameServer.ts index 237470877..59673e2fd 100644 --- a/src/server/GameServer.ts +++ b/src/server/GameServer.ts @@ -286,7 +286,7 @@ export class GameServer { ws: WebSocket, persistentID: string, lastTurn: number = 0, - identityUpdate?: { username: string; clanTag?: string | null }, + identityUpdate?: { username: string; clanTag: string | null }, ): boolean { const clientID = this.getClientIdForPersistentId(persistentID); if (!clientID) return false; @@ -308,12 +308,7 @@ export class GameServer { this.activeClients.push(client); if (identityUpdate && !this.hasStarted()) { client.username = identityUpdate.username; - // clanTag is only updated when explicitly provided. The reconnect - // fast-path omits it so a refreshed client can't swap to a tag the - // initial join didn't validate. - if (identityUpdate.clanTag !== undefined) { - client.clanTag = identityUpdate.clanTag; - } + client.clanTag = identityUpdate.clanTag; } client.lastPing = Date.now(); this.markClientDisconnected(client.clientID, false); diff --git a/src/server/Worker.ts b/src/server/Worker.ts index 48e61f761..b81067f5a 100644 --- a/src/server/Worker.ts +++ b/src/server/Worker.ts @@ -10,6 +10,7 @@ import { z } from "zod"; import { ClanExistsResponseSchema, clanExistsApiPath, + type UserMeResponse, } from "../core/ApiSchemas"; import { GameEnv } from "../core/configuration/Config"; import { GameType } from "../core/game/Game"; @@ -428,30 +429,18 @@ export async function startWorker() { return; } - // Normalize username and clan tag before any rejoin/join handling. + // Normalize username and clan tag. const { clanTag: censoredClanTag, username: censoredUsername } = privilegeRefresher .get() .censor(clientMsg.username, clientMsg.clanTag ?? null); - // Try to reconnect an existing client (e.g., page refresh). - // Username may have changed since initial join; clanTag is intentionally - // omitted so the reconnect can't swap to a tag that wasn't validated on - // the original join. To change clan tag, the player must fully rejoin. - if ( - gm.rejoinClient(ws, persistentId, clientMsg.gameID, 0, { - username: censoredUsername, - }) - ) { - return; - } - - let flares: string[] | undefined; - let publicId: string | undefined; - let friends: string[] = []; - let userClanTags: Set = new Set(); - + // Fetch user profile up front. Needed here so the clan-tag ownership + // check can run before the reconnect fast-path (otherwise a refresh + // would let a player swap to an unvalidated tag), and reused below + // for flares/cosmetics on new joins. const allowedFlares = ServerEnv.allowedFlares(); + let userMeResponse: UserMeResponse | null = null; if (claims === null) { if (allowedFlares !== undefined) { log.warn("Unauthorized: Anonymous user attempted to join game"); @@ -459,7 +448,6 @@ export async function startWorker() { return; } } else { - // Verify token and get player permissions const result = await getUserMe(clientMsg.token); if (result.type === "error") { log.warn(`Unauthorized: ${result.message}`, { @@ -469,29 +457,64 @@ export async function startWorker() { ws.close(1002, "Unauthorized: user me fetch failed"); return; } - flares = result.response.player.flares; - publicId = result.response.player.publicId; - friends = result.response.player.friends; - userClanTags = new Set( - (result.response.player.clans ?? []).map((c) => - c.tag.toUpperCase(), - ), - ); + userMeResponse = result.response; + } - if (allowedFlares !== undefined) { - const allowed = - allowedFlares.length === 0 || - allowedFlares.some((f) => flares?.includes(f)); - if (!allowed) { - log.warn( - "Forbidden: player without an allowed flare attempted to join game", - ); - ws.close(1002, "Forbidden"); - return; + // Enforce clan tag ownership. A player can wear a tag only if they're + // a member; if they're not and the tag belongs to a real clan, drop it + // to prevent impersonation. Fictional tags pass through. + let resolvedClanTag = censoredClanTag; + if (resolvedClanTag !== null) { + const userClanTags = new Set( + userMeResponse + ? (userMeResponse.player.clans ?? []).map((c) => + c.tag.toUpperCase(), + ) + : [], + ); + if (!userClanTags.has(resolvedClanTag.toUpperCase())) { + const exists = await clanExistsByTag(resolvedClanTag); + if (exists === true) { + log.warn("Dropped clan tag: player is not a member", { + persistentID: persistentId, + gameID: clientMsg.gameID, + clanTag: resolvedClanTag, + }); + resolvedClanTag = null; } } } + // Try to reconnect an existing client (e.g. page refresh). Pre-game, + // username and clan tag pick up the latest validated values from this + // connection. + if ( + gm.rejoinClient(ws, persistentId, clientMsg.gameID, 0, { + username: censoredUsername, + clanTag: resolvedClanTag, + }) + ) { + return; + } + + // New client — finish the join checks. + const flares = userMeResponse?.player.flares; + const publicId = userMeResponse?.player.publicId; + const friends = userMeResponse?.player.friends ?? []; + + if (userMeResponse !== null && allowedFlares !== undefined) { + const allowed = + allowedFlares.length === 0 || + allowedFlares.some((f) => flares?.includes(f)); + if (!allowed) { + log.warn( + "Forbidden: player without an allowed flare attempted to join game", + ); + ws.close(1002, "Forbidden"); + return; + } + } + const cosmeticResult = privilegeRefresher .get() .isAllowed(flares ?? [], clientMsg.cosmetics ?? {}); @@ -531,26 +554,6 @@ export async function startWorker() { } } - // Enforce clan tag ownership. A player can wear a tag only if they're - // a member; if they're not and the tag belongs to a real clan, drop it - // to prevent impersonation. Fictional tags pass through. Runs after - // turnstile so we don't burn an API call on rejected bot joins. - let resolvedClanTag = censoredClanTag; - if ( - resolvedClanTag !== null && - !userClanTags.has(resolvedClanTag.toUpperCase()) - ) { - const exists = await clanExistsByTag(resolvedClanTag); - if (exists === true) { - log.warn("Dropped clan tag: player is not a member", { - persistentID: persistentId, - gameID: clientMsg.gameID, - clanTag: resolvedClanTag, - }); - resolvedClanTag = null; - } - } - // Create client and add to game const client = new Client( generateID(), diff --git a/tests/server/GameLifecycle.test.ts b/tests/server/GameLifecycle.test.ts index 7618fb658..bf39cd9b1 100644 --- a/tests/server/GameLifecycle.test.ts +++ b/tests/server/GameLifecycle.test.ts @@ -138,20 +138,20 @@ describe("GameServer.rejoinClient — clanTag identityUpdate", () => { return client; }; - it("preserves clanTag on reconnect when identityUpdate omits it", () => { + it("updates username and clanTag on reconnect (pre-game)", () => { const game = new GameServer("g-1", mockLogger, Date.now(), { gameType: GameType.Private, } as any); const client = seedClient(game, "ABC"); - const newWs = mkWs(); - const ok = game.rejoinClient(newWs as any, "pid-1", 0, { + const ok = game.rejoinClient(mkWs() as any, "pid-1", 0, { username: "renamed", + clanTag: "XYZ", }); expect(ok).toBe(true); - expect(client.clanTag).toBe("ABC"); expect(client.username).toBe("renamed"); + expect(client.clanTag).toBe("XYZ"); }); it("clears clanTag on reconnect when identityUpdate passes null", () => { @@ -168,20 +168,6 @@ describe("GameServer.rejoinClient — clanTag identityUpdate", () => { expect(client.clanTag).toBeNull(); }); - it("updates clanTag on reconnect when identityUpdate passes a new tag", () => { - const game = new GameServer("g-3", mockLogger, Date.now(), { - gameType: GameType.Private, - } as any); - const client = seedClient(game, "ABC"); - - game.rejoinClient(mkWs() as any, "pid-1", 0, { - username: "tester", - clanTag: "XYZ", - }); - - expect(client.clanTag).toBe("XYZ"); - }); - it("does not change identity if the game has already started", () => { const game = new GameServer("g-4", mockLogger, Date.now(), { gameType: GameType.Private,