diff --git a/changelog.d/11053.bugfix b/changelog.d/11053.bugfix
new file mode 100644
index 0000000000..a59cfac931
--- /dev/null
+++ b/changelog.d/11053.bugfix
@@ -0,0 +1,2 @@
+Fix a bug introduced in Synapse 1.45.0rc1 where the user directory would stop updating if it processed an event from a
+user not in the `users` table.
diff --git a/synapse/storage/databases/main/user_directory.py b/synapse/storage/databases/main/user_directory.py
index 5c713a732e..e98a45b6af 100644
--- a/synapse/storage/databases/main/user_directory.py
+++ b/synapse/storage/databases/main/user_directory.py
@@ -26,6 +26,8 @@ from typing import (
cast,
)
+from synapse.api.errors import StoreError
+
if TYPE_CHECKING:
from synapse.server import HomeServer
@@ -383,7 +385,19 @@ class UserDirectoryBackgroundUpdateStore(StateDeltasStore):
"""Certain classes of local user are omitted from the user directory.
Is this user one of them?
"""
- # App service users aren't usually contactable, so exclude them.
+ # We're opting to exclude the appservice sender (user defined by the
+ # `sender_localpart` in the appservice registration) even though
+ # technically it could be DM-able. In the future, this could potentially
+ # be configurable per-appservice whether the appservice sender can be
+ # contacted.
+ if self.get_app_service_by_user_id(user) is not None:
+ return False
+
+ # We're opting to exclude appservice users (anyone matching the user
+ # namespace regex in the appservice registration) even though technically
+ # they could be DM-able. In the future, this could potentially
+ # be configurable per-appservice whether the appservice users can be
+ # contacted.
if self.get_if_app_services_interested_in_user(user):
# TODO we might want to make this configurable for each app service
return False
@@ -393,8 +407,14 @@ class UserDirectoryBackgroundUpdateStore(StateDeltasStore):
return False
# Deactivated users aren't contactable, so should not appear in the user directory.
- if await self.get_user_deactivated_status(user):
+ try:
+ if await self.get_user_deactivated_status(user):
+ return False
+ except StoreError:
+ # No such user in the users table. No need to do this when calling
+ # is_support_user---that returns False if the user is missing.
return False
+
return True
async def is_room_world_readable_or_publicly_joinable(self, room_id: str) -> bool:
diff --git a/tests/handlers/test_user_directory.py b/tests/handlers/test_user_directory.py
index db65253773..0120b4688b 100644
--- a/tests/handlers/test_user_directory.py
+++ b/tests/handlers/test_user_directory.py
@@ -63,7 +63,9 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase):
hostname="test",
id="1234",
namespaces={"users": [{"regex": r"@as_user.*", "exclusive": True}]},
- sender="@as:test",
+ # Note: this user does not match the regex above, so that tests
+ # can distinguish the sender from the AS user.
+ sender="@as_main:test",
)
mock_load_appservices = Mock(return_value=[self.appservice])
@@ -122,7 +124,7 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase):
{(alice, bob, private), (bob, alice, private)},
)
- # The next three tests (test_population_excludes_*) all setup
+ # The next four tests (test_excludes_*) all setup
# - A normal user included in the user dir
# - A public and private room created by that user
# - A user excluded from the room dir, belonging to both rooms
@@ -179,6 +181,34 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase):
)
self._check_only_one_user_in_directory(user, public)
+ def test_excludes_appservice_sender(self) -> None:
+ user = self.register_user("user", "pass")
+ token = self.login(user, "pass")
+ room = self.helper.create_room_as(user, is_public=True, tok=token)
+ self.helper.join(room, self.appservice.sender, tok=self.appservice.token)
+ self._check_only_one_user_in_directory(user, room)
+
+ def test_user_not_in_users_table(self) -> None:
+ """Unclear how it happens, but on matrix.org we've seen join events
+ for users who aren't in the users table. Test that we don't fall over
+ when processing such a user.
+ """
+ user1 = self.register_user("user1", "pass")
+ token1 = self.login(user1, "pass")
+ room = self.helper.create_room_as(user1, is_public=True, tok=token1)
+
+ # Inject a join event for a user who doesn't exist
+ self.get_success(inject_member_event(self.hs, room, "@not-a-user:test", "join"))
+
+ # Another new user registers and joins the room
+ user2 = self.register_user("user2", "pass")
+ token2 = self.login(user2, "pass")
+ self.helper.join(room, user2, tok=token2)
+
+ # The dodgy event should not have stopped us from processing user2's join.
+ in_public = self.get_success(self.user_dir_helper.get_users_in_public_rooms())
+ self.assertEqual(set(in_public), {(user1, room), (user2, room)})
+
def _create_rooms_and_inject_memberships(
self, creator: str, token: str, joiner: str
) -> Tuple[str, str]:
@@ -230,7 +260,7 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase):
)
)
profile = self.get_success(self.store.get_user_in_directory(support_user_id))
- self.assertTrue(profile is None)
+ self.assertIsNone(profile)
display_name = "display_name"
profile_info = ProfileInfo(avatar_url="avatar_url", display_name=display_name)
@@ -264,7 +294,7 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase):
# profile is not in directory
profile = self.get_success(self.store.get_user_in_directory(r_user_id))
- self.assertTrue(profile is None)
+ self.assertIsNone(profile)
# update profile after deactivation
self.get_success(
@@ -273,7 +303,7 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase):
# profile is furthermore not in directory
profile = self.get_success(self.store.get_user_in_directory(r_user_id))
- self.assertTrue(profile is None)
+ self.assertIsNone(profile)
def test_handle_local_profile_change_with_appservice_user(self) -> None:
# create user
@@ -283,7 +313,7 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase):
# profile is not in directory
profile = self.get_success(self.store.get_user_in_directory(as_user_id))
- self.assertTrue(profile is None)
+ self.assertIsNone(profile)
# update profile
profile_info = ProfileInfo(avatar_url="avatar_url", display_name="4L1c3")
@@ -293,7 +323,28 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase):
# profile is still not in directory
profile = self.get_success(self.store.get_user_in_directory(as_user_id))
- self.assertTrue(profile is None)
+ self.assertIsNone(profile)
+
+ 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.assertIsNone(profile)
+
+ # 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.assertIsNone(profile)
def test_handle_user_deactivated_support_user(self) -> None:
s_user_id = "@support:test"
diff --git a/tests/storage/test_user_directory.py b/tests/storage/test_user_directory.py
index 9f483ad681..be3ed64f5e 100644
--- a/tests/storage/test_user_directory.py
+++ b/tests/storage/test_user_directory.py
@@ -256,7 +256,7 @@ class UserDirectoryInitialPopulationTestcase(HomeserverTestCase):
users = self.get_success(self.user_dir_helper.get_users_in_user_directory())
self.assertEqual(users, {u1, u2, u3})
- # The next three tests (test_population_excludes_*) all set up
+ # The next four tests (test_population_excludes_*) all set up
# - A normal user included in the user dir
# - A public and private room created by that user
# - A user excluded from the room dir, belonging to both rooms
@@ -364,6 +364,21 @@ class UserDirectoryInitialPopulationTestcase(HomeserverTestCase):
# 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")
|
