From 4fdef9ae4a829f96011b9facbc21cb4327e39230 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tadeusz=20=E2=80=9Etadzik=E2=80=9D=20So=C5=9Bnierz?= Date: Thu, 28 Mar 2024 14:25:34 +0100 Subject: [PATCH 1/4] Verify Slack webhook tokens MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tadeusz „tadzik” Sośnierz --- package.json | 1 + src/AdminCommands.ts | 8 ++- src/BridgedRoom.ts | 13 ++++ src/Main.ts | 6 ++ src/SlackHookHandler.ts | 14 +++- src/datastore/Models.ts | 1 + tests/integration/WebhookTest.ts | 117 +++++++++++++++++++++++++++++++ yarn.lock | 66 ++++++++++++++--- 8 files changed, 215 insertions(+), 11 deletions(-) create mode 100644 tests/integration/WebhookTest.ts diff --git a/package.json b/package.json index d4a27436..66c4bf54 100644 --- a/package.json +++ b/package.json @@ -85,6 +85,7 @@ "eslint-plugin-react-hooks": "^4.6.0", "js-yaml": "^4.1.0", "mocha": "^10.0.0", + "node-mocks-http": "^1.14.1", "postcss": "^8.4.21", "prom-client": "^14.0.1", "source-map-support": "^0.5.19", diff --git a/src/AdminCommands.ts b/src/AdminCommands.ts index c4b987d0..73f66bc4 100644 --- a/src/AdminCommands.ts +++ b/src/AdminCommands.ts @@ -181,11 +181,12 @@ export class AdminCommands { return new AdminCommand( "link", "connect a Matrix and a Slack room together", - async ({respond, room, channel_id, webhook_url, slack_bot_token, team_id}: { + async ({respond, room, channel_id, webhook_url, webhook_token, slack_bot_token, team_id}: { respond: ResponseCallback, room?: string, channel_id?: string, webhook_url?: string, + webhook_token?: string, slack_bot_token?: string, team_id?: string, }) => { @@ -200,6 +201,7 @@ export class AdminCommands { team_id, slack_channel_id: channel_id, slack_webhook_uri: webhook_url, + slack_webhook_token: webhook_token, }); respond("Room is now " + r.getStatus()); if (r.SlackWebhookUri) { @@ -234,6 +236,10 @@ export class AdminCommands { alias: "u", description: "Slack webhook URL. Used with Slack outgoing hooks integration", }, + webhook_token: { + alias: "k", + description: "Slack webhook token. Used with Slack outgoing hooks integration", + }, }, ); } diff --git a/src/BridgedRoom.ts b/src/BridgedRoom.ts index 126d6815..a573582d 100644 --- a/src/BridgedRoom.ts +++ b/src/BridgedRoom.ts @@ -36,6 +36,7 @@ interface IBridgedRoomOpts { slack_channel_name?: string; slack_channel_id?: string; slack_webhook_uri?: string; + slack_webhook_token?: string; slack_team_id?: string; slack_type: SlackChannelTypes; is_private?: boolean; @@ -95,6 +96,14 @@ export class BridgedRoom { this.setValue("slackWebhookUri", value); } + public get SlackWebhookToken(): string|undefined { + return this.slackWebhookToken; + } + + public set SlackWebhookToken(value: string|undefined) { + this.setValue("slackWebhookToken", value); + } + public get MatrixRoomId(): string { return this.matrixRoomId; } @@ -135,6 +144,7 @@ export class BridgedRoom { slack_channel_name: entry.remote.name, slack_team_id: entry.remote.slack_team_id, slack_webhook_uri: entry.remote.webhook_uri, + slack_webhook_token: entry.remote.webhook_token, puppet_owner: entry.remote.puppet_owner, is_private: entry.remote.slack_private, slack_type: entry.remote.slack_type as SlackChannelTypes, @@ -146,6 +156,7 @@ export class BridgedRoom { private slackChannelName?: string; private slackChannelId?: string; private slackWebhookUri?: string; + private slackWebhookToken?: string; private slackTeamId?: string; private slackType: SlackChannelTypes; private isPrivate?: boolean; @@ -184,6 +195,7 @@ export class BridgedRoom { this.slackChannelName = opts.slack_channel_name; this.slackChannelId = opts.slack_channel_id; this.slackWebhookUri = opts.slack_webhook_uri; + this.slackWebhookToken = opts.slack_webhook_token; this.slackTeamId = opts.slack_team_id; this.slackType = opts.slack_type || "channel"; if (opts.is_private === undefined) { @@ -247,6 +259,7 @@ export class BridgedRoom { slack_type: this.slackType!, slack_private: this.isPrivate!, webhook_uri: this.slackWebhookUri!, + webhook_token: this.slackWebhookToken!, puppet_owner: this.puppetOwner!, }, remote_id: this.inboundId, diff --git a/src/Main.ts b/src/Main.ts index c69eb9c1..d69d8425 100644 --- a/src/Main.ts +++ b/src/Main.ts @@ -1351,6 +1351,7 @@ export class Main { public async actionLink(opts: { matrix_room_id: string, slack_webhook_uri?: string, + slack_webhook_token?: string, slack_channel_id?: string, slack_bot_token?: string, team_id?: string, @@ -1450,6 +1451,11 @@ export class Main { if (opts.slack_webhook_uri) { room.SlackWebhookUri = opts.slack_webhook_uri; + if (opts.slack_webhook_token) { + room.SlackWebhookToken = opts.slack_webhook_token; + } else { + throw new Error("Cannot link via a webhook without a webhook token"); + } } if (opts.slack_channel_id) { diff --git a/src/SlackHookHandler.ts b/src/SlackHookHandler.ts index 2909e946..ce20d81d 100644 --- a/src/SlackHookHandler.ts +++ b/src/SlackHookHandler.ts @@ -67,7 +67,7 @@ export class SlackHookHandler extends BaseSlackHandler { createServer = (cb) => httpsCreate(tlsOptions, cb); } return new Promise((resolve, reject) => { - const srv = createServer(this.onRequest.bind(this)); + const srv = createServer(this._onRequest.bind(this)); srv.once("error", reject); srv.listen(port, () => { const protocol = tlsConfig ? "https" : "http"; @@ -85,7 +85,7 @@ export class SlackHookHandler extends BaseSlackHandler { } } - private onRequest(req: IncomingMessage, res: ServerResponse) { + public _onRequest(req: IncomingMessage, res: ServerResponse) { const HTTP_SERVER_ERROR = 500; const {method, url } = req; if (!method || !url) { @@ -234,6 +234,16 @@ export class SlackHookHandler extends BaseSlackHandler { return; } + if (params.token !== room.SlackWebhookToken) { + log.warn(`Ignoring message for ${room.MatrixRoomId} due to webhook token mismatch`); + + response.writeHead(HTTP_CODES.FORBIDDEN); + response.end(); + + endTimer({outcome: "dropped"}); + return; + } + if (method === "POST" && path === "post") { try { if (!room) { diff --git a/src/datastore/Models.ts b/src/datastore/Models.ts index 3d0f15b2..ff3fb915 100644 --- a/src/datastore/Models.ts +++ b/src/datastore/Models.ts @@ -28,6 +28,7 @@ export interface RoomEntry { id: string; name: string; webhook_uri?: string; + webhook_token?: string; slack_private?: boolean; puppet_owner?: string; }; diff --git a/tests/integration/WebhookTest.ts b/tests/integration/WebhookTest.ts new file mode 100644 index 00000000..141d84ae --- /dev/null +++ b/tests/integration/WebhookTest.ts @@ -0,0 +1,117 @@ +/* +Copyright 2024 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +import { SlackHookHandler } from "../../src/SlackHookHandler"; +import { FakeMain } from "../utils/fakeMain"; +import { Main } from "../../src/Main"; +import { expect } from "chai"; +import * as httpMocks from "node-mocks-http"; +import * as randomstring from "randomstring"; +import { BridgedRoom } from "../../src/BridgedRoom"; + +const constructHarness = () => { + const main = new FakeMain({ + oauth2: false, + teams: [ + { + bot_token: "foo", + id: "12345", + name: "FakeTeam", + domain: "fake-domain", + user_id: "foo", + bot_id: "bar", + status: "ok", + scopes: "", + }, + ], + }); + const hooks = new SlackHookHandler(main as unknown as Main); + return { hooks, main }; +}; + +const DEFAULT_PAYLOAD = { + team_id: 'T06Q92QGCLC', + team_domain: 'mas', + service_id: '6899401468119', + channel_id: 'C06Q6525S71', + channel_name: 'bridge-testing', + timestamp: '1711628700.919889', + user_id: 'U06QMMZQRH5', + user_name: 'mario', + text: 'incoming!' +}; + +describe("WebhookTest", () => { + let harness: { hooks: SlackHookHandler, main: FakeMain }; + + beforeEach(() => { + harness = constructHarness(); + }); + + async function checkResult(req: httpMocks.MockRequest, expectations: (res: httpMocks.MockResponse) => void): Promise { + const res = httpMocks.createResponse({ eventEmitter: require('events').EventEmitter }); + const promise = new Promise((resolve, reject) => { + res.on('end', () => { + try { + expectations(res); + resolve(); + } catch (err: unknown) { + reject(err); + } + }); + }); + + harness.hooks._onRequest(req, res); + + req.emit('end'); + + return promise; + } + + it("will ignore webhooks sent to unknown room", () => { + const req = httpMocks.createRequest({ + method: 'POST', + url: 'http://foo.bar/webhooks/' + randomstring.generate(32), + params: DEFAULT_PAYLOAD, + }); + + return checkResult(req, res => { + expect(res.statusCode).to.equal(200); + }); + }); + + it("will reject webhooks not containing a valid token", () => { + let room = new BridgedRoom(harness.main as unknown as Main, { + matrix_room_id: '!foo:bar.baz', + inbound_id: randomstring.generate(32), + slack_webhook_token: randomstring.generate(24), + slack_type: "channel", + }); + harness.main.rooms.upsertRoom(room); + + const req = httpMocks.createRequest({ + method: 'POST', + url: 'http://foo.bar/webhooks/' + room.InboundId, + params: { + token: 'invalid', + ...DEFAULT_PAYLOAD, + }, + }); + + return checkResult(req, res => { + expect(res.statusCode).to.equal(403); + }); + }); +}); diff --git a/yarn.lock b/yarn.lock index c009975a..affe3ad6 100644 --- a/yarn.lock +++ b/yarn.lock @@ -616,6 +616,16 @@ "@types/qs" "*" "@types/serve-static" "*" +"@types/express@^4.17.21": + version "4.17.21" + resolved "https://registry.yarnpkg.com/@types/express/-/express-4.17.21.tgz#c26d4a151e60efe0084b23dc3369ebc631ed192d" + integrity sha512-ejlPM315qwLpaQlQDTjPdsUFSc6ZsP4AN6AlWnogPjQ7CVi7PYF3YVz+CY3jE2pwYf7E/7HlDAN0rV2GxTG0HQ== + dependencies: + "@types/body-parser" "*" + "@types/express-serve-static-core" "^4.17.33" + "@types/qs" "*" + "@types/serve-static" "*" + "@types/is-stream@^1.1.0": version "1.1.0" resolved "https://registry.yarnpkg.com/@types/is-stream/-/is-stream-1.1.0.tgz#b84d7bb207a210f2af9bed431dc0fbe9c4143be1" @@ -660,6 +670,13 @@ resolved "https://registry.yarnpkg.com/@types/node/-/node-18.6.1.tgz#828e4785ccca13f44e2fb6852ae0ef11e3e20ba5" integrity sha512-z+2vB6yDt1fNwKOeGbckpmirO+VBDuQqecXkgeIqDlaOtmKn6hPR/viQ8cxCfqLU4fTlvM3+YjM367TukWdxpg== +"@types/node@^20.10.6": + version "20.11.30" + resolved "https://registry.yarnpkg.com/@types/node/-/node-20.11.30.tgz#9c33467fc23167a347e73834f788f4b9f399d66f" + integrity sha512-dHM6ZxwlmuZaRmUPfv1p+KrdD1Dci04FbdEm/9wEMouFqxYoFl5aMkt0VMAUtYRQDyYvD41WJLukhq/ha3YuTw== + dependencies: + undici-types "~5.26.4" + "@types/nunjucks@^3.1.5": version "3.2.1" resolved "https://registry.yarnpkg.com/@types/nunjucks/-/nunjucks-3.2.1.tgz#02a3ade3dc4d3950029c6466a4034565dba7cf8c" @@ -879,7 +896,7 @@ abbrev@1: resolved "https://registry.yarnpkg.com/abbrev/-/abbrev-1.1.1.tgz#f8f2c887ad10bf67f634f005b6987fed3179aac8" integrity sha512-nne9/IiQ/hzIhY6pdDnbBtz7DjPTKrY00P/zvPSm5pOFkl6xuGrGnXn/VtTNNfNtAfZ9/1RtehkszU9qcTii0Q== -accepts@~1.3.8: +accepts@^1.3.7, accepts@~1.3.8: version "1.3.8" resolved "https://registry.yarnpkg.com/accepts/-/accepts-1.3.8.tgz#0bf0be125b67014adcb0b0921e62db7bffe16b2e" integrity sha512-PYAthTa2m2VKxuvSD3DPC/Gy+U+sOA1LAuT8mkmRuvw+NACSaeXEQ+NHcVF7rONl6qcaxV3Uuemwawk+7+SJLw== @@ -1447,7 +1464,7 @@ concat-map@0.0.1: resolved "https://registry.yarnpkg.com/concat-map/-/concat-map-0.0.1.tgz#d8a96bd77fd68df7793a73036a3ba0d5405d477b" integrity sha512-/Srv4dswyQNBfohGpz9o6Yb3Gz3SrUDqBH5rTuhGR7ahtlbYKnVxw2bCFMRljaA7EXHaXZ8wsHdodFvbkhKmqg== -content-disposition@0.5.4: +content-disposition@0.5.4, content-disposition@^0.5.3: version "0.5.4" resolved "https://registry.yarnpkg.com/content-disposition/-/content-disposition-0.5.4.tgz#8b82b4efac82512a02bb0b1dcec9d2c5e8eb5bfe" integrity sha512-FveZTNuGw04cxlAiWbzi6zTAL/lhehaWbTtgluJh4/E95DqMwTmha3KZN1aAWA8cFIhHzMZUvLevkw5Rqk+tSQ== @@ -1569,6 +1586,11 @@ depd@2.0.0, depd@~2.0.0: resolved "https://registry.yarnpkg.com/depd/-/depd-2.0.0.tgz#b696163cc757560d09cf22cc8fad1571b79e76df" integrity sha512-g7nH6P6dyDioJogAAGprGpCtVImJhpPk/roCzdb3fIh61/s/nPsfR6onyMwkCAR/OlC3yBC0lESvUoQEAssIrw== +depd@^1.1.0: + version "1.1.2" + resolved "https://registry.yarnpkg.com/depd/-/depd-1.1.2.tgz#9bcd52e14c097763e749b274c4346ed2e560b5a9" + integrity sha512-7emPTl6Dpo6JRXOXjLRxck+FlLRX5847cLKEn00PLAgc3g2hTZZgr+e4c2v6QpSmLeFP3n5yUo7ft6avBK/5jQ== + destroy@1.2.0: version "1.2.0" resolved "https://registry.yarnpkg.com/destroy/-/destroy-1.2.0.tgz#4803735509ad8be552934c67df614f94e66fa015" @@ -2286,7 +2308,7 @@ fraction.js@^4.2.0: resolved "https://registry.yarnpkg.com/fraction.js/-/fraction.js-4.2.0.tgz#448e5109a313a3527f5a3ab2119ec4cf0e0e2950" integrity sha512-MhLuK+2gUcnZe8ZHlaaINnQLl0xRIGRfcGk2yl8xoQAfHrSsL3rYu6FCmBdkdbhc9EPlwyGHewaRsvwRMJtAlA== -fresh@0.5.2: +fresh@0.5.2, fresh@^0.5.2: version "0.5.2" resolved "https://registry.yarnpkg.com/fresh/-/fresh-0.5.2.tgz#3d8cadd90d976569fa835ab1f8e4b23a105605a7" integrity sha512-zJ2mQYM18rEFOudeV4GShTGIQ7RbzA7ozbU9I/XBpm7kqgMywgmylMwXHxZJmkVoYkna9d2pVXVXPdYTP9ej8Q== @@ -3190,12 +3212,17 @@ merge-descriptors@1.0.1: resolved "https://registry.yarnpkg.com/merge-descriptors/-/merge-descriptors-1.0.1.tgz#b00aaa556dd8b44568150ec9d1b953f3f90cbb61" integrity sha512-cCi6g3/Zr1iqQi6ySbseM1Xvooa98N0w31jzUYrXPX2xqObmFGHJ0tQ5u74H3mVh7wLouTseZyYIq39g8cNp1w== +merge-descriptors@^1.0.1: + version "1.0.3" + resolved "https://registry.yarnpkg.com/merge-descriptors/-/merge-descriptors-1.0.3.tgz#d80319a65f3c7935351e5cfdac8f9318504dbed5" + integrity sha512-gaNvAS7TZ897/rVaZ0nMtAyxNyi/pdbjbAwUpFQpN70GqnVfOiXpeUUMKRBmzXaSQ8DdTX4/0ms62r2K+hE6mQ== + merge2@^1.3.0, merge2@^1.4.1: version "1.4.1" resolved "https://registry.yarnpkg.com/merge2/-/merge2-1.4.1.tgz#4368892f885e907455a6fd7dc55c0c9d404990ae" integrity sha512-8q7VEgMJW4J8tcfVPy8g09NcQwZdbwFEqhe/WZkoIzjn/3TGDwtOCYtXGxA3O8tPzpczCCDgv+P2P5y00ZJOOg== -methods@~1.1.2: +methods@^1.1.2, methods@~1.1.2: version "1.1.2" resolved "https://registry.yarnpkg.com/methods/-/methods-1.1.2.tgz#5529a4d67654134edcc5266656835b0f851afcee" integrity sha512-iclAHeNqNm68zFtnZ0e+1L2yUIdvzNoauKU4WBA3VvH/vPFieF7qfRlwUZU+DA9P9bPXIS90ulxoUoCH23sV2w== @@ -3220,7 +3247,7 @@ mime-types@^2.1.12, mime-types@~2.1.19, mime-types@~2.1.24, mime-types@~2.1.34: dependencies: mime-db "1.52.0" -mime@1.6.0: +mime@1.6.0, mime@^1.3.4: version "1.6.0" resolved "https://registry.yarnpkg.com/mime/-/mime-1.6.0.tgz#32cd9e5c64553bd58d19a568af452acff04981b1" integrity sha512-x0Vn8spI+wuJ1O6S7gnbaQg8Pxh4NNHb7KSINmEWKiPE4RKOplvijn+NkmYmmRgP68mc70j2EbeTFRsrswaQeg== @@ -3388,6 +3415,24 @@ node-emoji@^1.10.0: dependencies: lodash "^4.17.21" +node-mocks-http@^1.14.1: + version "1.14.1" + resolved "https://registry.yarnpkg.com/node-mocks-http/-/node-mocks-http-1.14.1.tgz#6a387ce09229fe545dcc0154d16bc3480618e013" + integrity sha512-mfXuCGonz0A7uG1FEjnypjm34xegeN5+HI6xeGhYKecfgaZhjsmYoLE9LEFmT+53G1n8IuagPZmVnEL/xNsFaA== + dependencies: + "@types/express" "^4.17.21" + "@types/node" "^20.10.6" + accepts "^1.3.7" + content-disposition "^0.5.3" + depd "^1.1.0" + fresh "^0.5.2" + merge-descriptors "^1.0.1" + methods "^1.1.2" + mime "^1.3.4" + parseurl "^1.3.3" + range-parser "^1.2.0" + type-is "^1.6.18" + node-releases@^2.0.8: version "2.0.9" resolved "https://registry.yarnpkg.com/node-releases/-/node-releases-2.0.9.tgz#fe66405285382b0c4ac6bcfbfbe7e8a510650b4d" @@ -3621,7 +3666,7 @@ parseley@^0.7.0: moo "^0.5.1" nearley "^2.20.1" -parseurl@~1.3.3: +parseurl@^1.3.3, parseurl@~1.3.3: version "1.3.3" resolved "https://registry.yarnpkg.com/parseurl/-/parseurl-1.3.3.tgz#9da19e7bee8d12dff0513ed5b76957793bc2e8d4" integrity sha512-CiyeOxFT/JZyN5m0z9PfXw4SCBJ6Sygz1Dpl0wqjlhDEGGBP1GnsUVEL0p63hoG1fcj3fHynXi9NYO4nWOL+qQ== @@ -3950,7 +3995,7 @@ randomstring@^1.2.1: array-uniq "1.0.2" randombytes "2.0.3" -range-parser@~1.2.1: +range-parser@^1.2.0, range-parser@~1.2.1: version "1.2.1" resolved "https://registry.yarnpkg.com/range-parser/-/range-parser-1.2.1.tgz#3cf37023d199e1c24d1a55b84800c2f3e6468031" integrity sha512-Hrgsx+orqoygnmhFbKaHE6c296J+HTAQXoxEF6gNupROmmGJRoyzfG3ccAveqCBrwr/2yxQ5BVd/GTl5agOwSg== @@ -4590,7 +4635,7 @@ type-fest@^0.20.2: resolved "https://registry.yarnpkg.com/type-fest/-/type-fest-0.20.2.tgz#1bf207f4b28f91583666cb5fbd327887301cd5f4" integrity sha512-Ne+eE4r0/iWnpAxD852z3A+N0Bt5RN//NjJwRd2VFHEmrywxf5vsZlh4R6lixl6B+wz/8d+maTSAkN1FIkI3LQ== -type-is@~1.6.18: +type-is@^1.6.18, type-is@~1.6.18: version "1.6.18" resolved "https://registry.yarnpkg.com/type-is/-/type-is-1.6.18.tgz#4e552cd05df09467dcbc4ef739de89f2cf37c131" integrity sha512-TkRKr9sUTxEH8MdfuCSP7VizJyzRNMjj2J2do2Jr3Kym598JVdEksuzPQCnlFPW4ky9Q+iA+ma9BGm06XQBy8g== @@ -4627,6 +4672,11 @@ underscore@~1.4.4: resolved "https://registry.yarnpkg.com/underscore/-/underscore-1.4.4.tgz#61a6a32010622afa07963bf325203cf12239d604" integrity sha512-ZqGrAgaqqZM7LGRzNjLnw5elevWb5M8LEoDMadxIW3OWbcv72wMMgKdwOKpd5Fqxe8choLD8HN3iSj3TUh/giQ== +undici-types@~5.26.4: + version "5.26.5" + resolved "https://registry.yarnpkg.com/undici-types/-/undici-types-5.26.5.tgz#bcd539893d00b56e964fd2657a4866b221a65617" + integrity sha512-JlCMO+ehdEIKqlFxk6IfVoAUVmgz7cU7zD/h9XZ0qzeosSHmUJVOzSQvvYSYWXkFXC+IfLKSIffhv0sVZup6pA== + unpipe@1.0.0, unpipe@~1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/unpipe/-/unpipe-1.0.0.tgz#b2bf4ee8514aae6165b4817829d21b2ef49904ec" From ab1aece1831b7b02f2be83f1542009fec6c92ae6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tadeusz=20=E2=80=9Etadzik=E2=80=9D=20So=C5=9Bnierz?= Date: Thu, 28 Mar 2024 14:30:02 +0100 Subject: [PATCH 2/4] Update linking documentation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tadeusz „tadzik” Sośnierz --- docs/link_channels.md | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/docs/link_channels.md b/docs/link_channels.md index ae3d2e09..f9a8da7e 100644 --- a/docs/link_channels.md +++ b/docs/link_channels.md @@ -6,6 +6,13 @@ first the individual Matrix room and Slack channel need to be created, and then a command needs to be issued in the administration console room to add the link to the bridge's database. +## Determining your channel ID + +You'll need a "channel ID" to link rooms. +To obtain it, right-click your channel name in Slack and select "Copy Link". +The channel id is the last argument in the url +(`https://XXX.Slack.com/messages//`) + ## RTM API The Real Time Messaging (RTM) API is the newer and recommended way to use the bridge. @@ -47,12 +54,9 @@ The Real Time Messaging (RTM) API is the newer and recommended way to use the br /invite @bot-user-name ``` - You will also need to determine the "channel ID" that Slack uses to identify - the channel. Right-click your channel name in Slack and select "Copy Link". - The channel id is the last argument in the url - (`https://XXX.Slack.com/messages//`) + 4. Obtain the channel ID (see "Determining your channel ID") - 4. Issue a ``link`` command in the administration control room with these + 5. Issue a ``link`` command in the administration control room with these collected values as arguments: ``` @@ -104,17 +108,13 @@ although it can be useful for single channels or if you are using Mattermost. of its `token` field. Add a URL to this web hook pointing back at the application service port you configured during setup. - You will also need to determine the "channel ID" that Slack uses to identify - the channel. Unfortunately, it is not easily obtained from the Slack UI. The - easiest way to do this is to send a message from Slack to the bridge; the - bridge will log the channel ID as part of the unrecognised message output. - You can then take note of the `channel_id` field. +1. Obtain the channel ID (see "Determining your channel ID") 1. Issue a ``link`` command in the administration control room with these collected values as arguments: ``` - link --channel_id CHANNELID --room !the-matrix:room.id --webhook_url https://hooks.Slack.com/services/ABC/DEF/123 + link --channel_id CHANNELID --room !the-matrix:room.id --webhook_url https://hooks.Slack.com/services/ABC/DEF/123 --webhook_token TOKEN ``` ## Unlink Channels From 1b0eb3a020371955c400ac857c36d44d1b04f092 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tadeusz=20=E2=80=9Etadzik=E2=80=9D=20So=C5=9Bnierz?= Date: Thu, 28 Mar 2024 16:59:39 +0100 Subject: [PATCH 3/4] Improve plaintext formatting of Matrix messages sent via webhooks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In webhook mode sending messages comes with some limitations, including us not being able to impersonate users or upload attachements. This makes us fallback to plaintext relaybot behaviour, while preserving as much from the original message as possible. Signed-off-by: Tadeusz „tadzik” Sośnierz --- src/BridgedRoom.ts | 30 ++++++++++-- tests/integration/WebhookTest.ts | 84 +++++++++++++++++++++----------- tests/utils/fakeMain.ts | 9 ++++ 3 files changed, 90 insertions(+), 33 deletions(-) diff --git a/src/BridgedRoom.ts b/src/BridgedRoom.ts index a573582d..7e6b6ca1 100644 --- a/src/BridgedRoom.ts +++ b/src/BridgedRoom.ts @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import axios from "axios"; +import axios, { AxiosInstance } from "axios"; import { Logger, Intent } from "matrix-appservice-bridge"; import { SlackGhost } from "./SlackGhost"; import { Main, METRIC_SENT_MESSAGES } from "./Main"; @@ -180,7 +180,13 @@ export class BridgedRoom { */ private dirty: boolean; - constructor(private main: Main, opts: IBridgedRoomOpts, private team?: TeamEntry, private botClient?: WebClient) { + constructor( + private main: Main, + opts: IBridgedRoomOpts, + private team?: TeamEntry, + private botClient?: WebClient, + private httpClient: AxiosInstance = axios + ) { this.MatrixRoomActive = true; if (!opts.inbound_id) { @@ -470,7 +476,7 @@ export class BridgedRoom { } log.debug("Room might be encrypted, uploading file to Slack"); // Media might be encrypted, upload it to Slack to be safe. - const response = await axios.get(matrixToSlackResult.encrypted_file, { + const response = await this.httpClient.get(matrixToSlackResult.encrypted_file, { headers: { Authorization: `Bearer ${slackClient.token}`, }, @@ -526,13 +532,27 @@ export class BridgedRoom { user.bumpATime(); this.matrixATime = Date.now() / 1000; + if (!slackClient) { if (!this.slackWebhookUri) { throw Error('No slackClient and slackWebhookUri'); } - const webhookRes = await axios.post(this.slackWebhookUri, body); + let plainText = body.text; + if (!plainText && body.attachments) { + const parts: string[] = []; + for (const attachment of body.attachments) { + parts.push(`Uploaded "${attachment.fallback}": ${attachment.image_url}`); + } + plainText = parts.join("\n"); + } + if (!plainText) { + log.warn("Nothing to send via webhook from message", body); + return false; + } + const webhookRes = await this.httpClient.post(this.slackWebhookUri, { text: `<${body.username}> ${plainText}` }); if (webhookRes.status !== 200) { log.error("Failed to send webhook message"); + return false; } // Webhooks don't give us any ID, so we can't store this. return true; @@ -898,7 +918,7 @@ export class BridgedRoom { if (file.mode === "snippet") { let htmlString: string; try { - const fileReq = await axios.get(filePrivateUrl, { + const fileReq = await this.httpClient.get(filePrivateUrl, { headers: { // Token is checked above. Authorization: `Bearer ${authToken}`, diff --git a/tests/integration/WebhookTest.ts b/tests/integration/WebhookTest.ts index 141d84ae..814e22a3 100644 --- a/tests/integration/WebhookTest.ts +++ b/tests/integration/WebhookTest.ts @@ -20,25 +20,28 @@ import { expect } from "chai"; import * as httpMocks from "node-mocks-http"; import * as randomstring from "randomstring"; import { BridgedRoom } from "../../src/BridgedRoom"; +import { FakeClientFactory } from "../utils/fakeClientFactory"; +import { AxiosInstance } from "axios"; const constructHarness = () => { - const main = new FakeMain({ - oauth2: false, - teams: [ - { - bot_token: "foo", - id: "12345", - name: "FakeTeam", - domain: "fake-domain", - user_id: "foo", - bot_id: "bar", - status: "ok", - scopes: "", - }, - ], - }); + const main = new FakeMain(); + main.clientFactory = { + getClientForUser: () => Promise.resolve(), + } as unknown as FakeClientFactory; const hooks = new SlackHookHandler(main as unknown as Main); - return { hooks, main }; + + const http = new MockAxios(); + + const room = new BridgedRoom(main as unknown as Main, { + matrix_room_id: '!foo:bar.baz', + inbound_id: randomstring.generate(32), + slack_webhook_token: randomstring.generate(24), + slack_webhook_uri: 'http://test.url', + slack_type: "channel", + }, undefined, undefined, http as unknown as AxiosInstance); + main.rooms.upsertRoom(room); + + return { hooks, http, main, room }; }; const DEFAULT_PAYLOAD = { @@ -53,8 +56,17 @@ const DEFAULT_PAYLOAD = { text: 'incoming!' }; +class MockAxios { + calls: { url: string, payload: any }[] = []; + + async post(url: string, payload: any) { + this.calls.push({ url, payload }); + return { status: 200 }; + } +} + describe("WebhookTest", () => { - let harness: { hooks: SlackHookHandler, main: FakeMain }; + let harness: { hooks: SlackHookHandler, http: MockAxios, main: FakeMain, room: BridgedRoom }; beforeEach(() => { harness = constructHarness(); @@ -80,7 +92,7 @@ describe("WebhookTest", () => { return promise; } - it("will ignore webhooks sent to unknown room", () => { + it("will ignore webhooks sent to unknown room", async () => { const req = httpMocks.createRequest({ method: 'POST', url: 'http://foo.bar/webhooks/' + randomstring.generate(32), @@ -92,18 +104,10 @@ describe("WebhookTest", () => { }); }); - it("will reject webhooks not containing a valid token", () => { - let room = new BridgedRoom(harness.main as unknown as Main, { - matrix_room_id: '!foo:bar.baz', - inbound_id: randomstring.generate(32), - slack_webhook_token: randomstring.generate(24), - slack_type: "channel", - }); - harness.main.rooms.upsertRoom(room); - + it("will reject webhooks not containing a valid token", async () => { const req = httpMocks.createRequest({ method: 'POST', - url: 'http://foo.bar/webhooks/' + room.InboundId, + url: 'http://foo.bar/webhooks/' + harness.room.InboundId, params: { token: 'invalid', ...DEFAULT_PAYLOAD, @@ -114,4 +118,28 @@ describe("WebhookTest", () => { expect(res.statusCode).to.equal(403); }); }); + + it('can send message via a webhook', async () => { + const res = await harness.room.onMatrixMessage({ + sender: '@testuser:mxserver.url', + content: { body: 'hi' }, + }); + + expect(res).to.equal(true); + expect(harness.http.calls[0].url).to.equal(harness.room.SlackWebhookUri); + expect(harness.http.calls[0].payload.text).to.contain('testuser'); + expect(harness.http.calls[0].payload.text).to.contain('hi'); + }); + + it('can send message via a webhook', async () => { + const res = await harness.room.onMatrixMessage({ + sender: '@testuser:mxserver.url', + content: { body: 'hi' }, + }); + + expect(res).to.equal(true); + expect(harness.http.calls[0].url).to.equal(harness.room.SlackWebhookUri); + expect(harness.http.calls[0].payload.text).to.contain('testuser'); + expect(harness.http.calls[0].payload.text).to.contain('hi'); + }); }); diff --git a/tests/utils/fakeMain.ts b/tests/utils/fakeMain.ts index 2bd50286..3d0c2933 100644 --- a/tests/utils/fakeMain.ts +++ b/tests/utils/fakeMain.ts @@ -50,6 +50,15 @@ export class FakeMain { }; } + public getOrCreateMatrixUser(mxid: string) { + const localpart = mxid.split(':')[0].substring(1); + return { + bumpATime: () => {}, + getAvatarUrlForRoom: () => Promise.resolve(), + getDisplaynameForRoom: () => Promise.resolve(localpart), + }; + } + public getUrlForMxc(mxcUrl: string): string { return "fake-" + mxcUrl; } From 828513634699e182aee2a350599eb112ec14f436 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tadeusz=20=E2=80=9Etadzik=E2=80=9D=20So=C5=9Bnierz?= Date: Tue, 4 Jun 2024 12:22:29 +0200 Subject: [PATCH 4/4] Changelog --- changelog.d/778.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/778.feature diff --git a/changelog.d/778.feature b/changelog.d/778.feature new file mode 100644 index 00000000..017a4979 --- /dev/null +++ b/changelog.d/778.feature @@ -0,0 +1 @@ +Improve plaintext formatting of Matrix messages sent via webhooks.