From 9fcc140dee4ecdfe30f559bad3eacf9929614869 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Thu, 17 Oct 2024 14:55:07 -0700 Subject: [PATCH 1/6] add admin to list of handlers used for background tasks + test --- synapse/server.py | 1 + tests/rest/admin/test_user.py | 100 +++++++++++++++++++++++++++++++++- 2 files changed, 100 insertions(+), 1 deletion(-) diff --git a/synapse/server.py b/synapse/server.py index 318c6abf3d8..c7b49188139 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -249,6 +249,7 @@ class HomeServer(metaclass=abc.ABCMeta): """ REQUIRED_ON_BACKGROUND_TASK_STARTUP = [ + "admin", "account_validity", "auth", "deactivate_account", diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index 6982c7291a6..e28b71f9618 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -56,6 +56,7 @@ from synapse.util import Clock from tests import unittest +from tests.replication._base import BaseMultiWorkerStreamTestCase from tests.test_utils import SMALL_PNG from tests.unittest import override_config @@ -5127,7 +5128,6 @@ def test_redact_messages_all_rooms(self) -> None: """ Test that request to redact events in all rooms user is member of is successful """ - # join rooms, send some messages originals = [] for rm in [self.rm1, self.rm2, self.rm3]: @@ -5404,3 +5404,101 @@ def test_admin_redact_works_if_user_kicked_or_banned(self) -> None: matches.append((event_id, event)) # we redacted 6 messages self.assertEqual(len(matches), 6) + + +class UserRedactionBackgroundTaskTestCase(BaseMultiWorkerStreamTestCase): + servlets = [ + synapse.rest.admin.register_servlets, + login.register_servlets, + admin.register_servlets, + room.register_servlets, + sync.register_servlets, + ] + + def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: + self.admin = self.register_user("thomas", "pass", True) + self.admin_tok = self.login("thomas", "pass") + + self.bad_user = self.register_user("teresa", "pass") + self.bad_user_tok = self.login("teresa", "pass") + + self.store = hs.get_datastores().main + + self.spam_checker = hs.get_module_api_callbacks().spam_checker + + # create rooms - room versions 11+ store the `redacts` key in content while + # earlier ones don't so we use a mix of room versions + self.rm1 = self.helper.create_room_as( + self.admin, tok=self.admin_tok, room_version="7" + ) + self.rm2 = self.helper.create_room_as(self.admin, tok=self.admin_tok) + self.rm3 = self.helper.create_room_as( + self.admin, tok=self.admin_tok, room_version="11" + ) + + @override_config({"run_background_tasks_on": "worker1"}) + def test_redact_messages_all_rooms(self) -> None: + """ + Test that redact task successfully runs when `run_background_tasks_on` is specified + """ + self.make_worker_hs( + "synapse.app.generic_worker", + extra_config={ + "worker_name": "worker1", + "run_background_tasks_on": "worker1", + "redis": {"enabled": True}, + }, + ) + + # join rooms, send some messages + originals = [] + for rm in [self.rm1, self.rm2, self.rm3]: + join = self.helper.join(rm, self.bad_user, tok=self.bad_user_tok) + originals.append(join["event_id"]) + for i in range(15): + event = {"body": f"hello{i}", "msgtype": "m.text"} + res = self.helper.send_event( + rm, "m.room.message", event, tok=self.bad_user_tok, expect_code=200 + ) + originals.append(res["event_id"]) + + # redact all events in all rooms + channel = self.make_request( + "POST", + f"/_synapse/admin/v1/user/{self.bad_user}/redact", + content={"rooms": []}, + access_token=self.admin_tok, + ) + self.assertEqual(channel.code, 200) + id = channel.json_body.get("redact_id") + + for _ in range(100): + channel2 = self.make_request( + "GET", + f"/_synapse/admin/v1/user/redact_status/{id}", + access_token=self.admin_tok, + ) + redact_result = channel2.json_body["status"] + if redact_result == "complete": + break + if redact_result == "failed": + self.fail("Redaction task failed.") + + matched = [] + for rm in [self.rm1, self.rm2, self.rm3]: + filter = json.dumps({"types": [EventTypes.Redaction]}) + channel = self.make_request( + "GET", + f"rooms/{rm}/messages?filter={filter}&limit=50", + access_token=self.admin_tok, + ) + self.assertEqual(channel.code, 200) + + for event in channel.json_body["chunk"]: + for event_id in originals: + if ( + event["type"] == "m.room.redaction" + and event["redacts"] == event_id + ): + matched.append(event_id) + self.assertEqual(len(matched), len(originals)) From b76981b12aebd5df90598f69046abc09bb1e1b9e Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Thu, 17 Oct 2024 15:06:02 -0700 Subject: [PATCH 2/6] use admin to redact if the user isn't ours --- synapse/handlers/admin.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/synapse/handlers/admin.py b/synapse/handlers/admin.py index 851fe57a177..d1989e9d2c6 100644 --- a/synapse/handlers/admin.py +++ b/synapse/handlers/admin.py @@ -73,6 +73,8 @@ def __init__(self, hs: "HomeServer"): self._redact_all_events, REDACT_ALL_EVENTS_ACTION_NAME ) + self.hs = hs + async def get_redact_task(self, redact_id: str) -> Optional[ScheduledTask]: """Get the current status of an active redaction process @@ -423,8 +425,10 @@ async def _redact_all_events( user_id = task.params.get("user_id") assert user_id is not None + # puppet the user if they're ours, otherwise use admin to redact requester = create_requester( - user_id, authenticated_entity=admin.user.to_string() + user_id if self.hs.is_mine_id(user_id) else admin.user.to_string(), + authenticated_entity=admin.user.to_string(), ) reason = task.params.get("reason") From e003c321eb5533047749568c7988708923541217 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Thu, 17 Oct 2024 15:16:40 -0700 Subject: [PATCH 3/6] newsfragement --- changelog.d/17847.bugfix | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 changelog.d/17847.bugfix diff --git a/changelog.d/17847.bugfix b/changelog.d/17847.bugfix new file mode 100644 index 00000000000..0914265ab13 --- /dev/null +++ b/changelog.d/17847.bugfix @@ -0,0 +1,2 @@ +Fix a bug in the admin redact endpoint where the background task would not run if a worker was specified in + the config option `run_background_tasks_on`. \ No newline at end of file From 4dd7b20f73f414791f7a18fd77b955a0f365ee18 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Mon, 28 Oct 2024 12:35:26 -0700 Subject: [PATCH 4/6] requested changes --- changelog.d/17847.bugfix | 2 +- docs/admin_api/user_admin_api.md | 3 +++ tests/rest/admin/test_user.py | 34 +++++++++++++++----------------- 3 files changed, 20 insertions(+), 19 deletions(-) diff --git a/changelog.d/17847.bugfix b/changelog.d/17847.bugfix index 0914265ab13..0ba39df94df 100644 --- a/changelog.d/17847.bugfix +++ b/changelog.d/17847.bugfix @@ -1,2 +1,2 @@ Fix a bug in the admin redact endpoint where the background task would not run if a worker was specified in - the config option `run_background_tasks_on`. \ No newline at end of file +the config option `run_background_tasks_on`. \ No newline at end of file diff --git a/docs/admin_api/user_admin_api.md b/docs/admin_api/user_admin_api.md index cb38e26005a..33a98e3adf7 100644 --- a/docs/admin_api/user_admin_api.md +++ b/docs/admin_api/user_admin_api.md @@ -1365,6 +1365,9 @@ _Added in Synapse 1.72.0._ ## Redact all the events of a user +This endpoint allows an admin to redact the events of a given user. There are no restrictions on redactions for a +local user. Redactions for non-local users are issued using the admin user, and will fail in rooms where the admin user is not admin/does not have the specified power level to issue redactions. + The API is ``` POST /_synapse/admin/v1/user/$user_id/redact diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index e28b71f9618..e46aab7a867 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -23,6 +23,7 @@ import hmac import json import os +import time import urllib.parse from binascii import unhexlify from http import HTTPStatus @@ -5422,10 +5423,6 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: self.bad_user = self.register_user("teresa", "pass") self.bad_user_tok = self.login("teresa", "pass") - self.store = hs.get_datastores().main - - self.spam_checker = hs.get_module_api_callbacks().spam_checker - # create rooms - room versions 11+ store the `redacts` key in content while # earlier ones don't so we use a mix of room versions self.rm1 = self.helper.create_room_as( @@ -5451,16 +5448,16 @@ def test_redact_messages_all_rooms(self) -> None: ) # join rooms, send some messages - originals = [] + original_event_ids = set() for rm in [self.rm1, self.rm2, self.rm3]: join = self.helper.join(rm, self.bad_user, tok=self.bad_user_tok) - originals.append(join["event_id"]) + original_event_ids.add(join["event_id"]) for i in range(15): event = {"body": f"hello{i}", "msgtype": "m.text"} res = self.helper.send_event( rm, "m.room.message", event, tok=self.bad_user_tok, expect_code=200 ) - originals.append(res["event_id"]) + original_event_ids.add(res["event_id"]) # redact all events in all rooms channel = self.make_request( @@ -5472,19 +5469,23 @@ def test_redact_messages_all_rooms(self) -> None: self.assertEqual(channel.code, 200) id = channel.json_body.get("redact_id") - for _ in range(100): + timeout = 10 + start_time = time.time() + redact_result = "" + while redact_result != "complete": + if start_time + timeout < time.time(): + self.fail("Timed out waiting for redactions.") + channel2 = self.make_request( "GET", f"/_synapse/admin/v1/user/redact_status/{id}", access_token=self.admin_tok, ) redact_result = channel2.json_body["status"] - if redact_result == "complete": - break if redact_result == "failed": self.fail("Redaction task failed.") - matched = [] + redaction_ids = set() for rm in [self.rm1, self.rm2, self.rm3]: filter = json.dumps({"types": [EventTypes.Redaction]}) channel = self.make_request( @@ -5495,10 +5496,7 @@ def test_redact_messages_all_rooms(self) -> None: self.assertEqual(channel.code, 200) for event in channel.json_body["chunk"]: - for event_id in originals: - if ( - event["type"] == "m.room.redaction" - and event["redacts"] == event_id - ): - matched.append(event_id) - self.assertEqual(len(matched), len(originals)) + if event["type"] == "m.room.redaction": + redaction_ids.add(event["redacts"]) + + self.assertIncludes(redaction_ids, original_event_ids, exact=True) From 4f105df857c08b2fac72061de47909d91a89b067 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 29 Oct 2024 09:03:22 -0700 Subject: [PATCH 5/6] label timeout better --- tests/rest/admin/test_user.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index e46aab7a867..f9ae50f40a8 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -5469,11 +5469,11 @@ def test_redact_messages_all_rooms(self) -> None: self.assertEqual(channel.code, 200) id = channel.json_body.get("redact_id") - timeout = 10 + timeout_s = 10 start_time = time.time() redact_result = "" while redact_result != "complete": - if start_time + timeout < time.time(): + if start_time + timeout_s < time.time(): self.fail("Timed out waiting for redactions.") channel2 = self.make_request( From 8b5a7521c7dfeace7a40d1236f5c3c69f8c6ce48 Mon Sep 17 00:00:00 2001 From: Shay Date: Tue, 29 Oct 2024 09:04:12 -0700 Subject: [PATCH 6/6] reword doc Co-authored-by: Eric Eastwood --- docs/admin_api/user_admin_api.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/admin_api/user_admin_api.md b/docs/admin_api/user_admin_api.md index 33a98e3adf7..96a2994b7b4 100644 --- a/docs/admin_api/user_admin_api.md +++ b/docs/admin_api/user_admin_api.md @@ -1366,7 +1366,7 @@ _Added in Synapse 1.72.0._ ## Redact all the events of a user This endpoint allows an admin to redact the events of a given user. There are no restrictions on redactions for a -local user. Redactions for non-local users are issued using the admin user, and will fail in rooms where the admin user is not admin/does not have the specified power level to issue redactions. +local user. By default, we puppet the user who sent the message to redact it themselves. Redactions for non-local users are issued using the admin user, and will fail in rooms where the admin user is not admin/does not have the specified power level to issue redactions. The API is ```