diff --git a/.github/workflows/node.js.yml b/.github/workflows/node.js.yml index 62d6c827..87011b64 100644 --- a/.github/workflows/node.js.yml +++ b/.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 diff --git a/CHANGELOG.md b/CHANGELOG.md index 84356d09..436827f4 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/package.json b/package.json index 34397619..468452c0 100644 --- a/package.json +++ b/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", diff --git a/packages/controller/controller.ts b/packages/controller/controller.ts index b4b9d4e2..d9bdaf9c 100644 --- a/packages/controller/controller.ts +++ b/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"); diff --git a/packages/controller/src/ControlConnection.ts b/packages/controller/src/ControlConnection.ts index ba59deaf..8ef23516 100644 --- a/packages/controller/src/ControlConnection.ts +++ b/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 { 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`); } diff --git a/packages/controller/src/HostConnection.ts b/packages/controller/src/HostConnection.ts index 6da8e93c..3b2155f1 100644 --- a/packages/controller/src/HostConnection.ts +++ b/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); } diff --git a/packages/controller/src/UserManager.ts b/packages/controller/src/UserManager.ts index e9239428..9d05b538 100644 --- a/packages/controller/src/UserManager.ts +++ b/packages/controller/src/UserManager.ts @@ -10,8 +10,8 @@ import ControllerUser from "./ControllerUser"; * @alias module:controller/src/UserManager */ export default class UserManager { - roles: Map = new Map(); - users: Map = new Map(); + roles: Map = new Map(); + users: Map = 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 { 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 { + async save(filePath: string): Promise { 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") ); } diff --git a/packages/controller/src/WsServer.ts b/packages/controller/src/WsServer.ts index 9def1c50..c303eb10 100644 --- a/packages/controller/src/WsServer.ts +++ b/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"); } diff --git a/packages/controller/src/routes.ts b/packages/controller/src/routes.ts index 068bf3e5..1e2b3b1f 100644 --- a/packages/controller/src/routes.ts +++ b/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"); } diff --git a/packages/host/src/Instance.ts b/packages/host/src/Instance.ts index c7208f0e..5fbe09fb 100644 --- a/packages/host/src/Instance.ts +++ b/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 { diff --git a/packages/lib/src/config/definitions.ts b/packages/lib/src/config/definitions.ts index 0584f4c3..c26ddf56 100644 --- a/packages/lib/src/config/definitions.ts +++ b/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; @@ -427,6 +428,14 @@ export class InstanceConfig extends classes.Config { 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", diff --git a/packages/lib/src/data/PlayerStats.ts b/packages/lib/src/data/PlayerStats.ts index 61ca3aed..0f297e8d 100644 --- a/packages/lib/src/data/PlayerStats.ts +++ b/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; + } + } } diff --git a/packages/lib/src/data/User.ts b/packages/lib/src/data/User.ts index 9cae6fde..641c3cab 100644 --- a/packages/lib/src/data/User.ts +++ b/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(); } } diff --git a/packages/web_ui/src/components/BaseConfigTree.tsx b/packages/web_ui/src/components/BaseConfigTree.tsx index 710aead8..606a6329 100644 --- a/packages/web_ui/src/components/BaseConfigTree.tsx +++ b/packages/web_ui/src/components/BaseConfigTree.tsx @@ -44,7 +44,7 @@ function renderInput(inputComponents: Record, def: lib.F } if (def.type === "string") { if (def.credential) { - return ; + return ; } return ; } diff --git a/packages/web_ui/src/model/user.tsx b/packages/web_ui/src/model/user.tsx index 69536cf3..981a57e0 100644 --- a/packages/web_ui/src/model/user.tsx +++ b/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() { diff --git a/plugins/global_chat/instance.ts b/plugins/global_chat/instance.ts index 2cfb2c6e..10c0401c 100644 --- a/plugins/global_chat/instance.ts +++ b/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)); } diff --git a/plugins/inventory_sync/instance.ts b/plugins/inventory_sync/instance.ts index c831e8fd..275d033e 100644 --- a/plugins/inventory_sync/instance.ts +++ b/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; diff --git a/plugins/player_auth/controller.ts b/plugins/player_auth/controller.ts index 77832ee7..01a69aef 100644 --- a/plugins/player_auth/controller.ts +++ b/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; diff --git a/plugins/research_sync/instance.ts b/plugins/research_sync/instance.ts index 0b47531f..c905ff34 100644 --- a/plugins/research_sync/instance.ts +++ b/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)); diff --git a/plugins/statistics_exporter/instance.ts b/plugins/statistics_exporter/instance.ts index 76502173..b824992d 100644 --- a/plugins/statistics_exporter/instance.ts +++ b/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() { diff --git a/plugins/subspace_storage/instance.ts b/plugins/subspace_storage/instance.ts index 9771eb26..cc9be259 100644 --- a/plugins/subspace_storage/instance.ts +++ b/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)); diff --git a/test/controller/UserManager.js b/test/controller/UserManager.js index a25067c3..da01ef92 100644 --- a/test/controller/UserManager.js +++ b/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"); + }); + }); }); }); diff --git a/test/controller/routes.js b/test/controller/routes.js index e69449ca..b2d5d07d 100644 --- a/test/controller/routes.js +++ b/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: { diff --git a/test/integration/clusterio.js b/test/integration/clusterio.js index 0c4fba15..1a0702a5 100644 --- a/test/integration/clusterio.js +++ b/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); diff --git a/test/integration/index.js b/test/integration/index.js index b9289881..5a2be83e 100644 --- a/test/integration/index.js +++ b/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, diff --git a/test/integration/routing.js b/test/integration/routing.js index 3b423786..f2b17d34 100644 --- a/test/integration/routing.js +++ b/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"); } diff --git a/test/mock.js b/test/mock.js index d1c6b30d..47bba6aa 100644 --- a/test/mock.js +++ b/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) => {