From c91528804454b405df1b2f170e787baddb197a1b Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 6 Sep 2021 14:34:06 +0100 Subject: [PATCH 1/2] Respect m.space.parent relations if they hold valid permissions --- src/stores/SpaceStore.tsx | 31 +++++++++++++++---- test/stores/SpaceStore-test.ts | 54 +++++++++++++++++++++++++++++++++- 2 files changed, 78 insertions(+), 7 deletions(-) diff --git a/src/stores/SpaceStore.tsx b/src/stores/SpaceStore.tsx index ff99b38fe3..edbe8327c7 100644 --- a/src/stores/SpaceStore.tsx +++ b/src/stores/SpaceStore.tsx @@ -366,16 +366,18 @@ export class SpaceStoreClass extends AsyncStoreWithClient { } public getParents(roomId: string, canonicalOnly = false): Room[] { + const userId = this.matrixClient?.getUserId(); const room = this.matrixClient?.getRoom(roomId); return room?.currentState.getStateEvents(EventType.SpaceParent) - .filter(ev => { + .map(ev => { const content = ev.getContent(); - if (!content?.via?.length) return false; - // TODO apply permissions check to verify that the parent mapping is valid - if (canonicalOnly && !content?.canonical) return false; - return true; + if (!Array.isArray(content?.via)) return; + const parent = this.matrixClient.getRoom(ev.getStateKey()); + if (canonicalOnly && !content?.canonical) return; + if (parent.currentState.maySendStateEvent(EventType.SpaceChild, userId)) { + return parent; + } }) - .map(ev => this.matrixClient.getRoom(ev.getStateKey())) .filter(Boolean) || []; } @@ -530,6 +532,14 @@ export class SpaceStoreClass extends AsyncStoreWithClient { }); } + const hiddenChildren = new EnhancedMap>(); + visibleRooms.forEach(room => { + if (room.getMyMembership() !== "join") return; + this.getParents(room.roomId).forEach(parent => { + hiddenChildren.getOrCreate(parent.roomId, new Set()).add(room.roomId); + }); + }); + this.rootSpaces.forEach(s => { // traverse each space tree in DFS to build up the supersets as you go up, // reusing results from like subtrees. @@ -559,6 +569,9 @@ export class SpaceStoreClass extends AsyncStoreWithClient { roomIds.add(roomId); }); }); + hiddenChildren.get(spaceId)?.forEach(roomId => { + roomIds.add(roomId); + }); this.spaceFilteredRooms.set(spaceId, roomIds); return roomIds; }; @@ -690,6 +703,12 @@ export class SpaceStoreClass extends AsyncStoreWithClient { } this.emit(room.roomId); break; + + case EventType.RoomPowerLevels: + if (room.isSpaceRoom()) { + this.onRoomsUpdate(); + } + break; } }; diff --git a/test/stores/SpaceStore-test.ts b/test/stores/SpaceStore-test.ts index 7cfd97b234..698bd01370 100644 --- a/test/stores/SpaceStore-test.ts +++ b/test/stores/SpaceStore-test.ts @@ -276,10 +276,12 @@ describe("SpaceStore", () => { describe("test fixture 1", () => { beforeEach(async () => { - [fav1, fav2, fav3, dm1, dm2, dm3, orphan1, orphan2, invite1, invite2, room1].forEach(mkRoom); + [fav1, fav2, fav3, dm1, dm2, dm3, orphan1, orphan2, invite1, invite2, room1, room2, room3] + .forEach(mkRoom); mkSpace(space1, [fav1, room1]); mkSpace(space2, [fav1, fav2, fav3, room1]); mkSpace(space3, [invite2]); + // client.getRoom.mockImplementation(roomId => rooms.find(room => room.roomId === roomId)); [fav1, fav2, fav3].forEach(roomId => { client.getRoom(roomId).tags = { @@ -329,6 +331,48 @@ describe("SpaceStore", () => { ]); // dmPartner3 is not in any common spaces with you + // room 2 claims to be a child of space2 and is so via a valid m.space.parent + const cliRoom2 = client.getRoom(room2); + cliRoom2.currentState.getStateEvents.mockImplementation(testUtils.mockStateEventImplementation([ + mkEvent({ + event: true, + type: EventType.SpaceParent, + room: room2, + user: client.getUserId(), + skey: space2, + content: { via: [], canonical: true }, + ts: Date.now(), + }), + ])); + const cliSpace2 = client.getRoom(space2); + cliSpace2.currentState.maySendStateEvent.mockImplementation((evType: string, userId: string) => { + if (evType === EventType.SpaceChild) { + return userId === client.getUserId(); + } + return true; + }); + + // room 3 claims to be a child of space3 but is not due to invalid m.space.parent (permissions) + const cliRoom3 = client.getRoom(room3); + cliRoom3.currentState.getStateEvents.mockImplementation(testUtils.mockStateEventImplementation([ + mkEvent({ + event: true, + type: EventType.SpaceParent, + room: room3, + user: client.getUserId(), + skey: space3, + content: { via: [], canonical: true }, + ts: Date.now(), + }), + ])); + const cliSpace3 = client.getRoom(space3); + cliSpace3.currentState.maySendStateEvent.mockImplementation((evType: string, userId: string) => { + if (evType === EventType.SpaceChild) { + return false; + } + return true; + }); + await run(); }); @@ -445,6 +489,14 @@ describe("SpaceStore", () => { expect(store.getNotificationState(space2).rooms.map(r => r.roomId).includes(room1)).toBeTruthy(); expect(store.getNotificationState(space3).rooms.map(r => r.roomId).includes(room1)).toBeFalsy(); }); + + it("honours m.space.parent if sender has permission in parent space", () => { + expect(store.getSpaceFilteredRoomIds(client.getRoom(space2)).has(room2)).toBeTruthy(); + }); + + it("does not honour m.space.parent if sender does not have permission in parent space", () => { + expect(store.getSpaceFilteredRoomIds(client.getRoom(space3)).has(room3)).toBeFalsy(); + }); }); }); From f4f4686270bfee51d4d96320b606b6c6f219977a Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 7 Sep 2021 12:07:18 +0100 Subject: [PATCH 2/2] tidy up code --- src/stores/SpaceStore.tsx | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/stores/SpaceStore.tsx b/src/stores/SpaceStore.tsx index edbe8327c7..ebc60b2f2c 100644 --- a/src/stores/SpaceStore.tsx +++ b/src/stores/SpaceStore.tsx @@ -371,12 +371,16 @@ export class SpaceStoreClass extends AsyncStoreWithClient { return room?.currentState.getStateEvents(EventType.SpaceParent) .map(ev => { const content = ev.getContent(); - if (!Array.isArray(content?.via)) return; - const parent = this.matrixClient.getRoom(ev.getStateKey()); - if (canonicalOnly && !content?.canonical) return; - if (parent.currentState.maySendStateEvent(EventType.SpaceChild, userId)) { - return parent; + if (Array.isArray(content?.via) && (!canonicalOnly || content?.canonical)) { + const parent = this.matrixClient.getRoom(ev.getStateKey()); + // only respect the relationship if the sender has sufficient permissions in the parent to set + // child relations, as per MSC1772. + // https://github.com/matrix-org/matrix-doc/blob/main/proposals/1772-groups-as-rooms.md#relationship-between-rooms-and-spaces + if (parent?.currentState.maySendStateEvent(EventType.SpaceChild, userId)) { + return parent; + } } + // else implicit undefined which causes this element to be filtered out }) .filter(Boolean) || []; }