From 655ba530f4e49f987f4b463575736f2f5579c4ea Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Mon, 28 Mar 2022 14:10:35 +0100 Subject: [PATCH 1/9] Add a configuration to ignore rooms from sync response --- synapse/config/server.py | 10 +++++++ synapse/handlers/sync.py | 23 +++++++++++----- synapse/storage/databases/main/roommember.py | 16 +++++++++-- synapse/storage/databases/main/stream.py | 28 +++++++++++++------- 4 files changed, 60 insertions(+), 17 deletions(-) diff --git a/synapse/config/server.py b/synapse/config/server.py index 38de4b80000e..da3054a02f8d 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -680,6 +680,10 @@ def read_config(self, config, **kwargs): config.get("use_account_validity_in_account_status") or False ) + self.rooms_to_exclude_from_sync: List[str] = ( + config.get("exclude_rooms_from_sync") or [] + ) + def has_tls_listener(self) -> bool: return any(listener.tls for listener in self.listeners) @@ -1234,6 +1238,12 @@ def generate_config_section( # information about using custom templates. # #custom_template_directory: /path/to/custom/templates/ + + # List of rooms to exclude from sync responses. + # No room is excluded by default. + # + #exclude_rooms_from_sync: + # - !foo:example.com """ % locals() ) diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index 6c569cfb1c88..a47acb14fe28 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -298,6 +298,8 @@ def __init__(self, hs: "HomeServer"): expiry_ms=LAZY_LOADED_MEMBERS_CACHE_MAX_AGE, ) + self.rooms_to_exclude = hs.config.server.rooms_to_exclude_from_sync + async def wait_for_sync_for_user( self, requester: Requester, @@ -1607,13 +1609,15 @@ async def _generate_sync_entry_for_rooms( ignored_users = await self.store.ignored_users(user_id) if since_token: room_changes = await self._get_rooms_changed( - sync_result_builder, ignored_users + sync_result_builder, ignored_users, self.rooms_to_exclude ) tags_by_room = await self.store.get_updated_tags( user_id, since_token.account_data_key ) else: - room_changes = await self._get_all_rooms(sync_result_builder, ignored_users) + room_changes = await self._get_all_rooms( + sync_result_builder, ignored_users, self.rooms_to_exclude + ) tags_by_room = await self.store.get_tags_for_user(user_id) log_kv({"rooms_changed": len(room_changes.room_entries)}) @@ -1689,7 +1693,10 @@ async def _have_rooms_changed( return False async def _get_rooms_changed( - self, sync_result_builder: "SyncResultBuilder", ignored_users: FrozenSet[str] + self, + sync_result_builder: "SyncResultBuilder", + ignored_users: FrozenSet[str], + ignored_rooms: List[str], ) -> _RoomChanges: """Determine the changes in rooms to report to the user. @@ -1721,7 +1728,7 @@ async def _get_rooms_changed( # _have_rooms_changed. We could keep the results in memory to avoid a # second query, at the cost of more complicated source code. membership_change_events = await self.store.get_membership_changes_for_user( - user_id, since_token.room_key, now_token.room_key + user_id, since_token.room_key, now_token.room_key, ignored_rooms ) mem_change_events_by_room_id: Dict[str, List[EventBase]] = {} @@ -1922,7 +1929,10 @@ async def _get_rooms_changed( ) async def _get_all_rooms( - self, sync_result_builder: "SyncResultBuilder", ignored_users: FrozenSet[str] + self, + sync_result_builder: "SyncResultBuilder", + ignored_users: FrozenSet[str], + ignored_rooms: List[str], ) -> _RoomChanges: """Returns entries for all rooms for the user. @@ -1933,7 +1943,7 @@ async def _get_all_rooms( Args: sync_result_builder ignored_users: Set of users ignored by user. - + ignored_rooms: List of rooms to ignore. """ user_id = sync_result_builder.sync_config.user.to_string() @@ -1944,6 +1954,7 @@ async def _get_all_rooms( room_list = await self.store.get_rooms_for_local_user_where_membership_is( user_id=user_id, membership_list=Membership.LIST, + ignore_rooms=ignored_rooms, ) room_entries = [] diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index 3248da5356a4..2ba2a65b6b87 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -361,7 +361,10 @@ async def get_invite_for_local_user_in_room( return None async def get_rooms_for_local_user_where_membership_is( - self, user_id: str, membership_list: Collection[str] + self, + user_id: str, + membership_list: Collection[str], + ignore_rooms: Optional[List[str]] = None, ) -> List[RoomsForUser]: """Get all the rooms for this *local* user where the membership for this user matches one in the membership list. @@ -372,6 +375,7 @@ async def get_rooms_for_local_user_where_membership_is( user_id: The user ID. membership_list: A list of synapse.api.constants.Membership values which the user must be in. + ignore_rooms: A list of rooms to ignore. Returns: The RoomsForUser that the user matches the membership types. @@ -391,7 +395,11 @@ async def get_rooms_for_local_user_where_membership_is( return [room for room in rooms if room.room_id not in forgotten_rooms] def _get_rooms_for_local_user_where_membership_is_txn( - self, txn, user_id: str, membership_list: List[str] + self, + txn, + user_id: str, + membership_list: List[str], + ignore_rooms: Optional[List[str]] = None, ) -> List[RoomsForUser]: # Paranoia check. if not self.hs.is_mine_id(user_id): @@ -404,6 +412,10 @@ def _get_rooms_for_local_user_where_membership_is_txn( self.database_engine, "c.membership", membership_list ) + if ignore_rooms is not None: + clause += "AND room_id NOT IN (%s)" % ",".join("?" for _ in ignore_rooms) + args = args + ignore_rooms + sql = """ SELECT room_id, e.sender, c.membership, event_id, e.stream_ordering, r.room_version FROM local_current_membership AS c diff --git a/synapse/storage/databases/main/stream.py b/synapse/storage/databases/main/stream.py index 39e1efe37348..007d3a3a0952 100644 --- a/synapse/storage/databases/main/stream.py +++ b/synapse/storage/databases/main/stream.py @@ -585,7 +585,11 @@ def f(txn: LoggingTransaction) -> List[_EventDictReturn]: return ret, key async def get_membership_changes_for_user( - self, user_id: str, from_key: RoomStreamToken, to_key: RoomStreamToken + self, + user_id: str, + from_key: RoomStreamToken, + to_key: RoomStreamToken, + ignored_rooms: Optional[List[str]] = None, ) -> List[EventBase]: """Fetch membership events for a given user. @@ -610,23 +614,29 @@ def f(txn: LoggingTransaction) -> List[_EventDictReturn]: min_from_id = from_key.stream max_to_id = to_key.get_max_stream_pos() + args = [user_id, min_from_id, max_to_id] + + ignore_room_clause = "" + if ignored_rooms is not None: + ignore_room_clause = "AND e.room_id NOT IN (%s)" % ",".join( + "?" for _ in ignored_rooms + ) + args = args + ignored_rooms + sql = """ SELECT m.event_id, instance_name, topological_ordering, stream_ordering FROM events AS e, room_memberships AS m WHERE e.event_id = m.event_id AND m.user_id = ? AND e.stream_ordering > ? AND e.stream_ordering <= ? + %s ORDER BY e.stream_ordering ASC - """ - txn.execute( - sql, - ( - user_id, - min_from_id, - max_to_id, - ), + """ % ( + ignore_room_clause, ) + txn.execute(sql, args) + rows = [ _EventDictReturn(event_id, None, stream_ordering) for event_id, instance_name, topological_ordering, stream_ordering in txn From e6bdf02296e644e0e08395bd98241346069bfc6a Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Mon, 28 Mar 2022 14:11:34 +0100 Subject: [PATCH 2/9] Changelog --- changelog.d/12310.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/12310.feature diff --git a/changelog.d/12310.feature b/changelog.d/12310.feature new file mode 100644 index 000000000000..f3fbb298f70e --- /dev/null +++ b/changelog.d/12310.feature @@ -0,0 +1 @@ +Add a configuration option to remove a specific set of rooms from sync responses. From c360f652d8f0875f4145a8cf4397af50d67a7ec6 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Mon, 28 Mar 2022 14:17:39 +0100 Subject: [PATCH 3/9] Lint --- docs/sample_config.yaml | 6 ++++++ synapse/storage/databases/main/stream.py | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index a21b48ab2e63..eee3bfe8f961 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -539,6 +539,12 @@ templates: # #custom_template_directory: /path/to/custom/templates/ +# List of rooms to exclude from sync responses. +# No room is excluded by default. +# +#exclude_rooms_from_sync: +# - !foo:example.com + # Message retention policy at the server level. # diff --git a/synapse/storage/databases/main/stream.py b/synapse/storage/databases/main/stream.py index 007d3a3a0952..5508f224aab5 100644 --- a/synapse/storage/databases/main/stream.py +++ b/synapse/storage/databases/main/stream.py @@ -36,7 +36,7 @@ """ import logging -from typing import TYPE_CHECKING, Collection, Dict, List, Optional, Set, Tuple +from typing import TYPE_CHECKING, Any, Collection, Dict, List, Optional, Set, Tuple import attr from frozendict import frozendict @@ -614,7 +614,7 @@ def f(txn: LoggingTransaction) -> List[_EventDictReturn]: min_from_id = from_key.stream max_to_id = to_key.get_max_stream_pos() - args = [user_id, min_from_id, max_to_id] + args: List[Any] = [user_id, min_from_id, max_to_id] ignore_room_clause = "" if ignored_rooms is not None: From ea6a13c407346b747a127daf8f8d1774499ce5d1 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Mon, 28 Mar 2022 17:43:30 +0100 Subject: [PATCH 4/9] Standardise naming --- synapse/handlers/sync.py | 6 +++--- synapse/storage/databases/main/roommember.py | 13 +++++++------ synapse/storage/databases/main/stream.py | 8 ++++---- 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index a47acb14fe28..bceafca3b1c6 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -1696,7 +1696,7 @@ async def _get_rooms_changed( self, sync_result_builder: "SyncResultBuilder", ignored_users: FrozenSet[str], - ignored_rooms: List[str], + excluded_rooms: List[str], ) -> _RoomChanges: """Determine the changes in rooms to report to the user. @@ -1728,7 +1728,7 @@ async def _get_rooms_changed( # _have_rooms_changed. We could keep the results in memory to avoid a # second query, at the cost of more complicated source code. membership_change_events = await self.store.get_membership_changes_for_user( - user_id, since_token.room_key, now_token.room_key, ignored_rooms + user_id, since_token.room_key, now_token.room_key, excluded_rooms ) mem_change_events_by_room_id: Dict[str, List[EventBase]] = {} @@ -1954,7 +1954,7 @@ async def _get_all_rooms( room_list = await self.store.get_rooms_for_local_user_where_membership_is( user_id=user_id, membership_list=Membership.LIST, - ignore_rooms=ignored_rooms, + excluded_rooms=ignored_rooms, ) room_entries = [] diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index 2ba2a65b6b87..952cf709398d 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -364,7 +364,7 @@ async def get_rooms_for_local_user_where_membership_is( self, user_id: str, membership_list: Collection[str], - ignore_rooms: Optional[List[str]] = None, + excluded_rooms: Optional[List[str]] = None, ) -> List[RoomsForUser]: """Get all the rooms for this *local* user where the membership for this user matches one in the membership list. @@ -375,7 +375,7 @@ async def get_rooms_for_local_user_where_membership_is( user_id: The user ID. membership_list: A list of synapse.api.constants.Membership values which the user must be in. - ignore_rooms: A list of rooms to ignore. + excluded_rooms: A list of rooms to ignore. Returns: The RoomsForUser that the user matches the membership types. @@ -388,6 +388,7 @@ async def get_rooms_for_local_user_where_membership_is( self._get_rooms_for_local_user_where_membership_is_txn, user_id, membership_list, + excluded_rooms, ) # Now we filter out forgotten rooms @@ -399,7 +400,7 @@ def _get_rooms_for_local_user_where_membership_is_txn( txn, user_id: str, membership_list: List[str], - ignore_rooms: Optional[List[str]] = None, + excluded_rooms: Optional[List[str]] = None, ) -> List[RoomsForUser]: # Paranoia check. if not self.hs.is_mine_id(user_id): @@ -412,9 +413,9 @@ def _get_rooms_for_local_user_where_membership_is_txn( self.database_engine, "c.membership", membership_list ) - if ignore_rooms is not None: - clause += "AND room_id NOT IN (%s)" % ",".join("?" for _ in ignore_rooms) - args = args + ignore_rooms + if excluded_rooms is not None and len(excluded_rooms) > 0: + clause += "AND room_id NOT IN (%s)" % ",".join("?" for _ in excluded_rooms) + args = args + excluded_rooms sql = """ SELECT room_id, e.sender, c.membership, event_id, e.stream_ordering, r.room_version diff --git a/synapse/storage/databases/main/stream.py b/synapse/storage/databases/main/stream.py index 5508f224aab5..8e764790dbc2 100644 --- a/synapse/storage/databases/main/stream.py +++ b/synapse/storage/databases/main/stream.py @@ -589,7 +589,7 @@ async def get_membership_changes_for_user( user_id: str, from_key: RoomStreamToken, to_key: RoomStreamToken, - ignored_rooms: Optional[List[str]] = None, + excluded_rooms: Optional[List[str]] = None, ) -> List[EventBase]: """Fetch membership events for a given user. @@ -617,11 +617,11 @@ def f(txn: LoggingTransaction) -> List[_EventDictReturn]: args: List[Any] = [user_id, min_from_id, max_to_id] ignore_room_clause = "" - if ignored_rooms is not None: + if excluded_rooms is not None and len(excluded_rooms) > 0: ignore_room_clause = "AND e.room_id NOT IN (%s)" % ",".join( - "?" for _ in ignored_rooms + "?" for _ in excluded_rooms ) - args = args + ignored_rooms + args = args + excluded_rooms sql = """ SELECT m.event_id, instance_name, topological_ordering, stream_ordering From 66d16d1502eeab749054f4a50c7712e419a88b23 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Mon, 28 Mar 2022 18:25:11 +0100 Subject: [PATCH 5/9] Add tests --- tests/rest/client/test_sync.py | 62 ++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/tests/rest/client/test_sync.py b/tests/rest/client/test_sync.py index 435101395204..f0f3a54f8234 100644 --- a/tests/rest/client/test_sync.py +++ b/tests/rest/client/test_sync.py @@ -772,3 +772,65 @@ def test_user_with_no_rooms_receives_self_device_list_updates(self) -> None: self.assertIn( self.user_id, device_list_changes, incremental_sync_channel.json_body ) + + +class ExcludeRoomTestCase(unittest.HomeserverTestCase): + + servlets = [ + synapse.rest.admin.register_servlets, + login.register_servlets, + sync.register_servlets, + room.register_servlets, + ] + + def prepare( + self, reactor: MemoryReactor, clock: Clock, homeserver: HomeServer + ) -> None: + self.user_id = self.register_user("user", "password") + self.tok = self.login("user", "password") + + self.excluded_room_id = self.helper.create_room_as(self.user_id, tok=self.tok) + self.included_room_id = self.helper.create_room_as(self.user_id, tok=self.tok) + + # We need to manually append the room ID, because we can't know the ID before + # creating the room, and we can't set the config after starting the homeserver. + self.hs.get_sync_handler().rooms_to_exclude.append(self.excluded_room_id) + + def test_join_leave(self) -> None: + """Tests that rooms are correctly excluded from the 'join' and 'leave' sections of + sync responses. + """ + channel = self.make_request("GET", "/sync", access_token=self.tok) + self.assertEqual(channel.code, 200, channel.result) + + self.assertNotIn(self.excluded_room_id, channel.json_body["rooms"]["join"]) + self.assertIn(self.included_room_id, channel.json_body["rooms"]["join"]) + + self.helper.leave(self.excluded_room_id, self.user_id, tok=self.tok) + self.helper.leave(self.included_room_id, self.user_id, tok=self.tok) + + channel = self.make_request( + "GET", + "/sync?since=" + channel.json_body["next_batch"], + access_token=self.tok, + ) + self.assertEqual(channel.code, 200, channel.result) + + self.assertNotIn(self.excluded_room_id, channel.json_body["rooms"]["leave"]) + self.assertIn(self.included_room_id, channel.json_body["rooms"]["leave"]) + + def test_invite(self) -> None: + """Tests that rooms are correctly excluded from the 'invite' section of sync + responses. + """ + invitee = self.register_user("invitee", "password") + invitee_tok = self.login("invitee", "password") + + self.helper.invite(self.excluded_room_id, self.user_id, invitee, tok=self.tok) + self.helper.invite(self.included_room_id, self.user_id, invitee, tok=self.tok) + + channel = self.make_request("GET", "/sync", access_token=invitee_tok) + self.assertEqual(channel.code, 200, channel.result) + + self.assertNotIn(self.excluded_room_id, channel.json_body["rooms"]["invite"]) + self.assertIn(self.included_room_id, channel.json_body["rooms"]["invite"]) From 4b10d4c17cd17cedfc888f5198516b3cd2afcfc8 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Mon, 28 Mar 2022 18:30:36 +0100 Subject: [PATCH 6/9] Rework phrasing in sample config --- docs/sample_config.yaml | 2 +- synapse/config/server.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index eee3bfe8f961..f11da74359ae 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -540,7 +540,7 @@ templates: #custom_template_directory: /path/to/custom/templates/ # List of rooms to exclude from sync responses. -# No room is excluded by default. +# By default, no room is excluded. # #exclude_rooms_from_sync: # - !foo:example.com diff --git a/synapse/config/server.py b/synapse/config/server.py index da3054a02f8d..2ac5101c1035 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -1240,7 +1240,7 @@ def generate_config_section( #custom_template_directory: /path/to/custom/templates/ # List of rooms to exclude from sync responses. - # No room is excluded by default. + # By default, no room is excluded. # #exclude_rooms_from_sync: # - !foo:example.com From 4153eb9f3c2be600510c138657ebad2d99b1d025 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 29 Mar 2022 14:49:49 +0100 Subject: [PATCH 7/9] Incorporate review --- synapse/config/server.py | 5 ++++- synapse/storage/databases/main/roommember.py | 13 ++++--------- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/synapse/config/server.py b/synapse/config/server.py index 2ac5101c1035..0f90302c9566 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -1239,7 +1239,10 @@ def generate_config_section( # #custom_template_directory: /path/to/custom/templates/ - # List of rooms to exclude from sync responses. + # List of rooms to exclude from sync responses. This is useful for server + # administrators wishing to group users into a room without these users being able + # to see it from their client. + # # By default, no room is excluded. # #exclude_rooms_from_sync: diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index 952cf709398d..a781305c44f7 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -388,19 +388,18 @@ async def get_rooms_for_local_user_where_membership_is( self._get_rooms_for_local_user_where_membership_is_txn, user_id, membership_list, - excluded_rooms, ) - # Now we filter out forgotten rooms - forgotten_rooms = await self.get_forgotten_rooms_for_user(user_id) - return [room for room in rooms if room.room_id not in forgotten_rooms] + # Now we filter out forgotten and excluded rooms + rooms_to_exclude: Set[str] = await self.get_forgotten_rooms_for_user(user_id) + rooms_to_exclude.update(set(excluded_rooms)) + return [room for room in rooms if room.room_id not in rooms_to_exclude] def _get_rooms_for_local_user_where_membership_is_txn( self, txn, user_id: str, membership_list: List[str], - excluded_rooms: Optional[List[str]] = None, ) -> List[RoomsForUser]: # Paranoia check. if not self.hs.is_mine_id(user_id): @@ -413,10 +412,6 @@ def _get_rooms_for_local_user_where_membership_is_txn( self.database_engine, "c.membership", membership_list ) - if excluded_rooms is not None and len(excluded_rooms) > 0: - clause += "AND room_id NOT IN (%s)" % ",".join("?" for _ in excluded_rooms) - args = args + excluded_rooms - sql = """ SELECT room_id, e.sender, c.membership, event_id, e.stream_ordering, r.room_version FROM local_current_membership AS c From 3db095e3bf950d48cdac8b615059f26d03106d11 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 29 Mar 2022 14:53:54 +0100 Subject: [PATCH 8/9] Config --- docs/sample_config.yaml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index f11da74359ae..b8d8c0dbf0a1 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -539,7 +539,10 @@ templates: # #custom_template_directory: /path/to/custom/templates/ -# List of rooms to exclude from sync responses. +# List of rooms to exclude from sync responses. This is useful for server +# administrators wishing to group users into a room without these users being able +# to see it from their client. +# # By default, no room is excluded. # #exclude_rooms_from_sync: From ad902899c5fe0a85c7192aecf98207ffa5d5a181 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 30 Mar 2022 10:12:50 +0100 Subject: [PATCH 9/9] Only filter out excluded rooms if there are any to filter out --- synapse/storage/databases/main/roommember.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index a781305c44f7..98d09b37366b 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -392,7 +392,10 @@ async def get_rooms_for_local_user_where_membership_is( # Now we filter out forgotten and excluded rooms rooms_to_exclude: Set[str] = await self.get_forgotten_rooms_for_user(user_id) - rooms_to_exclude.update(set(excluded_rooms)) + + if excluded_rooms is not None: + rooms_to_exclude.update(set(excluded_rooms)) + return [room for room in rooms if room.room_id not in rooms_to_exclude] def _get_rooms_for_local_user_where_membership_is_txn(