Add HostManager for host records and host tokens

controller.hosts is now a HostManager wrapping the hosts datastore, the
same way controller.users wraps the users datastore. It owns creating
hosts (moved from HostPairingManager), revoking them (moved from
ControlConnection) and signing and verifying host tokens (moved from
Controller, generateHostToken is renamed signHostToken to match
signUserToken).

UserManager gains verifyUserToken, replacing the copies of the user
token check in WsServer and routes. The WsServer copy accepted a token
without iat when the user had no tokenValidAfter; the shared check
always requires iat, which jsonwebtoken adds by default.

Follows Cooldude2606's comment on #1161.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

test/controller/ControlConnection.test.js

@@ -441,7 +441,7 @@ describe("controller/src/ControlConnection", function() {
 
 			beforeEach(function() {
 				record = new HostRecord(10, "host-10", "", new Map());
-				controller.hosts.set(record);
+				controller.hosts.records.set(record);
 				broadcasts = [];
 				controller.subscriptions.broadcast = event => {
 					if (event instanceof lib.HostUpdatesEvent) {
@@ -566,7 +566,7 @@ describe("controller/src/ControlConnection", function() {
 				await connection.handleHostPairingRejectRequest(new lib.HostPairingRejectRequest(code.toLowerCase()));
 				assert.equal(controller.hostPairing.records.has(code), false);
 				assert.throws(() => controller.hostPairing.poll(requestId), /Request was rejected/);
-				assert.equal(controller.hosts.size, 0);
+				assert.equal(controller.hosts.records.size, 0);
 				assert(audits.some(msg => msg.startsWith(`User admin rejected host Host A with code ${code}`)));
 			});
 		});

test/controller/HostConnection.test.js

@@ -53,10 +53,10 @@ describe("controller/src/HostConnection", function() {
 		beforeEach(function() {
 			controller = new Controller(lib.logger, [], new lib.ControllerConfig("controller"));
 			record = new HostRecord(10, "host-10", "1.0.0", new Map(), false, "", "203.0.113.1", 123);
-			controller.hosts.set(record);
+			controller.hosts.records.set(record);
 			connector = new MockConnector(addr("controller"), addr({ hostId: 10 }));
 			updates = [];
-			controller.hosts.on("update", changed => {
+			controller.hosts.records.on("update", changed => {
 				updates.push(...changed.map(host => ({ id: host.id, name: host.name, isDeleted: host.isDeleted })));
 			});
 		});
@@ -68,7 +68,7 @@ describe("controller/src/HostConnection", function() {
 		}
 
 		function revoke() {
-			controller.hosts.delete(record);
+			controller.hosts.records.delete(record);
 			updates.length = 0;
 		}
 
@@ -127,7 +127,7 @@ describe("controller/src/HostConnection", function() {
 			connect();
 			revoke();
 			const newer = new HostRecord(10, "newer", "", new Map());
-			controller.hosts.set(newer);
+			controller.hosts.records.set(newer);
 			updates.length = 0;
 			connector.emit("close");
 			assert.equal(controller.hosts.get(10), newer);

test/controller/HostManager.test.js

@@ -0,0 +1,123 @@
+import assert from "node:assert/strict";
+import crypto from "node:crypto";
+import jwt from "jsonwebtoken";
+
+import * as lib from "@clusterio/lib";
+import { Controller, HostManager, HostRecord } from "@clusterio/controller";
+
+import "../setup_logging.js";
+
+describe("controller/src/HostManager", function() {
+	/** @type {Controller} */
+	let controller;
+	/** @type {HostManager} */
+	let hosts;
+
+	beforeEach(function() {
+		const config = new lib.ControllerConfig("controller");
+		config.set("controller.auth_secret", Buffer.from("TestSecretDoNotUse").toString("base64"));
+		controller = new Controller(lib.logger, [], config);
+		hosts = controller.hosts;
+	});
+
+	it("should be the host manager of the controller", function() {
+		assert(hosts instanceof HostManager);
+	});
+
+	describe(".has() / .get() / .getMutable() / .values() / .getForRequest()", function() {
+		it("should return the host when it exists", function() {
+			const record = new HostRecord(5, "five", "", new Map());
+			hosts.records.set(record);
+			assert.equal(hosts.has(5), true);
+			assert.equal(hosts.get(5), record);
+			assert.equal(hosts.getMutable(5), record);
+			assert.deepEqual([...hosts.values()], [record]);
+			assert.equal(hosts.getForRequest(5), record);
+		});
+		it("should return undefined or throw when the host does not exist", function() {
+			assert.equal(hosts.has(5), false);
+			assert.equal(hosts.get(5), undefined);
+			assert.equal(hosts.getMutable(5), undefined);
+			assert.throws(() => hosts.getForRequest(5), new lib.NotFoundError("Unknown host id (5)"));
+		});
+	});
+
+	describe(".createHost()", function() {
+		let originalRandomInt;
+		afterEach(function() {
+			if (originalRandomInt) {
+				crypto.randomInt = originalRandomInt;
+				originalRandomInt = undefined;
+			}
+		});
+
+		it("should create a host record that its tokens are valid for", function() {
+			const before = Math.floor(Date.now() / 1000);
+			const host = hosts.createHost("Host A");
+
+			assert(host instanceof HostRecord);
+			assert(Number.isInteger(host.id), "id is not an integer");
+			assert(host.id >= 1 && host.id < 2 ** 31, `id ${host.id} out of range`);
+			assert.equal(hosts.get(host.id), host);
+			assert.equal(host.name, "Host A");
+			assert.equal(host.connected, false);
+			assert(host.tokenValidAfter >= before, "tokenValidAfter predates the call");
+			assert(host.tokenValidAfter <= Math.floor(Date.now() / 1000), "tokenValidAfter is in the future");
+			assert.equal(hosts.verifyHostToken(hosts.signHostToken(host.id)), host.id);
+		});
+		it("should not reuse the id of an existing host", function() {
+			hosts.records.set(new HostRecord(5, "existing", "", new Map()));
+			const values = [5, 5, 6];
+			originalRandomInt = crypto.randomInt;
+			crypto.randomInt = (...args) => (values.length ? values.shift() : originalRandomInt(...args));
+			const host = hosts.createHost("Host A");
+			assert.equal(host.id, 6);
+			assert.equal(hosts.get(5).name, "existing");
+			assert.equal(hosts.get(6).name, "Host A");
+		});
+	});
+
+	describe(".signHostToken()", function() {
+		it("should return a host token for the id", function() {
+			const payload = jwt.verify(hosts.signHostToken(5), controller.authSecret);
+			const { iat, ...rest } = payload;
+			assert.equal(typeof iat, "number");
+			assert.deepEqual(rest, { aud: "host", host: 5 });
+		});
+	});
+
+	describe(".verifyHostToken()", function() {
+		beforeEach(function() {
+			hosts.records.set(new HostRecord(5, "five", "", new Map()));
+		});
+
+		it("should return the id of the host", function() {
+			assert.equal(hosts.verifyHostToken(hosts.signHostToken(5)), 5);
+		});
+		it("should accept a token from before the slave rename", function() {
+			const token = jwt.sign({ aud: "slave", slave: 5 }, controller.authSecret);
+			assert.equal(hosts.verifyHostToken(token), 5);
+		});
+		it("should reject a token for an unknown host", function() {
+			assert.throws(() => hosts.verifyHostToken(hosts.signHostToken(6)), /host is not registered/);
+		});
+		it("should reject a token issued before tokenValidAfter", function() {
+			const now = Math.floor(Date.now() / 1000);
+			hosts.getMutable(5).tokenValidAfter = now;
+			const token = jwt.sign({ aud: "host", host: 5, iat: now - 1 }, controller.authSecret);
+			assert.throws(() => hosts.verifyHostToken(token), new Error("invalid token"));
+		});
+		it("should reject user tokens and bad signatures", function() {
+			const userToken = jwt.sign({ aud: "user", user: "test" }, controller.authSecret);
+			assert.throws(() => hosts.verifyHostToken(userToken), /audience invalid/);
+			const forged = jwt.sign({ aud: "host", host: 5 }, "wrong secret");
+			assert.throws(() => hosts.verifyHostToken(forged), /invalid signature/);
+		});
+	});
+
+	describe(".revokeHost()", function() {
+		it("should throw for an unknown host", async function() {
+			await assert.rejects(hosts.revokeHost(5, "admin"), new lib.NotFoundError("Unknown host id (5)"));
+		});
+	});
+});

test/controller/HostPairingManager.test.js

@@ -3,7 +3,7 @@ import crypto from "node:crypto";
 import jwt from "jsonwebtoken";
 
 import * as lib from "@clusterio/lib";
-import { Controller, HostPairingManager, HostPairingError, HostRecord } from "@clusterio/controller";
+import { Controller, HostPairingManager, HostPairingError } from "@clusterio/controller";
 import {
 	pollTimeoutMs, maxPendingPerAddress, maxNameLength,
 } from "@clusterio/controller/dist/node/src/HostPairingManager.js";
@@ -109,36 +109,6 @@ describe("controller/src/HostPairingManager", function() {
 			internal(requestId).lastPollMs -= ms;
 		}
 
-		describe(".createHost()", function() {
-			it("should create a host record with a valid token", function() {
-				const before = Math.floor(Date.now() / 1000);
-				const result = manager.createHost("Host A");
-
-				assert.equal(result.name, "Host A");
-				assert(Number.isInteger(result.hostId), "hostId is not an integer");
-				assert(result.hostId >= 1 && result.hostId < 2 ** 31, `hostId ${result.hostId} out of range`);
-
-				const record = controller.hosts.get(result.hostId);
-				assert(record instanceof HostRecord, "host record was not created");
-				assert.equal(record.name, "Host A");
-				assert.equal(record.connected, false);
-				assert(record.tokenValidAfter >= before, "tokenValidAfter predates the call");
-				assert(record.tokenValidAfter <= Math.floor(Date.now() / 1000), "tokenValidAfter is in the future");
-
-				const payload = jwt.verify(result.token, controller.authSecret, { audience: "host" });
-				assert.equal(payload.host, result.hostId);
-				assert(payload.iat >= record.tokenValidAfter, "token is not valid for the new record");
-			});
-			it("should not reuse the id of an existing host", function() {
-				controller.hosts.set(new HostRecord(5, "existing", "", new Map()));
-				restoreRandomInt = stubRandomInt([5, 5, 6]);
-				const result = manager.createHost("Host A");
-				assert.equal(result.hostId, 6);
-				assert.equal(controller.hosts.get(5).name, "existing");
-				assert.equal(controller.hosts.get(6).name, "Host A");
-			});
-		});
-
 		describe(".create()", function() {
 			it("should create a pending request", function() {
 				const before = Date.now();
@@ -348,10 +318,10 @@ describe("controller/src/HostPairingManager", function() {
 					() => manager.approve(code, "x".repeat(maxNameLength + 1), "admin"),
 					new lib.ValidationError("Host name must be at most 100 characters"),
 				);
-				assert.equal(controller.hosts.size, 0);
+				assert.equal(controller.hosts.records.size, 0);
 				assert.equal(manager.poll(requestId), null);
 				manager.approve(code, undefined, "admin");
-				assert.equal(controller.hosts.size, 1);
+				assert.equal(controller.hosts.records.size, 1);
 			});
 			it("should throw NotFoundError for an unknown code", function() {
 				assert.throws(
@@ -363,7 +333,7 @@ describe("controller/src/HostPairingManager", function() {
 				const { code } = manager.create("Host A", "", "10.0.0.1");
 				manager.approve(code, undefined, "admin");
 				assert.throws(() => manager.approve(code, undefined, "admin"), lib.NotFoundError);
-				assert.equal(controller.hosts.size, 1);
+				assert.equal(controller.hosts.records.size, 1);
 			});
 		});
 
@@ -373,7 +343,7 @@ describe("controller/src/HostPairingManager", function() {
 				manager.reject(code.toLowerCase(), "admin");
 				assert.equal(manager.records.has(code), false);
 				assert.deepEqual(updates, [{ code, isDeleted: false }, { code, isDeleted: true }]);
-				assert.equal(controller.hosts.size, 0);
+				assert.equal(controller.hosts.records.size, 0);
 			});
 			it("should throw NotFoundError for an unknown code", function() {
 				assert.throws(

test/controller/UserManager.test.js

@@ -135,6 +135,31 @@ describe("controller/UserManager", function () {
 		});
 	});
 
+	describe(".verifyUserToken()", function () {
+		const secret = () => Buffer.from(config.get("controller.auth_secret"), "base64");
+
+		it("should return the user the token was signed for", function () {
+			const user = userManager.createUser("Judy");
+			assert.equal(userManager.verifyUserToken(userManager.signUserToken(user)).id, user.id);
+		});
+		it("should reject a token for an unknown user", function () {
+			const token = userManager.signUserToken({ id: "nobody" });
+			assert.throws(() => userManager.verifyUserToken(token), new Error("invalid user"));
+		});
+		it("should reject a token issued before tokenValidAfter", function () {
+			const user = userManager.createUser("Mallory");
+			const token = jwt.sign({ aud: "user", user: user.id, iat: user.tokenValidAfter - 1 }, secret());
+			assert.throws(() => userManager.verifyUserToken(token), new Error("invalid token"));
+		});
+		it("should reject host tokens and bad signatures", function () {
+			const user = userManager.createUser("Niaj");
+			const hostToken = jwt.sign({ aud: "host", host: 5 }, secret());
+			assert.throws(() => userManager.verifyUserToken(hostToken), /audience invalid/);
+			const forged = jwt.sign({ aud: "user", user: user.id }, "wrong secret");
+			assert.throws(() => userManager.verifyUserToken(forged), /invalid signature/);
+		});
+	});
+
 	describe(".clearStatsOfInstance()", function () {
 		it("should clear instance stats for all users", function () {
 			const instanceId = 42;

test/controller/WsServer.js

@@ -136,7 +136,7 @@ describe("controller/src/WsServer", function() {
 
 		function addHost(id, tokenValidAfter = 0) {
 			const record = new HostRecord(id, `host-${id}`, "", new Map(), false, "", "203.0.113.1", tokenValidAfter);
-			controller.hosts.set(record);
+			controller.hosts.records.set(record);
 			return record;
 		}
 
@@ -152,7 +152,7 @@ describe("controller/src/WsServer", function() {
 		}
 
 		it("should reject a validly signed token for a host that is not registered", async function() {
-			await register(controller.generateHostToken(5));
+			await register(controller.hosts.signHostToken(5));
 			assertRejected(
 				"Authentication failed: host is not registered, start the host again to request to join the cluster"
 			);
@@ -160,12 +160,12 @@ describe("controller/src/WsServer", function() {
 		});
 		it("should accept a registered host", async function() {
 			const record = addHost(5);
-			await register(controller.generateHostToken(5));
+			await register(controller.hosts.signHostToken(5));
 			assert.equal(socket.closed, null);
 			assert(socket.sent.some(message => message.type === "ready"), "ready was not sent");
 			assert(wsServer.hostConnections.has(5));
 
-			assert.equal(controller.hosts.size, 1);
+			assert.equal(controller.hosts.records.size, 1);
 			assert.equal(controller.hosts.get(5), record, "host record was replaced");
 			assert.equal(record.connected, true);
 			assert.equal(record.version, packageJson.version);
@@ -178,7 +178,7 @@ describe("controller/src/WsServer", function() {
 			assert.equal(controller.hosts.get(5).connected, false);
 		});
 		it("should reject a token issued before tokenValidAfter", async function() {
-			const token = controller.generateHostToken(5);
+			const token = controller.hosts.signHostToken(5);
 			addHost(5, Math.floor(Date.now() / 1000) + 60);
 			await register(token);
 			assertRejected("Authentication failed: invalid token");
@@ -195,18 +195,18 @@ describe("controller/src/WsServer", function() {
 			wsServer.hostConnections.set(5, {
 				remoteAddress: "10.0.0.1",
 				async disconnect() {
-					controller.hosts.delete(record);
+					controller.hosts.records.delete(record);
 					wsServer.hostConnections.delete(5);
 				},
 			});
-			await register(controller.generateHostToken(5));
+			await register(controller.hosts.signHostToken(5));
 			assertRejected("Authentication failed: host was revoked");
 			assert.equal(controller.hosts.has(5), false, "host record was recreated");
 		});
 		it("should reject hosts in recovery mode", async function() {
 			addHost(5);
 			controller.recoveryMode = true;
-			await register(controller.generateHostToken(5));
+			await register(controller.hosts.signHostToken(5));
 			assert.deepEqual(socket.closed, {
 				code: lib.ConnectionClosed.RecoveryMode,
 				reason: "Connection Refused: Controller in recovery mode",

test/controller/routes.js

@@ -8,7 +8,7 @@ import jwt from "jsonwebtoken";
 import express from "express";
 
 import * as lib from "@clusterio/lib";
-import { Controller, HostPairingManager, HostRecord } from "@clusterio/controller";
+import { HostPairingManager, HostRecord } from "@clusterio/controller";
 
 import * as routes from "@clusterio/controller/dist/node/src/routes.js";
 import * as mock from "../mock.js";
@@ -258,15 +258,17 @@ describe("controller/src/routes", function() {
 		beforeEach(function() {
 			endpoint = `http://localhost:${port}/api/host-pairing`;
 			// The mock controller lacks what pairing needs, use the real implementations.
-			controller.hosts = new lib.SubscribableDatastore();
 			controller.hostPairing = new HostPairingManager(controller);
-			controller.generateHostToken = Controller.prototype.generateHostToken;
-			controller.verifyHostToken = Controller.prototype.verifyHostToken;
 			controller.wsServer.remoteAddr = req => req.socket.remoteAddress;
 			adminToken = controller.users.signUserToken(controller.users.getByName("test"));
 			playerToken = controller.users.signUserToken(controller.users.getByName("player"));
 		});
 
+		function createHost(name) {
+			const host = controller.hosts.createHost(name);
+			return { hostId: host.id, token: controller.hosts.signHostToken(host.id) };
+		}
+
 		function post(urlPath, body, headers = {}) {
 			return fetch(`${endpoint}${urlPath}`, {
 				method: "POST",
@@ -295,7 +297,7 @@ describe("controller/src/routes", function() {
 
 		function assertNothingCreated() {
 			assert.equal(controller.hostPairing.records.size, 0, "a pending request was created");
-			assert.equal(controller.hosts.size, 0, "a host was created");
+			assert.equal(controller.hosts.records.size, 0, "a host was created");
 		}
 
 		describe("POST /api/host-pairing", function() {
@@ -311,7 +313,7 @@ describe("controller/src/routes", function() {
 				assert.equal(details.name, "Host A");
 				assert.equal(details.version, "2.0.0");
 				assert.match(details.remoteAddress, /127\.0\.0\.1|::1/);
-				assert.equal(controller.hosts.size, 0);
+				assert.equal(controller.hosts.records.size, 0);
 			});
 			it("should trim the name and limit the version", async function() {
 				let { code } = await (await post("", { name: "  Host A ", version: "x".repeat(100) })).json();
@@ -383,7 +385,7 @@ describe("controller/src/routes", function() {
 					const payload = jwt.verify(body.token, controller.authSecret, { audience: "host" });
 					assert.equal(payload.host, body.hostId);
 				}
-				assert.equal(controller.hosts.size, 3);
+				assert.equal(controller.hosts.records.size, 3);
 				assert.equal(controller.hostPairing.records.size, 0, "a pending request was created");
 			});
 			it("should validate the name before approving directly", async function() {
@@ -406,9 +408,9 @@ describe("controller/src/routes", function() {
 				const user = controller.users.getOrCreateUser("test");
 				user.tokenValidAfter = Math.floor((Date.now() + 60e3) / 1000);
 				user.saveRecord();
-				controller.hosts.set(new HostRecord(5, "five", "", new Map()));
-				const hostToken = controller.generateHostToken(5);
-				controller.hosts.delete(controller.hosts.get(5));
+				controller.hosts.records.set(new HostRecord(5, "five", "", new Map()));
+				const hostToken = controller.hosts.signHostToken(5);
+				controller.hosts.records.delete(controller.hosts.get(5));
 
 				for (const headers of [
 					{ "Authorization": "Bearer invalid" },
@@ -510,7 +512,7 @@ describe("controller/src/routes", function() {
 
 		describe("GET /api/host-pairing/check", function() {
 			it("should respond with 200 for a valid host token", async function() {
-				const { hostId, token } = controller.hostPairing.createHost("Host A");
+				const { hostId, token } = createHost("Host A");
 				for (const headers of [
 					{ "Authorization": `Bearer ${token}` },
 					{ "X-Access-Token": token },
@@ -520,20 +522,20 @@ describe("controller/src/routes", function() {
 				assert(controller.hosts.has(hostId));
 			});
 			it("should respond with 200 for a host token from before the slave rename", async function() {
-				const { hostId } = controller.hostPairing.createHost("Host A");
+				const { hostId } = createHost("Host A");
 				const token = jwt.sign({ aud: "slave", slave: hostId }, controller.authSecret);
 				await assertResponse(await check({ "Authorization": `Bearer ${token}` }), 200, { valid: true });
 			});
 			it("should respond with 401 without a valid host token", async function() {
-				controller.hostPairing.createHost("Host A");
+				createHost("Host A");
 				assert.equal((await check({})).status, 401);
 				assert.equal((await check({ "Authorization": "Bearer invalid" })).status, 401);
 				assert.equal((await check({ "Authorization": `Bearer ${adminToken}` })).status, 401);
 			});
 			it("should respond with 401 once the host is revoked", async function() {
-				const { hostId, token } = controller.hostPairing.createHost("Host A");
+				const { hostId, token } = createHost("Host A");
 				await assertResponse(await check({ "Authorization": `Bearer ${token}` }), 200, { valid: true });
-				controller.hosts.delete(controller.hosts.get(hostId));
+				controller.hosts.records.delete(controller.hosts.get(hostId));
 				assert.equal((await check({ "Authorization": `Bearer ${token}` })).status, 401);
 			});
 		});