Ensure hookshot attempts to join configured permission room

changelog.d/1041.bugfix

@@ -0,0 +1 @@
+Hookshot will now attempt to join permission rooms on startup.

spec/permissions.spec.ts

@@ -0,0 +1,96 @@
+import { E2ESetupTestTimeout, E2ETestEnv } from "./util/e2e-test";
+import { describe, it, afterEach } from "@jest/globals";
+import { expect } from "chai";
+import { MessageEventContent } from "matrix-bot-sdk";
+
+describe('Permissions test', () => {
+    let testEnv: E2ETestEnv<'denied_user'|'allowed_user'>;
+
+    beforeEach(async () => {
+        try {
+            testEnv = await E2ETestEnv.createTestEnv({
+                matrixLocalparts: ['denied_user', 'allowed_user'],
+                permissionsRoom: {
+                    members: ['allowed_user'],
+                    permissions: [{
+                        level: "manageConnections",
+                        service: "webhooks"
+                    }]
+                },
+                e2eClientOpts: {
+                    autoAcceptInvite: true,
+                },
+                config: {
+                    gitlab: {
+                        instances: {"test": {
+                            url: "https://example.org/foo/bar"
+                        }},
+                        webhook: {
+                            secret: "foo!"
+                        }
+                    },
+                    generic: {
+                        enabled: true,
+                        urlPrefix: `http://localhost`
+                    },
+                }
+            });
+            console.log("setup");
+            await testEnv.setUp();
+            console.log("setuped");
+        } catch (ex) {
+            console.log("silent throw", ex);
+            throw ex;
+        }
+
+    }, E2ESetupTestTimeout);
+
+    afterEach(() => {
+        return testEnv?.tearDown();
+    });
+
+    it('should only allow users in the permissions room', async () => {
+        const deniedUser = testEnv.getUser('denied_user');
+        const allowedUser = testEnv.getUser('allowed_user');
+
+        // Invite allowed user to permissions room
+
+        const roomId = await deniedUser.createRoom({ name: 'Test room', invite: [await allowedUser.getUserId()]});
+
+        await deniedUser.inviteUser(testEnv.botMxid, roomId);
+        // User is not in the permissions room
+        const { data } = await deniedUser.waitForRoomLeave({sender: testEnv.botMxid, roomId });
+        // XXX: Missing type
+        expect((data.content as any)["reason"]).to.equal("You do not have permission to invite this bot.");
+
+        await allowedUser.inviteUser(testEnv.botMxid, roomId);
+        await deniedUser.waitForRoomJoin({sender: testEnv.botMxid, roomId });
+    }, E2ESetupTestTimeout);
+
+    it('should only allow users with permission to use a service', async () => {
+        const user = testEnv.getUser('allowed_user');
+        const roomId = await user.createRoom({ name: 'Test room', invite: [testEnv.botMxid]});
+        await user.waitForRoomJoin({sender: testEnv.botMxid, roomId });
+
+        // Try to create a GitHub connection, should fail.
+        const msgGitLab = user.waitForRoomEvent<MessageEventContent>({
+            eventType: 'm.room.message', sender: testEnv.botMxid, roomId
+        });
+        await user.sendText(roomId, "!hookshot gitlab project https://github.com/my/project");
+        expect((await msgGitLab).data.content.body).to.include('Failed to handle command: You are not permitted to provision connections for gitlab.');
+    }, E2ESetupTestTimeout);
+
+
+    it('should allow users with permission to use a service', async () => {
+        const user = testEnv.getUser('allowed_user');
+        const roomId = await user.createRoom({ name: 'Test room', invite: [testEnv.botMxid]});
+        await user.setUserPowerLevel(testEnv.botMxid, roomId, 50);
+        await user.waitForRoomJoin({sender: testEnv.botMxid, roomId });
+
+        const msgWebhooks = user.waitForRoomEvent<MessageEventContent>({
+            eventType: 'm.room.message', sender: testEnv.botMxid, roomId
+        });
+        await user.sendText(roomId, "!hookshot webhook test");
+        expect((await msgWebhooks).data.content.body).to.include('Room configured to bridge webhooks. See admin room for secret url.');
+    }, E2ESetupTestTimeout);
+});

spec/util/e2e-test.ts

@@ -1,21 +1,27 @@
 import { ComplementHomeServer, createHS, destroyHS } from "./homerunner";
