diff --git a/packages/controller/src/ControlConnection.ts b/packages/controller/src/ControlConnection.ts index 3a3af9f4..3b1cdd67 100644 --- a/packages/controller/src/ControlConnection.ts +++ b/packages/controller/src/ControlConnection.ts @@ -1074,6 +1074,11 @@ export default class ControlConnection extends BaseConnection { } this._controller.users.deleteUser(user); + for (let controlConnection of this._controller.wsServer.controlConnections.values()) { + if (controlConnection.user.id === user.id) { + controlConnection.connector.terminate(); + } + } if (user.isAdmin) { this._controller.sendTo("allInstances", new lib.InstanceAdminlistUpdateEvent(name, false)); diff --git a/packages/controller/src/UserManager.ts b/packages/controller/src/UserManager.ts index 60649e5f..9ef56d7c 100644 --- a/packages/controller/src/UserManager.ts +++ b/packages/controller/src/UserManager.ts @@ -119,7 +119,9 @@ export default class UserManager { roles.add(defaultRoleId); } - const user = new User(this.records, this._controllerRoles, 0, name, roles); + // Reject tokens issued before the user existed, e.g. for a previous user by the same name + const tokenValidAfter = Math.floor(Date.now() / 1000); + const user = new User(this.records, this._controllerRoles, tokenValidAfter, name, roles); this.records.set(user); return user; } diff --git a/packages/web_ui/src/components/UserViewPage.tsx b/packages/web_ui/src/components/UserViewPage.tsx index fd6b2848..4a4bd97f 100644 --- a/packages/web_ui/src/components/UserViewPage.tsx +++ b/packages/web_ui/src/components/UserViewPage.tsx @@ -146,6 +146,13 @@ export default function UserViewPage() { new lib.UserDeleteRequest(userName) ).then(() => { navigate("/users"); + }, err => { + if (err instanceof lib.SessionLost && userName === account.name) { + // Got kicked out after deleting our own account + navigate("/users"); + return; + } + throw err; }).catch(notifyErrorHandler("Error deleting user")); }} > diff --git a/test/controller/ControlConnection.test.js b/test/controller/ControlConnection.test.js index d82f6cc9..dfaf4087 100644 --- a/test/controller/ControlConnection.test.js +++ b/test/controller/ControlConnection.test.js @@ -2,7 +2,7 @@ import assert from "node:assert/strict"; import events from "node:events"; import * as lib from "@clusterio/lib"; import { Controller, ControlConnection, ControllerHooks } from "@clusterio/controller"; -import { MockConnector, MockLogger } from "../mock.js"; +import { MockConnector, MockController, MockLogger } from "../mock.js"; const addr = lib.Address.fromShorthand; @@ -123,4 +123,55 @@ describe("controller/src/ControlConnection", function() { assert.equal(mockController.debugEvents.listenerCount("message"), 0); }); }); + + describe(".handleUserDeleteRequest()", function() { + let mockController; + let nextId; + + beforeEach(function() { + mockController = new MockController(); + mockController.sendTo = () => {}; + mockController.wsServer.controlConnections = new Map(); + nextId = 1; + }); + + function connect(name) { + const connection = { + _controller: mockController, + user: mockController.users.getByName(name), + connector: { + terminated: false, + terminate() { this.terminated = true; }, + }, + }; + mockController.wsServer.controlConnections.set(nextId, connection); + nextId += 1; + return connection; + } + + async function deleteUser(connection, name) { + await ControlConnection.prototype.handleUserDeleteRequest.call( + connection, new lib.UserDeleteRequest(name) + ); + } + + it("terminates connections of the deleted user", async function() { + const admin = connect("test"); + const player1 = connect("player"); + const player2 = connect("player"); + + await deleteUser(admin, "player"); + assert.equal(mockController.users.getByName("player"), undefined); + assert.equal(player1.connector.terminated, true); + assert.equal(player2.connector.terminated, true); + assert.equal(admin.connector.terminated, false); + }); + + it("terminates its own connection when deleting itself", async function() { + const player = connect("player"); + + await deleteUser(player, "player"); + assert.equal(player.connector.terminated, true); + }); + }); }); diff --git a/test/controller/UserManager.test.js b/test/controller/UserManager.test.js index 5005875e..62ca5209 100644 --- a/test/controller/UserManager.test.js +++ b/test/controller/UserManager.test.js @@ -79,6 +79,23 @@ describe("controller/UserManager", function () { const user = userManager.createUser("Eve"); assert(user.roleIds.has(1)); }); + + it("should set tokenValidAfter to the current time", function () { + const before = Math.floor(Date.now() / 1000); + const user = userManager.createUser("Mallory"); + assert(user.tokenValidAfter >= before, "tokenValidAfter predates the call"); + assert(user.tokenValidAfter <= Math.floor(Date.now() / 1000), "tokenValidAfter is in the future"); + }); + + it("should invalidate tokens issued before a user of the same name was recreated", function () { + const user = userManager.createUser("Niaj"); + const oldTokenIat = user.tokenValidAfter - 60; + userManager.deleteUser(user); + + // WsServer.registerControl rejects the token when iat < tokenValidAfter + const recreated = userManager.createUser("Niaj"); + assert(oldTokenIat < recreated.tokenValidAfter, "token from before the deletion is still valid"); + }); }); describe(".getOrCreateUser()", function () { diff --git a/test/integration/index.js b/test/integration/index.js index c2816412..a1730c22 100644 --- a/test/integration/index.js +++ b/test/integration/index.js @@ -191,7 +191,8 @@ let haveFactorioInstall; const baseHostConfig = loadJSON("config-host.json"); export let url = "https://localhost:4443/"; -export let controlToken = jwt.sign({ aud: "user", user: "test" }, Buffer.from("TestSecretDoNotUse", "base64")); +// Signed in the root before hook, as the user has to exist before the token is issued +export let controlToken; export let instancesDir = path.join("temp", "test", "instances"); export let modsDir = path.join("temp", "test", "mods"); export let databaseDir = path.join("temp", "test", "database"); @@ -373,6 +374,7 @@ before(async function() { console.log("Bootstrapping"); await execController("bootstrap create-admin test"); await execController("bootstrap create-ctl-config test"); + controlToken = jwt.sign({ aud: "user", user: "test" }, Buffer.from("TestSecretDoNotUse", "base64")); controllerProcess = await spawnNode("controller:", "../../packages/controller run", /Started controller/); diff --git a/test/integration/link.js b/test/integration/link.js index d45a28ca..53b3c76b 100644 --- a/test/integration/link.js +++ b/test/integration/link.js @@ -2,10 +2,11 @@ import assert from "node:assert/strict"; import events from "node:events"; import jwt from "jsonwebtoken"; -import { TestControlConnector, TestHostConnector, TestControl, get, exec, url, slowTest } from "./index.js"; +import { + TestControlConnector, TestHostConnector, TestControl, get, exec, url, controlToken, slowTest, +} from "./index.js"; import { ConnectionClosed, ProtocolError, PolicyViolation, AuthenticationFailed } from "@clusterio/lib"; -let token = jwt.sign({ aud: "user", user: "test" }, Buffer.from("TestSecretDoNotUse", "base64")); let tokenHost = jwt.sign({ aud: "host", host: 0 }, Buffer.from("TestSecretDoNotUse", "base64")); describe("Integration of lib/link", function() { @@ -14,7 +15,7 @@ describe("Integration of lib/link", function() { let hostConnector; beforeEach(async function() { controlConnector = new TestControlConnector(url, 0.2); - controlConnector.token = token; + controlConnector.token = controlToken; control = new TestControl(controlConnector, false); hostConnector = new TestHostConnector(url, 0.2); hostConnector.token = tokenHost; @@ -100,7 +101,7 @@ describe("Integration of lib/link", function() { }); it("should refuse connection with incorrect audience", async function() { - hostConnector.token = token; + hostConnector.token = controlToken; await assert.rejects( hostConnector.connect(), new Error("Authentication failed: jwt audience invalid. expected: host or slave")