Simplify approving hosts

The list of waiting hosts opens the approval dialog instead of having its
own name input and buttons, so hosts are approved in one place. Removes
the limit on requests per minute, the limits on pending requests already
cap what an address can do.

Also updates text that still said the installer makes the requests.

docs/devs/protocol.md

@@ -61,7 +61,7 @@ If a user token is sent and the user has the `core.host.approve` permission the
 An invalid token gives 401 and a user without the permission gives 403, a token is never ignored to create a pending request instead.
 
 A missing or too long name gives 400.
-Too many requests from the same address or in total gives 429 with a `Retry-After` header in seconds.
+Too many pending requests from the same address or in total gives 429 with a `Retry-After` header in seconds.
 
 ### `POST /api/host-pairing/poll`
 

packages/controller/src/Controller.ts

@@ -92,7 +92,7 @@ export default class Controller {
 	users: UserManager;
 	/** Instance manager for the cluster, also contains access to the user records */
 	instances: InstanceManager;
-	/** Pending requests from host installers to join the cluster */
+	/** Pending requests from hosts to join the cluster */
 	hostPairing: HostPairingManager;
 
 	// Possible states are new, starting, running, stopping, stopped

packages/controller/src/HostPairingManager.ts

@@ -12,7 +12,7 @@ import HostRecord from "./HostRecord.js";
 const codeAlphabet = "23456789ABCDEFGHJKMNPQRSTUVWXYZ";
 const codeLength = 6;
 
-/** Time a request is kept after the last poll from the installer */
+/** Time a request is kept after the last poll from the host */
 export const pollTimeoutMs = 30e3;
 /** Maximum time a request can stay pending */
 export const maxPendingMs = 60 * 60e3;
@@ -20,21 +20,19 @@ export const maxPendingMs = 60 * 60e3;
 export const maxPending = 50;
 /** Maximum number of pending requests from one address */
 export const maxPendingPerAddress = 3;
-/** Maximum number of requests created from one address per minute */
-export const maxCreatesPerMinute = 10;
 /** Maximum length of a host name */
 export const maxNameLength = 100;
 /** Control and bidirectional override characters refused in host names */
 const nameControlCharacters = /[\p{Cc}\u202a-\u202e\u2066-\u2069]/u;
 
-/** Result of an approved request, sent to the installer */
+/** Result of an approved request, sent to the host */
 export interface HostPairingResult {
 	hostId: number,
 	token: string,
 	name: string,
 }
 
-/** Error with an HTTP status code to respond to the installer with */
+/** Error with an HTTP status code to respond to the host with */
 export class HostPairingError extends Error {
 	constructor(
 		message: string,
@@ -47,7 +45,7 @@ export class HostPairingError extends Error {
 }
 
 interface PairingRequest {
-	/** Secret the installer polls with, never shown to users */
+	/** Secret the host polls with, never shown to users */
 	secret: string,
 	details: lib.HostPairingDetails,
 	/** Address with IPv6 addresses grouped by /64, used for limits */
@@ -84,7 +82,7 @@ export function addressKey(address: string) {
  * Normalise a pairing code typed by a user
  *
  * @param code - Code as typed, may contain dashes and lower case letters.
- * @returns code formatted as shown by the installer.
+ * @returns code formatted as shown by the host.
  */
 export function normaliseCode(code: string) {
 	const plain = code.toUpperCase().replace(/[^0-9A-Z]/g, "");
@@ -100,9 +98,9 @@ function generateCode() {
 }
 
 /**
- * Keeps track of requests from host installers to join the cluster
+ * Keeps track of requests from hosts to join the cluster
  *
- * Requests only live in memory, and only while the installer keeps polling
+ * Requests only live in memory, and only while the host keeps polling
  * for the result.
  */
 export default class HostPairingManager {
@@ -110,8 +108,6 @@ export default class HostPairingManager {
 	requests = new lib.SubscribableDatastore<lib.HostPairingDetails>();
 	private _bySecret = new Map<string, PairingRequest>();
 	private _byCode = new Map<string, PairingRequest>();
-	/** Creation times of recent requests by address key */
-	private _recentCreates = new Map<string, number[]>();
 	private _pruneInterval?: ReturnType<typeof setInterval>;
 
 	constructor(
@@ -129,7 +125,7 @@ export default class HostPairingManager {
 	}
 
 	/**
-	 * Check and trim a host name given by an installer or a user
+	 * Check and trim a host name given by a host or a user
 	 *
 	 * @param name - Name to check.
 	 * @returns the trimmed name.
@@ -180,8 +176,8 @@ export default class HostPairingManager {
 	/**
 	 * Create a pending request
 	 *
-	 * @param name - Name the installer asked for.
-	 * @param version - Version of the installer.
+	 * @param name - Name the host asked for.
+	 * @param version - Version of the host.
 	 * @param remoteAddress - Address the request came from.
 	 * @returns the secret to poll with and the code to show the user.
 	 */
@@ -189,12 +185,6 @@ export default class HostPairingManager {
 		this.prune();
 		const key = addressKey(remoteAddress);
 		const now = Date.now();
-		const recent = (this._recentCreates.get(key) ?? []).filter(ms => ms > now - 60e3);
-		if (recent.length >= maxCreatesPerMinute) {
-			throw new HostPairingError(
-				"Too many requests from this address", 429, Math.ceil((recent[0] + 60e3 - now) / 1000),
-			);
-		}
 		const pendingFromAddress = [...this._bySecret.values()].filter(
 			request => request.addressKey === key && request.state === "pending"
 		).length;
@@ -204,8 +194,6 @@ export default class HostPairingManager {
 		if (this.requests.size >= maxPending) {
 			throw new HostPairingError("Too many pending requests", 429, 30);
 		}
-		recent.push(now);
-		this._recentCreates.set(key, recent);
 
 		let code;
 		do {
@@ -332,7 +320,7 @@ export default class HostPairingManager {
 		}
 	}
 
-	/** Remove requests the installer stopped polling for or that waited too long */
+	/** Remove requests the host stopped polling for or that waited too long */
 	prune() {
 		const now = Date.now();
 		for (const request of this._bySecret.values()) {
@@ -347,15 +335,10 @@ export default class HostPairingManager {
 			if (request.state === "approved") {
 				logger.warn(
 					`Approved host ${request.result!.name} (${request.result!.hostId}) was never picked up ` +
-					"by its installer, revoke it on the hosts page if it is not needed"
+					"by the host, revoke it on the hosts page if it is not needed"
 				);
 			}
 			this._remove(request);
 		}
-		for (const [key, times] of this._recentCreates) {
-			if (!times.some(ms => ms > now - 60e3)) {
-				this._recentCreates.delete(key);
-			}
-		}
 	}
 }

packages/controller/src/routes.ts

@@ -255,7 +255,7 @@ function checkHostPairingRequest(req: Request, res: Response, needsRequestId: bo
 	return true;
 }
 
-// Request from an installer for a host to join the cluster.
+// Request from a host for a host to join the cluster.
 async function createHostPairing(req: Request, res: Response) {
 	if (!checkHostPairingRequest(req, res, false)) {
 		return;
@@ -299,7 +299,7 @@ async function createHostPairing(req: Request, res: Response) {
 	}
 }
 
-// Poll from an installer for the result of its request.
+// Poll from a host for the result of its request.
 function pollHostPairing(req: Request, res: Response) {
 	if (!checkHostPairingRequest(req, res, true)) {
 		return;
@@ -318,7 +318,7 @@ function pollHostPairing(req: Request, res: Response) {
 	}
 }
 
-// Cancel from an installer that stopped waiting.
+// Cancel from a host that stopped waiting.
 function cancelHostPairing(req: Request, res: Response) {
 	if (!checkHostPairingRequest(req, res, true)) {
 		return;

packages/lib/src/data/HostPairingDetails.ts

@@ -1,20 +1,20 @@
 import { Type, Static } from "@sinclair/typebox";
 
 /**
- * A pending request from a host installer to join the cluster
+ * A pending request from a host to join the cluster
  *
- * Requests are kept in memory on the controller only while the installer
+ * Requests are kept in memory on the controller only while the host
  * waits for them to be approved or rejected.
  */
 export default class HostPairingDetails {
 	constructor(
-		/** Code shown by the installer, used to identify the request. */
+		/** Code shown by the host, used to identify the request. */
 		public code: string,
-		/** Name the installer asked for the host to have. */
+		/** Name the host asked to have. */
 		public name: string,
 		/** Address the request was made from. */
 		public remoteAddress: string,
-		/** Version of the installer that made the request. */
+		/** Version of the host that made the request. */
 		public version = "",
 		/** Millisecond Unix timestamp the request was made at. */
 		public createdAtMs = 0,

packages/lib/src/permissions.ts

@@ -199,8 +199,8 @@ export const corePermissions = [
 		name: "core.host.approve",
 		title: "Approve hosts",
 		description:
-			"List, approve and reject requests from host installers to join the cluster, and approve hosts " +
-			"directly by passing a token to the installer. Approving gives the new host full access to the cluster.",
+			"List, approve and reject requests from hosts to join the cluster, and approve hosts " +
+			"directly by starting them with a user token. Approving gives the new host full access to the cluster.",
 	},
 	{
 		name: "core.host.revoke",

packages/web_ui/src/components/HostsPage.tsx

@@ -46,69 +46,19 @@ function useNow(active: boolean) {
 }
 
 function AddHostModal(props: { open: boolean, onClose: () => void }) {
-	const control = useContext(ControlContext);
 	const [requests, synced] = useHostPairings();
-	const [names, setNames] = useState<ReadonlyMap<string, string>>(new Map());
-	const [busyCodes, setBusyCodes] = useState<ReadonlySet<string>>(new Set());
+	const [, setParams] = useSearchParams();
 	const now = useNow(props.open);
 
 	const controllerUrl = new URL(webRoot, document.location.href).href;
 	const installCommand = `npm init "@clusterio" -- --mode host --controller-url ${controllerUrl}`;
 
-	function setName(code: string, name?: string) {
-		setNames(current => {
-			const next = new Map(current);
-			if (name === undefined) {
-				next.delete(code);
-			} else {
-				next.set(code, name);
-			}
+	function review(code: string) {
+		setParams(current => {
+			const next = new URLSearchParams(current);
+			next.set("approve", code);
 			return next;
-		});
-	}
-
-	function setBusy(code: string, busy: boolean) {
-		setBusyCodes(current => {
-			const next = new Set(current);
-			if (busy) {
-				next.add(code);
-			} else {
-				next.delete(code);
-			}
-			return next;
-		});
-	}
-
-	function nameOf(request: Readonly<lib.HostPairingDetails>) {
-		return (names.get(request.code) ?? request.name).trim();
-	}
-
-	function approve(request: Readonly<lib.HostPairingDetails>) {
-		const name = nameOf(request);
-		setBusy(request.code, true);
-		control.send(
-			new lib.HostPairingApproveRequest(request.code, name !== request.name ? name : undefined)
-		).then(() => {
-			setName(request.code);
-			notify(`Approved host ${name}`, "success");
-		}).catch(
-			notifyErrorHandler(`Error approving host ${request.name}`)
-		).finally(() => {
-			setBusy(request.code, false);
-		});
-	}
-
-	function reject(request: Readonly<lib.HostPairingDetails>) {
-		setBusy(request.code, true);
-		control.send(
-			new lib.HostPairingRejectRequest(request.code)
-		).then(() => {
-			setName(request.code);
-		}).catch(
-			notifyErrorHandler(`Error rejecting host ${request.name}`)
-		).finally(() => {
-			setBusy(request.code, false);
-		});
+		}, { replace: true });
 	}
 
 	return <Modal
@@ -139,10 +89,6 @@ function AddHostModal(props: { open: boolean, onClose: () => void }) {
 			Later starts connect to the cluster straight away.
 		</Typography.Paragraph>
 		<Typography.Title level={5}>Waiting for approval</Typography.Title>
-		<Typography.Paragraph type="secondary">
-			Only approve a request if its code matches the code shown by the host.
-			An approved host has full access to the cluster.
-		</Typography.Paragraph>
 		<Table
 			size="small"
 			pagination={false}
@@ -153,13 +99,7 @@ function AddHostModal(props: { open: boolean, onClose: () => void }) {
 			columns={[
 				{
 					title: "Name",
-					key: "name",
-					render: (_, request) => <Input
-						size="small"
-						aria-label={`Name for host with code ${request.code}`}
-						value={names.get(request.code) ?? request.name}
-						onChange={event => { setName(request.code, event.target.value); }}
-					/>,
+					dataIndex: "name",
 				},
 				{
 					title: "Code",
@@ -180,23 +120,11 @@ function AddHostModal(props: { open: boolean, onClose: () => void }) {
 				{
 					key: "action",
 					align: "right",
-					render: (_, request) => {
-						const busy = busyCodes.has(request.code);
-						return <Space>
-							<Button
-								type="primary"
-								size="small"
-								disabled={busy || !nameOf(request)}
-								onClick={() => { approve(request); }}
-							>Approve</Button>
-							<Button
-								danger
-								size="small"
-								disabled={busy}
-								onClick={() => { reject(request); }}
-							>Reject</Button>
-						</Space>;
-					},
+					render: (_, request) => <Button
+						type="primary"
+						size="small"
+						onClick={() => { review(request.code); }}
+					>Review</Button>,
 				},
 			]}
 		/>

test/controller/HostPairingManager.test.js

@@ -6,7 +6,7 @@ import * as lib from "@clusterio/lib";
 import { Controller, HostPairingManager, HostPairingError, HostRecord } from "@clusterio/controller";
 import {
 	pollTimeoutMs, maxPendingMs, maxPending, maxPendingPerAddress,
-	maxCreatesPerMinute, maxNameLength,
+	maxNameLength,
 } from "@clusterio/controller/dist/node/src/HostPairingManager.js";
 
 import "../setup_logging.js";
@@ -277,48 +277,6 @@ describe("controller/src/HostPairingManager", function() {
 				});
 				assert.equal(manager.requests.size, maxPending);
 			});
-			it("should limit the number of requests created per minute from an address", function() {
-				assert.equal(maxCreatesPerMinute, 10);
-				for (let i = 0; i < maxCreatesPerMinute; i++) {
-					manager.cancel(manager.create("Host", "", "10.0.0.1").requestId);
-				}
-				assert.throws(() => manager.create("Host", "", "10.0.0.1"), err => {
-					assert(err instanceof HostPairingError);
-					assert.equal(err.status, 429);
-					assert.equal(err.message, "Too many requests from this address");
-					assert(err.retryAfter >= 1 && err.retryAfter <= 60, `bad retryAfter ${err.retryAfter}`);
-					return true;
-				});
-				// IPv6 addresses in the same /64 share the limit
-				for (let i = 0; i < maxCreatesPerMinute; i++) {
-					manager.cancel(manager.create("Host", "", `2001:db8:0:1::${i + 1}`).requestId);
-				}
-				assertPairingError(
-					() => manager.create("Host", "", "2001:db8:0:1:ffff::1"), 429, "Too many requests from this address"
-				);
-				manager.create("Host", "", "10.0.0.2");
-			});
-			it("should allow creating requests again after a minute", function() {
-				for (let i = 0; i < maxCreatesPerMinute; i++) {
-					manager.cancel(manager.create("Host", "", "10.0.0.1").requestId);
-				}
-				assertPairingError(() => manager.create("Host", "", "10.0.0.1"), 429);
-				for (const times of manager._recentCreates.values()) {
-					times.forEach((ms, index) => { times[index] = ms - 60e3 - 1; });
-				}
-				manager.create("Host", "", "10.0.0.1");
-			});
-			it("should not count refused requests towards the rate limit", function() {
-				const first = manager.create("Host", "", "10.0.0.1");
-				manager.create("Host", "", "10.0.0.1");
-				manager.create("Host", "", "10.0.0.1");
-				const createRefused = () => manager.create("Host", "", "10.0.0.1");
-				for (let i = 0; i < maxCreatesPerMinute; i++) {
-					assertPairingError(createRefused, 429);
-				}
-				manager.cancel(first.requestId);
-				manager.create("Host", "", "10.0.0.1");
-			});
 		});
 
 		describe(".poll()", function() {