From 570845a7eca8aba035e5e45f677c7657dc1d6262 Mon Sep 17 00:00:00 2001 From: David Teller Date: Wed, 27 Jul 2022 12:48:27 +0200 Subject: [PATCH 1/6] WIP: Part deux --- src/Mjolnir.ts | 6 ++-- src/commands/DumpRulesCommand.ts | 2 +- src/commands/ImportCommand.ts | 2 +- src/commands/UnbanBanCommand.ts | 2 +- src/models/PolicyList.ts | 34 +++++++++++------------ src/models/{ListRule.ts => PolicyRule.ts} | 24 ++++++++-------- src/models/RuleServer.ts | 20 ++++++------- test/integration/banListTest.ts | 12 ++++---- 8 files changed, 51 insertions(+), 51 deletions(-) rename src/models/{ListRule.ts => PolicyRule.ts} (91%) diff --git a/src/Mjolnir.ts b/src/Mjolnir.ts index 91525f23..c3a4496d 100644 --- a/src/Mjolnir.ts +++ b/src/Mjolnir.ts @@ -27,7 +27,7 @@ import { TextualMessageEventContent } from "matrix-bot-sdk"; -import { ALL_RULE_TYPES as ALL_BAN_LIST_RULE_TYPES, RULE_ROOM, RULE_SERVER, RULE_USER } from "./models/ListRule"; +import { ALL_RULE_TYPES as ALL_BAN_LIST_RULE_TYPES, RULE_ROOM, RULE_SERVER, RULE_USER } from "./models/PolicyRule"; import { applyServerAcls } from "./actions/ApplyAcl"; import { RoomUpdateError } from "./models/RoomUpdateError"; import { COMMAND_PREFIX, handleCommand } from "./commands/CommandHandler"; @@ -50,7 +50,7 @@ import RuleServer from "./models/RuleServer"; import { RoomMemberManager } from "./RoomMembers"; import { ProtectedRoomActivityTracker } from "./queues/ProtectedRoomActivityTracker"; import { ThrottlingQueue } from "./queues/ThrottlingQueue"; -import PolicyList, { ListRuleChange } from "./models/PolicyList"; +import PolicyList, { PolicyRuleChange } from "./models/PolicyList"; const levelToFn = { [LogLevel.DEBUG.toString()]: LogService.debug, @@ -1052,7 +1052,7 @@ export class Mjolnir { * @param ignoreSelf Whether to exclude changes that have been made by Mjolnir. * @returns true if the message was sent, false if it wasn't (because there there were no changes to report). */ - private async printBanlistChanges(changes: ListRuleChange[], list: PolicyList, ignoreSelf = false): Promise { + private async printBanlistChanges(changes: PolicyRuleChange[], list: PolicyList, ignoreSelf = false): Promise { if (ignoreSelf) { const sender = await this.client.getUserId(); changes = changes.filter(change => change.sender !== sender); diff --git a/src/commands/DumpRulesCommand.ts b/src/commands/DumpRulesCommand.ts index ca92e791..03d4f58a 100644 --- a/src/commands/DumpRulesCommand.ts +++ b/src/commands/DumpRulesCommand.ts @@ -16,7 +16,7 @@ limitations under the License. import { RichReply } from "matrix-bot-sdk"; import { Mjolnir } from "../Mjolnir"; -import { EntityType } from "../models/ListRule"; +import { EntityType } from "../models/PolicyRule"; import { htmlEscape } from "../utils"; /** diff --git a/src/commands/ImportCommand.ts b/src/commands/ImportCommand.ts index 8082efe9..45d12633 100644 --- a/src/commands/ImportCommand.ts +++ b/src/commands/ImportCommand.ts @@ -16,7 +16,7 @@ limitations under the License. import { Mjolnir } from "../Mjolnir"; import { RichReply } from "matrix-bot-sdk"; -import { EntityType, Recommendation } from "../models/ListRule"; +import { EntityType, Recommendation } from "../models/PolicyRule"; // !mjolnir import export async function execImportCommand(roomId: string, event: any, mjolnir: Mjolnir, parts: string[]) { diff --git a/src/commands/UnbanBanCommand.ts b/src/commands/UnbanBanCommand.ts index 8caba37c..3b0ba185 100644 --- a/src/commands/UnbanBanCommand.ts +++ b/src/commands/UnbanBanCommand.ts @@ -17,7 +17,7 @@ limitations under the License. import { Mjolnir } from "../Mjolnir"; import PolicyList from "../models/PolicyList"; import { extractRequestError, LogLevel, LogService, MatrixGlob, RichReply } from "matrix-bot-sdk"; -import { Recommendation, RULE_ROOM, RULE_SERVER, RULE_USER, USER_RULE_TYPES } from "../models/ListRule"; +import { Recommendation, RULE_ROOM, RULE_SERVER, RULE_USER, USER_RULE_TYPES } from "../models/PolicyRule"; import config from "../config"; import { DEFAULT_LIST_EVENT_TYPE } from "./SetDefaultBanListCommand"; diff --git a/src/models/PolicyList.ts b/src/models/PolicyList.ts index b4628937..9e9faa47 100644 --- a/src/models/PolicyList.ts +++ b/src/models/PolicyList.ts @@ -16,7 +16,7 @@ limitations under the License. import { extractRequestError, LogService, MatrixClient, UserID } from "matrix-bot-sdk"; import { EventEmitter } from "events"; -import { ALL_RULE_TYPES, EntityType, ListRule, Recommendation, ROOM_RULE_TYPES, RULE_ROOM, RULE_SERVER, RULE_USER, SERVER_RULE_TYPES, USER_RULE_TYPES } from "./ListRule"; +import { ALL_RULE_TYPES, EntityType, PolicyRule, Recommendation, ROOM_RULE_TYPES, RULE_ROOM, RULE_SERVER, RULE_USER, SERVER_RULE_TYPES, USER_RULE_TYPES } from "./PolicyRule"; export const SHORTCODE_EVENT_TYPE = "org.matrix.mjolnir.shortcode"; @@ -26,7 +26,7 @@ export enum ChangeType { Modified = "MODIFIED" } -export interface ListRuleChange { +export interface PolicyRuleChange { readonly changeType: ChangeType, /** * State event that caused the change. @@ -43,7 +43,7 @@ export interface ListRuleChange { * The current rule represented by the event. * If the rule has been removed, then this will show what the rule was. */ - readonly rule: ListRule, + readonly rule: PolicyRule, /** * The previous state that has been changed. Only (and always) provided when the change type is `ChangeType.Removed` or `Modified`. * This will be a copy of the same event as `event` when a redaction has occurred and this will show its unredacted state. @@ -53,8 +53,8 @@ export interface ListRuleChange { declare interface PolicyList { // PolicyList.update is emitted when the PolicyList has pulled new rules from Matrix and informs listeners of any changes. - on(event: 'PolicyList.update', listener: (list: PolicyList, changes: ListRuleChange[]) => void): this - emit(event: 'PolicyList.update', list: PolicyList, changes: ListRuleChange[]): boolean + on(event: 'PolicyList.update', listener: (list: PolicyList, changes: PolicyRuleChange[]) => void): this + emit(event: 'PolicyList.update', list: PolicyList, changes: PolicyRuleChange[]): boolean // PolicyList.batch is emitted when the PolicyList has created a batch from the events provided by `updateForEvent`. on(event: 'PolicyList.batch', listener: (list: PolicyList) => void): this emit(event: 'PolicyList.batch', list: PolicyList): boolean @@ -118,10 +118,10 @@ class PolicyList extends EventEmitter { /** * Return all the active rules of a given kind. * @param kind e.g. RULE_SERVER (m.policy.rule.server). Rule types are always normalised when they are interned into the PolicyList. - * @returns The active ListRules for the ban list of that kind. + * @returns The active PolicyRules for the ban list of that kind. */ - private rulesOfKind(kind: string): ListRule[] { - const rules: ListRule[] = [] + private rulesOfKind(kind: string): PolicyRule[] { + const rules: PolicyRule[] = [] const stateKeyMap = this.state.get(kind); if (stateKeyMap) { for (const event of stateKeyMap.values()) { @@ -146,19 +146,19 @@ class PolicyList extends EventEmitter { }); } - public get serverRules(): ListRule[] { + public get serverRules(): PolicyRule[] { return this.rulesOfKind(RULE_SERVER); } - public get userRules(): ListRule[] { + public get userRules(): PolicyRule[] { return this.rulesOfKind(RULE_USER); } - public get roomRules(): ListRule[] { + public get roomRules(): PolicyRule[] { return this.rulesOfKind(RULE_ROOM); } - public get allRules(): ListRule[] { + public get allRules(): PolicyRule[] { return [...this.serverRules, ...this.userRules, ...this.roomRules]; } @@ -169,7 +169,7 @@ class PolicyList extends EventEmitter { * @param entity The entity to test e.g. the user id, server name or a room id. * @returns All of the rules that match this entity. */ - public rulesMatchingEntity(entity: string, ruleKind?: string): ListRule[] { + public rulesMatchingEntity(entity: string, ruleKind?: string): PolicyRule[] { const ruleTypeOf: (entityPart: string) => string = (entityPart: string) => { if (ruleKind) { return ruleKind; @@ -233,8 +233,8 @@ class PolicyList extends EventEmitter { * and updating the model to reflect the room. * @returns A description of any rules that were added, modified or removed from the list as a result of this update. */ - public async updateList(): Promise { - let changes: ListRuleChange[] = []; + public async updateList(): Promise { + let changes: PolicyRuleChange[] = []; const state = await this.client.getRoomState(this.roomId); for (const event of state) { @@ -313,11 +313,11 @@ class PolicyList extends EventEmitter { changeType, event, sender, rule: previousState.unsigned.rule, ...previousState ? { previousState } : {} }); - // Event has no content and cannot be parsed as a ListRule. + // Event has no content and cannot be parsed as a PolicyRule. continue; } // It's a rule - parse it - const rule = ListRule.parse(event); + const rule = PolicyRule.parse(event); if (!rule) { // Invalid/unknown rule, just skip it. continue; diff --git a/src/models/ListRule.ts b/src/models/PolicyRule.ts similarity index 91% rename from src/models/ListRule.ts rename to src/models/PolicyRule.ts index f9927224..75245e88 100644 --- a/src/models/ListRule.ts +++ b/src/models/PolicyRule.ts @@ -51,9 +51,9 @@ export enum Recommendation { /// The rule specifies an "opinion", as a number in [-100, +100], /// where -100 represents a user who is considered absolutely toxic - /// by whoever issued this ListRule and +100 represents a user who + /// by whoever issued this PolicyRule and +100 represents a user who /// is considered absolutely absolutely perfect by whoever issued - /// this ListRule. + /// this PolicyRule. Opinion = "org.matrix.msc3845.opinion", } @@ -81,7 +81,7 @@ export const OPINION_MAX = +100; /** * Representation of a rule within a Policy List. */ -export abstract class ListRule { +export abstract class PolicyRule { /** * A glob for `entity`. */ @@ -115,12 +115,12 @@ export abstract class ListRule { } /** - * Validate and parse an event into a ListRule. + * Validate and parse an event into a PolicyRule. * * @param event An *untrusted* event. - * @returns null if the ListRule is invalid or not recognized by Mjölnir. + * @returns null if the PolicyRule is invalid or not recognized by Mjölnir. */ - public static parse(event: {type: string, content: any}): ListRule | null { + public static parse(event: {type: string, content: any}): PolicyRule | null { // Parse common fields. // If a field is ill-formed, discard the rule. const content = event['content']; @@ -155,17 +155,17 @@ export abstract class ListRule { // From this point, we may need specific fields. if (RECOMMENDATION_BAN_VARIANTS.includes(recommendation)) { - return new ListRuleBan(entity, reason, kind); + return new PolicyRuleBan(entity, reason, kind); } else if (RECOMMENDATION_OPINION_VARIANTS.includes(recommendation)) { let opinion = content['opinion']; if (!Number.isInteger(opinion)) { return null; } - return new ListRuleOpinion(entity, reason, kind, opinion); + return new PolicyRuleOpinion(entity, reason, kind, opinion); } else { // As long as the `recommendation` is defined, we assume // that the rule is correct, just unknown. - return new ListRuleUnknown(entity, reason, kind, content); + return new PolicyRuleUnknown(entity, reason, kind, content); } } } @@ -173,7 +173,7 @@ export abstract class ListRule { /** * A rule representing a "ban". */ -export class ListRuleBan extends ListRule { +export class PolicyRuleBan extends PolicyRule { constructor( /** * The entity covered by this rule, e.g. a glob user ID, a room ID, a server domain. @@ -195,7 +195,7 @@ export class ListRuleBan extends ListRule { /** * A rule representing an "opinion" */ -export class ListRuleOpinion extends ListRule { +export class PolicyRuleOpinion extends PolicyRule { constructor( /** * The entity covered by this rule, e.g. a glob user ID, a room ID, a server domain. @@ -229,7 +229,7 @@ export class ListRuleOpinion extends ListRule { /** * Any list rule that we do not understand. */ -export class ListRuleUnknown extends ListRule { +export class PolicyRuleUnknown extends PolicyRule { constructor( /** * The entity covered by this rule, e.g. a glob user ID, a room ID, a server domain. diff --git a/src/models/RuleServer.ts b/src/models/RuleServer.ts index 515c131e..6d92941a 100644 --- a/src/models/RuleServer.ts +++ b/src/models/RuleServer.ts @@ -13,10 +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. */ -import BanList, { ChangeType, ListRuleChange } from "./PolicyList" +import BanList, { ChangeType, PolicyRuleChange } from "./PolicyList" import * as crypto from "crypto"; import { LogService } from "matrix-bot-sdk"; -import { EntityType, ListRule } from "./ListRule"; +import { EntityType, PolicyRule } from "./PolicyRule"; import PolicyList from "./PolicyList"; export const USER_MAY_INVITE = 'user_may_invite'; @@ -149,10 +149,10 @@ export default class RuleServer { } /** - * Update the rule server to reflect a ListRule change. - * @param change A ListRuleChange sourced from a BanList. + * Update the rule server to reflect a PolicyRule change. + * @param change A PolicyRuleChange sourced from a BanList. */ - private applyRuleChange(change: ListRuleChange): void { + private applyRuleChange(change: PolicyRuleChange): void { if (change.changeType === ChangeType.Added) { const eventRules = new EventRules(change.event.event_id, change.event.room_id, toRuleServerFormat(change.rule), this.currentToken); this.addEventRules(eventRules); @@ -208,9 +208,9 @@ export default class RuleServer { * Process the changes that have been made to a BanList. * This will ususally be called as a callback from `BanList.onChange`. * @param banList The BanList that the changes happened in. - * @param changes An array of ListRuleChanges. + * @param changes An array of PolicyRuleChanges. */ - private update(banList: BanList, changes: ListRuleChange[]) { + private update(banList: BanList, changes: PolicyRuleChange[]) { if (changes.length > 0) { this.nextToken(); changes.forEach(this.applyRuleChange, this); @@ -257,11 +257,11 @@ export default class RuleServer { } /** -* Convert a ListRule into the format that can be served by the rule server. -* @param policyRule A ListRule to convert. +* Convert a PolicyRule into the format that can be served by the rule server. +* @param policyRule A PolicyRule to convert. * @returns An array of rules that can be served from the rule server. */ -function toRuleServerFormat(policyRule: ListRule): RuleServerRule[] { +function toRuleServerFormat(policyRule: PolicyRule): RuleServerRule[] { function makeLiteral(literal: string) { return { literal } } diff --git a/test/integration/banListTest.ts b/test/integration/banListTest.ts index 349aa375..a04ed8f7 100644 --- a/test/integration/banListTest.ts +++ b/test/integration/banListTest.ts @@ -3,11 +3,11 @@ import { strict as assert } from "assert"; import config from "../../src/config"; import { newTestUser } from "./clientHelper"; import { LogService, MatrixClient, Permalinks, UserID } from "matrix-bot-sdk"; -import PolicyList, { ChangeType, ListRuleChange } from "../../src/models/PolicyList"; +import PolicyList, { ChangeType, PolicyRuleChange } from "../../src/models/PolicyList"; import { ServerAcl } from "../../src/models/ServerAcl"; import { getFirstReaction } from "./commands/commandUtils"; import { getMessagesByUserIn } from "../../src/utils"; -import { ALL_RULE_TYPES, RULE_SERVER, RULE_USER, SERVER_RULE_TYPES } from "../../src/models/ListRule"; +import { ALL_RULE_TYPES, RULE_SERVER, RULE_USER, SERVER_RULE_TYPES } from "../../src/models/PolicyRule"; /** * Create a policy rule in a policy room. @@ -40,7 +40,7 @@ describe("Test: Updating the PolicyList", function() { // Test adding a new rule await createPolicyRule(mjolnir, banListId, RULE_USER, '@added:localhost:9999', ''); - let changes: ListRuleChange[] = await banList.updateList(); + let changes: PolicyRuleChange[] = await banList.updateList(); assert.equal(changes.length, 1, 'There should only be one change'); assert.equal(changes[0].changeType, ChangeType.Added); assert.equal(changes[0].sender, await mjolnir.getUserId()); @@ -187,7 +187,7 @@ describe("Test: Updating the PolicyList", function() { for (let i = 0; i < ALL_RULE_TYPES.length; i++) { await createPolicyRule(mjolnir, banListId, ALL_RULE_TYPES[i], `*${i}*`, ''); } - let changes: ListRuleChange[] = await banList.updateList(); + let changes: PolicyRuleChange[] = await banList.updateList(); assert.equal(changes.length, ALL_RULE_TYPES.length); assert.equal(banList.allRules.length, ALL_RULE_TYPES.length); }) @@ -199,7 +199,7 @@ describe('Test: We do not respond to recommendations other than m.ban in the ban const banListId = await mjolnir.createRoom(); const banList = new PolicyList(banListId, banListId, mjolnir); await createPolicyRule(mjolnir, banListId, RULE_SERVER, 'exmaple.org', '', { recommendation: 'something that is not m.ban' }); - let changes: ListRuleChange[] = await banList.updateList(); + let changes: PolicyRuleChange[] = await banList.updateList(); assert.equal(changes.length, 1, 'There should only be one change'); assert.equal(changes[0].changeType, ChangeType.Added); assert.equal(changes[0].sender, await mjolnir.getUserId()); @@ -219,7 +219,7 @@ describe('Test: We will not be able to ban ourselves via ACL.', function() { await createPolicyRule(mjolnir, banListId, RULE_SERVER, 'evil.com', ''); await createPolicyRule(mjolnir, banListId, RULE_SERVER, '*', ''); // We should still intern the matching rules rule. - let changes: ListRuleChange[] = await banList.updateList(); + let changes: PolicyRuleChange[] = await banList.updateList(); assert.equal(banList.serverRules.length, 3); // But when we construct an ACL, we should be safe. const acl = new ServerAcl(serverName) From 14a83974970e67997ad6745460b4aaf2b2c5ffd3 Mon Sep 17 00:00:00 2001 From: David Teller Date: Wed, 27 Jul 2022 12:57:57 +0200 Subject: [PATCH 2/6] WIP: More strict typing --- src/models/PolicyList.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/models/PolicyList.ts b/src/models/PolicyList.ts index 9e9faa47..bd0f6bd3 100644 --- a/src/models/PolicyList.ts +++ b/src/models/PolicyList.ts @@ -117,10 +117,10 @@ class PolicyList extends EventEmitter { /** * Return all the active rules of a given kind. - * @param kind e.g. RULE_SERVER (m.policy.rule.server). Rule types are always normalised when they are interned into the PolicyList. + * @param kind The type of entities we're looking for. * @returns The active PolicyRules for the ban list of that kind. */ - private rulesOfKind(kind: string): PolicyRule[] { + private rulesOfKind(kind: EntityType): PolicyRule[] { const rules: PolicyRule[] = [] const stateKeyMap = this.state.get(kind); if (stateKeyMap) { @@ -169,16 +169,16 @@ class PolicyList extends EventEmitter { * @param entity The entity to test e.g. the user id, server name or a room id. * @returns All of the rules that match this entity. */ - public rulesMatchingEntity(entity: string, ruleKind?: string): PolicyRule[] { - const ruleTypeOf: (entityPart: string) => string = (entityPart: string) => { + public rulesMatchingEntity(entity: string, ruleKind?: EntityType): PolicyRule[] { + const ruleTypeOf: (entityPart: string) => EntityType = (entityPart: string) => { if (ruleKind) { return ruleKind; - } else if (entityPart.startsWith("#") || entityPart.startsWith("#")) { - return RULE_ROOM; + } else if (entityPart.startsWith("#") || entityPart.startsWith("!")) { + return EntityType.RULE_ROOM; } else if (entity.startsWith("@")) { - return RULE_USER; + return EntityType.RULE_USER; } else { - return RULE_SERVER; + return EntityType.RULE_SERVER; } }; From 81eba65ae384d52e65d23f9d5deecfd5665adbd1 Mon Sep 17 00:00:00 2001 From: David Teller Date: Wed, 27 Jul 2022 13:13:24 +0200 Subject: [PATCH 3/6] WIP: PolicyList should now fully support opinions --- src/actions/ApplyAcl.ts | 3 +- src/actions/ApplyBan.ts | 3 +- src/commands/DumpRulesCommand.ts | 8 +++--- src/commands/StatusCommand.ts | 6 ++-- src/models/PolicyList.ts | 36 +++++++++++++----------- test/integration/banListTest.ts | 48 ++++++++++++++++---------------- 6 files changed, 54 insertions(+), 50 deletions(-) diff --git a/src/actions/ApplyAcl.ts b/src/actions/ApplyAcl.ts index 44767ce0..84192548 100644 --- a/src/actions/ApplyAcl.ts +++ b/src/actions/ApplyAcl.ts @@ -21,6 +21,7 @@ import { Mjolnir } from "../Mjolnir"; import config from "../config"; import { LogLevel, UserID } from "matrix-bot-sdk"; import { ERROR_KIND_FATAL, ERROR_KIND_PERMISSION } from "../ErrorCache"; +import { Recommendation } from "../models/PolicyRule"; /** * Applies the server ACLs represented by the ban lists to the provided rooms, returning the @@ -36,7 +37,7 @@ export async function applyServerAcls(lists: PolicyList[], roomIds: string[], mj // Construct a server ACL first const acl = new ServerAcl(serverName).denyIpAddresses().allowServer("*"); for (const list of lists) { - for (const rule of list.serverRules) { + for (const rule of list.getServerRules(Recommendation.Ban)) { acl.denyServer(rule.entity); } } diff --git a/src/actions/ApplyBan.ts b/src/actions/ApplyBan.ts index c76d651a..fa7226e7 100644 --- a/src/actions/ApplyBan.ts +++ b/src/actions/ApplyBan.ts @@ -20,6 +20,7 @@ import { Mjolnir } from "../Mjolnir"; import config from "../config"; import { LogLevel } from "matrix-bot-sdk"; import { ERROR_KIND_FATAL, ERROR_KIND_PERMISSION } from "../ErrorCache"; +import { Recommendation } from "../models/PolicyRule"; /** * Applies the member bans represented by the ban lists to the provided rooms, returning the @@ -57,7 +58,7 @@ export async function applyUserBans(lists: PolicyList[], roomIds: string[], mjol let banned = false; for (const list of lists) { - for (const userRule of list.userRules) { + for (const userRule of list.getUserRules(Recommendation.Ban)) { if (userRule.isMatch(member.userId)) { // User needs to be banned diff --git a/src/commands/DumpRulesCommand.ts b/src/commands/DumpRulesCommand.ts index 03d4f58a..5f79ca66 100644 --- a/src/commands/DumpRulesCommand.ts +++ b/src/commands/DumpRulesCommand.ts @@ -33,7 +33,7 @@ export async function execRulesMatchingCommand(roomId: string, event: any, mjoln let html = ""; let text = ""; for (const list of mjolnir.lists) { - const matches = list.rulesMatchingEntity(entity) + const matches = list.rulesMatchingEntity(entity, "*"); if (matches.length === 0) { continue; @@ -90,19 +90,19 @@ export async function execDumpRulesCommand(roomId: string, event: any, mjolnir: html += `${list.roomId}${shortcodeInfo}:
    `; text += `${list.roomRef}${shortcodeInfo}:\n`; - for (const rule of list.serverRules) { + for (const rule of list.getServerRules("*")) { hasRules = true; html += `
  • server (${rule.recommendation}): ${htmlEscape(rule.entity)} (${htmlEscape(rule.reason)})
    • `; text += `* server (${rule.recommendation}): ${rule.entity} (${rule.reason})\n`; } - for (const rule of list.userRules) { + for (const rule of list.getUserRules("*")) { hasRules = true; html += `
  • user (${rule.recommendation}): ${htmlEscape(rule.entity)} (${htmlEscape(rule.reason)})
    • `; text += `* user (${rule.recommendation}): ${rule.entity} (${rule.reason})\n`; } - for (const rule of list.roomRules) { + for (const rule of list.getRoomRules("*")) { hasRules = true; html += `
  • room (${rule.recommendation}): ${htmlEscape(rule.entity)} (${htmlEscape(rule.reason)})
    • `; text += `* room (${rule.recommendation}): ${rule.entity} (${rule.reason})\n`; diff --git a/src/commands/StatusCommand.ts b/src/commands/StatusCommand.ts index 987c1331..0fac1c7b 100644 --- a/src/commands/StatusCommand.ts +++ b/src/commands/StatusCommand.ts @@ -71,10 +71,10 @@ async function showMjolnirStatus(roomId: string, event: any, mjolnir: Mjolnir) { text += `Protected rooms: ${Object.keys(mjolnir.protectedRooms).length}\n`; // Append list information - html += "Subscribed ban lists:
      "; - text += "Subscribed ban lists:\n"; + html += "Subscribed policy lists:
        "; + text += "Subscribed policy lists:\n"; for (const list of mjolnir.lists) { - const ruleInfo = `rules: ${list.serverRules.length} servers, ${list.userRules.length} users, ${list.roomRules.length} rooms`; + const ruleInfo = `rules: ${list.getServerRules("*").length} servers, ${list.getUserRules("*").length} users, ${list.getRoomRules("*").length} rooms`; html += `
      • ${list.roomId} (${ruleInfo})
        • `; text += `* ${list.roomRef} (${ruleInfo})\n`; } diff --git a/src/models/PolicyList.ts b/src/models/PolicyList.ts index bd0f6bd3..f10e0417 100644 --- a/src/models/PolicyList.ts +++ b/src/models/PolicyList.ts @@ -117,20 +117,22 @@ class PolicyList extends EventEmitter { /** * Return all the active rules of a given kind. - * @param kind The type of entities we're looking for. + * @param type The type of entities we're looking for. * @returns The active PolicyRules for the ban list of that kind. */ - private rulesOfKind(kind: EntityType): PolicyRule[] { + private rulesOfKind(type: EntityType, recommendation: Recommendation | "*"): PolicyRule[] { const rules: PolicyRule[] = [] - const stateKeyMap = this.state.get(kind); + const stateKeyMap = this.state.get(type); if (stateKeyMap) { for (const event of stateKeyMap.values()) { const rule = event?.unsigned?.rule; // README! If you are refactoring this and/or introducing a mechanism to return the list of rules, // please make sure that you *only* return rules with `m.ban` or create a different method // (we don't want to accidentally ban entities). - if (rule && rule.kind === kind && rule.recommendation === Recommendation.Ban) { - rules.push(rule); + if (rule && rule.kind === type) { + if (recommendation === "*" || rule.recommendation === recommendation) { + rules.push(rule); + } } } } @@ -146,20 +148,20 @@ class PolicyList extends EventEmitter { }); } - public get serverRules(): PolicyRule[] { - return this.rulesOfKind(RULE_SERVER); + public getServerRules(recommendation: Recommendation | "*"): PolicyRule[] { + return this.rulesOfKind(RULE_SERVER, recommendation); } - public get userRules(): PolicyRule[] { - return this.rulesOfKind(RULE_USER); + public getUserRules(recommendation: Recommendation | "*"): PolicyRule[] { + return this.rulesOfKind(RULE_USER, recommendation); } - public get roomRules(): PolicyRule[] { - return this.rulesOfKind(RULE_ROOM); + public getRoomRules(recommendation: Recommendation | "*"): PolicyRule[] { + return this.rulesOfKind(RULE_ROOM, recommendation); } - public get allRules(): PolicyRule[] { - return [...this.serverRules, ...this.userRules, ...this.roomRules]; + public getAllRules(recommendation: Recommendation | "*"): PolicyRule[] { + return [...this.getServerRules(recommendation), ...this.getUserRules(recommendation), ...this.getRoomRules(recommendation)]; } /** @@ -169,7 +171,7 @@ class PolicyList extends EventEmitter { * @param entity The entity to test e.g. the user id, server name or a room id. * @returns All of the rules that match this entity. */ - public rulesMatchingEntity(entity: string, ruleKind?: EntityType): PolicyRule[] { + public rulesMatchingEntity(entity: string, recommendation: Recommendation | "*", ruleKind?: EntityType): PolicyRule[] { const ruleTypeOf: (entityPart: string) => EntityType = (entityPart: string) => { if (ruleKind) { return ruleKind; @@ -186,11 +188,11 @@ class PolicyList extends EventEmitter { // We special case because want to see whether a server ban is preventing this user from participating too. const userId = new UserID(entity); return [ - ...this.userRules.filter(rule => rule.isMatch(entity)), - ...this.serverRules.filter(rule => rule.isMatch(userId.domain)) + ...this.getUserRules(recommendation).filter(rule => rule.isMatch(entity)), + ...this.getServerRules(recommendation).filter(rule => rule.isMatch(userId.domain)) ] } else { - return this.rulesOfKind(ruleTypeOf(entity)).filter(rule => rule.isMatch(entity)); + return this.rulesOfKind(ruleTypeOf(entity), recommendation).filter(rule => rule.isMatch(entity)); } } diff --git a/test/integration/banListTest.ts b/test/integration/banListTest.ts index a04ed8f7..86b1993d 100644 --- a/test/integration/banListTest.ts +++ b/test/integration/banListTest.ts @@ -36,7 +36,7 @@ describe("Test: Updating the PolicyList", function() { const banList = new PolicyList(banListId, banListId, mjolnir); mjolnir.setUserPowerLevel(await moderator.getUserId(), banListId, 100); - assert.equal(banList.allRules.length, 0); + assert.equal(banList.getAllRules("*").length, 0); // Test adding a new rule await createPolicyRule(mjolnir, banListId, RULE_USER, '@added:localhost:9999', ''); @@ -44,8 +44,8 @@ describe("Test: Updating the PolicyList", function() { assert.equal(changes.length, 1, 'There should only be one change'); assert.equal(changes[0].changeType, ChangeType.Added); assert.equal(changes[0].sender, await mjolnir.getUserId()); - assert.equal(banList.userRules.length, 1); - assert.equal(banList.allRules.length, 1); + assert.equal(banList.getUserRules("*").length, 1); + assert.equal(banList.getAllRules("*").length, 1); // Test modifiying a rule let originalEventId = await createPolicyRule(mjolnir, banListId, RULE_USER, '@modified:localhost:9999', ''); @@ -62,12 +62,12 @@ describe("Test: Updating the PolicyList", function() { assert.equal(changes[0].changeType, ChangeType.Modified); assert.equal(changes[0].previousState['event_id'], modifyingEventId, 'There should be a previous state event for a modified rule'); assert.equal(changes[0].event['event_id'], modifyingAgainEventId); - assert.equal(banList.userRules.length, 2, 'There should be two rules, one for @modified:localhost:9999 and one for @added:localhost:9999'); + assert.equal(banList.getUserRules("*").length, 2, 'There should be two rules, one for @modified:localhost:9999 and one for @added:localhost:9999'); // Test redacting a rule const redactThis = await createPolicyRule(mjolnir, banListId, RULE_USER, '@redacted:localhost:9999', ''); await banList.updateList(); - assert.equal(banList.userRules.filter(r => r.entity === '@redacted:localhost:9999').length, 1); + assert.equal(banList.getUserRules("*").filter(r => r.entity === '@redacted:localhost:9999').length, 1); await mjolnir.redactEvent(banListId, redactThis); changes = await banList.updateList(); assert.equal(changes.length, 1); @@ -76,13 +76,13 @@ describe("Test: Updating the PolicyList", function() { assert.equal(Object.keys(changes[0].event['content']).length, 0, 'Should show the new version of the event with redacted content'); assert.notEqual(Object.keys(changes[0].previousState['content']), 0, 'Should have a copy of the unredacted state'); assert.notEqual(changes[0].rule, undefined, 'The previous rule should be present'); - assert.equal(banList.userRules.filter(r => r.entity === '@redacted:localhost:9999').length, 0, 'The rule should be removed.'); + assert.equal(banList.getUserRules("*").filter(r => r.entity === '@redacted:localhost:9999').length, 0, 'The rule should be removed.'); // Test soft redaction of a rule const softRedactedEntity = '@softredacted:localhost:9999' await createPolicyRule(mjolnir, banListId, RULE_USER, softRedactedEntity, ''); await banList.updateList(); - assert.equal(banList.userRules.filter(r => r.entity === softRedactedEntity).length, 1); + assert.equal(banList.getUserRules("*").filter(r => r.entity === softRedactedEntity).length, 1); await mjolnir.sendStateEvent(banListId, RULE_USER, `rule:${softRedactedEntity}`, {}); changes = await banList.updateList(); assert.equal(changes.length, 1); @@ -90,34 +90,34 @@ describe("Test: Updating the PolicyList", function() { assert.equal(Object.keys(changes[0].event['content']).length, 0, 'Should show the new version of the event with redacted content'); assert.notEqual(Object.keys(changes[0].previousState['content']), 0, 'Should have a copy of the unredacted state'); assert.notEqual(changes[0].rule, undefined, 'The previous rule should be present'); - assert.equal(banList.userRules.filter(r => r.entity === softRedactedEntity).length, 0, 'The rule should have been removed'); + assert.equal(banList.getUserRules("*").filter(r => r.entity === softRedactedEntity).length, 0, 'The rule should have been removed'); // Now test a double soft redaction just to make sure stuff doesn't explode await mjolnir.sendStateEvent(banListId, RULE_USER, `rule:${softRedactedEntity}`, {}); changes = await banList.updateList(); assert.equal(changes.length, 0, "It shouldn't detect a double soft redaction as a change, it should be seen as adding an invalid rule."); - assert.equal(banList.userRules.filter(r => r.entity === softRedactedEntity).length, 0, 'The rule should have been removed'); + assert.equal(banList.getUserRules("*").filter(r => r.entity === softRedactedEntity).length, 0, 'The rule should have been removed'); // Test that different (old) rule types will be modelled as the latest event type. originalEventId = await createPolicyRule(mjolnir, banListId, 'org.matrix.mjolnir.rule.user', '@old:localhost:9999', ''); changes = await banList.updateList(); assert.equal(changes.length, 1); assert.equal(changes[0].changeType, ChangeType.Added); - assert.equal(banList.userRules.filter(r => r.entity === '@old:localhost:9999').length, 1); + assert.equal(banList.getUserRules("*").filter(r => r.entity === '@old:localhost:9999').length, 1); modifyingEventId = await createPolicyRule(mjolnir, banListId, 'm.room.rule.user', '@old:localhost:9999', 'modified reason'); changes = await banList.updateList(); assert.equal(changes.length, 1); assert.equal(changes[0].changeType, ChangeType.Modified); assert.equal(changes[0].event['event_id'], modifyingEventId); assert.equal(changes[0].previousState['event_id'], originalEventId, 'There should be a previous state event for a modified rule'); - assert.equal(banList.userRules.filter(r => r.entity === '@old:localhost:9999').length, 1); + assert.equal(banList.getUserRules("*").filter(r => r.entity === '@old:localhost:9999').length, 1); modifyingAgainEventId = await createPolicyRule(mjolnir, banListId, RULE_USER, '@old:localhost:9999', 'changes again'); changes = await banList.updateList(); assert.equal(changes.length, 1); assert.equal(changes[0].changeType, ChangeType.Modified); assert.equal(changes[0].event['event_id'], modifyingAgainEventId); assert.equal(changes[0].previousState['event_id'], modifyingEventId, 'There should be a previous state event for a modified rule'); - assert.equal(banList.userRules.filter(r => r.entity === '@old:localhost:9999').length, 1); + assert.equal(banList.getUserRules("*").filter(r => r.entity === '@old:localhost:9999').length, 1); }) it("Will remove rules with old types when they are 'soft redacted' with a different but more recent event type.", async function() { this.timeout(3000); @@ -132,14 +132,14 @@ describe("Test: Updating the PolicyList", function() { let changes = await banList.updateList(); assert.equal(changes.length, 1); assert.equal(changes[0].changeType, ChangeType.Added); - assert.equal(banList.userRules.filter(rule => rule.entity === entity).length, 1, 'There should be a rule stored that we just added...') + assert.equal(banList.getUserRules("*").filter(rule => rule.entity === entity).length, 1, 'There should be a rule stored that we just added...') let softRedactingEventId = await mjolnir.sendStateEvent(banListId, RULE_USER, `rule:${entity}`, {}); changes = await banList.updateList(); assert.equal(changes.length, 1); assert.equal(changes[0].changeType, ChangeType.Removed); assert.equal(changes[0].event['event_id'], softRedactingEventId); assert.equal(changes[0].previousState['event_id'], originalEventId, 'There should be a previous state event for a modified rule'); - assert.equal(banList.userRules.filter(rule => rule.entity === entity).length, 0, 'The rule should no longer be stored.'); + assert.equal(banList.getUserRules("*").filter(rule => rule.entity === entity).length, 0, 'The rule should no longer be stored.'); }) it("A rule of the most recent type won't be deleted when an old rule is deleted for the same entity.", async function() { this.timeout(3000); @@ -154,7 +154,7 @@ describe("Test: Updating the PolicyList", function() { let changes = await banList.updateList(); assert.equal(changes.length, 1); assert.equal(changes[0].changeType, ChangeType.Added); - assert.equal(banList.userRules.filter(rule => rule.entity === entity).length, 1, 'There should be a rule stored that we just added...') + assert.equal(banList.getUserRules("*").filter(rule => rule.entity === entity).length, 1, 'There should be a rule stored that we just added...') let updatedEventId = await createPolicyRule(mjolnir, banListId, RULE_USER, entity, ''); changes = await banList.updateList(); // If in the future you change this and it fails, it's really subjective whether this constitutes a modification, since the only thing that has changed @@ -163,13 +163,13 @@ describe("Test: Updating the PolicyList", function() { assert.equal(changes[0].changeType, ChangeType.Modified); assert.equal(changes[0].event['event_id'], updatedEventId); assert.equal(changes[0].previousState['event_id'], originalEventId, 'There should be a previous state event for a modified rule'); - assert.equal(banList.userRules.filter(rule => rule.entity === entity).length, 1, 'Only the latest version of the rule gets returned.'); + assert.equal(banList.getUserRules("*").filter(rule => rule.entity === entity).length, 1, 'Only the latest version of the rule gets returned.'); // Now we delete the old version of the rule without consequence. await mjolnir.sendStateEvent(banListId, 'm.room.rule.user', `rule:${entity}`, {}); changes = await banList.updateList(); assert.equal(changes.length, 0); - assert.equal(banList.userRules.filter(rule => rule.entity === entity).length, 1, 'The rule should still be active.'); + assert.equal(banList.getUserRules("*").filter(rule => rule.entity === entity).length, 1, 'The rule should still be active.'); // And we can still delete the new version of the rule. let softRedactingEventId = await mjolnir.sendStateEvent(banListId, RULE_USER, `rule:${entity}`, {}); @@ -178,7 +178,7 @@ describe("Test: Updating the PolicyList", function() { assert.equal(changes[0].changeType, ChangeType.Removed); assert.equal(changes[0].event['event_id'], softRedactingEventId); assert.equal(changes[0].previousState['event_id'], updatedEventId, 'There should be a previous state event for a modified rule'); - assert.equal(banList.userRules.filter(rule => rule.entity === entity).length, 0, 'The rule should no longer be stored.'); + assert.equal(banList.getUserRules("*").filter(rule => rule.entity === entity).length, 0, 'The rule should no longer be stored.'); }) it('Test: PolicyList Supports all entity types.', async function() { const mjolnir = config.RUNTIME.client! @@ -189,7 +189,7 @@ describe("Test: Updating the PolicyList", function() { } let changes: PolicyRuleChange[] = await banList.updateList(); assert.equal(changes.length, ALL_RULE_TYPES.length); - assert.equal(banList.allRules.length, ALL_RULE_TYPES.length); + assert.equal(banList.getAllRules("*").length, ALL_RULE_TYPES.length); }) }); @@ -204,8 +204,8 @@ describe('Test: We do not respond to recommendations other than m.ban in the ban assert.equal(changes[0].changeType, ChangeType.Added); assert.equal(changes[0].sender, await mjolnir.getUserId()); // We really don't want things that aren't m.ban to end up being accessible in these APIs. - assert.equal(banList.serverRules.length, 0, `We should have an empty serverRules, got ${JSON.stringify(banList.serverRules)}`); - assert.equal(banList.allRules.length, 0, `We should have an empty allRules, got ${JSON.stringify(banList.allRules)}`); + assert.equal(banList.getServerRules("*").length, 0, `We should have an empty serverRules, got ${JSON.stringify(banList.getServerRules("*"))}`); + assert.equal(banList.getAllRules("*").length, 0, `We should have an empty allRules, got ${JSON.stringify(banList.getAllRules("*"))}`); }) }) @@ -220,7 +220,7 @@ describe('Test: We will not be able to ban ourselves via ACL.', function() { await createPolicyRule(mjolnir, banListId, RULE_SERVER, '*', ''); // We should still intern the matching rules rule. let changes: PolicyRuleChange[] = await banList.updateList(); - assert.equal(banList.serverRules.length, 3); + assert.equal(banList.getServerRules("*").length, 3); // But when we construct an ACL, we should be safe. const acl = new ServerAcl(serverName) changes.forEach(change => acl.denyServer(change.rule.entity)); @@ -335,7 +335,7 @@ describe('Test: unbaning entities via the PolicyList.', function() { await banList.updateList(); // rules are normalized, that's why there should only be 2. - assert.equal(banList.allRules.length, 2); + assert.equal(banList.getAllRules("*").length, 2); // Check that we have setup our test properly and therefore evil.com is banned. const acl = new ServerAcl(serverName).denyIpAddresses().allowServer("*").denyServer(olderBadServer).denyServer(newerBadServer); @@ -360,7 +360,7 @@ describe('Test: unbaning entities via the PolicyList.', function() { await this.mjolnir!.syncLists(); // Confirm that the server is unbanned. await banList.updateList(); - assert.equal(banList.allRules.length, 0); + assert.equal(banList.getAllRules("*").length, 0); const aclAfter = await mjolnir.getRoomStateEvent(protectedRoom, "m.room.server_acl", ""); assert.equal(aclAfter.deny.length, 0, 'Should be no servers denied anymore'); }) From 9425118b60d66bba8e06666e31d45bda47307d82 Mon Sep 17 00:00:00 2001 From: David Teller Date: Thu, 28 Jul 2022 16:39:18 +0200 Subject: [PATCH 4/6] WIP: Computing opinions --- src/Mjolnir.ts | 8 +-- src/commands/UnbanBanCommand.ts | 2 +- src/models/PolicyList.ts | 93 +++++++++++++++++++++++++++------ src/models/PolicyRule.ts | 45 ++++++++++++---- 4 files changed, 117 insertions(+), 31 deletions(-) diff --git a/src/Mjolnir.ts b/src/Mjolnir.ts index c3a4496d..ce6c6b0e 100644 --- a/src/Mjolnir.ts +++ b/src/Mjolnir.ts @@ -355,7 +355,7 @@ export class Mjolnir { this.currentState = STATE_SYNCING; if (config.syncOnStartup) { await this.logMessage(LogLevel.INFO, "Mjolnir@startup", "Syncing lists..."); - await this.syncLists(config.verboseLogging); + await this.syncPolicyLists(config.verboseLogging); await this.registerProtections(); } @@ -434,7 +434,7 @@ export class Mjolnir { const rooms = (additionalProtectedRooms?.rooms ?? []); rooms.push(roomId); await this.client.setAccountData(PROTECTED_ROOMS_EVENT_TYPE, { rooms: rooms }); - await this.syncLists(config.verboseLogging); + await this.syncPolicyLists(config.verboseLogging); } public async removeProtectedRoom(roomId: string) { @@ -482,7 +482,7 @@ export class Mjolnir { this.applyUnprotectedRooms(); if (withSync) { - await this.syncLists(config.verboseLogging); + await this.syncPolicyLists(config.verboseLogging); } } @@ -882,7 +882,7 @@ export class Mjolnir { * 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) { + public async syncPolicyLists(verbose = true) { for (const list of this.policyLists) { const changes = await list.updateList(); await this.printBanlistChanges(changes, list, true); diff --git a/src/commands/UnbanBanCommand.ts b/src/commands/UnbanBanCommand.ts index 3b0ba185..774828a9 100644 --- a/src/commands/UnbanBanCommand.ts +++ b/src/commands/UnbanBanCommand.ts @@ -163,7 +163,7 @@ export async function execUnbanCommand(roomId: string, event: any, mjolnir: Mjol if (unbannedSomeone) { await mjolnir.logMessage(LogLevel.DEBUG, "UnbanBanCommand", `Syncing lists to ensure no users were accidentally unbanned`); - await mjolnir.syncLists(config.verboseLogging); + await mjolnir.syncPolicyLists(config.verboseLogging); } } diff --git a/src/models/PolicyList.ts b/src/models/PolicyList.ts index f10e0417..e32b835e 100644 --- a/src/models/PolicyList.ts +++ b/src/models/PolicyList.ts @@ -16,7 +16,7 @@ limitations under the License. import { extractRequestError, LogService, MatrixClient, UserID } from "matrix-bot-sdk"; import { EventEmitter } from "events"; -import { ALL_RULE_TYPES, EntityType, PolicyRule, Recommendation, ROOM_RULE_TYPES, RULE_ROOM, RULE_SERVER, RULE_USER, SERVER_RULE_TYPES, USER_RULE_TYPES } from "./PolicyRule"; +import { ALL_RULE_TYPES, EntityType, PolicyRule, PolicyRuleOpinion, Recommendation, ROOM_RULE_TYPES, RULE_ROOM, RULE_SERVER, RULE_USER, SERVER_RULE_TYPES, USER_RULE_TYPES } from "./PolicyRule"; export const SHORTCODE_EVENT_TYPE = "org.matrix.mjolnir.shortcode"; @@ -61,8 +61,13 @@ declare interface PolicyList { } /** - * The PolicyList caches all of the rules that are active in a policy room so Mjolnir can refer to when applying bans etc. - * This cannot be used to update events in the modeled room, it is a readonly model of the policy room. + * A readonly view of the rules within a policy room. + * + * The interpretation of these rules is left to the client of this API, so e.g. a `m.ban` within a `PolicyList` + * could represent a room ban or a server ban. + * + * To update events in the policy room, send `m.policy.rule.*` events into the room. This will (eventually) cause + * a `/sync`, which will update the `PolicyList`. */ class PolicyList extends EventEmitter { private shortcode: string | null = null; @@ -167,24 +172,14 @@ class PolicyList extends EventEmitter { /** * Return all of the rules in this list that will match the provided entity. * If the entity is a user, then we match the domain part against server rules too. - * @param ruleKind The type of rule for the entity e.g. `RULE_USER`. + * @param ruleKind The type of rule for the entity e.g. `RULE_USER`. If unspecified, extract the type from `entity`. * @param entity The entity to test e.g. the user id, server name or a room id. * @returns All of the rules that match this entity. */ public rulesMatchingEntity(entity: string, recommendation: Recommendation | "*", ruleKind?: EntityType): PolicyRule[] { - const ruleTypeOf: (entityPart: string) => EntityType = (entityPart: string) => { - if (ruleKind) { - return ruleKind; - } else if (entityPart.startsWith("#") || entityPart.startsWith("!")) { - return EntityType.RULE_ROOM; - } else if (entity.startsWith("@")) { - return EntityType.RULE_USER; - } else { - return EntityType.RULE_SERVER; - } - }; + ruleKind = ruleKind || extractEntityType(entity); - if (ruleTypeOf(entity) === RULE_USER) { + if (ruleKind === RULE_USER) { // We special case because want to see whether a server ban is preventing this user from participating too. const userId = new UserID(entity); return [ @@ -192,10 +187,58 @@ class PolicyList extends EventEmitter { ...this.getServerRules(recommendation).filter(rule => rule.isMatch(userId.domain)) ] } else { - return this.rulesOfKind(ruleTypeOf(entity), recommendation).filter(rule => rule.isMatch(entity)); + return this.rulesOfKind(ruleKind, recommendation).filter(rule => rule.isMatch(entity)); } } + /** + * Return the opinion for a given entity. + * @returns The opinion specified by this list or `0` if no opinion was defined. + */ + public opinionForEntity(entity: string, ruleKind?: EntityType): number { + ruleKind = ruleKind || extractEntityType(entity); + + // Find all rules for this entity. + const allRules = this.rulesMatchingEntity(entity, Recommendation.Opinion, ruleKind); + + // Split across server rules and specialized rules. + const byGenericity: PolicyRule[][] = [ + /* User/room rules without wildcards */ [], + /* User/room rules with wildcards */ [], + /* Server rules without wildcards */ [], + /* Server rules with wildcards */ [], + ]; + for (let rule of allRules) { + const baseIndex = rule.kind === EntityType.RULE_SERVER ? + 2 : 0; + const finalIndex = rule.isGeneric ? + baseIndex + 1 : baseIndex; + byGenericity[finalIndex].push(rule); + } + + // Find the most specific rule for this entity. + // If there is more than one, pick the most recent entry. + for (let rules of byGenericity) { + let mostRecentRule = null; + for (let rule of rules) { + if (!mostRecentRule || rule.ts > mostRecentRule.ts) { + mostRecentRule = rule; + } + } + if (mostRecentRule) { + if (mostRecentRule instanceof PolicyRuleOpinion) { + return mostRecentRule.opinion; + } else { + // This should be impossible. + throw new TypeError(); + } + } + } + + // By default, the opinion is 0. + return 0; + } + /** * Remove all rules in the banList for this entity that have the same state key (as when we ban them) * by searching for rules that have legacy state types. @@ -404,3 +447,19 @@ class UpdateBatcher { this.checkBatch(eventId); } } + +/** + * Extract the entity type best matching a given entity. + * + * @param entity If this entity is a user id, return `RULE_USER`. If it is a room id or alias, return `RULE_ROOM`. + * Otherwise, return `RULE_SERVER`. + */ +function extractEntityType(entity: string): EntityType { + if (entity.startsWith("#") || entity.startsWith("!")) { + return EntityType.RULE_ROOM; + } else if (entity.startsWith("@")) { + return EntityType.RULE_ROOM; + } else { + return EntityType.RULE_SERVER; + } +} \ No newline at end of file diff --git a/src/models/PolicyRule.ts b/src/models/PolicyRule.ts index 75245e88..44fa058f 100644 --- a/src/models/PolicyRule.ts +++ b/src/models/PolicyRule.ts @@ -86,6 +86,11 @@ export abstract class PolicyRule { * A glob for `entity`. */ private glob: MatrixGlob; + + /** + * `true` if the rule contains at least one wildcard (`?` or `*`). + */ + public readonly isGeneric: boolean; constructor( /** * The entity covered by this rule, e.g. a glob user ID, a room ID, a server domain. @@ -95,6 +100,10 @@ export abstract class PolicyRule { * A human-readable reason for this rule, for audit purposes. */ public readonly reason: string, + /** + * A Matrix timestamp for the instant this rule was issued. + */ + public readonly ts: number, /** * The type of entity for this rule, e.g. user, server domain, etc. */ @@ -103,8 +112,10 @@ export abstract class PolicyRule { * The recommendation for this rule, e.g. "ban" or "opinion", or `null` * if the recommendation is one that Mjölnir doesn't understand. */ - public readonly recommendation: Recommendation | null) { + public readonly recommendation: Recommendation | null, + ) { this.glob = new MatrixGlob(entity); + this.isGeneric = entity.includes("?") || entity.includes("*"); } /** @@ -120,9 +131,13 @@ export abstract class PolicyRule { * @param event An *untrusted* event. * @returns null if the PolicyRule is invalid or not recognized by Mjölnir. */ - public static parse(event: {type: string, content: any}): PolicyRule | null { + public static parse(event: {type: string, origin_server_ts: number, content: any}): PolicyRule | null { // Parse common fields. // If a field is ill-formed, discard the rule. + const ts = event['origin_server_ts']; + if (!ts || typeof ts !== 'number') { + return null; + } const content = event['content']; if (!content || typeof content !== "object") { return null; @@ -155,17 +170,17 @@ export abstract class PolicyRule { // From this point, we may need specific fields. if (RECOMMENDATION_BAN_VARIANTS.includes(recommendation)) { - return new PolicyRuleBan(entity, reason, kind); + return new PolicyRuleBan(entity, reason, ts, kind); } else if (RECOMMENDATION_OPINION_VARIANTS.includes(recommendation)) { let opinion = content['opinion']; if (!Number.isInteger(opinion)) { return null; } - return new PolicyRuleOpinion(entity, reason, kind, opinion); + return new PolicyRuleOpinion(entity, reason, ts, kind, opinion); } else { // As long as the `recommendation` is defined, we assume // that the rule is correct, just unknown. - return new PolicyRuleUnknown(entity, reason, kind, content); + return new PolicyRuleUnknown(entity, reason, ts, kind, content); } } } @@ -183,12 +198,16 @@ export class PolicyRuleBan extends PolicyRule { * A human-readable reason for this rule, for audit purposes. */ reason: string, + /** + * A Matrix timestamp for the instant this rule was issued. + */ + ts: number, /** * The type of entity for this rule, e.g. user, server domain, etc. */ kind: EntityType, ) { - super(entity, reason, kind, Recommendation.Ban) + super(entity, reason, ts, kind, Recommendation.Ban) } } @@ -205,6 +224,10 @@ export class PolicyRuleOpinion extends PolicyRule { * A human-readable reason for this rule, for audit purposes. */ reason: string, + /** + * A Matrix timestamp for the instant this rule was issued. + */ + ts: number, /** * The type of entity for this rule, e.g. user, server domain, etc. */ @@ -214,9 +237,9 @@ export class PolicyRuleOpinion extends PolicyRule { * on the entity (e.g. toxic user or community) and +100 represents the best * possible opinion on the entity (e.g. pillar of the community). */ - public readonly opinion: number + public readonly opinion: number, ) { - super(entity, reason, kind, Recommendation.Opinion); + super(entity, reason, ts, kind, Recommendation.Opinion); if (!Number.isInteger(opinion)) { throw new TypeError(`The opinion must be an integer, got ${opinion}`); } @@ -240,6 +263,10 @@ export class PolicyRuleUnknown extends PolicyRule { */ reason: string, /** + * A Matrix timestamp for the instant this rule was issued. + */ + ts: number, + /** * The type of entity for this rule, e.g. user, server domain, etc. */ kind: EntityType, @@ -248,6 +275,6 @@ export class PolicyRuleUnknown extends PolicyRule { */ public readonly content: any, ) { - super(entity, reason, kind, null); + super(entity, reason, ts, kind, null); } } From 646b349be874109e12f2e44f7640c43a131e33f5 Mon Sep 17 00:00:00 2001 From: David Teller Date: Thu, 28 Jul 2022 16:42:05 +0200 Subject: [PATCH 5/6] WIP --- src/commands/SyncCommand.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/commands/SyncCommand.ts b/src/commands/SyncCommand.ts index 0ebb1743..70eddd74 100644 --- a/src/commands/SyncCommand.ts +++ b/src/commands/SyncCommand.ts @@ -18,5 +18,5 @@ import { Mjolnir } from "../Mjolnir"; // !mjolnir sync export async function execSyncCommand(roomId: string, event: any, mjolnir: Mjolnir) { - return mjolnir.syncLists(); + return mjolnir.syncPolicyLists(); } From ce77b351d81c1ab242fe470ba185a3d1bd096e1d Mon Sep 17 00:00:00 2001 From: David Teller Date: Fri, 29 Jul 2022 17:04:19 +0200 Subject: [PATCH 6/6] WIP --- src/Mjolnir.ts | 23 ++++++++++++++++++++++- src/models/PolicyList.ts | 8 ++++---- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/src/Mjolnir.ts b/src/Mjolnir.ts index ce6c6b0e..71264791 100644 --- a/src/Mjolnir.ts +++ b/src/Mjolnir.ts @@ -27,7 +27,7 @@ import { TextualMessageEventContent } from "matrix-bot-sdk"; -import { ALL_RULE_TYPES as ALL_BAN_LIST_RULE_TYPES, RULE_ROOM, RULE_SERVER, RULE_USER } from "./models/PolicyRule"; +import { ALL_RULE_TYPES as ALL_BAN_LIST_RULE_TYPES, EntityType, RULE_ROOM, RULE_SERVER, RULE_USER } from "./models/PolicyRule"; import { applyServerAcls } from "./actions/ApplyAcl"; import { RoomUpdateError } from "./models/RoomUpdateError"; import { COMMAND_PREFIX, handleCommand } from "./commands/CommandHandler"; @@ -1194,4 +1194,25 @@ export class Mjolnir { await protection.handleReport(this, roomId, reporterId, event, reason); } } + + /** + * Return the composite opinion for an entity. + * + * In the current implementation, we return the first opinion found in the list + * of policies. Future versions will support additional mechanisms for composing + * opinions. + * + * @param entity + * @param type + * @returns The opinion or null if no list defines an opinion on this entity. + */ + public opinionForEntity(entity: string, type: EntityType): number | null { + for (let policyList of this.policyLists) { + let opinion = policyList.opinionForEntity(entity, type); + if (opinion !== null) { + return opinion; + } + } + return null; + } } diff --git a/src/models/PolicyList.ts b/src/models/PolicyList.ts index e32b835e..833f6682 100644 --- a/src/models/PolicyList.ts +++ b/src/models/PolicyList.ts @@ -193,9 +193,9 @@ class PolicyList extends EventEmitter { /** * Return the opinion for a given entity. - * @returns The opinion specified by this list or `0` if no opinion was defined. + * @returns The opinion specified by this list or `null` if no opinion was defined. */ - public opinionForEntity(entity: string, ruleKind?: EntityType): number { + public opinionForEntity(entity: string, ruleKind?: EntityType): number | null { ruleKind = ruleKind || extractEntityType(entity); // Find all rules for this entity. @@ -235,8 +235,8 @@ class PolicyList extends EventEmitter { } } - // By default, the opinion is 0. - return 0; + // By default, the opinion is null. + return null; } /**