diff --git a/packages/ctl/src/commands.js b/packages/ctl/src/commands.js index 4f1c03ce..9e0cc35d 100644 --- a/packages/ctl/src/commands.js +++ b/packages/ctl/src/commands.js @@ -1480,6 +1480,15 @@ userCommands.add(new libCommand.Command({ }, })); +userCommands.add(new libCommand.Command({ + definition: ["revoke-token ", "Revoke token for user", (yargs) => { + yargs.positional("name", { describe: "Name of user to revoke token for", type: "string" }); + }], + handler: async function(args, control) { + await libLink.messages.revokeUserToken.send(control, { name: args.name }); + }, +})); + userCommands.add(new libCommand.Command({ definition: ["set-admin ", "Promote or demote a user to admin", (yargs) => { yargs.positional("user", { describe: "Name of user set admin status for", type: "string" }); diff --git a/packages/lib/config/classes.js b/packages/lib/config/classes.js index 0be19776..ac1591b0 100644 --- a/packages/lib/config/classes.js +++ b/packages/lib/config/classes.js @@ -677,6 +677,12 @@ class ConfigGroup { * @param {Array=} item.enum - * Array of all values that are acceptible. Useful for making * enumerated values. + * @param {boolean=} item.restartRequired - + * True if a restart of the entity this config is attached to is + * required for changes to take effect. Informative only, defaults + * to false. + * @param {Array=} item.restartRequiredProps - + * Properties to invert the value of restartRequired for if present. * @param {boolean=} item.optional - * True if this field can be set to null. Defaults to false. * @param {*=} item.initial_value - @@ -706,6 +712,7 @@ class ConfigGroup { // Validate properties let validTypes = ["boolean", "string", "number", "object"]; + // eslint-disable-next-line complexity Object.keys(item).forEach(key => { let value = item[key]; let valueType = basicType(value); @@ -746,6 +753,18 @@ class ConfigGroup { } break; + case "restartRequired": + if (valueType !== "boolean") { + throw new Error("restartRequired must be a boolean"); + } + break; + + case "restartRequiredProps": + if (valueType !== "array") { + throw new Error("restartRequiredProps must be a array"); + } + break; + case "optional": if (valueType !== "boolean") { throw new Error("optional must be a boolean"); @@ -771,9 +790,14 @@ class ConfigGroup { access: this.defaultAccess, optional: false, fullName: `${this.groupName}.${item.name}`, + restartRequired: false, ...item, }; + if (item.restartRequiredProps && item.type !== "object") { + throw new Error("restartRequiredProps is only allowed if type is object"); + } + if (!computed.access) { throw new Error(`access not set and no defaultAccess defined for ${this.name}`); } @@ -815,6 +839,7 @@ class PluginConfigGroup extends ConfigGroup { this.define({ name: "load_plugin", title: "Load Plugin", + restartRequired: true, type: "boolean", initial_value: true, }); diff --git a/packages/lib/config/definitions.js b/packages/lib/config/definitions.js index d6605c85..5165d021 100644 --- a/packages/lib/config/definitions.js +++ b/packages/lib/config/definitions.js @@ -37,6 +37,7 @@ MasterGroup.define({ name: "http_port", title: "HTTP Port", description: "Port to listen for HTTP connections on, set to null to not listen for HTTP connections.", + restartRequired: true, type: "number", optional: true, initial_value: 8080, @@ -45,6 +46,7 @@ MasterGroup.define({ name: "https_port", title: "HTTPS Port", description: "Port to listen for HTTPS connection on, set to null to not listen for HTTPS connections.", + restartRequired: true, type: "number", optional: true, }); @@ -52,6 +54,7 @@ MasterGroup.define({ name: "bind_address", title: "Bind Address", description: "IP address to bind the HTTP and HTTPS ports on.", + restartRequired: true, type: "string", optional: true, }); @@ -66,6 +69,7 @@ MasterGroup.define({ name: "tls_certificate", title: "TLS Certificate", description: "Path to the certificate to use for HTTPS.", + restartRequired: true, type: "string", optional: true, }); @@ -73,6 +77,7 @@ MasterGroup.define({ name: "tls_private_key", title: "TLS Private Key", description: "Path to the private key to use for HTTPS.", + restartRequired: true, type: "string", optional: true, }); @@ -84,6 +89,7 @@ MasterGroup.define({ "Secret used to generate and verify authentication tokens. " + "Should be a long string of random letters and numbers. " + "Do not share this.", + restartRequired: true, type: "string", optional: true, }); @@ -184,24 +190,28 @@ SlaveGroup.define({ SlaveGroup.define({ name: "instances_directory", description: "Path to directory to store instances in.", + restartRequired: true, type: "string", initial_value: "instances", }); SlaveGroup.define({ name: "master_url", description: "URL to connect to the master server at", + restartRequired: true, type: "string", initial_value: "http://localhost:8080/", }); SlaveGroup.define({ name: "master_token", description: "Token to authenticate to master server with.", + restartRequired: true, type: "string", initial_value: "enter token here", }); SlaveGroup.define({ name: "tls_ca", description: "Path to Certificate Authority to validate TLS connection to master against.", + restartRequired: true, type: "string", optional: true, }); @@ -244,7 +254,7 @@ InstanceGroup.define({ }); InstanceGroup.define({ name: "id", - description: "ID of the slave", + description: "ID of the instance", type: "number", initial_value: () => Math.random() * 2**31 | 0, }); @@ -272,24 +282,28 @@ FactorioGroup.groupName = "factorio"; FactorioGroup.define({ name: "version", description: "Version of the game to run, use latest to run the latest installed version", + restartRequired: true, type: "string", initial_value: "latest", }); FactorioGroup.define({ name: "game_port", description: "UDP port to run game on, uses a random port if null", + restartRequired: true, type: "number", optional: true, }); FactorioGroup.define({ name: "rcon_port", description: "TCP port to run RCON on, uses a random port if null", + restartRequired: true, type: "number", optional: true, }); FactorioGroup.define({ name: "rcon_password", description: "Password for RCON, randomly generated if null", + restartRequired: true, type: "string", optional: true, }); @@ -304,12 +318,14 @@ FactorioGroup.define({ FactorioGroup.define({ name: "mod_pack", description: "Mod pack to use on this server", + restartRequired: true, type: "number", optional: true, }); FactorioGroup.define({ name: "enable_save_patching", description: "Patch saves with Lua code. Required for Clusterio integrations, lua modules, and most plugins.", + restartRequired: true, type: "boolean", initial_value: true, }); @@ -322,24 +338,34 @@ FactorioGroup.define({ FactorioGroup.define({ name: "enable_authserver_bans", description: "Turn on Factorio.com based multiplayer bans.", + restartRequired: true, type: "boolean", initial_value: false, }); FactorioGroup.define({ name: "settings", description: "Settings overridden in server-settings.json", + restartRequired: true, + restartRequiredProps: [ + "afk_autokick_interval", "allow_commands", "autosave_interval", "autosave_only_on_server", "description", + "ignore_player_limit_for_returning_players", "max_players", "max_upload_slots", + "max_upload_in_kilobytes_per_second", "name", "only_admins_can_pause_the_game", "game_password", + "require_user_verification", "tags", "visibility", + ], type: "object", initial_value: {}, // See create instance handler in master. }); FactorioGroup.define({ name: "verbose_logging", description: "Enable verbose logging on the Factorio server", + restartRequired: true, type: "boolean", initial_value: false, }); FactorioGroup.define({ name: "strip_paths", description: "Strip down instance paths in the log", + restartRequired: true, type: "boolean", initial_value: true, }); @@ -364,6 +390,7 @@ FactorioGroup.define({ FactorioGroup.define({ name: "max_concurrent_commands", description: "Maximum number of RCON commands trasmitted in parallel", + restartRequired: true, type: "number", initial_value: 5, }); diff --git a/packages/lib/factorio/patch.js b/packages/lib/factorio/patch.js index 7f92232f..8e0c6a0f 100644 --- a/packages/lib/factorio/patch.js +++ b/packages/lib/factorio/patch.js @@ -251,8 +251,14 @@ async function patch(savePath, modules) { // Write back the save let tempSavePath = `${savePath}.tmp`; let stream = zip.generateNodeStream({ compression: "DEFLATE" }); - let pipe = stream.pipe(fs.createWriteStream(tempSavePath)); - await events.once(pipe, "finish"); + let fd = await fs.open(tempSavePath, "w"); + try { + let pipe = stream.pipe(fs.createWriteStream("", { fd, autoClose: false })); + await events.once(pipe, "finish"); + await fs.fsync(fd); + } finally { + await fs.close(fd); + } await fs.rename(tempSavePath, savePath); } diff --git a/packages/lib/link/messages.js b/packages/lib/link/messages.js index 21a063db..5fa4707b 100644 --- a/packages/lib/link/messages.js +++ b/packages/lib/link/messages.js @@ -1036,6 +1036,15 @@ messages.createUser = new Request({ }, }); +messages.revokeUserToken = new Request({ + type: "revoke_user_token", + links: ["control-master"], + permission: "core.user.revoke_token", + requestProperties: { + "name": { type: "string" }, + }, +}); + messages.updateUserRoles = new Request({ type: "update_user_roles", links: ["control-master"], diff --git a/packages/lib/users.js b/packages/lib/users.js index 00b90de9..1e0156cb 100644 --- a/packages/lib/users.js +++ b/packages/lib/users.js @@ -637,6 +637,17 @@ definePermission({ title: "Create users", description: "Create user acounts with a given name.", }); +definePermission({ + name: "core.user.revoke_token", + title: "Revoke user token", + description: "Revoke access token and terminate all existing control connections for oneself.", + grantByDefault: true, +}); +definePermission({ + name: "core.user.revoke_other_token", + title: "Revoke other user's token", + description: "Allow revoking tokens for other users than oneself. Requires Revoke user token.", +}); definePermission({ name: "core.user.update_roles", title: "Update user roles", diff --git a/packages/master/src/ControlConnection.js b/packages/master/src/ControlConnection.js index f695156e..5fe52b8f 100644 --- a/packages/master/src/ControlConnection.js +++ b/packages/master/src/ControlConnection.js @@ -20,9 +20,9 @@ const routes = require("./routes"); const strcmp = new Intl.Collator(undefined, { numerice: "true", sensitivity: "base" }).compare; -const lastQueryLogTime = new libPrometheus.Gauge( - "clusterio_master_last_query_log_duration_seconds", - "Time in seconds the last log query took to execute." +const queryLogTime = new libPrometheus.Summary( + "clusterio_master_query_log_duration_seconds", + "Time in seconds log queries took to execute." ); /** @@ -683,7 +683,7 @@ class ControlConnection extends BaseConnection { } async queryLogRequestHandler(message) { - let setDuration = lastQueryLogTime.startTimer(); + let observeDuration = queryLogTime.startTimer(); let { all, master, slave_ids, instance_ids } = message.data; let log; @@ -693,7 +693,7 @@ class ControlConnection extends BaseConnection { log = await this._master.queryClusterLog(message.data); } - setDuration(); + observeDuration(); return { log }; } @@ -833,6 +833,24 @@ class ControlConnection extends BaseConnection { this._master.userUpdated(user); } + async revokeUserTokenRequestHandler(message) { + let user = this._master.userManager.users.get(message.data.name); + if (!user) { + throw new libErrors.RequestError(`User '${message.data.name}' does not exist`); + } + if (user.name !== this.user.name) { + this.user.checkPermission("core.user.revoke_other_token"); + } + + user.invalidateToken(); + for (let controlConnection of this._master.wsServer.controlConnections) { + if (controlConnection.user.name === user.name) { + controlConnection.connector.terminate(); + } + } + this._master.userUpdated(user); + } + async updateUserRolesRequestHandler(message) { let user = this._master.userManager.users.get(message.data.name); if (!user) { diff --git a/packages/master/src/Master.js b/packages/master/src/Master.js index 6fb10989..e4605896 100644 --- a/packages/master/src/Master.js +++ b/packages/master/src/Master.js @@ -45,8 +45,8 @@ class Master { constructor(clusterLogger, pluginInfos, configPath, config) { this.clusterLogger = clusterLogger; /** - * Mapping of plugin name to info objects for known plugins - * @type {Map} + * Array of plugin info objects for known plugins + * @type {Array} */ this.pluginInfos = pluginInfos; this.configPath = configPath; diff --git a/packages/master/src/SlaveConnection.js b/packages/master/src/SlaveConnection.js index 35564264..c011d01f 100644 --- a/packages/master/src/SlaveConnection.js +++ b/packages/master/src/SlaveConnection.js @@ -23,6 +23,7 @@ class SlaveConnection extends BaseConnection { this._name = registerData.name; this._version = registerData.version; this.plugins = new Map(Object.entries(registerData.plugins)); + this._checkPluginVersions(); this._master.slaves.set(this._id, { agent: this._agent, @@ -57,6 +58,30 @@ class SlaveConnection extends BaseConnection { }); } + _checkPluginVersions() { + let pluginInfos = new Map(this._master.pluginInfos.map(i => [i.name, i])); + for (let [name, version] of this.plugins) { + let info = pluginInfos.get(name); + if (!info) { + logger.warn(`Slave ${this._name} has plugin ${name} ${version} which the master does not have`); + continue; + } + + if (info.version !== version) { + logger.warn( + `Slave ${this._name} has plugin ${name} ${version} which does not match the version of this ` + + `plugin on the master (${info.version})` + ); + } + } + + for (let [name, info] of pluginInfos) { + if (!this.plugins.has(name)) { + logger.warn(`Slave ${this._name} is missing plugin ${name} ${info.version}`); + } + } + } + /** * ID of the slave this connection is connected to * diff --git a/packages/master/src/WsServer.js b/packages/master/src/WsServer.js index 2be97122..2523873c 100644 --- a/packages/master/src/WsServer.js +++ b/packages/master/src/WsServer.js @@ -269,7 +269,16 @@ ${err.stack}` await connection.disconnect(1008, "Registered from another connection"); } - logger.verbose(`WsServer | registered slave ${data.id} version ${data.version}`); + logger.info( + `WsServer | registered slave ${data.name} (${data.id}) using agent ${data.agent} ${data.version}` + ); + if (data.agent === "Clusterio Slave" && data.version !== packageVersion) { + logger.warn( + `Slave ${data.name} (${data.id}) connected using version ${data.version} which does not match ` + + `the version of the master is currently running (${packageVersion}). It may not work as expected.` + ); + } + connection = new SlaveConnection(data, connector, this.master); connector.on("close", () => { if (this.slaveConnections.get(data.id) === connection) { diff --git a/packages/master/src/WsServerConnector.js b/packages/master/src/WsServerConnector.js index 1db846cb..84311284 100644 --- a/packages/master/src/WsServerConnector.js +++ b/packages/master/src/WsServerConnector.js @@ -191,6 +191,20 @@ class WsServerConnector extends libLink.WebSocketBaseConnector { await events.once(this, "close"); } + /** + * Forcefully close the connection immediately + * + * This should only be used if there's a security implication from + * letting the connection carry on with a normal close proceedure. + */ + terminate() { + this.setClosing(); + + if (this._socket) { + this._socket.terminate(); + } + } + /** * Notify the connection is expected to close * diff --git a/packages/slave/modules/clusterio/impl.lua b/packages/slave/modules/clusterio/impl.lua index 452a3a30..93175950 100644 --- a/packages/slave/modules/clusterio/impl.lua +++ b/packages/slave/modules/clusterio/impl.lua @@ -1,16 +1,18 @@ local api = require('modules/clusterio/api') -local impl = {} - -impl.events = {} -impl.events[defines.events.on_tick] = function() +local function check_patch() if global.clusterio_patch_number ~= clusterio_patch_number then global.clusterio_patch_number = clusterio_patch_number script.raise_event(api.events.on_server_startup, {}) end end +local impl = {} +impl.events = {} + +impl.events[defines.events.on_tick] = check_patch + impl.events[api.events.on_server_startup] = function() if not global.clusterio then global.clusterio = { @@ -47,6 +49,7 @@ end -- Internal API clusterio_private = {} function clusterio_private.update_instance(new_id, new_name) + check_patch() global.clusterio.instance_id = new_id global.clusterio.instance_name = new_name script.raise_event(api.events.on_instance_updated, { diff --git a/packages/slave/slave.js b/packages/slave/slave.js index d52a3d00..c252ded4 100755 --- a/packages/slave/slave.js +++ b/packages/slave/slave.js @@ -136,9 +136,6 @@ async function startSlave() { logger.info(`Starting Clusterio slave ${version}`); } - // add better stack traces on promise rejection - process.on("unhandledRejection", err => logger.error(`Unhandled rejection:\n${err.stack}`)); - logger.info(`Loading available plugins from ${args.pluginList}`); let pluginList = new Map(); try { diff --git a/packages/slave/src/Slave.js b/packages/slave/src/Slave.js index 4bb2f14c..e8dd3cc3 100644 --- a/packages/slave/src/Slave.js +++ b/packages/slave/src/Slave.js @@ -522,7 +522,7 @@ class Slave extends libLink.Link { ); } catch (err) { if (err.code === "ENOENT") { - throw new libErrors.RequestError(`${old_name} does not exist`); + throw new libErrors.RequestError(`${source} does not exist`); } throw err; } diff --git a/packages/web_ui/src/components/BaseConfigTree.jsx b/packages/web_ui/src/components/BaseConfigTree.jsx index 1e75338a..68377fff 100644 --- a/packages/web_ui/src/components/BaseConfigTree.jsx +++ b/packages/web_ui/src/components/BaseConfigTree.jsx @@ -1,5 +1,6 @@ import React, { useEffect, useState } from "react"; -import { Button, Card, Checkbox, Form, Input, InputNumber, Space, Spin, Tree, Typography } from "antd"; +import { Button, Card, Checkbox, Form, Input, InputNumber, Space, Spin, Tooltip, Tree, Typography } from "antd"; +import ReloadOutlined from "@ant-design/icons/ReloadOutlined"; const { Title } = Typography; @@ -87,6 +88,11 @@ export default function BaseConfigTree(props) { } } + let restartTip = ; + let treeData = []; let propsMap = new Map(); for (let [name, GroupClass] of props.ConfigClass.groups) { @@ -110,14 +116,27 @@ export default function BaseConfigTree(props) { }; if (def.type === "object") { - childNode.title = ; + let restartRequiredProps = new Set(def.restartRequiredProps || []); + childNode.title = + {def.title || def.name} + {!def.restartRequiredProps && def.restartRequired && restartTip} + } + tooltip={def.description} + />; for (let prop of Object.keys(value)) { let propPath = `${group.name}.${def.name}.${prop}`; + let restart = Boolean( + def.restartRequiredProps && def.restartRequired ^ restartRequiredProps.has(prop) + ); childNode.children.push({ key: propPath, title: + {`"${prop}"`} + {restart && restartTip} + } rules={[{ validator: (_, fieldValue) => { if (!fieldValue.length) { return Promise.reject(new Error("Will be removed")); @@ -184,7 +203,10 @@ export default function BaseConfigTree(props) { } else { childNode.title = + {def.title || def.name} + {def.restartRequired && restartTip} + } validateStatus={errorFields.has(fieldName) ? "error" : undefined} help={errorFields.get(fieldName)} tooltip={def.description} diff --git a/packages/web_ui/src/components/UserViewPage.jsx b/packages/web_ui/src/components/UserViewPage.jsx index 7015f6e9..084a7a60 100644 --- a/packages/web_ui/src/components/UserViewPage.jsx +++ b/packages/web_ui/src/components/UserViewPage.jsx @@ -6,7 +6,7 @@ import { } from "antd"; import DeleteOutlined from "@ant-design/icons/DeleteOutlined"; -import { libLink } from "@clusterio/lib"; +import { libErrors, libLink } from "@clusterio/lib"; import { useAccount } from "../model/account"; import { useInstanceList } from "../model/instance"; @@ -14,7 +14,7 @@ import ControlContext from "./ControlContext"; import PageLayout from "./PageLayout"; import PluginExtra from "./PluginExtra"; import SectionHeader from "./SectionHeader"; -import { notifyErrorHandler } from "../util/notify"; +import notify, { notifyErrorHandler } from "../util/notify"; import { formatDuration } from "../util/time_format"; import { formatLastSeen, sortLastSeen, useUser } from "../model/user"; @@ -121,8 +121,27 @@ export default function UserViewPage() { {user["is_banned"] && Banned} } - extra={ - account.hasPermission("core.user.delete") && + {account.hasPermission("core.user.revoke_token") && ( + account.name === userName || account.hasPermission("core.user.revoke_other_token") + ) && } + {account.hasPermission("core.user.delete") && Delete user account and all data associated with it? {(user["is_banned"] && <>
This will remove the user ban!)} @@ -141,8 +160,8 @@ export default function UserViewPage() { - - } + } + } />
level then + if not tech or tech.level > level then return end diff --git a/test/integration/clusterio.js b/test/integration/clusterio.js index ab94dd89..a8434a89 100644 --- a/test/integration/clusterio.js +++ b/test/integration/clusterio.js @@ -1,6 +1,7 @@ "use strict"; const assert = require("assert").strict; const fs = require("fs-extra"); +const jwt = require("jsonwebtoken"); const path = require("path"); const events = require("events"); const phin = require("phin"); @@ -1087,6 +1088,26 @@ describe("Integration of Clusterio", function() { }); }); + describe("user revoke-token", function() { + it("should kick existing sessions for the user", async function() { + slowTest(this); + await libLink.messages.createUser.send(getControl(), { name: "revokee" }); + let tlsCa = await fs.readFile("test/file/tls/cert.pem"); + let connector = new TestControlConnector(url, 2, tlsCa); + connector.token = jwt.sign( + { aud: "user", user: "revokee" }, Buffer.from("TestSecretDoNotUse", "base64") + ); + let revokeeControl = new TestControl(connector); + await connector.connect(); + connector.setClosing(); + let closed = new Promise(resolve => connector.once("close", resolve)); + + await execCtl("user revoke-token revokee"); + + await closed; + }); + }); + describe("user set-roles", function() { it("should set the roles on the user", async function() { getControl().userUpdates = []; diff --git a/test/lib/config/classes.js b/test/lib/config/classes.js index 49d5d224..23a2a292 100644 --- a/test/lib/config/classes.js +++ b/test/lib/config/classes.js @@ -113,6 +113,7 @@ describe("lib/config/classes", function() { name: "enum", title: "Enum", description: "Enum thingy", + restartRequired: true, enum: ["a", "b", "c"], optional: true, initial_value: "b",