diff --git a/src/BaseSlackHandler.ts b/src/BaseSlackHandler.ts index c9f29c0c..ee7bf875 100644 --- a/src/BaseSlackHandler.ts +++ b/src/BaseSlackHandler.ts @@ -50,6 +50,7 @@ export interface ISlackEventMessageAttachment { } export interface ISlackMessageEvent extends ISlackEvent { + team?: string; team_domain?: string; team_id?: string; user?: string; diff --git a/src/BridgedRoom.ts b/src/BridgedRoom.ts index 126d6815..0ccf525e 100644 --- a/src/BridgedRoom.ts +++ b/src/BridgedRoom.ts @@ -699,8 +699,9 @@ export class BridgedRoom { await new Promise((r) => setTimeout(r, PUPPET_INCOMING_DELAY_MS)); } } - if (this.recentSlackMessages.includes(message.ts)) { + if (this.recentSlackMessages.includes(message.event_ts ?? message.ts)) { // We sent this, ignore. + log.debug('Ignoring message recently sent by us'); return; } try { @@ -712,8 +713,8 @@ export class BridgedRoom { } this.slackSendLock = this.slackSendLock.then(() => { // Check again - if (this.recentSlackMessages.includes(message.ts)) { - // We sent this, ignore + if (this.recentSlackMessages.includes(message.event_ts ?? message.ts)) { + log.debug('Ignoring message recently sent by us'); return; } return this.handleSlackMessage(message, ghost).catch((ex) => { @@ -878,7 +879,7 @@ export class BridgedRoom { formatted_body: `${file.name}`, msgtype: "m.text", }; - await ghost.sendMessage(this.matrixRoomId, messageContent, channelId, slackEventId); + await ghost.sendMessage(this.matrixRoomId, messageContent, channelId, slackEventId, { attachment_id: file.id }); return; } @@ -917,7 +918,7 @@ export class BridgedRoom { formatted_body: htmlCode, msgtype: "m.text", }; - await ghost.sendMessage(this.matrixRoomId, messageContent, channelId, slackEventId); + await ghost.sendMessage(this.matrixRoomId, messageContent, channelId, slackEventId, { attachment_id: file.id }); return; } @@ -954,6 +955,7 @@ export class BridgedRoom { slackFileToMatrixMessage(file, fileContentUri, thumbnailContentUri), channelId, slackEventId, + { attachment_id: file.id }, ); } @@ -1018,6 +1020,30 @@ export class BridgedRoom { const newMessageRich = substitutions.slackToMatrix(message.text!); const newMessage = ghost.prepareBody(newMessageRich); + // Check if any of the attachments have been deleted. + // Slack unfortunately puts a "tombstone" in both message versions in this event, + // so let's try to remove every single one even if we may have deleted it before. + for (const file of message.message?.files ?? []) { + if (file.mode === 'tombstone') { + const events = await this.main.datastore.getEventsBySlackId(channelId, message.previous_message!.ts); + const event = events.find(e => e._extras.attachment_id === file.id); + if (event) { + const team = message.team_id ? await this.main.datastore.getTeam(message.team_id) : null; + if (!team) { + log.warn("Cannot determine team for message", message, "so we cannot delete attachment", file.id); + continue; + } + try { + await this.deleteMessage(message, event, team); + } catch (err) { + log.warn(`Failed to delete attachment ${file.id}:`, err); + } + } else { + log.warn(`Tried to remove tombstoned attachmend ${file.id} but we didn't find a Matrix event for it`); + } + } + } + // The substitutions might make the messages the same if (previousMessage === newMessage) { log.debug("Ignoring edit message because messages are the same post-substitutions."); @@ -1238,6 +1264,39 @@ export class BridgedRoom { this.recentSlackMessages.shift(); } } + + public async deleteMessage(msg: ISlackMessageEvent, event: EventEntry, team: TeamEntry): Promise { + const previousMessage = msg.previous_message; + if (!previousMessage) { + throw new Error(`Cannot delete message with no previous_message: ${JSON.stringify(msg)}`); + } + + // Try to determine the Matrix user responsible for deleting the message, fallback to our main bot if all else fails + if (!previousMessage.user) { + log.warn("We don't know the original sender of", previousMessage, "will try to remove with our bot"); + } + + const isOurMessage = previousMessage.subtype === 'bot_message' && (previousMessage.bot_id === team.bot_id); + + if (previousMessage.user && !isOurMessage) { + try { + const ghost = await this.main.ghostStore.get(previousMessage.user, previousMessage.team_domain, previousMessage.team); + await ghost.redactEvent(event.roomId, event.eventId); + return; + } catch (err) { + log.warn(`Failed to remove message on behalf of ${previousMessage.user}, falling back to our bot`); + } + } + + try { + const botClient = this.main.botIntent.matrixClient; + await botClient.redactEvent(event.roomId, event.eventId, "Deleted on Slack"); + } catch (err) { + throw new Error( + `Failed to remove message ${JSON.stringify(previousMessage)} with our Matrix bot. insufficient power level? Error: ${err}` + ); + } + } } /** diff --git a/src/SlackEventHandler.ts b/src/SlackEventHandler.ts index eeaa527f..e00c53eb 100644 --- a/src/SlackEventHandler.ts +++ b/src/SlackEventHandler.ts @@ -18,6 +18,7 @@ import { BaseSlackHandler, ISlackEvent, ISlackMessageEvent, ISlackUser } from ". import { BridgedRoom } from "./BridgedRoom"; import { Main, METRIC_RECEIVED_MESSAGE } from "./Main"; import { Logger } from "matrix-appservice-bridge"; +import { EventEntry, TeamEntry } from "./datastore/Models"; const log = new Logger("SlackEventHandler"); /** @@ -283,17 +284,18 @@ export class SlackEventHandler extends BaseSlackHandler { if (msg.message.bot_id !== undefined) { // Check the edit wasn't sent by us if (msg.message.bot_id === team.bot_id) { + log.debug('Ignoring a message_changed since it was sent by us'); return; } else { msg.user_id = msg.message.bot_id; } } } else if (msg.subtype === "message_deleted" && msg.deleted_ts) { - const originalEvent = await this.main.datastore.getEventBySlackId(msg.channel, msg.deleted_ts); - if (originalEvent) { - const botClient = this.main.botIntent.matrixClient; - await botClient.redactEvent(originalEvent.roomId, originalEvent.eventId); - return; + try { + const events = await this.main.datastore.getEventsBySlackId(msg.channel, msg.deleted_ts!); + await Promise.all(events.map(event => room.deleteMessage(msg, event, team))); + } catch (err) { + log.error(err); } // If we don't have the event throw Error("unknown_message"); diff --git a/src/SlackGhost.ts b/src/SlackGhost.ts index 9fd574e2..3c6c10b1 100644 --- a/src/SlackGhost.ts +++ b/src/SlackGhost.ts @@ -19,7 +19,7 @@ import * as Slackdown from "Slackdown"; import { ISlackUser } from "./BaseSlackHandler"; import { WebClient } from "@slack/web-api"; import { BotsInfoResponse, UsersInfoResponse } from "./SlackResponses"; -import { UserEntry, Datastore } from "./datastore/Models"; +import { UserEntry, Datastore, EventEntryExtra } from "./datastore/Models"; import axios from "axios"; const log = new Logger("SlackGhost"); @@ -321,6 +321,13 @@ export class SlackGhost { return Slackdown.parse(body); } + public async redactEvent(roomId: string, eventId: string) { + if (!this._intent) { + throw Error('No intent associated with ghost'); + } + await this._intent.matrixClient.redactEvent(roomId, eventId); + } + public async sendInThread(roomId: string, text: string, slackRoomId: string, slackEventTs: string, replyEvent: IMatrixReplyEvent): Promise { const content = { @@ -368,7 +375,13 @@ export class SlackGhost { await this.sendMessage(roomId, content, slackRoomID, slackEventTS); } - public async sendMessage(roomId: string, msg: Record, slackRoomId: string, slackEventTs: string): Promise<{event_id: string}> { + public async sendMessage( + roomId: string, + msg: Record, + slackRoomId: string, + slackEventTs: string, + eventExtras?: EventEntryExtra, + ): Promise<{event_id: string}> { if (!this._intent) { throw Error('No intent associated with ghost'); } @@ -383,6 +396,7 @@ export class SlackGhost { matrixEvent.event_id, slackRoomId, slackEventTs, + eventExtras, ); return { diff --git a/src/datastore/Models.ts b/src/datastore/Models.ts index 3d0f15b2..241a2429 100644 --- a/src/datastore/Models.ts +++ b/src/datastore/Models.ts @@ -50,6 +50,7 @@ export interface EventEntry { } export interface EventEntryExtra { + attachment_id?: string; slackThreadMessages?: string[]; } @@ -113,6 +114,7 @@ export interface Datastore extends ProvisioningStore { upsertEvent(roomId: string, eventId: string, channelId: string, ts: string, extras?: EventEntryExtra): Promise; upsertEvent(roomIdOrEntry: EventEntry): Promise; getEventByMatrixId(roomId: string, eventId: string): Promise; + getEventsBySlackId(channelId: string, ts: string): Promise; getEventBySlackId(channelId: string, ts: string): Promise; deleteEventByMatrixId(roomId: string, eventId: string): Promise; diff --git a/src/datastore/NedbDatastore.ts b/src/datastore/NedbDatastore.ts index bc2ebce0..3d0d9282 100644 --- a/src/datastore/NedbDatastore.ts +++ b/src/datastore/NedbDatastore.ts @@ -241,6 +241,10 @@ export class NedbDatastore implements Datastore { return this.storedEventToEventEntry(storedEvent); } + public async getEventsBySlackId(channelId: string, ts: string): Promise { + return this.getEventBySlackId(channelId, ts).then(e => e ? [e] : []); + } + public async getEventBySlackId(channelId: string, ts: string): Promise { const storedEvent = await this.eventStore.getEntryByRemoteId(channelId, ts); if (!storedEvent) { diff --git a/src/datastore/postgres/PgDatastore.ts b/src/datastore/postgres/PgDatastore.ts index 87663c89..bbe0ced9 100644 --- a/src/datastore/postgres/PgDatastore.ts +++ b/src/datastore/postgres/PgDatastore.ts @@ -64,7 +64,7 @@ export interface SchemaRunUserMessage { type SchemaRunFn = (db: IDatabase) => Promise; export class PgDatastore implements Datastore, ClientEncryptionStore, ProvisioningStore { - public static readonly LATEST_SCHEMA = 16; + public static readonly LATEST_SCHEMA = 17; public readonly postgresDb: IDatabase; constructor(connectionString: string) { @@ -178,17 +178,25 @@ export class PgDatastore implements Datastore, ClientEncryptionStore, Provisioni }); } + public async getEventsBySlackId(slackChannel: string, slackTs: string): Promise { + return this.postgresDb.manyOrNone( + "SELECT * FROM events WHERE slackChannel = ${slackChannel} AND slackTs = ${slackTs}", + { slackChannel, slackTs } + ).then(entries => entries.map(e => ({ + roomId: e.roomid, + eventId: e.eventid, + slackChannelId: slackChannel, + slackTs, + _extras: JSON.parse(e.extras) as EventEntryExtra, + }))); + } + + /** + * @deprecated One Slack event may map to many Matrix events -- use getEventsBySlackId() + */ public async getEventBySlackId(slackChannel: string, slackTs: string): Promise { - log.debug(`getEventBySlackId: ${slackChannel} ${slackTs}`); - return this.postgresDb.oneOrNone( - "SELECT * FROM events WHERE slackChannel = ${slackChannel} AND slackTs = ${slackTs} LIMIT 1", - { slackChannel, slackTs }, e => e && { - roomId: e.roomid, - eventId: e.eventid, - slackChannelId: slackChannel, - slackTs, - _extras: JSON.parse(e.extras), - }); + const events = await this.getEventsBySlackId(slackChannel, slackTs); + return events.find(e => !e._extras.attachment_id) ?? null; } public async deleteEventByMatrixId(roomId: string, eventId: string): Promise { diff --git a/src/datastore/postgres/schema/v17.ts b/src/datastore/postgres/schema/v17.ts new file mode 100644 index 00000000..d9b09b39 --- /dev/null +++ b/src/datastore/postgres/schema/v17.ts @@ -0,0 +1,11 @@ +import { IDatabase } from "pg-promise"; + +export const runSchema = async(db: IDatabase) => { + await db.none(` + ALTER TABLE events DROP CONSTRAINT cons_events_unique; + `); + + await db.none(` + ALTER TABLE events ADD CONSTRAINT cons_events_unique UNIQUE(eventid, roomid, slackchannel, slackts, extras); + `); +};