diff --git a/changelog.d/10560.feature b/changelog.d/10560.feature
new file mode 100644
index 0000000000..ffc4e4289c
--- /dev/null
+++ b/changelog.d/10560.feature
@@ -0,0 +1 @@
+Add pagination to the spaces summary based on updates to [MSC2946](https://github.com/matrix-org/matrix-doc/pull/2946).
diff --git a/synapse/handlers/space_summary.py b/synapse/handlers/space_summary.py
index 2517f278b6..d04afe6c31 100644
--- a/synapse/handlers/space_summary.py
+++ b/synapse/handlers/space_summary.py
@@ -158,48 +158,10 @@ class SpaceSummaryHandler:
room = room_entry.room
fed_room_id = room_entry.room_id
- # The room should only be included in the summary if:
- # a. the user is in the room;
- # b. the room is world readable; or
- # c. the user could join the room, e.g. the join rules
- # are set to public or the user is in a space that
- # has been granted access to the room.
- #
- # Note that we know the user is not in the root room (which is
- # why the remote call was made in the first place), but the user
- # could be in one of the children rooms and we just didn't know
- # about the link.
-
- # The API doesn't return the room version so assume that a
- # join rule of knock is valid.
- include_room = (
- room.get("join_rules") in (JoinRules.PUBLIC, JoinRules.KNOCK)
- or room.get("world_readable") is True
- )
-
- # Check if the user is a member of any of the allowed spaces
- # from the response.
- allowed_rooms = room.get("allowed_room_ids") or room.get(
- "allowed_spaces"
- )
- if (
- not include_room
- and allowed_rooms
- and isinstance(allowed_rooms, list)
- ):
- include_room = await self._event_auth_handler.is_user_in_rooms(
- allowed_rooms, requester
- )
-
- # Finally, if this isn't the requested room, check ourselves
- # if we can access the room.
- if not include_room and fed_room_id != queue_entry.room_id:
- include_room = await self._is_room_accessible(
- fed_room_id, requester, None
- )
-
# The user can see the room, include it!
- if include_room:
+ if await self._is_remote_room_accessible(
+ requester, fed_room_id, room
+ ):
# Before returning to the client, remove the allowed_room_ids
# and allowed_spaces keys.
room.pop("allowed_room_ids", None)
@@ -336,7 +298,7 @@ class SpaceSummaryHandler:
Returns:
A room entry if the room should be returned. None, otherwise.
"""
- if not await self._is_room_accessible(room_id, requester, origin):
+ if not await self._is_local_room_accessible(room_id, requester, origin):
return None
room_entry = await self._build_room_entry(room_id, for_federation=bool(origin))
@@ -438,7 +400,7 @@ class SpaceSummaryHandler:
return results
- async def _is_room_accessible(
+ async def _is_local_room_accessible(
self, room_id: str, requester: Optional[str], origin: Optional[str]
) -> bool:
"""
@@ -550,6 +512,51 @@ class SpaceSummaryHandler:
)
return False
+ async def _is_remote_room_accessible(
+ self, requester: str, room_id: str, room: JsonDict
+ ) -> bool:
+ """
+ Calculate whether the room received over federation should be shown in the spaces summary.
+
+ It should be included if:
+
+ * The requester is joined or can join the room (per MSC3173).
+ * The history visibility is set to world readable.
+
+ Note that the local server is not in the requested room (which is why the
+ remote call was made in the first place), but the user could have access
+ due to an invite, etc.
+
+ Args:
+ requester: The user requesting the summary.
+ room_id: The room ID returned over federation.
+ room: The summary of the child room returned over federation.
+
+ Returns:
+ True if the room should be included in the spaces summary.
+ """
+ # The API doesn't return the room version so assume that a
+ # join rule of knock is valid.
+ if (
+ room.get("join_rules") in (JoinRules.PUBLIC, JoinRules.KNOCK)
+ or room.get("world_readable") is True
+ ):
+ return True
+
+ # Check if the user is a member of any of the allowed spaces
+ # from the response.
+ allowed_rooms = room.get("allowed_room_ids") or room.get("allowed_spaces")
+ if allowed_rooms and isinstance(allowed_rooms, list):
+ if await self._event_auth_handler.is_user_in_rooms(
+ allowed_rooms, requester
+ ):
+ return True
+
+ # Finally, check locally if we can access the room. The user might
+ # already be in the room (if it was a child room), or there might be a
+ # pending invite, etc.
+ return await self._is_local_room_accessible(room_id, requester, None)
+
async def _build_room_entry(self, room_id: str, for_federation: bool) -> JsonDict:
"""
Generate en entry suitable for the 'rooms' list in the summary response.
diff --git a/tests/handlers/test_space_summary.py b/tests/handlers/test_space_summary.py
index 6cc1a02e12..f470c81ea2 100644
--- a/tests/handlers/test_space_summary.py
+++ b/tests/handlers/test_space_summary.py
@@ -30,7 +30,7 @@ from synapse.handlers.space_summary import _child_events_comparison_key, _RoomEn
from synapse.rest import admin
from synapse.rest.client.v1 import login, room
from synapse.server import HomeServer
-from synapse.types import JsonDict
+from synapse.types import JsonDict, UserID
from tests import unittest
@@ -149,6 +149,36 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase):
events,
)
+ def _poke_fed_invite(self, room_id: str, from_user: str) -> None:
+ """
+ Creates a invite (as if received over federation) for the room from the
+ given hostname.
+
+ Args:
+ room_id: The room ID to issue an invite for.
+ fed_hostname: The user to invite from.
+ """
+ # Poke an invite over federation into the database.
+ fed_handler = self.hs.get_federation_handler()
+ fed_hostname = UserID.from_string(from_user).domain
+ event = make_event_from_dict(
+ {
+ "room_id": room_id,
+ "event_id": "!abcd:" + fed_hostname,
+ "type": EventTypes.Member,
+ "sender": from_user,
+ "state_key": self.user,
+ "content": {"membership": Membership.INVITE},
+ "prev_events": [],
+ "auth_events": [],
+ "depth": 1,
+ "origin_server_ts": 1234,
+ }
+ )
+ self.get_success(
+ fed_handler.on_invite_request(fed_hostname, event, RoomVersions.V6)
+ )
+
def test_simple_space(self):
"""Test a simple space with a single room."""
result = self.get_success(self.handler.get_space_summary(self.user, self.space))
@@ -416,24 +446,7 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase):
joined_room = self.helper.create_room_as(self.user, tok=self.token)
# Poke an invite over federation into the database.
- fed_handler = self.hs.get_federation_handler()
- event = make_event_from_dict(
- {
- "room_id": invited_room,
- "event_id": "!abcd:" + fed_hostname,
- "type": EventTypes.Member,
- "sender": "@remote:" + fed_hostname,
- "state_key": self.user,
- "content": {"membership": Membership.INVITE},
- "prev_events": [],
- "auth_events": [],
- "depth": 1,
- "origin_server_ts": 1234,
- }
- )
- self.get_success(
- fed_handler.on_invite_request(fed_hostname, event, RoomVersions.V6)
- )
+ self._poke_fed_invite(invited_room, "@remote:" + fed_hostname)
async def summarize_remote_room(
_self, room, suggested_only, max_children, exclude_rooms
@@ -570,3 +583,58 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase):
(subspace, joined_room),
],
)
+
+ def test_fed_invited(self):
+ """
+ A room which the user was invited to should be included in the response.
+
+ This differs from test_fed_filtering in that the room itself is being
+ queried over federation, instead of it being included as a sub-room of
+ a space in the response.
+ """
+ fed_hostname = self.hs.hostname + "2"
+ fed_room = "#subroom:" + fed_hostname
+
+ # Poke an invite over federation into the database.
+ self._poke_fed_invite(fed_room, "@remote:" + fed_hostname)
+
+ async def summarize_remote_room(
+ _self, room, suggested_only, max_children, exclude_rooms
+ ):
+ return [
+ _RoomEntry(
+ fed_room,
+ {
+ "room_id": fed_room,
+ "world_readable": False,
+ "join_rules": JoinRules.INVITE,
+ },
+ ),
+ ]
+
+ # Add a room to the space which is on another server.
+ self._add_child(self.space, fed_room, self.token)
+
+ with mock.patch(
+ "synapse.handlers.space_summary.SpaceSummaryHandler._summarize_remote_room",
+ new=summarize_remote_room,
+ ):
+ result = self.get_success(
+ self.handler.get_space_summary(self.user, self.space)
+ )
+
+ self._assert_rooms(
+ result,
+ [
+ self.space,
+ self.room,
+ fed_room,
+ ],
+ )
+ self._assert_events(
+ result,
+ [
+ (self.space, self.room),
+ (self.space, fed_room),
+ ],
+ )
|
