Add bidirectional sync to admin and ban list (#709)

* Add bidirectional sync to admin and ban list

* Add tests

* Response to comments

* Skip broken tests from change of default value

packages/controller/src/HostConnection.ts

@@ -81,6 +81,9 @@ export default class HostConnection extends BaseConnection {
 		this.handle(lib.InstanceSaveDetailsUpdatesEvent, this.handleInstanceSaveDetailsUpdatesEvent.bind(this));
 		this.handle(lib.LogMessageEvent, this.handleLogMessageEvent.bind(this));
 		this.handle(lib.InstancePlayerUpdateEvent, this.handleInstancePlayerUpdateEvent.bind(this));
+		this.snoopEvent(lib.InstanceAdminlistUpdateEvent, this.snoopInstanceAdminlistUpdateEvent.bind(this));
+		this.snoopEvent(lib.InstanceBanlistUpdateEvent, this.snoopInstanceBanlistUpdateEvent.bind(this));
+		this.snoopEvent(lib.InstanceWhitelistUpdateEvent, this.snoopInstanceWhitelistUpdateEvent.bind(this));
 	}
 
 	validateIngress(message: lib.MessageRoutable) {
@@ -350,4 +353,66 @@ export default class HostConnection extends BaseConnection {
 			stats: event.stats,
 		});
 	}
+
+	async snoopInstanceAdminlistUpdateEvent(event: lib.InstanceAdminlistUpdateEvent, src: lib.Address) {
+		if (src.type !== lib.Address.instance) {
+			return;
+		}
+
+		let user = this._controller.userManager.getByName(event.name);
+		if (!user) {
+			user = this._controller.userManager.createUser(event.name);
+		}
+
+		user.isAdmin = event.admin;
+		this._controller.usersUpdated([user]);
+		const instance = this._controller.instances.get(src.id)!;
+		await lib.invokeHook(this._controller.plugins, "onPlayerEvent", instance, {
+			type: event.admin ? "promote" : "demote" as lib.PlayerEvent["type"],
+			name: event.name,
+			stats: user.instanceStats.get(src.id)!,
+		});
+	}
+
+	async snoopInstanceBanlistUpdateEvent(event: lib.InstanceBanlistUpdateEvent, src: lib.Address) {
+		if (src.type !== lib.Address.instance) {
+			return;
+		}
+
+		let user = this._controller.userManager.getByName(event.name);
+		if (!user) {
+			user = this._controller.userManager.createUser(event.name);
+		}
+
+		user.isBanned = event.banned;
+		user.banReason = event.reason;
+		this._controller.usersUpdated([user]);
+		const instance = this._controller.instances.get(src.id)!;
+		await lib.invokeHook(this._controller.plugins, "onPlayerEvent", instance, {
+			type: event.banned ? "ban" : "unban" as lib.PlayerEvent["type"],
+			name: event.name,
+			stats: user.instanceStats.get(src.id)!,
+			resaon: event.reason,
+		});
+	}
+
+	async snoopInstanceWhitelistUpdateEvent(event: lib.InstanceWhitelistUpdateEvent, src: lib.Address) {
+		if (src.type !== lib.Address.instance) {
+			return;
+		}
+
+		let user = this._controller.userManager.getByName(event.name);
+		if (!user) {
+			user = this._controller.userManager.createUser(event.name);
+		}
+
+		user.isWhitelisted = event.whitelisted;
+		this._controller.usersUpdated([user]);
+		const instance = this._controller.instances.get(src.id)!;
+		await lib.invokeHook(this._controller.plugins, "onPlayerEvent", instance, {
+			type: event.whitelisted ? "whitelisted" : "unwhitelisted" as lib.PlayerEvent["type"],
+			name: event.name,
+			stats: user.instanceStats.get(src.id)!,
+		});
+	}
 }

packages/host/modules/clusterio/impl.lua

@@ -50,6 +50,28 @@ impl.events[defines.events.on_player_left_game] = function(event)
 	})
 end
 
+impl.events[defines.events.on_player_banned] = function(event)
+	api.send_json("player_event", {
+		type = "BAN",
+		name = event.player_name,
+		reason = event.reason,
+	})
+end
+
+impl.events[defines.events.on_player_unbanned] = function(event)
+	api.send_json("player_event", { type = "UNBANNED", name = event.player_name })
+end
+
+impl.events[defines.events.on_player_promoted] = function(event)
+	local player = game.players[event.player_index]
+	api.send_json("player_event", { type = "PROMOTE", name = player.name })
+end
+
+impl.events[defines.events.on_player_demoted] = function(event)
+	local player = game.players[event.player_index]
+	api.send_json("player_event", { type = "DEMOTE", name = player.name })
+end
+
 -- Internal API
 clusterio_private = {}
 function clusterio_private.update_instance(new_id, new_name)

