diff --git a/changelog.d/19663.feature b/changelog.d/19663.feature new file mode 100644 index 00000000000..9f61182a908 --- /dev/null +++ b/changelog.d/19663.feature @@ -0,0 +1 @@ +Support [MSC4446](https://github.com/matrix-org/matrix-spec-proposals/pull/4446) for moving fully read markers backwards. Contributed by @SpiritCroc @ Beeper. diff --git a/rust/src/config/mod.rs b/rust/src/config/mod.rs index ef9ecb067ce..174e58a09e7 100644 --- a/rust/src/config/mod.rs +++ b/rust/src/config/mod.rs @@ -72,4 +72,5 @@ pub struct ExperimentalConfig { pub msc4222_enabled: bool, pub msc4491_enabled: bool, pub msc4143_enabled: bool, + pub msc4446_enabled: bool, } diff --git a/rust/src/handlers/versions.rs b/rust/src/handlers/versions.rs index db8bca82d68..28090d45bde 100644 --- a/rust/src/handlers/versions.rs +++ b/rust/src/handlers/versions.rs @@ -266,6 +266,9 @@ pub struct UnstableFeatureMap { /// MSC4143: Matrix RTC transports (LiveKit backend) #[serde(rename = "org.matrix.msc4143")] msc4143_enabled: bool, + /// MSC4446: Allow moving the fully read marker backwards. + #[serde(rename = "com.beeper.msc4446")] + msc4446_enabled: bool, // Whether new rooms will be set to encrypted or not (based on presets). #[serde(rename = "io.element.e2ee_forced.public")] @@ -319,6 +322,7 @@ pub fn synapse_config_to_global_unstable_feature_map( msc4445_initial_sync_timeline_topological_ordering: true, msc4491_enabled: config.experimental.msc4491_enabled, msc4143_enabled: config.experimental.msc4143_enabled, + msc4446_enabled: config.experimental.msc4446_enabled, e2ee_forced_public: config .room .encryption_enabled_by_default_for_room_presets diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index f99f7b139ec..b721705007c 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -287,6 +287,9 @@ def read_config( # (and MSC4308: Thread Subscriptions extension to Sliding Sync) self.msc4306_enabled: bool = experimental.get("msc4306_enabled", False) + # MSC4446: Allow moving the fully read marker backwards. + self.msc4446_enabled: bool = experimental.get("msc4446_enabled", False) + # MSC4354: Sticky Events # Tracked in: https://github.com/element-hq/synapse/issues/19409 # Note that sticky events persisted before this feature is enabled will not be diff --git a/synapse/handlers/read_marker.py b/synapse/handlers/read_marker.py index 85d2dd62bb7..3f3b9e6d8bd 100644 --- a/synapse/handlers/read_marker.py +++ b/synapse/handlers/read_marker.py @@ -41,7 +41,11 @@ def __init__(self, hs: "HomeServer"): ) async def received_client_read_marker( - self, room_id: str, user_id: str, event_id: str + self, + room_id: str, + user_id: str, + event_id: str, + allow_backward: bool = False, ) -> None: """Updates the read marker for a given user in a given room if the event ID given is ahead in the stream relative to the current read marker. @@ -59,7 +63,7 @@ async def received_client_read_marker( # Get event ordering, this also ensures we know about the event event_ordering = await self.store.get_event_ordering(event_id, room_id) - if existing_read_marker: + if existing_read_marker and not allow_backward: try: old_event_ordering = await self.store.get_event_ordering( existing_read_marker["event_id"], room_id diff --git a/synapse/rest/client/read_marker.py b/synapse/rest/client/read_marker.py index 874e7487bf6..8e0f2a2e7a8 100644 --- a/synapse/rest/client/read_marker.py +++ b/synapse/rest/client/read_marker.py @@ -23,6 +23,7 @@ from typing import TYPE_CHECKING from synapse.api.constants import ReceiptTypes +from synapse.api.errors import Codes, SynapseError from synapse.http.server import HttpServer from synapse.http.servlet import RestServlet, parse_json_object_from_request from synapse.http.site import SynapseRequest @@ -66,6 +67,21 @@ async def on_POST( body = parse_json_object_from_request(request) unrecognized_types = set(body.keys()) - self._known_receipt_types + + if self.config.experimental.msc4446_enabled: + allow_backward = body.get("com.beeper.allow_backward", False) + if not isinstance(allow_backward, bool): + raise SynapseError( + 400, + "com.beeper.allow_backward must be a boolean.", + Codes.INVALID_PARAM, + ) + + # Prevent considering the `allow_backward` field as a receipt type. + unrecognized_types -= {"com.beeper.allow_backward"} + else: + allow_backward = False + if unrecognized_types: # It's fine if there are unrecognized receipt types, but let's log # it to help debug clients that have typoed the receipt type. @@ -86,6 +102,7 @@ async def on_POST( room_id, user_id=requester.user.to_string(), event_id=event_id, + allow_backward=allow_backward, ) else: await self.receipts_handler.received_client_receipt( diff --git a/synapse/rest/client/receipts.py b/synapse/rest/client/receipts.py index d3a43537bb3..949a1e64ad2 100644 --- a/synapse/rest/client/receipts.py +++ b/synapse/rest/client/receipts.py @@ -20,6 +20,7 @@ # import logging +from http import HTTPStatus from typing import TYPE_CHECKING from synapse.api.constants import MAIN_TIMELINE, ReceiptTypes @@ -50,6 +51,7 @@ def __init__(self, hs: "HomeServer"): self.read_marker_handler = hs.get_read_marker_handler() self.presence_handler = hs.get_presence_handler() self._main_store = hs.get_datastores().main + self._msc4446_enabled = hs.config.experimental.msc4446_enabled self._known_receipt_types = { ReceiptTypes.READ, @@ -73,6 +75,25 @@ async def on_POST( body = parse_json_object_from_request(request) + if self._msc4446_enabled: + allow_backward = body.get("com.beeper.allow_backward", False) + if not isinstance(allow_backward, bool): + raise SynapseError( + HTTPStatus.BAD_REQUEST, + "com.beeper.allow_backward must be a boolean.", + Codes.INVALID_PARAM, + ) + + if allow_backward and receipt_type != ReceiptTypes.FULLY_READ: + raise SynapseError( + HTTPStatus.BAD_REQUEST, + "com.beeper.allow_backward is only allowed to be true for " + f"{ReceiptTypes.FULLY_READ}.", + Codes.INVALID_PARAM, + ) + else: + allow_backward = False + # Pull the thread ID, if one exists. thread_id = None if "thread_id" in body: @@ -108,6 +129,7 @@ async def on_POST( room_id, user_id=requester.user.to_string(), event_id=event_id, + allow_backward=allow_backward, ) else: await self.receipts_handler.received_client_receipt( diff --git a/tests/rest/client/test_read_marker.py b/tests/rest/client/test_read_marker.py index c8bb0da5e64..ad13d3607e7 100644 --- a/tests/rest/client/test_read_marker.py +++ b/tests/rest/client/test_read_marker.py @@ -66,6 +66,19 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: self.store = self.hs.get_datastores().main self.clock = self.hs.get_clock() + def _get_fully_read_marker(self, room_id: str) -> str | None: + content = self.get_success( + self.store.get_account_data_for_room_and_type( + self.owner, + room_id, + "m.fully_read", + ) + ) + if content is None: + return None + + return content.get("event_id") + def test_send_read_marker(self) -> None: room_id = self.helper.create_room_as(self.owner, tok=self.owner_tok) @@ -98,6 +111,123 @@ def send_message() -> str: ) self.assertEqual(channel.code, 200, channel.result) + def test_send_read_marker_does_not_move_backwards_by_default(self) -> None: + room_id = self.helper.create_room_as(self.owner, tok=self.owner_tok) + + older_event_id = self.helper.send( + room_id=room_id, body="1", tok=self.owner_tok + )["event_id"] + newer_event_id = self.helper.send( + room_id=room_id, body="2", tok=self.owner_tok + )["event_id"] + + channel = self.make_request( + "POST", + f"/rooms/{room_id}/read_markers", + content={"m.fully_read": newer_event_id}, + access_token=self.owner_tok, + ) + self.assertEqual(channel.code, 200, channel.result) + self.assertEqual(self._get_fully_read_marker(room_id), newer_event_id) + + # Expected to be a no-op. + channel = self.make_request( + "POST", + f"/rooms/{room_id}/read_markers", + content={"m.fully_read": older_event_id}, + access_token=self.owner_tok, + ) + self.assertEqual(channel.code, 200, channel.result) + self.assertEqual(self._get_fully_read_marker(room_id), newer_event_id) + + @unittest.override_config({"experimental_features": {"msc4446_enabled": True}}) + def test_send_read_marker_can_move_backwards_with_opt_in(self) -> None: + room_id = self.helper.create_room_as(self.owner, tok=self.owner_tok) + + older_event_id = self.helper.send( + room_id=room_id, body="1", tok=self.owner_tok + )["event_id"] + newer_event_id = self.helper.send( + room_id=room_id, body="2", tok=self.owner_tok + )["event_id"] + + channel = self.make_request( + "POST", + f"/rooms/{room_id}/read_markers", + content={"m.fully_read": newer_event_id}, + access_token=self.owner_tok, + ) + self.assertEqual(channel.code, 200, channel.result) + + channel = self.make_request( + "POST", + f"/rooms/{room_id}/read_markers", + content={"m.fully_read": older_event_id, "com.beeper.allow_backward": True}, + access_token=self.owner_tok, + ) + self.assertEqual(channel.code, 200, channel.result) + self.assertEqual(self._get_fully_read_marker(room_id), older_event_id) + + @unittest.override_config({"experimental_features": {"msc4446_enabled": True}}) + def test_send_read_marker_does_not_move_backwards_with_explicit_opt_out( + self, + ) -> None: + room_id = self.helper.create_room_as(self.owner, tok=self.owner_tok) + + older_event_id = self.helper.send( + room_id=room_id, body="1", tok=self.owner_tok + )["event_id"] + newer_event_id = self.helper.send( + room_id=room_id, body="2", tok=self.owner_tok + )["event_id"] + + channel = self.make_request( + "POST", + f"/rooms/{room_id}/read_markers", + content={"m.fully_read": newer_event_id}, + access_token=self.owner_tok, + ) + self.assertEqual(channel.code, 200, channel.result) + + # Expected to be a no-op. + channel = self.make_request( + "POST", + f"/rooms/{room_id}/read_markers", + content={ + "m.fully_read": older_event_id, + "com.beeper.allow_backward": False, + }, + access_token=self.owner_tok, + ) + self.assertEqual(channel.code, 200, channel.result) + self.assertEqual(self._get_fully_read_marker(room_id), newer_event_id) + + def test_send_read_marker_ignores_opt_in_when_feature_disabled(self) -> None: + room_id = self.helper.create_room_as(self.owner, tok=self.owner_tok) + older_event_id = self.helper.send( + room_id=room_id, body="1", tok=self.owner_tok + )["event_id"] + newer_event_id = self.helper.send( + room_id=room_id, body="2", tok=self.owner_tok + )["event_id"] + + channel = self.make_request( + "POST", + f"/rooms/{room_id}/read_markers", + content={"m.fully_read": newer_event_id}, + access_token=self.owner_tok, + ) + self.assertEqual(channel.code, 200, channel.result) + + channel = self.make_request( + "POST", + f"/rooms/{room_id}/read_markers", + content={"m.fully_read": older_event_id, "com.beeper.allow_backward": True}, + access_token=self.owner_tok, + ) + self.assertEqual(channel.code, 200, channel.result) + self.assertEqual(self._get_fully_read_marker(room_id), newer_event_id) + def test_send_read_marker_missing_previous_event(self) -> None: """ Test moving a read marker from an event that previously existed but was diff --git a/tests/rest/client/test_receipts.py b/tests/rest/client/test_receipts.py index 3a6a869c541..0835eec6deb 100644 --- a/tests/rest/client/test_receipts.py +++ b/tests/rest/client/test_receipts.py @@ -24,6 +24,7 @@ import synapse.rest.admin from synapse.api.constants import EduTypes, EventTypes, HistoryVisibility, ReceiptTypes +from synapse.api.errors import Codes from synapse.rest.client import login, receipts, room, sync from synapse.server import HomeServer from synapse.types import JsonDict @@ -44,6 +45,7 @@ class ReceiptsTestCase(unittest.HomeserverTestCase): def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: self.url = "/sync?since=%s" self.next_batch = "s0" + self.store = hs.get_datastores().main # Register the first user self.user_id = self.register_user("kermit", "monkey") @@ -59,6 +61,19 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: # Join the second user self.helper.join(room=self.room_id, user=self.user2, tok=self.tok2) + def _get_fully_read_marker(self) -> str | None: + content = self.get_success( + self.store.get_account_data_for_room_and_type( + self.user2, + self.room_id, + ReceiptTypes.FULLY_READ, + ) + ) + if content is None: + return None + + return content.get("event_id") + def test_send_receipt(self) -> None: # Send a message. res = self.helper.send(self.room_id, body="hello", tok=self.tok) @@ -258,6 +273,126 @@ def test_read_receipt_with_empty_body_is_rejected(self) -> None: self.assertEqual(channel.code, HTTPStatus.BAD_REQUEST) self.assertEqual(channel.json_body["errcode"], "M_NOT_JSON", channel.json_body) + def test_fully_read_receipt_does_not_move_backwards_by_default(self) -> None: + older_event_id = self.helper.send(self.room_id, body="1", tok=self.tok)[ + "event_id" + ] + newer_event_id = self.helper.send(self.room_id, body="2", tok=self.tok)[ + "event_id" + ] + + channel = self.make_request( + "POST", + f"/rooms/{self.room_id}/receipt/{ReceiptTypes.FULLY_READ}/{newer_event_id}", + {}, + access_token=self.tok2, + ) + self.assertEqual(channel.code, 200, channel.result) + self.assertEqual(self._get_fully_read_marker(), newer_event_id) + + # Expected to be a no-op. + channel = self.make_request( + "POST", + f"/rooms/{self.room_id}/receipt/{ReceiptTypes.FULLY_READ}/{older_event_id}", + {}, + access_token=self.tok2, + ) + self.assertEqual(channel.code, 200, channel.result) + self.assertEqual(self._get_fully_read_marker(), newer_event_id) + + @unittest.override_config({"experimental_features": {"msc4446_enabled": True}}) + def test_fully_read_receipt_can_move_backwards_with_opt_in(self) -> None: + older_event_id = self.helper.send(self.room_id, body="1", tok=self.tok)[ + "event_id" + ] + newer_event_id = self.helper.send(self.room_id, body="2", tok=self.tok)[ + "event_id" + ] + + channel = self.make_request( + "POST", + f"/rooms/{self.room_id}/receipt/{ReceiptTypes.FULLY_READ}/{newer_event_id}", + {}, + access_token=self.tok2, + ) + self.assertEqual(channel.code, 200, channel.result) + + channel = self.make_request( + "POST", + f"/rooms/{self.room_id}/receipt/{ReceiptTypes.FULLY_READ}/{older_event_id}", + {"com.beeper.allow_backward": True}, + access_token=self.tok2, + ) + self.assertEqual(channel.code, 200, channel.result) + self.assertEqual(self._get_fully_read_marker(), older_event_id) + + @unittest.override_config({"experimental_features": {"msc4446_enabled": True}}) + def test_fully_read_receipt_does_not_move_backwards_with_explicit_opt_out( + self, + ) -> None: + older_event_id = self.helper.send(self.room_id, body="1", tok=self.tok)[ + "event_id" + ] + newer_event_id = self.helper.send(self.room_id, body="2", tok=self.tok)[ + "event_id" + ] + + channel = self.make_request( + "POST", + f"/rooms/{self.room_id}/receipt/{ReceiptTypes.FULLY_READ}/{newer_event_id}", + {}, + access_token=self.tok2, + ) + self.assertEqual(channel.code, 200, channel.result) + + # Expected to be a no-op. + channel = self.make_request( + "POST", + f"/rooms/{self.room_id}/receipt/{ReceiptTypes.FULLY_READ}/{older_event_id}", + {"com.beeper.allow_backward": False}, + access_token=self.tok2, + ) + self.assertEqual(channel.code, 200, channel.result) + self.assertEqual(self._get_fully_read_marker(), newer_event_id) + + @unittest.override_config({"experimental_features": {"msc4446_enabled": True}}) + def test_allow_backward_is_rejected_for_read_receipts(self) -> None: + event_id = self.helper.send(self.room_id, body="1", tok=self.tok)["event_id"] + + channel = self.make_request( + "POST", + f"/rooms/{self.room_id}/receipt/{ReceiptTypes.READ}/{event_id}", + {"com.beeper.allow_backward": True}, + access_token=self.tok2, + ) + self.assertEqual(channel.code, HTTPStatus.BAD_REQUEST, channel.result) + self.assertEqual(channel.json_body["errcode"], Codes.INVALID_PARAM) + + def test_allow_backward_is_ignored_when_feature_disabled(self) -> None: + older_event_id = self.helper.send(self.room_id, body="1", tok=self.tok)[ + "event_id" + ] + newer_event_id = self.helper.send(self.room_id, body="2", tok=self.tok)[ + "event_id" + ] + + channel = self.make_request( + "POST", + f"/rooms/{self.room_id}/receipt/{ReceiptTypes.FULLY_READ}/{newer_event_id}", + {}, + access_token=self.tok2, + ) + self.assertEqual(channel.code, 200, channel.result) + + channel = self.make_request( + "POST", + f"/rooms/{self.room_id}/receipt/{ReceiptTypes.FULLY_READ}/{older_event_id}", + {"com.beeper.allow_backward": True}, + access_token=self.tok2, + ) + self.assertEqual(channel.code, 200, channel.result) + self.assertEqual(self._get_fully_read_marker(), newer_event_id) + def _get_read_receipt(self) -> JsonDict | None: """Syncs and returns the read receipt.""" diff --git a/tests/rest/client/test_versions.py b/tests/rest/client/test_versions.py index d6560984690..bbdbe38e072 100644 --- a/tests/rest/client/test_versions.py +++ b/tests/rest/client/test_versions.py @@ -142,6 +142,17 @@ def test_authenticated_with_per_user_feature(self) -> None: channel.json_body, ) + def test_msc4446_false_by_default(self) -> None: + channel = self.make_request("GET", "/_matrix/client/versions") + self.assertEqual(channel.code, 200, channel.result) + self.assertFalse(channel.json_body["unstable_features"]["com.beeper.msc4446"]) + + @unittest.override_config({"experimental_features": {"msc4446_enabled": True}}) + def test_msc4446_true_if_enabled(self) -> None: + channel = self.make_request("GET", "/_matrix/client/versions") + self.assertEqual(channel.code, 200, channel.result) + self.assertTrue(channel.json_body["unstable_features"]["com.beeper.msc4446"]) + def _sanity_check_versions_response(self, versions_response: JsonDict) -> None: """ Make sure this looks like a `/_matrix/client/versions` response