Refactor and improve the unban command.

We no longer want to accept an argument for the list. We will just
find all appropriate policies and remove them, like we do with the
unban prompt (which we still might want to update to use the new
`--no-confirm` prompt later).

We fix the bugs where the unban command was inviting users regardless
of whether the `--invite` option was provided.

The unban command now uses a preview which shows all the policies that
will have to be removed to unban a user, all the rooms they will need
to be unbanned from, and any rooms that they will be invited to if the
`--invite` option is used.
This commit is contained in:
gnuxie
2025-02-14 13:34:37 +00:00
parent 9587d6fcba
commit a45d308597
8 changed files with 1070 additions and 268 deletions

View File

@@ -11,7 +11,10 @@
import { MatrixClient } from "matrix-bot-sdk";
import { strict as assert } from "assert";
import * as crypto from "crypto";
import { MatrixEmitter } from "matrix-protection-suite-for-matrix-bot-sdk";
import {
MatrixEmitter,
MatrixSendClient,
} from "matrix-protection-suite-for-matrix-bot-sdk";
import {
NoticeMessageContent,
ReactionEvent,
@@ -21,6 +24,11 @@ import {
StringEventIDSchema,
} from "matrix-protection-suite";
import { Type } from "@sinclair/typebox";
import { Draupnir } from "../../../src/Draupnir";
import {
StringEventID,
StringRoomID,
} from "@the-draupnir-project/matrix-basic-types";
export const ReplyContent = Type.Intersect([
Type.Object({
@@ -249,3 +257,30 @@ export async function createBanList(
);
return listName;
}
export async function sendCommand(
draupnir: Draupnir,
command: string
): Promise<{ eventID: StringEventID }> {
const eventID = (await draupnir.client.sendMessage(
draupnir.managementRoomID,
{
msgtype: "m.text",
body: command,
}
)) as StringEventID;
return { eventID };
}
export async function acceptPropmt(
client: MatrixSendClient,
roomID: StringRoomID,
eventID: StringEventID,
promptKey: string
): Promise<void> {
// i suspect that adding a reaction using the unstable API doesn't work because it uses the usntable prefix
// whereas our schema doesn't have the unstable event type.
// we don't test for this anywhere and we should really unify the situation between draupnir, draupnir safe mode,
// and any new bots that just need all the same things.
await client.unstableApis.addReactionToEvent(roomID, eventID, promptKey);
}

View File

@@ -0,0 +1,346 @@
// SPDX-FileCopyrightText: 2025 Gnuxie <Gnuxie@protonmail.com>
//
// SPDX-License-Identifier: AFL-3.0
// 1. We need to test glob on user, literal on user, and server on user all get
// removed when using unban.
// 1.a. We need to test that nothing happens when you just enter the command
// without confirmation
// 1.b. We need to test the effects happen after confirmation
// 2. We need to test that invite behaviour is optional.
// 3. We need to test that inviting and unbanning works even when
// There are no policies.
// This probably all needs to be an integration test... So that we can
// Check the rendering.
import {
Membership,
PolicyRoomEditor,
PolicyRuleType,
Recommendation,
} from "matrix-protection-suite";
import { Draupnir } from "../../../src/Draupnir";
import {
MatrixRoomReference,
StringRoomID,
StringUserID,
userLocalpart,
userServerName,
} from "@the-draupnir-project/matrix-basic-types";
import { DraupnirTestContext } from "../mjolnirSetupUtils";
import { newTestUser } from "../clientHelper";
import {
UnbanMembersPreview,
UnbanMembersResult,
} from "../../../src/commands/unban/Unban";
import expect from "expect";
async function createProtectedRoomsSetWithBan(
draupnir: Draupnir,
userToBanUserID: StringUserID,
{ numberOfRooms }: { numberOfRooms: number }
): Promise<StringRoomID[]> {
return await Promise.all(
[...Array(numberOfRooms)].map(async (_) => {
const roomID = (await draupnir.client.createRoom()) as StringRoomID;
const room = MatrixRoomReference.fromRoomID(roomID, []);
(
await draupnir.clientPlatform
.toRoomBanner()
.banUser(room, userToBanUserID, "spam")
).expect("Should be able to ban the user from the room");
(
await draupnir.protectedRoomsSet.protectedRoomsManager.addRoom(room)
).expect("Should be able to protect the newly created room");
return roomID;
})
);
}
async function createPoliciesBanningUser(
policyRoomEditor: PolicyRoomEditor,
userToBanUserID: StringUserID
): Promise<void> {
(
await policyRoomEditor.createPolicy(
PolicyRuleType.Server,
Recommendation.Ban,
userServerName(userToBanUserID),
"spam",
{}
)
).expect("Should be able to create the server policy");
(
await policyRoomEditor.createPolicy(
PolicyRuleType.User,
Recommendation.Ban,
`@${userLocalpart(userToBanUserID)}:*`,
"spam",
{}
)
).expect("Should be able to create a glob policy");
(
await policyRoomEditor.createPolicy(
PolicyRuleType.User,
Recommendation.Ban,
userToBanUserID,
"spam",
{}
)
).expect("Should be able to ban the user directly");
}
async function createWatchedPolicyRoom(
draupnir: Draupnir
): Promise<StringRoomID> {
const policyRoomID = (await draupnir.client.createRoom()) as StringRoomID;
(
await draupnir.protectedRoomsSet.watchedPolicyRooms.watchPolicyRoomDirectly(
MatrixRoomReference.fromRoomID(policyRoomID)
)
).expect("Should be able to watch the new policy room");
return policyRoomID;
}
describe("unbanCommandTest", function () {
it(
"Should be able to unban members to protected rooms, removing all policies that will target them",
async function (this: DraupnirTestContext) {
const draupnir = this.draupnir;
if (draupnir === undefined) {
throw new TypeError(`setup didn't run properly`);
}
const falsePositiveUser = await newTestUser(this.config.homeserverUrl, {
name: { contains: "accidentally-banned" },
});
const falsePositiveUserID =
(await falsePositiveUser.getUserId()) as StringUserID;
const protectedRooms = await createProtectedRoomsSetWithBan(
draupnir,
falsePositiveUserID,
{ numberOfRooms: 5 }
);
const policyRoomID = await createWatchedPolicyRoom(draupnir);
const policyRoomEditor = (
await draupnir.policyRoomManager.getPolicyRoomEditor(
MatrixRoomReference.fromRoomID(policyRoomID)
)
).expect(
"Should be able to get a policy room editor for the newly created policy room"
);
await createPoliciesBanningUser(policyRoomEditor, falsePositiveUserID);
// wait for policies to be detected.
await new Promise((resolve) => setTimeout(resolve, 1000));
// Now we can use the unban command to test the preview has no effects
// So the way this can work is we can send the command, get back the event and just know that we can send 'OK' and 'Cancel' to it later and it'll work.
const previewResult = (
await draupnir.sendTextCommand(
draupnir.clientUserID,
`!draupnir unban ${falsePositiveUserID}`
)
).expect(
"We should have been able to get a preview"
) as UnbanMembersPreview;
expect(previewResult.membersToUnban.length).toBe(1); // hmm we're going to have to put the user on a different server...
expect(previewResult.policyMatchesToRemove.length).toBe(1);
const listMatches = previewResult.policyMatchesToRemove.at(0);
if (listMatches === undefined) {
throw new TypeError("We should have some matches");
}
expect(listMatches.matches.length).toBe(3);
const falsePositiveMember = previewResult.membersToUnban.at(0);
if (falsePositiveMember === undefined) {
throw new TypeError("We should have some details here");
}
expect(falsePositiveMember.roomsBannedFrom.length).toBe(5);
expect(falsePositiveMember.roomsToInviteTo.length).toBe(0);
expect(falsePositiveMember.member).toBe(falsePositiveUserID);
// now checked that the user is still banned in all 5 rooms
for (const roomID of protectedRooms) {
const membershipRevision =
draupnir.protectedRoomsSet.setRoomMembership.getRevision(roomID);
if (membershipRevision === undefined) {
throw new TypeError(
"Unable to find membership revision for a protected room, shouldn't happen"
);
}
expect(
membershipRevision.membershipForUser(falsePositiveUserID)?.membership
).toBe(Membership.Ban);
}
(
await draupnir.sendTextCommand(
draupnir.clientUserID,
`!draupnir unban ${falsePositiveUserID} --no-confirm`
)
).expect(
"We should have been able to run the command"
) as UnbanMembersResult;
// wait for events to come down sync
await new Promise((resolve) => setTimeout(resolve, 1000));
// now check that they are unbanned
for (const roomID of protectedRooms) {
const membershipRevision =
draupnir.protectedRoomsSet.setRoomMembership.getRevision(roomID);
if (membershipRevision === undefined) {
throw new TypeError(
"Unable to find membership revision for a protected room, shouldn't happen"
);
}
expect(
membershipRevision.membershipForUser(falsePositiveUserID)?.membership
).toBe(Membership.Leave);
}
// verify the policies are removed.
const policyRevision =
draupnir.protectedRoomsSet.watchedPolicyRooms.currentRevision;
expect(policyRevision.allRules.length).toBe(0);
// (Bonus) now check that if we run the command again, then the user will be reinvited even though they have been unbanned.
const inviteResult = (
await draupnir.sendTextCommand(
draupnir.clientUserID,
`!draupnir unban ${falsePositiveUserID} --no-confirm --invite`
)
).expect(
"We should have been able to run the command"
) as UnbanMembersResult;
expect(inviteResult.usersInvited.map.size).toBe(1);
expect(inviteResult.membersToUnban.at(0)?.roomsToInviteTo.length).toBe(5);
await new Promise((resolve) => setTimeout(resolve, 1000));
for (const roomID of protectedRooms) {
const membershipRevision =
draupnir.protectedRoomsSet.setRoomMembership.getRevision(roomID);
if (membershipRevision === undefined) {
throw new TypeError(
"Unable to find membership revision for a protected room, shouldn't happen"
);
}
expect(
membershipRevision.membershipForUser(falsePositiveUserID)?.membership
).toBe(Membership.Invite);
}
} as unknown as Mocha.AsyncFunc
);
it("Unbans users even when there are no policies", async function (
this: DraupnirTestContext
) {
const draupnir = this.draupnir;
if (draupnir === undefined) {
throw new TypeError(`setup didn't run properly`);
}
const falsePositiveUser = await newTestUser(this.config.homeserverUrl, {
name: { contains: "accidentally-banned" },
});
const falsePositiveUserID =
(await falsePositiveUser.getUserId()) as StringUserID;
const protectedRooms = await createProtectedRoomsSetWithBan(
draupnir,
falsePositiveUserID,
{ numberOfRooms: 5 }
);
// verify that there are no policies.
const policyRevision =
draupnir.protectedRoomsSet.watchedPolicyRooms.currentRevision;
expect(policyRevision.allRules.length).toBe(0);
(
await draupnir.sendTextCommand(
draupnir.clientUserID,
`!draupnir unban ${falsePositiveUserID} --no-confirm`
)
).expect(
"We should have been able to run the command"
) as UnbanMembersResult;
// wait for events to come down sync
await new Promise((resolve) => setTimeout(resolve, 1000));
// now check that they are unbanned
for (const roomID of protectedRooms) {
const membershipRevision =
draupnir.protectedRoomsSet.setRoomMembership.getRevision(roomID);
if (membershipRevision === undefined) {
throw new TypeError(
"Unable to find membership revision for a protected room, shouldn't happen"
);
}
expect(
membershipRevision.membershipForUser(falsePositiveUserID)?.membership
).toBe(Membership.Leave);
}
} as unknown as Mocha.AsyncFunc);
it(
"Unbans and reinvites users when the invite option is provided",
async function (this: DraupnirTestContext) {
const draupnir = this.draupnir;
if (draupnir === undefined) {
throw new TypeError(`setup didn't run properly`);
}
const falsePositiveUser = await newTestUser(this.config.homeserverUrl, {
name: { contains: "accidentally-banned" },
});
const falsePositiveUserID =
(await falsePositiveUser.getUserId()) as StringUserID;
const protectedRooms = await createProtectedRoomsSetWithBan(
draupnir,
falsePositiveUserID,
{ numberOfRooms: 5 }
);
// Now we can use the unban command to test the preview has no effects
// So the way this can work is we can send the command, get back the event and just know that we can send 'OK' and 'Cancel' to it later and it'll work.
const previewResult = (
await draupnir.sendTextCommand(
draupnir.clientUserID,
`!draupnir unban ${falsePositiveUserID} --invite`
)
).expect(
"We should have been able to get a preview"
) as UnbanMembersPreview;
expect(previewResult.membersToUnban.length).toBe(1); // hmm we're going to have to put the user on a different server...
expect(previewResult.policyMatchesToRemove.length).toBe(0);
const falsePositiveMember = previewResult.membersToUnban.at(0);
if (falsePositiveMember === undefined) {
throw new TypeError("We should have some details here");
}
expect(falsePositiveMember.roomsBannedFrom.length).toBe(5);
expect(falsePositiveMember.roomsToInviteTo.length).toBe(5);
expect(falsePositiveMember.member).toBe(falsePositiveUserID);
// now checked that the user is still banned in all 5 rooms
for (const roomID of protectedRooms) {
const membershipRevision =
draupnir.protectedRoomsSet.setRoomMembership.getRevision(roomID);
if (membershipRevision === undefined) {
throw new TypeError(
"Unable to find membership revision for a protected room, shouldn't happen"
);
}
expect(
membershipRevision.membershipForUser(falsePositiveUserID)?.membership
).toBe(Membership.Ban);
}
(
await draupnir.sendTextCommand(
draupnir.clientUserID,
`!draupnir unban ${falsePositiveUserID} --invite --no-confirm`
)
).expect(
"We should have been able to run the command"
) as UnbanMembersResult;
// wait for events to come down sync
await new Promise((resolve) => setTimeout(resolve, 1000));
// now check that they are unbanned
for (const roomID of protectedRooms) {
const membershipRevision =
draupnir.protectedRoomsSet.setRoomMembership.getRevision(roomID);
if (membershipRevision === undefined) {
throw new TypeError(
"Unable to find membership revision for a protected room, shouldn't happen"
);
}
expect(
membershipRevision.membershipForUser(falsePositiveUserID)?.membership
).toBe(Membership.Invite);
}
} as unknown as Mocha.AsyncFunc
);
});

View File

@@ -25,7 +25,7 @@ import {
} from "matrix-protection-suite";
import { createMock } from "ts-auto-mock";
import expect from "expect";
import { DraupnirUnbanCommand } from "../../../src/commands/Unban";
import { DraupnirUnbanCommand } from "../../../src/commands/unban/Unban";
import ManagementRoomOutput from "../../../src/managementroom/ManagementRoomOutput";
import { UnlistedUserRedactionQueue } from "../../../src/queues/UnlistedUserRedactionQueue";
@@ -129,7 +129,7 @@ describe("Test the DraupnirUnbanCommand", function () {
DraupnirUnbanCommand,
{
policyRoomManager: mockPolicyRoomManager,
setMembership: protectedRoomsSet.setRoomMembership,
setRoomMembership: protectedRoomsSet.setRoomMembership,
managementRoomOutput: createMock<ManagementRoomOutput>(),
roomResolver,
watchedPolicyRooms: protectedRoomsSet.watchedPolicyRooms,
@@ -142,12 +142,14 @@ describe("Test the DraupnirUnbanCommand", function () {
return Ok(undefined);
},
}),
setMembership: protectedRoomsSet.setMembership,
setPoliciesMatchingMembership:
protectedRoomsSet.setPoliciesMatchingMembership.currentRevision,
},
{
rest: ["spam"],
},
MatrixUserID.fromUserID(ExistingBanUserID),
policyRoom
MatrixUserID.fromUserID(ExistingBanUserID)
);
expect(banResult.isOkay).toBe(true);
const membership = protectedRoomsSet.setRoomMembership.getRevision(
@@ -159,7 +161,7 @@ describe("Test the DraupnirUnbanCommand", function () {
);
}
expect(membership.membershipForUser(ExistingBanUserID)?.membership).toBe(
Membership.Leave
Membership.Ban
);
});
});