diff --git a/changelog.d/10298.feature b/changelog.d/10298.feature
new file mode 100644
index 0000000000..7059db5075
--- /dev/null
+++ b/changelog.d/10298.feature
@@ -0,0 +1 @@
+The spaces summary API now returns any joinable rooms, not only rooms which are world-readable.
diff --git a/changelog.d/10305.feature b/changelog.d/10305.feature
new file mode 100644
index 0000000000..7059db5075
--- /dev/null
+++ b/changelog.d/10305.feature
@@ -0,0 +1 @@
+The spaces summary API now returns any joinable rooms, not only rooms which are world-readable.
diff --git a/changelog.d/10305.misc b/changelog.d/10305.misc
deleted file mode 100644
index 8488d47f6f..0000000000
--- a/changelog.d/10305.misc
+++ /dev/null
@@ -1 +0,0 @@
-Additional unit tests for the spaces summary API.
diff --git a/synapse/handlers/space_summary.py b/synapse/handlers/space_summary.py
index b585057ec3..366e6211e5 100644
--- a/synapse/handlers/space_summary.py
+++ b/synapse/handlers/space_summary.py
@@ -24,6 +24,7 @@ from synapse.api.constants import (
EventContentFields,
EventTypes,
HistoryVisibility,
+ JoinRules,
Membership,
RoomTypes,
)
@@ -150,14 +151,21 @@ class SpaceSummaryHandler:
# 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 is in a space that has been granted access to
- # the room.
+ # 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.
- include_room = room.get("world_readable") is True
+
+ # The API doesn't return the room version so assume that a
+ # join rule of knock is valid.
+ include_room = (
+ room.get("join_rules") in (JoinRules.PUBLIC, JoinRules.KNOCK)
+ or room.get("world_readable") is True
+ )
# Check if the user is a member of any of the allowed spaces
# from the response.
@@ -420,9 +428,8 @@ class SpaceSummaryHandler:
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:
@@ -441,13 +448,39 @@ class SpaceSummaryHandler:
# 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)
- # 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,9 +503,11 @@ class SpaceSummaryHandler:
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):
+ 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
@@ -490,18 +525,10 @@ class SpaceSummaryHandler:
):
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
@@ -535,6 +562,7 @@ class SpaceSummaryHandler:
"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
),
diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py
index 2796354a1f..4d82c4c26d 100644
--- a/synapse/storage/databases/main/roommember.py
+++ b/synapse/storage/databases/main/roommember.py
@@ -703,13 +703,22 @@ class RoomMemberWorkerStore(EventsWorkerStore):
@cached(max_entries=10000)
async def is_host_joined(self, room_id: str, host: str) -> bool:
+ return await self._check_host_room_membership(room_id, host, Membership.JOIN)
+
+ @cached(max_entries=10000)
+ async def is_host_invited(self, room_id: str, host: str) -> bool:
+ return await self._check_host_room_membership(room_id, host, Membership.INVITE)
+
+ async def _check_host_room_membership(
+ self, room_id: str, host: str, membership: str
+ ) -> bool:
if "%" in host or "_" in host:
raise Exception("Invalid host name")
sql = """
SELECT state_key FROM current_state_events AS c
INNER JOIN room_memberships AS m USING (event_id)
- WHERE m.membership = 'join'
+ WHERE m.membership = ?
AND type = 'm.room.member'
AND c.room_id = ?
AND state_key LIKE ?
@@ -722,7 +731,7 @@ class RoomMemberWorkerStore(EventsWorkerStore):
like_clause = "%:" + host
rows = await self.db_pool.execute(
- "is_host_joined", None, sql, room_id, like_clause
+ "is_host_joined", None, sql, membership, room_id, like_clause
)
if not rows:
diff --git a/tests/handlers/test_space_summary.py b/tests/handlers/test_space_summary.py
index faed1f1a18..3f73ad7f94 100644
--- a/tests/handlers/test_space_summary.py
+++ b/tests/handlers/test_space_summary.py
@@ -14,8 +14,18 @@
from typing import Any, Iterable, Optional, Tuple
from unittest import mock
-from synapse.api.constants import EventContentFields, JoinRules, RoomTypes
+from synapse.api.constants import (
+ EventContentFields,
+ EventTypes,
+ HistoryVisibility,
+ JoinRules,
+ Membership,
+ RestrictedJoinRuleTypes,
+ RoomTypes,
+)
from synapse.api.errors import AuthError
+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
from synapse.rest import admin
from synapse.rest.client.v1 import login, room
@@ -117,7 +127,7 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase):
"""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,
@@ -155,29 +165,129 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase):
# The user cannot see the space.
self.get_failure(self.handler.get_space_summary(user2, self.space), AuthError)
- # Joining the room causes it to be visible.
- self.helper.join(self.space, user2, tok=token2)
+ # 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))
-
- # The result should only have the space, but includes the link to the room.
- self._assert_rooms(result, [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."""
+ # Make it not world-readable again and confirm it results in an error.
self.helper.send_state(
self.space,
- event_type="m.room.history_visibility",
- body={"history_visibility": "world_readable"},
+ event_type=EventTypes.RoomHistoryVisibility,
+ body={"history_visibility": HistoryVisibility.JOINED},
tok=self.token,
)
+ self.get_failure(self.handler.get_space_summary(user2, self.space), AuthError)
+
+ # Join the space and results should be returned.
+ self.helper.join(self.space, 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 _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")
- # The space should be visible, as well as the link to the room.
+ # 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.MSC3083_RESTRICTED,
+ room_version=RoomVersions.MSC3083.identifier,
+ allow=[],
+ )
+ restricted_accessible_room = self._create_room_with_join_rule(
+ JoinRules.MSC3083_RESTRICTED,
+ room_version=RoomVersions.MSC3083.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))
- self._assert_rooms(result, [self.space])
- self._assert_events(result, [(self.space, self.room)])
+
+ self._assert_rooms(
+ result,
+ [
+ self.space,
+ self.room,
+ public_room,
+ knock_room,
+ invited_room,
+ restricted_accessible_room,
+ world_readable_room,
+ joined_room,
+ ],
+ )
+ self._assert_events(
+ result,
+ [
+ (self.space, self.room),
+ (self.space, public_room),
+ (self.space, knock_room),
+ (self.space, not_invited_room),
+ (self.space, invited_room),
+ (self.space, restricted_room),
+ (self.space, restricted_accessible_room),
+ (self.space, world_readable_room),
+ (self.space, joined_room),
+ ],
+ )
def test_complex_space(self):
"""
@@ -186,7 +296,7 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase):
# Create an inaccessible room.
user2 = self.register_user("user2", "pass")
token2 = self.login("user2", "pass")
- room2 = self.helper.create_room_as(user2, tok=token2)
+ 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)
@@ -292,17 +402,61 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase):
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.
+ fed_handler = self.hs.get_federation_handler()
+ event = make_event_from_dict(
+ {
+ "room_id": invited_room,
+ "event_id": "!abcd:" + fed_hostname,
+ "type": EventTypes.Member,
+ "sender": "@remote:" + fed_hostname,
+ "state_key": self.user,
+ "content": {"membership": Membership.INVITE},
+ "prev_events": [],
+ "auth_events": [],
+ "depth": 1,
+ "origin_server_ts": 1234,
+ }
+ )
+ self.get_success(
+ fed_handler.on_invite_request(fed_hostname, event, RoomVersions.V6)
+ )
+
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 = [
{
+ "room_id": public_room,
+ "world_readable": False,
+ "join_rules": JoinRules.PUBLIC,
+ },
+ {
+ "room_id": knock_room,
+ "world_readable": False,
+ "join_rules": JoinRules.KNOCK,
+ },
+ {
+ "room_id": not_invited_room,
+ "world_readable": False,
+ "join_rules": JoinRules.INVITE,
+ },
+ {
+ "room_id": invited_room,
+ "world_readable": False,
+ "join_rules": JoinRules.INVITE,
+ },
+ {
"room_id": restricted_room,
"world_readable": False,
"join_rules": JoinRules.MSC3083_RESTRICTED,
@@ -364,6 +518,9 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase):
self.space,
self.room,
subspace,
+ public_room,
+ knock_room,
+ invited_room,
restricted_accessible_room,
world_readable_room,
joined_room,
@@ -374,6 +531,10 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase):
[
(self.space, self.room),
(self.space, subspace),
+ (subspace, public_room),
+ (subspace, knock_room),
+ (subspace, not_invited_room),
+ (subspace, invited_room),
(subspace, restricted_room),
(subspace, restricted_accessible_room),
(subspace, world_readable_room),
|
