Allow a user who could join a restricted room to see it in spaces summary.

changelog.d/9922.feature

@@ -0,0 +1 @@
+Experimental support to allow a user who could join a restricted room to view it in the spaces summary.

synapse/federation/transport/server.py

@@ -1428,7 +1428,7 @@ async def on_POST(
             )
 
         return 200, await self.handler.federation_space_summary(
-            room_id, suggested_only, max_rooms_per_space, exclude_rooms
+            origin, room_id, suggested_only, max_rooms_per_space, exclude_rooms
         )
 
 

synapse/handlers/event_auth.py

@@ -11,7 +11,7 @@
 # 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 TYPE_CHECKING
+from typing import TYPE_CHECKING, Collection, Tuple
 
 from synapse.api.constants import EventTypes, JoinRules
 from synapse.api.room_versions import RoomVersion
@@ -29,46 +29,42 @@ class EventAuthHandler:
     def __init__(self, hs: "HomeServer"):
         self._store = hs.get_datastore()
 
-    async def can_join_without_invite(
-        self, state_ids: StateMap[str], room_version: RoomVersion, user_id: str
-    ) -> bool:
+    async def get_allowed_spaces(
+        self, state_ids: StateMap[str], room_version: RoomVersion
+    ) -> Tuple[bool, Collection[str]]:
         """
-        Check whether a user can join a room without an invite.
-
-        When joining a room with restricted joined rules (as defined in MSC3083),
-        the membership of spaces must be checked during join.
+        Generate a list of spaces allow access to a room.
 
         Args:
             state_ids: The state of the room as it currently is.
             room_version: The room version of the room being joined.
-            user_id: The user joining the room.
 
         Returns:
-            True if the user can join the room, false otherwise.
+            A tuple:
+                True if spaces give access to the room.
+                A collection of spaces (if any) which provide membership to the room.
         """
         # This only applies to room versions which support the new join rule.
         if not room_version.msc3083_join_rules:
-            return True
+            return False, ()
 
         # If there's no join rule, then it defaults to invite (so this doesn't apply).
         join_rules_event_id = state_ids.get((EventTypes.JoinRules, ""), None)
         if not join_rules_event_id:
-            return True
+            return False, ()
 
         # If the join rule is not restricted, this doesn't apply.
         join_rules_event = await self._store.get_event(join_rules_event_id)
         if join_rules_event.content.get("join_rule") != JoinRules.MSC3083_RESTRICTED:
-            return True
+            return False, ()
 
         # If allowed is of the wrong form, then only allow invited users.
         allowed_spaces = join_rules_event.content.get("allow", [])
         if not isinstance(allowed_spaces, list):
-            return False
-
-        # Get the list of joined rooms and see if there's an overlap.
-        joined_rooms = await self._store.get_rooms_for_user(user_id)
+            return True, ()
 
-        # Pull out the other room IDs, invalid data gets filtered.
+        # Pull out the room IDs, invalid data gets filtered.
+        result = []
         for space in allowed_spaces:
             if not isinstance(space, dict):
                 continue
