From eedf5b8f674caa52779fbac92b12c46ddfe6bf9d Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Mon, 16 Sep 2024 14:37:07 +0200 Subject: [PATCH 1/4] Credential option for config entries Implement a credential option to config entries which is a list of locations the config entry can be read from. When specified and the option is a string field this gets rendered as a password field in the config UI. If the credential is not readable from control the value of the fields is cleared when the UI is refreshed. --- CHANGELOG.md | 2 + packages/lib/src/config/classes.ts | 104 +++++++++++++----- .../web_ui/src/components/BaseConfigTree.tsx | 19 +++- test/lib/config/classes.js | 92 +++++++++++++--- 4 files changed, 168 insertions(+), 49 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 05e6faa9..8ac866c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,7 @@ Many thanks to the following for contributing to this release: - Added clean script to speed up publishing and debugging of build steps [#640](https://github.com/clusterio/clusterio/pull/640). - Added sub-tile option on plugin page headers [#642](https://github.com/clusterio/clusterio/pull/642). - Greatly improved plugin and module template generation [#641](https://github.com/clusterio/clusterio/pull/641). +- Added credential option to config entries to allow writing sensitive data that can't be read back remotely. ### Changes @@ -55,6 +56,7 @@ 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. Many thanks to the following for contributing to this release: [@CCpersonguy](https://github.com/CCpersonguy) diff --git a/packages/lib/src/config/classes.ts b/packages/lib/src/config/classes.ts index dae4fc8a..43903fbb 100644 --- a/packages/lib/src/config/classes.ts +++ b/packages/lib/src/config/classes.ts @@ -111,6 +111,11 @@ export interface FieldDefinition { * modified from. */ access?: ConfigLocation[]; + /** + * If present treat the config value as a credential and only allow reading + * from the locations specified. + */ + credential?: ConfigLocation[]; } export type ConfigDefs = { @@ -131,6 +136,12 @@ export type ConfigEvents = { fieldChanged: (name: string, curr: FieldValue, prev: FieldValue) => void; } +export enum ConfigAccess { + read = 0x1, + write = 0x2, + readWrite = 0x3, +} + /** * Collection of config entries * @@ -188,7 +199,7 @@ export class Config< Object.entries( this.constructor.fieldDefinitions ).filter( - ([name, def]) => this._checkAccess(name, def, location, false) + ([name, def]) => this._checkAccess(name, def, location, ConfigAccess.read, false) ).map( ([name, def]) => [name, defaultValue(def)] ) @@ -204,21 +215,28 @@ export class Config< * * @param name - Name of the field to check. * @param def - definition for for the field to check. - * @param remote - location of remote access to check for. + * @param location - location to check access for. + * @param mode - type of access to check for. * @param error - throw on failure to pass check. * @throws {InvalidAccess} * if access is not granted and error is true. * @returns true if access is granted * @private */ - _checkAccess(name: string, def: FieldDefinition, remote: ConfigLocation, error: boolean) { - for (let loc of [remote, this.location]) { - if (!(def.access ?? this.defaultAccess).includes(loc)) { - if (error) { - throw new InvalidAccess(`Field '${name}' is not accessible from ${loc}`); - } else { - return false; - } + _checkAccess(name: string, def: FieldDefinition, location: ConfigLocation, mode: ConfigAccess, error: boolean) { + // If credential array is present it acts as read permission and the access array as write permission, + // if not present access array acts as a combined read/write permission check. + if ( + def.credential && mode & ConfigAccess.read && !(def.credential).includes(location) + || ( + (!def.credential || mode & ConfigAccess.write) + && !(def.access ?? this.defaultAccess).includes(location) + ) + ) { + if (error) { + throw new InvalidAccess(`Field '${name}' is not accessible from ${location}`); + } else { + return false; } } return true; @@ -263,14 +281,17 @@ export class Config< /** * Serialize the config to a plain JavaScript object * - * @param location - Location used for access control. - * @returns JSON serializable representation of the group. + * @param remote - Location this serialised representation is for + * @returns JSON serializable representation of the config. */ - toRemote(location: ConfigLocation): Static { + toRemote(remote: ConfigLocation): Static { let fields: Record = {}; for (let [name, value] of Object.entries(this.fields)) { let def = this.constructor.fieldDefinitions[name]; - if (!this._checkAccess(name, def, location, false)) { + if ( + !this._checkAccess(name, def, this.location, ConfigAccess.write, false) + || this.location !== remote && !this._checkAccess(name, def, remote, ConfigAccess.read, false) + ) { continue; } @@ -293,12 +314,12 @@ export class Config< * @param json - * Result from a previous call to .toRemote(). * @param notify - Invoke fieldChanged events if true. - * @param location - Location used for access control. + * @param remote - Location this serialised representation was received from. */ update( json: Static, notify: boolean, - location = this.location + remote = this.location ) { const valid = Config.validate(json); if (!valid) { @@ -316,7 +337,9 @@ export class Config< continue; } - if (!this._checkAccess(name, def, location, false)) { + if ( + !this._checkAccess(name, def, remote, ConfigAccess.write, false) + ) { continue; } @@ -346,30 +369,45 @@ export class Config< * Check if field can be accessed from the given location * * @param name - Name of field to check. + * @param mode - Mode to access the field as. * @param location - Location used for access control. * @returns true if field is accessible */ - canAccess(name: string, location = this.location) { + canAccess(name: string, mode: ConfigAccess, location = this.location) { + if (typeof mode !== "number") { + throw new TypeError("mode argument is required to canAccess"); + } let def = this.constructor.fieldDefinitions[name]; if (!def) { throw new InvalidField(`No field named '${name}'`); } - return this._checkAccess(name, def, location, false); + return this._checkAccess(name, def, location, mode, false); } /** * Get value for field * * @param name - Name of field to get. - * @param location - Location used for access control. + * @param remote - Location this value is being read from. * @returns Value stored for the field. */ - get(name: Field, location = this.location) { + get(name: Field, remote = this.location) { let def = this.constructor.fieldDefinitions[name]; if (!def) { throw new InvalidField(`No field named '${name}'`); } - this._checkAccess(name, def, location, true); + // If remote access is requested require it to be able to read the field + if (remote !== this.location) { + this._checkAccess(name, def, remote, ConfigAccess.read, true); + } + // Require that the field is either read or writable locally so that you can get the value + // back of write only fields recently written to. + if ( + !this._checkAccess(name, def, this.location, ConfigAccess.write, false) + && !this._checkAccess(name, def, this.location, ConfigAccess.read, false) + ) { + throw new InvalidAccess(`Field '${name}' is not accessible from ${this.location}`); + } return this.fields[name]; } @@ -377,22 +415,25 @@ export class Config< /** * Set value of field * - * The field must be defined for the config group and the value is + * The field must be defined for the config and the value is * type checked against the field definition. * * @param name - Name of field to set. * @param newValue - Value to set for field. - * @param location - Location used for access control. + * @param remote - Location this field is being set from. * @throws {InvalidField} if field is not defined. * @throws {InvalidValue} if value is not allowed for the field. */ - set(name: Field, newValue: Fields[Field], location = this.location) { + set(name: Field, newValue: Fields[Field], remote = this.location) { let value: FieldValue = newValue; let def = this.constructor.fieldDefinitions[name]; if (!def) { throw new InvalidField(`No field named '${name}'`); } - this._checkAccess(name, def, location, true); + this._checkAccess(name, def, remote, ConfigAccess.write, true); + if (remote !== this.location) { + this._checkAccess(name, def, this.location, ConfigAccess.write, true); + } // Empty strings are treates as null if (value === "") { @@ -447,12 +488,12 @@ export class Config< * Set property of object field * * Update value of stored object field by setting the specified property - * on it. The field must be defined as an object for the config group. + * on it. The field must be defined as an object for the config. * * @param name - Name of field to set property on. * @param prop - Name of property to set on field. * @param value - the value to set the property to. - * @param location - Location used for access control. + * @param remote - Location this property is being set from * @throws {InvalidField} if field is not defined. * @throws {InvalidValue} if field is not an object. */ @@ -460,13 +501,16 @@ export class Config< name: Field, prop: string, value?: unknown, - location = this.location, + remote = this.location, ) { let def = this.constructor.fieldDefinitions[name]; if (!def) { throw new InvalidField(`No field named '${name}'`); } - this._checkAccess(name, def, location, true); + this._checkAccess(name, def, remote, ConfigAccess.write, true); + if (remote !== this.location) { + this._checkAccess(name, def, this.location, ConfigAccess.write, true); + } if (def.type !== "object") { throw new InvalidValue(`Cannot set property on non-object field '${name}'`); diff --git a/packages/web_ui/src/components/BaseConfigTree.tsx b/packages/web_ui/src/components/BaseConfigTree.tsx index f2a0e922..1c994170 100644 --- a/packages/web_ui/src/components/BaseConfigTree.tsx +++ b/packages/web_ui/src/components/BaseConfigTree.tsx @@ -16,13 +16,13 @@ const { Title } = Typography; function getInitialValues(config: lib.Config, props: BaseConfigTreeProps) { let initialValues: any = {}; for (let [name, def] of Object.entries(config.constructor.fieldDefinitions)) { - if (!config.canAccess(name)) { + if (!config.canAccess(name, lib.ConfigAccess.read)) { continue; } let value = config.get(name) as any; if (def.type === "object") { - for (let prop of Object.keys(value)) { + for (let prop of Object.keys(value ?? {})) { initialValues[`${name}.${prop}`] = JSON.stringify(value[prop]); } } else { @@ -43,6 +43,9 @@ function renderInput(inputComponents: Record, def: lib.F return ; } if (def.type === "string") { + if (def.credential) { + return ; + } return ; } if (def.type === "number") { @@ -110,7 +113,8 @@ export default function BaseConfigTree(props: BaseConfigTreeProps) { if (key.endsWith(":add.name") || key.endsWith(":add.value")) { continue; } - if (initialValues[key] === value) { + const def = config?.constructor.fieldDefinitions[key]; + if (!def?.credential && initialValues[key] === value) { changed = newChangedFields.delete(key) || changed; } else { changed = !newChangedFields.has(key) || changed; @@ -213,6 +217,7 @@ export default function BaseConfigTree(props: BaseConfigTreeProps) { ; } +// eslint-disable-next-line complexity function computeTreeData( control: Control, form: FormInstance, @@ -259,7 +264,9 @@ function computeTreeData( }; for (let [field, def] of Object.entries(groupDefs)) { - if (!config.canAccess(`${groupName}.${field}`)) { + const canWrite = config.canAccess(`${groupName}.${field}`, lib.ConfigAccess.write); + const canRead = config.canAccess(`${groupName}.${field}`, lib.ConfigAccess.read); + if (!canWrite && !canRead) { continue; } @@ -279,7 +286,7 @@ function computeTreeData( } tooltip={def.description} />; - for (let prop of Object.keys(value)) { + for (let prop of Object.keys(value ?? {})) { let propPath = `${groupName}.${field}.${prop}`; let restart = Boolean( def.restartRequiredProps && Number(def.restartRequired) ^ Number(restartRequiredProps.has(prop)) @@ -333,7 +340,7 @@ function computeTreeData( onClick={() => { let propName = form.getFieldValue(`${newPropPath}.name`); let propValue = form.getFieldValue(`${newPropPath}.value`); - if (!Object.prototype.hasOwnProperty.call(value, propName)) { + if (!value || !Object.prototype.hasOwnProperty.call(value, propName)) { let newConfig = ConfigClass.fromJSON(config!.toJSON(), "control"); (newConfig as lib.Config).setProp(fieldName, propName, null); setConfig(newConfig); diff --git a/test/lib/config/classes.js b/test/lib/config/classes.js index 03e69c4d..efa7c6aa 100644 --- a/test/lib/config/classes.js +++ b/test/lib/config/classes.js @@ -1,7 +1,7 @@ "use strict"; const lib = require("@clusterio/lib"); const assert = require("assert").strict; -const events = require("events"); +const CA = lib.ConfigAccess; describe("lib/config/classes", function() { @@ -26,6 +26,7 @@ describe("lib/config/classes", function() { "test.bool": { type: "boolean", initialValue: false, optional: true }, "test.json": { type: "object", initialValue: {}, optional: true }, "test.priv": { access: ["local"], type: "string", optional: true }, + "test.cred": { credential: ["local"] }, }; constructor(location, fields) { @@ -72,6 +73,7 @@ describe("lib/config/classes", function() { "test.bool": false, "test.json": {}, "test.priv": null, + "test.cred": null, }); }); }); @@ -148,6 +150,7 @@ describe("lib/config/classes", function() { "test.bool": false, "test.json": {}, "test.priv": null, + "test.cred": null, "test.alpha": null, "test.beta": "decay", "test.gamma": 99, @@ -181,6 +184,7 @@ describe("lib/config/classes", function() { "test.func": 42, "test.json": {}, "test.priv": null, + "test.cred": null, "test.test": null, }); }); @@ -227,20 +231,61 @@ describe("lib/config/classes", function() { describe(".canAccess()", function() { it("should throw if field does not exist", function() { let testInstance = new TestConfig("local"); - assert.throws(() => testInstance.canAccess("invalid"), new Error("No field named 'invalid'")); + assert.throws(() => testInstance.canAccess("invalid", CA.read), new Error("No field named 'invalid'")); }); - it("should return true for fields that are accessible", function() { + it("should throw if mode is not passed ", function() { let testInstance = new TestConfig("local"); - assert.equal(testInstance.canAccess("alpha.foo"), true); + assert.throws( + () => { testInstance.canAccess("alpha.foo"); }, + new TypeError("mode argument is required to canAccess") + ); + }); + + it("should return true checking read for fields that are readable", function() { + let testInstance = new TestConfig("local"); + assert.equal(testInstance.canAccess("alpha.foo", CA.read), true); for (let field of ["test.enum", "test.test", "test.func", "test.bool", "test.json"]) { - assert.equal(testInstance.canAccess(field), true); - assert.equal(testInstance.canAccess(field, "local"), true); - assert.equal(testInstance.canAccess(field, "remote"), true); + assert.equal(testInstance.canAccess(field, CA.read), true); + assert.equal(testInstance.canAccess(field, CA.read, "local"), true); + assert.equal(testInstance.canAccess(field, CA.read, "remote"), true); } - assert.equal(testInstance.canAccess("test.priv"), true); - assert.equal(testInstance.canAccess("test.priv", "local"), true); - assert.equal(testInstance.canAccess("test.priv", "remote"), false); + assert.equal(testInstance.canAccess("test.priv", CA.read), true); + assert.equal(testInstance.canAccess("test.priv", CA.read, "local"), true); + assert.equal(testInstance.canAccess("test.priv", CA.read, "remote"), false); + assert.equal(testInstance.canAccess("test.cred", CA.read), true); + assert.equal(testInstance.canAccess("test.cred", CA.read, "local"), true); + assert.equal(testInstance.canAccess("test.cred", CA.read, "remote"), false); + }); + it("should return true checking write for fields that are writeable", function() { + let testInstance = new TestConfig("local"); + assert.equal(testInstance.canAccess("alpha.foo", CA.write), true); + for (let field of ["test.enum", "test.test", "test.func", "test.bool", "test.json"]) { + assert.equal(testInstance.canAccess(field, CA.write), true); + assert.equal(testInstance.canAccess(field, CA.write, "local"), true); + assert.equal(testInstance.canAccess(field, CA.write, "remote"), true); + } + assert.equal(testInstance.canAccess("test.priv", CA.write), true); + assert.equal(testInstance.canAccess("test.priv", CA.write, "local"), true); + assert.equal(testInstance.canAccess("test.priv", CA.write, "remote"), false); + assert.equal(testInstance.canAccess("test.cred", CA.write), true); + assert.equal(testInstance.canAccess("test.cred", CA.write, "local"), true); + assert.equal(testInstance.canAccess("test.cred", CA.write, "remote"), true); + }); + it("should return true for checking readWrite for fields that are readable and writeable", function() { + let testInstance = new TestConfig("local"); + assert.equal(testInstance.canAccess("alpha.foo", CA.readWrite), true); + for (let field of ["test.enum", "test.test", "test.func", "test.bool", "test.json"]) { + assert.equal(testInstance.canAccess(field, CA.readWrite), true); + assert.equal(testInstance.canAccess(field, CA.readWrite, "local"), true); + assert.equal(testInstance.canAccess(field, CA.readWrite, "remote"), true); + } + assert.equal(testInstance.canAccess("test.priv", CA.readWrite), true); + assert.equal(testInstance.canAccess("test.priv", CA.readWrite, "local"), true); + assert.equal(testInstance.canAccess("test.priv", CA.readWrite, "remote"), false); + assert.equal(testInstance.canAccess("test.cred", CA.readWrite), true); + assert.equal(testInstance.canAccess("test.cred", CA.readWrite, "local"), true); + assert.equal(testInstance.canAccess("test.cred", CA.readWrite, "remote"), false); }); }); @@ -259,13 +304,20 @@ describe("lib/config/classes", function() { assert.equal(testInstance.get("test.priv"), "blah"); assert.equal(testInstance.get("test.func"), 27); }); - it("should throw if field is inaccessible", function() { + it("should throw if field is remotely inaccessible", function() { let testInstance = new TestConfig("local"); assert.throws( () => testInstance.get("test.priv", "remote"), new lib.InvalidAccess("Field 'test.priv' is not accessible from remote") ); }); + it("should throw if field is locally inaccessible", function() { + let testInstance = new TestConfig("remote"); + assert.throws( + () => testInstance.get("test.priv"), + new lib.InvalidAccess("Field 'test.priv' is not accessible from remote") + ); + }); }); describe(".set()", function() { @@ -290,12 +342,19 @@ describe("lib/config/classes", function() { ); }); - it("should throw if field is inaccesible", function() { + it("should throw if field is remotely inaccessible", function() { assert.throws( () => testInstance.set("test.priv", "bad", "remote"), new lib.InvalidAccess("Field 'test.priv' is not accessible from remote") ); }); + it("should throw if field is locally inaccessible", function() { + testInstance = new TestConfig("remote"); + assert.throws( + () => testInstance.set("test.priv", "bad", "local"), + new lib.InvalidAccess("Field 'test.priv' is not accessible from remote") + ); + }); it("should throw if field is not in enum", function() { assert.throws( @@ -398,12 +457,19 @@ describe("lib/config/classes", function() { ); }); - it("should throw if field is inaccesible", function() { + it("should throw if field is remotely inaccessible", function() { assert.throws( () => testInstance.setProp("test.priv", "prop", "bad", "remote"), new lib.InvalidAccess("Field 'test.priv' is not accessible from remote") ); }); + it("should throw if field is locally inaccessible", function() { + testInstance = new TestConfig("remote"); + assert.throws( + () => testInstance.setProp("test.priv", "prop", "bad", "local"), + new lib.InvalidAccess("Field 'test.priv' is not accessible from remote") + ); + }); it("should throw if field is not an object", function() { assert.throws( From 44e5847a1d439b43da9f6f5ba365a8975eac824f Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Mon, 16 Sep 2024 14:42:29 +0200 Subject: [PATCH 2/4] Implement shared Factorio credentials Add config entries to controller and host for the Factorio username and token to use when talking to the Factorio APIs. If set an username and token is not set in factorio.settings then these settings are used, making it easy to configure one set of credentials for a cluster/host. For running more public provide your own hardware clusters, there's an option to disable controller sharing the credential with the hosts so that each host have to set their own credentials. --- CHANGELOG.md | 3 + packages/controller/src/Controller.ts | 2 - packages/controller/src/HostConnection.ts | 11 +++ packages/host/src/Instance.ts | 96 ++++++++++++-------- packages/lib/src/config/definitions.ts | 43 +++++++++ packages/lib/src/data/messages_controller.ts | 12 +++ packages/lib/src/link/messages.ts | 1 + 7 files changed, 130 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ac866c1..08fc564b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,6 +51,9 @@ Many thanks to the following for contributing to this release: - Added instance version display [#573](https://github.com/clusterio/clusterio/pull/573) - Added `factorio.shutdown_timeout` config to set the time the host will wait for a Factorio server to shut down befor killing it. - Changed shutdown logic to prefer sending a /quit command via RCON instead of using diverging logic on Windows and Linux. +- 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. ### Breaking Changes diff --git a/packages/controller/src/Controller.ts b/packages/controller/src/Controller.ts index 4d9c2c80..22b54567 100644 --- a/packages/controller/src/Controller.ts +++ b/packages/controller/src/Controller.ts @@ -788,8 +788,6 @@ export default class Controller { "tags": ["clusterio"], "max_players": 0, "visibility": { "public": true, "lan": true }, - "username": "", - "token": "", "game_password": "", "require_user_verification": true, "max_upload_in_kilobytes_per_second": 0, diff --git a/packages/controller/src/HostConnection.ts b/packages/controller/src/HostConnection.ts index 388014d2..6da8e93c 100644 --- a/packages/controller/src/HostConnection.ts +++ b/packages/controller/src/HostConnection.ts @@ -74,6 +74,7 @@ export default class HostConnection extends BaseConnection { this._controller.hosts.set(this.info); }); + this.handle(lib.GetFactorioCredentialsRequest, this.handleGetFactorioCredentialsRequest.bind(this)); this.handle(lib.HostInfoUpdateEvent, this.handleHostInfoUpdateEvent.bind(this)); this.handle(lib.InstanceStatusChangedEvent, this.handleInstanceStatusChangedEvent.bind(this)); this.handle(lib.InstancesUpdateRequest, this.handleInstancesUpdateRequest.bind(this)); @@ -161,6 +162,16 @@ export default class HostConnection extends BaseConnection { return this.info.remoteAddress; } + async handleGetFactorioCredentialsRequest() { + if (this._controller.config.get("controller.share_factorio_credentials_with_hosts")) { + return { + username: this._controller.config.get("controller.factorio_username") ?? undefined, + token: this._controller.config.get("controller.factorio_token") ?? undefined, + }; + } + return {}; + } + async handleHostInfoUpdateEvent(event: lib.HostInfoUpdateEvent) { this.info.name = event.update.name; this.info.publicAddress = event.update.publicAddress; diff --git a/packages/host/src/Instance.ts b/packages/host/src/Instance.ts index 8cbc903b..9cbd3461 100644 --- a/packages/host/src/Instance.ts +++ b/packages/host/src/Instance.ts @@ -583,11 +583,23 @@ rcon.print(game.table_to_json(players))`.replace(/\r?\n/g, " "); * entries from the given settings object. * * @param overrides - Server settings to override. + * @param includeCredentials - Include Factorio username and token from host/controller config. * @returns * server example settings with the given settings applied over it. */ - async resolveServerSettings(overrides: Record) { + async resolveServerSettings(overrides: Record, includeCredentials: boolean) { let serverSettings = await this.server.exampleSettings(); + if (includeCredentials && !overrides.username && !overrides.token) { + let credentials = { + username: this._host.config.get("host.factorio_username") ?? undefined, + token: this._host.config.get("host.factorio_token") ?? undefined, + }; + if (!credentials.username && !credentials.token) { + Object.assign(credentials, await this.sendTo("controller", new lib.GetFactorioCredentialsRequest())); + } + if (credentials.username) { serverSettings.username = credentials.username; } + if (credentials.token) { serverSettings.token = credentials.token; } + } for (let [key, value] of Object.entries(overrides)) { if (!Object.hasOwnProperty.call(serverSettings, key)) { @@ -605,12 +617,14 @@ rcon.print(game.table_to_json(players))`.replace(/\r?\n/g, " "); * Generate the server-settings.json file from the example file in the * data directory and override any settings configured in the instance's * factorio_settings config entry. + * + * @param includeCredentials - Include Factorio username and token from host/controller config. */ - async writeServerSettings() { + async writeServerSettings(includeCredentials: boolean) { const warning = "Changes to this file will be overwitten by the factorio.settings config on the instance."; const serverSettings = { "_comment_warning": warning, - ...await this.resolveServerSettings(this.config.get("factorio.settings")), + ...await this.resolveServerSettings(this.config.get("factorio.settings"), includeCredentials), }; await lib.safeOutputFile( this.server.writePath("server-settings.json"), @@ -714,7 +728,7 @@ rcon.print(game.table_to_json(players))`.replace(/\r?\n/g, " "); */ async prepare() { this.logger.verbose("Writing server-settings.json"); - await this.writeServerSettings(); + await this.writeServerSettings(true); if (this.config.get("factorio.sync_adminlist")) { this.logger.verbose("Writing server-adminlist.json"); @@ -884,8 +898,8 @@ rcon.print(game.table_to_json(players))`.replace(/\r?\n/g, " "); } async updateFactorioSettings(current: Record, previous: Record) { - current = await this.resolveServerSettings(current); - previous = await this.resolveServerSettings(previous); + current = await this.resolveServerSettings(current, false); + previous = await this.resolveServerSettings(previous, false); for (let [key, action] of Object.entries(serverSettingsActions)) { if (current[key] !== undefined && !util.isDeepStrictEqual(current[key], previous[key])) { @@ -1008,21 +1022,26 @@ rcon.print(game.table_to_json(players))`.replace(/\r?\n/g, " "); async handleInstanceStartRequest(request: lib.InstanceStartRequest) { let saveName = request.save; try { - await this.prepare(); - saveName = await this.prepareSave(saveName); - } catch (err: any) { - this.logger.error(`Error preparing instance: ${err.message}`); - this.notifyExit(); - await this.sendSaveListUpdate(); - throw err; - } + try { + await this.prepare(); + saveName = await this.prepareSave(saveName); + } catch (err: any) { + this.logger.error(`Error preparing instance: ${err.message}`); + this.notifyExit(); + await this.sendSaveListUpdate(); + throw err; + } - try { - await this.start(saveName); - } catch (err: any) { - this.logger.error(`Error starting ${saveName}: ${err.message}`); - await this.stop(); - throw err; + try { + await this.start(saveName); + } catch (err: any) { + this.logger.error(`Error starting ${saveName}: ${err.message}`); + await this.stop(); + throw err; + } + } finally { + this.logger.verbose("Wiping credentials from server-settings.json"); + await this.writeServerSettings(false); } } @@ -1033,21 +1052,26 @@ rcon.print(game.table_to_json(players))`.replace(/\r?\n/g, " "); } try { - await this.prepare(); - } catch (err: any) { - this.logger.error(`Error preparing instance: ${err.message}`); - this.notifyExit(); - await this.sendSaveListUpdate(); - throw err; - } + try { + await this.prepare(); + } catch (err: any) { + this.logger.error(`Error preparing instance: ${err.message}`); + this.notifyExit(); + await this.sendSaveListUpdate(); + throw err; + } - let { scenario, seed, mapGenSettings, mapSettings } = request; - try { - await this.startScenario(scenario, seed, mapGenSettings, mapSettings); - } catch (err: any) { - this.logger.error(`Error starting scenario ${scenario}: ${err.message}`); - await this.stop(); - throw err; + let { scenario, seed, mapGenSettings, mapSettings } = request; + try { + await this.startScenario(scenario, seed, mapGenSettings, mapSettings); + } catch (err: any) { + this.logger.error(`Error starting scenario ${scenario}: ${err.message}`); + await this.stop(); + throw err; + } + } finally { + this.logger.verbose("Wiping credentials from server-settings.json"); + await this.writeServerSettings(false); } } @@ -1059,7 +1083,7 @@ rcon.print(game.table_to_json(players))`.replace(/\r?\n/g, " "); this.notifyStatus("creating_save"); try { this.logger.verbose("Writing server-settings.json"); - await this.writeServerSettings(); + await this.writeServerSettings(false); await this.syncMods(); } catch (err: any) { @@ -1087,7 +1111,7 @@ rcon.print(game.table_to_json(players))`.replace(/\r?\n/g, " "); this.notifyStatus("exporting_data"); try { this.logger.verbose("Writing server-settings.json"); - await this.writeServerSettings(); + await this.writeServerSettings(false); this.logger.info("Exporting data ....."); await this.syncMods(); diff --git a/packages/lib/src/config/definitions.ts b/packages/lib/src/config/definitions.ts index b50eeaa3..8f38de08 100644 --- a/packages/lib/src/config/definitions.ts +++ b/packages/lib/src/config/definitions.ts @@ -20,6 +20,9 @@ export interface ControllerConfigFields { "controller.metrics_timeout": number; "controller.system_metrics_interval": number; "controller.proxy_stream_timeout": number; + "controller.factorio_username": string | null; + "controller.factorio_token": string | null; + "controller.share_factorio_credentials_with_hosts": boolean; "controller.default_mod_pack_id": number | null; "controller.default_role_id": number | null; "controller.autosave_interval": number; @@ -147,6 +150,27 @@ export class ControllerConfig extends classes.Config { type: "number", initialValue: 15, }, + "controller.factorio_username": { + title: "Factorio Username", + description: "Username to authenticate with Factorio API with.", + type: "string", + optional: true, + }, + "controller.factorio_token": { + title: "Factorio Token", + description: "Token to authenticate with Factorio API with.", + type: "string", + credential: ["controller"], + optional: true, + }, + "controller.share_factorio_credentials_with_hosts": { + title: "Share Factorio credentials with Hosts", + description: + "If enabled, the Factorio Username and Token will be shared with hosts and " + + "used in the server settings of instances in this cluster.", + type: "boolean", + initialValue: true, + }, "controller.default_mod_pack_id": { title: "Default Mod Pack", description: "Mod pack used by default for instances.", @@ -175,6 +199,8 @@ export interface HostConfigFields { "host.tls_ca": string | null; "host.public_address": string; "host.factorio_port_range": string; + "host.factorio_username": string | null, + "host.factorio_token": string | null, "host.max_reconnect_delay": number; } @@ -245,6 +271,23 @@ export class HostConfig extends classes.Config { type: "string", initialValue: "34100-34199", }, + "host.factorio_username": { + title: "Factorio Username", + description: + "Username to authenticate with Factorio API with. If set this will be used in the server settings " + + "of all instances on this host.", + type: "string", + optional: true, + }, + "host.factorio_token": { + title: "Factorio Token", + description: + "Token to authenticate with Factorio API with. If set this will be used in the server settings " + + "of all instances on this host.", + type: "string", + credential: ["host"], + optional: true, + }, "host.max_reconnect_delay": { title: "Max Reconnect Delay", description: "Maximum delay to wait before attempting to reconnect WebSocket", diff --git a/packages/lib/src/data/messages_controller.ts b/packages/lib/src/data/messages_controller.ts index e24d0be4..41492305 100644 --- a/packages/lib/src/data/messages_controller.ts +++ b/packages/lib/src/data/messages_controller.ts @@ -122,6 +122,18 @@ export class HostConfigCreateRequest { static Response = plainJson(HostConfig.jsonSchema); } +export class GetFactorioCredentialsRequest { + declare ["constructor"]: typeof GetFactorioCredentialsRequest; + static type = "request" as const; + static src = ["host", "instance"] as const; + static dst = "controller" as const; + + static Response = plainJson(Type.Object({ + "username": Type.Optional(Type.String()), + "token": Type.Optional(Type.String()), + })); +} + export class LogSetSubscriptionsRequest { declare ["constructor"]: typeof LogSetSubscriptionsRequest; static type = "request" as const; diff --git a/packages/lib/src/link/messages.ts b/packages/lib/src/link/messages.ts index 7dd7ed99..4182db3a 100644 --- a/packages/lib/src/link/messages.ts +++ b/packages/lib/src/link/messages.ts @@ -24,6 +24,7 @@ export const dataClasses: (RequestClass | EventClass) controller.ControllerConfigSetPropRequest, controller.HostGenerateTokenRequest, controller.HostConfigCreateRequest, + controller.GetFactorioCredentialsRequest, controller.LogSetSubscriptionsRequest, controller.LogQueryRequest, controller.LogMessageEvent, From f873f4b369d2bfafb23b0726febb7480bbbbdfb5 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Wed, 18 Sep 2024 23:10:31 +0200 Subject: [PATCH 3/4] Add autocomplete attribute to credential fields Prevent browsers from automatically autofilling the password entry box for the Factorio credentials by setting the autocomplete property to new-password on the password input. --- packages/lib/src/config/classes.ts | 6 ++++++ packages/lib/src/config/definitions.ts | 4 ++++ packages/web_ui/src/components/BaseConfigTree.tsx | 6 +++--- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/packages/lib/src/config/classes.ts b/packages/lib/src/config/classes.ts index 43903fbb..8e3a16ee 100644 --- a/packages/lib/src/config/classes.ts +++ b/packages/lib/src/config/classes.ts @@ -77,6 +77,12 @@ export interface FieldDefinition { * Text used to describe the config field in user interfaces. */ description?: string; + /** + * Value to use for the autocomplete attribute in the Web UI's input element. + * Useful for making browsers behave properly with credential inputs. + * Only used for string and number fields. + */ + autoComplete?: string; /** * Web UI component to use for rending this input. Can be extended by * @{link "@clusterio/web_ui".BaseWebPlugin.inputComponents} diff --git a/packages/lib/src/config/definitions.ts b/packages/lib/src/config/definitions.ts index 8f38de08..aafce8f1 100644 --- a/packages/lib/src/config/definitions.ts +++ b/packages/lib/src/config/definitions.ts @@ -153,12 +153,14 @@ export class ControllerConfig extends classes.Config { "controller.factorio_username": { title: "Factorio Username", description: "Username to authenticate with Factorio API with.", + autoComplete: "section-factorio username", type: "string", optional: true, }, "controller.factorio_token": { title: "Factorio Token", description: "Token to authenticate with Factorio API with.", + autoComplete: "section-factorio new-password", type: "string", credential: ["controller"], optional: true, @@ -276,6 +278,7 @@ export class HostConfig extends classes.Config { description: "Username to authenticate with Factorio API with. If set this will be used in the server settings " + "of all instances on this host.", + autoComplete: "section-factorio username", type: "string", optional: true, }, @@ -284,6 +287,7 @@ export class HostConfig extends classes.Config { description: "Token to authenticate with Factorio API with. If set this will be used in the server settings " + "of all instances on this host.", + autoComplete: "section-factorio new-password", type: "string", credential: ["host"], optional: true, diff --git a/packages/web_ui/src/components/BaseConfigTree.tsx b/packages/web_ui/src/components/BaseConfigTree.tsx index 1c994170..b7a309f3 100644 --- a/packages/web_ui/src/components/BaseConfigTree.tsx +++ b/packages/web_ui/src/components/BaseConfigTree.tsx @@ -44,12 +44,12 @@ function renderInput(inputComponents: Record, def: lib.F } if (def.type === "string") { if (def.credential) { - return ; + return ; } - return ; + return ; } if (def.type === "number") { - return ; + return ; } return `Unknown type ${def.type}`; From a09a5643586d8b6161a0eda08fac7976de285737 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Wed, 18 Sep 2024 23:41:45 +0200 Subject: [PATCH 4/4] Log where Factorio credentials taken from Provide ease of diagnosing credential issues by accurately logging if the credentials were sourced from the instance config, the host config, the controller config, or if no credentials were found. --- packages/host/src/Instance.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/host/src/Instance.ts b/packages/host/src/Instance.ts index 9cbd3461..0df92ee0 100644 --- a/packages/host/src/Instance.ts +++ b/packages/host/src/Instance.ts @@ -589,16 +589,25 @@ rcon.print(game.table_to_json(players))`.replace(/\r?\n/g, " "); */ async resolveServerSettings(overrides: Record, includeCredentials: boolean) { let serverSettings = await this.server.exampleSettings(); - if (includeCredentials && !overrides.username && !overrides.token) { + if (includeCredentials && overrides.username === undefined && overrides.token === undefined) { let credentials = { username: this._host.config.get("host.factorio_username") ?? undefined, token: this._host.config.get("host.factorio_token") ?? undefined, }; if (!credentials.username && !credentials.token) { Object.assign(credentials, await this.sendTo("controller", new lib.GetFactorioCredentialsRequest())); + if (credentials.username || credentials.token) { + this.logger.info("Using Factorio credentials from controller config"); + } else { + this.logger.warn("No Factorio credentials found"); + } + } else { + this.logger.info("Using Factorio credentials from host config"); } if (credentials.username) { serverSettings.username = credentials.username; } if (credentials.token) { serverSettings.token = credentials.token; } + } else if (includeCredentials) { + this.logger.info("Using Factorio credentials from instance config"); } for (let [key, value] of Object.entries(overrides)) {