Show joinable rooms in the spaces summary.

changelog.d/10298.feature

@@ -0,0 +1 @@
+The spaces summary API now returns any joinable rooms, not only rooms which are world-readable.

synapse/handlers/space_summary.py

@@ -24,6 +24,7 @@
     EventContentFields,
     EventTypes,
     HistoryVisibility,
+    JoinRules,
     Membership,
     RoomTypes,
 )
@@ -420,9 +421,8 @@ async def _is_room_accessible(
 
         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 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:
@@ -446,8 +446,30 @@ async def _is_room_accessible(
 
         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.
+        # 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)
 
@@ -470,11 +492,14 @@ async def _is_room_accessible(
                     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.
+        # has a user who could join the room.
         elif origin:
             if await self._event_auth_handler.check_host_in_room(room_id, origin):
                 return True
 
+            # TODO This does not handle if a server has a pending invite to the
+            # room.
+
             # 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).
@@ -490,18 +515,10 @@ async def _is_room_accessible(
                     ):
                         return True
 
-        # otherwise, check if the room is peekable
-        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 / not allowed to join, omitting from summary",
+            "room %s is unpeekable and requester %s is not a member / not allowed to join, omitting from summary",
             room_id,
-            requester,
+            requester or origin,
         )
         return False
 

tests/handlers/test_space_summary.py

@@ -14,7 +14,13 @@
 from typing import Any, Iterable, Optional, Tuple
 from unittest import mock
 
-from synapse.api.constants import EventContentFields, RoomTypes
+from synapse.api.constants import (
+    EventContentFields,
+    EventTypes,
+    HistoryVisibility,
+    JoinRules,
+    RoomTypes,
+)
 from synapse.api.errors import AuthError
 from synapse.handlers.space_summary import _child_events_comparison_key
 from synapse.rest import admin
@@ -117,7 +123,7 @@ def _add_child(self, space_id: str, room_id: str, token: str) -> None:
         """Add a child room to a space."""
         self.helper.send_state(
             space_id,
-            event_type="m.space.child",
+            event_type=EventTypes.SpaceChild,
             body={"via": [self.hs.hostname]},
             tok=token,
             state_key=room_id,
@@ -148,7 +154,11 @@ def test_simple_space(self):
         self._assert_events(result, [(self.space, self.room)])
 
     def test_visibility(self):
-        """A user not in a space cannot inspect it."""
+        """
+        A user not in a space cannot inspect it.
+
+        Once in the space, they only see joined & joinable rooms.
+        """
         user2 = self.register_user("user2", "pass")
         token2 = self.login("user2", "pass")
 
@@ -159,22 +169,63 @@ def test_visibility(self):
         self.helper.join(self.space, user2, tok=token2)
         result = self.get_success(self.handler.get_space_summary(user2, self.space))
 
-        # The result should only have the space, but includes the link to the room.
+        # The result should have the space and room.
+        self._assert_rooms(result, [self.space, self.room])
+        self._assert_events(result, [(self.space, self.room)])
+
+        # Make the room's state only viewable to those in it.
+        self.helper.send_state(
+            self.room,
+            event_type=EventTypes.RoomHistoryVisibility,
+            body={"history_visibility": HistoryVisibility.JOINED},
+            tok=self.token,
+        )
+
+        # If the room is made invite-only, it disappears...
+        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_space_summary(user2, self.space))
         self._assert_rooms(result, [self.space])
         self._assert_events(result, [(self.space, self.room)])
 
+        # Until the user has an invite.
+        self.helper.invite(self.room, targ=user2, tok=self.token)
+        result = self.get_success(self.handler.get_space_summary(user2, self.space))
+        self._assert_rooms(result, [self.space, self.room])
+        self._assert_events(result, [(self.space, self.room)])
+
+        # Or joins it.
+        self.helper.join(self.room, user2, tok=token2)
+        result = self.get_success(self.handler.get_space_summary(user2, self.space))
+        self._assert_rooms(result, [self.space, self.room])
+        self._assert_events(result, [(self.space, self.room)])
+
     def test_world_readable(self):
-        """A world-readable room is visible to everyone."""
+        """A world-readable space is visible to everyone."""
         self.helper.send_state(
             self.space,
-            event_type="m.room.history_visibility",
-            body={"history_visibility": "world_readable"},
+            event_type=EventTypes.JoinRules,
+            body={"join_rule": JoinRules.INVITE},
             tok=self.token,
         )
 
         user2 = self.register_user("user2", "pass")
 
+        # The space should not be visible.
+        self.get_failure(self.handler.get_space_summary(user2, self.space), AuthError)
+
+        self.helper.send_state(
+            self.space,
+            event_type=EventTypes.RoomHistoryVisibility,
+            body={"history_visibility": HistoryVisibility.WORLD_READABLE},
+            tok=self.token,
+        )
+
         # The space should be visible, as well as the link to the room.
         result = self.get_success(self.handler.get_space_summary(user2, self.space))
-        self._assert_rooms(result, [self.space])
+        self._assert_rooms(result, [self.space, self.room])
         self._assert_events(result, [(self.space, self.room)])