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. diff --git a/synapse/handlers/user_directory.py b/synapse/handlers/user_directory.py index 97f60b58068a..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,15 +225,20 @@ 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 - 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 is_remote: + await self._handle_profile_change( + state_key, room_id, prev_event_id, event_id + ) continue if change is MatchChange.now_true: # The user joined diff --git a/tests/handlers/test_user_directory.py b/tests/handlers/test_user_directory.py index 03fd5a3e2c52..47217f054202 100644 --- a/tests/handlers/test_user_directory.py +++ b/tests/handlers/test_user_directory.py @@ -402,6 +402,40 @@ 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()) + 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