diff --git a/changelog.d/20427.misc b/changelog.d/20427.misc new file mode 100644 index 0000000000..86823c3cd3 --- /dev/null +++ b/changelog.d/20427.misc @@ -0,0 +1 @@ +Keep read receipts in server announcement rooms private to the user who sent them. diff --git a/synapse/handlers/initial_sync.py b/synapse/handlers/initial_sync.py index 56f4d86d41..8fc252e2e8 100644 --- a/synapse/handlers/initial_sync.py +++ b/synapse/handlers/initial_sync.py @@ -160,7 +160,9 @@ class InitialSyncHandler: to_key=now_token.receipt_key, ) - receipt = ReceiptEventSource.filter_out_private_receipts(receipt, user_id) + receipt = ReceiptEventSource.filter_out_private_receipts( + receipt, user_id, await self.store.get_server_announcement_room_ids() + ) tags_by_room = await self.store.get_tags_for_user(user_id) @@ -486,6 +488,7 @@ class InitialSyncHandler: return ReceiptEventSource.filter_out_private_receipts( receipts, requester.user.to_string(), + await self.store.get_server_announcement_room_ids(), ) presence, receipts, (messages, token) = await make_deferred_yieldable( diff --git a/synapse/handlers/receipts.py b/synapse/handlers/receipts.py index 5ced086ef2..ac26d1dce5 100644 --- a/synapse/handlers/receipts.py +++ b/synapse/handlers/receipts.py @@ -19,7 +19,7 @@ # # import logging -from typing import TYPE_CHECKING, Callable, Iterable, Sequence +from typing import TYPE_CHECKING, AbstractSet, Callable, Iterable, Sequence from synapse.api.constants import Direction, EduTypes, ReceiptTypes from synapse.appservice import ApplicationService @@ -222,13 +222,36 @@ class ReceiptEventSource(EventSource[MultiWriterStreamToken, JsonMapping]): self.store = hs.get_datastores().main self.config = hs.config + @staticmethod + def read_receipt_should_be_shared( + receipt_type: str, room_id: str, announcement_room_ids: AbstractSet[str] + ) -> bool: + """Whether a receipt may be sent to users other than the one who sent + it. + + Private read receipts are never shared, and neither are any receipts in + server announcement rooms, which most users see without being members + of them. + + Args: + receipt_type: The type of the receipt. + room_id: The room the receipt is in. + announcement_room_ids: The server announcement rooms. + """ + return ( + receipt_type != ReceiptTypes.READ_PRIVATE + and room_id not in announcement_room_ids + ) + @staticmethod def filter_out_private_receipts( - rooms: Sequence[JsonMapping], user_id: str + rooms: Sequence[JsonMapping], + user_id: str, + announcement_room_ids: AbstractSet[str], ) -> list[JsonMapping]: """ Filters a list of serialized receipts (as returned by /sync and /initialSync) - and removes private read receipts of other users. + and removes the receipts of other users that should not be shared. This operates on the return value of get_linearized_receipts_for_rooms(), which is wrapped in a cache. Care must be taken to ensure that the input @@ -237,31 +260,38 @@ class ReceiptEventSource(EventSource[MultiWriterStreamToken, JsonMapping]): Args: rooms: A list of mappings, each mapping has a `content` field, which is a map of event ID -> receipt type -> user ID -> receipt information. - user_id: The user whose private read receipts should be kept. + user_id: The user whose receipts should be kept. + announcement_room_ids: The server announcement rooms. Returns: The same as rooms, but filtered. """ return ReceiptEventSource._filter_private_receipts( - rooms, lambda receipt_user_id: receipt_user_id == user_id + rooms, + lambda receipt_user_id: receipt_user_id == user_id, + announcement_room_ids, ) @staticmethod def _filter_private_receipts( - rooms: Sequence[JsonMapping], is_visible: Callable[[str], bool] + rooms: Sequence[JsonMapping], + is_visible: Callable[[str], bool], + announcement_room_ids: AbstractSet[str], ) -> list[JsonMapping]: """ - Filters a list of serialized receipts and removes private read receipts - of users for which `is_visible` returns False. + Filters a list of serialized receipts and removes the receipts that + should not be shared of users for which `is_visible` returns False. This may operate on the return value of cached functions. Care must be taken to ensure that the input values are not modified. Args: - rooms: A list of mappings, each mapping has a `content` field, which - is a map of event ID -> receipt type -> user ID -> receipt information. - is_visible: Called with each private read receipt's user ID; returns - whether that user's private read receipts may be included. + rooms: A list of mappings, each mapping has a `room_id` and a + `content` field, which is a map of event ID -> receipt type -> + user ID -> receipt information. + is_visible: Called with the user ID of each receipt that should not + be shared; returns whether that user's receipt may be included. + announcement_room_ids: The server announcement rooms. Returns: The same as rooms, but filtered. @@ -271,43 +301,43 @@ class ReceiptEventSource(EventSource[MultiWriterStreamToken, JsonMapping]): # Iterate through each room's receipt content. for room in rooms: - # The receipt content with hidden users' private read receipts removed. + room_id = room["room_id"] + + # The receipt content with hidden users' unshared receipts removed. content = {} # Iterate over each event ID / receipts for that event. for event_id, orig_event_content in room.get("content", {}).items(): event_content = orig_event_content - # If there are private read receipts, additional logic is necessary. - if ReceiptTypes.READ_PRIVATE in event_content: - # Make a copy without private read receipts to avoid leaking - # hidden users' private read receipts.. - event_content = { - receipt_type: receipt_value - for receipt_type, receipt_value in event_content.items() - if receipt_type != ReceiptTypes.READ_PRIVATE - } + # If there are unshared receipts, additional logic is necessary. + if not all( + ReceiptEventSource.read_receipt_should_be_shared( + receipt_type, room_id, announcement_room_ids + ) + for receipt_type in orig_event_content + ): + # Make a copy that only keeps the visible users' unshared + # receipts, to avoid leaking hidden users' unshared receipts. + event_content = {} + for receipt_type, receipt_value in orig_event_content.items(): + if not ReceiptEventSource.read_receipt_should_be_shared( + receipt_type, room_id, announcement_room_ids + ): + receipt_value = { + receipt_user_id: receipt_user_value + for receipt_user_id, receipt_user_value in receipt_value.items() + if is_visible(receipt_user_id) + } + if receipt_value: + event_content[receipt_type] = receipt_value - # Copy the visible users' private read receipts from the - # original content, if there are any. - visible_private_read_receipts = { - receipt_user_id: receipt_value - for receipt_user_id, receipt_value in orig_event_content[ - ReceiptTypes.READ_PRIVATE - ].items() - if is_visible(receipt_user_id) - } - if visible_private_read_receipts: - event_content[ReceiptTypes.READ_PRIVATE] = ( - visible_private_read_receipts - ) - - # Include the event if there is at least one non-private read - # receipt or a visible user has a private read receipt. + # Include the event if there is at least one shared receipt or a + # visible user has an unshared receipt. if event_content: content[event_id] = event_content - # Include the event if there is at least one non-private read receipt - # or a visible user has a private read receipt. + # Include the event if there is at least one shared receipt or a + # visible user has an unshared receipt. if content: # Build a new event to avoid mutating the cache. new_room = {k: v for k, v in room.items() if k != "content"} @@ -341,7 +371,9 @@ class ReceiptEventSource(EventSource[MultiWriterStreamToken, JsonMapping]): ) events = ReceiptEventSource.filter_out_private_receipts( - events, user.to_string() + events, + user.to_string(), + await self.store.get_server_announcement_room_ids(), ) return events, to_key @@ -406,7 +438,11 @@ class ReceiptEventSource(EventSource[MultiWriterStreamToken, JsonMapping]): # Private read receipts must only be sent for users matching one of the # appservice's namespaces (or its sender user). See # https://spec.matrix.org/v1.19/application-service-api/#pushing-ephemeral-data - events = self._filter_private_receipts(events, service.is_interested_in_user) + events = self._filter_private_receipts( + events, + service.is_interested_in_user, + await self.store.get_server_announcement_room_ids(), + ) return events, reached_token diff --git a/synapse/handlers/sliding_sync/extensions.py b/synapse/handlers/sliding_sync/extensions.py index 5d58bb0a65..a8799cb9cf 100644 --- a/synapse/handlers/sliding_sync/extensions.py +++ b/synapse/handlers/sliding_sync/extensions.py @@ -842,7 +842,9 @@ class SlidingSyncExtensionHandler: ) fetched_receipts = ReceiptEventSource.filter_out_private_receipts( - fetched_receipts, sync_config.user.to_string() + fetched_receipts, + sync_config.user.to_string(), + await self.store.get_server_announcement_room_ids(), ) for receipt in fetched_receipts: diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index b4b260d156..9a48c3b28e 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -39,6 +39,7 @@ from prometheus_client import Counter from synapse.api.constants import ( AccountDataTypes, Direction, + EduTypes, EventContentFields, EventTypes, Membership, @@ -62,6 +63,7 @@ from synapse.logging.opentracing import ( ) from synapse.metrics import SERVER_NAME_LABEL from synapse.storage.databases.main.event_push_actions import RoomNotifCounts +from synapse.storage.databases.main.receipts import ReceiptInRoom from synapse.storage.databases.main.roommember import extract_heroes_from_room_summary from synapse.storage.databases.main.stream import PaginateFunction from synapse.storage.invite_rule import InviteRule @@ -778,6 +780,23 @@ class SyncHandler: room_id = event["room_id"] ephemeral_by_room.setdefault(room_id, []).append(event) + announcement_receipts = ( + await self.store.get_linearized_receipts_for_user_in_rooms( + user_id=sync_config.user.to_string(), + room_ids=sync_result_builder.synthetic_join_announcement_room_ids, + to_key=receipt_key, + from_key=since_token.receipt_key if since_token else None, + ) + ) + for room_id, room_receipts in announcement_receipts.items(): + ephemeral_by_room.setdefault(room_id, []).append( + { + "room_id": room_id, + "type": EduTypes.RECEIPT, + "content": ReceiptInRoom.merge_to_content(room_receipts), + } + ) + return now_token, ephemeral_by_room async def sticky_events_by_room( @@ -2985,6 +3004,7 @@ class SyncHandler: sync_result_builder, ignored_users, account_data_room_ids=account_data_by_room.keys(), + ephemeral_room_ids=frozenset(ephemeral_by_room), ) tags_by_room = await self.store.get_updated_tags( user_id, since_token.account_data_key @@ -3065,6 +3085,7 @@ class SyncHandler: sync_result_builder: "SyncResultBuilder", ignored_users: frozenset[str], account_data_room_ids: AbstractSet[str], + ephemeral_room_ids: AbstractSet[str], ) -> _RoomChanges: """Determine the changes in rooms to report to the user. @@ -3324,6 +3345,7 @@ class SyncHandler: sync_result_builder.synthetic_join_announcement_room_ids, left_room_ids=left_announcement_room_ids, account_data_room_ids=account_data_room_ids, + ephemeral_room_ids=ephemeral_room_ids, ) ) @@ -3341,6 +3363,7 @@ class SyncHandler: room_ids: AbstractSet[str], left_room_ids: AbstractSet[str] = frozenset(), account_data_room_ids: AbstractSet[str] = frozenset(), + ephemeral_room_ids: AbstractSet[str] = frozenset(), ) -> list["RoomSyncResultBuilder"]: """Returns entries for the given announcement rooms that have something to show. @@ -3349,9 +3372,10 @@ class SyncHandler: event, so that creating the room and joining its senders does not show it to every user. On an incremental sync, a room is included if it has had an announcement and there is any new event in it since the - `since_token`, such as a sender leaving or a name change, or a change - to the user's room account data for it. With `full_state` set, a room - is also included without any new event. On an + `since_token`, such as a sender leaving or a name change, a change + to the user's room account data for it, or any new ephemeral event + for the user, such as their own read receipt. With `full_state` set, + a room is also included without any new event. On an initial sync, a room is included if it has ever had an announcement, even when later state changes push that announcement out of the timeline. @@ -3385,6 +3409,8 @@ class SyncHandler: account_data_room_ids: Rooms whose room account data for the user changed since the `since_token` of `sync_result_builder`. They may include rooms that are not in `room_ids`. + ephemeral_room_ids: Rooms with new ephemeral events for the user + since the `since_token` of `sync_result_builder`. """ since_token = sync_result_builder.since_token now_token = sync_result_builder.now_token @@ -3427,8 +3453,9 @@ class SyncHandler: for room_id, (events, _, _) in room_to_events.items() if events and room_id not in left_room_ids } - # Also consider rooms whose account data has changed. - room_ids_to_check |= account_data_room_ids & ( + # Also consider rooms whose account data has changed, or with new + # ephemeral events for the user. + room_ids_to_check |= (account_data_room_ids | ephemeral_room_ids) & ( room_ids - room_ids_to_include ) first_announcement_positions = ( diff --git a/synapse/storage/databases/main/receipts.py b/synapse/storage/databases/main/receipts.py index 3720d25c1d..efdbfe2137 100644 --- a/synapse/storage/databases/main/receipts.py +++ b/synapse/storage/databases/main/receipts.py @@ -674,9 +674,14 @@ class ReceiptsWorkerStore(SQLBaseStore): return results, reached_token async def get_linearized_receipts_for_user_in_rooms( - self, user_id: str, room_ids: StrCollection, to_key: MultiWriterStreamToken + self, + user_id: str, + room_ids: StrCollection, + to_key: MultiWriterStreamToken, + from_key: MultiWriterStreamToken | None = None, ) -> Mapping[str, Sequence[ReceiptInRoom]]: - """Fetch all receipts for the user in the given room. + """Fetch all receipts for the user in the given room, after `from_key` + if given. Returns: A dict from room ID to receipts in the room. @@ -699,13 +704,17 @@ class ReceiptsWorkerStore(SQLBaseStore): args.append(user_id) args.append(to_key.get_max_stream_pos()) + if from_key: + sql += " AND stream_id > ?" + args.append(from_key.stream) + txn.execute(sql, args) return [ (room_id, receipt_type, user_id, event_id, thread_id, data) for instance_name, stream_id, room_id, receipt_type, user_id, event_id, thread_id, data in txn if MultiWriterStreamToken.is_stream_position_in_range( - low=None, + low=from_key, high=to_key, instance_name=instance_name, pos=stream_id, diff --git a/tests/handlers/test_receipts.py b/tests/handlers/test_receipts.py index 2be8e4f521..db23aeb5f3 100644 --- a/tests/handlers/test_receipts.py +++ b/tests/handlers/test_receipts.py @@ -339,7 +339,7 @@ class ReceiptsTestCase(unittest.HomeserverTestCase): ) -> None: """Tests that the _filter_out_private returns the expected output""" filtered_events = self.event_source.filter_out_private_receipts( - events, "@me:server.org" + events, "@me:server.org", frozenset() ) self.assertEqual(filtered_events, expected_output) diff --git a/tests/rest/client/sliding_sync/test_server_announcements.py b/tests/rest/client/sliding_sync/test_server_announcements.py index ed87d4622e..2de5b1e650 100644 --- a/tests/rest/client/sliding_sync/test_server_announcements.py +++ b/tests/rest/client/sliding_sync/test_server_announcements.py @@ -17,11 +17,11 @@ from parameterized import parameterized_class from twisted.internet.testing import MemoryReactor import synapse.rest.admin -from synapse.api.constants import EventTypes, Membership +from synapse.api.constants import EventTypes, Membership, ReceiptTypes, RelationTypes from synapse.config.server_announcements import ServerAnnouncementRoom from synapse.handlers.server_announcements import SYNTHETIC_JOIN_EVENT_ID_PREFIX from synapse.handlers.sliding_sync import StateValues -from synapse.rest.client import devices, login, room, sync +from synapse.rest.client import devices, login, read_marker, receipts, room, sync from synapse.server import HomeServer from synapse.types import JsonDict from synapse.util.clock import Clock @@ -50,6 +50,8 @@ class SlidingSyncServerAnnouncementsTestCase(SlidingSyncBase): room.register_servlets, sync.register_servlets, devices.register_servlets, + read_marker.register_servlets, + receipts.register_servlets, ] def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: @@ -61,6 +63,9 @@ class SlidingSyncServerAnnouncementsTestCase(SlidingSyncBase): self.former_tok = self.login("former", "pass") self.bob = self.register_user("bob", "pass") self.bob_tok = self.login("bob", "pass") + self.bob_other_tok = self.login("bob", "pass") + self.dan = self.register_user("dan", "pass") + self.dan_tok = self.login("dan", "pass") self.config = hs.config.serverannouncements self.config.rooms = [ @@ -552,3 +557,237 @@ class SlidingSyncServerAnnouncementsTestCase(SlidingSyncBase): .get("device_lists", {}) .get("changed", []), ) + + def _announce_in_thread(self) -> tuple[str, str]: + """Send an announcement and a reply in its thread, and return the IDs + of the thread root and of the reply. + """ + root_id = self._announce("hello") + reply_id = self.helper.send_event( + self.room_id, + EventTypes.Message, + content={ + "msgtype": "m.text", + "body": "reply", + "m.relates_to": {"rel_type": RelationTypes.THREAD, "event_id": root_id}, + }, + tok=self.announcer_tok, + )["event_id"] + return root_id, reply_id + + def _send_read_receipt( + self, + event_id: str, + tok: str, + receipt_type: str = ReceiptTypes.READ, + thread_id: str | None = None, + ) -> None: + channel = self.make_request( + "POST", + f"/rooms/{self.room_id}/receipt/{receipt_type}/{event_id}", + {} if thread_id is None else {"thread_id": thread_id}, + access_token=tok, + ) + self.assertEqual(channel.code, 200, channel.json_body) + + def _set_read_markers(self, event_id: str, tok: str) -> None: + channel = self.make_request( + "POST", + f"/rooms/{self.room_id}/read_markers", + {ReceiptTypes.FULLY_READ: event_id, ReceiptTypes.READ: event_id}, + access_token=tok, + ) + self.assertEqual(channel.code, 200, channel.json_body) + + def _receipts_sync_body(self) -> JsonDict: + sync_body = self._sync_body() + sync_body["extensions"] = {"receipts": {"enabled": True}} + return sync_body + + def _receipt_data( + self, + response: JsonDict, + event_id: str, + receipt_type: str = ReceiptTypes.READ, + ) -> JsonDict: + """Return the `receipt_type` receipts on `event_id` in the announcement + room of the receipts extension, by user ID. + """ + receipt = ( + response["extensions"]["receipts"].get("rooms", {}).get(self.room_id, {}) + ) + return dict(receipt.get("content", {}).get(event_id, {}).get(receipt_type, {})) + + def _receipt_readers( + self, + response: JsonDict, + event_id: str, + receipt_type: str = ReceiptTypes.READ, + ) -> set[str]: + """Return the users with a `receipt_type` receipt on `event_id` in the + announcement room of the receipts extension. + """ + return set(self._receipt_data(response, event_id, receipt_type)) + + def test_non_member_receipt_on_new_connection_of_other_device(self) -> None: + """A non-member's read receipt is sent to a new connection of their + other devices. + """ + event_id = self._announce("hello") + self._send_read_receipt(event_id, self.bob_tok) + + response, _ = self.do_sync(self._receipts_sync_body(), tok=self.bob_other_tok) + self.assertEqual(self._receipt_readers(response, event_id), {self.bob}) + + def test_non_member_receipt_on_existing_connection_of_other_device(self) -> None: + """A non-member's read receipt is sent to an existing connection of their + other devices, even with no new announcement since. + """ + event_id = self._announce("hello") + _, pos = self.do_sync(self._receipts_sync_body(), tok=self.bob_other_tok) + + self._send_read_receipt(event_id, self.bob_tok) + + response, _ = self.do_sync( + self._receipts_sync_body(), since=pos, tok=self.bob_other_tok + ) + self.assertEqual(self._receipt_readers(response, event_id), {self.bob}) + + def test_non_member_receipt_hidden_from_member(self) -> None: + """A non-member's read receipt is not sent to a member of the + announcement room, on a new connection or an existing one. + """ + event_id = self._announce("hello") + _, pos = self.do_sync(self._receipts_sync_body(), tok=self.announcer_tok) + + self._send_read_receipt(event_id, self.bob_tok) + + response, _ = self.do_sync( + self._receipts_sync_body(), since=pos, tok=self.announcer_tok + ) + self.assertEqual(self._receipt_readers(response, event_id), set()) + + response, _ = self.do_sync(self._receipts_sync_body(), tok=self.announcer_tok) + self.assertIn(self.room_id, response["rooms"]) + self.assertEqual(self._receipt_readers(response, event_id), set()) + + def test_non_member_receipt_hidden_from_other_non_member(self) -> None: + """A non-member's read receipt is not sent to another non-member, on a + new connection or an existing one. + """ + event_id = self._announce("hello") + _, pos = self.do_sync(self._receipts_sync_body(), tok=self.dan_tok) + + self._send_read_receipt(event_id, self.bob_tok) + + response, _ = self.do_sync( + self._receipts_sync_body(), since=pos, tok=self.dan_tok + ) + self.assertEqual(self._receipt_readers(response, event_id), set()) + + response, _ = self.do_sync(self._receipts_sync_body(), tok=self.dan_tok) + self._assert_delivered(response, self.dan) + self.assertEqual(self._receipt_readers(response, event_id), set()) + + def test_non_member_private_receipt_on_other_device(self) -> None: + """A non-member's private read receipt is sent to an existing and a new + connection of their other devices. + """ + event_id = self._announce("hello") + _, pos = self.do_sync(self._receipts_sync_body(), tok=self.bob_other_tok) + + self._send_read_receipt(event_id, self.bob_tok, ReceiptTypes.READ_PRIVATE) + + response, _ = self.do_sync( + self._receipts_sync_body(), since=pos, tok=self.bob_other_tok + ) + self.assertEqual( + self._receipt_readers(response, event_id, ReceiptTypes.READ_PRIVATE), + {self.bob}, + ) + + response, _ = self.do_sync(self._receipts_sync_body(), tok=self.bob_other_tok) + self.assertEqual( + self._receipt_readers(response, event_id, ReceiptTypes.READ_PRIVATE), + {self.bob}, + ) + + def test_non_member_private_receipt_hidden_from_other_users(self) -> None: + """A non-member's private read receipt is not sent to a member of the + announcement room, nor to another non-member. + """ + event_id = self._announce("hello") + + self._send_read_receipt(event_id, self.bob_tok, ReceiptTypes.READ_PRIVATE) + + for tok in (self.announcer_tok, self.dan_tok): + response, _ = self.do_sync(self._receipts_sync_body(), tok=tok) + self.assertIn(self.room_id, response["rooms"]) + self.assertEqual( + self._receipt_readers(response, event_id, ReceiptTypes.READ_PRIVATE), + set(), + ) + + def test_non_member_threaded_receipt_on_other_device(self) -> None: + """A non-member's threaded read receipt is sent to an existing and a + new connection of their other devices. + """ + root_id, reply_id = self._announce_in_thread() + _, pos = self.do_sync(self._receipts_sync_body(), tok=self.bob_other_tok) + + self._send_read_receipt(reply_id, self.bob_tok, thread_id=root_id) + + response, _ = self.do_sync( + self._receipts_sync_body(), since=pos, tok=self.bob_other_tok + ) + receipts = self._receipt_data(response, reply_id) + self.assertEqual(set(receipts), {self.bob}) + self.assertEqual(receipts[self.bob]["thread_id"], root_id) + + response, _ = self.do_sync(self._receipts_sync_body(), tok=self.bob_other_tok) + receipts = self._receipt_data(response, reply_id) + self.assertEqual(set(receipts), {self.bob}) + self.assertEqual(receipts[self.bob]["thread_id"], root_id) + + def test_non_member_threaded_receipt_hidden_from_other_users(self) -> None: + """A non-member's threaded read receipt is not sent to a member of the + announcement room, nor to another non-member. + """ + root_id, reply_id = self._announce_in_thread() + + self._send_read_receipt(reply_id, self.bob_tok, thread_id=root_id) + + for tok in (self.announcer_tok, self.dan_tok): + response, _ = self.do_sync(self._receipts_sync_body(), tok=tok) + self.assertIn(self.room_id, response["rooms"]) + self.assertEqual(self._receipt_readers(response, reply_id), set()) + + def test_non_member_read_markers_receipt_on_other_device(self) -> None: + """The read receipt a non-member sets with `/read_markers` is sent to an + existing and a new connection of their other devices. + """ + event_id = self._announce("hello") + _, pos = self.do_sync(self._receipts_sync_body(), tok=self.bob_other_tok) + + self._set_read_markers(event_id, self.bob_tok) + + response, _ = self.do_sync( + self._receipts_sync_body(), since=pos, tok=self.bob_other_tok + ) + self.assertEqual(self._receipt_readers(response, event_id), {self.bob}) + + response, _ = self.do_sync(self._receipts_sync_body(), tok=self.bob_other_tok) + self.assertEqual(self._receipt_readers(response, event_id), {self.bob}) + + def test_non_member_read_markers_receipt_hidden_from_other_users(self) -> None: + """The read receipt a non-member sets with `/read_markers` is not sent to + a member of the announcement room, nor to another non-member. + """ + event_id = self._announce("hello") + + self._set_read_markers(event_id, self.bob_tok) + + for tok in (self.announcer_tok, self.dan_tok): + response, _ = self.do_sync(self._receipts_sync_body(), tok=tok) + self.assertIn(self.room_id, response["rooms"]) + self.assertEqual(self._receipt_readers(response, event_id), set()) diff --git a/tests/rest/client/test_receipts.py b/tests/rest/client/test_receipts.py index 0835eec6de..f3bb98f573 100644 --- a/tests/rest/client/test_receipts.py +++ b/tests/rest/client/test_receipts.py @@ -23,9 +23,16 @@ from http import HTTPStatus from twisted.internet.testing import MemoryReactor import synapse.rest.admin -from synapse.api.constants import EduTypes, EventTypes, HistoryVisibility, ReceiptTypes +from synapse.api.constants import ( + EduTypes, + EventTypes, + HistoryVisibility, + ReceiptTypes, + RelationTypes, +) from synapse.api.errors import Codes -from synapse.rest.client import login, receipts, room, sync +from synapse.config.server_announcements import ServerAnnouncementRoom +from synapse.rest.client import login, read_marker, receipts, room, sync from synapse.server import HomeServer from synapse.types import JsonDict from synapse.util.clock import Clock @@ -420,3 +427,349 @@ class ReceiptsTestCase(unittest.HomeserverTestCase): ]["events"] receipt_event = filter(is_read_receipt, ephemeral_events) return next(receipt_event, None) + + +class ServerAnnouncementsReceiptsTestCase(unittest.HomeserverTestCase): + """Tests read receipts in server announcement rooms, which most users see + without being members of them. + """ + + servlets = [ + synapse.rest.admin.register_servlets, + login.register_servlets, + read_marker.register_servlets, + receipts.register_servlets, + room.register_servlets, + sync.register_servlets, + ] + + def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: + self.announcer = self.register_user("announcer", "pass") + self.announcer_tok = self.login("announcer", "pass") + self.bob = self.register_user("bob", "pass") + self.bob_tok = self.login("bob", "pass") + self.bob_other_tok = self.login("bob", "pass") + self.dan = self.register_user("dan", "pass") + self.dan_tok = self.login("dan", "pass") + + hs.config.serverannouncements.rooms = [ + ServerAnnouncementRoom( + config_id="news", + room_name="Server News", + room_topic="News from the server", + room_avatar_url=None, + room_avatar_path=None, + allowed_user_ids=[self.announcer], + ) + ] + self.store = hs.get_datastores().main + self.get_success(hs.get_server_announcements_handler().reconcile_rooms()) + room_id = self.get_success(self.store.get_server_announcement_room("news")) + assert room_id is not None + self.room_id = room_id + + def test_non_member_sends_read_receipt(self) -> None: + """A user who is not a member of the announcement room can send a read + receipt for an announcement. + """ + event_id = self.helper.send(self.room_id, "hello", tok=self.announcer_tok)[ + "event_id" + ] + + channel = self.make_request( + "POST", + f"/rooms/{self.room_id}/receipt/{ReceiptTypes.READ}/{event_id}", + {}, + access_token=self.bob_tok, + ) + self.assertEqual(channel.code, HTTPStatus.OK, channel.json_body) + + receipts = self.get_success( + self.store.get_receipts_for_user(self.bob, [ReceiptTypes.READ]) + ) + self.assertEqual(receipts, {self.room_id: event_id}) + + def _announce(self, body: str) -> str: + return self.helper.send(self.room_id, body, tok=self.announcer_tok)["event_id"] + + def _announce_in_thread(self) -> tuple[str, str]: + """Send an announcement and a reply in its thread, and return the IDs + of the thread root and of the reply. + """ + root_id = self._announce("hello") + reply_id = self.helper.send_event( + self.room_id, + EventTypes.Message, + content={ + "msgtype": "m.text", + "body": "reply", + "m.relates_to": {"rel_type": RelationTypes.THREAD, "event_id": root_id}, + }, + tok=self.announcer_tok, + )["event_id"] + return root_id, reply_id + + def _send_read_receipt( + self, + event_id: str, + tok: str, + receipt_type: str = ReceiptTypes.READ, + thread_id: str | None = None, + ) -> None: + channel = self.make_request( + "POST", + f"/rooms/{self.room_id}/receipt/{receipt_type}/{event_id}", + {} if thread_id is None else {"thread_id": thread_id}, + access_token=tok, + ) + self.assertEqual(channel.code, HTTPStatus.OK, channel.json_body) + + def _set_read_markers(self, event_id: str, tok: str) -> None: + channel = self.make_request( + "POST", + f"/rooms/{self.room_id}/read_markers", + {ReceiptTypes.FULLY_READ: event_id, ReceiptTypes.READ: event_id}, + access_token=tok, + ) + self.assertEqual(channel.code, HTTPStatus.OK, channel.json_body) + + def _sync(self, tok: str, since: str | None = None) -> JsonDict: + url = "/sync" if since is None else f"/sync?since={since}" + channel = self.make_request("GET", url, access_token=tok) + self.assertEqual(channel.code, HTTPStatus.OK, channel.json_body) + return channel.json_body + + def _receipt_data( + self, + response: JsonDict, + event_id: str, + receipt_type: str = ReceiptTypes.READ, + ) -> JsonDict: + """Return the `receipt_type` receipts on `event_id` in the announcement + room's ephemeral events of a `/sync` response, by user ID. + """ + room = response.get("rooms", {}).get("join", {}).get(self.room_id, {}) + return { + user_id: data + for event in room.get("ephemeral", {}).get("events", []) + if event["type"] == EduTypes.RECEIPT + for user_id, data in event["content"] + .get(event_id, {}) + .get(receipt_type, {}) + .items() + } + + def _receipt_readers( + self, + response: JsonDict, + event_id: str, + receipt_type: str = ReceiptTypes.READ, + ) -> set[str]: + """Return the users with a `receipt_type` receipt on `event_id` in the + announcement room's ephemeral events of a `/sync` response. + """ + return set(self._receipt_data(response, event_id, receipt_type)) + + def _assert_room_in_sync(self, response: JsonDict) -> None: + self.assertIn(self.room_id, response.get("rooms", {}).get("join", {})) + + def test_non_member_receipt_on_initial_sync_of_other_device(self) -> None: + """A non-member's read receipt is in the initial sync of their other + devices. + """ + event_id = self._announce("hello") + self._send_read_receipt(event_id, self.bob_tok) + + response = self._sync(self.bob_other_tok) + self.assertEqual(self._receipt_readers(response, event_id), {self.bob}) + + def test_non_member_receipt_on_incremental_sync_of_other_device(self) -> None: + """A non-member's read receipt is in the next incremental sync of their + other devices, even with no new announcement since. + """ + event_id = self._announce("hello") + since = self._sync(self.bob_other_tok)["next_batch"] + + self._send_read_receipt(event_id, self.bob_tok) + + response = self._sync(self.bob_other_tok, since) + self.assertEqual(self._receipt_readers(response, event_id), {self.bob}) + + def test_non_member_receipt_hidden_from_member_sync(self) -> None: + """A non-member's read receipt is not in the initial or incremental + sync of a member of the announcement room. + """ + event_id = self._announce("hello") + since = self._sync(self.announcer_tok)["next_batch"] + + self._send_read_receipt(event_id, self.bob_tok) + + response = self._sync(self.announcer_tok) + self._assert_room_in_sync(response) + self.assertEqual(self._receipt_readers(response, event_id), set()) + + response = self._sync(self.announcer_tok, since) + self.assertEqual(self._receipt_readers(response, event_id), set()) + + def test_non_member_receipt_hidden_from_other_non_member_sync(self) -> None: + """A non-member's read receipt is not in the initial or incremental + sync of another non-member. + """ + event_id = self._announce("hello") + since = self._sync(self.dan_tok)["next_batch"] + + self._send_read_receipt(event_id, self.bob_tok) + self._announce("again") + + response = self._sync(self.dan_tok) + self._assert_room_in_sync(response) + self.assertEqual(self._receipt_readers(response, event_id), set()) + + response = self._sync(self.dan_tok, since) + self._assert_room_in_sync(response) + self.assertEqual(self._receipt_readers(response, event_id), set()) + + def test_non_member_sends_private_read_receipt(self) -> None: + """A user who is not a member of the announcement room can send a + private read receipt for an announcement. + """ + event_id = self._announce("hello") + + self._send_read_receipt(event_id, self.bob_tok, ReceiptTypes.READ_PRIVATE) + + receipts = self.get_success( + self.store.get_receipts_for_user(self.bob, [ReceiptTypes.READ_PRIVATE]) + ) + self.assertEqual(receipts, {self.room_id: event_id}) + + def test_non_member_private_receipt_on_sync_of_other_device(self) -> None: + """A non-member's private read receipt is in the initial and next + incremental sync of their other devices. + """ + event_id = self._announce("hello") + since = self._sync(self.bob_other_tok)["next_batch"] + + self._send_read_receipt(event_id, self.bob_tok, ReceiptTypes.READ_PRIVATE) + + for response in ( + self._sync(self.bob_other_tok), + self._sync(self.bob_other_tok, since), + ): + self.assertEqual( + self._receipt_readers(response, event_id, ReceiptTypes.READ_PRIVATE), + {self.bob}, + ) + + def test_non_member_private_receipt_hidden_from_other_users_sync(self) -> None: + """A non-member's private read receipt is not in the sync of a member + of the announcement room, nor of another non-member. + """ + event_id = self._announce("hello") + + self._send_read_receipt(event_id, self.bob_tok, ReceiptTypes.READ_PRIVATE) + + for tok in (self.announcer_tok, self.dan_tok): + response = self._sync(tok) + self._assert_room_in_sync(response) + self.assertEqual( + self._receipt_readers(response, event_id, ReceiptTypes.READ_PRIVATE), + set(), + ) + + def test_non_member_sends_threaded_read_receipt(self) -> None: + """A user who is not a member of the announcement room can send a + threaded read receipt in an announcement's thread. + """ + root_id, reply_id = self._announce_in_thread() + + self._send_read_receipt(reply_id, self.bob_tok, thread_id=root_id) + + receipts = self.get_success( + self.store.get_linearized_receipts_for_user_in_rooms( + self.bob, [self.room_id], self.store.get_max_receipt_stream_id() + ) + ) + self.assertEqual( + [(r.receipt_type, r.event_id, r.thread_id) for r in receipts[self.room_id]], + [(ReceiptTypes.READ, reply_id, root_id)], + ) + + def test_non_member_threaded_receipt_on_sync_of_other_device(self) -> None: + """A non-member's threaded read receipt is in the initial and next + incremental sync of their other devices. + """ + root_id, reply_id = self._announce_in_thread() + since = self._sync(self.bob_other_tok)["next_batch"] + + self._send_read_receipt(reply_id, self.bob_tok, thread_id=root_id) + + for response in ( + self._sync(self.bob_other_tok), + self._sync(self.bob_other_tok, since), + ): + receipts = self._receipt_data(response, reply_id) + self.assertEqual(set(receipts), {self.bob}) + self.assertEqual(receipts[self.bob]["thread_id"], root_id) + + def test_non_member_threaded_receipt_hidden_from_other_users_sync(self) -> None: + """A non-member's threaded read receipt is not in the sync of a member + of the announcement room, nor of another non-member. + """ + root_id, reply_id = self._announce_in_thread() + + self._send_read_receipt(reply_id, self.bob_tok, thread_id=root_id) + + for tok in (self.announcer_tok, self.dan_tok): + response = self._sync(tok) + self._assert_room_in_sync(response) + self.assertEqual(self._receipt_readers(response, reply_id), set()) + + def test_non_member_sets_read_markers(self) -> None: + """A user who is not a member of the announcement room can set their + fully read marker and read receipt on an announcement with + `/read_markers`. + """ + event_id = self._announce("hello") + + self._set_read_markers(event_id, self.bob_tok) + + fully_read = self.get_success( + self.store.get_account_data_for_room_and_type( + self.bob, self.room_id, ReceiptTypes.FULLY_READ + ) + ) + self.assertEqual(fully_read, {"event_id": event_id}) + receipts = self.get_success( + self.store.get_receipts_for_user(self.bob, [ReceiptTypes.READ]) + ) + self.assertEqual(receipts, {self.room_id: event_id}) + + def test_non_member_read_markers_receipt_on_sync_of_other_device(self) -> None: + """The read receipt a non-member sets with `/read_markers` is in the + initial and next incremental sync of their other devices. + """ + event_id = self._announce("hello") + since = self._sync(self.bob_other_tok)["next_batch"] + + self._set_read_markers(event_id, self.bob_tok) + + for response in ( + self._sync(self.bob_other_tok), + self._sync(self.bob_other_tok, since), + ): + self.assertEqual(self._receipt_readers(response, event_id), {self.bob}) + + def test_non_member_read_markers_receipt_hidden_from_other_users_sync( + self, + ) -> None: + """The read receipt a non-member sets with `/read_markers` is not in the + sync of a member of the announcement room, nor of another non-member. + """ + event_id = self._announce("hello") + + self._set_read_markers(event_id, self.bob_tok) + + for tok in (self.announcer_tok, self.dan_tok): + response = self._sync(tok) + self._assert_room_in_sync(response) + self.assertEqual(self._receipt_readers(response, event_id), set())