From 61236879b72b750c3d80e6ddf6c371ebbaf0c46f Mon Sep 17 00:00:00 2001 From: Zixer1 <99333209+Zixer1@users.noreply.github.com> Date: Thu, 25 Jun 2026 12:53:13 -0400 Subject: [PATCH] fix: kick_player can target a disconnected account by publicId (#4409) ## Description: kick_player resolved a targetPublicID only against activeClients, but a client is dropped from activeClients on socket close (so players technically can disconnect right before getting kicked, then reconnect at a later point and continue playing), aka a disconnected account cannot not be kicked. Fall back to allClients (which persists) so the kick lands and bans the persistentID, blocking rejoin and reconnect. ## Please complete the following: - [x] I have added screenshots for all UI updates - [x] I process any text displayed to the user through translateText() and I've added it to the en.json file - [x] I have added relevant tests to the test directory ## Please put your Discord username so you can be contacted if a bug or regression is found: zixer._ --- src/server/GameServer.ts | 9 ++++---- tests/server/AdminBotIntent.test.ts | 34 +++++++++++++++++++++++------ 2 files changed, 32 insertions(+), 11 deletions(-) diff --git a/src/server/GameServer.ts b/src/server/GameServer.ts index c304d0ecc..827a7347e 100644 --- a/src/server/GameServer.ts +++ b/src/server/GameServer.ts @@ -276,12 +276,13 @@ export class GameServer { error: "only the lobby creator or an admin can kick players", }; } - // Resolve the target to a live clientID: an explicit clientID, or an - // account publicId matched against the connected clients (for callers - // that know the account but not the per-session clientID). + // Resolve the target to a clientID: an explicit clientID, or an account + // publicId matched against allClients (a superset of activeClients that + // retains disconnected players), so a disconnected account can still be + // kicked — its persistentID is banned, blocking rejoin/reconnect. let target = stamped.targetClientID; if (target === undefined && stamped.targetPublicID !== undefined) { - target = this.activeClients.find( + target = [...this.allClients.values()].find( (c) => c.publicId === stamped.targetPublicID, )?.clientID; } diff --git a/tests/server/AdminBotIntent.test.ts b/tests/server/AdminBotIntent.test.ts index 232d18a1a..d504973a6 100644 --- a/tests/server/AdminBotIntent.test.ts +++ b/tests/server/AdminBotIntent.test.ts @@ -131,13 +131,13 @@ describe("GameServer.handleIntent (admin bot)", () => { ).toBe(403); }); - it("resolves a publicID target to the connected client's clientID", () => { + it("resolves a publicID target to a connected client's clientID", () => { const game = makeGame(); - (game as any).activeClients.push({ - clientID: "liveCID1", - publicId: "pubABCD1", - }); - const spy = vi.spyOn(game, "kickClient"); + // A connected client is in both lists; allClients is the superset we match on. + const connected = { clientID: "liveCID1", publicId: "pubABCD1" }; + (game as any).activeClients.push(connected); + (game as any).allClients.set("liveCID1", connected); + const spy = vi.spyOn(game, "kickClient").mockImplementation(() => {}); const result = apply(game, { type: "kick_player", targetPublicID: "pubABCD1", @@ -146,7 +146,27 @@ describe("GameServer.handleIntent (admin bot)", () => { expect(spy).toHaveBeenCalledWith("liveCID1", expect.any(String)); }); - it("404s when no connected client matches the publicID", () => { + it("kicks a disconnected account by publicID via allClients (bans its persistentID)", () => { + const game = makeGame(); + // Disconnected: still known to the game (allClients) but already dropped + // from activeClients on socket close. Must stay kickable so the + // persistentID ban fires and blocks a rejoin/reconnect. + (game as any).allClients.set("goneCID1", { + clientID: "goneCID1", + publicId: "pubGONE1", + persistentID: "persist-gone-1", + }); + const result = apply(game, { + type: "kick_player", + targetPublicID: "pubGONE1", + } as any); + expect(result.status).toBe(200); + expect((game as any).kickedPersistentIds.has("persist-gone-1")).toBe( + true, + ); + }); + + it("404s when no client matches the publicID", () => { const game = makeGame(); expect( apply(game, {