Merge branch 'master' into feature/role-drop-down

CHANGELOG.md

@@ -35,6 +35,7 @@ Many thanks to the following for contributing to this release:
 - Controller will refuse duplicate host connections from different addresses [#647](https://github.com/clusterio/clusterio/pull/647).
 - 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.
 
 ### Changes
 
@@ -57,11 +58,15 @@ Many thanks to the following for contributing to this release:
 - 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.
 
 ### Breaking Changes
 
 - 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)

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,

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;

packages/host/src/Instance.ts

@@ -583,11 +583,32 @@ 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<string, unknown>) {
+	async resolveServerSettings(overrides: Record<string, unknown>, includeCredentials: boolean) {
 		let serverSettings = await this.server.exampleSettings();
+		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)) {
 			if (!Object.hasOwnProperty.call(serverSettings, key)) {
@@ -605,12 +626,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 +737,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 +907,8 @@ rcon.print(game.table_to_json(players))`.replace(/\r?\n/g, " ");
 	}
 
 	async updateFactorioSettings(current: Record<string, unknown>, previous: Record<string, unknown>) {
-		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 +1031,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 +1061,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 +1092,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 +1120,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();

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}
@@ -111,6 +117,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<Fields> = {
@@ -131,6 +142,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 +205,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 +221,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 +287,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<typeof Config.jsonSchema> {
+	toRemote(remote: ConfigLocation): Static<typeof Config.jsonSchema> {
 		let fields: Record<string, FieldValue> = {};
 		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 +320,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<typeof Config.jsonSchema>,
 		notify: boolean,
-		location = this.location
+		remote = this.location
 	) {
 		const valid = Config.validate(json);
 		if (!valid) {
@@ -316,7 +343,9 @@ export class Config<
 				continue;
 			}
 
-			if (!this._checkAccess(name, def, location, false)) {
+			if (
+				!this._checkAccess(name, def, remote, ConfigAccess.write, false)
+			) {
 				continue;
 			}
 
@@ -346,30 +375,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<Field extends keyof Fields & string>(name: Field, location = this.location) {
+	get<Field extends keyof Fields & string>(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 +421,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<Field extends keyof Fields & string>(name: Field, newValue: Fields[Field], location = this.location) {
+	set<Field extends keyof Fields & string>(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 +494,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 +507,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}'`);

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,29 @@ export class ControllerConfig extends classes.Config<ControllerConfigFields> {
 			type: "number",
 			initialValue: 15,
 		},
+		"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,
+		},
+		"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.",
@@ -176,6 +202,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;
 }
 
@@ -246,6 +274,25 @@ export class HostConfig extends classes.Config<HostConfigFields> {
 			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.",
+			autoComplete: "section-factorio username",
+			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.",
+			autoComplete: "section-factorio new-password",
+			type: "string",
+			credential: ["host"],
+			optional: true,
+		},
 		"host.max_reconnect_delay": {
 			title: "Max Reconnect Delay",
 			description: "Maximum delay to wait before attempting to reconnect WebSocket",

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;

packages/lib/src/link/messages.ts

@@ -24,6 +24,7 @@ export const dataClasses: (RequestClass<unknown, unknown> | EventClass<unknown>)
 	controller.ControllerConfigSetPropRequest,
 	controller.HostGenerateTokenRequest,
 	controller.HostConfigCreateRequest,
+	controller.GetFactorioCredentialsRequest,
 	controller.LogSetSubscriptionsRequest,
 	controller.LogQueryRequest,
 	controller.LogMessageEvent,

packages/web_ui/src/components/BaseConfigTree.tsx

@@ -16,13 +16,13 @@ const { Title } = Typography;
 function getInitialValues(config: lib.Config<any>, 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,10 +43,13 @@ function renderInput(inputComponents: Record<string, InputComponent>, def: lib.F
 		return <Checkbox/>;
 	}
 	if (def.type === "string") {
-		return <Input/>;
+		if (def.credential) {
+			return <Input.Password autoComplete={def.autoComplete} />;
+		}
+		return <Input autoComplete={def.autoComplete} />;
 	}
 	if (def.type === "number") {
-		return <InputNumber/>;
+		return <InputNumber autoComplete={def.autoComplete} />;
 	}
 
 	return `Unknown type ${def.type}`;
@@ -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<any>,
@@ -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<any>).setProp(fieldName, propName, null);
 									setConfig(newConfig);

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(