Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update server notice user profile local #1

Open
wants to merge 12 commits into
base: develop
Choose a base branch
from
2 changes: 2 additions & 0 deletions changelog.d/1.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix bug: Server notices user profile (display_name/avatar_url) are not updated when changed after server restart.
Contributed by Jorge Florian.
1 change: 1 addition & 0 deletions changelog.d/12115.bugfix
Original file line number Diff line number Diff line change
@@ -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.
59 changes: 55 additions & 4 deletions synapse/server_notices/server_notices_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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

Expand Down Expand Up @@ -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]
)
Expand All @@ -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
Expand All @@ -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={
Expand Down Expand Up @@ -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},
)
112 changes: 111 additions & 1 deletion tests/rest/admin/test_server_notice.py
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,13 @@ def test_send_server_notice(self) -> None:
self.assertEqual(messages[1]["content"]["body"], "test msg two")
self.assertEqual(messages[1]["sender"], "@notices:test")

@override_config({"server_notices": {"system_mxid_localpart": "notices"}})
@override_config(
{
"server_notices": {
"system_mxid_localpart": "notices",
}
}
)
def test_send_server_notice_leave_room(self) -> None:
"""
Tests that sending a server notices is successfully.
Expand Down Expand Up @@ -409,6 +415,110 @@ 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]:
Expand Down