diff --git a/docs/devs/protocol.md b/docs/devs/protocol.md index ab028efa..747137d0 100644 --- a/docs/devs/protocol.md +++ b/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` diff --git a/packages/controller/src/Controller.ts b/packages/controller/src/Controller.ts index b2341cc2..3bf2bdf4 100644 --- a/packages/controller/src/Controller.ts +++ b/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 diff --git a/packages/controller/src/HostPairingManager.ts b/packages/controller/src/HostPairingManager.ts index 55293dbf..8dc3ad99 100644 --- a/packages/controller/src/HostPairingManager.ts +++ b/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(); private _bySecret = new Map(); private _byCode = new Map(); - /** Creation times of recent requests by address key */ - private _recentCreates = new Map(); private _pruneInterval?: ReturnType; 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); - } - } } } diff --git a/packages/controller/src/routes.ts b/packages/controller/src/routes.ts index fa967fe2..fa1d7b24 100644 --- a/packages/controller/src/routes.ts +++ b/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; diff --git a/packages/lib/src/data/HostPairingDetails.ts b/packages/lib/src/data/HostPairingDetails.ts index 69524efd..7ef079fc 100644 --- a/packages/lib/src/data/HostPairingDetails.ts +++ b/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, diff --git a/packages/lib/src/permissions.ts b/packages/lib/src/permissions.ts index aeadc779..9a5e6acb 100644 --- a/packages/lib/src/permissions.ts +++ b/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", diff --git a/packages/web_ui/src/components/HostsPage.tsx b/packages/web_ui/src/components/HostsPage.tsx index 9be610d8..654ae644 100644 --- a/packages/web_ui/src/components/HostsPage.tsx +++ b/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>(new Map()); - const [busyCodes, setBusyCodes] = useState>(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) { - return (names.get(request.code) ?? request.name).trim(); - } - - function approve(request: Readonly) { - 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) { - 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 void }) { Later starts connect to the cluster straight away. Waiting for approval - - Only approve a request if its code matches the code shown by the host. - An approved host has full access to the cluster. - void }) { columns={[ { title: "Name", - key: "name", - render: (_, request) => { 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 - - - ; - }, + render: (_, request) => , }, ]} /> diff --git a/test/controller/HostPairingManager.test.js b/test/controller/HostPairingManager.test.js index ce6c8084..c47f20fe 100644 --- a/test/controller/HostPairingManager.test.js +++ b/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() {