Merge pull request #6746 from matrix-org/t3chguy/fix/10935

matrix-org · Sep 8, 2021 · 0d0eea3 · 0d0eea3
2 parents 075b061 + f4f4686
commit 0d0eea3
Show file tree

Hide file tree

Showing 2 changed files with 82 additions and 7 deletions.
diff --git a/src/stores/SpaceStore.tsx b/src/stores/SpaceStore.tsx
@@ -366,16 +366,22 @@ export class SpaceStoreClass extends AsyncStoreWithClient<IState> {
     }
 
     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) && (!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
             })
-            .map(ev => this.matrixClient.getRoom(ev.getStateKey()))
             .filter(Boolean) || [];
     }
 
@@ -530,6 +536,14 @@ export class SpaceStoreClass extends AsyncStoreWithClient<IState> {
             });
         }
 
+        const hiddenChildren = new EnhancedMap<string, Set<string>>();
+        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 +573,9 @@ export class SpaceStoreClass extends AsyncStoreWithClient<IState> {
                         roomIds.add(roomId);
                     });
                 });
+                hiddenChildren.get(spaceId)?.forEach(roomId => {
+                    roomIds.add(roomId);
+                });
                 this.spaceFilteredRooms.set(spaceId, roomIds);
                 return roomIds;
             };
@@ -690,6 +707,12 @@ export class SpaceStoreClass extends AsyncStoreWithClient<IState> {
                 }
                 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
@@ -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();
+            });
         });
     });