Fix widgets not working when bound to the same listener as webhooks. (#872)

* Bind listener in bridge.

* Add a test to confirm behaviour

* Fix test name

* changelog

changelog.d/872.bugfix

@@ -0,0 +1 @@
+Fix widgets not loading when bound to the same listener as "webhooks".

spec/util/e2e-test.ts

@@ -151,8 +151,13 @@ export class E2ETestMatrixClient extends MatrixClient {
 }
 
 export class E2ETestEnv {
+
+    static get workerId() {
+        return parseInt(process.env.JEST_WORKER_ID ?? '0');
+    }
+
     static async createTestEnv(opts: Opts): Promise<E2ETestEnv> {
-        const workerID = parseInt(process.env.JEST_WORKER_ID ?? '0');
+        const workerID = this.workerId;
         const { matrixLocalparts, config: providedConfig  } = opts;
         const keyPromise = new Promise<string>((resolve, reject) => generateKeyPair("rsa", {
             modulusLength: 4096,
@@ -178,6 +183,11 @@ export class E2ETestEnv {
         await writeFile(keyPath, privateKey, 'utf-8');
         const webhooksPort = 9500 + workerID;
 
+        if (providedConfig?.widgets) {
+            providedConfig.widgets.openIdOverrides = {
+                'hookshot': homeserver.url,
+            }
+        }
         const config = new BridgeConfig({
             bridge: {
                 domain: homeserver.domain,

spec/widgets.spec.ts

@@ -0,0 +1,45 @@
+import { E2ESetupTestTimeout, E2ETestEnv } from "./util/e2e-test";
+import { describe, it, beforeEach, afterEach } from "@jest/globals";
+import { BridgeAPI } from "../web/BridgeAPI";
+import { WidgetApi } from "matrix-widget-api";
+
+describe('Widgets', () => {
+    let testEnv: E2ETestEnv;
+
+    beforeEach(async () => {
+        const webhooksPort = 9500 + E2ETestEnv.workerId;
+        testEnv = await E2ETestEnv.createTestEnv({matrixLocalparts: ['user'], config: {
+            widgets: {
+                publicUrl: `http://localhost:${webhooksPort}`
+            },
+            listeners: [{
+                port: webhooksPort,
+                bindAddress: '0.0.0.0',
+                // Bind to the SAME listener to ensure we don't have conflicts.
+                resources: ['webhooks', 'widgets'],
+            }],
+            
+        }});
+        await testEnv.setUp();
+    }, E2ESetupTestTimeout);
+
+    afterEach(() => {
+        return testEnv?.tearDown();
+    });
+
+    it('should be able to authenticate with the widget API', async () => {
+        const user = testEnv.getUser('user');
+        const bridgeApi = await BridgeAPI.getBridgeAPI(testEnv.opts.config?.widgets?.publicUrl!, {
+            requestOpenIDConnectToken: () => {
+                return user.getOpenIDConnectToken()
+            },
+        } as unknown as WidgetApi, {
+            getItem() { return null},
+            setItem() { },
+        } as unknown as Storage);
+        expect(await bridgeApi.verify()).toEqual({
+            "type": "widget",
+            "userId": "@user:hookshot",
+        });
+    });
+});

src/App/BridgeApp.ts

@@ -60,13 +60,6 @@ export async function start(config: BridgeConfig, registration: IAppserviceRegis
         // Don't care to await this, as the process is about to end
         storage.disconnect?.();
     });
-
-    // XXX: Since the webhook listener listens on /, it must listen AFTER other resources
-    // have bound themselves.
-    if (config.queue.monolithic) {
-        const webhookHandler = new Webhooks(config);
-        listener.bindResource('webhooks', webhookHandler.expressRouter);
-    }
     return {
         bridgeApp,
         storage,

src/Bridge.ts

@@ -19,7 +19,7 @@ import { MessageQueue, MessageQueueMessageOut, createMessageQueue } from "./Mess
 import { MessageSenderClient } from "./MatrixSender";
 import { NotifFilter, NotificationFilterStateContent } from "./NotificationFilters";
 import { NotificationProcessor } from "./NotificationsProcessor";
-import { NotificationsEnableEvent, NotificationsDisableEvent } from "./Webhooks";
+import { NotificationsEnableEvent, NotificationsDisableEvent, Webhooks } from "./Webhooks";
 import { GitHubOAuthToken, GitHubOAuthTokenResponse, ProjectsGetResponseData } from "./github/Types";
 import { retry } from "./PromiseUtil";
 import { UserNotificationsEvent } from "./Notifications/UserNotificationWatcher";
@@ -788,6 +788,9 @@ export class Bridge {
             );
         }
 
+        const webhookHandler = new Webhooks(this.config);
+        this.listener.bindResource('webhooks', webhookHandler.expressRouter);
+
         await this.as.begin();
         log.info(`Bridge is now ready. Found ${this.connectionManager.size} connections`);
         this.ready = true;

src/config/Config.ts

@@ -334,6 +334,9 @@ export class BridgeConfigGenericWebhooks {
 
 interface BridgeWidgetConfigYAML {
     publicUrl: string;
+    /**
+     * @deprecated Prefer using listener config.
+     */
     port?: number;
     addToAdminRooms?: boolean;
     roomSetupWidget?: {

web/BridgeAPI.ts

@@ -24,9 +24,9 @@ interface RequestOpts {
 
 export class BridgeAPI {
     
-    static async getBridgeAPI(baseUrl: string, widgetApi: WidgetApi): Promise<BridgeAPI> {
+    static async getBridgeAPI(baseUrl: string, widgetApi: WidgetApi, storage = localStorage): Promise<BridgeAPI> {
         try {
-            const sessionToken = localStorage.getItem('hookshot-sessionToken');
+            const sessionToken = storage.getItem('hookshot-sessionToken');
             baseUrl = baseUrl.endsWith("/") ? baseUrl.slice(0, -1) : baseUrl;
             if (sessionToken) {
                 const client = new BridgeAPI(baseUrl, sessionToken);
@@ -36,7 +36,7 @@ export class BridgeAPI {
                 } catch (ex) {
                     // TODO: Check that the token is actually invalid, rather than just assuming we need to refresh.
                     console.warn(`Failed to verify token, fetching new token`, ex);
-                    localStorage.removeItem(sessionToken);
+                    storage.removeItem(sessionToken);
                 }
             }
         } catch (ex) {
@@ -74,7 +74,7 @@ export class BridgeAPI {
         }
         const response = await res.json() as ExchangeOpenAPIResponseBody;
         try {
-            localStorage.setItem('hookshot-sessionToken', response.token);
+            storage.setItem('hookshot-sessionToken', response.token);
         } catch (ex) {
             // E.g. Browser prevents storage access.
             console.debug(`Failed to store session token, continuing`, ex);