From a9d4cbfaf16355ed702ef7ec3622ccee3adeb041 Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Mon, 1 Mar 2021 22:06:23 +0100 Subject: [PATCH 1/4] Add type hints to user admin API --- synapse/rest/admin/users.py | 84 ++++++++++++------- synapse/storage/databases/main/__init__.py | 10 +-- .../databases/main/media_repository.py | 2 +- 3 files changed, 61 insertions(+), 35 deletions(-) diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index 9c701c734873..148749c950e4 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -16,7 +16,7 @@ import hmac import logging from http import HTTPStatus -from typing import TYPE_CHECKING, Tuple +from typing import TYPE_CHECKING, Dict, Optional, Tuple from synapse.api.constants import UserTypes from synapse.api.errors import Codes, NotFoundError, SynapseError @@ -47,13 +47,15 @@ class UsersRestServlet(RestServlet): PATTERNS = admin_patterns("/users/(?P[^/]*)$") - def __init__(self, hs): + 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, user_id): + async def on_GET( + self, request: SynapseRequest, user_id: str + ) -> Tuple[int, JsonDict]: target_user = UserID.from_string(user_id) await assert_requester_is_admin(self.auth, request) @@ -153,7 +155,7 @@ class UserRestServletV2(RestServlet): otherwise an error. """ - def __init__(self, hs): + def __init__(self, hs: "HomeServer"): self.hs = hs self.auth = hs.get_auth() self.admin_handler = hs.get_admin_handler() @@ -165,7 +167,7 @@ def __init__(self, hs): self.registration_handler = hs.get_registration_handler() self.pusher_pool = hs.get_pusherpool() - async def on_GET(self, request, user_id): + async def on_GET(self, request: SynapseRequest, user_id) -> Tuple[int, JsonDict]: await assert_requester_is_admin(self.auth, request) target_user = UserID.from_string(user_id) @@ -179,7 +181,9 @@ async def on_GET(self, request, user_id): return 200, ret - async def on_PUT(self, request, user_id): + async def on_PUT( + self, request: SynapseRequest, user_id: str + ) -> Tuple[int, JsonDict]: requester = await self.auth.get_user_by_req(request) await assert_user_is_admin(self.auth, requester.user) @@ -272,7 +276,9 @@ async def on_PUT(self, request, user_id): target_user.to_string() ) - user = await self.admin_handler.get_user(target_user) + user = await self.admin_handler.get_user( + target_user + ) # type: JsonDict # type: ignore return 200, user else: # create user @@ -330,7 +336,9 @@ async def on_PUT(self, request, user_id): target_user, requester, body["avatar_url"], True ) - ret = await self.admin_handler.get_user(target_user) + ret = await self.admin_handler.get_user( + target_user + ) # type: JsonDict # type: ignore return 201, ret @@ -346,10 +354,10 @@ class UserRegisterServlet(RestServlet): PATTERNS = admin_patterns("/register") NONCE_TIMEOUT = 60 - def __init__(self, hs): + def __init__(self, hs: "HomeServer"): self.auth_handler = hs.get_auth_handler() self.reactor = hs.get_reactor() - self.nonces = {} + self.nonces = {} # type: Dict self.hs = hs def _clear_old_nonces(self): @@ -362,7 +370,7 @@ def _clear_old_nonces(self): if now - v > self.NONCE_TIMEOUT: del self.nonces[k] - def on_GET(self, request): + def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]: """ Generate a new nonce. """ @@ -372,7 +380,7 @@ def on_GET(self, request): self.nonces[nonce] = int(self.reactor.seconds()) return 200, {"nonce": nonce} - async def on_POST(self, request): + async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]: self._clear_old_nonces() if not self.hs.config.registration_shared_secret: @@ -478,12 +486,14 @@ class WhoisRestServlet(RestServlet): client_patterns("/admin" + path_regex, v1=True) ) - def __init__(self, hs): + def __init__(self, hs: "HomeServer"): self.hs = hs self.auth = hs.get_auth() self.admin_handler = hs.get_admin_handler() - async def on_GET(self, request, user_id): + async def on_GET( + self, request: SynapseRequest, user_id: str + ) -> Tuple[int, JsonDict]: target_user = UserID.from_string(user_id) requester = await self.auth.get_user_by_req(request) auth_user = requester.user @@ -508,7 +518,9 @@ def __init__(self, hs: "HomeServer"): self.is_mine = hs.is_mine self.store = hs.get_datastore() - async def on_POST(self, request: str, target_user_id: str) -> Tuple[int, JsonDict]: + async def on_POST( + self, request: SynapseRequest, target_user_id: str + ) -> Tuple[int, JsonDict]: requester = await self.auth.get_user_by_req(request) await assert_user_is_admin(self.auth, requester.user) @@ -550,7 +562,7 @@ def __init__(self, hs): self.account_activity_handler = hs.get_account_validity_handler() self.auth = hs.get_auth() - async def on_POST(self, request): + async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]: await assert_requester_is_admin(self.auth, request) body = parse_json_object_from_request(request) @@ -584,14 +596,16 @@ class ResetPasswordRestServlet(RestServlet): PATTERNS = admin_patterns("/reset_password/(?P[^/]*)") - def __init__(self, hs): + def __init__(self, hs: "HomeServer"): self.store = hs.get_datastore() self.hs = hs self.auth = hs.get_auth() self.auth_handler = hs.get_auth_handler() self._set_password_handler = hs.get_set_password_handler() - async def on_POST(self, request, target_user_id): + async def on_POST( + self, request: SynapseRequest, target_user_id: str + ) -> Tuple[int, JsonDict]: """Post request to allow an administrator reset password for a user. This needs user to have administrator access in Synapse. """ @@ -626,12 +640,14 @@ class SearchUsersRestServlet(RestServlet): PATTERNS = admin_patterns("/search_users/(?P[^/]*)") - def __init__(self, hs): + def __init__(self, hs: "HomeServer"): self.hs = hs self.store = hs.get_datastore() self.auth = hs.get_auth() - async def on_GET(self, request, target_user_id): + async def on_GET( + self, request: SynapseRequest, target_user_id: str + ) -> Tuple[int, Optional[JsonDict]]: """Get request to search user table for specific users according to search term. This needs user to have a administrator access in Synapse. @@ -682,12 +698,14 @@ class UserAdminServlet(RestServlet): PATTERNS = admin_patterns("/users/(?P[^/]*)/admin$") - def __init__(self, hs): + def __init__(self, hs: "HomeServer"): self.hs = hs self.store = hs.get_datastore() self.auth = hs.get_auth() - async def on_GET(self, request, user_id): + async def on_GET( + self, request: SynapseRequest, user_id: str + ) -> Tuple[int, JsonDict]: await assert_requester_is_admin(self.auth, request) target_user = UserID.from_string(user_id) @@ -699,7 +717,9 @@ async def on_GET(self, request, user_id): return 200, {"admin": is_admin} - async def on_PUT(self, request, user_id): + async def on_PUT( + self, request: SynapseRequest, user_id: str + ) -> Tuple[int, JsonDict]: requester = await self.auth.get_user_by_req(request) await assert_user_is_admin(self.auth, requester.user) auth_user = requester.user @@ -730,12 +750,14 @@ class UserMembershipRestServlet(RestServlet): PATTERNS = admin_patterns("/users/(?P[^/]+)/joined_rooms$") - def __init__(self, hs): + def __init__(self, hs: "HomeServer"): self.is_mine = hs.is_mine self.auth = hs.get_auth() self.store = hs.get_datastore() - async def on_GET(self, request, user_id): + async def on_GET( + self, request: SynapseRequest, user_id: str + ) -> Tuple[int, JsonDict]: await assert_requester_is_admin(self.auth, request) room_ids = await self.store.get_rooms_for_user(user_id) @@ -758,7 +780,7 @@ class PushersRestServlet(RestServlet): PATTERNS = admin_patterns("/users/(?P[^/]*)/pushers$") - def __init__(self, hs): + def __init__(self, hs: "HomeServer"): self.is_mine = hs.is_mine self.store = hs.get_datastore() self.auth = hs.get_auth() @@ -799,7 +821,7 @@ class UserMediaRestServlet(RestServlet): PATTERNS = admin_patterns("/users/(?P[^/]+)/media$") - def __init__(self, hs): + def __init__(self, hs: "HomeServer"): self.is_mine = hs.is_mine self.auth = hs.get_auth() self.store = hs.get_datastore() @@ -891,7 +913,9 @@ def __init__(self, hs: "HomeServer"): self.auth = hs.get_auth() self.auth_handler = hs.get_auth_handler() - async def on_POST(self, request, user_id): + async def on_POST( + self, request: SynapseRequest, user_id: str + ) -> Tuple[int, JsonDict]: requester = await self.auth.get_user_by_req(request) await assert_user_is_admin(self.auth, requester.user) auth_user = requester.user @@ -943,7 +967,9 @@ def __init__(self, hs: "HomeServer"): self.store = hs.get_datastore() self.auth = hs.get_auth() - async def on_POST(self, request, user_id): + 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): diff --git a/synapse/storage/databases/main/__init__.py b/synapse/storage/databases/main/__init__.py index 70b49854cf48..67172b871e1d 100644 --- a/synapse/storage/databases/main/__init__.py +++ b/synapse/storage/databases/main/__init__.py @@ -16,7 +16,7 @@ # limitations under the License. import logging -from typing import Any, Dict, List, Optional, Tuple +from typing import Optional, Tuple from synapse.api.constants import PresenceState from synapse.config.homeserver import HomeServerConfig @@ -27,7 +27,7 @@ MultiWriterIdGenerator, StreamIdGenerator, ) -from synapse.types import get_domain_from_id +from synapse.types import JsonDict, get_domain_from_id from synapse.util.caches.stream_change_cache import StreamChangeCache from .account_data import AccountDataStore @@ -264,7 +264,7 @@ def _get_active_presence(self, db_conn): return [UserPresenceState(**row) for row in rows] - async def get_users(self) -> List[Dict[str, Any]]: + async def get_users(self) -> JsonDict: """Function to retrieve a list of users in users table. Returns: @@ -292,7 +292,7 @@ async def get_users_paginate( name: Optional[str] = None, guests: bool = True, deactivated: bool = False, - ) -> Tuple[List[Dict[str, Any]], int]: + ) -> Tuple[JsonDict, int]: """Function to retrieve a paginated list of users from users list. This will return a json list of users and the total number of users matching the filter criteria. @@ -353,7 +353,7 @@ def get_users_paginate_txn(txn): "get_users_paginate_txn", get_users_paginate_txn ) - async def search_users(self, term: str) -> Optional[List[Dict[str, Any]]]: + async def search_users(self, term: str) -> Optional[JsonDict]: """Function to search users list for one or more users with the matched term. diff --git a/synapse/storage/databases/main/media_repository.py b/synapse/storage/databases/main/media_repository.py index 274f8de595ce..4f3d19256233 100644 --- a/synapse/storage/databases/main/media_repository.py +++ b/synapse/storage/databases/main/media_repository.py @@ -139,7 +139,7 @@ async def get_local_media_by_user_paginate( start: int, limit: int, user_id: str, - order_by: MediaSortOrder = MediaSortOrder.CREATED_TS.value, + order_by: str = MediaSortOrder.CREATED_TS.value, direction: str = "f", ) -> Tuple[List[Dict[str, Any]], int]: """Get a paginated list of metadata for a local piece of media From 479e2e76b2cae848068fe4a49d0d71d69f43dbe7 Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Mon, 1 Mar 2021 22:09:54 +0100 Subject: [PATCH 2/4] add changelog --- changelog.d/9521.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/9521.misc diff --git a/changelog.d/9521.misc b/changelog.d/9521.misc new file mode 100644 index 000000000000..1424d9c188a7 --- /dev/null +++ b/changelog.d/9521.misc @@ -0,0 +1 @@ +Add type hints to user admin API. \ No newline at end of file From 6e2710bf2461b67db86eb3a8dbc99982afcb6ba3 Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Tue, 2 Mar 2021 19:39:03 +0100 Subject: [PATCH 3/4] Update type hints after review --- synapse/rest/admin/users.py | 12 +++++++----- synapse/storage/databases/main/__init__.py | 8 ++++---- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index 148749c950e4..5a575673dfa5 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -16,7 +16,7 @@ import hmac import logging from http import HTTPStatus -from typing import TYPE_CHECKING, Dict, Optional, Tuple +from typing import TYPE_CHECKING, Dict, List, Optional, Tuple from synapse.api.constants import UserTypes from synapse.api.errors import Codes, NotFoundError, SynapseError @@ -55,7 +55,7 @@ def __init__(self, hs: "HomeServer"): async def on_GET( self, request: SynapseRequest, user_id: str - ) -> Tuple[int, JsonDict]: + ) -> Tuple[int, List[JsonDict]]: target_user = UserID.from_string(user_id) await assert_requester_is_admin(self.auth, request) @@ -167,7 +167,9 @@ def __init__(self, hs: "HomeServer"): self.registration_handler = hs.get_registration_handler() self.pusher_pool = hs.get_pusherpool() - async def on_GET(self, request: SynapseRequest, user_id) -> Tuple[int, JsonDict]: + async def on_GET( + self, request: SynapseRequest, user_id: str + ) -> Tuple[int, JsonDict]: await assert_requester_is_admin(self.auth, request) target_user = UserID.from_string(user_id) @@ -357,7 +359,7 @@ class UserRegisterServlet(RestServlet): def __init__(self, hs: "HomeServer"): self.auth_handler = hs.get_auth_handler() self.reactor = hs.get_reactor() - self.nonces = {} # type: Dict + self.nonces = {} # type: Dict[str, int] self.hs = hs def _clear_old_nonces(self): @@ -647,7 +649,7 @@ def __init__(self, hs: "HomeServer"): async def on_GET( self, request: SynapseRequest, target_user_id: str - ) -> Tuple[int, Optional[JsonDict]]: + ) -> Tuple[int, Optional[List[JsonDict]]]: """Get request to search user table for specific users according to search term. This needs user to have a administrator access in Synapse. diff --git a/synapse/storage/databases/main/__init__.py b/synapse/storage/databases/main/__init__.py index 67172b871e1d..1d44c3aa2c29 100644 --- a/synapse/storage/databases/main/__init__.py +++ b/synapse/storage/databases/main/__init__.py @@ -16,7 +16,7 @@ # limitations under the License. import logging -from typing import Optional, Tuple +from typing import List, Optional, Tuple from synapse.api.constants import PresenceState from synapse.config.homeserver import HomeServerConfig @@ -264,7 +264,7 @@ def _get_active_presence(self, db_conn): return [UserPresenceState(**row) for row in rows] - async def get_users(self) -> JsonDict: + async def get_users(self) -> List[JsonDict]: """Function to retrieve a list of users in users table. Returns: @@ -292,7 +292,7 @@ async def get_users_paginate( name: Optional[str] = None, guests: bool = True, deactivated: bool = False, - ) -> Tuple[JsonDict, int]: + ) -> 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 total number of users matching the filter criteria. @@ -353,7 +353,7 @@ def get_users_paginate_txn(txn): "get_users_paginate_txn", get_users_paginate_txn ) - async def search_users(self, term: str) -> Optional[JsonDict]: + async def search_users(self, term: str) -> Optional[List[JsonDict]]: """Function to search users list for one or more users with the matched term. From 4299bb7ae85831f32ad8e938eece6a8c930c540a Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Tue, 2 Mar 2021 20:07:03 +0100 Subject: [PATCH 4/4] assert user is not None --- synapse/rest/admin/users.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index 5a575673dfa5..267a993430eb 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -278,9 +278,9 @@ async def on_PUT( target_user.to_string() ) - user = await self.admin_handler.get_user( - target_user - ) # type: JsonDict # type: ignore + user = await self.admin_handler.get_user(target_user) + assert user is not None + return 200, user else: # create user @@ -338,11 +338,10 @@ async def on_PUT( target_user, requester, body["avatar_url"], True ) - ret = await self.admin_handler.get_user( - target_user - ) # type: JsonDict # type: ignore + user = await self.admin_handler.get_user(target_user) + assert user is not None - return 201, ret + return 201, user class UserRegisterServlet(RestServlet):