From 5e9477972d68d9ff6c6d86faf7bef3f356c1b949 Mon Sep 17 00:00:00 2001 From: David Teller Date: Tue, 30 Aug 2022 12:12:07 +0200 Subject: [PATCH 1/6] Base support for MSC3847: Ignore invites with policy rooms Type: enhancement --- spec/unit/matrix-client.spec.ts | 153 +++++++++++++++++++- src/client.ts | 6 + src/models/invites-ignorer.ts | 245 ++++++++++++++++++++++++++++++++ 3 files changed, 403 insertions(+), 1 deletion(-) create mode 100644 src/models/invites-ignorer.ts diff --git a/spec/unit/matrix-client.spec.ts b/spec/unit/matrix-client.spec.ts index d1d0bf5baa..b31893add5 100644 --- a/spec/unit/matrix-client.spec.ts +++ b/spec/unit/matrix-client.spec.ts @@ -36,9 +36,10 @@ import { ReceiptType } from "../../src/@types/read_receipts"; import * as testUtils from "../test-utils/test-utils"; import { makeBeaconInfoContent } from "../../src/content-helpers"; import { M_BEACON_INFO } from "../../src/@types/beacon"; -import { ContentHelpers, Room } from "../../src"; +import { ContentHelpers, EventTimeline, Room } from "../../src"; import { supportsMatrixCall } from "../../src/webrtc/call"; import { makeBeaconEvent } from "../test-utils/beacon"; +import { PolicyScope } from "../../src/models/invites-ignorer"; jest.useFakeTimers(); @@ -1412,4 +1413,154 @@ describe("MatrixClient", function() { expect(client.crypto.encryptAndSendToDevices).toHaveBeenLastCalledWith(deviceInfos, payload); }); }); + + describe("support for ignoring invites", () => { + beforeEach(() => { + // Mockup `getAccountData`/`setAccountData`. + const dataStore = new Map(); + client.setAccountData = function(eventType, content) { + dataStore.set(eventType, content); + return Promise.resolve(); + }; + client.getAccountData = function(eventType) { + const data = dataStore.get(eventType); + return new MatrixEvent({ + content: data, + }); + }; + + // Mockup `createRoom`/`getRoom`, including state. + const rooms = new Map(); + client.createRoom = function(options) { + const roomId = `!room-${rooms.size}:example.org`; + const state = new Map(); + const room = { + roomId, + _options: options, + _state: state, + getUnfilteredTimelineSet: function() { + return { + getLiveTimeline: function() { + return { + getState: function(direction) { + expect(direction).toBe(EventTimeline.FORWARDS); + return { + getStateEvents: function(type) { + const store = state.get(type) || {}; + return Object.keys(store).map(key => store[key]); + }, + }; + }, + }; + }, + }; + }, + }; + rooms.set(roomId, room); + return Promise.resolve({ room_id: roomId }); + }; + client.getRoom = function(roomId) { + return rooms.get(roomId); + }; + + // Mockup state events + client.sendStateEvent = function(roomId, type, content) { + const room = this.getRoom(roomId); + const state: Map = room._state; + let store = state.get(type); + if (!store) { + store = {}; + state.set(type, store); + } + const eventId = `$event-${Math.random()}:example.org`; + store[eventId] = { + getId: function() { + return eventId; + }, + getRoomId: function() { + return roomId; + }, + getContent: function() { + return content; + }, + }; + }; + client.redactEvent = function(roomId, eventId) { + const room = this.getRoom(roomId); + const state: Map = room._state; + for (const store of state.values()) { + delete store[eventId]; + } + }; + }); + it("should initialize and return the same `target` consistently", async () => { + const target1 = await client.ignoredInvites.getOrCreateTargetRoom(); + const target2 = await client.ignoredInvites.getOrCreateTargetRoom(); + expect(target1).toBeTruthy(); + expect(target1).toBe(target2); + }); + it("should initialize and return the same `sources` consistently", async () => { + const sources1 = await client.ignoredInvites.getOrCreateSourceRooms(); + const sources2 = await client.ignoredInvites.getOrCreateSourceRooms(); + expect(sources1).toBeTruthy(); + expect(sources1).toHaveLength(1); + expect(sources1).toEqual(sources2); + }); + it("should initially not reject any invite", async () => { + const rule = await client.ignoredInvites.getRuleForInvite({ + sender: "@foobar:example.org", + roomId: "!snafu:somewhere.org", + }); + expect(rule).toBeFalsy(); + }); + it("should reject invites once we have added a matching rule", async () => { + await client.ignoredInvites.addRule(PolicyScope.User, "*:example.org", "just a test"); + + // We should reject this invite. + const ruleMatch = await client.ignoredInvites.getRuleForInvite({ + sender: "@foobar:example.org", + roomId: "!snafu:somewhere.org", + }); + expect(ruleMatch).toBeTruthy(); + expect(ruleMatch.getContent()).toMatchObject({ + recommendation: "m.ban", + reason: "just a test", + }); + + // We should let these invites go through. + const ruleWrongServer = await client.ignoredInvites.getRuleForInvite({ + sender: "@foobar:somewhere.org", + roomId: "!snafu:somewhere.org", + }); + expect(ruleWrongServer).toBeFalsy(); + + const ruleWrongServerRoom = await client.ignoredInvites.getRuleForInvite({ + sender: "@foobar:somewhere.org", + roomId: "!snafu:example.org", + }); + expect(ruleWrongServerRoom).toBeFalsy(); + }); + it("should not reject invites anymore once we have removed a rule", async () => { + await client.ignoredInvites.addRule(PolicyScope.User, "*:example.org", "just a test"); + + // We should reject this invite. + const ruleMatch = await client.ignoredInvites.getRuleForInvite({ + sender: "@foobar:example.org", + roomId: "!snafu:somewhere.org", + }); + expect(ruleMatch).toBeTruthy(); + expect(ruleMatch.getContent()).toMatchObject({ + recommendation: "m.ban", + reason: "just a test", + }); + + // After removing the invite, we shouldn't reject it anymore. + await client.ignoredInvites.removeRule(ruleMatch); + const ruleMatch2 = await client.ignoredInvites.getRuleForInvite({ + sender: "@foobar:example.org", + roomId: "!snafu:somewhere.org", + }); + expect(ruleMatch2).toBeFalsy(); + }); + }); }); diff --git a/src/client.ts b/src/client.ts index d8fc75d56f..d734d256c7 100644 --- a/src/client.ts +++ b/src/client.ts @@ -197,6 +197,7 @@ import { Thread, THREAD_RELATION_TYPE } from "./models/thread"; import { MBeaconInfoEventContent, M_BEACON_INFO } from "./@types/beacon"; import { ToDeviceMessageQueue } from "./ToDeviceMessageQueue"; import { ToDeviceBatch } from "./models/ToDeviceMessage"; +import { IgnoredInvites } from "./models/invites-ignorer"; export type Store = IStore; @@ -954,6 +955,9 @@ export class MatrixClient extends TypedEventEmitter> { + // In this implementation, we perform a very naive lookup: + // - search in each policy room; + // - turn each (potentially glob) rule entity into a regexp. + // + // Real-world testing will tell us whether this is performant enough. + // In the (unfortunately likely) case it isn't, there are several manners + // in which we could optimize this: + // - match several entities per go; + // - pre-compile each rule entity into a regexp; + // - pre-compile entire rooms into a single regexp. + const policyRooms = await this.getOrCreateSourceRooms(); + const senderServer = sender.split(":")[1]; + const roomServer = roomId.split(":")[1]; + for (const room of policyRooms) { + const state = room.getUnfilteredTimelineSet().getLiveTimeline().getState(EventTimeline.FORWARDS); + + for (const { scope, entities } of [ + { scope: PolicyScope.Room, entities: [roomId] }, + { scope: PolicyScope.User, entities: [sender] }, + { scope: PolicyScope.Server, entities: [senderServer, roomServer] }, + ]) { + const events = state.getStateEvents(scope); + for (const event of events) { + const content = event.getContent(); + if (content?.recommendation != PolicyRecommendation.Ban) { + // Ignoring invites only looks at `m.ban` recommendations. + continue; + } + const glob = content?.entity; + if (!glob) { + // Invalid event. + continue; + } + let regexp; + try { + regexp = globToRegexp(glob, false); + } catch (ex) { + // Assume invalid event. + continue; + } + for (const entity of entities) { + if (entity && entity.search(regexp) >= 0) { + return event; + } + } + // No match. + } + } + } + return null; + } + + /** + * Get the target room, i.e. the room in which any new rule should be written. + * + * If there is no target room setup, a target room is created. + * + * Note: This method is public for testing reasons. Most clients should not need + * to call it directly. + */ + public async getOrCreateTargetRoom(): Promise { + const policies = this.client.getAccountData(POLICIES_ACCOUNT_EVENT_TYPE)?.getContent() || {}; + const ignoreInvitesPolicies = policies[IGNORE_INVITES_ACCOUNT_EVENT_KEY] || {}; + let target = ignoreInvitesPolicies.target; + // Validate `target`. If it is invalid, trash out the current `target` + // and create a new room. + if (typeof target !== "string") { + target = null; + } + if (target) { + // Check that the room exists and is valid. + const room = this.client.getRoom(target); + if (room) { + return room; + } else { + target = null; + } + } + // We need to create our own policy room for ignoring invites. + target = (await this.client.createRoom({ + name: "Individual Policy Room", + preset: Preset.PrivateChat, + })).room_id; + ignoreInvitesPolicies.target = target; + policies[IGNORE_INVITES_ACCOUNT_EVENT_KEY] = ignoreInvitesPolicies; + await this.client.setAccountData(POLICIES_ACCOUNT_EVENT_TYPE, policies); + + // Since we have just called `createRoom`, `getRoom` should not be `null`. + return this.client.getRoom(target)!; + } + + /** + * Get the list of source rooms, i.e. the rooms from which rules need to be read. + * + * If no source rooms are setup, the target room is used as sole source room. + * + * Note: This method is public for testing reasons. Most clients should not need + * to call it directly. + */ + public async getOrCreateSourceRooms(): Promise { + const policies = this.client.getAccountData(POLICIES_ACCOUNT_EVENT_TYPE)?.getContent() || {}; + const ignoreInvitesPolicies = policies[IGNORE_INVITES_ACCOUNT_EVENT_KEY] || {}; + let sources = ignoreInvitesPolicies.sources; + + // Validate `sources`. If it is invalid, trash out the current `sources` + // and create a new list of sources from `target`. + let hasChanges = false; + if (!Array.isArray(sources)) { + // `sources` could not be an array. + hasChanges = true; + sources = []; + } + let sourceRooms: Room[] = sources + // `sources` could contain non-string / invalid room ids + .filter(roomId => typeof roomId === "string") + .map(roomId => this.client.getRoom(roomId)) + .filter(room => !!room); + if (sourceRooms.length != sources.length) { + hasChanges = true; + } + if (sourceRooms.length == 0) { + // `sources` could be empty (possibly because we've removed + // invalid content) + const target = await this.getOrCreateTargetRoom(); + hasChanges = true; + sourceRooms = [target]; + } + if (hasChanges) { + // Reload `policies`/`ignoreInvitesPolicies` in case it has been changed + // during or by our call to `this.getTargetRoom()`. + const policies = this.client.getAccountData(POLICIES_ACCOUNT_EVENT_TYPE)?.getContent() || {}; + const ignoreInvitesPolicies = policies[IGNORE_INVITES_ACCOUNT_EVENT_KEY] || {}; + sources = sourceRooms.map(room => room.roomId); + policies[IGNORE_INVITES_ACCOUNT_EVENT_KEY] = ignoreInvitesPolicies; + ignoreInvitesPolicies.sources = sources; + await this.client.setAccountData(POLICIES_ACCOUNT_EVENT_TYPE, policies); + } + return sourceRooms; + } +} From 48ec626496000f1136205c77b22781c3deb212b1 Mon Sep 17 00:00:00 2001 From: David Teller Date: Tue, 30 Aug 2022 12:12:07 +0200 Subject: [PATCH 2/6] Base support for MSC3847: Ignore invites with policy rooms Type: enhancement --- spec/unit/matrix-client.spec.ts | 149 ++++++++++++++++++++++++++++++-- src/models/invites-ignorer.ts | 66 ++++++++++++-- 2 files changed, 205 insertions(+), 10 deletions(-) diff --git a/spec/unit/matrix-client.spec.ts b/spec/unit/matrix-client.spec.ts index b31893add5..8508c945ec 100644 --- a/spec/unit/matrix-client.spec.ts +++ b/spec/unit/matrix-client.spec.ts @@ -39,7 +39,8 @@ import { M_BEACON_INFO } from "../../src/@types/beacon"; import { ContentHelpers, EventTimeline, Room } from "../../src"; import { supportsMatrixCall } from "../../src/webrtc/call"; import { makeBeaconEvent } from "../test-utils/beacon"; -import { PolicyScope } from "../../src/models/invites-ignorer"; +import { IGNORE_INVITES_ACCOUNT_EVENT_KEY, POLICIES_ACCOUNT_EVENT_TYPE, PolicyScope } + from "../../src/models/invites-ignorer"; jest.useFakeTimers(); @@ -1429,10 +1430,10 @@ describe("MatrixClient", function() { }); }; - // Mockup `createRoom`/`getRoom`, including state. + // Mockup `createRoom`/`getRoom`/`joinRoom`, including state. const rooms = new Map(); - client.createRoom = function(options) { - const roomId = `!room-${rooms.size}:example.org`; + client.createRoom = function(options = {}) { + const roomId = options["_roomId"] || `!room-${rooms.size}:example.org`; const state = new Map(); const room = { roomId, @@ -1462,6 +1463,9 @@ describe("MatrixClient", function() { client.getRoom = function(roomId) { return rooms.get(roomId); }; + client.joinRoom = function(roomId) { + return this.getRoom(roomId) || this.createRoom({ _roomId: roomId }); + }; // Mockup state events client.sendStateEvent = function(roomId, type, content) { @@ -1484,6 +1488,7 @@ describe("MatrixClient", function() { return content; }, }; + return { event_id: eventId }; }; client.redactEvent = function(roomId, eventId) { const room = this.getRoom(roomId); @@ -1513,7 +1518,7 @@ describe("MatrixClient", function() { }); expect(rule).toBeFalsy(); }); - it("should reject invites once we have added a matching rule", async () => { + it("should reject invites once we have added a matching rule in the target room (scope: user)", async () => { await client.ignoredInvites.addRule(PolicyScope.User, "*:example.org", "just a test"); // We should reject this invite. @@ -1540,6 +1545,101 @@ describe("MatrixClient", function() { }); expect(ruleWrongServerRoom).toBeFalsy(); }); + it("should reject invites once we have added a matching rule in the target room (scope: server)", async () => { + const REASON = `Just a test ${Math.random()}`; + await client.ignoredInvites.addRule(PolicyScope.Server, "example.org", REASON); + + // We should reject these invites. + const ruleSenderMatch = await client.ignoredInvites.getRuleForInvite({ + sender: "@foobar:example.org", + roomId: "!snafu:somewhere.org", + }); + expect(ruleSenderMatch).toBeTruthy(); + expect(ruleSenderMatch.getContent()).toMatchObject({ + recommendation: "m.ban", + reason: REASON, + }); + + const ruleRoomMatch = await client.ignoredInvites.getRuleForInvite({ + sender: "@foobar:somewhere.org", + roomId: "!snafu:example.org", + }); + expect(ruleRoomMatch).toBeTruthy(); + expect(ruleRoomMatch.getContent()).toMatchObject({ + recommendation: "m.ban", + reason: REASON, + }); + + // We should let these invites go through. + const ruleWrongServer = await client.ignoredInvites.getRuleForInvite({ + sender: "@foobar:somewhere.org", + roomId: "!snafu:somewhere.org", + }); + expect(ruleWrongServer).toBeFalsy(); + }); + it("should reject invites once we have added a matching rule in the target room (scope: room)", async () => { + const REASON = `Just a test ${Math.random()}`; + const BAD_ROOM_ID = "!bad:example.org"; + const GOOD_ROOM_ID = "!good:example.org"; + await client.ignoredInvites.addRule(PolicyScope.Room, BAD_ROOM_ID, REASON); + + // We should reject this invite. + const ruleSenderMatch = await client.ignoredInvites.getRuleForInvite({ + sender: "@foobar:example.org", + roomId: BAD_ROOM_ID, + }); + expect(ruleSenderMatch).toBeTruthy(); + expect(ruleSenderMatch.getContent()).toMatchObject({ + recommendation: "m.ban", + reason: REASON, + }); + + // We should let these invites go through. + const ruleWrongRoom = await client.ignoredInvites.getRuleForInvite({ + sender: BAD_ROOM_ID, + roomId: GOOD_ROOM_ID, + }); + expect(ruleWrongRoom).toBeFalsy(); + }); + it("should reject invites once we have added a matching rule in a non-target source room", async () => { + const NEW_SOURCE_ROOM_ID = "!another-source:example.org"; + + // Make sure that everything is initialized. + await client.ignoredInvites.getOrCreateSourceRooms(); + await client.joinRoom(NEW_SOURCE_ROOM_ID); + await client.ignoredInvites.addSource(NEW_SOURCE_ROOM_ID); + + // Add a rule in the new source room. + await client.sendStateEvent(NEW_SOURCE_ROOM_ID, PolicyScope.User, { + entity: "*:example.org", + reason: "just a test", + recommendation: "m.ban", + }); + + // We should reject this invite. + const ruleMatch = await client.ignoredInvites.getRuleForInvite({ + sender: "@foobar:example.org", + roomId: "!snafu:somewhere.org", + }); + expect(ruleMatch).toBeTruthy(); + expect(ruleMatch.getContent()).toMatchObject({ + recommendation: "m.ban", + reason: "just a test", + }); + + // We should let these invites go through. + const ruleWrongServer = await client.ignoredInvites.getRuleForInvite({ + sender: "@foobar:somewhere.org", + roomId: "!snafu:somewhere.org", + }); + expect(ruleWrongServer).toBeFalsy(); + + const ruleWrongServerRoom = await client.ignoredInvites.getRuleForInvite({ + sender: "@foobar:somewhere.org", + roomId: "!snafu:example.org", + }); + expect(ruleWrongServerRoom).toBeFalsy(); + }); it("should not reject invites anymore once we have removed a rule", async () => { await client.ignoredInvites.addRule(PolicyScope.User, "*:example.org", "just a test"); @@ -1562,5 +1662,44 @@ describe("MatrixClient", function() { }); expect(ruleMatch2).toBeFalsy(); }); + it("should add new rules in the target room, rather than any other source room", async () => { + const NEW_SOURCE_ROOM_ID = "!another-source:example.org"; + + // Make sure that everything is initialized. + await client.ignoredInvites.getOrCreateSourceRooms(); + await client.joinRoom(NEW_SOURCE_ROOM_ID); + const newSourceRoom = client.getRoom(NEW_SOURCE_ROOM_ID); + + // Fetch the list of sources and check that we do not have the new room yet. + const policies = await client.getAccountData(POLICIES_ACCOUNT_EVENT_TYPE).getContent(); + expect(policies).toBeTruthy(); + const ignoreInvites = policies[IGNORE_INVITES_ACCOUNT_EVENT_KEY]; + expect(ignoreInvites).toBeTruthy(); + expect(ignoreInvites.sources).toBeTruthy(); + expect(ignoreInvites.sources).not.toContain(NEW_SOURCE_ROOM_ID); + + // Add a source. + const added = await client.ignoredInvites.addSource(NEW_SOURCE_ROOM_ID); + expect(added).toBe(true); + const added2 = await client.ignoredInvites.addSource(NEW_SOURCE_ROOM_ID); + expect(added2).toBe(false); + + // Fetch the list of sources and check that we have added the new room. + const policies2 = await client.getAccountData(POLICIES_ACCOUNT_EVENT_TYPE).getContent(); + expect(policies2).toBeTruthy(); + const ignoreInvites2 = policies2[IGNORE_INVITES_ACCOUNT_EVENT_KEY]; + expect(ignoreInvites2).toBeTruthy(); + expect(ignoreInvites2.sources).toBeTruthy(); + expect(ignoreInvites2.sources).toContain(NEW_SOURCE_ROOM_ID); + + // Add a rule. + const eventId = await client.ignoredInvites.addRule(PolicyScope.User, "*:example.org", "just a test"); + + // Check where it shows up. + const targetRoomId = ignoreInvites2.target; + const targetRoom = client.getRoom(targetRoomId); + expect(targetRoom._state.get(PolicyScope.User)[eventId]).toBeTruthy(); + expect(newSourceRoom._state.get(PolicyScope.User)?.[eventId]).toBeFalsy(); + }); }); }); diff --git a/src/models/invites-ignorer.ts b/src/models/invites-ignorer.ts index 75f3ae3db4..9709ff9fc1 100644 --- a/src/models/invites-ignorer.ts +++ b/src/models/invites-ignorer.ts @@ -20,10 +20,14 @@ import { globToRegexp } from "../utils"; import { Room } from "./room"; /// The event type storing the user's individual policies. -const POLICIES_ACCOUNT_EVENT_TYPE = "org.matrix.msc3847.policies"; +/// +/// Exported for testing purposes. +export const POLICIES_ACCOUNT_EVENT_TYPE = "org.matrix.msc3847.policies"; /// The key within the user's individual policies storing the user's ignored invites. -const IGNORE_INVITES_ACCOUNT_EVENT_KEY = "org.matrix.msc3847.ignore.invites"; +/// +/// Exported for testing purposes. +export const IGNORE_INVITES_ACCOUNT_EVENT_KEY = "org.matrix.msc3847.ignore.invites"; /// The types of recommendations understood. enum PolicyRecommendation { @@ -75,14 +79,16 @@ export class IgnoredInvites { * @param scope The scope for this rule. * @param entity The entity covered by this rule. Globs are supported. * @param reason A human-readable reason for introducing this new rule. + * @return The event id for the new rule. */ - public async addRule(scope: PolicyScope, entity: string, reason: string) { + public async addRule(scope: PolicyScope, entity: string, reason: string): Promise { const target = await this.getOrCreateTargetRoom(); - await this.client.sendStateEvent(target.roomId, scope, { + const response = await this.client.sendStateEvent(target.roomId, scope, { entity, reason, recommendation: PolicyRecommendation.Ban, }); + return response.event_id; } /** @@ -92,6 +98,44 @@ export class IgnoredInvites { await this.client.redactEvent(event.getRoomId()!, event.getId()!); } + /** + * Add a new room to the list of sources. If the user isn't a member of the + * room, attempt to join it. + * + * @param roomId A valid room id. If this room is already in the list + * of sources, it will not be duplicated. + * @return `true` if the source was added, `false` if it was already present. + * @throws If `roomId` isn't the id of a room that the current user is already + * member of or can join. + * + * # Safety + * + * This method will rewrite the `Policies` object in the user's account data. + * This rewrite is inherently racy and could overwrite or be overwritten by + * other concurrent rewrites of the same object. + */ + public async addSource(roomId: string): Promise { + // We attempt to join the room *before* calling + // `await this.getOrCreateSourceRooms()` to decrease the duration + // of the racy section. + await this.client.joinRoom(roomId); + // Race starts. + const sources = (await this.getOrCreateSourceRooms()) + .map(room => room.roomId); + if (sources.includes(roomId)) { + return false; + } + sources.push(roomId); + const policies = this.client.getAccountData(POLICIES_ACCOUNT_EVENT_TYPE)?.getContent() || {}; + + const ignoreInvitesPolicies = policies[IGNORE_INVITES_ACCOUNT_EVENT_KEY] || {}; + ignoreInvitesPolicies.sources = sources; + await this.client.setAccountData(POLICIES_ACCOUNT_EVENT_TYPE, policies); + + // Race ends. + return true; + } + /** * Find out whether an invite should be ignored. * @@ -136,7 +180,7 @@ export class IgnoredInvites { // Invalid event. continue; } - let regexp; + let regexp: string; try { regexp = globToRegexp(glob, false); } catch (ex) { @@ -162,6 +206,12 @@ export class IgnoredInvites { * * Note: This method is public for testing reasons. Most clients should not need * to call it directly. + * + * # Safety + * + * This method will rewrite the `Policies` object in the user's account data. + * This rewrite is inherently racy and could overwrite or be overwritten by + * other concurrent rewrites of the same object. */ public async getOrCreateTargetRoom(): Promise { const policies = this.client.getAccountData(POLICIES_ACCOUNT_EVENT_TYPE)?.getContent() || {}; @@ -201,6 +251,12 @@ export class IgnoredInvites { * * Note: This method is public for testing reasons. Most clients should not need * to call it directly. + * + * # Safety + * + * This method will rewrite the `Policies` object in the user's account data. + * This rewrite is inherently racy and could overwrite or be overwritten by + * other concurrent rewrites of the same object. */ public async getOrCreateSourceRooms(): Promise { const policies = this.client.getAccountData(POLICIES_ACCOUNT_EVENT_TYPE)?.getContent() || {}; From 8fb9b58c3a1e9101a7f7984c7fd85475289e9841 Mon Sep 17 00:00:00 2001 From: David Teller Date: Thu, 1 Sep 2022 10:58:12 +0200 Subject: [PATCH 3/6] WIP: Applying feedback --- src/models/invites-ignorer.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/models/invites-ignorer.ts b/src/models/invites-ignorer.ts index 9709ff9fc1..5b5676265f 100644 --- a/src/models/invites-ignorer.ts +++ b/src/models/invites-ignorer.ts @@ -180,15 +180,15 @@ export class IgnoredInvites { // Invalid event. continue; } - let regexp: string; + let regexp: RegExp; try { - regexp = globToRegexp(glob, false); + regexp = new RegExp(globToRegexp(glob, false)); } catch (ex) { // Assume invalid event. continue; } for (const entity of entities) { - if (entity && entity.search(regexp) >= 0) { + if (entity && regexp.test(entity)) { return event; } } From 2dc184be1a548e7cb271ed5c2541b4541d9a221f Mon Sep 17 00:00:00 2001 From: David Teller Date: Mon, 5 Sep 2022 18:50:26 +0200 Subject: [PATCH 4/6] WIP: Applying feedback --- spec/unit/matrix-client.spec.ts | 24 ++++++-- src/models/invites-ignorer.ts | 97 ++++++++++++++++++++++++++------- 2 files changed, 95 insertions(+), 26 deletions(-) diff --git a/spec/unit/matrix-client.spec.ts b/spec/unit/matrix-client.spec.ts index 8508c945ec..2b8faf5065 100644 --- a/spec/unit/matrix-client.spec.ts +++ b/spec/unit/matrix-client.spec.ts @@ -39,8 +39,11 @@ import { M_BEACON_INFO } from "../../src/@types/beacon"; import { ContentHelpers, EventTimeline, Room } from "../../src"; import { supportsMatrixCall } from "../../src/webrtc/call"; import { makeBeaconEvent } from "../test-utils/beacon"; -import { IGNORE_INVITES_ACCOUNT_EVENT_KEY, POLICIES_ACCOUNT_EVENT_TYPE, PolicyScope } - from "../../src/models/invites-ignorer"; +import { + IGNORE_INVITES_ACCOUNT_EVENT_KEY, + POLICIES_ACCOUNT_EVENT_TYPE, + PolicyScope, +} from "../../src/models/invites-ignorer"; jest.useFakeTimers(); @@ -1498,12 +1501,14 @@ describe("MatrixClient", function() { } }; }); + it("should initialize and return the same `target` consistently", async () => { const target1 = await client.ignoredInvites.getOrCreateTargetRoom(); const target2 = await client.ignoredInvites.getOrCreateTargetRoom(); expect(target1).toBeTruthy(); expect(target1).toBe(target2); }); + it("should initialize and return the same `sources` consistently", async () => { const sources1 = await client.ignoredInvites.getOrCreateSourceRooms(); const sources2 = await client.ignoredInvites.getOrCreateSourceRooms(); @@ -1511,6 +1516,7 @@ describe("MatrixClient", function() { expect(sources1).toHaveLength(1); expect(sources1).toEqual(sources2); }); + it("should initially not reject any invite", async () => { const rule = await client.ignoredInvites.getRuleForInvite({ sender: "@foobar:example.org", @@ -1518,6 +1524,7 @@ describe("MatrixClient", function() { }); expect(rule).toBeFalsy(); }); + it("should reject invites once we have added a matching rule in the target room (scope: user)", async () => { await client.ignoredInvites.addRule(PolicyScope.User, "*:example.org", "just a test"); @@ -1545,6 +1552,7 @@ describe("MatrixClient", function() { }); expect(ruleWrongServerRoom).toBeFalsy(); }); + it("should reject invites once we have added a matching rule in the target room (scope: server)", async () => { const REASON = `Just a test ${Math.random()}`; await client.ignoredInvites.addRule(PolicyScope.Server, "example.org", REASON); @@ -1577,6 +1585,7 @@ describe("MatrixClient", function() { }); expect(ruleWrongServer).toBeFalsy(); }); + it("should reject invites once we have added a matching rule in the target room (scope: room)", async () => { const REASON = `Just a test ${Math.random()}`; const BAD_ROOM_ID = "!bad:example.org"; @@ -1601,6 +1610,7 @@ describe("MatrixClient", function() { }); expect(ruleWrongRoom).toBeFalsy(); }); + it("should reject invites once we have added a matching rule in a non-target source room", async () => { const NEW_SOURCE_ROOM_ID = "!another-source:example.org"; @@ -1640,6 +1650,7 @@ describe("MatrixClient", function() { }); expect(ruleWrongServerRoom).toBeFalsy(); }); + it("should not reject invites anymore once we have removed a rule", async () => { await client.ignoredInvites.addRule(PolicyScope.User, "*:example.org", "just a test"); @@ -1662,6 +1673,7 @@ describe("MatrixClient", function() { }); expect(ruleMatch2).toBeFalsy(); }); + it("should add new rules in the target room, rather than any other source room", async () => { const NEW_SOURCE_ROOM_ID = "!another-source:example.org"; @@ -1671,9 +1683,9 @@ describe("MatrixClient", function() { const newSourceRoom = client.getRoom(NEW_SOURCE_ROOM_ID); // Fetch the list of sources and check that we do not have the new room yet. - const policies = await client.getAccountData(POLICIES_ACCOUNT_EVENT_TYPE).getContent(); + const policies = await client.getAccountData(POLICIES_ACCOUNT_EVENT_TYPE.name).getContent(); expect(policies).toBeTruthy(); - const ignoreInvites = policies[IGNORE_INVITES_ACCOUNT_EVENT_KEY]; + const ignoreInvites = policies[IGNORE_INVITES_ACCOUNT_EVENT_KEY.name]; expect(ignoreInvites).toBeTruthy(); expect(ignoreInvites.sources).toBeTruthy(); expect(ignoreInvites.sources).not.toContain(NEW_SOURCE_ROOM_ID); @@ -1685,9 +1697,9 @@ describe("MatrixClient", function() { expect(added2).toBe(false); // Fetch the list of sources and check that we have added the new room. - const policies2 = await client.getAccountData(POLICIES_ACCOUNT_EVENT_TYPE).getContent(); + const policies2 = await client.getAccountData(POLICIES_ACCOUNT_EVENT_TYPE.name).getContent(); expect(policies2).toBeTruthy(); - const ignoreInvites2 = policies2[IGNORE_INVITES_ACCOUNT_EVENT_KEY]; + const ignoreInvites2 = policies2[IGNORE_INVITES_ACCOUNT_EVENT_KEY.name]; expect(ignoreInvites2).toBeTruthy(); expect(ignoreInvites2.sources).toBeTruthy(); expect(ignoreInvites2.sources).toContain(NEW_SOURCE_ROOM_ID); diff --git a/src/models/invites-ignorer.ts b/src/models/invites-ignorer.ts index 5b5676265f..6754827be9 100644 --- a/src/models/invites-ignorer.ts +++ b/src/models/invites-ignorer.ts @@ -14,6 +14,8 @@ See the License for the specific language governing permissions and limitations under the License. */ +import { UnstableValue } from "matrix-events-sdk"; + import { MatrixClient } from "../client"; import { EventTimeline, MatrixEvent, Preset } from "../matrix"; import { globToRegexp } from "../utils"; @@ -22,12 +24,12 @@ import { Room } from "./room"; /// The event type storing the user's individual policies. /// /// Exported for testing purposes. -export const POLICIES_ACCOUNT_EVENT_TYPE = "org.matrix.msc3847.policies"; +export const POLICIES_ACCOUNT_EVENT_TYPE = new UnstableValue(null, "org.matrix.msc3847.policies"); /// The key within the user's individual policies storing the user's ignored invites. /// /// Exported for testing purposes. -export const IGNORE_INVITES_ACCOUNT_EVENT_KEY = "org.matrix.msc3847.ignore.invites"; +export const IGNORE_INVITES_ACCOUNT_EVENT_KEY = new UnstableValue(null, "org.matrix.msc3847.ignore.invites"); /// The types of recommendations understood. enum PolicyRecommendation { @@ -126,11 +128,9 @@ export class IgnoredInvites { return false; } sources.push(roomId); - const policies = this.client.getAccountData(POLICIES_ACCOUNT_EVENT_TYPE)?.getContent() || {}; - - const ignoreInvitesPolicies = policies[IGNORE_INVITES_ACCOUNT_EVENT_KEY] || {}; - ignoreInvitesPolicies.sources = sources; - await this.client.setAccountData(POLICIES_ACCOUNT_EVENT_TYPE, policies); + await this.withIgnoreInvitesPolicies(ignoreInvitesPolicies => { + ignoreInvitesPolicies.sources = sources; + }); // Race ends. return true; @@ -214,8 +214,7 @@ export class IgnoredInvites { * other concurrent rewrites of the same object. */ public async getOrCreateTargetRoom(): Promise { - const policies = this.client.getAccountData(POLICIES_ACCOUNT_EVENT_TYPE)?.getContent() || {}; - const ignoreInvitesPolicies = policies[IGNORE_INVITES_ACCOUNT_EVENT_KEY] || {}; + const ignoreInvitesPolicies = this.getIgnoreInvitesPolicies(); let target = ignoreInvitesPolicies.target; // Validate `target`. If it is invalid, trash out the current `target` // and create a new room. @@ -236,9 +235,9 @@ export class IgnoredInvites { name: "Individual Policy Room", preset: Preset.PrivateChat, })).room_id; - ignoreInvitesPolicies.target = target; - policies[IGNORE_INVITES_ACCOUNT_EVENT_KEY] = ignoreInvitesPolicies; - await this.client.setAccountData(POLICIES_ACCOUNT_EVENT_TYPE, policies); + await this.withIgnoreInvitesPolicies(ignoreInvitesPolicies => { + ignoreInvitesPolicies.target = target; + }); // Since we have just called `createRoom`, `getRoom` should not be `null`. return this.client.getRoom(target)!; @@ -259,8 +258,7 @@ export class IgnoredInvites { * other concurrent rewrites of the same object. */ public async getOrCreateSourceRooms(): Promise { - const policies = this.client.getAccountData(POLICIES_ACCOUNT_EVENT_TYPE)?.getContent() || {}; - const ignoreInvitesPolicies = policies[IGNORE_INVITES_ACCOUNT_EVENT_KEY] || {}; + const ignoreInvitesPolicies = this.getIgnoreInvitesPolicies(); let sources = ignoreInvitesPolicies.sources; // Validate `sources`. If it is invalid, trash out the current `sources` @@ -289,13 +287,72 @@ export class IgnoredInvites { if (hasChanges) { // Reload `policies`/`ignoreInvitesPolicies` in case it has been changed // during or by our call to `this.getTargetRoom()`. - const policies = this.client.getAccountData(POLICIES_ACCOUNT_EVENT_TYPE)?.getContent() || {}; - const ignoreInvitesPolicies = policies[IGNORE_INVITES_ACCOUNT_EVENT_KEY] || {}; - sources = sourceRooms.map(room => room.roomId); - policies[IGNORE_INVITES_ACCOUNT_EVENT_KEY] = ignoreInvitesPolicies; - ignoreInvitesPolicies.sources = sources; - await this.client.setAccountData(POLICIES_ACCOUNT_EVENT_TYPE, policies); + await this.withIgnoreInvitesPolicies(ignoreInvitesPolicies => { + ignoreInvitesPolicies.sources = sources; + }); } return sourceRooms; } + + /** + * Fetch the `IGNORE_INVITES_POLICIES` object from account data. + * + * If both an unstable prefix version and a stable prefix version are available, + * it will return the stable prefix version preferentially. + * + * The result is *not* validated but is guaranteed to be a non-null object. + * + * @returns A non-null object. + */ + private getIgnoreInvitesPolicies(): {[key: string]: any} { + return this.getPoliciesAndIgnoreInvitesPolicies().ignoreInvitesPolicies; + } + + /** + * Modify in place the `IGNORE_INVITES_POLICIES` object from account data. + */ + private async withIgnoreInvitesPolicies(cb: (ignoreInvitesPolicies: {[key: string]: any}) => void) { + const { policies, ignoreInvitesPolicies } = this.getPoliciesAndIgnoreInvitesPolicies(); + cb(ignoreInvitesPolicies); + policies[IGNORE_INVITES_ACCOUNT_EVENT_KEY.name] = ignoreInvitesPolicies; + await this.client.setAccountData(POLICIES_ACCOUNT_EVENT_TYPE.name, policies); + } + + /** + * As `getIgnoreInvitesPolicies` but also return the `POLICIES_ACCOUNT_EVENT_TYPE` + * object. + */ + private getPoliciesAndIgnoreInvitesPolicies(): + {policies: {[key: string]: any}, ignoreInvitesPolicies: {[key: string]: any}} { + let policies = {}; + for (const key of [POLICIES_ACCOUNT_EVENT_TYPE.name, POLICIES_ACCOUNT_EVENT_TYPE.altName]) { + if (!key) { + continue; + } + const value = this.client.getAccountData(key)?.getContent(); + if (value) { + policies = value; + break; + } + } + + let ignoreInvitesPolicies = {}; + let hasIgnoreInvitesPolicies = false; + for (const key of [IGNORE_INVITES_ACCOUNT_EVENT_KEY.name, IGNORE_INVITES_ACCOUNT_EVENT_KEY.altName]) { + if (!key) { + continue; + } + const value = policies[key]; + if (value && typeof value == "object") { + ignoreInvitesPolicies = value; + hasIgnoreInvitesPolicies = true; + break; + } + } + if (!hasIgnoreInvitesPolicies) { + policies[IGNORE_INVITES_ACCOUNT_EVENT_KEY.name] = ignoreInvitesPolicies; + } + + return { policies, ignoreInvitesPolicies }; + } } From 335cf9f1dc45559d0ce15b85a54dd40fc2334318 Mon Sep 17 00:00:00 2001 From: David Teller Date: Mon, 5 Sep 2022 19:07:34 +0200 Subject: [PATCH 5/6] WIP: CI linter gives me different errors, weird --- src/models/invites-ignorer.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/models/invites-ignorer.ts b/src/models/invites-ignorer.ts index 6754827be9..d2c61ef787 100644 --- a/src/models/invites-ignorer.ts +++ b/src/models/invites-ignorer.ts @@ -24,12 +24,12 @@ import { Room } from "./room"; /// The event type storing the user's individual policies. /// /// Exported for testing purposes. -export const POLICIES_ACCOUNT_EVENT_TYPE = new UnstableValue(null, "org.matrix.msc3847.policies"); +export const POLICIES_ACCOUNT_EVENT_TYPE = new UnstableValue("m.policies", "org.matrix.msc3847.policies"); /// The key within the user's individual policies storing the user's ignored invites. /// /// Exported for testing purposes. -export const IGNORE_INVITES_ACCOUNT_EVENT_KEY = new UnstableValue(null, "org.matrix.msc3847.ignore.invites"); +export const IGNORE_INVITES_ACCOUNT_EVENT_KEY = new UnstableValue("m.ignore.invites", "org.matrix.msc3847.ignore.invites"); /// The types of recommendations understood. enum PolicyRecommendation { From 78d2f2a3fa2cece6fb1e41a6ed53a486a89c1a6c Mon Sep 17 00:00:00 2001 From: David Teller Date: Tue, 6 Sep 2022 08:39:51 +0200 Subject: [PATCH 6/6] WIP: A little more linting --- src/models/invites-ignorer.ts | 3 ++- src/utils.ts | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/models/invites-ignorer.ts b/src/models/invites-ignorer.ts index d2c61ef787..d70a366749 100644 --- a/src/models/invites-ignorer.ts +++ b/src/models/invites-ignorer.ts @@ -29,7 +29,8 @@ export const POLICIES_ACCOUNT_EVENT_TYPE = new UnstableValue("m.policies", "org. /// The key within the user's individual policies storing the user's ignored invites. /// /// Exported for testing purposes. -export const IGNORE_INVITES_ACCOUNT_EVENT_KEY = new UnstableValue("m.ignore.invites", "org.matrix.msc3847.ignore.invites"); +export const IGNORE_INVITES_ACCOUNT_EVENT_KEY = new UnstableValue("m.ignore.invites", + "org.matrix.msc3847.ignore.invites"); /// The types of recommendations understood. enum PolicyRecommendation { diff --git a/src/utils.ts b/src/utils.ts index ffb438d5b9..13f5dd44ce 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -341,7 +341,7 @@ export function escapeRegExp(string: string): string { return string.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); } -export function globToRegexp(glob: string, extended?: any): string { +export function globToRegexp(glob: string, extended = false): string { // From // https://github.com/matrix-org/synapse/blob/abbee6b29be80a77e05730707602f3bbfc3f38cb/synapse/push/__init__.py#L132 // Because micromatch is about 130KB with dependencies, @@ -349,7 +349,7 @@ export function globToRegexp(glob: string, extended?: any): string { const replacements: ([RegExp, string | ((substring: string, ...args: any[]) => string) ])[] = [ [/\\\*/g, '.*'], [/\?/g, '.'], - extended !== false && [ + !extended && [ /\\\[(!|)(.*)\\]/g, (_match: string, neg: string, pat: string) => [ '[',