Fix a missing await when in the spaces summary. (#10208)
This could cause a minor data leak if someone defined a non-restricted join rule with an allow key or used a restricted join rule in an older room version, but this is unlikely. Additionally this starts adding unit tests to the spaces summary handler.pull/10221/head
parent
e9f2ad8603
commit
0bd968921c
|
@ -0,0 +1 @@
|
||||||
|
Fix a bug introduced in v1.35.1 where an `allow` key of a `m.room.join_rules` event could be applied for incorrect room versions and configurations.
|
|
@ -445,14 +445,13 @@ class SpaceSummaryHandler:
|
||||||
member_event_id = state_ids.get((EventTypes.Member, requester), None)
|
member_event_id = state_ids.get((EventTypes.Member, requester), None)
|
||||||
|
|
||||||
# If they're in the room they can see info on it.
|
# If they're in the room they can see info on it.
|
||||||
member_event = None
|
|
||||||
if member_event_id:
|
if member_event_id:
|
||||||
member_event = await self._store.get_event(member_event_id)
|
member_event = await self._store.get_event(member_event_id)
|
||||||
if member_event.membership in (Membership.JOIN, Membership.INVITE):
|
if member_event.membership in (Membership.JOIN, Membership.INVITE):
|
||||||
return True
|
return True
|
||||||
|
|
||||||
# Otherwise, check if they should be allowed access via membership in a space.
|
# Otherwise, check if they should be allowed access via membership in a space.
|
||||||
if self._event_auth_handler.has_restricted_join_rules(
|
if await self._event_auth_handler.has_restricted_join_rules(
|
||||||
state_ids, room_version
|
state_ids, room_version
|
||||||
):
|
):
|
||||||
allowed_rooms = (
|
allowed_rooms = (
|
||||||
|
|
|
@ -11,10 +11,15 @@
|
||||||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||||
# See the License for the specific language governing permissions and
|
# See the License for the specific language governing permissions and
|
||||||
# limitations under the License.
|
# limitations under the License.
|
||||||
from typing import Any, Optional
|
from typing import Any, Iterable, Optional, Tuple
|
||||||
from unittest import mock
|
from unittest import mock
|
||||||
|
|
||||||
|
from synapse.api.errors import AuthError
|
||||||
from synapse.handlers.space_summary import _child_events_comparison_key
|
from synapse.handlers.space_summary import _child_events_comparison_key
|
||||||
|
from synapse.rest import admin
|
||||||
|
from synapse.rest.client.v1 import login, room
|
||||||
|
from synapse.server import HomeServer
|
||||||
|
from synapse.types import JsonDict
|
||||||
|
|
||||||
from tests import unittest
|
from tests import unittest
|
||||||
|
|
||||||
|
@ -79,3 +84,95 @@ class TestSpaceSummarySort(unittest.TestCase):
|
||||||
|
|
||||||
ev1 = _create_event("!abc:test", "a" * 51)
|
ev1 = _create_event("!abc:test", "a" * 51)
|
||||||
self.assertEqual([ev2, ev1], _order(ev1, ev2))
|
self.assertEqual([ev2, ev1], _order(ev1, ev2))
|
||||||
|
|
||||||
|
|
||||||
|
class SpaceSummaryTestCase(unittest.HomeserverTestCase):
|
||||||
|
servlets = [
|
||||||
|
admin.register_servlets_for_client_rest_resource,
|
||||||
|
room.register_servlets,
|
||||||
|
login.register_servlets,
|
||||||
|
]
|
||||||
|
|
||||||
|
def prepare(self, reactor, clock, hs: HomeServer):
|
||||||
|
self.hs = hs
|
||||||
|
self.handler = self.hs.get_space_summary_handler()
|
||||||
|
|
||||||
|
self.user = self.register_user("user", "pass")
|
||||||
|
self.token = self.login("user", "pass")
|
||||||
|
|
||||||
|
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",
|
||||||
|
body={"via": [self.hs.hostname]},
|
||||||
|
tok=token,
|
||||||
|
state_key=room_id,
|
||||||
|
)
|
||||||
|
|
||||||
|
def _assert_rooms(self, result: JsonDict, rooms: Iterable[str]) -> None:
|
||||||
|
"""Assert that the expected room IDs are in the response."""
|
||||||
|
self.assertCountEqual([room.get("room_id") for room in result["rooms"]], rooms)
|
||||||
|
|
||||||
|
def _assert_events(
|
||||||
|
self, result: JsonDict, events: Iterable[Tuple[str, str]]
|
||||||
|
) -> None:
|
||||||
|
"""Assert that the expected parent / child room IDs are in the response."""
|
||||||
|
self.assertCountEqual(
|
||||||
|
[
|
||||||
|
(event.get("room_id"), event.get("state_key"))
|
||||||
|
for event in result["events"]
|
||||||
|
],
|
||||||
|
events,
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_simple_space(self):
|
||||||
|
"""Test a simple space with a single room."""
|
||||||
|
space = self.helper.create_room_as(self.user, tok=self.token)
|
||||||
|
room = self.helper.create_room_as(self.user, tok=self.token)
|
||||||
|
self._add_child(space, room, self.token)
|
||||||
|
|
||||||
|
result = self.get_success(self.handler.get_space_summary(self.user, space))
|
||||||
|
# The result should have the space and the room in it, along with a link
|
||||||
|
# from space -> room.
|
||||||
|
self._assert_rooms(result, [space, room])
|
||||||
|
self._assert_events(result, [(space, room)])
|
||||||
|
|
||||||
|
def test_visibility(self):
|
||||||
|
"""A user not in a space cannot inspect it."""
|
||||||
|
space = self.helper.create_room_as(self.user, tok=self.token)
|
||||||
|
room = self.helper.create_room_as(self.user, tok=self.token)
|
||||||
|
self._add_child(space, room, self.token)
|
||||||
|
|
||||||
|
user2 = self.register_user("user2", "pass")
|
||||||
|
token2 = self.login("user2", "pass")
|
||||||
|
|
||||||
|
# The user cannot see the space.
|
||||||
|
self.get_failure(self.handler.get_space_summary(user2, space), AuthError)
|
||||||
|
|
||||||
|
# Joining the room causes it to be visible.
|
||||||
|
self.helper.join(space, user2, tok=token2)
|
||||||
|
result = self.get_success(self.handler.get_space_summary(user2, space))
|
||||||
|
|
||||||
|
# The result should only have the space, but includes the link to the room.
|
||||||
|
self._assert_rooms(result, [space])
|
||||||
|
self._assert_events(result, [(space, room)])
|
||||||
|
|
||||||
|
def test_world_readable(self):
|
||||||
|
"""A world-readable room is visible to everyone."""
|
||||||
|
space = self.helper.create_room_as(self.user, tok=self.token)
|
||||||
|
room = self.helper.create_room_as(self.user, tok=self.token)
|
||||||
|
self._add_child(space, room, self.token)
|
||||||
|
self.helper.send_state(
|
||||||
|
space,
|
||||||
|
event_type="m.room.history_visibility",
|
||||||
|
body={"history_visibility": "world_readable"},
|
||||||
|
tok=self.token,
|
||||||
|
)
|
||||||
|
|
||||||
|
user2 = self.register_user("user2", "pass")
|
||||||
|
|
||||||
|
# The space should be visible, as well as the link to the room.
|
||||||
|
result = self.get_success(self.handler.get_space_summary(user2, space))
|
||||||
|
self._assert_rooms(result, [space])
|
||||||
|
self._assert_events(result, [(space, room)])
|
||||||
|
|
Loading…
Reference in New Issue