Add forward-compatibility for the redacts property (MSC2174). (#16013)

The location of the redacts field changes in room version 11. Ensure
it is copied to the *new* location for *old* room versions for
forwards-compatibility with clients.

Note that copying it to the *old* location for the *new* room version
was previously handled.

3 files changed, 61 insertions, 25 deletions

diff --git a/changelog.d/16013.misc b/changelog.d/16013.misc
new file mode 100644
index 0000000000..bd161e13ed
--- /dev/null
+++ b/changelog.d/16013.misc
@@ -0,0 +1 @@
+Properly overwrite the `redacts` content-property for forwards-compatibility with room versions 1 through 10.
diff --git a/synapse/events/utils.py b/synapse/events/utils.py
index c890833b1d..967a6c245b 100644
--- a/synapse/events/utils.py
+++ b/synapse/events/utils.py
@@ -475,14 +475,16 @@ def serialize_event(
     if config.as_client_event:
         d = config.event_format(d)
 
-    # If the event is a redaction, copy the redacts field from the content to
-    # top-level for backwards compatibility.
-    if (
-        e.type == EventTypes.Redaction
-        and e.room_version.updated_redaction_rules
-        and e.redacts is not None
-    ):
-        d["redacts"] = e.redacts
+    # If the event is a redaction, the field with the redacted event ID appears
+    # in a different location depending on the room version. e.redacts handles
+    # fetching from the proper location; copy it to the other location for forwards-
+    # and backwards-compatibility with clients.
+    if e.type == EventTypes.Redaction and e.redacts is not None:
+        if e.room_version.updated_redaction_rules:
+            d["redacts"] = e.redacts
+        else:
+            d["content"] = dict(d["content"])
+            d["content"]["redacts"] = e.redacts
 
     only_event_fields = config.only_event_fields
     if only_event_fields:
diff --git a/tests/rest/client/test_redactions.py b/tests/rest/client/test_redactions.py
index 6028886bd6..180b635ea6 100644
--- a/tests/rest/client/test_redactions.py
+++ b/tests/rest/client/test_redactions.py
@@ -13,10 +13,12 @@
 # limitations under the License.
 from typing import List, Optional
 
+from parameterized import parameterized
+
 from twisted.test.proto_helpers import MemoryReactor
 
 from synapse.api.constants import EventTypes, RelationTypes
-from synapse.api.room_versions import RoomVersions
+from synapse.api.room_versions import RoomVersion, RoomVersions
 from synapse.rest import admin
 from synapse.rest.client import login, room, sync
 from synapse.server import HomeServer
@@ -569,50 +571,81 @@ class RedactionsTestCase(HomeserverTestCase):
         self.assertIn("body", event_dict["content"], event_dict)
         self.assertEqual("I'm in a thread!", event_dict["content"]["body"])
 
-    def test_content_redaction(self) -> None:
-        """MSC2174 moved the redacts property to the content."""
+    @parameterized.expand(
+        [
+            # Tuples of:
+            #   Room version
+            #   Boolean: True if the redaction event content should include the event ID.
+            #   Boolean: true if the resulting redaction event is expected to include the
+            #            event ID in the content.
+            (RoomVersions.V10, False, False),
+            (RoomVersions.V11, True, True),
+            (RoomVersions.V11, False, True),
+        ]
+    )
+    def test_redaction_content(
+        self, room_version: RoomVersion, include_content: bool, expect_content: bool
+    ) -> None:
+        """
+        Room version 11 moved the redacts property to the content.
+
+        Ensure that the event gets created properly and that the Client-Server
+        API servers the proper backwards-compatible version.
+        """
         # Create a room with the newer room version.
         room_id = self.helper.create_room_as(
             self.mod_user_id,
             tok=self.mod_access_token,
-            room_version=RoomVersions.V11.identifier,
+            room_version=room_version.identifier,
         )
 
         # Create an event.
         b = self.helper.send(room_id=room_id, tok=self.mod_access_token)
         event_id = b["event_id"]
 
-        # Attempt to redact it with a bogus event ID.
-        self._redact_event(
+        # Ensure the event ID in the URL and the content must match.
+        if include_content:
+            self._redact_event(
+                self.mod_access_token,
+                room_id,
+                event_id,
+                expect_code=400,
+                content={"redacts": "foo"},
+            )
+
+        # Redact it for real.
+        result = self._redact_event(
             self.mod_access_token,
             room_id,
             event_id,
-            expect_code=400,
-            content={"redacts": "foo"},
+            content={"redacts": event_id} if include_content else {},
         )
-
-        # Redact it for real.
-        self._redact_event(self.mod_access_token, room_id, event_id)
+        redaction_event_id = result["event_id"]
 
         # Sync the room, to get the id of the create event
         timeline = self._sync_room_timeline(self.mod_access_token, room_id)
         redact_event = timeline[-1]
         self.assertEqual(redact_event["type"], EventTypes.Redaction)
-        # The redacts key should be in the content.
+        # The redacts key should be in the content and the redacts keys.
         self.assertEquals(redact_event["content"]["redacts"], event_id)
-
-        # It should also be copied as the top-level redacts field for backwards
-        # compatibility.
         self.assertEquals(redact_event["redacts"], event_id)
 
         # But it isn't actually part of the event.
         def get_event(txn: LoggingTransaction) -> JsonDict:
             return db_to_json(
-                main_datastore._fetch_event_rows(txn, [event_id])[event_id].json
+                main_datastore._fetch_event_rows(txn, [redaction_event_id])[
+                    redaction_event_id
+                ].json
             )
 
         main_datastore = self.hs.get_datastores().main
         event_json = self.get_success(
             main_datastore.db_pool.runInteraction("get_event", get_event)
         )
-        self.assertNotIn("redacts", event_json)
+        self.assertEquals(event_json["type"], EventTypes.Redaction)
+        if expect_content:
+            self.assertNotIn("redacts", event_json)
+            self.assertEquals(event_json["content"]["redacts"], event_id)
+        else:
+            self.assertEquals(event_json["redacts"], event_id)
+            self.assertNotIn("redacts", event_json["content"])