From 963f4309fe29206f3ba92b493e922280feea30ed Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 30 Mar 2021 12:06:09 +0100 Subject: Make RateLimiter class check for ratelimit overrides (#9711) This should fix a class of bug where we forget to check if e.g. the appservice shouldn't be ratelimited. We also check the `ratelimit_override` table to check if the user has ratelimiting disabled. That table is really only meant to override the event sender ratelimiting, so we don't use any values from it (as they might not make sense for different rate limits), but we do infer that if ratelimiting is disabled for the user we should disabled all ratelimits. Fixes #9663 --- synapse/rest/client/v1/login.py | 14 +++++++++----- synapse/rest/client/v2_alpha/account.py | 10 +++++++--- synapse/rest/client/v2_alpha/register.py | 8 +++++--- 3 files changed, 21 insertions(+), 11 deletions(-) (limited to 'synapse/rest') diff --git a/synapse/rest/client/v1/login.py b/synapse/rest/client/v1/login.py index e4c352f572..3151e72d4f 100644 --- a/synapse/rest/client/v1/login.py +++ b/synapse/rest/client/v1/login.py @@ -74,11 +74,13 @@ class LoginRestServlet(RestServlet): self._well_known_builder = WellKnownBuilder(hs) self._address_ratelimiter = Ratelimiter( + store=hs.get_datastore(), clock=hs.get_clock(), rate_hz=self.hs.config.rc_login_address.per_second, burst_count=self.hs.config.rc_login_address.burst_count, ) self._account_ratelimiter = Ratelimiter( + store=hs.get_datastore(), clock=hs.get_clock(), rate_hz=self.hs.config.rc_login_account.per_second, burst_count=self.hs.config.rc_login_account.burst_count, @@ -141,20 +143,22 @@ class LoginRestServlet(RestServlet): appservice = self.auth.get_appservice_by_req(request) if appservice.is_rate_limited(): - self._address_ratelimiter.ratelimit(request.getClientIP()) + await self._address_ratelimiter.ratelimit( + None, request.getClientIP() + ) result = await self._do_appservice_login(login_submission, appservice) elif self.jwt_enabled and ( login_submission["type"] == LoginRestServlet.JWT_TYPE or login_submission["type"] == LoginRestServlet.JWT_TYPE_DEPRECATED ): - self._address_ratelimiter.ratelimit(request.getClientIP()) + await self._address_ratelimiter.ratelimit(None, request.getClientIP()) result = await self._do_jwt_login(login_submission) elif login_submission["type"] == LoginRestServlet.TOKEN_TYPE: - self._address_ratelimiter.ratelimit(request.getClientIP()) + await self._address_ratelimiter.ratelimit(None, request.getClientIP()) result = await self._do_token_login(login_submission) else: - self._address_ratelimiter.ratelimit(request.getClientIP()) + await self._address_ratelimiter.ratelimit(None, request.getClientIP()) result = await self._do_other_login(login_submission) except KeyError: raise SynapseError(400, "Missing JSON keys.") @@ -258,7 +262,7 @@ class LoginRestServlet(RestServlet): # too often. This happens here rather than before as we don't # necessarily know the user before now. if ratelimit: - self._account_ratelimiter.ratelimit(user_id.lower()) + await self._account_ratelimiter.ratelimit(None, user_id.lower()) if create_non_existent_users: canonical_uid = await self.auth_handler.check_user_exists(user_id) diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index c2ba790bab..411fb57c47 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -103,7 +103,9 @@ class EmailPasswordRequestTokenRestServlet(RestServlet): # Raise if the provided next_link value isn't valid assert_valid_next_link(self.hs, next_link) - self.identity_handler.ratelimit_request_token_requests(request, "email", email) + await self.identity_handler.ratelimit_request_token_requests( + request, "email", email + ) # The email will be sent to the stored address. # This avoids a potential account hijack by requesting a password reset to @@ -387,7 +389,9 @@ class EmailThreepidRequestTokenRestServlet(RestServlet): Codes.THREEPID_DENIED, ) - self.identity_handler.ratelimit_request_token_requests(request, "email", email) + await self.identity_handler.ratelimit_request_token_requests( + request, "email", email + ) if next_link: # Raise if the provided next_link value isn't valid @@ -468,7 +472,7 @@ class MsisdnThreepidRequestTokenRestServlet(RestServlet): Codes.THREEPID_DENIED, ) - self.identity_handler.ratelimit_request_token_requests( + await self.identity_handler.ratelimit_request_token_requests( request, "msisdn", msisdn ) diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py index 8f68d8dfc8..c212da0cb2 100644 --- a/synapse/rest/client/v2_alpha/register.py +++ b/synapse/rest/client/v2_alpha/register.py @@ -126,7 +126,9 @@ class EmailRegisterRequestTokenRestServlet(RestServlet): Codes.THREEPID_DENIED, ) - self.identity_handler.ratelimit_request_token_requests(request, "email", email) + await self.identity_handler.ratelimit_request_token_requests( + request, "email", email + ) existing_user_id = await self.hs.get_datastore().get_user_id_by_threepid( "email", email @@ -208,7 +210,7 @@ class MsisdnRegisterRequestTokenRestServlet(RestServlet): Codes.THREEPID_DENIED, ) - self.identity_handler.ratelimit_request_token_requests( + await self.identity_handler.ratelimit_request_token_requests( request, "msisdn", msisdn ) @@ -406,7 +408,7 @@ class RegisterRestServlet(RestServlet): client_addr = request.getClientIP() - self.ratelimiter.ratelimit(client_addr, update=False) + await self.ratelimiter.ratelimit(None, client_addr, update=False) kind = b"user" if b"kind" in request.args: -- cgit 1.5.1 From bb0fe02a5278d0033825bee31a7a49af838d4a9a Mon Sep 17 00:00:00 2001 From: Dirk Klimpel <5740567+dklimpel@users.noreply.github.com> Date: Thu, 1 Apr 2021 12:28:53 +0200 Subject: Add `order_by` to list user admin API (#9691) --- changelog.d/9691.feature | 1 + docs/admin_api/user_admin_api.rst | 85 ++++++++++++++------ synapse/rest/admin/users.py | 21 ++++- synapse/storage/databases/main/__init__.py | 26 ++++++- synapse/storage/databases/main/stats.py | 25 +++++- tests/rest/admin/test_user.py | 121 ++++++++++++++++++++++++++++- 6 files changed, 248 insertions(+), 31 deletions(-) create mode 100644 changelog.d/9691.feature (limited to 'synapse/rest') diff --git a/changelog.d/9691.feature b/changelog.d/9691.feature new file mode 100644 index 0000000000..3c711db4f5 --- /dev/null +++ b/changelog.d/9691.feature @@ -0,0 +1 @@ +Add `order_by` to the admin API `GET /_synapse/admin/v2/users`. Contributed by @dklimpel. \ No newline at end of file diff --git a/docs/admin_api/user_admin_api.rst b/docs/admin_api/user_admin_api.rst index 8d4ec5a6f9..a8a5a2628c 100644 --- a/docs/admin_api/user_admin_api.rst +++ b/docs/admin_api/user_admin_api.rst @@ -111,35 +111,16 @@ List Accounts ============= This API returns all local user accounts. +By default, the response is ordered by ascending user ID. -The api is:: +The API is:: GET /_synapse/admin/v2/users?from=0&limit=10&guests=false To use it, you will need to authenticate by providing an ``access_token`` for a server admin: see `README.rst `_. -The parameter ``from`` 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. - -The parameter ``limit`` is optional but is used for pagination, denoting the -maximum number of items to return in this call. Defaults to ``100``. - -The parameter ``user_id`` is optional and filters to only return users with user IDs -that contain this value. This parameter is ignored when using the ``name`` parameter. - -The parameter ``name`` is optional and filters to only return users with user ID localparts -**or** displaynames that contain this value. - -The parameter ``guests`` is optional and if ``false`` will **exclude** guest users. -Defaults to ``true`` to include guest users. - -The parameter ``deactivated`` is optional and if ``true`` will **include** deactivated users. -Defaults to ``false`` to exclude deactivated users. - -A JSON body is returned with the following shape: +A response body like the following is returned: .. code:: json @@ -175,6 +156,66 @@ with ``from`` set to the value of ``next_token``. This will return a new page. If the endpoint does not return a ``next_token`` then there are no more users to paginate through. +**Parameters** + +The following parameters should be set in the URL: + +- ``user_id`` - Is optional and filters to only return users with user IDs + that contain this value. This parameter is ignored when using the ``name`` parameter. +- ``name`` - Is optional and filters to only return users with user ID localparts + **or** displaynames that contain this value. +- ``guests`` - string representing a bool - Is optional and if ``false`` will **exclude** guest users. + Defaults to ``true`` to include guest users. +- ``deactivated`` - string representing a bool - Is optional and if ``true`` will **include** deactivated users. + Defaults to ``false`` to exclude deactivated users. +- ``limit`` - string representing a positive integer - Is optional but is used for pagination, + denoting the maximum number of items to return in this call. Defaults to ``100``. +- ``from`` - string representing a positive 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``. +- ``order_by`` - The method by which to sort the returned list of users. + If the ordered field has duplicates, the second order is always by ascending ``name``, + which guarantees a stable ordering. Valid values are: + + - ``name`` - Users are ordered alphabetically by ``name``. This is the default. + - ``is_guest`` - Users are ordered by ``is_guest`` status. + - ``admin`` - Users are ordered by ``admin`` status. + - ``user_type`` - Users are ordered alphabetically by ``user_type``. + - ``deactivated`` - Users are ordered by ``deactivated`` status. + - ``shadow_banned`` - Users are ordered by ``shadow_banned`` status. + - ``displayname`` - Users are ordered alphabetically by ``displayname``. + - ``avatar_url`` - Users are ordered alphabetically by avatar URL. + +- ``dir`` - Direction of media order. Either ``f`` for forwards or ``b`` for backwards. + Setting this value to ``b`` will reverse the above sort order. Defaults to ``f``. + +Caution. The database only has indexes on the columns ``name`` and ``created_ts``. +This means that if a different sort order is used (``is_guest``, ``admin``, +``user_type``, ``deactivated``, ``shadow_banned``, ``avatar_url`` or ``displayname``), +this can cause a large load on the database, especially for large environments. + +**Response** + +The following fields are returned in the JSON response body: + +- ``users`` - An array of objects, each containing information about an user. + User objects contain the following fields: + + - ``name`` - string - Fully-qualified user ID (ex. `@user:server.com`). + - ``is_guest`` - bool - Status if that user is a guest account. + - ``admin`` - bool - Status if that user is a server administrator. + - ``user_type`` - string - Type of the user. Normal users are type ``None``. + This allows user type specific behaviour. There are also types ``support`` and ``bot``. + - ``deactivated`` - bool - Status if that user has been marked as deactivated. + - ``shadow_banned`` - bool - Status if that user has been marked as shadow banned. + - ``displayname`` - string - The user's display name if they have set one. + - ``avatar_url`` - string - The user's avatar URL if they have set one. + +- ``next_token``: string representing a positive integer - Indication for pagination. See above. +- ``total`` - integer - Total number of media. + + Query current sessions for a user ================================= diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index 309bd2771b..fa7804583a 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -36,6 +36,7 @@ from synapse.rest.admin._base import ( ) from synapse.rest.client.v2_alpha._base import client_patterns from synapse.storage.databases.main.media_repository import MediaSortOrder +from synapse.storage.databases.main.stats import UserSortOrder from synapse.types import JsonDict, UserID if TYPE_CHECKING: @@ -117,8 +118,26 @@ class UsersRestServletV2(RestServlet): guests = parse_boolean(request, "guests", default=True) deactivated = parse_boolean(request, "deactivated", default=False) + order_by = parse_string( + request, + "order_by", + default=UserSortOrder.NAME.value, + allowed_values=( + UserSortOrder.NAME.value, + UserSortOrder.DISPLAYNAME.value, + UserSortOrder.GUEST.value, + UserSortOrder.ADMIN.value, + UserSortOrder.DEACTIVATED.value, + UserSortOrder.USER_TYPE.value, + UserSortOrder.AVATAR_URL.value, + UserSortOrder.SHADOW_BANNED.value, + ), + ) + + direction = parse_string(request, "dir", default="f", allowed_values=("f", "b")) + users, total = await self.store.get_users_paginate( - start, limit, user_id, name, guests, deactivated + start, limit, user_id, name, guests, deactivated, order_by, direction ) ret = {"users": users, "total": total} if (start + limit) < total: diff --git a/synapse/storage/databases/main/__init__.py b/synapse/storage/databases/main/__init__.py index 1d44c3aa2c..b3d16ca7ac 100644 --- a/synapse/storage/databases/main/__init__.py +++ b/synapse/storage/databases/main/__init__.py @@ -21,6 +21,7 @@ from typing import List, Optional, Tuple from synapse.api.constants import PresenceState from synapse.config.homeserver import HomeServerConfig from synapse.storage.database import DatabasePool +from synapse.storage.databases.main.stats import UserSortOrder from synapse.storage.engines import PostgresEngine from synapse.storage.util.id_generators import ( IdGenerator, @@ -292,6 +293,8 @@ class DataStore( name: Optional[str] = None, guests: bool = True, deactivated: bool = False, + order_by: UserSortOrder = UserSortOrder.USER_ID.value, + direction: str = "f", ) -> Tuple[List[JsonDict], int]: """Function to retrieve a paginated list of users from users list. This will return a json list of users and the @@ -304,6 +307,8 @@ class DataStore( name: search for local part of user_id or display name guests: whether to in include guest users deactivated: whether to include deactivated users + order_by: the sort order of the returned list + direction: sort ascending or descending Returns: A tuple of a list of mappings from user to information and a count of total users. """ @@ -312,6 +317,14 @@ class DataStore( filters = [] args = [self.hs.config.server_name] + # Set ordering + order_by_column = UserSortOrder(order_by).value + + if direction == "b": + order = "DESC" + else: + order = "ASC" + # `name` is in database already in lower case if name: filters.append("(name LIKE ? OR LOWER(displayname) LIKE ?)") @@ -339,10 +352,15 @@ class DataStore( txn.execute(sql, args) count = txn.fetchone()[0] - sql = ( - "SELECT name, user_type, is_guest, admin, deactivated, shadow_banned, displayname, avatar_url " - + sql_base - + " ORDER BY u.name LIMIT ? OFFSET ?" + sql = """ + SELECT name, user_type, is_guest, admin, deactivated, shadow_banned, displayname, avatar_url + {sql_base} + ORDER BY {order_by_column} {order}, u.name ASC + LIMIT ? OFFSET ? + """.format( + sql_base=sql_base, + order_by_column=order_by_column, + order=order, ) args += [limit, start] txn.execute(sql, args) diff --git a/synapse/storage/databases/main/stats.py b/synapse/storage/databases/main/stats.py index 1c99393c65..bce8946c21 100644 --- a/synapse/storage/databases/main/stats.py +++ b/synapse/storage/databases/main/stats.py @@ -66,18 +66,37 @@ TYPE_TO_ORIGIN_TABLE = {"room": ("rooms", "room_id"), "user": ("users", "name")} class UserSortOrder(Enum): """ Enum to define the sorting method used when returning users - with get_users_media_usage_paginate + with get_users_paginate in __init__.py + and get_users_media_usage_paginate in stats.py - MEDIA_LENGTH = ordered by size of uploaded media. Smallest to largest. - MEDIA_COUNT = ordered by number of uploaded media. Smallest to largest. + When moves this to __init__.py gets `builtins.ImportError` with + `most likely due to a circular import` + + MEDIA_LENGTH = ordered by size of uploaded media. + MEDIA_COUNT = ordered by number of uploaded media. USER_ID = ordered alphabetically by `user_id`. + NAME = ordered alphabetically by `user_id`. This is for compatibility reasons, + as the user_id is returned in the name field in the response in list users admin API. DISPLAYNAME = ordered alphabetically by `displayname` + GUEST = ordered by `is_guest` + ADMIN = ordered by `admin` + DEACTIVATED = ordered by `deactivated` + USER_TYPE = ordered alphabetically by `user_type` + AVATAR_URL = ordered alphabetically by `avatar_url` + SHADOW_BANNED = ordered by `shadow_banned` """ MEDIA_LENGTH = "media_length" MEDIA_COUNT = "media_count" USER_ID = "user_id" + NAME = "name" DISPLAYNAME = "displayname" + GUEST = "is_guest" + ADMIN = "admin" + DEACTIVATED = "deactivated" + USER_TYPE = "user_type" + AVATAR_URL = "avatar_url" + SHADOW_BANNED = "shadow_banned" class StatsStore(StateDeltasStore): diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index cf61f284cb..0c9ec133c2 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -28,7 +28,7 @@ from synapse.api.errors import Codes, HttpResponseException, ResourceLimitError from synapse.api.room_versions import RoomVersions from synapse.rest.client.v1 import login, logout, profile, room from synapse.rest.client.v2_alpha import devices, sync -from synapse.types import JsonDict +from synapse.types import JsonDict, UserID from tests import unittest from tests.server import FakeSite, make_request @@ -467,6 +467,8 @@ class UsersListTestCase(unittest.HomeserverTestCase): url = "/_synapse/admin/v2/users" def prepare(self, reactor, clock, hs): + self.store = hs.get_datastore() + self.admin_user = self.register_user("admin", "pass", admin=True) self.admin_user_tok = self.login("admin", "pass") @@ -634,6 +636,26 @@ class UsersListTestCase(unittest.HomeserverTestCase): self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"]) self.assertEqual(Codes.UNKNOWN, channel.json_body["errcode"]) + # unkown order_by + channel = self.make_request( + "GET", + self.url + "?order_by=bar", + access_token=self.admin_user_tok, + ) + + self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(Codes.UNKNOWN, channel.json_body["errcode"]) + + # invalid search order + channel = self.make_request( + "GET", + self.url + "?dir=bar", + access_token=self.admin_user_tok, + ) + + self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(Codes.UNKNOWN, channel.json_body["errcode"]) + def test_limit(self): """ Testing list of users with limit @@ -759,6 +781,103 @@ class UsersListTestCase(unittest.HomeserverTestCase): self.assertEqual(len(channel.json_body["users"]), 1) self.assertNotIn("next_token", channel.json_body) + def test_order_by(self): + """ + Testing order list with parameter `order_by` + """ + + user1 = self.register_user("user1", "pass1", admin=False, displayname="Name Z") + user2 = self.register_user("user2", "pass2", admin=False, displayname="Name Y") + + # Modify user + self.get_success(self.store.set_user_deactivated_status(user1, True)) + self.get_success(self.store.set_shadow_banned(UserID.from_string(user1), True)) + + # Set avatar URL to all users, that no user has a NULL value to avoid + # different sort order between SQlite and PostreSQL + self.get_success(self.store.set_profile_avatar_url("user1", "mxc://url3")) + self.get_success(self.store.set_profile_avatar_url("user2", "mxc://url2")) + self.get_success(self.store.set_profile_avatar_url("admin", "mxc://url1")) + + # order by default (name) + self._order_test([self.admin_user, user1, user2], None) + self._order_test([self.admin_user, user1, user2], None, "f") + self._order_test([user2, user1, self.admin_user], None, "b") + + # order by name + self._order_test([self.admin_user, user1, user2], "name") + self._order_test([self.admin_user, user1, user2], "name", "f") + self._order_test([user2, user1, self.admin_user], "name", "b") + + # order by displayname + self._order_test([user2, user1, self.admin_user], "displayname") + self._order_test([user2, user1, self.admin_user], "displayname", "f") + self._order_test([self.admin_user, user1, user2], "displayname", "b") + + # order by is_guest + # like sort by ascending name, as no guest user here + self._order_test([self.admin_user, user1, user2], "is_guest") + self._order_test([self.admin_user, user1, user2], "is_guest", "f") + self._order_test([self.admin_user, user1, user2], "is_guest", "b") + + # order by admin + self._order_test([user1, user2, self.admin_user], "admin") + self._order_test([user1, user2, self.admin_user], "admin", "f") + self._order_test([self.admin_user, user1, user2], "admin", "b") + + # order by deactivated + self._order_test([self.admin_user, user2, user1], "deactivated") + self._order_test([self.admin_user, user2, user1], "deactivated", "f") + self._order_test([user1, self.admin_user, user2], "deactivated", "b") + + # order by user_type + # like sort by ascending name, as no special user type here + self._order_test([self.admin_user, user1, user2], "user_type") + self._order_test([self.admin_user, user1, user2], "user_type", "f") + self._order_test([self.admin_user, user1, user2], "is_guest", "b") + + # order by shadow_banned + self._order_test([self.admin_user, user2, user1], "shadow_banned") + self._order_test([self.admin_user, user2, user1], "shadow_banned", "f") + self._order_test([user1, self.admin_user, user2], "shadow_banned", "b") + + # order by avatar_url + self._order_test([self.admin_user, user2, user1], "avatar_url") + self._order_test([self.admin_user, user2, user1], "avatar_url", "f") + self._order_test([user1, user2, self.admin_user], "avatar_url", "b") + + def _order_test( + self, + expected_user_list: List[str], + order_by: Optional[str], + dir: Optional[str] = None, + ): + """Request the list of users in a certain order. Assert that order is what + we expect + Args: + expected_user_list: The list of user_id in the order we expect to get + back from the server + order_by: The type of ordering to give the server + dir: The direction of ordering to give the server + """ + + url = self.url + "?deactivated=true&" + if order_by is not None: + url += "order_by=%s&" % (order_by,) + if dir is not None and dir in ("b", "f"): + url += "dir=%s" % (dir,) + channel = self.make_request( + "GET", + url.encode("ascii"), + access_token=self.admin_user_tok, + ) + self.assertEqual(200, channel.code, msg=channel.json_body) + self.assertEqual(channel.json_body["total"], len(expected_user_list)) + + returned_order = [row["name"] for row in channel.json_body["users"]] + self.assertEqual(expected_user_list, returned_order) + self._check_fields(channel.json_body["users"]) + def _check_fields(self, content: JsonDict): """Checks that the expected user attributes are present in content Args: -- cgit 1.5.1 From 44bb881096d7ad4d730e2dc31e0094c0324e0970 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 6 Apr 2021 08:58:18 -0400 Subject: Add type hints to expiring cache. (#9730) --- changelog.d/9730.misc | 1 + synapse/federation/federation_client.py | 2 +- synapse/handlers/device.py | 4 +- synapse/handlers/e2e_keys.py | 12 ---- synapse/handlers/sync.py | 10 ++-- synapse/rest/media/v1/preview_url_resource.py | 2 +- synapse/state/__init__.py | 5 +- synapse/util/caches/expiringcache.py | 83 ++++++++++++++++----------- 8 files changed, 65 insertions(+), 54 deletions(-) create mode 100644 changelog.d/9730.misc (limited to 'synapse/rest') diff --git a/changelog.d/9730.misc b/changelog.d/9730.misc new file mode 100644 index 0000000000..8063059b0b --- /dev/null +++ b/changelog.d/9730.misc @@ -0,0 +1 @@ +Add type hints to expiring cache. diff --git a/synapse/federation/federation_client.py b/synapse/federation/federation_client.py index afdb5bf2fa..55533d7501 100644 --- a/synapse/federation/federation_client.py +++ b/synapse/federation/federation_client.py @@ -102,7 +102,7 @@ class FederationClient(FederationBase): max_len=1000, expiry_ms=120 * 1000, reset_expiry_on_get=False, - ) + ) # type: ExpiringCache[str, EventBase] def _clear_tried_cache(self): """Clear pdu_destination_tried cache""" diff --git a/synapse/handlers/device.py b/synapse/handlers/device.py index 54293d0b9c..7e76db3e2a 100644 --- a/synapse/handlers/device.py +++ b/synapse/handlers/device.py @@ -631,7 +631,7 @@ class DeviceListUpdater: max_len=10000, expiry_ms=30 * 60 * 1000, iterable=True, - ) + ) # type: ExpiringCache[str, Set[str]] # Attempt to resync out of sync device lists every 30s. self._resync_retry_in_progress = False @@ -760,7 +760,7 @@ class DeviceListUpdater: """Given a list of updates for a user figure out if we need to do a full resync, or whether we have enough data that we can just apply the delta. """ - seen_updates = self._seen_updates.get(user_id, set()) + seen_updates = self._seen_updates.get(user_id, set()) # type: Set[str] extremity = await self.store.get_device_list_last_stream_id_for_remote(user_id) diff --git a/synapse/handlers/e2e_keys.py b/synapse/handlers/e2e_keys.py index 739653a3fa..92b18378fc 100644 --- a/synapse/handlers/e2e_keys.py +++ b/synapse/handlers/e2e_keys.py @@ -38,7 +38,6 @@ from synapse.types import ( ) from synapse.util import json_decoder, unwrapFirstError from synapse.util.async_helpers import Linearizer -from synapse.util.caches.expiringcache import ExpiringCache from synapse.util.retryutils import NotRetryingDestination if TYPE_CHECKING: @@ -1292,17 +1291,6 @@ class SigningKeyEduUpdater: # user_id -> list of updates waiting to be handled. self._pending_updates = {} # type: Dict[str, List[Tuple[JsonDict, JsonDict]]] - # Recently seen stream ids. We don't bother keeping these in the DB, - # but they're useful to have them about to reduce the number of spurious - # resyncs. - self._seen_updates = ExpiringCache( - cache_name="signing_key_update_edu", - clock=self.clock, - max_len=10000, - expiry_ms=30 * 60 * 1000, - iterable=True, - ) - async def incoming_signing_key_update( self, origin: str, edu_content: JsonDict ) -> None: diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index 7b356ba7e5..ff11266c67 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -252,13 +252,13 @@ class SyncHandler: self.storage = hs.get_storage() self.state_store = self.storage.state - # ExpiringCache((User, Device)) -> LruCache(state_key => event_id) + # ExpiringCache((User, Device)) -> LruCache(user_id => event_id) self.lazy_loaded_members_cache = ExpiringCache( "lazy_loaded_members_cache", self.clock, max_len=0, expiry_ms=LAZY_LOADED_MEMBERS_CACHE_MAX_AGE, - ) + ) # type: ExpiringCache[Tuple[str, Optional[str]], LruCache[str, str]] async def wait_for_sync_for_user( self, @@ -733,8 +733,10 @@ class SyncHandler: def get_lazy_loaded_members_cache( self, cache_key: Tuple[str, Optional[str]] - ) -> LruCache: - cache = self.lazy_loaded_members_cache.get(cache_key) + ) -> LruCache[str, str]: + cache = self.lazy_loaded_members_cache.get( + cache_key + ) # type: Optional[LruCache[str, str]] if cache is None: logger.debug("creating LruCache for %r", cache_key) cache = LruCache(LAZY_LOADED_MEMBERS_CACHE_MAX_SIZE) diff --git a/synapse/rest/media/v1/preview_url_resource.py b/synapse/rest/media/v1/preview_url_resource.py index c4ed9dfdb4..814145a04a 100644 --- a/synapse/rest/media/v1/preview_url_resource.py +++ b/synapse/rest/media/v1/preview_url_resource.py @@ -175,7 +175,7 @@ class PreviewUrlResource(DirectServeJsonResource): clock=self.clock, # don't spider URLs more often than once an hour expiry_ms=ONE_HOUR, - ) + ) # type: ExpiringCache[str, ObservableDeferred] if self._worker_run_media_background_jobs: self._cleaner_loop = self.clock.looping_call( diff --git a/synapse/state/__init__.py b/synapse/state/__init__.py index c3d6e80c49..c0f79ffdc8 100644 --- a/synapse/state/__init__.py +++ b/synapse/state/__init__.py @@ -22,6 +22,7 @@ from typing import ( Callable, DefaultDict, Dict, + FrozenSet, Iterable, List, Optional, @@ -515,7 +516,7 @@ class StateResolutionHandler: expiry_ms=EVICTION_TIMEOUT_SECONDS * 1000, iterable=True, reset_expiry_on_get=True, - ) + ) # type: ExpiringCache[FrozenSet[int], _StateCacheEntry] # # stuff for tracking time spent on state-res by room @@ -536,7 +537,7 @@ class StateResolutionHandler: state_groups_ids: Dict[int, StateMap[str]], event_map: Optional[Dict[str, EventBase]], state_res_store: "StateResolutionStore", - ): + ) -> _StateCacheEntry: """Resolves conflicts between a set of state groups Always generates a new state group (unless we hit the cache), so should diff --git a/synapse/util/caches/expiringcache.py b/synapse/util/caches/expiringcache.py index e15f7ee698..4dc3477e89 100644 --- a/synapse/util/caches/expiringcache.py +++ b/synapse/util/caches/expiringcache.py @@ -15,40 +15,50 @@ import logging from collections import OrderedDict +from typing import Any, Generic, Optional, TypeVar, Union, overload + +import attr +from typing_extensions import Literal from synapse.config import cache as cache_config from synapse.metrics.background_process_metrics import run_as_background_process +from synapse.util import Clock from synapse.util.caches import register_cache logger = logging.getLogger(__name__) -SENTINEL = object() +SENTINEL = object() # type: Any + +T = TypeVar("T") +KT = TypeVar("KT") +VT = TypeVar("VT") -class ExpiringCache: + +class ExpiringCache(Generic[KT, VT]): def __init__( self, - cache_name, - clock, - max_len=0, - expiry_ms=0, - reset_expiry_on_get=False, - iterable=False, + cache_name: str, + clock: Clock, + max_len: int = 0, + expiry_ms: int = 0, + reset_expiry_on_get: bool = False, + iterable: bool = False, ): """ Args: - cache_name (str): Name of this cache, used for logging. - clock (Clock) - max_len (int): Max size of dict. If the dict grows larger than this + cache_name: Name of this cache, used for logging. + clock + max_len: Max size of dict. If the dict grows larger than this then the oldest items get automatically evicted. Default is 0, which indicates there is no max limit. - expiry_ms (int): How long before an item is evicted from the cache + expiry_ms: How long before an item is evicted from the cache in milliseconds. Default is 0, indicating items never get evicted based on time. - reset_expiry_on_get (bool): If true, will reset the expiry time for + reset_expiry_on_get: If true, will reset the expiry time for an item on access. Defaults to False. - iterable (bool): If true, the size is calculated by summing the + iterable: If true, the size is calculated by summing the sizes of all entries, rather than the number of entries. """ self._cache_name = cache_name @@ -62,7 +72,7 @@ class ExpiringCache: self._expiry_ms = expiry_ms self._reset_expiry_on_get = reset_expiry_on_get - self._cache = OrderedDict() + self._cache = OrderedDict() # type: OrderedDict[KT, _CacheEntry] self.iterable = iterable @@ -79,12 +89,12 @@ class ExpiringCache: self._clock.looping_call(f, self._expiry_ms / 2) - def __setitem__(self, key, value): + def __setitem__(self, key: KT, value: VT) -> None: now = self._clock.time_msec() self._cache[key] = _CacheEntry(now, value) self.evict() - def evict(self): + def evict(self) -> None: # Evict if there are now too many items while self._max_size and len(self) > self._max_size: _key, value = self._cache.popitem(last=False) @@ -93,7 +103,7 @@ class ExpiringCache: else: self.metrics.inc_evictions() - def __getitem__(self, key): + def __getitem__(self, key: KT) -> VT: try: entry = self._cache[key] self.metrics.inc_hits() @@ -106,7 +116,7 @@ class ExpiringCache: return entry.value - def pop(self, key, default=SENTINEL): + def pop(self, key: KT, default: T = SENTINEL) -> Union[VT, T]: """Removes and returns the value with the given key from the cache. If the key isn't in the cache then `default` will be returned if @@ -115,29 +125,40 @@ class ExpiringCache: Identical functionality to `dict.pop(..)`. """ - value = self._cache.pop(key, default) + value = self._cache.pop(key, SENTINEL) + # The key was not found. if value is SENTINEL: - raise KeyError(key) + if default is SENTINEL: + raise KeyError(key) + return default - return value + return value.value - def __contains__(self, key): + def __contains__(self, key: KT) -> bool: return key in self._cache - def get(self, key, default=None): + @overload + def get(self, key: KT, default: Literal[None] = None) -> Optional[VT]: + ... + + @overload + def get(self, key: KT, default: T) -> Union[VT, T]: + ... + + def get(self, key: KT, default: Optional[T] = None) -> Union[VT, Optional[T]]: try: return self[key] except KeyError: return default - def setdefault(self, key, value): + def setdefault(self, key: KT, value: VT) -> VT: try: return self[key] except KeyError: self[key] = value return value - def _prune_cache(self): + def _prune_cache(self) -> None: if not self._expiry_ms: # zero expiry time means don't expire. This should never get called # since we have this check in start too. @@ -166,7 +187,7 @@ class ExpiringCache: len(self), ) - def __len__(self): + def __len__(self) -> int: if self.iterable: return sum(len(entry.value) for entry in self._cache.values()) else: @@ -190,9 +211,7 @@ class ExpiringCache: return False +@attr.s(slots=True) class _CacheEntry: - __slots__ = ["time", "value"] - - def __init__(self, time, value): - self.time = time - self.value = value + time = attr.ib(type=int) + value = attr.ib() -- cgit 1.5.1 From 48a1f4db313026c79fbc7d9d950175693368d98b Mon Sep 17 00:00:00 2001 From: Dirk Klimpel <5740567+dklimpel@users.noreply.github.com> Date: Fri, 9 Apr 2021 10:44:40 +0200 Subject: Remove old admin API `GET /_synapse/admin/v1/users/` (#9401) Related: #8334 Deprecated in: #9429 - Synapse 1.28.0 (2021-02-25) `GET /_synapse/admin/v1/users/` has no - unit tests - documentation API in v2 is available (#5925 - 12/2019, v1.7.0). API is misleading. It expects `user_id` and returns a list of all users. Signed-off-by: Dirk Klimpel dirk@klimpel.org --- UPGRADE.rst | 13 +++++++++++++ changelog.d/9401.removal | 1 + synapse/rest/admin/__init__.py | 2 -- synapse/rest/admin/users.py | 23 ----------------------- tests/storage/test_client_ips.py | 4 ++-- 5 files changed, 16 insertions(+), 27 deletions(-) create mode 100644 changelog.d/9401.removal (limited to 'synapse/rest') diff --git a/UPGRADE.rst b/UPGRADE.rst index ba488e1041..665821d4ef 100644 --- a/UPGRADE.rst +++ b/UPGRADE.rst @@ -85,6 +85,19 @@ for example: wget https://packages.matrix.org/debian/pool/main/m/matrix-synapse-py3/matrix-synapse-py3_1.3.0+stretch1_amd64.deb dpkg -i matrix-synapse-py3_1.3.0+stretch1_amd64.deb +Upgrading to v1.32.0 +==================== + +Removal of old List Accounts Admin API +-------------------------------------- + +The deprecated v1 "list accounts" admin API (``GET /_synapse/admin/v1/users/``) has been removed in this version. + +The `v2 list accounts API `_ +has been available since Synapse 1.7.0 (2019-12-13), and is accessible under ``GET /_synapse/admin/v2/users``. + +The deprecation of the old endpoint was announced with Synapse 1.28.0 (released on 2021-02-25). + Upgrading to v1.29.0 ==================== diff --git a/changelog.d/9401.removal b/changelog.d/9401.removal new file mode 100644 index 0000000000..9c813e0215 --- /dev/null +++ b/changelog.d/9401.removal @@ -0,0 +1 @@ +Remove old admin API `GET /_synapse/admin/v1/users/`. \ No newline at end of file diff --git a/synapse/rest/admin/__init__.py b/synapse/rest/admin/__init__.py index 8457db1e22..5daa795df1 100644 --- a/synapse/rest/admin/__init__.py +++ b/synapse/rest/admin/__init__.py @@ -62,7 +62,6 @@ from synapse.rest.admin.users import ( UserMembershipRestServlet, UserRegisterServlet, UserRestServletV2, - UsersRestServlet, UsersRestServletV2, UserTokenRestServlet, WhoisRestServlet, @@ -248,7 +247,6 @@ def register_servlets_for_client_rest_resource(hs, http_server): PurgeHistoryStatusRestServlet(hs).register(http_server) DeactivateAccountRestServlet(hs).register(http_server) PurgeHistoryRestServlet(hs).register(http_server) - UsersRestServlet(hs).register(http_server) ResetPasswordRestServlet(hs).register(http_server) SearchUsersRestServlet(hs).register(http_server) ShutdownRoomRestServlet(hs).register(http_server) diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index fa7804583a..595898c259 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -45,29 +45,6 @@ if TYPE_CHECKING: logger = logging.getLogger(__name__) -class UsersRestServlet(RestServlet): - PATTERNS = admin_patterns("/users/(?P[^/]*)$") - - def __init__(self, hs: "HomeServer"): - self.hs = hs - self.store = hs.get_datastore() - self.auth = hs.get_auth() - self.admin_handler = hs.get_admin_handler() - - async def on_GET( - self, request: SynapseRequest, user_id: str - ) -> Tuple[int, List[JsonDict]]: - target_user = UserID.from_string(user_id) - await assert_requester_is_admin(self.auth, request) - - if not self.hs.is_mine(target_user): - raise SynapseError(400, "Can only users a local user") - - ret = await self.store.get_users() - - return 200, ret - - class UsersRestServletV2(RestServlet): PATTERNS = admin_patterns("/users$", "v2") diff --git a/tests/storage/test_client_ips.py b/tests/storage/test_client_ips.py index 34e6526097..a8a6ddc466 100644 --- a/tests/storage/test_client_ips.py +++ b/tests/storage/test_client_ips.py @@ -390,7 +390,7 @@ class ClientIpStoreTestCase(unittest.HomeserverTestCase): class ClientIpAuthTestCase(unittest.HomeserverTestCase): servlets = [ - synapse.rest.admin.register_servlets_for_client_rest_resource, + synapse.rest.admin.register_servlets, login.register_servlets, ] @@ -434,7 +434,7 @@ class ClientIpAuthTestCase(unittest.HomeserverTestCase): self.reactor, self.site, "GET", - "/_synapse/admin/v1/users/" + self.user_id, + "/_synapse/admin/v2/users/" + self.user_id, access_token=access_token, custom_headers=headers1.items(), **make_request_args, -- cgit 1.5.1 From e300ef64b16280ce2fdb7a8a9baef68b08aa9c88 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Mon, 12 Apr 2021 15:13:55 +0100 Subject: Require AppserviceRegistrationType (#9548) This change ensures that the appservice registration behaviour follows the spec. We decided to do this for Dendrite, so it made sense to also make a PR for synapse to correct the behaviour. --- changelog.d/9548.removal | 1 + synapse/api/constants.py | 5 +++++ synapse/rest/client/v2_alpha/register.py | 23 ++++++++++++++------- tests/rest/client/v2_alpha/test_register.py | 31 +++++++++++++++++++++++++---- tests/test_mau.py | 23 ++++++++++----------- 5 files changed, 60 insertions(+), 23 deletions(-) create mode 100644 changelog.d/9548.removal (limited to 'synapse/rest') diff --git a/changelog.d/9548.removal b/changelog.d/9548.removal new file mode 100644 index 0000000000..1fb88236c6 --- /dev/null +++ b/changelog.d/9548.removal @@ -0,0 +1 @@ +Make `/_matrix/client/r0/register` expect a type of `m.login.application_service` when an Application Service registers a user, to align with [the relevant spec](https://spec.matrix.org/unstable/application-service-api/#server-admin-style-permissions). diff --git a/synapse/api/constants.py b/synapse/api/constants.py index 6856dab06c..a8ae41de48 100644 --- a/synapse/api/constants.py +++ b/synapse/api/constants.py @@ -73,6 +73,11 @@ class LoginType: DUMMY = "m.login.dummy" +# This is used in the `type` parameter for /register when called by +# an appservice to register a new user. +APP_SERVICE_REGISTRATION_TYPE = "m.login.application_service" + + class EventTypes: Member = "m.room.member" Create = "m.room.create" diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py index c212da0cb2..4a064849c1 100644 --- a/synapse/rest/client/v2_alpha/register.py +++ b/synapse/rest/client/v2_alpha/register.py @@ -13,7 +13,6 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. - import hmac import logging import random @@ -22,7 +21,7 @@ from typing import List, Union import synapse import synapse.api.auth import synapse.types -from synapse.api.constants import LoginType +from synapse.api.constants import APP_SERVICE_REGISTRATION_TYPE, LoginType from synapse.api.errors import ( Codes, InteractiveAuthIncompleteError, @@ -430,15 +429,20 @@ class RegisterRestServlet(RestServlet): raise SynapseError(400, "Invalid username") desired_username = body["username"] - appservice = None - if self.auth.has_access_token(request): - appservice = self.auth.get_appservice_by_req(request) - # fork off as soon as possible for ASes which have completely # different registration flows to normal users # == Application Service Registration == - if appservice: + if body.get("type") == APP_SERVICE_REGISTRATION_TYPE: + if not self.auth.has_access_token(request): + raise SynapseError( + 400, + "Appservice token must be provided when using a type of m.login.application_service", + ) + + # Verify the AS + self.auth.get_appservice_by_req(request) + # Set the desired user according to the AS API (which uses the # 'user' key not 'username'). Since this is a new addition, we'll # fallback to 'username' if they gave one. @@ -459,6 +463,11 @@ class RegisterRestServlet(RestServlet): ) return 200, result + elif self.auth.has_access_token(request): + raise SynapseError( + 400, + "An access token should not be provided on requests to /register (except if type is m.login.application_service)", + ) # == Normal User Registration == (everyone else) if not self._registration_enabled: diff --git a/tests/rest/client/v2_alpha/test_register.py b/tests/rest/client/v2_alpha/test_register.py index 27db4f551e..cd60ea7081 100644 --- a/tests/rest/client/v2_alpha/test_register.py +++ b/tests/rest/client/v2_alpha/test_register.py @@ -14,7 +14,6 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. - import datetime import json import os @@ -22,7 +21,7 @@ import os import pkg_resources import synapse.rest.admin -from synapse.api.constants import LoginType +from synapse.api.constants import APP_SERVICE_REGISTRATION_TYPE, LoginType from synapse.api.errors import Codes from synapse.appservice import ApplicationService from synapse.rest.client.v1 import login, logout @@ -59,7 +58,9 @@ class RegisterRestServletTestCase(unittest.HomeserverTestCase): ) self.hs.get_datastore().services_cache.append(appservice) - request_data = json.dumps({"username": "as_user_kermit"}) + request_data = json.dumps( + {"username": "as_user_kermit", "type": APP_SERVICE_REGISTRATION_TYPE} + ) channel = self.make_request( b"POST", self.url + b"?access_token=i_am_an_app_service", request_data @@ -69,9 +70,31 @@ class RegisterRestServletTestCase(unittest.HomeserverTestCase): det_data = {"user_id": user_id, "home_server": self.hs.hostname} self.assertDictContainsSubset(det_data, channel.json_body) + def test_POST_appservice_registration_no_type(self): + as_token = "i_am_an_app_service" + + appservice = ApplicationService( + as_token, + self.hs.config.server_name, + id="1234", + namespaces={"users": [{"regex": r"@as_user.*", "exclusive": True}]}, + sender="@as:test", + ) + + self.hs.get_datastore().services_cache.append(appservice) + request_data = json.dumps({"username": "as_user_kermit"}) + + channel = self.make_request( + b"POST", self.url + b"?access_token=i_am_an_app_service", request_data + ) + + self.assertEquals(channel.result["code"], b"400", channel.result) + def test_POST_appservice_registration_invalid(self): self.appservice = None # no application service exists - request_data = json.dumps({"username": "kermit"}) + request_data = json.dumps( + {"username": "kermit", "type": APP_SERVICE_REGISTRATION_TYPE} + ) channel = self.make_request( b"POST", self.url + b"?access_token=i_am_an_app_service", request_data ) diff --git a/tests/test_mau.py b/tests/test_mau.py index 75d28a42df..7d92a16a8d 100644 --- a/tests/test_mau.py +++ b/tests/test_mau.py @@ -15,9 +15,7 @@ """Tests REST events for /rooms paths.""" -import json - -from synapse.api.constants import LoginType +from synapse.api.constants import APP_SERVICE_REGISTRATION_TYPE, LoginType from synapse.api.errors import Codes, HttpResponseException, SynapseError from synapse.appservice import ApplicationService from synapse.rest.client.v2_alpha import register, sync @@ -113,7 +111,7 @@ class TestMauLimit(unittest.HomeserverTestCase): ) ) - self.create_user("as_kermit4", token=as_token) + self.create_user("as_kermit4", token=as_token, appservice=True) def test_allowed_after_a_month_mau(self): # Create and sync so that the MAU counts get updated @@ -232,14 +230,15 @@ class TestMauLimit(unittest.HomeserverTestCase): self.reactor.advance(100) self.assertEqual(2, self.successResultOf(count)) - def create_user(self, localpart, token=None): - request_data = json.dumps( - { - "username": localpart, - "password": "monkey", - "auth": {"type": LoginType.DUMMY}, - } - ) + def create_user(self, localpart, token=None, appservice=False): + request_data = { + "username": localpart, + "password": "monkey", + "auth": {"type": LoginType.DUMMY}, + } + + if appservice: + request_data["type"] = APP_SERVICE_REGISTRATION_TYPE channel = self.make_request( "POST", -- cgit 1.5.1 From 1fc97ee876c6f383a6148897d82dbc58703ea9d1 Mon Sep 17 00:00:00 2001 From: Dirk Klimpel <5740567+dklimpel@users.noreply.github.com> Date: Tue, 13 Apr 2021 11:26:37 +0200 Subject: Add an admin API to manage ratelimit for a specific user (#9648) --- changelog.d/9648.feature | 1 + docs/admin_api/user_admin_api.rst | 117 +++++++++++++- synapse/rest/admin/__init__.py | 2 + synapse/rest/admin/users.py | 111 +++++++++++++ synapse/storage/databases/main/room.py | 64 +++++++- tests/rest/admin/test_user.py | 284 +++++++++++++++++++++++++++++++++ 6 files changed, 573 insertions(+), 6 deletions(-) create mode 100644 changelog.d/9648.feature (limited to 'synapse/rest') diff --git a/changelog.d/9648.feature b/changelog.d/9648.feature new file mode 100644 index 0000000000..bc77026039 --- /dev/null +++ b/changelog.d/9648.feature @@ -0,0 +1 @@ +Add an admin API to manage ratelimit for a specific user. \ No newline at end of file diff --git a/docs/admin_api/user_admin_api.rst b/docs/admin_api/user_admin_api.rst index a8a5a2628c..dbce9c90b6 100644 --- a/docs/admin_api/user_admin_api.rst +++ b/docs/admin_api/user_admin_api.rst @@ -202,7 +202,7 @@ The following fields are returned in the JSON response body: - ``users`` - An array of objects, each containing information about an user. User objects contain the following fields: - - ``name`` - string - Fully-qualified user ID (ex. `@user:server.com`). + - ``name`` - string - Fully-qualified user ID (ex. ``@user:server.com``). - ``is_guest`` - bool - Status if that user is a guest account. - ``admin`` - bool - Status if that user is a server administrator. - ``user_type`` - string - Type of the user. Normal users are type ``None``. @@ -864,3 +864,118 @@ The following parameters should be set in the URL: - ``user_id`` - The fully qualified MXID: for example, ``@user:server.com``. The user must be local. + +Override ratelimiting for users +=============================== + +This API allows to override or disable ratelimiting for a specific user. +There are specific APIs to set, get and delete a ratelimit. + +Get status of ratelimit +----------------------- + +The API is:: + + GET /_synapse/admin/v1/users//override_ratelimit + +To use it, you will need to authenticate by providing an ``access_token`` for a +server admin: see `README.rst `_. + +A response body like the following is returned: + +.. code:: json + + { + "messages_per_second": 0, + "burst_count": 0 + } + +**Parameters** + +The following parameters should be set in the URL: + +- ``user_id`` - The fully qualified MXID: for example, ``@user:server.com``. The user must + be local. + +**Response** + +The following fields are returned in the JSON response body: + +- ``messages_per_second`` - integer - The number of actions that can + be performed in a second. `0` mean that ratelimiting is disabled for this user. +- ``burst_count`` - integer - How many actions that can be performed before + being limited. + +If **no** custom ratelimit is set, an empty JSON dict is returned. + +.. code:: json + + {} + +Set ratelimit +------------- + +The API is:: + + POST /_synapse/admin/v1/users//override_ratelimit + +To use it, you will need to authenticate by providing an ``access_token`` for a +server admin: see `README.rst `_. + +A response body like the following is returned: + +.. code:: json + + { + "messages_per_second": 0, + "burst_count": 0 + } + +**Parameters** + +The following parameters should be set in the URL: + +- ``user_id`` - The fully qualified MXID: for example, ``@user:server.com``. The user must + be local. + +Body parameters: + +- ``messages_per_second`` - positive integer, optional. The number of actions that can + be performed in a second. Defaults to ``0``. +- ``burst_count`` - positive integer, optional. How many actions that can be performed + before being limited. Defaults to ``0``. + +To disable users' ratelimit set both values to ``0``. + +**Response** + +The following fields are returned in the JSON response body: + +- ``messages_per_second`` - integer - The number of actions that can + be performed in a second. +- ``burst_count`` - integer - How many actions that can be performed before + being limited. + +Delete ratelimit +---------------- + +The API is:: + + DELETE /_synapse/admin/v1/users//override_ratelimit + +To use it, you will need to authenticate by providing an ``access_token`` for a +server admin: see `README.rst `_. + +An empty JSON dict is returned. + +.. code:: json + + {} + +**Parameters** + +The following parameters should be set in the URL: + +- ``user_id`` - The fully qualified MXID: for example, ``@user:server.com``. The user must + be local. + diff --git a/synapse/rest/admin/__init__.py b/synapse/rest/admin/__init__.py index 5daa795df1..2dec818a5f 100644 --- a/synapse/rest/admin/__init__.py +++ b/synapse/rest/admin/__init__.py @@ -54,6 +54,7 @@ from synapse.rest.admin.users import ( AccountValidityRenewServlet, DeactivateAccountRestServlet, PushersRestServlet, + RateLimitRestServlet, ResetPasswordRestServlet, SearchUsersRestServlet, ShadowBanRestServlet, @@ -239,6 +240,7 @@ def register_servlets(hs, http_server): ShadowBanRestServlet(hs).register(http_server) ForwardExtremitiesRestServlet(hs).register(http_server) RoomEventContextServlet(hs).register(http_server) + RateLimitRestServlet(hs).register(http_server) def register_servlets_for_client_rest_resource(hs, http_server): diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index 595898c259..04990c71fb 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -981,3 +981,114 @@ class ShadowBanRestServlet(RestServlet): await self.store.set_shadow_banned(UserID.from_string(user_id), True) return 200, {} + + +class RateLimitRestServlet(RestServlet): + """An admin API to override ratelimiting for an user. + + Example: + POST /_synapse/admin/v1/users/@test:example.com/override_ratelimit + { + "messages_per_second": 0, + "burst_count": 0 + } + 200 OK + { + "messages_per_second": 0, + "burst_count": 0 + } + """ + + PATTERNS = admin_patterns("/users/(?P[^/]*)/override_ratelimit") + + def __init__(self, hs: "HomeServer"): + self.hs = hs + self.store = hs.get_datastore() + self.auth = hs.get_auth() + + async def on_GET( + self, request: SynapseRequest, user_id: str + ) -> Tuple[int, JsonDict]: + await assert_requester_is_admin(self.auth, request) + + if not self.hs.is_mine_id(user_id): + raise SynapseError(400, "Can only lookup local users") + + if not await self.store.get_user_by_id(user_id): + raise NotFoundError("User not found") + + ratelimit = await self.store.get_ratelimit_for_user(user_id) + + if ratelimit: + # convert `null` to `0` for consistency + # both values do the same in retelimit handler + ret = { + "messages_per_second": 0 + if ratelimit.messages_per_second is None + else ratelimit.messages_per_second, + "burst_count": 0 + if ratelimit.burst_count is None + else ratelimit.burst_count, + } + else: + ret = {} + + return 200, ret + + async def on_POST( + self, request: SynapseRequest, user_id: str + ) -> Tuple[int, JsonDict]: + await assert_requester_is_admin(self.auth, request) + + if not self.hs.is_mine_id(user_id): + raise SynapseError(400, "Only local users can be ratelimited") + + if not await self.store.get_user_by_id(user_id): + raise NotFoundError("User not found") + + body = parse_json_object_from_request(request, allow_empty_body=True) + + messages_per_second = body.get("messages_per_second", 0) + burst_count = body.get("burst_count", 0) + + if not isinstance(messages_per_second, int) or messages_per_second < 0: + raise SynapseError( + 400, + "%r parameter must be a positive int" % (messages_per_second,), + errcode=Codes.INVALID_PARAM, + ) + + if not isinstance(burst_count, int) or burst_count < 0: + raise SynapseError( + 400, + "%r parameter must be a positive int" % (burst_count,), + errcode=Codes.INVALID_PARAM, + ) + + await self.store.set_ratelimit_for_user( + user_id, messages_per_second, burst_count + ) + ratelimit = await self.store.get_ratelimit_for_user(user_id) + assert ratelimit is not None + + ret = { + "messages_per_second": ratelimit.messages_per_second, + "burst_count": ratelimit.burst_count, + } + + return 200, ret + + async def on_DELETE( + self, request: SynapseRequest, user_id: str + ) -> Tuple[int, JsonDict]: + await assert_requester_is_admin(self.auth, request) + + if not self.hs.is_mine_id(user_id): + raise SynapseError(400, "Only local users can be ratelimited") + + if not await self.store.get_user_by_id(user_id): + raise NotFoundError("User not found") + + await self.store.delete_ratelimit_for_user(user_id) + + return 200, {} diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index 9cbcd53026..47fb12f3f6 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -521,13 +521,11 @@ class RoomWorkerStore(SQLBaseStore): ) @cached(max_entries=10000) - async def get_ratelimit_for_user(self, user_id): - """Check if there are any overrides for ratelimiting for the given - user + async def get_ratelimit_for_user(self, user_id: str) -> Optional[RatelimitOverride]: + """Check if there are any overrides for ratelimiting for the given user Args: - user_id (str) - + user_id: user ID of the user Returns: RatelimitOverride if there is an override, else None. If the contents of RatelimitOverride are None or 0 then ratelimitng has been @@ -549,6 +547,62 @@ class RoomWorkerStore(SQLBaseStore): else: return None + async def set_ratelimit_for_user( + self, user_id: str, messages_per_second: int, burst_count: int + ) -> None: + """Sets whether a user is set an overridden ratelimit. + Args: + user_id: user ID of the user + messages_per_second: The number of actions that can be performed in a second. + burst_count: How many actions that can be performed before being limited. + """ + + def set_ratelimit_txn(txn): + self.db_pool.simple_upsert_txn( + txn, + table="ratelimit_override", + keyvalues={"user_id": user_id}, + values={ + "messages_per_second": messages_per_second, + "burst_count": burst_count, + }, + ) + + self._invalidate_cache_and_stream( + txn, self.get_ratelimit_for_user, (user_id,) + ) + + await self.db_pool.runInteraction("set_ratelimit", set_ratelimit_txn) + + async def delete_ratelimit_for_user(self, user_id: str) -> None: + """Delete an overridden ratelimit for a user. + Args: + user_id: user ID of the user + """ + + def delete_ratelimit_txn(txn): + row = self.db_pool.simple_select_one_txn( + txn, + table="ratelimit_override", + keyvalues={"user_id": user_id}, + retcols=["user_id"], + allow_none=True, + ) + + if not row: + return + + # They are there, delete them. + self.db_pool.simple_delete_one_txn( + txn, "ratelimit_override", keyvalues={"user_id": user_id} + ) + + self._invalidate_cache_and_stream( + txn, self.get_ratelimit_for_user, (user_id,) + ) + + await self.db_pool.runInteraction("delete_ratelimit", delete_ratelimit_txn) + @cached() async def get_retention_policy_for_room(self, room_id): """Get the retention policy for a given room. diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index e47fd3ded8..5070c96984 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -3011,3 +3011,287 @@ class ShadowBanRestTestCase(unittest.HomeserverTestCase): # Ensure the user is shadow-banned (and the cache was cleared). result = self.get_success(self.store.get_user_by_access_token(other_user_token)) self.assertTrue(result.shadow_banned) + + +class RateLimitTestCase(unittest.HomeserverTestCase): + + servlets = [ + synapse.rest.admin.register_servlets, + login.register_servlets, + ] + + def prepare(self, reactor, clock, hs): + self.store = hs.get_datastore() + + self.admin_user = self.register_user("admin", "pass", admin=True) + self.admin_user_tok = self.login("admin", "pass") + + self.other_user = self.register_user("user", "pass") + self.url = ( + "/_synapse/admin/v1/users/%s/override_ratelimit" + % urllib.parse.quote(self.other_user) + ) + + def test_no_auth(self): + """ + Try to get information of a user without authentication. + """ + channel = self.make_request("GET", self.url, b"{}") + + self.assertEqual(401, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(Codes.MISSING_TOKEN, channel.json_body["errcode"]) + + channel = self.make_request("POST", self.url, b"{}") + + self.assertEqual(401, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(Codes.MISSING_TOKEN, channel.json_body["errcode"]) + + channel = self.make_request("DELETE", self.url, b"{}") + + self.assertEqual(401, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(Codes.MISSING_TOKEN, channel.json_body["errcode"]) + + def test_requester_is_no_admin(self): + """ + If the user is not a server admin, an error is returned. + """ + other_user_token = self.login("user", "pass") + + channel = self.make_request( + "GET", + self.url, + access_token=other_user_token, + ) + + self.assertEqual(403, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(Codes.FORBIDDEN, channel.json_body["errcode"]) + + channel = self.make_request( + "POST", + self.url, + access_token=other_user_token, + ) + + self.assertEqual(403, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(Codes.FORBIDDEN, channel.json_body["errcode"]) + + channel = self.make_request( + "DELETE", + self.url, + access_token=other_user_token, + ) + + self.assertEqual(403, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(Codes.FORBIDDEN, channel.json_body["errcode"]) + + def test_user_does_not_exist(self): + """ + Tests that a lookup for a user that does not exist returns a 404 + """ + url = "/_synapse/admin/v1/users/@unknown_person:test/override_ratelimit" + + channel = self.make_request( + "GET", + url, + access_token=self.admin_user_tok, + ) + + self.assertEqual(404, channel.code, msg=channel.json_body) + self.assertEqual(Codes.NOT_FOUND, channel.json_body["errcode"]) + + channel = self.make_request( + "POST", + url, + access_token=self.admin_user_tok, + ) + + self.assertEqual(404, channel.code, msg=channel.json_body) + self.assertEqual(Codes.NOT_FOUND, channel.json_body["errcode"]) + + channel = self.make_request( + "DELETE", + url, + access_token=self.admin_user_tok, + ) + + self.assertEqual(404, channel.code, msg=channel.json_body) + self.assertEqual(Codes.NOT_FOUND, channel.json_body["errcode"]) + + def test_user_is_not_local(self): + """ + Tests that a lookup for a user that is not a local returns a 400 + """ + url = ( + "/_synapse/admin/v1/users/@unknown_person:unknown_domain/override_ratelimit" + ) + + channel = self.make_request( + "GET", + url, + access_token=self.admin_user_tok, + ) + + self.assertEqual(400, channel.code, msg=channel.json_body) + self.assertEqual("Can only lookup local users", channel.json_body["error"]) + + channel = self.make_request( + "POST", + url, + access_token=self.admin_user_tok, + ) + + self.assertEqual(400, channel.code, msg=channel.json_body) + self.assertEqual( + "Only local users can be ratelimited", channel.json_body["error"] + ) + + channel = self.make_request( + "DELETE", + url, + access_token=self.admin_user_tok, + ) + + self.assertEqual(400, channel.code, msg=channel.json_body) + self.assertEqual( + "Only local users can be ratelimited", channel.json_body["error"] + ) + + def test_invalid_parameter(self): + """ + If parameters are invalid, an error is returned. + """ + # messages_per_second is a string + channel = self.make_request( + "POST", + self.url, + access_token=self.admin_user_tok, + content={"messages_per_second": "string"}, + ) + + self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(Codes.INVALID_PARAM, channel.json_body["errcode"]) + + # messages_per_second is negative + channel = self.make_request( + "POST", + self.url, + access_token=self.admin_user_tok, + content={"messages_per_second": -1}, + ) + + self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(Codes.INVALID_PARAM, channel.json_body["errcode"]) + + # burst_count is a string + channel = self.make_request( + "POST", + self.url, + access_token=self.admin_user_tok, + content={"burst_count": "string"}, + ) + + self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(Codes.INVALID_PARAM, channel.json_body["errcode"]) + + # burst_count is negative + channel = self.make_request( + "POST", + self.url, + access_token=self.admin_user_tok, + content={"burst_count": -1}, + ) + + self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(Codes.INVALID_PARAM, channel.json_body["errcode"]) + + def test_return_zero_when_null(self): + """ + If values in database are `null` API should return an int `0` + """ + + self.get_success( + self.store.db_pool.simple_upsert( + table="ratelimit_override", + keyvalues={"user_id": self.other_user}, + values={ + "messages_per_second": None, + "burst_count": None, + }, + ) + ) + + # request status + channel = self.make_request( + "GET", + self.url, + access_token=self.admin_user_tok, + ) + self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(0, channel.json_body["messages_per_second"]) + self.assertEqual(0, channel.json_body["burst_count"]) + + def test_success(self): + """ + Rate-limiting (set/update/delete) should succeed for an admin. + """ + # request status + channel = self.make_request( + "GET", + self.url, + access_token=self.admin_user_tok, + ) + self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) + self.assertNotIn("messages_per_second", channel.json_body) + self.assertNotIn("burst_count", channel.json_body) + + # set ratelimit + channel = self.make_request( + "POST", + self.url, + access_token=self.admin_user_tok, + content={"messages_per_second": 10, "burst_count": 11}, + ) + self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(10, channel.json_body["messages_per_second"]) + self.assertEqual(11, channel.json_body["burst_count"]) + + # update ratelimit + channel = self.make_request( + "POST", + self.url, + access_token=self.admin_user_tok, + content={"messages_per_second": 20, "burst_count": 21}, + ) + self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(20, channel.json_body["messages_per_second"]) + self.assertEqual(21, channel.json_body["burst_count"]) + + # request status + channel = self.make_request( + "GET", + self.url, + access_token=self.admin_user_tok, + ) + self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(20, channel.json_body["messages_per_second"]) + self.assertEqual(21, channel.json_body["burst_count"]) + + # delete ratelimit + channel = self.make_request( + "DELETE", + self.url, + access_token=self.admin_user_tok, + ) + self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) + self.assertNotIn("messages_per_second", channel.json_body) + self.assertNotIn("burst_count", channel.json_body) + + # request status + channel = self.make_request( + "GET", + self.url, + access_token=self.admin_user_tok, + ) + self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) + self.assertNotIn("messages_per_second", channel.json_body) + self.assertNotIn("burst_count", channel.json_body) -- cgit 1.5.1