From 93e57cbf2902658dc76df23a6396dddafdeda921 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Wed, 6 Oct 2021 15:09:04 +0100 Subject: [PATCH 1/6] Test that local users' per-room profiles don't go to directory --- tests/handlers/test_user_directory.py | 35 +++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/tests/handlers/test_user_directory.py b/tests/handlers/test_user_directory.py index 03fd5a3e2c52..5a8637358d1d 100644 --- a/tests/handlers/test_user_directory.py +++ b/tests/handlers/test_user_directory.py @@ -402,6 +402,41 @@ def test_process_join_after_server_leaves_room(self) -> None: public3 = self.get_success(self.user_dir_helper.get_users_in_public_rooms()) self.assertEqual(set(public3), {(alice, room2), (bob, room2)}) + def test_per_room_profile_doesnt_alter_directory_entry(self) -> None: + alice = self.register_user("alice", "pass") + alice_token = self.login(alice, "pass") + bob = self.register_user("bob", "pass") + + # Alice should have a user directory entry created at registration. + users = self.get_success(self.user_dir_helper.get_profiles_in_user_directory()) + print(users) + self.assertEqual( + users[alice], ProfileInfo(display_name="alice", avatar_url=None) + ) + + # Alice makes a room for herself. + room = self.helper.create_room_as(alice, is_public=True, tok=alice_token) + + # Alice sets a nickname unique to that room. + self.helper.send_state( + room, + "m.room.member", + { + "displayname": "Freddy Mercury", + "membership": "join", + }, + alice_token, + state_key=alice, + ) + + # Alice's display name remains the same in the user directory. + search_result = self.get_success(self.handler.search_users(bob, alice, 10)) + self.assertEqual( + search_result["results"], + [{"display_name": "alice", "avatar_url": None, "user_id": alice}], + 0, + ) + def test_private_room(self) -> None: """ A user can be searched for only by people that are either in a public From 7ffac3c3209285e590732ae2a99c18110c60e3e2 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Wed, 6 Oct 2021 15:10:29 +0100 Subject: [PATCH 2/6] Ignore per-room profile changes for local users --- synapse/handlers/user_directory.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/synapse/handlers/user_directory.py b/synapse/handlers/user_directory.py index 97f60b58068a..132a1a9153c9 100644 --- a/synapse/handlers/user_directory.py +++ b/synapse/handlers/user_directory.py @@ -229,10 +229,14 @@ async def _handle_deltas(self, deltas: List[Dict[str, Any]]) -> None: ) or await self.store.should_include_local_user_in_dir(state_key) if include_in_dir: if change is MatchChange.no_change: - # Handle any profile changes - await self._handle_profile_change( - state_key, room_id, prev_event_id, event_id - ) + # Handle any profile changes for remote users. + # (For local users we are not forced to scan membership + # events; instead the rest of the application calls + # `_handle_local_profile_change`.) + if not self.is_mine_id(state_key): + await self._handle_profile_change( + state_key, room_id, prev_event_id, event_id + ) continue if change is MatchChange.now_true: # The user joined From 97016f38eeaf35280d791fd6d18b3041fc906932 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Wed, 6 Oct 2021 15:37:17 +0100 Subject: [PATCH 3/6] Changelog --- changelog.d/11002.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/11002.bugfix diff --git a/changelog.d/11002.bugfix b/changelog.d/11002.bugfix new file mode 100644 index 000000000000..cf894a6314b4 --- /dev/null +++ b/changelog.d/11002.bugfix @@ -0,0 +1 @@ +Fix a long-standing bug where local users' per-room nicknames/avatars were visible to anyone who could see you in the user_directory. From 69e44b0207b1cea63f06cb1334827e63cdb9feae Mon Sep 17 00:00:00 2001 From: David Robertson Date: Wed, 6 Oct 2021 16:39:58 +0100 Subject: [PATCH 4/6] Remove debug print call --- tests/handlers/test_user_directory.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/handlers/test_user_directory.py b/tests/handlers/test_user_directory.py index 5a8637358d1d..47217f054202 100644 --- a/tests/handlers/test_user_directory.py +++ b/tests/handlers/test_user_directory.py @@ -409,7 +409,6 @@ def test_per_room_profile_doesnt_alter_directory_entry(self) -> None: # Alice should have a user directory entry created at registration. users = self.get_success(self.user_dir_helper.get_profiles_in_user_directory()) - print(users) self.assertEqual( users[alice], ProfileInfo(display_name="alice", avatar_url=None) ) From 5f9e345b1c407cc32c810c431a4e1fdffc306e11 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Thu, 7 Oct 2021 11:11:12 +0100 Subject: [PATCH 5/6] Update synapse/handlers/user_directory.py Co-authored-by: Patrick Cloke --- synapse/handlers/user_directory.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/handlers/user_directory.py b/synapse/handlers/user_directory.py index 132a1a9153c9..ef0843c12556 100644 --- a/synapse/handlers/user_directory.py +++ b/synapse/handlers/user_directory.py @@ -232,7 +232,7 @@ async def _handle_deltas(self, deltas: List[Dict[str, Any]]) -> None: # Handle any profile changes for remote users. # (For local users we are not forced to scan membership # events; instead the rest of the application calls - # `_handle_local_profile_change`.) + # `handle_local_profile_change`.) if not self.is_mine_id(state_key): await self._handle_profile_change( state_key, room_id, prev_event_id, event_id From 6725348cdb37c056dbcd618e0ddd99273d847716 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Thu, 7 Oct 2021 12:10:35 +0100 Subject: [PATCH 6/6] Pull out `is_remote` --- synapse/handlers/user_directory.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/synapse/handlers/user_directory.py b/synapse/handlers/user_directory.py index ef0843c12556..b7b19733461e 100644 --- a/synapse/handlers/user_directory.py +++ b/synapse/handlers/user_directory.py @@ -203,6 +203,7 @@ async def _handle_deltas(self, deltas: List[Dict[str, Any]]) -> None: public_value=Membership.JOIN, ) + is_remote = not self.is_mine_id(state_key) if change is MatchChange.now_false: # Need to check if the server left the room entirely, if so # we might need to remove all the users in that room @@ -224,16 +225,17 @@ async def _handle_deltas(self, deltas: List[Dict[str, Any]]) -> None: else: logger.debug("Server is still in room: %r", room_id) - include_in_dir = not self.is_mine_id( - state_key - ) or await self.store.should_include_local_user_in_dir(state_key) + include_in_dir = ( + is_remote + or await self.store.should_include_local_user_in_dir(state_key) + ) if include_in_dir: if change is MatchChange.no_change: # Handle any profile changes for remote users. # (For local users we are not forced to scan membership # events; instead the rest of the application calls # `handle_local_profile_change`.) - if not self.is_mine_id(state_key): + if is_remote: await self._handle_profile_change( state_key, room_id, prev_event_id, event_id )