From e1dcfc2a63d4f116b7ba2b03fc0dd3c93ae1fe9d Mon Sep 17 00:00:00 2001 From: Shay Date: Tue, 22 Oct 2024 14:31:21 -0700 Subject: [PATCH] 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 * add join->message->leave->ban test * Update test/integration/commands/redactCommandTest.ts Co-authored-by: Travis Ralston --------- Co-authored-by: Travis Ralston --- src/utils.ts | 60 ++++++++++++++----- .../integration/commands/redactCommandTest.ts | 31 +++++++++- 2 files changed, 76 insertions(+), 15 deletions(-) diff --git a/src/utils.ts b/src/utils.ts index fb480a24..05893a34 100644 --- 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 { + 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 index e0d20afb..aa2da561 100644 --- 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); + }); });