From 2e2d8cc2f9b9af5f8b48d75e22c474e08feca236 Mon Sep 17 00:00:00 2001 From: Jorge Florian Date: Fri, 8 Apr 2022 13:51:27 +0200 Subject: [PATCH] Update the server notices user profile in room if changed. (#12115) --- changelog.d/12115.bugfix | 1 + .../server_notices/server_notices_manager.py | 59 +++++++++++- tests/rest/admin/test_server_notice.py | 92 +++++++++++++++++++ 3 files changed, 148 insertions(+), 4 deletions(-) create mode 100644 changelog.d/12115.bugfix diff --git a/changelog.d/12115.bugfix b/changelog.d/12115.bugfix new file mode 100644 index 0000000000..eff842b2a5 --- /dev/null +++ b/changelog.d/12115.bugfix @@ -0,0 +1 @@ +Fix a long-standing bug that updating the server notices user profile (display name/avatar URL) in the configuration would not be applied to pre-existing rooms. Contributed by Jorge Florian. diff --git a/synapse/server_notices/server_notices_manager.py b/synapse/server_notices/server_notices_manager.py index 7b4814e049..48eae5fa06 100644 --- a/synapse/server_notices/server_notices_manager.py +++ b/synapse/server_notices/server_notices_manager.py @@ -16,7 +16,7 @@ from typing import TYPE_CHECKING, Optional from synapse.api.constants import EventTypes, Membership, RoomCreationPreset from synapse.events import EventBase -from synapse.types import UserID, create_requester +from synapse.types import Requester, UserID, create_requester from synapse.util.caches.descriptors import cached if TYPE_CHECKING: @@ -35,6 +35,7 @@ class ServerNoticesManager: self._room_creation_handler = hs.get_room_creation_handler() self._room_member_handler = hs.get_room_member_handler() self._event_creation_handler = hs.get_event_creation_handler() + self._message_handler = hs.get_message_handler() self._is_mine_id = hs.is_mine_id self._server_name = hs.hostname @@ -107,6 +108,10 @@ class ServerNoticesManager: assert self._is_mine_id(user_id), "Cannot send server notices to remote users" + requester = create_requester( + self.server_notices_mxid, authenticated_entity=self._server_name + ) + rooms = await self._store.get_rooms_for_local_user_where_membership_is( user_id, [Membership.INVITE, Membership.JOIN] ) @@ -125,6 +130,12 @@ class ServerNoticesManager: room.room_id, user_id, ) + await self._update_notice_user_profile_if_changed( + requester, + room.room_id, + self._config.servernotices.server_notices_mxid_display_name, + self._config.servernotices.server_notices_mxid_avatar_url, + ) return room.room_id # apparently no existing notice room: create a new one @@ -143,9 +154,6 @@ class ServerNoticesManager: "avatar_url": self._config.servernotices.server_notices_mxid_avatar_url, } - requester = create_requester( - self.server_notices_mxid, authenticated_entity=self._server_name - ) info, _ = await self._room_creation_handler.create_room( requester, config={ @@ -194,3 +202,46 @@ class ServerNoticesManager: room_id=room_id, action="invite", ) + + async def _update_notice_user_profile_if_changed( + self, + requester: Requester, + room_id: str, + display_name: Optional[str], + avatar_url: Optional[str], + ) -> None: + """ + Updates the notice user's profile if it's different from what is in the room. + + Args: + requester: The user who is performing the update. + room_id: The ID of the server notice room + display_name: The displayname of the server notice user + avatar_url: The avatar url of the server notice user + """ + logger.debug("Checking whether notice user profile has changed for %s", room_id) + + assert self.server_notices_mxid is not None + + notice_user_data_in_room = await self._message_handler.get_room_data( + self.server_notices_mxid, + room_id, + EventTypes.Member, + self.server_notices_mxid, + ) + + assert notice_user_data_in_room is not None + + notice_user_profile_changed = ( + display_name != notice_user_data_in_room.content.get("displayname") + or avatar_url != notice_user_data_in_room.content.get("avatar_url") + ) + if notice_user_profile_changed: + logger.info("Updating notice user profile in room %s", room_id) + await self._room_member_handler.update_membership( + requester=requester, + target=UserID.from_string(self.server_notices_mxid), + room_id=room_id, + action="join", + content={"displayname": display_name, "avatar_url": avatar_url}, + ) diff --git a/tests/rest/admin/test_server_notice.py b/tests/rest/admin/test_server_notice.py index a53463c9ba..dbcba2663c 100644 --- a/tests/rest/admin/test_server_notice.py +++ b/tests/rest/admin/test_server_notice.py @@ -409,6 +409,98 @@ class ServerNoticeTestCase(unittest.HomeserverTestCase): # second room has new ID self.assertNotEqual(first_room_id, second_room_id) + @override_config({"server_notices": {"system_mxid_localpart": "notices"}}) + def test_update_notice_user_name_when_changed(self) -> None: + """ + Tests that existing server notices user name in room is updated after + server notice config changes. + """ + server_notice_request_content = { + "user_id": self.other_user, + "content": {"msgtype": "m.text", "body": "test msg one"}, + } + + self.make_request( + "POST", + self.url, + access_token=self.admin_user_tok, + content=server_notice_request_content, + ) + + # simulate a change in server config after a server restart. + new_display_name = "new display name" + self.server_notices_manager._config.servernotices.server_notices_mxid_display_name = ( + new_display_name + ) + self.server_notices_manager.get_or_create_notice_room_for_user.cache.invalidate_all() + + self.make_request( + "POST", + self.url, + access_token=self.admin_user_tok, + content=server_notice_request_content, + ) + + invited_rooms = self._check_invite_and_join_status(self.other_user, 1, 0) + notice_room_id = invited_rooms[0].room_id + self.helper.join( + room=notice_room_id, user=self.other_user, tok=self.other_user_token + ) + + notice_user_state_in_room = self.helper.get_state( + notice_room_id, + "m.room.member", + self.other_user_token, + state_key="@notices:test", + ) + self.assertEqual(notice_user_state_in_room["displayname"], new_display_name) + + @override_config({"server_notices": {"system_mxid_localpart": "notices"}}) + def test_update_notice_user_avatar_when_changed(self) -> None: + """ + Tests that existing server notices user avatar in room is updated when is + different from the one in homeserver config. + """ + server_notice_request_content = { + "user_id": self.other_user, + "content": {"msgtype": "m.text", "body": "test msg one"}, + } + + self.make_request( + "POST", + self.url, + access_token=self.admin_user_tok, + content=server_notice_request_content, + ) + + # simulate a change in server config after a server restart. + new_avatar_url = "test/new-url" + self.server_notices_manager._config.servernotices.server_notices_mxid_avatar_url = ( + new_avatar_url + ) + self.server_notices_manager.get_or_create_notice_room_for_user.cache.invalidate_all() + + self.make_request( + "POST", + self.url, + access_token=self.admin_user_tok, + content=server_notice_request_content, + ) + + invited_rooms = self._check_invite_and_join_status(self.other_user, 1, 0) + notice_room_id = invited_rooms[0].room_id + self.helper.join( + room=notice_room_id, user=self.other_user, tok=self.other_user_token + ) + + notice_user_state = self.helper.get_state( + notice_room_id, + "m.room.member", + self.other_user_token, + state_key="@notices:test", + ) + self.assertEqual(notice_user_state["avatar_url"], new_avatar_url) + def _check_invite_and_join_status( self, user_id: str, expected_invites: int, expected_memberships: int ) -> List[RoomsForUser]: