diff --git a/src/ProtectedRoomsSet.ts b/src/ProtectedRoomsSet.ts index 6b564501..9833b876 100644 --- a/src/ProtectedRoomsSet.ts +++ b/src/ProtectedRoomsSet.ts @@ -373,6 +373,16 @@ export class ProtectedRoomsSet { private async applyUserBans(roomIds: string[]): Promise { // We can only ban people who are not already banned, and who match the rules. const errors: RoomUpdateError[] = []; + + const addErrorToReport = (roomId: string, e: any) => { + const message = e.message || (e.body ? e.body.error : ''); + errors.push({ + roomId, + errorMessage: message, + errorKind: message && message.includes("You don't have permission to ban") ? ERROR_KIND_PERMISSION : ERROR_KIND_FATAL, + }); + }; + for (const roomId of roomIds) { try { // We specifically use sendNotice to avoid having to escape HTML @@ -405,9 +415,13 @@ export class ProtectedRoomsSet { await this.managementRoomOutput.logMessage(LogLevel.INFO, "ApplyBan", `Banning ${member.userId} in ${roomId} for: ${reason}`, roomId); if (!this.config.noop) { - await this.client.banUser(member.userId, roomId, memberAccess.rule!.reason); - if (this.automaticRedactGlobs.find(g => g.test(reason.toLowerCase()))) { - this.redactUser(member.userId, roomId); + try { + await this.client.banUser(member.userId, roomId, memberAccess.rule!.reason); + if (this.automaticRedactGlobs.find(g => g.test(reason.toLowerCase()))) { + this.redactUser(member.userId, roomId); + } + } catch (e) { + addErrorToReport(roomId, e); } } else { await this.managementRoomOutput.logMessage(LogLevel.WARN, "ApplyBan", `Tried to ban ${member.userId} in ${roomId} but Mjolnir is running in no-op mode`, roomId); @@ -415,12 +429,7 @@ export class ProtectedRoomsSet { } } } catch (e) { - const message = e.message || (e.body ? e.body.error : ''); - errors.push({ - roomId, - errorMessage: message, - errorKind: message && message.includes("You don't have permission to ban") ? ERROR_KIND_PERMISSION : ERROR_KIND_FATAL, - }); + addErrorToReport(roomId, e) } } diff --git a/test/integration/banListTest.ts b/test/integration/banListTest.ts index 929e1a4a..5bfca29c 100644 --- a/test/integration/banListTest.ts +++ b/test/integration/banListTest.ts @@ -1,6 +1,6 @@ import { strict as assert } from "assert"; import { newTestUser } from "./clientHelper"; -import { LogService, MatrixClient, Permalinks, UserID } from "matrix-bot-sdk"; +import { LogService, MatrixClient, Permalinks, UserID, MembershipEvent } from "matrix-bot-sdk"; import PolicyList, { ChangeType } from "../../src/models/PolicyList"; import { ServerAcl } from "../../src/models/ServerAcl"; import { getFirstReaction } from "./commands/commandUtils"; @@ -11,6 +11,7 @@ import AccessControlUnit, { Access, EntityAccess } from "../../src/models/Access import { randomUUID } from "crypto"; import { MatrixSendClient } from "../../src/MatrixEmitter"; import { MatrixRoomReference } from "../../src/commands/interface-manager/MatrixRoomReference"; +import { MjolnirTestContext } from "./mjolnirSetupUtils"; /** * Create a policy rule in a policy room. @@ -595,3 +596,44 @@ describe('Test: Creating policy lists.', function() { ); }) }) + +describe.only('Test: Continue to ban other marked members when one member cannot be banned', function() { + it('Failing to ban a moderator should not stop other members being banned.', async function(this: MjolnirTestContext) { + if (this.mjolnir === undefined) { + throw new TypeError("Mjolnir was never created.") + } + const mjolnir: Mjolnir = this.mjolnir; + const moderator = await newTestUser(this.config.homeserverUrl, { name: { contains: "mx-moderator" } }); + await moderator.joinRoom(mjolnir.managementRoomId); + const mjolnirId = await mjolnir.client.getUserId(); + + const protectedRoom = await moderator.createRoom({ invite: [mjolnirId], preset: 'public_chat'}); + await mjolnir.client.joinRoom(protectedRoom); + await moderator.setUserPowerLevel(mjolnirId, protectedRoom, 100); + await mjolnir.addProtectedRoom(protectedRoom); + + const banListId = await moderator.createRoom({ invite: [mjolnirId] }); + await mjolnir.client.joinRoom(banListId); + await mjolnir.policyListManager.watchList(MatrixRoomReference.fromRoomId(banListId)); + + await mjolnir.protectedRoomsTracker.syncLists(); + + const spamUser = await newTestUser(this.config.homeserverUrl, { name: { contains: "mx-spammer" } }); + const rudeUser = await newTestUser(this.config.homeserverUrl, { name: { contains: "mx-rude" } }); + const sillyModerator = await newTestUser(this.config.homeserverUrl, { name: { contains: "mx-moderator" } }); + await Promise.all([spamUser, rudeUser, sillyModerator].map(c => c.joinRoom(protectedRoom))); + await moderator.setUserPowerLevel((await sillyModerator.getUserId()), protectedRoom, 100); + + await createPolicyRule(moderator, banListId, RULE_USER, '@*mx*:*', "don't like them go away."); + await mjolnir.protectedRoomsTracker.syncLists(); + + const roomMembers = await moderator.getRoomMembers(protectedRoom); + + const assertMembership = (userId: string, membership: string, members: MembershipEvent[]) => { + assert.equal(members.find(e => e.stateKey === userId)?.membership, membership); + }; + + assertMembership(await spamUser.getUserId(), 'ban', roomMembers); + assertMembership(await rudeUser.getUserId(), 'ban', roomMembers); + }) +})