diff --git a/synapse/config/server_announcements.py b/synapse/config/server_announcements.py index 85a55478d5..2efc4ca1d8 100644 --- a/synapse/config/server_announcements.py +++ b/synapse/config/server_announcements.py @@ -21,7 +21,7 @@ incomplete. ## server_announcements This feature allows homeserver admins to create and manage "announcement" rooms. These -rooms and their events will be visible to all users on the homeserver, except those +rooms and their events are delivered to all users on the homeserver, except those listed in the room's `excluded_user_ids`. Read-only users don't *actually* join announcement rooms, but they will see it in their @@ -76,8 +76,8 @@ Options for each entry: rooms are not federated. Defaults to an empty list, allowing no users to send announcements. Sender accounts must already exist and must not be deactivated. * `excluded_user_ids` (array): Full Matrix user IDs excluded from automatically - receiving announcements from this room, for example appservice users or audit - bots. + receiving announcements from this room, for example bots that act on the messages + they receive. Each must be a valid, local user, and must not be in `allowed_user_ids`: senders are joined to the room, so they always receive its events. Defaults to an empty @@ -87,6 +87,11 @@ Options for each entry: rooms manually (as they are world-readable). But they will not automatically receive announcement events. + Changes to this list do not update rooms that clients already have. Clients of a + newly excluded user keep the room but stop receiving its events. A user removed + from the list receives new events but not the room's state until their client does + an initial sync. + The internal room manager user (which is used to create announcement rooms and update their room state) is configured with `room_management_localpart` below. It is a management identity, not a user-visible sender, and must not be included in diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index b4b260d156..edf566c67a 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -2371,7 +2371,7 @@ class SyncHandler: synthetic_join_announcement_room_ids = frozenset( room_id for room_id in sync_config.filter_collection.filter_rooms( - await self.store.get_server_announcement_room_ids() + await self.store.get_server_announcement_room_ids_for_user(user_id) ) if room_id not in mutable_joined_room_ids and room_id not in mutable_rooms_to_exclude diff --git a/synapse/storage/databases/main/cache.py b/synapse/storage/databases/main/cache.py index 5ca908ac5b..4f2a14bbe0 100644 --- a/synapse/storage/databases/main/cache.py +++ b/synapse/storage/databases/main/cache.py @@ -612,7 +612,9 @@ class CacheInvalidationWorkerStore(SQLBaseStore): self._attempt_to_invalidate_cache("get_room_version_id", (room_id,)) self._attempt_to_invalidate_cache("get_room_type", (room_id,)) self._attempt_to_invalidate_cache("get_room_encryption", (room_id,)) - self._attempt_to_invalidate_cache("get_server_announcement_room_ids", None) + self._attempt_to_invalidate_cache( + "get_server_announcement_config_ids_by_room_id", None + ) self._attempt_to_invalidate_cache("_get_max_event_pos", (room_id,)) diff --git a/synapse/storage/databases/main/server_announcements.py b/synapse/storage/databases/main/server_announcements.py index 444fd9a12f..ff36b91554 100644 --- a/synapse/storage/databases/main/server_announcements.py +++ b/synapse/storage/databases/main/server_announcements.py @@ -12,6 +12,8 @@ # . # +from typing import Mapping + from synapse.storage.database import LoggingTransaction from synapse.storage.databases.main.cache import CacheInvalidationWorkerStore from synapse.util.caches.descriptors import cached @@ -23,7 +25,8 @@ class ServerAnnouncementsStore(CacheInvalidationWorkerStore): The unique announcement `id` identifies the same room even when its configured details change. It is separate from the actual room ID. - Only the set of configured room IDs is cached, as it is read on every sync. + Only the mapping from room ID to config ID is cached, as it is read on every + sync. """ async def get_server_announcement_room(self, config_id: str) -> str | None: @@ -47,8 +50,11 @@ class ServerAnnouncementsStore(CacheInvalidationWorkerStore): ) @cached(num_args=0) - async def get_server_announcement_room_ids(self) -> frozenset[str]: - """Return the room IDs of the currently configured announcement rooms. + async def get_server_announcement_config_ids_by_room_id( + self, + ) -> Mapping[str, str]: + """Return the config ID of each currently configured announcement room, + keyed by room ID. Rooms whose config ID is no longer in the config are left out, as are configured rooms that have not been created yet. @@ -59,10 +65,36 @@ class ServerAnnouncementsStore(CacheInvalidationWorkerStore): iterable=[ room.config_id for room in self.hs.config.serverannouncements.rooms ], - retcols=("room_id",), - desc="get_server_announcement_room_ids", + retcols=("room_id", "config_id"), + desc="get_server_announcement_config_ids_by_room_id", + ) + return dict(rows) + + async def get_server_announcement_room_ids(self) -> frozenset[str]: + """Return the room IDs of the currently configured announcement rooms, + as in `get_server_announcement_config_ids_by_room_id`. + """ + return frozenset(await self.get_server_announcement_config_ids_by_room_id()) + + async def get_server_announcement_room_ids_for_user( + self, user_id: str + ) -> frozenset[str]: + """Return the room IDs of the announcement rooms delivered to a user: + the currently configured ones, minus those listing the user in + `excluded_user_ids`. + """ + excluded_config_ids = { + room.config_id + for room in self.hs.config.serverannouncements.rooms + if user_id in room.excluded_user_ids + } + return frozenset( + room_id + for room_id, config_id in ( + await self.get_server_announcement_config_ids_by_room_id() + ).items() + if config_id not in excluded_config_ids ) - return frozenset(room_id for (room_id,) in rows) async def store_server_announcement_room( self, config_id: str, room_id: str @@ -76,7 +108,7 @@ class ServerAnnouncementsStore(CacheInvalidationWorkerStore): values={"config_id": config_id, "room_id": room_id}, ) self._invalidate_cache_and_stream( - txn, self.get_server_announcement_room_ids, () + txn, self.get_server_announcement_config_ids_by_room_id, () ) await self.db_pool.runInteraction( diff --git a/tests/rest/client/test_sync.py b/tests/rest/client/test_sync.py index 418c2810dd..4e5598342f 100644 --- a/tests/rest/client/test_sync.py +++ b/tests/rest/client/test_sync.py @@ -2486,6 +2486,48 @@ class ServerAnnouncementsSyncTestCase(unittest.HomeserverTestCase): self._sync(self.bob_tok, since, full_state=True, filter=filter) ) + def test_excluded_user(self) -> None: + """A user in `excluded_user_ids` is not delivered the room, while other + non-members still are. + """ + carol = self.register_user("carol", "pass") + carol_tok = self.login("carol", "pass") + self.config.rooms = [ + attr.evolve(self.config.rooms[0], excluded_user_ids=[self.bob]) + ] + self._announce("hello") + + since = self._sync(self.bob_tok)["next_batch"] + self._announce("again") + + self._assert_not_delivered(self._sync(self.bob_tok)) + self._assert_not_delivered(self._sync(self.bob_tok, since)) + self._assert_not_delivered(self._sync(self.bob_tok, since, full_state=True)) + self._assert_delivered(self._sync(carol_tok), carol) + + def test_excluded_former_member(self) -> None: + """A member removed from the room by the config and excluded from it + gets their real leave instead of the synthetic join. + """ + since = self._sync(self.former_tok)["next_batch"] + + self.config.rooms = [ + attr.evolve( + self.config.rooms[0], + allowed_user_ids=[self.announcer], + excluded_user_ids=[self.former], + ) + ] + self.get_success(self.handler.reconcile_rooms()) + self._announce("hello") + + for response in ( + self._sync(self.former_tok, since), + self._sync(self.former_tok), + ): + self.assertNotIn(self.room_id, response["rooms"].get("join", {})) + self.assertIn(self.room_id, response["rooms"]["leave"]) + def test_removed_member(self) -> None: """A member removed from the room by the config switches straight to the synthetic join and the room's current power levels, then gets later diff --git a/tests/storage/test_server_announcements.py b/tests/storage/test_server_announcements.py index 839493abdf..d1a7d6bed1 100644 --- a/tests/storage/test_server_announcements.py +++ b/tests/storage/test_server_announcements.py @@ -16,6 +16,7 @@ from parameterized import parameterized from twisted.internet.testing import MemoryReactor +from synapse.config.server_announcements import ServerAnnouncementRoom from synapse.rest.client import room from synapse.server import HomeServer from synapse.storage.databases.main.server_announcements import ServerAnnouncementsStore @@ -126,3 +127,57 @@ class ServerAnnouncementsStoreTestCase(HomeserverTestCase): self.assertIsNone( self.get_success(reader.get_server_announcement_avatar(self.config_id)) ) + + def test_room_ids_for_user_leave_out_excluded_rooms(self) -> None: + """A user listed in one room's `excluded_user_ids` gets every other + configured room, and a newly stored room shows up for both users. + """ + excluded_user_id = "@bot:test" + self.hs.config.serverannouncements.rooms = [ + ServerAnnouncementRoom( + config_id=config_id, + room_name=config_id, + room_topic=None, + room_avatar_url=None, + room_avatar_path=None, + allowed_user_ids=[], + excluded_user_ids=excluded_user_ids, + ) + for config_id, excluded_user_ids in ( + ("news", [excluded_user_id]), + ("maintenance", []), + ("incidents", []), + ) + ] + self.get_success( + self.store.store_server_announcement_room("news", "!news:test") + ) + self.get_success( + self.store.store_server_announcement_room( + "maintenance", "!maintenance:test" + ) + ) + + self.assertEqual( + self.get_success( + self.store.get_server_announcement_room_ids_for_user(excluded_user_id) + ), + {"!maintenance:test"}, + ) + self.assertEqual( + self.get_success( + self.store.get_server_announcement_room_ids_for_user(self.user_id) + ), + {"!news:test", "!maintenance:test"}, + ) + + self.get_success( + self.store.store_server_announcement_room("incidents", "!incidents:test") + ) + + self.assertEqual( + self.get_success( + self.store.get_server_announcement_room_ids_for_user(excluded_user_id) + ), + {"!maintenance:test", "!incidents:test"}, + )