From 9c70a02a9cddf36521c3d6ae6f72e3b46a5f4c2d Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Wed, 17 Jul 2019 19:08:02 +0100 Subject: [PATCH] Ignore redactions of m.room.create events (#5701) --- changelog.d/5701.bugfix | 1 + synapse/api/auth.py | 15 ------------- synapse/handlers/message.py | 33 ++++++++++++++++++++-------- synapse/storage/events_worker.py | 12 ++++++++++ tests/rest/client/test_redactions.py | 20 +++++++++++++++++ 5 files changed, 57 insertions(+), 24 deletions(-) create mode 100644 changelog.d/5701.bugfix diff --git a/changelog.d/5701.bugfix b/changelog.d/5701.bugfix new file mode 100644 index 000000000..fd2866e16 --- /dev/null +++ b/changelog.d/5701.bugfix @@ -0,0 +1 @@ +Ignore redactions of m.room.create events. diff --git a/synapse/api/auth.py b/synapse/api/auth.py index d9e943c39..7ce6540bd 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -606,21 +606,6 @@ class Auth(object): defer.returnValue(auth_ids) - def check_redaction(self, room_version, event, auth_events): - """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 - target event was created by them. - False if the sender is allowed to redact the target event with no - further checks. - - Raises: - AuthError if the event sender is definitely not allowed to redact - the target event. - """ - return event_auth.check_redaction(room_version, event, auth_events) - @defer.inlineCallbacks def check_can_change_room_list(self, room_id, user): """Check if the user is allowed to edit the room's entry in the diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index eaeda7a5c..6d7a987f1 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -23,6 +23,7 @@ from canonicaljson import encode_canonical_json, json from twisted.internet import defer from twisted.internet.defer import succeed +from synapse import event_auth from synapse.api.constants import EventTypes, Membership, RelationTypes from synapse.api.errors import ( AuthError, @@ -784,6 +785,20 @@ class EventCreationHandler(object): event.signatures.update(returned_invite.signatures) if event.type == EventTypes.Redaction: + original_event = yield self.store.get_event( + event.redacts, + check_redacted=False, + get_prev_content=False, + allow_rejected=False, + allow_none=True, + check_room_id=event.room_id, + ) + + # we can make some additional checks now if we have the original event. + if original_event: + if original_event.type == EventTypes.Create: + raise AuthError(403, "Redacting create events is not permitted") + prev_state_ids = yield context.get_prev_state_ids(self.store) auth_events_ids = yield self.auth.compute_auth_events( event, prev_state_ids, for_verification=True @@ -791,18 +806,18 @@ class EventCreationHandler(object): auth_events = yield self.store.get_events(auth_events_ids) auth_events = {(e.type, e.state_key): e for e in auth_events.values()} room_version = yield self.store.get_room_version(event.room_id) - if self.auth.check_redaction(room_version, event, auth_events=auth_events): - original_event = yield self.store.get_event( - event.redacts, - check_redacted=False, - get_prev_content=False, - allow_rejected=False, - allow_none=False, - ) + + if event_auth.check_redaction(room_version, event, auth_events=auth_events): + # this user doesn't have 'redact' rights, so we need to do some more + # checks on the original event. Let's start by checking the original + # event exists. + if not original_event: + raise NotFoundError("Could not find event %s" % (event.redacts,)) + if event.user_id != original_event.user_id: raise AuthError(403, "You don't have permission to redact events") - # We've already checked. + # all the checks are done. event.internal_metadata.recheck_redaction = False if event.type == EventTypes.Create: diff --git a/synapse/storage/events_worker.py b/synapse/storage/events_worker.py index 1d969d70b..858fc755a 100644 --- a/synapse/storage/events_worker.py +++ b/synapse/storage/events_worker.py @@ -259,6 +259,14 @@ class EventsWorkerStore(SQLBaseStore): continue original_event = original_event_entry.event + if original_event.type == EventTypes.Create: + # we never serve redactions of Creates to clients. + logger.info( + "Withholding redaction %s of create event %s", + event_id, + redacted_event_id, + ) + continue if entry.event.internal_metadata.need_to_check_redaction(): original_domain = get_domain_from_id(original_event.sender) @@ -617,6 +625,10 @@ class EventsWorkerStore(SQLBaseStore): Deferred[EventBase|None]: if the event should be redacted, a pruned event object. Otherwise, None. """ + if original_ev.type == "m.room.create": + # we choose to ignore redactions of m.room.create events. + return None + redaction_map = yield self._get_events_from_cache_or_db(redactions) for redaction_id in redactions: diff --git a/tests/rest/client/test_redactions.py b/tests/rest/client/test_redactions.py index 7d5df9585..fe66e397c 100644 --- a/tests/rest/client/test_redactions.py +++ b/tests/rest/client/test_redactions.py @@ -157,3 +157,23 @@ class RedactionsTestCase(HomeserverTestCase): 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_create_event(self): + # control case: an existing event + b = self.helper.send(room_id=self.room_id, tok=self.mod_access_token) + msg_id = b["event_id"] + self._redact_event(self.mod_access_token, self.room_id, msg_id) + + # sync the room, to get the id of the create event + timeline = self._sync_room_timeline(self.other_access_token, self.room_id) + create_event_id = timeline[0]["event_id"] + + # room moderators cannot send redactions for create events + self._redact_event( + self.mod_access_token, self.room_id, create_event_id, expect_code=403 + ) + + # and nor can normals + self._redact_event( + self.other_access_token, self.room_id, create_event_id, expect_code=403 + )