From e0bf34dada709776ae00843e47cd811d1cd195c6 Mon Sep 17 00:00:00 2001
From: David Robertson <davidr@element.io>
Date: Thu, 7 Oct 2021 13:26:11 +0100
Subject: [PATCH] Don't alter directory entries for local users when setting a
 per-room nickname (#11002)

Co-authored-by: Patrick Cloke <clokep@users.noreply.github.com>
---
 changelog.d/11002.bugfix              |  1 +
 synapse/handlers/user_directory.py    | 20 ++++++++++------
 tests/handlers/test_user_directory.py | 34 +++++++++++++++++++++++++++
 3 files changed, 48 insertions(+), 7 deletions(-)
 create mode 100644 changelog.d/11002.bugfix

diff --git a/changelog.d/11002.bugfix b/changelog.d/11002.bugfix
new file mode 100644
index 000000000..cf894a631
--- /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 97f60b580..b7b197334 100644
--- a/synapse/handlers/user_directory.py
+++ b/synapse/handlers/user_directory.py
@@ -203,6 +203,7 @@ class UserDirectoryHandler(StateDeltasHandler):
                     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 @@ class UserDirectoryHandler(StateDeltasHandler):
                     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 03fd5a3e2..47217f054 100644
--- a/tests/handlers/test_user_directory.py
+++ b/tests/handlers/test_user_directory.py
@@ -402,6 +402,40 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase):
         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