From 405af0e173919bbcd6a440b84d0d8d0f425fd968 Mon Sep 17 00:00:00 2001 From: Cooldude2606 <25043174+Cooldude2606@users.noreply.github.com> Date: Tue, 6 May 2025 17:14:27 +0100 Subject: [PATCH 1/2] Add bidirectional whitelist sync --- packages/host/src/Instance.ts | 10 +++- packages/host/src/server.ts | 74 ++++++++++++++++++++++++++ packages/lib/src/config/definitions.ts | 13 ++--- test/host/Instance.js | 4 +- 4 files changed, 91 insertions(+), 10 deletions(-) diff --git a/packages/host/src/Instance.ts b/packages/host/src/Instance.ts index d1026e49..c0440d44 100644 --- a/packages/host/src/Instance.ts +++ b/packages/host/src/Instance.ts @@ -266,6 +266,15 @@ export default class Instance extends lib.Link { } }); + this.server.on("whitelist-change", (added: string[], removed: string[]) => { + for (const player of added) { + this._recordUserUpdate("WHITELISTED", player); + } + for (const player of removed) { + this._recordUserUpdate("UNWHITELISTED", player); + } + }); + this.handle(lib.InstanceExtractPlayersRequest, this.handleInstanceExtractPlayersRequest.bind(this)); this.handle(lib.InstanceAdminlistUpdateEvent, this.handleInstanceAdminlistUpdateEvent.bind(this)); this.handle(lib.InstanceBanlistUpdateEvent, this.handleInstanceBanlistUpdateEvent.bind(this)); @@ -1031,7 +1040,6 @@ end`.replace(/\r?\n/g, " "); name: string, reason: string = "" ) { - // TODO: Implement bidirectional whitelist sync, likely by watching the json file const addr = lib.Address.fromShorthand("allInstances"); const expectedIndex = this._expectedUserUpdates.findIndex( expected => expected.name === name && expected.action === action && expected.reason === reason diff --git a/packages/host/src/server.ts b/packages/host/src/server.ts index 7ff2ac20..f8c1cfb9 100644 --- a/packages/host/src/server.ts +++ b/packages/host/src/server.ts @@ -500,6 +500,10 @@ export class FactorioServer extends events.EventEmitter { _version: string | null = null; _dataDir: string | null = null; + // Due to inconsistencies in the factorio api, we must manually watch the whitelist + // https://forums.factorio.com/viewtopic.php?t=123673 + _whitelistWatcher: fs.FSWatcher | null = null; + _whitelist = new Set(); _logger: lib.Logger; _targetVersion: string; @@ -741,6 +745,62 @@ export class FactorioServer extends events.EventEmitter { } } + _watchWhitelist() { + if (this._whitelistWatcher !== null) { + throw new Error("Whitelist watcher already started"); + } + + if (!this.enableWhitelist) { + return; // The file is not updated if the whitelist is disabled + } + + const filePath = this.writePath("server-whitelist.json"); + try { + this._whitelistWatcher = fs.watch(filePath); + } catch (err: any) { + this._logger.error(`Unable to watch whitelist, bidirectional sync will not be available:\n${err}`); + return; + } + + this._whitelistWatcher.unref(); + this._whitelistWatcher.on("error", (err) => { + this._whitelistWatcher = null; + this._logger.error(`Whitelist Watcher has errored:\n${err?.stack ?? err?.message ?? err}`); + }); + this._whitelistWatcher.on("close", () => { + this._whitelistWatcher = null; + }); + this._whitelistWatcher.on("change", async (eventType) => { + if (eventType !== "change") { + this._logger.warn(`Unexpected file watcher event: ${eventType}`); + return; + } + + let newWhitelistJson; + try { + newWhitelistJson = await fs.readJSON(filePath); + } catch (err: any) { + this._logger.error(`Unable to read whitelist, bidirectional sync will not be available:\n${err}`); + return; + } + + if (!(newWhitelistJson instanceof Array) || !newWhitelistJson.every(e => typeof e === "string")) { + // The whitelist must be an array of strings + this._logger.error("Unable to read whitelist, bidirectional sync will not be available:" + + "Unexpected whitelist format"); + return; + } + + const newWhitelist = new Set(newWhitelistJson); + // Set.difference added in node v22 and so we can not use this due to v20 support + const added = newWhitelistJson.filter(e => !this._whitelist.has(e)); + const removed = [...this._whitelist.values()].filter(e => !newWhitelist.has(e)); + this._whitelist = newWhitelist; + + this.emit("whitelist-change", added, removed); + }); + } + /** Maximum number of RCON commands transmitted in parallel on the RCON connection */ get maxConcurrentCommands() { return this._maxConcurrentCommands; @@ -854,6 +914,10 @@ export class FactorioServer extends events.EventEmitter { this._rconClient.end().catch(() => {}); } + if (this._whitelistWatcher) { + this._whitelistWatcher.close(); + } + this._resetState(); this.emit("exit"); }); @@ -868,6 +932,10 @@ export class FactorioServer extends events.EventEmitter { this._rconClient.end().catch(() => {}); } + if (this._whitelistWatcher) { + this._whitelistWatcher.close(); + } + this._resetState(); this.emit("exit"); }); @@ -986,6 +1054,7 @@ export class FactorioServer extends events.EventEmitter { stdio: "pipe", } ); + this._watchWhitelist(); this._attachStdio(); this._watchExit(); /* eslint-enable node/no-sync */ @@ -1036,12 +1105,14 @@ export class FactorioServer extends events.EventEmitter { ...(this.enableWhitelist ? ["--use-server-whitelist"] : []), ...(this.enableAuthserverBans ? ["--use-authserver-bans"] : []), ...(this.verboseLogging ? ["--verbose"] : []), + ...(this.consoleLogging ? ["--console-log", this.writePath("console.log")] : []), ], { detached: true, stdio: "pipe", } ); + this._watchWhitelist(); this._attachStdio(); this._watchExit(); /* eslint-enable node/no-sync */ @@ -1117,6 +1188,9 @@ export class FactorioServer extends events.EventEmitter { if (this._rconClient) { this._rconClient.end().catch(() => {}); } + if (this._whitelistWatcher) { + this._whitelistWatcher.close(); + } this._server!.kill("SIGKILL"); } diff --git a/packages/lib/src/config/definitions.ts b/packages/lib/src/config/definitions.ts index b49e2a0a..eab8d8f6 100644 --- a/packages/lib/src/config/definitions.ts +++ b/packages/lib/src/config/definitions.ts @@ -476,12 +476,13 @@ export class InstanceConfig extends classes.Config { }, "factorio.enable_whitelist": { description: "Turn on whitelist for joining the server.", + restartRequired: true, // Because of the cli option "--use-server-whitelist" type: "boolean", initialValue: false, }, "factorio.enable_authserver_bans": { description: "Turn on Factorio.com based multiplayer bans.", - restartRequired: true, + restartRequired: true, // Because of the cli option "--use-authserver-bans" type: "boolean", initialValue: false, }, @@ -499,13 +500,13 @@ export class InstanceConfig extends classes.Config { }, "factorio.verbose_logging": { description: "Enable verbose logging on the Factorio server", - restartRequired: true, + restartRequired: true, // Because of the cli option "--verbose" type: "boolean", initialValue: false, }, "factorio.console_logging": { description: "Enable console logging to a separate file, useful for 3rd party integrations", - restartRequired: true, + restartRequired: true, // Because of the cli option "--console-log" type: "boolean", initialValue: false, }, @@ -522,10 +523,10 @@ export class InstanceConfig extends classes.Config { initialValue: "bidirectional", }, "factorio.sync_whitelist": { - description: "Synchronize whitelist with the controller", + description: "Synchronize whitelist with the controller (whitelist must be enabled)", type: "string", - enum: ["disabled", "enabled"], // TODO: Implement bidirectional - initialValue: "enabled", + enum: ["disabled", "enabled", "bidirectional"], + initialValue: "bidirectional", }, "factorio.sync_banlist": { description: "Synchronize banlist with the controller", diff --git a/test/host/Instance.js b/test/host/Instance.js index 36c5ae06..8dd66f49 100644 --- a/test/host/Instance.js +++ b/test/host/Instance.js @@ -200,7 +200,7 @@ describe("class Instance", function() { beforeEach(function() { instance.config.set("factorio.sync_banlist", "bidirectional"); instance.config.set("factorio.sync_adminlist", "bidirectional"); - // instance.config.set("factorio.sync_whitelist", "bidirectional"); // Not Implemented + instance.config.set("factorio.sync_whitelist", "bidirectional"); }); it("should send InstanceBanlistUpdateEvent for bans", function() { instance._recordUserUpdate("BAN", "player", "reason"); @@ -257,7 +257,6 @@ describe("class Instance", function() { ); }); it("should send InstanceWhitelistUpdateEvent for whitelist add", function() { - this.skip(); // Bidirectional not implemented instance._recordUserUpdate("WHITELISTED", "player"); assert.deepEqual( connector.sentMessages[0], @@ -271,7 +270,6 @@ describe("class Instance", function() { ); }); it("should send InstanceWhitelistUpdateEvent for whitelist remove", function() { - this.skip(); // Bidirectional not implemented instance._recordUserUpdate("UNWHITELISTED", "player"); assert.deepEqual( connector.sentMessages[0], From 90de1ecbf032ddd41f6af649b1009fc11fa48dea Mon Sep 17 00:00:00 2001 From: Cooldude2606 <25043174+Cooldude2606@users.noreply.github.com> Date: Tue, 6 May 2025 18:05:05 +0100 Subject: [PATCH 2/2] Fix bidirectional sync test --- test/integration/instance.js | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/test/integration/instance.js b/test/integration/instance.js index 2cc90dd6..ef94a935 100644 --- a/test/integration/instance.js +++ b/test/integration/instance.js @@ -30,6 +30,7 @@ describe("Clusterio Instance", function() { // Create a new instance await execCtl(`instance create ${instName} --id ${instId}`); + await execCtl(`instance config set ${instName} factorio.enable_whitelist true`); await execCtl(`instance config set-prop ${instName} factorio.settings visibility "${visibility}"`); await execCtl(`instance config set-prop ${instName} factorio.settings require_user_verification false`); await execCtl(`instance assign ${instName} 4`); @@ -42,6 +43,7 @@ describe("Clusterio Instance", function() { // Create a new alt instance await execCtl(`instance create ${instAltName} --id ${instAltId}`); + await execCtl(`instance config set ${instAltName} factorio.enable_whitelist true`); await execCtl(`instance config set-prop ${instAltName} factorio.settings visibility "${visibility}"`); await execCtl(`instance config set-prop ${instAltName} factorio.settings require_user_verification false`); await execCtl(`instance assign ${instAltName} 4`); @@ -98,12 +100,14 @@ describe("Clusterio Instance", function() { it("should respond to a player ban and unban event", async function() { await execCtl(`instance config set ${instName} factorio.sync_banlist bidirectional`); await sendRcon(instId, "/ban BannedPlayer"); + await lib.wait(15); // Propagation time, 10ms worked for me, so i made it 15ms const userBanned = await getUser("BannedPlayer"); assert(userBanned.isBanned, "Player is not banned"); const isAltBanned = await sendRcon(instAltId, "/banlist get BannedPlayer"); assert.equal(isAltBanned, "BannedPlayer is banned.\n", "User is not banned on alt"); await sendRcon(instId, "/unban BannedPlayer"); + await lib.wait(15); // Propagation time, 10ms worked for me, so i made it 15ms const userUnbanned = await getUser("BannedPlayer"); assert(!userUnbanned.isBanned, "Player is not unbanned"); const isAltUnbanned = await sendRcon(instAltId, "/banlist get BannedPlayer"); @@ -113,6 +117,7 @@ describe("Clusterio Instance", function() { // Because there is no admin list command this test will be different to the other two await execCtl(`instance config set ${instName} factorio.sync_adminlist bidirectional`); await sendRcon(instId, "/promote AdminPlayer"); + await lib.wait(15); // Propagation time, 10ms worked for me, so i made it 15ms const userPromote = await getUser("AdminPlayer"); assert(userPromote.isAdmin, "Player is not promoted"); const isAltPromoted = await sendRcon(instAltId, "/promote AdminPlayer"); @@ -122,6 +127,7 @@ describe("Clusterio Instance", function() { ); await sendRcon(instId, "/demote AdminPlayer"); + await lib.wait(15); // Propagation time, 10ms worked for me, so i made it 15ms const userDemote = await getUser("AdminPlayer"); assert(!userDemote.isAdmin, "Player is not demoted"); const isAltDemoted = await sendRcon(instAltId, "/demote AdminPlayer"); @@ -131,21 +137,22 @@ describe("Clusterio Instance", function() { ); }); it("should respond to a player whitelist add and remove event", async function() { - this.skip(); // Not implemented await execCtl(`instance config set ${instName} factorio.sync_whitelist bidirectional`); await sendRcon(instId, "/whitelist add WhitelistPlayer"); + await lib.wait(15); // Propagation time, 10ms worked for me, so i made it 15ms const userAdded = await getUser("WhitelistPlayer"); assert(userAdded.isWhitelisted, "Player is not whitelisted"); - const isAltAdded = await sendRcon(instAltId, "/whitelist get whitelisted"); + const isAltAdded = await sendRcon(instAltId, "/whitelist get WhitelistPlayer"); assert.equal(isAltAdded, "WhitelistPlayer is whitelisted.\n", "User is not whitelisted on alt" ); await sendRcon(instId, "/whitelist remove WhitelistPlayer"); + await lib.wait(15); // Propagation time, 10ms worked for me, so i made it 15ms const userRemoved = await getUser("WhitelistPlayer"); assert(!userRemoved.isWhitelisted, "Player is not unwhitelisted"); - const isAltRemoved = await sendRcon(instAltId, "/whitelist get whitelisted"); + const isAltRemoved = await sendRcon(instAltId, "/whitelist get WhitelistPlayer"); assert.equal(isAltRemoved, "WhitelistPlayer is not whitelisted.\n", "User is not unwhitelisted on alt"