Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Implement MSC2174: move redacts to a content property. #15395

Merged
merged 4 commits into from
Apr 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/15395.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Implement [MSC2174](https://github.com/matrix-org/matrix-spec-proposals/pull/2174) to move the `redacts` key to a `content` property.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that MSC2174 says to redact this property, but since the implementation is pair with MSC2176 which states that the entire body of the create event is to be preserved I think we're OK here since we're not adding it to the body anyway?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I am unsure what you mean here. I don't think MSC2174 is talking about how/if we want to redact redacts?
MSC2176 however seems to suggest that we want to keep redacts for m.room.redaction events, which seems to be the case.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh, sorry. I meant that MSC2176 says that we should not redact the redacts key, which will now exist with this implementation. 👍

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment thread would likely have been more useful on the msc2176_redaction_rules variable in RoomVersion -- I think my point was that this is essentially paired with MSC2176 in implementation. You can't enable just this MSC.

3 changes: 2 additions & 1 deletion synapse/api/room_versions.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ class RoomVersion:
limit_notifications_power_levels: bool
# MSC2175: No longer include the creator in m.room.create events.
msc2175_implicit_room_creator: bool
# MSC2174/MSC2176: Apply updated redaction rules algorithm.
# MSC2174/MSC2176: Apply updated redaction rules algorithm, move redacts to
# content property.
msc2176_redaction_rules: bool
# MSC3083: Support the 'restricted' join_rule.
msc3083_join_rules: bool
Expand Down
2 changes: 1 addition & 1 deletion synapse/event_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -793,7 +793,7 @@ def check_redaction(
"""Check whether the event sender is allowed to redact the target event.

Returns:
True if the the sender is allowed to redact the target event if the
True if the sender is allowed to redact the target event if the
target event was created by them.
False if the sender is allowed to redact the target event with no
further checks.
Expand Down
8 changes: 7 additions & 1 deletion synapse/events/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,6 @@ def __init__(
hashes: DictProperty[Dict[str, str]] = DictProperty("hashes")
origin: DictProperty[str] = DictProperty("origin")
origin_server_ts: DictProperty[int] = DictProperty("origin_server_ts")
redacts: DefaultDictProperty[Optional[str]] = DefaultDictProperty("redacts", None)
room_id: DictProperty[str] = DictProperty("room_id")
sender: DictProperty[str] = DictProperty("sender")
# TODO state_key should be Optional[str]. This is generally asserted in Synapse
Expand All @@ -346,6 +345,13 @@ def event_id(self) -> str:
def membership(self) -> str:
return self.content["membership"]

@property
def redacts(self) -> Optional[str]:
"""MSC2176 moved the redacts field into the content."""
if self.room_version.msc2176_redaction_rules:
return self.content.get("redacts")
return self.get("redacts")

def is_state(self) -> bool:
return self.get_state_key() is not None

Expand Down
4 changes: 3 additions & 1 deletion synapse/events/builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,9 @@ async def build(
if self.is_state():
event_dict["state_key"] = self._state_key

if self._redacts is not None:
# MSC2174 moves the redacts property to the content, it is invalid to
# provide it as a top-level property.
if self._redacts is not None and not self.room_version.msc2176_redaction_rules:
event_dict["redacts"] = self._redacts

if self._origin_server_ts is not None:
Expand Down
35 changes: 26 additions & 9 deletions synapse/rest/client/room.py
Original file line number Diff line number Diff line change
Expand Up @@ -1096,6 +1096,7 @@ def __init__(self, hs: "HomeServer"):
super().__init__(hs)
self.event_creation_handler = hs.get_event_creation_handler()
self.auth = hs.get_auth()
self._store = hs.get_datastores().main
self._relation_handler = hs.get_relations_handler()
self._msc3912_enabled = hs.config.experimental.msc3912_enabled

Expand All @@ -1113,6 +1114,19 @@ async def _do(
) -> Tuple[int, JsonDict]:
content = parse_json_object_from_request(request)

# Ensure the redacts property in the content matches the one provided in
# the URL.
room_version = await self._store.get_room_version(room_id)
if room_version.msc2176_redaction_rules:
if "redacts" in content and content["redacts"] != event_id:
raise SynapseError(
400,
"Cannot provide a redacts value incoherent with the event_id of the URL parameter",
Codes.INVALID_PARAM,
)
else:
content["redacts"] = event_id

try:
with_relations = None
if self._msc3912_enabled and "org.matrix.msc3912.with_relations" in content:
Expand All @@ -1128,20 +1142,23 @@ async def _do(
requester, txn_id, room_id
)

# Event is not yet redacted, create a new event to redact it.
if event is None:
event_dict = {
"type": EventTypes.Redaction,
"content": content,
"room_id": room_id,
"sender": requester.user.to_string(),
}
# Earlier room versions had a top-level redacts property.
if not room_version.msc2176_redaction_rules:
event_dict["redacts"] = event_id

(
event,
_,
) = await self.event_creation_handler.create_and_send_nonmember_event(
requester,
{
"type": EventTypes.Redaction,
"content": content,
"room_id": room_id,
"sender": requester.user.to_string(),
"redacts": event_id,
},
txn_id=txn_id,
requester, event_dict, txn_id=txn_id
)

if with_relations:
Expand Down
12 changes: 10 additions & 2 deletions tests/events/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,11 @@ def test_redacts(self) -> None:
"""Redaction events have no special behaviour until MSC2174/MSC2176."""

self.run_test(
{"type": "m.room.redaction", "content": {"redacts": "$test2:domain"}},
{
"type": "m.room.redaction",
"content": {"redacts": "$test2:domain"},
"redacts": "$test2:domain",
},
{
"type": "m.room.redaction",
"content": {},
Expand All @@ -330,7 +334,11 @@ def test_redacts(self) -> None:

# After MSC2174, redaction events keep the redacts content key.
self.run_test(
{"type": "m.room.redaction", "content": {"redacts": "$test2:domain"}},
{
"type": "m.room.redaction",
"content": {"redacts": "$test2:domain"},
"redacts": "$test2:domain",
},
{
"type": "m.room.redaction",
"content": {"redacts": "$test2:domain"},
Expand Down
39 changes: 37 additions & 2 deletions tests/rest/client/test_redactions.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
from twisted.test.proto_helpers import MemoryReactor

from synapse.api.constants import EventTypes, RelationTypes
from synapse.api.room_versions import RoomVersions
from synapse.rest import admin
from synapse.rest.client import login, room, sync
from synapse.server import HomeServer
Expand Down Expand Up @@ -74,14 +75,15 @@ def _redact_event(
event_id: str,
expect_code: int = 200,
with_relations: Optional[List[str]] = None,
content: Optional[JsonDict] = None,
) -> JsonDict:
"""Helper function to send a redaction event.

Returns the json body.
"""
path = "/_matrix/client/r0/rooms/%s/redact/%s" % (room_id, event_id)

request_content = {}
request_content = content or {}
if with_relations:
request_content["org.matrix.msc3912.with_relations"] = with_relations

Expand All @@ -92,7 +94,7 @@ def _redact_event(
return channel.json_body

def _sync_room_timeline(self, access_token: str, room_id: str) -> List[JsonDict]:
channel = self.make_request("GET", "sync", access_token=self.mod_access_token)
channel = self.make_request("GET", "sync", access_token=access_token)
self.assertEqual(channel.code, 200)
room_sync = channel.json_body["rooms"]["join"][room_id]
return room_sync["timeline"]["events"]
Expand Down Expand Up @@ -466,3 +468,36 @@ def test_redact_relations_txn_id_reuse(self) -> None:
)
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."""
# 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.MSC2176.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(
self.mod_access_token,
room_id,
event_id,
expect_code=400,
content={"redacts": "foo"},
)

# Redact it for real.
self._redact_event(self.mod_access_token, room_id, 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.
self.assertNotIn("redacts", redact_event)
self.assertEquals(redact_event["content"]["redacts"], event_id)