From eb5056c2ef98e9a70bd4395bdc109183bb61fd15 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 18 Sep 2024 12:25:10 -0500 Subject: [PATCH 1/6] Ingore invites from ignored users --- synapse/handlers/sliding_sync/room_lists.py | 30 +++++++++- .../client/sliding_sync/test_sliding_sync.py | 56 ++++++++++++++++++- 2 files changed, 83 insertions(+), 3 deletions(-) diff --git a/synapse/handlers/sliding_sync/room_lists.py b/synapse/handlers/sliding_sync/room_lists.py index 475bfbbbcb9..0c5ca2054dc 100644 --- a/synapse/handlers/sliding_sync/room_lists.py +++ b/synapse/handlers/sliding_sync/room_lists.py @@ -223,16 +223,30 @@ async def _compute_interested_rooms_new_tables( room_membership_for_user_map = await self.store.get_sliding_sync_rooms_for_user( user_id ) + # TODO: It would be nice to avoid these copies + room_membership_for_user_map = dict(room_membership_for_user_map) + + # Remove invites from ignored users + ignored_users = await self.store.ignored_users(user_id) + if ignored_users: + # Make a copy so we don't run into an error: `dictionary changed size during + # iteration`, when we remove items + for room_id in list(room_membership_for_user_map.keys()): + room_for_user_sliding_sync = room_membership_for_user_map[room_id] + if ( + room_for_user_sliding_sync.membership == Membership.INVITE + and room_for_user_sliding_sync.sender in ignored_users + ): + room_membership_for_user_map.pop(room_id, None) changes = await self._get_rewind_changes_to_current_membership_to_token( sync_config.user, room_membership_for_user_map, to_token=to_token ) if changes: - room_membership_for_user_map = dict(room_membership_for_user_map) for room_id, change in changes.items(): if change is None: # Remove rooms that the user joined after the `to_token` - room_membership_for_user_map.pop(room_id) + room_membership_for_user_map.pop(room_id, None) continue existing_room = room_membership_for_user_map.get(room_id) @@ -926,6 +940,18 @@ async def get_room_membership_for_user_at_to_token( excluded_rooms=self.rooms_to_exclude_globally, ) + # Remove invites from ignored users + ignored_users = await self.store.ignored_users(user_id) + if ignored_users: + room_for_user_list = [ + room_for_user + for room_for_user in room_for_user_list + if not ( + room_for_user.membership == Membership.INVITE + and room_for_user.sender in ignored_users + ) + ] + # If the user has never joined any rooms before, we can just return an empty list if not room_for_user_list: return {}, set(), set() diff --git a/tests/rest/client/sliding_sync/test_sliding_sync.py b/tests/rest/client/sliding_sync/test_sliding_sync.py index fe35cbb532b..f0ca1624384 100644 --- a/tests/rest/client/sliding_sync/test_sliding_sync.py +++ b/tests/rest/client/sliding_sync/test_sliding_sync.py @@ -29,7 +29,7 @@ from synapse.api.room_versions import RoomVersions from synapse.events import EventBase, StrippedStateEvent, make_event_from_dict from synapse.events.snapshot import EventContext -from synapse.rest.client import devices, login, receipts, room, sync +from synapse.rest.client import account_data, devices, login, receipts, room, sync from synapse.server import HomeServer from synapse.types import ( JsonDict, @@ -413,6 +413,7 @@ class SlidingSyncTestCase(SlidingSyncBase): sync.register_servlets, devices.register_servlets, receipts.register_servlets, + account_data.register_servlets, ] def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: @@ -670,6 +671,59 @@ def test_forgotten_up_to_date(self) -> None: exact=True, ) + def test_ignored_user_invites(self) -> None: + """ + Make sure we ignore invites if they are from one of the `m.ignored_user_list` + """ + user1_id = self.register_user("user1", "pass") + user1_tok = self.login(user1_id, "pass") + user2_id = self.register_user("user2", "pass") + user2_tok = self.login(user2_id, "pass") + + # Create a room that user1 is already in + room_id1 = self.helper.create_room_as(user1_id, tok=user1_tok) + + # Create a room that user2 is already in + room_id2 = self.helper.create_room_as(user2_id, tok=user2_tok) + + # User1 is invited to room_id2 + self.helper.invite(room_id2, src=user2_id, targ=user1_id, tok=user2_tok) + + sync_body = { + "lists": { + "foo-list": { + "ranges": [[0, 99]], + "required_state": [], + "timeline_limit": 0, + }, + } + } + response_body, _ = self.do_sync(sync_body, tok=user1_tok) + # The room_id2 shows up because we haven't ignored the user yet + self.assertIncludes( + set(response_body["lists"]["foo-list"]["ops"][0]["room_ids"]), + {room_id1, room_id2}, + exact=True, + ) + + # User1 ignores user2 + channel = self.make_request( + "PUT", + f"/_matrix/client/v3/user/{user1_id}/account_data/{AccountDataTypes.IGNORED_USER_LIST}", + content={"ignored_users": {user2_id: {}}}, + access_token=user1_tok, + ) + self.assertEqual(channel.code, 200, channel.result) + + # Sync again (initial sync) + response_body, _ = self.do_sync(sync_body, tok=user1_tok) + # The invite for room_id2 should no longer show up + self.assertIncludes( + set(response_body["lists"]["foo-list"]["ops"][0]["room_ids"]), + {room_id1}, + exact=True, + ) + def test_sort_list(self) -> None: """ Test that the `lists` are sorted by `stream_ordering` From 227bee12ee07f6291a224c1ce0d1a6aee85ce4ee Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 18 Sep 2024 12:28:11 -0500 Subject: [PATCH 2/6] Better comments --- tests/rest/client/sliding_sync/test_sliding_sync.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/rest/client/sliding_sync/test_sliding_sync.py b/tests/rest/client/sliding_sync/test_sliding_sync.py index f0ca1624384..4c7853ce6ec 100644 --- a/tests/rest/client/sliding_sync/test_sliding_sync.py +++ b/tests/rest/client/sliding_sync/test_sliding_sync.py @@ -689,6 +689,7 @@ def test_ignored_user_invites(self) -> None: # User1 is invited to room_id2 self.helper.invite(room_id2, src=user2_id, targ=user1_id, tok=user2_tok) + # Sync once before we ignore to make sure the rooms can show up sync_body = { "lists": { "foo-list": { @@ -717,7 +718,7 @@ def test_ignored_user_invites(self) -> None: # Sync again (initial sync) response_body, _ = self.do_sync(sync_body, tok=user1_tok) - # The invite for room_id2 should no longer show up + # The invite for room_id2 should no longer show up because user2 is ignored self.assertIncludes( set(response_body["lists"]["foo-list"]["ops"][0]["room_ids"]), {room_id1}, From a48c1b5e766fc1115524bed5bcbfa6c457d30872 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 18 Sep 2024 12:28:49 -0500 Subject: [PATCH 3/6] Add changelog --- changelog.d/17729.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/17729.bugfix diff --git a/changelog.d/17729.bugfix b/changelog.d/17729.bugfix new file mode 100644 index 00000000000..4ba4e551c68 --- /dev/null +++ b/changelog.d/17729.bugfix @@ -0,0 +1 @@ +Ignore invites from ignored users in Sliding Sync. From 54476d8bd3424445a479e451f96ac6ef594f246e Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 18 Sep 2024 12:30:42 -0500 Subject: [PATCH 4/6] Only copy when we need to mutate --- synapse/handlers/sliding_sync/room_lists.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/synapse/handlers/sliding_sync/room_lists.py b/synapse/handlers/sliding_sync/room_lists.py index 0c5ca2054dc..8457526a458 100644 --- a/synapse/handlers/sliding_sync/room_lists.py +++ b/synapse/handlers/sliding_sync/room_lists.py @@ -223,12 +223,12 @@ async def _compute_interested_rooms_new_tables( room_membership_for_user_map = await self.store.get_sliding_sync_rooms_for_user( user_id ) - # TODO: It would be nice to avoid these copies - room_membership_for_user_map = dict(room_membership_for_user_map) # Remove invites from ignored users ignored_users = await self.store.ignored_users(user_id) if ignored_users: + # TODO: It would be nice to avoid these copies + room_membership_for_user_map = dict(room_membership_for_user_map) # Make a copy so we don't run into an error: `dictionary changed size during # iteration`, when we remove items for room_id in list(room_membership_for_user_map.keys()): @@ -243,6 +243,8 @@ async def _compute_interested_rooms_new_tables( sync_config.user, room_membership_for_user_map, to_token=to_token ) if changes: + # TODO: It would be nice to avoid these copies + room_membership_for_user_map = dict(room_membership_for_user_map) for room_id, change in changes.items(): if change is None: # Remove rooms that the user joined after the `to_token` From 269759b05ed377a1dc986d39d136a0dc57a4bf9d Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 18 Sep 2024 13:02:38 -0500 Subject: [PATCH 5/6] Make sure ignored invites do not show up in incremental syncs --- .../client/sliding_sync/test_sliding_sync.py | 62 ++++++++++++++++++- 1 file changed, 59 insertions(+), 3 deletions(-) diff --git a/tests/rest/client/sliding_sync/test_sliding_sync.py b/tests/rest/client/sliding_sync/test_sliding_sync.py index 4c7853ce6ec..5bdf65b5869 100644 --- a/tests/rest/client/sliding_sync/test_sliding_sync.py +++ b/tests/rest/client/sliding_sync/test_sliding_sync.py @@ -671,9 +671,10 @@ def test_forgotten_up_to_date(self) -> None: exact=True, ) - def test_ignored_user_invites(self) -> None: + def test_ignored_user_invites_initial_sync(self) -> None: """ - Make sure we ignore invites if they are from one of the `m.ignored_user_list` + Make sure we ignore invites if they are from one of the `m.ignored_user_list` on + initial sync. """ user1_id = self.register_user("user1", "pass") user1_tok = self.login(user1_id, "pass") @@ -700,7 +701,7 @@ def test_ignored_user_invites(self) -> None: } } response_body, _ = self.do_sync(sync_body, tok=user1_tok) - # The room_id2 shows up because we haven't ignored the user yet + # room_id2 shows up because we haven't ignored the user yet self.assertIncludes( set(response_body["lists"]["foo-list"]["ops"][0]["room_ids"]), {room_id1, room_id2}, @@ -725,6 +726,61 @@ def test_ignored_user_invites(self) -> None: exact=True, ) + def test_ignored_user_invites_incremental_sync(self) -> None: + """ + Make sure we ignore invites if they are from one of the `m.ignored_user_list` on + incremental sync. + """ + user1_id = self.register_user("user1", "pass") + user1_tok = self.login(user1_id, "pass") + user2_id = self.register_user("user2", "pass") + user2_tok = self.login(user2_id, "pass") + + # Create a room that user1 is already in + room_id1 = self.helper.create_room_as(user1_id, tok=user1_tok) + + # Create a room that user2 is already in + room_id2 = self.helper.create_room_as(user2_id, tok=user2_tok) + + # User1 ignores user2 + channel = self.make_request( + "PUT", + f"/_matrix/client/v3/user/{user1_id}/account_data/{AccountDataTypes.IGNORED_USER_LIST}", + content={"ignored_users": {user2_id: {}}}, + access_token=user1_tok, + ) + self.assertEqual(channel.code, 200, channel.result) + + # Sync once before we ignore to make sure the rooms can show up + sync_body = { + "lists": { + "foo-list": { + "ranges": [[0, 99]], + "required_state": [], + "timeline_limit": 0, + }, + } + } + response_body, from_token = self.do_sync(sync_body, tok=user1_tok) + # User1 only has membership in room_id1 at this point + self.assertIncludes( + set(response_body["lists"]["foo-list"]["ops"][0]["room_ids"]), + {room_id1}, + exact=True, + ) + + # User1 is invited to room_id2 after the initial sync + self.helper.invite(room_id2, src=user2_id, targ=user1_id, tok=user2_tok) + + # Sync again (incremental sync) + response_body, _ = self.do_sync(sync_body, since=from_token, tok=user1_tok) + # The invite for room_id2 doesn't show up because user2 is ignored + self.assertIncludes( + set(response_body["lists"]["foo-list"]["ops"][0]["room_ids"]), + {room_id1}, + exact=True, + ) + def test_sort_list(self) -> None: """ Test that the `lists` are sorted by `stream_ordering` From d909c39c4475877637f0adb3858a14b810e2b647 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 18 Sep 2024 17:34:22 -0500 Subject: [PATCH 6/6] Update comment See https://github.com/element-hq/synapse/pull/17729#discussion_r1765791479 --- tests/rest/client/sliding_sync/test_sliding_sync.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/rest/client/sliding_sync/test_sliding_sync.py b/tests/rest/client/sliding_sync/test_sliding_sync.py index 5bdf65b5869..1126258c434 100644 --- a/tests/rest/client/sliding_sync/test_sliding_sync.py +++ b/tests/rest/client/sliding_sync/test_sliding_sync.py @@ -751,7 +751,7 @@ def test_ignored_user_invites_incremental_sync(self) -> None: ) self.assertEqual(channel.code, 200, channel.result) - # Sync once before we ignore to make sure the rooms can show up + # Initial sync sync_body = { "lists": { "foo-list": {