Fix an edge-case with invited rooms over federation in the spaces summary. (#10560)
If a room which the requesting user was invited to was queried over federation it will now properly appear in the spaces summary (instead of being stripped out by the requesting server).
This commit is contained in:
Patrick Cloke
GitHub
parent
52bfa2d59a
commit
691593bf71
|
|
@ -0,0 +1 @@
|
|||
Add pagination to the spaces summary based on updates to [MSC2946](https://github.com/matrix-org/matrix-doc/pull/2946).
|
||||
|
|
@ -158,48 +158,10 @@ class SpaceSummaryHandler:
|
|||
room = room_entry.room
|
||||
fed_room_id = room_entry.room_id
|
||||
|
||||
# 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 spaces
|
||||
# from the response.
|
||||
allowed_rooms = room.get("allowed_room_ids") or room.get(
|
||||
"allowed_spaces"
|
||||
)
|
||||
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
|
||||
)
|
||||
|
||||
# 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(
|
||||
fed_room_id, requester, None
|
||||
)
|
||||
|
||||
# The user can see the room, include it!
|
||||
if include_room:
|
||||
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)
|
||||
|
|
@ -336,7 +298,7 @@ class SpaceSummaryHandler:
|
|||
Returns:
|
||||
A room entry if the room should be returned. None, otherwise.
|
||||
"""
|
||||
if not await self._is_room_accessible(room_id, requester, origin):
|
||||
if not await self._is_local_room_accessible(room_id, requester, origin):
|
||||
return None
|
||||
|
||||
room_entry = await self._build_room_entry(room_id, for_federation=bool(origin))
|
||||
|
|
@ -438,7 +400,7 @@ class SpaceSummaryHandler:
|
|||
|
||||
return results
|
||||
|
||||
async def _is_room_accessible(
|
||||
async def _is_local_room_accessible(
|
||||
self, room_id: str, requester: Optional[str], origin: Optional[str]
|
||||
) -> bool:
|
||||
"""
|
||||
|
|
@ -550,6 +512,51 @@ class SpaceSummaryHandler:
|
|||
)
|
||||
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, None)
|
||||
|
||||
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.
|
||||
|
|
|
|||
|
|
@ -30,7 +30,7 @@ from synapse.handlers.space_summary import _child_events_comparison_key, _RoomEn
|
|||
from synapse.rest import admin
|
||||
from synapse.rest.client.v1 import login, room
|
||||
from synapse.server import HomeServer
|
||||
from synapse.types import JsonDict
|
||||
from synapse.types import JsonDict, UserID
|
||||
|
||||
from tests import unittest
|
||||
|
||||
|
|
@ -149,6 +149,36 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase):
|
|||
events,
|
||||
)
|
||||
|
||||
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))
|
||||
|
|
@ -416,24 +446,7 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase):
|
|||
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)
|
||||
)
|
||||
self._poke_fed_invite(invited_room, "@remote:" + fed_hostname)
|
||||
|
||||
async def summarize_remote_room(
|
||||
_self, room, suggested_only, max_children, exclude_rooms
|
||||
|
|
@ -570,3 +583,58 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase):
|
|||
(subspace, joined_room),
|
||||
],
|
||||
)
|
||||
|
||||
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)
|
||||
)
|
||||
|
||||
self._assert_rooms(
|
||||
result,
|
||||
[
|
||||
self.space,
|
||||
self.room,
|
||||
fed_room,
|
||||
],
|
||||
)
|
||||
self._assert_events(
|
||||
result,
|
||||
[
|
||||
(self.space, self.room),
|
||||
(self.space, fed_room),
|
||||
],
|
||||
)
|
||||
|
|
|
|||
Loading…
Reference in New Issue