Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Commit

Permalink
Fix exception thrown when attempting to notify appservice sender
Browse files Browse the repository at this point in the history
Fix #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 #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.
  • Loading branch information
MadLittleMods committed Oct 7, 2021
1 parent f563676 commit 2c7c5c2
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 0 deletions.
4 changes: 4 additions & 0 deletions synapse/storage/databases/main/user_directory.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 21 additions & 0 deletions tests/handlers/test_user_directory.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
15 changes: 15 additions & 0 deletions tests/storage/test_user_directory.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down

0 comments on commit 2c7c5c2

Please sign in to comment.