From c5b5026d4d69edb99fc938a2af3fe302adfc3fb5 Mon Sep 17 00:00:00 2001 From: gnuxie Date: Tue, 14 Sep 2021 12:17:29 +0100 Subject: [PATCH 1/5] WIP: Redact events after all bans have been applied. --- src/Mjolnir.ts | 25 ++++++- src/actions/ApplyBan.ts | 3 +- src/queues/AutomaticRedactionQueue.ts | 5 +- src/queues/EventRedactionQueue.ts | 97 +++++++++++++++++++++++++++ 4 files changed, 124 insertions(+), 6 deletions(-) create mode 100644 src/queues/EventRedactionQueue.ts diff --git a/src/Mjolnir.ts b/src/Mjolnir.ts index b134afd9..626c41a9 100644 --- a/src/Mjolnir.ts +++ b/src/Mjolnir.ts @@ -36,6 +36,7 @@ import { IProtection } from "./protections/IProtection"; import { PROTECTIONS } from "./protections/protections"; import { AutomaticRedactionQueue } from "./queues/AutomaticRedactionQueue"; import { Healthz } from "./health/healthz"; +import { EventRedactionQueue, RedactUserInRoom } from "./queues/EventRedactionQueue"; export const STATE_NOT_STARTED = "not_started"; export const STATE_CHECKING_PERMISSIONS = "checking_permissions"; @@ -53,7 +54,8 @@ export class Mjolnir { private localpart: string; private currentState: string = STATE_NOT_STARTED; private protections: IProtection[] = []; - private redactionQueue = new AutomaticRedactionQueue(); + private spamRedactionQueue = new AutomaticRedactionQueue(); + private eventRedactionQueue = new EventRedactionQueue(); private automaticRedactionReasons: MatrixGlob[] = []; private protectedJoinedRoomIds: string[] = []; private explicitlyProtectedRoomIds: string[] = []; @@ -139,7 +141,7 @@ export class Mjolnir { } public get redactionHandler(): AutomaticRedactionQueue { - return this.redactionQueue; + return this.spamRedactionQueue; } public get automaticRedactGlobs(): MatrixGlob[] { @@ -493,8 +495,10 @@ export class Mjolnir { const aclErrors = await applyServerAcls(this.banLists, Object.keys(this.protectedRooms), this); const banErrors = await applyUserBans(this.banLists, Object.keys(this.protectedRooms), this); + const redactionErrors = await this.processRedactionQueue(); hadErrors = hadErrors || await this.printActionResult(aclErrors, "Errors updating server ACLs:"); hadErrors = hadErrors || await this.printActionResult(banErrors, "Errors updating member bans:"); + hadErrors = hadErrors || await this.printActionResult(redactionErrors, "Error updating redactions:"); if (!hadErrors && verbose) { const html = `Done updating rooms - no errors`; @@ -521,8 +525,10 @@ export class Mjolnir { const aclErrors = await applyServerAcls(this.banLists, Object.keys(this.protectedRooms), this); const banErrors = await applyUserBans(this.banLists, Object.keys(this.protectedRooms), this); + const redactionErrors = await this.processRedactionQueue(); hadErrors = hadErrors || await this.printActionResult(aclErrors, "Errors updating server ACLs:"); hadErrors = hadErrors || await this.printActionResult(banErrors, "Errors updating member bans:"); + hadErrors = hadErrors || await this.printActionResult(redactionErrors, "Error updating redactions:"); if (!hadErrors) { const html = `Done updating rooms - no errors`; @@ -575,7 +581,7 @@ export class Mjolnir { // Run the event handlers - we always run this after protections so that the protections // can flag the event for redaction. - await this.redactionQueue.handleEvent(roomId, event, this.client); + await this.spamRedactionQueue.handleEvent(roomId, event, this.client); if (event['type'] === 'm.room.power_levels' && event['state_key'] === '') { // power levels were updated - recheck permissions @@ -590,6 +596,8 @@ export class Mjolnir { } else if (event['type'] === "m.room.member") { // Only apply bans in the room we're looking at. const errors = await applyUserBans(this.banLists, [roomId], this); + // do we need room scoped redaction here? yes... + // I want to get an inital review before i check this bit. await this.printActionResult(errors); } } @@ -658,4 +666,15 @@ export class Mjolnir { message: message /* If `undefined`, we'll use Synapse's default message. */ }); } + + // This naming is horrible and clashes with the other redaction queue which isn't + // really the same thing. The old one is more about an ongoing user who we haven't + // banned, whereas this one is about redaction of users who aren't active. + public queueRedactUserMessagesIn(userId: string, roomId: string) { + this.eventRedactionQueue.add(new RedactUserInRoom(userId, roomId)); + } + + public async processRedactionQueue() { + return await this.eventRedactionQueue.process(this.client); + } } diff --git a/src/actions/ApplyBan.ts b/src/actions/ApplyBan.ts index 77317678..b7922adf 100644 --- a/src/actions/ApplyBan.ts +++ b/src/actions/ApplyBan.ts @@ -21,7 +21,6 @@ import config from "../config"; import { logMessage } from "../LogProxy"; import { LogLevel } from "matrix-bot-sdk"; import { ERROR_KIND_FATAL, ERROR_KIND_PERMISSION } from "../ErrorCache"; -import { redactUserMessagesIn } from "../utils"; /** * Applies the member bans represented by the ban lists to the provided rooms, returning the @@ -69,7 +68,7 @@ export async function applyUserBans(lists: BanList[], roomIds: string[], mjolnir if (!config.noop) { 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]); + mjolnir.queueRedactUserMessagesIn(member.userId, roomId); } } 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/queues/AutomaticRedactionQueue.ts b/src/queues/AutomaticRedactionQueue.ts index a2a4efb2..63a54747 100644 --- a/src/queues/AutomaticRedactionQueue.ts +++ b/src/queues/AutomaticRedactionQueue.ts @@ -13,7 +13,10 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. */ - +//// NOTE: This is a queue of users whose events should be redacted +////////// Not a queue of events to be redacted. +////////// This is also unrelated to the AutomaticRedactionReasons. +////////// It is as of writing only used by the flood/spam protections. import { extractRequestError, LogLevel, LogService, MatrixClient, Permalinks } from "matrix-bot-sdk"; import { logMessage } from "../LogProxy"; import config from "../config"; diff --git a/src/queues/EventRedactionQueue.ts b/src/queues/EventRedactionQueue.ts new file mode 100644 index 00000000..b6ac9e90 --- /dev/null +++ b/src/queues/EventRedactionQueue.ts @@ -0,0 +1,97 @@ +/* +Copyright 2019-2021 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +//// NOTE: This is a queue for events so that other protections can happen first (bans and ACL) + +import { LogLevel, MatrixClient } from "matrix-bot-sdk" +import { ERROR_KIND_FATAL } from "../ErrorCache"; +import { logMessage } from "../LogProxy"; +import { RoomUpdateError } from "../models/RoomUpdateError"; +import { redactUserMessagesIn } from "../utils"; + +export interface QueuedRedaction { + redact(client: MatrixClient): Promise + redactionEqual(redaction: QueuedRedaction): boolean + report(e): RoomUpdateError +} + +export class RedactUserInRoom implements QueuedRedaction { + userId: string; + roomId: string; + + constructor(userId: string, roomId: string) { + this.userId = userId; + this.roomId = roomId; + } + + public async redact(client: MatrixClient) { + await logMessage(LogLevel.DEBUG, "Mjolnir", `Redacting events from ${this.userId} in room ${this.roomId}.`); + await redactUserMessagesIn(client, this.userId, [this.roomId]); + } + + public redactionEqual(redaction: QueuedRedaction): boolean { + if (redaction instanceof RedactUserInRoom) { + return redaction.userId === this.userId && redaction.roomId === this.roomId; + } else { + return false; + } + } + + public report(e): RoomUpdateError { + const message = e.message || (e.body ? e.body.error : ''); + return { + roomId: this.roomId, + errorMessage: message, + errorKind: ERROR_KIND_FATAL, + }; + } +} + +export class EventRedactionQueue { + private toRedact: Array = new Array(); + + public has(redaction: QueuedRedaction) { + return this.toRedact.find(r => r.redactionEqual(redaction)); + } + + public add(redaction: QueuedRedaction) { + if (this.has(redaction)) { + return; + } else { + this.toRedact.push(redaction); + } + } + + public delete(redaction: QueuedRedaction) { + this.toRedact = this.toRedact.filter(r => r.redactionEqual(redaction)); + } + + public async process(client: MatrixClient): Promise { + const errors: RoomUpdateError[]= []; + // need to change this so it pops the array until empty + // otherwise this will be cringe. + for (const redaction of this.toRedact) { + try { + await redaction.redact(client); + } catch (e) { + errors.push(redaction.report(e)); + } finally { + // FIXME: Need to figure out in which circumstances we want to retry. + this.delete(redaction); + } + } + return errors; + } +} \ No newline at end of file From c949d26582522ad67fb934031fee7c38f52c8b7c Mon Sep 17 00:00:00 2001 From: gnuxie Date: Tue, 14 Sep 2021 12:34:46 +0100 Subject: [PATCH 2/5] When checking a member change, process redactions for that room. --- src/Mjolnir.ts | 12 ++++++------ src/queues/EventRedactionQueue.ts | 30 ++++++++++++++++++------------ 2 files changed, 24 insertions(+), 18 deletions(-) diff --git a/src/Mjolnir.ts b/src/Mjolnir.ts index 626c41a9..e02eec88 100644 --- a/src/Mjolnir.ts +++ b/src/Mjolnir.ts @@ -595,10 +595,10 @@ export class Mjolnir { return; } else if (event['type'] === "m.room.member") { // Only apply bans in the room we're looking at. - const errors = await applyUserBans(this.banLists, [roomId], this); - // do we need room scoped redaction here? yes... - // I want to get an inital review before i check this bit. - await this.printActionResult(errors); + const banErrors = await applyUserBans(this.banLists, [roomId], this); + const redactionErrors = await this.processRedactionQueue(roomId); + await this.printActionResult(banErrors); + await this.printActionResult(redactionErrors); } } } @@ -674,7 +674,7 @@ export class Mjolnir { this.eventRedactionQueue.add(new RedactUserInRoom(userId, roomId)); } - public async processRedactionQueue() { - return await this.eventRedactionQueue.process(this.client); + public async processRedactionQueue(roomId?: string) { + return await this.eventRedactionQueue.process(this.client, roomId); } } diff --git a/src/queues/EventRedactionQueue.ts b/src/queues/EventRedactionQueue.ts index b6ac9e90..a263e824 100644 --- a/src/queues/EventRedactionQueue.ts +++ b/src/queues/EventRedactionQueue.ts @@ -13,8 +13,6 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. */ -//// NOTE: This is a queue for events so that other protections can happen first (bans and ACL) - import { LogLevel, MatrixClient } from "matrix-bot-sdk" import { ERROR_KIND_FATAL } from "../ErrorCache"; import { logMessage } from "../LogProxy"; @@ -22,9 +20,10 @@ import { RoomUpdateError } from "../models/RoomUpdateError"; import { redactUserMessagesIn } from "../utils"; export interface QueuedRedaction { + roomId: string; // The room which the redaction will take place in. redact(client: MatrixClient): Promise redactionEqual(redaction: QueuedRedaction): boolean - report(e): RoomUpdateError + report(e: any): RoomUpdateError } export class RedactUserInRoom implements QueuedRedaction { @@ -43,7 +42,7 @@ export class RedactUserInRoom implements QueuedRedaction { public redactionEqual(redaction: QueuedRedaction): boolean { if (redaction instanceof RedactUserInRoom) { - return redaction.userId === this.userId && redaction.roomId === this.roomId; + return redaction.userId === this.userId && redaction.roomId === this.roomId; } else { return false; } @@ -58,7 +57,9 @@ export class RedactUserInRoom implements QueuedRedaction { }; } } - +/** + * This is a queue for events so that other protections can happen first (e.g. applying room bans to every room). + */ export class EventRedactionQueue { private toRedact: Array = new Array(); @@ -78,20 +79,25 @@ export class EventRedactionQueue { this.toRedact = this.toRedact.filter(r => r.redactionEqual(redaction)); } - public async process(client: MatrixClient): Promise { - const errors: RoomUpdateError[]= []; - // need to change this so it pops the array until empty - // otherwise this will be cringe. - for (const redaction of this.toRedact) { + /** + * Process the redaction queue, carrying out the action of each QueuedRedaction in sequence. + * @param client The matrix client to use for processing redactions. + * @param roomId If the roomId is provided, only redactions for that room will be processed. + * @returns A description of any errors encountered by each QueuedRedaction that was processed. + */ + public async process(client: MatrixClient, roomId?: string): Promise { + const errors: RoomUpdateError[] = []; + const currentBatch = roomId ? this.toRedact.filter(r => r.roomId === roomId) : this.toRedact; + for (const redaction of currentBatch) { try { await redaction.redact(client); } catch (e) { errors.push(redaction.report(e)); } finally { - // FIXME: Need to figure out in which circumstances we want to retry. + // We need to figure out in which circumstances we want to retry here. this.delete(redaction); } } return errors; - } + } } \ No newline at end of file From 2889599eb2ecb42350d3734c39b1f3ff062e6c61 Mon Sep 17 00:00:00 2001 From: gnuxie Date: Tue, 14 Sep 2021 14:36:53 +0100 Subject: [PATCH 3/5] Rename AutomaticRedactionQueue to UnlistedUserRedactionQueue. This seems to be a more descriptive name and it also doesn't clash with the new redaction queue. --- src/Mjolnir.ts | 13 +++++-------- src/protections/BasicFlooding.ts | 2 +- src/protections/FirstMessageIsImage.ts | 2 +- ...actionQueue.ts => UnlistedUserRedactionQueue.ts} | 10 +++++----- 4 files changed, 12 insertions(+), 15 deletions(-) rename src/queues/{AutomaticRedactionQueue.ts => UnlistedUserRedactionQueue.ts} (86%) diff --git a/src/Mjolnir.ts b/src/Mjolnir.ts index e02eec88..1626d9bd 100644 --- a/src/Mjolnir.ts +++ b/src/Mjolnir.ts @@ -34,7 +34,7 @@ import { logMessage } from "./LogProxy"; import ErrorCache, { ERROR_KIND_FATAL, ERROR_KIND_PERMISSION } from "./ErrorCache"; import { IProtection } from "./protections/IProtection"; import { PROTECTIONS } from "./protections/protections"; -import { AutomaticRedactionQueue } from "./queues/AutomaticRedactionQueue"; +import { UnlistedUserRedactionQueue } from "./queues/UnlistedUserRedactionQueue"; import { Healthz } from "./health/healthz"; import { EventRedactionQueue, RedactUserInRoom } from "./queues/EventRedactionQueue"; @@ -54,7 +54,7 @@ export class Mjolnir { private localpart: string; private currentState: string = STATE_NOT_STARTED; private protections: IProtection[] = []; - private spamRedactionQueue = new AutomaticRedactionQueue(); + private unlistedUserRedactionQueue = new UnlistedUserRedactionQueue(); private eventRedactionQueue = new EventRedactionQueue(); private automaticRedactionReasons: MatrixGlob[] = []; private protectedJoinedRoomIds: string[] = []; @@ -140,8 +140,8 @@ export class Mjolnir { return this.protections; } - public get redactionHandler(): AutomaticRedactionQueue { - return this.spamRedactionQueue; + public get unlistedUserRedactionHandler(): UnlistedUserRedactionQueue { + return this.unlistedUserRedactionQueue; } public get automaticRedactGlobs(): MatrixGlob[] { @@ -581,7 +581,7 @@ export class Mjolnir { // Run the event handlers - we always run this after protections so that the protections // can flag the event for redaction. - await this.spamRedactionQueue.handleEvent(roomId, event, this.client); + await this.unlistedUserRedactionHandler.handleEvent(roomId, event, this.client); if (event['type'] === 'm.room.power_levels' && event['state_key'] === '') { // power levels were updated - recheck permissions @@ -667,9 +667,6 @@ export class Mjolnir { }); } - // This naming is horrible and clashes with the other redaction queue which isn't - // really the same thing. The old one is more about an ongoing user who we haven't - // banned, whereas this one is about redaction of users who aren't active. public queueRedactUserMessagesIn(userId: string, roomId: string) { this.eventRedactionQueue.add(new RedactUserInRoom(userId, roomId)); } diff --git a/src/protections/BasicFlooding.ts b/src/protections/BasicFlooding.ts index 92d6a86c..017b5684 100644 --- a/src/protections/BasicFlooding.ts +++ b/src/protections/BasicFlooding.ts @@ -65,7 +65,7 @@ export class BasicFlooding implements IProtection { } if (this.recentlyBanned.includes(event['sender'])) return; // already handled (will be redacted) - mjolnir.redactionHandler.addUser(event['sender']); + mjolnir.unlistedUserRedactionHandler.addUser(event['sender']); this.recentlyBanned.push(event['sender']); // flag to reduce spam // Redact all the things the user said too diff --git a/src/protections/FirstMessageIsImage.ts b/src/protections/FirstMessageIsImage.ts index c42a63cc..89a7865e 100644 --- a/src/protections/FirstMessageIsImage.ts +++ b/src/protections/FirstMessageIsImage.ts @@ -59,7 +59,7 @@ export class FirstMessageIsImage implements IProtection { } if (this.recentlyBanned.includes(event['sender'])) return; // already handled (will be redacted) - mjolnir.redactionHandler.addUser(event['sender']); + mjolnir.unlistedUserRedactionHandler.addUser(event['sender']); this.recentlyBanned.push(event['sender']); // flag to reduce spam // Redact the event diff --git a/src/queues/AutomaticRedactionQueue.ts b/src/queues/UnlistedUserRedactionQueue.ts similarity index 86% rename from src/queues/AutomaticRedactionQueue.ts rename to src/queues/UnlistedUserRedactionQueue.ts index 63a54747..083df87b 100644 --- a/src/queues/AutomaticRedactionQueue.ts +++ b/src/queues/UnlistedUserRedactionQueue.ts @@ -13,15 +13,15 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. */ -//// NOTE: This is a queue of users whose events should be redacted -////////// Not a queue of events to be redacted. -////////// This is also unrelated to the AutomaticRedactionReasons. -////////// It is as of writing only used by the flood/spam protections. import { extractRequestError, LogLevel, LogService, MatrixClient, Permalinks } from "matrix-bot-sdk"; import { logMessage } from "../LogProxy"; import config from "../config"; -export class AutomaticRedactionQueue { +/** + * This is used to redact new events from users who are not banned from a watched list, but have been flagged + * for redaction by the flooding or image protection. + */ +export class UnlistedUserRedactionQueue { private usersToRedact: Set = new Set(); constructor() { From ad199cc7d7fa68b65104056f263acbad88c123a6 Mon Sep 17 00:00:00 2001 From: gnuxie Date: Wed, 15 Sep 2021 11:06:03 +0100 Subject: [PATCH 4/5] Use a Map for queueing redactions by roomId. --- src/Mjolnir.ts | 26 +++++- src/queues/EventRedactionQueue.ts | 102 +++++++++++++++-------- src/queues/UnlistedUserRedactionQueue.ts | 6 +- 3 files changed, 98 insertions(+), 36 deletions(-) diff --git a/src/Mjolnir.ts b/src/Mjolnir.ts index 1626d9bd..ff9b2927 100644 --- a/src/Mjolnir.ts +++ b/src/Mjolnir.ts @@ -54,7 +54,15 @@ export class Mjolnir { private localpart: string; private currentState: string = STATE_NOT_STARTED; private protections: IProtection[] = []; + /** + * This is for users who are not listed on a watchlist, + * but have been flagged by the automatic spam detection as suispicous + */ private unlistedUserRedactionQueue = new UnlistedUserRedactionQueue(); + /** + * This is a queue for redactions to process after mjolnir + * has finished applying ACL and bans when syncing. + */ private eventRedactionQueue = new EventRedactionQueue(); private automaticRedactionReasons: MatrixGlob[] = []; private protectedJoinedRoomIds: string[] = []; @@ -140,6 +148,10 @@ export class Mjolnir { return this.protections; } + /** + * Retruns the handler to flag a user that is not listed on a watchlist + * for redaction, removing any future messages that they send. + */ public get unlistedUserRedactionHandler(): UnlistedUserRedactionQueue { return this.unlistedUserRedactionQueue; } @@ -486,6 +498,10 @@ export class Mjolnir { return errors; } + /** + * Sync all the rooms with all the watched lists, banning and applying any changes ACLS. + * @param verbose Whether to report any errors to the management room. + */ public async syncLists(verbose = true) { for (const list of this.banLists) { await list.updateList(); @@ -594,7 +610,7 @@ export class Mjolnir { } return; } else if (event['type'] === "m.room.member") { - // Only apply bans in the room we're looking at. + // Only apply bans and then redactions in the room we're looking at. const banErrors = await applyUserBans(this.banLists, [roomId], this); const redactionErrors = await this.processRedactionQueue(roomId); await this.printActionResult(banErrors); @@ -671,7 +687,13 @@ export class Mjolnir { this.eventRedactionQueue.add(new RedactUserInRoom(userId, roomId)); } - public async processRedactionQueue(roomId?: string) { + /** + * Process all queued redactions, this is usually called at the end of the sync process, + * after all users have been banned and ACLs applied. + * @param roomId Limit processing to one room only, otherwise process redactions for all rooms. + * @returns An array of descriptive errors if any were encountered that can be reported to a management room. + */ + public async processRedactionQueue(roomId?: string): Promise { return await this.eventRedactionQueue.process(this.client, roomId); } } diff --git a/src/queues/EventRedactionQueue.ts b/src/queues/EventRedactionQueue.ts index a263e824..96cf869e 100644 --- a/src/queues/EventRedactionQueue.ts +++ b/src/queues/EventRedactionQueue.ts @@ -20,12 +20,24 @@ import { RoomUpdateError } from "../models/RoomUpdateError"; import { redactUserMessagesIn } from "../utils"; export interface QueuedRedaction { - roomId: string; // The room which the redaction will take place in. + /** The room which the redaction will take place in. */ + readonly roomId: string; + /** + * Carries out the redaction task and is called by the EventRedactionQueue + * when processing this redaction. + * @param client A MatrixClient to use to carry out the redaction. + */ redact(client: MatrixClient): Promise + /** + * Used to test whether the redaction is the equivalent to another redaction. + * @param redaction Another QueuedRedaction to test if this redaction is an equivalent to. + */ redactionEqual(redaction: QueuedRedaction): boolean - report(e: any): RoomUpdateError } +/** + * Redacts all of the messages a user has sent to one room. + */ export class RedactUserInRoom implements QueuedRedaction { userId: string; roomId: string; @@ -47,57 +59,81 @@ export class RedactUserInRoom implements QueuedRedaction { return false; } } - - public report(e): RoomUpdateError { - const message = e.message || (e.body ? e.body.error : ''); - return { - roomId: this.roomId, - errorMessage: message, - errorKind: ERROR_KIND_FATAL, - }; - } } /** * This is a queue for events so that other protections can happen first (e.g. applying room bans to every room). */ export class EventRedactionQueue { - private toRedact: Array = new Array(); + private toRedact: Map = new Map(); - public has(redaction: QueuedRedaction) { - return this.toRedact.find(r => r.redactionEqual(redaction)); + /** + * Test whether the redaction is already present in the queue. + * @param redaction a QueuedRedaction. + * @returns True if the queue already has the redaction, false otherwise. + */ + public has(redaction: QueuedRedaction): boolean { + return !!this.toRedact.get(redaction.roomId)?.find(r => r.redactionEqual(redaction)); } - public add(redaction: QueuedRedaction) { + /** + * Adds a QueuedRedaction to the queue to be processed when process is called. + * @param redaction A QueuedRedaction to await processing + * @returns A boolean that is true if the redaction was added to the queue and false if it was duplicated. + */ + public add(redaction: QueuedRedaction): boolean { if (this.has(redaction)) { - return; + return false; } else { - this.toRedact.push(redaction); + let entry = this.toRedact.get(redaction.roomId); + if (entry) { + entry.push(redaction); + } else { + this.toRedact.set(redaction.roomId, [redaction]); + }return true; } } - public delete(redaction: QueuedRedaction) { - this.toRedact = this.toRedact.filter(r => r.redactionEqual(redaction)); - } - /** * Process the redaction queue, carrying out the action of each QueuedRedaction in sequence. * @param client The matrix client to use for processing redactions. - * @param roomId If the roomId is provided, only redactions for that room will be processed. + * @param limitToRoomId If the roomId is provided, only redactions for that room will be processed. * @returns A description of any errors encountered by each QueuedRedaction that was processed. */ - public async process(client: MatrixClient, roomId?: string): Promise { + public async process(client: MatrixClient, limitToRoomId?: string): Promise { const errors: RoomUpdateError[] = []; - const currentBatch = roomId ? this.toRedact.filter(r => r.roomId === roomId) : this.toRedact; - for (const redaction of currentBatch) { - try { - await redaction.redact(client); - } catch (e) { - errors.push(redaction.report(e)); - } finally { - // We need to figure out in which circumstances we want to retry here. - this.delete(redaction); + const redact = async (currentBatch: QueuedRedaction[]) => { + for (const redaction of currentBatch) { + try { + await redaction.redact(client); + } catch (e) { + let roomError: RoomUpdateError; + if (e.roomId && e.errorMessage && e.errorKind) { + roomError = e; + } else { + const message = e.message || (e.body ? e.body.error : ''); + roomError = { + roomId: redaction.roomId, + errorMessage: message, + errorKind: ERROR_KIND_FATAL, + }; + } + errors.push(roomError); + } + } + } + if (limitToRoomId) { + // There might not actually be any queued redactions for this room. + let queuedRedactions = this.toRedact.get(limitToRoomId); + if (queuedRedactions) { + await redact(queuedRedactions); + this.toRedact.delete(limitToRoomId); + } + } else { + for (const [roomId, redactions] of this.toRedact) { + await redact(redactions); + this.toRedact.delete(roomId); } } return errors; } -} \ No newline at end of file +} diff --git a/src/queues/UnlistedUserRedactionQueue.ts b/src/queues/UnlistedUserRedactionQueue.ts index 083df87b..4130e9da 100644 --- a/src/queues/UnlistedUserRedactionQueue.ts +++ b/src/queues/UnlistedUserRedactionQueue.ts @@ -18,8 +18,12 @@ import { logMessage } from "../LogProxy"; import config from "../config"; /** - * This is used to redact new events from users who are not banned from a watched list, but have been flagged + * This class is a queue of users who have been flagged * for redaction by the flooding or image protection. + * Specifically any new events sent by a queued user will be redacted. + * This does not handle previously sent events, for that see the EventRedactionQueue. + * These users are not listed as banned in any watch list and so may continue + * to view a room until a moderator can investigate. */ export class UnlistedUserRedactionQueue { private usersToRedact: Set = new Set(); From 5acc38c8b58b45635586838315ada3579fa241a7 Mon Sep 17 00:00:00 2001 From: gnuxie Date: Thu, 16 Sep 2021 11:46:44 +0100 Subject: [PATCH 5/5] EventRedactionQueue documentation improvements from review --- src/Mjolnir.ts | 16 +++++++++----- src/queues/EventRedactionQueue.ts | 28 +++++++++++++++--------- src/queues/UnlistedUserRedactionQueue.ts | 5 ++--- 3 files changed, 31 insertions(+), 18 deletions(-) diff --git a/src/Mjolnir.ts b/src/Mjolnir.ts index ff9b2927..8ce085d9 100644 --- a/src/Mjolnir.ts +++ b/src/Mjolnir.ts @@ -149,8 +149,9 @@ export class Mjolnir { } /** - * Retruns the handler to flag a user that is not listed on a watchlist - * for redaction, removing any future messages that they send. + * Returns the handler to flag a user for redaction, removing any future messages that they send. + * Typically this is used by the flooding or image protection on users that have not been banned from a list yet. + * It cannot used to redact any previous messages the user has sent, in that cas you should use the `EventRedactionQueue`. */ public get unlistedUserRedactionHandler(): UnlistedUserRedactionQueue { return this.unlistedUserRedactionQueue; @@ -499,7 +500,7 @@ export class Mjolnir { } /** - * Sync all the rooms with all the watched lists, banning and applying any changes ACLS. + * Sync all the rooms with all the watched lists, banning and applying any changed ACLS. * @param verbose Whether to report any errors to the management room. */ public async syncLists(verbose = true) { @@ -610,7 +611,10 @@ export class Mjolnir { } return; } else if (event['type'] === "m.room.member") { - // Only apply bans and then redactions in the room we're looking at. + // The reason we have to apply bans on each member change is because + // we cannot eagerly ban users (that is to ban them when they have never been a member) + // as they can be force joined to a room they might not have known existed. + // Only apply bans and then redactions in the room we are currently looking at. const banErrors = await applyUserBans(this.banLists, [roomId], this); const redactionErrors = await this.processRedactionQueue(roomId); await this.printActionResult(banErrors); @@ -690,8 +694,10 @@ export class Mjolnir { /** * Process all queued redactions, this is usually called at the end of the sync process, * after all users have been banned and ACLs applied. + * If a redaction cannot be processed, the redaction is skipped and removed from the queue. + * We then carry on processing the next redactions. * @param roomId Limit processing to one room only, otherwise process redactions for all rooms. - * @returns An array of descriptive errors if any were encountered that can be reported to a management room. + * @returns The list of errors encountered, for reporting to the management room. */ public async processRedactionQueue(roomId?: string): Promise { return await this.eventRedactionQueue.process(this.client, roomId); diff --git a/src/queues/EventRedactionQueue.ts b/src/queues/EventRedactionQueue.ts index 96cf869e..f71b9180 100644 --- a/src/queues/EventRedactionQueue.ts +++ b/src/queues/EventRedactionQueue.ts @@ -23,11 +23,11 @@ export interface QueuedRedaction { /** The room which the redaction will take place in. */ readonly roomId: string; /** - * Carries out the redaction task and is called by the EventRedactionQueue - * when processing this redaction. + * Carry out the redaction. + * Called by the EventRedactionQueue. * @param client A MatrixClient to use to carry out the redaction. */ - redact(client: MatrixClient): Promise + redact(client: MatrixClient): Promise /** * Used to test whether the redaction is the equivalent to another redaction. * @param redaction Another QueuedRedaction to test if this redaction is an equivalent to. @@ -64,6 +64,9 @@ export class RedactUserInRoom implements QueuedRedaction { * This is a queue for events so that other protections can happen first (e.g. applying room bans to every room). */ export class EventRedactionQueue { + /** + * This map is indexed by roomId and its values are a list of redactions waiting to be processed for that room. + */ private toRedact: Map = new Map(); /** @@ -76,9 +79,9 @@ export class EventRedactionQueue { } /** - * Adds a QueuedRedaction to the queue to be processed when process is called. - * @param redaction A QueuedRedaction to await processing - * @returns A boolean that is true if the redaction was added to the queue and false if it was duplicated. + * Adds a `QueuedRedaction` to the queue. It will be processed when `process` is called. + * @param redaction A `QueuedRedaction` to await processing + * @returns `true` if the redaction was added to the queue, `false` if it is a duplicate of a redaction already present in the queue. */ public add(redaction: QueuedRedaction): boolean { if (this.has(redaction)) { @@ -89,12 +92,17 @@ export class EventRedactionQueue { entry.push(redaction); } else { this.toRedact.set(redaction.roomId, [redaction]); - }return true; + } + return true; } } /** - * Process the redaction queue, carrying out the action of each QueuedRedaction in sequence. + * Process the redaction queue, carrying out the action of each `QueuedRedaction` in sequence. + * If a redaction cannot be processed, the redaction is skipped and removed from the queue. + * We then carry on processing the next redactions. + * The reason we skip is at the moment is that we would have to think about all of the situations + * where we would not want failures to try again (e.g. messages were already redacted) and handle them explicitly. * @param client The matrix client to use for processing redactions. * @param limitToRoomId If the roomId is provided, only redactions for that room will be processed. * @returns A description of any errors encountered by each QueuedRedaction that was processed. @@ -125,13 +133,13 @@ export class EventRedactionQueue { // There might not actually be any queued redactions for this room. let queuedRedactions = this.toRedact.get(limitToRoomId); if (queuedRedactions) { - await redact(queuedRedactions); this.toRedact.delete(limitToRoomId); + await redact(queuedRedactions); } } else { for (const [roomId, redactions] of this.toRedact) { - await redact(redactions); this.toRedact.delete(roomId); + await redact(redactions); } } return errors; diff --git a/src/queues/UnlistedUserRedactionQueue.ts b/src/queues/UnlistedUserRedactionQueue.ts index 4130e9da..1d3cbd66 100644 --- a/src/queues/UnlistedUserRedactionQueue.ts +++ b/src/queues/UnlistedUserRedactionQueue.ts @@ -18,10 +18,9 @@ import { logMessage } from "../LogProxy"; import config from "../config"; /** - * This class is a queue of users who have been flagged - * for redaction by the flooding or image protection. + * A queue of users who have been flagged for redaction typically by the flooding or image protection. * Specifically any new events sent by a queued user will be redacted. - * This does not handle previously sent events, for that see the EventRedactionQueue. + * This does not handle previously sent events, for that see the `EventRedactionQueue`. * These users are not listed as banned in any watch list and so may continue * to view a room until a moderator can investigate. */