Made user ids case insensitive

CHANGELOG.md

@@ -75,6 +75,7 @@ Many thanks to the following for contributing to this release:
 - @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)

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, content);
 			}
 
 		} 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/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.banReason.length > otherUser.banReason.length ? 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/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/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;
 

test/controller/UserManager.js

@@ -8,7 +8,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());
 

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

@@ -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) {
@@ -663,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);
@@ -691,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
@@ -714,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");
 			});
 		});
 
@@ -1278,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");
 			});
 		});
 
@@ -1310,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);
 			});
@@ -1346,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/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");
 	}