From b4501c1c33b9f6ac5710b121f119643a34df1d83 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 18 Jun 2021 09:55:20 -0400 Subject: [PATCH 1/7] Add a simple spaces summary test. --- tests/handlers/test_space_summary.py | 44 ++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/tests/handlers/test_space_summary.py b/tests/handlers/test_space_summary.py index 2c5e81531b9e..c226995ba90c 100644 --- a/tests/handlers/test_space_summary.py +++ b/tests/handlers/test_space_summary.py @@ -15,6 +15,9 @@ from unittest import mock 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 tests import unittest @@ -79,3 +82,44 @@ def test_invalid_ordering_value(self): ev1 = _create_event("!abc:test", "a" * 51) 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() + + def test_simple_space(self): + """Test a simple space with a single room.""" + user = self.register_user("user", "pass") + token = self.login("user", "pass") + + space = self.helper.create_room_as(user, tok=token) + room = self.helper.create_room_as(user, tok=token) + self.helper.send_state( + space, + event_type="m.space.child", + body={"via": [self.hs.hostname]}, + tok=token, + state_key=room, + ) + + result = self.get_success(self.handler.get_space_summary(user, space)) + # The result should have the space and the room in it, along with a link + # from space -> room. + self.assertCountEqual( + [room.get("room_id") for room in result["rooms"]], [space, room] + ) + self.assertCountEqual( + [ + (event.get("room_id"), event.get("state_key")) + for event in result["events"] + ], + [(space, room)], + ) From 596bab19f592baa44e11c76467361a2c1506659e Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 18 Jun 2021 10:04:00 -0400 Subject: [PATCH 2/7] Abstract assertions. --- tests/handlers/test_space_summary.py | 43 ++++++++++++++++++---------- 1 file changed, 28 insertions(+), 15 deletions(-) diff --git a/tests/handlers/test_space_summary.py b/tests/handlers/test_space_summary.py index c226995ba90c..7e8edbc07d5f 100644 --- a/tests/handlers/test_space_summary.py +++ b/tests/handlers/test_space_summary.py @@ -11,13 +11,14 @@ # 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 Any, Optional +from typing import Any, Iterable, Optional, Tuple from unittest import mock 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 @@ -95,31 +96,43 @@ def prepare(self, reactor, clock, hs: HomeServer): self.hs = hs self.handler = self.hs.get_space_summary_handler() - def test_simple_space(self): - """Test a simple space with a single room.""" - user = self.register_user("user", "pass") - token = self.login("user", "pass") - - space = self.helper.create_room_as(user, tok=token) - room = self.helper.create_room_as(user, tok=token) + 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, + space_id, event_type="m.space.child", body={"via": [self.hs.hostname]}, tok=token, - state_key=room, + state_key=room_id, ) - result = self.get_success(self.handler.get_space_summary(user, space)) - # The result should have the space and the room in it, along with a link - # from space -> room. + 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"]], [space, room] + [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"] ], - [(space, room)], + events, ) + + def test_simple_space(self): + """Test a simple space with a single room.""" + user = self.register_user("user", "pass") + token = self.login("user", "pass") + + space = self.helper.create_room_as(user, tok=token) + room = self.helper.create_room_as(user, tok=token) + self._add_child(space, room, token) + + result = self.get_success(self.handler.get_space_summary(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)]) From adbb15a7a365cb6729e7e3cb4e0f61727acef3a0 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 18 Jun 2021 10:27:57 -0400 Subject: [PATCH 3/7] Add a test for visibility. --- synapse/handlers/space_summary.py | 2 +- tests/handlers/test_space_summary.py | 32 ++++++++++++++++++++++++---- 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/synapse/handlers/space_summary.py b/synapse/handlers/space_summary.py index e953a8afe6b7..38f863d78c59 100644 --- a/synapse/handlers/space_summary.py +++ b/synapse/handlers/space_summary.py @@ -452,7 +452,7 @@ async def _is_room_accessible( return True # 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 ): allowed_rooms = ( diff --git a/tests/handlers/test_space_summary.py b/tests/handlers/test_space_summary.py index 7e8edbc07d5f..8d1e88eac6b0 100644 --- a/tests/handlers/test_space_summary.py +++ b/tests/handlers/test_space_summary.py @@ -14,6 +14,7 @@ from typing import Any, Iterable, Optional, Tuple from unittest import mock +from synapse.api.errors import AuthError from synapse.handlers.space_summary import _child_events_comparison_key from synapse.rest import admin from synapse.rest.client.v1 import login, room @@ -108,11 +109,11 @@ def _add_child(self, space_id: str, room_id: str, token: str) -> None: 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 - ) + self.assertCountEqual([room.get("room_id") for room in result["rooms"]], rooms) - def _assert_events(self, result: JsonDict, events: Iterable[Tuple[str, str]]) -> None: + 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( [ @@ -136,3 +137,26 @@ def test_simple_space(self): # 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.""" + user = self.register_user("user", "pass") + token = self.login("user", "pass") + + space = self.helper.create_room_as(user, tok=token) + room = self.helper.create_room_as(user, tok=token) + self._add_child(space, room, 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 has the space, but includes the link to the room. + self._assert_rooms(result, [space]) + self._assert_events(result, [(space, room)]) From 51988b731841cfca0fec94f66adbe9742bf2e727 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 18 Jun 2021 10:35:10 -0400 Subject: [PATCH 4/7] Add a test for world-readable rooms. --- tests/handlers/test_space_summary.py | 42 +++++++++++++++++++--------- 1 file changed, 29 insertions(+), 13 deletions(-) diff --git a/tests/handlers/test_space_summary.py b/tests/handlers/test_space_summary.py index 8d1e88eac6b0..c618b16bd1a8 100644 --- a/tests/handlers/test_space_summary.py +++ b/tests/handlers/test_space_summary.py @@ -97,6 +97,9 @@ 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( @@ -125,14 +128,11 @@ def _assert_events( def test_simple_space(self): """Test a simple space with a single room.""" - user = self.register_user("user", "pass") - token = self.login("user", "pass") - - space = self.helper.create_room_as(user, tok=token) - room = self.helper.create_room_as(user, tok=token) - self._add_child(space, room, token) + 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(user, space)) + 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]) @@ -140,12 +140,9 @@ def test_simple_space(self): def test_visibility(self): """A user not in a space cannot inspect it.""" - user = self.register_user("user", "pass") - token = self.login("user", "pass") - - space = self.helper.create_room_as(user, tok=token) - room = self.helper.create_room_as(user, tok=token) - self._add_child(space, room, token) + 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") @@ -160,3 +157,22 @@ def test_visibility(self): # The result should only has 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)]) From 6a3f2205599b92ce159fb3cc6e3eb61c63c51c71 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 18 Jun 2021 10:37:52 -0400 Subject: [PATCH 5/7] Remove an unused variable. --- synapse/handlers/space_summary.py | 1 - 1 file changed, 1 deletion(-) diff --git a/synapse/handlers/space_summary.py b/synapse/handlers/space_summary.py index 38f863d78c59..17fc47ce1630 100644 --- a/synapse/handlers/space_summary.py +++ b/synapse/handlers/space_summary.py @@ -445,7 +445,6 @@ async def _is_room_accessible( member_event_id = state_ids.get((EventTypes.Member, requester), None) # If they're in the room they can see info on it. - member_event = None if member_event_id: member_event = await self._store.get_event(member_event_id) if member_event.membership in (Membership.JOIN, Membership.INVITE): From 6a5ee4405e21bf3b15d477af81d3ca98bbae52e8 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 18 Jun 2021 10:43:58 -0400 Subject: [PATCH 6/7] Newsfragment --- changelog.d/10208.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/10208.bugfix diff --git a/changelog.d/10208.bugfix b/changelog.d/10208.bugfix new file mode 100644 index 000000000000..32b646571702 --- /dev/null +++ b/changelog.d/10208.bugfix @@ -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. From 14d97416226db311258cc4719990b73ea21c90bf Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 18 Jun 2021 13:14:12 -0400 Subject: [PATCH 7/7] Fix typo. Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> --- tests/handlers/test_space_summary.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/handlers/test_space_summary.py b/tests/handlers/test_space_summary.py index c618b16bd1a8..131d362ccc81 100644 --- a/tests/handlers/test_space_summary.py +++ b/tests/handlers/test_space_summary.py @@ -154,7 +154,7 @@ def test_visibility(self): self.helper.join(space, user2, tok=token2) result = self.get_success(self.handler.get_space_summary(user2, space)) - # The result should only has the space, but includes the link to the room. + # 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)])