Merge #20427: Handle read receipts in Server Announcement rooms

changelog.d/20427.misc

@@ -0,0 +1 @@
+Keep read receipts in server announcement rooms private to the user who sent them.

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(

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
 

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:

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 = (

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,

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)
 

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())

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())