packages/host/src/Instance.ts

@@ -167,7 +167,7 @@ export default class Instance extends lib.Link {
 	_playerCheckInterval: ReturnType<typeof setInterval> | undefined;
 	_hadPlayersOnline = false;
 	_playerAutosaveSlot = 1;
-
+	_expectedUserUpdates: { action: string, name: string, reason: string }[] = [];
 
 	constructor(
 		host: Host,
@@ -258,6 +258,10 @@ export default class Instance extends lib.Link {
 				this._recordPlayerJoin(event.name);
 			} else if (event.type === "leave") {
 				this._recordPlayerLeave(event.name, event.reason);
+			} else if (["BAN", "UNBANNED", "PROMOTE", "DEMOTE"].includes(event.type)) {
+				this._recordUserUpdate(event.type, event.name, event.reason);
+			} else {
+				this.logger.warn(`Unknown type from player event ipc: ${event.type}`);
 			}
 		});
 
@@ -280,13 +284,15 @@ export default class Instance extends lib.Link {
 		this.handle(lib.InstanceSendRconRequest, this.handleInstanceSendRconRequest.bind(this));
 	}
 
-	_watchPlayerJoinsByChat() {
+	_watchServerLogActions() {
 		this.server.on("output", (parsed: lib.ParsedFactorioOutput, line: string) => {
 			if (parsed.type !== "action") {
 				return;
 			}
 
 			let name = /^([^ ]+)/.exec(parsed.message)![1];
+
+			// Detect player leave and join
 			if (parsed.action === "JOIN") {
 				this._recordPlayerJoin(name);
 			} else if (["LEAVE", "KICK", "BAN"].includes(parsed.action)) {
@@ -297,6 +303,14 @@ export default class Instance extends lib.Link {
 				}[parsed.action]!;
 				this._recordPlayerLeave(name, reason);
 			}
+
+			// Detect banned and admin state change, whitelist changes are not logged
+			if (parsed.action === "BAN") {
+				const reason = /\. Reason: (.+)\.$/.exec(parsed.message)![1];
+				this._recordUserUpdate(parsed.action, name, reason !== "unspecified" ? reason : "");
+			} else if (["UNBANNED", "PROMOTE", "DEMOTE"].includes(parsed.action)) {
+				this._recordUserUpdate(parsed.action as "UNBANNED" | "PROMOTE" | "DEMOTE", name);
+			}
 		});
 
 		// Leave log entries are unreliable and sometimes don't show up.
@@ -307,6 +321,22 @@ export default class Instance extends lib.Link {
 		}, 60e3);
 	}
 
+	// Needed to cover promote/demote when player is not on map, not required when _watchServerLogActions is used
+	_watchPlayerPromote() {
+		this.server.on("output", (parsed: lib.ParsedFactorioOutput, line: string) => {
+			if (
+				parsed.type !== "action"
+				|| !["PROMOTE", "DEMOTE"].includes(parsed.action)
+				|| !parsed.message.includes("be promoted upon joining the game.")
+			) {
+				return;
+			}
+
+			const name = /^([^ ]+)/.exec(parsed.message)![1];
+			this._recordUserUpdate(parsed.action as "PROMOTE" | "DEMOTE", name);
+		});
+	}
+
 	async _checkOnlinePlayers() {
 		if (this.playersOnline.size) {
 			let actualPlayers = (await this.sendRcon("/players online"))
@@ -787,8 +817,9 @@ rcon.print(game.table_to_json(players))`.replace(/\r?\n/g, " ");
 	async prepare() {
 		this.logger.verbose("Writing server-settings.json");
 		await this.writeServerSettings(true);
+		this._expectedUserUpdates = [];
 
-		if (this.config.get("factorio.sync_adminlist")) {
+		if (this.config.get("factorio.sync_adminlist") !== "disabled") {
 			this.logger.verbose("Writing server-adminlist.json");
 			lib.safeOutputFile(
 				this.server.writePath("server-adminlist.json"),
@@ -796,7 +827,7 @@ rcon.print(game.table_to_json(players))`.replace(/\r?\n/g, " ");
 			);
 		}
 
