diff --git a/src/ProtectedRooms.ts b/src/ProtectedRooms.ts index fee4230..797baed 100644 --- a/src/ProtectedRooms.ts +++ b/src/ProtectedRooms.ts @@ -18,10 +18,10 @@ import { LogLevel, LogService, MatrixClient, MatrixGlob, Permalinks, UserID } fr import { IConfig } from "./config"; import ErrorCache, { ERROR_KIND_FATAL, ERROR_KIND_PERMISSION } from "./ErrorCache"; import ManagementRoomOutput from "./ManagementRoomOutput"; +import AccessControlUnit, { Access } from "./models/AccessControlUnit"; import { RULE_ROOM, RULE_SERVER, RULE_USER } from "./models/ListRule"; import PolicyList, { ListRuleChange } from "./models/PolicyList"; import { RoomUpdateError } from "./models/RoomUpdateError"; -import { ServerAcl } from "./models/ServerAcl"; import { ProtectionManager } from "./protections/ProtectionManager"; import { EventRedactionQueue, RedactUserInRoom } from "./queues/EventRedactionQueue"; import { ProtectedRoomActivityTracker } from "./queues/ProtectedRoomActivityTracker"; @@ -82,6 +82,11 @@ export class ProtectedRooms { */ private aclChain: Promise = Promise.resolve(); + /** + * A utility to test the access that users have in the set of protected rooms according to the policies of the watched lists. + */ + private readonly accessControlUnit = new AccessControlUnit([]); + constructor( private readonly client: MatrixClient, private readonly clientUserId: string, @@ -136,11 +141,13 @@ export class ProtectedRooms { public watchList(policyList: PolicyList): void { if (!this.policyLists.includes(policyList)) { this.policyLists.push(policyList); + this.accessControlUnit.watchList(policyList); } } public unwatchList(policyList: PolicyList): void { this.policyLists = this.policyLists.filter(list => list.roomId !== policyList.roomId); + this.accessControlUnit.unwatchList(policyList); } /** @@ -182,7 +189,7 @@ export class ProtectedRooms { // 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 this.applyUserBans(this.policyLists, [roomId]); + const banErrors = await this.applyUserBans([roomId]); const redactionErrors = await this.processRedactionQueue(roomId); await this.printActionResult(banErrors); await this.printActionResult(redactionErrors); @@ -202,7 +209,7 @@ export class ProtectedRooms { let hadErrors = false; const [aclErrors, banErrors] = await Promise.all([ this.applyServerAcls(this.policyLists, this.protectedRoomsByActivity()), - this.applyUserBans(this.policyLists, this.protectedRoomsByActivity()) + this.applyUserBans(this.protectedRoomsByActivity()) ]); const redactionErrors = await this.processRedactionQueue(); hadErrors = hadErrors || await this.printActionResult(aclErrors, "Errors updating server ACLs:"); @@ -250,7 +257,7 @@ export class ProtectedRooms { let hadErrors = false; const [aclErrors, banErrors] = await Promise.all([ this.applyServerAcls(this.policyLists, this.protectedRoomsByActivity()), - this.applyUserBans(this.policyLists, this.protectedRoomsByActivity()) + this.applyUserBans(this.protectedRoomsByActivity()) ]); const redactionErrors = await this.processRedactionQueue(); hadErrors = hadErrors || await this.printActionResult(aclErrors, "Errors updating server ACLs:"); @@ -293,19 +300,9 @@ export class ProtectedRooms { const serverName: string = new UserID(await this.client.getUserId()).domain; // Construct a server ACL first - const acl = new ServerAcl(serverName).denyIpAddresses().allowServer("*"); - for (const list of lists) { - for (const rule of list.serverRules) { - acl.denyServer(rule.entity); - } - } - + const acl = this.accessControlUnit.compileServerAcl(serverName); const finalAcl = acl.safeAclContent(); - if (finalAcl.deny.length !== acl.literalAclContent().deny.length) { - this.managementRoomOutput.logMessage(LogLevel.WARN, "ApplyAcl", `Mjölnir has detected and removed an ACL that would exclude itself. Please check the ACL lists.`); - } - if (this.config.verboseLogging) { // We specifically use sendNotice to avoid having to escape HTML await this.client.sendNotice(this.managementRoomId, `Constructed server ACL:\n${JSON.stringify(finalAcl, null, 2)}`); @@ -346,11 +343,10 @@ export class ProtectedRooms { /** * Applies the member bans represented by the ban lists to the provided rooms, returning the * room IDs that could not be updated and their error. - * @param {PolicyList[]} lists The lists to determine bans from. * @param {string[]} roomIds The room IDs to apply the bans in. * @param {Mjolnir} mjolnir The Mjolnir client to apply the bans with. */ - private async applyUserBans(lists: PolicyList[], roomIds: string[]): Promise { + private async applyUserBans(roomIds: string[]): Promise { // We can only ban people who are not already banned, and who match the rules. const errors: RoomUpdateError[] = []; for (const roomId of roomIds) { @@ -377,29 +373,21 @@ export class ProtectedRooms { continue; // user already banned } - let banned = false; - for (const list of lists) { - for (const userRule of list.userRules) { - if (userRule.isMatch(member.userId)) { - // User needs to be banned + // We don't want to ban people based on server ACL as this would flood the room with bans. + const memberAccess = this.accessControlUnit.getAccessForUser(member.userId, "IGNORE_SERVER"); + if (memberAccess.outcome === Access.Banned) { + const reason = memberAccess.rule ? memberAccess.rule.reason : ''; + // We specifically use sendNotice to avoid having to escape HTML + await this.managementRoomOutput.logMessage(LogLevel.INFO, "ApplyBan", `Banning ${member.userId} in ${roomId} for: ${reason}`, roomId); - // We specifically use sendNotice to avoid having to escape HTML - await this.managementRoomOutput.logMessage(LogLevel.INFO, "ApplyBan", `Banning ${member.userId} in ${roomId} for: ${userRule.reason}`, roomId); - - if (!this.config.noop) { - await this.client.banUser(member.userId, roomId, userRule.reason); - if (this.automaticRedactGlobs.find(g => g.test(userRule.reason.toLowerCase()))) { - this.redactUser(member.userId, roomId); - } - } else { - await this.managementRoomOutput.logMessage(LogLevel.WARN, "ApplyBan", `Tried to ban ${member.userId} in ${roomId} but Mjolnir is running in no-op mode`, roomId); - } - - banned = true; - break; + 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); } + } else { + await this.managementRoomOutput.logMessage(LogLevel.WARN, "ApplyBan", `Tried to ban ${member.userId} in ${roomId} but Mjolnir is running in no-op mode`, roomId); } - if (banned) break; } } } catch (e) { diff --git a/src/models/AccessControlUnit.ts b/src/models/AccessControlUnit.ts new file mode 100644 index 0000000..add6adc --- /dev/null +++ b/src/models/AccessControlUnit.ts @@ -0,0 +1,314 @@ +/* +Copyright 2019-2022 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. +*/ + +import PolicyList, { ChangeType, ListRuleChange } from "./PolicyList"; +import { EntityType, ListRule, Recommendation, RULE_SERVER, RULE_USER } from "./ListRule"; +import { LogService, UserID } from "matrix-bot-sdk"; +import { ServerAcl } from "./ServerAcl"; + +/** + * The ListRuleCache is a cache for all the rules in a set of lists for a specific entity type and recommendation. + * The cache can then be used to quickly test against all the rules for that specific entity/recommendation. + * E.g. The cache can be used for all the m.ban rules for users in a set of lists to conveniently test members of a room. + * While some effort has been made to optimize the testing of entities, the main purpose of this class is to stop + * ad-hoc destructuring of policy lists to test rules against entities. + * + * Note: This cache should not be used to unban or introspect about the state of `PolicyLists`, for this + * see `PolicyList.unban` and `PolicyList.rulesMatchingEntity`, as these will make sure to account + * for unnormalized entity types. + */ +class ListRuleCache { + /** + * Glob rules always have to be scanned against every entity. + */ + private readonly globRules: Map = new Map(); + /** + * This table allows us to skip matching an entity against every literal. + */ + private readonly literalRules: Map = new Map(); + private readonly listUpdateListener: ((list: PolicyList, changes: ListRuleChange[]) => void); + + constructor( + /** + * The entity type that this cache is for e.g. RULE_USER. + */ + public readonly entityType: EntityType, + /** + * The recommendation that this cache is for e.g. m.ban (RECOMMENDATION_BAN). + */ + public readonly recommendation: Recommendation, + ) { + this.listUpdateListener = (list: PolicyList, changes: ListRuleChange[]) => this.updateCache(changes); + } + + /** + * Test the entitiy for the first matching rule out of all the watched lists. + * @param entity e.g. an mxid for a user, the server name for a server. + * @returns A single `ListRule` matching the entity. + */ + public getAnyRuleForEntity(entity: string): ListRule|null { + const literalRule = this.literalRules.get(entity); + if (literalRule !== undefined) { + return literalRule[0]; + } + for (const rule of this.globRules.values()) { + if (rule[0].isMatch(entity)) { + return rule[0]; + } + } + return null; + } + + /** + * Watch a list and add all its rules (and future rules) to the cache. + * Will automatically update with the list. + * @param list A PolicyList. + */ + public watchList(list: PolicyList): void { + list.on('PolicyList.update', this.listUpdateListener); + const rules = list.rulesOfKind(this.entityType, this.recommendation); + rules.forEach(this.internRule, this); + } + + /** + * Unwatch a list and remove all of its rules from the cache. + * Will stop updating the cache from this list. + * @param list A PolicyList. + */ + public unwatchList(list: PolicyList): void { + list.removeListener('PolicyList.update', this.listUpdateListener); + const rules = list.rulesOfKind(this.entityType, this.recommendation); + rules.forEach(this.uninternRule, this); + } + + /** + * @returns True when there are no rules in the cache. + */ + public isEmpty(): boolean { + return this.globRules.size + this.literalRules.size === 0; + } + + /** + * Returns all the rules in the cache, without duplicates from different lists. + */ + public get allRules(): ListRule[] { + return [...this.literalRules.values(), ...this.globRules.values()].map(rules => rules[0]); + } + + /** + * Remove a rule from the cache as it is now invalid. e.g. it was removed from a policy list. + * @param rule The rule to remove. + */ + private uninternRule(rule: ListRule) { + /** + * Remove a rule from the map, there may be rules from different lists in the cache. + * We don't want to invalidate those. + * @param map A map of entities to rules. + */ + const removeRuleFromMap = (map: Map) => { + const entry = map.get(rule.entity); + if (entry !== undefined) { + const newEntry = entry.filter(internedRule => internedRule.sourceEvent.event_id !== rule.sourceEvent.event_id); + if (newEntry.length === 0) { + map.delete(rule.entity); + } else { + map.set(rule.entity, newEntry); + } + } + }; + if (rule.isGlob()) { + removeRuleFromMap(this.globRules); + } else { + removeRuleFromMap(this.literalRules); + } + } + + /** + * Add a rule to the cache e.g. it was added to a policy list. + * @param rule The rule to add. + */ + private internRule(rule: ListRule) { + /** + * Add a rule to the map, there might be duplicates of this rule in other lists. + * @param map A map of entities to rules. + */ + const addRuleToMap = (map: Map) => { + const entry = map.get(rule.entity); + if (entry !== undefined) { + entry.push(rule); + } else { + map.set(rule.entity, [rule]); + } + } + if (rule.isGlob()) { + addRuleToMap(this.globRules); + } else { + addRuleToMap(this.literalRules); + } + } + + /** + * Update the cache for a single `ListRuleChange`. + * @param change The change made to a rule that was present in the policy list. + */ + private updateCacheForChange(change: ListRuleChange): void { + if (change.rule.kind !== this.entityType || change.rule.recommendation !== this.recommendation) { + return; + } + switch (change.changeType) { + case ChangeType.Added: + case ChangeType.Modified: + this.internRule(change.rule); + break; + case ChangeType.Removed: + this.uninternRule(change.rule); + break; + default: + throw new TypeError(`Uknown ListRule change type: ${change.changeType}`); + } + } + + /** + * Update the cache for a change in a policy list. + * @param changes The changes that were made to list rules since the last update to this policy list. + */ + private updateCache(changes: ListRuleChange[]) { + changes.forEach(this.updateCacheForChange, this); + } +} + +export enum Access { + /// The entity was explicitly banned by a policy list. + Banned, + /// The entity did not match any allow rule. + NotAllowed, + /// The user was allowed and didn't match any ban. + Allowed, +} + +/** + * A description of the access an entity has. + * If the access is `Banned`, then a single rule that bans the entity will be included. + */ +export interface EntityAccess { + readonly outcome: Access, + readonly rule?: ListRule, +} + +/** + * This allows us to work out the access an entity has to some thing based on a set of watched/unwatched lists. + */ +export default class AccessControlUnit { + private readonly userBans = new ListRuleCache(RULE_USER, Recommendation.Ban); + private readonly serverBans = new ListRuleCache(RULE_SERVER, Recommendation.Ban); + private readonly userAllows = new ListRuleCache(RULE_USER, Recommendation.Allow); + private readonly serverAllows = new ListRuleCache(RULE_SERVER, Recommendation.Allow); + private readonly caches = [this.userBans, this.serverBans, this.userAllows, this.serverAllows] + + constructor(policyLists: PolicyList[]) { + policyLists.forEach(this.watchList, this); + } + + public watchList(list: PolicyList) { + for (const cache of this.caches) { + cache.watchList(list); + } + } + + public unwatchList(list: PolicyList) { + for (const cache of this.caches) { + cache.watchList(list); + } + } + + /** + * Test whether the server is allowed by the ACL unit. + * @param domain The server name to test. + * @returns A description of the access that the server has. + */ + public getAccessForServer(domain: string): EntityAccess { + return this.getAccessForEntity(domain, this.serverAllows, this.serverBans); + } + + /** + * Get the level of access the user has for the ACL unit. + * @param mxid The user id to test. + * @param policy Whether to check the server part of the user id against server rules. + * @returns A description of the access that the user has. + */ + public getAccessForUser(mxid: string, policy: "CHECK_SERVER" | "IGNORE_SERVER"): EntityAccess { + const userAccess = this.getAccessForEntity(mxid, this.userAllows, this.userBans); + if (userAccess.outcome === Access.Allowed) { + if (policy === "IGNORE_SERVER") { + return userAccess; + } else { + const userId = new UserID(mxid); + return this.getAccessForServer(userId.domain); + } + } else { + return userAccess; + } + } + + private getAccessForEntity(entity: string, allowCache: ListRuleCache, bannedCache: ListRuleCache): EntityAccess { + // Check if the entity is explicitly allowed. + // We have to infer that a rule exists for '*' if the allowCache is empty, otherwise you brick the ACL. + const allowRule = allowCache.getAnyRuleForEntity(entity); + if (allowRule === null && !allowCache.isEmpty()) { + return { outcome: Access.NotAllowed } + } + // Now check if the entity is banned. + const banRule = bannedCache.getAnyRuleForEntity(entity); + if (banRule !== null) { + return { outcome: Access.Banned, rule: banRule }; + } + // If they got to this point, they're allowed!! + return { outcome: Access.Allowed }; + } + + /** + * Create a ServerAcl instance from the rules contained in this unit. + * @param serverName The name of the server that you are operating from, used to ensure you cannot brick yourself. + * @returns A new `ServerAcl` instance with deny and allow entries created from the rules in this unit. + */ + public compileServerAcl(serverName: string): ServerAcl { + const acl = new ServerAcl(serverName).denyIpAddresses(); + const allowedServers = this.serverAllows.allRules; + // Allowed servers (allow). + if (allowedServers.length === 0) { + acl.allowServer('*'); + } else { + for (const rule of allowedServers) { + acl.allowServer(rule.entity); + } + if (this.getAccessForServer(serverName).outcome === Access.NotAllowed) { + acl.allowServer(serverName); + LogService.warn('AccessControlUnit', `The server ${serverName} we are operating from was not on the allowed when constructing the server ACL, so it will be injected it into the server acl. Please check the ACL lists.`) + } + } + // Banned servers (deny). + for (const rule of this.serverBans.allRules) { + if (rule.isMatch(serverName)) { + LogService.warn('AccessControlUnit', `The server ${serverName} we are operating from was found to be banned by ${rule.entity} by a rule from the event: ${rule.sourceEvent.event_id}, ` + + 'while constructing a server acl. Ignoring the rule. Please check the ACL lists.' + ); + } else { + acl.denyServer(rule.entity); + } + } + return acl; + } +} diff --git a/src/models/ListRule.ts b/src/models/ListRule.ts index 9ad0764..18988f5 100644 --- a/src/models/ListRule.ts +++ b/src/models/ListRule.ts @@ -55,6 +55,12 @@ export enum Recommendation { /// is considered absolutely absolutely perfect by whoever issued /// this ListRule. Opinion = "org.matrix.msc3845.opinion", + + /** + * This is a rule that recommends allowing a user to participate. + * Used for the construction of allow lists. + */ + Allow = "org.matrix.mjolnir.allow", } /** @@ -75,6 +81,11 @@ const RECOMMENDATION_OPINION_VARIANTS: string[] = [ Recommendation.Opinion ]; +const RECOMMENDATION_ALLOW_VARIANTS: string[] = [ + // Unstable + Recommendation.Allow +] + export const OPINION_MIN = -100; export const OPINION_MAX = +100; @@ -125,6 +136,13 @@ export abstract class ListRule { return this.glob.test(entity); } + /** + * @returns Whether the entity in he rule represents a Matrix glob (and not a literal). + */ + public isGlob(): boolean { + return /[*?]/.test(this.entity); + } + /** * Validate and parse an event into a ListRule. * @@ -173,6 +191,8 @@ export abstract class ListRule { return null; } return new ListRuleOpinion(event, entity, reason, kind, opinion); + } else if (RECOMMENDATION_ALLOW_VARIANTS.includes(recommendation)) { + return new ListRuleAllow(event, entity, reason, kind); } else { // As long as the `recommendation` is defined, we assume // that the rule is correct, just unknown. @@ -207,6 +227,32 @@ export class ListRuleBan extends ListRule { } } +/** + * A rule representing an "allow". + */ + export class ListRuleAllow extends ListRule { + constructor( + /** + * The event source for the rule. + */ + sourceEvent: MatrixStateEvent, + /** + * The entity covered by this rule, e.g. a glob user ID, a room ID, a server domain. + */ + entity: string, + /** + * A human-readable reason for this rule, for audit purposes. + */ + reason: string, + /** + * The type of entity for this rule, e.g. user, server domain, etc. + */ + kind: EntityType, + ) { + super(sourceEvent, entity, reason, kind, Recommendation.Allow) + } +} + /** * A rule representing an "opinion" */ diff --git a/src/models/PolicyList.ts b/src/models/PolicyList.ts index 8fc4113..9c3aba0 100644 --- a/src/models/PolicyList.ts +++ b/src/models/PolicyList.ts @@ -86,6 +86,14 @@ class PolicyList extends EventEmitter { // Events that we have already informed the batcher about, that we haven't loaded from the room state yet. private batchedEvents = new Set(); + /** + * This is used to annotate state events we store with the rule they are associated with. + * If we refactor this, it is important to also refactor any listeners to 'PolicyList.update' + * which may assume `ListRule`s that are removed will be identital (Object.is) to when they were added. + * If you are adding new listeners, you should check the source event_id of the rule. + */ + private static readonly EVENT_RULE_ANNOTATION_KEY = 'org.matrix.mjolnir.annotation.rule'; + /** * Construct a PolicyList, does not synchronize with the room. * @param roomId The id of the policy room, i.e. a room containing MSC2313 policies. @@ -134,19 +142,21 @@ 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 recommendation A specific recommendation to filter for e.g. `m.ban`. Please remember recommendation varients are normalized. * @returns The active ListRules for the ban list of that kind. */ - private rulesOfKind(kind: string): ListRule[] { + public rulesOfKind(kind: string, recommendation?: Recommendation): ListRule[] { const rules: ListRule[] = [] const stateKeyMap = this.state.get(kind); 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); + const rule = event[PolicyList.EVENT_RULE_ANNOTATION_KEY]; + if (rule && rule.kind === kind) { + if (recommendation === undefined) { + rules.push(rule); + } else if (rule.recommendation === recommendation) { + rules.push(rule); + } } } } @@ -353,10 +363,10 @@ class PolicyList extends EventEmitter { // If we haven't got any information about what the rule used to be, then it wasn't a valid rule to begin with // and so will not have been used. Removing a rule like this therefore results in no change. - if (changeType === ChangeType.Removed && previousState?.unsigned?.rule) { + if (changeType === ChangeType.Removed && previousState?.[PolicyList.EVENT_RULE_ANNOTATION_KEY]) { const sender = event.unsigned['redacted_because'] ? event.unsigned['redacted_because']['sender'] : event.sender; changes.push({ - changeType, event, sender, rule: previousState.unsigned.rule, + changeType, event, sender, rule: previousState[PolicyList.EVENT_RULE_ANNOTATION_KEY], ...previousState ? { previousState } : {} }); // Event has no content and cannot be parsed as a ListRule. @@ -368,7 +378,7 @@ class PolicyList extends EventEmitter { // Invalid/unknown rule, just skip it. continue; } - event.unsigned.rule = rule; + event[PolicyList.EVENT_RULE_ANNOTATION_KEY] = rule; if (changeType) { changes.push({ rule, changeType, event, sender: event.sender, ...previousState ? { previousState } : {} }); } diff --git a/test/integration/banListTest.ts b/test/integration/banListTest.ts index 40add9b..89660a4 100644 --- a/test/integration/banListTest.ts +++ b/test/integration/banListTest.ts @@ -6,7 +6,8 @@ import { ServerAcl } from "../../src/models/ServerAcl"; import { getFirstReaction } from "./commands/commandUtils"; import { getMessagesByUserIn } from "../../src/utils"; import { Mjolnir } from "../../src/Mjolnir"; -import { ALL_RULE_TYPES, RULE_SERVER, RULE_USER, SERVER_RULE_TYPES } from "../../src/models/ListRule"; +import { ALL_RULE_TYPES, Recommendation, RULE_SERVER, RULE_USER, SERVER_RULE_TYPES } from "../../src/models/ListRule"; +import AccessControlUnit, { Access, EntityAccess } from "../../src/models/AccessControlUnit"; /** * Create a policy rule in a policy room. @@ -26,6 +27,19 @@ async function createPolicyRule(client: MatrixClient, policyRoomId: string, poli }); } +/** + * Remove a policy rule from a list. + * @param client A matrix client that is logged in + * @param policyRoomId The room id to add the policy to. + * @param policyType The type of policy to add e.g. m.policy.rule.user. (Use RULE_USER though). + * @param entity The entity to ban e.g. @foo:example.org + * @param stateKey The key for the rule. + * @returns The event id of the void rule that was created to override the old one. + */ +async function removePolicyRule(client: MatrixClient, policyRoomId: string, policyType: string, entity: string, stateKey = `rule:${entity}`) { + return await client.sendStateEvent(policyRoomId, policyType, stateKey, {}); +} + describe("Test: Updating the PolicyList", function() { it("Calculates what has changed correctly.", async function() { this.timeout(10000); @@ -191,28 +205,13 @@ describe("Test: Updating the PolicyList", function() { }) }); -describe('Test: We do not respond to recommendations other than m.ban in the PolicyList', function() { - it('Will not respond to a rule that has a different recommendation to m.ban (or the unstable equivalent).', async function() { - const mjolnir: Mjolnir = this.mjolnir! - const banListId = await mjolnir.client.createRoom(); - const banList = new PolicyList(banListId, banListId, mjolnir.client); - await createPolicyRule(mjolnir.client, banListId, RULE_SERVER, 'exmaple.org', '', { recommendation: 'something that is not m.ban' }); - let changes: ListRuleChange[] = 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.client.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)}`); - }) -}) - describe('Test: We will not be able to ban ourselves via ACL.', function() { it('We do not ban ourselves when we put ourselves into the policy list.', async function() { const mjolnir: Mjolnir = this.mjolnir const serverName = new UserID(await mjolnir.client.getUserId()).domain; const banListId = await mjolnir.client.createRoom(); const banList = new PolicyList(banListId, banListId, mjolnir.client); + const aclUnit = new AccessControlUnit([banList]); await createPolicyRule(mjolnir.client, banListId, RULE_SERVER, serverName, ''); await createPolicyRule(mjolnir.client, banListId, RULE_SERVER, 'evil.com', ''); await createPolicyRule(mjolnir.client, banListId, RULE_SERVER, '*', ''); @@ -224,6 +223,10 @@ describe('Test: We will not be able to ban ourselves via ACL.', function() { changes.forEach(change => acl.denyServer(change.rule.entity)); assert.equal(acl.safeAclContent().deny.length, 1); assert.equal(acl.literalAclContent().deny.length, 3); + + const aclUnitAcl = aclUnit.compileServerAcl(serverName); + assert.equal(aclUnitAcl.literalAclContent().deny.length, 1); + }) }) @@ -457,3 +460,107 @@ describe('Test: should apply bans to the most recently active rooms first', func } }) }) + +/** + * Assert that the AccessUnitOutcome entity test has the right access. + * @param expected The Access we expect the entity to have, See Access. + * @param query The result of a test on AccessControlUnit e.g. `unit.getAccessForUser'@meow:localhost')` + * @param message A message for the console if the entity doesn't have the expected access. + */ +function assertAccess(expected: Access, query: EntityAccess, message?: string) { + assert.equal(query.outcome, expected, message); +} + +describe('Test: AccessControlUnit interaction with policy lists.', function() { + it('The AccessControlUnit correctly reflects the policies that have been set in its watched lists.', async function() { + const mjolnir: Mjolnir = this.mjolnir! + const policyListId = await mjolnir.client.createRoom(); + const policyList = new PolicyList(policyListId, Permalinks.forRoom(policyListId), mjolnir.client); + const aclUnit = new AccessControlUnit([policyList]); + assertAccess(Access.Allowed, aclUnit.getAccessForUser('@anyone:anywhere.example.com', "CHECK_SERVER"), 'Empty lists should implicitly allow.'); + + await createPolicyRule(mjolnir.client, policyListId, RULE_SERVER, 'matrix.org', '', { recommendation: Recommendation.Allow }); + // we want to imagine that the banned server was never taken off the allow after being banned. + await createPolicyRule(mjolnir.client, policyListId, RULE_SERVER, 'bad.example.com', '', { recommendation: Recommendation.Allow }, 'something-else'); + await createPolicyRule(mjolnir.client, policyListId, RULE_SERVER, 'bad.example.com', '', { recommendation: Recommendation.Ban }); + await createPolicyRule(mjolnir.client, policyListId, RULE_SERVER, '*.ddns.example.com', '', { recommendation: Recommendation.Ban }); + + await policyList.updateList(); + + assertAccess(Access.Allowed, aclUnit.getAccessForServer('matrix.org')); + assertAccess(Access.Allowed, aclUnit.getAccessForUser('@someone:matrix.org', "CHECK_SERVER")); + assertAccess(Access.NotAllowed, aclUnit.getAccessForServer('anywhere.else.example.com')); + assertAccess(Access.NotAllowed, aclUnit.getAccessForUser('@anyone:anywhere.else.example.com', "CHECK_SERVER")); + assertAccess(Access.Banned, aclUnit.getAccessForServer('bad.example.com')); + assertAccess(Access.Banned, aclUnit.getAccessForUser('@anyone:bad.example.com', "CHECK_SERVER")); + // They're not allowed in the first place, never mind that they are also banned. + assertAccess(Access.NotAllowed, aclUnit.getAccessForServer('meow.ddns.example.com')); + assertAccess(Access.NotAllowed, aclUnit.getAccessForUser('@anyone:meow.ddns.example.com', "CHECK_SERVER")); + + assertAccess(Access.Allowed, aclUnit.getAccessForUser('@spam:matrix.org', "CHECK_SERVER")); + await createPolicyRule(mjolnir.client, policyListId, RULE_USER, '@spam:matrix.org', '', { recommendation: Recommendation.Ban }); + await policyList.updateList(); + assertAccess(Access.Banned, aclUnit.getAccessForUser('@spam:matrix.org', "CHECK_SERVER")); + assertAccess(Access.Allowed, aclUnit.getAccessForUser('@someone:matrix.org', "CHECK_SERVER")); + + // protect a room and check that only bad.example.com, *.ddns.example.com are in the deny ACL and not matrix.org + await mjolnir.watchList(policyList.roomRef); + const protectedRoom = await mjolnir.client.createRoom(); + await mjolnir.protectedRoomsTracker.addProtectedRoom(protectedRoom); + await mjolnir.protectedRoomsTracker.syncLists(); + const roomAcl = await mjolnir.client.getRoomStateEvent(protectedRoom, "m.room.server_acl", ""); + assert.equal(roomAcl?.deny?.length ?? 0, 2, 'There should be two entries in the deny ACL.'); + for (const serverGlob of ["*.ddns.example.com", "bad.example.com"]) { + assert.equal((roomAcl?.deny ?? []).includes(serverGlob), true); + } + assert.equal(roomAcl.deny.includes("matrix.org"), false); + assert.equal(roomAcl.allow.includes("matrix.org"), true); + + // Now we remove the rules and hope that everything functions noramally. + await removePolicyRule(mjolnir.client, policyListId, RULE_SERVER, 'matrix.org'); + await removePolicyRule(mjolnir.client, policyListId, RULE_SERVER, 'bad.example.com', 'something-else'); + await removePolicyRule(mjolnir.client, policyListId, RULE_SERVER, 'bad.example.com'); + await removePolicyRule(mjolnir.client, policyListId, RULE_SERVER, '*.ddns.example.com'); + await removePolicyRule(mjolnir.client, policyListId, RULE_USER, "@spam:matrix.org"); + const changes = await policyList.updateList() + await mjolnir.protectedRoomsTracker.syncLists(); + + assert.equal(changes.length, 5, "The rules should have correctly been removed"); + assertAccess(Access.Allowed, aclUnit.getAccessForServer('matrix.org')); + assertAccess(Access.Allowed, aclUnit.getAccessForUser('@someone:matrix.org', "CHECK_SERVER")); + assertAccess(Access.Allowed, aclUnit.getAccessForServer('anywhere.else.example.com')); + assertAccess(Access.Allowed, aclUnit.getAccessForUser('@anyone:anywhere.else.example.com', "CHECK_SERVER")); + assertAccess(Access.Allowed, aclUnit.getAccessForServer('bad.example.com')); + assertAccess(Access.Allowed, aclUnit.getAccessForUser('@anyone:bad.example.com', "CHECK_SERVER")); + assertAccess(Access.Allowed, aclUnit.getAccessForServer('meow.ddns.example.com')); + assertAccess(Access.Allowed, aclUnit.getAccessForUser('@anyone:meow.ddns.example.com', "CHECK_SERVER")); + assertAccess(Access.Allowed, aclUnit.getAccessForUser('@spam:matrix.org', "CHECK_SERVER")); + + const roomAclAfter = await mjolnir.client.getRoomStateEvent(protectedRoom, "m.room.server_acl", ""); + assert.equal(roomAclAfter.deny?.length ?? 0, 0, 'There should be no entries in the deny ACL.'); + assert.equal(roomAclAfter.allow?.length ?? 0, 1, 'There should be 1 entry in the allow ACL.'); + assert.equal(roomAclAfter.allow.includes("*"), true); + }) + it('removing a rule from a different list will not clobber anything.', async function() { + const mjolnir: Mjolnir = this.mjolnir! + const policyLists = await Promise.all([...Array(2).keys()].map(async _ => { + const policyListId = await mjolnir.client.createRoom(); + return new PolicyList(policyListId, Permalinks.forRoom(policyListId), mjolnir.client); + })); + const banMeServer = 'banme.example.com'; + const aclUnit = new AccessControlUnit(policyLists); + await Promise.all(policyLists.map(policyList => { + return createPolicyRule(mjolnir.client, policyList.roomId, RULE_SERVER, banMeServer, '', { recommendation: Recommendation.Ban }) + })); + await Promise.all(policyLists.map(list => list.updateList())); + assertAccess(Access.Banned, aclUnit.getAccessForServer(banMeServer)); + + // remove the rule that bans `banme.example.com` from just one of the lists. + await removePolicyRule(mjolnir.client, policyLists[0].roomId, RULE_SERVER, banMeServer); + await Promise.all(policyLists.map(list => list.updateList())); + assertAccess(Access.Banned, aclUnit.getAccessForServer(banMeServer), "Should still be banned at this point."); + await removePolicyRule(mjolnir.client, policyLists[1].roomId, RULE_SERVER, banMeServer); + await Promise.all(policyLists.map(list => list.updateList())); + assertAccess(Access.Allowed, aclUnit.getAccessForServer(banMeServer), "Should not longer be any rules"); + }) +})