From 2c7c5c223c0100362fb4fbccd74522c1fe9c8460 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Thu, 7 Oct 2021 16:56:50 -0500 Subject: [PATCH] Fix exception thrown when attempting to notify appservice sender Fix https://github.com/matrix-org/synapse/issues/11025 Before in [`user_directory_handler._handle_deltas`, we just checked for `is_support_user(user_id)`](https://github.com/matrix-org/synapse/pull/10960/files#diff-e02a9a371e03b8615b53c6b6552f76fc7d3ef58931ca64d28b3512caf305449fL232) which works just fine. Now with https://github.com/matrix-org/synapse/pull/10960, we [call `should_include_local_user_in_dir`](https://github.com/matrix-org/synapse/pull/10960/files#diff-e02a9a371e03b8615b53c6b6552f76fc7d3ef58931ca64d28b3512caf305449fR229) which does a [bunch of additional checks](https://github.com/matrix-org/synapse/blob/e79ee48313404abf8fbb7c88361e4ab1efa29a81/synapse/storage/databases/main/user_directory.py#L382-L398) which aren't all compatible with the main appservice sender (main bridge user/sender). More specifically, we can't check the `users` database table for whether the user is deactivated. In the `should_include_local_user_in_dir` checks, we should return early if we encounter a main appservice sender before the incompatible checks. --- .../storage/databases/main/user_directory.py | 4 ++++ tests/handlers/test_user_directory.py | 21 +++++++++++++++++++ tests/storage/test_user_directory.py | 15 +++++++++++++ 3 files changed, 40 insertions(+) diff --git a/synapse/storage/databases/main/user_directory.py b/synapse/storage/databases/main/user_directory.py index 5c713a732ee9..7957af19bfe4 100644 --- a/synapse/storage/databases/main/user_directory.py +++ b/synapse/storage/databases/main/user_directory.py @@ -383,6 +383,10 @@ async def should_include_local_user_in_dir(self, user: str) -> bool: """Certain classes of local user are omitted from the user directory. Is this user one of them? """ + # The main app service sender isn't usually contactable, so exclude them + if self.get_app_service_by_user_id(user) is not None: + return False + # App service users aren't usually contactable, so exclude them. if self.get_if_app_services_interested_in_user(user): # TODO we might want to make this configurable for each app service diff --git a/tests/handlers/test_user_directory.py b/tests/handlers/test_user_directory.py index 03fd5a3e2c52..9f17662690ff 100644 --- a/tests/handlers/test_user_directory.py +++ b/tests/handlers/test_user_directory.py @@ -295,6 +295,27 @@ def test_handle_local_profile_change_with_appservice_user(self) -> None: profile = self.get_success(self.store.get_user_in_directory(as_user_id)) self.assertTrue(profile is None) + def test_handle_local_profile_change_with_appservice_sender(self) -> None: + # profile is not in directory + profile = self.get_success( + self.store.get_user_in_directory(self.appservice.sender) + ) + self.assertTrue(profile is None) + + # update profile + profile_info = ProfileInfo(avatar_url="avatar_url", display_name="4L1c3") + self.get_success( + self.handler.handle_local_profile_change( + self.appservice.sender, profile_info + ) + ) + + # profile is still not in directory + profile = self.get_success( + self.store.get_user_in_directory(self.appservice.sender) + ) + self.assertTrue(profile is None) + def test_handle_user_deactivated_support_user(self) -> None: s_user_id = "@support:test" self.get_success( diff --git a/tests/storage/test_user_directory.py b/tests/storage/test_user_directory.py index 9f483ad681c6..3831fbc8ef2d 100644 --- a/tests/storage/test_user_directory.py +++ b/tests/storage/test_user_directory.py @@ -364,6 +364,21 @@ def test_population_excludes_appservice_user(self) -> None: # Check the AS user is not in the directory. self._check_room_sharing_tables(user, public, private) + def test_population_excludes_appservice_sender(self) -> None: + user = self.register_user("user", "pass") + token = self.login(user, "pass") + + # Join the AS sender to rooms owned by the normal user. + public, private = self._create_rooms_and_inject_memberships( + user, token, self.appservice.sender + ) + + # Rebuild the directory. + self._purge_and_rebuild_user_dir() + + # Check the AS sender is not in the directory. + self._check_room_sharing_tables(user, public, private) + def test_population_conceals_private_nickname(self) -> None: # Make a private room, and set a nickname within user = self.register_user("aaaa", "pass")