Apply the MSC4354 Sticky Events history visibility exemptions on all client-server endpoints. (#20295)

Part of: MSC4354 (Experimental Feature tracking issue:
https://github.com/element-hq/synapse/issues/19409)

---------

Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org>

changelog.d/20295.bugfix

@@ -0,0 +1 @@
+Apply the [MSC4354 Sticky Events](https://github.com/matrix-org/matrix-spec-proposals/pull/4354) history visibility exemptions on all client-server endpoints.

synapse/handlers/sliding_sync/extensions.py

@@ -1100,10 +1100,6 @@ class SlidingSyncExtensionHandler:
             self._storage_controllers,
             sync_config.user.to_string(),
             unfiltered_events,
-            # As per MSC4354:
-            # > History visibility checks MUST NOT be applied to sticky events.
-            # > Any joined user is authorised to see sticky events for the duration they remain sticky.
-            always_include_ids=frozenset(all_sticky_event_ids),
         )
         filtered_event_map = {ev.event.event_id: ev for ev in filtered_events}
 

synapse/handlers/sync.py

@@ -3553,10 +3553,6 @@ class SyncHandler:
                             self._storage_controllers,
                             sync_result_builder.sync_config.user.to_string(),
                             await self.store.get_events_as_list(sticky_event_ids),
-                            # As per MSC4354:
-                            # > History visibility checks MUST NOT be applied to sticky events.
-                            # > Any joined user is authorised to see sticky events for the duration they remain sticky.
-                            always_include_ids=frozenset(sticky_event_ids),
                         )
                 room_sync = JoinedSyncResult(
                     room_id=room_id,

synapse/visibility.py

@@ -182,6 +182,7 @@ async def filter_and_transform_events_for_client(
             user_id=user_id,
             event=event,
             clock=storage.main.clock,
+            msc4354_enabled=storage.main.hs.config.experimental.msc4354_enabled,
             filter_send_to_client=filter_send_to_client,
             sender_ignored=event.sender in ignore_list,
             always_include_ids=always_include_ids,
@@ -372,6 +373,7 @@ def _check_client_allowed_to_see_event(
     user_id: str,
     event: EventBase,
     clock: Clock,
+    msc4354_enabled: bool,
     filter_send_to_client: bool,
     is_peeking: bool,
     always_include_ids: frozenset[str],
@@ -388,6 +390,7 @@ def _check_client_allowed_to_see_event(
         user_id
         event
         clock
+        msc4354_enabled: Whether MSC4354 Sticky Events are enabled.
         filter_send_to_client
         is_peeking
         always_include_ids
@@ -444,6 +447,25 @@ def _check_client_allowed_to_see_event(
     if state is None:
         raise Exception("Missing state for non-outlier event")
 
+    if msc4354_enabled and not is_peeking:
+        # For sticky events, apply an exemption from the membership test part of history visibility rules.
+        # Essentially, this alters `invited`/`joined` history visibility rules to `shared` for sticky events.
+        #
+        # > History visibility **checks** MUST NOT be applied to sticky events. This applies to all endpoints where the sticky events could be returned.
+        # > Any joined user or server is authorised to see sticky events for the duration they remain sticky.[^hisvis]
+        # > — https://github.com/matrix-org/matrix-spec-proposals/blob/3635b765267452eb452cd2ced963aee52d1fd185/proposals/4354-sticky-events.md#L101-L102
+        #
+        # > [^hisvis]: This ensures that newly joined servers can see sticky events sent from before they were joined to the room, regardless
+        # > of the history visibility setting. This matches the behaviour of state events.
+        # > — https://github.com/matrix-org/matrix-spec-proposals/blob/3635b765267452eb452cd2ced963aee52d1fd185/proposals/4354-sticky-events.md#L501-L502
+        sticky_until_ts = event.locally_sticky_until_ts()
+        if (
+            sticky_until_ts is not None
+            # Check the event is still sticky
+            and clock.time_msec() < sticky_until_ts
+        ):
+            return event
+
     # get the room_visibility at the time of the event.
     visibility = get_effective_room_visibility_from_state(state)
 

tests/rest/client/sliding_sync/test_extension_sticky_events.py

@@ -519,15 +519,14 @@ class SlidingSyncStickyEventsExtensionTestCase(SlidingSyncBase):
         }
         response_body, _ = self.do_sync(sync_body, tok=user2_tok)
 
-        # The sticky event is fully visible in its own right,
-        # but AFAICT the timeline only includes events since we join the room
-        # (regardless of history visibility),
-        # so this comes down in the sticky extension
-        self._assert_sticky_events_response(response_body, {room_id: [sticky_event_id]})
-
-        # Instead the sticky event is in the timeline
         timeline_events = response_body["rooms"][room_id]["timeline"]
         timeline_event_ids = [e["event_id"] for e in timeline_events]
+
+        # The sticky event is recent enough to be included in timeline instead of in the sticky section.
+        self.assertIn(sticky_event_id, timeline_event_ids)
+        self._assert_sticky_events_response(response_body, {room_id: []})
+
+        # Sanity check: the regular (non-sticky) event is NOT history-visible.
         self.assertNotIn(
             regular_event_id,
             timeline_event_ids,

tests/rest/client/test_sticky_events.py

@@ -14,6 +14,7 @@
 #
 
 import sqlite3
+from http import HTTPStatus
 
 from twisted.internet.testing import MemoryReactor
 
@@ -123,6 +124,71 @@ class StickyEventsClientTestCase(unittest.HomeserverTestCase):
         self.reactor.advance(Duration(seconds=1).as_secs())
         self._assert_event_not_sticky(event_id)
 
+    def test_joined_history_visibility_messages_returns_sticky_events(
+        self,
+    ) -> None:
+        """
+        Tests that, in a joined history visibility room, `/messages` returns sticky events
+        from before your join.
+        """
+
+        room_id = self.helper.create_room_as("resident", tok=self.token)
+        self.helper.send_state(
+            room_id,
+            "m.room.history_visibility",
+            body={"history_visibility": "joined"},
+            tok=self.token,
+        )
+
+        # Now send 3 timeline events:
+        # regular, sticky, regular
+        # (We have this 'sandwich' to prove that history visibility filtering is working)
+
+        # A message sent before the joiner joins; must not be visible to them.
+        self.helper.send(room_id, body="regular 1", tok=self.token)
+        sticky_event_id = self.helper.send_sticky_event(
+            room_id,
+            type=EventTypes.Message,
+            content={"body": "", "msgtype": ""},
+            duration=Duration(hours=1),
+            tok=self.token,
+        )["event_id"]
+        self.helper.send(room_id, body="regular 2", tok=self.token)
+
+        # Now get another user to join
+        self.register_user("joiner", "p2")
+        joiner_token = self.login("joiner", "p2")
+        self.helper.join(room_id, "@joiner:test", tok=joiner_token)
+
+        # A message sent after the newly-joined joins; must be visible to them.
+        sent_after_join_event_id = self.helper.send(
+            room_id, body="after_join", tok=self.token
+        )["event_id"]
+
+        # The newly-joined user backpaginates the room via /messages.
+        channel = self.make_request(
+            "GET",
+            f"/rooms/{room_id}/messages?dir=b",
+            access_token=joiner_token,
+        )
+        self.assertEqual(channel.code, HTTPStatus.OK, channel.result)
+
+        visible_message_event_ids = [
+            e["event_id"]
+            for e in channel.json_body["chunk"]
+            if e["type"] == EventTypes.Message
+        ]
+
+        # The newly-joined user only sees the message sent since they joined
+        # and the sticky event (which is exempt from history visibility)
+        self.assertEqual(
+            visible_message_event_ids,
+            [
+                sent_after_join_event_id,
+                sticky_event_id,
+            ],
+        )
+
 
 class StickyEventsDisabledClientTestCase(unittest.HomeserverTestCase):
     """

tests/rest/client/test_sync_sticky_events.py

@@ -607,27 +607,27 @@ class SyncStickyEventsTestCase(unittest.HomeserverTestCase):
         )
         self.assertEqual(channel.code, 200, channel.result)
 
-        # Get sticky events from the sync response
-        sticky_events = channel.json_body["rooms"]["join"][room_id]["msc4354_sticky"][
-            "events"
-        ]
-        sticky_event_ids = [event["event_id"] for event in sticky_events]
+        room_entry = channel.json_body["rooms"]["join"][room_id]
 
-        # The sticky event should be visible to user2
-        self.assertEqual(
-            [sticky_event_id],
-            sticky_event_ids,
+        # As it is recent enough, the sticky event goes down the timeline section.
+        timeline_events = room_entry["timeline"]["events"]
+        timeline_event_ids = [event["event_id"] for event in timeline_events]
+        self.assertIn(
+            sticky_event_id,
+            timeline_event_ids,
             f"Sticky event {sticky_event_id} should be visible despite history visibility",
         )
 
-        # Also check that the regular (non-sticky) event sent at the same time
-        # is NOT visible. This is to verify our test setup.
-        timeline_events = channel.json_body["rooms"]["join"][room_id]["timeline"][
-            "events"
-        ]
-        timeline_event_ids = [event["event_id"] for event in timeline_events]
+        # The sticky event was sent down the timeline,
+        # so it must not be duplicated in the sticky events section.
+        self.assertNotIn(
+            "msc4354_sticky",
+            room_entry,
+            f"Sticky event {sticky_event_id} was already sent down the timeline, so not expecting to see a sticky events section: {room_entry}",
+        )
 
-        # The regular message should NOT be visible (history visibility = joined)
+        # Also check that the regular (non-sticky) event sent at the same time
+        # is NOT visible (because history visibility is `joined`). This is to verify our test setup.
         self.assertNotIn(
             regular_event_id,
             timeline_event_ids,