From 7d9c34d69b672befdb3bc133329caa00df119bc4 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 14 Jul 2021 12:48:40 +0100 Subject: [PATCH 01/35] Extract ResolveRoomIdMixin and reuse it for /_matrix/client/r0/join/{roomIdOrAlias} Signed-off-by: Michael Telatynski --- synapse/http/servlet.py | 50 ++++++++++++++++++++++++++++++++-- synapse/rest/admin/rooms.py | 45 ++---------------------------- synapse/rest/client/v1/room.py | 32 ++++++++-------------- 3 files changed, 62 insertions(+), 65 deletions(-) diff --git a/synapse/http/servlet.py b/synapse/http/servlet.py index 6ba2ce1e53a2..88e8cdbfbc6c 100644 --- a/synapse/http/servlet.py +++ b/synapse/http/servlet.py @@ -14,7 +14,7 @@ """ This module contains base REST classes for constructing REST servlets. """ import logging -from typing import Dict, Iterable, List, Optional, overload +from typing import TYPE_CHECKING, Dict, Iterable, List, Optional, overload, Tuple from typing_extensions import Literal @@ -22,13 +22,17 @@ from synapse.api.errors import Codes, SynapseError from synapse.util import json_decoder +from synapse.types import RoomAlias, RoomID + +if TYPE_CHECKING: + from synapse.server import HomeServer logger = logging.getLogger(__name__) def parse_integer(request, name, default=None, required=False): """Parse an integer parameter from the request string - +thank Args: request: the twisted HTTP request. name (bytes/unicode): the name of the query parameter. @@ -509,3 +513,45 @@ def register(self, http_server): else: raise NotImplementedError("RestServlet must register something.") + + +class ResolveRoomIdMixin: + def __init__(self, hs: "HomeServer"): + self.room_member_handler = hs.get_room_member_handler() + + async def resolve_room_id( + self, room_identifier: str, remote_room_hosts: Optional[List[str]] = None + ) -> Tuple[str, Optional[List[str]]]: + """ + Resolve a room identifier to a room ID, if necessary. + + This also performanes checks to ensure the room ID is of the proper form. + + Args: + room_identifier: The room ID or alias. + remote_room_hosts: The potential remote room hosts to use. + + Returns: + The resolved room ID. + + Raises: + SynapseError if the room ID is of the wrong form. + """ + if RoomID.is_valid(room_identifier): + resolved_room_id = room_identifier + elif RoomAlias.is_valid(room_identifier): + room_alias = RoomAlias.from_string(room_identifier) + ( + room_id, + remote_room_hosts, + ) = await self.room_member_handler.lookup_room_alias(room_alias) + resolved_room_id = room_id.to_string() + else: + raise SynapseError( + 400, "%s was not legal room ID or room alias" % (room_identifier,) + ) + if not resolved_room_id: + raise SynapseError( + 400, "Unknown room ID or room alias %s" % room_identifier + ) + return resolved_room_id, remote_room_hosts \ No newline at end of file diff --git a/synapse/rest/admin/rooms.py b/synapse/rest/admin/rooms.py index 3c51a742bfc7..aa1591a3fc83 100644 --- a/synapse/rest/admin/rooms.py +++ b/synapse/rest/admin/rooms.py @@ -25,6 +25,7 @@ parse_integer, parse_json_object_from_request, parse_string, + ResolveRoomIdMixin, ) from synapse.http.site import SynapseRequest from synapse.rest.admin._base import ( @@ -33,7 +34,7 @@ assert_user_is_admin, ) from synapse.storage.databases.main.room import RoomSortOrder -from synapse.types import JsonDict, RoomAlias, RoomID, UserID, create_requester +from synapse.types import JsonDict, UserID, create_requester from synapse.util import json_decoder if TYPE_CHECKING: @@ -45,48 +46,6 @@ logger = logging.getLogger(__name__) -class ResolveRoomIdMixin: - def __init__(self, hs: "HomeServer"): - self.room_member_handler = hs.get_room_member_handler() - - async def resolve_room_id( - self, room_identifier: str, remote_room_hosts: Optional[List[str]] = None - ) -> Tuple[str, Optional[List[str]]]: - """ - Resolve a room identifier to a room ID, if necessary. - - This also performanes checks to ensure the room ID is of the proper form. - - Args: - room_identifier: The room ID or alias. - remote_room_hosts: The potential remote room hosts to use. - - Returns: - The resolved room ID. - - Raises: - SynapseError if the room ID is of the wrong form. - """ - if RoomID.is_valid(room_identifier): - resolved_room_id = room_identifier - elif RoomAlias.is_valid(room_identifier): - room_alias = RoomAlias.from_string(room_identifier) - ( - room_id, - remote_room_hosts, - ) = await self.room_member_handler.lookup_room_alias(room_alias) - resolved_room_id = room_id.to_string() - else: - raise SynapseError( - 400, "%s was not legal room ID or room alias" % (room_identifier,) - ) - if not resolved_room_id: - raise SynapseError( - 400, "Unknown room ID or room alias %s" % room_identifier - ) - return resolved_room_id, remote_room_hosts - - class ShutdownRoomRestServlet(RestServlet): """Shuts down a room by removing all local users from the room and blocking all future invites and joins to the room. Any local aliases will be repointed diff --git a/synapse/rest/client/v1/room.py b/synapse/rest/client/v1/room.py index ebf4e3223089..68e18ef9a072 100644 --- a/synapse/rest/client/v1/room.py +++ b/synapse/rest/client/v1/room.py @@ -39,6 +39,7 @@ parse_json_object_from_request, parse_string, parse_strings_from_args, + ResolveRoomIdMixin, ) from synapse.http.site import SynapseRequest from synapse.logging.opentracing import set_tag @@ -650,10 +651,10 @@ def on_PUT(self, request, room_id): # TODO: Needs unit testing for room ID + alias joins -class JoinRoomAliasServlet(TransactionRestServlet): +class JoinRoomAliasServlet(ResolveRoomIdMixin, TransactionRestServlet): def __init__(self, hs): super().__init__(hs) - self.room_member_handler = hs.get_room_member_handler() + super(ResolveRoomIdMixin, self).__init__(hs) # ensure the Mixin is set up self.auth = hs.get_auth() def register(self, http_server): @@ -676,24 +677,15 @@ async def on_POST( # cheekily send invalid bodies. content = {} - if RoomID.is_valid(room_identifier): - room_id = room_identifier - - # twisted.web.server.Request.args is incorrectly defined as Optional[Any] - args: Dict[bytes, List[bytes]] = request.args # type: ignore - - remote_room_hosts = parse_strings_from_args( - args, "server_name", required=False - ) - elif RoomAlias.is_valid(room_identifier): - handler = self.room_member_handler - room_alias = RoomAlias.from_string(room_identifier) - room_id_obj, remote_room_hosts = await handler.lookup_room_alias(room_alias) - room_id = room_id_obj.to_string() - else: - raise SynapseError( - 400, "%s was not legal room ID or room alias" % (room_identifier,) - ) + # twisted.web.server.Request.args is incorrectly defined as Optional[Any] + args: Dict[bytes, List[bytes]] = request.args # type: ignore + remote_room_hosts = parse_strings_from_args( + args, "server_name", required=False + ) + room_id, remote_room_hosts = await self.resolve_room_id( + room_identifier, + remote_room_hosts, + ) await self.room_member_handler.update_membership( requester=requester, From ef295d1267d776703f5c4237ae2a55b6be322166 Mon Sep 17 00:00:00 2001 From: Michael Telatynski Date: Wed, 14 Jul 2021 14:34:19 +0100 Subject: [PATCH 02/35] First cut of a local-only poc for MSC3266 Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- synapse/config/experimental.py | 3 + synapse/handlers/room_summary.py | 285 +++++++++++++++++++++++++++++++ synapse/rest/client/v1/room.py | 43 +++++ synapse/server.py | 5 + 4 files changed, 336 insertions(+) create mode 100644 synapse/handlers/room_summary.py diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index 7fb1f7021f5e..299550b72513 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -32,3 +32,6 @@ def read_config(self, config: JsonDict, **kwargs): # MSC2716 (backfill existing history) self.msc2716_enabled = experimental.get("msc2716_enabled", False) # type: bool + + # MSC3266 (room summary api) + self.mxc3266_enabled = experimental.get("mxc3266_enabled", False) # type: bool diff --git a/synapse/handlers/room_summary.py b/synapse/handlers/room_summary.py new file mode 100644 index 000000000000..7d010f7b187a --- /dev/null +++ b/synapse/handlers/room_summary.py @@ -0,0 +1,285 @@ +# Copyright 2021 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 logging +from typing import TYPE_CHECKING, List, Optional + +from synapse.api.constants import ( + EventContentFields, + EventTypes, + HistoryVisibility, + JoinRules, + Membership, +) +from synapse.types import JsonDict + +if TYPE_CHECKING: + from synapse.server import HomeServer + +logger = logging.getLogger(__name__) + + +class RoomSummaryHandler: + def __init__(self, hs: "HomeServer"): + self._clock = hs.get_clock() + self._auth = hs.get_auth() + self._event_auth_handler = hs.get_event_auth_handler() + self._store = hs.get_datastore() + self._event_serializer = hs.get_event_client_serializer() + self._server_name = hs.hostname + self._federation_client = hs.get_federation_client() + + async def get_room_summary( + self, + requester: Optional[str], + room_id: str, + remote_room_hosts: List[str], + ) -> JsonDict: + """ + Implementation of the room summary C-S API MSC3244 + + Args: + requester: user id of the user making this request, + can be None for unauthenticated requests + + room_id: room id to start the summary at + + remote_room_hosts: a list of homeservers to try fetching data through + if we don't know it ourselves + + Returns: + summary dict to return + """ + is_in_room = await self._store.is_host_joined(room_id, self._server_name) + + if is_in_room: + room_summary = await self._summarize_local_room(requester, None, room_id) + + if requester: + membership, _ = await self._store.get_local_current_membership_for_user_in_room( + requester, room_id + ) + + room_summary["membership"] = membership or "leave" + else: + room_summary = await self._summarize_remote_room(room_id, remote_room_hosts) + + # TODO validate that the requester has permission to see this room + # https://github.com/matrix-org/matrix-doc/pull/3266/files#diff-97aeb566f3ce4bd6ec3b98e71ecbca3d6e86c0407e6a82afbc57e86bf0316607R106-R108 + + return room_summary + + async def _summarize_local_room( + self, + requester: Optional[str], + origin: Optional[str], + room_id: str, + ) -> JsonDict: + """ + Generate a room entry and a list of event entries for a given room. + + Args: + requester: + The user requesting the summary, if it is a local request. None + if this is a federation request. + origin: + The server requesting the summary, if it is a federation request. + None if this is a local request. + room_id: The room ID to summarize. + + Returns: + summary dict to return + """ + if not await self._is_room_accessible(room_id, requester, origin): + return None + + return await self._build_room_entry(room_id) + + async def _summarize_remote_room( + self, + room_id: str, + remote_room_hosts: List[str], + ) -> JsonDict: + """ + Request room entries and a list of event entries for a given room by querying a remote server. + + Args: + room_id: The room to summarize. + remote_room_hosts: List of homeservers to attempt to fetch the data from. + + Returns: + summary dict to return + """ + logger.info("Requesting summary for %s via %s", room_id, remote_room_hosts) + + return None # TODO federation API + + # TODO extract into mixin/helper method + async def _is_room_accessible( + self, room_id: str, requester: Optional[str], origin: Optional[str] + ) -> bool: + """ + Calculate whether the room should be shown to the requester. + + It should be included if: + + * The requester is joined or can join the room (per MSC3173). + * The origin server has any user that is joined or can join the room. + * The history visibility is set to world readable. + + Args: + room_id: The room ID to summarize. + requester: + The user requesting the summary, if it is a local request. None + if this is a federation request. + origin: + The server requesting the summary, if it is a federation request. + None if this is a local request. + + Returns: + True if the room should be visible to the requester. + """ + state_ids = await self._store.get_current_state_ids(room_id) + + # If there's no state for the room, it isn't known. + if not state_ids: + # The user might have a pending invite for the room. + if requester and await self._store.get_invite_for_local_user_in_room( + requester, room_id + ): + return True + + logger.info("room %s is unknown, omitting from summary", room_id) + return False + + room_version = await self._store.get_room_version(room_id) + + # Include the room if it has join rules of public or knock. + join_rules_event_id = state_ids.get((EventTypes.JoinRules, "")) + if join_rules_event_id: + join_rules_event = await self._store.get_event(join_rules_event_id) + join_rule = join_rules_event.content.get("join_rule") + if join_rule == JoinRules.PUBLIC or ( + room_version.msc2403_knocking and join_rule == JoinRules.KNOCK + ): + return True + + # Include the room if it is peekable. + hist_vis_event_id = state_ids.get((EventTypes.RoomHistoryVisibility, "")) + if hist_vis_event_id: + hist_vis_ev = await self._store.get_event(hist_vis_event_id) + hist_vis = hist_vis_ev.content.get("history_visibility") + if hist_vis == HistoryVisibility.WORLD_READABLE: + return True + + # Otherwise we need to check information specific to the user or server. + + # If we have an authenticated requesting user, check if they are a member + # of the room (or can join the room). + if requester: + member_event_id = state_ids.get((EventTypes.Member, requester), None) + + # If they're in the room they can see info on it. + if member_event_id: + member_event = await self._store.get_event(member_event_id) + if member_event.membership in (Membership.JOIN, Membership.INVITE): + return True + + # Otherwise, check if they should be allowed access via membership in a space. + if await self._event_auth_handler.has_restricted_join_rules( + state_ids, room_version + ): + allowed_rooms = ( + await self._event_auth_handler.get_rooms_that_allow_join(state_ids) + ) + if await self._event_auth_handler.is_user_in_rooms( + allowed_rooms, requester + ): + return True + + # If this is a request over federation, check if the host is in the room or + # has a user who could join the room. + elif origin: + if await self._event_auth_handler.check_host_in_room( + room_id, origin + ) or await self._store.is_host_invited(room_id, origin): + return True + + # Alternately, if the host has a user in any of the spaces specified + # for access, then the host can see this room (and should do filtering + # if the requester cannot see it). + if await self._event_auth_handler.has_restricted_join_rules( + state_ids, room_version + ): + allowed_rooms = ( + await self._event_auth_handler.get_rooms_that_allow_join(state_ids) + ) + for space_id in allowed_rooms: + if await self._event_auth_handler.check_host_in_room( + space_id, origin + ): + return True + + logger.info( + "room %s is unpeekable and requester %s is not a member / not allowed to join, omitting from summary", + room_id, + requester or origin, + ) + return False + + async def _build_room_entry(self, room_id: str) -> JsonDict: + """Generate en entry suitable for the 'rooms' list in the summary response""" + stats = await self._store.get_room_with_stats(room_id) + + # currently this should be impossible because we call + # check_user_in_room_or_world_readable on the room before we get here, so + # there should always be an entry + assert stats is not None, "unable to retrieve stats for %s" % (room_id,) + + current_state_ids = await self._store.get_current_state_ids(room_id) + create_event = await self._store.get_event( + current_state_ids[(EventTypes.Create, "")] + ) + + room_version = await self._store.get_room_version(room_id) + allowed_rooms = None + if await self._event_auth_handler.has_restricted_join_rules( + current_state_ids, room_version + ): + allowed_rooms = await self._event_auth_handler.get_rooms_that_allow_join( + current_state_ids + ) + + entry = { + "room_id": stats["room_id"], + "name": stats["name"], + "topic": stats["topic"], + "canonical_alias": stats["canonical_alias"], + "num_joined_members": stats["joined_members"], + "avatar_url": stats["avatar"], + "join_rules": stats["join_rules"], + "world_readable": ( + stats["history_visibility"] == HistoryVisibility.WORLD_READABLE + ), + "guest_can_join": stats["guest_access"] == "can_join", + "creation_ts": create_event.origin_server_ts, + "room_type": create_event.content.get(EventContentFields.ROOM_TYPE), + "allowed_spaces": allowed_rooms, # TODO unspecced for MSC3266 + "is_encrypted": (EventTypes.RoomEncryption, "") in current_state_ids, + } + + # Filter out Nones – rather omit the field altogether + room_entry = {k: v for k, v in entry.items() if v is not None} + + return room_entry diff --git a/synapse/rest/client/v1/room.py b/synapse/rest/client/v1/room.py index 68e18ef9a072..fd49817f961e 100644 --- a/synapse/rest/client/v1/room.py +++ b/synapse/rest/client/v1/room.py @@ -27,6 +27,7 @@ InvalidClientCredentialsError, ShadowBanError, SynapseError, + MissingClientTokenError, ) from synapse.api.filtering import Filter from synapse.appservice import ApplicationService @@ -1428,8 +1429,48 @@ async def on_POST( ) +class RoomSummaryRestServlet(ResolveRoomIdMixin, RestServlet): + PATTERNS = ( + re.compile( + "^/_matrix/client/unstable/im.nheko.summary" + "/rooms/(?P[^/]*)/summary$" + ), + ) + + def __init__(self, hs: "HomeServer"): + super().__init__(hs) + self._auth = hs.get_auth() + self._room_summary_handler = hs.get_room_summary_handler() + + async def on_GET( + self, request: SynapseRequest, room_identifier: str + ) -> Tuple[int, JsonDict]: + try: + requester = await self._auth.get_user_by_req(request, allow_guest=True) + requester_user_id = requester.user.to_string() + except MissingClientTokenError: + requester_user_id = None + + # twisted.web.server.Request.args is incorrectly defined as Optional[Any] + args: Dict[bytes, List[bytes]] = request.args # type: ignore + remote_room_hosts = parse_strings_from_args( + args, "via", required=False + ) + room_id, remote_room_hosts = await self.resolve_room_id( + room_identifier, + remote_room_hosts, + ) + + return 200, await self._room_summary_handler.get_room_summary( + requester_user_id, + room_id, + remote_room_hosts, + ) + + def register_servlets(hs: "HomeServer", http_server, is_worker=False): msc2716_enabled = hs.config.experimental.msc2716_enabled + mxc3266_enabled = hs.config.experimental.mxc3266_enabled RoomStateEventRestServlet(hs).register(http_server) RoomMemberListRestServlet(hs).register(http_server) @@ -1446,6 +1487,8 @@ def register_servlets(hs: "HomeServer", http_server, is_worker=False): RoomTypingRestServlet(hs).register(http_server) RoomEventContextServlet(hs).register(http_server) RoomSpaceSummaryRestServlet(hs).register(http_server) + if mxc3266_enabled: + RoomSummaryRestServlet(hs).register(http_server) RoomEventServlet(hs).register(http_server) JoinedRoomsRestServlet(hs).register(http_server) RoomAliasListServlet(hs).register(http_server) diff --git a/synapse/server.py b/synapse/server.py index 2c27d2a7e8bb..8c72f810ee05 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -101,6 +101,7 @@ from synapse.handlers.room_list import RoomListHandler from synapse.handlers.room_member import RoomMemberHandler, RoomMemberMasterHandler from synapse.handlers.room_member_worker import RoomMemberWorkerHandler +from synapse.handlers.room_summary import RoomSummaryHandler from synapse.handlers.search import SearchHandler from synapse.handlers.send_email import SendEmailHandler from synapse.handlers.set_password import SetPasswordHandler @@ -781,6 +782,10 @@ def get_account_data_handler(self) -> AccountDataHandler: def get_space_summary_handler(self) -> SpaceSummaryHandler: return SpaceSummaryHandler(self) + @cache_in_self + def get_room_summary_handler(self) -> RoomSummaryHandler: + return RoomSummaryHandler(self) + @cache_in_self def get_event_auth_handler(self) -> EventAuthHandler: return EventAuthHandler(self) From 85abffc7a7be9b499cc01b71bcd39e3f9ed23239 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 15 Jul 2021 08:45:09 +0100 Subject: [PATCH 03/35] Update allowed_spaces to allowed_room_ids to match Space Summary API Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- synapse/handlers/room_summary.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/synapse/handlers/room_summary.py b/synapse/handlers/room_summary.py index 7d010f7b187a..502689137455 100644 --- a/synapse/handlers/room_summary.py +++ b/synapse/handlers/room_summary.py @@ -78,6 +78,9 @@ async def get_room_summary( # TODO validate that the requester has permission to see this room # https://github.com/matrix-org/matrix-doc/pull/3266/files#diff-97aeb566f3ce4bd6ec3b98e71ecbca3d6e86c0407e6a82afbc57e86bf0316607R106-R108 + # Before returning to the client, remove the allowed_room_ids key. + room_summary.pop("allowed_room_ids", None) + return room_summary async def _summarize_local_room( @@ -275,8 +278,8 @@ async def _build_room_entry(self, room_id: str) -> JsonDict: "guest_can_join": stats["guest_access"] == "can_join", "creation_ts": create_event.origin_server_ts, "room_type": create_event.content.get(EventContentFields.ROOM_TYPE), - "allowed_spaces": allowed_rooms, # TODO unspecced for MSC3266 "is_encrypted": (EventTypes.RoomEncryption, "") in current_state_ids, + "allowed_room_ids": allowed_rooms, # this field is stripped from the cs response } # Filter out Nones – rather omit the field altogether From b0c8e02c3c503b31262b1f501fe22c93a305d35d Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 15 Jul 2021 08:53:05 +0100 Subject: [PATCH 04/35] Extract _is_room_accessible into synapse.api.auth.Auth Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- synapse/api/auth.py | 115 +++++++++++++++++++++++++++- synapse/handlers/room_summary.py | 121 +----------------------------- synapse/handlers/space_summary.py | 117 +---------------------------- 3 files changed, 119 insertions(+), 234 deletions(-) diff --git a/synapse/api/auth.py b/synapse/api/auth.py index 42476a18e504..b021c8bb5401 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -21,7 +21,7 @@ from synapse import event_auth from synapse.api.auth_blocking import AuthBlocking -from synapse.api.constants import EventTypes, HistoryVisibility, Membership +from synapse.api.constants import EventTypes, HistoryVisibility, Membership, JoinRules from synapse.api.errors import ( AuthError, Codes, @@ -68,6 +68,7 @@ def __init__(self, hs: "HomeServer"): ) # type: LruCache[str, Tuple[str, bool]] self._auth_blocking = AuthBlocking(self.hs) + self._event_auth_handler = hs.get_event_auth_handler() self._account_validity_enabled = ( hs.config.account_validity.account_validity_enabled @@ -617,3 +618,115 @@ async def check_user_in_room_or_world_readable( async def check_auth_blocking(self, *args, **kwargs) -> None: await self._auth_blocking.check_auth_blocking(*args, **kwargs) + + async def is_room_accessible( + self, room_id: str, requester: Optional[str], origin: Optional[str] + ) -> bool: + """ + Calculate whether the room should be shown to the requester. + + It should be included if: + + * The requester is joined or can join the room (per MSC3173). + * The origin server has any user that is joined or can join the room. + * The history visibility is set to world readable. + + Args: + room_id: The room ID to summarize. + requester: + The user requesting the summary, if it is a local request. None + if this is a federation request. + origin: + The server requesting the summary, if it is a federation request. + None if this is a local request. + + Returns: + True if the room should be visible to the requester. + """ + state_ids = await self.store.get_current_state_ids(room_id) + + # If there's no state for the room, it isn't known. + if not state_ids: + # The user might have a pending invite for the room. + if requester and await self.store.get_invite_for_local_user_in_room( + requester, room_id + ): + return True + + logger.info("room %s is unknown, omitting from summary", room_id) + return False + + room_version = await self.store.get_room_version(room_id) + + # Include the room if it has join rules of public or knock. + join_rules_event_id = state_ids.get((EventTypes.JoinRules, "")) + if join_rules_event_id: + join_rules_event = await self.store.get_event(join_rules_event_id) + join_rule = join_rules_event.content.get("join_rule") + if join_rule == JoinRules.PUBLIC or ( + room_version.msc2403_knocking and join_rule == JoinRules.KNOCK + ): + return True + + # Include the room if it is peekable. + hist_vis_event_id = state_ids.get((EventTypes.RoomHistoryVisibility, "")) + if hist_vis_event_id: + hist_vis_ev = await self.store.get_event(hist_vis_event_id) + hist_vis = hist_vis_ev.content.get("history_visibility") + if hist_vis == HistoryVisibility.WORLD_READABLE: + return True + + # Otherwise we need to check information specific to the user or server. + + # If we have an authenticated requesting user, check if they are a member + # of the room (or can join the room). + if requester: + member_event_id = state_ids.get((EventTypes.Member, requester), None) + + # If they're in the room they can see info on it. + if member_event_id: + member_event = await self.store.get_event(member_event_id) + if member_event.membership in (Membership.JOIN, Membership.INVITE): + return True + + # Otherwise, check if they should be allowed access via membership in a space. + if await self._event_auth_handler.has_restricted_join_rules( + state_ids, room_version + ): + allowed_rooms = ( + await self._event_auth_handler.get_rooms_that_allow_join(state_ids) + ) + if await self._event_auth_handler.is_user_in_rooms( + allowed_rooms, requester + ): + return True + + # If this is a request over federation, check if the host is in the room or + # has a user who could join the room. + elif origin: + if await self._event_auth_handler.check_host_in_room( + room_id, origin + ) or await self.store.is_host_invited(room_id, origin): + return True + + # Alternately, if the host has a user in any of the spaces specified + # for access, then the host can see this room (and should do filtering + # if the requester cannot see it). + if await self._event_auth_handler.has_restricted_join_rules( + state_ids, room_version + ): + allowed_rooms = ( + await self._event_auth_handler.get_rooms_that_allow_join(state_ids) + ) + for space_id in allowed_rooms: + if await self._event_auth_handler.check_host_in_room( + space_id, origin + ): + return True + + logger.info( + "room %s is unpeekable and requester %s is not a member / not allowed to join, omitting from summary", + room_id, + requester or origin, + ) + return False diff --git a/synapse/handlers/room_summary.py b/synapse/handlers/room_summary.py index 502689137455..45edfe46adbd 100644 --- a/synapse/handlers/room_summary.py +++ b/synapse/handlers/room_summary.py @@ -19,8 +19,6 @@ EventContentFields, EventTypes, HistoryVisibility, - JoinRules, - Membership, ) from synapse.types import JsonDict @@ -88,7 +86,7 @@ async def _summarize_local_room( requester: Optional[str], origin: Optional[str], room_id: str, - ) -> JsonDict: + ) -> Optional[JsonDict]: """ Generate a room entry and a list of event entries for a given room. @@ -104,7 +102,7 @@ async def _summarize_local_room( Returns: summary dict to return """ - if not await self._is_room_accessible(room_id, requester, origin): + if not await self._auth.is_room_accessible(room_id, requester, origin): return None return await self._build_room_entry(room_id) @@ -113,7 +111,7 @@ async def _summarize_remote_room( self, room_id: str, remote_room_hosts: List[str], - ) -> JsonDict: + ) -> Optional[JsonDict]: """ Request room entries and a list of event entries for a given room by querying a remote server. @@ -128,119 +126,6 @@ async def _summarize_remote_room( return None # TODO federation API - # TODO extract into mixin/helper method - async def _is_room_accessible( - self, room_id: str, requester: Optional[str], origin: Optional[str] - ) -> bool: - """ - Calculate whether the room should be shown to the requester. - - It should be included if: - - * The requester is joined or can join the room (per MSC3173). - * The origin server has any user that is joined or can join the room. - * The history visibility is set to world readable. - - Args: - room_id: The room ID to summarize. - requester: - The user requesting the summary, if it is a local request. None - if this is a federation request. - origin: - The server requesting the summary, if it is a federation request. - None if this is a local request. - - Returns: - True if the room should be visible to the requester. - """ - state_ids = await self._store.get_current_state_ids(room_id) - - # If there's no state for the room, it isn't known. - if not state_ids: - # The user might have a pending invite for the room. - if requester and await self._store.get_invite_for_local_user_in_room( - requester, room_id - ): - return True - - logger.info("room %s is unknown, omitting from summary", room_id) - return False - - room_version = await self._store.get_room_version(room_id) - - # Include the room if it has join rules of public or knock. - join_rules_event_id = state_ids.get((EventTypes.JoinRules, "")) - if join_rules_event_id: - join_rules_event = await self._store.get_event(join_rules_event_id) - join_rule = join_rules_event.content.get("join_rule") - if join_rule == JoinRules.PUBLIC or ( - room_version.msc2403_knocking and join_rule == JoinRules.KNOCK - ): - return True - - # Include the room if it is peekable. - hist_vis_event_id = state_ids.get((EventTypes.RoomHistoryVisibility, "")) - if hist_vis_event_id: - hist_vis_ev = await self._store.get_event(hist_vis_event_id) - hist_vis = hist_vis_ev.content.get("history_visibility") - if hist_vis == HistoryVisibility.WORLD_READABLE: - return True - - # Otherwise we need to check information specific to the user or server. - - # If we have an authenticated requesting user, check if they are a member - # of the room (or can join the room). - if requester: - member_event_id = state_ids.get((EventTypes.Member, requester), None) - - # If they're in the room they can see info on it. - if member_event_id: - member_event = await self._store.get_event(member_event_id) - if member_event.membership in (Membership.JOIN, Membership.INVITE): - return True - - # Otherwise, check if they should be allowed access via membership in a space. - if await self._event_auth_handler.has_restricted_join_rules( - state_ids, room_version - ): - allowed_rooms = ( - await self._event_auth_handler.get_rooms_that_allow_join(state_ids) - ) - if await self._event_auth_handler.is_user_in_rooms( - allowed_rooms, requester - ): - return True - - # If this is a request over federation, check if the host is in the room or - # has a user who could join the room. - elif origin: - if await self._event_auth_handler.check_host_in_room( - room_id, origin - ) or await self._store.is_host_invited(room_id, origin): - return True - - # Alternately, if the host has a user in any of the spaces specified - # for access, then the host can see this room (and should do filtering - # if the requester cannot see it). - if await self._event_auth_handler.has_restricted_join_rules( - state_ids, room_version - ): - allowed_rooms = ( - await self._event_auth_handler.get_rooms_that_allow_join(state_ids) - ) - for space_id in allowed_rooms: - if await self._event_auth_handler.check_host_in_room( - space_id, origin - ): - return True - - logger.info( - "room %s is unpeekable and requester %s is not a member / not allowed to join, omitting from summary", - room_id, - requester or origin, - ) - return False - async def _build_room_entry(self, room_id: str) -> JsonDict: """Generate en entry suitable for the 'rooms' list in the summary response""" stats = await self._store.get_room_with_stats(room_id) diff --git a/synapse/handlers/space_summary.py b/synapse/handlers/space_summary.py index 366e6211e56d..9eee78dc87b6 100644 --- a/synapse/handlers/space_summary.py +++ b/synapse/handlers/space_summary.py @@ -25,7 +25,6 @@ EventTypes, HistoryVisibility, JoinRules, - Membership, RoomTypes, ) from synapse.events import EventBase @@ -182,7 +181,7 @@ async def get_space_summary( # 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( + include_room = await self._auth.is_room_accessible( fed_room_id, requester, None ) @@ -333,7 +332,7 @@ async def _summarize_local_room( An iterable of the sorted children events. This may be limited to a maximum size or may include all children. """ - if not await self._is_room_accessible(room_id, requester, origin): + if not await self._auth.is_room_accessible(room_id, requester, origin): return None, () room_entry = await self._build_room_entry(room_id) @@ -420,118 +419,6 @@ async def _summarize_remote_room( ev.data for ev in res.events if ev.event_type == EventTypes.SpaceChild ) - async def _is_room_accessible( - self, room_id: str, requester: Optional[str], origin: Optional[str] - ) -> bool: - """ - Calculate whether the room should be shown in the spaces summary. - - It should be included if: - - * The requester is joined or can join the room (per MSC3173). - * The origin server has any user that is joined or can join the room. - * The history visibility is set to world readable. - - Args: - room_id: The room ID to summarize. - requester: - The user requesting the summary, if it is a local request. None - if this is a federation request. - origin: - The server requesting the summary, if it is a federation request. - None if this is a local request. - - Returns: - True if the room should be included in the spaces summary. - """ - state_ids = await self._store.get_current_state_ids(room_id) - - # If there's no state for the room, it isn't known. - if not state_ids: - # The user might have a pending invite for the room. - if requester and await self._store.get_invite_for_local_user_in_room( - requester, room_id - ): - return True - - logger.info("room %s is unknown, omitting from summary", room_id) - return False - - room_version = await self._store.get_room_version(room_id) - - # Include the room if it has join rules of public or knock. - join_rules_event_id = state_ids.get((EventTypes.JoinRules, "")) - if join_rules_event_id: - join_rules_event = await self._store.get_event(join_rules_event_id) - join_rule = join_rules_event.content.get("join_rule") - if join_rule == JoinRules.PUBLIC or ( - room_version.msc2403_knocking and join_rule == JoinRules.KNOCK - ): - return True - - # Include the room if it is peekable. - hist_vis_event_id = state_ids.get((EventTypes.RoomHistoryVisibility, "")) - if hist_vis_event_id: - hist_vis_ev = await self._store.get_event(hist_vis_event_id) - hist_vis = hist_vis_ev.content.get("history_visibility") - if hist_vis == HistoryVisibility.WORLD_READABLE: - return True - - # Otherwise we need to check information specific to the user or server. - - # If we have an authenticated requesting user, check if they are a member - # of the room (or can join the room). - if requester: - member_event_id = state_ids.get((EventTypes.Member, requester), None) - - # If they're in the room they can see info on it. - if member_event_id: - member_event = await self._store.get_event(member_event_id) - if member_event.membership in (Membership.JOIN, Membership.INVITE): - return True - - # Otherwise, check if they should be allowed access via membership in a space. - if await self._event_auth_handler.has_restricted_join_rules( - state_ids, room_version - ): - allowed_rooms = ( - await self._event_auth_handler.get_rooms_that_allow_join(state_ids) - ) - if await self._event_auth_handler.is_user_in_rooms( - allowed_rooms, requester - ): - return True - - # If this is a request over federation, check if the host is in the room or - # has a user who could join the room. - elif origin: - if await self._event_auth_handler.check_host_in_room( - room_id, origin - ) or await self._store.is_host_invited(room_id, origin): - return True - - # Alternately, if the host has a user in any of the spaces specified - # for access, then the host can see this room (and should do filtering - # if the requester cannot see it). - if await self._event_auth_handler.has_restricted_join_rules( - state_ids, room_version - ): - allowed_rooms = ( - await self._event_auth_handler.get_rooms_that_allow_join(state_ids) - ) - for space_id in allowed_rooms: - if await self._event_auth_handler.check_host_in_room( - space_id, origin - ): - return True - - logger.info( - "room %s is unpeekable and requester %s is not a member / not allowed to join, omitting from summary", - room_id, - requester or origin, - ) - return False - async def _build_room_entry(self, room_id: str) -> JsonDict: """Generate en entry suitable for the 'rooms' list in the summary response""" stats = await self._store.get_room_with_stats(room_id) From d169cd82e8ef7d18f61719327a21d3680f25e0a2 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 15 Jul 2021 09:00:37 +0100 Subject: [PATCH 05/35] Extract build_room_entry into a mixin, updates Space Summary API allowed_spaces->allowed_room_ids Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- synapse/handlers/room_summary.py | 105 ++++++++++++++------------- synapse/handlers/space_summary.py | 63 ++-------------- tests/handlers/test_space_summary.py | 4 +- 3 files changed, 65 insertions(+), 107 deletions(-) diff --git a/synapse/handlers/room_summary.py b/synapse/handlers/room_summary.py index 45edfe46adbd..dcc3ef25213e 100644 --- a/synapse/handlers/room_summary.py +++ b/synapse/handlers/room_summary.py @@ -28,12 +28,63 @@ logger = logging.getLogger(__name__) -class RoomSummaryHandler: +class RoomSummaryMixin: def __init__(self, hs: "HomeServer"): + self._store = hs.get_datastore() + self._event_auth_handler = hs.get_event_auth_handler() + + async def build_room_entry(self, room_id: str) -> JsonDict: + """Generate en entry suitable for the 'rooms' list in the summary response""" + stats = await self._store.get_room_with_stats(room_id) + + # currently this should be impossible because we call + # check_user_in_room_or_world_readable on the room before we get here, so + # there should always be an entry + assert stats is not None, "unable to retrieve stats for %s" % (room_id,) + + current_state_ids = await self._store.get_current_state_ids(room_id) + create_event = await self._store.get_event( + current_state_ids[(EventTypes.Create, "")] + ) + + room_version = await self._store.get_room_version(room_id) + allowed_rooms = None + if await self._event_auth_handler.has_restricted_join_rules( + current_state_ids, room_version + ): + allowed_rooms = await self._event_auth_handler.get_rooms_that_allow_join( + current_state_ids + ) + + entry = { + "room_id": stats["room_id"], + "name": stats["name"], + "topic": stats["topic"], + "canonical_alias": stats["canonical_alias"], + "num_joined_members": stats["joined_members"], + "avatar_url": stats["avatar"], + "join_rules": stats["join_rules"], + "world_readable": ( + stats["history_visibility"] == HistoryVisibility.WORLD_READABLE + ), + "guest_can_join": stats["guest_access"] == "can_join", + "creation_ts": create_event.origin_server_ts, + "room_type": create_event.content.get(EventContentFields.ROOM_TYPE), + "is_encrypted": (EventTypes.RoomEncryption, "") in current_state_ids, + "allowed_room_ids": allowed_rooms, # this field is stripped from the cs response + } + + # Filter out Nones – rather omit the field altogether + room_entry = {k: v for k, v in entry.items() if v is not None} + + return room_entry + + +class RoomSummaryHandler(RoomSummaryMixin): + def __init__(self, hs: "HomeServer"): + super().__init__(hs) self._clock = hs.get_clock() self._auth = hs.get_auth() - self._event_auth_handler = hs.get_event_auth_handler() - self._store = hs.get_datastore() self._event_serializer = hs.get_event_client_serializer() self._server_name = hs.hostname self._federation_client = hs.get_federation_client() @@ -105,7 +156,7 @@ async def _summarize_local_room( if not await self._auth.is_room_accessible(room_id, requester, origin): return None - return await self._build_room_entry(room_id) + return await self.build_room_entry(room_id) async def _summarize_remote_room( self, @@ -125,49 +176,3 @@ async def _summarize_remote_room( logger.info("Requesting summary for %s via %s", room_id, remote_room_hosts) return None # TODO federation API - - async def _build_room_entry(self, room_id: str) -> JsonDict: - """Generate en entry suitable for the 'rooms' list in the summary response""" - stats = await self._store.get_room_with_stats(room_id) - - # currently this should be impossible because we call - # check_user_in_room_or_world_readable on the room before we get here, so - # there should always be an entry - assert stats is not None, "unable to retrieve stats for %s" % (room_id,) - - current_state_ids = await self._store.get_current_state_ids(room_id) - create_event = await self._store.get_event( - current_state_ids[(EventTypes.Create, "")] - ) - - room_version = await self._store.get_room_version(room_id) - allowed_rooms = None - if await self._event_auth_handler.has_restricted_join_rules( - current_state_ids, room_version - ): - allowed_rooms = await self._event_auth_handler.get_rooms_that_allow_join( - current_state_ids - ) - - entry = { - "room_id": stats["room_id"], - "name": stats["name"], - "topic": stats["topic"], - "canonical_alias": stats["canonical_alias"], - "num_joined_members": stats["joined_members"], - "avatar_url": stats["avatar"], - "join_rules": stats["join_rules"], - "world_readable": ( - stats["history_visibility"] == HistoryVisibility.WORLD_READABLE - ), - "guest_can_join": stats["guest_access"] == "can_join", - "creation_ts": create_event.origin_server_ts, - "room_type": create_event.content.get(EventContentFields.ROOM_TYPE), - "is_encrypted": (EventTypes.RoomEncryption, "") in current_state_ids, - "allowed_room_ids": allowed_rooms, # this field is stripped from the cs response - } - - # Filter out Nones – rather omit the field altogether - room_entry = {k: v for k, v in entry.items() if v is not None} - - return room_entry diff --git a/synapse/handlers/space_summary.py b/synapse/handlers/space_summary.py index 9eee78dc87b6..25af803b3f1c 100644 --- a/synapse/handlers/space_summary.py +++ b/synapse/handlers/space_summary.py @@ -21,14 +21,13 @@ import attr from synapse.api.constants import ( - EventContentFields, EventTypes, - HistoryVisibility, JoinRules, RoomTypes, ) from synapse.events import EventBase from synapse.events.utils import format_event_for_client_v2 +from synapse.handlers.room_summary import RoomSummaryMixin from synapse.types import JsonDict if TYPE_CHECKING: @@ -46,12 +45,11 @@ MAX_SERVERS_PER_SPACE = 3 -class SpaceSummaryHandler: +class SpaceSummaryHandler(RoomSummaryMixin): def __init__(self, hs: "HomeServer"): + super().__init__(hs) self._clock = hs.get_clock() self._auth = hs.get_auth() - self._event_auth_handler = hs.get_event_auth_handler() - self._store = hs.get_datastore() self._event_serializer = hs.get_event_client_serializer() self._server_name = hs.hostname self._federation_client = hs.get_federation_client() @@ -166,9 +164,9 @@ async def get_space_summary( or room.get("world_readable") is True ) - # Check if the user is a member of any of the allowed spaces + # Check if the user is a member of any of the allowed rooms # from the response. - allowed_rooms = room.get("allowed_spaces") + allowed_rooms = room.get("allowed_room_ids") if ( not include_room and allowed_rooms @@ -229,10 +227,10 @@ async def get_space_summary( ) processed_events.add(ev_key) - # Before returning to the client, remove the allowed_spaces key for any + # Before returning to the client, remove the allowed_room_ids key for any # rooms. for room in rooms_result: - room.pop("allowed_spaces", None) + room.pop("allowed_room_ids", None) return {"rooms": rooms_result, "events": events_result} @@ -335,7 +333,7 @@ async def _summarize_local_room( if not await self._auth.is_room_accessible(room_id, requester, origin): return None, () - room_entry = await self._build_room_entry(room_id) + room_entry = await self.build_room_entry(room_id) # If the room is not a space, return just the room information. if room_entry.get("room_type") != RoomTypes.SPACE: @@ -419,51 +417,6 @@ async def _summarize_remote_room( ev.data for ev in res.events if ev.event_type == EventTypes.SpaceChild ) - async def _build_room_entry(self, room_id: str) -> JsonDict: - """Generate en entry suitable for the 'rooms' list in the summary response""" - stats = await self._store.get_room_with_stats(room_id) - - # currently this should be impossible because we call - # check_user_in_room_or_world_readable on the room before we get here, so - # there should always be an entry - assert stats is not None, "unable to retrieve stats for %s" % (room_id,) - - current_state_ids = await self._store.get_current_state_ids(room_id) - create_event = await self._store.get_event( - current_state_ids[(EventTypes.Create, "")] - ) - - room_version = await self._store.get_room_version(room_id) - allowed_rooms = None - if await self._event_auth_handler.has_restricted_join_rules( - current_state_ids, room_version - ): - allowed_rooms = await self._event_auth_handler.get_rooms_that_allow_join( - current_state_ids - ) - - entry = { - "room_id": stats["room_id"], - "name": stats["name"], - "topic": stats["topic"], - "canonical_alias": stats["canonical_alias"], - "num_joined_members": stats["joined_members"], - "avatar_url": stats["avatar"], - "join_rules": stats["join_rules"], - "world_readable": ( - stats["history_visibility"] == HistoryVisibility.WORLD_READABLE - ), - "guest_can_join": stats["guest_access"] == "can_join", - "creation_ts": create_event.origin_server_ts, - "room_type": create_event.content.get(EventContentFields.ROOM_TYPE), - "allowed_spaces": allowed_rooms, - } - - # Filter out Nones – rather omit the field altogether - room_entry = {k: v for k, v in entry.items() if v is not None} - - return room_entry - async def _get_child_events(self, room_id: str) -> Iterable[EventBase]: """ Get the child events for a given room. diff --git a/tests/handlers/test_space_summary.py b/tests/handlers/test_space_summary.py index 3f73ad7f9478..6c80664b0d69 100644 --- a/tests/handlers/test_space_summary.py +++ b/tests/handlers/test_space_summary.py @@ -460,13 +460,13 @@ async def summarize_remote_room( "room_id": restricted_room, "world_readable": False, "join_rules": JoinRules.MSC3083_RESTRICTED, - "allowed_spaces": [], + "allowed_room_ids": [], }, { "room_id": restricted_accessible_room, "world_readable": False, "join_rules": JoinRules.MSC3083_RESTRICTED, - "allowed_spaces": [self.room], + "allowed_room_ids": [self.room], }, { "room_id": world_readable_room, From 01b222aee1a498b26f6969b9bfca439fc91cbf7f Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 15 Jul 2021 09:01:01 +0100 Subject: [PATCH 06/35] remove unused variables Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- synapse/handlers/room_summary.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/synapse/handlers/room_summary.py b/synapse/handlers/room_summary.py index dcc3ef25213e..67fa878a755a 100644 --- a/synapse/handlers/room_summary.py +++ b/synapse/handlers/room_summary.py @@ -85,9 +85,7 @@ def __init__(self, hs: "HomeServer"): super().__init__(hs) self._clock = hs.get_clock() self._auth = hs.get_auth() - self._event_serializer = hs.get_event_client_serializer() self._server_name = hs.hostname - self._federation_client = hs.get_federation_client() async def get_room_summary( self, From 9227efc8500ed7a97ca5a6009cb6db9e79045809 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 15 Jul 2021 09:29:52 +0100 Subject: [PATCH 07/35] delint Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- synapse/api/auth.py | 4 ++-- synapse/handlers/room_summary.py | 11 +++++------ synapse/handlers/space_summary.py | 6 +----- synapse/http/servlet.py | 8 ++++---- synapse/rest/admin/rooms.py | 2 +- synapse/rest/client/v1/room.py | 14 ++++---------- 6 files changed, 17 insertions(+), 28 deletions(-) diff --git a/synapse/api/auth.py b/synapse/api/auth.py index b021c8bb5401..c14c373be113 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -21,7 +21,7 @@ from synapse import event_auth from synapse.api.auth_blocking import AuthBlocking -from synapse.api.constants import EventTypes, HistoryVisibility, Membership, JoinRules +from synapse.api.constants import EventTypes, HistoryVisibility, JoinRules, Membership from synapse.api.errors import ( AuthError, Codes, @@ -728,5 +728,5 @@ async def is_room_accessible( "room %s is unpeekable and requester %s is not a member / not allowed to join, omitting from summary", room_id, requester or origin, - ) + ) return False diff --git a/synapse/handlers/room_summary.py b/synapse/handlers/room_summary.py index 67fa878a755a..62467ee7623c 100644 --- a/synapse/handlers/room_summary.py +++ b/synapse/handlers/room_summary.py @@ -15,11 +15,7 @@ import logging from typing import TYPE_CHECKING, List, Optional -from synapse.api.constants import ( - EventContentFields, - EventTypes, - HistoryVisibility, -) +from synapse.api.constants import EventContentFields, EventTypes, HistoryVisibility from synapse.types import JsonDict if TYPE_CHECKING: @@ -114,7 +110,10 @@ async def get_room_summary( room_summary = await self._summarize_local_room(requester, None, room_id) if requester: - membership, _ = await self._store.get_local_current_membership_for_user_in_room( + ( + membership, + _, + ) = await self._store.get_local_current_membership_for_user_in_room( requester, room_id ) diff --git a/synapse/handlers/space_summary.py b/synapse/handlers/space_summary.py index 25af803b3f1c..7eed9780b8b9 100644 --- a/synapse/handlers/space_summary.py +++ b/synapse/handlers/space_summary.py @@ -20,11 +20,7 @@ import attr -from synapse.api.constants import ( - EventTypes, - JoinRules, - RoomTypes, -) +from synapse.api.constants import EventTypes, JoinRules, RoomTypes from synapse.events import EventBase from synapse.events.utils import format_event_for_client_v2 from synapse.handlers.room_summary import RoomSummaryMixin diff --git a/synapse/http/servlet.py b/synapse/http/servlet.py index 88e8cdbfbc6c..ab9fa11a2458 100644 --- a/synapse/http/servlet.py +++ b/synapse/http/servlet.py @@ -14,15 +14,15 @@ """ This module contains base REST classes for constructing REST servlets. """ import logging -from typing import TYPE_CHECKING, Dict, Iterable, List, Optional, overload, Tuple +from typing import TYPE_CHECKING, Dict, Iterable, List, Optional, Tuple, overload from typing_extensions import Literal from twisted.web.server import Request from synapse.api.errors import Codes, SynapseError -from synapse.util import json_decoder from synapse.types import RoomAlias, RoomID +from synapse.util import json_decoder if TYPE_CHECKING: from synapse.server import HomeServer @@ -32,7 +32,7 @@ def parse_integer(request, name, default=None, required=False): """Parse an integer parameter from the request string -thank + Args: request: the twisted HTTP request. name (bytes/unicode): the name of the query parameter. @@ -554,4 +554,4 @@ async def resolve_room_id( raise SynapseError( 400, "Unknown room ID or room alias %s" % room_identifier ) - return resolved_room_id, remote_room_hosts \ No newline at end of file + return resolved_room_id, remote_room_hosts diff --git a/synapse/rest/admin/rooms.py b/synapse/rest/admin/rooms.py index aa1591a3fc83..cf9512df44d8 100644 --- a/synapse/rest/admin/rooms.py +++ b/synapse/rest/admin/rooms.py @@ -20,12 +20,12 @@ from synapse.api.errors import AuthError, Codes, NotFoundError, SynapseError from synapse.api.filtering import Filter from synapse.http.servlet import ( + ResolveRoomIdMixin, RestServlet, assert_params_in_dict, parse_integer, parse_json_object_from_request, parse_string, - ResolveRoomIdMixin, ) from synapse.http.site import SynapseRequest from synapse.rest.admin._base import ( diff --git a/synapse/rest/client/v1/room.py b/synapse/rest/client/v1/room.py index fd49817f961e..fe5aca37207b 100644 --- a/synapse/rest/client/v1/room.py +++ b/synapse/rest/client/v1/room.py @@ -25,14 +25,15 @@ Codes, HttpResponseException, InvalidClientCredentialsError, + MissingClientTokenError, ShadowBanError, SynapseError, - MissingClientTokenError, ) from synapse.api.filtering import Filter from synapse.appservice import ApplicationService from synapse.events.utils import format_event_for_client_v2 from synapse.http.servlet import ( + ResolveRoomIdMixin, RestServlet, assert_params_in_dict, parse_boolean, @@ -40,7 +41,6 @@ parse_json_object_from_request, parse_string, parse_strings_from_args, - ResolveRoomIdMixin, ) from synapse.http.site import SynapseRequest from synapse.logging.opentracing import set_tag @@ -51,8 +51,6 @@ from synapse.types import ( JsonDict, Requester, - RoomAlias, - RoomID, StreamToken, ThirdPartyInstanceID, UserID, @@ -680,9 +678,7 @@ async def on_POST( # twisted.web.server.Request.args is incorrectly defined as Optional[Any] args: Dict[bytes, List[bytes]] = request.args # type: ignore - remote_room_hosts = parse_strings_from_args( - args, "server_name", required=False - ) + remote_room_hosts = parse_strings_from_args(args, "server_name", required=False) room_id, remote_room_hosts = await self.resolve_room_id( room_identifier, remote_room_hosts, @@ -1453,9 +1449,7 @@ async def on_GET( # twisted.web.server.Request.args is incorrectly defined as Optional[Any] args: Dict[bytes, List[bytes]] = request.args # type: ignore - remote_room_hosts = parse_strings_from_args( - args, "via", required=False - ) + remote_room_hosts = parse_strings_from_args(args, "via", required=False) room_id, remote_room_hosts = await self.resolve_room_id( room_identifier, remote_room_hosts, From c02485c159a84220a2d71bd1a81eadbf55dbd873 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 15 Jul 2021 09:46:31 +0100 Subject: [PATCH 08/35] fix types Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- synapse/handlers/room_summary.py | 14 ++++++++------ synapse/rest/client/v1/room.py | 2 +- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/synapse/handlers/room_summary.py b/synapse/handlers/room_summary.py index 62467ee7623c..4b96ba4dc9ee 100644 --- a/synapse/handlers/room_summary.py +++ b/synapse/handlers/room_summary.py @@ -16,6 +16,7 @@ from typing import TYPE_CHECKING, List, Optional from synapse.api.constants import EventContentFields, EventTypes, HistoryVisibility +from synapse.api.errors import NotFoundError from synapse.types import JsonDict if TYPE_CHECKING: @@ -87,7 +88,7 @@ async def get_room_summary( self, requester: Optional[str], room_id: str, - remote_room_hosts: List[str], + remote_room_hosts: Optional[List[str]], ) -> JsonDict: """ Implementation of the room summary C-S API MSC3244 @@ -134,7 +135,7 @@ async def _summarize_local_room( requester: Optional[str], origin: Optional[str], room_id: str, - ) -> Optional[JsonDict]: + ) -> JsonDict: """ Generate a room entry and a list of event entries for a given room. @@ -151,15 +152,15 @@ async def _summarize_local_room( summary dict to return """ if not await self._auth.is_room_accessible(room_id, requester, origin): - return None + raise NotFoundError("Room not found or is not accessible") return await self.build_room_entry(room_id) async def _summarize_remote_room( self, room_id: str, - remote_room_hosts: List[str], - ) -> Optional[JsonDict]: + remote_room_hosts: Optional[List[str]], + ) -> JsonDict: """ Request room entries and a list of event entries for a given room by querying a remote server. @@ -172,4 +173,5 @@ async def _summarize_remote_room( """ logger.info("Requesting summary for %s via %s", room_id, remote_room_hosts) - return None # TODO federation API + # TODO federation API + raise NotFoundError("Room not found or is not accessible") diff --git a/synapse/rest/client/v1/room.py b/synapse/rest/client/v1/room.py index fe5aca37207b..30d49860fee9 100644 --- a/synapse/rest/client/v1/room.py +++ b/synapse/rest/client/v1/room.py @@ -1443,7 +1443,7 @@ async def on_GET( ) -> Tuple[int, JsonDict]: try: requester = await self._auth.get_user_by_req(request, allow_guest=True) - requester_user_id = requester.user.to_string() + requester_user_id: Optional[str] = requester.user.to_string() except MissingClientTokenError: requester_user_id = None From 0fa04fe6cd9cd738de378dc0ce28ef114bf8eca4 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 15 Jul 2021 15:34:53 +0100 Subject: [PATCH 09/35] Fix typo in the msc3266 experimental config flag Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- synapse/config/experimental.py | 2 +- synapse/rest/client/v1/room.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index 299550b72513..5250ffe45566 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -34,4 +34,4 @@ def read_config(self, config: JsonDict, **kwargs): self.msc2716_enabled = experimental.get("msc2716_enabled", False) # type: bool # MSC3266 (room summary api) - self.mxc3266_enabled = experimental.get("mxc3266_enabled", False) # type: bool + self.msc3266_enabled = experimental.get("msc3266_enabled", False) # type: bool diff --git a/synapse/rest/client/v1/room.py b/synapse/rest/client/v1/room.py index 30d49860fee9..6352f0d9746d 100644 --- a/synapse/rest/client/v1/room.py +++ b/synapse/rest/client/v1/room.py @@ -1464,7 +1464,7 @@ async def on_GET( def register_servlets(hs: "HomeServer", http_server, is_worker=False): msc2716_enabled = hs.config.experimental.msc2716_enabled - mxc3266_enabled = hs.config.experimental.mxc3266_enabled + msc3266_enabled = hs.config.experimental.msc3266_enabled RoomStateEventRestServlet(hs).register(http_server) RoomMemberListRestServlet(hs).register(http_server) @@ -1481,7 +1481,7 @@ def register_servlets(hs: "HomeServer", http_server, is_worker=False): RoomTypingRestServlet(hs).register(http_server) RoomEventContextServlet(hs).register(http_server) RoomSpaceSummaryRestServlet(hs).register(http_server) - if mxc3266_enabled: + if msc3266_enabled: RoomSummaryRestServlet(hs).register(http_server) RoomEventServlet(hs).register(http_server) JoinedRoomsRestServlet(hs).register(http_server) From 9994f1233699a7527ecbe34a42a972ba6cdcecbf Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 5 Aug 2021 09:16:04 +0100 Subject: [PATCH 10/35] extract requester_can_see_room_entry to reuse in both space and room summary federation Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- synapse/handlers/room_summary.py | 52 ++++++++++++++++++++++++++++--- synapse/handlers/space_summary.py | 35 ++------------------- 2 files changed, 50 insertions(+), 37 deletions(-) diff --git a/synapse/handlers/room_summary.py b/synapse/handlers/room_summary.py index 4b96ba4dc9ee..a7012606e9a1 100644 --- a/synapse/handlers/room_summary.py +++ b/synapse/handlers/room_summary.py @@ -15,7 +15,12 @@ import logging from typing import TYPE_CHECKING, List, Optional -from synapse.api.constants import EventContentFields, EventTypes, HistoryVisibility +from synapse.api.constants import ( + EventContentFields, + EventTypes, + HistoryVisibility, + JoinRules, +) from synapse.api.errors import NotFoundError from synapse.types import JsonDict @@ -76,6 +81,40 @@ async def build_room_entry(self, room_id: str) -> JsonDict: return room_entry + async def requester_can_see_room_entry( + self, + room: JsonDict, + requester: str, + ) -> bool: + # The room should only be allowed 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 rooms + # from the response. + allowed_rooms = room.get("allowed_room_ids") + 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 + ) + + return include_room + class RoomSummaryHandler(RoomSummaryMixin): def __init__(self, hs: "HomeServer"): @@ -91,7 +130,7 @@ async def get_room_summary( remote_room_hosts: Optional[List[str]], ) -> JsonDict: """ - Implementation of the room summary C-S API MSC3244 + Implementation of the room summary C-S API MSC3266 Args: requester: user id of the user making this request, @@ -122,8 +161,11 @@ async def get_room_summary( else: room_summary = await self._summarize_remote_room(room_id, remote_room_hosts) - # TODO validate that the requester has permission to see this room - # https://github.com/matrix-org/matrix-doc/pull/3266/files#diff-97aeb566f3ce4bd6ec3b98e71ecbca3d6e86c0407e6a82afbc57e86bf0316607R106-R108 + # validate that the requester has permission to see this room + include_room = self.requester_can_see_room_entry(room_summary, requester) + + if not include_room: + raise NotFoundError("Room not found or is not accessible") # Before returning to the client, remove the allowed_room_ids key. room_summary.pop("allowed_room_ids", None) @@ -173,5 +215,5 @@ async def _summarize_remote_room( """ logger.info("Requesting summary for %s via %s", room_id, remote_room_hosts) - # TODO federation API + # TODO federation API, descoped from initial unstable implementation as MSC needs more maturing on that side. raise NotFoundError("Room not found or is not accessible") diff --git a/synapse/handlers/space_summary.py b/synapse/handlers/space_summary.py index e3d597aa7953..6fcbdc415e15 100644 --- a/synapse/handlers/space_summary.py +++ b/synapse/handlers/space_summary.py @@ -20,7 +20,7 @@ import attr -from synapse.api.constants import EventTypes, JoinRules, RoomTypes +from synapse.api.constants import EventTypes, RoomTypes from synapse.events import EventBase from synapse.events.utils import format_event_for_client_v2 from synapse.handlers.room_summary import RoomSummaryMixin @@ -141,42 +141,13 @@ async def get_space_summary( if not fed_room_id or not isinstance(fed_room_id, str): continue - # 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 rooms - # from the response. - allowed_rooms = room.get("allowed_room_ids") - 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 - ) + include_room = self.requester_can_see_room_entry(room, 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._auth.is_room_accessible( - fed_room_id, requester, None + room_id, requester, None ) # The user can see the room, include it! From b2c8a7c86e185d17eeb62b2b08c277db4a32c426 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 5 Aug 2021 09:16:12 +0100 Subject: [PATCH 11/35] Add newsfragment Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- changelog.d/10394.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/10394.feature diff --git a/changelog.d/10394.feature b/changelog.d/10394.feature new file mode 100644 index 000000000000..b1712c3405a8 --- /dev/null +++ b/changelog.d/10394.feature @@ -0,0 +1 @@ +Initial support for MSC3266, Room Summary over the unstable /rooms/{roomIdOrAlias}/summary API. From e6e7ecf3df40c6dda9f088eb20b1fc045783e30b Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 5 Aug 2021 11:22:11 +0100 Subject: [PATCH 12/35] Add tests for the room_summary handler and make remote_room_hosts optional Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- synapse/handlers/room_summary.py | 2 +- tests/handlers/test_room_summary.py | 98 +++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 1 deletion(-) create mode 100644 tests/handlers/test_room_summary.py diff --git a/synapse/handlers/room_summary.py b/synapse/handlers/room_summary.py index a7012606e9a1..691f55ba205d 100644 --- a/synapse/handlers/room_summary.py +++ b/synapse/handlers/room_summary.py @@ -127,7 +127,7 @@ async def get_room_summary( self, requester: Optional[str], room_id: str, - remote_room_hosts: Optional[List[str]], + remote_room_hosts: Optional[List[str]] = None, ) -> JsonDict: """ Implementation of the room summary C-S API MSC3266 diff --git a/tests/handlers/test_room_summary.py b/tests/handlers/test_room_summary.py new file mode 100644 index 000000000000..ee0569892d23 --- /dev/null +++ b/tests/handlers/test_room_summary.py @@ -0,0 +1,98 @@ +# Copyright 2021 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. +from synapse.api.constants import EventTypes, HistoryVisibility, JoinRules +from synapse.api.errors import NotFoundError +from synapse.rest import admin +from synapse.rest.client.v1 import login, room +from synapse.server import HomeServer + +from tests import unittest + + +class SpaceSummaryTestCase(unittest.HomeserverTestCase): + servlets = [ + admin.register_servlets_for_client_rest_resource, + room.register_servlets, + login.register_servlets, + ] + + def prepare(self, reactor, clock, hs: HomeServer): + self.hs = hs + self.handler = self.hs.get_room_summary_handler() + + # Create a user. + self.user = self.register_user("user", "pass") + self.token = self.login("user", "pass") + + # Create a simple room. + self.room = self.helper.create_room_as(self.user, tok=self.token) + self.helper.send_state( + self.room, + event_type=EventTypes.JoinRules, + body={"join_rule": JoinRules.INVITE}, + tok=self.token, + ) + + def test_own_room(self): + """Test a simple room created by the requester.""" + result = self.get_success(self.handler.get_room_summary(self.user, self.room)) + self.assertEqual(result.get("room_id"), self.room) + + def test_visibility(self): + """A user not in a private room cannot get its summary.""" + user2 = self.register_user("user2", "pass") + token2 = self.login("user2", "pass") + + # The user cannot see the room. + self.get_failure(self.handler.get_room_summary(user2, self.room), NotFoundError) + + # If the room is made world-readable it should return a result. + self.helper.send_state( + self.room, + event_type=EventTypes.RoomHistoryVisibility, + body={"history_visibility": HistoryVisibility.WORLD_READABLE}, + tok=self.token, + ) + result = self.get_success(self.handler.get_room_summary(user2, self.room)) + self.assertEqual(result.get("room_id"), self.room) + + # Make it not world-readable again and confirm it results in an error. + self.helper.send_state( + self.room, + event_type=EventTypes.RoomHistoryVisibility, + body={"history_visibility": HistoryVisibility.JOINED}, + tok=self.token, + ) + self.get_failure(self.handler.get_room_summary(user2, self.room), NotFoundError) + + # If the room is made public it should return a result. + self.helper.send_state( + self.room, + event_type=EventTypes.JoinRules, + body={"join_rule": JoinRules.PUBLIC}, + tok=self.token, + ) + result = self.get_success(self.handler.get_room_summary(user2, self.room)) + self.assertEqual(result.get("room_id"), self.room) + + # Join the space, make it invite-only again and results should be returned. + self.helper.join(self.room, user2, tok=token2) + self.helper.send_state( + self.room, + event_type=EventTypes.JoinRules, + body={"join_rule": JoinRules.INVITE}, + tok=self.token, + ) + result = self.get_success(self.handler.get_room_summary(user2, self.room)) + self.assertEqual(result.get("room_id"), self.room) From cc6c2713d81b79cf5f9c56497f7d42d94a908671 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 5 Aug 2021 11:43:22 +0100 Subject: [PATCH 13/35] fix mypy lint and missing await Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- synapse/handlers/room_summary.py | 9 +++++++-- synapse/handlers/space_summary.py | 2 +- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/synapse/handlers/room_summary.py b/synapse/handlers/room_summary.py index 691f55ba205d..f5ada8b250bd 100644 --- a/synapse/handlers/room_summary.py +++ b/synapse/handlers/room_summary.py @@ -84,7 +84,7 @@ async def build_room_entry(self, room_id: str) -> JsonDict: async def requester_can_see_room_entry( self, room: JsonDict, - requester: str, + requester: Optional[str], ) -> bool: # The room should only be allowed in the summary if: # a. the user is in the room; @@ -108,7 +108,12 @@ async def requester_can_see_room_entry( # Check if the user is a member of any of the allowed rooms # from the response. allowed_rooms = room.get("allowed_room_ids") - if not include_room and allowed_rooms and isinstance(allowed_rooms, list): + if ( + not include_room + and requester + and allowed_rooms + and isinstance(allowed_rooms, list) + ): include_room = await self._event_auth_handler.is_user_in_rooms( allowed_rooms, requester ) diff --git a/synapse/handlers/space_summary.py b/synapse/handlers/space_summary.py index 6fcbdc415e15..d7cb4bdd4363 100644 --- a/synapse/handlers/space_summary.py +++ b/synapse/handlers/space_summary.py @@ -141,7 +141,7 @@ async def get_space_summary( if not fed_room_id or not isinstance(fed_room_id, str): continue - include_room = self.requester_can_see_room_entry(room, requester) + include_room = await self.requester_can_see_room_entry(room, requester) # Finally, if this isn't the requested room, check ourselves # if we can access the room. From 0c5952bf2d65b83c0a0be7d77468bd1bbbf083fa Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 5 Aug 2021 11:51:30 +0100 Subject: [PATCH 14/35] delint s'more Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- synapse/handlers/space_summary.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/synapse/handlers/space_summary.py b/synapse/handlers/space_summary.py index d7cb4bdd4363..d3fef50c1d38 100644 --- a/synapse/handlers/space_summary.py +++ b/synapse/handlers/space_summary.py @@ -141,7 +141,9 @@ async def get_space_summary( if not fed_room_id or not isinstance(fed_room_id, str): continue - include_room = await self.requester_can_see_room_entry(room, requester) + include_room = await self.requester_can_see_room_entry( + room, requester + ) # Finally, if this isn't the requested room, check ourselves # if we can access the room. From 9e113aa341a275cd8ff405dcf2ffff0caf519b85 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 5 Aug 2021 12:16:18 +0100 Subject: [PATCH 15/35] fix test_state by stubbing get_event_auth_handler Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- tests/test_state.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_state.py b/tests/test_state.py index e5488df1ac71..7b33a662d238 100644 --- a/tests/test_state.py +++ b/tests/test_state.py @@ -166,6 +166,7 @@ def setUp(self): "get_storage", "get_auth", "get_state_handler", + "get_event_auth_handler", "get_clock", "get_state_resolution_handler", "get_account_validity_handler", @@ -175,6 +176,7 @@ def setUp(self): hs.config = default_config("tesths", True) hs.get_datastore.return_value = self.store hs.get_state_handler.return_value = None + hs.get_event_auth_handler.return_value = None hs.get_clock.return_value = MockClock() hs.get_auth.return_value = Auth(hs) hs.get_state_resolution_handler = lambda: StateResolutionHandler(hs) From caa801508216455f8ca82f86a40368f05d8c2de7 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 12 Aug 2021 11:00:05 +0100 Subject: [PATCH 16/35] re-apply consolidation between room_summary and space_summary Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- changelog.d/10394.feature | 2 +- synapse/api/auth.py | 4 +- synapse/handlers/room_summary.py | 107 +--------- synapse/handlers/space_summary.py | 343 +++++++++++------------------- 4 files changed, 128 insertions(+), 328 deletions(-) diff --git a/changelog.d/10394.feature b/changelog.d/10394.feature index b1712c3405a8..52be7f11a463 100644 --- a/changelog.d/10394.feature +++ b/changelog.d/10394.feature @@ -1 +1 @@ -Initial support for MSC3266, Room Summary over the unstable /rooms/{roomIdOrAlias}/summary API. +Initial local support for MSC3266, Room Summary over the unstable /rooms/{roomIdOrAlias}/summary API. diff --git a/synapse/api/auth.py b/synapse/api/auth.py index 32c44e2d9d4c..e047e6528c4c 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -623,7 +623,7 @@ async def check_auth_blocking(self, *args, **kwargs) -> None: await self._auth_blocking.check_auth_blocking(*args, **kwargs) async def is_room_accessible( - self, room_id: str, requester: Optional[str], origin: Optional[str] + self, room_id: str, requester: Optional[str], origin: Optional[str] = None ) -> bool: """ Calculate whether the room should be shown to the requester. @@ -728,7 +728,7 @@ async def is_room_accessible( return True logger.info( - "room %s is unpeekable and requester %s is not a member / not allowed to join, omitting from summary", + "room %s is unpeekable and requester %s is not a member / not allowed to join", room_id, requester or origin, ) diff --git a/synapse/handlers/room_summary.py b/synapse/handlers/room_summary.py index f5ada8b250bd..f7024c42fe9b 100644 --- a/synapse/handlers/room_summary.py +++ b/synapse/handlers/room_summary.py @@ -15,13 +15,8 @@ import logging from typing import TYPE_CHECKING, List, Optional -from synapse.api.constants import ( - EventContentFields, - EventTypes, - HistoryVisibility, - JoinRules, -) from synapse.api.errors import NotFoundError +from synapse.handlers.space_summary import RoomSummaryMixin from synapse.types import JsonDict if TYPE_CHECKING: @@ -30,102 +25,10 @@ logger = logging.getLogger(__name__) -class RoomSummaryMixin: - def __init__(self, hs: "HomeServer"): - self._store = hs.get_datastore() - self._event_auth_handler = hs.get_event_auth_handler() - - async def build_room_entry(self, room_id: str) -> JsonDict: - """Generate en entry suitable for the 'rooms' list in the summary response""" - stats = await self._store.get_room_with_stats(room_id) - - # currently this should be impossible because we call - # check_user_in_room_or_world_readable on the room before we get here, so - # there should always be an entry - assert stats is not None, "unable to retrieve stats for %s" % (room_id,) - - current_state_ids = await self._store.get_current_state_ids(room_id) - create_event = await self._store.get_event( - current_state_ids[(EventTypes.Create, "")] - ) - - room_version = await self._store.get_room_version(room_id) - allowed_rooms = None - if await self._event_auth_handler.has_restricted_join_rules( - current_state_ids, room_version - ): - allowed_rooms = await self._event_auth_handler.get_rooms_that_allow_join( - current_state_ids - ) - - entry = { - "room_id": stats["room_id"], - "name": stats["name"], - "topic": stats["topic"], - "canonical_alias": stats["canonical_alias"], - "num_joined_members": stats["joined_members"], - "avatar_url": stats["avatar"], - "join_rules": stats["join_rules"], - "world_readable": ( - stats["history_visibility"] == HistoryVisibility.WORLD_READABLE - ), - "guest_can_join": stats["guest_access"] == "can_join", - "creation_ts": create_event.origin_server_ts, - "room_type": create_event.content.get(EventContentFields.ROOM_TYPE), - "is_encrypted": (EventTypes.RoomEncryption, "") in current_state_ids, - "allowed_room_ids": allowed_rooms, # this field is stripped from the cs response - } - - # Filter out Nones – rather omit the field altogether - room_entry = {k: v for k, v in entry.items() if v is not None} - - return room_entry - - async def requester_can_see_room_entry( - self, - room: JsonDict, - requester: Optional[str], - ) -> bool: - # The room should only be allowed 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 rooms - # from the response. - allowed_rooms = room.get("allowed_room_ids") - if ( - not include_room - and requester - and allowed_rooms - and isinstance(allowed_rooms, list) - ): - include_room = await self._event_auth_handler.is_user_in_rooms( - allowed_rooms, requester - ) - - return include_room - - class RoomSummaryHandler(RoomSummaryMixin): def __init__(self, hs: "HomeServer"): super().__init__(hs) self._clock = hs.get_clock() - self._auth = hs.get_auth() self._server_name = hs.hostname async def get_room_summary( @@ -167,13 +70,15 @@ async def get_room_summary( room_summary = await self._summarize_remote_room(room_id, remote_room_hosts) # validate that the requester has permission to see this room - include_room = self.requester_can_see_room_entry(room_summary, requester) + include_room = self._is_remote_room_accessible(requester, room_id, room_summary) if not include_room: raise NotFoundError("Room not found or is not accessible") - # Before returning to the client, remove the allowed_room_ids key. + # Before returning to the client, remove the allowed_room_ids + # and allowed_spaces keys. room_summary.pop("allowed_room_ids", None) + room_summary.pop("allowed_spaces", None) return room_summary @@ -201,7 +106,7 @@ async def _summarize_local_room( if not await self._auth.is_room_accessible(room_id, requester, origin): raise NotFoundError("Room not found or is not accessible") - return await self.build_room_entry(room_id) + return await self._build_room_entry(room_id, for_federation=bool(origin)) async def _summarize_remote_room( self, diff --git a/synapse/handlers/space_summary.py b/synapse/handlers/space_summary.py index d0060f90462e..ece1c2734949 100644 --- a/synapse/handlers/space_summary.py +++ b/synapse/handlers/space_summary.py @@ -35,7 +35,6 @@ EventTypes, HistoryVisibility, JoinRules, - Membership, RoomTypes, ) from synapse.api.errors import AuthError, Codes, SynapseError @@ -85,11 +84,126 @@ class _PaginationSession: processed_rooms: Set[str] -class SpaceSummaryHandler: +class RoomSummaryMixin: + def __init__(self, hs: "HomeServer"): + self._store = hs.get_datastore() + self._auth = hs.get_auth() + self._event_auth_handler = hs.get_event_auth_handler() + + 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. + + Args: + room_id: The room ID to summarize. + for_federation: True if this is a summary requested over federation + (which includes additional fields). + + Returns: + The JSON dictionary for the room. + """ + stats = await self._store.get_room_with_stats(room_id) + + # currently this should be impossible because we call + # _is_local_room_accessible on the room before we get here, so + # there should always be an entry + assert stats is not None, "unable to retrieve stats for %s" % (room_id,) + + current_state_ids = await self._store.get_current_state_ids(room_id) + create_event = await self._store.get_event( + current_state_ids[(EventTypes.Create, "")] + ) + + entry = { + "room_id": stats["room_id"], + "name": stats["name"], + "topic": stats["topic"], + "canonical_alias": stats["canonical_alias"], + "num_joined_members": stats["joined_members"], + "avatar_url": stats["avatar"], + "join_rules": stats["join_rules"], + "world_readable": ( + stats["history_visibility"] == HistoryVisibility.WORLD_READABLE + ), + "guest_can_join": stats["guest_access"] == "can_join", + "creation_ts": create_event.origin_server_ts, + "room_type": create_event.content.get(EventContentFields.ROOM_TYPE), + } + + # Federation requests need to provide additional information so the + # requested server is able to filter the response appropriately. + if for_federation: + room_version = await self._store.get_room_version(room_id) + if await self._event_auth_handler.has_restricted_join_rules( + current_state_ids, room_version + ): + allowed_rooms = ( + await self._event_auth_handler.get_rooms_that_allow_join( + current_state_ids + ) + ) + if allowed_rooms: + entry["allowed_room_ids"] = allowed_rooms + # TODO Remove this key once the API is stable. + entry["allowed_spaces"] = allowed_rooms + + # Filter out Nones – rather omit the field altogether + room_entry = {k: v for k, v in entry.items() if v is not None} + + return room_entry + + 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 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._auth.is_room_accessible(room_id, requester) + + +class SpaceSummaryHandler(RoomSummaryMixin): # The time a pagination session remains valid for. _PAGINATION_SESSION_VALIDITY_PERIOD_MS = 5 * 60 * 1000 def __init__(self, hs: "HomeServer"): + super().__init__(hs) self._clock = hs.get_clock() self._event_auth_handler = hs.get_event_auth_handler() self._store = hs.get_datastore() @@ -153,7 +267,7 @@ async def get_space_summary( summary dict to return """ # First of all, check that the room is accessible. - if not await self._is_local_room_accessible(room_id, requester): + if not await self._auth.is_room_accessible(room_id, requester): raise AuthError( 403, "User %s not in room %s, and room previews are disabled" @@ -328,7 +442,7 @@ async def _get_room_hierarchy( """See docstring for SpaceSummaryHandler.get_room_hierarchy.""" # First of all, check that the room is accessible. - if not await self._is_local_room_accessible(requested_room_id, requester): + if not await self._auth.is_room_accessible(requested_room_id, requester): raise AuthError( 403, "User %s not in room %s, and room previews are disabled" @@ -514,7 +628,7 @@ async def _summarize_local_room( Returns: A room entry if the room should be returned. None, otherwise. """ - if not await self._is_local_room_accessible(room_id, requester, origin): + if not await self._auth.is_room_accessible(room_id, requester, origin): return None room_entry = await self._build_room_entry(room_id, for_federation=bool(origin)) @@ -616,225 +730,6 @@ async def _summarize_remote_room( return results - async def _is_local_room_accessible( - self, room_id: str, requester: Optional[str], origin: Optional[str] = None - ) -> bool: - """ - Calculate whether the room should be shown in the spaces summary. - - It should be included if: - - * The requester is joined or can join the room (per MSC3173). - * The origin server has any user that is joined or can join the room. - * The history visibility is set to world readable. - - Args: - room_id: The room ID to summarize. - requester: - The user requesting the summary, if it is a local request. None - if this is a federation request. - origin: - The server requesting the summary, if it is a federation request. - None if this is a local request. - - Returns: - True if the room should be included in the spaces summary. - """ - state_ids = await self._store.get_current_state_ids(room_id) - - # If there's no state for the room, it isn't known. - if not state_ids: - # The user might have a pending invite for the room. - if requester and await self._store.get_invite_for_local_user_in_room( - requester, room_id - ): - return True - - logger.info("room %s is unknown, omitting from summary", room_id) - return False - - room_version = await self._store.get_room_version(room_id) - - # Include the room if it has join rules of public or knock. - join_rules_event_id = state_ids.get((EventTypes.JoinRules, "")) - if join_rules_event_id: - join_rules_event = await self._store.get_event(join_rules_event_id) - join_rule = join_rules_event.content.get("join_rule") - if join_rule == JoinRules.PUBLIC or ( - room_version.msc2403_knocking and join_rule == JoinRules.KNOCK - ): - return True - - # Include the room if it is peekable. - hist_vis_event_id = state_ids.get((EventTypes.RoomHistoryVisibility, "")) - if hist_vis_event_id: - hist_vis_ev = await self._store.get_event(hist_vis_event_id) - hist_vis = hist_vis_ev.content.get("history_visibility") - if hist_vis == HistoryVisibility.WORLD_READABLE: - return True - - # Otherwise we need to check information specific to the user or server. - - # If we have an authenticated requesting user, check if they are a member - # of the room (or can join the room). - if requester: - member_event_id = state_ids.get((EventTypes.Member, requester), None) - - # If they're in the room they can see info on it. - if member_event_id: - member_event = await self._store.get_event(member_event_id) - if member_event.membership in (Membership.JOIN, Membership.INVITE): - return True - - # Otherwise, check if they should be allowed access via membership in a space. - if await self._event_auth_handler.has_restricted_join_rules( - state_ids, room_version - ): - allowed_rooms = ( - await self._event_auth_handler.get_rooms_that_allow_join(state_ids) - ) - if await self._event_auth_handler.is_user_in_rooms( - allowed_rooms, requester - ): - return True - - # If this is a request over federation, check if the host is in the room or - # has a user who could join the room. - elif origin: - if await self._event_auth_handler.check_host_in_room( - room_id, origin - ) or await self._store.is_host_invited(room_id, origin): - return True - - # Alternately, if the host has a user in any of the spaces specified - # for access, then the host can see this room (and should do filtering - # if the requester cannot see it). - if await self._event_auth_handler.has_restricted_join_rules( - state_ids, room_version - ): - allowed_rooms = ( - await self._event_auth_handler.get_rooms_that_allow_join(state_ids) - ) - for space_id in allowed_rooms: - if await self._event_auth_handler.check_host_in_room( - space_id, origin - ): - return True - - logger.info( - "room %s is unpeekable and requester %s is not a member / not allowed to join, omitting from summary", - room_id, - requester or origin, - ) - 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) - - 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. - - Args: - room_id: The room ID to summarize. - for_federation: True if this is a summary requested over federation - (which includes additional fields). - - Returns: - The JSON dictionary for the room. - """ - stats = await self._store.get_room_with_stats(room_id) - - # currently this should be impossible because we call - # _is_local_room_accessible on the room before we get here, so - # there should always be an entry - assert stats is not None, "unable to retrieve stats for %s" % (room_id,) - - current_state_ids = await self._store.get_current_state_ids(room_id) - create_event = await self._store.get_event( - current_state_ids[(EventTypes.Create, "")] - ) - - entry = { - "room_id": stats["room_id"], - "name": stats["name"], - "topic": stats["topic"], - "canonical_alias": stats["canonical_alias"], - "num_joined_members": stats["joined_members"], - "avatar_url": stats["avatar"], - "join_rules": stats["join_rules"], - "world_readable": ( - stats["history_visibility"] == HistoryVisibility.WORLD_READABLE - ), - "guest_can_join": stats["guest_access"] == "can_join", - "creation_ts": create_event.origin_server_ts, - "room_type": create_event.content.get(EventContentFields.ROOM_TYPE), - } - - # Federation requests need to provide additional information so the - # requested server is able to filter the response appropriately. - if for_federation: - room_version = await self._store.get_room_version(room_id) - if await self._event_auth_handler.has_restricted_join_rules( - current_state_ids, room_version - ): - allowed_rooms = ( - await self._event_auth_handler.get_rooms_that_allow_join( - current_state_ids - ) - ) - if allowed_rooms: - entry["allowed_room_ids"] = allowed_rooms - # TODO Remove this key once the API is stable. - entry["allowed_spaces"] = allowed_rooms - - # Filter out Nones – rather omit the field altogether - room_entry = {k: v for k, v in entry.items() if v is not None} - - return room_entry - async def _get_child_events(self, room_id: str) -> Iterable[EventBase]: """ Get the child events for a given room. From f5c67406646f276cd37e8159cd66ae2f7a90dcc5 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 12 Aug 2021 11:02:35 +0100 Subject: [PATCH 17/35] delint Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- synapse/handlers/room_summary.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/synapse/handlers/room_summary.py b/synapse/handlers/room_summary.py index f7024c42fe9b..53f92905ae00 100644 --- a/synapse/handlers/room_summary.py +++ b/synapse/handlers/room_summary.py @@ -70,7 +70,9 @@ async def get_room_summary( room_summary = await self._summarize_remote_room(room_id, remote_room_hosts) # validate that the requester has permission to see this room - include_room = self._is_remote_room_accessible(requester, room_id, room_summary) + include_room = self._is_remote_room_accessible( + requester, room_id, room_summary + ) if not include_room: raise NotFoundError("Room not found or is not accessible") From 2a9b9611002400f273f853774f4029757a5a0446 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 12 Aug 2021 11:09:23 +0100 Subject: [PATCH 18/35] Make requester optional to be more generically re-usable Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- synapse/handlers/space_summary.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/synapse/handlers/space_summary.py b/synapse/handlers/space_summary.py index ece1c2734949..c251ce1c14ff 100644 --- a/synapse/handlers/space_summary.py +++ b/synapse/handlers/space_summary.py @@ -153,7 +153,7 @@ async def _build_room_entry(self, room_id: str, for_federation: bool) -> JsonDic return room_entry async def _is_remote_room_accessible( - self, requester: str, room_id: str, room: JsonDict + self, requester: Optional[str], room_id: str, room: JsonDict ) -> bool: """ Calculate whether the room received over federation should be shown in the summary. @@ -168,7 +168,7 @@ async def _is_remote_room_accessible( due to an invite, etc. Args: - requester: The user requesting the summary. + requester: The user requesting the summary, if authenticated. room_id: The room ID returned over federation. room: The summary of the child room returned over federation. @@ -186,7 +186,7 @@ async def _is_remote_room_accessible( # 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 requester and allowed_rooms and isinstance(allowed_rooms, list): if await self._event_auth_handler.is_user_in_rooms( allowed_rooms, requester ): From b277031ed14f109f59cd9f7ddc75a6d55aeb365e Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 12 Aug 2021 11:16:43 +0100 Subject: [PATCH 19/35] update naming and comments Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- synapse/api/auth.py | 10 +++++----- synapse/handlers/room_summary.py | 2 +- synapse/handlers/space_summary.py | 8 ++++---- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/synapse/api/auth.py b/synapse/api/auth.py index e047e6528c4c..b52513889984 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -622,23 +622,23 @@ async def check_user_in_room_or_world_readable( async def check_auth_blocking(self, *args, **kwargs) -> None: await self._auth_blocking.check_auth_blocking(*args, **kwargs) - async def is_room_accessible( + async def is_room_visible( self, room_id: str, requester: Optional[str], origin: Optional[str] = None ) -> bool: """ Calculate whether the room should be shown to the requester. - It should be included if: + It should return true if: * The requester is joined or can join the room (per MSC3173). * The origin server has any user that is joined or can join the room. * The history visibility is set to world readable. Args: - room_id: The room ID to summarize. + room_id: The room ID to check visibility of. requester: - The user requesting the summary, if it is a local request. None - if this is a federation request. + The user requesting the summary, if it is a local request. + None if this is a federation request. origin: The server requesting the summary, if it is a federation request. None if this is a local request. diff --git a/synapse/handlers/room_summary.py b/synapse/handlers/room_summary.py index 53f92905ae00..16541776610b 100644 --- a/synapse/handlers/room_summary.py +++ b/synapse/handlers/room_summary.py @@ -105,7 +105,7 @@ async def _summarize_local_room( Returns: summary dict to return """ - if not await self._auth.is_room_accessible(room_id, requester, origin): + if not await self._auth.is_room_visible(room_id, requester, origin): raise NotFoundError("Room not found or is not accessible") return await self._build_room_entry(room_id, for_federation=bool(origin)) diff --git a/synapse/handlers/space_summary.py b/synapse/handlers/space_summary.py index c251ce1c14ff..fd1f2b33707b 100644 --- a/synapse/handlers/space_summary.py +++ b/synapse/handlers/space_summary.py @@ -195,7 +195,7 @@ async def _is_remote_room_accessible( # 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._auth.is_room_accessible(room_id, requester) + return await self._auth.is_room_visible(room_id, requester) class SpaceSummaryHandler(RoomSummaryMixin): @@ -267,7 +267,7 @@ async def get_space_summary( summary dict to return """ # First of all, check that the room is accessible. - if not await self._auth.is_room_accessible(room_id, requester): + if not await self._auth.is_room_visible(room_id, requester): raise AuthError( 403, "User %s not in room %s, and room previews are disabled" @@ -442,7 +442,7 @@ async def _get_room_hierarchy( """See docstring for SpaceSummaryHandler.get_room_hierarchy.""" # First of all, check that the room is accessible. - if not await self._auth.is_room_accessible(requested_room_id, requester): + if not await self._auth.is_room_visible(requested_room_id, requester): raise AuthError( 403, "User %s not in room %s, and room previews are disabled" @@ -628,7 +628,7 @@ async def _summarize_local_room( Returns: A room entry if the room should be returned. None, otherwise. """ - if not await self._auth.is_room_accessible(room_id, requester, origin): + if not await self._auth.is_room_visible(room_id, requester, origin): return None room_entry = await self._build_room_entry(room_id, for_federation=bool(origin)) From 61d9312b23ff53f4aca8f6b80ebc76d7c54fc099 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@googlemail.com> Date: Fri, 13 Aug 2021 11:11:19 +0100 Subject: [PATCH 20/35] Apply suggestions from code review Co-authored-by: Patrick Cloke --- changelog.d/10394.feature | 2 +- synapse/api/auth.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/changelog.d/10394.feature b/changelog.d/10394.feature index 52be7f11a463..c8bbc5a740fd 100644 --- a/changelog.d/10394.feature +++ b/changelog.d/10394.feature @@ -1 +1 @@ -Initial local support for MSC3266, Room Summary over the unstable /rooms/{roomIdOrAlias}/summary API. +Initial local support for [MSC3266](https://github.com/matrix-org/synapse/pull/10394), Room Summary over the unstable `/rooms/{roomIdOrAlias}/summary` API. diff --git a/synapse/api/auth.py b/synapse/api/auth.py index b52513889984..509a0f6ab80a 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -644,7 +644,7 @@ async def is_room_visible( None if this is a local request. Returns: - True if the room should be visible to the requester. + True if the room should be visible to the requesting user or server. """ state_ids = await self.store.get_current_state_ids(room_id) From dfa8247c57b6571fb973e3e6129eb8cc890f2687 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 13 Aug 2021 11:13:32 +0100 Subject: [PATCH 21/35] remove references to summary Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- synapse/api/auth.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/synapse/api/auth.py b/synapse/api/auth.py index 509a0f6ab80a..0cfa77b3587a 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -637,10 +637,10 @@ async def is_room_visible( Args: room_id: The room ID to check visibility of. requester: - The user requesting the summary, if it is a local request. + The user making the request, if it is a local request. None if this is a federation request. origin: - The server requesting the summary, if it is a federation request. + The server making the request, if it is a federation request. None if this is a local request. Returns: @@ -656,7 +656,7 @@ async def is_room_visible( ): return True - logger.info("room %s is unknown, omitting from summary", room_id) + logger.info("room %s is unknown", room_id) return False room_version = await self.store.get_room_version(room_id) From 9dbe660a775f3740dd73b225b5ae65ff6b1d7ba9 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 13 Aug 2021 11:38:37 +0100 Subject: [PATCH 22/35] Consolidate the two handlers into 1 Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- synapse/federation/transport/server.py | 2 +- synapse/handlers/room_summary.py | 815 +++++++++++++++++++++++- synapse/handlers/space_summary.py | 837 ------------------------- synapse/rest/client/v1/room.py | 11 +- synapse/server.py | 5 - tests/handlers/test_room_summary.py | 815 +++++++++++++++++++++++- tests/handlers/test_space_summary.py | 835 ------------------------ 7 files changed, 1626 insertions(+), 1694 deletions(-) delete mode 100644 synapse/handlers/space_summary.py delete mode 100644 tests/handlers/test_space_summary.py diff --git a/synapse/federation/transport/server.py b/synapse/federation/transport/server.py index 640f46fff6bf..7f532de308e7 100644 --- a/synapse/federation/transport/server.py +++ b/synapse/federation/transport/server.py @@ -1887,7 +1887,7 @@ def __init__( server_name: str, ): super().__init__(hs, authenticator, ratelimiter, server_name) - self.handler = hs.get_space_summary_handler() + self.handler = hs.get_room_summary_handler() async def on_GET( self, diff --git a/synapse/handlers/room_summary.py b/synapse/handlers/room_summary.py index 16541776610b..919302627651 100644 --- a/synapse/handlers/room_summary.py +++ b/synapse/handlers/room_summary.py @@ -12,24 +12,745 @@ # See the License for the specific language governing permissions and # limitations under the License. +import itertools import logging -from typing import TYPE_CHECKING, List, Optional +import re +from collections import deque +from typing import ( + TYPE_CHECKING, + Deque, + Dict, + Iterable, + List, + Optional, + Sequence, + Set, + Tuple, +) -from synapse.api.errors import NotFoundError -from synapse.handlers.space_summary import RoomSummaryMixin +import attr + +from synapse.api.constants import ( + EventContentFields, + EventTypes, + HistoryVisibility, + JoinRules, + RoomTypes, +) +from synapse.api.errors import AuthError, Codes, NotFoundError, SynapseError +from synapse.events import EventBase +from synapse.events.utils import format_event_for_client_v2 from synapse.types import JsonDict +from synapse.util.caches.response_cache import ResponseCache +from synapse.util.stringutils import random_string if TYPE_CHECKING: from synapse.server import HomeServer logger = logging.getLogger(__name__) +# number of rooms to return. We'll stop once we hit this limit. +MAX_ROOMS = 50 + +# max number of events to return per room. +MAX_ROOMS_PER_SPACE = 50 + +# max number of federation servers to hit per room +MAX_SERVERS_PER_SPACE = 3 + + +@attr.s(slots=True, frozen=True, auto_attribs=True) +class _PaginationKey: + """The key used to find unique pagination session.""" + + # The first three entries match the request parameters (and cannot change + # during a pagination session). + room_id: str + suggested_only: bool + max_depth: Optional[int] + # The randomly generated token. + token: str + + +@attr.s(slots=True, frozen=True, auto_attribs=True) +class _PaginationSession: + """The information that is stored for pagination.""" + + # The time the pagination session was created, in milliseconds. + creation_time_ms: int + # The queue of rooms which are still to process. + room_queue: Deque["_RoomQueueEntry"] + # A set of rooms which have been processed. + processed_rooms: Set[str] + + +class RoomSummaryHandler: + # The time a pagination session remains valid for. + _PAGINATION_SESSION_VALIDITY_PERIOD_MS = 5 * 60 * 1000 -class RoomSummaryHandler(RoomSummaryMixin): def __init__(self, hs: "HomeServer"): - super().__init__(hs) self._clock = hs.get_clock() + self._event_auth_handler = hs.get_event_auth_handler() + self._store = hs.get_datastore() + self._auth = hs.get_auth() + self._event_auth_handler = hs.get_event_auth_handler() + self._event_serializer = hs.get_event_client_serializer() self._server_name = hs.hostname + self._federation_client = hs.get_federation_client() + + # A map of query information to the current pagination state. + # + # TODO Allow for multiple workers to share this data. + # TODO Expire pagination tokens. + self._pagination_sessions: Dict[_PaginationKey, _PaginationSession] = {} + + # If a user tries to fetch the same page multiple times in quick succession, + # only process the first attempt and return its result to subsequent requests. + self._pagination_response_cache: ResponseCache[ + Tuple[str, bool, Optional[int], Optional[int], Optional[str]] + ] = ResponseCache( + hs.get_clock(), + "get_room_hierarchy", + ) + + 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. + + Args: + room_id: The room ID to summarize. + for_federation: True if this is a summary requested over federation + (which includes additional fields). + + Returns: + The JSON dictionary for the room. + """ + stats = await self._store.get_room_with_stats(room_id) + + # currently this should be impossible because we call + # _is_local_room_accessible on the room before we get here, so + # there should always be an entry + assert stats is not None, "unable to retrieve stats for %s" % (room_id,) + + current_state_ids = await self._store.get_current_state_ids(room_id) + create_event = await self._store.get_event( + current_state_ids[(EventTypes.Create, "")] + ) + + entry = { + "room_id": stats["room_id"], + "name": stats["name"], + "topic": stats["topic"], + "canonical_alias": stats["canonical_alias"], + "num_joined_members": stats["joined_members"], + "avatar_url": stats["avatar"], + "join_rules": stats["join_rules"], + "world_readable": ( + stats["history_visibility"] == HistoryVisibility.WORLD_READABLE + ), + "guest_can_join": stats["guest_access"] == "can_join", + "creation_ts": create_event.origin_server_ts, + "room_type": create_event.content.get(EventContentFields.ROOM_TYPE), + } + + # Federation requests need to provide additional information so the + # requested server is able to filter the response appropriately. + if for_federation: + room_version = await self._store.get_room_version(room_id) + if await self._event_auth_handler.has_restricted_join_rules( + current_state_ids, room_version + ): + allowed_rooms = ( + await self._event_auth_handler.get_rooms_that_allow_join( + current_state_ids + ) + ) + if allowed_rooms: + entry["allowed_room_ids"] = allowed_rooms + # TODO Remove this key once the API is stable. + entry["allowed_spaces"] = allowed_rooms + + # Filter out Nones – rather omit the field altogether + room_entry = {k: v for k, v in entry.items() if v is not None} + + return room_entry + + async def _is_remote_room_accessible( + self, requester: Optional[str], room_id: str, room: JsonDict + ) -> bool: + """ + Calculate whether the room received over federation should be shown in the 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, if authenticated. + 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 requester and 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._auth.is_room_visible(room_id, requester) + + def _expire_pagination_sessions(self): + """Expire pagination session which are old.""" + expire_before = ( + self._clock.time_msec() - self._PAGINATION_SESSION_VALIDITY_PERIOD_MS + ) + to_expire = [] + + for key, value in self._pagination_sessions.items(): + if value.creation_time_ms < expire_before: + to_expire.append(key) + + for key in to_expire: + logger.debug("Expiring pagination session id %s", key) + del self._pagination_sessions[key] + + async def get_space_summary( + self, + requester: str, + room_id: str, + suggested_only: bool = False, + max_rooms_per_space: Optional[int] = None, + ) -> JsonDict: + """ + Implementation of the space summary C-S API + + Args: + requester: user id of the user making this request + + room_id: room id to start the hierarchy from + + suggested_only: whether we should only return children with the "suggested" + flag set. + + max_rooms_per_space: an optional limit on the number of child rooms we will + return. This does not apply to the root room (ie, room_id), and + is overridden by MAX_ROOMS_PER_SPACE. + + Returns: + hierarchy dict to return + """ + # First of all, check that the room is accessible. + if not await self._auth.is_room_visible(room_id, requester): + raise AuthError( + 403, + "User %s not in room %s, and room previews are disabled" + % (requester, room_id), + ) + + # the queue of rooms to process + room_queue = deque((_RoomQueueEntry(room_id, ()),)) + + # rooms we have already processed + processed_rooms: Set[str] = set() + + # events we have already processed. We don't necessarily have their event ids, + # so instead we key on (room id, state key) + processed_events: Set[Tuple[str, str]] = set() + + rooms_result: List[JsonDict] = [] + events_result: List[JsonDict] = [] + + while room_queue and len(rooms_result) < MAX_ROOMS: + queue_entry = room_queue.popleft() + room_id = queue_entry.room_id + if room_id in processed_rooms: + # already done this room + continue + + logger.debug("Processing room %s", room_id) + + is_in_room = await self._store.is_host_joined(room_id, self._server_name) + + # The client-specified max_rooms_per_space limit doesn't apply to the + # room_id specified in the request, so we ignore it if this is the + # first room we are processing. + max_children = max_rooms_per_space if processed_rooms else None + + if is_in_room: + room_entry = await self._summarize_local_room_hierarchy( + requester, None, room_id, suggested_only, max_children + ) + + events: Sequence[JsonDict] = [] + if room_entry: + rooms_result.append(room_entry.room) + events = room_entry.children + + logger.debug( + "Query of local room %s returned events %s", + room_id, + ["%s->%s" % (ev["room_id"], ev["state_key"]) for ev in events], + ) + else: + fed_rooms = await self._summarize_remote_room_hierarchy( + queue_entry, + suggested_only, + max_children, + exclude_rooms=processed_rooms, + ) + + # The results over federation might include rooms that the we, + # as the requesting server, are allowed to see, but the requesting + # user is not permitted see. + # + # Filter the returned results to only what is accessible to the user. + events = [] + for room_entry in fed_rooms: + room = room_entry.room + fed_room_id = room_entry.room_id + + # The user can see the room, include it! + 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) + room.pop("allowed_spaces", None) + + rooms_result.append(room) + events.extend(room_entry.children) + + # All rooms returned don't need visiting again (even if the user + # didn't have access to them). + processed_rooms.add(fed_room_id) + + logger.debug( + "Query of %s returned rooms %s, events %s", + room_id, + [room_entry.room.get("room_id") for room_entry in fed_rooms], + ["%s->%s" % (ev["room_id"], ev["state_key"]) for ev in events], + ) + + # the room we queried may or may not have been returned, but don't process + # it again, anyway. + processed_rooms.add(room_id) + + # XXX: is it ok that we blindly iterate through any events returned by + # a remote server, whether or not they actually link to any rooms in our + # tree? + for ev in events: + # remote servers might return events we have already processed + # (eg, Dendrite returns inward pointers as well as outward ones), so + # we need to filter them out, to avoid returning duplicate links to the + # client. + ev_key = (ev["room_id"], ev["state_key"]) + if ev_key in processed_events: + continue + events_result.append(ev) + + # add the child to the queue. we have already validated + # that the vias are a list of server names. + room_queue.append( + _RoomQueueEntry(ev["state_key"], ev["content"]["via"]) + ) + processed_events.add(ev_key) + + return {"rooms": rooms_result, "events": events_result} + + async def get_room_hierarchy( + self, + requester: str, + requested_room_id: str, + suggested_only: bool = False, + max_depth: Optional[int] = None, + limit: Optional[int] = None, + from_token: Optional[str] = None, + ) -> JsonDict: + """ + Implementation of the room hierarchy C-S API. + + Args: + requester: The user ID of the user making this request. + requested_room_id: The room ID to start the hierarchy at (the "root" room). + suggested_only: Whether we should only return children with the "suggested" + flag set. + max_depth: The maximum depth in the tree to explore, must be a + non-negative integer. + + 0 would correspond to just the root room, 1 would include just + the root room's children, etc. + limit: An optional limit on the number of rooms to return per + page. Must be a positive integer. + from_token: An optional pagination token. + + Returns: + The JSON hierarchy dictionary. + """ + # If a user tries to fetch the same page multiple times in quick succession, + # only process the first attempt and return its result to subsequent requests. + # + # This is due to the pagination process mutating internal state, attempting + # to process multiple requests for the same page will result in errors. + return await self._pagination_response_cache.wrap( + (requested_room_id, suggested_only, max_depth, limit, from_token), + self._get_room_hierarchy, + requester, + requested_room_id, + suggested_only, + max_depth, + limit, + from_token, + ) + + async def _get_room_hierarchy( + self, + requester: str, + requested_room_id: str, + suggested_only: bool = False, + max_depth: Optional[int] = None, + limit: Optional[int] = None, + from_token: Optional[str] = None, + ) -> JsonDict: + """See docstring for SpaceSummaryHandler.get_room_hierarchy.""" + + # First of all, check that the room is accessible. + if not await self._auth.is_room_visible(requested_room_id, requester): + raise AuthError( + 403, + "User %s not in room %s, and room previews are disabled" + % (requester, requested_room_id), + ) + + # If this is continuing a previous session, pull the persisted data. + if from_token: + self._expire_pagination_sessions() + + pagination_key = _PaginationKey( + requested_room_id, suggested_only, max_depth, from_token + ) + if pagination_key not in self._pagination_sessions: + raise SynapseError(400, "Unknown pagination token", Codes.INVALID_PARAM) + + # Load the previous state. + pagination_session = self._pagination_sessions[pagination_key] + room_queue = pagination_session.room_queue + processed_rooms = pagination_session.processed_rooms + else: + # the queue of rooms to process + room_queue = deque((_RoomQueueEntry(requested_room_id, ()),)) + + # Rooms we have already processed. + processed_rooms = set() + + rooms_result: List[JsonDict] = [] + + # Cap the limit to a server-side maximum. + if limit is None: + limit = MAX_ROOMS + else: + limit = min(limit, MAX_ROOMS) + + # Iterate through the queue until we reach the limit or run out of + # rooms to include. + while room_queue and len(rooms_result) < limit: + queue_entry = room_queue.popleft() + room_id = queue_entry.room_id + current_depth = queue_entry.depth + if room_id in processed_rooms: + # already done this room + continue + + logger.debug("Processing room %s", room_id) + + is_in_room = await self._store.is_host_joined(room_id, self._server_name) + if is_in_room: + room_entry = await self._summarize_local_room_hierarchy( + requester, + None, + room_id, + suggested_only, + # TODO Handle max children. + max_children=None, + ) + + if room_entry: + rooms_result.append(room_entry.as_json()) + + # Add the child to the queue. We have already validated + # that the vias are a list of server names. + # + # If the current depth is the maximum depth, do not queue + # more entries. + if max_depth is None or current_depth < max_depth: + room_queue.extendleft( + _RoomQueueEntry( + ev["state_key"], ev["content"]["via"], current_depth + 1 + ) + for ev in reversed(room_entry.children) + ) + + processed_rooms.add(room_id) + else: + # TODO Federation. + pass + + result: JsonDict = {"rooms": rooms_result} + + # If there's additional data, generate a pagination token (and persist state). + if room_queue: + next_batch = random_string(24) + result["next_batch"] = next_batch + pagination_key = _PaginationKey( + requested_room_id, suggested_only, max_depth, next_batch + ) + self._pagination_sessions[pagination_key] = _PaginationSession( + self._clock.time_msec(), room_queue, processed_rooms + ) + + return result + + async def federation_space_summary( + self, + origin: str, + room_id: str, + suggested_only: bool, + max_rooms_per_space: Optional[int], + exclude_rooms: Iterable[str], + ) -> JsonDict: + """ + Implementation of the space summary Federation API + + Args: + origin: The server requesting the spaces summary. + + room_id: room id to start the summary at + + suggested_only: whether we should only return children with the "suggested" + flag set. + + max_rooms_per_space: an optional limit on the number of child rooms we will + return. Unlike the C-S API, this applies to the root room (room_id). + It is clipped to MAX_ROOMS_PER_SPACE. + + exclude_rooms: a list of rooms to skip over (presumably because the + calling server has already seen them). + + Returns: + summary dict to return + """ + # the queue of rooms to process + room_queue = deque((room_id,)) + + # the set of rooms that we should not walk further. Initialise it with the + # excluded-rooms list; we will add other rooms as we process them so that + # we do not loop. + processed_rooms: Set[str] = set(exclude_rooms) + + rooms_result: List[JsonDict] = [] + events_result: List[JsonDict] = [] + + while room_queue and len(rooms_result) < MAX_ROOMS: + room_id = room_queue.popleft() + if room_id in processed_rooms: + # already done this room + continue + + room_entry = await self._summarize_local_room_hierarchy( + None, origin, room_id, suggested_only, max_rooms_per_space + ) + + processed_rooms.add(room_id) + + if room_entry: + rooms_result.append(room_entry.room) + events_result.extend(room_entry.children) + + # add any children to the queue + room_queue.extend( + edge_event["state_key"] for edge_event in room_entry.children + ) + + return {"rooms": rooms_result, "events": events_result} + + async def _summarize_local_room_hierarchy( + self, + requester: Optional[str], + origin: Optional[str], + room_id: str, + suggested_only: bool, + max_children: Optional[int], + ) -> Optional["_RoomEntry"]: + """ + Generate a room entry and a list of event entries for a given room. + + Args: + requester: + The user requesting the summary, if it is a local request. None + if this is a federation request. + origin: + The server requesting the summary, if it is a federation request. + None if this is a local request. + room_id: The room ID to summarize. + suggested_only: True if only suggested children should be returned. + Otherwise, all children are returned. + max_children: + The maximum number of children rooms to include. This is capped + to a server-set limit. + + Returns: + A room entry if the room should be returned. None, otherwise. + """ + try: + room_entry = await self._summarize_local_room(room_id, requester, origin) + except NotFoundError: + return None + + # If the room is not a space, return just the room information. + if room_entry.get("room_type") != RoomTypes.SPACE: + return _RoomEntry(room_id, room_entry) + + # Otherwise, look for child rooms/spaces. + child_events = await self._get_child_events(room_id) + + if suggested_only: + # we only care about suggested children + child_events = filter(_is_suggested_child_event, child_events) + + if max_children is None or max_children > MAX_ROOMS_PER_SPACE: + max_children = MAX_ROOMS_PER_SPACE + + now = self._clock.time_msec() + events_result: List[JsonDict] = [] + for edge_event in itertools.islice(child_events, max_children): + events_result.append( + await self._event_serializer.serialize_event( + edge_event, + time_now=now, + event_format=format_event_for_client_v2, + ) + ) + + return _RoomEntry(room_id, room_entry, events_result) + + async def _summarize_remote_room_hierarchy( + self, + room: "_RoomQueueEntry", + suggested_only: bool, + max_children: Optional[int], + exclude_rooms: Iterable[str], + ) -> Iterable["_RoomEntry"]: + """ + Request room entries and a list of event entries for a given room by querying a remote server. + + Args: + room: The room to summarize. + suggested_only: True if only suggested children should be returned. + Otherwise, all children are returned. + max_children: + The maximum number of children rooms to include. This is capped + to a server-set limit. + exclude_rooms: + Rooms IDs which do not need to be summarized. + + Returns: + An iterable of room entries. + """ + room_id = room.room_id + logger.info("Requesting summary for %s via %s", room_id, room.via) + + # we need to make the exclusion list json-serialisable + exclude_rooms = list(exclude_rooms) + + via = itertools.islice(room.via, MAX_SERVERS_PER_SPACE) + try: + res = await self._federation_client.get_space_summary( + via, + room_id, + suggested_only=suggested_only, + max_rooms_per_space=max_children, + exclude_rooms=exclude_rooms, + ) + except Exception as e: + logger.warning( + "Unable to get summary of %s via federation: %s", + room_id, + e, + exc_info=logger.isEnabledFor(logging.DEBUG), + ) + return () + + # Group the events by their room. + children_by_room: Dict[str, List[JsonDict]] = {} + for ev in res.events: + if ev.event_type == EventTypes.SpaceChild: + children_by_room.setdefault(ev.room_id, []).append(ev.data) + + # Generate the final results. + results = [] + for fed_room in res.rooms: + fed_room_id = fed_room.get("room_id") + if not fed_room_id or not isinstance(fed_room_id, str): + continue + + results.append( + _RoomEntry( + fed_room_id, + fed_room, + children_by_room.get(fed_room_id, []), + ) + ) + + return results + + async def _get_child_events(self, room_id: str) -> Iterable[EventBase]: + """ + Get the child events for a given room. + + The returned results are sorted for stability. + + Args: + room_id: The room id to get the children of. + + Returns: + An iterable of sorted child events. + """ + + # look for child rooms/spaces. + current_state_ids = await self._store.get_current_state_ids(room_id) + + events = await self._store.get_events_as_list( + [ + event_id + for key, event_id in current_state_ids.items() + if key[0] == EventTypes.SpaceChild + ] + ) + + # filter out any events without a "via" (which implies it has been redacted), + # and order to ensure we return stable results. + return sorted(filter(_has_valid_via, events), key=_child_events_comparison_key) async def get_room_summary( self, @@ -91,7 +812,7 @@ async def _summarize_local_room( room_id: str, ) -> JsonDict: """ - Generate a room entry and a list of event entries for a given room. + Generate a room entry for a given room. Args: requester: @@ -103,7 +824,7 @@ async def _summarize_local_room( room_id: The room ID to summarize. Returns: - summary dict to return + room summary dict to return """ if not await self._auth.is_room_visible(room_id, requester, origin): raise NotFoundError("Room not found or is not accessible") @@ -116,7 +837,7 @@ async def _summarize_remote_room( remote_room_hosts: Optional[List[str]], ) -> JsonDict: """ - Request room entries and a list of event entries for a given room by querying a remote server. + Request room summary entry for a given room by querying a remote server. Args: room_id: The room to summarize. @@ -129,3 +850,81 @@ async def _summarize_remote_room( # TODO federation API, descoped from initial unstable implementation as MSC needs more maturing on that side. raise NotFoundError("Room not found or is not accessible") + + +@attr.s(frozen=True, slots=True, auto_attribs=True) +class _RoomQueueEntry: + room_id: str + via: Sequence[str] + depth: int = 0 + + +@attr.s(frozen=True, slots=True, auto_attribs=True) +class _RoomEntry: + room_id: str + # The room summary for this room. + room: JsonDict + # An iterable of the sorted, stripped children events for children of this room. + # + # This may not include all children. + children: Sequence[JsonDict] = () + + def as_json(self) -> JsonDict: + result = dict(self.room) + result["children_state"] = self.children + return result + + +def _has_valid_via(e: EventBase) -> bool: + via = e.content.get("via") + if not via or not isinstance(via, Sequence): + return False + for v in via: + if not isinstance(v, str): + logger.debug("Ignoring edge event %s with invalid via entry", e.event_id) + return False + return True + + +def _is_suggested_child_event(edge_event: EventBase) -> bool: + suggested = edge_event.content.get("suggested") + if isinstance(suggested, bool) and suggested: + return True + logger.debug("Ignorning not-suggested child %s", edge_event.state_key) + return False + + +# Order may only contain characters in the range of \x20 (space) to \x7E (~) inclusive. +_INVALID_ORDER_CHARS_RE = re.compile(r"[^\x20-\x7E]") + + +def _child_events_comparison_key(child: EventBase) -> Tuple[bool, Optional[str], str]: + """ + Generate a value for comparing two child events for ordering. + + The rules for ordering are supposed to be: + + 1. The 'order' key, if it is valid. + 2. The 'origin_server_ts' of the 'm.room.create' event. + 3. The 'room_id'. + + But we skip step 2 since we may not have any state from the room. + + Args: + child: The event for generating a comparison key. + + Returns: + The comparison key as a tuple of: + False if the ordering is valid. + The ordering field. + The room ID. + """ + order = child.content.get("order") + # If order is not a string or doesn't meet the requirements, ignore it. + if not isinstance(order, str): + order = None + elif len(order) > 50 or _INVALID_ORDER_CHARS_RE.search(order): + order = None + + # Items without an order come last. + return (order is None, order, child.room_id) diff --git a/synapse/handlers/space_summary.py b/synapse/handlers/space_summary.py deleted file mode 100644 index fd1f2b33707b..000000000000 --- a/synapse/handlers/space_summary.py +++ /dev/null @@ -1,837 +0,0 @@ -# Copyright 2021 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 itertools -import logging -import re -from collections import deque -from typing import ( - TYPE_CHECKING, - Deque, - Dict, - Iterable, - List, - Optional, - Sequence, - Set, - Tuple, -) - -import attr - -from synapse.api.constants import ( - EventContentFields, - EventTypes, - HistoryVisibility, - JoinRules, - RoomTypes, -) -from synapse.api.errors import AuthError, Codes, SynapseError -from synapse.events import EventBase -from synapse.events.utils import format_event_for_client_v2 -from synapse.types import JsonDict -from synapse.util.caches.response_cache import ResponseCache -from synapse.util.stringutils import random_string - -if TYPE_CHECKING: - from synapse.server import HomeServer - -logger = logging.getLogger(__name__) - -# number of rooms to return. We'll stop once we hit this limit. -MAX_ROOMS = 50 - -# max number of events to return per room. -MAX_ROOMS_PER_SPACE = 50 - -# max number of federation servers to hit per room -MAX_SERVERS_PER_SPACE = 3 - - -@attr.s(slots=True, frozen=True, auto_attribs=True) -class _PaginationKey: - """The key used to find unique pagination session.""" - - # The first three entries match the request parameters (and cannot change - # during a pagination session). - room_id: str - suggested_only: bool - max_depth: Optional[int] - # The randomly generated token. - token: str - - -@attr.s(slots=True, frozen=True, auto_attribs=True) -class _PaginationSession: - """The information that is stored for pagination.""" - - # The time the pagination session was created, in milliseconds. - creation_time_ms: int - # The queue of rooms which are still to process. - room_queue: Deque["_RoomQueueEntry"] - # A set of rooms which have been processed. - processed_rooms: Set[str] - - -class RoomSummaryMixin: - def __init__(self, hs: "HomeServer"): - self._store = hs.get_datastore() - self._auth = hs.get_auth() - self._event_auth_handler = hs.get_event_auth_handler() - - 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. - - Args: - room_id: The room ID to summarize. - for_federation: True if this is a summary requested over federation - (which includes additional fields). - - Returns: - The JSON dictionary for the room. - """ - stats = await self._store.get_room_with_stats(room_id) - - # currently this should be impossible because we call - # _is_local_room_accessible on the room before we get here, so - # there should always be an entry - assert stats is not None, "unable to retrieve stats for %s" % (room_id,) - - current_state_ids = await self._store.get_current_state_ids(room_id) - create_event = await self._store.get_event( - current_state_ids[(EventTypes.Create, "")] - ) - - entry = { - "room_id": stats["room_id"], - "name": stats["name"], - "topic": stats["topic"], - "canonical_alias": stats["canonical_alias"], - "num_joined_members": stats["joined_members"], - "avatar_url": stats["avatar"], - "join_rules": stats["join_rules"], - "world_readable": ( - stats["history_visibility"] == HistoryVisibility.WORLD_READABLE - ), - "guest_can_join": stats["guest_access"] == "can_join", - "creation_ts": create_event.origin_server_ts, - "room_type": create_event.content.get(EventContentFields.ROOM_TYPE), - } - - # Federation requests need to provide additional information so the - # requested server is able to filter the response appropriately. - if for_federation: - room_version = await self._store.get_room_version(room_id) - if await self._event_auth_handler.has_restricted_join_rules( - current_state_ids, room_version - ): - allowed_rooms = ( - await self._event_auth_handler.get_rooms_that_allow_join( - current_state_ids - ) - ) - if allowed_rooms: - entry["allowed_room_ids"] = allowed_rooms - # TODO Remove this key once the API is stable. - entry["allowed_spaces"] = allowed_rooms - - # Filter out Nones – rather omit the field altogether - room_entry = {k: v for k, v in entry.items() if v is not None} - - return room_entry - - async def _is_remote_room_accessible( - self, requester: Optional[str], room_id: str, room: JsonDict - ) -> bool: - """ - Calculate whether the room received over federation should be shown in the 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, if authenticated. - 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 requester and 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._auth.is_room_visible(room_id, requester) - - -class SpaceSummaryHandler(RoomSummaryMixin): - # The time a pagination session remains valid for. - _PAGINATION_SESSION_VALIDITY_PERIOD_MS = 5 * 60 * 1000 - - def __init__(self, hs: "HomeServer"): - super().__init__(hs) - self._clock = hs.get_clock() - self._event_auth_handler = hs.get_event_auth_handler() - self._store = hs.get_datastore() - self._event_serializer = hs.get_event_client_serializer() - self._server_name = hs.hostname - self._federation_client = hs.get_federation_client() - - # A map of query information to the current pagination state. - # - # TODO Allow for multiple workers to share this data. - # TODO Expire pagination tokens. - self._pagination_sessions: Dict[_PaginationKey, _PaginationSession] = {} - - # If a user tries to fetch the same page multiple times in quick succession, - # only process the first attempt and return its result to subsequent requests. - self._pagination_response_cache: ResponseCache[ - Tuple[str, bool, Optional[int], Optional[int], Optional[str]] - ] = ResponseCache( - hs.get_clock(), - "get_room_hierarchy", - ) - - def _expire_pagination_sessions(self): - """Expire pagination session which are old.""" - expire_before = ( - self._clock.time_msec() - self._PAGINATION_SESSION_VALIDITY_PERIOD_MS - ) - to_expire = [] - - for key, value in self._pagination_sessions.items(): - if value.creation_time_ms < expire_before: - to_expire.append(key) - - for key in to_expire: - logger.debug("Expiring pagination session id %s", key) - del self._pagination_sessions[key] - - async def get_space_summary( - self, - requester: str, - room_id: str, - suggested_only: bool = False, - max_rooms_per_space: Optional[int] = None, - ) -> JsonDict: - """ - Implementation of the space summary C-S API - - Args: - requester: user id of the user making this request - - room_id: room id to start the summary at - - suggested_only: whether we should only return children with the "suggested" - flag set. - - max_rooms_per_space: an optional limit on the number of child rooms we will - return. This does not apply to the root room (ie, room_id), and - is overridden by MAX_ROOMS_PER_SPACE. - - Returns: - summary dict to return - """ - # First of all, check that the room is accessible. - if not await self._auth.is_room_visible(room_id, requester): - raise AuthError( - 403, - "User %s not in room %s, and room previews are disabled" - % (requester, room_id), - ) - - # the queue of rooms to process - room_queue = deque((_RoomQueueEntry(room_id, ()),)) - - # rooms we have already processed - processed_rooms: Set[str] = set() - - # events we have already processed. We don't necessarily have their event ids, - # so instead we key on (room id, state key) - processed_events: Set[Tuple[str, str]] = set() - - rooms_result: List[JsonDict] = [] - events_result: List[JsonDict] = [] - - while room_queue and len(rooms_result) < MAX_ROOMS: - queue_entry = room_queue.popleft() - room_id = queue_entry.room_id - if room_id in processed_rooms: - # already done this room - continue - - logger.debug("Processing room %s", room_id) - - is_in_room = await self._store.is_host_joined(room_id, self._server_name) - - # The client-specified max_rooms_per_space limit doesn't apply to the - # room_id specified in the request, so we ignore it if this is the - # first room we are processing. - max_children = max_rooms_per_space if processed_rooms else None - - if is_in_room: - room_entry = await self._summarize_local_room( - requester, None, room_id, suggested_only, max_children - ) - - events: Sequence[JsonDict] = [] - if room_entry: - rooms_result.append(room_entry.room) - events = room_entry.children - - logger.debug( - "Query of local room %s returned events %s", - room_id, - ["%s->%s" % (ev["room_id"], ev["state_key"]) for ev in events], - ) - else: - fed_rooms = await self._summarize_remote_room( - queue_entry, - suggested_only, - max_children, - exclude_rooms=processed_rooms, - ) - - # The results over federation might include rooms that the we, - # as the requesting server, are allowed to see, but the requesting - # user is not permitted see. - # - # Filter the returned results to only what is accessible to the user. - events = [] - for room_entry in fed_rooms: - room = room_entry.room - fed_room_id = room_entry.room_id - - # The user can see the room, include it! - 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) - room.pop("allowed_spaces", None) - - rooms_result.append(room) - events.extend(room_entry.children) - - # All rooms returned don't need visiting again (even if the user - # didn't have access to them). - processed_rooms.add(fed_room_id) - - logger.debug( - "Query of %s returned rooms %s, events %s", - room_id, - [room_entry.room.get("room_id") for room_entry in fed_rooms], - ["%s->%s" % (ev["room_id"], ev["state_key"]) for ev in events], - ) - - # the room we queried may or may not have been returned, but don't process - # it again, anyway. - processed_rooms.add(room_id) - - # XXX: is it ok that we blindly iterate through any events returned by - # a remote server, whether or not they actually link to any rooms in our - # tree? - for ev in events: - # remote servers might return events we have already processed - # (eg, Dendrite returns inward pointers as well as outward ones), so - # we need to filter them out, to avoid returning duplicate links to the - # client. - ev_key = (ev["room_id"], ev["state_key"]) - if ev_key in processed_events: - continue - events_result.append(ev) - - # add the child to the queue. we have already validated - # that the vias are a list of server names. - room_queue.append( - _RoomQueueEntry(ev["state_key"], ev["content"]["via"]) - ) - processed_events.add(ev_key) - - return {"rooms": rooms_result, "events": events_result} - - async def get_room_hierarchy( - self, - requester: str, - requested_room_id: str, - suggested_only: bool = False, - max_depth: Optional[int] = None, - limit: Optional[int] = None, - from_token: Optional[str] = None, - ) -> JsonDict: - """ - Implementation of the room hierarchy C-S API. - - Args: - requester: The user ID of the user making this request. - requested_room_id: The room ID to start the hierarchy at (the "root" room). - suggested_only: Whether we should only return children with the "suggested" - flag set. - max_depth: The maximum depth in the tree to explore, must be a - non-negative integer. - - 0 would correspond to just the root room, 1 would include just - the root room's children, etc. - limit: An optional limit on the number of rooms to return per - page. Must be a positive integer. - from_token: An optional pagination token. - - Returns: - The JSON hierarchy dictionary. - """ - # If a user tries to fetch the same page multiple times in quick succession, - # only process the first attempt and return its result to subsequent requests. - # - # This is due to the pagination process mutating internal state, attempting - # to process multiple requests for the same page will result in errors. - return await self._pagination_response_cache.wrap( - (requested_room_id, suggested_only, max_depth, limit, from_token), - self._get_room_hierarchy, - requester, - requested_room_id, - suggested_only, - max_depth, - limit, - from_token, - ) - - async def _get_room_hierarchy( - self, - requester: str, - requested_room_id: str, - suggested_only: bool = False, - max_depth: Optional[int] = None, - limit: Optional[int] = None, - from_token: Optional[str] = None, - ) -> JsonDict: - """See docstring for SpaceSummaryHandler.get_room_hierarchy.""" - - # First of all, check that the room is accessible. - if not await self._auth.is_room_visible(requested_room_id, requester): - raise AuthError( - 403, - "User %s not in room %s, and room previews are disabled" - % (requester, requested_room_id), - ) - - # If this is continuing a previous session, pull the persisted data. - if from_token: - self._expire_pagination_sessions() - - pagination_key = _PaginationKey( - requested_room_id, suggested_only, max_depth, from_token - ) - if pagination_key not in self._pagination_sessions: - raise SynapseError(400, "Unknown pagination token", Codes.INVALID_PARAM) - - # Load the previous state. - pagination_session = self._pagination_sessions[pagination_key] - room_queue = pagination_session.room_queue - processed_rooms = pagination_session.processed_rooms - else: - # the queue of rooms to process - room_queue = deque((_RoomQueueEntry(requested_room_id, ()),)) - - # Rooms we have already processed. - processed_rooms = set() - - rooms_result: List[JsonDict] = [] - - # Cap the limit to a server-side maximum. - if limit is None: - limit = MAX_ROOMS - else: - limit = min(limit, MAX_ROOMS) - - # Iterate through the queue until we reach the limit or run out of - # rooms to include. - while room_queue and len(rooms_result) < limit: - queue_entry = room_queue.popleft() - room_id = queue_entry.room_id - current_depth = queue_entry.depth - if room_id in processed_rooms: - # already done this room - continue - - logger.debug("Processing room %s", room_id) - - is_in_room = await self._store.is_host_joined(room_id, self._server_name) - if is_in_room: - room_entry = await self._summarize_local_room( - requester, - None, - room_id, - suggested_only, - # TODO Handle max children. - max_children=None, - ) - - if room_entry: - rooms_result.append(room_entry.as_json()) - - # Add the child to the queue. We have already validated - # that the vias are a list of server names. - # - # If the current depth is the maximum depth, do not queue - # more entries. - if max_depth is None or current_depth < max_depth: - room_queue.extendleft( - _RoomQueueEntry( - ev["state_key"], ev["content"]["via"], current_depth + 1 - ) - for ev in reversed(room_entry.children) - ) - - processed_rooms.add(room_id) - else: - # TODO Federation. - pass - - result: JsonDict = {"rooms": rooms_result} - - # If there's additional data, generate a pagination token (and persist state). - if room_queue: - next_batch = random_string(24) - result["next_batch"] = next_batch - pagination_key = _PaginationKey( - requested_room_id, suggested_only, max_depth, next_batch - ) - self._pagination_sessions[pagination_key] = _PaginationSession( - self._clock.time_msec(), room_queue, processed_rooms - ) - - return result - - async def federation_space_summary( - self, - origin: str, - room_id: str, - suggested_only: bool, - max_rooms_per_space: Optional[int], - exclude_rooms: Iterable[str], - ) -> JsonDict: - """ - Implementation of the space summary Federation API - - Args: - origin: The server requesting the spaces summary. - - room_id: room id to start the summary at - - suggested_only: whether we should only return children with the "suggested" - flag set. - - max_rooms_per_space: an optional limit on the number of child rooms we will - return. Unlike the C-S API, this applies to the root room (room_id). - It is clipped to MAX_ROOMS_PER_SPACE. - - exclude_rooms: a list of rooms to skip over (presumably because the - calling server has already seen them). - - Returns: - summary dict to return - """ - # the queue of rooms to process - room_queue = deque((room_id,)) - - # the set of rooms that we should not walk further. Initialise it with the - # excluded-rooms list; we will add other rooms as we process them so that - # we do not loop. - processed_rooms: Set[str] = set(exclude_rooms) - - rooms_result: List[JsonDict] = [] - events_result: List[JsonDict] = [] - - while room_queue and len(rooms_result) < MAX_ROOMS: - room_id = room_queue.popleft() - if room_id in processed_rooms: - # already done this room - continue - - room_entry = await self._summarize_local_room( - None, origin, room_id, suggested_only, max_rooms_per_space - ) - - processed_rooms.add(room_id) - - if room_entry: - rooms_result.append(room_entry.room) - events_result.extend(room_entry.children) - - # add any children to the queue - room_queue.extend( - edge_event["state_key"] for edge_event in room_entry.children - ) - - return {"rooms": rooms_result, "events": events_result} - - async def _summarize_local_room( - self, - requester: Optional[str], - origin: Optional[str], - room_id: str, - suggested_only: bool, - max_children: Optional[int], - ) -> Optional["_RoomEntry"]: - """ - Generate a room entry and a list of event entries for a given room. - - Args: - requester: - The user requesting the summary, if it is a local request. None - if this is a federation request. - origin: - The server requesting the summary, if it is a federation request. - None if this is a local request. - room_id: The room ID to summarize. - suggested_only: True if only suggested children should be returned. - Otherwise, all children are returned. - max_children: - The maximum number of children rooms to include. This is capped - to a server-set limit. - - Returns: - A room entry if the room should be returned. None, otherwise. - """ - if not await self._auth.is_room_visible(room_id, requester, origin): - return None - - room_entry = await self._build_room_entry(room_id, for_federation=bool(origin)) - - # If the room is not a space, return just the room information. - if room_entry.get("room_type") != RoomTypes.SPACE: - return _RoomEntry(room_id, room_entry) - - # Otherwise, look for child rooms/spaces. - child_events = await self._get_child_events(room_id) - - if suggested_only: - # we only care about suggested children - child_events = filter(_is_suggested_child_event, child_events) - - if max_children is None or max_children > MAX_ROOMS_PER_SPACE: - max_children = MAX_ROOMS_PER_SPACE - - now = self._clock.time_msec() - events_result: List[JsonDict] = [] - for edge_event in itertools.islice(child_events, max_children): - events_result.append( - await self._event_serializer.serialize_event( - edge_event, - time_now=now, - event_format=format_event_for_client_v2, - ) - ) - - return _RoomEntry(room_id, room_entry, events_result) - - async def _summarize_remote_room( - self, - room: "_RoomQueueEntry", - suggested_only: bool, - max_children: Optional[int], - exclude_rooms: Iterable[str], - ) -> Iterable["_RoomEntry"]: - """ - Request room entries and a list of event entries for a given room by querying a remote server. - - Args: - room: The room to summarize. - suggested_only: True if only suggested children should be returned. - Otherwise, all children are returned. - max_children: - The maximum number of children rooms to include. This is capped - to a server-set limit. - exclude_rooms: - Rooms IDs which do not need to be summarized. - - Returns: - An iterable of room entries. - """ - room_id = room.room_id - logger.info("Requesting summary for %s via %s", room_id, room.via) - - # we need to make the exclusion list json-serialisable - exclude_rooms = list(exclude_rooms) - - via = itertools.islice(room.via, MAX_SERVERS_PER_SPACE) - try: - res = await self._federation_client.get_space_summary( - via, - room_id, - suggested_only=suggested_only, - max_rooms_per_space=max_children, - exclude_rooms=exclude_rooms, - ) - except Exception as e: - logger.warning( - "Unable to get summary of %s via federation: %s", - room_id, - e, - exc_info=logger.isEnabledFor(logging.DEBUG), - ) - return () - - # Group the events by their room. - children_by_room: Dict[str, List[JsonDict]] = {} - for ev in res.events: - if ev.event_type == EventTypes.SpaceChild: - children_by_room.setdefault(ev.room_id, []).append(ev.data) - - # Generate the final results. - results = [] - for fed_room in res.rooms: - fed_room_id = fed_room.get("room_id") - if not fed_room_id or not isinstance(fed_room_id, str): - continue - - results.append( - _RoomEntry( - fed_room_id, - fed_room, - children_by_room.get(fed_room_id, []), - ) - ) - - return results - - async def _get_child_events(self, room_id: str) -> Iterable[EventBase]: - """ - Get the child events for a given room. - - The returned results are sorted for stability. - - Args: - room_id: The room id to get the children of. - - Returns: - An iterable of sorted child events. - """ - - # look for child rooms/spaces. - current_state_ids = await self._store.get_current_state_ids(room_id) - - events = await self._store.get_events_as_list( - [ - event_id - for key, event_id in current_state_ids.items() - if key[0] == EventTypes.SpaceChild - ] - ) - - # filter out any events without a "via" (which implies it has been redacted), - # and order to ensure we return stable results. - return sorted(filter(_has_valid_via, events), key=_child_events_comparison_key) - - -@attr.s(frozen=True, slots=True, auto_attribs=True) -class _RoomQueueEntry: - room_id: str - via: Sequence[str] - depth: int = 0 - - -@attr.s(frozen=True, slots=True, auto_attribs=True) -class _RoomEntry: - room_id: str - # The room summary for this room. - room: JsonDict - # An iterable of the sorted, stripped children events for children of this room. - # - # This may not include all children. - children: Sequence[JsonDict] = () - - def as_json(self) -> JsonDict: - result = dict(self.room) - result["children_state"] = self.children - return result - - -def _has_valid_via(e: EventBase) -> bool: - via = e.content.get("via") - if not via or not isinstance(via, Sequence): - return False - for v in via: - if not isinstance(v, str): - logger.debug("Ignoring edge event %s with invalid via entry", e.event_id) - return False - return True - - -def _is_suggested_child_event(edge_event: EventBase) -> bool: - suggested = edge_event.content.get("suggested") - if isinstance(suggested, bool) and suggested: - return True - logger.debug("Ignorning not-suggested child %s", edge_event.state_key) - return False - - -# Order may only contain characters in the range of \x20 (space) to \x7E (~) inclusive. -_INVALID_ORDER_CHARS_RE = re.compile(r"[^\x20-\x7E]") - - -def _child_events_comparison_key(child: EventBase) -> Tuple[bool, Optional[str], str]: - """ - Generate a value for comparing two child events for ordering. - - The rules for ordering are supposed to be: - - 1. The 'order' key, if it is valid. - 2. The 'origin_server_ts' of the 'm.room.create' event. - 3. The 'room_id'. - - But we skip step 2 since we may not have any state from the room. - - Args: - child: The event for generating a comparison key. - - Returns: - The comparison key as a tuple of: - False if the ordering is valid. - The ordering field. - The room ID. - """ - order = child.content.get("order") - # If order is not a string or doesn't meet the requirements, ignore it. - if not isinstance(order, str): - order = None - elif len(order) > 50 or _INVALID_ORDER_CHARS_RE.search(order): - order = None - - # Items without an order come last. - return (order is None, order, child.room_id) diff --git a/synapse/rest/client/v1/room.py b/synapse/rest/client/v1/room.py index 77e0f0fc6e48..740758eb2653 100644 --- a/synapse/rest/client/v1/room.py +++ b/synapse/rest/client/v1/room.py @@ -1395,14 +1395,14 @@ class RoomSpaceSummaryRestServlet(RestServlet): def __init__(self, hs: "HomeServer"): super().__init__() self._auth = hs.get_auth() - self._space_summary_handler = hs.get_space_summary_handler() + self._room_summary_handler = hs.get_room_summary_handler() async def on_GET( self, request: SynapseRequest, room_id: str ) -> Tuple[int, JsonDict]: requester = await self._auth.get_user_by_req(request, allow_guest=True) - return 200, await self._space_summary_handler.get_space_summary( + return 200, await self._room_summary_handler.get_space_summary( requester.user.to_string(), room_id, suggested_only=parse_boolean(request, "suggested_only", default=False), @@ -1428,7 +1428,7 @@ async def on_POST( 400, "'max_rooms_per_space' must be an integer", Codes.BAD_JSON ) - return 200, await self._space_summary_handler.get_space_summary( + return 200, await self._room_summary_handler.get_space_summary( requester.user.to_string(), room_id, suggested_only=suggested_only, @@ -1447,7 +1447,7 @@ class RoomHierarchyRestServlet(RestServlet): def __init__(self, hs: "HomeServer"): super().__init__() self._auth = hs.get_auth() - self._space_summary_handler = hs.get_space_summary_handler() + self._room_summary_handler = hs.get_room_summary_handler() async def on_GET( self, request: SynapseRequest, room_id: str @@ -1466,7 +1466,7 @@ async def on_GET( 400, "'limit' must be a positive integer", Codes.BAD_JSON ) - return 200, await self._space_summary_handler.get_room_hierarchy( + return 200, await self._room_summary_handler.get_room_hierarchy( requester.user.to_string(), room_id, suggested_only=parse_boolean(request, "suggested_only", default=False), @@ -1496,6 +1496,7 @@ async def on_GET( requester = await self._auth.get_user_by_req(request, allow_guest=True) requester_user_id: Optional[str] = requester.user.to_string() except MissingClientTokenError: + # auth is optional requester_user_id = None # twisted.web.server.Request.args is incorrectly defined as Optional[Any] diff --git a/synapse/server.py b/synapse/server.py index df77ea0b6fbc..de6517663e6b 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -103,7 +103,6 @@ from synapse.handlers.search import SearchHandler from synapse.handlers.send_email import SendEmailHandler from synapse.handlers.set_password import SetPasswordHandler -from synapse.handlers.space_summary import SpaceSummaryHandler from synapse.handlers.sso import SsoHandler from synapse.handlers.stats import StatsHandler from synapse.handlers.sync import SyncHandler @@ -772,10 +771,6 @@ def get_module_api(self) -> ModuleApi: def get_account_data_handler(self) -> AccountDataHandler: return AccountDataHandler(self) - @cache_in_self - def get_space_summary_handler(self) -> SpaceSummaryHandler: - return SpaceSummaryHandler(self) - @cache_in_self def get_room_summary_handler(self) -> RoomSummaryHandler: return RoomSummaryHandler(self) diff --git a/tests/handlers/test_room_summary.py b/tests/handlers/test_room_summary.py index ee0569892d23..29f6866580f3 100644 --- a/tests/handlers/test_room_summary.py +++ b/tests/handlers/test_room_summary.py @@ -11,16 +11,825 @@ # 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. -from synapse.api.constants import EventTypes, HistoryVisibility, JoinRules -from synapse.api.errors import NotFoundError +from typing import Any, Iterable, List, Optional, Tuple +from unittest import mock + +from synapse.api.constants import ( + EventContentFields, + EventTypes, + HistoryVisibility, + JoinRules, + Membership, + RestrictedJoinRuleTypes, + RoomTypes, +) +from synapse.api.errors import AuthError, NotFoundError, SynapseError +from synapse.api.room_versions import RoomVersions +from synapse.events import make_event_from_dict +from synapse.handlers.room_summary import _child_events_comparison_key, _RoomEntry from synapse.rest import admin from synapse.rest.client.v1 import login, room from synapse.server import HomeServer +from synapse.types import JsonDict, UserID from tests import unittest -class SpaceSummaryTestCase(unittest.HomeserverTestCase): +def _create_event(room_id: str, order: Optional[Any] = None): + result = mock.Mock() + result.room_id = room_id + result.content = {} + if order is not None: + result.content["order"] = order + return result + + +def _order(*events): + return sorted(events, key=_child_events_comparison_key) + + +class TestHierarchySort(unittest.TestCase): + def test_no_order_last(self): + """An event with no ordering is placed behind those with an ordering.""" + ev1 = _create_event("!abc:test") + ev2 = _create_event("!xyz:test", "xyz") + + self.assertEqual([ev2, ev1], _order(ev1, ev2)) + + def test_order(self): + """The ordering should be used.""" + ev1 = _create_event("!abc:test", "xyz") + ev2 = _create_event("!xyz:test", "abc") + + self.assertEqual([ev2, ev1], _order(ev1, ev2)) + + def test_order_room_id(self): + """Room ID is a tie-breaker for ordering.""" + ev1 = _create_event("!abc:test", "abc") + ev2 = _create_event("!xyz:test", "abc") + + self.assertEqual([ev1, ev2], _order(ev1, ev2)) + + def test_invalid_ordering_type(self): + """Invalid orderings are considered the same as missing.""" + ev1 = _create_event("!abc:test", 1) + ev2 = _create_event("!xyz:test", "xyz") + + self.assertEqual([ev2, ev1], _order(ev1, ev2)) + + ev1 = _create_event("!abc:test", {}) + self.assertEqual([ev2, ev1], _order(ev1, ev2)) + + ev1 = _create_event("!abc:test", []) + self.assertEqual([ev2, ev1], _order(ev1, ev2)) + + ev1 = _create_event("!abc:test", True) + self.assertEqual([ev2, ev1], _order(ev1, ev2)) + + def test_invalid_ordering_value(self): + """Invalid orderings are considered the same as missing.""" + ev1 = _create_event("!abc:test", "foo\n") + ev2 = _create_event("!xyz:test", "xyz") + + self.assertEqual([ev2, ev1], _order(ev1, ev2)) + + ev1 = _create_event("!abc:test", "a" * 51) + self.assertEqual([ev2, ev1], _order(ev1, ev2)) + + +class RoomHierarchyTestCase(unittest.HomeserverTestCase): + servlets = [ + admin.register_servlets_for_client_rest_resource, + room.register_servlets, + login.register_servlets, + ] + + def prepare(self, reactor, clock, hs: HomeServer): + self.hs = hs + self.handler = self.hs.get_room_summary_handler() + + # Create a user. + self.user = self.register_user("user", "pass") + self.token = self.login("user", "pass") + + # Create a space and a child room. + self.space = self.helper.create_room_as( + self.user, + tok=self.token, + extra_content={ + "creation_content": {EventContentFields.ROOM_TYPE: RoomTypes.SPACE} + }, + ) + self.room = self.helper.create_room_as(self.user, tok=self.token) + self._add_child(self.space, self.room, self.token) + + def _add_child( + self, space_id: str, room_id: str, token: str, order: Optional[str] = None + ) -> None: + """Add a child room to a space.""" + content: JsonDict = {"via": [self.hs.hostname]} + if order is not None: + content["order"] = order + self.helper.send_state( + space_id, + event_type=EventTypes.SpaceChild, + body=content, + tok=token, + state_key=room_id, + ) + + def _assert_rooms( + self, result: JsonDict, rooms_and_children: Iterable[Tuple[str, Iterable[str]]] + ) -> None: + """ + Assert that the expected room IDs and events are in the response. + + Args: + result: The result from the API call. + rooms_and_children: An iterable of tuples where each tuple is: + The expected room ID. + The expected IDs of any children rooms. + """ + room_ids = [] + children_ids = [] + for room_id, children in rooms_and_children: + room_ids.append(room_id) + if children: + children_ids.extend([(room_id, child_id) for child_id in children]) + self.assertCountEqual( + [room.get("room_id") for room in result["rooms"]], room_ids + ) + self.assertCountEqual( + [ + (event.get("room_id"), event.get("state_key")) + for event in result["events"] + ], + children_ids, + ) + + def _assert_hierarchy( + self, result: JsonDict, rooms_and_children: Iterable[Tuple[str, Iterable[str]]] + ) -> None: + """ + Assert that the expected room IDs are in the response. + + Args: + result: The result from the API call. + rooms_and_children: An iterable of tuples where each tuple is: + The expected room ID. + The expected IDs of any children rooms. + """ + result_room_ids = [] + result_children_ids = [] + for result_room in result["rooms"]: + result_room_ids.append(result_room["room_id"]) + result_children_ids.append( + [ + (cs["room_id"], cs["state_key"]) + for cs in result_room.get("children_state") + ] + ) + + room_ids = [] + children_ids = [] + for room_id, children in rooms_and_children: + room_ids.append(room_id) + children_ids.append([(room_id, child_id) for child_id in children]) + + # Note that order matters. + self.assertEqual(result_room_ids, room_ids) + self.assertEqual(result_children_ids, children_ids) + + 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)) + # The result should have the space and the room in it, along with a link + # from space -> room. + expected = [(self.space, [self.room]), (self.room, ())] + self._assert_rooms(result, expected) + + result = self.get_success( + self.handler.get_room_hierarchy(self.user, self.space) + ) + self._assert_hierarchy(result, expected) + + def test_visibility(self): + """A user not in a space cannot inspect it.""" + user2 = self.register_user("user2", "pass") + token2 = self.login("user2", "pass") + + # The user can see the space since it is publicly joinable. + result = self.get_success(self.handler.get_space_summary(user2, self.space)) + expected = [(self.space, [self.room]), (self.room, ())] + self._assert_rooms(result, expected) + + result = self.get_success(self.handler.get_room_hierarchy(user2, self.space)) + self._assert_hierarchy(result, expected) + + # If the space is made invite-only, it should no longer be viewable. + self.helper.send_state( + self.space, + event_type=EventTypes.JoinRules, + body={"join_rule": JoinRules.INVITE}, + tok=self.token, + ) + self.get_failure(self.handler.get_space_summary(user2, self.space), AuthError) + self.get_failure(self.handler.get_room_hierarchy(user2, self.space), AuthError) + + # If the space is made world-readable it should return a result. + self.helper.send_state( + self.space, + event_type=EventTypes.RoomHistoryVisibility, + body={"history_visibility": HistoryVisibility.WORLD_READABLE}, + tok=self.token, + ) + result = self.get_success(self.handler.get_space_summary(user2, self.space)) + self._assert_rooms(result, expected) + + result = self.get_success(self.handler.get_room_hierarchy(user2, self.space)) + self._assert_hierarchy(result, expected) + + # Make it not world-readable again and confirm it results in an error. + self.helper.send_state( + self.space, + event_type=EventTypes.RoomHistoryVisibility, + body={"history_visibility": HistoryVisibility.JOINED}, + tok=self.token, + ) + self.get_failure(self.handler.get_space_summary(user2, self.space), AuthError) + self.get_failure(self.handler.get_room_hierarchy(user2, self.space), AuthError) + + # Join the space and results should be returned. + self.helper.invite(self.space, targ=user2, tok=self.token) + self.helper.join(self.space, user2, tok=token2) + result = self.get_success(self.handler.get_space_summary(user2, self.space)) + self._assert_rooms(result, expected) + + result = self.get_success(self.handler.get_room_hierarchy(user2, self.space)) + self._assert_hierarchy(result, expected) + + # Attempting to view an unknown room returns the same error. + self.get_failure( + self.handler.get_space_summary(user2, "#not-a-space:" + self.hs.hostname), + AuthError, + ) + self.get_failure( + self.handler.get_room_hierarchy(user2, "#not-a-space:" + self.hs.hostname), + AuthError, + ) + + def _create_room_with_join_rule( + self, join_rule: str, room_version: Optional[str] = None, **extra_content + ) -> str: + """Create a room with the given join rule and add it to the space.""" + room_id = self.helper.create_room_as( + self.user, + room_version=room_version, + tok=self.token, + extra_content={ + "initial_state": [ + { + "type": EventTypes.JoinRules, + "state_key": "", + "content": { + "join_rule": join_rule, + **extra_content, + }, + } + ] + }, + ) + self._add_child(self.space, room_id, self.token) + return room_id + + def test_filtering(self): + """ + Rooms should be properly filtered to only include rooms the user has access to. + """ + user2 = self.register_user("user2", "pass") + token2 = self.login("user2", "pass") + + # Create a few rooms which will have different properties. + public_room = self._create_room_with_join_rule(JoinRules.PUBLIC) + knock_room = self._create_room_with_join_rule( + JoinRules.KNOCK, room_version=RoomVersions.V7.identifier + ) + not_invited_room = self._create_room_with_join_rule(JoinRules.INVITE) + invited_room = self._create_room_with_join_rule(JoinRules.INVITE) + self.helper.invite(invited_room, targ=user2, tok=self.token) + restricted_room = self._create_room_with_join_rule( + JoinRules.RESTRICTED, + room_version=RoomVersions.V8.identifier, + allow=[], + ) + restricted_accessible_room = self._create_room_with_join_rule( + JoinRules.RESTRICTED, + room_version=RoomVersions.V8.identifier, + allow=[ + { + "type": RestrictedJoinRuleTypes.ROOM_MEMBERSHIP, + "room_id": self.space, + "via": [self.hs.hostname], + } + ], + ) + world_readable_room = self._create_room_with_join_rule(JoinRules.INVITE) + self.helper.send_state( + world_readable_room, + event_type=EventTypes.RoomHistoryVisibility, + body={"history_visibility": HistoryVisibility.WORLD_READABLE}, + tok=self.token, + ) + joined_room = self._create_room_with_join_rule(JoinRules.INVITE) + self.helper.invite(joined_room, targ=user2, tok=self.token) + self.helper.join(joined_room, user2, tok=token2) + + # Join the space. + self.helper.join(self.space, user2, tok=token2) + result = self.get_success(self.handler.get_space_summary(user2, self.space)) + expected = [ + ( + self.space, + [ + self.room, + public_room, + knock_room, + not_invited_room, + invited_room, + restricted_room, + restricted_accessible_room, + world_readable_room, + joined_room, + ], + ), + (self.room, ()), + (public_room, ()), + (knock_room, ()), + (invited_room, ()), + (restricted_accessible_room, ()), + (world_readable_room, ()), + (joined_room, ()), + ] + self._assert_rooms(result, expected) + + result = self.get_success(self.handler.get_room_hierarchy(user2, self.space)) + self._assert_hierarchy(result, expected) + + def test_complex_space(self): + """ + Create a "complex" space to see how it handles things like loops and subspaces. + """ + # Create an inaccessible room. + user2 = self.register_user("user2", "pass") + token2 = self.login("user2", "pass") + room2 = self.helper.create_room_as(user2, is_public=False, tok=token2) + # This is a bit odd as "user" is adding a room they don't know about, but + # it works for the tests. + self._add_child(self.space, room2, self.token) + + # Create a subspace under the space with an additional room in it. + subspace = self.helper.create_room_as( + self.user, + tok=self.token, + extra_content={ + "creation_content": {EventContentFields.ROOM_TYPE: RoomTypes.SPACE} + }, + ) + subroom = self.helper.create_room_as(self.user, tok=self.token) + self._add_child(self.space, subspace, token=self.token) + self._add_child(subspace, subroom, token=self.token) + # Also add the two rooms from the space into this subspace (causing loops). + self._add_child(subspace, self.room, token=self.token) + self._add_child(subspace, room2, self.token) + + result = self.get_success(self.handler.get_space_summary(self.user, self.space)) + + # The result should include each room a single time and each link. + expected = [ + (self.space, [self.room, room2, subspace]), + (self.room, ()), + (subspace, [subroom, self.room, room2]), + (subroom, ()), + ] + self._assert_rooms(result, expected) + + result = self.get_success( + self.handler.get_room_hierarchy(self.user, self.space) + ) + self._assert_hierarchy(result, expected) + + def test_pagination(self): + """Test simple pagination works.""" + room_ids = [] + for i in range(1, 10): + room = self.helper.create_room_as(self.user, tok=self.token) + self._add_child(self.space, room, self.token, order=str(i)) + room_ids.append(room) + # The room created initially doesn't have an order, so comes last. + room_ids.append(self.room) + + result = self.get_success( + self.handler.get_room_hierarchy(self.user, self.space, limit=7) + ) + # The result should have the space and all of the links, plus some of the + # rooms and a pagination token. + expected: List[Tuple[str, Iterable[str]]] = [(self.space, room_ids)] + expected += [(room_id, ()) for room_id in room_ids[:6]] + self._assert_hierarchy(result, expected) + self.assertIn("next_batch", result) + + # Check the next page. + result = self.get_success( + self.handler.get_room_hierarchy( + self.user, self.space, limit=5, from_token=result["next_batch"] + ) + ) + # The result should have the space and the room in it, along with a link + # from space -> room. + expected = [(room_id, ()) for room_id in room_ids[6:]] + self._assert_hierarchy(result, expected) + self.assertNotIn("next_batch", result) + + def test_invalid_pagination_token(self): + """""" + room_ids = [] + for i in range(1, 10): + room = self.helper.create_room_as(self.user, tok=self.token) + self._add_child(self.space, room, self.token, order=str(i)) + room_ids.append(room) + # The room created initially doesn't have an order, so comes last. + room_ids.append(self.room) + + result = self.get_success( + self.handler.get_room_hierarchy(self.user, self.space, limit=7) + ) + self.assertIn("next_batch", result) + + # Changing the room ID, suggested-only, or max-depth causes an error. + self.get_failure( + self.handler.get_room_hierarchy( + self.user, self.room, from_token=result["next_batch"] + ), + SynapseError, + ) + self.get_failure( + self.handler.get_room_hierarchy( + self.user, + self.space, + suggested_only=True, + from_token=result["next_batch"], + ), + SynapseError, + ) + self.get_failure( + self.handler.get_room_hierarchy( + self.user, self.space, max_depth=0, from_token=result["next_batch"] + ), + SynapseError, + ) + + # An invalid token is ignored. + self.get_failure( + self.handler.get_room_hierarchy(self.user, self.space, from_token="foo"), + SynapseError, + ) + + def test_max_depth(self): + """Create a deep tree to test the max depth against.""" + spaces = [self.space] + rooms = [self.room] + for _ in range(5): + spaces.append( + self.helper.create_room_as( + self.user, + tok=self.token, + extra_content={ + "creation_content": { + EventContentFields.ROOM_TYPE: RoomTypes.SPACE + } + }, + ) + ) + self._add_child(spaces[-2], spaces[-1], self.token) + rooms.append(self.helper.create_room_as(self.user, tok=self.token)) + self._add_child(spaces[-1], rooms[-1], self.token) + + # Test just the space itself. + result = self.get_success( + self.handler.get_room_hierarchy(self.user, self.space, max_depth=0) + ) + expected: List[Tuple[str, Iterable[str]]] = [(spaces[0], [rooms[0], spaces[1]])] + self._assert_hierarchy(result, expected) + + # A single additional layer. + result = self.get_success( + self.handler.get_room_hierarchy(self.user, self.space, max_depth=1) + ) + expected += [ + (rooms[0], ()), + (spaces[1], [rooms[1], spaces[2]]), + ] + self._assert_hierarchy(result, expected) + + # A few layers. + result = self.get_success( + self.handler.get_room_hierarchy(self.user, self.space, max_depth=3) + ) + expected += [ + (rooms[1], ()), + (spaces[2], [rooms[2], spaces[3]]), + (rooms[2], ()), + (spaces[3], [rooms[3], spaces[4]]), + ] + self._assert_hierarchy(result, expected) + + def test_fed_complex(self): + """ + Return data over federation and ensure that it is handled properly. + """ + fed_hostname = self.hs.hostname + "2" + subspace = "#subspace:" + fed_hostname + subroom = "#subroom:" + fed_hostname + + async def summarize_remote_room( + _self, room, suggested_only, max_children, exclude_rooms + ): + # Return some good data, and some bad data: + # + # * Event *back* to the root room. + # * Unrelated events / rooms + # * Multiple levels of events (in a not-useful order, e.g. grandchild + # events before child events). + + # Note that these entries are brief, but should contain enough info. + return [ + _RoomEntry( + subspace, + { + "room_id": subspace, + "world_readable": True, + "room_type": RoomTypes.SPACE, + }, + [ + { + "room_id": subspace, + "state_key": subroom, + "content": {"via": [fed_hostname]}, + } + ], + ), + _RoomEntry( + subroom, + { + "room_id": subroom, + "world_readable": True, + }, + ), + ] + + # Add a room to the space which is on another server. + self._add_child(self.space, subspace, 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)) + + expected = [ + (self.space, [self.room, subspace]), + (self.room, ()), + (subspace, [subroom]), + (subroom, ()), + ] + self._assert_rooms(result, expected) + + def test_fed_filtering(self): + """ + Rooms returned over federation should be properly filtered to only include + rooms the user has access to. + """ + fed_hostname = self.hs.hostname + "2" + subspace = "#subspace:" + fed_hostname + + # Create a few rooms which will have different properties. + public_room = "#public:" + fed_hostname + knock_room = "#knock:" + fed_hostname + not_invited_room = "#not_invited:" + fed_hostname + invited_room = "#invited:" + fed_hostname + restricted_room = "#restricted:" + fed_hostname + restricted_accessible_room = "#restricted_accessible:" + fed_hostname + world_readable_room = "#world_readable:" + fed_hostname + joined_room = self.helper.create_room_as(self.user, tok=self.token) + + # Poke an invite over federation into the database. + self._poke_fed_invite(invited_room, "@remote:" + fed_hostname) + + async def summarize_remote_room( + _self, room, suggested_only, max_children, exclude_rooms + ): + # Note that these entries are brief, but should contain enough info. + rooms = [ + _RoomEntry( + public_room, + { + "room_id": public_room, + "world_readable": False, + "join_rules": JoinRules.PUBLIC, + }, + ), + _RoomEntry( + knock_room, + { + "room_id": knock_room, + "world_readable": False, + "join_rules": JoinRules.KNOCK, + }, + ), + _RoomEntry( + not_invited_room, + { + "room_id": not_invited_room, + "world_readable": False, + "join_rules": JoinRules.INVITE, + }, + ), + _RoomEntry( + invited_room, + { + "room_id": invited_room, + "world_readable": False, + "join_rules": JoinRules.INVITE, + }, + ), + _RoomEntry( + restricted_room, + { + "room_id": restricted_room, + "world_readable": False, + "join_rules": JoinRules.RESTRICTED, + "allowed_spaces": [], + }, + ), + _RoomEntry( + restricted_accessible_room, + { + "room_id": restricted_accessible_room, + "world_readable": False, + "join_rules": JoinRules.RESTRICTED, + "allowed_spaces": [self.room], + }, + ), + _RoomEntry( + world_readable_room, + { + "room_id": world_readable_room, + "world_readable": True, + "join_rules": JoinRules.INVITE, + }, + ), + _RoomEntry( + joined_room, + { + "room_id": joined_room, + "world_readable": False, + "join_rules": JoinRules.INVITE, + }, + ), + ] + + # Also include the subspace. + rooms.insert( + 0, + _RoomEntry( + subspace, + { + "room_id": subspace, + "world_readable": True, + }, + # Place each room in the sub-space. + [ + { + "room_id": subspace, + "state_key": room.room_id, + "content": {"via": [fed_hostname]}, + } + for room in rooms + ], + ), + ) + return rooms + + # Add a room to the space which is on another server. + self._add_child(self.space, subspace, 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)) + + expected = [ + (self.space, [self.room, subspace]), + (self.room, ()), + ( + subspace, + [ + public_room, + knock_room, + not_invited_room, + invited_room, + restricted_room, + restricted_accessible_room, + world_readable_room, + joined_room, + ], + ), + (public_room, ()), + (knock_room, ()), + (invited_room, ()), + (restricted_accessible_room, ()), + (world_readable_room, ()), + (joined_room, ()), + ] + self._assert_rooms(result, expected) + + 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)) + + expected = [ + (self.space, [self.room, fed_room]), + (self.room, ()), + (fed_room, ()), + ] + self._assert_rooms(result, expected) + + +class RoomSummaryTestCase(unittest.HomeserverTestCase): servlets = [ admin.register_servlets_for_client_rest_resource, room.register_servlets, diff --git a/tests/handlers/test_space_summary.py b/tests/handlers/test_space_summary.py deleted file mode 100644 index 83c2bdd8f99c..000000000000 --- a/tests/handlers/test_space_summary.py +++ /dev/null @@ -1,835 +0,0 @@ -# Copyright 2021 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. -from typing import Any, Iterable, List, Optional, Tuple -from unittest import mock - -from synapse.api.constants import ( - EventContentFields, - EventTypes, - HistoryVisibility, - JoinRules, - Membership, - RestrictedJoinRuleTypes, - RoomTypes, -) -from synapse.api.errors import AuthError, SynapseError -from synapse.api.room_versions import RoomVersions -from synapse.events import make_event_from_dict -from synapse.handlers.space_summary import _child_events_comparison_key, _RoomEntry -from synapse.rest import admin -from synapse.rest.client.v1 import login, room -from synapse.server import HomeServer -from synapse.types import JsonDict, UserID - -from tests import unittest - - -def _create_event(room_id: str, order: Optional[Any] = None): - result = mock.Mock() - result.room_id = room_id - result.content = {} - if order is not None: - result.content["order"] = order - return result - - -def _order(*events): - return sorted(events, key=_child_events_comparison_key) - - -class TestSpaceSummarySort(unittest.TestCase): - def test_no_order_last(self): - """An event with no ordering is placed behind those with an ordering.""" - ev1 = _create_event("!abc:test") - ev2 = _create_event("!xyz:test", "xyz") - - self.assertEqual([ev2, ev1], _order(ev1, ev2)) - - def test_order(self): - """The ordering should be used.""" - ev1 = _create_event("!abc:test", "xyz") - ev2 = _create_event("!xyz:test", "abc") - - self.assertEqual([ev2, ev1], _order(ev1, ev2)) - - def test_order_room_id(self): - """Room ID is a tie-breaker for ordering.""" - ev1 = _create_event("!abc:test", "abc") - ev2 = _create_event("!xyz:test", "abc") - - self.assertEqual([ev1, ev2], _order(ev1, ev2)) - - def test_invalid_ordering_type(self): - """Invalid orderings are considered the same as missing.""" - ev1 = _create_event("!abc:test", 1) - ev2 = _create_event("!xyz:test", "xyz") - - self.assertEqual([ev2, ev1], _order(ev1, ev2)) - - ev1 = _create_event("!abc:test", {}) - self.assertEqual([ev2, ev1], _order(ev1, ev2)) - - ev1 = _create_event("!abc:test", []) - self.assertEqual([ev2, ev1], _order(ev1, ev2)) - - ev1 = _create_event("!abc:test", True) - self.assertEqual([ev2, ev1], _order(ev1, ev2)) - - def test_invalid_ordering_value(self): - """Invalid orderings are considered the same as missing.""" - ev1 = _create_event("!abc:test", "foo\n") - ev2 = _create_event("!xyz:test", "xyz") - - self.assertEqual([ev2, ev1], _order(ev1, ev2)) - - ev1 = _create_event("!abc:test", "a" * 51) - self.assertEqual([ev2, ev1], _order(ev1, ev2)) - - -class SpaceSummaryTestCase(unittest.HomeserverTestCase): - servlets = [ - admin.register_servlets_for_client_rest_resource, - room.register_servlets, - login.register_servlets, - ] - - def prepare(self, reactor, clock, hs: HomeServer): - self.hs = hs - self.handler = self.hs.get_space_summary_handler() - - # Create a user. - self.user = self.register_user("user", "pass") - self.token = self.login("user", "pass") - - # Create a space and a child room. - self.space = self.helper.create_room_as( - self.user, - tok=self.token, - extra_content={ - "creation_content": {EventContentFields.ROOM_TYPE: RoomTypes.SPACE} - }, - ) - self.room = self.helper.create_room_as(self.user, tok=self.token) - self._add_child(self.space, self.room, self.token) - - def _add_child( - self, space_id: str, room_id: str, token: str, order: Optional[str] = None - ) -> None: - """Add a child room to a space.""" - content: JsonDict = {"via": [self.hs.hostname]} - if order is not None: - content["order"] = order - self.helper.send_state( - space_id, - event_type=EventTypes.SpaceChild, - body=content, - tok=token, - state_key=room_id, - ) - - def _assert_rooms( - self, result: JsonDict, rooms_and_children: Iterable[Tuple[str, Iterable[str]]] - ) -> None: - """ - Assert that the expected room IDs and events are in the response. - - Args: - result: The result from the API call. - rooms_and_children: An iterable of tuples where each tuple is: - The expected room ID. - The expected IDs of any children rooms. - """ - room_ids = [] - children_ids = [] - for room_id, children in rooms_and_children: - room_ids.append(room_id) - if children: - children_ids.extend([(room_id, child_id) for child_id in children]) - self.assertCountEqual( - [room.get("room_id") for room in result["rooms"]], room_ids - ) - self.assertCountEqual( - [ - (event.get("room_id"), event.get("state_key")) - for event in result["events"] - ], - children_ids, - ) - - def _assert_hierarchy( - self, result: JsonDict, rooms_and_children: Iterable[Tuple[str, Iterable[str]]] - ) -> None: - """ - Assert that the expected room IDs are in the response. - - Args: - result: The result from the API call. - rooms_and_children: An iterable of tuples where each tuple is: - The expected room ID. - The expected IDs of any children rooms. - """ - result_room_ids = [] - result_children_ids = [] - for result_room in result["rooms"]: - result_room_ids.append(result_room["room_id"]) - result_children_ids.append( - [ - (cs["room_id"], cs["state_key"]) - for cs in result_room.get("children_state") - ] - ) - - room_ids = [] - children_ids = [] - for room_id, children in rooms_and_children: - room_ids.append(room_id) - children_ids.append([(room_id, child_id) for child_id in children]) - - # Note that order matters. - self.assertEqual(result_room_ids, room_ids) - self.assertEqual(result_children_ids, children_ids) - - 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)) - # The result should have the space and the room in it, along with a link - # from space -> room. - expected = [(self.space, [self.room]), (self.room, ())] - self._assert_rooms(result, expected) - - result = self.get_success( - self.handler.get_room_hierarchy(self.user, self.space) - ) - self._assert_hierarchy(result, expected) - - def test_visibility(self): - """A user not in a space cannot inspect it.""" - user2 = self.register_user("user2", "pass") - token2 = self.login("user2", "pass") - - # The user can see the space since it is publicly joinable. - result = self.get_success(self.handler.get_space_summary(user2, self.space)) - expected = [(self.space, [self.room]), (self.room, ())] - self._assert_rooms(result, expected) - - result = self.get_success(self.handler.get_room_hierarchy(user2, self.space)) - self._assert_hierarchy(result, expected) - - # If the space is made invite-only, it should no longer be viewable. - self.helper.send_state( - self.space, - event_type=EventTypes.JoinRules, - body={"join_rule": JoinRules.INVITE}, - tok=self.token, - ) - self.get_failure(self.handler.get_space_summary(user2, self.space), AuthError) - self.get_failure(self.handler.get_room_hierarchy(user2, self.space), AuthError) - - # If the space is made world-readable it should return a result. - self.helper.send_state( - self.space, - event_type=EventTypes.RoomHistoryVisibility, - body={"history_visibility": HistoryVisibility.WORLD_READABLE}, - tok=self.token, - ) - result = self.get_success(self.handler.get_space_summary(user2, self.space)) - self._assert_rooms(result, expected) - - result = self.get_success(self.handler.get_room_hierarchy(user2, self.space)) - self._assert_hierarchy(result, expected) - - # Make it not world-readable again and confirm it results in an error. - self.helper.send_state( - self.space, - event_type=EventTypes.RoomHistoryVisibility, - body={"history_visibility": HistoryVisibility.JOINED}, - tok=self.token, - ) - self.get_failure(self.handler.get_space_summary(user2, self.space), AuthError) - self.get_failure(self.handler.get_room_hierarchy(user2, self.space), AuthError) - - # Join the space and results should be returned. - self.helper.invite(self.space, targ=user2, tok=self.token) - self.helper.join(self.space, user2, tok=token2) - result = self.get_success(self.handler.get_space_summary(user2, self.space)) - self._assert_rooms(result, expected) - - result = self.get_success(self.handler.get_room_hierarchy(user2, self.space)) - self._assert_hierarchy(result, expected) - - # Attempting to view an unknown room returns the same error. - self.get_failure( - self.handler.get_space_summary(user2, "#not-a-space:" + self.hs.hostname), - AuthError, - ) - self.get_failure( - self.handler.get_room_hierarchy(user2, "#not-a-space:" + self.hs.hostname), - AuthError, - ) - - def _create_room_with_join_rule( - self, join_rule: str, room_version: Optional[str] = None, **extra_content - ) -> str: - """Create a room with the given join rule and add it to the space.""" - room_id = self.helper.create_room_as( - self.user, - room_version=room_version, - tok=self.token, - extra_content={ - "initial_state": [ - { - "type": EventTypes.JoinRules, - "state_key": "", - "content": { - "join_rule": join_rule, - **extra_content, - }, - } - ] - }, - ) - self._add_child(self.space, room_id, self.token) - return room_id - - def test_filtering(self): - """ - Rooms should be properly filtered to only include rooms the user has access to. - """ - user2 = self.register_user("user2", "pass") - token2 = self.login("user2", "pass") - - # Create a few rooms which will have different properties. - public_room = self._create_room_with_join_rule(JoinRules.PUBLIC) - knock_room = self._create_room_with_join_rule( - JoinRules.KNOCK, room_version=RoomVersions.V7.identifier - ) - not_invited_room = self._create_room_with_join_rule(JoinRules.INVITE) - invited_room = self._create_room_with_join_rule(JoinRules.INVITE) - self.helper.invite(invited_room, targ=user2, tok=self.token) - restricted_room = self._create_room_with_join_rule( - JoinRules.RESTRICTED, - room_version=RoomVersions.V8.identifier, - allow=[], - ) - restricted_accessible_room = self._create_room_with_join_rule( - JoinRules.RESTRICTED, - room_version=RoomVersions.V8.identifier, - allow=[ - { - "type": RestrictedJoinRuleTypes.ROOM_MEMBERSHIP, - "room_id": self.space, - "via": [self.hs.hostname], - } - ], - ) - world_readable_room = self._create_room_with_join_rule(JoinRules.INVITE) - self.helper.send_state( - world_readable_room, - event_type=EventTypes.RoomHistoryVisibility, - body={"history_visibility": HistoryVisibility.WORLD_READABLE}, - tok=self.token, - ) - joined_room = self._create_room_with_join_rule(JoinRules.INVITE) - self.helper.invite(joined_room, targ=user2, tok=self.token) - self.helper.join(joined_room, user2, tok=token2) - - # Join the space. - self.helper.join(self.space, user2, tok=token2) - result = self.get_success(self.handler.get_space_summary(user2, self.space)) - expected = [ - ( - self.space, - [ - self.room, - public_room, - knock_room, - not_invited_room, - invited_room, - restricted_room, - restricted_accessible_room, - world_readable_room, - joined_room, - ], - ), - (self.room, ()), - (public_room, ()), - (knock_room, ()), - (invited_room, ()), - (restricted_accessible_room, ()), - (world_readable_room, ()), - (joined_room, ()), - ] - self._assert_rooms(result, expected) - - result = self.get_success(self.handler.get_room_hierarchy(user2, self.space)) - self._assert_hierarchy(result, expected) - - def test_complex_space(self): - """ - Create a "complex" space to see how it handles things like loops and subspaces. - """ - # Create an inaccessible room. - user2 = self.register_user("user2", "pass") - token2 = self.login("user2", "pass") - room2 = self.helper.create_room_as(user2, is_public=False, tok=token2) - # This is a bit odd as "user" is adding a room they don't know about, but - # it works for the tests. - self._add_child(self.space, room2, self.token) - - # Create a subspace under the space with an additional room in it. - subspace = self.helper.create_room_as( - self.user, - tok=self.token, - extra_content={ - "creation_content": {EventContentFields.ROOM_TYPE: RoomTypes.SPACE} - }, - ) - subroom = self.helper.create_room_as(self.user, tok=self.token) - self._add_child(self.space, subspace, token=self.token) - self._add_child(subspace, subroom, token=self.token) - # Also add the two rooms from the space into this subspace (causing loops). - self._add_child(subspace, self.room, token=self.token) - self._add_child(subspace, room2, self.token) - - result = self.get_success(self.handler.get_space_summary(self.user, self.space)) - - # The result should include each room a single time and each link. - expected = [ - (self.space, [self.room, room2, subspace]), - (self.room, ()), - (subspace, [subroom, self.room, room2]), - (subroom, ()), - ] - self._assert_rooms(result, expected) - - result = self.get_success( - self.handler.get_room_hierarchy(self.user, self.space) - ) - self._assert_hierarchy(result, expected) - - def test_pagination(self): - """Test simple pagination works.""" - room_ids = [] - for i in range(1, 10): - room = self.helper.create_room_as(self.user, tok=self.token) - self._add_child(self.space, room, self.token, order=str(i)) - room_ids.append(room) - # The room created initially doesn't have an order, so comes last. - room_ids.append(self.room) - - result = self.get_success( - self.handler.get_room_hierarchy(self.user, self.space, limit=7) - ) - # The result should have the space and all of the links, plus some of the - # rooms and a pagination token. - expected: List[Tuple[str, Iterable[str]]] = [(self.space, room_ids)] - expected += [(room_id, ()) for room_id in room_ids[:6]] - self._assert_hierarchy(result, expected) - self.assertIn("next_batch", result) - - # Check the next page. - result = self.get_success( - self.handler.get_room_hierarchy( - self.user, self.space, limit=5, from_token=result["next_batch"] - ) - ) - # The result should have the space and the room in it, along with a link - # from space -> room. - expected = [(room_id, ()) for room_id in room_ids[6:]] - self._assert_hierarchy(result, expected) - self.assertNotIn("next_batch", result) - - def test_invalid_pagination_token(self): - """""" - room_ids = [] - for i in range(1, 10): - room = self.helper.create_room_as(self.user, tok=self.token) - self._add_child(self.space, room, self.token, order=str(i)) - room_ids.append(room) - # The room created initially doesn't have an order, so comes last. - room_ids.append(self.room) - - result = self.get_success( - self.handler.get_room_hierarchy(self.user, self.space, limit=7) - ) - self.assertIn("next_batch", result) - - # Changing the room ID, suggested-only, or max-depth causes an error. - self.get_failure( - self.handler.get_room_hierarchy( - self.user, self.room, from_token=result["next_batch"] - ), - SynapseError, - ) - self.get_failure( - self.handler.get_room_hierarchy( - self.user, - self.space, - suggested_only=True, - from_token=result["next_batch"], - ), - SynapseError, - ) - self.get_failure( - self.handler.get_room_hierarchy( - self.user, self.space, max_depth=0, from_token=result["next_batch"] - ), - SynapseError, - ) - - # An invalid token is ignored. - self.get_failure( - self.handler.get_room_hierarchy(self.user, self.space, from_token="foo"), - SynapseError, - ) - - def test_max_depth(self): - """Create a deep tree to test the max depth against.""" - spaces = [self.space] - rooms = [self.room] - for _ in range(5): - spaces.append( - self.helper.create_room_as( - self.user, - tok=self.token, - extra_content={ - "creation_content": { - EventContentFields.ROOM_TYPE: RoomTypes.SPACE - } - }, - ) - ) - self._add_child(spaces[-2], spaces[-1], self.token) - rooms.append(self.helper.create_room_as(self.user, tok=self.token)) - self._add_child(spaces[-1], rooms[-1], self.token) - - # Test just the space itself. - result = self.get_success( - self.handler.get_room_hierarchy(self.user, self.space, max_depth=0) - ) - expected: List[Tuple[str, Iterable[str]]] = [(spaces[0], [rooms[0], spaces[1]])] - self._assert_hierarchy(result, expected) - - # A single additional layer. - result = self.get_success( - self.handler.get_room_hierarchy(self.user, self.space, max_depth=1) - ) - expected += [ - (rooms[0], ()), - (spaces[1], [rooms[1], spaces[2]]), - ] - self._assert_hierarchy(result, expected) - - # A few layers. - result = self.get_success( - self.handler.get_room_hierarchy(self.user, self.space, max_depth=3) - ) - expected += [ - (rooms[1], ()), - (spaces[2], [rooms[2], spaces[3]]), - (rooms[2], ()), - (spaces[3], [rooms[3], spaces[4]]), - ] - self._assert_hierarchy(result, expected) - - def test_fed_complex(self): - """ - Return data over federation and ensure that it is handled properly. - """ - fed_hostname = self.hs.hostname + "2" - subspace = "#subspace:" + fed_hostname - subroom = "#subroom:" + fed_hostname - - async def summarize_remote_room( - _self, room, suggested_only, max_children, exclude_rooms - ): - # Return some good data, and some bad data: - # - # * Event *back* to the root room. - # * Unrelated events / rooms - # * Multiple levels of events (in a not-useful order, e.g. grandchild - # events before child events). - - # Note that these entries are brief, but should contain enough info. - return [ - _RoomEntry( - subspace, - { - "room_id": subspace, - "world_readable": True, - "room_type": RoomTypes.SPACE, - }, - [ - { - "room_id": subspace, - "state_key": subroom, - "content": {"via": [fed_hostname]}, - } - ], - ), - _RoomEntry( - subroom, - { - "room_id": subroom, - "world_readable": True, - }, - ), - ] - - # Add a room to the space which is on another server. - self._add_child(self.space, subspace, 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) - ) - - expected = [ - (self.space, [self.room, subspace]), - (self.room, ()), - (subspace, [subroom]), - (subroom, ()), - ] - self._assert_rooms(result, expected) - - def test_fed_filtering(self): - """ - Rooms returned over federation should be properly filtered to only include - rooms the user has access to. - """ - fed_hostname = self.hs.hostname + "2" - subspace = "#subspace:" + fed_hostname - - # Create a few rooms which will have different properties. - public_room = "#public:" + fed_hostname - knock_room = "#knock:" + fed_hostname - not_invited_room = "#not_invited:" + fed_hostname - invited_room = "#invited:" + fed_hostname - restricted_room = "#restricted:" + fed_hostname - restricted_accessible_room = "#restricted_accessible:" + fed_hostname - world_readable_room = "#world_readable:" + fed_hostname - joined_room = self.helper.create_room_as(self.user, tok=self.token) - - # Poke an invite over federation into the database. - self._poke_fed_invite(invited_room, "@remote:" + fed_hostname) - - async def summarize_remote_room( - _self, room, suggested_only, max_children, exclude_rooms - ): - # Note that these entries are brief, but should contain enough info. - rooms = [ - _RoomEntry( - public_room, - { - "room_id": public_room, - "world_readable": False, - "join_rules": JoinRules.PUBLIC, - }, - ), - _RoomEntry( - knock_room, - { - "room_id": knock_room, - "world_readable": False, - "join_rules": JoinRules.KNOCK, - }, - ), - _RoomEntry( - not_invited_room, - { - "room_id": not_invited_room, - "world_readable": False, - "join_rules": JoinRules.INVITE, - }, - ), - _RoomEntry( - invited_room, - { - "room_id": invited_room, - "world_readable": False, - "join_rules": JoinRules.INVITE, - }, - ), - _RoomEntry( - restricted_room, - { - "room_id": restricted_room, - "world_readable": False, - "join_rules": JoinRules.RESTRICTED, - "allowed_spaces": [], - }, - ), - _RoomEntry( - restricted_accessible_room, - { - "room_id": restricted_accessible_room, - "world_readable": False, - "join_rules": JoinRules.RESTRICTED, - "allowed_spaces": [self.room], - }, - ), - _RoomEntry( - world_readable_room, - { - "room_id": world_readable_room, - "world_readable": True, - "join_rules": JoinRules.INVITE, - }, - ), - _RoomEntry( - joined_room, - { - "room_id": joined_room, - "world_readable": False, - "join_rules": JoinRules.INVITE, - }, - ), - ] - - # Also include the subspace. - rooms.insert( - 0, - _RoomEntry( - subspace, - { - "room_id": subspace, - "world_readable": True, - }, - # Place each room in the sub-space. - [ - { - "room_id": subspace, - "state_key": room.room_id, - "content": {"via": [fed_hostname]}, - } - for room in rooms - ], - ), - ) - return rooms - - # Add a room to the space which is on another server. - self._add_child(self.space, subspace, 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) - ) - - expected = [ - (self.space, [self.room, subspace]), - (self.room, ()), - ( - subspace, - [ - public_room, - knock_room, - not_invited_room, - invited_room, - restricted_room, - restricted_accessible_room, - world_readable_room, - joined_room, - ], - ), - (public_room, ()), - (knock_room, ()), - (invited_room, ()), - (restricted_accessible_room, ()), - (world_readable_room, ()), - (joined_room, ()), - ] - self._assert_rooms(result, expected) - - 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) - ) - - expected = [ - (self.space, [self.room, fed_room]), - (self.room, ()), - (fed_room, ()), - ] - self._assert_rooms(result, expected) From b7ba82acf575ee495cf105db61b4f9ea9ebc596a Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 13 Aug 2021 11:47:46 +0100 Subject: [PATCH 23/35] Fix mocks Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- mypy.ini | 2 +- tests/handlers/test_room_summary.py | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/mypy.ini b/mypy.ini index 5d6cd557bca2..ba6543cd5b3a 100644 --- a/mypy.ini +++ b/mypy.ini @@ -85,8 +85,8 @@ files = tests/replication, tests/test_event_auth.py, tests/test_utils, + tests/handlers/test_room_summary.py, tests/handlers/test_password_providers.py, - tests/handlers/test_space_summary.py, tests/rest/client/v1/test_login.py, tests/rest/client/v2_alpha/test_auth.py, tests/util/test_itertools.py, diff --git a/tests/handlers/test_room_summary.py b/tests/handlers/test_room_summary.py index 29f6866580f3..49c63d08fcdc 100644 --- a/tests/handlers/test_room_summary.py +++ b/tests/handlers/test_room_summary.py @@ -48,7 +48,7 @@ def _order(*events): return sorted(events, key=_child_events_comparison_key) -class TestHierarchySort(unittest.TestCase): +class TestSpaceSummarySort(unittest.TestCase): def test_no_order_last(self): """An event with no ordering is placed behind those with an ordering.""" ev1 = _create_event("!abc:test") @@ -97,7 +97,7 @@ def test_invalid_ordering_value(self): self.assertEqual([ev2, ev1], _order(ev1, ev2)) -class RoomHierarchyTestCase(unittest.HomeserverTestCase): +class SpaceSummaryTestCase(unittest.HomeserverTestCase): servlets = [ admin.register_servlets_for_client_rest_resource, room.register_servlets, @@ -621,7 +621,7 @@ async def summarize_remote_room( self._add_child(self.space, subspace, self.token) with mock.patch( - "synapse.handlers.space_summary.SpaceSummaryHandler._summarize_remote_room", + "synapse.handlers.room_summary.RoomSummaryHandler._summarize_remote_room_hierarchy", new=summarize_remote_room, ): result = self.get_success(self.handler.get_space_summary(self.user, self.space)) @@ -754,7 +754,7 @@ async def summarize_remote_room( self._add_child(self.space, subspace, self.token) with mock.patch( - "synapse.handlers.space_summary.SpaceSummaryHandler._summarize_remote_room", + "synapse.handlers.room_summary.RoomSummaryHandler._summarize_remote_room_hierarchy", new=summarize_remote_room, ): result = self.get_success(self.handler.get_space_summary(self.user, self.space)) @@ -816,7 +816,7 @@ async def summarize_remote_room( self._add_child(self.space, fed_room, self.token) with mock.patch( - "synapse.handlers.space_summary.SpaceSummaryHandler._summarize_remote_room", + "synapse.handlers.room_summary.RoomSummaryHandler._summarize_remote_room_hierarchy", new=summarize_remote_room, ): result = self.get_success(self.handler.get_space_summary(self.user, self.space)) From 7a57bcc3118dae3236fb4faab1d51bc910fc1dee Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 13 Aug 2021 11:57:05 +0100 Subject: [PATCH 24/35] fix arg order Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- synapse/handlers/room_summary.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/handlers/room_summary.py b/synapse/handlers/room_summary.py index 919302627651..9a71c4bdf38a 100644 --- a/synapse/handlers/room_summary.py +++ b/synapse/handlers/room_summary.py @@ -623,7 +623,7 @@ async def _summarize_local_room_hierarchy( A room entry if the room should be returned. None, otherwise. """ try: - room_entry = await self._summarize_local_room(room_id, requester, origin) + room_entry = await self._summarize_local_room(requester, origin, room_id) except NotFoundError: return None From 8371e26068a7b31933c9069ff11cade4de381148 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 13 Aug 2021 12:00:19 +0100 Subject: [PATCH 25/35] minimise diff Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- synapse/handlers/room_summary.py | 124 +++++++++++++++---------------- 1 file changed, 62 insertions(+), 62 deletions(-) diff --git a/synapse/handlers/room_summary.py b/synapse/handlers/room_summary.py index 9a71c4bdf38a..30341cbe57b0 100644 --- a/synapse/handlers/room_summary.py +++ b/synapse/handlers/room_summary.py @@ -113,68 +113,6 @@ def __init__(self, hs: "HomeServer"): "get_room_hierarchy", ) - 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. - - Args: - room_id: The room ID to summarize. - for_federation: True if this is a summary requested over federation - (which includes additional fields). - - Returns: - The JSON dictionary for the room. - """ - stats = await self._store.get_room_with_stats(room_id) - - # currently this should be impossible because we call - # _is_local_room_accessible on the room before we get here, so - # there should always be an entry - assert stats is not None, "unable to retrieve stats for %s" % (room_id,) - - current_state_ids = await self._store.get_current_state_ids(room_id) - create_event = await self._store.get_event( - current_state_ids[(EventTypes.Create, "")] - ) - - entry = { - "room_id": stats["room_id"], - "name": stats["name"], - "topic": stats["topic"], - "canonical_alias": stats["canonical_alias"], - "num_joined_members": stats["joined_members"], - "avatar_url": stats["avatar"], - "join_rules": stats["join_rules"], - "world_readable": ( - stats["history_visibility"] == HistoryVisibility.WORLD_READABLE - ), - "guest_can_join": stats["guest_access"] == "can_join", - "creation_ts": create_event.origin_server_ts, - "room_type": create_event.content.get(EventContentFields.ROOM_TYPE), - } - - # Federation requests need to provide additional information so the - # requested server is able to filter the response appropriately. - if for_federation: - room_version = await self._store.get_room_version(room_id) - if await self._event_auth_handler.has_restricted_join_rules( - current_state_ids, room_version - ): - allowed_rooms = ( - await self._event_auth_handler.get_rooms_that_allow_join( - current_state_ids - ) - ) - if allowed_rooms: - entry["allowed_room_ids"] = allowed_rooms - # TODO Remove this key once the API is stable. - entry["allowed_spaces"] = allowed_rooms - - # Filter out Nones – rather omit the field altogether - room_entry = {k: v for k, v in entry.items() if v is not None} - - return room_entry - async def _is_remote_room_accessible( self, requester: Optional[str], room_id: str, room: JsonDict ) -> bool: @@ -805,6 +743,68 @@ async def get_room_summary( return room_summary + 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. + + Args: + room_id: The room ID to summarize. + for_federation: True if this is a summary requested over federation + (which includes additional fields). + + Returns: + The JSON dictionary for the room. + """ + stats = await self._store.get_room_with_stats(room_id) + + # currently this should be impossible because we call + # _is_local_room_accessible on the room before we get here, so + # there should always be an entry + assert stats is not None, "unable to retrieve stats for %s" % (room_id,) + + current_state_ids = await self._store.get_current_state_ids(room_id) + create_event = await self._store.get_event( + current_state_ids[(EventTypes.Create, "")] + ) + + entry = { + "room_id": stats["room_id"], + "name": stats["name"], + "topic": stats["topic"], + "canonical_alias": stats["canonical_alias"], + "num_joined_members": stats["joined_members"], + "avatar_url": stats["avatar"], + "join_rules": stats["join_rules"], + "world_readable": ( + stats["history_visibility"] == HistoryVisibility.WORLD_READABLE + ), + "guest_can_join": stats["guest_access"] == "can_join", + "creation_ts": create_event.origin_server_ts, + "room_type": create_event.content.get(EventContentFields.ROOM_TYPE), + } + + # Federation requests need to provide additional information so the + # requested server is able to filter the response appropriately. + if for_federation: + room_version = await self._store.get_room_version(room_id) + if await self._event_auth_handler.has_restricted_join_rules( + current_state_ids, room_version + ): + allowed_rooms = ( + await self._event_auth_handler.get_rooms_that_allow_join( + current_state_ids + ) + ) + if allowed_rooms: + entry["allowed_room_ids"] = allowed_rooms + # TODO Remove this key once the API is stable. + entry["allowed_spaces"] = allowed_rooms + + # Filter out Nones – rather omit the field altogether + room_entry = {k: v for k, v in entry.items() if v is not None} + + return room_entry + async def _summarize_local_room( self, requester: Optional[str], From d4020d85b5b34f21607f0bdfe4a6f7146e7b97f9 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 13 Aug 2021 12:37:13 +0100 Subject: [PATCH 26/35] delint Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- tests/handlers/test_room_summary.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tests/handlers/test_room_summary.py b/tests/handlers/test_room_summary.py index 49c63d08fcdc..5ef8290eac35 100644 --- a/tests/handlers/test_room_summary.py +++ b/tests/handlers/test_room_summary.py @@ -624,7 +624,9 @@ async def summarize_remote_room( "synapse.handlers.room_summary.RoomSummaryHandler._summarize_remote_room_hierarchy", new=summarize_remote_room, ): - result = self.get_success(self.handler.get_space_summary(self.user, self.space)) + result = self.get_success( + self.handler.get_space_summary(self.user, self.space) + ) expected = [ (self.space, [self.room, subspace]), @@ -757,7 +759,9 @@ async def summarize_remote_room( "synapse.handlers.room_summary.RoomSummaryHandler._summarize_remote_room_hierarchy", new=summarize_remote_room, ): - result = self.get_success(self.handler.get_space_summary(self.user, self.space)) + result = self.get_success( + self.handler.get_space_summary(self.user, self.space) + ) expected = [ (self.space, [self.room, subspace]), @@ -819,7 +823,9 @@ async def summarize_remote_room( "synapse.handlers.room_summary.RoomSummaryHandler._summarize_remote_room_hierarchy", new=summarize_remote_room, ): - result = self.get_success(self.handler.get_space_summary(self.user, self.space)) + result = self.get_success( + self.handler.get_space_summary(self.user, self.space) + ) expected = [ (self.space, [self.room, fed_room]), From 3fbf6210b5d43266d0a761d078153c2b2a49c0c9 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 13 Aug 2021 15:28:56 +0100 Subject: [PATCH 27/35] revert bits of the PR Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- synapse/api/auth.py | 114 +---------------- synapse/handlers/room_summary.py | 212 +++++++++++++++++++++++-------- tests/test_state.py | 1 - 3 files changed, 163 insertions(+), 164 deletions(-) diff --git a/synapse/api/auth.py b/synapse/api/auth.py index 0cfa77b3587a..b5f2b00fbb78 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -21,7 +21,7 @@ from synapse import event_auth from synapse.api.auth_blocking import AuthBlocking -from synapse.api.constants import EventTypes, HistoryVisibility, JoinRules, Membership +from synapse.api.constants import EventTypes, HistoryVisibility, Membership from synapse.api.errors import ( AuthError, Codes, @@ -621,115 +621,3 @@ async def check_user_in_room_or_world_readable( async def check_auth_blocking(self, *args, **kwargs) -> None: await self._auth_blocking.check_auth_blocking(*args, **kwargs) - - async def is_room_visible( - self, room_id: str, requester: Optional[str], origin: Optional[str] = None - ) -> bool: - """ - Calculate whether the room should be shown to the requester. - - It should return true if: - - * The requester is joined or can join the room (per MSC3173). - * The origin server has any user that is joined or can join the room. - * The history visibility is set to world readable. - - Args: - room_id: The room ID to check visibility of. - requester: - The user making the request, if it is a local request. - None if this is a federation request. - origin: - The server making the request, if it is a federation request. - None if this is a local request. - - Returns: - True if the room should be visible to the requesting user or server. - """ - state_ids = await self.store.get_current_state_ids(room_id) - - # If there's no state for the room, it isn't known. - if not state_ids: - # The user might have a pending invite for the room. - if requester and await self.store.get_invite_for_local_user_in_room( - requester, room_id - ): - return True - - logger.info("room %s is unknown", room_id) - return False - - room_version = await self.store.get_room_version(room_id) - - # Include the room if it has join rules of public or knock. - join_rules_event_id = state_ids.get((EventTypes.JoinRules, "")) - if join_rules_event_id: - join_rules_event = await self.store.get_event(join_rules_event_id) - join_rule = join_rules_event.content.get("join_rule") - if join_rule == JoinRules.PUBLIC or ( - room_version.msc2403_knocking and join_rule == JoinRules.KNOCK - ): - return True - - # Include the room if it is peekable. - hist_vis_event_id = state_ids.get((EventTypes.RoomHistoryVisibility, "")) - if hist_vis_event_id: - hist_vis_ev = await self.store.get_event(hist_vis_event_id) - hist_vis = hist_vis_ev.content.get("history_visibility") - if hist_vis == HistoryVisibility.WORLD_READABLE: - return True - - # Otherwise we need to check information specific to the user or server. - - # If we have an authenticated requesting user, check if they are a member - # of the room (or can join the room). - if requester: - member_event_id = state_ids.get((EventTypes.Member, requester), None) - - # If they're in the room they can see info on it. - if member_event_id: - member_event = await self.store.get_event(member_event_id) - if member_event.membership in (Membership.JOIN, Membership.INVITE): - return True - - # Otherwise, check if they should be allowed access via membership in a space. - if await self._event_auth_handler.has_restricted_join_rules( - state_ids, room_version - ): - allowed_rooms = ( - await self._event_auth_handler.get_rooms_that_allow_join(state_ids) - ) - if await self._event_auth_handler.is_user_in_rooms( - allowed_rooms, requester - ): - return True - - # If this is a request over federation, check if the host is in the room or - # has a user who could join the room. - elif origin: - if await self._event_auth_handler.check_host_in_room( - room_id, origin - ) or await self.store.is_host_invited(room_id, origin): - return True - - # Alternately, if the host has a user in any of the spaces specified - # for access, then the host can see this room (and should do filtering - # if the requester cannot see it). - if await self._event_auth_handler.has_restricted_join_rules( - state_ids, room_version - ): - allowed_rooms = ( - await self._event_auth_handler.get_rooms_that_allow_join(state_ids) - ) - for space_id in allowed_rooms: - if await self._event_auth_handler.check_host_in_room( - space_id, origin - ): - return True - - logger.info( - "room %s is unpeekable and requester %s is not a member / not allowed to join", - room_id, - requester or origin, - ) - return False diff --git a/synapse/handlers/room_summary.py b/synapse/handlers/room_summary.py index 30341cbe57b0..b37c2d20cb73 100644 --- a/synapse/handlers/room_summary.py +++ b/synapse/handlers/room_summary.py @@ -35,6 +35,7 @@ EventTypes, HistoryVisibility, JoinRules, + Membership, RoomTypes, ) from synapse.api.errors import AuthError, Codes, NotFoundError, SynapseError @@ -92,7 +93,6 @@ def __init__(self, hs: "HomeServer"): self._clock = hs.get_clock() self._event_auth_handler = hs.get_event_auth_handler() self._store = hs.get_datastore() - self._auth = hs.get_auth() self._event_auth_handler = hs.get_event_auth_handler() self._event_serializer = hs.get_event_client_serializer() self._server_name = hs.hostname @@ -113,51 +113,6 @@ def __init__(self, hs: "HomeServer"): "get_room_hierarchy", ) - async def _is_remote_room_accessible( - self, requester: Optional[str], room_id: str, room: JsonDict - ) -> bool: - """ - Calculate whether the room received over federation should be shown in the 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, if authenticated. - 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 requester and 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._auth.is_room_visible(room_id, requester) - def _expire_pagination_sessions(self): """Expire pagination session which are old.""" expire_before = ( @@ -196,10 +151,10 @@ async def get_space_summary( is overridden by MAX_ROOMS_PER_SPACE. Returns: - hierarchy dict to return + summary dict to return """ # First of all, check that the room is accessible. - if not await self._auth.is_room_visible(room_id, requester): + if not await self._is_local_room_accessible(room_id, requester): raise AuthError( 403, "User %s not in room %s, and room previews are disabled" @@ -374,7 +329,7 @@ async def _get_room_hierarchy( """See docstring for SpaceSummaryHandler.get_room_hierarchy.""" # First of all, check that the room is accessible. - if not await self._auth.is_room_visible(requested_room_id, requester): + if not await self._is_local_room_accessible(requested_room_id, requester): raise AuthError( 403, "User %s not in room %s, and room previews are disabled" @@ -662,6 +617,163 @@ async def _summarize_remote_room_hierarchy( return results + async def _is_remote_room_accessible( + self, requester: Optional[str], room_id: str, room: JsonDict + ) -> bool: + """ + Calculate whether the room received over federation should be shown in the 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, if authenticated. + 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 requester and 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) + + async def _is_local_room_accessible( + self, room_id: str, requester: Optional[str], origin: Optional[str] = None + ) -> bool: + """ + Calculate whether the room should be shown to the requester. + + It should return true if: + + * The requester is joined or can join the room (per MSC3173). + * The origin server has any user that is joined or can join the room. + * The history visibility is set to world readable. + + Args: + room_id: The room ID to check visibility of. + requester: + The user making the request, if it is a local request. + None if this is a federation request. + origin: + The server making the request, if it is a federation request. + None if this is a local request. + + Returns: + True if the room should be visible to the requesting user or server. + """ + state_ids = await self._store.get_current_state_ids(room_id) + + # If there's no state for the room, it isn't known. + if not state_ids: + # The user might have a pending invite for the room. + if requester and await self._store.get_invite_for_local_user_in_room( + requester, room_id + ): + return True + + logger.info("room %s is unknown, omitting from summary", room_id) + return False + + room_version = await self._store.get_room_version(room_id) + + # Include the room if it has join rules of public or knock. + join_rules_event_id = state_ids.get((EventTypes.JoinRules, "")) + if join_rules_event_id: + join_rules_event = await self._store.get_event(join_rules_event_id) + join_rule = join_rules_event.content.get("join_rule") + if join_rule == JoinRules.PUBLIC or ( + room_version.msc2403_knocking and join_rule == JoinRules.KNOCK + ): + return True + + # Include the room if it is peekable. + hist_vis_event_id = state_ids.get((EventTypes.RoomHistoryVisibility, "")) + if hist_vis_event_id: + hist_vis_ev = await self._store.get_event(hist_vis_event_id) + hist_vis = hist_vis_ev.content.get("history_visibility") + if hist_vis == HistoryVisibility.WORLD_READABLE: + return True + + # Otherwise we need to check information specific to the user or server. + + # If we have an authenticated requesting user, check if they are a member + # of the room (or can join the room). + if requester: + member_event_id = state_ids.get((EventTypes.Member, requester), None) + + # If they're in the room they can see info on it. + if member_event_id: + member_event = await self._store.get_event(member_event_id) + if member_event.membership in (Membership.JOIN, Membership.INVITE): + return True + + # Otherwise, check if they should be allowed access via membership in a space. + if await self._event_auth_handler.has_restricted_join_rules( + state_ids, room_version + ): + allowed_rooms = ( + await self._event_auth_handler.get_rooms_that_allow_join(state_ids) + ) + if await self._event_auth_handler.is_user_in_rooms( + allowed_rooms, requester + ): + return True + + # If this is a request over federation, check if the host is in the room or + # has a user who could join the room. + elif origin: + if await self._event_auth_handler.check_host_in_room( + room_id, origin + ) or await self._store.is_host_invited(room_id, origin): + return True + + # Alternately, if the host has a user in any of the spaces specified + # for access, then the host can see this room (and should do filtering + # if the requester cannot see it). + if await self._event_auth_handler.has_restricted_join_rules( + state_ids, room_version + ): + allowed_rooms = ( + await self._event_auth_handler.get_rooms_that_allow_join(state_ids) + ) + for space_id in allowed_rooms: + if await self._event_auth_handler.check_host_in_room( + space_id, origin + ): + return True + + logger.info( + "room %s is unpeekable and requester %s is not a member / not allowed to join, omitting from summary", + room_id, + requester or origin, + ) + return False + async def _get_child_events(self, room_id: str) -> Iterable[EventBase]: """ Get the child events for a given room. @@ -826,7 +938,7 @@ async def _summarize_local_room( Returns: room summary dict to return """ - if not await self._auth.is_room_visible(room_id, requester, origin): + if not await self._is_local_room_accessible(room_id, requester, origin): raise NotFoundError("Room not found or is not accessible") return await self._build_room_entry(room_id, for_federation=bool(origin)) diff --git a/tests/test_state.py b/tests/test_state.py index 7b33a662d238..40210b14ce22 100644 --- a/tests/test_state.py +++ b/tests/test_state.py @@ -176,7 +176,6 @@ def setUp(self): hs.config = default_config("tesths", True) hs.get_datastore.return_value = self.store hs.get_state_handler.return_value = None - hs.get_event_auth_handler.return_value = None hs.get_clock.return_value = MockClock() hs.get_auth.return_value = Auth(hs) hs.get_state_resolution_handler = lambda: StateResolutionHandler(hs) From e99ca2961cac83e40bb662f552cec4d6832d38b3 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 13 Aug 2021 15:31:27 +0100 Subject: [PATCH 28/35] revert s'more Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- synapse/api/auth.py | 1 - synapse/handlers/room_summary.py | 217 +++++++++++++++---------------- 2 files changed, 108 insertions(+), 110 deletions(-) diff --git a/synapse/api/auth.py b/synapse/api/auth.py index b5f2b00fbb78..05699714eea7 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -69,7 +69,6 @@ def __init__(self, hs: "HomeServer"): ) self._auth_blocking = AuthBlocking(self.hs) - self._event_auth_handler = hs.get_event_auth_handler() self._track_appservice_user_ips = hs.config.track_appservice_user_ips self._macaroon_secret_key = hs.config.macaroon_secret_key diff --git a/synapse/handlers/room_summary.py b/synapse/handlers/room_summary.py index b37c2d20cb73..7b08dc6c56d3 100644 --- a/synapse/handlers/room_summary.py +++ b/synapse/handlers/room_summary.py @@ -93,7 +93,6 @@ def __init__(self, hs: "HomeServer"): self._clock = hs.get_clock() self._event_auth_handler = hs.get_event_auth_handler() self._store = hs.get_datastore() - self._event_auth_handler = hs.get_event_auth_handler() self._event_serializer = hs.get_event_client_serializer() self._server_name = hs.hostname self._federation_client = hs.get_federation_client() @@ -141,7 +140,7 @@ async def get_space_summary( Args: requester: user id of the user making this request - room_id: room id to start the hierarchy from + room_id: room id to start the summary at suggested_only: whether we should only return children with the "suggested" flag set. @@ -617,51 +616,6 @@ async def _summarize_remote_room_hierarchy( return results - async def _is_remote_room_accessible( - self, requester: Optional[str], room_id: str, room: JsonDict - ) -> bool: - """ - Calculate whether the room received over federation should be shown in the 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, if authenticated. - 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 requester and 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) - async def _is_local_room_accessible( self, room_id: str, requester: Optional[str], origin: Optional[str] = None ) -> bool: @@ -774,6 +728,113 @@ async def _is_local_room_accessible( ) return False + async def _is_remote_room_accessible( + self, requester: Optional[str], room_id: str, room: JsonDict + ) -> bool: + """ + Calculate whether the room received over federation should be shown in the 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, if authenticated. + 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 requester and 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) + + 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. + + Args: + room_id: The room ID to summarize. + for_federation: True if this is a summary requested over federation + (which includes additional fields). + + Returns: + The JSON dictionary for the room. + """ + stats = await self._store.get_room_with_stats(room_id) + + # currently this should be impossible because we call + # _is_local_room_accessible on the room before we get here, so + # there should always be an entry + assert stats is not None, "unable to retrieve stats for %s" % (room_id,) + + current_state_ids = await self._store.get_current_state_ids(room_id) + create_event = await self._store.get_event( + current_state_ids[(EventTypes.Create, "")] + ) + + entry = { + "room_id": stats["room_id"], + "name": stats["name"], + "topic": stats["topic"], + "canonical_alias": stats["canonical_alias"], + "num_joined_members": stats["joined_members"], + "avatar_url": stats["avatar"], + "join_rules": stats["join_rules"], + "world_readable": ( + stats["history_visibility"] == HistoryVisibility.WORLD_READABLE + ), + "guest_can_join": stats["guest_access"] == "can_join", + "creation_ts": create_event.origin_server_ts, + "room_type": create_event.content.get(EventContentFields.ROOM_TYPE), + } + + # Federation requests need to provide additional information so the + # requested server is able to filter the response appropriately. + if for_federation: + room_version = await self._store.get_room_version(room_id) + if await self._event_auth_handler.has_restricted_join_rules( + current_state_ids, room_version + ): + allowed_rooms = ( + await self._event_auth_handler.get_rooms_that_allow_join( + current_state_ids + ) + ) + if allowed_rooms: + entry["allowed_room_ids"] = allowed_rooms + # TODO Remove this key once the API is stable. + entry["allowed_spaces"] = allowed_rooms + + # Filter out Nones – rather omit the field altogether + room_entry = {k: v for k, v in entry.items() if v is not None} + + return room_entry + async def _get_child_events(self, room_id: str) -> Iterable[EventBase]: """ Get the child events for a given room. @@ -855,68 +916,6 @@ async def get_room_summary( return room_summary - 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. - - Args: - room_id: The room ID to summarize. - for_federation: True if this is a summary requested over federation - (which includes additional fields). - - Returns: - The JSON dictionary for the room. - """ - stats = await self._store.get_room_with_stats(room_id) - - # currently this should be impossible because we call - # _is_local_room_accessible on the room before we get here, so - # there should always be an entry - assert stats is not None, "unable to retrieve stats for %s" % (room_id,) - - current_state_ids = await self._store.get_current_state_ids(room_id) - create_event = await self._store.get_event( - current_state_ids[(EventTypes.Create, "")] - ) - - entry = { - "room_id": stats["room_id"], - "name": stats["name"], - "topic": stats["topic"], - "canonical_alias": stats["canonical_alias"], - "num_joined_members": stats["joined_members"], - "avatar_url": stats["avatar"], - "join_rules": stats["join_rules"], - "world_readable": ( - stats["history_visibility"] == HistoryVisibility.WORLD_READABLE - ), - "guest_can_join": stats["guest_access"] == "can_join", - "creation_ts": create_event.origin_server_ts, - "room_type": create_event.content.get(EventContentFields.ROOM_TYPE), - } - - # Federation requests need to provide additional information so the - # requested server is able to filter the response appropriately. - if for_federation: - room_version = await self._store.get_room_version(room_id) - if await self._event_auth_handler.has_restricted_join_rules( - current_state_ids, room_version - ): - allowed_rooms = ( - await self._event_auth_handler.get_rooms_that_allow_join( - current_state_ids - ) - ) - if allowed_rooms: - entry["allowed_room_ids"] = allowed_rooms - # TODO Remove this key once the API is stable. - entry["allowed_spaces"] = allowed_rooms - - # Filter out Nones – rather omit the field altogether - room_entry = {k: v for k, v in entry.items() if v is not None} - - return room_entry - async def _summarize_local_room( self, requester: Optional[str], From 2823459fc1bf119bd942a095a7866fc7e6f6cd06 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 13 Aug 2021 15:33:03 +0100 Subject: [PATCH 29/35] minimise diff Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- tests/test_state.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_state.py b/tests/test_state.py index 40210b14ce22..e5488df1ac71 100644 --- a/tests/test_state.py +++ b/tests/test_state.py @@ -166,7 +166,6 @@ def setUp(self): "get_storage", "get_auth", "get_state_handler", - "get_event_auth_handler", "get_clock", "get_state_resolution_handler", "get_account_validity_handler", From 82c6f40cb8ec6e2a2f59ccb076a3c2bc5da1f3b4 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 13 Aug 2021 14:55:19 -0400 Subject: [PATCH 30/35] Fix ordering of mypy. --- mypy.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mypy.ini b/mypy.ini index ba6543cd5b3a..e1b9405daa85 100644 --- a/mypy.ini +++ b/mypy.ini @@ -85,8 +85,8 @@ files = tests/replication, tests/test_event_auth.py, tests/test_utils, - tests/handlers/test_room_summary.py, tests/handlers/test_password_providers.py, + tests/handlers/test_room_summary.py, tests/rest/client/v1/test_login.py, tests/rest/client/v2_alpha/test_auth.py, tests/util/test_itertools.py, From 82ed39cda5665d38c327924ab303e2c926c85b45 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 13 Aug 2021 15:02:41 -0400 Subject: [PATCH 31/35] Update comments. --- synapse/handlers/room_summary.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/synapse/handlers/room_summary.py b/synapse/handlers/room_summary.py index 7b08dc6c56d3..57efad655c17 100644 --- a/synapse/handlers/room_summary.py +++ b/synapse/handlers/room_summary.py @@ -629,7 +629,7 @@ async def _is_local_room_accessible( * The history visibility is set to world readable. Args: - room_id: The room ID to check visibility of. + room_id: The room ID to check accessibility of. requester: The user making the request, if it is a local request. None if this is a federation request. @@ -638,7 +638,7 @@ async def _is_local_room_accessible( None if this is a local request. Returns: - True if the room should be visible to the requesting user or server. + True if the room is accessible to the requesting user or server. """ state_ids = await self._store.get_current_state_ids(room_id) @@ -732,9 +732,9 @@ async def _is_remote_room_accessible( self, requester: Optional[str], room_id: str, room: JsonDict ) -> bool: """ - Calculate whether the room received over federation should be shown in the summary. + Calculate whether the room received over federation should be shown to the requester. - It should be included if: + It should return true if: * The requester is joined or can join the room (per MSC3173). * The history visibility is set to world readable. @@ -746,10 +746,10 @@ async def _is_remote_room_accessible( Args: requester: The user requesting the summary, if authenticated. room_id: The room ID returned over federation. - room: The summary of the child room returned over federation. + room: The summary of the room returned over federation. Returns: - True if the room should be included in the spaces summary. + True if the room is accessible to the requesting user. """ # The API doesn't return the room version so assume that a # join rule of knock is valid. From 0a74384ce5a41b0f926f8693d0358b23a0dd111f Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 13 Aug 2021 15:03:02 -0400 Subject: [PATCH 32/35] Rollback an unused change. --- synapse/handlers/room_summary.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/synapse/handlers/room_summary.py b/synapse/handlers/room_summary.py index 57efad655c17..852c2c99b1ec 100644 --- a/synapse/handlers/room_summary.py +++ b/synapse/handlers/room_summary.py @@ -729,7 +729,7 @@ async def _is_local_room_accessible( return False async def _is_remote_room_accessible( - self, requester: Optional[str], room_id: str, room: JsonDict + self, requester: str, room_id: str, room: JsonDict ) -> bool: """ Calculate whether the room received over federation should be shown to the requester. @@ -744,7 +744,7 @@ async def _is_remote_room_accessible( due to an invite, etc. Args: - requester: The user requesting the summary, if authenticated. + requester: The user requesting the summary. room_id: The room ID returned over federation. room: The summary of the room returned over federation. @@ -762,7 +762,7 @@ async def _is_remote_room_accessible( # 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 requester and allowed_rooms and isinstance(allowed_rooms, list): + if allowed_rooms and isinstance(allowed_rooms, list): if await self._event_auth_handler.is_user_in_rooms( allowed_rooms, requester ): From 5dcbf1a8ee69e13e3f07962eb96fc06b52ea5b8b Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 13 Aug 2021 15:16:37 -0400 Subject: [PATCH 33/35] Strip out code for federation, which is not yet supported. --- synapse/handlers/room_summary.py | 48 ++++++-------------------------- 1 file changed, 9 insertions(+), 39 deletions(-) diff --git a/synapse/handlers/room_summary.py b/synapse/handlers/room_summary.py index 852c2c99b1ec..f2e423dc810e 100644 --- a/synapse/handlers/room_summary.py +++ b/synapse/handlers/room_summary.py @@ -775,7 +775,7 @@ async def _is_remote_room_accessible( 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. + Generate en entry summarising a single room. Args: room_id: The room ID to summarize. @@ -870,13 +870,13 @@ async def get_room_summary( remote_room_hosts: Optional[List[str]] = None, ) -> JsonDict: """ - Implementation of the room summary C-S API MSC3266 + Implementation of the room summary C-S API from MSC3266 Args: - requester: user id of the user making this request, - can be None for unauthenticated requests + requester: user id of the user making this request, will be None + for unauthenticated requests - room_id: room id to start the summary at + room_id: room id to summarise. remote_room_hosts: a list of homeservers to try fetching data through if we don't know it ourselves @@ -889,6 +889,7 @@ async def get_room_summary( if is_in_room: room_summary = await self._summarize_local_room(requester, None, room_id) + # If there was a requester, add their membership. if requester: ( membership, @@ -899,20 +900,9 @@ async def get_room_summary( room_summary["membership"] = membership or "leave" else: - room_summary = await self._summarize_remote_room(room_id, remote_room_hosts) - - # validate that the requester has permission to see this room - include_room = self._is_remote_room_accessible( - requester, room_id, room_summary - ) - - if not include_room: - raise NotFoundError("Room not found or is not accessible") - - # Before returning to the client, remove the allowed_room_ids - # and allowed_spaces keys. - room_summary.pop("allowed_room_ids", None) - room_summary.pop("allowed_spaces", None) + # TODO federation API, descoped from initial unstable implementation + # as MSC needs more maturing on that side. + raise SynapseError(400, "Federation is not currently supported.") return room_summary @@ -942,26 +932,6 @@ async def _summarize_local_room( return await self._build_room_entry(room_id, for_federation=bool(origin)) - async def _summarize_remote_room( - self, - room_id: str, - remote_room_hosts: Optional[List[str]], - ) -> JsonDict: - """ - Request room summary entry for a given room by querying a remote server. - - Args: - room_id: The room to summarize. - remote_room_hosts: List of homeservers to attempt to fetch the data from. - - Returns: - summary dict to return - """ - logger.info("Requesting summary for %s via %s", room_id, remote_room_hosts) - - # TODO federation API, descoped from initial unstable implementation as MSC needs more maturing on that side. - raise NotFoundError("Room not found or is not accessible") - @attr.s(frozen=True, slots=True, auto_attribs=True) class _RoomQueueEntry: From a168618892320d6f305ab9a410950ead0667977f Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 13 Aug 2021 15:29:08 -0400 Subject: [PATCH 34/35] Pull in changes from #10569 to simplify code. --- synapse/handlers/room_summary.py | 63 ++++++++++++----------------- tests/handlers/test_room_summary.py | 6 +-- 2 files changed, 28 insertions(+), 41 deletions(-) diff --git a/synapse/handlers/room_summary.py b/synapse/handlers/room_summary.py index f2e423dc810e..6663ea7f3279 100644 --- a/synapse/handlers/room_summary.py +++ b/synapse/handlers/room_summary.py @@ -190,7 +190,7 @@ async def get_space_summary( max_children = max_rooms_per_space if processed_rooms else None if is_in_room: - room_entry = await self._summarize_local_room_hierarchy( + room_entry = await self._summarize_local_room( requester, None, room_id, suggested_only, max_children ) @@ -205,7 +205,7 @@ async def get_space_summary( ["%s->%s" % (ev["room_id"], ev["state_key"]) for ev in events], ) else: - fed_rooms = await self._summarize_remote_room_hierarchy( + fed_rooms = await self._summarize_remote_room( queue_entry, suggested_only, max_children, @@ -378,7 +378,7 @@ async def _get_room_hierarchy( is_in_room = await self._store.is_host_joined(room_id, self._server_name) if is_in_room: - room_entry = await self._summarize_local_room_hierarchy( + room_entry = await self._summarize_local_room( requester, None, room_id, @@ -469,7 +469,7 @@ async def federation_space_summary( # already done this room continue - room_entry = await self._summarize_local_room_hierarchy( + room_entry = await self._summarize_local_room( None, origin, room_id, suggested_only, max_rooms_per_space ) @@ -486,7 +486,7 @@ async def federation_space_summary( return {"rooms": rooms_result, "events": events_result} - async def _summarize_local_room_hierarchy( + async def _summarize_local_room( self, requester: Optional[str], origin: Optional[str], @@ -514,13 +514,14 @@ async def _summarize_local_room_hierarchy( Returns: A room entry if the room should be returned. None, otherwise. """ - try: - room_entry = await self._summarize_local_room(requester, origin, room_id) - except NotFoundError: + if not await self._is_local_room_accessible(room_id, requester, origin): return None - # If the room is not a space, return just the room information. - if room_entry.get("room_type") != RoomTypes.SPACE: + room_entry = await self._build_room_entry(room_id, for_federation=bool(origin)) + + # If the room is not a space or the children don't matter, return just + # the room information. + if room_entry.get("room_type") != RoomTypes.SPACE or max_children == 0: return _RoomEntry(room_id, room_entry) # Otherwise, look for child rooms/spaces. @@ -546,7 +547,7 @@ async def _summarize_local_room_hierarchy( return _RoomEntry(room_id, room_entry, events_result) - async def _summarize_remote_room_hierarchy( + async def _summarize_remote_room( self, room: "_RoomQueueEntry", suggested_only: bool, @@ -887,7 +888,19 @@ async def get_room_summary( is_in_room = await self._store.is_host_joined(room_id, self._server_name) if is_in_room: - room_summary = await self._summarize_local_room(requester, None, room_id) + room_entry = await self._summarize_local_room( + requester, + None, + room_id, + # Suggested-only doesn't matter since no children are requested. + suggested_only=False, + max_children=0, + ) + + if not room_entry: + raise NotFoundError("Room not found or is not accessible") + + room_summary = room_entry.room # If there was a requester, add their membership. if requester: @@ -906,32 +919,6 @@ async def get_room_summary( return room_summary - async def _summarize_local_room( - self, - requester: Optional[str], - origin: Optional[str], - room_id: str, - ) -> JsonDict: - """ - Generate a room entry for a given room. - - Args: - requester: - The user requesting the summary, if it is a local request. None - if this is a federation request. - origin: - The server requesting the summary, if it is a federation request. - None if this is a local request. - room_id: The room ID to summarize. - - Returns: - room summary dict to return - """ - if not await self._is_local_room_accessible(room_id, requester, origin): - raise NotFoundError("Room not found or is not accessible") - - return await self._build_room_entry(room_id, for_federation=bool(origin)) - @attr.s(frozen=True, slots=True, auto_attribs=True) class _RoomQueueEntry: diff --git a/tests/handlers/test_room_summary.py b/tests/handlers/test_room_summary.py index 5ef8290eac35..2cde476371f9 100644 --- a/tests/handlers/test_room_summary.py +++ b/tests/handlers/test_room_summary.py @@ -621,7 +621,7 @@ async def summarize_remote_room( self._add_child(self.space, subspace, self.token) with mock.patch( - "synapse.handlers.room_summary.RoomSummaryHandler._summarize_remote_room_hierarchy", + "synapse.handlers.room_summary.RoomSummaryHandler._summarize_remote_room", new=summarize_remote_room, ): result = self.get_success( @@ -756,7 +756,7 @@ async def summarize_remote_room( self._add_child(self.space, subspace, self.token) with mock.patch( - "synapse.handlers.room_summary.RoomSummaryHandler._summarize_remote_room_hierarchy", + "synapse.handlers.room_summary.RoomSummaryHandler._summarize_remote_room", new=summarize_remote_room, ): result = self.get_success( @@ -820,7 +820,7 @@ async def summarize_remote_room( self._add_child(self.space, fed_room, self.token) with mock.patch( - "synapse.handlers.room_summary.RoomSummaryHandler._summarize_remote_room_hierarchy", + "synapse.handlers.room_summary.RoomSummaryHandler._summarize_remote_room", new=summarize_remote_room, ): result = self.get_success( From 4e23960b9d8432b2e4a9cb6898bb5588e4055146 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 16 Aug 2021 09:26:16 -0400 Subject: [PATCH 35/35] Inline a variable. --- synapse/rest/client/v1/room.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/synapse/rest/client/v1/room.py b/synapse/rest/client/v1/room.py index 8afd691e917b..d3882a84e2d0 100644 --- a/synapse/rest/client/v1/room.py +++ b/synapse/rest/client/v1/room.py @@ -1106,8 +1106,6 @@ async def on_GET( def register_servlets(hs: "HomeServer", http_server, is_worker=False): - msc3266_enabled = hs.config.experimental.msc3266_enabled - RoomStateEventRestServlet(hs).register(http_server) RoomMemberListRestServlet(hs).register(http_server) JoinedRoomMemberListRestServlet(hs).register(http_server) @@ -1122,7 +1120,7 @@ def register_servlets(hs: "HomeServer", http_server, is_worker=False): RoomEventContextServlet(hs).register(http_server) RoomSpaceSummaryRestServlet(hs).register(http_server) RoomHierarchyRestServlet(hs).register(http_server) - if msc3266_enabled: + if hs.config.experimental.msc3266_enabled: RoomSummaryRestServlet(hs).register(http_server) RoomEventServlet(hs).register(http_server) JoinedRoomsRestServlet(hs).register(http_server)