-import { IAppserviceRegistration, MatrixClient, MembershipEventContent, PowerLevelsEventContent } from "matrix-bot-sdk";
+import { Appservice, IAppserviceRegistration, MatrixClient, Membership, MembershipEventContent, PowerLevelsEventContent } from "matrix-bot-sdk";
 import { mkdtemp, rm, writeFile } from "node:fs/promises";
 import { BridgeConfig, BridgeConfigRoot } from "../../src/config/Config";
 import { start } from "../../src/App/BridgeApp";
 import { RSAKeyPairOptions, generateKeyPair } from "node:crypto";
 import path from "node:path";
 import Redis from "ioredis";
+import { BridgeConfigActorPermission, BridgeConfigServicePermission } from "../../src/libRs";
 
 const WAIT_EVENT_TIMEOUT = 20000;
 export const E2ESetupTestTimeout = 60000;
 const REDIS_DATABASE_URI = process.env.HOOKSHOT_E2E_REDIS_DB_URI ?? "redis://localhost:6379";
 
-interface Opts {
-    matrixLocalparts?: string[];
+interface Opts<ML extends string> {
+    matrixLocalparts?: ML[];
+    permissionsRoom?: {
+        members: string[],
+        permissions: Array<BridgeConfigServicePermission>,
+    };
     config?: Partial<BridgeConfigRoot>,
     enableE2EE?: boolean,
     useRedis?: boolean,
+    e2eClientOpts?: E2ETestMatrixClientOpts,
 }
 
 interface WaitForEventResponse<T extends object = Record<string, unknown>> {
@@ -25,8 +31,22 @@ interface WaitForEventResponse<T extends object = Record<string, unknown>> {
     }
 }
 
+export interface E2ETestMatrixClientOpts {
+    autoAcceptInvite: boolean;
+}
+
 export class E2ETestMatrixClient extends MatrixClient {
 
+    constructor(private e2eOpts: E2ETestMatrixClientOpts, ...args: ConstructorParameters<typeof MatrixClient>) {
+        super(...args);
+        if (e2eOpts.autoAcceptInvite) {
+            this.on('room.invite', (eventRoomId: string) => {
+                this.joinRoom(eventRoomId);
+            });
+        }
+    }
+
+
     public async waitForPowerLevel(
         roomId: string, expected: Partial<PowerLevelsEventContent>,
     ): Promise<{roomId: string, data: {
@@ -111,10 +131,13 @@ export class E2ETestMatrixClient extends MatrixClient {
         return this.innerWaitForRoomEvent(opts, true);
     }
 
-    public async waitForRoomJoin(
-        opts: {sender: string, roomId?: string}
-    ): Promise<{roomId: string, data: unknown}> {
-        const {sender, roomId} = opts;
+    public async waitForRoomMembership(
+        {sender, roomId, membership}: {membership: Membership, sender: string, roomId?: string}
+    ): Promise<{roomId: string, data: {
+        sender: string,
+        state_key: string,
+        content: MembershipEventContent,
+    }}> {
         return this.waitForEvent('room.event', (eventRoomId: string, eventData: {
             sender: string,
             state_key: string,
@@ -126,13 +149,25 @@ export class E2ETestMatrixClient extends MatrixClient {
             if (roomId && eventRoomId !== roomId) {
                 return;
             }
-            if (eventData.content.membership !== "join") {
+            if (eventData.content.membership !== membership) {
                 return;
             }
             return {roomId: eventRoomId, data: eventData};
         }, `Timed out waiting for join to ${roomId || "any room"} from ${sender}`)
     }
 
+    public async waitForRoomJoin(
+        opts: {sender: string, roomId?: string}
+    ): ReturnType<E2ETestMatrixClient["waitForRoomMembership"]> {
+        return this.waitForRoomMembership({...opts, membership: "join"});
+    }
+
+    public async waitForRoomLeave(
+        opts: {sender: string, roomId?: string}
+    ): ReturnType<E2ETestMatrixClient["waitForRoomMembership"]> {
+        return this.waitForRoomMembership({...opts, membership: "leave"});
+    }
+
     public async waitForRoomInvite(
         opts: {sender: string, roomId?: string}
     ): Promise<{roomId: string, data: unknown}> {
@@ -173,13 +208,14 @@ export class E2ETestMatrixClient extends MatrixClient {
     }
 }
 
-export class E2ETestEnv {
+export class E2ETestEnv<ML extends string = string> {
 
     static get workerId() {
         return parseInt(process.env.JEST_WORKER_ID ?? '0');
     }
 
-    static async createTestEnv(opts: Opts): Promise<E2ETestEnv> {
+    static async createTestEnv<ML extends string>(opts: Opts<ML>): Promise<E2ETestEnv<ML>> {
+        console.log("createTestEnv", new Error().stack)
         const workerID = this.workerId;
         const { matrixLocalparts, config: providedConfig  } = opts;
         const keyPromise = new Promise<string>((resolve, reject) => generateKeyPair("rsa", {
@@ -197,16 +233,19 @@ export class E2ETestEnv {
         }));
 
         const dir = await mkdtemp('hookshot-int-test');
-
+        const clientOpts = opts.e2eClientOpts ?? { 
+            autoAcceptInvite: false,
+        };
         // Configure homeserver and bots
         const [homeserver, privateKey] = await Promise.all([
-            createHS([...matrixLocalparts || []], workerID,  opts.enableE2EE ? path.join(dir, 'client-crypto') : undefined),
+            createHS([...matrixLocalparts || []], clientOpts, workerID,  opts.enableE2EE ? path.join(dir, 'client-crypto') : undefined),
             keyPromise,
         ]);
         const keyPath = path.join(dir, 'key.pem');
         await writeFile(keyPath, privateKey, 'utf-8');
         const webhooksPort = 9500 + workerID;
 
+
         if (providedConfig?.widgets) {
             providedConfig.widgets.openIdOverrides = {
                 'hookshot': homeserver.url,
@@ -226,6 +265,33 @@ export class E2ETestEnv {
             }
         }
 
+        const registration: IAppserviceRegistration = {
+            as_token: homeserver.asToken,
+            hs_token: homeserver.hsToken,
+            sender_localpart: 'hookshot',
+            namespaces: {
+                users: [{
+                    regex: `@hookshot:${homeserver.domain}`,
+                    exclusive: true,
+                }],
+                rooms: [],
+                aliases: [],
+            },
+            "de.sorunome.msc2409.push_ephemeral": true
+        };
+
+        let permissions: BridgeConfigActorPermission[] = [];
+        if (opts.permissionsRoom) {
+            const as = new Appservice({ registration, port: 0, bindAddress: "", homeserverName: homeserver.domain, homeserverUrl: homeserver.url });
+            const permsRoom = await as.botClient.createRoom({name: "Permissions room", invite: opts.permissionsRoom.members.map(localpart => `@${localpart}:${homeserver.domain}`)});
+            permissions.push({ actor: permsRoom, services: opts.permissionsRoom.permissions});
+        } else {
+            permissions = [{
+                actor: "*",
+                services: [{level: "manageConnections"}]
+            }];
+        }
+
         const config = new BridgeConfig({
             bridge: {
                 domain: homeserver.domain,
@@ -253,22 +319,9 @@ export class E2ETestEnv {
                 }
             } : undefined),
             cache: cacheConfig,
+            permissions,
             ...providedConfig,
         });
-        const registration: IAppserviceRegistration = {
-            as_token: homeserver.asToken,
-            hs_token: homeserver.hsToken,
-            sender_localpart: 'hookshot',
-            namespaces: {
-                users: [{
-                    regex: `@hookshot:${homeserver.domain}`,
-                    exclusive: true,
-                }],
-                rooms: [],
-                aliases: [],
-            },
-            "de.sorunome.msc2409.push_ephemeral": true
-        };
         const app = await start(config, registration);
         app.listener.finaliseListeners();
 
@@ -278,7 +331,7 @@ export class E2ETestEnv {
     private constructor(
         public readonly homeserver: ComplementHomeServer,
         public app: Awaited<ReturnType<typeof start>>,
-        public readonly opts: Opts,
+        public readonly opts: Opts<ML>,
         private readonly config: BridgeConfig,
         private readonly dir: string,
     ) { }
@@ -306,7 +359,7 @@ export class E2ETestEnv {
         await rm(this.dir, { recursive: true });
     }
 
-    public getUser(localpart: string) {
+    public getUser(localpart: ML) {
         const u = this.homeserver.users.find(u => u.userId === `@${localpart}:${this.homeserver.domain}`);
         if (!u) {
             throw Error("User missing from test");

spec/util/homerunner.ts

@@ -1,7 +1,7 @@
 import { MatrixClient, MemoryStorageProvider, RustSdkCryptoStorageProvider, RustSdkCryptoStoreType } from "matrix-bot-sdk";
 import { createHash, createHmac, randomUUID } from "crypto";
 import { Homerunner } from "homerunner-client";
-import { E2ETestMatrixClient } from "./e2e-test";
+import { E2ETestMatrixClient, E2ETestMatrixClientOpts } from "./e2e-test";
 import path from "node:path";
 
 const HOMERUNNER_IMAGE = process.env.HOMERUNNER_IMAGE || 'ghcr.io/element-hq/synapse/complement-synapse:nightly';
@@ -42,11 +42,12 @@ async function waitForHomerunner() {
     }
 }
 
-export async function createHS(localparts: string[] = [], workerId: number, cryptoRootPath?: string): Promise<ComplementHomeServer> {
+export async function createHS(localparts: string[] = [], clientOpts: E2ETestMatrixClientOpts, workerId: number, cryptoRootPath?: string): Promise<ComplementHomeServer> {
     await waitForHomerunner();
 
     const appPort = 49600 + workerId;
     const blueprint = `hookshot_integration_test_${Date.now()}`;
+
     const asToken = randomUUID();
     const hsToken = randomUUID();
     const blueprintResponse = await homerunner.create({
@@ -75,7 +76,7 @@ export async function createHS(localparts: string[] = [], workerId: number, cryp
         .filter(([_uId, accessToken]) => accessToken !== asToken)
         .map(async ([userId, accessToken]) => {
             const cryptoStore = cryptoRootPath ? new RustSdkCryptoStorageProvider(path.join(cryptoRootPath, userId), RustSdkCryptoStoreType.Sqlite) : undefined;
-            const client = new E2ETestMatrixClient(homeserver.BaseURL, accessToken, new MemoryStorageProvider(), cryptoStore);
+            const client = new E2ETestMatrixClient(clientOpts, homeserver.BaseURL, accessToken, new MemoryStorageProvider(), cryptoStore);
             if (cryptoStore) {
                 await client.crypto.prepare();
             }

spec/webhooks.spec.ts

@@ -1,5 +1,5 @@
 import { E2ESetupTestTimeout, E2ETestEnv, E2ETestMatrixClient } from "./util/e2e-test";
-import { describe, it, beforeEach, afterEach } from "@jest/globals";
+import { describe, it } from "@jest/globals";
 import { OutboundHookConnection } from "../src/Connections";
 import { TextualMessageEventContent } from "matrix-bot-sdk";
 import { IncomingHttpHeaders, createServer } from "http";

src/Connections/SetupConnection.ts

@@ -472,7 +472,7 @@ export class SetupConnection extends CommandConnection {
 
     private async checkUserPermissions(userId: string, service: string, stateEventType: string): Promise<void> {
         if (!this.config.checkPermission(userId, service, BridgePermissionLevel.manageConnections)) {
-            throw new CommandError(`You are not permitted to provision connections for ${service}.`);
+            throw new CommandError(`${userId} does not have permission to manageConnections for ${service}`, `You are not permitted to provision connections for ${service}.`);
         }
         if (!await this.client.userHasPowerLevelFor(userId, this.roomId, "", true)) {
             throw new CommandError("not-configured", "You must be able to set state in a room ('Change settings') in order to set up new integrations.");

src/config/Config.ts

@@ -184,7 +184,7 @@ export interface BridgeConfigGitLabYAML {
         secret: string;
     },
     instances: {[name: string]: GitLabInstance};
-    userIdPrefix: string;
+    userIdPrefix?: string;
     commentDebounceMs?: number;
 }
 
@@ -322,7 +322,7 @@ export class BridgeWidgetConfig {
         try {
             this.parsedPublicUrl = makePrefixedUrl(yaml.publicUrl)
             this.publicUrl = () => { return this.parsedPublicUrl.href; }
-        } catch (err) {
+        } catch {
             throw new ConfigError("widgets.publicUrl", "is not defined or not a valid URL");
         }
         this.branding = yaml.branding || {
@@ -635,7 +635,10 @@ export class BridgeConfig {
         const permissionRooms = this.bridgePermissions.getInterestedRooms();
         log.info(`Prefilling room membership for permissions for ${permissionRooms.length} rooms`);
         for(const roomEntry of permissionRooms) {
-            const membership = await client.getJoinedRoomMembers(await client.resolveRoom(roomEntry));
+            const roomId = await client.resolveRoom(roomEntry);
+            // Attempt to join the room
+            await client.joinRoom(roomEntry);
+            const membership = await client.getJoinedRoomMembers(roomId);
             membership.forEach(userId => this.bridgePermissions.addMemberToCache(roomEntry, userId));
             log.debug(`Found ${membership.length} users for ${roomEntry}`);
         }

src/config/sections/generichooks.ts

@@ -50,7 +50,7 @@ export class BridgeConfigGenericWebhooks {
         try {
             this.parsedUrlPrefix = makePrefixedUrl(yaml.urlPrefix);
             this.urlPrefix = () => { return this.parsedUrlPrefix.href; }
-        } catch (err) {
+        } catch {
             throw new ConfigError("generic.urlPrefix", "is not defined or not a valid URL");
         }
         this.userIdPrefix = yaml.userIdPrefix;