From cb8000f3378263f772a8426c5e4f84e27beb06e2 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Thu, 7 May 2020 12:35:14 +0100 Subject: [PATCH 01/16] Add config option --- config/config.sample.yaml | 2 ++ config/slack-config-schema.yaml | 2 ++ src/IConfig.ts | 1 + 3 files changed, 5 insertions(+) diff --git a/config/config.sample.yaml b/config/config.sample.yaml index a4ea8716..27223c59 100644 --- a/config/config.sample.yaml +++ b/config/config.sample.yaml @@ -104,6 +104,8 @@ provisioning: enabled: true # Should the bridge deny users bridging channels to private rooms. require_public_room: true + # Should the bridge allow usesr to bridge private channels. + allow_private_channels: true limits: room_count: 20 team_count: 1 diff --git a/config/slack-config-schema.yaml b/config/slack-config-schema.yaml index c19b5c21..5f464ed4 100644 --- a/config/slack-config-schema.yaml +++ b/config/slack-config-schema.yaml @@ -112,6 +112,8 @@ properties: type: boolean require_public_room: type: boolean + allow_private_channels: + type: boolean limits: type: object properties: diff --git a/src/IConfig.ts b/src/IConfig.ts index b7faea2c..ab889cd7 100644 --- a/src/IConfig.ts +++ b/src/IConfig.ts @@ -83,6 +83,7 @@ export interface IConfig { provisioning?: { enable: boolean; require_public_room?: boolean; + allow_private_channels?: boolean; limits?: { team_count?: number; room_count?: number; From b80146898ae6b6ae536609f3c48cfcca93956f2e Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Thu, 7 May 2020 12:35:21 +0100 Subject: [PATCH 02/16] List channels by userId --- src/Provisioning.ts | 34 ++++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/src/Provisioning.ts b/src/Provisioning.ts index 40aba67d..3501337f 100644 --- a/src/Provisioning.ts +++ b/src/Provisioning.ts @@ -87,6 +87,20 @@ export class Provisioner { return (currentCount >= this.main.config.provisioning?.limits?.room_count); } + private async determineSlackIdForRequest(matrixUserId, teamId) { + const matrixUser = await this.main.datastore.getMatrixUser(matrixUserId); + if (matrixUser === null) { + throw Error('No users found'); + } + const accounts: {[userId: string]: {team_id: string}} = matrixUser.get("accounts"); + for (const [key, value] of Object.entries(accounts)) { + if (value.team_id === teamId) { + return key; + } + } + return null; + } + @command() private async getconfig(_, res) { const hasRoomLimit = this.main.config.provisioning?.limits?.room_count; @@ -150,14 +164,9 @@ export class Provisioner { @command("user_id", "team_id") private async channels(req, res, userId, teamId) { log.debug(`${userId} for ${teamId} requested their channels`); - const matrixUser = await this.main.datastore.getMatrixUser(userId); - const isAllowed = matrixUser !== null && - Object.values(matrixUser.get("accounts") as {[key: string]: {team_id: string}}).find((acct) => - acct.team_id === teamId, - ); - if (!isAllowed) { - res.status(HTTP_CODES.CLIENT_ERROR).json({error: "User is not part of this team!"}); - throw undefined; + const slackUserId = await this.determineSlackIdForRequest(userId, teamId); + if (!slackUserId) { + return res.status(HTTP_CODES.CLIENT_ERROR).json({error: "User is not part of this team!"}); } const team = await this.main.datastore.getTeam(teamId); if (team === null) { @@ -165,10 +174,15 @@ export class Provisioner { } const cli = await this.main.clientFactory.getTeamClient(teamId); try { - const response = (await cli.conversations.list({ + let types = "public_channel"; + if (this.main.config.provisioning?.allow_private_channels !== false) { + types = `public_channel,private_channel`; + } + const response = (await cli.users.list({ exclude_archived: true, limit: 1000, // TODO: Pagination - types: "public_channel", // TODO: In order to show private channels, we need the identity of the caller. + user: slackUserId, // In order to show private channels, we need the identity of the caller. + types, })) as ConversationsListResponse; if (!response.ok) { throw Error(response.error); From d1ab2fef75fecef05548d60317a4b6ba35377bf8 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Thu, 7 May 2020 12:39:33 +0100 Subject: [PATCH 03/16] changelog --- changelog.d/403.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/403.bugfix diff --git a/changelog.d/403.bugfix b/changelog.d/403.bugfix new file mode 100644 index 00000000..87d38701 --- /dev/null +++ b/changelog.d/403.bugfix @@ -0,0 +1 @@ +Allow bridging to private channels via the provisioner \ No newline at end of file From 1309f7d8c8449b46dc30e00ca0d303f67fe0156a Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Fri, 8 May 2020 13:53:16 +0100 Subject: [PATCH 04/16] Fix exception thrown in exception handler --- src/SlackClientFactory.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SlackClientFactory.ts b/src/SlackClientFactory.ts index 0a7b583d..530c1ae9 100644 --- a/src/SlackClientFactory.ts +++ b/src/SlackClientFactory.ts @@ -234,7 +234,7 @@ export class SlackClientFactory { } return { slackClient, team: teamInfo.team, auth, user }; } catch (ex) { - throw Error("Could not create team client: " + ex.data.error); + throw Error("Could not create team client: " + (ex.data?.error || ex)); } } } From 9c6a46b32377c20a7b9424d38d7f4f769ce00fbf Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Fri, 8 May 2020 13:55:06 +0100 Subject: [PATCH 05/16] changelog --- changelog.d/404.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/404.misc diff --git a/changelog.d/404.misc b/changelog.d/404.misc new file mode 100644 index 00000000..201d0529 --- /dev/null +++ b/changelog.d/404.misc @@ -0,0 +1 @@ +Fix exception on missing `error` in createTeamClient \ No newline at end of file From 6e4a2a224f763b1bb496fc5ff18d32eac5f4802d Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Fri, 8 May 2020 13:56:36 +0100 Subject: [PATCH 06/16] Update pg-promise to ^10.5.3 --- package-lock.json | 48 +++++++++++++++++++++++------------------------ package.json | 2 +- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/package-lock.json b/package-lock.json index 7b5f31ef..d6c48aed 100644 --- a/package-lock.json +++ b/package-lock.json @@ -397,9 +397,9 @@ } }, "assert-options": { - "version": "0.6.1", - "resolved": "https://registry.npmjs.org/assert-options/-/assert-options-0.6.1.tgz", - "integrity": "sha512-jH2pNULN0t3uFLb7Fh0SAuMo/Ei5yWiRirvLez2g+sd16d0xKl+DGdGkD6sqkrZTnCZK5lWRjUa4X3sxHQkg9g==" + "version": "0.6.2", + "resolved": "https://registry.npmjs.org/assert-options/-/assert-options-0.6.2.tgz", + "integrity": "sha512-KP9S549XptFAPGYmLRnIjQBL4/Ry8Jx5YNLQZ/l+eejqbTidBMnw4uZSAsUrzBq/lgyqDYqxcTF7cOxZb9gyEw==" }, "assert-plus": { "version": "1.0.0", @@ -2103,15 +2103,15 @@ "integrity": "sha1-Ywn04OX6kT7BxpMHrjZLSzd8nns=" }, "pg": { - "version": "7.18.2", - "resolved": "https://registry.npmjs.org/pg/-/pg-7.18.2.tgz", - "integrity": "sha512-Mvt0dGYMwvEADNKy5PMQGlzPudKcKKzJds/VbOeZJpb6f/pI3mmoXX0JksPgI3l3JPP/2Apq7F36O63J7mgveA==", + "version": "8.0.3", + "resolved": "https://registry.npmjs.org/pg/-/pg-8.0.3.tgz", + "integrity": "sha512-fvcNXn4o/iq4jKq15Ix/e58q3jPSmzOp6/8C3CaHoSR/bsxdg+1FXfDRePdtE/zBb3++TytvOrS1hNef3WC/Kg==", "requires": { "buffer-writer": "2.0.0", "packet-reader": "1.0.0", "pg-connection-string": "0.1.3", - "pg-packet-stream": "^1.1.0", - "pg-pool": "^2.0.10", + "pg-pool": "^3.1.1", + "pg-protocol": "^1.2.2", "pg-types": "^2.1.0", "pgpass": "1.x", "semver": "4.3.2" @@ -2139,27 +2139,27 @@ "resolved": "https://registry.npmjs.org/pg-minify/-/pg-minify-1.5.2.tgz", "integrity": "sha512-uZn/gXkGmO5JBdopxNLSpFMS1lXr+KJqynI8Di1Qyr8ZVXt67ruh+XNfzLMVdLzYv+MQRdNYQdVwWPSs0qM7xQ==" }, - "pg-packet-stream": { - "version": "1.1.0", - "resolved": "https://registry.npmjs.org/pg-packet-stream/-/pg-packet-stream-1.1.0.tgz", - "integrity": "sha512-kRBH0tDIW/8lfnnOyTwKD23ygJ/kexQVXZs7gEyBljw4FYqimZFxnMMx50ndZ8In77QgfGuItS5LLclC2TtjYg==" - }, "pg-pool": { - "version": "2.0.10", - "resolved": "https://registry.npmjs.org/pg-pool/-/pg-pool-2.0.10.tgz", - "integrity": "sha512-qdwzY92bHf3nwzIUcj+zJ0Qo5lpG/YxchahxIN8+ZVmXqkahKXsnl2aiJPHLYN9o5mB/leG+Xh6XKxtP7e0sjg==" + "version": "3.1.1", + "resolved": "https://registry.npmjs.org/pg-pool/-/pg-pool-3.1.1.tgz", + "integrity": "sha512-kYH6S0mcZF1TPg1F9boFee2JlCSm2oqnlR2Mz2Wgn1psQbEBNVeNTJCw2wCK48QsctwvGUzbxLMg/lYV6hL/3A==" }, "pg-promise": { - "version": "10.4.4", - "resolved": "https://registry.npmjs.org/pg-promise/-/pg-promise-10.4.4.tgz", - "integrity": "sha512-N2NsOgKxrnNPwP0Q609ZmxmAZEo2TQ26SzSvlbZWQb8vteqUhOPpU/pHi9DGatJrPcXNoyr4xjRw42CNfEBg/w==", + "version": "10.5.3", + "resolved": "https://registry.npmjs.org/pg-promise/-/pg-promise-10.5.3.tgz", + "integrity": "sha512-cfHgFpcqOc0IIRDABre//k1eda7s94Wys67P9r3q590nmvO0AzZucqWTVmgRUzNTIrDChUwY4hVOMBTIsU/ZiA==", "requires": { - "assert-options": "0.6.1", - "pg": "7.18.2", + "assert-options": "0.6.2", + "pg": "8.0.3", "pg-minify": "1.5.2", "spex": "3.0.1" } }, + "pg-protocol": { + "version": "1.2.2", + "resolved": "https://registry.npmjs.org/pg-protocol/-/pg-protocol-1.2.2.tgz", + "integrity": "sha512-r8hGxHOk3ccMjjmhFJ/QOSVW5A+PP84TeRlEwB/cQ9Zu+bvtZg8Z59Cx3AMfVQc9S0Z+EG+HKhicF1W1GN5Eqg==" + }, "pg-types": { "version": "2.2.0", "resolved": "https://registry.npmjs.org/pg-types/-/pg-types-2.2.0.tgz", @@ -2197,9 +2197,9 @@ "integrity": "sha1-AntTPAqokOJtFy1Hz5zOzFIazTU=" }, "postgres-date": { - "version": "1.0.4", - "resolved": "https://registry.npmjs.org/postgres-date/-/postgres-date-1.0.4.tgz", - "integrity": "sha512-bESRvKVuTrjoBluEcpv2346+6kgB7UlnqWZsnbnCccTNq/pqfj1j6oBaN5+b/NrDXepYUT/HKadqv3iS9lJuVA==" + "version": "1.0.5", + "resolved": "https://registry.npmjs.org/postgres-date/-/postgres-date-1.0.5.tgz", + "integrity": "sha512-pdau6GRPERdAYUQwkBnGKxEfPyhVZXG/JiS44iZWiNdSOWE09N2lUgN6yshuq6fVSon4Pm0VMXd1srUUkLe9iA==" }, "postgres-interval": { "version": "1.2.0", diff --git a/package.json b/package.json index 85435968..5bec5a99 100644 --- a/package.json +++ b/package.json @@ -40,7 +40,7 @@ "nedb": "^1.8.0", "node-emoji": "^1.10.0", "p-queue": "^6.3.0", - "pg-promise": "^10.4.4", + "pg-promise": "^10.5.3", "quick-lru": "^5.0.0", "randomstring": "^1", "request-promise-native": "^1.0.8", From 485c8d946aaa70e21207532cfe710310d0ee0fed Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Fri, 8 May 2020 14:08:26 +0100 Subject: [PATCH 07/16] prepare --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 5bec5a99..10b79cda 100644 --- a/package.json +++ b/package.json @@ -4,7 +4,7 @@ "description": "A Matrix <--> Slack bridge", "main": "app.js", "scripts": { - "postinstall": "npm run build", + "prepare": "npm run build", "start": "node ./lib/app.js", "build": "tsc", "test": "npm run test:unit && npm run test:integration", From 1b4daa296ff52ade31c33e1cd4cb4dfe77971bf9 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Fri, 8 May 2020 14:08:31 +0100 Subject: [PATCH 08/16] Pin to v14 to avoid further breakages --- .dockerignore | 2 +- Dockerfile | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/.dockerignore b/.dockerignore index 2435d360..4a89ff17 100644 --- a/.dockerignore +++ b/.dockerignore @@ -41,4 +41,4 @@ node_modules # typescript lib - +tsconfig.tsbuildinfo diff --git a/Dockerfile b/Dockerfile index 5ebbdad3..63e9da6c 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,21 +1,21 @@ -FROM node:current-alpine AS BUILD -COPY . /tmp/src +FROM node:14-alpine AS BUILD +COPY . /src # git is needed to install Half-Shot/slackdown RUN apk add git -RUN cd /tmp/src \ - && npm install \ - && npm run build +WORKDIR /src +RUN npm install +RUN npm run build -FROM node:current-alpine +FROM node:14-alpine VOLUME /data/ /config/ COPY package.json /usr/src/app/ COPY package-lock.json /usr/src/app/ -COPY --from=BUILD /tmp/src/config /usr/src/app/config -COPY --from=BUILD /tmp/src/lib /usr/src/app/lib +COPY --from=BUILD /src/config /usr/src/app/config +COPY --from=BUILD /src/lib /usr/src/app/lib WORKDIR /usr/src/app From 445e075fd35f52dc7a7566d9a650f172672b289b Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Fri, 8 May 2020 14:13:10 +0100 Subject: [PATCH 09/16] changelog --- changelog.d/405.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/405.bugfix diff --git a/changelog.d/405.bugfix b/changelog.d/405.bugfix new file mode 100644 index 00000000..e7e22d6d --- /dev/null +++ b/changelog.d/405.bugfix @@ -0,0 +1 @@ +Fix postgress configurations failing to start when using the offical docker image. \ No newline at end of file From be1f5e1b862ec1d058214faac1a228bb6dcb0663 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Mon, 11 May 2020 13:08:29 +0100 Subject: [PATCH 10/16] Log details of error --- src/SlackClientFactory.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/SlackClientFactory.ts b/src/SlackClientFactory.ts index 530c1ae9..394fb045 100644 --- a/src/SlackClientFactory.ts +++ b/src/SlackClientFactory.ts @@ -234,7 +234,8 @@ export class SlackClientFactory { } return { slackClient, team: teamInfo.team, auth, user }; } catch (ex) { - throw Error("Could not create team client: " + (ex.data?.error || ex)); + log.error("Could not create team client: " + (ex.data?.error || ex)); + throw Error("Could not create team client"); } } } From 5d9f4d7b05cf2c71f0f7ac7d56f669899e910a5b Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Mon, 11 May 2020 13:16:04 +0100 Subject: [PATCH 11/16] Ensure user's displayname is not modified when they delete a bot --- changelog.d/408.bugfix | 1 + src/SlackGhost.ts | 6 +++++- 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 changelog.d/408.bugfix diff --git a/changelog.d/408.bugfix b/changelog.d/408.bugfix new file mode 100644 index 00000000..629534f6 --- /dev/null +++ b/changelog.d/408.bugfix @@ -0,0 +1 @@ +Bridge will no longer update user's displayname with a bots name when a bot is modified \ No newline at end of file diff --git a/src/SlackGhost.ts b/src/SlackGhost.ts index 8880cb62..286db9bf 100644 --- a/src/SlackGhost.ts +++ b/src/SlackGhost.ts @@ -155,7 +155,11 @@ export class SlackGhost { let displayName = message.username || message.user_name; if (room.SlackClient) { // We can be smarter if we have the bot. - if (message.bot_id) { + if (message.bot_id && message.user_id) { + // In the case of operations on bots, we will have both a bot_id and a user_id. + // Ignore updating the displayname in this case. + return; + } else if (message.bot_id) { displayName = await this.getBotName(message.bot_id, room.SlackClient); } else if (message.user_id) { displayName = await this.getDisplayname(room.SlackClient); From d1d98e5ba2613b1fb3792d2290f106e397ce888a Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Mon, 11 May 2020 13:23:31 +0100 Subject: [PATCH 12/16] Also fix the actual bug --- src/Main.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Main.ts b/src/Main.ts index 19e8baa4..373d786d 100644 --- a/src/Main.ts +++ b/src/Main.ts @@ -798,7 +798,9 @@ export class Main { this.clientfactory = new SlackClientFactory(this.datastore, this.config, (method: string) => { this.incRemoteCallCounter(method); }, (teamId: string, delta: number) => { - this.metricPuppets.inc({ team_id: teamId }, delta); + if (this.metricPuppets) { + this.metricPuppets.inc({ team_id: teamId }, delta); + } }); let puppetsWaiting: Promise = Promise.resolve(); if (this.slackRtm) { From 3f3f8ba3f9959e0db34d0d04b92c3f99ccb25090 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Mon, 11 May 2020 13:24:14 +0100 Subject: [PATCH 13/16] fix tests --- src/tests/unit/SlackClientFactory.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tests/unit/SlackClientFactory.ts b/src/tests/unit/SlackClientFactory.ts index 3f2c5f19..22a1277e 100644 --- a/src/tests/unit/SlackClientFactory.ts +++ b/src/tests/unit/SlackClientFactory.ts @@ -125,7 +125,7 @@ describe("SlackClientFactory", () => { await factory.getTeamClient("faketeam"); throw Error("Call didn't throw as expected"); } catch (ex) { - expect(ex.message).to.equal("Could not create team client: Team not allowed for test"); + expect(ex.message).to.equal("Could not create team client"); expect(ds.teams[0].status).to.equal("bad_auth"); } }); @@ -146,7 +146,7 @@ describe("SlackClientFactory", () => { await factory.getTeamClient("faketeam"); throw Error("Call didn't throw as expected"); } catch (ex) { - expect(ex.message).to.equal("Could not create team client: Team not allowed for test"); + expect(ex.message).to.equal("Could not create team client"); expect(ds.teams[0].status).to.equal("bad_auth"); } }); From ae75e4b3525186bb0e453ca46eb0354afbcd31fc Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Mon, 11 May 2020 13:37:32 +0100 Subject: [PATCH 14/16] Fix getMatrixUser types --- src/datastore/postgres/PgDatastore.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/datastore/postgres/PgDatastore.ts b/src/datastore/postgres/PgDatastore.ts index f4a8a5d2..7f8fd5b8 100644 --- a/src/datastore/postgres/PgDatastore.ts +++ b/src/datastore/postgres/PgDatastore.ts @@ -47,7 +47,7 @@ export class PgDatastore implements Datastore { }); } - public async getUser(id: string): Promise { + public async getUser(id: string): Promise { const dbEntry = await this.postgresDb.oneOrNone("SELECT * FROM users WHERE userId = ${id}", { id }); if (!dbEntry) { return null; @@ -55,7 +55,7 @@ export class PgDatastore implements Datastore { return JSON.parse(dbEntry.json); } - public async getMatrixUser(userId: string): Promise { + public async getMatrixUser(userId: string): Promise { userId = new MatrixUser(userId).getId(); // Ensure ID correctness const userData = await this.getUser(userId); return userData !== null ? new MatrixUser(userId, userData) : null; From 6b0b37a81c30687b01e5fb12b06543283f58a82c Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Mon, 11 May 2020 13:37:43 +0100 Subject: [PATCH 15/16] Twiddle --- src/Provisioning.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Provisioning.ts b/src/Provisioning.ts index 3501337f..f0da1196 100644 --- a/src/Provisioning.ts +++ b/src/Provisioning.ts @@ -89,8 +89,9 @@ export class Provisioner { private async determineSlackIdForRequest(matrixUserId, teamId) { const matrixUser = await this.main.datastore.getMatrixUser(matrixUserId); - if (matrixUser === null) { - throw Error('No users found'); + if (!matrixUser) { + // No Slack user entry found for MXID + return null; } const accounts: {[userId: string]: {team_id: string}} = matrixUser.get("accounts"); for (const [key, value] of Object.entries(accounts)) { @@ -175,6 +176,7 @@ export class Provisioner { const cli = await this.main.clientFactory.getTeamClient(teamId); try { let types = "public_channel"; + // Unless we *explicity* set this to false, allow it. if (this.main.config.provisioning?.allow_private_channels !== false) { types = `public_channel,private_channel`; } From 1be593d0a199c352f162a107d2697821e2cdeea9 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Mon, 11 May 2020 13:56:04 +0100 Subject: [PATCH 16/16] Ignore avatar changes too --- src/SlackGhost.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/SlackGhost.ts b/src/SlackGhost.ts index 286db9bf..3f095e1d 100644 --- a/src/SlackGhost.ts +++ b/src/SlackGhost.ts @@ -249,7 +249,11 @@ export class SlackGhost { } let avatarUrl; let hash: string|undefined; - if (message.bot_id) { + if (message.bot_id && message.user_id) { + // In the case of operations on bots, we will have both a bot_id and a user_id. + // Ignore updating the displayname in this case. + return; + } else if (message.bot_id) { avatarUrl = await this.getBotAvatarUrl(message.bot_id, room.SlackClient); hash = avatarUrl; } else if (message.user_id) {