From 096f3382774336e4938e514d2179144701ae496e Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Fri, 8 Jul 2022 00:30:35 +0200 Subject: [PATCH] Add live updating of users Provied a subscription for user updates and send updates whenever the details of a user account is changed. --- packages/ctl/ctl.js | 2 + packages/lib/link/messages.js | 53 +++++++++--- packages/lib/users.js | 16 ++++ packages/master/src/ControlConnection.js | 55 ++++++++++++- packages/master/src/Master.js | 10 +++ packages/master/src/SlaveConnection.js | 1 + .../web_ui/src/components/UserViewPage.jsx | 24 +----- packages/web_ui/src/components/UsersPage.jsx | 8 +- packages/web_ui/src/model/user.jsx | 81 +++++++++++++++++++ packages/web_ui/src/util/websocket.jsx | 61 ++++++++++++++ test/integration/clusterio.js | 12 +++ test/integration/index.js | 8 ++ 12 files changed, 292 insertions(+), 39 deletions(-) create mode 100644 packages/web_ui/src/model/user.jsx diff --git a/packages/ctl/ctl.js b/packages/ctl/ctl.js index 41325876..9211543c 100755 --- a/packages/ctl/ctl.js +++ b/packages/ctl/ctl.js @@ -89,6 +89,8 @@ class Control extends libLink.Link { async saveListUpdateEventHandler() { } + async userUpdateEventHandler() { } + async setLogSubscriptions({ all = false, master = false, diff --git a/packages/lib/link/messages.js b/packages/lib/link/messages.js index 01f75da0..1e7050cf 100644 --- a/packages/lib/link/messages.js +++ b/packages/lib/link/messages.js @@ -786,6 +786,28 @@ messages.deleteRole = new Request({ }, }); +const userRequired = ["name", "roles", "instances"]; +const userProperties = { + "name": { type: "string" }, + "roles": { type: "array", items: { type: "integer" }}, + "is_admin": { type: "boolean" }, + "is_banned": { type: "boolean" }, + "is_whitelisted": { type: "boolean" }, + "instances": { type: "array", items: { type: "integer" }}, + "is_deleted": { type: "boolean" }, +}; + +messages.getUser = new Request({ + type: "get_user", + links: ["control-master"], + permission: "core.user.get", + requestProperties: { + "name": { type: "string" }, + }, + responseRequired: userRequired, + responseProperties: userProperties, +}); + messages.listUsers = new Request({ type: "list_users", links: ["control-master"], @@ -795,20 +817,26 @@ messages.listUsers = new Request({ type: "array", items: { additionalProperties: false, - required: ["name", "roles", "instances"], - properties: { - "name": { type: "string" }, - "roles": { type: "array", items: { type: "integer" }}, - "is_admin": { type: "boolean" }, - "is_banned": { type: "boolean" }, - "is_whitelisted": { type: "boolean" }, - "instances": { type: "array", items: { type: "integer" }}, - }, + required: userRequired, + properties: userProperties, }, }, }, }); +messages.setUserSubscriptions = new Request({ + type: "set_user_subscriptions", + links: ["control-master"], + permission: "core.user.subscribe", + requestProperties: { + "all": { type: "boolean" }, + "names": { + type: "array", + items: { type: "string" }, + }, + }, +}); + messages.createUser = new Request({ type: "create_user", links: ["control-master"], @@ -1218,6 +1246,13 @@ messages.saveListUpdate = new Event({ }, }); +messages.userUpdate = new Event({ + type: "user_update", + links: ["master-control"], + eventRequired: userRequired, + eventProperties: userProperties, +}); + messages.masterConnectionEvent = new Event({ type: "master_connection_event", links: ["slave-instance"], diff --git a/packages/lib/users.js b/packages/lib/users.js index 8432191b..8171a499 100644 --- a/packages/lib/users.js +++ b/packages/lib/users.js @@ -195,6 +195,12 @@ class User { } } } + + /** + * True if this user object has been removed from the cluster. + * @type {boolean} + */ + this.isDeleted = false; } serialize() { @@ -487,11 +493,21 @@ definePermission({ description: "Permanently delete roles.", }); +definePermission({ + name: "core.user.get", + title: "Get user", + description: "Get the details of a user in the cluster.", +}); definePermission({ name: "core.user.list", title: "List users", description: "Get the full list of users in the cluster.", }); +definePermission({ + name: "core.user.subscribe", + title: "Subscribe to user updates", + description: "Subscribe to be notified on updates on the details and status of users.", +}); definePermission({ name: "core.user.create", title: "Create users", diff --git a/packages/master/src/ControlConnection.js b/packages/master/src/ControlConnection.js index 05a31320..3b0b4f33 100644 --- a/packages/master/src/ControlConnection.js +++ b/packages/master/src/ControlConnection.js @@ -53,6 +53,11 @@ class ControlConnection extends BaseConnection { instance_ids: [], }; + this.userSubscriptions = { + all: false, + names: [], + }; + this.logTransport = null; this.logSubscriptions = { all: false, @@ -559,6 +564,23 @@ class ControlConnection extends BaseConnection { } } + async getUserRequestHandler(message) { + let name = message.data.name; + let user = this._master.userManager.users.get(name); + if (!user) { + throw new libErrors.RequestError(`User ${name} does not exist`); + } + + return { + name: user.name, + roles: [...user.roles].map(role => role.id), + is_admin: user.isAdmin, + is_banned: user.isBanned, + is_whitelisted: user.isWhitelisted, + instances: [...user.instances], + }; + } + async listUsersRequestHandler(message) { let list = []; for (let user of this._master.userManager.users.values()) { @@ -574,8 +596,30 @@ class ControlConnection extends BaseConnection { return { list }; } + async setUserSubscriptionsRequestHandler(message) { + this.userSubscriptions = message.data; + } + + userUpdated(user) { + if ( + this.userSubscriptions.all + || this.userSubscriptions.names.includes(user.name) + ) { + libLink.messages.userUpdate.send(this, { + name: user.name, + roles: [...user.roles].map(role => role.id), + is_admin: user.isAdmin, + is_banned: user.isBanned, + is_whitelisted: user.isWhitelisted, + instances: [...user.instances], + is_deleted: user.isDeleted, + }); + } + } + async createUserRequestHandler(message) { - this._master.userManager.createUser(message.data.name); + let user = this._master.userManager.createUser(message.data.name); + this._master.userUpdated(user); } async updateUserRolesRequestHandler(message) { @@ -596,6 +640,7 @@ class ControlConnection extends BaseConnection { user.roles = resolvedRoles; this._master.userPermissionsUpdated(user); + this._master.userUpdated(user); } async setUserAdminRequestHandler(message) { @@ -611,6 +656,7 @@ class ControlConnection extends BaseConnection { } user.isAdmin = admin; + this._master.userUpdated(user); this.broadcastEventToSlaves({ data: { name, admin }}, libLink.messages.adminlistUpdate); } @@ -628,6 +674,7 @@ class ControlConnection extends BaseConnection { user.isBanned = banned; user.banReason = reason; + this._master.userUpdated(user); this.broadcastEventToSlaves({ data: { name, banned, reason }}, libLink.messages.banlistUpdate); } @@ -644,6 +691,7 @@ class ControlConnection extends BaseConnection { } user.isWhitelisted = whitelisted; + this._master.userUpdated(user); this.broadcastEventToSlaves({ data: { name, whitelisted }}, libLink.messages.whitelistUpdate); } @@ -653,6 +701,10 @@ class ControlConnection extends BaseConnection { throw new libErrors.RequestError(`User '${message.data.name}' does not exist`); } + user.isDeleted = true; + this._master.userManager.users.delete(message.data.name); + this._master.userUpdated(user); + if (user.is_admin) { this.broadcastEventToSlaves({ data: { name, admin: false }}, libLink.messages.adminlistUpdate); } @@ -662,7 +714,6 @@ class ControlConnection extends BaseConnection { if (user.is_banned) { this.broadcastEventToSlaves({ data: { name, banned: false, reason: "" }}, libLink.messages.banlistUpdate); } - this._master.userManager.users.delete(message.data.name); } async debugDumpWsRequestHandler(message) { diff --git a/packages/master/src/Master.js b/packages/master/src/Master.js index eb292e62..8268021c 100644 --- a/packages/master/src/Master.js +++ b/packages/master/src/Master.js @@ -503,6 +503,16 @@ class Master { } } + userUpdated(user) { + for (let controlConnection of this.wsServer.controlConnections) { + if (controlConnection.connector.closing) { + continue; + } + + controlConnection.userUpdated(user); + } + } + /** * Notify connected control clients under the given user that the * permissions for this user may have changed. diff --git a/packages/master/src/SlaveConnection.js b/packages/master/src/SlaveConnection.js index a8e1e456..c2788adb 100644 --- a/packages/master/src/SlaveConnection.js +++ b/packages/master/src/SlaveConnection.js @@ -186,6 +186,7 @@ class SlaveConnection extends BaseConnection { } else if (type === "leave") { user.notifyLeave(instance_id); } + this._master.userUpdated(user); let instance = this._master.instances.get(instance_id); await libPlugin.invokeHook(this._master.plugins, "onPlayerEvent", instance, message.data); diff --git a/packages/web_ui/src/components/UserViewPage.jsx b/packages/web_ui/src/components/UserViewPage.jsx index f2730fa4..07858a95 100644 --- a/packages/web_ui/src/components/UserViewPage.jsx +++ b/packages/web_ui/src/components/UserViewPage.jsx @@ -10,31 +10,9 @@ import ControlContext from "./ControlContext"; import PageLayout from "./PageLayout"; import PluginExtra from "./PluginExtra"; import { notifyErrorHandler } from "../util/notify"; +import { useUser } from "../model/user"; -function useUser(name) { - let control = useContext(ControlContext); - let [user, setUser] = useState({ loading: true }); - - function updateUser() { - // XXX optimize by requesting only the user in question - libLink.messages.listUsers.send(control).then(result => { - let match = result.list.find(u => u.name === name); - if (!match) { - setUser({ missing: true }); - } else { - setUser({ ...match, present: true }); - } - }); - } - - useEffect(() => { - updateUser(); - }, [name]); - - return [user, updateUser]; -} - export default function UserViewPage() { let params = useParams(); let userName = params.name; diff --git a/packages/web_ui/src/components/UsersPage.jsx b/packages/web_ui/src/components/UsersPage.jsx index 397db3d9..9405b8cd 100644 --- a/packages/web_ui/src/components/UsersPage.jsx +++ b/packages/web_ui/src/components/UsersPage.jsx @@ -9,6 +9,7 @@ import ControlContext from "./ControlContext"; import { notifyErrorHandler } from "../util/notify"; import PageLayout from "./PageLayout"; import PluginExtra from "./PluginExtra"; +import { useUserList } from "../model/user"; const strcmp = new Intl.Collator(undefined, { numerice: "true", sensitivity: "base" }).compare; @@ -57,14 +58,11 @@ export default function UsersPage() { let account = useAccount(); let control = useContext(ControlContext); let history = useHistory(); + let [userList] = useUserList(); - let [users, setUsers] = useState([]); let [roles, setRoles] = useState(new Map()); useEffect(() => { - libLink.messages.listUsers.send(control).then(result => { - setUsers(result["list"]); - }).catch(notifyErrorHandler("Error fetching user list")); libLink.messages.listRoles.send(control).then(result => { setRoles(new Map(result["list"].map(role => [role["id"], role]))); }).catch(() => { @@ -113,7 +111,7 @@ export default function UsersPage() { responsive: ["lg"], }, ]} - dataSource={users} + dataSource={userList} pagination={false} rowKey={user => user["name"]} onRow={(user, rowIndex) => ({ diff --git a/packages/web_ui/src/model/user.jsx b/packages/web_ui/src/model/user.jsx new file mode 100644 index 00000000..6741430a --- /dev/null +++ b/packages/web_ui/src/model/user.jsx @@ -0,0 +1,81 @@ +import { useEffect, useContext, useState } from "react"; +import ControlContext from "../components/ControlContext"; + +import { libLink, libLogging } from "@clusterio/lib"; +const { logger } = libLogging; + + +export function useUser(name) { + let control = useContext(ControlContext); + let [user, setUser] = useState({ loading: true }); + + function updateUser() { + libLink.messages.getUser.send(control, { name }).then(result => { + setUser({ ...result, present: true }); + }).catch(err => { + logger.error(`Failed to get user: ${err}`); + setUser({ missing: true }); + }); + + } + + useEffect(() => { + if (typeof name !== "string") { + setUser({ missing: true }); + return undefined; + } + updateUser(); + + function updateHandler(newUser) { + setUser({ ...newUser, present: true }); + } + + control.onUserUpdate(name, updateHandler); + return () => { + control.offUserUpdate(name, updateHandler); + }; + }, [name]); + + return [user, updateUser]; +} + +export function useUserList() { + let control = useContext(ControlContext); + let [userList, setUserList] = useState([]); + + function updateUserList() { + libLink.messages.listUsers.send(control).then(result => { + setUserList(result.list); + }).catch(err => { + logger.error(`Failed to list users:\n${err}`); + }); + } + + useEffect(() => { + updateUserList(); + + function updateHandler(newUser) { + setUserList(oldList => { + let newList = oldList.concat(); + let index = newList.findIndex(u => u.name === newUser.name); + if (!newUser.is_deleted) { + if (index !== -1) { + newList[index] = newUser; + } else { + newList.push(newUser); + } + } else if (index !== -1) { + newList.splice(index, 1); + } + return newList; + }); + } + + control.onUserUpdate(null, updateHandler); + return () => { + control.offUserUpdate(null, updateHandler); + }; + }, []); + + return [userList]; +} diff --git a/packages/web_ui/src/util/websocket.jsx b/packages/web_ui/src/util/websocket.jsx index 5473ad49..12d8eacd 100644 --- a/packages/web_ui/src/util/websocket.jsx +++ b/packages/web_ui/src/util/websocket.jsx @@ -86,6 +86,7 @@ export class Control extends libLink.Link { this.slaveUpdateHandlers = new Map(); this.instanceUpdateHandlers = new Map(); this.saveListUpdateHandlers = new Map(); + this.userUpdateHandlers = new Map(); this.logHandlers = new Map(); this.connector.on("connect", data => { @@ -101,6 +102,9 @@ export class Control extends libLink.Link { this.updateSaveListSubscriptions().catch(err => logger.error( `Unexpected error updating save list subscriptions:\n${err.stack}` )); + this.updateUserSubscriptions().catch(err => logger.error( + `Unexpected error updating user subscriptions:\n${err.stack}` + )); this.updateLogSubscriptions().catch(err => logger.error( `Unexpected error updating log subscriptions:\n${err.stack}` )); @@ -315,6 +319,63 @@ export class Control extends libLink.Link { }); } + async userUpdateEventHandler(message) { + let handlers = [].concat( + this.userUpdateHandlers.get(null) || [], + this.userUpdateHandlers.get(message.data.name) || [], + ); + for (let handler of handlers) { + handler(message.data); + } + }; + + async onUserUpdate(name, handler) { + if (name !== null && typeof name !== "string") { + throw new Error("Invalid user name"); + } + + let handlers = this.userUpdateHandlers.get(name); + if (!handlers) { + handlers = []; + this.userUpdateHandlers.set(name, handlers); + } + + handlers.push(handler); + + if (handlers.length === 1) { + await this.updateUserSubscriptions(); + } + } + + async offUserUpdate(name, handler) { + let handlers = this.userUpdateHandlers.get(name); + if (!handlers || !handlers.length) { + throw new Error(`No handlers for user ${name} exist`); + } + + let index = handlers.lastIndexOf(handler); + if (index === -1) { + throw new Error(`Given handler is not registered for user ${name}`); + } + + handlers.splice(index, 1); + if (!handlers.length) { + this.userUpdateHandlers.delete(name); + await this.updateUserSubscriptions(); + } + } + + async updateUserSubscriptions() { + if (!this.connector.connected) { + return; + } + + await libLink.messages.setUserSubscriptions.send(this, { + all: this.userUpdateHandlers.has(null), + names: [...this.userUpdateHandlers.keys()].filter(e => e !== null), + }); + } + async logMessageEventHandler(message) { let info = message.data.info; diff --git a/test/integration/clusterio.js b/test/integration/clusterio.js index d4051043..aab1af97 100644 --- a/test/integration/clusterio.js +++ b/test/integration/clusterio.js @@ -483,6 +483,7 @@ describe("Integration of Clusterio", function() { let lists = ["admin", "whitelisted", "banned"]; it("should add and remove the given user to the list", async function() { slowTest(this); + getControl().userUpdates = []; await libLink.messages.createUser.send(getControl(), { name: "list_test" }); let user = (await getUsers()).get("list_test"); for (let list of lists) { @@ -499,6 +500,7 @@ describe("Integration of Clusterio", function() { for (let list of lists) { assert.equal(user[`is_${list}`], false, `unexpected ${list} status`); } + assert.equal(getControl().userUpdates.length, 7); }); it("should not create the user if not instructed to", async function() { slowTest(this); @@ -512,11 +514,13 @@ describe("Integration of Clusterio", function() { }); it("should create the user if instructed to", async function() { slowTest(this); + getControl().userUpdates = []; for (let list of lists) { await execCtl(`user set-${list} --create test_create_${list}`); let user = (await getUsers()).get(`test_create_${list}`); assert.equal(user && user[`is_${list}`], true, `user not created and added to ${list}`); } + assert.equal(getControl().userUpdates.length, 3); }); }); @@ -868,28 +872,36 @@ describe("Integration of Clusterio", function() { describe("user create", function() { it("should create the given user", async function() { + getControl().userUpdates = []; await execCtl("user create temp"); let result = await libLink.messages.listUsers.send(getControl()); let tempUser = result.list.find(user => user.name === "temp"); assert(tempUser, "user was not created"); + assert.equal(getControl().userUpdates.length, 1); + assert.equal(getControl().userUpdates[0].name, "temp"); }); }); describe("user set-roles", function() { it("should set the roles on the user", async function() { + getControl().userUpdates = []; await execCtl('user set-roles temp "Cluster Admin"'); let result = await libLink.messages.listUsers.send(getControl()); let tempUser = result.list.find(user => user.name === "temp"); assert.deepEqual(tempUser.roles, [0]); + assert.equal(getControl().userUpdates.length, 1); }); }); describe("user delete", function() { it("should delete the user", async function() { + getControl().userUpdates = []; await execCtl("user delete temp"); let result = await libLink.messages.listUsers.send(getControl()); let tempUser = result.list.find(user => user.name === "temp"); assert(!tempUser, "user was note deleted"); + assert.equal(getControl().userUpdates.length, 1); + assert.equal(getControl().userUpdates[0].is_deleted, true); }); }); }); diff --git a/test/integration/index.js b/test/integration/index.js index f8c40f46..1084570e 100644 --- a/test/integration/index.js +++ b/test/integration/index.js @@ -28,6 +28,7 @@ class TestControl extends libLink.Link { this.slaveUpdates = []; this.instanceUpdates = []; this.saveListUpdates = []; + this.userUpdates = []; this.connector.on("connect", () => { libLink.messages.setSlaveSubscriptions.send( @@ -39,6 +40,9 @@ class TestControl extends libLink.Link { libLink.messages.setSaveListSubscriptions.send( this, { all: true, instance_ids: [] } ).catch(err => logger.error(`Error setting save list subscriptions:\n${err.stack}`)); + libLink.messages.setUserSubscriptions.send( + this, { all: true, names: [] } + ).catch(err => logger.error(`Error setting user subscriptions:\n${err.stack}`)); }); } @@ -63,6 +67,10 @@ class TestControl extends libLink.Link { this.saveListUpdates.push(message.data); } + async userUpdateEventHandler(message) { + this.userUpdates.push(message.data); + } + async logMessageEventHandler() { } }