From e3eb26670463d1513380e9c7008ae5d0b211270f Mon Sep 17 00:00:00 2001 From: Cooldude2606 <25043174+Cooldude2606@users.noreply.github.com> Date: Sat, 21 Sep 2024 01:52:01 +0100 Subject: [PATCH 1/3] Add hidden and readonly config property --- CHANGELOG.md | 1 + packages/lib/src/config/classes.ts | 9 +++++++++ packages/lib/src/config/definitions.ts | 3 +++ packages/web_ui/src/components/BaseConfigTree.tsx | 11 +++++++---- 4 files changed, 20 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 64951976..071647c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,7 @@ Many thanks to the following for contributing to this release: - Monorepos are now supported for external plugins [#654](https://github.com/clusterio/clusterio/pull/654). - Changed default role selection to be a dropdown menu rather than id [#664](https://github.com/clusterio/clusterio/pull/664). - 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). ### Changes diff --git a/packages/lib/src/config/classes.ts b/packages/lib/src/config/classes.ts index 8e3a16ee..ac1c4261 100644 --- a/packages/lib/src/config/classes.ts +++ b/packages/lib/src/config/classes.ts @@ -122,6 +122,15 @@ export interface FieldDefinition { * from the locations specified. */ credential?: ConfigLocation[]; + /** + * True if graphical interfaces should prevent the user modifying the value + * This is client side and does not provide any validation logic + */ + readonly?: boolean; + /** + * True if graphical interfaces should hide the config value + */ + hidden?: boolean; } export type ConfigDefs = { diff --git a/packages/lib/src/config/definitions.ts b/packages/lib/src/config/definitions.ts index f9f9dc47..132d0727 100644 --- a/packages/lib/src/config/definitions.ts +++ b/packages/lib/src/config/definitions.ts @@ -223,6 +223,7 @@ export class HostConfig extends classes.Config { description: "ID of the host", type: "number", initialValue: () => Math.random() * 2**31 | 0, + hidden: true, }, "host.factorio_directory": { description: "Path to directory to look for factorio installs", @@ -345,10 +346,12 @@ export class InstanceConfig extends classes.Config { description: "ID of the instance", type: "number", initialValue: () => Math.random() * 2**31 | 0, + hidden: true, }, "instance.assigned_host": { type: "number", optional: true, + hidden: true, }, "instance.auto_start": { description: "Automatically start this instance when the host hosting it is started up", diff --git a/packages/web_ui/src/components/BaseConfigTree.tsx b/packages/web_ui/src/components/BaseConfigTree.tsx index b7a309f3..6cd5174b 100644 --- a/packages/web_ui/src/components/BaseConfigTree.tsx +++ b/packages/web_ui/src/components/BaseConfigTree.tsx @@ -40,16 +40,16 @@ function renderInput(inputComponents: Record, def: lib.F return ; } if (def.type === "boolean") { - return ; + return ; } if (def.type === "string") { if (def.credential) { - return ; + return ; } - return ; + return ; } if (def.type === "number") { - return ; + return ; } return `Unknown type ${def.type}`; @@ -269,6 +269,9 @@ function computeTreeData( if (!canWrite && !canRead) { continue; } + if (def.hidden === true) { + continue; + } let fieldName = `${groupName}.${field}`; let value = config.get(fieldName) as any; From 49d584394d18b8c77b5ecb2ad10c4ca69cfe8842 Mon Sep 17 00:00:00 2001 From: Cooldude2606 <25043174+Cooldude2606@users.noreply.github.com> Date: Sat, 21 Sep 2024 17:41:46 +0100 Subject: [PATCH 2/3] Add security logic to readonly and object support --- packages/lib/src/config/classes.ts | 21 ++-- packages/web_ui/src/BaseWebPlugin.ts | 1 + .../web_ui/src/components/BaseConfigTree.tsx | 115 ++++++++++-------- .../web_ui/src/components/InputModPack.tsx | 2 +- packages/web_ui/src/components/InputRole.tsx | 1 + 5 files changed, 77 insertions(+), 63 deletions(-) diff --git a/packages/lib/src/config/classes.ts b/packages/lib/src/config/classes.ts index ac1c4261..f6ec27a5 100644 --- a/packages/lib/src/config/classes.ts +++ b/packages/lib/src/config/classes.ts @@ -118,15 +118,17 @@ export interface FieldDefinition { */ access?: ConfigLocation[]; /** - * If present treat the config value as a credential and only allow reading - * from the locations specified. + * If present treat the config value as a credential. + * Only the locations specified will be able to read the current value. + * Access can be used to specify which locations are able to overwrite the value. */ credential?: ConfigLocation[]; /** - * True if graphical interfaces should prevent the user modifying the value - * This is client side and does not provide any validation logic + * If present treat the config value as readonly. + * Only the locations specified will be able to update the config value. + * Access can be used to specify which locations are able to read the value. */ - readonly?: boolean; + readonly?: ConfigLocation[]; /** * True if graphical interfaces should hide the config value */ @@ -240,11 +242,14 @@ export class Config< */ _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 readonly array is present it acts as write permission and the access array as the read permission. + // If neither is present access array acts as a combined read/write permission check. if ( def.credential && mode & ConfigAccess.read && !(def.credential).includes(location) + || def.readonly && mode & ConfigAccess.write && !(def.readonly).includes(location) || ( (!def.credential || mode & ConfigAccess.write) + && (!def.readonly || mode & ConfigAccess.read) && !(def.access ?? this.defaultAccess).includes(location) ) ) { @@ -304,7 +309,7 @@ export class Config< for (let [name, value] of Object.entries(this.fields)) { let def = this.constructor.fieldDefinitions[name]; if ( - !this._checkAccess(name, def, this.location, ConfigAccess.write, false) + this.location !== "control" && !this._checkAccess(name, def, this.location, ConfigAccess.write, false) || this.location !== remote && !this._checkAccess(name, def, remote, ConfigAccess.read, false) ) { continue; @@ -353,7 +358,7 @@ export class Config< } if ( - !this._checkAccess(name, def, remote, ConfigAccess.write, false) + this.location !== "control" && !this._checkAccess(name, def, remote, ConfigAccess.write, false) ) { continue; } diff --git a/packages/web_ui/src/BaseWebPlugin.ts b/packages/web_ui/src/BaseWebPlugin.ts index 286ef9af..4ce654ae 100644 --- a/packages/web_ui/src/BaseWebPlugin.ts +++ b/packages/web_ui/src/BaseWebPlugin.ts @@ -24,6 +24,7 @@ export interface PluginLoginForm { } export interface InputComponentProps { + disabled?: boolean, fieldDefinition: FieldDefinition, value: null | boolean | number | string, onChange: (value: null | boolean | number | string) => void, diff --git a/packages/web_ui/src/components/BaseConfigTree.tsx b/packages/web_ui/src/components/BaseConfigTree.tsx index 6cd5174b..710aead8 100644 --- a/packages/web_ui/src/components/BaseConfigTree.tsx +++ b/packages/web_ui/src/components/BaseConfigTree.tsx @@ -32,24 +32,24 @@ function getInitialValues(config: lib.Config, props: BaseConfigTreeProps) { return initialValues; } -function renderInput(inputComponents: Record, def: lib.FieldDefinition) { +function renderInput(inputComponents: Record, def: lib.FieldDefinition, readonly: boolean) { if (def.inputComponent && Object.prototype.hasOwnProperty.call(inputComponents, def.inputComponent)) { // Field.Item will provide the value and onChange props to the component. type InputPartial = React.ComponentClass>; const CustomInput = inputComponents[def.inputComponent] as unknown as InputPartial; - return ; + return ; } if (def.type === "boolean") { - return ; + return ; } if (def.type === "string") { if (def.credential) { - return ; + return ; } - return ; + return ; } if (def.type === "number") { - return ; + return ; } return `Unknown type ${def.type}`; @@ -264,16 +264,17 @@ function computeTreeData( }; for (let [field, def] of Object.entries(groupDefs)) { - const canWrite = config.canAccess(`${groupName}.${field}`, lib.ConfigAccess.write); - const canRead = config.canAccess(`${groupName}.${field}`, lib.ConfigAccess.read); - if (!canWrite && !canRead) { - continue; - } if (def.hidden === true) { continue; } - let fieldName = `${groupName}.${field}`; + const fieldName = `${groupName}.${field}`; + const canWrite = config.canAccess(fieldName, lib.ConfigAccess.write); + const canRead = config.canAccess(fieldName, lib.ConfigAccess.read); + if (!canWrite && !canRead) { + continue; + } + let value = config.get(fieldName) as any; let childNode: ChildNode = { key: fieldName, @@ -314,7 +315,7 @@ function computeTreeData( return Promise.resolve(); }}]} > - + , }); @@ -322,47 +323,53 @@ function computeTreeData( propsMap.set(propPath, [fieldName, prop]); } - let newPropPath = `${groupName}.${field}:add`; - childNode.children.push({ - key: newPropPath, - title: - - - : - - - - - , - }); + let propPath = `${groupName}.${field}.${propName}`; + form.setFieldsValue({ + [`${newPropPath}.name`]: "", + [`${newPropPath}.value`]: "", + [propPath]: propValue, + }); + form.validateFields([propPath]); + onValuesChange({ [propPath]: propValue }); + }} + > + Add + + , + }); + } + + if (!childNode.children.length) { + continue; + } } else { childNode.title = - {renderInput(control.inputComponents, def)} + {renderInput(control.inputComponents, def, !canWrite)} ; initialValues[fieldName] = value; } diff --git a/packages/web_ui/src/components/InputModPack.tsx b/packages/web_ui/src/components/InputModPack.tsx index fd16a5cf..953a999e 100644 --- a/packages/web_ui/src/components/InputModPack.tsx +++ b/packages/web_ui/src/components/InputModPack.tsx @@ -15,6 +15,6 @@ export default function InputModPack(props: InputComponentProps) { value: modPack.id, }))} allowClear={props.fieldDefinition.optional} + disabled={props.disabled} />; } - diff --git a/packages/web_ui/src/components/InputRole.tsx b/packages/web_ui/src/components/InputRole.tsx index 94712e54..174f0b42 100644 --- a/packages/web_ui/src/components/InputRole.tsx +++ b/packages/web_ui/src/components/InputRole.tsx @@ -26,5 +26,6 @@ export default function InputRole(props: InputComponentProps) { value: role.id, }))} allowClear={props.fieldDefinition.optional} + disabled={props.disabled} />; } From 8a47fd84144f84a4a4c884d40e328e423cdf5a38 Mon Sep 17 00:00:00 2001 From: Cooldude2606 <25043174+Cooldude2606@users.noreply.github.com> Date: Sat, 21 Sep 2024 18:09:29 +0100 Subject: [PATCH 3/3] Update tests --- test/lib/config/classes.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/lib/config/classes.js b/test/lib/config/classes.js index efa7c6aa..cec34c9d 100644 --- a/test/lib/config/classes.js +++ b/test/lib/config/classes.js @@ -27,6 +27,7 @@ describe("lib/config/classes", function() { "test.json": { type: "object", initialValue: {}, optional: true }, "test.priv": { access: ["local"], type: "string", optional: true }, "test.cred": { credential: ["local"] }, + "test.rdo": { readonly: ["local"] }, }; constructor(location, fields) { @@ -74,6 +75,7 @@ describe("lib/config/classes", function() { "test.json": {}, "test.priv": null, "test.cred": null, + "test.rdo": null, }); }); }); @@ -89,6 +91,7 @@ describe("lib/config/classes", function() { "test.func": 42, "test.bool": false, "test.json": {}, + "test.rdo": null, }); }); }); @@ -151,6 +154,7 @@ describe("lib/config/classes", function() { "test.json": {}, "test.priv": null, "test.cred": null, + "test.rdo": null, "test.alpha": null, "test.beta": "decay", "test.gamma": 99, @@ -185,6 +189,7 @@ describe("lib/config/classes", function() { "test.json": {}, "test.priv": null, "test.cred": null, + "test.rdo": null, "test.test": null, }); }); @@ -256,6 +261,9 @@ describe("lib/config/classes", function() { 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); + assert.equal(testInstance.canAccess("test.rdo", CA.read), true); + assert.equal(testInstance.canAccess("test.rdo", CA.read, "local"), true); + assert.equal(testInstance.canAccess("test.rdo", CA.read, "remote"), true); }); it("should return true checking write for fields that are writeable", function() { let testInstance = new TestConfig("local"); @@ -271,6 +279,9 @@ describe("lib/config/classes", function() { 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); + assert.equal(testInstance.canAccess("test.rdo", CA.write), true); + assert.equal(testInstance.canAccess("test.rdo", CA.write, "local"), true); + assert.equal(testInstance.canAccess("test.rdo", CA.write, "remote"), false); }); it("should return true for checking readWrite for fields that are readable and writeable", function() { let testInstance = new TestConfig("local"); @@ -286,6 +297,9 @@ describe("lib/config/classes", function() { 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); + assert.equal(testInstance.canAccess("test.rdo", CA.readWrite), true); + assert.equal(testInstance.canAccess("test.rdo", CA.readWrite, "local"), true); + assert.equal(testInstance.canAccess("test.rdo", CA.readWrite, "remote"), false); }); });