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

Commit a27e991

Browse files
committed
More refactoring in get_events_as_list (#5707)
2 parents 4956165 + 2091c91 commit a27e991

4 files changed

Lines changed: 197 additions & 36 deletions

File tree

changelog.d/5707.bugfix

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix some problems with authenticating redactions in recent room versions.

synapse/storage/events_worker.py

Lines changed: 36 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -236,38 +236,47 @@ def get_events_as_list(
236236
"allow_rejected=False"
237237
)
238238

239-
# Starting in room version v3, some redactions need to be rechecked if we
240-
# didn't have the redacted event at the time, so we recheck on read
241-
# instead.
242-
if not allow_rejected and entry.event.type == EventTypes.Redaction:
243-
orig_event_info = yield self._simple_select_one(
244-
table="events",
245-
keyvalues={"event_id": entry.event.redacts},
246-
retcols=["sender", "room_id", "type"],
247-
allow_none=True,
248-
)
239+
# We may not have had the original event when we received a redaction, so
240+
# we have to recheck auth now.
249241

250-
if not orig_event_info:
251-
# We don't have the event that is being redacted, so we
252-
# assume that the event isn't authorized for now. (If we
253-
# later receive the event, then we will always redact
254-
# it anyway, since we have this redaction)
242+
if not allow_rejected and entry.event.type == EventTypes.Redaction:
243+
redacted_event_id = entry.event.redacts
244+
event_map = yield self._get_events_from_cache_or_db([redacted_event_id])
245+
original_event_entry = event_map.get(redacted_event_id)
246+
if not original_event_entry:
247+
# we don't have the redacted event (or it was rejected).
248+
#
249+
# We assume that the redaction isn't authorized for now; if the
250+
# redacted event later turns up, the redaction will be re-checked,
251+
# and if it is found valid, the original will get redacted before it
252+
# is served to the client.
253+
logger.debug(
254+
"Withholding redaction event %s since we don't (yet) have the "
255+
"original %s",
256+
event_id,
257+
redacted_event_id,
258+
)
255259
continue
256260

257-
if orig_event_info["room_id"] != entry.event.room_id:
258-
# Don't process redactions if the redacted event doesn't belong to the
259-
# redaction's room.
260-
logger.info("Ignoring redation in another room.")
261-
continue
261+
original_event = original_event_entry.event
262262

263263
if entry.event.internal_metadata.need_to_check_redaction():
264-
# XXX: we should probably use _get_events_from_cache_or_db here,
265-
# so that we can benefit from caching.
266-
expected_domain = get_domain_from_id(entry.event.sender)
267-
if get_domain_from_id(orig_event_info["sender"]) == expected_domain:
268-
# This redaction event is allowed. Mark as not needing a
269-
# recheck.
270-
entry.event.internal_metadata.recheck_redaction = False
264+
original_domain = get_domain_from_id(original_event.sender)
265+
redaction_domain = get_domain_from_id(entry.event.sender)
266+
if original_domain != redaction_domain:
267+
# the senders don't match, so this is forbidden
268+
logger.info(
269+
"Withholding redaction %s whose sender domain %s doesn't "
270+
"match that of redacted event %s %s",
271+
event_id,
272+
redaction_domain,
273+
redacted_event_id,
274+
original_domain,
275+
)
276+
continue
277+
278+
# Update the cache to save doing the checks again.
279+
entry.event.internal_metadata.recheck_redaction = False
271280

272281
if check_redacted and entry.redacted_event:
273282
event = entry.redacted_event
@@ -631,15 +640,6 @@ def _maybe_redact_event_row(self, original_ev, redactions):
631640
# Senders don't match, so the event isn't actually redacted
632641
continue
633642

634-
if redaction_event.room_id != original_ev.room_id:
635-
continue
636-
637-
else:
638-
# The lack of a redaction likely means that the redaction is invalid
639-
# and therefore not returned by get_event, so it should be safe to
640-
# just ignore it here.
641-
continue
642-
643643
# we found a good redaction event. Redact!
644644
redacted_event = prune_event(original_ev)
645645
redacted_event.unsigned["redacted_by"] = redaction_id
Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,159 @@
1+
# -*- coding: utf-8 -*-
2+
# Copyright 2019 The Matrix.org Foundation C.I.C.
3+
#
4+
# Licensed under the Apache License, Version 2.0 (the "License");
5+
# you may not use this file except in compliance with the License.
6+
# You may obtain a copy of the License at
7+
#
8+
# http://www.apache.org/licenses/LICENSE-2.0
9+
#
10+
# Unless required by applicable law or agreed to in writing, software
11+
# distributed under the License is distributed on an "AS IS" BASIS,
12+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
# See the License for the specific language governing permissions and
14+
# limitations under the License.
15+
16+
from synapse.rest import admin
17+
from synapse.rest.client.v1 import login, room
18+
from synapse.rest.client.v2_alpha import sync
19+
20+
from tests.unittest import HomeserverTestCase
21+
22+
23+
class RedactionsTestCase(HomeserverTestCase):
24+
"""Tests that various redaction events are handled correctly"""
25+
26+
servlets = [
27+
admin.register_servlets,
28+
room.register_servlets,
29+
login.register_servlets,
30+
sync.register_servlets,
31+
]
32+
33+
def prepare(self, reactor, clock, hs):
34+
# register a couple of users
35+
self.mod_user_id = self.register_user("user1", "pass")
36+
self.mod_access_token = self.login("user1", "pass")
37+
self.other_user_id = self.register_user("otheruser", "pass")
38+
self.other_access_token = self.login("otheruser", "pass")
39+
40+
# Create a room
41+
self.room_id = self.helper.create_room_as(
42+
self.mod_user_id, tok=self.mod_access_token
43+
)
44+
45+
# Invite the other user
46+
self.helper.invite(
47+
room=self.room_id,
48+
src=self.mod_user_id,
49+
tok=self.mod_access_token,
50+
targ=self.other_user_id,
51+
)
52+
# The other user joins
53+
self.helper.join(
54+
room=self.room_id, user=self.other_user_id, tok=self.other_access_token
55+
)
56+
57+
def _redact_event(self, access_token, room_id, event_id, expect_code=200):
58+
"""Helper function to send a redaction event.
59+
60+
Returns the json body.
61+
"""
62+
path = "/_matrix/client/r0/rooms/%s/redact/%s" % (room_id, event_id)
63+
64+
request, channel = self.make_request(
65+
"POST", path, content={}, access_token=access_token
66+
)
67+
self.render(request)
68+
self.assertEqual(int(channel.result["code"]), expect_code)
69+
return channel.json_body
70+
71+
def _sync_room_timeline(self, access_token, room_id):
72+
request, channel = self.make_request(
73+
"GET", "sync", access_token=self.mod_access_token
74+
)
75+
self.render(request)
76+
self.assertEqual(channel.result["code"], b"200")
77+
room_sync = channel.json_body["rooms"]["join"][room_id]
78+
return room_sync["timeline"]["events"]
79+
80+
def test_redact_event_as_moderator(self):
81+
# as a regular user, send a message to redact
82+
b = self.helper.send(room_id=self.room_id, tok=self.other_access_token)
83+
msg_id = b["event_id"]
84+
85+
# as the moderator, send a redaction
86+
b = self._redact_event(self.mod_access_token, self.room_id, msg_id)
87+
redaction_id = b["event_id"]
88+
89+
# now sync
90+
timeline = self._sync_room_timeline(self.mod_access_token, self.room_id)
91+
92+
# the last event should be the redaction
93+
self.assertEqual(timeline[-1]["event_id"], redaction_id)
94+
self.assertEqual(timeline[-1]["redacts"], msg_id)
95+
96+
# and the penultimate should be the redacted original
97+
self.assertEqual(timeline[-2]["event_id"], msg_id)
98+
self.assertEqual(timeline[-2]["unsigned"]["redacted_by"], redaction_id)
99+
self.assertEqual(timeline[-2]["content"], {})
100+
101+
def test_redact_event_as_normal(self):
102+
# as a regular user, send a message to redact
103+
b = self.helper.send(room_id=self.room_id, tok=self.other_access_token)
104+
normal_msg_id = b["event_id"]
105+
106+
# also send one as the admin
107+
b = self.helper.send(room_id=self.room_id, tok=self.mod_access_token)
108+
admin_msg_id = b["event_id"]
109+
110+
# as a normal, try to redact the admin's event
111+
self._redact_event(
112+
self.other_access_token, self.room_id, admin_msg_id, expect_code=403
113+
)
114+
115+
# now try to redact our own event
116+
b = self._redact_event(self.other_access_token, self.room_id, normal_msg_id)
117+
redaction_id = b["event_id"]
118+
119+
# now sync
120+
timeline = self._sync_room_timeline(self.other_access_token, self.room_id)
121+
122+
# the last event should be the redaction of the normal event
123+
self.assertEqual(timeline[-1]["event_id"], redaction_id)
124+
self.assertEqual(timeline[-1]["redacts"], normal_msg_id)
125+
126+
# the penultimate should be the unredacted one from the admin
127+
self.assertEqual(timeline[-2]["event_id"], admin_msg_id)
128+
self.assertNotIn("redacted_by", timeline[-2]["unsigned"])
129+
self.assertTrue(timeline[-2]["content"]["body"], {})
130+
131+
# and the antepenultimate should be the redacted normal
132+
self.assertEqual(timeline[-3]["event_id"], normal_msg_id)
133+
self.assertEqual(timeline[-3]["unsigned"]["redacted_by"], redaction_id)
134+
self.assertEqual(timeline[-3]["content"], {})
135+
136+
def test_redact_nonexistent_event(self):
137+
# control case: an existing event
138+
b = self.helper.send(room_id=self.room_id, tok=self.other_access_token)
139+
msg_id = b["event_id"]
140+
b = self._redact_event(self.other_access_token, self.room_id, msg_id)
141+
redaction_id = b["event_id"]
142+
143+
# room moderators can send redactions for non-existent events
144+
self._redact_event(self.mod_access_token, self.room_id, "$zzz")
145+
146+
# ... but normals cannot
147+
self._redact_event(
148+
self.other_access_token, self.room_id, "$zzz", expect_code=404
149+
)
150+
151+
# when we sync, we should see only the valid redaction
152+
timeline = self._sync_room_timeline(self.other_access_token, self.room_id)
153+
self.assertEqual(timeline[-1]["event_id"], redaction_id)
154+
self.assertEqual(timeline[-1]["redacts"], msg_id)
155+
156+
# and the penultimate should be the redacted original
157+
self.assertEqual(timeline[-2]["event_id"], msg_id)
158+
self.assertEqual(timeline[-2]["unsigned"]["redacted_by"], redaction_id)
159+
self.assertEqual(timeline[-2]["content"], {})

tests/unittest.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -447,6 +447,7 @@ def register_user(self, username, password, admin=False):
447447
# Create the user
448448
request, channel = self.make_request("GET", "/_matrix/client/r0/admin/register")
449449
self.render(request)
450+
self.assertEqual(channel.code, 200)
450451
nonce = channel.json_body["nonce"]
451452

452453
want_mac = hmac.new(key=b"shared", digestmod=hashlib.sha1)

0 commit comments

Comments
 (0)