Merge remote-tracking branch 'origin/release-v1.45' into develop

This commit is contained in:
David Robertson 2021-10-13 12:46:30 +01:00
commit b3e9b00fb2
No known key found for this signature in database
GPG Key ID: 903ECE108A39DEDD
4 changed files with 98 additions and 10 deletions

2
changelog.d/11053.bugfix Normal file
View File

@ -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.

View File

@ -26,6 +26,8 @@ from typing import (
cast, cast,
) )
from synapse.api.errors import StoreError
if TYPE_CHECKING: if TYPE_CHECKING:
from synapse.server import HomeServer from synapse.server import HomeServer
@ -383,7 +385,19 @@ class UserDirectoryBackgroundUpdateStore(StateDeltasStore):
"""Certain classes of local user are omitted from the user directory. """Certain classes of local user are omitted from the user directory.
Is this user one of them? 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): if self.get_if_app_services_interested_in_user(user):
# TODO we might want to make this configurable for each app service # TODO we might want to make this configurable for each app service
return False return False
@ -393,8 +407,14 @@ class UserDirectoryBackgroundUpdateStore(StateDeltasStore):
return False return False
# Deactivated users aren't contactable, so should not appear in the user directory. # 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 False
return True return True
async def is_room_world_readable_or_publicly_joinable(self, room_id: str) -> bool: async def is_room_world_readable_or_publicly_joinable(self, room_id: str) -> bool:

View File

@ -63,7 +63,9 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase):
hostname="test", hostname="test",
id="1234", id="1234",
namespaces={"users": [{"regex": r"@as_user.*", "exclusive": True}]}, 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]) mock_load_appservices = Mock(return_value=[self.appservice])
@ -122,7 +124,7 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase):
{(alice, bob, private), (bob, alice, private)}, {(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 normal user included in the user dir
# - A public and private room created by that user # - A public and private room created by that user
# - A user excluded from the room dir, belonging to both rooms # - 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) 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( def _create_rooms_and_inject_memberships(
self, creator: str, token: str, joiner: str self, creator: str, token: str, joiner: str
) -> Tuple[str, 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)) 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" display_name = "display_name"
profile_info = ProfileInfo(avatar_url="avatar_url", 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 is not in directory
profile = self.get_success(self.store.get_user_in_directory(r_user_id)) 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 # update profile after deactivation
self.get_success( self.get_success(
@ -273,7 +303,7 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase):
# profile is furthermore not in directory # profile is furthermore not in directory
profile = self.get_success(self.store.get_user_in_directory(r_user_id)) 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: def test_handle_local_profile_change_with_appservice_user(self) -> None:
# create user # create user
@ -283,7 +313,7 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase):
# profile is not in directory # profile is not in directory
profile = self.get_success(self.store.get_user_in_directory(as_user_id)) profile = self.get_success(self.store.get_user_in_directory(as_user_id))
self.assertTrue(profile is None) self.assertIsNone(profile)
# update profile # update profile
profile_info = ProfileInfo(avatar_url="avatar_url", display_name="4L1c3") 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 is still not in directory
profile = self.get_success(self.store.get_user_in_directory(as_user_id)) 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: def test_handle_user_deactivated_support_user(self) -> None:
s_user_id = "@support:test" s_user_id = "@support:test"

View File

@ -256,7 +256,7 @@ class UserDirectoryInitialPopulationTestcase(HomeserverTestCase):
users = self.get_success(self.user_dir_helper.get_users_in_user_directory()) users = self.get_success(self.user_dir_helper.get_users_in_user_directory())
self.assertEqual(users, {u1, u2, u3}) 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 normal user included in the user dir
# - A public and private room created by that user # - A public and private room created by that user
# - A user excluded from the room dir, belonging to both rooms # - 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. # Check the AS user is not in the directory.
self._check_room_sharing_tables(user, public, private) 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: def test_population_conceals_private_nickname(self) -> None:
# Make a private room, and set a nickname within # Make a private room, and set a nickname within
user = self.register_user("aaaa", "pass") user = self.register_user("aaaa", "pass")