-		if (this.config.get("factorio.sync_banlist")) {
+		if (this.config.get("factorio.sync_banlist") !== "disabled") {
 			this.logger.verbose("Writing server-banlist.json");
 			lib.safeOutputFile(
 				this.server.writePath("server-banlist.json"),
@@ -806,7 +837,7 @@ rcon.print(game.table_to_json(players))`.replace(/\r?\n/g, " ");
 			);
 		}
 
-		if (this.config.get("factorio.sync_whitelist")) {
+		if (this.config.get("factorio.sync_whitelist") !== "disabled") {
 			this.logger.verbose("Writing server-whitelist.json");
 			lib.safeOutputFile(
 				this.server.writePath("server-whitelist.json"),
@@ -912,8 +943,9 @@ rcon.print(game.table_to_json(players))`.replace(/\r?\n/g, " ");
 		if (this.config.get("factorio.enable_save_patching") && this.config.get("factorio.enable_script_commands")) {
 			await this.server.disableAchievements();
 			await this.updateInstanceData();
+			this._watchPlayerPromote();
 		} else {
-			this._watchPlayerJoinsByChat();
+			this._watchServerLogActions();
 		}
 
 		await this.sendSaveListUpdate();
@@ -940,7 +972,7 @@ rcon.print(game.table_to_json(players))`.replace(/\r?\n/g, " ");
 
 		this.server.on("exit", () => this.notifyExit());
 		await this.server.startScenario(scenario, seed, mapGenSettings, mapSettings);
-		this._watchPlayerJoinsByChat();
+		this._watchServerLogActions();
 
 		await lib.invokeHook(this.plugins, "onStart");
 
@@ -977,7 +1009,7 @@ rcon.print(game.table_to_json(players))`.replace(/\r?\n/g, " ");
 			await this.sendRcon("/whitelist disable");
 		}
 
-		if (this.config.get("factorio.sync_whitelist")) {
+		if (this.config.get("factorio.sync_whitelist") !== "disabled") {
 			await this.sendRcon("/whitelist clear");
 			for (let player of this._host.whitelist) {
 				await this.sendRcon(`/whitelist ${player}`);
@@ -989,32 +1021,81 @@ rcon.print(game.table_to_json(players))`.replace(/\r?\n/g, " ");
 		}
 	}
 
+	_recordUserUpdate(
+		action: "BAN" | "UNBANNED" | "PROMOTE" | "DEMOTE" | "WHITELISTED" | "UNWHITELISTED",
+		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
+		);
+		if (expectedIndex >= 0) {
+			this._expectedUserUpdates = this._expectedUserUpdates.splice(expectedIndex, 1);
+		} else if (action === "BAN") {
+			if (this.config.get("factorio.sync_banlist") !== "bidirectional") { return; }
+			this.sendTo(addr, new lib.InstanceBanlistUpdateEvent(name, true, reason ?? ""));
+		} else if (action === "UNBANNED") {
+			if (this.config.get("factorio.sync_banlist") !== "bidirectional") { return; }
+			this.sendTo(addr, new lib.InstanceBanlistUpdateEvent(name, false, ""));
+		} else if (action === "PROMOTE") {
+			if (this.config.get("factorio.sync_adminlist") !== "bidirectional") { return; }
+			this.sendTo(addr, new lib.InstanceAdminlistUpdateEvent(name, true));
+		} else if (action === "DEMOTE") {
+			if (this.config.get("factorio.sync_adminlist") !== "bidirectional") { return; }
+			this.sendTo(addr, new lib.InstanceAdminlistUpdateEvent(name, false));
+		} else if (action === "WHITELISTED") {
+			if (this.config.get("factorio.sync_whitelist") !== "bidirectional") { return; }
+			this.sendTo(addr, new lib.InstanceWhitelistUpdateEvent(name, true));
+		} else if (action === "UNWHITELISTED") {
+			if (this.config.get("factorio.sync_whitelist") !== "bidirectional") { return; }
+			this.sendTo(addr, new lib.InstanceWhitelistUpdateEvent(name, false));
+		} else {
+			throw new Error(`Unexpected Action: ${action} for ${name}`);
+		}
+	}
+
 	async handleInstanceAdminlistUpdateEvent(request: lib.InstanceAdminlistUpdateEvent) {
-		if (!this.config.get("factorio.sync_adminlist")) {
+		const { name, admin } = request;
+		const sync = this.config.get("factorio.sync_adminlist");
+		if (sync === "disabled") {
 			return;
+		} else if (sync === "bidirectional") {
+			this._expectedUserUpdates.push({action: admin ? "PROMOTE" : "DEMOTE", name: name, reason: "" });
 		}
 
-		let { name, admin } = request;
 		let command = admin ? `/promote ${name}` : `/demote ${name}`;
 		await this.sendRcon(command);
 	}
 
 	async handleInstanceBanlistUpdateEvent(request: lib.InstanceBanlistUpdateEvent) {
-		if (!this.config.get("factorio.sync_banlist")) {
+		const { name, banned, reason } = request;
+		const sync = this.config.get("factorio.sync_banlist");
+		if (sync === "disabled") {
 			return;
+		} else if (sync === "bidirectional") {
+			this._expectedUserUpdates.push({action: banned ? "BAN" : "UNBANNED", name: name, reason: reason});
 		}
 
-		let { name, banned, reason } = request;
 		let command = banned ? `/ban ${name} ${reason}` : `/unban ${name}`;
 		await this.sendRcon(command);
 	}
 
 	async handleInstanceWhitelistUpdateEvent(request: lib.InstanceWhitelistUpdateEvent) {
-		if (!this.config.get("factorio.sync_whitelist")) {
+		const { name, whitelisted } = request;
+		const sync = this.config.get("factorio.sync_whitelist");
+		if (this.config.get("factorio.sync_whitelist") === "disabled") {
 			return;
+		} else if (sync === "bidirectional") {
+			// Factorio does not contain log events for whitelist so these values currently have no special meaning
+			this._expectedUserUpdates.push({
+				action: whitelisted ? "WHITELISTED" : "UNWHITELISTED",
+				name: name,
+				reason: "",
+			});
 		}
 
-		let { name, whitelisted } = request;
 		let command = whitelisted ? `/whitelist add ${name}` : `/whitelist remove ${name}`;
 		await this.sendRcon(command);
 	}

packages/lib/src/config/definitions.ts

@@ -334,9 +334,9 @@ export interface InstanceConfigFields {
 	"factorio.settings": Record<string, unknown>;
 	"factorio.verbose_logging": boolean;
 	"factorio.strip_paths": boolean;
-	"factorio.sync_adminlist": boolean;
-	"factorio.sync_whitelist": boolean;
-	"factorio.sync_banlist": boolean;
+	"factorio.sync_adminlist": "enabled" | "disabled" | "bidirectional";
+	"factorio.sync_whitelist": "enabled" | "disabled" | "bidirectional";
+	"factorio.sync_banlist": "enabled" | "disabled" | "bidirectional";
 	"factorio.max_concurrent_commands": number;
 }
 
@@ -347,6 +347,20 @@ export interface InstanceConfigFields {
  */
 export class InstanceConfig extends classes.Config<InstanceConfigFields> {
 	declare static fromJSON: (json: classes.ConfigSchema, location: classes.ConfigLocation) => InstanceConfig;
+	static migrations(config: Static<typeof this.jsonSchema>) {
+		function boolToEnableDisable(name: string) {
+			if (config.hasOwnProperty(name) && typeof config[name] === "boolean") {
+				config[name] = config[name] ? "enabled" : "disabled";
+			}
+		}
+
+		boolToEnableDisable("factorio.sync_adminlist");
+		boolToEnableDisable("factorio.sync_whitelist");
+		boolToEnableDisable("factorio.sync_banlist");
+
+		return config;
+	}
+
 	static fieldDefinitions: classes.ConfigDefs<InstanceConfigFields> = {
 		"instance.name": {
 			type: "string",
@@ -481,19 +495,22 @@ export class InstanceConfig extends classes.Config<InstanceConfigFields> {
 			initialValue: true,
 		},
 		"factorio.sync_adminlist": {
-			description: "Synchronize adminlist with controller",
-			type: "boolean",
-			initialValue: true,
+			description: "Synchronize adminlist with the controller",
+			type: "string",
+			enum: ["disabled", "enabled", "bidirectional"],
+			initialValue: "bidirectional",
 		},
 		"factorio.sync_whitelist": {
-			description: "Synchronize whitelist with controller",
-			type: "boolean",
-			initialValue: true,
+			description: "Synchronize whitelist with the controller",
+			type: "string",
+			enum: ["disabled", "enabled"], // TODO: Implement bidirectional
+			initialValue: "enabled",
 		},
 		"factorio.sync_banlist": {
-			description: "Synchronize banlist with controller",
-			type: "boolean",
-			initialValue: true,
+			description: "Synchronize banlist with the controller",
+			type: "string",
+			enum: ["disabled", "enabled", "bidirectional"],
+			initialValue: "bidirectional",
 		},
 		"factorio.max_concurrent_commands": {
 			description: "Maximum number of RCON commands trasmitted in parallel",

packages/lib/src/data/messages_instance.ts

@@ -864,7 +864,7 @@ export class InstanceDetailsChangedEvent {
 export class InstanceBanlistUpdateEvent {
 	declare ["constructor"]: typeof InstanceBanlistUpdateEvent;
 	static type = "event" as const;
-	static src = ["controller", "host"] as const;
+	static src = ["controller", "host", "instance"] as const;
 	static dst = "instance" as const;
 
 	constructor(
@@ -887,7 +887,7 @@ export class InstanceBanlistUpdateEvent {
 export class InstanceAdminlistUpdateEvent {
 	declare ["constructor"]: typeof InstanceAdminlistUpdateEvent;
 	static type = "event" as const;
-	static src = ["controller", "host"] as const;
+	static src = ["controller", "host", "instance"] as const;
 	static dst = "instance" as const;
 
 	constructor(
@@ -908,7 +908,7 @@ export class InstanceAdminlistUpdateEvent {
 export class InstanceWhitelistUpdateEvent {
 	declare ["constructor"]: typeof InstanceWhitelistUpdateEvent;
 	static type = "event" as const;
-	static src = ["controller", "host"] as const;
+	static src = ["controller", "host", "instance"] as const;
 	static dst = "instance" as const;
 
 	constructor(

packages/lib/src/plugin.ts

@@ -53,13 +53,14 @@ export type PluginWebpackEnvInfo = PluginDeclaration & {
  * Information about the event.
  */
 export interface PlayerEvent {
-	type: "join" | "leave" | "import";
-	/** Name of the player that joined/left */
+	type: "join" | "leave" | "import" | "promote" | "demote" | "ban" | "unban" | "whitelisted" | "unwhitelisted";
+	/** Name of the player that caused the event */
 	name: string,
 	/**
-	 * Only present for type "leave". Reason for player leaving the
+	 * Only present for type "leave" and "ban". Reason for player leaving the
 	 * game, one of the possible reasons in defines.disconnect_reason
 	 * or "server_quit" if the server exits while the player is online.
+	 * When type is "ban" this is the reason given for the ban.
 	 */
 	reason?: string;
 	/**

test/host/Instance.js

@@ -195,4 +195,112 @@ describe("class Instance", function() {
 			);
 		});
 	});
+
+	describe("_recordUserUpdate()", 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
+		});
+		it("should send InstanceBanlistUpdateEvent for bans", function() {
+			instance._recordUserUpdate("BAN", "player", "reason");
+			assert.deepEqual(
+				connector.sentMessages[0],
+				new lib.MessageEvent(
+					1, src, addr("allInstances"), "InstanceBanlistUpdateEvent",
+					new lib.InstanceBanlistUpdateEvent(
+						"player",
+						true,
+						"reason",
+					)
+				),
+			);
+		});
+		it("should send InstanceBanlistUpdateEvent for unbans", function() {
+			instance._recordUserUpdate("UNBANNED", "player");
+			assert.deepEqual(
+				connector.sentMessages[0],
+				new lib.MessageEvent(
+					1, src, addr("allInstances"), "InstanceBanlistUpdateEvent",
+					new lib.InstanceBanlistUpdateEvent(
+						"player",
+						false,
+						"",
+					)
+				),
+			);
+		});
+		it("should send InstanceAdminlistUpdateEvent for promotes", function() {
+			instance._recordUserUpdate("PROMOTE", "player");
+			assert.deepEqual(
+				connector.sentMessages[0],
+				new lib.MessageEvent(
+					1, src, addr("allInstances"), "InstanceAdminlistUpdateEvent",
+					new lib.InstanceAdminlistUpdateEvent(
+						"player",
+						true,
+					)
+				),
+			);
+		});
+		it("should send InstanceAdminlistUpdateEvent for demotes", function() {
+			instance._recordUserUpdate("DEMOTE", "player");
+			assert.deepEqual(
+				connector.sentMessages[0],
+				new lib.MessageEvent(
+					1, src, addr("allInstances"), "InstanceAdminlistUpdateEvent",
+					new lib.InstanceAdminlistUpdateEvent(
+						"player",
+						false,
+					)
+				),
+			);
+		});
+		it("should send InstanceWhitelistUpdateEvent for whitelist add", function() {
+			this.skip(); // Bidirectional not implemented
+			instance._recordUserUpdate("WHITELISTED", "player");
+			assert.deepEqual(
+				connector.sentMessages[0],
+				new lib.MessageEvent(
+					1, src, addr("allInstances"), "InstanceWhitelistUpdateEvent",
+					new lib.InstanceWhitelistUpdateEvent(
+						"player",
+						true,
+					)
+				),
+			);
+		});
+		it("should send InstanceWhitelistUpdateEvent for whitelist remove", function() {
+			this.skip(); // Bidirectional not implemented
+			instance._recordUserUpdate("UNWHITELISTED", "player");
+			assert.deepEqual(
+				connector.sentMessages[0],
+				new lib.MessageEvent(
+					1, src, addr("allInstances"), "InstanceWhitelistUpdateEvent",
+					new lib.InstanceWhitelistUpdateEvent(
+						"player",
+						false,
+					)
+				),
+			);
+		});
+		it("should throw for unknown event types", function() {
+			assert.throws(() => {
+				instance._recordUserUpdate("INVALID TYPE", "player");
+			});
+		});
+		it("should not send events when it is not bidirectional", function() {
+			for (const configValue of ["disabled", "enabled"]) { // Excludes Bidirectional
+				instance.config.set("factorio.sync_banlist", configValue);
+				instance.config.set("factorio.sync_adminlist", configValue);
+				instance.config.set("factorio.sync_whitelist", configValue);
+				for (const eventType of
+					["BAN", "UNBANNED", "PROMOTE", "DEMOTE", "WHITELISTED", "UNWHITELISTED"]
+				) {
+					instance._recordUserUpdate(eventType, "player");
+					assert.equal(connector.sentMessages[0], undefined);
+				}
+			}
+		});
+	});
 });

test/integration/clusterio.js

@@ -209,6 +209,7 @@ describe("Integration of Clusterio", function() {
 		});
 		it("should auto start instances with auto_start enabled", async function() {
 			slowTest(this);
+			this.timeout(30000); // Need an even longer timeout for this test
 
 			let hostProcess;
 			try {
@@ -661,12 +662,12 @@ describe("Integration of Clusterio", function() {
 				await execCtl("user set-banned --create test_RCON_ban --pardon");
 				const removeCommandStateLower = await sendRcon(44, "/banlist get test_rcon_ban");
 				const removeCommandStateUpper = await sendRcon(44, "/banlist get test_RCON_ban");
-				assert.equal(removeCommandStateLower, "test_rcon_ban is not banned.\n", "User is banned");
-				assert.equal(removeCommandStateUpper, "test_RCON_ban is not banned.\n", "User is banned");
+				assert.equal(removeCommandStateLower, "test_rcon_ban is not banned.\n", "User is not unbanned");
+				assert.equal(removeCommandStateUpper, "test_RCON_ban is not banned.\n", "User is not unbanned");
 
 				// Check it is case insensitive by checking it was converted correctly
 				const users = await getUsers();
-				assert(users.has("test_rcon_ban"), "User ID was not lowercase");
+				assert(users.has("test_rcon_ban"), "The first User ID was not used");
 				assert(!users.has("test_RCON_ban"), "Username was not converted to User ID");
 			});
 			it("should send whitelist commands to running instances", async function() {
@@ -693,41 +694,59 @@ describe("Integration of Clusterio", function() {
 				const removeCommandStateLower = await sendRcon(44, "/whitelist get test_rcon_whitelist");
 				const removeCommandStateUpper = await sendRcon(44, "/whitelist get test_RCON_whitelist");
 				assert.equal(removeCommandStateLower,
-					"test_rcon_whitelist is not whitelisted.\n", "User is whitelisted");
+					"test_rcon_whitelist is not whitelisted.\n", "User is not unwhitelisted");
 				assert.equal(removeCommandStateUpper,
-					"test_RCON_whitelist is not whitelisted.\n", "User is whitelisted");
+					"test_RCON_whitelist is not whitelisted.\n", "User is not unwhitelisted");
 
 				// Check it is case insensitive by checking it was converted correctly
 				const users = await getUsers();
-				assert(users.has("test_rcon_whitelist"), "User ID was not lowercase");
+				assert(users.has("test_rcon_whitelist"), "The first User ID was not used");
 				assert(!users.has("test_RCON_whitelist"), "Username was not converted to User ID");
 			});
 			it("should send admin list commands to running instances", async function() {
-				this.skip(); // It is not currently possible to get admin state without the player joining the game
+				// Because there is no admin list command this test will be different to the other two above
 				slowTest(this);
 				// Check that both names are not on the list
-				const preCommandStateLower = await sendRcon(44, "/admins get test_rcon_admin");
-				const preCommandStateUpper = await sendRcon(44, "/admins get test_RCON_admin");
-				assert.equal(preCommandStateLower, "test_rcon_admin is not admin.\n", "User is already admin");
-				assert.equal(preCommandStateUpper, "test_RCON_admin is not admin.\n", "User is already admin");
+				const preCommandStateLower = await sendRcon(44, "/demote test_rcon_admin");
+				const preCommandStateUpper = await sendRcon(44, "/demote test_RCON_admin");
+				assert.equal(preCommandStateLower,
+					"test_rcon_admin is not in the admin list.\n",
+					"User is already admin"
+				);
+				assert.equal(preCommandStateUpper,
+					"test_RCON_admin is not in the admin list.\n",
+					"User is already admin"
+				);
 
 				// Add the lower case name to the list
 				await execCtl("user set-admin --create test_rcon_admin");
-				const addCommandStateLower = await sendRcon(44, "/admins get test_rcon_admin");
-				const addCommandStateUpper = await sendRcon(44, "/admins get test_RCON_admin");
-				assert.equal(addCommandStateLower, "test_rcon_admin is admin.\n", "User is not admin");
-				assert.equal(addCommandStateUpper, "test_RCON_admin is admin.\n", "User is not admin");
+				const addCommandStateLower = await sendRcon(44, "/promote test_rcon_admin");
+				const addCommandStateUpper = await sendRcon(44, "/promote test_RCON_admin");
+				assert.equal(addCommandStateLower,
+					"test_rcon_admin is already in the admin list and will be promoted upon joining the game.\n",
+					"User is not admin"
+				);
+				assert.equal(addCommandStateUpper,
+					"test_RCON_admin is already in the admin list and will be promoted upon joining the game.\n",
+					"User is not admin"
+				);
 
 				// Remove the upper case name from the list
 				await execCtl("user set-admin --create test_RCON_admin --revoke");
-				const removeCommandStateLower = await sendRcon(44, "/admins get test_rcon_admin");
-				const removeCommandStateUpper = await sendRcon(44, "/admins get test_RCON_admin");
-				assert.equal(removeCommandStateLower, "test_rcon_admin is not admin.\n", "User is admin");
-				assert.equal(removeCommandStateUpper, "test_RCON_admin is not admin.\n", "User is admin");
+				const removeCommandStateLower = await sendRcon(44, "/demote test_rcon_admin");
+				const removeCommandStateUpper = await sendRcon(44, "/demote test_RCON_admin");
+				assert.equal(removeCommandStateLower,
+					"test_rcon_admin is not in the admin list.\n",
+					"User is not demoted"
+				);
+				assert.equal(removeCommandStateUpper,
+					"test_RCON_admin is not in the admin list.\n",
+					"User is not demoted"
+				);
 
 				// Check it is case insensitive by checking it was converted correctly
 				const users = await getUsers();
-				assert(users.has("test_rcon_admin"), "User ID was not lowercase");
+				assert(users.has("test_rcon_admin"), "The first User ID was not used");
 				assert(!users.has("test_RCON_admin"), "Username was not converted to User ID");
 			});
 		});

test/integration/instance.js

@@ -0,0 +1,157 @@
+"use strict";
+const assert = require("assert").strict;
+const lib = require("@clusterio/lib");
+
+const {
+	slowTest, exec, execCtl, sendRcon, getControl,
+} = require("./index");
+
+const instId = 48;
+const instName = "Integration";
+const instAltId = 49;
+const instAltName = "IntegrationAlt";
+
+const requireApi = [
+	"local api =",
+	"package.loaded['modules/clusterio/api']", // 1.1.110
+	"or package.loaded['__level__/modules/clusterio/api.lua']", // 2.0.0
+].join(" ");
+
+function getUser(name) {
+	return getControl().send(new lib.UserGetRequest(name));
+}
+
+describe("Clusterio Instance", function() {
+	before(async function() {
+		this.timeout(20000);
+		const visibility = JSON.stringify({ lan: true, public: false }).replace(
+			/"/g, process.platform === "win32" ? '""' : '\\"'
+		);
+
+		// Create a new instance
+		await execCtl(`instance create ${instName} --id ${instId}`);
+		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`);
+
+		for (const plugin of [
+			"global_chat", "research_sync", "statistics_exporter", "subspace_storage", "player_auth",
+		]) {
+			await execCtl(`instance config set ${instName} ${plugin}.load_plugin false`);
+		}
+
+		// Create a new alt instance
+		await execCtl(`instance create ${instAltName} --id ${instAltId}`);
+		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`);
+		await execCtl(`instance start ${instAltName}`);
+		await sendRcon(instAltId, "/sc disable achievements");
+	});
+	after(async function() {
+		this.timeout(20000);
+		// Delete the instances
+		await execCtl(`instance delete ${instName}`);
+		await execCtl(`instance stop ${instAltName}`);
+		await execCtl(`instance delete ${instAltName}`);
+	});
+	for (const savePatchingEnabled of [true, false]) {
+		describe(`Integration ${savePatchingEnabled ? "with" : "without"} save patching`, function() {
+			before(async function() {
+				this.timeout(20000);
+				// Create and start a patched save
+				await execCtl(`instance config set ${instName} factorio.enable_save_patching ${savePatchingEnabled}`);
+				await execCtl(`instance save create ${instName} ${savePatchingEnabled ? "patched" : "unpatched"}`);
+				await execCtl(`instance start ${instName} --save ${savePatchingEnabled ? "patched" : "unpatched"}.zip`);
+				await sendRcon(instId, "/sc disable achievements");
+			});
+			after(async function() {
+				// Stop the instance
+				await execCtl(`instance stop ${instName}`);
+			});
+			describe("player event", function() {
+				if (savePatchingEnabled) {
+					// IPC expects save patching to be enabled
+					it("should do nothing for an unknown type", async function() {
+						assert.rejects(getUser("DoesNotExist"), "User should not exist");
+						await sendRcon(instId,
+							`/sc ${requireApi} api.send_json("player_event",` +
+							"{ type='invalid type', name='DoesNotExist' })"
+						);
+						assert.rejects(getUser("DoesNotExist"), "User was created");
+					});
+					it("should respond to a player join and leave event", async function() {
+						await sendRcon(instId,
+							`/sc ${requireApi} api.send_json("player_event",` +
+							"{ type='join', name='JoiningPlayer' })"
+						);
+						const userJoin = await getUser("JoiningPlayer");
+						assert(userJoin.instances.has(instId), "Player is not shown as online");
+						await sendRcon(instId,
+							`/sc ${requireApi} api.send_json("player_event",` +
+							"{ type='leave', name='JoiningPlayer', reason='quit' })"
+						);
+						const userLeave = await getUser("JoiningPlayer");
+						assert(!userLeave.instances.has(instId), "Player is not shown as offline");
+					});
+				}
+				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");
+					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");
+					const userUnbanned = await getUser("BannedPlayer");
+					assert(!userUnbanned.isBanned, "Player is not unbanned");
+					const isAltUnbanned = await sendRcon(instAltId, "/banlist get BannedPlayer");
+					assert.equal(isAltUnbanned, "BannedPlayer is not banned.\n", "User is not unbanned on alt");
+				});
+				it("should respond to a player promote and demote event", async 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");
+					const userPromote = await getUser("AdminPlayer");
+					assert(userPromote.isAdmin, "Player is not promoted");
+					const isAltPromoted = await sendRcon(instAltId, "/promote AdminPlayer");
+					assert.equal(isAltPromoted,
+						"AdminPlayer is already in the admin list and will be promoted upon joining the game.\n",
+						"User is not promoted on alt"
+					);
+
+					await sendRcon(instId, "/demote AdminPlayer");
+					const userDemote = await getUser("AdminPlayer");
+					assert(!userDemote.isAdmin, "Player is not demoted");
+					const isAltDemoted = await sendRcon(instAltId, "/demote AdminPlayer");
+					assert.equal(isAltDemoted,
+						"AdminPlayer is not in the admin list.\n",
+						"User is not demoted on alt"
+					);
+				});
+				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");
+					const userAdded = await getUser("WhitelistPlayer");
+					assert(userAdded.isWhitelisted, "Player is not whitelisted");
+					const isAltAdded = await sendRcon(instAltId, "/whitelist get whitelisted");
+					assert.equal(isAltAdded,
+						"WhitelistPlayer is whitelisted.\n",
+						"User is not whitelisted on alt"
+					);
+
+					await sendRcon(instId, "/whitelist remove WhitelistPlayer");
+					const userRemoved = await getUser("WhitelistPlayer");
+					assert(!userRemoved.isWhitelisted, "Player is not unwhitelisted");
+					const isAltRemoved = await sendRcon(instAltId, "/whitelist get whitelisted");
+					assert.equal(isAltRemoved,
+						"WhitelistPlayer is not whitelisted.\n",
+						"User is not unwhitelisted on alt"
+					);
+				});
+			});
+		});
+	}
+});

test/lib/config/migrations.js

@@ -0,0 +1,49 @@
+"use strict";
+const lib = require("@clusterio/lib");
+const assert = require("assert").strict;
+
+// Each migration test should indicate which version introduced the migration
+
+describe("lib/config/migrations", function() {
+	describe("Controller Config", function() {
+		it("should migrate 'external_address' to 'public_url'", function() { // Alpha 19
+			const config = lib.ControllerConfig.fromJSON({
+				"controller.external_address": "https://example.com:4000",
+			}, "controller");
+
+			assert.equal(config.get("controller.public_url"), "https://example.com:4000",
+				"public_url contains the wrong value");
+			assert.equal(config.fields["controller.external_address"], undefined,
+				"external_address was not removed");
+		});
+	});
+	describe("Host Config", function() {
+
+	});
+	describe("Instance Config", function() {
+		it("should migrate boolean sync to string enum", function() { // Alpha 19
+			const configTrue = lib.InstanceConfig.fromJSON({
+				"factorio.sync_adminlist": true,
+				"factorio.sync_whitelist": true,
+				"factorio.sync_banlist": true,
+			}, "controller");
+
+			assert.equal(configTrue.get("factorio.sync_adminlist"), "enabled", "sync_adminlist contains wrong value");
+			assert.equal(configTrue.get("factorio.sync_whitelist"), "enabled", "sync_adminlist contains wrong value");
+			assert.equal(configTrue.get("factorio.sync_banlist"), "enabled", "sync_adminlist contains wrong value");
+
+			const configFalse = lib.InstanceConfig.fromJSON({
+				"factorio.sync_adminlist": false,
+				"factorio.sync_whitelist": false,
+				"factorio.sync_banlist": false,
+			}, "controller");
+
+			assert.equal(configFalse.get("factorio.sync_adminlist"), "disabled", "sync_adminlist contains wrong value");
+			assert.equal(configFalse.get("factorio.sync_whitelist"), "disabled", "sync_adminlist contains wrong value");
+			assert.equal(configFalse.get("factorio.sync_banlist"), "disabled", "sync_adminlist contains wrong value");
+		});
+	});
+	describe("Control Config", function() {
+
+	});
+});