Update the server notices user profile in room if changed. (#12115)

Author: watson28

Diff for: 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 for: synapse/server_notices/server_notices_manager.py

@@ -16,7 +16,7 @@
 
 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 @@ def __init__(self, hs: "HomeServer"):
         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 @@ async def get_or_create_notice_room_for_user(self, user_id: str) -> str:
 
         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 @@ async def get_or_create_notice_room_for_user(self, user_id: str) -> str:
                     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 @@ async def get_or_create_notice_room_for_user(self, user_id: str) -> str:
                 "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 @@ async def maybe_invite_user_to_room(self, user_id: str, room_id: str) -> None:
             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 for: tests/rest/admin/test_server_notice.py

@@ -409,6 +409,98 @@ def test_send_server_notice_delete_room(self) -> None:
         # 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]: