Prioritize bans over redactions, and queue redactions faster

This could do with some cleanup, particularly around the part where it uses a callback.
matrix-org · Jun 12, 2020 · 635f9ba · 635f9ba
1 parent 4751b09
commit 635f9ba
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 34 deletions.
diff --git a/src/actions/ApplyBan.ts b/src/actions/ApplyBan.ts
@@ -68,12 +68,10 @@ export async function applyUserBans(lists: BanList[], roomIds: string[], mjolnir
                             await logMessage(LogLevel.INFO, "ApplyBan", `Banning ${member.userId} in ${roomId} for: ${userRule.reason}`, roomId);
 
                             if (!config.noop) {
-                                // Always prioritize redactions above bans
+                                await mjolnir.client.banUser(member.userId, roomId, userRule.reason);
                                 if (mjolnir.automaticRedactGlobs.find(g => g.test(userRule.reason.toLowerCase()))) {
                                     await redactUserMessagesIn(mjolnir.client, member.userId, [roomId]);
                                 }
-
-                                await mjolnir.client.banUser(member.userId, roomId, userRule.reason);
                             } else {
                                 await logMessage(LogLevel.WARN, "ApplyBan", `Tried to ban ${member.userId} in ${roomId} but Mjolnir is running in no-op mode`, roomId);
                             }

diff --git a/src/protections/BasicFlooding.ts b/src/protections/BasicFlooding.ts
@@ -57,7 +57,12 @@ export class BasicFlooding implements IProtection {
         }
 
         if (messageCount >= MAX_PER_MINUTE) {
-            // Prioritize redaction over ban - we can always keep redacting what the user said.
+            await logMessage(LogLevel.WARN, "BasicFlooding", `Banning ${event['sender']} in ${roomId} for flooding (${messageCount} messages in the last minute)`, roomId);
+            if (!config.noop) {
+                await mjolnir.client.banUser(event['sender'], roomId, "spam");
+            } else {
+                await logMessage(LogLevel.WARN, "BasicFlooding", `Tried to ban ${event['sender']} in ${roomId} but Mjolnir is running in no-op mode`, roomId);
+            }
 
             if (this.recentlyBanned.includes(event['sender'])) return; // already handled (will be redacted)
             mjolnir.redactionHandler.addUser(event['sender']);
@@ -72,13 +77,6 @@ export class BasicFlooding implements IProtection {
                 await logMessage(LogLevel.WARN, "BasicFlooding", `Tried to redact messages for ${event['sender']} in ${roomId} but Mjolnir is running in no-op mode`, roomId);
             }
 
-            await logMessage(LogLevel.WARN, "BasicFlooding", `Banning ${event['sender']} in ${roomId} for flooding (${messageCount} messages in the last minute)`, roomId);
-            if (!config.noop) {
-                await mjolnir.client.banUser(event['sender'], roomId, "spam");
-            } else {
-                await logMessage(LogLevel.WARN, "BasicFlooding", `Tried to ban ${event['sender']} in ${roomId} but Mjolnir is running in no-op mode`, roomId);
-            }
-
             // Free up some memory now that we're ready to handle it elsewhere
             forUser = forRoom[event['sender']] = []; // reset the user's list
         }

diff --git a/src/protections/FirstMessageIsImage.ts b/src/protections/FirstMessageIsImage.ts
@@ -51,7 +51,12 @@ export class FirstMessageIsImage implements IProtection {
             const formattedBody = content['formatted_body'] || '';
             const isMedia = msgtype === 'm.image' || msgtype === 'm.video' || formattedBody.toLowerCase().includes('<img');
             if (isMedia && this.justJoined[roomId].includes(event['sender'])) {
-                // Prioritize redaction over ban because we can always keep redacting the user's messages
+                await logMessage(LogLevel.WARN, "FirstMessageIsImage", `Banning ${event['sender']} for posting an image as the first thing after joining in ${roomId}.`);
+                if (!config.noop) {
+                    await mjolnir.client.banUser(event['sender'], roomId, "spam");
+                } else {
+                    await logMessage(LogLevel.WARN, "FirstMessageIsImage", `Tried to ban ${event['sender']} in ${roomId} but Mjolnir is running in no-op mode`, roomId);
+                }
 
                 if (this.recentlyBanned.includes(event['sender'])) return; // already handled (will be redacted)
                 mjolnir.redactionHandler.addUser(event['sender']);
@@ -63,13 +68,6 @@ export class FirstMessageIsImage implements IProtection {
                 } else {
                     await logMessage(LogLevel.WARN, "FirstMessageIsImage", `Tried to redact ${event['event_id']} in ${roomId} but Mjolnir is running in no-op mode`, roomId);
                 }
-
-                await logMessage(LogLevel.WARN, "FirstMessageIsImage", `Banning ${event['sender']} for posting an image as the first thing after joining in ${roomId}.`);
-                if (!config.noop) {
-                    await mjolnir.client.banUser(event['sender'], roomId, "spam");
-                } else {
-                    await logMessage(LogLevel.WARN, "FirstMessageIsImage", `Tried to ban ${event['sender']} in ${roomId} but Mjolnir is running in no-op mode`, roomId);
-                }
             }
         }
 

diff --git a/src/utils.ts b/src/utils.ts
@@ -51,15 +51,16 @@ export async function redactUserMessagesIn(client: MatrixClient, userIdOrGlob: s
     for (const targetRoomId of targetRoomIds) {
         await logMessage(LogLevel.DEBUG, "utils#redactUserMessagesIn", `Fetching sent messages for ${userIdOrGlob} in ${targetRoomId} to redact...`, targetRoomId);
 
-        const eventsToRedact = await getMessagesByUserIn(client, userIdOrGlob, targetRoomId, limit);
-        for (const victimEvent of eventsToRedact) {
-            await logMessage(LogLevel.DEBUG, "utils#redactUserMessagesIn", `Redacting ${victimEvent['event_id']} in ${targetRoomId}`, targetRoomId);
-            if (!config.noop) {
-                await client.redactEvent(targetRoomId, victimEvent['event_id']);
-            } else {
-                await logMessage(LogLevel.WARN, "utils#redactUserMessagesIn", `Tried to redact ${victimEvent['event_id']} in ${targetRoomId} but Mjolnir is running in no-op mode`, targetRoomId);
+        await getMessagesByUserIn(client, userIdOrGlob, targetRoomId, limit, async (eventsToRedact) => {
+            for (const victimEvent of eventsToRedact) {
+                await logMessage(LogLevel.DEBUG, "utils#redactUserMessagesIn", `Redacting ${victimEvent['event_id']} in ${targetRoomId}`, targetRoomId);
+                if (!config.noop) {
+                    await client.redactEvent(targetRoomId, victimEvent['event_id']);
+                } else {
+                    await logMessage(LogLevel.WARN, "utils#redactUserMessagesIn", `Tried to redact ${victimEvent['event_id']} in ${targetRoomId} but Mjolnir is running in no-op mode`, targetRoomId);
+                }
             }
-        }
+        });
     }
 }
 
@@ -70,9 +71,10 @@ export async function redactUserMessagesIn(client: MatrixClient, userIdOrGlob: s
  * @param {string} sender The sender. Can include wildcards to match multiple people.
  * @param {string} roomId The room ID to search in.
  * @param {number} limit The maximum number of messages to search. Defaults to 1000.
- * @returns {Promise<any>} Resolves to the events sent by the user(s) prior to join.
+ * @param {function} cb Callback function to handle the events as they are received.
+ * @returns {Promise<any>} Resolves when complete.
  */
-export async function getMessagesByUserIn(client: MatrixClient, sender: string, roomId: string, limit: number): Promise<any[]> {
+export async function getMessagesByUserIn(client: MatrixClient, sender: string, roomId: string, limit: number, cb: (events: any[]) => void): Promise<any> {
     const filter = {
         room: {
             rooms: [roomId],
@@ -140,16 +142,16 @@ export async function getMessagesByUserIn(client: MatrixClient, sender: string,
     const response = await initialSync();
     if (!response) return [];
 
-    const messages = [];
     let processed = 0;
 
     const timeline = (((response['rooms'] || {})['join'] || {})[roomId] || {})['timeline'] || {};
     const syncedMessages = timeline['events'] || [];
     let token = timeline['prev_batch'] || response['next_batch'];
     let bfMessages = {chunk: syncedMessages, end: token};
     do {
+        const messages = [];
         for (const event of (bfMessages['chunk'] || [])) {
-            if (processed >= limit) return messages; // we're done even if we don't want to be
+            if (processed >= limit) return; // we're done even if we don't want to be
             processed++;
 
             if (testUser(event['sender'])) messages.push(event);
@@ -161,12 +163,13 @@ export async function getMessagesByUserIn(client: MatrixClient, sender: string,
             token = bfMessages['end'];
             if (lastToken === token) {
                 LogService.warn("utils", "Backfill returned same end token - returning");
-                return messages;
+                cb(messages);
+                return;
             }
         }
-    } while (token);
 
-    return messages;
+        cb(messages);
+    } while (token);
 }
 
 export async function replaceRoomIdsWithPills(client: MatrixClient, text: string, roomIds: string[] | string, msgtype: MessageType = "m.text"): Promise<TextualMessageEventContent> {