Add ability to respond to webhooks. (#838)

* Add ability to respond to webhooks.

* Refactor hook handling code for simplicity

* Cleanup checkbox

* Ensure we can send a response even we send no content.

* Add docs

* changelog

* Reflect local and global state in the UI

* Revert CHANGELOG for now

* Emphasize Slack the service in webhooks docs

and not time slack

* Guard against falsey non-string values

---------

Co-authored-by: Andrew Ferrazzutti <andrewf@element.io>

changelog.d/839.feature

@@ -0,0 +1 @@
+Add new `webhookResponse` field to the transformation API to specify your own response data. See the documentation for help.

docs/setup/webhooks.md

@@ -75,6 +75,7 @@ If the body *also* contains a `username` key, then the message will be prepended
 
 If the body does NOT contain a `text` field, the full payload will be sent to the room. This can be adapted into a message by creating a **JavaScript transformation function**.
 
+
 ### Payload formats
 
 If the request is a `POST`/`PUT`, the body of the request will be decoded and stored inside the event. Currently, Hookshot supports:
@@ -101,6 +102,15 @@ to a string representation of that value. This change is <strong>not applied</st
 variable, so it will contain proper float values.
 </section>
 
+### Wait for complete
+
+It is possible to choose whether a webhook response should be instant, or after hookshot has handled the message. The reason
+for this is that some services expect a quick response time (like Slack) whereas others will wait for the request to complete. You
+can specify this either globally in your config, or on the widget with `waitForComplete`.
+
+If you make use of the `webhookResponse` feature, you will need to enable `waitForComplete` as otherwise hookshot will
+immeditately respond with it's default response values.
+
 ## JavaScript Transformations
 
 <section class="notice">
@@ -142,6 +152,11 @@ The `v2` api expects an object to be returned from the `result` variable.
   "plain": "Some text", // The plaintext value to be used for the Matrix message.
   "html": "<b>Some</b> text", // The HTML value to be used for the Matrix message. If not provided, plain will be interpreted as markdown.
   "msgtype": "some.type", // The message type, such as m.notice or m.text, to be used for the Matrix message. If not provided, m.notice will be used.
+  "webhookResponse": { // Optional response to send to the webhook requestor. All fields are optional. Defaults listed.
+    "body": "{ \"ok\": true }",
+    "contentType": "application/json",
+    "statusCode": 200
+  }
 }
 ```
 

src/Bridge.ts

@@ -10,7 +10,7 @@ import { GetIssueResponse, GetIssueOpts } from "./Gitlab/Types"
 import { GithubInstance } from "./github/GithubInstance";
 import { IBridgeStorageProvider } from "./Stores/StorageProvider";
 import { IConnection, GitHubDiscussionSpace, GitHubDiscussionConnection, GitHubUserSpace, JiraProjectConnection, GitLabRepoConnection,
-    GitHubIssueConnection, GitHubProjectConnection, GitHubRepoConnection, GitLabIssueConnection, FigmaFileConnection, FeedConnection, GenericHookConnection } from "./Connections";
+    GitHubIssueConnection, GitHubProjectConnection, GitHubRepoConnection, GitLabIssueConnection, FigmaFileConnection, FeedConnection, GenericHookConnection, WebhookResponse } from "./Connections";
 import { IGitLabWebhookIssueStateEvent, IGitLabWebhookMREvent, IGitLabWebhookNoteEvent, IGitLabWebhookPushEvent, IGitLabWebhookReleaseEvent, IGitLabWebhookTagPushEvent, IGitLabWebhookWikiPageEvent } from "./Gitlab/WebhookTypes";
 import { JiraIssueEvent, JiraIssueUpdatedEvent, JiraVersionEvent } from "./jira/WebhookTypes";
 import { JiraOAuthResult } from "./jira/Types";
@@ -615,19 +615,27 @@ export class Bridge {
                         return;
                     }
                     let successful: boolean|null = null;
-                    if (this.config.generic?.waitForComplete) {
-                        successful = await c.onGenericHook(data.hookData);
-                    }
-                    await this.queue.push<GenericWebhookEventResult>({
-                        data: {successful},
-                        sender: "Bridge",
-                        messageId,
-                        eventName: "response.generic-webhook.event",
-                    });
-                    didPush = true;
-                    if (!this.config.generic?.waitForComplete) {
+                    let response: WebhookResponse|undefined;
+                    if (this.config.generic?.waitForComplete || c.waitForComplete) {
+                        const result = await c.onGenericHook(data.hookData);
+                        successful = result.successful;
+                        response = result.response;
+                        await this.queue.push<GenericWebhookEventResult>({
+                            data: {successful, response},
+                            sender: "Bridge",
+                            messageId,
+                            eventName: "response.generic-webhook.event",
+                        });
+                    } else {
+                        await this.queue.push<GenericWebhookEventResult>({
+                            data: {},
+                            sender: "Bridge",
+                            messageId,
+                            eventName: "response.generic-webhook.event",
+                        });
                         await c.onGenericHook(data.hookData);
                     }
+                    didPush = true;
                 }
                 catch (ex) {
                     log.warn(`Failed to handle generic webhook`, ex);

src/Connections/GenericHook.ts

@@ -2,7 +2,7 @@ import { Connection, IConnection, IConnectionState, InstantiateConnectionOpts, P
 import { Logger } from "matrix-appservice-bridge";
 import { MessageSenderClient } from "../MatrixSender"
 import markdownit from "markdown-it";
-import { QuickJSRuntime, QuickJSWASMModule, newQuickJSWASMModule, shouldInterruptAfterDeadline } from "quickjs-emscripten";
+import { QuickJSWASMModule, newQuickJSWASMModule, shouldInterruptAfterDeadline } from "quickjs-emscripten";
 import { MatrixEvent } from "../MatrixEvent";
 import { Appservice, Intent, StateEvent } from "matrix-bot-sdk";
 import { ApiError, ErrCode } from "../api";
@@ -22,6 +22,10 @@ export interface GenericHookConnectionState extends IConnectionState {
      */
     name: string;
     transformationFunction?: string;
+    /**
+     * Should the webhook only respond on completion.
+     */
+    waitForComplete?: boolean;
 }
 
 export interface GenericHookSecrets {
@@ -45,12 +49,19 @@ export interface GenericHookAccountData {
     [hookId: string]: string;
 }
 
+export interface WebhookResponse {
+    body: string;
+    contentType?: string;
+    statusCode?: number;
+}
+
 interface WebhookTransformationResult {
     version: string;
     plain?: string;
     html?: string;
     msgtype?: string;
     empty?: boolean;
+    webhookResponse?: WebhookResponse;
 }
 
 const log = new Logger("GenericHookConnection");
@@ -113,13 +124,16 @@ export class GenericHookConnection extends BaseConnection implements IConnection
     }
 
     static validateState(state: Record<string, unknown>): GenericHookConnectionState {
-        const {name, transformationFunction} = state;
+        const {name, transformationFunction, waitForComplete} = state;
         if (!name) {
             throw new ApiError('Missing name', ErrCode.BadValue);
         }
         if (typeof name !== "string" || name.length < 3 || name.length > 64) {
             throw new ApiError("'name' must be a string between 3-64 characters long", ErrCode.BadValue);
         }
+        if (waitForComplete !== undefined && typeof waitForComplete !== "boolean") {
+            throw new ApiError("'waitForComplete' must be a boolean", ErrCode.BadValue);
+        }
         // Use !=, not !==, to check for both undefined and null
         if (transformationFunction != undefined) {
             if (!this.quickModule) {
@@ -132,6 +146,7 @@ export class GenericHookConnection extends BaseConnection implements IConnection
         return {
             name,
             ...(transformationFunction && {transformationFunction}),
+            waitForComplete,
         };
     }
 
@@ -222,6 +237,14 @@ export class GenericHookConnection extends BaseConnection implements IConnection
         }
     }
 
+    /**
+     * Should the webhook handler wait for this to finish before
+     * sending a response back.
+     */
+    public get waitForComplete(): boolean {
+        return this.state.waitForComplete ?? false;
+    }
+
     public get priority(): number {
         return this.state.priority || super.priority;
     }
@@ -323,7 +346,7 @@ export class GenericHookConnection extends BaseConnection implements IConnection
         return msg;
     }
 
-    public executeTransformationFunction(data: unknown): {plain: string, html?: string, msgtype?: string}|null {
+    public executeTransformationFunction(data: unknown): {content?: {plain: string, html?: string, msgtype?: string}, webhookResponse?: WebhookResponse} {
         if (!this.transformationFunction) {
             throw Error('Transformation function not defined');
         }
@@ -351,34 +374,48 @@ export class GenericHookConnection extends BaseConnection implements IConnection
 
         // Legacy v1 api
         if (typeof result === "string") {
-            return {plain: `Received webhook: ${result}`};
+            return {content: {plain: `Received webhook: ${result}`}};
         } else if (typeof result !== "object") {
-            return {plain: `No content`};
+            return {content: {plain: `No content`}};
         }
         const transformationResult = result as WebhookTransformationResult;
         if (transformationResult.version !== "v2") {
             throw Error("Result returned from transformation didn't specify version = v2");
         }
 
-        if (transformationResult.empty) {
-            return null; // No-op
+        let content;
+        if (!transformationResult.empty) {
+            if (typeof transformationResult.plain !== "string") {
+                throw Error("Result returned from transformation didn't provide a string value for plain");
+            }
+            if (transformationResult.html !== undefined && typeof transformationResult.html !== "string") {
+                throw Error("Result returned from transformation didn't provide a string value for html");
+            }
+            if (transformationResult.msgtype !== undefined && typeof transformationResult.msgtype !== "string") {
+                throw Error("Result returned from transformation didn't provide a string value for msgtype");
+            }
+            content = {
+                plain: transformationResult.plain,
+                html: transformationResult.html,
+                msgtype: transformationResult.msgtype,
+            };
         }
 
-        const plain = transformationResult.plain;
-        if (typeof plain !== "string") {
-            throw Error("Result returned from transformation didn't provide a string value for plain");
-        }
-        if (transformationResult.html && typeof transformationResult.html !== "string") {
-            throw Error("Result returned from transformation didn't provide a string value for html");
-        }
-        if (transformationResult.msgtype && typeof transformationResult.msgtype !== "string") {
-            throw Error("Result returned from transformation didn't provide a string value for msgtype");
+        if (transformationResult.webhookResponse) {
+            if (typeof transformationResult.webhookResponse.body !== "string") {
+                throw Error("Result returned from transformation didn't provide a string value for webhookResponse.body");
+            }
+            if (transformationResult.webhookResponse.statusCode !== undefined && typeof transformationResult.webhookResponse.statusCode !== "number" && Number.isInteger(transformationResult.webhookResponse.statusCode)) {
+                throw Error("Result returned from transformation didn't provide a number value for webhookResponse.statusCode");
+            }
+            if (transformationResult.webhookResponse.contentType !== undefined && typeof transformationResult.webhookResponse.contentType !== "string") {
+                throw Error("Result returned from transformation didn't provide a contentType value for msgtype");
+            }
         }
 
         return {
-            plain: plain,
-            html: transformationResult.html,
-            msgtype: transformationResult.msgtype,
+            content,
+            webhookResponse: transformationResult.webhookResponse,
         }
     }
 
@@ -387,45 +424,49 @@ export class GenericHookConnection extends BaseConnection implements IConnection
      * @param data Structured data. This may either be a string, or an object.
      * @returns `true` if the webhook completed, or `false` if it failed to complete
      */
-    public async onGenericHook(data: unknown): Promise<boolean> {
+    public async onGenericHook(data: unknown): Promise<{successful: boolean, response?: WebhookResponse}> {
         log.info(`onGenericHook ${this.roomId} ${this.hookId}`);
-        let content: {plain: string, html?: string, msgtype?: string};
-        let success = true;
+        let content: {plain: string, html?: string, msgtype?: string}|undefined;
+        let webhookResponse: WebhookResponse|undefined;
+        let successful = true;
         if (!this.transformationFunction) {
             content = this.transformHookData(data);
         } else {
             try {
-                const potentialContent = this.executeTransformationFunction(data);
-                if (potentialContent === null) {
-                    // Explitly no action
-                    return true;
-                }
-                content = potentialContent;
+                const result = this.executeTransformationFunction(data);
+                content = result.content;
+                webhookResponse = result.webhookResponse;
             } catch (ex) {
                 log.warn(`Failed to run transformation function`, ex);
                 content = {plain: `Webhook received but failed to process via transformation function`};
-                success = false;
+                successful = false;
             }
         }
 
-        const sender = this.getUserId();
-        const senderIntent = this.as.getIntentForUserId(sender);
-        await this.ensureDisplayname(senderIntent);
+        if (content) {
+            const sender = this.getUserId();
+            const senderIntent = this.as.getIntentForUserId(sender);
+            await this.ensureDisplayname(senderIntent);
+    
+            await ensureUserIsInRoom(senderIntent, this.intent.underlyingClient, this.roomId);
+    
+            // Matrix cannot handle float data, so make sure we parse out any floats.
+            const safeData = GenericHookConnection.sanitiseObjectForMatrixJSON(data);
+    
+            await this.messageClient.sendMatrixMessage(this.roomId, {
+                msgtype: content.msgtype || "m.notice",
+                body: content.plain,
+                // render can output redundant trailing newlines, so trim it.
+                formatted_body: content.html || md.render(content.plain).trim(),
+                format: "org.matrix.custom.html",
+                "uk.half-shot.hookshot.webhook_data": safeData,
+            }, 'm.room.message', sender);
+        }
 
-        await ensureUserIsInRoom(senderIntent, this.intent.underlyingClient, this.roomId);
-
-        // Matrix cannot handle float data, so make sure we parse out any floats.
-        const safeData = GenericHookConnection.sanitiseObjectForMatrixJSON(data);
-
-        await this.messageClient.sendMatrixMessage(this.roomId, {
-            msgtype: content.msgtype || "m.notice",
-            body: content.plain,
-            // render can output redundant trailing newlines, so trim it.
-            formatted_body: content.html || md.render(content.plain).trim(),
-            format: "org.matrix.custom.html",
-            "uk.half-shot.hookshot.webhook_data": safeData,
-        }, 'm.room.message', sender);
-        return success;
+        return {
+            successful,
+            response: webhookResponse,
+        };
 
     }
 
@@ -445,6 +486,7 @@ export class GenericHookConnection extends BaseConnection implements IConnection
             id: this.connectionId,
             config: {
                 transformationFunction: this.state.transformationFunction,
+                waitForComplete: this.waitForComplete,
                 name: this.state.name,
             },
             ...(showSecrets ? { secrets: {

src/config/Config.ts

@@ -325,6 +325,7 @@ export class BridgeConfigGenericWebhooks {
         return {
             userIdPrefix: this.userIdPrefix,
             allowJsTransformationFunctions: this.allowJsTransformationFunctions,
+            waitForComplete: this.waitForComplete,
         }
     }
 

src/generic/Router.ts

@@ -11,7 +11,7 @@ const log = new Logger('GenericWebhooksRouter');
 export class GenericWebhooksRouter {
     constructor(private readonly queue: MessageQueue, private readonly deprecatedPath = false, private readonly allowGet: boolean) { }
 
-    private onWebhook(req: Request<{hookId: string}, unknown, unknown, unknown>, res: Response<{ok: true}|{ok: false, error: string}>, next: NextFunction) {
+    private onWebhook(req: Request<{hookId: string}, unknown, unknown, unknown>, res: Response<unknown|{ok: false, error: string}>, next: NextFunction) {
         if (req.method === "GET" && !this.allowGet) {
             throw new ApiError("Invalid Method. Expecting PUT or POST", ErrCode.MethodNotAllowed);
         }
@@ -43,7 +43,11 @@ export class GenericWebhooksRouter {
                 }
                 res.status(404).send({ok: false, error: "Webhook not found"});
             } else if (response.successful) {
-                res.status(200).send({ok: true});
+                const body = response.response?.body ?? {ok: true};
+                if (response.response?.contentType) {
+                    res.contentType(response.response.contentType);
+                }
+                res.status(response.response?.statusCode ?? 200).send(body);
             } else if (response.successful === false) {
                 res.status(500).send({ok: false, error: "Failed to process webhook"});
             } else {

src/generic/types.ts

@@ -1,3 +1,5 @@
+import { WebhookResponse } from "../Connections";
+
 export interface GenericWebhookEvent {
     hookData: unknown;
     hookId: string;
@@ -5,5 +7,6 @@ export interface GenericWebhookEvent {
 
 export interface GenericWebhookEventResult {
     successful?: boolean|null;
+    response?: WebhookResponse,
     notFound?: boolean;
 }

web/components/roomConfig/GenericWebhookConfig.tsx

@@ -31,6 +31,8 @@ const CODE_MIRROR_EXTENSIONS = [javascript({})];
 const ConnectionConfiguration: FunctionComponent<ConnectionConfigurationProps<ServiceConfig, GenericHookResponseItem, GenericHookConnectionState>> = ({serviceConfig, existingConnection, onSave, onRemove, isUpdating}) => {
     const [transFn, setTransFn] = useState<string>(existingConnection?.config.transformationFunction as string || EXAMPLE_SCRIPT);
     const [transFnEnabled, setTransFnEnabled] = useState(serviceConfig.allowJsTransformationFunctions && !!existingConnection?.config.transformationFunction);
+    const [waitForComplete, setWaitForComplete] = useState(existingConnection?.config.waitForComplete ?? false);
+
     const nameRef = createRef<HTMLInputElement>();
 
     const canEdit = !existingConnection || (existingConnection?.canEdit ?? false);
@@ -41,9 +43,10 @@ const ConnectionConfiguration: FunctionComponent<ConnectionConfigurationProps<Se
         }
         onSave({
             name: nameRef?.current?.value || existingConnection?.config.name || "Generic Webhook",
+            waitForComplete,
             ...(transFnEnabled ? { transformationFunction: transFn } : undefined),
         });
-    }, [canEdit, onSave, nameRef, transFn, existingConnection, transFnEnabled]);
+    }, [canEdit, onSave, nameRef, transFn, existingConnection, transFnEnabled, waitForComplete]);
 
     return <form onSubmit={handleSave}>
         <InputField visible={!existingConnection} label="Friendly name" noPadding={true}>
@@ -58,6 +61,11 @@ const ConnectionConfiguration: FunctionComponent<ConnectionConfigurationProps<Se
             <input disabled={!canEdit} type="checkbox" checked={transFnEnabled} onChange={useCallback(() => setTransFnEnabled(v => !v), [])} />
         </InputField>
 
+
+        <InputField visible={serviceConfig.allowJsTransformationFunctions && transFnEnabled} label="Respond after function completes" noPadding={true}>
+            <input disabled={!canEdit || serviceConfig.waitForComplete} type="checkbox" checked={waitForComplete || serviceConfig.waitForComplete} onChange={useCallback(() => setWaitForComplete(v => !v), [])} />
+        </InputField>
+
         <InputField visible={transFnEnabled} noPadding={true}>
             <CodeMirror
                 value={transFn}
@@ -74,7 +82,8 @@ const ConnectionConfiguration: FunctionComponent<ConnectionConfigurationProps<Se
 };
 
 interface ServiceConfig {
-    allowJsTransformationFunctions: boolean
+    allowJsTransformationFunctions: boolean,
+    waitForComplete: boolean,
 }
 
 const RoomConfigText = {