Sign webhook requests (#4156)

Co-authored-by: Tom Moor <tom.moor@gmail.com>

app/models/WebhookSubscription.ts

@@ -15,6 +15,10 @@ class WebhookSubscription extends BaseModel {
   @observable
   url: string;
 
+  @Field
+  @observable
+  secret: string;
+
   @Field
   @observable
   enabled: boolean;

app/scenes/Settings/components/WebhookSubscriptionForm.tsx

@@ -146,6 +146,7 @@ type Props = {
 interface FormData {
   name: string;
   url: string;
+  secret: string;
   events: string[];
 }
 
@@ -163,6 +164,7 @@ function WebhookSubscriptionForm({ handleSubmit, webhookSubscription }: Props) {
       events: webhookSubscription ? [...webhookSubscription.events] : [],
       name: webhookSubscription?.name,
       url: webhookSubscription?.url,
+      secret: webhookSubscription?.secret,
     },
   });
 
@@ -237,6 +239,7 @@ function WebhookSubscriptionForm({ handleSubmit, webhookSubscription }: Props) {
           autoFocus
           flex
           label={t("Name")}
+          placeholder={t("A memorable identifer")}
           {...register("name", {
             required: true,
           })}
@@ -250,6 +253,14 @@ function WebhookSubscriptionForm({ handleSubmit, webhookSubscription }: Props) {
           label={t("URL")}
           {...register("url", { required: true })}
         />
+        <ReactHookWrappedInput
+          flex
+          label={t("Secret") + ` (${t("Optional")})`}
+          placeholder={t("Used to sign payload")}
+          {...register("secret", {
+            required: false,
+          })}
+        />
       </TextFields>
 
       <EventCheckbox label={t("All events")} value="*" />

server/migrations/20220922073737-webhook-signing-secret.js

@@ -0,0 +1,14 @@
+"use strict";
+
+module.exports = {
+  async up (queryInterface, Sequelize) {
+    return queryInterface.addColumn("webhook_subscriptions", "secret", {
+      type: Sequelize.BLOB,
+      allowNull: true,
+    });
+  },
+
+  async down (queryInterface, Sequelize) {
+    return queryInterface.removeColumn("webhook_subscriptions", "secret");
+  }
+};

server/models/WebhookSubscription.ts

@@ -1,4 +1,6 @@
+import crypto from "crypto";
 import { bool } from "aws-sdk/clients/signer";
+import { isEmpty } from "lodash";
 import {
   Column,
   Table,
@@ -9,6 +11,7 @@ import {
   IsUrl,
   BeforeCreate,
   DefaultScope,
+  AllowNull,
 } from "sequelize-typescript";
 import { SaveOptions } from "sequelize/types";
 import { WebhookSubscriptionValidation } from "@shared/validations";
@@ -17,6 +20,10 @@ import { Event } from "@server/types";
 import Team from "./Team";
 import User from "./User";
 import ParanoidModel from "./base/ParanoidModel";
+import Encrypted, {
+  setEncryptedColumn,
+  getEncryptedColumn,
+} from "./decorators/Encrypted";
 import Fix from "./decorators/Fix";
 import Length from "./validators/Length";
 
@@ -51,6 +58,21 @@ class WebhookSubscription extends ParanoidModel {
   @Column(DataType.ARRAY(DataType.STRING))
   events: string[];
 
+  @AllowNull
+  @Encrypted
+  @Column(DataType.BLOB)
+  get secret() {
+    const val = getEncryptedColumn(this, "secret");
+    // Turns out that `val` evals to `{}` instead
+    // of `null` even if secret's value in db is `null`.
+    // https://github.com/defunctzombie/sequelize-encrypted/blob/c3854e76ae4b80318c8f10f94e6c898c67659ca6/index.js#L30-L33 explains it possibly.
+    return isEmpty(val) ? "" : val;
+  }
+
+  set secret(value: string) {
+    setEncryptedColumn(this, "secret", value);
+  }
+
   // associations
 
   @BelongsTo(() => User, "createdById")
@@ -87,12 +109,19 @@ class WebhookSubscription extends ParanoidModel {
    * Disables the webhook subscription
    *
    * @param options Save options
-   * @returns Promise<void>
+   * @returns Promise<WebhookSubscription>
    */
   public async disable(options?: SaveOptions<WebhookSubscription>) {
     return this.update({ enabled: false }, options);
   }
 
+  /**
+   * Determines if an event should be processed for this webhook subscription
+   * based on the event configuration.
+   *
+   * @param event Event to ceck
+   * @returns true if event is valid
+   */
   public validForEvent = (event: Event): bool => {
     if (this.events.length === 1 && this.events[0] === "*") {
       return true;
@@ -106,6 +135,28 @@ class WebhookSubscription extends ParanoidModel {
 
     return false;
   };
+
+  /**
+   * Calculates the signature for a webhook payload if the webhook subscription
+   * has an associated secret stored.
+   *
+   * @param payload The text payload of a webhook delivery
+   * @returns the signature as a string
+   */
+  public signature = (payload: string) => {
+    if (isEmpty(this.secret)) {
+      return;
+    }
+
+    const signTimestamp = Date.now();
+
+    const signature = crypto
+      .createHmac("sha256", this.secret)
+      .update(`${signTimestamp}.${payload}`)
+      .digest("hex");
+
+    return `t=${signTimestamp},s=${signature}`;
+  };
 }
 
 export default WebhookSubscription;

server/presenters/webhookSubscription.ts

@@ -5,6 +5,7 @@ export default function present(webhook: WebhookSubscription) {
     id: webhook.id,
     name: webhook.name,
     url: webhook.url,
+    secret: webhook.secret,
     events: webhook.events,
     enabled: webhook.enabled,
     createdAt: webhook.createdAt,

server/queues/tasks/DeliverWebhookTask.test.ts

@@ -70,6 +70,33 @@ describe("DeliverWebhookTask", () => {
     expect(delivery.responseBody).toEqual("SUCCESS");
   });
 
+  test("should hit the subscription url with signature header", async () => {
+    const subscription = await buildWebhookSubscription({
+      url: "http://example.com",
+      events: ["*"],
+      secret: "secret",
+    });
+    const signedInUser = await buildUser({ teamId: subscription.teamId });
+    const processor = new DeliverWebhookTask();
+
+    const event: UserEvent = {
+      name: "users.signin",
+      userId: signedInUser.id,
+      teamId: subscription.teamId,
+      actorId: signedInUser.id,
+      ip,
+    };
+    await processor.perform({
+      subscriptionId: subscription.id,
+      event,
+    });
+
+    const headers = fetchMock.mock.calls[0]![1]!.headers!;
+
+    expect(fetchMock).toHaveBeenCalledTimes(1);
+    expect(headers["Outline-Signature"]).toMatch(/^t=[0-9]+,s=[a-z0-9]+$/);
+  });
+
   test("should hit the subscription url when the eventing model doesn't exist", async () => {
     const subscription = await buildWebhookSubscription({
       url: "http://example.com",

server/queues/tasks/DeliverWebhookTask.ts

@@ -209,12 +209,20 @@ export default class DeliverWebhookTask extends BaseTask<Props> {
       paranoid: false,
     });
 
+    let data = null;
+    if (model) {
+      data = {
+        ...presentWebhookSubscription(model),
+        secret: undefined,
+      };
+    }
+
     await this.sendWebhook({
       event,
       subscription,
       payload: {
         id: event.modelId,
-        model: model && presentWebhookSubscription(model),
+        model: data,
       },
     });
   }
@@ -540,6 +548,12 @@ export default class DeliverWebhookTask extends BaseTask<Props> {
           env.VERSION ? `/${env.VERSION.slice(0, 7)}` : ""
         }`,
       };
+
+      const signature = subscription.signature(JSON.stringify(requestBody));
+      if (signature) {
+        requestHeaders["Outline-Signature"] = signature;
+      }
+
       response = await fetch(subscription.url, {
         method: "POST",
         headers: requestHeaders,

server/routes/api/webhookSubscriptions.ts

@@ -1,5 +1,5 @@
 import Router from "koa-router";
-import { compact } from "lodash";
+import { compact, isEmpty } from "lodash";
 import { ValidationError } from "@server/errors";
 import auth from "@server/middlewares/authentication";
 import { WebhookSubscription, Event } from "@server/models";
@@ -41,7 +41,7 @@ router.post(
     const { user } = ctx.state;
     authorize(user, "createWebhookSubscription", user.team);
 
-    const { name, url } = ctx.request.body;
+    const { name, url, secret } = ctx.request.body;
     const events: string[] = compact(ctx.request.body.events);
     assertPresent(name, "name is required");
     assertPresent(url, "url is required");
@@ -57,6 +57,7 @@ router.post(
       teamId: user.teamId,
       url,
       enabled: true,
+      secret: isEmpty(secret) ? undefined : secret,
     });
 
     const event: WebhookSubscriptionEvent = {
@@ -116,7 +117,7 @@ router.post(
     assertUuid(id, "id is required");
     const { user } = ctx.state;
 
-    const { name, url } = ctx.request.body;
+    const { name, url, secret } = ctx.request.body;
     const events: string[] = compact(ctx.request.body.events);
     assertPresent(name, "name is required");
     assertPresent(url, "url is required");
@@ -129,7 +130,13 @@ router.post(
 
     authorize(user, "update", webhookSubscription);
 
-    await webhookSubscription.update({ name, url, events, enabled: true });
+    await webhookSubscription.update({
+      name,
+      url,
+      events,
+      enabled: true,
+      secret: isEmpty(secret) ? undefined : secret,
+    });
 
     const event: WebhookSubscriptionEvent = {
       name: "webhook_subscriptions.update",

shared/i18n/locales/en_US/translation.json

@@ -619,7 +619,11 @@
   "Updating": "Updating",
   "Provide a descriptive name for this webhook and the URL we should send a POST request to when matching events are created.": "Provide a descriptive name for this webhook and the URL we should send a POST request to when matching events are created.",
   "Subscribe to all events, groups, or individual events. We recommend only subscribing to the minimum amount of events that your application needs to function.": "Subscribe to all events, groups, or individual events. We recommend only subscribing to the minimum amount of events that your application needs to function.",
+  "A memorable identifer": "A memorable identifer",
   "URL": "URL",
+  "Secret": "Secret",
+  "Optional": "Optional",
+  "Used to sign payload": "Used to sign payload",
   "All events": "All events",
   "All {{ groupName }} events": "All {{ groupName }} events",
   "Delete webhook": "Delete webhook",