Merge branch 'master' into feature/2.0-scenarios

.github/workflows/node.js.yml

@@ -12,10 +12,10 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
+        node-version: [18.x, 20.x, 22.x]
+        # See supported Node.js release schedule at https://nodejs.org/en/about/releases/
         factorio-version: [1.1.110]
         # Supported versions are latest stable/experimental and one minor version before stable (even if from a previous major) 
-        node-version: [18.x, 20.x]
-        # See supported Node.js release schedule at https://nodejs.org/en/about/releases/
 
     steps:
     - uses: actions/checkout@v4
@@ -26,7 +26,8 @@ jobs:
       uses: actions/setup-node@v4
       with:
         node-version: ${{ matrix.node-version }}
-    - run: pnpm i --no-frozen-lockfile
+    - name: Prepare and install
+      run: pnpm i --no-frozen-lockfile
     - name: Use Factorio ${{ matrix.factorio-version }}
       run: wget -q -O factorio.tar.gz https://www.factorio.com/get-download/${{ matrix.factorio-version }}/headless/linux64 && tar -xf factorio.tar.gz && rm factorio.tar.gz
     - name: Run tests

CHANGELOG.md

@@ -42,6 +42,7 @@ Many thanks to the following for contributing to this release:
 - Added credential option to config entries to allow writing sensitive data that can't be read back remotely.
 - Allow hidden and readonly graphical representations of config values. [#665](https://github.com/clusterio/clusterio/pull/665).
 - Added locale exporting for all builtin mods, not just for the base mod. [#684](https://github.com/clusterio/clusterio/pull/684).
+- Added option for silent testing which only shows the results, can be ran using `pnpm silent-test` [#655](https://github.com/clusterio/clusterio/pull/655).
 
 ### Changes
 
@@ -63,10 +64,11 @@ Many thanks to the following for contributing to this release:
 - Changed shutdown logic to prefer sending a /quit command via RCON instead of using diverging logic on Windows and Linux.
 - Fixed issues with subscriptions which prevented them from working on instances or generic events [#655](https://github.com/clusterio/clusterio/pull/655).
 - Fixed unreliable datastore tests [#662](https://github.com/clusterio/clusterio/pull/662).
-- Added option for silent tests.
 - Added `controller.factorio_username` and `controller.factorio_token` config to set Factorio credentials used for the whole cluster.
 - Added `controller.share_factorio_credential_with_hosts` config to optionally require hosts to provide their own credentials.
 - Added `host.factorio_username` and `host.factorio_token` config to set Factorio credentials used on a given host.
+- Changed `instance.rcon_password` config to be a credential field that can not be read by the control interface.
+- Changed `host.id` `instance.id` and `instance.assign_host` config to be hidden fields that can not be seen on the web ui.
 - Fixed 2.0 version extraction from linux headless downloaded during installation of clusterio. [#671](https://github.com/clusterio/clusterio/pull/671)
 - Updated display name and description of `controller.external_address` to avoid confusion. [#674](https://github.com/clusterio/clusterio/pull/674)
 - Fixed invalid transient state during server start. [#676](https://github.com/clusterio/clusterio/issues/676)
@@ -79,6 +81,8 @@ Many thanks to the following for contributing to this release:
 - Previously silent errors for controller.sendEvent now throw exceptions [#625](https://github.com/clusterio/clusterio/pull/625).
 - @clusterio/controller export InstanceInfo has added factorioVersion parameter.
 - Argument order for `Config.canAccess` changed to require an access mode passed as the second argument.
+- Instances can now have `factorio.enable_script_commands` disabled which will throw an error when any script command is used over rcon. [#681](https://github.com/clusterio/clusterio/pull/681).
+- User IDs are now case insensitive, duplicate users will be automatically merged with a backup created. [#682](https://github.com/clusterio/clusterio/pull/682)
 
 Many thanks to the following for contributing to this release:  
 [@CCpersonguy](https://github.com/CCpersonguy)

package.json

@@ -5,7 +5,8 @@
 	"scripts": {
 		"build": "tsc --build",
 		"watch": "tsc --build --watch",
-		"test": "pnpm run build && mocha --check-leaks -R spec --recursive test \"plugins/*/test/**\" --exclude \"test/manual/**\"",
+		"test": "pnpm run build && mocha --enable-source-maps --check-leaks -R spec --recursive test \"plugins/*/test/**\" --exclude \"test/manual/**\"",
+		"silent-test": "SILENT_TEST=y pnpm test",
 		"fast-test": "FAST_TEST=y pnpm test",
 		"cover": "nyc pnpm test",
 		"fast-cover": "FAST_TEST=y nyc pnpm test",

packages/controller/controller.ts

@@ -139,7 +139,7 @@ async function handleBootstrapCommand(
 			return;
 		}
 
-		let admin = userManager.users.get(args.name);
+		let admin = userManager.getByName(args.name);
 		if (!admin) {
 			admin = userManager.createUser(args.name);
 		}
@@ -150,14 +150,14 @@ async function handleBootstrapCommand(
 		await userManager.save(path.join(controllerConfig.get("controller.database_directory"), "users.json"));
 
 	} else if (subCommand === "generate-user-token") {
-		let user = userManager.users.get(args.name);
+		let user = userManager.getByName(args.name);
 		if (!user) {
 			logger.error(`No user named '${args.name}'`);
 			process.exitCode = 1;
 			return;
 		}
 		// eslint-disable-next-line no-console
-		console.log(userManager.signUserToken(user.name));
+		console.log(userManager.signUserToken(user));
 
 	} else if (subCommand === "generate-host-token") {
 		// eslint-disable-next-line no-console
@@ -167,7 +167,7 @@ async function handleBootstrapCommand(
 		));
 
 	} else if (subCommand === "create-ctl-config") {
-		let admin = userManager.users.get(args.name);
+		let admin = userManager.getByName(args.name);
 		if (!admin) {
 			logger.error(`No user named '${args.name}'`);
 			process.exitCode = 1;
@@ -178,7 +178,7 @@ async function handleBootstrapCommand(
 		controlConfig.set("control.controller_url", Controller.calculateControllerUrl(controllerConfig));
 		controlConfig.set(
 			"control.controller_token",
-			userManager.signUserToken(admin.name),
+			userManager.signUserToken(admin),
 		);
 
 		let content = JSON.stringify(controlConfig, null, "\t");

packages/controller/src/ControlConnection.ts

@@ -134,7 +134,7 @@ export default class ControlConnection extends BaseConnection {
 			this.checkPermission(message, entry);
 		} catch (err: any) {
 			this.connector.sendResponseError(new lib.ResponseError(err.message, err.code), message.src);
-			logger.audit(`Permission denied for ${message.name} by ${this.user.name} from ${this.connector.dst}`);
+			logger.audit(`Permission denied for ${message.name} by ${this.user.id} from ${this.connector.dst}`);
 			throw err;
 		}
 	}
@@ -626,7 +626,7 @@ export default class ControlConnection extends BaseConnection {
 
 	async handleUserGetRequest(request: lib.UserGetRequest): Promise<lib.User> {
 		let name = request.name;
-		let user = this._controller.userManager.users.get(name);
+		let user = this._controller.userManager.getByName(name);
 		if (!user) {
 			throw new lib.RequestError(`User ${name} does not exist`);
 		}
@@ -644,17 +644,17 @@ export default class ControlConnection extends BaseConnection {
 	}
 
 	async handleUserRevokeTokenRequest(request: lib.UserRevokeTokenRequest) {
-		let user = this._controller.userManager.users.get(request.name);
+		let user = this._controller.userManager.getByName(request.name);
 		if (!user) {
 			throw new lib.RequestError(`User '${request.name}' does not exist`);
 		}
-		if (user.name !== this.user.name) {
+		if (user.id !== this.user.id) {
 			this.user.checkPermission("core.user.revoke_other_token");
 		}
 
 		user.invalidateToken();
 		for (let controlConnection of this._controller.wsServer.controlConnections.values()) {
-			if (controlConnection.user.name === user.name) {
+			if (controlConnection.user.id === user.id) {
 				controlConnection.connector.terminate();
 			}
 		}
@@ -662,7 +662,7 @@ export default class ControlConnection extends BaseConnection {
 	}
 
 	async handleUserUpdateRolesRequest(request: lib.UserUpdateRolesRequest) {
-		let user = this._controller.userManager.users.get(request.name);
+		let user = this._controller.userManager.getByName(request.name);
 		if (!user) {
 			throw new lib.RequestError(`User '${request.name}' does not exist`);
 		}
@@ -680,7 +680,7 @@ export default class ControlConnection extends BaseConnection {
 
 	async handleUserSetAdminRequest(request: lib.UserSetAdminRequest) {
 		let { name, create, admin } = request;
-		let user = this._controller.userManager.users.get(name);
+		let user = this._controller.userManager.getByName(name);
 		if (!user) {
 			if (create) {
 				this.user.checkPermission("core.user.create");
@@ -697,7 +697,7 @@ export default class ControlConnection extends BaseConnection {
 
 	async handleUserSetBannedRequest(request: lib.UserSetBannedRequest) {
 		let { name, create, banned, reason } = request;
-		let user = this._controller.userManager.users.get(name);
+		let user = this._controller.userManager.getByName(name);
 		if (!user) {
 			if (create) {
 				this.user.checkPermission("core.user.create");
@@ -715,7 +715,7 @@ export default class ControlConnection extends BaseConnection {
 
 	async handleUserSetWhitelistedRequest(request: lib.UserSetWhitelistedRequest) {
 		let { name, create, whitelisted } = request;
-		let user = this._controller.userManager.users.get(name);
+		let user = this._controller.userManager.getByName(name);
 		if (!user) {
 			if (create) {
 				this.user.checkPermission("core.user.create");
@@ -732,7 +732,7 @@ export default class ControlConnection extends BaseConnection {
 
 	async handleUserDeleteRequest(request: lib.UserDeleteRequest) {
 		let name = request.name;
-		let user = this._controller.userManager.users.get(name);
+		let user = this._controller.userManager.getByName(name);
 		if (!user) {
 			throw new lib.RequestError(`User '${name}' does not exist`);
 		}

packages/controller/src/HostConnection.ts

@@ -279,13 +279,13 @@ export default class HostConnection extends BaseConnection {
 
 		for (let user of this._controller.userManager.users.values()) {
 			if (user.isAdmin) {
-				adminlist.add(user.name);
+				adminlist.add(user.id);
 			}
 			if (user.isBanned) {
-				banlist.set(user.name, user.banReason);
+				banlist.set(user.id, user.banReason);
 			}
 			if (user.isWhitelisted) {
-				whitelist.add(user.name);
+				whitelist.add(user.id);
 			}
 		}
 
@@ -326,7 +326,7 @@ export default class HostConnection extends BaseConnection {
 
 	async handleInstancePlayerUpdateEvent(event: lib.InstancePlayerUpdateEvent, src: lib.Address) {
 		let instanceId = src.id;
-		let user = this._controller.userManager.users.get(event.name);
+		let user = this._controller.userManager.getByName(event.name);
 		if (!user) {
 			user = this._controller.userManager.createUser(event.name);
 		}

packages/controller/src/UserManager.ts

@@ -10,8 +10,8 @@ import ControllerUser from "./ControllerUser";
  * @alias module:controller/src/UserManager
  */
 export default class UserManager {
-	roles: Map<number, lib.Role> = new Map();
-	users: Map<string, ControllerUser> = new Map();
+	roles: Map<lib.Role["id"], lib.Role> = new Map();
+	users: Map<ControllerUser["id"], ControllerUser> = new Map();
 	dirty = false;
 
 	/**
@@ -24,6 +24,10 @@ export default class UserManager {
 	) {
 	}
 
+	getByName(name: string) {
+		return this.users.get(name.toLowerCase());
+	}
+
 	async load(filePath: string): Promise<void> {
 		try {
 			let content = JSON.parse(await fs.readFile(filePath, { encoding: "utf8" }));
@@ -32,9 +36,29 @@ export default class UserManager {
 				this.roles.set(role.id, role);
 			}
 
+			let duplicates = 0;
 			for (let serializedUser of content.users) {
 				let user = ControllerUser.fromJSON(serializedUser, this);
-				this.users.set(user.name, user);
+				const existingUser = this.users.get(user.id);
+				if (existingUser) {
+					// Required migration to all lowercase ids in alpha 19
+					duplicates += 1;
+					// We assume the user with the lower online time is the duplicate
+					if (user.playerStats.onlineTimeMs <= existingUser.playerStats.onlineTimeMs) {
+						existingUser.merge(user);
+						continue; // Skip users.set
+					}
+					user.merge(existingUser);
+				}
+				this.users.set(user.id, user);
+			}
+
+			if (duplicates) {
+				const backupPath = `${filePath}.${Date.now()}.bak`;
+				lib.logger.warn(
+					`A total of ${duplicates} users were merged, a backup was written to: ${backupPath}`
+				);
+				await lib.safeOutputFile(backupPath, JSON.stringify(content, null, "\t"));
 			}
 
 		} catch (err: any) {
@@ -48,7 +72,7 @@ export default class UserManager {
 		}
 	}
 
-	async save(filePath:string): Promise<void> {
+	async save(filePath: string): Promise<void> {
 		let serializedRoles = [];
 		for (let role of this.roles.values()) {
 			serializedRoles.push(role.toJSON());
@@ -72,8 +96,8 @@ export default class UserManager {
 	 * @param name - Name of the user to create.
 	 * @returns The created user.
 	 */
-	createUser(name:string): ControllerUser {
-		if (this.users.has(name)) {
+	createUser(name: string): ControllerUser {
+		if (this.getByName(name)) {
 			throw new Error(`User '${name}' already exists`);
 		}
 
@@ -84,20 +108,20 @@ export default class UserManager {
 		}
 
 		let user = new ControllerUser(this, 0, name, roles);
-		this.users.set(name, user);
+		this.users.set(user.id, user);
 		this.dirty = true;
 		return user;
 	}
 
 	/**
-	 * Sign access token for the given user name
+	 * Sign access token for the given user
 	 *
-	 * @param name - user name to sign token for
+	 * @param user - user to sign token for
 	 * @returns JWT access token for the user.
 	 */
-	signUserToken(name: string): string {
+	signUserToken(user: ControllerUser): string {
 		return jwt.sign(
-			{ aud: "user", user: name },
+			{ aud: "user", user: user.id },
 			Buffer.from(this._config.get("controller.auth_secret"), "base64")
 		);
 	}

packages/controller/src/WsServer.ts

@@ -327,7 +327,7 @@ ${err.stack}`
 				throw new Error("unexpected JsonWebToken type");
 			}
 
-			user = this.controller.userManager.users.get(tokenPayload.user);
+			user = this.controller.userManager.getByName(tokenPayload.user);
 			if (!user) {
 				throw new Error("invalid user");
 			}

packages/controller/src/routes.ts

@@ -185,7 +185,7 @@ function validateUserToken(req: Request, res: Response, next: any) {
 		if (typeof tokenPayload === "string") {
 			throw new Error("unexpected JsonWebToken type");
 		}
-		let user = req.app.locals.controller.userManager.users.get(tokenPayload.user);
+		let user = req.app.locals.controller.userManager.getByName(tokenPayload.user);
 		if (!user) {
 			throw new Error("invalid user");
 		}

packages/host/src/Instance.ts

@@ -14,6 +14,12 @@ import { exportData } from "./export";
 import type Host from "./Host";
 import BaseInstancePlugin from "./BaseInstancePlugin";
 
+const scriptCommands = [
+	"/cheat", "/editor",
+	"/command", "/c",
+	"/measured-command", "/mc",
+	"/silent-command", "/sc",
+];
 
 const instanceRconCommandDuration = new lib.Histogram(
 	"clusterio_instance_rcon_command_duration_seconds",
@@ -371,6 +377,17 @@ rcon.print(game.table_to_json(players))`.replace(/\r?\n/g, " ");
 	}
 
 	async sendRcon(message: string, expectEmpty = false, plugin = "") {
+		const trimmedMessage = message.trim();
+		if (
+			!this.config.get("factorio.enable_script_commands")
+			&& scriptCommands.find(cmd => trimmedMessage.startsWith(cmd))
+		) {
+			throw new Error(
+				"Attempted to use script command while disabled. See config factorio.enable_script_commands.\n" +
+				`Command: ${message}`
+			);
+		}
+
 		let instanceId = String(this.id);
 		let observeDuration = instanceRconCommandDuration.labels(instanceId).startTimer();
 		try {
@@ -860,7 +877,7 @@ rcon.print(game.table_to_json(players))`.replace(/\r?\n/g, " ");
 		this._loadedSave = saveName;
 		await this.server.start(saveName);
 
-		if (this.config.get("factorio.enable_save_patching")) {
+		if (this.config.get("factorio.enable_save_patching") && this.config.get("factorio.enable_script_commands")) {
 			await this.server.disableAchievements();
 			await this.updateInstanceData();
 		} else {

packages/lib/src/config/definitions.ts

@@ -319,6 +319,7 @@ export interface InstanceConfigFields {
 	"factorio.player_online_autosave_slots": number;
 	"factorio.mod_pack_id": number | null;
 	"factorio.enable_save_patching": boolean;
+	"factorio.enable_script_commands": boolean;
 	"factorio.enable_whitelist": boolean;
 	"factorio.enable_authserver_bans": boolean;
 	"factorio.settings": Record<string, unknown>;
@@ -427,6 +428,14 @@ export class InstanceConfig extends classes.Config<InstanceConfigFields> {
 			type: "boolean",
 			initialValue: true,
 		},
+		"factorio.enable_script_commands": {
+			description:
+				"Allows achievement breaking commands to be executed over rcon. " +
+				"Required for Clusterio integrations and most plugins. " +
+				"This does not prevent players using script commands.",
+			type: "boolean",
+			initialValue: true,
+		},
 		"factorio.enable_whitelist": {
 			description: "Turn on whitelist for joining the server.",
 			type: "boolean",

packages/lib/src/data/PlayerStats.ts

@@ -92,4 +92,19 @@ export default class PlayerStats {
 		}
 		return json;
 	}
+
+	merge(other: PlayerStats) {
+		this.joinCount += other.joinCount;
+		this.onlineTimeMs += other.onlineTimeMs;
+		if (!this.firstJoinAt || (other.firstJoinAt && other.firstJoinAt < this.firstJoinAt)) {
+			this.firstJoinAt = other.firstJoinAt;
+		}
+		if (!this.lastJoinAt || (other.lastJoinAt && other.lastJoinAt > this.lastJoinAt)) {
+			this.lastJoinAt = other.lastJoinAt;
+		}
+		if (!this.lastLeaveAt || (other.lastLeaveAt && other.lastLeaveAt > this.lastLeaveAt)) {
+			this.lastLeaveAt = other.lastLeaveAt;
+			this.lastLeaveReason = other.lastLeaveReason;
+		}
+	}
 }

packages/lib/src/data/User.ts

@@ -150,9 +150,39 @@ export default class User {
 		return playerStats;
 	}
 
-	/** Name of the user, used by event subscriptions */
+	/**
+	 * Merge another user into this one, must have matching ids, user is not deleted
+	 * @param otherUser - User who's details are merged from
+	*/
+	merge(otherUser: User) {
+		if (this.id !== otherUser.id) {
+			throw new Error("Cannot merge users with different ids");
+		}
+
+		// Merge properties
+		this.roleIds = new Set([...this.roleIds, ...otherUser.roleIds]);
+		this.instances = new Set([...this.instances, ...otherUser.instances]);
+		this.isAdmin = this.isAdmin && otherUser.isAdmin; // More secure to use && rather ||
+		this.isBanned = this.isBanned || otherUser.isBanned;
+		this.isBanned = this.isWhitelisted || otherUser.isWhitelisted;
+		this.banReason = this.updatedAtMs > otherUser.updatedAtMs ? this.banReason : otherUser.banReason;
+		this.updatedAtMs = Date.now();
+
+		// Merge instance stats
+		const thisInstanceStats = this.instanceStats;
+		for (const [instanceId, instanceStats] of otherUser.instanceStats.entries()) {
+			if (thisInstanceStats.has(instanceId)) {
+				thisInstanceStats.get(instanceId)!.merge(instanceStats);
+			} else {
+				thisInstanceStats.set(instanceId, instanceStats);
+			}
+		}
+		this.recalculatePlayerStats();
+	}
+
+	/** Name of the user, used by event subscriptions and to ensure case insensitivity */
 	get id() {
-		return this.name;
+		return this.name.toLowerCase();
 	}
 }
 

packages/web_ui/src/components/BaseConfigTree.tsx

@@ -44,7 +44,7 @@ function renderInput(inputComponents: Record<string, InputComponent>, def: lib.F
 	}
 	if (def.type === "string") {
 		if (def.credential) {
-			return <Input.Password autoComplete={def.autoComplete} disabled={readonly} />;
+			return <Input.Password autoComplete={def.autoComplete ?? "new-password"} disabled={readonly} />;
 		}
 		return <Input autoComplete={def.autoComplete} disabled={readonly} />;
 	}

packages/web_ui/src/model/user.tsx

@@ -73,7 +73,7 @@ export function sortLastSeen(userA: lib.User, userB: lib.User, instanceIdA?: num
 
 export function useUser(name?: string) {
 	const [users, synced] = useUsers();
-	return [name !== undefined ? users.get(name) : undefined, synced] as const;
+	return [name !== undefined ? users.get(name.toLowerCase()) : undefined, synced] as const;
 }
 
 export function useUsers() {

plugins/global_chat/instance.ts

@@ -16,6 +16,10 @@ export class InstancePlugin extends BaseInstancePlugin {
 	messageQueue: string[] = [];
 
 	async init() {
+		if (!this.instance.config.get("factorio.enable_script_commands")) {
+			throw new Error("global_chat plugin requires script commands.");
+		}
+
 		this.instance.handle(ChatEvent, this.handleChatEvent.bind(this));
 	}
 

plugins/inventory_sync/instance.ts

@@ -31,6 +31,12 @@ export class InstancePlugin extends BaseInstancePlugin {
 	disconnecting!: boolean;
 
 	async init() {
+		if (!this.instance.config.get("factorio.enable_save_patching")) {
+			throw new Error("inventory_sync plugin requires save patching.");
+		}
+		if (!this.instance.config.get("factorio.enable_script_commands")) {
+			throw new Error("inventory_sync plugin requires script commands.");
+		}
 		this.playersToRelease = new Set();
 		this.disconnecting = false;
 

plugins/player_auth/controller.ts

@@ -152,13 +152,13 @@ export class ControllerPlugin extends BaseControllerPlugin {
 		for (let [player, entry] of this.players) {
 			if (entry.playerCode === playerCode && entry.expiresMs > Date.now()) {
 				if (entry.verifyCode === verifyCode) {
-					let user = this.controller.userManager.users.get(player);
+					let user = this.controller.userManager.getByName(player);
 					if (!user) {
 						res.send({ error: true, message: "invalid user" });
 						return;
 					}
 
-					let token = this.controller.userManager.signUserToken(user.name);
+					let token = this.controller.userManager.signUserToken(user);
 					res.send({ verified: true, token });
 					return;
 

plugins/research_sync/instance.ts

@@ -30,6 +30,9 @@ export class InstancePlugin extends BaseInstancePlugin {
 		if (!this.instance.config.get("factorio.enable_save_patching")) {
 			throw new Error("research_sync plugin requires save patching.");
 		}
+		if (!this.instance.config.get("factorio.enable_script_commands")) {
+			throw new Error("research_sync plugin requires script commands.");
+		}
 
 		this.instance.server.on("ipc-research_sync:contribution", (tech: IpcContribution) => {
 			this.researchContribution(tech).catch(err => this.unexpectedError(err));

plugins/statistics_exporter/instance.ts

@@ -66,6 +66,9 @@ export class InstancePlugin extends BaseInstancePlugin {
 		if (!this.instance.config.get("factorio.enable_save_patching")) {
 			throw new Error("statistics_exporter plugin requires save patching.");
 		}
+		if (!this.instance.config.get("factorio.enable_script_commands")) {
+			throw new Error("statistics_exporter plugin requires script commands.");
+		}
 	}
 
 	async gatherMetrics() {

plugins/subspace_storage/instance.ts

@@ -20,6 +20,10 @@ export class InstancePlugin extends BaseInstancePlugin {
 	}
 
 	async init() {
+		if (!this.instance.config.get("factorio.enable_script_commands")) {
+			throw new Error("subspace_storage plugin requires script commands.");
+		}
+
 		this.pendingTasks = new Set();
 		this.instance.server.on("ipc-subspace_storage:output", (output: IpcItems) => {
 			this.provideItems(output).catch(err => this.unexpectedError(err));

test/controller/UserManager.js

@@ -1,4 +1,5 @@
 "use strict";
+const fs = require("fs-extra");
 const assert = require("assert").strict;
 const { PlayerStats } = require("@clusterio/lib");
 const { ControllerUser, UserManager } = require("@clusterio/controller");
@@ -8,7 +9,7 @@ describe("controller/src/UserManager", function() {
 		const userManager = new UserManager({});
 		it("should track online users", function() {
 			let user = ControllerUser.fromJSON({ name: "admin", roles: [1] });
-			userManager.users.set(user.name, user);
+			userManager.users.set(user.id, user);
 			assert(!userManager.onlineUsers.has(user));
 			assert.deepEqual(user.instances, new Set());
 
@@ -36,8 +37,8 @@ describe("controller/src/UserManager", function() {
 			it("should remove the instance stats from existing users", function() {
 				let user1 = ControllerUser.fromJSON({ name: "admin", roles: [1] });
 				let user2 = ControllerUser.fromJSON({ name: "player", roles: [1] });
-				userManager.users.set(user1.name, user1);
-				userManager.users.set(user2.name, user2);
+				userManager.users.set(user1.id, user1);
+				userManager.users.set(user2.id, user2);
 				user1.instanceStats.set(10, new PlayerStats({ join_count: 1 }));
 				user1.instanceStats.set(11, new PlayerStats({ join_count: 6 }));
 				user1.recalculatePlayerStats();
@@ -55,5 +56,66 @@ describe("controller/src/UserManager", function() {
 				assert.equal(user2.playerStats.joinCount, 3);
 			});
 		});
+		describe(".load()", function() {
+			it("should merge users on load", async function() {
+				// Create a user db file containing duplicate user ids
+				const names = ["user1", "User1", "user3"];
+				names.forEach(name => {
+					const user = ControllerUser.fromJSON({ name: name, roles: [1] });
+					userManager.users.set(user.name, user);
+				});
+
+				// Setup the player stats
+				const user1Before = userManager.users.get("user1");
+				user1Before.instanceStats.set(9, new PlayerStats({ join_count: 3 }));
+				user1Before.instanceStats.set(10, new PlayerStats({ join_count: 5 }));
+				user1Before.banReason = "ban reason here";
+				user1Before.isWhitelisted = true;
+				user1Before.isBanned = true;
+				user1Before.isAdmin = true;
+				user1Before.updatedAtMs = 100;
+
+				const user2Before = userManager.users.get("User1");
+				user2Before.instanceStats.set(10, new PlayerStats({ join_count: 7 }));
+				user2Before.instanceStats.set(11, new PlayerStats({ join_count: 9 }));
+				user2Before.isWhitelisted = false;
+				user2Before.isBanned = false;
+				user2Before.isAdmin = false;
+				user1Before.updatedAtMs = 200;
+
+				const user3Before = userManager.users.get("user3");
+				user3Before.instanceStats.set(10, new PlayerStats({ join_count: 11 }));
+				user3Before.instanceStats.set(11, new PlayerStats({ join_count: 13 }));
+				user3Before.isWhitelisted = false;
+				user3Before.isAdmin = true;
+
+				// Save and reload the user data
+				await fs.emptyDir("./temp/test/user_manager");
+				await userManager.save("./temp/test/user_manager/load.json");
+				names.map(name => userManager.users.delete(name));
+				await userManager.load("./temp/test/user_manager/load.json");
+
+				// Check it was loaded correctly
+				const user1After = userManager.users.get("user1");
+				const user2After = userManager.users.get("User1");
+				const user3After = userManager.users.get("user3");
+				assert.notEqual(user1After, undefined, "'user1' was not loaded");
+				assert.equal(user2After, undefined, "'User1' was loaded");
+				assert.notEqual(user3After, undefined, "'user3' was not loaded");
+
+				assert.equal(user1After.isAdmin, false, "User 1 is admin");
+				assert.equal(user1After.isBanned, true, "User 1 is not banned");
+				assert.equal(user1After.isWhitelisted, true, "User 1 is not whitelisted");
+				assert.equal(user1After.banReason, "ban reason here", "User 1 ban reason is wrong");
+				assert.equal(user1After.instanceStats.get(9)?.joinCount, 3, "User 1 instance 9 stats are wrong");
+				assert.equal(user1After.instanceStats.get(10)?.joinCount, 12, "User 1 instance 10 stats are wrong");
+				assert.equal(user1After.instanceStats.get(11)?.joinCount, 9, "User 1 instance 11 stats are wrong");
+
+				assert.equal(user3After.isAdmin, true, "User 3 is not admin");
+				assert.equal(user3After.isWhitelisted, false, "User 3 is whitelisted");
+				assert.equal(user3After.instanceStats.get(10)?.joinCount, 11, "User 3 instance 10 stats are wrong");
+				assert.equal(user3After.instanceStats.get(11)?.joinCount, 13, "User 3 instance 11 stats are wrong");
+			});
+		});
 	});
 });

test/controller/routes.js

@@ -104,7 +104,7 @@ describe("controller/src/routes", function() {
 				data: "totally a zip file",
 			});
 			assert.equal(response.statusCode, 401);
-			controller.userManager.users.get("test").tokenValidAfter = Math.floor((Date.now() + 60e3) / 1000);
+			controller.userManager.getByName("test").tokenValidAfter = Math.floor((Date.now() + 60e3) / 1000);
 			response = await phin({
 				url: `http://localhost:${port}/api/upload-save?instance_id=123&filename=file.zip`, method: "POST",
 				headers: {

test/integration/clusterio.js

@@ -13,7 +13,7 @@ const { wait } = lib;
 const testStrings = require("../lib/factorio/test_strings");
 const {
 	TestControl, TestControlConnector, url, controlToken, slowTest,
-	exec, execCtl, sendRcon, getControl, spawn, instancesDir, factorioDir,
+	exec, execCtl, sendRcon, getControl, spawnNode, instancesDir, factorioDir,
 } = require("./index");
 
 
@@ -29,7 +29,7 @@ async function checkInstanceStatus(id, status) {
 }
 
 async function spawnAltHost(config = "alt-host-config.json") {
-	return await spawn("alt-host:", `node ../../packages/host run --config ${config}`, /Started host/);
+	return await spawnNode("alt-host:", `../../packages/host run --config ${config}`, /Started host/);
 }
 
 async function startAltHost() {
@@ -97,7 +97,7 @@ async function deleteSave(instanceId, save) {
 
 async function getUsers() {
 	let users = await getControl().send(new lib.UserListRequest());
-	return new Map(users.map(user => [user.name, user]));
+	return new Map(users.map(user => [user.id, user]));
 }
 
 function jsonArg(value) {
@@ -442,6 +442,10 @@ describe("Integration of Clusterio", function() {
 		});
 
 		describe("instance send-rcon", function() {
+			this.afterAll(async function() {
+				// Prevents cascading failure where enable_script_commands is expected to be true
+				await execCtl("instance config set test factorio.enable_script_commands true");
+			});
 			it("sends the command", async function() {
 				slowTest(this);
 				await execCtl("instance send-rcon test technobabble");
@@ -465,6 +469,24 @@ describe("Integration of Clusterio", function() {
 				}
 				assert(received, "InstanceSaveDetailsUpdatesEvent not sent");
 			});
+			it("should prevent execution of script commands when disabled", async function() {
+				slowTest(this);
+				await execCtl("instance send-rcon test /c");
+				const r1 = await getControl().send(
+					new lib.LogQueryRequest(false, false, [], [44], undefined, 10, "desc")
+				);
+				assert(r1.log.some(info => /\[COMMAND\]/.test(info.message)), "Command was not sent");
+				await execCtl("instance config set test factorio.enable_script_commands false");
+
+				assert.rejects(
+					execCtl("instance send-rcon test /c"),
+					new Error(
+						"Attempted to use script command while disabled. " +
+						"See config factorio.enable_script_commands.\nCommand: /c"
+					)
+				);
+				await execCtl("instance config set test factorio.enable_script_commands true");
+			});
 		});
 
 		describe("instance config set-prop", function() {
@@ -641,6 +663,11 @@ describe("Integration of Clusterio", function() {
 				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");
+
+				// 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"), "Username was not converted to User ID");
 			});
 			it("should send whitelist commands to running instances", async function() {
 				slowTest(this);
@@ -669,6 +696,11 @@ describe("Integration of Clusterio", function() {
 					"test_rcon_whitelist is not whitelisted.\n", "User is whitelisted");
 				assert.equal(removeCommandStateUpper,
 					"test_RCON_whitelist is not whitelisted.\n", "User is whitelisted");
+
+				// 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"), "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
@@ -692,6 +724,11 @@ describe("Integration of Clusterio", function() {
 				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");
+
+				// 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"), "Username was not converted to User ID");
 			});
 		});
 
@@ -1256,10 +1293,10 @@ describe("Integration of Clusterio", function() {
 				getControl().userUpdates = [];
 				await execCtl("user create temp");
 				let users = await getControl().send(new lib.UserListRequest());
-				let tempUser = users.find(user => user.name === "temp");
+				let tempUser = users.find(user => user.id === "temp");
 				assert(tempUser, "user was not created");
 				assert.equal(getControl().userUpdates.length, 1);
-				assert.equal(getControl().userUpdates[0].name, "temp");
+				assert.equal(getControl().userUpdates[0].id, "temp");
 			});
 		});
 
@@ -1288,7 +1325,7 @@ describe("Integration of Clusterio", function() {
 				getControl().userUpdates = [];
 				await execCtl('user set-roles temp "Cluster Admin"');
 				let users = await getControl().send(new lib.UserListRequest());
-				let tempUser = users.find(user => user.name === "temp");
+				let tempUser = users.find(user => user.id === "temp");
 				assert.deepEqual(tempUser.roleIds, new Set([0]));
 				assert.equal(getControl().userUpdates.length, 1);
 			});
@@ -1324,7 +1361,7 @@ describe("Integration of Clusterio", function() {
 				getControl().userUpdates = [];
 				await execCtl("user delete temp");
 				let users = await getControl().send(new lib.UserListRequest());
-				let tempUser = users.find(user => user.name === "temp");
+				let tempUser = users.find(user => user.id === "temp");
 				assert(!tempUser, "user was note deleted");
 				assert.equal(getControl().userUpdates.length, 1);
 				assert.equal(getControl().userUpdates[0].isDeleted, true);

test/integration/index.js

@@ -179,7 +179,7 @@ async function exec(command, options = {}) {
 }
 
 async function execCtl(...args) {
-	args[0] = `node ../../packages/ctl ${args[0]}`;
+	args[0] = `node --enable-source-maps ../../packages/ctl ${args[0]}`;
 	return await exec(...args);
 }
 
@@ -221,6 +221,10 @@ function spawn(name, cmd, waitFor) {
 	});
 }
 
+async function spawnNode(name, cmd, waitFor) {
+	return await spawn(name, `node --enable-source-maps ${cmd}`, waitFor);
+}
+
 before(async function() {
 	this.timeout(40000);
 
@@ -302,7 +306,7 @@ before(async function() {
 	await exec("node ../../packages/controller bootstrap create-ctl-config test");
 	await exec("node ../../packages/ctl control-config set control.tls_ca ../../test/file/tls/cert.pem");
 
-	controllerProcess = await spawn("controller:", "node ../../packages/controller run", /Started controller/);
+	controllerProcess = await spawnNode("controller:", "../../packages/controller run", /Started controller/);
 
 	await execCtl("host create-config --id 4 --name host --generate-token");
 
@@ -310,7 +314,7 @@ before(async function() {
 	await exec(`node ../../packages/host config set host.factorio_directory ${relativeFactorioDir}`);
 	await exec("node ../../packages/host config set host.tls_ca ../../test/file/tls/cert.pem");
 
-	hostProcess = await spawn("host:", "node ../../packages/host run", /Started host/);
+	hostProcess = await spawnNode("host:", "../../packages/host run", /Started host/);
 
 	let tlsCa = await fs.readFile("test/file/tls/cert.pem");
 	let controlConnector = new TestControlConnector(url, 2, tlsCa);
@@ -366,6 +370,7 @@ module.exports = {
 	sendRcon,
 	getControl,
 	spawn,
+	spawnNode,
 
 	url,
 	controlToken,

test/integration/routing.js

@@ -53,7 +53,7 @@ function connectInstance(controller, host, instanceId) {
 function connectControl(controller, controlId) {
 	const [controllerSide, controlSide] = lib.VirtualConnector.makePair(addr("controller"), addr({ controlId }));
 	const registerData = new lib.RegisterControlData("", "0.0.0");
-	let user = controller.userManager.users.get("test");
+	let user = controller.userManager.getByName("test");
 	if (!user) {
 		user = controller.userManager.createUser("test");
 	}

test/mock.js

@@ -129,6 +129,7 @@ class MockInstance extends lib.Link {
 		this.mockConfigEntries = new Map([
 			["instance.id", 7357],
 			["factorio.enable_save_patching", true],
+			["factorio.enable_script_commands", true],
 		]);
 		this.config = {
 			get: (name) => {