From 8dd2ea65a9566fd0850df0d989f700f61b490ed9 Mon Sep 17 00:00:00 2001 From: Dirk Klimpel <5740567+dklimpel@users.noreply.github.com> Date: Thu, 18 Mar 2021 17:54:08 +0100 Subject: Consistently check whether a password may be set for a user. (#9636) --- synapse/rest/admin/users.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'synapse/rest/admin/users.py') diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index 2c89b62e25..aaa56a7024 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -271,7 +271,7 @@ class UserRestServletV2(RestServlet): elif not deactivate and user["deactivated"]: if ( "password" not in body - and self.hs.config.password_localdb_enabled + and self.auth_handler.can_change_password() ): raise SynapseError( 400, "Must provide a password to re-activate an account." -- cgit 1.5.1 From b5efcb577e2c9b8b38cb86f87cf65fa93eb2566b Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 26 Mar 2021 16:49:46 +0000 Subject: Make it possible to use dmypy (#9692) Running `dmypy run` will do a `mypy` check while spinning up a daemon that makes rerunning `dmypy run` a lot faster. `dmypy` doesn't support `follow_imports = silent` and has `local_partial_types` enabled, so this PR enables those options and fixes the issues that were newly raised. Note that `local_partial_types` will be enabled by default in upcoming mypy releases. --- changelog.d/9692.misc | 1 + mypy.ini | 3 ++- synapse/api/auth.py | 5 +++++ synapse/config/cache.py | 6 ++++-- synapse/handlers/oidc_handler.py | 3 +++ synapse/logging/opentracing.py | 2 +- synapse/replication/tcp/protocol.py | 2 +- synapse/rest/admin/rooms.py | 3 +++ synapse/rest/admin/users.py | 3 +++ synapse/rest/client/v2_alpha/sync.py | 3 +++ synapse/rest/media/v1/preview_url_resource.py | 2 ++ synapse/rest/synapse/client/pick_username.py | 3 +++ synapse/util/caches/__init__.py | 4 ++-- tests/replication/tcp/streams/test_typing.py | 1 + tests/replication/test_multi_media_repo.py | 4 ++-- tests/server.py | 28 +++++++++++++++++++-------- 16 files changed, 56 insertions(+), 17 deletions(-) create mode 100644 changelog.d/9692.misc (limited to 'synapse/rest/admin/users.py') diff --git a/changelog.d/9692.misc b/changelog.d/9692.misc new file mode 100644 index 0000000000..d02002586e --- /dev/null +++ b/changelog.d/9692.misc @@ -0,0 +1 @@ +Make it possible to use `dmypy`. diff --git a/mypy.ini b/mypy.ini index 709a8d07a5..3ae5d45787 100644 --- a/mypy.ini +++ b/mypy.ini @@ -1,12 +1,13 @@ [mypy] namespace_packages = True plugins = mypy_zope:plugin, scripts-dev/mypy_synapse_plugin.py -follow_imports = silent +follow_imports = normal check_untyped_defs = True show_error_codes = True show_traceback = True mypy_path = stubs warn_unreachable = True +local_partial_types = True # To find all folders that pass mypy you run: # diff --git a/synapse/api/auth.py b/synapse/api/auth.py index e10e33fd23..7d9930ae7b 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -558,6 +558,9 @@ class Auth: Returns: bool: False if no access_token was given, True otherwise. """ + # This will always be set by the time Twisted calls us. + assert request.args is not None + query_params = request.args.get(b"access_token") auth_headers = request.requestHeaders.getRawHeaders(b"Authorization") return bool(query_params) or bool(auth_headers) @@ -574,6 +577,8 @@ class Auth: MissingClientTokenError: If there isn't a single access_token in the request """ + # This will always be set by the time Twisted calls us. + assert request.args is not None auth_headers = request.requestHeaders.getRawHeaders(b"Authorization") query_params = request.args.get(b"access_token") diff --git a/synapse/config/cache.py b/synapse/config/cache.py index 8e03f14005..4e8abbf88a 100644 --- a/synapse/config/cache.py +++ b/synapse/config/cache.py @@ -24,7 +24,7 @@ from ._base import Config, ConfigError _CACHE_PREFIX = "SYNAPSE_CACHE_FACTOR" # Map from canonicalised cache name to cache. -_CACHES = {} +_CACHES = {} # type: Dict[str, Callable[[float], None]] # a lock on the contents of _CACHES _CACHES_LOCK = threading.Lock() @@ -59,7 +59,9 @@ def _canonicalise_cache_name(cache_name: str) -> str: return cache_name.lower() -def add_resizable_cache(cache_name: str, cache_resize_callback: Callable): +def add_resizable_cache( + cache_name: str, cache_resize_callback: Callable[[float], None] +): """Register a cache that's size can dynamically change Args: diff --git a/synapse/handlers/oidc_handler.py b/synapse/handlers/oidc_handler.py index bc3630e9e9..6624212d6f 100644 --- a/synapse/handlers/oidc_handler.py +++ b/synapse/handlers/oidc_handler.py @@ -149,6 +149,9 @@ class OidcHandler: Args: request: the incoming request from the browser. """ + # This will always be set by the time Twisted calls us. + assert request.args is not None + # The provider might redirect with an error. # In that case, just display it as-is. if b"error" in request.args: diff --git a/synapse/logging/opentracing.py b/synapse/logging/opentracing.py index 10bd4a1461..c6e6335740 100644 --- a/synapse/logging/opentracing.py +++ b/synapse/logging/opentracing.py @@ -262,7 +262,7 @@ logger = logging.getLogger(__name__) # Block everything by default # A regex which matches the server_names to expose traces for. # None means 'block everything'. -_homeserver_whitelist = None +_homeserver_whitelist = None # type: Optional[re.Pattern[str]] # Util methods diff --git a/synapse/replication/tcp/protocol.py b/synapse/replication/tcp/protocol.py index 825900f64c..e829add257 100644 --- a/synapse/replication/tcp/protocol.py +++ b/synapse/replication/tcp/protocol.py @@ -104,7 +104,7 @@ tcp_outbound_commands_counter = Counter( # A list of all connected protocols. This allows us to send metrics about the # connections. -connected_connections = [] +connected_connections = [] # type: List[BaseReplicationStreamProtocol] logger = logging.getLogger(__name__) diff --git a/synapse/rest/admin/rooms.py b/synapse/rest/admin/rooms.py index 263d8ec076..cfe1bebb91 100644 --- a/synapse/rest/admin/rooms.py +++ b/synapse/rest/admin/rooms.py @@ -390,6 +390,9 @@ class JoinRoomAliasServlet(ResolveRoomIdMixin, RestServlet): async def on_POST( self, request: SynapseRequest, room_identifier: str ) -> Tuple[int, JsonDict]: + # This will always be set by the time Twisted calls us. + assert request.args is not None + requester = await self.auth.get_user_by_req(request) await assert_user_is_admin(self.auth, requester.user) diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index aaa56a7024..309bd2771b 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -833,6 +833,9 @@ class UserMediaRestServlet(RestServlet): async def on_GET( self, request: SynapseRequest, user_id: str ) -> Tuple[int, JsonDict]: + # This will always be set by the time Twisted calls us. + assert request.args is not None + await assert_requester_is_admin(self.auth, request) if not self.is_mine(UserID.from_string(user_id)): diff --git a/synapse/rest/client/v2_alpha/sync.py b/synapse/rest/client/v2_alpha/sync.py index a0db0a054b..3481770c83 100644 --- a/synapse/rest/client/v2_alpha/sync.py +++ b/synapse/rest/client/v2_alpha/sync.py @@ -91,6 +91,9 @@ class SyncRestServlet(RestServlet): self._event_serializer = hs.get_event_client_serializer() async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]: + # This will always be set by the time Twisted calls us. + assert request.args is not None + if b"from" in request.args: # /events used to use 'from', but /sync uses 'since'. # Lets be helpful and whine if we see a 'from'. diff --git a/synapse/rest/media/v1/preview_url_resource.py b/synapse/rest/media/v1/preview_url_resource.py index e590a0deab..c4ed9dfdb4 100644 --- a/synapse/rest/media/v1/preview_url_resource.py +++ b/synapse/rest/media/v1/preview_url_resource.py @@ -187,6 +187,8 @@ class PreviewUrlResource(DirectServeJsonResource): respond_with_json(request, 200, {}, send_cors=True) async def _async_render_GET(self, request: SynapseRequest) -> None: + # This will always be set by the time Twisted calls us. + assert request.args is not None # XXX: if get_user_by_req fails, what should we do in an async render? requester = await self.auth.get_user_by_req(request) diff --git a/synapse/rest/synapse/client/pick_username.py b/synapse/rest/synapse/client/pick_username.py index 51acaa9a92..d9ffe84489 100644 --- a/synapse/rest/synapse/client/pick_username.py +++ b/synapse/rest/synapse/client/pick_username.py @@ -104,6 +104,9 @@ class AccountDetailsResource(DirectServeHtmlResource): respond_with_html(request, 200, html) async def _async_render_POST(self, request: SynapseRequest): + # This will always be set by the time Twisted calls us. + assert request.args is not None + try: session_id = get_username_mapping_session_cookie_from_request(request) except SynapseError as e: diff --git a/synapse/util/caches/__init__.py b/synapse/util/caches/__init__.py index f968706334..48f64eeb38 100644 --- a/synapse/util/caches/__init__.py +++ b/synapse/util/caches/__init__.py @@ -25,8 +25,8 @@ from synapse.config.cache import add_resizable_cache logger = logging.getLogger(__name__) -caches_by_name = {} -collectors_by_name = {} # type: Dict +caches_by_name = {} # type: Dict[str, Sized] +collectors_by_name = {} # type: Dict[str, CacheMetric] cache_size = Gauge("synapse_util_caches_cache:size", "", ["name"]) cache_hits = Gauge("synapse_util_caches_cache:hits", "", ["name"]) diff --git a/tests/replication/tcp/streams/test_typing.py b/tests/replication/tcp/streams/test_typing.py index 5acfb3e53e..ca49d4dd3a 100644 --- a/tests/replication/tcp/streams/test_typing.py +++ b/tests/replication/tcp/streams/test_typing.py @@ -69,6 +69,7 @@ class TypingStreamTestCase(BaseStreamTestCase): self.assert_request_is_get_repl_stream_updates(request, "typing") # The from token should be the token from the last RDATA we got. + assert request.args is not None self.assertEqual(int(request.args[b"from_token"][0]), token) self.test_handler.on_rdata.assert_called_once() diff --git a/tests/replication/test_multi_media_repo.py b/tests/replication/test_multi_media_repo.py index 7ff11cde10..b0800f9840 100644 --- a/tests/replication/test_multi_media_repo.py +++ b/tests/replication/test_multi_media_repo.py @@ -15,7 +15,7 @@ import logging import os from binascii import unhexlify -from typing import Tuple +from typing import Optional, Tuple from twisted.internet.protocol import Factory from twisted.protocols.tls import TLSMemoryBIOFactory @@ -32,7 +32,7 @@ from tests.server import FakeChannel, FakeSite, FakeTransport, make_request logger = logging.getLogger(__name__) -test_server_connection_factory = None +test_server_connection_factory = None # type: Optional[TestServerTLSConnectionFactory] class MediaRepoShardTestCase(BaseMultiWorkerStreamTestCase): diff --git a/tests/server.py b/tests/server.py index 57cc4ac605..b535a5d886 100644 --- a/tests/server.py +++ b/tests/server.py @@ -2,7 +2,7 @@ import json import logging from collections import deque from io import SEEK_END, BytesIO -from typing import Callable, Iterable, MutableMapping, Optional, Tuple, Union +from typing import Callable, Dict, Iterable, MutableMapping, Optional, Tuple, Union import attr from typing_extensions import Deque @@ -13,8 +13,11 @@ from twisted.internet._resolver import SimpleResolverComplexifier from twisted.internet.defer import Deferred, fail, succeed from twisted.internet.error import DNSLookupError from twisted.internet.interfaces import ( + IHostnameResolver, + IProtocol, + IPullProducer, + IPushProducer, IReactorPluggableNameResolver, - IReactorTCP, IResolverSimple, ITransport, ) @@ -45,11 +48,11 @@ class FakeChannel: wire). """ - site = attr.ib(type=Site) + site = attr.ib(type=Union[Site, "FakeSite"]) _reactor = attr.ib() result = attr.ib(type=dict, default=attr.Factory(dict)) _ip = attr.ib(type=str, default="127.0.0.1") - _producer = None + _producer = None # type: Optional[Union[IPullProducer, IPushProducer]] @property def json_body(self): @@ -159,7 +162,11 @@ class FakeChannel: Any cookines found are added to the given dict """ - for h in self.headers.getRawHeaders("Set-Cookie"): + headers = self.headers.getRawHeaders("Set-Cookie") + if not headers: + return + + for h in headers: parts = h.split(";") k, v = parts[0].split("=", maxsplit=1) cookies[k] = v @@ -311,8 +318,8 @@ class ThreadedMemoryReactorClock(MemoryReactorClock): self._tcp_callbacks = {} self._udp = [] - lookups = self.lookups = {} - self._thread_callbacks = deque() # type: Deque[Callable[[], None]]() + lookups = self.lookups = {} # type: Dict[str, str] + self._thread_callbacks = deque() # type: Deque[Callable[[], None]] @implementer(IResolverSimple) class FakeResolver: @@ -324,6 +331,9 @@ class ThreadedMemoryReactorClock(MemoryReactorClock): self.nameResolver = SimpleResolverComplexifier(FakeResolver()) super().__init__() + def installNameResolver(self, resolver: IHostnameResolver) -> IHostnameResolver: + raise NotImplementedError() + def listenUDP(self, port, protocol, interface="", maxPacketSize=8196): p = udp.Port(port, protocol, interface, maxPacketSize, self) p.startListening() @@ -621,7 +631,9 @@ class FakeTransport: self.disconnected = True -def connect_client(reactor: IReactorTCP, client_id: int) -> AccumulatingProtocol: +def connect_client( + reactor: ThreadedMemoryReactorClock, client_id: int +) -> Tuple[IProtocol, AccumulatingProtocol]: """ Connect a client to a fake TCP transport. -- 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/admin/users.py') 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