Merge branch 'develop' into release-1.0.0

changelog.d/235.feature

@@ -0,0 +1 @@
+Suppport puppeted reactions/redactions

changelog.d/236.bugfix

@@ -0,0 +1 @@
+Remove option slack_user_token on link command

changelog.d/237.bugfix

@@ -0,0 +1 @@
+Messages from puppeted accounts are no longer duplicated over the bridge

changelog.d/238.bugfix

@@ -0,0 +1 @@
+Do not send messages to slack with no content

src/AdminCommands.ts

@@ -198,9 +198,6 @@ export class AdminCommands {
                     alias: "t",
                     description: "Slack bot user token. Used with Slack bot user & Events api",
                 },
-                slack_user_token: {
-                    description: "Slack user token. Used to bridge files",
-                },
                 webhook_url: {
                     alias: "u",
                     description: "Slack webhook URL. Used with Slack outgoing hooks integration",

src/BridgedRoom.ts

@@ -22,7 +22,7 @@ import { default as substitutions, getFallbackForMissingEmoji, ISlackToMatrixRes
 import * as emoji from "node-emoji";
 import { ISlackMessageEvent, ISlackEvent } from "./BaseSlackHandler";
 import { WebClient } from "@slack/web-api";
-import { TeamInfoResponse, AuthTestResponse, UsersInfoResponse, ChatUpdateResponse,
+import { ChatUpdateResponse,
     ChatPostMessageResponse, ConversationsInfoResponse } from "./SlackResponses";
 import { RoomEntry, EventEntry, TeamEntry } from "./datastore/Models";
 
@@ -47,6 +47,9 @@ interface ISlackChatMessagePayload extends ISlackToMatrixResult {
     icon_url?: string;
 }
 
+const RECENT_MESSAGE_MAX = 10;
+const PUPPET_INCOMING_DELAY_MS = 1500;
+
 export class BridgedRoom {
     public get isDirty() {
         return this.dirty;
@@ -142,6 +145,7 @@ export class BridgedRoom {
     private intent: Intent;
     // Is the matrix room in use by the bridge.
     public MatrixRoomActive: boolean;
+    private recentSlackMessages: string[] = [];
 
     /**
      * True if this instance has changed from the version last read/written to the RoomStore.
@@ -251,15 +255,24 @@ export class BridgedRoom {
                 emojiKeyName = emojiKeyName.substring(1, emojiKeyName.length - 1);
             }
         }
+        let client: WebClient = this.botClient;
+        const puppet = await this.main.clientFactory.getClientForUserWithId(this.SlackTeamId!, message.sender);
+        if (puppet) {
+            client = puppet.client;
+            // We must do this before sending to avoid racing
+            // Use the unicode key for uniqueness
+            this.addRecentSlackMessage(`reactadd:${relatesTo.key}:${puppet.id}:${event.slackTs}`);
+        }
 
-        // TODO: This only works once from matrix as we are sending the event as the
+        // TODO: This only works once from matrix if we are sending the event as the
         // bot user.
-        const res = await this.botClient.reactions.add({
+        const res = await client.reactions.add({
             as_user: false,
             channel: this.slackChannelId,
             name: emojiKeyName,
             timestamp: event.slackTs,
         });
+        log.info(`Reaction :${emojiKeyName}: added to ${event.slackTs}`);
 
         if (!res.ok) {
             log.error("HTTP Error: ", res);
@@ -280,7 +293,8 @@ export class BridgedRoom {
             return;
         }
 
-        const res = await this.botClient.chat.delete({
+        const client = (await this.main.clientFactory.getClientForUser(this.SlackTeamId!, message.sender)) || this.botClient;
+        const res = await client.chat.delete({
             as_user: false,
             channel: this.slackChannelId!,
             ts: event.slackTs,
@@ -345,6 +359,11 @@ export class BridgedRoom {
             username: user.getDisplaynameForRoom(message.room_id) || matrixToSlackResult.username,
         };
 
+        if (!body.attachments && !body.text) {
+            // The message type might not be understood. In any case, we can't send something without
+            // text.
+            return;
+        }
         const reply = await this.findParentReply(message);
         let parentStoredEvent: EventEntry | null = null;
         if (reply !== message.event_id) {
@@ -388,6 +407,8 @@ export class BridgedRoom {
             channel: this.slackChannelId!,
         })) as ChatPostMessageResponse;
 
+        this.addRecentSlackMessage(res.ts);
+
         this.main.incCounter(METRIC_SENT_MESSAGES, {side: "remote"});
 
         if (!res.ok) {
@@ -414,6 +435,19 @@ export class BridgedRoom {
     }
 
     public async onSlackMessage(message: ISlackMessageEvent, content?: Buffer) {
+        if (this.slackTeamId && message.user) {
+            // This just checks if the user *could* be puppeted. If they are, delay handling their incoming messages.
+            const hasPuppet = null !== await this.main.datastore.getPuppetTokenBySlackId(this.slackTeamId, message.user);
+            if (hasPuppet) {
+                await new Promise((r) => setTimeout(r, PUPPET_INCOMING_DELAY_MS));
+            }
+        }
+        if (this.recentSlackMessages.includes(message.ts)) {
+            // We sent this, ignore.
+            return;
+        }
+        // Dedupe across RTM/Event streams
+        this.addRecentSlackMessage(message.ts);
         try {
             const ghost = await this.main.getGhostForSlackMessage(message, this.slackTeamId!);
             await ghost.update(message, this);
@@ -429,18 +463,23 @@ export class BridgedRoom {
         if (message.user_id === this.team!.user_id) {
             return;
         }
-        const ghost = await this.main.getGhostForSlackMessage(message, teamId);
-        await ghost.update(message, this);
 
         const reaction = `:${message.reaction}:`;
         const reactionKey = emoji.emojify(reaction, getFallbackForMissingEmoji);
 
+        if (this.recentSlackMessages.includes(`reactadd:${reactionKey}:${message.user_id}:${message.item.ts}`)) {
+            // We sent this, ignore.
+            return;
+        }
+        const ghost = await this.main.getGhostForSlackMessage(message, teamId);
+        await ghost.update(message, this);
+
         const event = await this.main.datastore.getEventBySlackId(message.item.channel, message.item.ts);
 
         if (event === null) {
             return;
         }
-
+        log.debug(`Sending reaction ${reactionKey} for ${event.eventId} as ${ghost.userId}`);
         return ghost.sendReaction(this.MatrixRoomId, event.eventId, reactionKey,
                                   message.item.channel, message.event_ts);
     }
@@ -743,6 +782,14 @@ export class BridgedRoom {
         this.intent =  this.main.getIntent(firstGhost);
         return this.intent;
     }
+
+    private addRecentSlackMessage(ts: string) {
+        log.debug("Recent message key add:", ts);
+        this.recentSlackMessages.push(ts);
+        if (this.recentSlackMessages.length > RECENT_MESSAGE_MAX) {
+            this.recentSlackMessages.shift();
+        }
+    }
 }
 
 /**

src/Main.ts

@@ -782,11 +782,10 @@ export class Main {
         const joinedRooms = await roomListPromise;
         await Promise.all(entries.map(async (entry) => {
             // If we aren't in the room, mark as inactive until we get re-invited.
-            const activeRoom = joinedRooms.includes(entry.matrix_id);
+            const activeRoom = entry.remote.puppet_owner !== undefined || joinedRooms.includes(entry.matrix_id);
             if (!activeRoom) {
                 log.warn(`${entry.matrix_id} marked as inactive, bot is not joined to room`);
             }
-
             const teamId = entry.remote.slack_team_id;
             const teamEntry = teamId ? await this.datastore.getTeam(teamId) || undefined : undefined;
             let slackClient: WebClient|null = null;

src/SlackClientFactory.ts

@@ -17,8 +17,8 @@ interface RequiredConfigOptions {
 
 export class SlackClientFactory {
     private teamClients: Map<string, WebClient> = new Map();
-    private puppets: Map<string, WebClient> = new Map();
-    constructor(private datastore: Datastore, private config: RequiredConfigOptions, private onRemoteCall: (method: string) => void) {
+    private puppets: Map<string, {client: WebClient, id: string}> = new Map();
+    constructor(private datastore: Datastore, private config?: RequiredConfigOptions, private onRemoteCall?: (method: string) => void) {
 
     }
 
@@ -119,7 +119,7 @@ export class SlackClientFactory {
         return teamRes!.id;
     }
 
-    public async getClientForUser(teamId: string, matrixUser: string): Promise<WebClient|null> {
+    public async getClientForUserWithId(teamId: string, matrixUser: string): Promise<{client: WebClient, id: string}|null> {
         const key = `${teamId}:${matrixUser}`;
         if (this.puppets.has(key)) {
             return this.puppets.get(key) || null;
@@ -129,25 +129,34 @@ export class SlackClientFactory {
             return null;
         }
         const client = new WebClient(token);
+        let id: string;
         try {
-            await client.auth.test();
+            const res = (await client.auth.test()) as AuthTestResponse;
+            id = res.user_id;
         } catch (ex) {
             log.warn("Failed to auth puppeted client for user:", ex);
             return null;
         }
-        this.puppets.set(key, client);
-        return client;
+        this.puppets.set(key, {id, client});
+        return {id, client};
+    }
+
+    public async getClientForUser(teamId: string, matrixUser: string): Promise<WebClient|null> {
+        const res = await this.getClientForUserWithId(teamId, matrixUser);
+        return res !== null ? res.client : null;
     }
 
     private async createTeamClient(token: string) {
-        const opts = this.config.slack_client_opts;
+        const opts = this.config ? this.config.slack_client_opts : undefined;
         const slackClient = new WebClient(token, {
             logger: {
                 setLevel: () => {}, // We don't care about these.
                 setName: () => {},
                 debug: (msg: string) => {
                     // non-ideal way to detect calls to slack.
-                    const match = /apiCall\('([\w\.]+)'\) start/.exec(msg);
+                    webLog.debug.bind(webLog);
+                    if (!this.onRemoteCall) { return; }
+                    const match = /apiCall\('([\w\.]+)'\) start/.exec(msg[0]);
                     if (match && match[1]) {
                         this.onRemoteCall(match[1]);
                     }

src/datastore/Models.ts

@@ -26,7 +26,7 @@ export interface RoomEntry {
         slack_type?: string;
         id: string;
         name: string;
-        webhook_uri: string;
+        webhook_uri?: string;
         slack_private?: boolean;
         puppet_owner?: string;
     };