From a6ae7b7d69c536ce369401e8947d95703a38780d Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 19 Nov 2024 09:47:39 -0800 Subject: [PATCH 01/40] add functionality to fetch event reports against a specific user --- synapse/rest/admin/__init__.py | 2 + synapse/rest/admin/event_reports.py | 32 +++++++++++++ synapse/storage/databases/main/room.py | 63 ++++++++++++++++++++++++++ 3 files changed, 97 insertions(+) diff --git a/synapse/rest/admin/__init__.py b/synapse/rest/admin/__init__.py index 4db89756747..f227e203493 100644 --- a/synapse/rest/admin/__init__.py +++ b/synapse/rest/admin/__init__.py @@ -54,6 +54,7 @@ DevicesRestServlet, ) from synapse.rest.admin.event_reports import ( + EventReportAgainstUserRestServlet, EventReportDetailRestServlet, EventReportsRestServlet, ) @@ -298,6 +299,7 @@ def register_servlets(hs: "HomeServer", http_server: HttpServer) -> None: UsersRestServletV3(hs).register(http_server) UserMediaStatisticsRestServlet(hs).register(http_server) LargestRoomsStatistics(hs).register(http_server) + EventReportAgainstUserRestServlet(hs).register(http_server) EventReportDetailRestServlet(hs).register(http_server) EventReportsRestServlet(hs).register(http_server) AccountDataRestServlet(hs).register(http_server) diff --git a/synapse/rest/admin/event_reports.py b/synapse/rest/admin/event_reports.py index 9fb68bfa462..a6723b097b2 100644 --- a/synapse/rest/admin/event_reports.py +++ b/synapse/rest/admin/event_reports.py @@ -168,3 +168,35 @@ async def on_DELETE( return HTTPStatus.OK, {} raise NotFoundError("Event report not found") + + +class EventReportAgainstUserRestServlet(RestServlet): + """ + Get all the event reports where the original sender of the reported event is the provided user + + GET /_synapse/admin/v1/event_reports/user/ + + """ + + PATTERNS = admin_patterns("/event_reports/user/(?P[^/]*)$") + + def __init__(self, hs: "HomeServer"): + self._auth = hs.get_auth() + self._store = hs.get_datastores().main + + async def on_GET( + self, request: SynapseRequest, user_id: str + ) -> Tuple[HTTPStatus, dict[str, list[str]]]: + await assert_requester_is_admin(self._auth, request) + + start = parse_integer(request, "start", default=0) + limit = parse_integer(request, "limit", default=100) + direction = parse_enum(request, "dir", Direction, Direction.BACKWARDS) + + res = await self._store.get_event_report_IDs_by_sender( + user_id, start, limit, direction + ) + if not res: + raise NotFoundError("User ID not found") + + return HTTPStatus.OK, {"report_ids": res} diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index cc3ce0951e7..f1961b7565a 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -1518,6 +1518,69 @@ def get_un_partial_stated_rooms_from_stream_txn( get_un_partial_stated_rooms_from_stream_txn, ) + async def get_event_report_IDs_by_sender( + self, + sender_id: str, + start: int, + limit: int, + direction: Direction = Direction.BACKWARDS, + ) -> Optional[List[str]]: + """ + Get all the event reports IDs where the sender is the provided user id + + Args: + sender_id: The user_id of the sender of the reported event + start: event offset to begin the query from + limit: number of rows to retrieve + direction: Whether to fetch the most recent first (backwards) or the + oldest first (forwards) + """ + + def _get_event_report_IDs_txn( + txn: LoggingTransaction, + sender_id: str, + start: int, + limit: int, + direction: Direction, + ) -> Optional[List[str]]: + if direction == Direction.BACKWARDS: + order = "DESC" + else: + order = "ASC" + + sql = f""" + SELECT + er.id + FROM event_reports AS er + LEFT JOIN events + ON events.event_id = er.event_id + WHERE events.sender = ? + ORDER BY er.received_ts {order} + LIMIT ? + OFFSET ? + """ + + txn.execute(sql, (sender_id, limit, start)) + rows = txn.fetchall() + + if not rows: + return None + + report_ids = [] + for r in rows: + report_ids.append(r[0]) + + return report_ids + + return await self.db_pool.runInteraction( + "get_event_report_IDs_by_sender", + _get_event_report_IDs_txn, + sender_id, + start, + limit, + direction, + ) + async def get_event_report(self, report_id: int) -> Optional[Dict[str, Any]]: """Retrieve an event report From bc8930d007452dfaf58571c4d40a78b7493f06f4 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 19 Nov 2024 09:47:47 -0800 Subject: [PATCH 02/40] tests --- tests/rest/admin/test_event_reports.py | 136 +++++++++++++++++++++++++ 1 file changed, 136 insertions(+) diff --git a/tests/rest/admin/test_event_reports.py b/tests/rest/admin/test_event_reports.py index feb410a11d6..bdb3196ef28 100644 --- a/tests/rest/admin/test_event_reports.py +++ b/tests/rest/admin/test_event_reports.py @@ -745,3 +745,139 @@ def test_report_id_not_found(self) -> None: self.assertEqual(404, channel.code, msg=channel.json_body) self.assertEqual(Codes.NOT_FOUND, channel.json_body["errcode"]) self.assertEqual("Event report not found", channel.json_body["error"]) + + +class EventReportsAgainstUserTestCase(unittest.HomeserverTestCase): + servlets = [ + synapse.rest.admin.register_servlets, + login.register_servlets, + room.register_servlets, + reporting.register_servlets, + ] + + def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: + self.admin_user = self.register_user("admin", "pass", admin=True) + self.admin_user_tok = self.login("admin", "pass") + + self.bad_user_1 = self.register_user("user", "pass") + self.bad_user_1_tok = self.login("user", "pass") + + self.bad_user_2 = self.register_user("user2", "pass") + self.bad_user_2_tok = self.login("user2", "pass") + + self.room_id1 = self.helper.create_room_as( + self.bad_user_1, tok=self.bad_user_1_tok, is_public=True + ) + self.helper.join(self.room_id1, user=self.admin_user, tok=self.admin_user_tok) + self.helper.join(self.room_id1, user=self.bad_user_2, tok=self.bad_user_2_tok) + + self.room_id2 = self.helper.create_room_as( + self.bad_user_1, tok=self.bad_user_1_tok, is_public=True + ) + self.helper.join(self.room_id2, user=self.admin_user, tok=self.admin_user_tok) + self.helper.join(self.room_id2, user=self.bad_user_2, tok=self.bad_user_2_tok) + + for _ in range(5): + self._create_event_and_report( + room_id=self.room_id1, + sender_tok=self.bad_user_1_tok, + reporter_tok=self.admin_user_tok, + ) + for _ in range(5): + self._create_event_and_report( + room_id=self.room_id2, + sender_tok=self.bad_user_2_tok, + reporter_tok=self.admin_user_tok, + ) + + self.url = "/_synapse/admin/v1/event_reports" + + def _create_event_and_report( + self, room_id: str, sender_tok: str, reporter_tok: str + ) -> None: + """Create and report events""" + resp = self.helper.send(room_id, tok=sender_tok) + event_id = resp["event_id"] + + channel = self.make_request( + "POST", + "rooms/%s/report/%s" % (room_id, event_id), + {"score": -100, "reason": "this makes me sad"}, + access_token=reporter_tok, + ) + self.assertEqual(200, channel.code, msg=channel.json_body) + + def test_get_reports_against_user_no_params(self) -> None: + # first grab all the reports + channel = self.make_request( + "GET", + self.url, + access_token=self.admin_user_tok, + ) + self.assertEqual(channel.code, 200) + self.assertEqual(len(channel.json_body["event_reports"]), 10) + + # filter out set of report ids of events sent by bad user + filtered_report_ids = [] + for event_report in channel.json_body["event_reports"]: + if event_report["sender"] == self.bad_user_1: + filtered_report_ids.append(event_report["id"]) + + # grab the report ids by sender and compare to filtered report ids + channel = self.make_request( + "GET", + f"{self.url}/user/{self.bad_user_1}", + access_token=self.admin_user_tok, + ) + self.assertEqual(200, channel.code) + report_ids_by_user = channel.json_body["report_ids"] + self.assertEqual(True, set(filtered_report_ids) == set(report_ids_by_user)) + + def test_get_reports_against_user_params(self) -> None: + # first grab all the reports + channel = self.make_request( + "GET", + self.url, + access_token=self.admin_user_tok, + ) + self.assertEqual(channel.code, 200) + self.assertEqual(len(channel.json_body["event_reports"]), 10) + + # filter out set of report ids of events sent by bad user + filtered_report_ids = [] + for event_report in channel.json_body["event_reports"]: + if event_report["sender"] == self.bad_user_2: + filtered_report_ids.append(event_report["id"]) + + channel = self.make_request( + "GET", + f"{self.url}/user/{self.bad_user_2}?start=2&limit=2", + access_token=self.admin_user_tok, + ) + self.assertEqual(200, channel.code) + report_ids_by_user = channel.json_body["report_ids"] + + # check that limit is respected + self.assertEqual(len(report_ids_by_user), 2) + + # check that offset is respected + self.assertEqual([9, 8], report_ids_by_user) + + # check that response is for correct user + self.assertEqual( + True, set(report_ids_by_user).issubset(set(filtered_report_ids)) + ) + + channel = self.make_request( + "GET", + f"{self.url}/user/{self.bad_user_2}?dir=f", + access_token=self.admin_user_tok, + ) + self.assertEqual(200, channel.code) + oldest_report_ids_by_user = channel.json_body["report_ids"] + + # check reversal param + self.assertEqual([7, 8, 9, 10, 11], oldest_report_ids_by_user) + self.assertEqual( + True, set(oldest_report_ids_by_user).issubset(set(filtered_report_ids)) + ) From 106cb4f6d97961d3686d70184ffbeb32cfc0a8e5 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 19 Nov 2024 17:26:08 -0800 Subject: [PATCH 03/40] add admin apis to get invite count and joined room count for last 24hrs for user --- synapse/rest/admin/__init__.py | 4 + synapse/rest/admin/users.py | 42 ++++++ .../storage/databases/main/events_worker.py | 56 ++++++++ tests/rest/admin/test_user.py | 122 ++++++++++++++++++ 4 files changed, 224 insertions(+) diff --git a/synapse/rest/admin/__init__.py b/synapse/rest/admin/__init__.py index f227e203493..47ff19f72b1 100644 --- a/synapse/rest/admin/__init__.py +++ b/synapse/rest/admin/__init__.py @@ -108,10 +108,12 @@ UserAdminServlet, UserByExternalId, UserByThreePid, + UserInvitesCount, UserMembershipRestServlet, UserRegisterServlet, UserReplaceMasterCrossSigningKeyRestServlet, UserRestServletV2, + UserRoomJoinCount, UsersRestServletV2, UsersRestServletV3, UserTokenRestServlet, @@ -325,6 +327,8 @@ def register_servlets(hs: "HomeServer", http_server: HttpServer) -> None: UserByThreePid(hs).register(http_server) RedactUser(hs).register(http_server) RedactUserStatus(hs).register(http_server) + UserInvitesCount(hs).register(http_server) + UserRoomJoinCount(hs).register(http_server) DeviceRestServlet(hs).register(http_server) DevicesRestServlet(hs).register(http_server) diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index b146c2754d6..91d7781c0ca 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -1501,3 +1501,45 @@ async def on_GET( } else: raise NotFoundError("redact id '%s' not found" % redact_id) + + +class UserInvitesCount(RestServlet): + """ + Return the count of invites that the user has sent in the last 24 hours + """ + + PATTERNS = admin_patterns("/user/invite_count/(?P[^/]*)$") + + def __init__(self, hs: "HomeServer"): + self._auth = hs.get_auth() + self.store = hs.get_datastores().main + + async def on_GET( + self, request: SynapseRequest, user_id: str + ) -> Tuple[int, JsonDict]: + await assert_requester_is_admin(self._auth, request) + + res = await self.store.get_invite_count_by_user(user_id) + + return HTTPStatus.OK, {"invite_count": res} + + +class UserRoomJoinCount(RestServlet): + """ + Return the count of rooms that the user has joined in the last 24 hours + """ + + PATTERNS = admin_patterns("/user/room_count/(?P[^/]*)$") + + def __init__(self, hs: "HomeServer"): + self._auth = hs.get_auth() + self.store = hs.get_datastores().main + + async def on_GET( + self, request: SynapseRequest, user_id: str + ) -> Tuple[int, JsonDict]: + await assert_requester_is_admin(self._auth, request) + + res = await self.store.get_join_count_by_user(user_id) + + return HTTPStatus.OK, {"room_count": res} diff --git a/synapse/storage/databases/main/events_worker.py b/synapse/storage/databases/main/events_worker.py index 403407068c5..eb7cb9f191f 100644 --- a/synapse/storage/databases/main/events_worker.py +++ b/synapse/storage/databases/main/events_worker.py @@ -2580,3 +2580,59 @@ async def have_finished_sliding_sync_background_jobs(self) -> bool: _BackgroundUpdates.SLIDING_SYNC_MEMBERSHIP_SNAPSHOTS_BG_UPDATE, ) ) + + async def get_invite_count_by_user(self, user_id: str) -> int: + """ + Get the number of invites sent by the given user in the past 24 hours + """ + timestamp = self._clock.time_msec() - (24 * 60 * 60 * 1000) # 24 hours ago + + def _get_invite_count_by_user_txn( + txn: LoggingTransaction, user_id: str, timestamp: int + ) -> int: + sql = """ + SELECT COUNT(event_id) FROM events WHERE sender = ? AND type = "m.room.member" AND received_ts > ? AND state_key != ? + """ + + txn.execute(sql, (user_id, timestamp, user_id)) + res = txn.fetchone() + + if res is None: + return 0 + return int(res[0]) + + return await self.db_pool.runInteraction( + "_get_invite_count_by_user_txn", + _get_invite_count_by_user_txn, + user_id, + timestamp, + ) + + async def get_join_count_by_user(self, user_id: str) -> int: + """ + Get the number of rooms the user has joined in the last 24hrs + """ + timestamp = self._clock.time_msec() - (24 * 60 * 60 * 1000) # 24 hours ago + + def _get_join_count_by_user_txn( + txn: LoggingTransaction, user_id: str, timestamp: int + ) -> int: + sql = """ + SELECT COUNT(c.event_id) + FROM current_state_events c + JOIN events e ON c.event_id = e.event_id + WHERE c.membership = 'join' AND c.state_key = ? AND e.received_ts > ? + """ + txn.execute(sql, (user_id, timestamp)) + res = txn.fetchone() + + if res is None: + return 0 + return int(res[0]) + + return await self.db_pool.runInteraction( + "_get_join_count_by_user_txn", + _get_join_count_by_user_txn, + user_id, + timestamp, + ) diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index 668ccb89ff1..17777527c61 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -5500,3 +5500,125 @@ def test_redact_messages_all_rooms(self) -> None: redaction_ids.add(event["redacts"]) self.assertIncludes(redaction_ids, original_event_ids, exact=True) + + +class GetInvitesFromUserTestCase(unittest.HomeserverTestCase): + servlets = [ + synapse.rest.admin.register_servlets, + login.register_servlets, + admin.register_servlets, + room.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.random_users = [] + for i in range(4): + self.random_users.append(self.register_user(f"user{i}", f"pass{i}")) + + self.rm1 = self.helper.create_room_as(self.bad_user, tok=self.bad_user_tok) + self.rm2 = self.helper.create_room_as(self.bad_user, tok=self.bad_user_tok) + self.rm3 = self.helper.create_room_as(self.bad_user, tok=self.bad_user_tok) + + def test_get_user_invite_count_test_case(self) -> None: + # bad user send some invites + for rm in [self.rm1, self.rm2]: + for user in self.random_users: + self.helper.invite(rm, self.bad_user, user, tok=self.bad_user_tok) + + channel = self.make_request( + "GET", + f"/_synapse/admin/v1/user/invite_count/{self.bad_user}", + access_token=self.admin_tok, + ) + self.assertEqual(channel.code, 200) + self.assertEqual(channel.json_body["invite_count"], 8) + + # advance clock 48 hours and check again, should return 0 + self.reactor.advance(60 * 60 * 48 * 1000) + channel = self.make_request( + "GET", + f"/_synapse/admin/v1/user/invite_count/{self.bad_user}", + access_token=self.admin_tok, + ) + self.assertEqual(channel.code, 200) + self.assertEqual(channel.json_body["invite_count"], 0) + + # send some more invites, they should show up + for user in self.random_users: + self.helper.invite(self.rm3, self.bad_user, user, tok=self.bad_user_tok) + + channel = self.make_request( + "GET", + f"/_synapse/admin/v1/user/invite_count/{self.bad_user}", + access_token=self.admin_tok, + ) + self.assertEqual(channel.code, 200) + self.assertEqual(channel.json_body["invite_count"], 4) + + +class GetRoomCountForUserTestCase(unittest.HomeserverTestCase): + servlets = [ + synapse.rest.admin.register_servlets, + login.register_servlets, + admin.register_servlets, + room.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.rooms1 = [] + self.rooms2 = [] + for i in range(10): + rm = self.helper.create_room_as( + self.admin, is_public=True, tok=self.admin_tok + ) + if i % 2 == 0: + self.rooms1.append(rm) + else: + self.rooms2.append(rm) + + def test_room_count_for_user(self) -> None: + # bad user go on a join spree + for rm in self.rooms1: + self.helper.join(rm, self.bad_user, tok=self.bad_user_tok) + + channel = self.make_request( + "GET", + f"/_synapse/admin/v1/user/room_count/{self.bad_user}", + access_token=self.admin_tok, + ) + self.assertEqual(channel.code, 200) + self.assertEqual(channel.json_body["room_count"], 5) + + # advance clock 48 hours and check again, should return 0 + self.reactor.advance(60 * 60 * 48 * 1000) + channel = self.make_request( + "GET", + f"/_synapse/admin/v1/user/room_count/{self.bad_user}", + access_token=self.admin_tok, + ) + self.assertEqual(channel.code, 200) + self.assertEqual(channel.json_body["room_count"], 0) + + # join some more + for rm in self.rooms2: + self.helper.join(rm, self.bad_user, tok=self.bad_user_tok) + + channel = self.make_request( + "GET", + f"/_synapse/admin/v1/user/room_count/{self.bad_user}", + access_token=self.admin_tok, + ) + self.assertEqual(channel.code, 200) + self.assertEqual(channel.json_body["room_count"], 5) From 3c6e281853065766649bba9506e8ad6e85904857 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Wed, 20 Nov 2024 09:02:19 -0800 Subject: [PATCH 04/40] docs --- docs/admin_api/event_reports.md | 30 ++++++++++++++++++++++++ docs/admin_api/user_admin_api.md | 39 +++++++++++++++++++++++++++++++- 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/docs/admin_api/event_reports.md b/docs/admin_api/event_reports.md index 83f7dc37f41..7ca297cc8a6 100644 --- a/docs/admin_api/event_reports.md +++ b/docs/admin_api/event_reports.md @@ -183,3 +183,33 @@ DELETE /_synapse/admin/v1/event_reports/ **URL parameters:** * `report_id`: string - The ID of the event report. + + +# Fetch the report IDs for a specific user + +Fetches the report IDs where the sender of the reported event is the provided user ID. + +The api is: +``` +GET /_synapse/admin/v1/event_reports/user/$user_id +``` + +**URL paramters** + +* `user_id`: string - Required. The ID of the user to search for +* `limit`: integer - Is optional but is used for pagination, denoting the maximum number + of items to return in this call. Defaults to `100`. +* `from`: integer - Is optional but used for pagination, denoting the offset in the + returned results. This should be treated as an opaque value and not explicitly set to + anything other than the return value of `next_token` from a previous call. Defaults to `0`. +* `dir`: string - Direction of event report order. Whether to fetch the most recent + first (`b`) or the oldest first (`f`). Defaults to `b`. + +A response body like the following is returned: +``` + { + "report_ids": [2, 3, 4] + } +``` + +_Added in Synapse 1.120.0_ \ 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 96a2994b7b4..5ee345f2ab8 100644 --- a/docs/admin_api/user_admin_api.md +++ b/docs/admin_api/user_admin_api.md @@ -1443,4 +1443,41 @@ The following fields are returned in the JSON response body: - `failed_redactions` - dictionary - the keys of the dict are event ids the process was unable to redact, if any, and the values are the corresponding error that caused the redaction to fail -_Added in Synapse 1.116.0._ \ No newline at end of file +_Added in Synapse 1.116.0._ + + +## Get the number of invites sent by the user in the last 24 hours + +Fetches the count of the number of invites sent by the provided user ID across all rooms in the last 24 hours. + +``` +GET /_synapse/admin/v1/user/invite_count/$user_id +``` + +A response body like the following is returned: + +``` +{ + "invite_count": 30 +} +``` + +_Added in Synapse 1.120.0_ + +## Get the number of rooms the user has joined in the last 24 hours + +Fetches the number of rooms the provided user ID has joined in the last 24 hours. + +``` +GET /_synapse/admin/v1/user/room_count/$user_id +``` + +A response body like the following is returned: + +``` +{ + "room_count": 50 +} +``` + +_Added in Synapse 1.120.0_ From 81f8ce8798dba4b43ecafc85bc2e955a1ce76f04 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Wed, 20 Nov 2024 09:19:18 -0800 Subject: [PATCH 05/40] newsfragment --- changelog.d/17948.feature | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog.d/17948.feature diff --git a/changelog.d/17948.feature b/changelog.d/17948.feature new file mode 100644 index 00000000000..c85c2f60c34 --- /dev/null +++ b/changelog.d/17948.feature @@ -0,0 +1,3 @@ +Add endpoints to Admin API to fetch the number of invites the provided user has sent in the past 24 hours, +fetch the number of rooms the provided user has joined in the past 24 hours, and get report IDs of event +reports where the provided user was the sender of the reported event. From d7a6a8c4ea7965ff0445ffb8bf70264499f06943 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Wed, 20 Nov 2024 10:55:28 -0800 Subject: [PATCH 06/40] fix postgres quotes situation --- synapse/storage/databases/main/events_worker.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/events_worker.py b/synapse/storage/databases/main/events_worker.py index eb7cb9f191f..b13f92fec27 100644 --- a/synapse/storage/databases/main/events_worker.py +++ b/synapse/storage/databases/main/events_worker.py @@ -2591,7 +2591,7 @@ def _get_invite_count_by_user_txn( txn: LoggingTransaction, user_id: str, timestamp: int ) -> int: sql = """ - SELECT COUNT(event_id) FROM events WHERE sender = ? AND type = "m.room.member" AND received_ts > ? AND state_key != ? + SELECT COUNT(event_id) FROM events WHERE sender = ? AND type = 'm.room.member' AND received_ts > ? AND state_key != ? """ txn.execute(sql, (user_id, timestamp, user_id)) From 408e1de61ec21bf34fde4de8620005fa9cc1761b Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Wed, 20 Nov 2024 13:48:24 -0800 Subject: [PATCH 07/40] should be users --- docs/admin_api/user_admin_api.md | 4 ++-- synapse/rest/admin/users.py | 4 ++-- tests/rest/admin/test_user.py | 12 ++++++------ 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/docs/admin_api/user_admin_api.md b/docs/admin_api/user_admin_api.md index 5ee345f2ab8..639f8aba6ed 100644 --- a/docs/admin_api/user_admin_api.md +++ b/docs/admin_api/user_admin_api.md @@ -1451,7 +1451,7 @@ _Added in Synapse 1.116.0._ Fetches the count of the number of invites sent by the provided user ID across all rooms in the last 24 hours. ``` -GET /_synapse/admin/v1/user/invite_count/$user_id +GET /_synapse/admin/v1/users/invite_count/$user_id ``` A response body like the following is returned: @@ -1469,7 +1469,7 @@ _Added in Synapse 1.120.0_ Fetches the number of rooms the provided user ID has joined in the last 24 hours. ``` -GET /_synapse/admin/v1/user/room_count/$user_id +GET /_synapse/admin/v1/users/room_count/$user_id ``` A response body like the following is returned: diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index 91d7781c0ca..f92fdea49bc 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -1508,7 +1508,7 @@ class UserInvitesCount(RestServlet): Return the count of invites that the user has sent in the last 24 hours """ - PATTERNS = admin_patterns("/user/invite_count/(?P[^/]*)$") + PATTERNS = admin_patterns("/users/invite_count/(?P[^/]*)$") def __init__(self, hs: "HomeServer"): self._auth = hs.get_auth() @@ -1529,7 +1529,7 @@ class UserRoomJoinCount(RestServlet): Return the count of rooms that the user has joined in the last 24 hours """ - PATTERNS = admin_patterns("/user/room_count/(?P[^/]*)$") + PATTERNS = admin_patterns("/users/room_count/(?P[^/]*)$") def __init__(self, hs: "HomeServer"): self._auth = hs.get_auth() diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index 17777527c61..38c54f78a90 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -5533,7 +5533,7 @@ def test_get_user_invite_count_test_case(self) -> None: channel = self.make_request( "GET", - f"/_synapse/admin/v1/user/invite_count/{self.bad_user}", + f"/_synapse/admin/v1/users/invite_count/{self.bad_user}", access_token=self.admin_tok, ) self.assertEqual(channel.code, 200) @@ -5543,7 +5543,7 @@ def test_get_user_invite_count_test_case(self) -> None: self.reactor.advance(60 * 60 * 48 * 1000) channel = self.make_request( "GET", - f"/_synapse/admin/v1/user/invite_count/{self.bad_user}", + f"/_synapse/admin/v1/users/invite_count/{self.bad_user}", access_token=self.admin_tok, ) self.assertEqual(channel.code, 200) @@ -5555,7 +5555,7 @@ def test_get_user_invite_count_test_case(self) -> None: channel = self.make_request( "GET", - f"/_synapse/admin/v1/user/invite_count/{self.bad_user}", + f"/_synapse/admin/v1/users/invite_count/{self.bad_user}", access_token=self.admin_tok, ) self.assertEqual(channel.code, 200) @@ -5595,7 +5595,7 @@ def test_room_count_for_user(self) -> None: channel = self.make_request( "GET", - f"/_synapse/admin/v1/user/room_count/{self.bad_user}", + f"/_synapse/admin/v1/users/room_count/{self.bad_user}", access_token=self.admin_tok, ) self.assertEqual(channel.code, 200) @@ -5605,7 +5605,7 @@ def test_room_count_for_user(self) -> None: self.reactor.advance(60 * 60 * 48 * 1000) channel = self.make_request( "GET", - f"/_synapse/admin/v1/user/room_count/{self.bad_user}", + f"/_synapse/admin/v1/users/room_count/{self.bad_user}", access_token=self.admin_tok, ) self.assertEqual(channel.code, 200) @@ -5617,7 +5617,7 @@ def test_room_count_for_user(self) -> None: channel = self.make_request( "GET", - f"/_synapse/admin/v1/user/room_count/{self.bad_user}", + f"/_synapse/admin/v1/users/room_count/{self.bad_user}", access_token=self.admin_tok, ) self.assertEqual(channel.code, 200) From 0a1f5afde973fb195c3b4fa3bd696406cdd42cb3 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Thu, 21 Nov 2024 11:32:12 -0800 Subject: [PATCH 08/40] expand event reports API to filter for the sender of a reported event --- docs/admin_api/event_reports.md | 32 +---- docs/admin_api/user_admin_api.md | 2 +- synapse/rest/admin/__init__.py | 2 - synapse/rest/admin/event_reports.py | 38 +----- synapse/storage/databases/main/room.py | 71 ++--------- tests/rest/admin/test_event_reports.py | 167 +++++-------------------- 6 files changed, 46 insertions(+), 266 deletions(-) diff --git a/docs/admin_api/event_reports.md b/docs/admin_api/event_reports.md index 7ca297cc8a6..6a250a1662e 100644 --- a/docs/admin_api/event_reports.md +++ b/docs/admin_api/event_reports.md @@ -64,6 +64,8 @@ paginate through. contain this value. This is the user who reported the event and wrote the reason. * `room_id`: string - Is optional and filters to only return rooms with room IDs that contain this value. +* `sender_user_id`: string - Is optional and filters to only return event reports against the provided + user (i.e. where the user is the reported event's sender) **Response** @@ -183,33 +185,3 @@ DELETE /_synapse/admin/v1/event_reports/ **URL parameters:** * `report_id`: string - The ID of the event report. - - -# Fetch the report IDs for a specific user - -Fetches the report IDs where the sender of the reported event is the provided user ID. - -The api is: -``` -GET /_synapse/admin/v1/event_reports/user/$user_id -``` - -**URL paramters** - -* `user_id`: string - Required. The ID of the user to search for -* `limit`: integer - Is optional but is used for pagination, denoting the maximum number - of items to return in this call. Defaults to `100`. -* `from`: integer - Is optional but used for pagination, denoting the offset in the - returned results. This should be treated as an opaque value and not explicitly set to - anything other than the return value of `next_token` from a previous call. Defaults to `0`. -* `dir`: string - Direction of event report order. Whether to fetch the most recent - first (`b`) or the oldest first (`f`). Defaults to `b`. - -A response body like the following is returned: -``` - { - "report_ids": [2, 3, 4] - } -``` - -_Added in Synapse 1.120.0_ \ 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 639f8aba6ed..a8c6cc268f7 100644 --- a/docs/admin_api/user_admin_api.md +++ b/docs/admin_api/user_admin_api.md @@ -1448,7 +1448,7 @@ _Added in Synapse 1.116.0._ ## Get the number of invites sent by the user in the last 24 hours -Fetches the count of the number of invites sent by the provided user ID across all rooms in the last 24 hours. +Fetches the number of invites sent by the provided user ID across all rooms in the last 24 hours. ``` GET /_synapse/admin/v1/users/invite_count/$user_id diff --git a/synapse/rest/admin/__init__.py b/synapse/rest/admin/__init__.py index 47ff19f72b1..85f4daa6d3b 100644 --- a/synapse/rest/admin/__init__.py +++ b/synapse/rest/admin/__init__.py @@ -54,7 +54,6 @@ DevicesRestServlet, ) from synapse.rest.admin.event_reports import ( - EventReportAgainstUserRestServlet, EventReportDetailRestServlet, EventReportsRestServlet, ) @@ -301,7 +300,6 @@ def register_servlets(hs: "HomeServer", http_server: HttpServer) -> None: UsersRestServletV3(hs).register(http_server) UserMediaStatisticsRestServlet(hs).register(http_server) LargestRoomsStatistics(hs).register(http_server) - EventReportAgainstUserRestServlet(hs).register(http_server) EventReportDetailRestServlet(hs).register(http_server) EventReportsRestServlet(hs).register(http_server) AccountDataRestServlet(hs).register(http_server) diff --git a/synapse/rest/admin/event_reports.py b/synapse/rest/admin/event_reports.py index a6723b097b2..136878c5f09 100644 --- a/synapse/rest/admin/event_reports.py +++ b/synapse/rest/admin/event_reports.py @@ -50,8 +50,9 @@ class EventReportsRestServlet(RestServlet): The parameters `from` and `limit` are required only for pagination. By default, a `limit` of 100 is used. The parameter `dir` can be used to define the order of results. - The parameter `user_id` can be used to filter by user id. + The parameter `user_id` can be used to filter the user id of the reporter of the event. The parameter `room_id` can be used to filter by room id. + The parameter `sender_user_id` can be used to filter by the user id of the sender of the reported event. Returns: A list of reported events and an integer representing the total number of reported events that exist given this query @@ -71,6 +72,7 @@ async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]: direction = parse_enum(request, "dir", Direction, Direction.BACKWARDS) user_id = parse_string(request, "user_id") room_id = parse_string(request, "room_id") + sender_user_id = parse_string(request, "sender_user_id") if start < 0: raise SynapseError( @@ -87,7 +89,7 @@ async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]: ) event_reports, total = await self._store.get_event_reports_paginate( - start, limit, direction, user_id, room_id + start, limit, direction, user_id, room_id, sender_user_id ) ret = {"event_reports": event_reports, "total": total} if (start + limit) < total: @@ -168,35 +170,3 @@ async def on_DELETE( return HTTPStatus.OK, {} raise NotFoundError("Event report not found") - - -class EventReportAgainstUserRestServlet(RestServlet): - """ - Get all the event reports where the original sender of the reported event is the provided user - - GET /_synapse/admin/v1/event_reports/user/ - - """ - - PATTERNS = admin_patterns("/event_reports/user/(?P[^/]*)$") - - def __init__(self, hs: "HomeServer"): - self._auth = hs.get_auth() - self._store = hs.get_datastores().main - - async def on_GET( - self, request: SynapseRequest, user_id: str - ) -> Tuple[HTTPStatus, dict[str, list[str]]]: - await assert_requester_is_admin(self._auth, request) - - start = parse_integer(request, "start", default=0) - limit = parse_integer(request, "limit", default=100) - direction = parse_enum(request, "dir", Direction, Direction.BACKWARDS) - - res = await self._store.get_event_report_IDs_by_sender( - user_id, start, limit, direction - ) - if not res: - raise NotFoundError("User ID not found") - - return HTTPStatus.OK, {"report_ids": res} diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index f1961b7565a..7f3d2f0786a 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -1518,69 +1518,6 @@ def get_un_partial_stated_rooms_from_stream_txn( get_un_partial_stated_rooms_from_stream_txn, ) - async def get_event_report_IDs_by_sender( - self, - sender_id: str, - start: int, - limit: int, - direction: Direction = Direction.BACKWARDS, - ) -> Optional[List[str]]: - """ - Get all the event reports IDs where the sender is the provided user id - - Args: - sender_id: The user_id of the sender of the reported event - start: event offset to begin the query from - limit: number of rows to retrieve - direction: Whether to fetch the most recent first (backwards) or the - oldest first (forwards) - """ - - def _get_event_report_IDs_txn( - txn: LoggingTransaction, - sender_id: str, - start: int, - limit: int, - direction: Direction, - ) -> Optional[List[str]]: - if direction == Direction.BACKWARDS: - order = "DESC" - else: - order = "ASC" - - sql = f""" - SELECT - er.id - FROM event_reports AS er - LEFT JOIN events - ON events.event_id = er.event_id - WHERE events.sender = ? - ORDER BY er.received_ts {order} - LIMIT ? - OFFSET ? - """ - - txn.execute(sql, (sender_id, limit, start)) - rows = txn.fetchall() - - if not rows: - return None - - report_ids = [] - for r in rows: - report_ids.append(r[0]) - - return report_ids - - return await self.db_pool.runInteraction( - "get_event_report_IDs_by_sender", - _get_event_report_IDs_txn, - sender_id, - start, - limit, - direction, - ) - async def get_event_report(self, report_id: int) -> Optional[Dict[str, Any]]: """Retrieve an event report @@ -1649,6 +1586,7 @@ async def get_event_reports_paginate( direction: Direction = Direction.BACKWARDS, user_id: Optional[str] = None, room_id: Optional[str] = None, + sender_user_id: Optional[str] = None, ) -> Tuple[List[Dict[str, Any]], int]: """Retrieve a paginated list of event reports @@ -1659,6 +1597,7 @@ async def get_event_reports_paginate( oldest first (forwards) user_id: search for user_id. Ignored if user_id is None room_id: search for room_id. Ignored if room_id is None + sender_user_id: search for the sender of the reported event. Ignored if sender_user_id is None Returns: Tuple of: json list of event reports @@ -1678,6 +1617,10 @@ def _get_event_reports_paginate_txn( filters.append("er.room_id LIKE ?") args.extend(["%" + room_id + "%"]) + if sender_user_id: + filters.append("events.sender LIKE ?") + args.extend(["%" + sender_user_id + "%"]) + if direction == Direction.BACKWARDS: order = "DESC" else: @@ -1693,6 +1636,8 @@ def _get_event_reports_paginate_txn( sql = """ SELECT COUNT(*) as total_event_reports FROM event_reports AS er + LEFT JOIN events + ON events.event_id = er.event_id JOIN room_stats_state ON room_stats_state.room_id = er.room_id {} """.format(where_clause) diff --git a/tests/rest/admin/test_event_reports.py b/tests/rest/admin/test_event_reports.py index bdb3196ef28..4fb957b2117 100644 --- a/tests/rest/admin/test_event_reports.py +++ b/tests/rest/admin/test_event_reports.py @@ -378,6 +378,37 @@ def test_next_token(self) -> None: self.assertEqual(len(channel.json_body["event_reports"]), 1) self.assertNotIn("next_token", channel.json_body) + def test_filter_against_user(self) -> None: + # first grab all the reports + channel = self.make_request( + "GET", + self.url, + access_token=self.admin_user_tok, + ) + self.assertEqual(channel.code, 200) + + # filter out set of report ids of events sent by one of the users + filtered_report_ids = [] + for event_report in channel.json_body["event_reports"]: + if event_report["sender"] == self.other_user: + filtered_report_ids.append(event_report["id"]) + + # grab the report ids by sender and compare to filtered report ids + channel = self.make_request( + "GET", + f"{self.url}?sender_user_id={self.other_user}", + access_token=self.admin_user_tok, + ) + self.assertEqual(200, channel.code) + self.assertEqual(channel.json_body["total"], len(filtered_report_ids)) + + event_reports = channel.json_body["event_reports"] + returned_report_ids = [] + for event_report in event_reports: + if event_report["sender"] == self.other_user: + returned_report_ids.append(event_report["id"]) + self.assertEqual(True, set(filtered_report_ids) == set(returned_report_ids)) + def _create_event_and_report(self, room_id: str, user_tok: str) -> None: """Create and report events""" resp = self.helper.send(room_id, tok=user_tok) @@ -745,139 +776,3 @@ def test_report_id_not_found(self) -> None: self.assertEqual(404, channel.code, msg=channel.json_body) self.assertEqual(Codes.NOT_FOUND, channel.json_body["errcode"]) self.assertEqual("Event report not found", channel.json_body["error"]) - - -class EventReportsAgainstUserTestCase(unittest.HomeserverTestCase): - servlets = [ - synapse.rest.admin.register_servlets, - login.register_servlets, - room.register_servlets, - reporting.register_servlets, - ] - - def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: - self.admin_user = self.register_user("admin", "pass", admin=True) - self.admin_user_tok = self.login("admin", "pass") - - self.bad_user_1 = self.register_user("user", "pass") - self.bad_user_1_tok = self.login("user", "pass") - - self.bad_user_2 = self.register_user("user2", "pass") - self.bad_user_2_tok = self.login("user2", "pass") - - self.room_id1 = self.helper.create_room_as( - self.bad_user_1, tok=self.bad_user_1_tok, is_public=True - ) - self.helper.join(self.room_id1, user=self.admin_user, tok=self.admin_user_tok) - self.helper.join(self.room_id1, user=self.bad_user_2, tok=self.bad_user_2_tok) - - self.room_id2 = self.helper.create_room_as( - self.bad_user_1, tok=self.bad_user_1_tok, is_public=True - ) - self.helper.join(self.room_id2, user=self.admin_user, tok=self.admin_user_tok) - self.helper.join(self.room_id2, user=self.bad_user_2, tok=self.bad_user_2_tok) - - for _ in range(5): - self._create_event_and_report( - room_id=self.room_id1, - sender_tok=self.bad_user_1_tok, - reporter_tok=self.admin_user_tok, - ) - for _ in range(5): - self._create_event_and_report( - room_id=self.room_id2, - sender_tok=self.bad_user_2_tok, - reporter_tok=self.admin_user_tok, - ) - - self.url = "/_synapse/admin/v1/event_reports" - - def _create_event_and_report( - self, room_id: str, sender_tok: str, reporter_tok: str - ) -> None: - """Create and report events""" - resp = self.helper.send(room_id, tok=sender_tok) - event_id = resp["event_id"] - - channel = self.make_request( - "POST", - "rooms/%s/report/%s" % (room_id, event_id), - {"score": -100, "reason": "this makes me sad"}, - access_token=reporter_tok, - ) - self.assertEqual(200, channel.code, msg=channel.json_body) - - def test_get_reports_against_user_no_params(self) -> None: - # first grab all the reports - channel = self.make_request( - "GET", - self.url, - access_token=self.admin_user_tok, - ) - self.assertEqual(channel.code, 200) - self.assertEqual(len(channel.json_body["event_reports"]), 10) - - # filter out set of report ids of events sent by bad user - filtered_report_ids = [] - for event_report in channel.json_body["event_reports"]: - if event_report["sender"] == self.bad_user_1: - filtered_report_ids.append(event_report["id"]) - - # grab the report ids by sender and compare to filtered report ids - channel = self.make_request( - "GET", - f"{self.url}/user/{self.bad_user_1}", - access_token=self.admin_user_tok, - ) - self.assertEqual(200, channel.code) - report_ids_by_user = channel.json_body["report_ids"] - self.assertEqual(True, set(filtered_report_ids) == set(report_ids_by_user)) - - def test_get_reports_against_user_params(self) -> None: - # first grab all the reports - channel = self.make_request( - "GET", - self.url, - access_token=self.admin_user_tok, - ) - self.assertEqual(channel.code, 200) - self.assertEqual(len(channel.json_body["event_reports"]), 10) - - # filter out set of report ids of events sent by bad user - filtered_report_ids = [] - for event_report in channel.json_body["event_reports"]: - if event_report["sender"] == self.bad_user_2: - filtered_report_ids.append(event_report["id"]) - - channel = self.make_request( - "GET", - f"{self.url}/user/{self.bad_user_2}?start=2&limit=2", - access_token=self.admin_user_tok, - ) - self.assertEqual(200, channel.code) - report_ids_by_user = channel.json_body["report_ids"] - - # check that limit is respected - self.assertEqual(len(report_ids_by_user), 2) - - # check that offset is respected - self.assertEqual([9, 8], report_ids_by_user) - - # check that response is for correct user - self.assertEqual( - True, set(report_ids_by_user).issubset(set(filtered_report_ids)) - ) - - channel = self.make_request( - "GET", - f"{self.url}/user/{self.bad_user_2}?dir=f", - access_token=self.admin_user_tok, - ) - self.assertEqual(200, channel.code) - oldest_report_ids_by_user = channel.json_body["report_ids"] - - # check reversal param - self.assertEqual([7, 8, 9, 10, 11], oldest_report_ids_by_user) - self.assertEqual( - True, set(oldest_report_ids_by_user).issubset(set(filtered_report_ids)) - ) From 121f7d4d10082baf22403d6ff3a2d88a72889aa1 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Thu, 21 Nov 2024 14:06:41 -0800 Subject: [PATCH 09/40] extend admin api to get joined rooms to filter via optional timestamp --- synapse/rest/admin/users.py | 35 ++----- .../storage/databases/main/events_worker.py | 29 ------ synapse/storage/databases/main/roommember.py | 28 ++++++ tests/rest/admin/test_user.py | 96 +++++++------------ 4 files changed, 72 insertions(+), 116 deletions(-) diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index f92fdea49bc..8d3e847b656 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -983,7 +983,8 @@ async def on_PUT( class UserMembershipRestServlet(RestServlet): """ - Get room list of an user. + Get room list of a user. Optionally get a list of rooms joined after a given timestamp, via + the optional `from_ts` query param. """ PATTERNS = admin_patterns("/users/(?P[^/]*)/joined_rooms$") @@ -997,10 +998,15 @@ async def on_GET( self, request: SynapseRequest, user_id: str ) -> Tuple[int, JsonDict]: await assert_requester_is_admin(self.auth, request) + from_ts = parse_integer(request, "from_ts") - room_ids = await self.store.get_rooms_for_user(user_id) - ret = {"joined_rooms": list(room_ids), "total": len(room_ids)} - return HTTPStatus.OK, ret + if from_ts: + room_ids = await self.store.get_rooms_for_user_by_date(user_id, from_ts) + else: + room_ids = await self.store.get_rooms_for_user(user_id) + rooms_list = {"joined_rooms": list(room_ids), "total": len(room_ids)} + + return HTTPStatus.OK, rooms_list class PushersRestServlet(RestServlet): @@ -1522,24 +1528,3 @@ async def on_GET( res = await self.store.get_invite_count_by_user(user_id) return HTTPStatus.OK, {"invite_count": res} - - -class UserRoomJoinCount(RestServlet): - """ - Return the count of rooms that the user has joined in the last 24 hours - """ - - PATTERNS = admin_patterns("/users/room_count/(?P[^/]*)$") - - def __init__(self, hs: "HomeServer"): - self._auth = hs.get_auth() - self.store = hs.get_datastores().main - - async def on_GET( - self, request: SynapseRequest, user_id: str - ) -> Tuple[int, JsonDict]: - await assert_requester_is_admin(self._auth, request) - - res = await self.store.get_join_count_by_user(user_id) - - return HTTPStatus.OK, {"room_count": res} diff --git a/synapse/storage/databases/main/events_worker.py b/synapse/storage/databases/main/events_worker.py index b13f92fec27..6b35783ccf5 100644 --- a/synapse/storage/databases/main/events_worker.py +++ b/synapse/storage/databases/main/events_worker.py @@ -2607,32 +2607,3 @@ def _get_invite_count_by_user_txn( user_id, timestamp, ) - - async def get_join_count_by_user(self, user_id: str) -> int: - """ - Get the number of rooms the user has joined in the last 24hrs - """ - timestamp = self._clock.time_msec() - (24 * 60 * 60 * 1000) # 24 hours ago - - def _get_join_count_by_user_txn( - txn: LoggingTransaction, user_id: str, timestamp: int - ) -> int: - sql = """ - SELECT COUNT(c.event_id) - FROM current_state_events c - JOIN events e ON c.event_id = e.event_id - WHERE c.membership = 'join' AND c.state_key = ? AND e.received_ts > ? - """ - txn.execute(sql, (user_id, timestamp)) - res = txn.fetchone() - - if res is None: - return 0 - return int(res[0]) - - return await self.db_pool.runInteraction( - "_get_join_count_by_user_txn", - _get_join_count_by_user_txn, - user_id, - timestamp, - ) diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index 4249cf77e55..46bb11d9d4a 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -1572,6 +1572,34 @@ def get_sliding_sync_room_for_user_batch_txn( get_sliding_sync_room_for_user_batch_txn, ) + async def get_rooms_for_user_by_date(self, user_id: str, from_ts: int) -> frozenset: + """ + Fetch a list of rooms that the user has joined since the given timestamp. + + Args: + user_id: user ID of the user to search for + from_ts: a timestamp in ms from the unix epoch at which to begin the search at + """ + + def _get_rooms_for_user_by_join_date_txn( + txn: LoggingTransaction, user_id: str, timestamp: int + ) -> frozenset: + sql = """ + SELECT c.room_id + FROM current_state_events c + JOIN events e ON c.event_id = e.event_id + WHERE c.state_key = ? AND c.membership = 'join' AND e.received_ts > ? + """ + txn.execute(sql, (user_id, from_ts)) + return frozenset([r[0] for r in txn]) + + return await self.db_pool.runInteraction( + "_get_rooms_for_user_by_join_date_txn", + _get_rooms_for_user_by_join_date_txn, + user_id, + from_ts, + ) + class RoomMemberBackgroundUpdateStore(SQLBaseStore): def __init__( diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index 38c54f78a90..ccbdd6c0792 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -3384,6 +3384,40 @@ def test_get_rooms_with_nonlocal_user(self) -> None: self.assertEqual(1, channel.json_body["total"]) self.assertEqual([local_and_remote_room_id], channel.json_body["joined_rooms"]) + def test_from_ts_param(self) -> None: + # Create rooms and join, grab timestamp of room creation + room_creation_timestamp = self.reactor.rightNow + other_user_tok = self.login("user", "pass") + number_rooms = 5 + for _ in range(number_rooms): + self.helper.create_room_as(self.other_user, tok=other_user_tok) + + # advance clock 48 hours + self.reactor.advance(60 * 60 * 48 * 1000) + + # get a timestamp beginning 24 hours ago + current_time = self.reactor.rightNow + from_ts = current_time - (24 * 60 * 60 * 1000) # 24 hours ago + + # Get rooms using this timestamp, there should be none + channel = self.make_request( + "GET", + f"{self.url}?from_ts={int(from_ts)}", + access_token=self.admin_user_tok, + ) + self.assertEqual(200, channel.code, msg=channel.json_body) + self.assertEqual(0, channel.json_body["total"]) + + # fetch rooms with the older timestamp, this should return the rooms + channel = self.make_request( + "GET", + f"{self.url}?from_ts={int(room_creation_timestamp)}", + access_token=self.admin_user_tok, + ) + self.assertEqual(200, channel.code, msg=channel.json_body) + self.assertEqual(number_rooms, channel.json_body["total"]) + self.assertEqual(number_rooms, len(channel.json_body["joined_rooms"])) + class PushersRestTestCase(unittest.HomeserverTestCase): servlets = [ @@ -5560,65 +5594,3 @@ def test_get_user_invite_count_test_case(self) -> None: ) self.assertEqual(channel.code, 200) self.assertEqual(channel.json_body["invite_count"], 4) - - -class GetRoomCountForUserTestCase(unittest.HomeserverTestCase): - servlets = [ - synapse.rest.admin.register_servlets, - login.register_servlets, - admin.register_servlets, - room.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.rooms1 = [] - self.rooms2 = [] - for i in range(10): - rm = self.helper.create_room_as( - self.admin, is_public=True, tok=self.admin_tok - ) - if i % 2 == 0: - self.rooms1.append(rm) - else: - self.rooms2.append(rm) - - def test_room_count_for_user(self) -> None: - # bad user go on a join spree - for rm in self.rooms1: - self.helper.join(rm, self.bad_user, tok=self.bad_user_tok) - - channel = self.make_request( - "GET", - f"/_synapse/admin/v1/users/room_count/{self.bad_user}", - access_token=self.admin_tok, - ) - self.assertEqual(channel.code, 200) - self.assertEqual(channel.json_body["room_count"], 5) - - # advance clock 48 hours and check again, should return 0 - self.reactor.advance(60 * 60 * 48 * 1000) - channel = self.make_request( - "GET", - f"/_synapse/admin/v1/users/room_count/{self.bad_user}", - access_token=self.admin_tok, - ) - self.assertEqual(channel.code, 200) - self.assertEqual(channel.json_body["room_count"], 0) - - # join some more - for rm in self.rooms2: - self.helper.join(rm, self.bad_user, tok=self.bad_user_tok) - - channel = self.make_request( - "GET", - f"/_synapse/admin/v1/users/room_count/{self.bad_user}", - access_token=self.admin_tok, - ) - self.assertEqual(channel.code, 200) - self.assertEqual(channel.json_body["room_count"], 5) From cda34bbebfbb636b4b01f9b5637254e159eef13a Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Thu, 21 Nov 2024 14:41:11 -0800 Subject: [PATCH 10/40] misc cleanup + add an index on events.received_ts --- docs/admin_api/user_admin_api.md | 23 ++++--------------- synapse/rest/admin/__init__.py | 2 -- synapse/rest/admin/users.py | 2 +- .../storage/databases/main/events_worker.py | 9 +++++++- .../delta/88/05_events_received_ts_index.sql | 16 +++++++++++++ tests/rest/admin/test_user.py | 16 ++++++------- 6 files changed, 37 insertions(+), 31 deletions(-) create mode 100644 synapse/storage/schema/main/delta/88/05_events_received_ts_index.sql diff --git a/docs/admin_api/user_admin_api.md b/docs/admin_api/user_admin_api.md index a8c6cc268f7..20677454f42 100644 --- a/docs/admin_api/user_admin_api.md +++ b/docs/admin_api/user_admin_api.md @@ -478,7 +478,8 @@ with a body of: ## List room memberships of a user -Gets a list of all `room_id` that a specific `user_id` is member. +Gets a list of all `room_id` that a specific `user_id` is member. Optionally, get the list of all the rooms +the user has joined after a given time, via the `from_ts` param. The API is: @@ -507,6 +508,7 @@ member are returned. The following parameters should be set in the URL: - `user_id` - fully qualified: for example, `@user:server.com`. +- `from_ts` - int. Optional. A timestamp in ms from the unix epoch - only rooms joined after the provided timestamp will be returned. Note: https://currentmillis.com/ is a useful tool for converting dates into timestamps and vice versa. **Response** @@ -1451,7 +1453,7 @@ _Added in Synapse 1.116.0._ Fetches the number of invites sent by the provided user ID across all rooms in the last 24 hours. ``` -GET /_synapse/admin/v1/users/invite_count/$user_id +GET /_synapse/admin/v1/users/$user_id/invite_count ``` A response body like the following is returned: @@ -1464,20 +1466,3 @@ A response body like the following is returned: _Added in Synapse 1.120.0_ -## Get the number of rooms the user has joined in the last 24 hours - -Fetches the number of rooms the provided user ID has joined in the last 24 hours. - -``` -GET /_synapse/admin/v1/users/room_count/$user_id -``` - -A response body like the following is returned: - -``` -{ - "room_count": 50 -} -``` - -_Added in Synapse 1.120.0_ diff --git a/synapse/rest/admin/__init__.py b/synapse/rest/admin/__init__.py index 85f4daa6d3b..e9abe83c900 100644 --- a/synapse/rest/admin/__init__.py +++ b/synapse/rest/admin/__init__.py @@ -112,7 +112,6 @@ UserRegisterServlet, UserReplaceMasterCrossSigningKeyRestServlet, UserRestServletV2, - UserRoomJoinCount, UsersRestServletV2, UsersRestServletV3, UserTokenRestServlet, @@ -326,7 +325,6 @@ def register_servlets(hs: "HomeServer", http_server: HttpServer) -> None: RedactUser(hs).register(http_server) RedactUserStatus(hs).register(http_server) UserInvitesCount(hs).register(http_server) - UserRoomJoinCount(hs).register(http_server) DeviceRestServlet(hs).register(http_server) DevicesRestServlet(hs).register(http_server) diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index 8d3e847b656..d6be800fa4f 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -1514,7 +1514,7 @@ class UserInvitesCount(RestServlet): Return the count of invites that the user has sent in the last 24 hours """ - PATTERNS = admin_patterns("/users/invite_count/(?P[^/]*)$") + PATTERNS = admin_patterns("/users/(?P[^/]*)/invite_count") def __init__(self, hs: "HomeServer"): self._auth = hs.get_auth() diff --git a/synapse/storage/databases/main/events_worker.py b/synapse/storage/databases/main/events_worker.py index 6b35783ccf5..74fdf539e72 100644 --- a/synapse/storage/databases/main/events_worker.py +++ b/synapse/storage/databases/main/events_worker.py @@ -331,6 +331,13 @@ def get_chain_id_txn(txn: Cursor) -> int: writers=["master"], ) + self.db_pool.updates.register_background_index_update( + update_name="events_received_ts_index", + index_name="received_ts_idx", + table="events", + columns=("received_ts",), + ) + def get_un_partial_stated_events_token(self, instance_name: str) -> int: return ( self._un_partial_stated_events_stream_id_gen.get_current_token_for_writer( @@ -2591,7 +2598,7 @@ def _get_invite_count_by_user_txn( txn: LoggingTransaction, user_id: str, timestamp: int ) -> int: sql = """ - SELECT COUNT(event_id) FROM events WHERE sender = ? AND type = 'm.room.member' AND received_ts > ? AND state_key != ? + SELECT COUNT(event_id) FROM events WHERE sender = ? AND type = 'm.room.member' AND received_ts > ? AND state_key != ? """ txn.execute(sql, (user_id, timestamp, user_id)) diff --git a/synapse/storage/schema/main/delta/88/05_events_received_ts_index.sql b/synapse/storage/schema/main/delta/88/05_events_received_ts_index.sql new file mode 100644 index 00000000000..72d1ef1a204 --- /dev/null +++ b/synapse/storage/schema/main/delta/88/05_events_received_ts_index.sql @@ -0,0 +1,16 @@ +-- +-- This file is licensed under the Affero General Public License (AGPL) version 3. +-- +-- Copyright (C) 2024 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: +-- . + +-- Add an index on events.received_ts to allow for efficient lookup of events by timestamp +INSERT INTO background_updates (ordering, update_name, progress_json) VALUES + (8805, 'events_received_ts_index', '{}'); diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index ccbdd6c0792..a428496a931 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -5555,19 +5555,19 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: for i in range(4): self.random_users.append(self.register_user(f"user{i}", f"pass{i}")) - self.rm1 = self.helper.create_room_as(self.bad_user, tok=self.bad_user_tok) - self.rm2 = self.helper.create_room_as(self.bad_user, tok=self.bad_user_tok) - self.rm3 = self.helper.create_room_as(self.bad_user, tok=self.bad_user_tok) + self.room1 = self.helper.create_room_as(self.bad_user, tok=self.bad_user_tok) + self.room2 = self.helper.create_room_as(self.bad_user, tok=self.bad_user_tok) + self.room3 = self.helper.create_room_as(self.bad_user, tok=self.bad_user_tok) def test_get_user_invite_count_test_case(self) -> None: # bad user send some invites - for rm in [self.rm1, self.rm2]: + for rm in [self.room1, self.room2]: for user in self.random_users: self.helper.invite(rm, self.bad_user, user, tok=self.bad_user_tok) channel = self.make_request( "GET", - f"/_synapse/admin/v1/users/invite_count/{self.bad_user}", + f"/_synapse/admin/v1/users/{self.bad_user}/invite_count", access_token=self.admin_tok, ) self.assertEqual(channel.code, 200) @@ -5577,7 +5577,7 @@ def test_get_user_invite_count_test_case(self) -> None: self.reactor.advance(60 * 60 * 48 * 1000) channel = self.make_request( "GET", - f"/_synapse/admin/v1/users/invite_count/{self.bad_user}", + f"/_synapse/admin/v1/users/{self.bad_user}/invite_count", access_token=self.admin_tok, ) self.assertEqual(channel.code, 200) @@ -5585,11 +5585,11 @@ def test_get_user_invite_count_test_case(self) -> None: # send some more invites, they should show up for user in self.random_users: - self.helper.invite(self.rm3, self.bad_user, user, tok=self.bad_user_tok) + self.helper.invite(self.room3, self.bad_user, user, tok=self.bad_user_tok) channel = self.make_request( "GET", - f"/_synapse/admin/v1/users/invite_count/{self.bad_user}", + f"/_synapse/admin/v1/users/{self.bad_user}/invite_count", access_token=self.admin_tok, ) self.assertEqual(channel.code, 200) From e2427fd5ac92bd2778201dd83dde613f423dd860 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Thu, 21 Nov 2024 14:51:07 -0800 Subject: [PATCH 11/40] clarify timestamp --- synapse/storage/databases/main/events_worker.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/events_worker.py b/synapse/storage/databases/main/events_worker.py index 74fdf539e72..03f9070047a 100644 --- a/synapse/storage/databases/main/events_worker.py +++ b/synapse/storage/databases/main/events_worker.py @@ -104,6 +104,9 @@ logger = logging.getLogger(__name__) +ONE_HOUR_MS = 60 * 60 * 1000 +ONE_DAY_MS = 24 * ONE_HOUR_MS + class DatabaseCorruptionError(RuntimeError): """We found an event in the DB that has a persisted event ID that doesn't @@ -2592,7 +2595,7 @@ async def get_invite_count_by_user(self, user_id: str) -> int: """ Get the number of invites sent by the given user in the past 24 hours """ - timestamp = self._clock.time_msec() - (24 * 60 * 60 * 1000) # 24 hours ago + timestamp = self._clock.time_msec() - ONE_DAY_MS # 24 hours ago def _get_invite_count_by_user_txn( txn: LoggingTransaction, user_id: str, timestamp: int From dbd120ea378cfa4d3fae51d9641740c04b9ff375 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Mon, 2 Dec 2024 13:34:07 -0800 Subject: [PATCH 12/40] revise docs --- docs/admin_api/event_reports.md | 2 +- docs/admin_api/user_admin_api.md | 52 ++++++++++++------- ...ex.sql => 06_events_received_ts_index.sql} | 0 3 files changed, 34 insertions(+), 20 deletions(-) rename synapse/storage/schema/main/delta/88/{05_events_received_ts_index.sql => 06_events_received_ts_index.sql} (100%) diff --git a/docs/admin_api/event_reports.md b/docs/admin_api/event_reports.md index 6a250a1662e..5d1844b59fa 100644 --- a/docs/admin_api/event_reports.md +++ b/docs/admin_api/event_reports.md @@ -64,7 +64,7 @@ paginate through. contain this value. This is the user who reported the event and wrote the reason. * `room_id`: string - Is optional and filters to only return rooms with room IDs that contain this value. -* `sender_user_id`: string - Is optional and filters to only return event reports against the provided +* `event_sender_user_id`: string - Is optional and filters to only return event reports against the provided user (i.e. where the user is the reported event's sender) **Response** diff --git a/docs/admin_api/user_admin_api.md b/docs/admin_api/user_admin_api.md index 20677454f42..bc7a3cb7f85 100644 --- a/docs/admin_api/user_admin_api.md +++ b/docs/admin_api/user_admin_api.md @@ -508,7 +508,10 @@ member are returned. The following parameters should be set in the URL: - `user_id` - fully qualified: for example, `@user:server.com`. -- `from_ts` - int. Optional. A timestamp in ms from the unix epoch - only rooms joined after the provided timestamp will be returned. Note: https://currentmillis.com/ is a useful tool for converting dates into timestamps and vice versa. +- `from_ts` - int. Optional. A timestamp in ms from the unix + epoch - only rooms joined after the provided timestamp will be returned. + Note: https://currentmillis.com/ is a useful tool for converting dates + into timestamps and vice versa. **Response** @@ -517,6 +520,35 @@ The following fields are returned in the JSON response body: - `joined_rooms` - An array of `room_id`. - `total` - Number of rooms. +## Get the number of invites sent by the user + +Fetches the number of invites sent by the provided user ID across all rooms +after the given timestamp. + +``` +GET /_synapse/admin/v1/users/$user_id/sent_invite_count +``` + +**Parameters** + +The following parameters should be set in the URL: + +* `from_ts`: int. A timestamp in ms from the unix epoch - only + invites sent after the provided timestamp will be returned. + Note: https://currentmillis.com/ is a useful tool for converting dates + into timestamps and vice versa. + + +A response body like the following is returned: + +``` +{ + "invite_count": 30 +} +``` + +_Added in Synapse 1.121.0_ + ## Account Data Gets information about account data for a specific `user_id`. @@ -1448,21 +1480,3 @@ The following fields are returned in the JSON response body: _Added in Synapse 1.116.0._ -## Get the number of invites sent by the user in the last 24 hours - -Fetches the number of invites sent by the provided user ID across all rooms in the last 24 hours. - -``` -GET /_synapse/admin/v1/users/$user_id/invite_count -``` - -A response body like the following is returned: - -``` -{ - "invite_count": 30 -} -``` - -_Added in Synapse 1.120.0_ - diff --git a/synapse/storage/schema/main/delta/88/05_events_received_ts_index.sql b/synapse/storage/schema/main/delta/88/06_events_received_ts_index.sql similarity index 100% rename from synapse/storage/schema/main/delta/88/05_events_received_ts_index.sql rename to synapse/storage/schema/main/delta/88/06_events_received_ts_index.sql From 4af03879502c3cf0e444f8f8b1a10c498533d58e Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Mon, 2 Dec 2024 13:34:25 -0800 Subject: [PATCH 13/40] requested changes --- synapse/rest/admin/event_reports.py | 6 +++--- synapse/rest/admin/users.py | 7 ++++--- .../storage/databases/main/events_worker.py | 21 ++++++++++++------- synapse/storage/databases/main/room.py | 10 ++++----- synapse/storage/databases/main/roommember.py | 2 +- 5 files changed, 26 insertions(+), 20 deletions(-) diff --git a/synapse/rest/admin/event_reports.py b/synapse/rest/admin/event_reports.py index 136878c5f09..da033aaeb77 100644 --- a/synapse/rest/admin/event_reports.py +++ b/synapse/rest/admin/event_reports.py @@ -52,7 +52,7 @@ class EventReportsRestServlet(RestServlet): The parameter `dir` can be used to define the order of results. The parameter `user_id` can be used to filter the user id of the reporter of the event. The parameter `room_id` can be used to filter by room id. - The parameter `sender_user_id` can be used to filter by the user id of the sender of the reported event. + The parameter `event_sender_user_id` can be used to filter by the user id of the sender of the reported event. Returns: A list of reported events and an integer representing the total number of reported events that exist given this query @@ -72,7 +72,7 @@ async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]: direction = parse_enum(request, "dir", Direction, Direction.BACKWARDS) user_id = parse_string(request, "user_id") room_id = parse_string(request, "room_id") - sender_user_id = parse_string(request, "sender_user_id") + event_sender_user_id = parse_string(request, "event_sender_user_id") if start < 0: raise SynapseError( @@ -89,7 +89,7 @@ async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]: ) event_reports, total = await self._store.get_event_reports_paginate( - start, limit, direction, user_id, room_id, sender_user_id + start, limit, direction, user_id, room_id, event_sender_user_id ) ret = {"event_reports": event_reports, "total": total} if (start + limit) < total: diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index d6be800fa4f..c167abee85c 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -1511,10 +1511,10 @@ async def on_GET( class UserInvitesCount(RestServlet): """ - Return the count of invites that the user has sent in the last 24 hours + Return the count of invites that the user has sent after the given timestamp """ - PATTERNS = admin_patterns("/users/(?P[^/]*)/invite_count") + PATTERNS = admin_patterns("/users/(?P[^/]*)/sent_invite_count") def __init__(self, hs: "HomeServer"): self._auth = hs.get_auth() @@ -1524,7 +1524,8 @@ async def on_GET( self, request: SynapseRequest, user_id: str ) -> Tuple[int, JsonDict]: await assert_requester_is_admin(self._auth, request) + from_ts = parse_integer(request, "from_ts", required=True) - res = await self.store.get_invite_count_by_user(user_id) + res = await self.store.get_invite_count_by_user(user_id, from_ts) return HTTPStatus.OK, {"invite_count": res} diff --git a/synapse/storage/databases/main/events_worker.py b/synapse/storage/databases/main/events_worker.py index 03f9070047a..f3b25d203cd 100644 --- a/synapse/storage/databases/main/events_worker.py +++ b/synapse/storage/databases/main/events_worker.py @@ -339,6 +339,7 @@ def get_chain_id_txn(txn: Cursor) -> int: index_name="received_ts_idx", table="events", columns=("received_ts",), + where_clause="type = 'm.room.member'", ) def get_un_partial_stated_events_token(self, instance_name: str) -> int: @@ -2591,20 +2592,24 @@ async def have_finished_sliding_sync_background_jobs(self) -> bool: ) ) - async def get_invite_count_by_user(self, user_id: str) -> int: + async def get_invite_count_by_user(self, user_id: str, from_ts: int) -> int: """ - Get the number of invites sent by the given user in the past 24 hours + Get the number of invites sent by the given user after the provided timestamp. + + Args: + user_id: user ID to search against + from_ts: a timestamp in milliseconds from the unix epoch + """ - timestamp = self._clock.time_msec() - ONE_DAY_MS # 24 hours ago def _get_invite_count_by_user_txn( - txn: LoggingTransaction, user_id: str, timestamp: int + txn: LoggingTransaction, user_id: str, from_ts: int ) -> int: sql = """ - SELECT COUNT(event_id) FROM events WHERE sender = ? AND type = 'm.room.member' AND received_ts > ? AND state_key != ? - """ + SELECT COUNT(e.event_id) FROM events e JOIN room_memberships rm ON e.event_id = rm.event_id WHERE e.sender = ? AND e.received_ts > ? AND e.state_key != ? AND rm.membership = 'invite' + """ - txn.execute(sql, (user_id, timestamp, user_id)) + txn.execute(sql, (user_id, from_ts, user_id)) res = txn.fetchone() if res is None: @@ -2615,5 +2620,5 @@ def _get_invite_count_by_user_txn( "_get_invite_count_by_user_txn", _get_invite_count_by_user_txn, user_id, - timestamp, + from_ts, ) diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index 7f3d2f0786a..c15cf5c2132 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -1586,7 +1586,7 @@ async def get_event_reports_paginate( direction: Direction = Direction.BACKWARDS, user_id: Optional[str] = None, room_id: Optional[str] = None, - sender_user_id: Optional[str] = None, + event_sender_user_id: Optional[str] = None, ) -> Tuple[List[Dict[str, Any]], int]: """Retrieve a paginated list of event reports @@ -1597,7 +1597,7 @@ async def get_event_reports_paginate( oldest first (forwards) user_id: search for user_id. Ignored if user_id is None room_id: search for room_id. Ignored if room_id is None - sender_user_id: search for the sender of the reported event. Ignored if sender_user_id is None + event_sender_user_id: search for the sender of the reported event. Ignored if sender_user_id is None Returns: Tuple of: json list of event reports @@ -1617,9 +1617,9 @@ def _get_event_reports_paginate_txn( filters.append("er.room_id LIKE ?") args.extend(["%" + room_id + "%"]) - if sender_user_id: - filters.append("events.sender LIKE ?") - args.extend(["%" + sender_user_id + "%"]) + if event_sender_user_id: + filters.append("events.sender = ?") + args.extend([event_sender_user_id]) if direction == Direction.BACKWARDS: order = "DESC" diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index 46bb11d9d4a..f99db05e016 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -1588,7 +1588,7 @@ def _get_rooms_for_user_by_join_date_txn( SELECT c.room_id FROM current_state_events c JOIN events e ON c.event_id = e.event_id - WHERE c.state_key = ? AND c.membership = 'join' AND e.received_ts > ? + WHERE c.state_key = ? AND c.membership = 'join' AND e.received_ts > ? AND c.type = 'm.room.member' """ txn.execute(sql, (user_id, from_ts)) return frozenset([r[0] for r in txn]) From 04a3ab7bd78d34aa45906bd23853123ca79e9441 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Mon, 2 Dec 2024 13:34:35 -0800 Subject: [PATCH 14/40] update tests to reflect changes --- tests/rest/admin/test_event_reports.py | 10 ++-- tests/rest/admin/test_user.py | 77 ++++++++++++++++++++------ 2 files changed, 64 insertions(+), 23 deletions(-) diff --git a/tests/rest/admin/test_event_reports.py b/tests/rest/admin/test_event_reports.py index 4fb957b2117..bab5a65ff8c 100644 --- a/tests/rest/admin/test_event_reports.py +++ b/tests/rest/admin/test_event_reports.py @@ -378,7 +378,7 @@ def test_next_token(self) -> None: self.assertEqual(len(channel.json_body["event_reports"]), 1) self.assertNotIn("next_token", channel.json_body) - def test_filter_against_user(self) -> None: + def test_filter_against_event_sender(self) -> None: # first grab all the reports channel = self.make_request( "GET", @@ -396,18 +396,18 @@ def test_filter_against_user(self) -> None: # grab the report ids by sender and compare to filtered report ids channel = self.make_request( "GET", - f"{self.url}?sender_user_id={self.other_user}", + f"{self.url}?event_sender_user_id={self.other_user}", access_token=self.admin_user_tok, ) self.assertEqual(200, channel.code) self.assertEqual(channel.json_body["total"], len(filtered_report_ids)) event_reports = channel.json_body["event_reports"] - returned_report_ids = [] + returned_report_ids = set() for event_report in event_reports: if event_report["sender"] == self.other_user: - returned_report_ids.append(event_report["id"]) - self.assertEqual(True, set(filtered_report_ids) == set(returned_report_ids)) + returned_report_ids.add(event_report["id"]) + self.assertEqual(True, set(filtered_report_ids) == returned_report_ids) def _create_event_and_report(self, room_id: str, user_tok: str) -> None: """Create and report events""" diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index a428496a931..f4e15f44832 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -61,6 +61,9 @@ from tests.test_utils import SMALL_PNG from tests.unittest import override_config +ONE_HOUR_MS = 60 * 60 * 1000 +ONE_DAY_MS = 24 * ONE_HOUR_MS + class UserRegisterTestCase(unittest.HomeserverTestCase): servlets = [ @@ -3386,20 +3389,21 @@ def test_get_rooms_with_nonlocal_user(self) -> None: def test_from_ts_param(self) -> None: # Create rooms and join, grab timestamp of room creation - room_creation_timestamp = self.reactor.rightNow + room_creation_timestamp = self.hs.get_clock().time_msec() other_user_tok = self.login("user", "pass") number_rooms = 5 for _ in range(number_rooms): self.helper.create_room_as(self.other_user, tok=other_user_tok) # advance clock 48 hours - self.reactor.advance(60 * 60 * 48 * 1000) + self.reactor.advance(ONE_DAY_MS * 2) # get a timestamp beginning 24 hours ago - current_time = self.reactor.rightNow - from_ts = current_time - (24 * 60 * 60 * 1000) # 24 hours ago + current_time = self.hs.get_clock().time_msec() + from_ts = current_time - (ONE_DAY_MS) - # Get rooms using this timestamp, there should be none + # Get rooms using this timestamp, there should be none since all rooms were created + # outside of the `from_ts` 24 hour range channel = self.make_request( "GET", f"{self.url}?from_ts={int(from_ts)}", @@ -3408,7 +3412,8 @@ def test_from_ts_param(self) -> None: self.assertEqual(200, channel.code, msg=channel.json_body) self.assertEqual(0, channel.json_body["total"]) - # fetch rooms with the older timestamp, this should return the rooms + # fetch rooms with the older timestamp before they were created, this should + # return the rooms channel = self.make_request( "GET", f"{self.url}?from_ts={int(room_creation_timestamp)}", @@ -5551,6 +5556,9 @@ 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.to_kick = self.register_user("kick_me", "pass") + self.to_kick_tok = self.login("kick_me", "pass") + self.random_users = [] for i in range(4): self.random_users.append(self.register_user(f"user{i}", f"pass{i}")) @@ -5559,38 +5567,71 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: self.room2 = self.helper.create_room_as(self.bad_user, tok=self.bad_user_tok) self.room3 = self.helper.create_room_as(self.bad_user, tok=self.bad_user_tok) + self.helper.join(self.room1, self.to_kick, tok=self.to_kick_tok) + def test_get_user_invite_count_test_case(self) -> None: - # bad user send some invites - for rm in [self.room1, self.room2]: + # grab a current timestamp + invites_sent_ts = self.hs.get_clock().time_msec() + + # bad user sends some invites + for room_id in [self.room1, self.room2]: for user in self.random_users: - self.helper.invite(rm, self.bad_user, user, tok=self.bad_user_tok) + self.helper.invite(room_id, self.bad_user, user, tok=self.bad_user_tok) + # advance clock 48 hours + self.reactor.advance(ONE_DAY_MS * 2) + + # get a timestamp beginning 24 hours ago + current_time = self.hs.get_clock().time_msec() + from_ts = current_time - (ONE_DAY_MS) + + # fetch invites with timestamp, none should be returned channel = self.make_request( "GET", - f"/_synapse/admin/v1/users/{self.bad_user}/invite_count", + f"/_synapse/admin/v1/users/{self.bad_user}/sent_invite_count?from_ts={from_ts}", access_token=self.admin_tok, ) self.assertEqual(channel.code, 200) - self.assertEqual(channel.json_body["invite_count"], 8) + self.assertEqual(channel.json_body["invite_count"], 0) - # advance clock 48 hours and check again, should return 0 - self.reactor.advance(60 * 60 * 48 * 1000) + # fetch using original timestamp, all should be returned channel = self.make_request( "GET", - f"/_synapse/admin/v1/users/{self.bad_user}/invite_count", + f"/_synapse/admin/v1/users/{self.bad_user}/sent_invite_count?from_ts={invites_sent_ts}", access_token=self.admin_tok, ) self.assertEqual(channel.code, 200) - self.assertEqual(channel.json_body["invite_count"], 0) + self.assertEqual(channel.json_body["invite_count"], 8) - # send some more invites, they should show up + # send some more invites, they should show up in addition to original 8 using original ts for user in self.random_users: self.helper.invite(self.room3, self.bad_user, user, tok=self.bad_user_tok) channel = self.make_request( "GET", - f"/_synapse/admin/v1/users/{self.bad_user}/invite_count", + f"/_synapse/admin/v1/users/{self.bad_user}/sent_invite_count?from_ts={invites_sent_ts}", + access_token=self.admin_tok, + ) + self.assertEqual(channel.code, 200) + self.assertEqual(channel.json_body["invite_count"], 12) + + # send a kick and some bans and make sure these aren't counted against invite total + for user in self.random_users: + self.helper.ban(self.room1, self.bad_user, user, tok=self.bad_user_tok) + + channel = self.make_request( + "POST", + f"/_matrix/client/v3/rooms/{self.room1}/kick", + content={"user_id": f"{self.to_kick}"}, + access_token=self.bad_user_tok, + shorthand=False, + ) + self.assertEqual(channel.code, 200) + + channel = self.make_request( + "GET", + f"/_synapse/admin/v1/users/{self.bad_user}/sent_invite_count?from_ts={invites_sent_ts}", access_token=self.admin_tok, ) self.assertEqual(channel.code, 200) - self.assertEqual(channel.json_body["invite_count"], 4) + self.assertEqual(channel.json_body["invite_count"], 12) From 69029a9d452f725fc77251f69e2081309e0d8a2d Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 3 Dec 2024 11:11:00 -0800 Subject: [PATCH 15/40] docs update --- docs/admin_api/event_reports.md | 10 ++++------ docs/admin_api/user_admin_api.md | 15 +++++++++++---- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/docs/admin_api/event_reports.md b/docs/admin_api/event_reports.md index 5d1844b59fa..28e42769892 100644 --- a/docs/admin_api/event_reports.md +++ b/docs/admin_api/event_reports.md @@ -60,12 +60,10 @@ paginate through. anything other than the return value of `next_token` from a previous call. Defaults to `0`. * `dir`: string - Direction of event report order. Whether to fetch the most recent first (`b`) or the oldest first (`f`). Defaults to `b`. -* `user_id`: string - Is optional and filters to only return users with user IDs that - contain this value. This is the user who reported the event and wrote the reason. -* `room_id`: string - Is optional and filters to only return rooms with room IDs that - contain this value. -* `event_sender_user_id`: string - Is optional and filters to only return event reports against the provided - user (i.e. where the user is the reported event's sender) +* `user_id`: optional string - Filter by the user ID of the reporter. This is the user who reported the event + and wrote the reason. +* `room_id`: optional string - Filter by room id. +* `event_sender_user_id`: optional string - Filter by the sender of the reported event. This is the user who the report was made against. **Response** diff --git a/docs/admin_api/user_admin_api.md b/docs/admin_api/user_admin_api.md index bc7a3cb7f85..bc3f1bcc7ff 100644 --- a/docs/admin_api/user_admin_api.md +++ b/docs/admin_api/user_admin_api.md @@ -476,10 +476,9 @@ with a body of: } ``` -## List room memberships of a user +## List joined rooms of a user -Gets a list of all `room_id` that a specific `user_id` is member. Optionally, get the list of all the rooms -the user has joined after a given time, via the `from_ts` param. +Gets a list of all `room_id` that a specific `user_id` is joined to and is a member of (participating in). The API is: @@ -508,6 +507,9 @@ member are returned. The following parameters should be set in the URL: - `user_id` - fully qualified: for example, `@user:server.com`. + +The following parameters should be set as query parameters in the URL: + - `from_ts` - int. Optional. A timestamp in ms from the unix epoch - only rooms joined after the provided timestamp will be returned. Note: https://currentmillis.com/ is a useful tool for converting dates @@ -533,12 +535,17 @@ GET /_synapse/admin/v1/users/$user_id/sent_invite_count The following parameters should be set in the URL: -* `from_ts`: int. A timestamp in ms from the unix epoch - only +* `user_id`: fully qualified: for example, `@user:server.com` + +The following parameters should be set as query parameters in the URL: + +* `from_ts`: int. A timestamp in ms from the unix epoch. Only invites sent after the provided timestamp will be returned. Note: https://currentmillis.com/ is a useful tool for converting dates into timestamps and vice versa. + A response body like the following is returned: ``` From 40da1b41720288c04b2a5d386364c2db5bb65712 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 3 Dec 2024 11:11:15 -0800 Subject: [PATCH 16/40] docstring update --- synapse/rest/admin/event_reports.py | 7 ++++--- synapse/rest/admin/users.py | 3 +-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/synapse/rest/admin/event_reports.py b/synapse/rest/admin/event_reports.py index da033aaeb77..ff1abc0697c 100644 --- a/synapse/rest/admin/event_reports.py +++ b/synapse/rest/admin/event_reports.py @@ -50,9 +50,10 @@ class EventReportsRestServlet(RestServlet): The parameters `from` and `limit` are required only for pagination. By default, a `limit` of 100 is used. The parameter `dir` can be used to define the order of results. - The parameter `user_id` can be used to filter the user id of the reporter of the event. - The parameter `room_id` can be used to filter by room id. - The parameter `event_sender_user_id` can be used to filter by the user id of the sender of the reported event. + The `user_id` query parameter filters by the user ID of the reporter of the event. + The `room_id` query parameter filters by room id. + The `event_sender_user_id` query parameter can be used to filter by the user id + of the sender of the reported event. Returns: A list of reported events and an integer representing the total number of reported events that exist given this query diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index c167abee85c..b1b7e0e0945 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -983,8 +983,7 @@ async def on_PUT( class UserMembershipRestServlet(RestServlet): """ - Get room list of a user. Optionally get a list of rooms joined after a given timestamp, via - the optional `from_ts` query param. + Get list of joined room ID's for a user. """ PATTERNS = admin_patterns("/users/(?P[^/]*)/joined_rooms$") From 574e5c46a7e8f6324c341e3c2ed1e3e5b2226e03 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 3 Dec 2024 11:11:31 -0800 Subject: [PATCH 17/40] clean up quesries --- synapse/storage/databases/main/events_worker.py | 8 ++++++-- synapse/storage/databases/main/room.py | 6 +++--- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/synapse/storage/databases/main/events_worker.py b/synapse/storage/databases/main/events_worker.py index f3b25d203cd..431e41beb3b 100644 --- a/synapse/storage/databases/main/events_worker.py +++ b/synapse/storage/databases/main/events_worker.py @@ -2606,10 +2606,14 @@ def _get_invite_count_by_user_txn( txn: LoggingTransaction, user_id: str, from_ts: int ) -> int: sql = """ - SELECT COUNT(e.event_id) FROM events e JOIN room_memberships rm ON e.event_id = rm.event_id WHERE e.sender = ? AND e.received_ts > ? AND e.state_key != ? AND rm.membership = 'invite' + SELECT COUNT(e.event_id) + FROM events e + INNER JOIN room_memberships rm USING(event_id) + WHERE e.sender = ? AND rm.membership = 'invite' + AND e.type = 'm.room.member' AND e.received_ts > ? """ - txn.execute(sql, (user_id, from_ts, user_id)) + txn.execute(sql, (user_id, from_ts)) res = txn.fetchone() if res is None: diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index c15cf5c2132..55f20a5b4a1 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -1597,7 +1597,8 @@ async def get_event_reports_paginate( oldest first (forwards) user_id: search for user_id. Ignored if user_id is None room_id: search for room_id. Ignored if room_id is None - event_sender_user_id: search for the sender of the reported event. Ignored if sender_user_id is None + event_sender_user_id: search for the sender of the reported event. Ignored if + event_sender_user_id is None Returns: Tuple of: json list of event reports @@ -1636,8 +1637,7 @@ def _get_event_reports_paginate_txn( sql = """ SELECT COUNT(*) as total_event_reports FROM event_reports AS er - LEFT JOIN events - ON events.event_id = er.event_id + LEFT JOIN events USING(event_id) JOIN room_stats_state ON room_stats_state.room_id = er.room_id {} """.format(where_clause) From 52d2905127436b4f95afd95a4c8a2bdfa6546377 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 3 Dec 2024 11:11:39 -0800 Subject: [PATCH 18/40] use set from beginning --- tests/rest/admin/test_event_reports.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/rest/admin/test_event_reports.py b/tests/rest/admin/test_event_reports.py index bab5a65ff8c..986d15ca450 100644 --- a/tests/rest/admin/test_event_reports.py +++ b/tests/rest/admin/test_event_reports.py @@ -388,10 +388,10 @@ def test_filter_against_event_sender(self) -> None: self.assertEqual(channel.code, 200) # filter out set of report ids of events sent by one of the users - filtered_report_ids = [] + filtered_report_ids = set() for event_report in channel.json_body["event_reports"]: if event_report["sender"] == self.other_user: - filtered_report_ids.append(event_report["id"]) + filtered_report_ids.add(event_report["id"]) # grab the report ids by sender and compare to filtered report ids channel = self.make_request( @@ -407,7 +407,7 @@ def test_filter_against_event_sender(self) -> None: for event_report in event_reports: if event_report["sender"] == self.other_user: returned_report_ids.add(event_report["id"]) - self.assertEqual(True, set(filtered_report_ids) == returned_report_ids) + self.assertEqual(True, filtered_report_ids == returned_report_ids) def _create_event_and_report(self, room_id: str, user_tok: str) -> None: """Create and report events""" From 056d49db6551dfbaa2fbddc2754d88b818d49126 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 3 Dec 2024 13:17:21 -0800 Subject: [PATCH 19/40] requested changes --- docs/admin_api/user_admin_api.md | 6 +++--- synapse/storage/databases/main/room.py | 2 +- synapse/storage/databases/main/roommember.py | 3 +++ 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/docs/admin_api/user_admin_api.md b/docs/admin_api/user_admin_api.md index fd072040463..3ea9a206986 100644 --- a/docs/admin_api/user_admin_api.md +++ b/docs/admin_api/user_admin_api.md @@ -509,7 +509,7 @@ The following parameters should be set in the URL: - `user_id` - fully qualified: for example, `@user:server.com`. -The following parameters should be set as query parameters in the URL: +The following should be set as query parameters in the URL: - `from_ts` - int. Optional. A timestamp in ms from the unix epoch - only rooms joined after the provided timestamp will be returned. @@ -538,9 +538,9 @@ The following parameters should be set in the URL: * `user_id`: fully qualified: for example, `@user:server.com` -The following parameters should be set as query parameters in the URL: +The following should be set as query parameters in the URL: -* `from_ts`: int. A timestamp in ms from the unix epoch. Only +* `from_ts`: int, required. A timestamp in ms from the unix epoch. Only invites sent after the provided timestamp will be returned. Note: https://currentmillis.com/ is a useful tool for converting dates into timestamps and vice versa. diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index 55f20a5b4a1..ffb0a85ace0 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -1597,7 +1597,7 @@ async def get_event_reports_paginate( oldest first (forwards) user_id: search for user_id. Ignored if user_id is None room_id: search for room_id. Ignored if room_id is None - event_sender_user_id: search for the sender of the reported event. Ignored if + event_sender_user_id: search for the sender of the reported event. Ignored if event_sender_user_id is None Returns: Tuple of: diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index f99db05e016..0e160149e19 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -1584,6 +1584,9 @@ async def get_rooms_for_user_by_date(self, user_id: str, from_ts: int) -> frozen def _get_rooms_for_user_by_join_date_txn( txn: LoggingTransaction, user_id: str, timestamp: int ) -> frozenset: + # Note that matching on c.type = 'm.room.member' allows us to benefit from + # this index: "current_state_events_member_index" btree (state_key) WHERE type = 'm.room.member'::text + # on the current_state_events_table sql = """ SELECT c.room_id FROM current_state_events c From 5140082c2435a1110f0d5057e549a321159853d3 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 3 Dec 2024 15:17:07 -0800 Subject: [PATCH 20/40] update changelog --- changelog.d/17948.feature | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/changelog.d/17948.feature b/changelog.d/17948.feature index c85c2f60c34..5e278d106bc 100644 --- a/changelog.d/17948.feature +++ b/changelog.d/17948.feature @@ -1,3 +1,3 @@ -Add endpoints to Admin API to fetch the number of invites the provided user has sent in the past 24 hours, -fetch the number of rooms the provided user has joined in the past 24 hours, and get report IDs of event -reports where the provided user was the sender of the reported event. +Add endpoints to Admin API to fetch the number of invites the provided user has after a given timestamp, +fetch the number of rooms the provided user has joined after a given timestamp, and get report IDs of event +reports against a provided user (ie where the user was the sender of the reported event). From b651ed817cbe4058b1a64a54435ebe742866d30f Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 3 Dec 2024 15:17:25 -0800 Subject: [PATCH 21/40] doc rewording --- docs/admin_api/event_reports.md | 3 ++- docs/admin_api/user_admin_api.md | 6 +++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/docs/admin_api/event_reports.md b/docs/admin_api/event_reports.md index 28e42769892..9075e928822 100644 --- a/docs/admin_api/event_reports.md +++ b/docs/admin_api/event_reports.md @@ -63,7 +63,8 @@ paginate through. * `user_id`: optional string - Filter by the user ID of the reporter. This is the user who reported the event and wrote the reason. * `room_id`: optional string - Filter by room id. -* `event_sender_user_id`: optional string - Filter by the sender of the reported event. This is the user who the report was made against. +* `event_sender_user_id`: optional string - Filter by the sender of the reported event. This is the user who + the report was made against. **Response** diff --git a/docs/admin_api/user_admin_api.md b/docs/admin_api/user_admin_api.md index 3ea9a206986..903c6b5b28e 100644 --- a/docs/admin_api/user_admin_api.md +++ b/docs/admin_api/user_admin_api.md @@ -513,6 +513,8 @@ The following should be set as query parameters in the URL: - `from_ts` - int. Optional. A timestamp in ms from the unix epoch - only rooms joined after the provided timestamp will be returned. + This works by comparing the provided timestamp to the `received_ts` + column in the `events` table. Note: https://currentmillis.com/ is a useful tool for converting dates into timestamps and vice versa. @@ -542,6 +544,8 @@ The following should be set as query parameters in the URL: * `from_ts`: int, required. A timestamp in ms from the unix epoch. Only invites sent after the provided timestamp will be returned. + This works by comparing the provided timestamp to the `received_ts` + column in the `events` table. Note: https://currentmillis.com/ is a useful tool for converting dates into timestamps and vice versa. @@ -549,7 +553,7 @@ The following should be set as query parameters in the URL: A response body like the following is returned: -``` +```json { "invite_count": 30 } From dc15f5884ac7a32ef8b7dd4e377a8ee9f3fa3592 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 3 Dec 2024 15:17:51 -0800 Subject: [PATCH 22/40] fix zero check --- synapse/rest/admin/users.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index b1b7e0e0945..d6b72f8535a 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -999,7 +999,7 @@ async def on_GET( await assert_requester_is_admin(self.auth, request) from_ts = parse_integer(request, "from_ts") - if from_ts: + if from_ts is not None: room_ids = await self.store.get_rooms_for_user_by_date(user_id, from_ts) else: room_ids = await self.store.get_rooms_for_user(user_id) From 9e7453f919fe5043d21909052b20332012af36e5 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 3 Dec 2024 15:18:18 -0800 Subject: [PATCH 23/40] clean up querys/docstrings --- synapse/storage/databases/main/events_worker.py | 5 ++++- synapse/storage/databases/main/room.py | 6 ++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/synapse/storage/databases/main/events_worker.py b/synapse/storage/databases/main/events_worker.py index 698ba4f7ce0..54b92406e3b 100644 --- a/synapse/storage/databases/main/events_worker.py +++ b/synapse/storage/databases/main/events_worker.py @@ -342,6 +342,8 @@ def get_chain_id_txn(txn: Cursor) -> int: writers=["master"], ) + # Added to accommodate some queries for the admin API in order to fetch/filter + # membership events by when it was received self.db_pool.updates.register_background_index_update( update_name="events_received_ts_index", index_name="received_ts_idx", @@ -2606,7 +2608,8 @@ async def get_invite_count_by_user(self, user_id: str, from_ts: int) -> int: Args: user_id: user ID to search against - from_ts: a timestamp in milliseconds from the unix epoch + from_ts: a timestamp in milliseconds from the unix epoch. Filters against + `events.received_ts` """ diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index ffb0a85ace0..5029f43bb9a 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -1656,10 +1656,8 @@ def _get_event_reports_paginate_txn( room_stats_state.canonical_alias, room_stats_state.name FROM event_reports AS er - LEFT JOIN events - ON events.event_id = er.event_id - JOIN room_stats_state - ON room_stats_state.room_id = er.room_id + LEFT JOIN events USING(event_id) + JOIN room_stats_state USING(room_id) {where_clause} ORDER BY er.received_ts {order} LIMIT ? From af2b99f9065c9c274de559c308b0428ec71fbd8d Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 3 Dec 2024 15:18:30 -0800 Subject: [PATCH 24/40] use correct delta --- .../schema/main/delta/88/06_events_received_ts_index.sql | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/synapse/storage/schema/main/delta/88/06_events_received_ts_index.sql b/synapse/storage/schema/main/delta/88/06_events_received_ts_index.sql index 72d1ef1a204..d70a4a8dbcb 100644 --- a/synapse/storage/schema/main/delta/88/06_events_received_ts_index.sql +++ b/synapse/storage/schema/main/delta/88/06_events_received_ts_index.sql @@ -11,6 +11,7 @@ -- See the GNU Affero General Public License for more details: -- . --- Add an index on events.received_ts to allow for efficient lookup of events by timestamp +-- Add an index on `events.received_ts` for `m.room.member` events to allow for +-- efficient lookup of events by timestamp in some Admin API's INSERT INTO background_updates (ordering, update_name, progress_json) VALUES - (8805, 'events_received_ts_index', '{}'); + (8806, 'events_received_ts_index', '{}'); From 867669aae1366f06f561f0b172b96e62389d72b3 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 3 Dec 2024 15:18:59 -0800 Subject: [PATCH 25/40] add test descriptions and separate test concerns --- tests/rest/admin/test_event_reports.py | 16 +++--- tests/rest/admin/test_user.py | 78 ++++++++++++++++++++------ 2 files changed, 70 insertions(+), 24 deletions(-) diff --git a/tests/rest/admin/test_event_reports.py b/tests/rest/admin/test_event_reports.py index 986d15ca450..4fe79216b21 100644 --- a/tests/rest/admin/test_event_reports.py +++ b/tests/rest/admin/test_event_reports.py @@ -379,6 +379,9 @@ def test_next_token(self) -> None: self.assertNotIn("next_token", channel.json_body) def test_filter_against_event_sender(self) -> None: + """ + Tests filtering by the sender of the reported event + """ # first grab all the reports channel = self.make_request( "GET", @@ -388,10 +391,10 @@ def test_filter_against_event_sender(self) -> None: self.assertEqual(channel.code, 200) # filter out set of report ids of events sent by one of the users - filtered_report_ids = set() + locally_filtered_report_ids = set() for event_report in channel.json_body["event_reports"]: if event_report["sender"] == self.other_user: - filtered_report_ids.add(event_report["id"]) + locally_filtered_report_ids.add(event_report["id"]) # grab the report ids by sender and compare to filtered report ids channel = self.make_request( @@ -400,14 +403,13 @@ def test_filter_against_event_sender(self) -> None: access_token=self.admin_user_tok, ) self.assertEqual(200, channel.code) - self.assertEqual(channel.json_body["total"], len(filtered_report_ids)) + self.assertEqual(channel.json_body["total"], len(locally_filtered_report_ids)) event_reports = channel.json_body["event_reports"] - returned_report_ids = set() + server_filtered_report_ids = set() for event_report in event_reports: - if event_report["sender"] == self.other_user: - returned_report_ids.add(event_report["id"]) - self.assertEqual(True, filtered_report_ids == returned_report_ids) + server_filtered_report_ids.add(event_report["id"]) + self.assertEqual(True, locally_filtered_report_ids == server_filtered_report_ids) def _create_event_and_report(self, room_id: str, user_tok: str) -> None: """Create and report events""" diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index bfa2411cc4f..4867e515334 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -3390,8 +3390,11 @@ def test_get_rooms_with_nonlocal_user(self) -> None: self.assertEqual([local_and_remote_room_id], channel.json_body["joined_rooms"]) def test_from_ts_param(self) -> None: + """ + Tests filtering joined room results by timestamp + """ # Create rooms and join, grab timestamp of room creation - room_creation_timestamp = self.hs.get_clock().time_msec() + before_room_creation_timestamp = self.hs.get_clock().time_msec() other_user_tok = self.login("user", "pass") number_rooms = 5 for _ in range(number_rooms): @@ -3418,7 +3421,7 @@ def test_from_ts_param(self) -> None: # return the rooms channel = self.make_request( "GET", - f"{self.url}?from_ts={int(room_creation_timestamp)}", + f"{self.url}?from_ts={int(before_room_creation_timestamp)}", access_token=self.admin_user_tok, ) self.assertEqual(200, channel.code, msg=channel.json_body) @@ -5572,7 +5575,10 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: self.helper.join(self.room1, self.to_kick, tok=self.to_kick_tok) - def test_get_user_invite_count_test_case(self) -> None: + def test_get_user_invite_count_new_invites_test_case(self) -> None: + """ + Test that new invites that arrive after a provided timestamp are counted + """ # grab a current timestamp invites_sent_ts = self.hs.get_clock().time_msec() @@ -5584,6 +5590,40 @@ def test_get_user_invite_count_test_case(self) -> None: # advance clock 48 hours self.reactor.advance(ONE_DAY_MS * 2) + # fetch using timestamp, all should be returned + channel = self.make_request( + "GET", + f"/_synapse/admin/v1/users/{self.bad_user}/sent_invite_count?from_ts={invites_sent_ts}", + access_token=self.admin_tok, + ) + self.assertEqual(channel.code, 200) + self.assertEqual(channel.json_body["invite_count"], 8) + + # send some more invites, they should show up in addition to original 8 using same timestamp + for user in self.random_users: + self.helper.invite(self.room3, src=self.bad_user, targ=user, tok=self.bad_user_tok) + + channel = self.make_request( + "GET", + f"/_synapse/admin/v1/users/{self.bad_user}/sent_invite_count?from_ts={invites_sent_ts}", + access_token=self.admin_tok, + ) + self.assertEqual(channel.code, 200) + self.assertEqual(channel.json_body["invite_count"], 12) + + + def test_get_user_invite_count_invites_before_ts_test_case(self) -> None: + """ + Test that invites sent before provided ts are not counted + """ + # bad user sends some invites + for room_id in [self.room1, self.room2]: + for user in self.random_users: + self.helper.invite(room_id, self.bad_user, user, tok=self.bad_user_tok) + + # advance clock 48 hours + self.reactor.advance(ONE_DAY_MS * 2) + # get a timestamp beginning 24 hours ago current_time = self.hs.get_clock().time_msec() from_ts = current_time - (ONE_DAY_MS) @@ -5597,30 +5637,34 @@ def test_get_user_invite_count_test_case(self) -> None: self.assertEqual(channel.code, 200) self.assertEqual(channel.json_body["invite_count"], 0) - # fetch using original timestamp, all should be returned - channel = self.make_request( - "GET", - f"/_synapse/admin/v1/users/{self.bad_user}/sent_invite_count?from_ts={invites_sent_ts}", - access_token=self.admin_tok, - ) - self.assertEqual(channel.code, 200) - self.assertEqual(channel.json_body["invite_count"], 8) + def test_user_invite_count_kick_ban_not_counted(self) -> None: + """ + Test that kicks and bans are not counted in invite count + """ + # grab a current timestamp + invites_sent_ts = self.hs.get_clock().time_msec() - # send some more invites, they should show up in addition to original 8 using original ts - for user in self.random_users: - self.helper.invite(self.room3, self.bad_user, user, tok=self.bad_user_tok) + # bad user sends some invites (8) + for room_id in [self.room1, self.room2]: + for user in self.random_users: + self.helper.invite(room_id, self.bad_user, user, tok=self.bad_user_tok) + # advance clock 48 hours + self.reactor.advance(ONE_DAY_MS * 2) + + # fetch using timestamp, all invites sent should be counted channel = self.make_request( "GET", f"/_synapse/admin/v1/users/{self.bad_user}/sent_invite_count?from_ts={invites_sent_ts}", access_token=self.admin_tok, ) self.assertEqual(channel.code, 200) - self.assertEqual(channel.json_body["invite_count"], 12) + self.assertEqual(channel.json_body["invite_count"], 8) # send a kick and some bans and make sure these aren't counted against invite total for user in self.random_users: - self.helper.ban(self.room1, self.bad_user, user, tok=self.bad_user_tok) + self.helper.ban(self.room1, src=self.bad_user, targ=user, + tok=self.bad_user_tok) channel = self.make_request( "POST", @@ -5637,4 +5681,4 @@ def test_get_user_invite_count_test_case(self) -> None: access_token=self.admin_tok, ) self.assertEqual(channel.code, 200) - self.assertEqual(channel.json_body["invite_count"], 12) + self.assertEqual(channel.json_body["invite_count"], 8) From d75484984b809f5eeae3a596ef12880934a1a60a Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 3 Dec 2024 15:21:18 -0800 Subject: [PATCH 26/40] lint --- tests/rest/admin/test_event_reports.py | 4 +++- tests/rest/admin/test_user.py | 10 ++++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/tests/rest/admin/test_event_reports.py b/tests/rest/admin/test_event_reports.py index 4fe79216b21..c29ce3e29af 100644 --- a/tests/rest/admin/test_event_reports.py +++ b/tests/rest/admin/test_event_reports.py @@ -409,7 +409,9 @@ def test_filter_against_event_sender(self) -> None: server_filtered_report_ids = set() for event_report in event_reports: server_filtered_report_ids.add(event_report["id"]) - self.assertEqual(True, locally_filtered_report_ids == server_filtered_report_ids) + self.assertEqual( + True, locally_filtered_report_ids == server_filtered_report_ids + ) def _create_event_and_report(self, room_id: str, user_tok: str) -> None: """Create and report events""" diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index 4867e515334..18d0ef74b4f 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -5601,7 +5601,9 @@ def test_get_user_invite_count_new_invites_test_case(self) -> None: # send some more invites, they should show up in addition to original 8 using same timestamp for user in self.random_users: - self.helper.invite(self.room3, src=self.bad_user, targ=user, tok=self.bad_user_tok) + self.helper.invite( + self.room3, src=self.bad_user, targ=user, tok=self.bad_user_tok + ) channel = self.make_request( "GET", @@ -5611,7 +5613,6 @@ def test_get_user_invite_count_new_invites_test_case(self) -> None: self.assertEqual(channel.code, 200) self.assertEqual(channel.json_body["invite_count"], 12) - def test_get_user_invite_count_invites_before_ts_test_case(self) -> None: """ Test that invites sent before provided ts are not counted @@ -5663,8 +5664,9 @@ def test_user_invite_count_kick_ban_not_counted(self) -> None: # send a kick and some bans and make sure these aren't counted against invite total for user in self.random_users: - self.helper.ban(self.room1, src=self.bad_user, targ=user, - tok=self.bad_user_tok) + self.helper.ban( + self.room1, src=self.bad_user, targ=user, tok=self.bad_user_tok + ) channel = self.make_request( "POST", From 347e4f7868de3ed9b34d9a93875a1c3cf2ce4b90 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 3 Dec 2024 15:45:42 -0800 Subject: [PATCH 27/40] rescind USING shorthand as it made postgres unhappy --- synapse/storage/databases/main/room.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index 5029f43bb9a..46bf43650dc 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -1657,7 +1657,8 @@ def _get_event_reports_paginate_txn( room_stats_state.name FROM event_reports AS er LEFT JOIN events USING(event_id) - JOIN room_stats_state USING(room_id) + JOIN room_stats_state + ON room_stats_state.room_id = er.room_id {where_clause} ORDER BY er.received_ts {order} LIMIT ? From a5762b37f3774833e98150ffad688657b5afde5e Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 3 Dec 2024 15:49:48 -0800 Subject: [PATCH 28/40] lint --- synapse/storage/databases/main/room.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index 46bf43650dc..bdf013cc432 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -1657,7 +1657,7 @@ def _get_event_reports_paginate_txn( room_stats_state.name FROM event_reports AS er LEFT JOIN events USING(event_id) - JOIN room_stats_state + JOIN room_stats_state ON room_stats_state.room_id = er.room_id {where_clause} ORDER BY er.received_ts {order} From b4ec5c2e7c9fe049807f8cb17e4e41df5bc4478a Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Wed, 4 Dec 2024 10:15:25 -0800 Subject: [PATCH 29/40] clarify terms --- synapse/rest/admin/users.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index d6b72f8535a..0e1a6a6336b 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -1003,9 +1003,9 @@ async def on_GET( room_ids = await self.store.get_rooms_for_user_by_date(user_id, from_ts) else: room_ids = await self.store.get_rooms_for_user(user_id) - rooms_list = {"joined_rooms": list(room_ids), "total": len(room_ids)} + rooms_response = {"joined_rooms": list(room_ids), "total": len(room_ids)} - return HTTPStatus.OK, rooms_list + return HTTPStatus.OK, rooms_response class PushersRestServlet(RestServlet): @@ -1525,6 +1525,8 @@ async def on_GET( await assert_requester_is_admin(self._auth, request) from_ts = parse_integer(request, "from_ts", required=True) - res = await self.store.get_invite_count_by_user(user_id, from_ts) + sent_invite_count = await self.store.get_sent_invite_count_by_user( + user_id, from_ts + ) - return HTTPStatus.OK, {"invite_count": res} + return HTTPStatus.OK, {"invite_count": sent_invite_count} From 2641961afea9101df6a1caa6228baba0c530a934 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Wed, 4 Dec 2024 10:15:54 -0800 Subject: [PATCH 30/40] rework queries --- .../storage/databases/main/events_worker.py | 22 ++++++++++--------- synapse/storage/databases/main/room.py | 2 +- synapse/storage/databases/main/roommember.py | 11 ++++++---- 3 files changed, 20 insertions(+), 15 deletions(-) diff --git a/synapse/storage/databases/main/events_worker.py b/synapse/storage/databases/main/events_worker.py index 54b92406e3b..1658bcd65d1 100644 --- a/synapse/storage/databases/main/events_worker.py +++ b/synapse/storage/databases/main/events_worker.py @@ -2602,26 +2602,28 @@ async def have_finished_sliding_sync_background_jobs(self) -> bool: ) ) - async def get_invite_count_by_user(self, user_id: str, from_ts: int) -> int: + async def get_sent_invite_count_by_user(self, user_id: str, from_ts: int) -> int: """ Get the number of invites sent by the given user after the provided timestamp. Args: user_id: user ID to search against from_ts: a timestamp in milliseconds from the unix epoch. Filters against - `events.received_ts` + `events.received_ts` """ - def _get_invite_count_by_user_txn( + def _get_sent_invite_count_by_user_txn( txn: LoggingTransaction, user_id: str, from_ts: int ) -> int: sql = """ - SELECT COUNT(e.event_id) - FROM events e - INNER JOIN room_memberships rm USING(event_id) - WHERE e.sender = ? AND rm.membership = 'invite' - AND e.type = 'm.room.member' AND e.received_ts > ? + SELECT COUNT(rm.event_id) + FROM room_memberships AS rm + INNER JOIN events AS e USING(event_id) + WHERE rm.sender = ? + AND rm.membership = 'invite' + AND e.type = 'm.room.member' + AND e.received_ts >= ? """ txn.execute(sql, (user_id, from_ts)) @@ -2632,8 +2634,8 @@ def _get_invite_count_by_user_txn( return int(res[0]) return await self.db_pool.runInteraction( - "_get_invite_count_by_user_txn", - _get_invite_count_by_user_txn, + "_get_sent_invite_count_by_user_txn", + _get_sent_invite_count_by_user_txn, user_id, from_ts, ) diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index bdf013cc432..2522bebd728 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -1598,7 +1598,7 @@ async def get_event_reports_paginate( user_id: search for user_id. Ignored if user_id is None room_id: search for room_id. Ignored if room_id is None event_sender_user_id: search for the sender of the reported event. Ignored if - event_sender_user_id is None + event_sender_user_id is None Returns: Tuple of: json list of event reports diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index 0e160149e19..eba8ce23cb3 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -1588,10 +1588,13 @@ def _get_rooms_for_user_by_join_date_txn( # this index: "current_state_events_member_index" btree (state_key) WHERE type = 'm.room.member'::text # on the current_state_events_table sql = """ - SELECT c.room_id - FROM current_state_events c - JOIN events e ON c.event_id = e.event_id - WHERE c.state_key = ? AND c.membership = 'join' AND e.received_ts > ? AND c.type = 'm.room.member' + SELECT room_id + FROM room_memberships AS rm + INNER JOIN events AS e USING (event_id) + WHERE rm.user_id = ? + AND rm.membership = 'join' + AND e.type = 'm.room.member' + AND e.received_ts > ? """ txn.execute(sql, (user_id, from_ts)) return frozenset([r[0] for r in txn]) From e62a1d96553c26433c0510d702d7972cb813f396 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Wed, 4 Dec 2024 10:16:01 -0800 Subject: [PATCH 31/40] clean up tests --- tests/rest/admin/test_user.py | 30 +++++++++++------------------- 1 file changed, 11 insertions(+), 19 deletions(-) diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index 18d0ef74b4f..0cf14eb8529 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -5580,25 +5580,25 @@ def test_get_user_invite_count_new_invites_test_case(self) -> None: Test that new invites that arrive after a provided timestamp are counted """ # grab a current timestamp - invites_sent_ts = self.hs.get_clock().time_msec() + before_invites_sent_ts = self.hs.get_clock().time_msec() # bad user sends some invites for room_id in [self.room1, self.room2]: for user in self.random_users: self.helper.invite(room_id, self.bad_user, user, tok=self.bad_user_tok) - # advance clock 48 hours - self.reactor.advance(ONE_DAY_MS * 2) - # fetch using timestamp, all should be returned channel = self.make_request( "GET", - f"/_synapse/admin/v1/users/{self.bad_user}/sent_invite_count?from_ts={invites_sent_ts}", + f"/_synapse/admin/v1/users/{self.bad_user}/sent_invite_count?from_ts={before_invites_sent_ts}", access_token=self.admin_tok, ) self.assertEqual(channel.code, 200) self.assertEqual(channel.json_body["invite_count"], 8) + # advance clock slightly to avoid ratelimiting when sending new invites + self.reactor.advance(60) + # send some more invites, they should show up in addition to original 8 using same timestamp for user in self.random_users: self.helper.invite( @@ -5607,7 +5607,7 @@ def test_get_user_invite_count_new_invites_test_case(self) -> None: channel = self.make_request( "GET", - f"/_synapse/admin/v1/users/{self.bad_user}/sent_invite_count?from_ts={invites_sent_ts}", + f"/_synapse/admin/v1/users/{self.bad_user}/sent_invite_count?from_ts={before_invites_sent_ts}", access_token=self.admin_tok, ) self.assertEqual(channel.code, 200) @@ -5622,17 +5622,12 @@ def test_get_user_invite_count_invites_before_ts_test_case(self) -> None: for user in self.random_users: self.helper.invite(room_id, self.bad_user, user, tok=self.bad_user_tok) - # advance clock 48 hours - self.reactor.advance(ONE_DAY_MS * 2) - - # get a timestamp beginning 24 hours ago - current_time = self.hs.get_clock().time_msec() - from_ts = current_time - (ONE_DAY_MS) + after_invites_sent_ts = self.hs.get_clock().time_msec() # fetch invites with timestamp, none should be returned channel = self.make_request( "GET", - f"/_synapse/admin/v1/users/{self.bad_user}/sent_invite_count?from_ts={from_ts}", + f"/_synapse/admin/v1/users/{self.bad_user}/sent_invite_count?from_ts={after_invites_sent_ts}", access_token=self.admin_tok, ) self.assertEqual(channel.code, 200) @@ -5643,20 +5638,17 @@ def test_user_invite_count_kick_ban_not_counted(self) -> None: Test that kicks and bans are not counted in invite count """ # grab a current timestamp - invites_sent_ts = self.hs.get_clock().time_msec() + before_invites_sent_ts = self.hs.get_clock().time_msec() # bad user sends some invites (8) for room_id in [self.room1, self.room2]: for user in self.random_users: self.helper.invite(room_id, self.bad_user, user, tok=self.bad_user_tok) - # advance clock 48 hours - self.reactor.advance(ONE_DAY_MS * 2) - # fetch using timestamp, all invites sent should be counted channel = self.make_request( "GET", - f"/_synapse/admin/v1/users/{self.bad_user}/sent_invite_count?from_ts={invites_sent_ts}", + f"/_synapse/admin/v1/users/{self.bad_user}/sent_invite_count?from_ts={before_invites_sent_ts}", access_token=self.admin_tok, ) self.assertEqual(channel.code, 200) @@ -5679,7 +5671,7 @@ def test_user_invite_count_kick_ban_not_counted(self) -> None: channel = self.make_request( "GET", - f"/_synapse/admin/v1/users/{self.bad_user}/sent_invite_count?from_ts={invites_sent_ts}", + f"/_synapse/admin/v1/users/{self.bad_user}/sent_invite_count?from_ts={before_invites_sent_ts}", access_token=self.admin_tok, ) self.assertEqual(channel.code, 200) From 8f73f15369ef4c2b0b4881d3aa538ab889c17ffb Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Wed, 4 Dec 2024 10:53:31 -0800 Subject: [PATCH 32/40] fix tests --- synapse/storage/databases/main/roommember.py | 2 +- tests/rest/admin/test_user.py | 20 ++++++++------------ 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index eba8ce23cb3..2ca2299ceb1 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -1588,7 +1588,7 @@ def _get_rooms_for_user_by_join_date_txn( # this index: "current_state_events_member_index" btree (state_key) WHERE type = 'm.room.member'::text # on the current_state_events_table sql = """ - SELECT room_id + SELECT rm.room_id FROM room_memberships AS rm INNER JOIN events AS e USING (event_id) WHERE rm.user_id = ? diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index 0cf14eb8529..52f8858b6cb 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -3393,31 +3393,27 @@ def test_from_ts_param(self) -> None: """ Tests filtering joined room results by timestamp """ - # Create rooms and join, grab timestamp of room creation + # Create rooms and join, grab timestamp before room creation before_room_creation_timestamp = self.hs.get_clock().time_msec() other_user_tok = self.login("user", "pass") number_rooms = 5 for _ in range(number_rooms): self.helper.create_room_as(self.other_user, tok=other_user_tok) - # advance clock 48 hours - self.reactor.advance(ONE_DAY_MS * 2) + # get a timestamp after room creation and join + after_room_creation = self.hs.get_clock().time_msec() - # get a timestamp beginning 24 hours ago - current_time = self.hs.get_clock().time_msec() - from_ts = current_time - (ONE_DAY_MS) - - # Get rooms using this timestamp, there should be none since all rooms were created - # outside of the `from_ts` 24 hour range + # Get rooms using this timestamp, there should be none since all rooms were created and joined + # before provided timestamp channel = self.make_request( "GET", - f"{self.url}?from_ts={int(from_ts)}", + f"{self.url}?from_ts={int(after_room_creation)}", access_token=self.admin_user_tok, ) self.assertEqual(200, channel.code, msg=channel.json_body) self.assertEqual(0, channel.json_body["total"]) - # fetch rooms with the older timestamp before they were created, this should + # fetch rooms with the older timestamp before they were created and joined, this should # return the rooms channel = self.make_request( "GET", @@ -5622,7 +5618,7 @@ def test_get_user_invite_count_invites_before_ts_test_case(self) -> None: for user in self.random_users: self.helper.invite(room_id, self.bad_user, user, tok=self.bad_user_tok) - after_invites_sent_ts = self.hs.get_clock().time_msec() + after_invites_sent_ts = self.hs.get_clock().time_msec() + 1 # add a ms of space between invite and ts # fetch invites with timestamp, none should be returned channel = self.make_request( From d5ad780817469bf231ad9870dd1cb680094392b6 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Mon, 9 Dec 2024 11:54:24 -0800 Subject: [PATCH 33/40] use new endpoint --- docs/admin_api/user_admin_api.md | 44 ++++-- synapse/rest/admin/__init__.py | 2 + synapse/rest/admin/users.py | 29 +++- synapse/storage/databases/main/roommember.py | 22 ++- tests/rest/admin/test_user.py | 157 ++++++++++++++----- 5 files changed, 190 insertions(+), 64 deletions(-) diff --git a/docs/admin_api/user_admin_api.md b/docs/admin_api/user_admin_api.md index 903c6b5b28e..5dce99cba32 100644 --- a/docs/admin_api/user_admin_api.md +++ b/docs/admin_api/user_admin_api.md @@ -509,15 +509,6 @@ The following parameters should be set in the URL: - `user_id` - fully qualified: for example, `@user:server.com`. -The following should be set as query parameters in the URL: - -- `from_ts` - int. Optional. A timestamp in ms from the unix - epoch - only rooms joined after the provided timestamp will be returned. - This works by comparing the provided timestamp to the `received_ts` - column in the `events` table. - Note: https://currentmillis.com/ is a useful tool for converting dates - into timestamps and vice versa. - **Response** The following fields are returned in the JSON response body: @@ -549,8 +540,6 @@ The following should be set as query parameters in the URL: Note: https://currentmillis.com/ is a useful tool for converting dates into timestamps and vice versa. - - A response body like the following is returned: ```json @@ -561,6 +550,39 @@ A response body like the following is returned: _Added in Synapse 1.121.0_ +## Get the cumulative number of rooms a user has joined after a given timestamp + +Fetches the number of rooms that the user joined after the given timestamp, even +if they have subsequently left/been banned from those rooms. + +``` +GET /_synapse/admin/v1/users/$ None: RedactUser(hs).register(http_server) RedactUserStatus(hs).register(http_server) UserInvitesCount(hs).register(http_server) + UserJoinedRoomCount(hs).register(http_server) DeviceRestServlet(hs).register(http_server) DevicesRestServlet(hs).register(http_server) diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index 0e1a6a6336b..5d39f55b471 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -997,12 +997,8 @@ async def on_GET( self, request: SynapseRequest, user_id: str ) -> Tuple[int, JsonDict]: await assert_requester_is_admin(self.auth, request) - from_ts = parse_integer(request, "from_ts") - if from_ts is not None: - room_ids = await self.store.get_rooms_for_user_by_date(user_id, from_ts) - else: - room_ids = await self.store.get_rooms_for_user(user_id) + room_ids = await self.store.get_rooms_for_user(user_id) rooms_response = {"joined_rooms": list(room_ids), "total": len(room_ids)} return HTTPStatus.OK, rooms_response @@ -1530,3 +1526,26 @@ async def on_GET( ) return HTTPStatus.OK, {"invite_count": sent_invite_count} + + +class UserJoinedRoomCount(RestServlet): + """ + Return the count of rooms that the user has joined after the given timestamp, even + if they have subsequently left/been banned from those rooms. + """ + + PATTERNS = admin_patterns("/users/(?P[^/]*)/cumulative_joined_room_count") + + def __init__(self, hs: "HomeServer"): + self._auth = hs.get_auth() + self.store = hs.get_datastores().main + + async def on_GET( + self, request: SynapseRequest, user_id: str + ) -> Tuple[int, JsonDict]: + await assert_requester_is_admin(self._auth, request) + from_ts = parse_integer(request, "from_ts", required=True) + + joined_rooms = await self.store.get_rooms_for_user_by_date(user_id, from_ts) + + return HTTPStatus.OK, {"cumulative_joined_room_count": len(joined_rooms)} diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index 2ca2299ceb1..da2510e27c5 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -1574,7 +1574,8 @@ def get_sliding_sync_room_for_user_batch_txn( async def get_rooms_for_user_by_date(self, user_id: str, from_ts: int) -> frozenset: """ - Fetch a list of rooms that the user has joined since the given timestamp. + Fetch a list of rooms that the user has joined since the given timestamp, including + those they subsequently have left/been banned from. Args: user_id: user ID of the user to search for @@ -1584,19 +1585,16 @@ async def get_rooms_for_user_by_date(self, user_id: str, from_ts: int) -> frozen def _get_rooms_for_user_by_join_date_txn( txn: LoggingTransaction, user_id: str, timestamp: int ) -> frozenset: - # Note that matching on c.type = 'm.room.member' allows us to benefit from - # this index: "current_state_events_member_index" btree (state_key) WHERE type = 'm.room.member'::text - # on the current_state_events_table sql = """ - SELECT rm.room_id - FROM room_memberships AS rm - INNER JOIN events AS e USING (event_id) - WHERE rm.user_id = ? - AND rm.membership = 'join' - AND e.type = 'm.room.member' - AND e.received_ts > ? + SELECT rm.room_id + FROM room_memberships AS rm + INNER JOIN events AS e USING (event_id) + WHERE rm.user_id = ? + AND rm.membership = 'join' + AND e.type = 'm.room.member' + AND e.received_ts > ? """ - txn.execute(sql, (user_id, from_ts)) + txn.execute(sql, (user_id, timestamp)) return frozenset([r[0] for r in txn]) return await self.db_pool.runInteraction( diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index 52f8858b6cb..cf41e08eae0 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -3389,41 +3389,6 @@ def test_get_rooms_with_nonlocal_user(self) -> None: self.assertEqual(1, channel.json_body["total"]) self.assertEqual([local_and_remote_room_id], channel.json_body["joined_rooms"]) - def test_from_ts_param(self) -> None: - """ - Tests filtering joined room results by timestamp - """ - # Create rooms and join, grab timestamp before room creation - before_room_creation_timestamp = self.hs.get_clock().time_msec() - other_user_tok = self.login("user", "pass") - number_rooms = 5 - for _ in range(number_rooms): - self.helper.create_room_as(self.other_user, tok=other_user_tok) - - # get a timestamp after room creation and join - after_room_creation = self.hs.get_clock().time_msec() - - # Get rooms using this timestamp, there should be none since all rooms were created and joined - # before provided timestamp - channel = self.make_request( - "GET", - f"{self.url}?from_ts={int(after_room_creation)}", - access_token=self.admin_user_tok, - ) - self.assertEqual(200, channel.code, msg=channel.json_body) - self.assertEqual(0, channel.json_body["total"]) - - # fetch rooms with the older timestamp before they were created and joined, this should - # return the rooms - channel = self.make_request( - "GET", - f"{self.url}?from_ts={int(before_room_creation_timestamp)}", - access_token=self.admin_user_tok, - ) - self.assertEqual(200, channel.code, msg=channel.json_body) - self.assertEqual(number_rooms, channel.json_body["total"]) - self.assertEqual(number_rooms, len(channel.json_body["joined_rooms"])) - class PushersRestTestCase(unittest.HomeserverTestCase): servlets = [ @@ -5618,7 +5583,9 @@ def test_get_user_invite_count_invites_before_ts_test_case(self) -> None: for user in self.random_users: self.helper.invite(room_id, self.bad_user, user, tok=self.bad_user_tok) - after_invites_sent_ts = self.hs.get_clock().time_msec() + 1 # add a ms of space between invite and ts + after_invites_sent_ts = ( + self.hs.get_clock().time_msec() + 1 + ) # add a ms of space between invite and ts # fetch invites with timestamp, none should be returned channel = self.make_request( @@ -5672,3 +5639,121 @@ def test_user_invite_count_kick_ban_not_counted(self) -> None: ) self.assertEqual(channel.code, 200) self.assertEqual(channel.json_body["invite_count"], 8) + + +class GetCumulativeJoinedRoomCountForUserTestCase(unittest.HomeserverTestCase): + servlets = [ + synapse.rest.admin.register_servlets, + login.register_servlets, + admin.register_servlets, + room.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") + + def test_user_cumulative_joined_room_count(self) -> None: + """ + Tests proper count returned from /cumulative_joined_room_count endpoint + """ + # Create rooms and join, grab timestamp before room creation + before_room_creation_timestamp = self.hs.get_clock().time_msec() + + joined_rooms = [] + for _ in range(3): + room = self.helper.create_room_as(self.admin, tok=self.admin_tok) + self.helper.join( + room, user=self.bad_user, expect_code=200, tok=self.bad_user_tok + ) + joined_rooms.append(room) + + # get a timestamp after room creation and join + after_room_creation = self.hs.get_clock().time_msec() + + # Get rooms using this timestamp, there should be none since all rooms were created and joined + # before provided timestamp + channel = self.make_request( + "GET", + f"/_synapse/admin/v1/users/{self.bad_user}/cumulative_joined_room_count?from_ts={int(after_room_creation)}", + access_token=self.admin_tok, + ) + self.assertEqual(200, channel.code, msg=channel.json_body) + self.assertEqual(0, channel.json_body["cumulative_joined_room_count"]) + + # fetch rooms with the older timestamp before they were created and joined, this should + # return the rooms + channel = self.make_request( + "GET", + f"/_synapse/admin/v1/users/{self.bad_user}/cumulative_joined_room_count?from_ts={int(before_room_creation_timestamp)}", + access_token=self.admin_tok, + ) + self.assertEqual(200, channel.code, msg=channel.json_body) + self.assertEqual( + len(joined_rooms), channel.json_body["cumulative_joined_room_count"] + ) + + def test_user_joined_room_count_includes_left_and_banned_rooms(self) -> None: + """ + Tests proper count returned from /joined_room_count endpoint when user has left + or been banned from joined rooms + """ + # Create rooms and join, grab timestamp before room creation + before_room_creation_timestamp = self.hs.get_clock().time_msec() + + joined_rooms = [] + for _ in range(3): + room = self.helper.create_room_as(self.admin, tok=self.admin_tok) + self.helper.join( + room, user=self.bad_user, expect_code=200, tok=self.bad_user_tok + ) + joined_rooms.append(room) + + # fetch rooms with the older timestamp before they were created and joined + channel = self.make_request( + "GET", + f"/_synapse/admin/v1/users/{self.bad_user}/cumulative_joined_room_count?from_ts={int(before_room_creation_timestamp)}", + access_token=self.admin_tok, + ) + self.assertEqual(200, channel.code, msg=channel.json_body) + self.assertEqual( + len(joined_rooms), channel.json_body["cumulative_joined_room_count"] + ) + + # have the user banned from/leave the joined rooms + self.helper.ban( + joined_rooms[0], + src=self.admin, + targ=self.bad_user, + expect_code=200, + tok=self.admin_tok, + ) + self.helper.change_membership( + joined_rooms[1], + src=self.bad_user, + targ=self.bad_user, + membership="leave", + expect_code=200, + tok=self.bad_user_tok, + ) + self.helper.ban( + joined_rooms[2], + src=self.admin, + targ=self.bad_user, + expect_code=200, + tok=self.admin_tok, + ) + + # fetch the joined room count again, the number should remain the same as the collected joined rooms + channel = self.make_request( + "GET", + f"/_synapse/admin/v1/users/{self.bad_user}/cumulative_joined_room_count?from_ts={int(before_room_creation_timestamp)}", + access_token=self.admin_tok, + ) + self.assertEqual(200, channel.code, msg=channel.json_body) + self.assertEqual( + len(joined_rooms), channel.json_body["cumulative_joined_room_count"] + ) From b3ff8027035a67ffff30c52875b56e35d26ac4f7 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Mon, 9 Dec 2024 12:08:03 -0800 Subject: [PATCH 34/40] remove unused constants --- synapse/storage/databases/main/events_worker.py | 3 --- tests/rest/admin/test_user.py | 3 --- 2 files changed, 6 deletions(-) diff --git a/synapse/storage/databases/main/events_worker.py b/synapse/storage/databases/main/events_worker.py index 1658bcd65d1..1cb130bac8d 100644 --- a/synapse/storage/databases/main/events_worker.py +++ b/synapse/storage/databases/main/events_worker.py @@ -104,9 +104,6 @@ logger = logging.getLogger(__name__) -ONE_HOUR_MS = 60 * 60 * 1000 -ONE_DAY_MS = 24 * ONE_HOUR_MS - class DatabaseCorruptionError(RuntimeError): """We found an event in the DB that has a persisted event ID that doesn't diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index cf41e08eae0..01ee7b691a2 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -62,9 +62,6 @@ from tests.test_utils import SMALL_PNG from tests.unittest import override_config -ONE_HOUR_MS = 60 * 60 * 1000 -ONE_DAY_MS = 24 * ONE_HOUR_MS - class UserRegisterTestCase(unittest.HomeserverTestCase): servlets = [ From 312dbf11409303212ecc271589c08d44daedca9e Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 10 Dec 2024 12:09:25 -0800 Subject: [PATCH 35/40] match ts on > rather than >= --- synapse/storage/databases/main/events_worker.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/events_worker.py b/synapse/storage/databases/main/events_worker.py index 1cb130bac8d..91e2e9fe3b1 100644 --- a/synapse/storage/databases/main/events_worker.py +++ b/synapse/storage/databases/main/events_worker.py @@ -2620,7 +2620,7 @@ def _get_sent_invite_count_by_user_txn( WHERE rm.sender = ? AND rm.membership = 'invite' AND e.type = 'm.room.member' - AND e.received_ts >= ? + AND e.received_ts > ? """ txn.execute(sql, (user_id, from_ts)) From 197ed96fa948160d1797bf6607c1ee3d3a4b8a0e Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 10 Dec 2024 12:09:34 -0800 Subject: [PATCH 36/40] clean up tests --- tests/rest/admin/test_event_reports.py | 4 +- tests/rest/admin/test_user.py | 66 ++++++++++++-------------- 2 files changed, 33 insertions(+), 37 deletions(-) diff --git a/tests/rest/admin/test_event_reports.py b/tests/rest/admin/test_event_reports.py index c29ce3e29af..6047ce1f4af 100644 --- a/tests/rest/admin/test_event_reports.py +++ b/tests/rest/admin/test_event_reports.py @@ -409,8 +409,8 @@ def test_filter_against_event_sender(self) -> None: server_filtered_report_ids = set() for event_report in event_reports: server_filtered_report_ids.add(event_report["id"]) - self.assertEqual( - True, locally_filtered_report_ids == server_filtered_report_ids + self.assertIncludes( + locally_filtered_report_ids, server_filtered_report_ids, exact=True ) def _create_event_and_report(self, room_id: str, user_tok: str) -> None: diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index 01ee7b691a2..dcccbb45905 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -5520,9 +5520,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.to_kick = self.register_user("kick_me", "pass") - self.to_kick_tok = self.login("kick_me", "pass") - self.random_users = [] for i in range(4): self.random_users.append(self.register_user(f"user{i}", f"pass{i}")) @@ -5531,8 +5528,9 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: self.room2 = self.helper.create_room_as(self.bad_user, tok=self.bad_user_tok) self.room3 = self.helper.create_room_as(self.bad_user, tok=self.bad_user_tok) - self.helper.join(self.room1, self.to_kick, tok=self.to_kick_tok) - + @unittest.override_config( + {"rc_invites": {"per_issuer": {"per_second": 1000, "burst_count": 1000}}} + ) def test_get_user_invite_count_new_invites_test_case(self) -> None: """ Test that new invites that arrive after a provided timestamp are counted @@ -5554,9 +5552,6 @@ def test_get_user_invite_count_new_invites_test_case(self) -> None: self.assertEqual(channel.code, 200) self.assertEqual(channel.json_body["invite_count"], 8) - # advance clock slightly to avoid ratelimiting when sending new invites - self.reactor.advance(60) - # send some more invites, they should show up in addition to original 8 using same timestamp for user in self.random_users: self.helper.invite( @@ -5580,9 +5575,7 @@ def test_get_user_invite_count_invites_before_ts_test_case(self) -> None: for user in self.random_users: self.helper.invite(room_id, self.bad_user, user, tok=self.bad_user_tok) - after_invites_sent_ts = ( - self.hs.get_clock().time_msec() + 1 - ) # add a ms of space between invite and ts + after_invites_sent_ts = self.hs.get_clock().time_msec() # fetch invites with timestamp, none should be returned channel = self.make_request( @@ -5597,13 +5590,20 @@ def test_user_invite_count_kick_ban_not_counted(self) -> None: """ Test that kicks and bans are not counted in invite count """ + to_kick_user_id = self.register_user("kick_me", "pass") + to_kick_tok = self.login("kick_me", "pass") + + self.helper.join(self.room1, to_kick_user_id, tok=to_kick_tok) + # grab a current timestamp before_invites_sent_ts = self.hs.get_clock().time_msec() # bad user sends some invites (8) for room_id in [self.room1, self.room2]: for user in self.random_users: - self.helper.invite(room_id, self.bad_user, user, tok=self.bad_user_tok) + self.helper.invite( + room_id, src=self.bad_user, targ=user, tok=self.bad_user_tok + ) # fetch using timestamp, all invites sent should be counted channel = self.make_request( @@ -5623,7 +5623,7 @@ def test_user_invite_count_kick_ban_not_counted(self) -> None: channel = self.make_request( "POST", f"/_matrix/client/v3/rooms/{self.room1}/kick", - content={"user_id": f"{self.to_kick}"}, + content={"user_id": f"{to_kick_user_id}"}, access_token=self.bad_user_tok, shorthand=False, ) @@ -5721,28 +5721,24 @@ def test_user_joined_room_count_includes_left_and_banned_rooms(self) -> None: ) # have the user banned from/leave the joined rooms - self.helper.ban( - joined_rooms[0], - src=self.admin, - targ=self.bad_user, - expect_code=200, - tok=self.admin_tok, - ) - self.helper.change_membership( - joined_rooms[1], - src=self.bad_user, - targ=self.bad_user, - membership="leave", - expect_code=200, - tok=self.bad_user_tok, - ) - self.helper.ban( - joined_rooms[2], - src=self.admin, - targ=self.bad_user, - expect_code=200, - tok=self.admin_tok, - ) + for i, room in enumerate(joined_rooms): + if i == 1: + self.helper.change_membership( + room, + src=self.bad_user, + targ=self.bad_user, + membership="leave", + expect_code=200, + tok=self.bad_user_tok, + ) + else: + self.helper.ban( + room, + src=self.admin, + targ=self.bad_user, + expect_code=200, + tok=self.admin_tok, + ) # fetch the joined room count again, the number should remain the same as the collected joined rooms channel = self.make_request( From fc9e4ea10b01d1d24685403cb8cdb94487249677 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 10 Dec 2024 12:14:49 -0800 Subject: [PATCH 37/40] update docs --- docs/admin_api/user_admin_api.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/admin_api/user_admin_api.md b/docs/admin_api/user_admin_api.md index 5dce99cba32..a3ac96bae44 100644 --- a/docs/admin_api/user_admin_api.md +++ b/docs/admin_api/user_admin_api.md @@ -548,7 +548,7 @@ A response body like the following is returned: } ``` -_Added in Synapse 1.121.0_ +_Added in Synapse 1.122.0_ ## Get the cumulative number of rooms a user has joined after a given timestamp @@ -581,7 +581,7 @@ A response body like the following is returned: "cumulative_joined_room_count": 30 } ``` -_Added in Synapse 1.121.0_ +_Added in Synapse 1.122.0_ ## Account Data Gets information about account data for a specific `user_id`. From 9b4bad5c4faf9062ac44d0cf4d0763123eb62f53 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Wed, 11 Dec 2024 10:10:35 -0800 Subject: [PATCH 38/40] match on `>=` --- docs/admin_api/user_admin_api.md | 4 ++-- synapse/rest/admin/users.py | 2 +- synapse/storage/databases/main/events_worker.py | 4 ++-- synapse/storage/databases/main/roommember.py | 4 ++-- tests/rest/admin/test_user.py | 7 ++++--- 5 files changed, 11 insertions(+), 10 deletions(-) diff --git a/docs/admin_api/user_admin_api.md b/docs/admin_api/user_admin_api.md index a3ac96bae44..c63b7068c5e 100644 --- a/docs/admin_api/user_admin_api.md +++ b/docs/admin_api/user_admin_api.md @@ -534,7 +534,7 @@ The following parameters should be set in the URL: The following should be set as query parameters in the URL: * `from_ts`: int, required. A timestamp in ms from the unix epoch. Only - invites sent after the provided timestamp will be returned. + invites sent at or after the provided timestamp will be returned. This works by comparing the provided timestamp to the `received_ts` column in the `events` table. Note: https://currentmillis.com/ is a useful tool for converting dates @@ -568,7 +568,7 @@ The following parameters should be set in the URL: The following should be set as query parameters in the URL: * `from_ts`: int, required. A timestamp in ms from the unix epoch. Only - invites sent after the provided timestamp will be returned. + invites sent at or after the provided timestamp will be returned. This works by comparing the provided timestamp to the `received_ts` column in the `events` table. Note: https://currentmillis.com/ is a useful tool for converting dates diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index 5d39f55b471..7b8f1d1b2a9 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -1530,7 +1530,7 @@ async def on_GET( class UserJoinedRoomCount(RestServlet): """ - Return the count of rooms that the user has joined after the given timestamp, even + Return the count of rooms that the user has joined at or after the given timestamp, even if they have subsequently left/been banned from those rooms. """ diff --git a/synapse/storage/databases/main/events_worker.py b/synapse/storage/databases/main/events_worker.py index 91e2e9fe3b1..222df8757ac 100644 --- a/synapse/storage/databases/main/events_worker.py +++ b/synapse/storage/databases/main/events_worker.py @@ -2601,7 +2601,7 @@ async def have_finished_sliding_sync_background_jobs(self) -> bool: async def get_sent_invite_count_by_user(self, user_id: str, from_ts: int) -> int: """ - Get the number of invites sent by the given user after the provided timestamp. + Get the number of invites sent by the given user at or after the provided timestamp. Args: user_id: user ID to search against @@ -2620,7 +2620,7 @@ def _get_sent_invite_count_by_user_txn( WHERE rm.sender = ? AND rm.membership = 'invite' AND e.type = 'm.room.member' - AND e.received_ts > ? + AND e.received_ts >= ? """ txn.execute(sql, (user_id, from_ts)) diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index da2510e27c5..ec0c6f115d4 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -1574,7 +1574,7 @@ def get_sliding_sync_room_for_user_batch_txn( async def get_rooms_for_user_by_date(self, user_id: str, from_ts: int) -> frozenset: """ - Fetch a list of rooms that the user has joined since the given timestamp, including + Fetch a list of rooms that the user has joined at or after the given timestamp, including those they subsequently have left/been banned from. Args: @@ -1592,7 +1592,7 @@ def _get_rooms_for_user_by_join_date_txn( WHERE rm.user_id = ? AND rm.membership = 'join' AND e.type = 'm.room.member' - AND e.received_ts > ? + AND e.received_ts >= ? """ txn.execute(sql, (user_id, timestamp)) return frozenset([r[0] for r in txn]) diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index eec668fb6e6..6e154ebebde 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -5574,7 +5574,8 @@ def test_get_user_invite_count_invites_before_ts_test_case(self) -> None: for user in self.random_users: self.helper.invite(room_id, self.bad_user, user, tok=self.bad_user_tok) - after_invites_sent_ts = self.hs.get_clock().time_msec() + # add a msec between last invite and ts + after_invites_sent_ts = self.hs.get_clock().time_msec() + 1 # fetch invites with timestamp, none should be returned channel = self.make_request( @@ -5667,8 +5668,8 @@ def test_user_cumulative_joined_room_count(self) -> None: ) joined_rooms.append(room) - # get a timestamp after room creation and join - after_room_creation = self.hs.get_clock().time_msec() + # get a timestamp after room creation and join, add a msec between last join and ts + after_room_creation = self.hs.get_clock().time_msec() + 1 # Get rooms using this timestamp, there should be none since all rooms were created and joined # before provided timestamp From b8f7b90cd5a111b0ec30c73c810961fa66c76ee0 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Wed, 11 Dec 2024 10:44:30 -0800 Subject: [PATCH 39/40] frozenset + test cleanup --- synapse/storage/databases/main/roommember.py | 4 +- tests/rest/admin/test_user.py | 43 +++++++++++--------- 2 files changed, 26 insertions(+), 21 deletions(-) diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index ec0c6f115d4..50ed6a28bf0 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -1572,7 +1572,9 @@ def get_sliding_sync_room_for_user_batch_txn( get_sliding_sync_room_for_user_batch_txn, ) - async def get_rooms_for_user_by_date(self, user_id: str, from_ts: int) -> frozenset: + async def get_rooms_for_user_by_date( + self, user_id: str, from_ts: int + ) -> FrozenSet[str]: """ Fetch a list of rooms that the user has joined at or after the given timestamp, including those they subsequently have left/been banned from. diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index 6e154ebebde..b517aefd0c5 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -5623,9 +5623,8 @@ def test_user_invite_count_kick_ban_not_counted(self) -> None: channel = self.make_request( "POST", f"/_matrix/client/v3/rooms/{self.room1}/kick", - content={"user_id": f"{to_kick_user_id}"}, + content={"user_id": to_kick_user_id}, access_token=self.bad_user_tok, - shorthand=False, ) self.assertEqual(channel.code, 200) @@ -5721,24 +5720,28 @@ def test_user_joined_room_count_includes_left_and_banned_rooms(self) -> None: ) # have the user banned from/leave the joined rooms - for i, room in enumerate(joined_rooms): - if i == 1: - self.helper.change_membership( - room, - src=self.bad_user, - targ=self.bad_user, - membership="leave", - expect_code=200, - tok=self.bad_user_tok, - ) - else: - self.helper.ban( - room, - src=self.admin, - targ=self.bad_user, - expect_code=200, - tok=self.admin_tok, - ) + self.helper.ban( + joined_rooms[0], + src=self.admin, + targ=self.bad_user, + expect_code=200, + tok=self.admin_tok, + ) + self.helper.change_membership( + joined_rooms[1], + src=self.bad_user, + targ=self.bad_user, + membership="leave", + expect_code=200, + tok=self.bad_user_tok, + ) + self.helper.ban( + joined_rooms[2], + src=self.admin, + targ=self.bad_user, + expect_code=200, + tok=self.admin_tok, + ) # fetch the joined room count again, the number should remain the same as the collected joined rooms channel = self.make_request( From 092e5db8b5edd8eeb9afb641b21886e472589faf Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Mon, 16 Dec 2024 09:32:45 -0800 Subject: [PATCH 40/40] fix changelog wording --- changelog.d/17948.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/17948.feature b/changelog.d/17948.feature index 5e278d106bc..d404996cd67 100644 --- a/changelog.d/17948.feature +++ b/changelog.d/17948.feature @@ -1,3 +1,3 @@ -Add endpoints to Admin API to fetch the number of invites the provided user has after a given timestamp, +Add endpoints to Admin API to fetch the number of invites the provided user has sent after a given timestamp, fetch the number of rooms the provided user has joined after a given timestamp, and get report IDs of event reports against a provided user (ie where the user was the sender of the reported event).