diff --git a/docs/devs/plugin-migration-to-2.1.md b/docs/devs/plugin-migration-to-2.1.md index 56b96d6a..6a01c757 100644 --- a/docs/devs/plugin-migration-to-2.1.md +++ b/docs/devs/plugin-migration-to-2.1.md @@ -83,5 +83,22 @@ A host joins the cluster by having the code it shows when it first starts approv Host records are no longer created when a host connects, only by approving it, and the controller rejects hosts it has no record of. Revoking a host deletes its record and note. Plugins that keep data per host should remove it when the host is deleted. -On the controller the `update` event of `controller.hosts` gives the record with `isDeleted` set to true, and subscribers of `HostUpdatesEvent` receive the host with `isDeleted` set to true. +On the controller the `update` event of `controller.hosts.records` gives the record with `isDeleted` set to true, and subscribers of `HostUpdatesEvent` receive the host with `isDeleted` set to true. A host that is revoked and approved again gets a new id. + + +## Host manager + +`controller.hosts` is now a `HostManager`, like `controller.users` is a `UserManager`. +`get`, `getMutable`, `has` and `values` work as before. +Everything else on the datastore, like `set`, `delete`, `size` and the `update` event, is on `controller.hosts.records`. + +The host manager also signs and checks host tokens and creates and deletes hosts: + +- `controller.generateHostToken(hostId)` is now `controller.hosts.signHostToken(hostId)`. +- `controller.verifyHostToken(token)` is now `controller.hosts.verifyHostToken(token)`. +- `controller.hostPairing.createHost(name)` is now `controller.hosts.createHost(name)`. + It returns the new `HostRecord` instead of `{ hostId, token, name }`, sign a token for it with `signHostToken`. +- `controller.hosts.revokeHost(hostId, revokedBy)` deletes a host the same way `HostRevokeRequest` does. + +User tokens are checked with `controller.users.verifyUserToken(token)`, which returns the user or throws if the token is invalid. diff --git a/packages/controller/index.ts b/packages/controller/index.ts index 4b3e8f8a..eec80517 100644 --- a/packages/controller/index.ts +++ b/packages/controller/index.ts @@ -5,6 +5,7 @@ export { default as ControlConnection } from "./src/ControlConnection.js"; export { default as HostConnection } from "./src/HostConnection.js"; export { default as InstanceRecord } from "./src/InstanceRecord.js"; export { default as InstanceManager } from "./src/InstanceManager.js"; +export { default as HostManager } from "./src/HostManager.js"; export { default as HostRecord } from "./src/HostRecord.js"; export { default as HostPairingManager, HostPairingError } from "./src/HostPairingManager.js"; export { BaseControllerPlugin, ControllerHooks, type ControllerPluginContext } from "./src/BaseControllerPlugin.js"; diff --git a/packages/controller/src/ControlConnection.ts b/packages/controller/src/ControlConnection.ts index aa618603..05c1e833 100644 --- a/packages/controller/src/ControlConnection.ts +++ b/packages/controller/src/ControlConnection.ts @@ -252,30 +252,7 @@ export default class ControlConnection extends BaseConnection { } async handleHostRevokeRequest(request: lib.HostRevokeRequest) { - const host = this._controller.hosts.getMutable(request.hostId); - if (!host) { - throw new lib.NotFoundError(`Unknown host id (${request.hostId})`); - } - - this._controller.hosts.delete(host); - this._controller.deleteNote("host", request.hostId, this.user.name); - const system = this._controller.systems.getMutable(request.hostId); - if (system) { - this._controller.systems.delete(system); - } - - // Disconnected first so unassigning the instances does not depend on the host responding. - const hostConnection = this._controller.wsServer.hostConnections.get(request.hostId); - if (hostConnection) { - hostConnection.connector.terminate(); - } - const assigned = [...this._controller.instances.values()].filter( - instance => instance.config.get("instance.assigned_host") === request.hostId - ); - for (const instance of assigned) { - await this._controller.instances.unassignInstance(instance.id); - } - logger.audit(`User ${this.user.name} revoked host ${host.name} (${host.id})`); + await this._controller.hosts.revokeHost(request.hostId, this.user.name); } async handleHostListRequest(): Promise { diff --git a/packages/controller/src/Controller.ts b/packages/controller/src/Controller.ts index e51d78fa..e5058ff2 100644 --- a/packages/controller/src/Controller.ts +++ b/packages/controller/src/Controller.ts @@ -11,7 +11,6 @@ import events from "events"; import fs from "node:fs/promises"; import http from "http"; import https from "https"; -import jwt from "jsonwebtoken"; import path from "path"; import semver from "semver"; import stream from "stream"; @@ -28,6 +27,7 @@ import User from "./User.js"; import UserRecord from "./UserRecord.js"; import UserManager from "./UserManager.js"; import WsServer from "./WsServer.js"; +import HostManager from "./HostManager.js"; import HostRecord from "./HostRecord.js"; import HostPairingManager from "./HostPairingManager.js"; import { BaseControllerPlugin, ControllerHooks } from "./BaseControllerPlugin.js"; @@ -88,6 +88,8 @@ export default class Controller { /** Event subscription controller */ subscriptions: lib.SubscriptionController; + /** Host manager for the cluster, also contains access to the host records */ + hosts: HostManager; /** User manager for the cluster, also contains access to the user records */ users: UserManager; /** Instance manager for the cluster, also contains access to the user records */ @@ -272,7 +274,7 @@ export default class Controller { public recoveryMode: boolean = false, public systems = new lib.SubscribableDatastore(), /** Mapping of host id to host record */ - public hosts = new lib.SubscribableDatastore(), + hosts = new lib.SubscribableDatastore(), /** Mapping of instance id to instance record */ instances = new lib.SubscribableDatastore(), /** Mapping of save id to save details */ @@ -299,6 +301,7 @@ export default class Controller { this.trustedProxies = this.parseTrustedProxies(); this.wsServer = new WsServer(this); + this.hosts = new HostManager(hosts, this); this.users = new UserManager(users, roles, config); this.instances = new InstanceManager(instances, this); this.hostPairing = new HostPairingManager(this); @@ -324,7 +327,7 @@ export default class Controller { // Handle updates for datastores this.systems.on("update", this.systemsUpdated.bind(this)); - this.hosts.on("update", this.hostsUpdated.bind(this)); + this.hosts.records.on("update", this.hostsUpdated.bind(this)); this.hostPairing.records.on("update", this.hostPairingRequestsUpdated.bind(this)); this.instances.records.on("update", this.instanceUpdated.bind(this)); this.saves.on("update", this.savesUpdated.bind(this)); @@ -796,7 +799,7 @@ export default class Controller { await Promise.all([ this.config.save(), this.systems.save(), - this.hosts.save(), + this.hosts.records.save(), this.instances.records.save(), this.saves.save(), this.modPacks.save(), @@ -1049,40 +1052,6 @@ export default class Controller { return systems.length ? new lib.SystemInfoUpdateEvent(systems) : null; } - /** - * Generate access token for host - * - * @returns access token for host - * @param hostId - ID of host to generate a token for. - */ - generateHostToken(hostId: number): string { - return jwt.sign({ aud: "host", host: hostId }, this.authSecret); - } - - /** - * Check a host token - * - * @param token - Host token to check. - * @returns ID of the host the token was issued to. - * @throws if the token is invalid, its host is not registered or it was issued before tokenValidAfter. - */ - verifyHostToken(token: string): number { - // migrate: allow pre-rename tokens issued to hosts before alpha-14 - const payload = jwt.verify(token, this.authSecret, { audience: ["host", "slave"] }); - if (typeof payload === "string") { - throw new Error("unexpected JsonWebToken type"); - } - const hostId = payload.host !== undefined ? payload.host : payload.slave; - const host = this.hosts.get(hostId); - if (!host) { - throw new Error("host is not registered, start the host again to request to join the cluster"); - } - if (!payload.iat || payload.iat < host.tokenValidAfter) { - throw new Error("invalid token"); - } - return hostId; - } - hostsUpdated(hosts: HostRecord[]) { let updates = hosts.map(host => host.toHostDetails()); this.subscriptions.broadcast(new lib.HostUpdatesEvent(updates)); diff --git a/packages/controller/src/HostConnection.ts b/packages/controller/src/HostConnection.ts index 59eedfef..a37c5ea0 100644 --- a/packages/controller/src/HostConnection.ts +++ b/packages/controller/src/HostConnection.ts @@ -44,7 +44,7 @@ export default class HostConnection extends BaseConnection { record.connected = true; record.remoteAddress = remoteAddress; this.info = record; - this._controller.hosts.set(this.info); + this._controller.hosts.records.set(this.info); this._checkPluginVersions(); @@ -175,7 +175,7 @@ export default class HostConnection extends BaseConnection { /** Store changes to the host record, unless the host was revoked */ private _saveInfo() { if (this._controller.hosts.getMutable(this.id) === this.info) { - this._controller.hosts.set(this.info); + this._controller.hosts.records.set(this.info); } } diff --git a/packages/controller/src/HostManager.ts b/packages/controller/src/HostManager.ts new file mode 100644 index 00000000..af46dfa2 --- /dev/null +++ b/packages/controller/src/HostManager.ts @@ -0,0 +1,149 @@ +import crypto from "node:crypto"; +import jwt from "jsonwebtoken"; + +import * as lib from "@clusterio/lib"; +import { logger } from "@clusterio/lib"; + +import type Controller from "./Controller.js"; +import HostRecord from "./HostRecord.js"; + +/** + * Access layer for hosts on the controller. + * + * @alias module:controller/src/HostManager + */ +export default class HostManager { + constructor( + public readonly records: lib.SubscribableDatastore, + private readonly _controller: Controller, + ) { } + + /* ------------------------------------------------------------------------- */ + /* Basic Accessors */ + /* ------------------------------------------------------------------------- */ + + has(id: HostRecord["id"]): boolean { + return this.records.has(id); + } + + getMutable(id: HostRecord["id"]): HostRecord | undefined { + return this.records.getMutable(id); + } + + get(id: HostRecord["id"]): Readonly | undefined { + return this.records.get(id); + } + + valuesMutable(): IterableIterator { + return this.records.values(); + } + + values(): IterableIterator> { + return this.valuesMutable(); + } + + /** + * Get host by ID for a request. + * + * @param hostId - ID of host to get. + * @returns The host record. + * @throws {module:lib.NotFoundError} if the host does not exist. + */ + getForRequest(hostId: number): HostRecord { + const host = this.records.getMutable(hostId); + if (!host) { + throw new lib.NotFoundError(`Unknown host id (${hostId})`); + } + return host; + } + + /* ------------------------------------------------------------------------- */ + /* Lifecycle */ + /* ------------------------------------------------------------------------- */ + + /** + * Create a host record with a new random id + * + * @param name - Name of the host. + * @returns the created host record. + */ + createHost(name: string): HostRecord { + let hostId; + do { + hostId = crypto.randomInt(1, 2 ** 31); + } while (this.records.has(hostId)); + + const host = new HostRecord( + hostId, name, "", new Map(), false, "", "", Math.floor(Date.now() / 1000), + ); + this.records.set(host); + return host; + } + + /** + * Delete a host, disconnect it and unassign its instances + * + * @param hostId - ID of host to revoke. + * @param revokedBy - Name of the user revoking it, for logging. + */ + async revokeHost(hostId: number, revokedBy: string) { + const host = this.getForRequest(hostId); + this.records.delete(host); + this._controller.deleteNote("host", hostId, revokedBy); + const system = this._controller.systems.getMutable(hostId); + if (system) { + this._controller.systems.delete(system); + } + + // Disconnected first so unassigning the instances does not depend on the host responding. + const hostConnection = this._controller.wsServer.hostConnections.get(hostId); + if (hostConnection) { + hostConnection.connector.terminate(); + } + const assigned = [...this._controller.instances.values()].filter( + instance => instance.config.get("instance.assigned_host") === hostId + ); + for (const instance of assigned) { + await this._controller.instances.unassignInstance(instance.id); + } + logger.audit(`User ${revokedBy} revoked host ${host.name} (${host.id})`); + } + + /* ------------------------------------------------------------------------- */ + /* Helpers */ + /* ------------------------------------------------------------------------- */ + + /** + * Sign access token for the given host + * + * @param hostId - ID of host to sign token for. + * @returns JWT access token for the host. + */ + signHostToken(hostId: number): string { + return jwt.sign({ aud: "host", host: hostId }, this._controller.authSecret); + } + + /** + * Check a host token + * + * @param token - Host token to check. + * @returns ID of the host the token was issued to. + * @throws if the token is invalid, its host is not registered or it was issued before tokenValidAfter. + */ + verifyHostToken(token: string): number { + // migrate: allow pre-rename tokens issued to hosts before alpha-14 + const payload = jwt.verify(token, this._controller.authSecret, { audience: ["host", "slave"] }); + if (typeof payload === "string") { + throw new Error("unexpected JsonWebToken type"); + } + const hostId = payload.host !== undefined ? payload.host : payload.slave; + const host = this.records.get(hostId); + if (!host) { + throw new Error("host is not registered, start the host again to request to join the cluster"); + } + if (!payload.iat || payload.iat < host.tokenValidAfter) { + throw new Error("invalid token"); + } + return hostId; + } +} diff --git a/packages/controller/src/HostPairingManager.ts b/packages/controller/src/HostPairingManager.ts index 32daf21f..eceb925e 100644 --- a/packages/controller/src/HostPairingManager.ts +++ b/packages/controller/src/HostPairingManager.ts @@ -6,8 +6,6 @@ import net from "net"; import * as lib from "@clusterio/lib"; import { logger } from "@clusterio/lib"; -import HostRecord from "./HostRecord.js"; - /** Symbols used in pairing codes, avoids 0/O and 1/I/L */ const codeAlphabet = "23456789ABCDEFGHJKMNPQRSTUVWXYZ"; const codeLength = 6; @@ -141,25 +139,6 @@ export default class HostPairingManager { return trimmed; } - /** - * Create an approved host with a new id and token - * - * @param name - Name of the host. - * @returns the id, token and name of the new host. - */ - createHost(name: string): HostPairingResult { - const hosts = this._controller.hosts; - let hostId; - do { - hostId = crypto.randomInt(1, 2 ** 31); - } while (hosts.has(hostId)); - - hosts.set(new HostRecord( - hostId, name, "", new Map(), false, "", "", Math.floor(Date.now() / 1000), - )); - return { hostId, token: this._controller.generateHostToken(hostId), name }; - } - /** * Create a pending request * @@ -265,7 +244,8 @@ export default class HostPairingManager { throw new lib.ValidationError(err.message); } - const result = this.createHost(hostName); + const host = this._controller.hosts.createHost(hostName); + const result = { hostId: host.id, token: this._controller.hosts.signHostToken(host.id), name: host.name }; Object.assign(request, { state: "approved", result }); this.records.delete(request.details); logger.audit( diff --git a/packages/controller/src/UserManager.ts b/packages/controller/src/UserManager.ts index 4e19bc28..c143ef82 100644 --- a/packages/controller/src/UserManager.ts +++ b/packages/controller/src/UserManager.ts @@ -151,11 +151,34 @@ export default class UserManager { * @returns JWT access token for the user. */ signUserToken(user: UserRecord): string { - return jwt.sign( - { aud: "user", user: user.id }, - // auth secret is never null after startup - Buffer.from(this._controllerConfig.get("controller.auth_secret")!, "base64") - ); + return jwt.sign({ aud: "user", user: user.id }, this._authSecret); + } + + /** + * Check a user token + * + * @param token - User token to check. + * @returns the user the token was issued to. + * @throws if the token is invalid, its user does not exist or it was issued before tokenValidAfter. + */ + verifyUserToken(token: string): User { + const payload = jwt.verify(token, this._authSecret, { audience: "user" }); + if (typeof payload === "string") { + throw new Error("unexpected JsonWebToken type"); + } + const user = this.getByNameMutable(payload.user); + if (!user) { + throw new Error("invalid user"); + } + if (!payload.iat || payload.iat < user.tokenValidAfter) { + throw new Error("invalid token"); + } + return user; + } + + private get _authSecret() { + // auth secret is never null after startup + return Buffer.from(this._controllerConfig.get("controller.auth_secret")!, "base64"); } /** Clear all player stats for specific instance */ diff --git a/packages/controller/src/WsServer.ts b/packages/controller/src/WsServer.ts index 2d88b91c..5d2b2518 100644 --- a/packages/controller/src/WsServer.ts +++ b/packages/controller/src/WsServer.ts @@ -252,7 +252,7 @@ ${err.stack}` } try { - if (this.controller.verifyHostToken(data.token) !== data.id) { + if (this.controller.hosts.verifyHostToken(data.token) !== data.id) { throw new Error("missmatched host id"); } } catch (err: any) { @@ -320,18 +320,7 @@ ${err.stack}` async registerControl(data: lib.RegisterControlData, socket: WebSocket, req: IncomingMessage) { let user; try { - const tokenPayload = jwt.verify(data.token, this.controller.authSecret, { audience: "user" }); - if (typeof tokenPayload === "string") { - throw new Error("unexpected JsonWebToken type"); - } - - user = this.controller.users.getByNameMutable(tokenPayload.user); - if (!user) { - throw new Error("invalid user"); - } - if ((tokenPayload.iat || 0) < user.tokenValidAfter) { - throw new Error("invalid token"); - } + user = this.controller.users.verifyUserToken(data.token); user.checkPermission("core.control.connect"); } catch (err: any) { diff --git a/packages/controller/src/routes.ts b/packages/controller/src/routes.ts index 785283be..2d2a36ca 100644 --- a/packages/controller/src/routes.ts +++ b/packages/controller/src/routes.ts @@ -6,7 +6,6 @@ import busboy from "busboy"; import crypto from "crypto"; import events from "events"; import fs from "node:fs/promises"; -import jwt from "jsonwebtoken"; import path from "path"; import nodeStream from "stream"; import util from "util"; @@ -189,7 +188,7 @@ function validateHostToken(req: Request, res: Response, next: any) { } try { - req.app.locals.controller.verifyHostToken(token); + req.app.locals.controller.hosts.verifyHostToken(token); } catch (err) { res.sendStatus(401); return; @@ -206,7 +205,7 @@ function validateUserToken(req: Request, res: Response, next: any) { } try { - res.locals.user = userFromToken(req.app.locals.controller, token); + res.locals.user = req.app.locals.controller.users.verifyUserToken(token); } catch (err) { res.sendStatus(401); return; @@ -215,21 +214,6 @@ function validateUserToken(req: Request, res: Response, next: any) { next(); } -function userFromToken(controller: Controller, token: string) { - const tokenPayload = jwt.verify(token, controller.authSecret, { audience: "user" }); - if (typeof tokenPayload === "string") { - throw new Error("unexpected JsonWebToken type"); - } - let user = controller.users.getByName(tokenPayload.user); - if (!user) { - throw new Error("invalid user"); - } - if (!tokenPayload.iat || tokenPayload.iat < user.tokenValidAfter) { - throw new Error("invalid token"); - } - return user; -} - // Responds with the status of a pairing error, returns false for other errors. function sendHostPairingError(res: Response, err: unknown) { if (!(err instanceof HostPairingError)) { @@ -271,7 +255,7 @@ function createHostPairing(req: Request, res: Response) { if (token !== undefined) { let user; try { - user = userFromToken(controller, token); + user = controller.users.verifyUserToken(token); } catch (err) { res.status(401).json({ error: "Invalid user token" }); return; @@ -285,12 +269,11 @@ function createHostPairing(req: Request, res: Response) { res.status(403).json({ error: `User ${user.name} is not allowed to approve hosts` }); return; } - const result = controller.hostPairing.createHost(name); + const host = controller.hosts.createHost(name); logger.audit( - `User ${user.name} approved host ${result.name} (${result.hostId}) from ${remoteAddress} ` + - "with their token" + `User ${user.name} approved host ${host.name} (${host.id}) from ${remoteAddress} with their token` ); - res.status(200).json(result); + res.status(200).json({ hostId: host.id, token: controller.hosts.signHostToken(host.id), name: host.name }); return; } diff --git a/plugins/player_auth/test/plugin.js b/plugins/player_auth/test/plugin.js index d419bad5..274c9014 100644 --- a/plugins/player_auth/test/plugin.js +++ b/plugins/player_auth/test/plugin.js @@ -174,9 +174,7 @@ describe("player_auth", function() { addInstance(8, "running", true, "invalid host", "1.1.8", 34197, 2); addInstance(9, "running", true, "no version", undefined, 34197); - mockController.hosts.set(1, { - publicAddress: "127.0.0.1", - }); + mockController.hosts.records.set({ id: 1, publicAddress: "127.0.0.1" }); const result = await fetch(`${controllerUrl}/api/player_auth/servers`); assert.deepEqual(await result.json(), [ diff --git a/test/controller/ControlConnection.test.js b/test/controller/ControlConnection.test.js index 845c56fb..c3bac0c7 100644 --- a/test/controller/ControlConnection.test.js +++ b/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}`))); }); }); diff --git a/test/controller/HostConnection.test.js b/test/controller/HostConnection.test.js index d8722af2..123c9d3d 100644 --- a/test/controller/HostConnection.test.js +++ b/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); diff --git a/test/controller/HostManager.test.js b/test/controller/HostManager.test.js new file mode 100644 index 00000000..4a9d807b --- /dev/null +++ b/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)")); + }); + }); +}); diff --git a/test/controller/HostPairingManager.test.js b/test/controller/HostPairingManager.test.js index a1db9b2a..9294ceeb 100644 --- a/test/controller/HostPairingManager.test.js +++ b/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( diff --git a/test/controller/UserManager.test.js b/test/controller/UserManager.test.js index 62ca5209..c9034274 100644 --- a/test/controller/UserManager.test.js +++ b/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; diff --git a/test/controller/WsServer.js b/test/controller/WsServer.js index c9d7bf63..f6fc513f 100644 --- a/test/controller/WsServer.js +++ b/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", diff --git a/test/controller/routes.js b/test/controller/routes.js index 53d5d083..50d81986 100644 --- a/test/controller/routes.js +++ b/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); }); }); diff --git a/test/integration/routing.js b/test/integration/routing.js index 4bfc7834..7e93d449 100644 --- a/test/integration/routing.js +++ b/test/integration/routing.js @@ -20,7 +20,7 @@ function connectHost(controller, hostId, plugins) { const [controllerSide, hostSide] = lib.VirtualConnector.makePair(addr("controller"), addr({ hostId })); const registerData = new lib.RegisterHostData("", "0.0.0", hostId, plugins); // Hosts can only connect once approved, which creates their record. - controller.hosts.set(new HostRecord(hostId, `host-${hostId}`, "", new Map())); + controller.hosts.records.set(new HostRecord(hostId, `host-${hostId}`, "", new Map())); const hostConnection = new HostConnection(registerData, controllerSide, controller, "host-a.test"); controller.wsServer.hostConnections.set(hostId, hostConnection); const hostConfig = new lib.HostConfig("host"); diff --git a/test/mock.js b/test/mock.js index 337e8d7e..57b428a7 100644 --- a/test/mock.js +++ b/test/mock.js @@ -3,7 +3,7 @@ import http from "node:http"; import express from "express"; import * as lib from "@clusterio/lib"; -import { User, UserManager, UserRecord, InstanceManager, ControllerHooks } from "@clusterio/controller"; +import { User, UserManager, UserRecord, HostManager, InstanceManager, ControllerHooks } from "@clusterio/controller"; import { HostHooks, InstanceHooks } from "@clusterio/host"; const addr = lib.Address.fromShorthand; @@ -208,7 +208,7 @@ export class MockController { this.instances = new InstanceManager(new lib.KeyValueDatastore(), this); - this.hosts = new lib.KeyValueDatastore(); + this.hosts = new HostManager(new lib.SubscribableDatastore(), this); this.handles = new Map(); this.hooks = new ControllerHooks(new MockLogger()); this.loadedPlugins = new Set();