From 9864efe7bee2b38e8ce2888e60c59a09d6d9a853 Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Fri, 19 Aug 2022 13:22:30 +0200 Subject: [PATCH 1/4] Fix that sending server notices fail if avatar is `None` --- synapse/handlers/room_member.py | 2 +- tests/rest/admin/test_server_notice.py | 57 +++++++++++++++++++ .../test_resource_limits_server_notices.py | 9 ++- 3 files changed, 64 insertions(+), 4 deletions(-) diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index 70dc69c80931..867af8da290d 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -689,7 +689,7 @@ async def update_membership_locked( errcode=Codes.BAD_JSON, ) - if "avatar_url" in content: + if "avatar_url" in content and content.get("avatar_url") is not None: if not await self.profile_handler.check_avatar_size_and_mime_type( content["avatar_url"], ): diff --git a/tests/rest/admin/test_server_notice.py b/tests/rest/admin/test_server_notice.py index 81e125e27da4..3037ede48e11 100644 --- a/tests/rest/admin/test_server_notice.py +++ b/tests/rest/admin/test_server_notice.py @@ -159,6 +159,63 @@ def test_invalid_parameter(self) -> None: self.assertEqual(Codes.UNKNOWN, channel.json_body["errcode"]) self.assertEqual("'msgtype' not in content", channel.json_body["error"]) + @override_config( + { + "server_notices": { + "system_mxid_localpart": "notices", + "system_mxid_avatar_url": "somthingwrong", + }, + "max_avatar_size": "10M", + } + ) + def test_invalid_avatar_url(self) -> None: + """If avatar url in homeserver.yaml is invalid and + "check avatar size and mime type" is set, an error is returned. + TODO: validate when reading config""" + channel = self.make_request( + "POST", + self.url, + access_token=self.admin_user_tok, + content={ + "user_id": self.other_user, + "content": {"msgtype": "m.text", "body": "test msg"}, + }, + ) + + self.assertEqual(500, channel.code, msg=channel.json_body) + self.assertEqual(Codes.UNKNOWN, channel.json_body["errcode"]) + + @override_config( + { + "server_notices": { + "system_mxid_localpart": "notices", + "system_mxid_display_name": "test display name", + "system_mxid_avatar_url": None, + }, + "max_avatar_size": "10M", + } + ) + def test_displayname_is_set_avatar_ist_none(self) -> None: + """ + Tests that sending a server notices is successfully, + if a display_name is set, avatar_url is `None` and + "check avatar size and mime type" is set. + """ + channel = self.make_request( + "POST", + self.url, + access_token=self.admin_user_tok, + content={ + "user_id": self.other_user, + "content": {"msgtype": "m.text", "body": "test msg"}, + }, + ) + self.assertEqual(200, channel.code, msg=channel.json_body) + + # user has one invite + invited_rooms = self._check_invite_and_join_status(self.other_user, 1, 0) + room_id = invited_rooms[0].room_id + def test_server_notice_disabled(self) -> None: """Tests that server returns error if server notice is disabled""" channel = self.make_request( diff --git a/tests/server_notices/test_resource_limits_server_notices.py b/tests/server_notices/test_resource_limits_server_notices.py index e07ae78fc4c0..bf403045e973 100644 --- a/tests/server_notices/test_resource_limits_server_notices.py +++ b/tests/server_notices/test_resource_limits_server_notices.py @@ -11,16 +11,19 @@ # 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 unittest.mock import Mock +from twisted.test.proto_helpers import MemoryReactor + from synapse.api.constants import EventTypes, LimitBlockingTypes, ServerNoticeMsgType from synapse.api.errors import ResourceLimitError from synapse.rest import admin from synapse.rest.client import login, room, sync +from synapse.server import HomeServer from synapse.server_notices.resource_limits_server_notices import ( ResourceLimitsServerNotices, ) +from synapse.util import Clock from tests import unittest from tests.test_utils import make_awaitable @@ -52,7 +55,7 @@ def default_config(self): return config - def prepare(self, reactor, clock, hs): + def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: self.server_notices_sender = self.hs.get_server_notices_sender() # relying on [1] is far from ideal, but the only case where @@ -251,7 +254,7 @@ def default_config(self): c["admin_contact"] = "mailto:user@test.com" return c - def prepare(self, reactor, clock, hs): + def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: self.store = self.hs.get_datastores().main self.server_notices_sender = self.hs.get_server_notices_sender() self.server_notices_manager = self.hs.get_server_notices_manager() From 970f7a5cb79bd65b58de80c9d87e2fa841b425af Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Fri, 19 Aug 2022 13:27:03 +0200 Subject: [PATCH 2/4] newsfile --- changelog.d/13566.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/13566.bugfix diff --git a/changelog.d/13566.bugfix b/changelog.d/13566.bugfix new file mode 100644 index 000000000000..6c44024add16 --- /dev/null +++ b/changelog.d/13566.bugfix @@ -0,0 +1 @@ +Fix a bug introduced in Synapse 1.52.0 where sending server notices fails if `max_avatar_size` or `allowed_avatar_mimetypes` is set and not `system_mxid_avatar_url`. \ No newline at end of file From 5022d55e83a595d1924ac38f97891fae6e042c91 Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Fri, 19 Aug 2022 13:28:21 +0200 Subject: [PATCH 3/4] code style --- tests/rest/admin/test_server_notice.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/rest/admin/test_server_notice.py b/tests/rest/admin/test_server_notice.py index 3037ede48e11..da8a623601b7 100644 --- a/tests/rest/admin/test_server_notice.py +++ b/tests/rest/admin/test_server_notice.py @@ -213,8 +213,7 @@ def test_displayname_is_set_avatar_ist_none(self) -> None: self.assertEqual(200, channel.code, msg=channel.json_body) # user has one invite - invited_rooms = self._check_invite_and_join_status(self.other_user, 1, 0) - room_id = invited_rooms[0].room_id + self._check_invite_and_join_status(self.other_user, 1, 0) def test_server_notice_disabled(self) -> None: """Tests that server returns error if server notice is disabled""" From a8b53fb1e2037452ef2514831b58480970cdc298 Mon Sep 17 00:00:00 2001 From: Dirk Klimpel <5740567+dklimpel@users.noreply.github.com> Date: Fri, 19 Aug 2022 14:13:02 +0200 Subject: [PATCH 4/4] typo --- tests/rest/admin/test_server_notice.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/rest/admin/test_server_notice.py b/tests/rest/admin/test_server_notice.py index da8a623601b7..a2f347f666e8 100644 --- a/tests/rest/admin/test_server_notice.py +++ b/tests/rest/admin/test_server_notice.py @@ -171,7 +171,7 @@ def test_invalid_parameter(self) -> None: def test_invalid_avatar_url(self) -> None: """If avatar url in homeserver.yaml is invalid and "check avatar size and mime type" is set, an error is returned. - TODO: validate when reading config""" + TODO: Should be checked when reading the configuration.""" channel = self.make_request( "POST", self.url, @@ -195,7 +195,7 @@ def test_invalid_avatar_url(self) -> None: "max_avatar_size": "10M", } ) - def test_displayname_is_set_avatar_ist_none(self) -> None: + def test_displayname_is_set_avatar_is_none(self) -> None: """ Tests that sending a server notices is successfully, if a display_name is set, avatar_url is `None` and