@@ -79,6 +75,41 @@ async def can_join_without_invite(
 
             # The user was joined to one of the spaces specified, they can join
             # this room!
+            result.append(space_id)
+
+        return True, result
+
+    async def can_join_without_invite(
+        self, state_ids: StateMap[str], room_version: RoomVersion, user_id: str
+    ) -> bool:
+        """
+        Check whether a user can join a room without an invite.
+
+        When joining a room with restricted joined rules (as defined in MSC3083),
+        the membership of spaces must be checked during join.
+
+        Args:
+            state_ids: The state of the room as it currently is.
+            room_version: The room version of the room being joined.
+            user_id: The user joining the room.
+
+        Returns:
+            True if the user can join the room, false otherwise.
+        """
+        # Get whether spaces allow access to the room and the allowed spaces.
+        allow_via_spaces, allowed_spaces = await self.get_allowed_spaces(
+            state_ids, room_version
+        )
+        # Access via spaces doesn't apply to this room.
+        if not allow_via_spaces:
+            return True
+
+        # Get the list of joined rooms and see if there's an overlap.
+        joined_rooms = await self._store.get_rooms_for_user(user_id)
+
+        # If the user was joined to one of the spaces specified, they can join
+        # this room!
+        for space_id in allowed_spaces:
             if space_id in joined_rooms:
                 return True
 

synapse/handlers/space_summary.py

@@ -20,8 +20,12 @@
 
 import attr
 
-from synapse.api.constants import EventContentFields, EventTypes, HistoryVisibility
-from synapse.api.errors import AuthError
+from synapse.api.constants import (
+    EventContentFields,
+    EventTypes,
+    HistoryVisibility,
+    Membership,
+)
 from synapse.events import EventBase
 from synapse.events.utils import format_event_for_client_v2
 from synapse.types import JsonDict
@@ -47,6 +51,7 @@ def __init__(self, hs: "HomeServer"):
         self._auth = hs.get_auth()
         self._room_list_handler = hs.get_room_list_handler()
         self._state_handler = hs.get_state_handler()
+        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
@@ -111,28 +116,54 @@ async def get_space_summary(
             max_children = max_rooms_per_space if processed_rooms else None
 
             if is_in_room:
-                rooms, events = await self._summarize_local_room(
-                    requester, room_id, suggested_only, max_children
+                room, events = await self._summarize_local_room(
+                    requester, None, room_id, suggested_only, max_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],
+                )
+
+                if room:
+                    rooms_result.append(room)
             else:
-                rooms, events = await self._summarize_remote_room(
+                fed_rooms, fed_events = await self._summarize_remote_room(
                     queue_entry,
                     suggested_only,
                     max_children,
                     exclude_rooms=processed_rooms,
                 )
 
-            logger.debug(
-                "Query of %s returned rooms %s, events %s",
-                queue_entry.room_id,
-                [room.get("room_id") for room in rooms],
-                ["%s->%s" % (ev["room_id"], ev["state_key"]) for ev in events],
-            )
-
-            rooms_result.extend(rooms)
-
-            # any rooms returned don't need visiting again
-            processed_rooms.update(cast(str, room.get("room_id")) for room in 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. Handle that filtering.
+                room_ids = set()
+                rooms = []  # type: List[JsonDict]
+                events = []
+                for room in fed_rooms:
+                    fed_room_id = room.get("room_id")
+                    if fed_room_id and await self._is_room_accessible(
+                        fed_room_id, requester, None
+                    ):
+                        rooms_result.append(room)
+                        room_ids.add(fed_room_id)
+
+                    # any rooms returned don't need visiting again (even if the user
+                    # didn't have access to them).
+                    processed_rooms.add(cast(str, room_id))
+
+                for event in fed_events:
+                    if event.get("room_id") in room_ids:
+                        events.append(event)
+
+                logger.debug(
+                    "Query of %s returned rooms %s, events %s",
+                    room_id,
+                    [room.get("room_id") for room in 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.
@@ -162,6 +193,7 @@ async def get_space_summary(
 
     async def federation_space_summary(
         self,
+        origin: str,
         room_id: str,
         suggested_only: bool,
         max_rooms_per_space: Optional[int],
@@ -171,6 +203,8 @@ async def federation_space_summary(
         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"
@@ -205,14 +239,15 @@ async def federation_space_summary(
 
             logger.debug("Processing room %s", room_id)
 
-            rooms, events = await self._summarize_local_room(
-                None, room_id, suggested_only, max_rooms_per_space
+            room, events = await self._summarize_local_room(
+                None, origin, room_id, suggested_only, max_rooms_per_space
             )
 
             processed_rooms.add(room_id)
 
-            rooms_result.extend(rooms)
-            events_result.extend(events)
+            if room:
+                rooms_result.append(room)
+                events_result.extend(events)
 
             # add any children to the queue
             room_queue.extend(edge_event["state_key"] for edge_event in events)
@@ -222,17 +257,21 @@ async def federation_space_summary(
     async def _summarize_local_room(
         self,
         requester: Optional[str],
+        origin: Optional[str],
         room_id: str,
         suggested_only: bool,
         max_children: Optional[int],
-    ) -> Tuple[Sequence[JsonDict], Sequence[JsonDict]]:
+    ) -> Tuple[Optional[JsonDict], Sequence[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.
             suggested_only: True if only suggested children should be returned.
                 Otherwise, all children are returned.
@@ -247,8 +286,8 @@ 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):
-            return (), ()
+        if not await self._is_room_accessible(room_id, requester, origin):
+            return None, ()
 
         room_entry = await self._build_room_entry(room_id)
 
@@ -272,7 +311,7 @@ async def _summarize_local_room(
                     event_format=format_event_for_client_v2,
                 )
             )
-        return (room_entry,), events_result
+        return room_entry, events_result
 
     async def _summarize_remote_room(
         self,
@@ -332,45 +371,81 @@ async def _summarize_remote_room(
             or ev.event_type == EventTypes.SpaceChild
         )
 
-    async def _is_room_accessible(self, room_id: str, requester: Optional[str]) -> bool:
+    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 invited to the room.
+        * The requester can join without an invite (per MSC3083).
+        * The origin server has any user that is joined or invited to 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)
+        room_version = await self._store.get_room_version(room_id)
 
         # if we have an authenticated requesting user, first check if they are able to view
         # stripped state in the room.
         if requester:
-            try:
-                await self._auth.check_user_in_room(room_id, 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, if they should be allowed access via membership in a space.
+            if await self._event_auth_handler.can_join_without_invite(
+                state_ids, room_version, requester
+            ):
+                return True
+
+        # If this is a request over federation, check if the host is in the room or
+        # is in one of the spaces specified via the join rules.
+        elif origin:
+            if await self._auth.check_host_in_room(room_id, origin):
                 return True
-            except AuthError:
-                pass
+
+            # 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).
+            (
+                allow_via_spaces,
+                allowed_spaces,
+            ) = await self._event_auth_handler.get_allowed_spaces(
+                state_ids, room_version
+            )
+            if allow_via_spaces:
+                for space_id in allowed_spaces:
+                    if await self._auth.check_host_in_room(space_id, origin):
+                        return True
 
         # otherwise, check if the room is peekable
-        hist_vis_ev = await self._state_handler.get_current_state(
-            room_id, EventTypes.RoomHistoryVisibility, ""
-        )
-        if hist_vis_ev:
+        hist_vis_event_id = state_ids.get((EventTypes.RoomHistoryVisibility, ""), None)
+        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
 
         logger.info(
-            "room %s is unpeekable and user %s is not a member, omitting from summary",
+            "room %s is unpeekable and user %s is not a member / not allowed to join, omitting from summary",
             room_id,
             requester,
         )