From 3ebb6694f018eedb7d3c4fda829540f07b45a5b1 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 11 Aug 2021 15:04:51 -0400 Subject: [PATCH] Allow requesting the summary of a space which is joinable. (#10580) As opposed to only allowing the summary of spaces which the user is already in or has world-readable visibility. This makes the logic consistent with whether a space/room is returned as part of a space and whether a space summary can start at a space. --- changelog.d/10580.bugfix | 1 + synapse/handlers/space_summary.py | 31 ++++++++++++++++------------ tests/handlers/test_space_summary.py | 28 +++++++++++++++++++++++-- 3 files changed, 45 insertions(+), 15 deletions(-) create mode 100644 changelog.d/10580.bugfix diff --git a/changelog.d/10580.bugfix b/changelog.d/10580.bugfix new file mode 100644 index 0000000000..f8da7382b7 --- /dev/null +++ b/changelog.d/10580.bugfix @@ -0,0 +1 @@ +Allow public rooms to be previewed in the spaces summary APIs from [MSC2946](https://github.com/matrix-org/matrix-doc/pull/2946). diff --git a/synapse/handlers/space_summary.py b/synapse/handlers/space_summary.py index 8c9852bc89..893546e661 100644 --- a/synapse/handlers/space_summary.py +++ b/synapse/handlers/space_summary.py @@ -38,7 +38,7 @@ from synapse.api.constants import ( Membership, RoomTypes, ) -from synapse.api.errors import Codes, SynapseError +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 @@ -91,7 +91,6 @@ class SpaceSummaryHandler: 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() @@ -153,9 +152,13 @@ class SpaceSummaryHandler: Returns: summary dict to return """ - # first of all, check that the user is in the room in question (or it's - # world-readable) - await self._auth.check_user_in_room_or_world_readable(room_id, requester) + # First of all, check that the room is accessible. + 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" + % (requester, room_id), + ) # the queue of rooms to process room_queue = deque((_RoomQueueEntry(room_id, ()),)) @@ -324,11 +327,13 @@ class SpaceSummaryHandler: ) -> JsonDict: """See docstring for SpaceSummaryHandler.get_room_hierarchy.""" - # first of all, check that the user is in the room in question (or it's - # world-readable) - await self._auth.check_user_in_room_or_world_readable( - requested_room_id, requester - ) + # First of all, check that the room is accessible. + 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" + % (requester, requested_room_id), + ) # If this is continuing a previous session, pull the persisted data. if from_token: @@ -612,7 +617,7 @@ class SpaceSummaryHandler: return results async def _is_local_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 in the spaces summary. @@ -766,7 +771,7 @@ class SpaceSummaryHandler: # Finally, check locally if we can access the room. The user might # already be in the room (if it was a child room), or there might be a # pending invite, etc. - return await self._is_local_room_accessible(room_id, requester, None) + return await self._is_local_room_accessible(room_id, requester) async def _build_room_entry(self, room_id: str, for_federation: bool) -> JsonDict: """ @@ -783,7 +788,7 @@ class SpaceSummaryHandler: 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 + # _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,) diff --git a/tests/handlers/test_space_summary.py b/tests/handlers/test_space_summary.py index 04da9bcc25..806b886fe4 100644 --- a/tests/handlers/test_space_summary.py +++ b/tests/handlers/test_space_summary.py @@ -248,7 +248,21 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase): user2 = self.register_user("user2", "pass") token2 = self.login("user2", "pass") - # The user cannot see the space. + # 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) @@ -260,7 +274,6 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase): tok=self.token, ) 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)) @@ -277,6 +290,7 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase): 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) @@ -284,6 +298,16 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase): 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: