diff --git a/changelog.d/5707.bugfix b/changelog.d/5707.bugfix
new file mode 100644
index 0000000000..aa3046c5e1
--- /dev/null
+++ b/changelog.d/5707.bugfix
@@ -0,0 +1 @@
+Fix some problems with authenticating redactions in recent room versions.
diff --git a/synapse/storage/events_worker.py b/synapse/storage/events_worker.py
index e2fc7bc047..1d969d70be 100644
--- a/synapse/storage/events_worker.py
+++ b/synapse/storage/events_worker.py
@@ -236,38 +236,48 @@ class EventsWorkerStore(SQLBaseStore):
"allow_rejected=False"
)
- # Starting in room version v3, some redactions need to be rechecked if we
- # didn't have the redacted event at the time, so we recheck on read
- # instead.
+ # We may not have had the original event when we received a redaction, so
+ # we have to recheck auth now.
+
if not allow_rejected and entry.event.type == EventTypes.Redaction:
- if entry.event.internal_metadata.need_to_check_redaction():
- # XXX: we should probably use _get_events_from_cache_or_db here,
- # so that we can benefit from caching.
- orig_sender = yield self._simple_select_one_onecol(
- table="events",
- keyvalues={"event_id": entry.event.redacts},
- retcol="sender",
- allow_none=True,
+ redacted_event_id = entry.event.redacts
+ event_map = yield self._get_events_from_cache_or_db([redacted_event_id])
+ original_event_entry = event_map.get(redacted_event_id)
+ if not original_event_entry:
+ # we don't have the redacted event (or it was rejected).
+ #
+ # We assume that the redaction isn't authorized for now; if the
+ # redacted event later turns up, the redaction will be re-checked,
+ # and if it is found valid, the original will get redacted before it
+ # is served to the client.
+ logger.debug(
+ "Withholding redaction event %s since we don't (yet) have the "
+ "original %s",
+ event_id,
+ redacted_event_id,
)
+ continue
- expected_domain = get_domain_from_id(entry.event.sender)
- if (
- orig_sender
- and get_domain_from_id(orig_sender) == expected_domain
- ):
- # This redaction event is allowed. Mark as not needing a
- # recheck.
- entry.event.internal_metadata.recheck_redaction = False
- else:
- # We either don't have the event that is being redacted (so we
- # assume that the event isn't authorised for now), or the
- # senders don't match (so it will never be authorised). Either
- # way, we shouldn't return it.
- #
- # (If we later receive the event, then we will redact it anyway,
- # since we have this redaction)
+ original_event = original_event_entry.event
+
+ if entry.event.internal_metadata.need_to_check_redaction():
+ original_domain = get_domain_from_id(original_event.sender)
+ redaction_domain = get_domain_from_id(entry.event.sender)
+ if original_domain != redaction_domain:
+ # the senders don't match, so this is forbidden
+ logger.info(
+ "Withholding redaction %s whose sender domain %s doesn't "
+ "match that of redacted event %s %s",
+ event_id,
+ redaction_domain,
+ redacted_event_id,
+ original_domain,
+ )
continue
+ # Update the cache to save doing the checks again.
+ entry.event.internal_metadata.recheck_redaction = False
+
if check_redacted and entry.redacted_event:
event = entry.redacted_event
else:
diff --git a/tests/rest/client/test_redactions.py b/tests/rest/client/test_redactions.py
new file mode 100644
index 0000000000..7d5df95855
--- /dev/null
+++ b/tests/rest/client/test_redactions.py
@@ -0,0 +1,159 @@
+# -*- coding: utf-8 -*-
+# Copyright 2019 The Matrix.org Foundation C.I.C.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from synapse.rest import admin
+from synapse.rest.client.v1 import login, room
+from synapse.rest.client.v2_alpha import sync
+
+from tests.unittest import HomeserverTestCase
+
+
+class RedactionsTestCase(HomeserverTestCase):
+ """Tests that various redaction events are handled correctly"""
+
+ servlets = [
+ admin.register_servlets,
+ room.register_servlets,
+ login.register_servlets,
+ sync.register_servlets,
+ ]
+
+ def prepare(self, reactor, clock, hs):
+ # register a couple of users
+ self.mod_user_id = self.register_user("user1", "pass")
+ self.mod_access_token = self.login("user1", "pass")
+ self.other_user_id = self.register_user("otheruser", "pass")
+ self.other_access_token = self.login("otheruser", "pass")
+
+ # Create a room
+ self.room_id = self.helper.create_room_as(
+ self.mod_user_id, tok=self.mod_access_token
+ )
+
+ # Invite the other user
+ self.helper.invite(
+ room=self.room_id,
+ src=self.mod_user_id,
+ tok=self.mod_access_token,
+ targ=self.other_user_id,
+ )
+ # The other user joins
+ self.helper.join(
+ room=self.room_id, user=self.other_user_id, tok=self.other_access_token
+ )
+
+ def _redact_event(self, access_token, room_id, event_id, expect_code=200):
+ """Helper function to send a redaction event.
+
+ Returns the json body.
+ """
+ path = "/_matrix/client/r0/rooms/%s/redact/%s" % (room_id, event_id)
+
+ request, channel = self.make_request(
+ "POST", path, content={}, access_token=access_token
+ )
+ self.render(request)
+ self.assertEqual(int(channel.result["code"]), expect_code)
+ return channel.json_body
+
+ def _sync_room_timeline(self, access_token, room_id):
+ request, channel = self.make_request(
+ "GET", "sync", access_token=self.mod_access_token
+ )
+ self.render(request)
+ self.assertEqual(channel.result["code"], b"200")
+ room_sync = channel.json_body["rooms"]["join"][room_id]
+ return room_sync["timeline"]["events"]
+
+ def test_redact_event_as_moderator(self):
+ # as a regular user, send a message to redact
+ b = self.helper.send(room_id=self.room_id, tok=self.other_access_token)
+ msg_id = b["event_id"]
+
+ # as the moderator, send a redaction
+ b = self._redact_event(self.mod_access_token, self.room_id, msg_id)
+ redaction_id = b["event_id"]
+
+ # now sync
+ timeline = self._sync_room_timeline(self.mod_access_token, self.room_id)
+
+ # the last event should be the redaction
+ self.assertEqual(timeline[-1]["event_id"], redaction_id)
+ self.assertEqual(timeline[-1]["redacts"], msg_id)
+
+ # and the penultimate should be the redacted original
+ self.assertEqual(timeline[-2]["event_id"], msg_id)
+ self.assertEqual(timeline[-2]["unsigned"]["redacted_by"], redaction_id)
+ self.assertEqual(timeline[-2]["content"], {})
+
+ def test_redact_event_as_normal(self):
+ # as a regular user, send a message to redact
+ b = self.helper.send(room_id=self.room_id, tok=self.other_access_token)
+ normal_msg_id = b["event_id"]
+
+ # also send one as the admin
+ b = self.helper.send(room_id=self.room_id, tok=self.mod_access_token)
+ admin_msg_id = b["event_id"]
+
+ # as a normal, try to redact the admin's event
+ self._redact_event(
+ self.other_access_token, self.room_id, admin_msg_id, expect_code=403
+ )
+
+ # now try to redact our own event
+ b = self._redact_event(self.other_access_token, self.room_id, normal_msg_id)
+ redaction_id = b["event_id"]
+
+ # now sync
+ timeline = self._sync_room_timeline(self.other_access_token, self.room_id)
+
+ # the last event should be the redaction of the normal event
+ self.assertEqual(timeline[-1]["event_id"], redaction_id)
+ self.assertEqual(timeline[-1]["redacts"], normal_msg_id)
+
+ # the penultimate should be the unredacted one from the admin
+ self.assertEqual(timeline[-2]["event_id"], admin_msg_id)
+ self.assertNotIn("redacted_by", timeline[-2]["unsigned"])
+ self.assertTrue(timeline[-2]["content"]["body"], {})
+
+ # and the antepenultimate should be the redacted normal
+ self.assertEqual(timeline[-3]["event_id"], normal_msg_id)
+ self.assertEqual(timeline[-3]["unsigned"]["redacted_by"], redaction_id)
+ self.assertEqual(timeline[-3]["content"], {})
+
+ def test_redact_nonexistent_event(self):
+ # control case: an existing event
+ b = self.helper.send(room_id=self.room_id, tok=self.other_access_token)
+ msg_id = b["event_id"]
+ b = self._redact_event(self.other_access_token, self.room_id, msg_id)
+ redaction_id = b["event_id"]
+
+ # room moderators can send redactions for non-existent events
+ self._redact_event(self.mod_access_token, self.room_id, "$zzz")
+
+ # ... but normals cannot
+ self._redact_event(
+ self.other_access_token, self.room_id, "$zzz", expect_code=404
+ )
+
+ # when we sync, we should see only the valid redaction
+ timeline = self._sync_room_timeline(self.other_access_token, self.room_id)
+ self.assertEqual(timeline[-1]["event_id"], redaction_id)
+ self.assertEqual(timeline[-1]["redacts"], msg_id)
+
+ # and the penultimate should be the redacted original
+ self.assertEqual(timeline[-2]["event_id"], msg_id)
+ self.assertEqual(timeline[-2]["unsigned"]["redacted_by"], redaction_id)
+ self.assertEqual(timeline[-2]["content"], {})
diff --git a/tests/unittest.py b/tests/unittest.py
index cabe787cb4..f5fae21317 100644
--- a/tests/unittest.py
+++ b/tests/unittest.py
@@ -447,6 +447,7 @@ class HomeserverTestCase(TestCase):
# Create the user
request, channel = self.make_request("GET", "/_matrix/client/r0/admin/register")
self.render(request)
+ self.assertEqual(channel.code, 200)
nonce = channel.json_body["nonce"]
want_mac = hmac.new(key=b"shared", digestmod=hashlib.sha1)
|
