From 0766119fc93dbfd1cd35978a2d5ad0e1d8a24ad1 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Wed, 11 Jun 2025 17:55:15 -0700 Subject: [PATCH 01/18] add support for MSC4293 --- synapse/config/experimental.py | 3 + synapse/handlers/federation_event.py | 39 +++++++++++ synapse/storage/databases/main/events.py | 69 ++++++++++++++++++- .../06_redactions_multitarget.sql.postgres | 7 ++ .../92/06_redactions_multitarget.sql.sqlite | 23 +++++++ 5 files changed, 139 insertions(+), 2 deletions(-) create mode 100644 synapse/storage/schema/main/delta/92/06_redactions_multitarget.sql.postgres create mode 100644 synapse/storage/schema/main/delta/92/06_redactions_multitarget.sql.sqlite diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index 259b2c70cbf..f09f6f2e7ec 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -569,3 +569,6 @@ def read_config( # MSC4155: Invite filtering self.msc4155_enabled: bool = experimental.get("msc4155_enabled", False) + + # MSC4293: Redact on Kick/Ban + self.msc4239_enabled: bool = experimental.get("msc4239_enabled", False) diff --git a/synapse/handlers/federation_event.py b/synapse/handlers/federation_event.py index 1e738f484f9..8b6422ac031 100644 --- a/synapse/handlers/federation_event.py +++ b/synapse/handlers/federation_event.py @@ -1966,6 +1966,9 @@ async def _check_for_soft_fail( Does nothing for events in rooms with partial state, since we may not have an accurate membership event for the sender in the current state. + Also checks if event should be redacted due to a MSC4293 redaction flag in kick/ban + event for user + Args: event context: The `EventContext` which we are about to persist the event with. @@ -2065,6 +2068,42 @@ async def _check_for_soft_fail( soft_failed_event_counter.inc() event.internal_metadata.soft_failed = True + if self._config.experimental.msc4239_enabled: + # Use already calculated auth events to determine if the event should be redacted due to kick/ban + if event.type == EventTypes.Message: + for auth_event in current_auth_events: + if ( + auth_event.type == EventTypes.Member + and auth_event.state_key == event.sender + ): + if auth_event.membership == Membership.BAN or ( + auth_event.membership == Membership.LEAVE + and auth_event.sender != event.sender + ): + # we have a ban or kick for this sender, check for redaction flag and apply if found + autoredact = auth_event.content.get( + "org.matrix.msc4293.redact_events", False + ) + if autoredact: + await self._store.db_pool.simple_upsert( + table="redactions", + keyvalues={ + "event_id": auth_event.event_id, + "redacts": event.event_id, + }, + values={"received_ts": self._clock.time_msec()}, + insertion_values={ + "event_id": auth_event.event_id, + "redacts": event.event_id, + "received_ts": self._clock.time_msec(), + }, + ) + await self._store.db_pool.runInteraction( + "invalidate cache", + self._store.invalidate_get_event_cache_after_txn, + event.event_id, + ) + async def _load_or_fetch_auth_events_for_event( self, destination: Optional[str], event: EventBase ) -> Collection[EventBase]: diff --git a/synapse/storage/databases/main/events.py b/synapse/storage/databases/main/events.py index b7cc0433e75..9b509481819 100644 --- a/synapse/storage/databases/main/events.py +++ b/synapse/storage/databases/main/events.py @@ -376,6 +376,72 @@ async def _persist_events_and_state_updates( event_counter.labels(event.type, origin_type, origin_entity).inc() + if self.hs.config.experimental.msc4239_enabled: + if event.type == EventTypes.Member and event.content.get( + "org.matrix.msc4293.redact_events", False + ): + if event.membership != Membership.BAN and not ( + event.membership == Membership.LEAVE + and event.sender != event.state_key + ): + return + # check that sender can redact + state_filter = StateFilter.from_types( + [(EventTypes.PowerLevels, "")] + ) + state = await self.store.get_partial_filtered_current_state_ids( + event.room_id, state_filter + ) + pl_id = state[(EventTypes.PowerLevels, "")] + pl_event_map = await self.store.get_events([pl_id]) + pl_event = pl_event_map.get(pl_id) + if pl_event: + sender_level = pl_event.content.get("users", {}).get( + event.sender + ) + if sender_level is None: + sender_level = pl_event.content.get("users_default", 0) + redact_level = pl_event.content.get("redact", 50) + + if sender_level > redact_level: + ids_to_redact = ( + await self.store.get_events_sent_by_user_in_room( + event.state_key, + event.room_id, + limit=1000000, # arbitrarily large number + filter=[ + "m.room.member", + "m.room.message", + "m.room.encrypted", + ], + ) + ) + if ids_to_redact: + key_values = [ + (event.event_id, x) for x in ids_to_redact + ] + value_values = [ + (self._clock.time_msec(),) + for x in ids_to_redact + ] + await self.db_pool.simple_upsert_many( + table="redactions", + key_names=["event_id", "redacts"], + key_values=key_values, + value_names=["received_ts"], + value_values=value_values, + desc="redact_on_ban_redaction_txn", + ) + # normally the cache entry for a redacted event would be invalidated + # by an arriving redaction event, but since we are not creating redaction + # events we invalidate manually + for id in ids_to_redact: + await self.db_pool.runInteraction( + "invalidate cache", + self.store.invalidate_get_event_cache_after_txn, + id, + ) + if new_forward_extremities: self.store.get_latest_event_ids_in_room.prefill( (room_id,), frozenset(new_forward_extremities) @@ -2768,9 +2834,8 @@ def _store_redaction(self, txn: LoggingTransaction, event: EventBase) -> None: self.db_pool.simple_upsert_txn( txn, table="redactions", - keyvalues={"event_id": event.event_id}, + keyvalues={"event_id": event.event_id, "redacts": event.redacts}, values={ - "redacts": event.redacts, "received_ts": self._clock.time_msec(), }, ) diff --git a/synapse/storage/schema/main/delta/92/06_redactions_multitarget.sql.postgres b/synapse/storage/schema/main/delta/92/06_redactions_multitarget.sql.postgres new file mode 100644 index 00000000000..3989c9befe6 --- /dev/null +++ b/synapse/storage/schema/main/delta/92/06_redactions_multitarget.sql.postgres @@ -0,0 +1,7 @@ +ALTER TABLE ONLY redactions ADD CONSTRAINT redactions_event_id_redacts_key UNIQUE (event_id, redacts); + +-- Replace the index we're about to drop +CREATE INDEX redactions_event_id ON redactions USING btree (event_id); + +-- Drop the old constraint +ALTER TABLE ONLY redactions DROP CONSTRAINT redactions_event_id_key; \ No newline at end of file diff --git a/synapse/storage/schema/main/delta/92/06_redactions_multitarget.sql.sqlite b/synapse/storage/schema/main/delta/92/06_redactions_multitarget.sql.sqlite new file mode 100644 index 00000000000..d0aa5c32f74 --- /dev/null +++ b/synapse/storage/schema/main/delta/92/06_redactions_multitarget.sql.sqlite @@ -0,0 +1,23 @@ +-- recreate table to drop UNIQUE(event_id) constraint and add a new unique constraint on (event_id, redacts) +CREATE TABLE redactions2 ( + event_id TEXT NOT NULL, + redacts TEXT NOT NULL, + have_censored BOOL NOT NULL DEFAULT false, + received_ts BIGINT, + UNIQUE(event_id, redacts) +); + +INSERT INTO redactions2 ( + event_id, + redacts, + have_censored, + received_ts +) SELECT r.event_id, r.redacts, r.have_censored, r.received_ts FROM redactions AS r; + +DROP INDEX redactions_redacts; +DROP TABLE redactions; +ALTER TABLE redactions2 RENAME TO redactions; + +CREATE INDEX redactions_redacts ON redactions(redacts); +CREATE INDEX redactions_event_id ON redactions(event_id); -- replace the index we dropped +CREATE INDEX redactions_have_censored_ts ON redactions(received_ts) WHERE NOT have_censored; \ No newline at end of file From ce555e7cf77c3f13f3964f11ec2cd321368a2ec3 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Wed, 11 Jun 2025 17:56:15 -0700 Subject: [PATCH 02/18] tests --- tests/rest/client/test_rooms.py | 866 +++++++++++++++++++++++++++++++- 1 file changed, 865 insertions(+), 1 deletion(-) diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index 6c93ead3b89..055bca61a94 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -43,8 +43,9 @@ RoomTypes, ) from synapse.api.errors import Codes, HttpResponseException +from synapse.api.room_versions import RoomVersions from synapse.appservice import ApplicationService -from synapse.events import EventBase +from synapse.events import EventBase, make_event_from_dict from synapse.events.snapshot import EventContext from synapse.rest import admin from synapse.rest.client import ( @@ -4401,3 +4402,866 @@ def test_sending_event_and_leaving_does_not_record_participation( self.store.get_room_participation(self.user2, self.room1) ) self.assertFalse(participant) + + +class MSC4293RedactOnBanKickTestCase(unittest.FederatingHomeserverTestCase): + servlets = [ + synapse.rest.admin.register_servlets_for_client_rest_resource, + room.register_servlets, + login.register_servlets, + admin.register_servlets, + ] + + def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: + super().prepare(reactor, clock, hs) + self.creator = self.register_user("creator", "test") + self.creator_tok = self.login("creator", "test") + + self.bad_user_id = self.register_user("bad", "test") + self.bad_tok = self.login("bad", "test") + + self.room_id = self.helper.create_room_as(self.creator, tok=self.creator_tok) + + self.store = hs.get_datastores().main + self._storage_controllers = hs.get_storage_controllers() + + self.federation_event_handler = self.hs.get_federation_event_handler() + + self.hs.config.experimental.msc4239_enabled = True + + def _check_redactions( + self, + original_events: List[EventBase], + pulled_events: List[JsonDict], + expect_redaction: bool, + reason: Optional[str] = None, + ) -> None: + if expect_redaction: + redacted_count = 0 + for pulled_event in pulled_events: + for old_event in original_events: + if pulled_event["event_id"] == old_event.event_id: + redacted_because = pulled_event.get("redacted_because") + if redacted_because: + content = redacted_because.get("content") + if content: + self.assertEqual(content["reason"], reason) + self.assertEqual( + content["org.matrix.msc4293.redact_events"], True + ) + redacted_count += 1 + self.assertEqual(len(original_events), redacted_count) + else: + unredacted_events = 0 + for pulled_event in pulled_events: + for old_event in original_events: + if pulled_event["event_id"] == old_event.event_id: + redacted_because = pulled_event.get("redacted_because") + if redacted_because: + self.fail("Event should not have been redacted") + self.assertEqual(old_event.content, pulled_event["content"]) + unredacted_events += 1 + self.assertEqual(unredacted_events, len(original_events)) + + def test_banning_local_member_with_flag_redacts_their_events(self) -> None: + self.helper.join(self.room_id, self.bad_user_id, tok=self.bad_tok) + + # bad user send some messages + originals = [] + for i in range(5): + event = {"body": f"bothersome noise {i}", "msgtype": "m.text"} + res = self.helper.send_event( + self.room_id, "m.room.message", event, tok=self.bad_tok, expect_code=200 + ) + originals.append(res["event_id"]) + + # grab original events for comparison + original_events = [self.get_success(self.store.get_event(x)) for x in originals] + + # creator bans user with redaction flag set + content = { + "reason": "flooding", + "org.matrix.msc4293.redact_events": True, + } + self.helper.change_membership( + self.room_id, + self.creator, + self.bad_user_id, + "ban", + content, + self.creator_tok, + ) + + filter = json.dumps({"types": [EventTypes.Message]}) + channel = self.make_request( + "GET", + f"rooms/{self.room_id}/messages?filter={filter}&limit=50", + access_token=self.creator_tok, + ) + self.assertEqual(channel.code, 200) + self._check_redactions( + original_events, + channel.json_body["chunk"], + expect_redaction=True, + reason="flooding", + ) + + def test_banning_remote_member_with_flag_redacts_their_events(self) -> None: + bad_user = "@remote_bad_user:" + self.OTHER_SERVER_NAME + channel = self.make_signed_federation_request( + "GET", + f"/_matrix/federation/v1/make_join/{self.room_id}/{bad_user}?ver=10", + ) + self.assertEqual(channel.code, HTTPStatus.OK, channel.json_body) + join_result = channel.json_body + + join_event_dict = join_result["event"] + self.add_hashes_and_signatures_from_other_server( + join_event_dict, + RoomVersions.V10, + ) + channel = self.make_signed_federation_request( + "PUT", + f"/_matrix/federation/v2/send_join/{self.room_id}/x", + content=join_event_dict, + ) + self.assertEqual(channel.code, HTTPStatus.OK, channel.json_body) + + # the room should show that the bad user is a member + r = self.get_success( + self._storage_controllers.state.get_current_state(self.room_id) + ) + self.assertEqual(r[("m.room.member", bad_user)].membership, "join") + + auth_ids = [ + r[("m.room.create", "")].event_id, + r[("m.room.power_levels", "")].event_id, + r[("m.room.member", "@remote_bad_user:other.example.com")].event_id, + ] + original_messages = [] + for i in range(5): + remote_message = make_event_from_dict( + self.add_hashes_and_signatures_from_other_server( + { + "room_id": self.room_id, + "sender": bad_user, + "depth": 1000, + "origin_server_ts": 1, + "type": "m.room.message", + "content": {"body": f"remote bummer{i}"}, + "auth_events": auth_ids, + "prev_events": auth_ids, + } + ), + room_version=RoomVersions.V10, + ) + + self.get_success( + self.federation_event_handler.on_receive_pdu( + self.OTHER_SERVER_NAME, remote_message + ) + ) + original_messages.append(remote_message) + + # creator bans bad user with redaction flag set + content = { + "reason": "bummer messages", + "org.matrix.msc4293.redact_events": True, + } + res = self.helper.change_membership( + self.room_id, self.creator, bad_user, "ban", content, self.creator_tok + ) + ban_event_id = res["event_id"] + + filter = json.dumps({"types": [EventTypes.Message]}) + channel = self.make_request( + "GET", + f"rooms/{self.room_id}/messages?filter={filter}&limit=50", + access_token=self.creator_tok, + ) + self.assertEqual(channel.code, 200) + self._check_redactions( + original_messages, + channel.json_body["chunk"], + expect_redaction=True, + reason="bummer messages", + ) + + # any future messages that are soft-failed are also redacted - send messages referencing + # dag before ban, they should be soft-failed but also redacted + new_original_messages = [] + for i in range(5): + remote_message = make_event_from_dict( + self.add_hashes_and_signatures_from_other_server( + { + "room_id": self.room_id, + "sender": bad_user, + "depth": 1000, + "origin_server_ts": 1, + "type": "m.room.message", + "content": {"body": f"soft-fail remote bummer{i}"}, + "auth_events": auth_ids, + "prev_events": auth_ids, + } + ), + room_version=RoomVersions.V10, + ) + + self.get_success( + self.federation_event_handler.on_receive_pdu( + self.OTHER_SERVER_NAME, remote_message + ) + ) + new_original_messages.append(remote_message) + + # pull them from the db to check because they should be soft-failed and thus not available over + # cs-api + for message in new_original_messages: + original = self.get_success(self.store.get_event(message.event_id)) + if not original: + self.fail("Expected to find remote message in DB") + self.assertEqual(original.unsigned["redacted_by"], ban_event_id) + + def test_unbanning_remote_user_stops_redaction_action(self) -> None: + bad_user = "@remote_bad_user:" + self.OTHER_SERVER_NAME + channel = self.make_signed_federation_request( + "GET", + f"/_matrix/federation/v1/make_join/{self.room_id}/{bad_user}?ver=10", + ) + self.assertEqual(channel.code, HTTPStatus.OK, channel.json_body) + join_result = channel.json_body + + join_event_dict = join_result["event"] + self.add_hashes_and_signatures_from_other_server( + join_event_dict, + RoomVersions.V10, + ) + channel = self.make_signed_federation_request( + "PUT", + f"/_matrix/federation/v2/send_join/{self.room_id}/x", + content=join_event_dict, + ) + self.assertEqual(channel.code, HTTPStatus.OK, channel.json_body) + + # the room should show that the bad user is a member + r = self.get_success( + self._storage_controllers.state.get_current_state(self.room_id) + ) + self.assertEqual(r[("m.room.member", bad_user)].membership, "join") + + auth_ids = [ + r[("m.room.create", "")].event_id, + r[("m.room.power_levels", "")].event_id, + r[("m.room.member", "@remote_bad_user:other.example.com")].event_id, + ] + original_messages = [] + for i in range(5): + remote_message = make_event_from_dict( + self.add_hashes_and_signatures_from_other_server( + { + "room_id": self.room_id, + "sender": bad_user, + "depth": 1000, + "origin_server_ts": 1, + "type": "m.room.message", + "content": {"body": f"annoying messages {i}"}, + "auth_events": auth_ids, + "prev_events": auth_ids, + } + ), + room_version=RoomVersions.V10, + ) + + self.get_success( + self.federation_event_handler.on_receive_pdu( + self.OTHER_SERVER_NAME, remote_message + ) + ) + original_messages.append(remote_message) + + # creator bans bad user with redaction flag set + content = { + "reason": "this dude sucks", + "org.matrix.msc4293.redact_events": True, + } + self.helper.change_membership( + self.room_id, self.creator, bad_user, "ban", content, self.creator_tok + ) + + filter = json.dumps({"types": [EventTypes.Message]}) + channel = self.make_request( + "GET", + f"rooms/{self.room_id}/messages?filter={filter}&limit=50", + access_token=self.creator_tok, + ) + self.assertEqual(channel.code, 200) + self._check_redactions( + original_messages, + channel.json_body["chunk"], + True, + reason="this dude sucks", + ) + + # unban user + self.helper.change_membership( + self.room_id, self.creator, bad_user, "unban", content, self.creator_tok + ) + + # user should be able to join again + channel = self.make_signed_federation_request( + "GET", + f"/_matrix/federation/v1/make_join/{self.room_id}/{bad_user}?ver=10", + ) + self.assertEqual(channel.code, HTTPStatus.OK, channel.json_body) + join_result = channel.json_body + + join_event_dict = join_result["event"] + self.add_hashes_and_signatures_from_other_server( + join_event_dict, + RoomVersions.V10, + ) + channel = self.make_signed_federation_request( + "PUT", + f"/_matrix/federation/v2/send_join/{self.room_id}/x", + content=join_event_dict, + ) + self.assertEqual(channel.code, HTTPStatus.OK, channel.json_body) + + # the room should show that the bad user is a member again + new_state = self.get_success( + self._storage_controllers.state.get_current_state(self.room_id) + ) + self.assertEqual(new_state[("m.room.member", bad_user)].membership, "join") + + new_state = self.get_success( + self._storage_controllers.state.get_current_state(self.room_id) + ) + auth_ids = [ + new_state[("m.room.create", "")].event_id, + new_state[("m.room.power_levels", "")].event_id, + new_state[("m.room.member", "@remote_bad_user:other.example.com")].event_id, + ] + + # messages after unban and join proceed unredacted + new_original_messages = [] + for i in range(5): + remote_message = make_event_from_dict( + self.add_hashes_and_signatures_from_other_server( + { + "room_id": self.room_id, + "sender": bad_user, + "depth": 1000, + "origin_server_ts": 1, + "type": "m.room.message", + "content": {"body": f"no longer a bummer {i}"}, + "auth_events": auth_ids, + "prev_events": auth_ids, + } + ), + room_version=RoomVersions.V10, + ) + + self.get_success( + self.federation_event_handler.on_receive_pdu( + self.OTHER_SERVER_NAME, remote_message + ) + ) + new_original_messages.append(remote_message) + + filter = json.dumps({"types": [EventTypes.Message]}) + channel = self.make_request( + "GET", + f"rooms/{self.room_id}/messages?filter={filter}&limit=50", + access_token=self.creator_tok, + ) + self.assertEqual(channel.code, 200) + self._check_redactions(new_original_messages, channel.json_body["chunk"], False) + + def test_redaction_flag_ignored_for_user_if_banner_lacks_redaction_power( + self, + ) -> None: + # change power levels so creator can ban but not redact + self.helper.send_state( + self.room_id, + "m.room.power_levels", + {"events_default": 0, "redact": 100, "users": {self.creator: 75}}, + tok=self.creator_tok, + ) + self.helper.join(self.room_id, self.bad_user_id, tok=self.bad_tok) + + # bad user send some messages + original_ids = [] + for i in range(15): + event = {"body": f"being a menace {i}", "msgtype": "m.text"} + res = self.helper.send_event( + self.room_id, "m.room.message", event, tok=self.bad_tok, expect_code=200 + ) + original_ids.append(res["event_id"]) + + # grab original events before ban + originals = [self.get_success(self.store.get_event(x)) for x in original_ids] + + # creator bans bad user with redaction flag + content = { + "reason": "flooding", + "org.matrix.msc4293.redact_events": True, + } + self.helper.change_membership( + self.room_id, + self.creator, + self.bad_user_id, + "ban", + content, + self.creator_tok, + ) + + filter = json.dumps({"types": [EventTypes.Message]}) + channel = self.make_request( + "GET", + f"rooms/{self.room_id}/messages?filter={filter}&limit=50", + access_token=self.creator_tok, + ) + self.assertEqual(channel.code, 200) + # messages are not redacted + self._check_redactions(originals, channel.json_body["chunk"], False) + + def test_kicking_local_member_with_flag_redacts_their_events(self) -> None: + self.helper.join(self.room_id, self.bad_user_id, tok=self.bad_tok) + + # bad user send some messages + originals = [] + for i in range(5): + event = {"body": f"bothersome noise {i}", "msgtype": "m.text"} + res = self.helper.send_event( + self.room_id, "m.room.message", event, tok=self.bad_tok, expect_code=200 + ) + originals.append(res["event_id"]) + + # grab original events for comparison + original_events = [self.get_success(self.store.get_event(x)) for x in originals] + + # creator kicks user with redaction flag set + content = { + "reason": "flooding", + "org.matrix.msc4293.redact_events": True, + } + self.helper.change_membership( + self.room_id, + self.creator, + self.bad_user_id, + "kick", + content, + self.creator_tok, + ) + + filter = json.dumps({"types": [EventTypes.Message]}) + channel = self.make_request( + "GET", + f"rooms/{self.room_id}/messages?filter={filter}&limit=50", + access_token=self.creator_tok, + ) + self.assertEqual(channel.code, 200) + self._check_redactions( + original_events, + channel.json_body["chunk"], + expect_redaction=True, + reason="flooding", + ) + + def test_kicking_remote_member_with_flag_redacts_their_events(self) -> None: + bad_user = "@remote_bad_user:" + self.OTHER_SERVER_NAME + channel = self.make_signed_federation_request( + "GET", + f"/_matrix/federation/v1/make_join/{self.room_id}/{bad_user}?ver=10", + ) + self.assertEqual(channel.code, HTTPStatus.OK, channel.json_body) + join_result = channel.json_body + + join_event_dict = join_result["event"] + self.add_hashes_and_signatures_from_other_server( + join_event_dict, + RoomVersions.V10, + ) + channel = self.make_signed_federation_request( + "PUT", + f"/_matrix/federation/v2/send_join/{self.room_id}/x", + content=join_event_dict, + ) + self.assertEqual(channel.code, HTTPStatus.OK, channel.json_body) + + # the room should show that the bad user is a member + r = self.get_success( + self._storage_controllers.state.get_current_state(self.room_id) + ) + self.assertEqual(r[("m.room.member", bad_user)].membership, "join") + + auth_ids = [ + r[("m.room.create", "")].event_id, + r[("m.room.power_levels", "")].event_id, + r[("m.room.member", "@remote_bad_user:other.example.com")].event_id, + ] + original_messages = [] + for i in range(5): + remote_message = make_event_from_dict( + self.add_hashes_and_signatures_from_other_server( + { + "room_id": self.room_id, + "sender": bad_user, + "depth": 1000, + "origin_server_ts": 1, + "type": "m.room.message", + "content": {"body": f"remote bummer{i}"}, + "auth_events": auth_ids, + "prev_events": auth_ids, + } + ), + room_version=RoomVersions.V10, + ) + + self.get_success( + self.federation_event_handler.on_receive_pdu( + self.OTHER_SERVER_NAME, remote_message + ) + ) + original_messages.append(remote_message) + + # creator kicks bad user with redaction flag set + content = { + "reason": "bummer messages", + "org.matrix.msc4293.redact_events": True, + } + res = self.helper.change_membership( + self.room_id, self.creator, bad_user, "kick", content, self.creator_tok + ) + ban_event_id = res["event_id"] + + filter = json.dumps({"types": [EventTypes.Message]}) + channel = self.make_request( + "GET", + f"rooms/{self.room_id}/messages?filter={filter}&limit=50", + access_token=self.creator_tok, + ) + self.assertEqual(channel.code, 200) + self._check_redactions( + original_messages, + channel.json_body["chunk"], + expect_redaction=True, + reason="bummer messages", + ) + + # any future messages that are soft-failed are also redacted - send messages referencing + # dag before ban, they should be soft-failed but also redacted + new_original_messages = [] + for i in range(5): + remote_message = make_event_from_dict( + self.add_hashes_and_signatures_from_other_server( + { + "room_id": self.room_id, + "sender": bad_user, + "depth": 1000, + "origin_server_ts": 1, + "type": "m.room.message", + "content": {"body": f"soft-fail remote bummer{i}"}, + "auth_events": auth_ids, + "prev_events": auth_ids, + } + ), + room_version=RoomVersions.V10, + ) + + self.get_success( + self.federation_event_handler.on_receive_pdu( + self.OTHER_SERVER_NAME, remote_message + ) + ) + new_original_messages.append(remote_message) + + # pull them from the db to check because they should be soft-failed and thus not available over + # cs-api + for message in new_original_messages: + original = self.get_success(self.store.get_event(message.event_id)) + if not original: + self.fail("Expected to find remote message in DB") + self.assertEqual(original.unsigned["redacted_by"], ban_event_id) + + def test_rejoining_kicked_remote_user_stops_redaction_action(self) -> None: + bad_user = "@remote_bad_user:" + self.OTHER_SERVER_NAME + channel = self.make_signed_federation_request( + "GET", + f"/_matrix/federation/v1/make_join/{self.room_id}/{bad_user}?ver=10", + ) + self.assertEqual(channel.code, HTTPStatus.OK, channel.json_body) + join_result = channel.json_body + + join_event_dict = join_result["event"] + self.add_hashes_and_signatures_from_other_server( + join_event_dict, + RoomVersions.V10, + ) + channel = self.make_signed_federation_request( + "PUT", + f"/_matrix/federation/v2/send_join/{self.room_id}/x", + content=join_event_dict, + ) + self.assertEqual(channel.code, HTTPStatus.OK, channel.json_body) + + # the room should show that the bad user is a member + r = self.get_success( + self._storage_controllers.state.get_current_state(self.room_id) + ) + self.assertEqual(r[("m.room.member", bad_user)].membership, "join") + + auth_ids = [ + r[("m.room.create", "")].event_id, + r[("m.room.power_levels", "")].event_id, + r[("m.room.member", "@remote_bad_user:other.example.com")].event_id, + ] + original_messages = [] + for i in range(5): + remote_message = make_event_from_dict( + self.add_hashes_and_signatures_from_other_server( + { + "room_id": self.room_id, + "sender": bad_user, + "depth": 1000, + "origin_server_ts": 1, + "type": "m.room.message", + "content": {"body": f"annoying messages {i}"}, + "auth_events": auth_ids, + "prev_events": auth_ids, + } + ), + room_version=RoomVersions.V10, + ) + + self.get_success( + self.federation_event_handler.on_receive_pdu( + self.OTHER_SERVER_NAME, remote_message + ) + ) + original_messages.append(remote_message) + + # creator kicks bad user with redaction flag set + content = { + "reason": "this dude sucks", + "org.matrix.msc4293.redact_events": True, + } + self.helper.change_membership( + self.room_id, self.creator, bad_user, "kick", content, self.creator_tok + ) + + filter = json.dumps({"types": [EventTypes.Message]}) + channel = self.make_request( + "GET", + f"rooms/{self.room_id}/messages?filter={filter}&limit=50", + access_token=self.creator_tok, + ) + self.assertEqual(channel.code, 200) + self._check_redactions( + original_messages, + channel.json_body["chunk"], + True, + reason="this dude sucks", + ) + + # user re-joins after kick + channel = self.make_signed_federation_request( + "GET", + f"/_matrix/federation/v1/make_join/{self.room_id}/{bad_user}?ver=10", + ) + self.assertEqual(channel.code, HTTPStatus.OK, channel.json_body) + join_result = channel.json_body + + join_event_dict = join_result["event"] + self.add_hashes_and_signatures_from_other_server( + join_event_dict, + RoomVersions.V10, + ) + channel = self.make_signed_federation_request( + "PUT", + f"/_matrix/federation/v2/send_join/{self.room_id}/x", + content=join_event_dict, + ) + self.assertEqual(channel.code, HTTPStatus.OK, channel.json_body) + + # the room should show that the bad user is a member again + new_state = self.get_success( + self._storage_controllers.state.get_current_state(self.room_id) + ) + self.assertEqual(new_state[("m.room.member", bad_user)].membership, "join") + + new_state = self.get_success( + self._storage_controllers.state.get_current_state(self.room_id) + ) + auth_ids = [ + new_state[("m.room.create", "")].event_id, + new_state[("m.room.power_levels", "")].event_id, + new_state[("m.room.member", "@remote_bad_user:other.example.com")].event_id, + ] + + # messages after kick and re-join proceed unredacted + new_original_messages = [] + for i in range(5): + remote_message = make_event_from_dict( + self.add_hashes_and_signatures_from_other_server( + { + "room_id": self.room_id, + "sender": bad_user, + "depth": 1000, + "origin_server_ts": 1, + "type": "m.room.message", + "content": {"body": f"no longer a bummer {i}"}, + "auth_events": auth_ids, + "prev_events": auth_ids, + } + ), + room_version=RoomVersions.V10, + ) + + self.get_success( + self.federation_event_handler.on_receive_pdu( + self.OTHER_SERVER_NAME, remote_message + ) + ) + new_original_messages.append(remote_message) + + filter = json.dumps({"types": [EventTypes.Message]}) + channel = self.make_request( + "GET", + f"rooms/{self.room_id}/messages?filter={filter}&limit=50", + access_token=self.creator_tok, + ) + self.assertEqual(channel.code, 200) + self._check_redactions(new_original_messages, channel.json_body["chunk"], False) + + def test_redaction_flag_ignored_for_user_if_kicker_lacks_redaction_power( + self, + ) -> None: + # change power levels so creator can kick but not redact + self.helper.send_state( + self.room_id, + "m.room.power_levels", + {"events_default": 0, "redact": 100, "users": {self.creator: 75}}, + tok=self.creator_tok, + ) + self.helper.join(self.room_id, self.bad_user_id, tok=self.bad_tok) + + # bad user send some messages + original_ids = [] + for i in range(15): + event = {"body": f"being a menace {i}", "msgtype": "m.text"} + res = self.helper.send_event( + self.room_id, "m.room.message", event, tok=self.bad_tok, expect_code=200 + ) + original_ids.append(res["event_id"]) + + # grab original events before ban + originals = [self.get_success(self.store.get_event(x)) for x in original_ids] + + # creator kicks bad user with redaction flag + content = { + "reason": "flooding", + "org.matrix.msc4293.redact_events": True, + } + self.helper.change_membership( + self.room_id, + self.creator, + self.bad_user_id, + "kick", + content, + self.creator_tok, + ) + + filter = json.dumps({"types": [EventTypes.Message]}) + channel = self.make_request( + "GET", + f"rooms/{self.room_id}/messages?filter={filter}&limit=50", + access_token=self.creator_tok, + ) + self.assertEqual(channel.code, 200) + # messages are not redacted + self._check_redactions(originals, channel.json_body["chunk"], False) + + def test_MSC4293_flag_ignored_in_other_membership_events(self) -> None: + self.helper.join(self.room_id, self.bad_user_id, tok=self.bad_tok) + + # bad user send some messages + original_ids = [] + for i in range(15): + event = {"body": f"being a menace {i}", "msgtype": "m.text"} + res = self.helper.send_event( + self.room_id, "m.room.message", event, tok=self.bad_tok, expect_code=200 + ) + original_ids.append(res["event_id"]) + + # grab original events before ban + originals = [self.get_success(self.store.get_event(x)) for x in original_ids] + + # bad user leaves on their own with flag + content = { + "org.matrix.msc4293.redact_events": True, + } + self.helper.change_membership( + self.room_id, + self.bad_user_id, + self.bad_user_id, + "leave", + content, + self.bad_tok, + ) + + # their messages are not redacted + filter = json.dumps({"types": [EventTypes.Message]}) + channel = self.make_request( + "GET", + f"rooms/{self.room_id}/messages?filter={filter}&limit=50", + access_token=self.creator_tok, + ) + self.assertEqual(channel.code, 200) + self._check_redactions(originals, channel.json_body["chunk"], False) + + # bad user is invited with flag in invite event + content = { + "org.matrix.msc4293.redact_events": True, + } + self.helper.change_membership( + self.room_id, + self.creator, + self.bad_user_id, + "invite", + content, + self.creator_tok, + ) + + # their messages are still not redacted + filter = json.dumps({"types": [EventTypes.Message]}) + channel = self.make_request( + "GET", + f"rooms/{self.room_id}/messages?filter={filter}&limit=50", + access_token=self.creator_tok, + ) + self.assertEqual(channel.code, 200) + self._check_redactions(originals, channel.json_body["chunk"], False) + + # bad user joins with flag in invite event + content = { + "org.matrix.msc4293.redact_events": True, + } + self.helper.change_membership( + self.room_id, + self.bad_user_id, + self.bad_user_id, + "join", + content, + self.bad_tok, + ) + + # and still their messages are not redacted + filter = json.dumps({"types": [EventTypes.Message]}) + channel = self.make_request( + "GET", + f"rooms/{self.room_id}/messages?filter={filter}&limit=50", + access_token=self.creator_tok, + ) + self.assertEqual(channel.code, 200) + self._check_redactions(originals, channel.json_body["chunk"], False) From 8c606cd39732a4500510039594a3a05b56f20a55 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Thu, 12 Jun 2025 14:10:33 -0700 Subject: [PATCH 03/18] fix msc number typo --- synapse/config/experimental.py | 2 +- synapse/handlers/federation_event.py | 2 +- synapse/storage/databases/main/events.py | 2 +- tests/rest/client/test_rooms.py | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index f09f6f2e7ec..28f3d612170 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -571,4 +571,4 @@ def read_config( self.msc4155_enabled: bool = experimental.get("msc4155_enabled", False) # MSC4293: Redact on Kick/Ban - self.msc4239_enabled: bool = experimental.get("msc4239_enabled", False) + self.msc4293_enabled: bool = experimental.get("msc4293_enabled", False) diff --git a/synapse/handlers/federation_event.py b/synapse/handlers/federation_event.py index 8b6422ac031..ef1057d8cd0 100644 --- a/synapse/handlers/federation_event.py +++ b/synapse/handlers/federation_event.py @@ -2068,7 +2068,7 @@ async def _check_for_soft_fail( soft_failed_event_counter.inc() event.internal_metadata.soft_failed = True - if self._config.experimental.msc4239_enabled: + if self._config.experimental.msc4293_enabled: # Use already calculated auth events to determine if the event should be redacted due to kick/ban if event.type == EventTypes.Message: for auth_event in current_auth_events: diff --git a/synapse/storage/databases/main/events.py b/synapse/storage/databases/main/events.py index 9b509481819..974e44f078f 100644 --- a/synapse/storage/databases/main/events.py +++ b/synapse/storage/databases/main/events.py @@ -376,7 +376,7 @@ async def _persist_events_and_state_updates( event_counter.labels(event.type, origin_type, origin_entity).inc() - if self.hs.config.experimental.msc4239_enabled: + if self.hs.config.experimental.msc4293_enabled: if event.type == EventTypes.Member and event.content.get( "org.matrix.msc4293.redact_events", False ): diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index 055bca61a94..ca73ec593d9 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -4427,7 +4427,7 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: self.federation_event_handler = self.hs.get_federation_event_handler() - self.hs.config.experimental.msc4239_enabled = True + self.hs.config.experimental.msc4293_enabled = True def _check_redactions( self, From e8d328f3bbda7534617f9f4be4d03ad9a573f607 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Thu, 12 Jun 2025 14:10:57 -0700 Subject: [PATCH 04/18] add indexes in background --- synapse/storage/databases/main/room.py | 22 ++++++ .../delta/92/06_redactions_multitarget.py | 74 +++++++++++++++++++ .../06_redactions_multitarget.sql.postgres | 7 -- .../92/06_redactions_multitarget.sql.sqlite | 23 ------ 4 files changed, 96 insertions(+), 30 deletions(-) create mode 100644 synapse/storage/schema/main/delta/92/06_redactions_multitarget.py delete mode 100644 synapse/storage/schema/main/delta/92/06_redactions_multitarget.sql.postgres delete mode 100644 synapse/storage/schema/main/delta/92/06_redactions_multitarget.sql.sqlite diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index 46491cc0fe0..0e01ff4011b 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -1960,6 +1960,28 @@ def __init__( ): super().__init__(database, db_conn, hs) + self.db_pool.updates.register_background_index_update( + "redactions_add_event_id_idx", + "redactions_event_id", + "redactions", + ["event_id"], + ) + + self.db_pool.updates.register_background_index_update( + "redactions_add_redacts_idx", + "redactions_redacts", + "redactions", + ["redacts"], + ) + + self.db_pool.updates.register_background_index_update( + "redactions_add_have_censored_ts", + "redactions_have_censored_ts", + "redactions", + ["received_ts"], + where_clause="NOT have_censored", + ) + self.db_pool.updates.register_background_update_handler( "insert_room_retention", self._background_insert_retention, diff --git a/synapse/storage/schema/main/delta/92/06_redactions_multitarget.py b/synapse/storage/schema/main/delta/92/06_redactions_multitarget.py new file mode 100644 index 00000000000..e4264088cf3 --- /dev/null +++ b/synapse/storage/schema/main/delta/92/06_redactions_multitarget.py @@ -0,0 +1,74 @@ +from synapse.config.homeserver import HomeServerConfig +from synapse.storage.database import LoggingTransaction +from synapse.storage.engines import PostgresEngine, BaseDatabaseEngine + + +def run_create( + cur: LoggingTransaction, + database_engine: BaseDatabaseEngine, +) -> None: + """ + Drop unique constraint event_id and add unique constraint (event_id, redacts) + """ + print("running upgrade") + if isinstance(database_engine, PostgresEngine): + add_constraint_sql = """ + ALTER TABLE ONLY redactions ADD CONSTRAINT redactions_event_id_redacts_key UNIQUE (event_id, redacts); + """ + cur.execute(add_constraint_sql) + + drop_constraint_sql = """ + ALTER TABLE ONLY redactions DROP CONSTRAINT redactions_event_id_key; + """ + cur.execute(drop_constraint_sql) + + else: + # in SQLite we need to rewrite the table to change the constraint. + # First drop any temporary table that might be here from a previous failed migration. + cur.execute("DROP TABLE IF EXISTS temp_redactions") + + create_sql = """ + CREATE TABLE temp_redactions ( + event_id TEXT NOT NULL, + redacts TEXT NOT NULL, + have_censored BOOL NOT NULL DEFAULT false, + received_ts BIGINT, + UNIQUE(event_id, redacts) + ); + """ + cur.execute(create_sql) + + copy_sql = """ + INSERT INTO temp_redactions ( + event_id, + redacts, + have_censored, + received_ts + ) SELECT r.event_id, r.redacts, r.have_censored, r.received_ts FROM redactions AS r; + """ + cur.execute(copy_sql) + + drop_sql = """ + DROP TABLE redactions + """ + cur.execute(drop_sql) + + rename_sql = """ + ALTER TABLE temp_redactions RENAME to redactions + """ + cur.execute(rename_sql) + + sqlite3_idx_update_sql = """ + INSERT INTO background_updates (ordering, update_name, progress_json) \ + VALUES (?, ?, ?); + """ + cur.execute(sqlite3_idx_update_sql, (9206, "redactions_add_redacts_idx", '{}')) + cur.execute(sqlite3_idx_update_sql, (9206, "redactions_add_have_censored_ts", '{}')) + + # in either case the event_id index needs to be re-created + idx_sql = """ + INSERT INTO background_updates (ordering, update_name, progress_json) VALUES (?, ?, ?); + """ + cur.execute(idx_sql, (9206, "redactions_add_event_id_idx", '{}')) + + diff --git a/synapse/storage/schema/main/delta/92/06_redactions_multitarget.sql.postgres b/synapse/storage/schema/main/delta/92/06_redactions_multitarget.sql.postgres deleted file mode 100644 index 3989c9befe6..00000000000 --- a/synapse/storage/schema/main/delta/92/06_redactions_multitarget.sql.postgres +++ /dev/null @@ -1,7 +0,0 @@ -ALTER TABLE ONLY redactions ADD CONSTRAINT redactions_event_id_redacts_key UNIQUE (event_id, redacts); - --- Replace the index we're about to drop -CREATE INDEX redactions_event_id ON redactions USING btree (event_id); - --- Drop the old constraint -ALTER TABLE ONLY redactions DROP CONSTRAINT redactions_event_id_key; \ No newline at end of file diff --git a/synapse/storage/schema/main/delta/92/06_redactions_multitarget.sql.sqlite b/synapse/storage/schema/main/delta/92/06_redactions_multitarget.sql.sqlite deleted file mode 100644 index d0aa5c32f74..00000000000 --- a/synapse/storage/schema/main/delta/92/06_redactions_multitarget.sql.sqlite +++ /dev/null @@ -1,23 +0,0 @@ --- recreate table to drop UNIQUE(event_id) constraint and add a new unique constraint on (event_id, redacts) -CREATE TABLE redactions2 ( - event_id TEXT NOT NULL, - redacts TEXT NOT NULL, - have_censored BOOL NOT NULL DEFAULT false, - received_ts BIGINT, - UNIQUE(event_id, redacts) -); - -INSERT INTO redactions2 ( - event_id, - redacts, - have_censored, - received_ts -) SELECT r.event_id, r.redacts, r.have_censored, r.received_ts FROM redactions AS r; - -DROP INDEX redactions_redacts; -DROP TABLE redactions; -ALTER TABLE redactions2 RENAME TO redactions; - -CREATE INDEX redactions_redacts ON redactions(redacts); -CREATE INDEX redactions_event_id ON redactions(event_id); -- replace the index we dropped -CREATE INDEX redactions_have_censored_ts ON redactions(received_ts) WHERE NOT have_censored; \ No newline at end of file From b6aa6c0ccfd89963fa8866c681e90abd66acacd7 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Thu, 12 Jun 2025 14:34:25 -0700 Subject: [PATCH 05/18] newsfragment + run indexes in background --- changelog.d/18540.feature | 1 + .../main/delta/92/06_redactions_multitarget.py | 13 ++++++------- 2 files changed, 7 insertions(+), 7 deletions(-) create mode 100644 changelog.d/18540.feature diff --git a/changelog.d/18540.feature b/changelog.d/18540.feature new file mode 100644 index 00000000000..9a46af380ac --- /dev/null +++ b/changelog.d/18540.feature @@ -0,0 +1 @@ +Add support for [MSC4293](https://github.com/matrix-org/matrix-spec-proposals/pull/4293) - Redact on Kick/Ban \ No newline at end of file diff --git a/synapse/storage/schema/main/delta/92/06_redactions_multitarget.py b/synapse/storage/schema/main/delta/92/06_redactions_multitarget.py index e4264088cf3..8faa8f20308 100644 --- a/synapse/storage/schema/main/delta/92/06_redactions_multitarget.py +++ b/synapse/storage/schema/main/delta/92/06_redactions_multitarget.py @@ -1,6 +1,5 @@ -from synapse.config.homeserver import HomeServerConfig from synapse.storage.database import LoggingTransaction -from synapse.storage.engines import PostgresEngine, BaseDatabaseEngine +from synapse.storage.engines import BaseDatabaseEngine, PostgresEngine def run_create( @@ -62,13 +61,13 @@ def run_create( INSERT INTO background_updates (ordering, update_name, progress_json) \ VALUES (?, ?, ?); """ - cur.execute(sqlite3_idx_update_sql, (9206, "redactions_add_redacts_idx", '{}')) - cur.execute(sqlite3_idx_update_sql, (9206, "redactions_add_have_censored_ts", '{}')) + cur.execute(sqlite3_idx_update_sql, (9206, "redactions_add_redacts_idx", "{}")) + cur.execute( + sqlite3_idx_update_sql, (9206, "redactions_add_have_censored_ts", "{}") + ) # in either case the event_id index needs to be re-created idx_sql = """ INSERT INTO background_updates (ordering, update_name, progress_json) VALUES (?, ?, ?); """ - cur.execute(idx_sql, (9206, "redactions_add_event_id_idx", '{}')) - - + cur.execute(idx_sql, (9206, "redactions_add_event_id_idx", "{}")) From e4a1014209d8ef99b44803356d4970e7fb0fd030 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Thu, 12 Jun 2025 14:36:23 -0700 Subject: [PATCH 06/18] lint --- changelog.d/18540.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/18540.feature b/changelog.d/18540.feature index 9a46af380ac..2f1910c9e38 100644 --- a/changelog.d/18540.feature +++ b/changelog.d/18540.feature @@ -1 +1 @@ -Add support for [MSC4293](https://github.com/matrix-org/matrix-spec-proposals/pull/4293) - Redact on Kick/Ban \ No newline at end of file +Add support for [MSC4293](https://github.com/matrix-org/matrix-spec-proposals/pull/4293) - Redact on Kick/Ban. \ No newline at end of file From 5ad9e66fdd9b64f794ecca92cc6c959b6b1b25b4 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Thu, 12 Jun 2025 15:25:55 -0700 Subject: [PATCH 07/18] ensure redact flag is respected when using /kick and /ban --- synapse/rest/client/room.py | 13 +++++ tests/rest/client/test_rooms.py | 90 +++++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+) diff --git a/synapse/rest/client/room.py b/synapse/rest/client/room.py index bb41575d463..84659c1b956 100644 --- a/synapse/rest/client/room.py +++ b/synapse/rest/client/room.py @@ -1087,6 +1087,7 @@ def __init__(self, hs: "HomeServer"): super().__init__(hs) self.room_member_handler = hs.get_room_member_handler() self.auth = hs.get_auth() + self.config = hs.config def register(self, http_server: HttpServer) -> None: # /rooms/$roomid/[join|invite|leave|ban|unban|kick] @@ -1146,6 +1147,18 @@ async def _do( event_content = None if "reason" in content: event_content = {"reason": content["reason"]} + if self.config.experimental.msc4293_enabled: + if "org.matrix.msc4293.redact_events" in content: + if event_content: + event_content["org.matrix.msc4293.redact_events"] = content[ + "org.matrix.msc4293.redact_events" + ] + else: + event_content = { + "org.matrix.msc4293.redact_events": content[ + "org.matrix.msc4293.redact_events" + ] + } try: await self.room_member_handler.update_membership( diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index ca73ec593d9..9fc64b90a4b 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -5265,3 +5265,93 @@ def test_MSC4293_flag_ignored_in_other_membership_events(self) -> None: ) self.assertEqual(channel.code, 200) self._check_redactions(originals, channel.json_body["chunk"], False) + + def test_MSC4293_redaction_applied_via_kick_api(self) -> None: + """ + Test that MSC4239 field passed through and applied when using /kick + """ + self.helper.join(self.room_id, self.bad_user_id, tok=self.bad_tok) + + # bad user send some messages + original_ids = [] + for i in range(15): + event = {"body": f"being a menace {i}", "msgtype": "m.text"} + res = self.helper.send_event( + self.room_id, "m.room.message", event, tok=self.bad_tok, expect_code=200 + ) + original_ids.append(res["event_id"]) + + # grab original events before kick + originals = [self.get_success(self.store.get_event(x)) for x in original_ids] + + channel = self.make_request( + "POST", + f"/_matrix/client/v3/rooms/{self.room_id}/kick", + access_token=self.creator_tok, + content={ + "reason": "being annoying", + "org.matrix.msc4293.redact_events": True, + "user_id": self.bad_user_id, + }, + shorthand=False, + ) + self.assertEqual(channel.code, 200) + + filter = json.dumps({"types": [EventTypes.Message]}) + channel = self.make_request( + "GET", + f"rooms/{self.room_id}/messages?filter={filter}&limit=50", + access_token=self.creator_tok, + ) + self.assertEqual(channel.code, 200) + self._check_redactions( + originals, + channel.json_body["chunk"], + expect_redaction=True, + reason="being annoying", + ) + + def test_MSC4293_redaction_applied_via_ban_api(self) -> None: + """ + Test that MSC4239 field passed through and applied when using /ban + """ + self.helper.join(self.room_id, self.bad_user_id, tok=self.bad_tok) + + # bad user send some messages + original_ids = [] + for i in range(15): + event = {"body": f"being a menace {i}", "msgtype": "m.text"} + res = self.helper.send_event( + self.room_id, "m.room.message", event, tok=self.bad_tok, expect_code=200 + ) + original_ids.append(res["event_id"]) + + # grab original events before ban + originals = [self.get_success(self.store.get_event(x)) for x in original_ids] + + channel = self.make_request( + "POST", + f"/_matrix/client/v3/rooms/{self.room_id}/ban", + access_token=self.creator_tok, + content={ + "reason": "being disruptive", + "org.matrix.msc4293.redact_events": True, + "user_id": self.bad_user_id, + }, + shorthand=False, + ) + self.assertEqual(channel.code, 200) + + filter = json.dumps({"types": [EventTypes.Message]}) + channel = self.make_request( + "GET", + f"rooms/{self.room_id}/messages?filter={filter}&limit=50", + access_token=self.creator_tok, + ) + self.assertEqual(channel.code, 200) + self._check_redactions( + originals, + channel.json_body["chunk"], + expect_redaction=True, + reason="being disruptive", + ) From 656b64842021594c236012f982cc231975d243e5 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Thu, 12 Jun 2025 16:14:46 -0700 Subject: [PATCH 08/18] remove debugging artifact --- .../storage/schema/main/delta/92/06_redactions_multitarget.py | 1 - 1 file changed, 1 deletion(-) diff --git a/synapse/storage/schema/main/delta/92/06_redactions_multitarget.py b/synapse/storage/schema/main/delta/92/06_redactions_multitarget.py index 8faa8f20308..e31a4f693e9 100644 --- a/synapse/storage/schema/main/delta/92/06_redactions_multitarget.py +++ b/synapse/storage/schema/main/delta/92/06_redactions_multitarget.py @@ -9,7 +9,6 @@ def run_create( """ Drop unique constraint event_id and add unique constraint (event_id, redacts) """ - print("running upgrade") if isinstance(database_engine, PostgresEngine): add_constraint_sql = """ ALTER TABLE ONLY redactions ADD CONSTRAINT redactions_event_id_redacts_key UNIQUE (event_id, redacts); From 5a53b9337f917f5007a381e475134fb53e83588d Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Mon, 16 Jun 2025 16:10:29 -0700 Subject: [PATCH 09/18] requested changes --- synapse/handlers/federation_event.py | 107 +++++++++++------ synapse/rest/client/room.py | 41 +++---- synapse/storage/databases/main/events.py | 141 +++++++++++++---------- tests/rest/client/test_rooms.py | 63 +++++++--- 4 files changed, 215 insertions(+), 137 deletions(-) diff --git a/synapse/handlers/federation_event.py b/synapse/handlers/federation_event.py index ef1057d8cd0..90479aafebc 100644 --- a/synapse/handlers/federation_event.py +++ b/synapse/handlers/federation_event.py @@ -2068,41 +2068,78 @@ async def _check_for_soft_fail( soft_failed_event_counter.inc() event.internal_metadata.soft_failed = True - if self._config.experimental.msc4293_enabled: - # Use already calculated auth events to determine if the event should be redacted due to kick/ban - if event.type == EventTypes.Message: - for auth_event in current_auth_events: - if ( - auth_event.type == EventTypes.Member - and auth_event.state_key == event.sender - ): - if auth_event.membership == Membership.BAN or ( - auth_event.membership == Membership.LEAVE - and auth_event.sender != event.sender - ): - # we have a ban or kick for this sender, check for redaction flag and apply if found - autoredact = auth_event.content.get( - "org.matrix.msc4293.redact_events", False - ) - if autoredact: - await self._store.db_pool.simple_upsert( - table="redactions", - keyvalues={ - "event_id": auth_event.event_id, - "redacts": event.event_id, - }, - values={"received_ts": self._clock.time_msec()}, - insertion_values={ - "event_id": auth_event.event_id, - "redacts": event.event_id, - "received_ts": self._clock.time_msec(), - }, - ) - await self._store.db_pool.runInteraction( - "invalidate cache", - self._store.invalidate_get_event_cache_after_txn, - event.event_id, - ) + if not self._config.experimental.msc4293_enabled: + return + # Use already calculated auth events to determine if the event should be redacted due to kick/ban + pl_event = None + for auth_e in current_auth_events: + if auth_e.type == EventTypes.PowerLevels: + pl_event = auth_e + break + # this should never happen but just in case + if pl_event is None: + return + if event.type in [EventTypes.Message, EventTypes.Encrypted, EventTypes.Member]: + for auth_event in current_auth_events: + if auth_event.get("state_key") is None: + continue + if ( + auth_event.type != EventTypes.Member + or auth_event.state_key != event.sender + ): + continue + if auth_event.membership not in [Membership.LEAVE, Membership.BAN]: + continue + # self-bans currently are not authorized so we don't check for that + # case + if ( + auth_event.membership == Membership.LEAVE + and auth_event.sender == auth_event.state_key + ): + continue + # we have a ban or kick for this sender, + # check for redaction flag and apply if found + autoredact = auth_event.content.get( + "org.matrix.msc4293.redact_events", False + ) + if not autoredact or not isinstance(autoredact, bool): + continue + + # check that the sender of the kick/ban can redact + sender_level = pl_event.content.get("users", {}).get(auth_event.sender) + if sender_level is None: + sender_level = pl_event.content.get("users_default", 0) + + redact_level = pl_event.content.get("redact", 50) + room_redaction_level = pl_event.content.get("events", {}).get( + "m.room.redaction" + ) + if room_redaction_level: + if sender_level < room_redaction_level: + continue + + if sender_level > redact_level: + # copy through the redacting event + event.unsigned["redacted_because"] = auth_event.get_dict() + await self._store.db_pool.simple_upsert( + table="redactions", + keyvalues={ + "event_id": auth_event.event_id, + "redacts": event.event_id, + }, + values={"received_ts": self._clock.time_msec()}, + insertion_values={ + "event_id": auth_event.event_id, + "redacts": event.event_id, + "received_ts": self._clock.time_msec(), + }, + ) + await self._store.db_pool.runInteraction( + "invalidate cache", + self._store.invalidate_get_event_cache_after_txn, + event.event_id, + ) + break async def _load_or_fetch_auth_events_for_event( self, destination: Optional[str], event: EventBase diff --git a/synapse/rest/client/room.py b/synapse/rest/client/room.py index 84659c1b956..2ab551e0eca 100644 --- a/synapse/rest/client/room.py +++ b/synapse/rest/client/room.py @@ -1111,12 +1111,12 @@ async def _do( }: raise AuthError(403, "Guest access not allowed") - content = parse_json_object_from_request(request, allow_empty_body=True) + request_body = parse_json_object_from_request(request, allow_empty_body=True) if membership_action == "invite" and all( - key in content for key in ("medium", "address") + key in request_body for key in ("medium", "address") ): - if not all(key in content for key in ("id_server", "id_access_token")): + if not all(key in request_body for key in ("id_server", "id_access_token")): raise SynapseError( HTTPStatus.BAD_REQUEST, "`id_server` and `id_access_token` are required when doing 3pid invite", @@ -1127,12 +1127,12 @@ async def _do( await self.room_member_handler.do_3pid_invite( room_id, requester.user, - content["medium"], - content["address"], - content["id_server"], + request_body["medium"], + request_body["address"], + request_body["id_server"], requester, txn_id, - content["id_access_token"], + request_body["id_access_token"], ) except ShadowBanError: # Pretend the request succeeded. @@ -1141,24 +1141,19 @@ async def _do( target = requester.user if membership_action in ["invite", "ban", "unban", "kick"]: - assert_params_in_dict(content, ["user_id"]) - target = UserID.from_string(content["user_id"]) + assert_params_in_dict(request_body, ["user_id"]) + target = UserID.from_string(request_body["user_id"]) event_content = None - if "reason" in content: - event_content = {"reason": content["reason"]} + if "reason" in request_body: + event_content = {"reason": request_body["reason"]} if self.config.experimental.msc4293_enabled: - if "org.matrix.msc4293.redact_events" in content: - if event_content: - event_content["org.matrix.msc4293.redact_events"] = content[ - "org.matrix.msc4293.redact_events" - ] - else: - event_content = { - "org.matrix.msc4293.redact_events": content[ - "org.matrix.msc4293.redact_events" - ] - } + if "org.matrix.msc4293.redact_events" in request_body: + if event_content is None: + event_content = {} + event_content["org.matrix.msc4293.redact_events"] = request_body[ + "org.matrix.msc4293.redact_events" + ] try: await self.room_member_handler.update_membership( @@ -1167,7 +1162,7 @@ async def _do( room_id=room_id, action=membership_action, txn_id=txn_id, - third_party_signed=content.get("third_party_signed", None), + third_party_signed=request_body.get("third_party_signed", None), content=event_content, ) except ShadowBanError: diff --git a/synapse/storage/databases/main/events.py b/synapse/storage/databases/main/events.py index 974e44f078f..5769a2e5e3b 100644 --- a/synapse/storage/databases/main/events.py +++ b/synapse/storage/databases/main/events.py @@ -376,71 +376,88 @@ async def _persist_events_and_state_updates( event_counter.labels(event.type, origin_type, origin_entity).inc() - if self.hs.config.experimental.msc4293_enabled: - if event.type == EventTypes.Member and event.content.get( - "org.matrix.msc4293.redact_events", False - ): - if event.membership != Membership.BAN and not ( - event.membership == Membership.LEAVE - and event.sender != event.state_key - ): - return - # check that sender can redact - state_filter = StateFilter.from_types( - [(EventTypes.PowerLevels, "")] + # check for msc4293 redact_events flag and apply if found + if ( + not self.hs.config.experimental.msc4293_enabled + or event.type != EventTypes.Member + ): + continue + if event.membership not in [Membership.LEAVE, Membership.BAN]: + continue + redact = event.content.get("org.matrix.msc4293.redact_events", False) + if not redact or not isinstance(redact, bool): + continue + # self-bans currently are not authorized so we don't check for that + # case + if ( + event.membership == Membership.LEAVE + and event.sender == event.state_key + ): + continue + # check that sender can redact + state_filter = StateFilter.from_types([(EventTypes.PowerLevels, "")]) + state = await self.store.get_partial_filtered_current_state_ids( + event.room_id, state_filter + ) + pl_id = state[(EventTypes.PowerLevels, "")] + pl_event = await self.store.get_event(pl_id) + if pl_event: + sender_level = pl_event.content.get("users", {}).get(event.sender) + if sender_level is None: + sender_level = pl_event.content.get("users_default", 0) + redact_level = pl_event.content.get("redact", 50) + + room_redaction_level = pl_event.content.get("events", {}).get( + "m.room.redaction" + ) + if room_redaction_level: + if sender_level < room_redaction_level: + continue + + if sender_level > redact_level: + ids_to_redact = ( + await self.store.get_events_sent_by_user_in_room( + event.state_key, + event.room_id, + limit=1000000, # arbitrarily large number + filter=[ + EventTypes.Member, + EventTypes.Message, + EventTypes.Encrypted, + ], + ) ) - state = await self.store.get_partial_filtered_current_state_ids( - event.room_id, state_filter + if not ids_to_redact: + continue + + key_values = [(event.event_id, x) for x in ids_to_redact] + value_values = [ + (self._clock.time_msec(),) for x in ids_to_redact + ] + await self.db_pool.simple_upsert_many( + table="redactions", + key_names=["event_id", "redacts"], + key_values=key_values, + value_names=["received_ts"], + value_values=value_values, + desc="redact_on_ban_redaction_txn", + ) + + redacted_events = await self.store.get_events_as_list( + ids_to_redact ) - pl_id = state[(EventTypes.PowerLevels, "")] - pl_event_map = await self.store.get_events([pl_id]) - pl_event = pl_event_map.get(pl_id) - if pl_event: - sender_level = pl_event.content.get("users", {}).get( - event.sender + for redacted_event in redacted_events: + redacted_event.unsigned["redacted_because"] = event + + # normally the cache entry for a redacted event would be invalidated + # by an arriving redaction event, but since we are not creating redaction + # events we invalidate manually + for id in ids_to_redact: + await self.db_pool.runInteraction( + "invalidate cache", + self.store.invalidate_get_event_cache_after_txn, + id, ) - if sender_level is None: - sender_level = pl_event.content.get("users_default", 0) - redact_level = pl_event.content.get("redact", 50) - - if sender_level > redact_level: - ids_to_redact = ( - await self.store.get_events_sent_by_user_in_room( - event.state_key, - event.room_id, - limit=1000000, # arbitrarily large number - filter=[ - "m.room.member", - "m.room.message", - "m.room.encrypted", - ], - ) - ) - if ids_to_redact: - key_values = [ - (event.event_id, x) for x in ids_to_redact - ] - value_values = [ - (self._clock.time_msec(),) - for x in ids_to_redact - ] - await self.db_pool.simple_upsert_many( - table="redactions", - key_names=["event_id", "redacts"], - key_values=key_values, - value_names=["received_ts"], - value_values=value_values, - desc="redact_on_ban_redaction_txn", - ) - # normally the cache entry for a redacted event would be invalidated - # by an arriving redaction event, but since we are not creating redaction - # events we invalidate manually - for id in ids_to_redact: - await self.db_pool.runInteraction( - "invalidate cache", - self.store.invalidate_get_event_cache_after_txn, - id, - ) if new_forward_extremities: self.store.get_latest_event_ids_in_room.prefill( diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index 9fc64b90a4b..eff06c16339 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -4436,31 +4436,57 @@ def _check_redactions( expect_redaction: bool, reason: Optional[str] = None, ) -> None: + """ + Checks a set of original events against a second set of the same events, pulled + from the /messages api. If expect_redaction is true, we expect that the second + set of events will be redacted, and the test will fail if that is not the case. + Otherwise, verifies that the events have not been redacted and fails if not. + + Args: + original_events: A list of the original events sent + pulled_events: A list of the same events as the orignal events, fetched + over the /messages api + expect_redaction: Whether or not the pulled_events should be redacted + reason: If the events are expected to be redacted, the expected reason + for the redaction + + """ if expect_redaction: redacted_count = 0 for pulled_event in pulled_events: for old_event in original_events: - if pulled_event["event_id"] == old_event.event_id: - redacted_because = pulled_event.get("redacted_because") - if redacted_because: - content = redacted_because.get("content") - if content: - self.assertEqual(content["reason"], reason) - self.assertEqual( - content["org.matrix.msc4293.redact_events"], True - ) - redacted_count += 1 + if pulled_event["event_id"] != old_event.event_id: + continue + # we have a matching event, check that it is redacted + event_content = pulled_event["content"] + if event_content: + self.fail(f"Expected event {pulled_event} to be redacted") + redacting_event = pulled_event.get("redacted_because") + if not redacting_event: + self.fail( + f"Expected event {pulled_event} to have a redacting event." + ) + # check that the redacting event records the expected reason, and the + # redact_events flag + content = redacting_event["content"] + self.assertEqual(content["reason"], reason) + self.assertEqual(content["org.matrix.msc4293.redact_events"], True) + redacted_count += 1 + # all provided events should be redacted self.assertEqual(len(original_events), redacted_count) else: unredacted_events = 0 for pulled_event in pulled_events: for old_event in original_events: - if pulled_event["event_id"] == old_event.event_id: - redacted_because = pulled_event.get("redacted_because") - if redacted_because: - self.fail("Event should not have been redacted") - self.assertEqual(old_event.content, pulled_event["content"]) - unredacted_events += 1 + if pulled_event["event_id"] != old_event.event_id: + continue + # we have a matching event, make sure it is not redacted + redacted_because = pulled_event.get("redacted_because") + if redacted_because: + self.fail("Event should not have been redacted") + self.assertEqual(old_event.content, pulled_event["content"]) + unredacted_events += 1 + # all provided events should not have been redacted self.assertEqual(unredacted_events, len(original_events)) def test_banning_local_member_with_flag_redacts_their_events(self) -> None: @@ -4620,7 +4646,10 @@ def test_banning_remote_member_with_flag_redacts_their_events(self) -> None: original = self.get_success(self.store.get_event(message.event_id)) if not original: self.fail("Expected to find remote message in DB") - self.assertEqual(original.unsigned["redacted_by"], ban_event_id) + redacted_because = original.unsigned.get("redacted_because") + if not redacted_because: + self.fail("Did not find redacted_because field") + self.assertEqual(redacted_because.event_id, ban_event_id) def test_unbanning_remote_user_stops_redaction_action(self) -> None: bad_user = "@remote_bad_user:" + self.OTHER_SERVER_NAME From 4323879d23e5dbbfae96e88c40a6f68b92e92178 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Mon, 23 Jun 2025 13:52:23 -0700 Subject: [PATCH 10/18] requested changes --- synapse/handlers/federation_event.py | 116 ++++++++++------------- synapse/storage/databases/main/events.py | 17 ++-- 2 files changed, 58 insertions(+), 75 deletions(-) diff --git a/synapse/handlers/federation_event.py b/synapse/handlers/federation_event.py index 90479aafebc..8c557eac1ca 100644 --- a/synapse/handlers/federation_event.py +++ b/synapse/handlers/federation_event.py @@ -2071,75 +2071,59 @@ async def _check_for_soft_fail( if not self._config.experimental.msc4293_enabled: return # Use already calculated auth events to determine if the event should be redacted due to kick/ban - pl_event = None - for auth_e in current_auth_events: - if auth_e.type == EventTypes.PowerLevels: - pl_event = auth_e - break - # this should never happen but just in case - if pl_event is None: - return - if event.type in [EventTypes.Message, EventTypes.Encrypted, EventTypes.Member]: - for auth_event in current_auth_events: - if auth_event.get("state_key") is None: - continue - if ( - auth_event.type != EventTypes.Member - or auth_event.state_key != event.sender - ): - continue - if auth_event.membership not in [Membership.LEAVE, Membership.BAN]: - continue - # self-bans currently are not authorized so we don't check for that - # case - if ( - auth_event.membership == Membership.LEAVE - and auth_event.sender == auth_event.state_key - ): - continue - # we have a ban or kick for this sender, - # check for redaction flag and apply if found - autoredact = auth_event.content.get( - "org.matrix.msc4293.redact_events", False - ) - if not autoredact or not isinstance(autoredact, bool): - continue + state_map = {} + for auth_event in current_auth_events: + state_map[(auth_event.type, auth_event.state_key)] = auth_event - # check that the sender of the kick/ban can redact - sender_level = pl_event.content.get("users", {}).get(auth_event.sender) - if sender_level is None: - sender_level = pl_event.content.get("users_default", 0) + for auth_event in current_auth_events: + if auth_event.get("state_key") is None: + continue + if ( + auth_event.type != EventTypes.Member + or auth_event.state_key != event.sender + ): + continue + if auth_event.membership not in [Membership.LEAVE, Membership.BAN]: + continue + # self-bans currently are not authorized so we don't check for that + # case + if ( + auth_event.membership == Membership.LEAVE + and auth_event.sender == auth_event.state_key + ): + continue + # we have a ban or kick for this sender, + # check for redaction flag and apply if found + autoredact = auth_event.content.get( + "org.matrix.msc4293.redact_events", False + ) + if not autoredact or not isinstance(autoredact, bool): + continue - redact_level = pl_event.content.get("redact", 50) - room_redaction_level = pl_event.content.get("events", {}).get( - "m.room.redaction" + # check that the sender of the kick/ban can redact + room_version_obj = KNOWN_ROOM_VERSIONS[room_version] + if event_auth.check_redaction(room_version_obj, event, state_map): + # copy through the redacting event + event.unsigned["redacted_because"] = auth_event.get_dict() + await self._store.db_pool.simple_upsert( + table="redactions", + keyvalues={ + "event_id": auth_event.event_id, + "redacts": event.event_id, + }, + values={"received_ts": self._clock.time_msec()}, + insertion_values={ + "event_id": auth_event.event_id, + "redacts": event.event_id, + "received_ts": self._clock.time_msec(), + }, ) - if room_redaction_level: - if sender_level < room_redaction_level: - continue - - if sender_level > redact_level: - # copy through the redacting event - event.unsigned["redacted_because"] = auth_event.get_dict() - await self._store.db_pool.simple_upsert( - table="redactions", - keyvalues={ - "event_id": auth_event.event_id, - "redacts": event.event_id, - }, - values={"received_ts": self._clock.time_msec()}, - insertion_values={ - "event_id": auth_event.event_id, - "redacts": event.event_id, - "received_ts": self._clock.time_msec(), - }, - ) - await self._store.db_pool.runInteraction( - "invalidate cache", - self._store.invalidate_get_event_cache_after_txn, - event.event_id, - ) - break + await self._store.db_pool.runInteraction( + "invalidate cache", + self._store.invalidate_get_event_cache_after_txn, + event.event_id, + ) + break async def _load_or_fetch_auth_events_for_event( self, destination: Optional[str], event: EventBase diff --git a/synapse/storage/databases/main/events.py b/synapse/storage/databases/main/events.py index 5769a2e5e3b..b45648ad411 100644 --- a/synapse/storage/databases/main/events.py +++ b/synapse/storage/databases/main/events.py @@ -108,6 +108,9 @@ (EventTypes.Tombstone, ""), ) +# An arbitrarily large number +MAX_EVENTS = 1000000 + @attr.s(slots=True, auto_attribs=True) class DeltaState: @@ -405,7 +408,10 @@ async def _persist_events_and_state_updates( sender_level = pl_event.content.get("users", {}).get(event.sender) if sender_level is None: sender_level = pl_event.content.get("users_default", 0) - redact_level = pl_event.content.get("redact", 50) + + redact_level = pl_event.content.get("redact") + if not redact_level: + redact_level = pl_event.content.get("events_default", 0) room_redaction_level = pl_event.content.get("events", {}).get( "m.room.redaction" @@ -417,14 +423,7 @@ async def _persist_events_and_state_updates( if sender_level > redact_level: ids_to_redact = ( await self.store.get_events_sent_by_user_in_room( - event.state_key, - event.room_id, - limit=1000000, # arbitrarily large number - filter=[ - EventTypes.Member, - EventTypes.Message, - EventTypes.Encrypted, - ], + event.state_key, event.room_id, limit=MAX_EVENTS ) ) if not ids_to_redact: From 65d7b41b1c92caac61b6db2e9bf0de5e5795c585 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Fri, 27 Jun 2025 11:21:30 -0700 Subject: [PATCH 11/18] change table architecture --- synapse/storage/databases/main/room.py | 22 ------ .../delta/92/06_redactions_multitarget.py | 72 ------------------- .../main/delta/92/08_room_ban_redactions.sql | 21 ++++++ 3 files changed, 21 insertions(+), 94 deletions(-) delete mode 100644 synapse/storage/schema/main/delta/92/06_redactions_multitarget.py create mode 100644 synapse/storage/schema/main/delta/92/08_room_ban_redactions.sql diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index 5f6610b1bb7..58451d3ff19 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -1962,28 +1962,6 @@ def __init__( ): super().__init__(database, db_conn, hs) - self.db_pool.updates.register_background_index_update( - "redactions_add_event_id_idx", - "redactions_event_id", - "redactions", - ["event_id"], - ) - - self.db_pool.updates.register_background_index_update( - "redactions_add_redacts_idx", - "redactions_redacts", - "redactions", - ["redacts"], - ) - - self.db_pool.updates.register_background_index_update( - "redactions_add_have_censored_ts", - "redactions_have_censored_ts", - "redactions", - ["received_ts"], - where_clause="NOT have_censored", - ) - self.db_pool.updates.register_background_update_handler( "insert_room_retention", self._background_insert_retention, diff --git a/synapse/storage/schema/main/delta/92/06_redactions_multitarget.py b/synapse/storage/schema/main/delta/92/06_redactions_multitarget.py deleted file mode 100644 index e31a4f693e9..00000000000 --- a/synapse/storage/schema/main/delta/92/06_redactions_multitarget.py +++ /dev/null @@ -1,72 +0,0 @@ -from synapse.storage.database import LoggingTransaction -from synapse.storage.engines import BaseDatabaseEngine, PostgresEngine - - -def run_create( - cur: LoggingTransaction, - database_engine: BaseDatabaseEngine, -) -> None: - """ - Drop unique constraint event_id and add unique constraint (event_id, redacts) - """ - if isinstance(database_engine, PostgresEngine): - add_constraint_sql = """ - ALTER TABLE ONLY redactions ADD CONSTRAINT redactions_event_id_redacts_key UNIQUE (event_id, redacts); - """ - cur.execute(add_constraint_sql) - - drop_constraint_sql = """ - ALTER TABLE ONLY redactions DROP CONSTRAINT redactions_event_id_key; - """ - cur.execute(drop_constraint_sql) - - else: - # in SQLite we need to rewrite the table to change the constraint. - # First drop any temporary table that might be here from a previous failed migration. - cur.execute("DROP TABLE IF EXISTS temp_redactions") - - create_sql = """ - CREATE TABLE temp_redactions ( - event_id TEXT NOT NULL, - redacts TEXT NOT NULL, - have_censored BOOL NOT NULL DEFAULT false, - received_ts BIGINT, - UNIQUE(event_id, redacts) - ); - """ - cur.execute(create_sql) - - copy_sql = """ - INSERT INTO temp_redactions ( - event_id, - redacts, - have_censored, - received_ts - ) SELECT r.event_id, r.redacts, r.have_censored, r.received_ts FROM redactions AS r; - """ - cur.execute(copy_sql) - - drop_sql = """ - DROP TABLE redactions - """ - cur.execute(drop_sql) - - rename_sql = """ - ALTER TABLE temp_redactions RENAME to redactions - """ - cur.execute(rename_sql) - - sqlite3_idx_update_sql = """ - INSERT INTO background_updates (ordering, update_name, progress_json) \ - VALUES (?, ?, ?); - """ - cur.execute(sqlite3_idx_update_sql, (9206, "redactions_add_redacts_idx", "{}")) - cur.execute( - sqlite3_idx_update_sql, (9206, "redactions_add_have_censored_ts", "{}") - ) - - # in either case the event_id index needs to be re-created - idx_sql = """ - INSERT INTO background_updates (ordering, update_name, progress_json) VALUES (?, ?, ?); - """ - cur.execute(idx_sql, (9206, "redactions_add_event_id_idx", "{}")) diff --git a/synapse/storage/schema/main/delta/92/08_room_ban_redactions.sql b/synapse/storage/schema/main/delta/92/08_room_ban_redactions.sql new file mode 100644 index 00000000000..4243fce8112 --- /dev/null +++ b/synapse/storage/schema/main/delta/92/08_room_ban_redactions.sql @@ -0,0 +1,21 @@ +-- +-- This file is licensed under the Affero General Public License (AGPL) version 3. +-- +-- Copyright (C) 2025 New Vector, Ltd +-- +-- This program is free software: you can redistribute it and/or modify +-- it under the terms of the GNU Affero General Public License as +-- published by the Free Software Foundation, either version 3 of the +-- License, or (at your option) any later version. +-- +-- See the GNU Affero General Public License for more details: +-- . + +CREATE TABLE room_ban_redactions( + room_id string NOT NULL, + user_id string NOT NULL, + redacting_event_id string NOT NULL, + redact_end_ordering stream_ordering bigint DEFAULT NULL, -- stream ordering after which redactions are not applied + CONSTRAINT room_ban_redaction_uniqueness UNIQUE (room_id, user_id) +); + From 8d7eddd6f98b22d07b9d3dcb148ec9a58b265153 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Fri, 27 Jun 2025 11:22:08 -0700 Subject: [PATCH 12/18] update code to reflect new table architecture --- synapse/handlers/federation_event.py | 60 --------------- synapse/storage/databases/main/events.py | 75 ++++++++++++------- .../storage/databases/main/events_worker.py | 38 +++++++++- tests/rest/client/test_rooms.py | 2 +- 4 files changed, 88 insertions(+), 87 deletions(-) diff --git a/synapse/handlers/federation_event.py b/synapse/handlers/federation_event.py index 8c557eac1ca..1e738f484f9 100644 --- a/synapse/handlers/federation_event.py +++ b/synapse/handlers/federation_event.py @@ -1966,9 +1966,6 @@ async def _check_for_soft_fail( Does nothing for events in rooms with partial state, since we may not have an accurate membership event for the sender in the current state. - Also checks if event should be redacted due to a MSC4293 redaction flag in kick/ban - event for user - Args: event context: The `EventContext` which we are about to persist the event with. @@ -2068,63 +2065,6 @@ async def _check_for_soft_fail( soft_failed_event_counter.inc() event.internal_metadata.soft_failed = True - if not self._config.experimental.msc4293_enabled: - return - # Use already calculated auth events to determine if the event should be redacted due to kick/ban - state_map = {} - for auth_event in current_auth_events: - state_map[(auth_event.type, auth_event.state_key)] = auth_event - - for auth_event in current_auth_events: - if auth_event.get("state_key") is None: - continue - if ( - auth_event.type != EventTypes.Member - or auth_event.state_key != event.sender - ): - continue - if auth_event.membership not in [Membership.LEAVE, Membership.BAN]: - continue - # self-bans currently are not authorized so we don't check for that - # case - if ( - auth_event.membership == Membership.LEAVE - and auth_event.sender == auth_event.state_key - ): - continue - # we have a ban or kick for this sender, - # check for redaction flag and apply if found - autoredact = auth_event.content.get( - "org.matrix.msc4293.redact_events", False - ) - if not autoredact or not isinstance(autoredact, bool): - continue - - # check that the sender of the kick/ban can redact - room_version_obj = KNOWN_ROOM_VERSIONS[room_version] - if event_auth.check_redaction(room_version_obj, event, state_map): - # copy through the redacting event - event.unsigned["redacted_because"] = auth_event.get_dict() - await self._store.db_pool.simple_upsert( - table="redactions", - keyvalues={ - "event_id": auth_event.event_id, - "redacts": event.event_id, - }, - values={"received_ts": self._clock.time_msec()}, - insertion_values={ - "event_id": auth_event.event_id, - "redacts": event.event_id, - "received_ts": self._clock.time_msec(), - }, - ) - await self._store.db_pool.runInteraction( - "invalidate cache", - self._store.invalidate_get_event_cache_after_txn, - event.event_id, - ) - break - async def _load_or_fetch_auth_events_for_event( self, destination: Optional[str], event: EventBase ) -> Collection[EventBase]: diff --git a/synapse/storage/databases/main/events.py b/synapse/storage/databases/main/events.py index b45648ad411..18684ebb4d7 100644 --- a/synapse/storage/databases/main/events.py +++ b/synapse/storage/databases/main/events.py @@ -379,12 +379,40 @@ async def _persist_events_and_state_updates( event_counter.labels(event.type, origin_type, origin_entity).inc() - # check for msc4293 redact_events flag and apply if found if ( not self.hs.config.experimental.msc4293_enabled or event.type != EventTypes.Member + or event.state_key is None ): continue + + # check if this is an unban/join that will undo a ban/kick redaction for + # user in room + if event.membership in [Membership.LEAVE, Membership.JOIN]: + if event.membership == Membership.LEAVE: + # self-leave, ignore + if event.sender == event.state_key: + continue + # check to see if there is an existing ban/leave causing redactions for + # this user/room combination + res = await self.db_pool.simple_select_list( + "room_ban_redactions", + {"room_id": event.room_id, "user_id": event.state_key}, + ["room_id", "user_id"], + ) + if res: + # if so, update the entry with the stream ordering when the redactions should + # stop + await self.db_pool.simple_update( + "room_ban_redactions", + {"room_id": event.room_id, "user_id": event.state_key}, + { + "redact_end_ordering": event.internal_metadata.stream_ordering + }, + desc="room_ban_redactions update redact_end_ordering", + ) + + # check for msc4293 redact_events flag and apply if found if event.membership not in [Membership.LEAVE, Membership.BAN]: continue redact = event.content.get("org.matrix.msc4293.redact_events", False) @@ -420,7 +448,25 @@ async def _persist_events_and_state_updates( if sender_level < room_redaction_level: continue - if sender_level > redact_level: + if sender_level >= redact_level: + await self.db_pool.simple_upsert( + "room_ban_redactions", + {"room_id": event.room_id, "user_id": event.state_key}, + { + "redacting_event_id": event.event_id, + "redact_end_ordering": None, + }, + { + "room_id": event.room_id, + "user_id": event.state_key, + "redacting_event_id": event.event_id, + "redact_end_ordering": None, + }, + ) + + # normally the cache entry for a redacted event would be invalidated + # by an arriving redaction event, but since we are not creating redaction + # events we invalidate manually ids_to_redact = ( await self.store.get_events_sent_by_user_in_room( event.state_key, event.room_id, limit=MAX_EVENTS @@ -429,28 +475,6 @@ async def _persist_events_and_state_updates( if not ids_to_redact: continue - key_values = [(event.event_id, x) for x in ids_to_redact] - value_values = [ - (self._clock.time_msec(),) for x in ids_to_redact - ] - await self.db_pool.simple_upsert_many( - table="redactions", - key_names=["event_id", "redacts"], - key_values=key_values, - value_names=["received_ts"], - value_values=value_values, - desc="redact_on_ban_redaction_txn", - ) - - redacted_events = await self.store.get_events_as_list( - ids_to_redact - ) - for redacted_event in redacted_events: - redacted_event.unsigned["redacted_because"] = event - - # normally the cache entry for a redacted event would be invalidated - # by an arriving redaction event, but since we are not creating redaction - # events we invalidate manually for id in ids_to_redact: await self.db_pool.runInteraction( "invalidate cache", @@ -2850,8 +2874,9 @@ def _store_redaction(self, txn: LoggingTransaction, event: EventBase) -> None: self.db_pool.simple_upsert_txn( txn, table="redactions", - keyvalues={"event_id": event.event_id, "redacts": event.redacts}, + keyvalues={"event_id": event.event_id}, values={ + "redacts": event.redacts, "received_ts": self._clock.time_msec(), }, ) diff --git a/synapse/storage/databases/main/events_worker.py b/synapse/storage/databases/main/events_worker.py index 3db4460f571..eca7421a06a 100644 --- a/synapse/storage/databases/main/events_worker.py +++ b/synapse/storage/databases/main/events_worker.py @@ -17,7 +17,7 @@ # [This file includes modifications made by New Vector Limited] # # - +import json import logging import threading import weakref @@ -1558,6 +1558,42 @@ def _fetch_event_rows( if d: d.redactions.append(redacter) + # check for MSC4932 redactions + to_check = [] + events: List[_EventRow] = [] + for e in evs: + event = event_dict.get(e) + if not event: + continue + events.append(event) + event_json = json.loads(event.json) + room_id = event_json.get("room_id") + user_id = event_json.get("sender") + to_check.append((room_id, user_id)) + + # likely that some of these events may be for the same room/user combo, in + # which case we don't need to do redundant queries + to_check_set = set(to_check) + for room_and_user in to_check_set: + room_redactions_sql = "SELECT redacting_event_id, redact_end_ordering FROM room_ban_redactions WHERE room_id = ? and user_id = ?" + txn.execute(room_redactions_sql, room_and_user) + + res = txn.fetchone() + # we have a redaction for a room, user_id combo - apply it to matching events + if not res: + continue + for e_row in events: + e_json = json.loads(e_row.json) + room_id = e_json.get("room_id") + user_id = e_json.get("sender") + if room_and_user != (room_id, user_id): + continue + redacting_event_id, redact_end_ordering = res + if redact_end_ordering: + if e_row.stream_ordering < redact_end_ordering: + e_row.redactions.append(redacting_event_id) + else: + e_row.redactions.append(redacting_event_id) return event_dict def _maybe_redact_event_row( diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index eff06c16339..912de44a3c8 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -4733,7 +4733,7 @@ def test_unbanning_remote_user_stops_redaction_action(self) -> None: # unban user self.helper.change_membership( - self.room_id, self.creator, bad_user, "unban", content, self.creator_tok + self.room_id, self.creator, bad_user, "unban", {}, self.creator_tok ) # user should be able to join again From 17ce1940811e2bc4cd713dffbee6ea3475c6d00c Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Fri, 27 Jun 2025 11:30:04 -0700 Subject: [PATCH 13/18] fix column type --- synapse/storage/schema/main/delta/92/08_room_ban_redactions.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/storage/schema/main/delta/92/08_room_ban_redactions.sql b/synapse/storage/schema/main/delta/92/08_room_ban_redactions.sql index 4243fce8112..317d4f3ad4f 100644 --- a/synapse/storage/schema/main/delta/92/08_room_ban_redactions.sql +++ b/synapse/storage/schema/main/delta/92/08_room_ban_redactions.sql @@ -15,7 +15,7 @@ CREATE TABLE room_ban_redactions( room_id string NOT NULL, user_id string NOT NULL, redacting_event_id string NOT NULL, - redact_end_ordering stream_ordering bigint DEFAULT NULL, -- stream ordering after which redactions are not applied + redact_end_ordering bigint DEFAULT NULL, -- stream ordering after which redactions are not applied CONSTRAINT room_ban_redaction_uniqueness UNIQUE (room_id, user_id) ); From af6ca26af04dbad5ad2a3813e96a64d7834fa0fb Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Fri, 27 Jun 2025 11:40:29 -0700 Subject: [PATCH 14/18] fix other column type --- .../storage/schema/main/delta/92/08_room_ban_redactions.sql | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/synapse/storage/schema/main/delta/92/08_room_ban_redactions.sql b/synapse/storage/schema/main/delta/92/08_room_ban_redactions.sql index 317d4f3ad4f..566ddcbdd7a 100644 --- a/synapse/storage/schema/main/delta/92/08_room_ban_redactions.sql +++ b/synapse/storage/schema/main/delta/92/08_room_ban_redactions.sql @@ -12,9 +12,9 @@ -- . CREATE TABLE room_ban_redactions( - room_id string NOT NULL, - user_id string NOT NULL, - redacting_event_id string NOT NULL, + room_id text NOT NULL, + user_id text NOT NULL, + redacting_event_id text NOT NULL, redact_end_ordering bigint DEFAULT NULL, -- stream ordering after which redactions are not applied CONSTRAINT room_ban_redaction_uniqueness UNIQUE (room_id, user_id) ); From bf8d9c2197b51f5427f1971835d41313021b4cfa Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Mon, 7 Jul 2025 14:46:56 -0700 Subject: [PATCH 15/18] invalidate cache by room id --- synapse/storage/databases/main/events.py | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/synapse/storage/databases/main/events.py b/synapse/storage/databases/main/events.py index 18684ebb4d7..2b9767c9efc 100644 --- a/synapse/storage/databases/main/events.py +++ b/synapse/storage/databases/main/events.py @@ -467,20 +467,9 @@ async def _persist_events_and_state_updates( # normally the cache entry for a redacted event would be invalidated # by an arriving redaction event, but since we are not creating redaction # events we invalidate manually - ids_to_redact = ( - await self.store.get_events_sent_by_user_in_room( - event.state_key, event.room_id, limit=MAX_EVENTS - ) + self.store._invalidate_local_get_event_cache_room_id( + event.room_id ) - if not ids_to_redact: - continue - - for id in ids_to_redact: - await self.db_pool.runInteraction( - "invalidate cache", - self.store.invalidate_get_event_cache_after_txn, - id, - ) if new_forward_extremities: self.store.get_latest_event_ids_in_room.prefill( From c06ab2ebb60df6d57880effd7faf219bc5c26cf2 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Mon, 14 Jul 2025 14:51:28 -0700 Subject: [PATCH 16/18] requested changes --- synapse/storage/databases/main/events.py | 131 +++++++++--------- .../storage/databases/main/events_worker.py | 14 +- 2 files changed, 76 insertions(+), 69 deletions(-) diff --git a/synapse/storage/databases/main/events.py b/synapse/storage/databases/main/events.py index 2b9767c9efc..edb59be988f 100644 --- a/synapse/storage/databases/main/events.py +++ b/synapse/storage/databases/main/events.py @@ -108,9 +108,6 @@ (EventTypes.Tombstone, ""), ) -# An arbitrarily large number -MAX_EVENTS = 1000000 - @attr.s(slots=True, auto_attribs=True) class DeltaState: @@ -387,30 +384,26 @@ async def _persist_events_and_state_updates( continue # check if this is an unban/join that will undo a ban/kick redaction for - # user in room + # a user in the room if event.membership in [Membership.LEAVE, Membership.JOIN]: - if event.membership == Membership.LEAVE: + if ( + event.membership == Membership.LEAVE + and event.sender == event.state_key + ): # self-leave, ignore - if event.sender == event.state_key: - continue - # check to see if there is an existing ban/leave causing redactions for - # this user/room combination - res = await self.db_pool.simple_select_list( + continue + + # if there is an existing ban/leave causing redactions for + # this user/room combination update the entry with the stream + # ordering when the redactions should stop + await self.db_pool.simple_update( "room_ban_redactions", {"room_id": event.room_id, "user_id": event.state_key}, - ["room_id", "user_id"], + { + "redact_end_ordering": event.internal_metadata.stream_ordering + }, + desc="room_ban_redactions update redact_end_ordering", ) - if res: - # if so, update the entry with the stream ordering when the redactions should - # stop - await self.db_pool.simple_update( - "room_ban_redactions", - {"room_id": event.room_id, "user_id": event.state_key}, - { - "redact_end_ordering": event.internal_metadata.stream_ordering - }, - desc="room_ban_redactions update redact_end_ordering", - ) # check for msc4293 redact_events flag and apply if found if event.membership not in [Membership.LEAVE, Membership.BAN]: @@ -421,61 +414,69 @@ async def _persist_events_and_state_updates( # self-bans currently are not authorized so we don't check for that # case if ( - event.membership == Membership.LEAVE + event.membership == Membership.BAN and event.sender == event.state_key ): continue + # check that sender can redact - state_filter = StateFilter.from_types([(EventTypes.PowerLevels, "")]) - state = await self.store.get_partial_filtered_current_state_ids( - event.room_id, state_filter - ) - pl_id = state[(EventTypes.PowerLevels, "")] - pl_event = await self.store.get_event(pl_id) - if pl_event: - sender_level = pl_event.content.get("users", {}).get(event.sender) - if sender_level is None: - sender_level = pl_event.content.get("users_default", 0) - - redact_level = pl_event.content.get("redact") - if not redact_level: - redact_level = pl_event.content.get("events_default", 0) - - room_redaction_level = pl_event.content.get("events", {}).get( - "m.room.redaction" + redact_allowed = await self._can_sender_redact(event) + if redact_allowed: + await self.db_pool.simple_upsert( + "room_ban_redactions", + {"room_id": event.room_id, "user_id": event.state_key}, + { + "redacting_event_id": event.event_id, + "redact_end_ordering": None, + }, + { + "room_id": event.room_id, + "user_id": event.state_key, + "redacting_event_id": event.event_id, + "redact_end_ordering": None, + }, ) - if room_redaction_level: - if sender_level < room_redaction_level: - continue - - if sender_level >= redact_level: - await self.db_pool.simple_upsert( - "room_ban_redactions", - {"room_id": event.room_id, "user_id": event.state_key}, - { - "redacting_event_id": event.event_id, - "redact_end_ordering": None, - }, - { - "room_id": event.room_id, - "user_id": event.state_key, - "redacting_event_id": event.event_id, - "redact_end_ordering": None, - }, - ) - # normally the cache entry for a redacted event would be invalidated - # by an arriving redaction event, but since we are not creating redaction - # events we invalidate manually - self.store._invalidate_local_get_event_cache_room_id( - event.room_id - ) + # normally the cache entry for a redacted event would be invalidated + # by an arriving redaction event, but since we are not creating redaction + # events we invalidate manually + self.store._invalidate_local_get_event_cache_room_id(event.room_id) + + self.store._invalidate_async_get_event_cache_room_id(event.room_id) if new_forward_extremities: self.store.get_latest_event_ids_in_room.prefill( (room_id,), frozenset(new_forward_extremities) ) + async def _can_sender_redact(self, event: EventBase) -> bool: + state_filter = StateFilter.from_types([(EventTypes.PowerLevels, "")]) + state = await self.store.get_partial_filtered_current_state_ids( + event.room_id, state_filter + ) + pl_id = state[(EventTypes.PowerLevels, "")] + pl_event = await self.store.get_event(pl_id) + if pl_event: + sender_level = pl_event.content.get("users", {}).get(event.sender) + if sender_level is None: + sender_level = pl_event.content.get("users_default", 0) + + redact_level = pl_event.content.get("redact") + if not redact_level: + redact_level = pl_event.content.get("events_default", 0) + + room_redaction_level = pl_event.content.get("events", {}).get( + "m.room.redaction" + ) + if room_redaction_level: + if sender_level < room_redaction_level: + return False + + if sender_level >= redact_level: + return True + + return False + async def _calculate_sliding_sync_table_changes( self, room_id: str, diff --git a/synapse/storage/databases/main/events_worker.py b/synapse/storage/databases/main/events_worker.py index eca7421a06a..0591200afb3 100644 --- a/synapse/storage/databases/main/events_worker.py +++ b/synapse/storage/databases/main/events_worker.py @@ -961,6 +961,13 @@ def _invalidate_local_get_event_cache_room_id(self, room_id: str) -> None: self._event_ref.clear() self._current_event_fetches.clear() + def _invalidate_async_get_event_cache_room_id(self, room_id: str) -> None: + """ + Clears the async get_event cache for a room. Currently a no-op until + an async get_event cache is implemented - see https://github.com/matrix-org/synapse/pull/13242 + for preliminary work. + """ + async def _get_events_from_cache( self, events: Iterable[str], update_metrics: bool = True ) -> Dict[str, EventCacheEntry]: @@ -1590,10 +1597,9 @@ def _fetch_event_rows( continue redacting_event_id, redact_end_ordering = res if redact_end_ordering: - if e_row.stream_ordering < redact_end_ordering: - e_row.redactions.append(redacting_event_id) - else: - e_row.redactions.append(redacting_event_id) + if e_row.stream_ordering > redact_end_ordering: + continue + e_row.redactions.append(redacting_event_id) return event_dict def _maybe_redact_event_row( From c43a5a389c1b267819dd6e10fef5814ed1617e4a Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Mon, 21 Jul 2025 11:09:28 -0700 Subject: [PATCH 17/18] requested changes --- synapse/config/experimental.py | 3 +- synapse/storage/databases/main/events.py | 56 ++++++++++++------- .../storage/databases/main/events_worker.py | 5 +- tests/rest/client/test_rooms.py | 14 ++--- 4 files changed, 49 insertions(+), 29 deletions(-) diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index ef39d477fc5..c4181a6b0b3 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -584,8 +584,7 @@ def read_config( # MSC4293: Redact on Kick/Ban self.msc4293_enabled: bool = experimental.get("msc4293_enabled", False) - + # MSC4306: Thread Subscriptions # (and MSC4308: sliding sync extension for thread subscriptions) self.msc4306_enabled: bool = experimental.get("msc4306_enabled", False) - diff --git a/synapse/storage/databases/main/events.py b/synapse/storage/databases/main/events.py index 4de5640e9f8..7ef6c90831d 100644 --- a/synapse/storage/databases/main/events.py +++ b/synapse/storage/databases/main/events.py @@ -422,6 +422,10 @@ async def _persist_events_and_state_updates( # check that sender can redact redact_allowed = await self._can_sender_redact(event) + + # Signal that this user's past events in this room + # should be redacted by adding an entry to + # `room_ban_redactions`. if redact_allowed: await self.db_pool.simple_upsert( "room_ban_redactions", @@ -451,31 +455,45 @@ async def _persist_events_and_state_updates( ) async def _can_sender_redact(self, event: EventBase) -> bool: - state_filter = StateFilter.from_types([(EventTypes.PowerLevels, "")]) + state_filter = StateFilter.from_types( + [(EventTypes.PowerLevels, ""), (EventTypes.Create, "")] + ) state = await self.store.get_partial_filtered_current_state_ids( event.room_id, state_filter ) pl_id = state[(EventTypes.PowerLevels, "")] - pl_event = await self.store.get_event(pl_id) - if pl_event: - sender_level = pl_event.content.get("users", {}).get(event.sender) - if sender_level is None: - sender_level = pl_event.content.get("users_default", 0) - - redact_level = pl_event.content.get("redact") - if not redact_level: - redact_level = pl_event.content.get("events_default", 0) - - room_redaction_level = pl_event.content.get("events", {}).get( - "m.room.redaction" - ) - if room_redaction_level: - if sender_level < room_redaction_level: - return False - - if sender_level >= redact_level: + pl_event = await self.store.get_event(pl_id, allow_none=True) + + if pl_event is None: + # per the spec, if a power level event isn't in the room, grant the creator + # level 100 and all other users 0 + create_id = state[(EventTypes.Create, "")] + create_event = await self.store.get_event(create_id, allow_none=True) + if create_event is None: + # not sure how this would happen but if it does then just deny the redaction + return False + if create_event.sender == event.sender: return True + assert pl_event is not None + sender_level = pl_event.content.get("users", {}).get(event.sender) + if sender_level is None: + sender_level = pl_event.content.get("users_default", 0) + + redact_level = pl_event.content.get("redact") + if not redact_level: + redact_level = pl_event.content.get("events_default", 0) + + room_redaction_level = pl_event.content.get("events", {}).get( + "m.room.redaction" + ) + if room_redaction_level: + if sender_level < room_redaction_level: + return False + + if sender_level >= redact_level: + return True + return False async def _calculate_sliding_sync_table_changes( diff --git a/synapse/storage/databases/main/events_worker.py b/synapse/storage/databases/main/events_worker.py index 134763a04b1..7927842d0f0 100644 --- a/synapse/storage/databases/main/events_worker.py +++ b/synapse/storage/databases/main/events_worker.py @@ -1614,7 +1614,10 @@ def _fetch_event_rows( continue redacting_event_id, redact_end_ordering = res if redact_end_ordering: - if e_row.stream_ordering > redact_end_ordering: + # Avoid redacting any events arriving *after* the membership event which + # ends an active redaction - note that this will always redact + # backfilled events, as they have a negative stream ordering + if e_row.stream_ordering >= redact_end_ordering: continue e_row.redactions.append(redacting_event_id) return event_dict diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index fe0b7527db2..4bfd95f2cb3 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -4590,7 +4590,7 @@ def _check_redactions( def test_banning_local_member_with_flag_redacts_their_events(self) -> None: self.helper.join(self.room_id, self.bad_user_id, tok=self.bad_tok) - # bad user send some messages + # bad user sends some messages originals = [] for i in range(5): event = {"body": f"bothersome noise {i}", "msgtype": "m.text"} @@ -4916,7 +4916,7 @@ def test_redaction_flag_ignored_for_user_if_banner_lacks_redaction_power( ) self.helper.join(self.room_id, self.bad_user_id, tok=self.bad_tok) - # bad user send some messages + # bad user sends some messages original_ids = [] for i in range(15): event = {"body": f"being a menace {i}", "msgtype": "m.text"} @@ -4955,7 +4955,7 @@ def test_redaction_flag_ignored_for_user_if_banner_lacks_redaction_power( def test_kicking_local_member_with_flag_redacts_their_events(self) -> None: self.helper.join(self.room_id, self.bad_user_id, tok=self.bad_tok) - # bad user send some messages + # bad user sends some messages originals = [] for i in range(5): event = {"body": f"bothersome noise {i}", "msgtype": "m.text"} @@ -5273,7 +5273,7 @@ def test_redaction_flag_ignored_for_user_if_kicker_lacks_redaction_power( ) self.helper.join(self.room_id, self.bad_user_id, tok=self.bad_tok) - # bad user send some messages + # bad user sends some messages original_ids = [] for i in range(15): event = {"body": f"being a menace {i}", "msgtype": "m.text"} @@ -5312,7 +5312,7 @@ def test_redaction_flag_ignored_for_user_if_kicker_lacks_redaction_power( def test_MSC4293_flag_ignored_in_other_membership_events(self) -> None: self.helper.join(self.room_id, self.bad_user_id, tok=self.bad_tok) - # bad user send some messages + # bad user sends some messages original_ids = [] for i in range(15): event = {"body": f"being a menace {i}", "msgtype": "m.text"} @@ -5399,7 +5399,7 @@ def test_MSC4293_redaction_applied_via_kick_api(self) -> None: """ self.helper.join(self.room_id, self.bad_user_id, tok=self.bad_tok) - # bad user send some messages + # bad user sends some messages original_ids = [] for i in range(15): event = {"body": f"being a menace {i}", "msgtype": "m.text"} @@ -5444,7 +5444,7 @@ def test_MSC4293_redaction_applied_via_ban_api(self) -> None: """ self.helper.join(self.room_id, self.bad_user_id, tok=self.bad_tok) - # bad user send some messages + # bad user sends some messages original_ids = [] for i in range(15): event = {"body": f"being a menace {i}", "msgtype": "m.text"} From 47de415defb4d377fc36eebf76702539d0fd2ee3 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 22 Jul 2025 11:13:07 -0700 Subject: [PATCH 18/18] requested changes --- synapse/storage/databases/main/events.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/synapse/storage/databases/main/events.py b/synapse/storage/databases/main/events.py index 7ef6c90831d..c0299cb62e8 100644 --- a/synapse/storage/databases/main/events.py +++ b/synapse/storage/databases/main/events.py @@ -396,13 +396,17 @@ async def _persist_events_and_state_updates( # if there is an existing ban/leave causing redactions for # this user/room combination update the entry with the stream - # ordering when the redactions should stop + # ordering when the redactions should stop - in the case of a backfilled + # event where the stream ordering is negative, use the current max stream + # ordering + stream_ordering = event.internal_metadata.stream_ordering + assert stream_ordering is not None + if stream_ordering < 0: + stream_ordering = self._stream_id_gen.get_current_token() await self.db_pool.simple_update( "room_ban_redactions", {"room_id": event.room_id, "user_id": event.state_key}, - { - "redact_end_ordering": event.internal_metadata.stream_ordering - }, + {"redact_end_ordering": stream_ordering}, desc="room_ban_redactions update redact_end_ordering", ) @@ -471,6 +475,7 @@ async def _can_sender_redact(self, event: EventBase) -> bool: create_event = await self.store.get_event(create_id, allow_none=True) if create_event is None: # not sure how this would happen but if it does then just deny the redaction + logger.warning("No create event found for room %s", event.room_id) return False if create_event.sender == event.sender: return True @@ -481,13 +486,13 @@ async def _can_sender_redact(self, event: EventBase) -> bool: sender_level = pl_event.content.get("users_default", 0) redact_level = pl_event.content.get("redact") - if not redact_level: + if redact_level is None: redact_level = pl_event.content.get("events_default", 0) room_redaction_level = pl_event.content.get("events", {}).get( "m.room.redaction" ) - if room_redaction_level: + if room_redaction_level is not None: if sender_level < room_redaction_level: return False