diff --git a/packages/controller/src/Controller.ts b/packages/controller/src/Controller.ts index 75a0e084..68cf411a 100644 --- a/packages/controller/src/Controller.ts +++ b/packages/controller/src/Controller.ts @@ -1086,6 +1086,7 @@ export default class Controller { userPermissionsUpdated(user: Readonly) { for (let controlConnection of this.wsServer.controlConnections.values()) { if (controlConnection.user === user) { + this.subscriptions.unsubscribeUnpermitted(controlConnection, user); controlConnection.send( new lib.AccountUpdateEvent([...user.roles].map(r => ({ name: r.name, @@ -1104,6 +1105,7 @@ export default class Controller { for (const role of roles) { for (let controlConnection of this.wsServer.controlConnections.values()) { if (controlConnection.user.roles.has(role)) { + this.subscriptions.unsubscribeUnpermitted(controlConnection, controlConnection.user); controlConnection.send( new lib.AccountUpdateEvent( [...controlConnection.user.roles].map(r => ({ diff --git a/packages/lib/src/subscriptions.ts b/packages/lib/src/subscriptions.ts index 298bf5d7..0222e1ed 100644 --- a/packages/lib/src/subscriptions.ts +++ b/packages/lib/src/subscriptions.ts @@ -329,6 +329,31 @@ export class SubscriptionController { } } + /** + * Unsubscribe a link from all events the user no longer has permission to subscribe to. + * Used when the permissions of a user changes. + * @param link - Link to check the subscriptions of. + * @param user - User the link is authenticated as. + */ + unsubscribeUnpermitted(link: Link, user: IUser) { + for (const [eventName, eventData] of this._events) { + for (const [addressIndex, subscriber] of eventData.subscriptions) { + if (subscriber.link !== link) { + continue; + } + const request = new SubscriptionRequest(eventName, "subscribe", 0, subscriber.filters); + const message = new MessageRequest( + 0, subscriber.address, new Address(Address.controller, 0), "SubscriptionRequest", request.toJSON(), + ); + try { + SubscriptionRequest.permission(user, message); + } catch { + eventData.subscriptions.delete(addressIndex); + } + } + } + } + /** * Handle incoming subscription requests on a link * @param link - Link message was received on diff --git a/test/controller/Controller.js b/test/controller/Controller.js index efcc69ee..fa5444db 100644 --- a/test/controller/Controller.js +++ b/test/controller/Controller.js @@ -261,6 +261,31 @@ describe("controller/src/Controller", function() { }); }); + describe("permission changes", function() { + let role, connection, checked, originalWsServer; + beforeEach(function() { + role = new Role(1, "Test", "", new Set()); + const user = { roles: new Set([role]) }; + connection = { user, send() {} }; + originalWsServer = controller.wsServer; + controller.wsServer = { controlConnections: new Map([[1, connection]]) }; + checked = []; + controller.subscriptions.unsubscribeUnpermitted = (link, linkUser) => checked.push([link, linkUser]); + }); + afterEach(function() { + controller.wsServer = originalWsServer; + delete controller.subscriptions.unsubscribeUnpermitted; + }); + it("should recheck subscriptions when a user's permissions change", function() { + controller.userPermissionsUpdated(connection.user); + assert.deepEqual(checked, [[connection, connection.user]]); + }); + it("should recheck subscriptions of users with an updated role", function() { + controller.rolesUpdated([role]); + assert.deepEqual(checked, [[connection, connection.user]]); + }); + }); + describe(".instances", function() { it("should set the dirty flag if the config is updated", function() { controller.instances.records.dirty = false; diff --git a/test/lib/subscriptions.test.js b/test/lib/subscriptions.test.js index 780d58b8..ebc6a79c 100644 --- a/test/lib/subscriptions.test.js +++ b/test/lib/subscriptions.test.js @@ -552,6 +552,57 @@ describe("lib/subscriptions", function() { }); }); + describe(".unsubscribeUnpermitted()", function() { + class DenyingUser { + constructor(denied) { + this.denied = denied; + } + + checkPermission(permission) { + if (permission === this.denied) { + throw new lib.PermissionError("Permission denied"); + } + } + } + beforeEach(async function() { + subscriptions.handle(StringPermissionEvent); + subscriptions.handle(FunctionPermissionEvent); + for (const connectorData of connectorSetupDate) { + for (const Event of [StringPermissionEvent, FunctionPermissionEvent]) { + const request = new lib.SubscriptionRequest(Event.name, "subscribe"); + await subscriptions.handleRequest(getLink(connectorData.id), request, connectorData.dst); + } + } + }); + + it("should remove subscriptions with a string permission the user lost", async function() { + subscriptions.unsubscribeUnpermitted(getLink(0), new DenyingUser("StringPermission")); + subscriptions.broadcast(new StringPermissionEvent()); + await onceConnectorSend(1); + assertNoEvent(0); + subscriptions.broadcast(new FunctionPermissionEvent()); + await onceConnectorSend(0); + assertLastEvent(0, FunctionPermissionEvent); + }); + + it("should remove subscriptions with a function permission the user lost", async function() { + subscriptions.unsubscribeUnpermitted(getLink(0), new DenyingUser("FunctionPermission")); + subscriptions.broadcast(new FunctionPermissionEvent()); + await onceConnectorSend(1); + assertNoEvent(0); + subscriptions.broadcast(new StringPermissionEvent()); + await onceConnectorSend(0); + assertLastEvent(0, StringPermissionEvent); + }); + + it("should keep subscriptions the user still has permission for", async function() { + subscriptions.unsubscribeUnpermitted(getLink(0), new DenyingUser(null)); + subscriptions.broadcast(new StringPermissionEvent()); + await onceConnectorSend(0); + assertLastEvent(0, StringPermissionEvent); + }); + }); + describe(".handleRequest()", function() { beforeEach(function() { subscriptions.handle(RegisteredEvent);