Filter out rooms where user was never a member when redacting rooms (#…

…551) * check if user was member of protected room before pulling all the messages * move matcher into branch Co-authored-by: Travis Ralston <travisr@matrix.org> * add join->message->leave->ban test * Update test/integration/commands/redactCommandTest.ts Co-authored-by: Travis Ralston <travisr@matrix.org> --------- Co-authored-by: Travis Ralston <travisr@matrix.org>
matrix-org · Oct 22, 2024 · e1dcfc2 · e1dcfc2
1 parent 93282da
commit e1dcfc2
Show file tree

Hide file tree

Showing 2 changed files with 76 additions and 15 deletions.
diff --git a/src/utils.ts b/src/utils.ts
@@ -140,6 +140,46 @@ async function botRedactUserMessagesIn(
     }
 }
 
+/**
+ * Match a user ID or glob against a target ID
+ * @param userId - user ID or glob to test
+ * @param targetId - user ID to test against
+ * @param isGlob - whether the targetID is a glob
+ */
+function matchUserId(userId: string, targetId: string, isGlob: boolean): boolean {
+    if (isGlob) {
+        const matcher = new MatrixGlob(targetId);
+        return matcher.test(userId);
+    } else {
+        return userId === targetId;
+    }
+}
+
+/**
+ * Filter out any rooms the user to redact was never in
+ * @param targetRooms - list of rooms to check membership in
+ * @param user - user ID or glob to check the membership of
+ * @param isGlob - whether the user ID is a glob
+ * @param client - Matrix client
+ */
+export async function filterRooms(
+    targetRooms: string[],
+    user: string,
+    isGlob: boolean,
+    client: MatrixSendClient,
+): Promise<string[]> {
+    let filteredRooms: string[] = [];
+    for (const room of targetRooms) {
+        const chunk = await client.getRoomMembers(room);
+        for (const event of chunk) {
+            if (matchUserId(event["stateKey"], user, isGlob)) {
+                filteredRooms.push(room);
+            }
+        }
+    }
+    return filteredRooms;
+}
+
 /**
  * Redact a user's messages in a set of rooms.
  * See `getMessagesByUserIn`.
@@ -164,10 +204,12 @@ export async function redactUserMessagesIn(
     noop = false,
 ) {
     const usingGlob = userIdOrGlob.includes("*");
+    const filteredRooms = await filterRooms(targetRoomIds, userIdOrGlob, usingGlob, client);
+
     // if admin use the Admin API, but admin endpoint does not support globs
     if (isAdmin && !usingGlob) {
         try {
-            await adminRedactUserMessagesIn(client, managementRoom, userIdOrGlob, targetRoomIds, limit);
+            await adminRedactUserMessagesIn(client, managementRoom, userIdOrGlob, filteredRooms, limit);
         } catch (e) {
             LogService.error(
                 "utils#redactUserMessagesIn",
@@ -179,10 +221,10 @@ export async function redactUserMessagesIn(
                 `Error using admin API to redact messages for user ${userIdOrGlob}, please check logs for more info - falling
             back to non-admin redaction process.`,
             );
-            await botRedactUserMessagesIn(client, managementRoom, userIdOrGlob, targetRoomIds, limit, noop);
+            await botRedactUserMessagesIn(client, managementRoom, userIdOrGlob, filteredRooms, limit, noop);
         }
     } else {
-        await botRedactUserMessagesIn(client, managementRoom, userIdOrGlob, targetRoomIds, limit, noop);
+        await botRedactUserMessagesIn(client, managementRoom, userIdOrGlob, filteredRooms, limit, noop);
     }
 }
 
@@ -215,16 +257,6 @@ export async function getMessagesByUserIn(
         ...(isGlob ? {} : { senders: [sender] }),
     };
 
-    const matcher = new MatrixGlob(sender);
-
-    function testUser(userId: string): boolean {
-        if (isGlob) {
-            return matcher.test(userId);
-        } else {
-            return userId === sender;
-        }
-    }
-
     /**
      * The response returned from `backfill`
      * See https://spec.matrix.org/latest/client-server-api/#get_matrixclientv3roomsroomidmessages
@@ -267,7 +299,7 @@ export async function getMessagesByUserIn(
             if (processed >= limit) return messages; // we have provided enough events.
             processed++;
 
-            if (testUser(event["sender"])) messages.push(event);
+            if (matchUserId(event["sender"], sender, isGlob)) messages.push(event);
         }
         return messages;
     }

diff --git a/test/integration/commands/redactCommandTest.ts b/test/integration/commands/redactCommandTest.ts
@@ -1,7 +1,7 @@
 import { strict as assert } from "assert";
 
 import { newTestUser } from "../clientHelper";
-import { getMessagesByUserIn } from "../../../src/utils";
+import { getMessagesByUserIn, filterRooms } from "../../../src/utils";
 import { LogService } from "@vector-im/matrix-bot-sdk";
 import { getFirstReaction } from "./commandUtils";
 import { SynapseAdminApis } from "@vector-im/matrix-bot-sdk";
@@ -413,4 +413,33 @@ describe("Test: The redaction command - if not admin", function () {
             throw new Error(`Error restoring mjolnir to admin.`);
         }
     });
+
+    it("Correctly tracks room membership of redactee", async function () {
+        this.timeout(60000);
+        let badUser = await newTestUser(this.config.homeserverUrl, { name: { contains: "spammer" } });
+        let moderator = await newTestUser(this.config.homeserverUrl, { name: { contains: "moderator" } });
+        this.moderator = moderator;
+        const mjolnir = this.config.RUNTIME.client!;
+        let mjolnirUserId = await mjolnir.getUserId();
+        const badUserId = await badUser.getUserId();
+
+        await moderator.joinRoom(this.config.managementRoom);
+        let targetRoom = await moderator.createRoom({ invite: [await badUser.getUserId(), mjolnirUserId] });
+        await badUser.joinRoom(targetRoom);
+        await badUser.createRoom(); // create a room that Mjolnir won't have any interest in
+
+        // send a message, leave, then get banned
+        badUser.sendMessage(targetRoom, {
+            msgtype: "m.text.",
+            body: `a bad message`,
+        });
+        badUser.leaveRoom(targetRoom);
+        await moderator.banUser(badUserId, targetRoom, "spam");
+
+        // check that filterRooms tracks that badUser was in target room, and doesn't pick up other room badUser
+        // is in
+        const rooms = await filterRooms([targetRoom], badUserId, false, moderator);
+        assert.equal(rooms.length, 1);
+        assert.equal(rooms[0], targetRoom);
+    });
 });