From 7ecaa3b976b04dc5b2c6786aa18845016b80dd01 Mon Sep 17 00:00:00 2001 From: Dirk Klimpel <5740567+dklimpel@users.noreply.github.com> Date: Wed, 8 Dec 2021 17:59:40 +0100 Subject: Clean up `synapse.rest.admin` (#11535) --- tests/rest/admin/test_statistics.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tests/rest/admin') diff --git a/tests/rest/admin/test_statistics.py b/tests/rest/admin/test_statistics.py index 7cb8ec57ba..f6e85fdaad 100644 --- a/tests/rest/admin/test_statistics.py +++ b/tests/rest/admin/test_statistics.py @@ -92,7 +92,7 @@ class UserMediaStatisticsTestCase(unittest.HomeserverTestCase): channel.code, msg=channel.json_body, ) - self.assertEqual(Codes.INVALID_PARAM, channel.json_body["errcode"]) + self.assertEqual(Codes.UNKNOWN, channel.json_body["errcode"]) # negative from channel = self.make_request( -- cgit 1.5.1 From b3bcacf3c1c72bfadeb46fe4d0198ca155a8c615 Mon Sep 17 00:00:00 2001 From: Dirk Klimpel <5740567+dklimpel@users.noreply.github.com> Date: Thu, 9 Dec 2021 12:23:34 +0100 Subject: Add missing `errcode` to `parse_string` and `parse_boolean` (#11542) --- changelog.d/11542.misc | 1 + synapse/http/servlet.py | 4 ++-- tests/rest/admin/test_federation.py | 4 ++-- tests/rest/admin/test_media.py | 2 +- tests/rest/admin/test_statistics.py | 2 +- tests/rest/admin/test_user.py | 12 ++++++------ 6 files changed, 13 insertions(+), 12 deletions(-) create mode 100644 changelog.d/11542.misc (limited to 'tests/rest/admin') diff --git a/changelog.d/11542.misc b/changelog.d/11542.misc new file mode 100644 index 0000000000..f614165037 --- /dev/null +++ b/changelog.d/11542.misc @@ -0,0 +1 @@ +Add missing `errcode` to `parse_string` and `parse_boolean`. \ No newline at end of file diff --git a/synapse/http/servlet.py b/synapse/http/servlet.py index 6dd9b9ad03..1627225f28 100644 --- a/synapse/http/servlet.py +++ b/synapse/http/servlet.py @@ -246,7 +246,7 @@ def parse_boolean_from_args( message = ( "Boolean query parameter %r must be one of ['true', 'false']" ) % (name,) - raise SynapseError(400, message) + raise SynapseError(400, message, errcode=Codes.INVALID_PARAM) else: if required: message = "Missing boolean query parameter %r" % (name,) @@ -414,7 +414,7 @@ def _parse_string_value( name, ", ".join(repr(v) for v in allowed_values), ) - raise SynapseError(400, message) + raise SynapseError(400, message, errcode=Codes.INVALID_PARAM) else: return value_str diff --git a/tests/rest/admin/test_federation.py b/tests/rest/admin/test_federation.py index 5188499ef2..d1cd5b0751 100644 --- a/tests/rest/admin/test_federation.py +++ b/tests/rest/admin/test_federation.py @@ -95,7 +95,7 @@ class FederationTestCase(unittest.HomeserverTestCase): ) self.assertEqual(HTTPStatus.BAD_REQUEST, channel.code, msg=channel.json_body) - self.assertEqual(Codes.UNKNOWN, channel.json_body["errcode"]) + self.assertEqual(Codes.INVALID_PARAM, channel.json_body["errcode"]) # invalid search order channel = self.make_request( @@ -105,7 +105,7 @@ class FederationTestCase(unittest.HomeserverTestCase): ) self.assertEqual(HTTPStatus.BAD_REQUEST, channel.code, msg=channel.json_body) - self.assertEqual(Codes.UNKNOWN, channel.json_body["errcode"]) + self.assertEqual(Codes.INVALID_PARAM, channel.json_body["errcode"]) # invalid destination channel = self.make_request( diff --git a/tests/rest/admin/test_media.py b/tests/rest/admin/test_media.py index 81e578fd26..3f727788ce 100644 --- a/tests/rest/admin/test_media.py +++ b/tests/rest/admin/test_media.py @@ -360,7 +360,7 @@ class DeleteMediaByDateSizeTestCase(unittest.HomeserverTestCase): channel.code, msg=channel.json_body, ) - self.assertEqual(Codes.UNKNOWN, channel.json_body["errcode"]) + self.assertEqual(Codes.INVALID_PARAM, channel.json_body["errcode"]) self.assertEqual( "Boolean query parameter 'keep_profiles' must be one of ['true', 'false']", channel.json_body["error"], diff --git a/tests/rest/admin/test_statistics.py b/tests/rest/admin/test_statistics.py index f6e85fdaad..7cb8ec57ba 100644 --- a/tests/rest/admin/test_statistics.py +++ b/tests/rest/admin/test_statistics.py @@ -92,7 +92,7 @@ class UserMediaStatisticsTestCase(unittest.HomeserverTestCase): channel.code, msg=channel.json_body, ) - self.assertEqual(Codes.UNKNOWN, channel.json_body["errcode"]) + self.assertEqual(Codes.INVALID_PARAM, channel.json_body["errcode"]) # negative from channel = self.make_request( diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index 4fedd5fd08..294d429ce1 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -608,7 +608,7 @@ class UsersListTestCase(unittest.HomeserverTestCase): ) self.assertEqual(HTTPStatus.BAD_REQUEST, channel.code, msg=channel.json_body) - self.assertEqual(Codes.UNKNOWN, channel.json_body["errcode"]) + self.assertEqual(Codes.INVALID_PARAM, channel.json_body["errcode"]) # invalid deactivated channel = self.make_request( @@ -618,7 +618,7 @@ class UsersListTestCase(unittest.HomeserverTestCase): ) self.assertEqual(HTTPStatus.BAD_REQUEST, channel.code, msg=channel.json_body) - self.assertEqual(Codes.UNKNOWN, channel.json_body["errcode"]) + self.assertEqual(Codes.INVALID_PARAM, channel.json_body["errcode"]) # unkown order_by channel = self.make_request( @@ -628,7 +628,7 @@ class UsersListTestCase(unittest.HomeserverTestCase): ) self.assertEqual(HTTPStatus.BAD_REQUEST, channel.code, msg=channel.json_body) - self.assertEqual(Codes.UNKNOWN, channel.json_body["errcode"]) + self.assertEqual(Codes.INVALID_PARAM, channel.json_body["errcode"]) # invalid search order channel = self.make_request( @@ -638,7 +638,7 @@ class UsersListTestCase(unittest.HomeserverTestCase): ) self.assertEqual(HTTPStatus.BAD_REQUEST, channel.code, msg=channel.json_body) - self.assertEqual(Codes.UNKNOWN, channel.json_body["errcode"]) + self.assertEqual(Codes.INVALID_PARAM, channel.json_body["errcode"]) def test_limit(self): """ @@ -2896,7 +2896,7 @@ class UserMediaRestTestCase(unittest.HomeserverTestCase): ) self.assertEqual(HTTPStatus.BAD_REQUEST, channel.code, msg=channel.json_body) - self.assertEqual(Codes.UNKNOWN, channel.json_body["errcode"]) + self.assertEqual(Codes.INVALID_PARAM, channel.json_body["errcode"]) # invalid search order channel = self.make_request( @@ -2906,7 +2906,7 @@ class UserMediaRestTestCase(unittest.HomeserverTestCase): ) self.assertEqual(HTTPStatus.BAD_REQUEST, channel.code, msg=channel.json_body) - self.assertEqual(Codes.UNKNOWN, channel.json_body["errcode"]) + self.assertEqual(Codes.INVALID_PARAM, channel.json_body["errcode"]) # negative limit channel = self.make_request( -- cgit 1.5.1 From 9562f0c2f1bd3489bfbb64fddbbd21ed657b44dd Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 10 Dec 2021 07:17:28 -0500 Subject: Ensure emails are canonicalized before fetching associated user. (#11547) This should fix pushers with an email in non-canonical form is used as the pushkey. --- changelog.d/11547.bugfix | 1 + synapse/push/pusherpool.py | 5 ++++- synapse/storage/databases/main/monthly_active_users.py | 3 ++- synapse/storage/databases/main/registration.py | 3 ++- tests/rest/admin/test_user.py | 3 ++- 5 files changed, 11 insertions(+), 4 deletions(-) create mode 100644 changelog.d/11547.bugfix (limited to 'tests/rest/admin') diff --git a/changelog.d/11547.bugfix b/changelog.d/11547.bugfix new file mode 100644 index 0000000000..3950c4c8d3 --- /dev/null +++ b/changelog.d/11547.bugfix @@ -0,0 +1 @@ +Fix a bug introduced in Synapse 1.17.0 where a pusher created for an email with capital letters would fail to be created. diff --git a/synapse/push/pusherpool.py b/synapse/push/pusherpool.py index 26735447a6..7912311d24 100644 --- a/synapse/push/pusherpool.py +++ b/synapse/push/pusherpool.py @@ -27,6 +27,7 @@ from synapse.push.pusher import PusherFactory from synapse.replication.http.push import ReplicationRemovePusherRestServlet from synapse.types import JsonDict, RoomStreamToken from synapse.util.async_helpers import concurrently_execute +from synapse.util.threepids import canonicalise_email if TYPE_CHECKING: from synapse.server import HomeServer @@ -113,7 +114,9 @@ class PusherPool: """ if kind == "email": - email_owner = await self.store.get_user_id_by_threepid("email", pushkey) + email_owner = await self.store.get_user_id_by_threepid( + "email", canonicalise_email(pushkey) + ) if email_owner != user_id: raise SynapseError(400, "Email not found", Codes.THREEPID_NOT_FOUND) diff --git a/synapse/storage/databases/main/monthly_active_users.py b/synapse/storage/databases/main/monthly_active_users.py index b5284e4f67..3c98ef876f 100644 --- a/synapse/storage/databases/main/monthly_active_users.py +++ b/synapse/storage/databases/main/monthly_active_users.py @@ -18,6 +18,7 @@ from synapse.metrics.background_process_metrics import wrap_as_background_proces from synapse.storage._base import SQLBaseStore from synapse.storage.database import DatabasePool, make_in_list_sql_clause from synapse.util.caches.descriptors import cached +from synapse.util.threepids import canonicalise_email if TYPE_CHECKING: from synapse.server import HomeServer @@ -103,7 +104,7 @@ class MonthlyActiveUsersWorkerStore(SQLBaseStore): : self.hs.config.server.max_mau_value ]: user_id = await self.hs.get_datastore().get_user_id_by_threepid( - tp["medium"], tp["address"] + tp["medium"], canonicalise_email(tp["address"]) ) if user_id: users.append(user_id) diff --git a/synapse/storage/databases/main/registration.py b/synapse/storage/databases/main/registration.py index e1ddf06916..86c3425716 100644 --- a/synapse/storage/databases/main/registration.py +++ b/synapse/storage/databases/main/registration.py @@ -856,7 +856,8 @@ class RegistrationWorkerStore(CacheInvalidationWorkerStore): Args: medium: threepid medium e.g. email - address: threepid address e.g. me@example.com + address: threepid address e.g. me@example.com. This must already be + in canonical form. Returns: The user ID or None if no user id/threepid mapping exists diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index 294d429ce1..eea675991c 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -1550,7 +1550,8 @@ class UserRestTestCase(unittest.HomeserverTestCase): # Create user body = { "password": "abc123", - "threepids": [{"medium": "email", "address": "bob@bob.bob"}], + # Note that the given email is not in canonical form. + "threepids": [{"medium": "email", "address": "Bob@bob.bob"}], } channel = self.make_request( -- cgit 1.5.1 From 8428ef66c73efcc01cdbe05ac8bc1c99c4c20a20 Mon Sep 17 00:00:00 2001 From: Dirk Klimpel <5740567+dklimpel@users.noreply.github.com> Date: Thu, 16 Dec 2021 20:59:56 +0100 Subject: Add type hints to `synapse/tests/rest/admin` (#11590) --- changelog.d/11590.misc | 1 + mypy.ini | 3 ++ tests/rest/admin/test_background_updates.py | 3 +- tests/rest/admin/test_federation.py | 33 ++++++++------- tests/rest/admin/test_media.py | 4 +- tests/rest/admin/test_registration_tokens.py | 25 +++++++---- tests/rest/admin/test_room.py | 62 ++++++++++++++-------------- 7 files changed, 74 insertions(+), 57 deletions(-) create mode 100644 changelog.d/11590.misc (limited to 'tests/rest/admin') diff --git a/changelog.d/11590.misc b/changelog.d/11590.misc new file mode 100644 index 0000000000..40e01194df --- /dev/null +++ b/changelog.d/11590.misc @@ -0,0 +1 @@ +Add type hints to `synapse/tests/rest/admin`. \ No newline at end of file diff --git a/mypy.ini b/mypy.ini index c546487bdb..c2cb71e0c0 100644 --- a/mypy.ini +++ b/mypy.ini @@ -242,6 +242,9 @@ disallow_untyped_defs = True [mypy-tests.storage.test_user_directory] disallow_untyped_defs = True +[mypy-tests.rest.admin.*] +disallow_untyped_defs = True + [mypy-tests.rest.client.test_directory] disallow_untyped_defs = True diff --git a/tests/rest/admin/test_background_updates.py b/tests/rest/admin/test_background_updates.py index 4d152c0d66..1e3fe9c62c 100644 --- a/tests/rest/admin/test_background_updates.py +++ b/tests/rest/admin/test_background_updates.py @@ -23,6 +23,7 @@ from synapse.api.errors import Codes from synapse.rest.client import login from synapse.server import HomeServer from synapse.storage.background_updates import BackgroundUpdater +from synapse.types import JsonDict from synapse.util import Clock from tests import unittest @@ -96,7 +97,7 @@ class BackgroundUpdatesTestCase(unittest.HomeserverTestCase): def _register_bg_update(self) -> None: "Adds a bg update but doesn't start it" - async def _fake_update(progress, batch_size) -> int: + async def _fake_update(progress: JsonDict, batch_size: int) -> int: await self.clock.sleep(0.2) return batch_size diff --git a/tests/rest/admin/test_federation.py b/tests/rest/admin/test_federation.py index d1cd5b0751..742f194257 100644 --- a/tests/rest/admin/test_federation.py +++ b/tests/rest/admin/test_federation.py @@ -16,11 +16,14 @@ from typing import List, Optional from parameterized import parameterized +from twisted.test.proto_helpers import MemoryReactor + import synapse.rest.admin from synapse.api.errors import Codes from synapse.rest.client import login from synapse.server import HomeServer from synapse.types import JsonDict +from synapse.util import Clock from tests import unittest @@ -31,7 +34,7 @@ class FederationTestCase(unittest.HomeserverTestCase): login.register_servlets, ] - def prepare(self, reactor, clock, hs: HomeServer): + def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: self.store = hs.get_datastore() self.register_user("admin", "pass", admin=True) self.admin_user_tok = self.login("admin", "pass") @@ -44,7 +47,7 @@ class FederationTestCase(unittest.HomeserverTestCase): ("/_synapse/admin/v1/federation/destinations/dummy",), ] ) - def test_requester_is_no_admin(self, url: str): + def test_requester_is_no_admin(self, url: str) -> None: """ If the user is not a server admin, an error 403 is returned. """ @@ -62,7 +65,7 @@ class FederationTestCase(unittest.HomeserverTestCase): self.assertEqual(HTTPStatus.FORBIDDEN, channel.code, msg=channel.json_body) self.assertEqual(Codes.FORBIDDEN, channel.json_body["errcode"]) - def test_invalid_parameter(self): + def test_invalid_parameter(self) -> None: """ If parameters are invalid, an error is returned. """ @@ -117,7 +120,7 @@ class FederationTestCase(unittest.HomeserverTestCase): self.assertEqual(HTTPStatus.NOT_FOUND, channel.code, msg=channel.json_body) self.assertEqual(Codes.NOT_FOUND, channel.json_body["errcode"]) - def test_limit(self): + def test_limit(self) -> None: """ Testing list of destinations with limit """ @@ -137,7 +140,7 @@ class FederationTestCase(unittest.HomeserverTestCase): self.assertEqual(channel.json_body["next_token"], "5") self._check_fields(channel.json_body["destinations"]) - def test_from(self): + def test_from(self) -> None: """ Testing list of destinations with a defined starting point (from) """ @@ -157,7 +160,7 @@ class FederationTestCase(unittest.HomeserverTestCase): self.assertNotIn("next_token", channel.json_body) self._check_fields(channel.json_body["destinations"]) - def test_limit_and_from(self): + def test_limit_and_from(self) -> None: """ Testing list of destinations with a defined starting point and limit """ @@ -177,7 +180,7 @@ class FederationTestCase(unittest.HomeserverTestCase): self.assertEqual(len(channel.json_body["destinations"]), 10) self._check_fields(channel.json_body["destinations"]) - def test_next_token(self): + def test_next_token(self) -> None: """ Testing that `next_token` appears at the right place """ @@ -238,7 +241,7 @@ class FederationTestCase(unittest.HomeserverTestCase): self.assertEqual(len(channel.json_body["destinations"]), 1) self.assertNotIn("next_token", channel.json_body) - def test_list_all_destinations(self): + def test_list_all_destinations(self) -> None: """ List all destinations. """ @@ -259,7 +262,7 @@ class FederationTestCase(unittest.HomeserverTestCase): # Check that all fields are available self._check_fields(channel.json_body["destinations"]) - def test_order_by(self): + def test_order_by(self) -> None: """ Testing order list with parameter `order_by` """ @@ -268,7 +271,7 @@ class FederationTestCase(unittest.HomeserverTestCase): expected_destination_list: List[str], order_by: Optional[str], dir: Optional[str] = None, - ): + ) -> None: """Request the list of destinations in a certain order. Assert that order is what we expect @@ -358,13 +361,13 @@ class FederationTestCase(unittest.HomeserverTestCase): [dest[0][0], dest[2][0], dest[1][0]], "last_successful_stream_ordering", "b" ) - def test_search_term(self): + def test_search_term(self) -> None: """Test that searching for a destination works correctly""" def _search_test( expected_destination: Optional[str], search_term: str, - ): + ) -> None: """Search for a destination and check that the returned destinationis a match Args: @@ -410,7 +413,7 @@ class FederationTestCase(unittest.HomeserverTestCase): _search_test(None, "foo") _search_test(None, "bar") - def test_get_single_destination(self): + def test_get_single_destination(self) -> None: """ Get one specific destinations. """ @@ -429,7 +432,7 @@ class FederationTestCase(unittest.HomeserverTestCase): # convert channel.json_body into a List self._check_fields([channel.json_body]) - def _create_destinations(self, number_destinations: int): + def _create_destinations(self, number_destinations: int) -> None: """Create a number of destinations Args: @@ -442,7 +445,7 @@ class FederationTestCase(unittest.HomeserverTestCase): self.store.set_destination_last_successful_stream_ordering(dest, 100) ) - def _check_fields(self, content: List[JsonDict]): + def _check_fields(self, content: List[JsonDict]) -> None: """Checks that the expected destination attributes are present in content Args: diff --git a/tests/rest/admin/test_media.py b/tests/rest/admin/test_media.py index 3f727788ce..86aff7575c 100644 --- a/tests/rest/admin/test_media.py +++ b/tests/rest/admin/test_media.py @@ -580,7 +580,9 @@ class DeleteMediaByDateSizeTestCase(unittest.HomeserverTestCase): return server_and_media_id - def _access_media(self, server_and_media_id, expect_success=True) -> None: + def _access_media( + self, server_and_media_id: str, expect_success: bool = True + ) -> None: """ Try to access a media and check the result """ diff --git a/tests/rest/admin/test_registration_tokens.py b/tests/rest/admin/test_registration_tokens.py index 350a62dda6..81f3ac7f04 100644 --- a/tests/rest/admin/test_registration_tokens.py +++ b/tests/rest/admin/test_registration_tokens.py @@ -14,6 +14,7 @@ import random import string from http import HTTPStatus +from typing import Optional from twisted.test.proto_helpers import MemoryReactor @@ -42,21 +43,27 @@ class ManageRegistrationTokensTestCase(unittest.HomeserverTestCase): self.url = "/_synapse/admin/v1/registration_tokens" - def _new_token(self, **kwargs) -> str: + def _new_token( + self, + token: Optional[str] = None, + uses_allowed: Optional[int] = None, + pending: int = 0, + completed: int = 0, + expiry_time: Optional[int] = None, + ) -> str: """Helper function to create a token.""" - token = kwargs.get( - "token", - "".join(random.choices(string.ascii_letters, k=8)), - ) + if token is None: + token = "".join(random.choices(string.ascii_letters, k=8)) + self.get_success( self.store.db_pool.simple_insert( "registration_tokens", { "token": token, - "uses_allowed": kwargs.get("uses_allowed", None), - "pending": kwargs.get("pending", 0), - "completed": kwargs.get("completed", 0), - "expiry_time": kwargs.get("expiry_time", None), + "uses_allowed": uses_allowed, + "pending": pending, + "completed": completed, + "expiry_time": expiry_time, }, ) ) diff --git a/tests/rest/admin/test_room.py b/tests/rest/admin/test_room.py index 22f9aa6234..d2c8781cd4 100644 --- a/tests/rest/admin/test_room.py +++ b/tests/rest/admin/test_room.py @@ -66,7 +66,7 @@ class DeleteRoomTestCase(unittest.HomeserverTestCase): ) self.url = "/_synapse/admin/v1/rooms/%s" % self.room_id - def test_requester_is_no_admin(self): + def test_requester_is_no_admin(self) -> None: """ If the user is not a server admin, an error HTTPStatus.FORBIDDEN is returned. """ @@ -81,7 +81,7 @@ class DeleteRoomTestCase(unittest.HomeserverTestCase): self.assertEqual(HTTPStatus.FORBIDDEN, channel.code, msg=channel.json_body) self.assertEqual(Codes.FORBIDDEN, channel.json_body["errcode"]) - def test_room_does_not_exist(self): + def test_room_does_not_exist(self) -> None: """ Check that unknown rooms/server return 200 """ @@ -96,7 +96,7 @@ class DeleteRoomTestCase(unittest.HomeserverTestCase): self.assertEqual(HTTPStatus.OK, channel.code, msg=channel.json_body) - def test_room_is_not_valid(self): + def test_room_is_not_valid(self) -> None: """ Check that invalid room names, return an error HTTPStatus.BAD_REQUEST. """ @@ -115,7 +115,7 @@ class DeleteRoomTestCase(unittest.HomeserverTestCase): channel.json_body["error"], ) - def test_new_room_user_does_not_exist(self): + def test_new_room_user_does_not_exist(self) -> None: """ Tests that the user ID must be from local server but it does not have to exist. """ @@ -133,7 +133,7 @@ class DeleteRoomTestCase(unittest.HomeserverTestCase): self.assertIn("failed_to_kick_users", channel.json_body) self.assertIn("local_aliases", channel.json_body) - def test_new_room_user_is_not_local(self): + def test_new_room_user_is_not_local(self) -> None: """ Check that only local users can create new room to move members. """ @@ -151,7 +151,7 @@ class DeleteRoomTestCase(unittest.HomeserverTestCase): channel.json_body["error"], ) - def test_block_is_not_bool(self): + def test_block_is_not_bool(self) -> None: """ If parameter `block` is not boolean, return an error """ @@ -166,7 +166,7 @@ class DeleteRoomTestCase(unittest.HomeserverTestCase): self.assertEqual(HTTPStatus.BAD_REQUEST, channel.code, msg=channel.json_body) self.assertEqual(Codes.BAD_JSON, channel.json_body["errcode"]) - def test_purge_is_not_bool(self): + def test_purge_is_not_bool(self) -> None: """ If parameter `purge` is not boolean, return an error """ @@ -181,7 +181,7 @@ class DeleteRoomTestCase(unittest.HomeserverTestCase): self.assertEqual(HTTPStatus.BAD_REQUEST, channel.code, msg=channel.json_body) self.assertEqual(Codes.BAD_JSON, channel.json_body["errcode"]) - def test_purge_room_and_block(self): + def test_purge_room_and_block(self) -> None: """Test to purge a room and block it. Members will not be moved to a new room and will not receive a message. """ @@ -212,7 +212,7 @@ class DeleteRoomTestCase(unittest.HomeserverTestCase): self._is_blocked(self.room_id, expect=True) self._has_no_members(self.room_id) - def test_purge_room_and_not_block(self): + def test_purge_room_and_not_block(self) -> None: """Test to purge a room and do not block it. Members will not be moved to a new room and will not receive a message. """ @@ -243,7 +243,7 @@ class DeleteRoomTestCase(unittest.HomeserverTestCase): self._is_blocked(self.room_id, expect=False) self._has_no_members(self.room_id) - def test_block_room_and_not_purge(self): + def test_block_room_and_not_purge(self) -> None: """Test to block a room without purging it. Members will not be moved to a new room and will not receive a message. The room will not be purged. @@ -299,7 +299,7 @@ class DeleteRoomTestCase(unittest.HomeserverTestCase): self.assertEqual(HTTPStatus.OK, channel.code, msg=channel.json_body) self._is_blocked(room_id) - def test_shutdown_room_consent(self): + def test_shutdown_room_consent(self) -> None: """Test that we can shutdown rooms with local users who have not yet accepted the privacy policy. This used to fail when we tried to force part the user from the old room. @@ -351,7 +351,7 @@ class DeleteRoomTestCase(unittest.HomeserverTestCase): self._is_purged(self.room_id) self._has_no_members(self.room_id) - def test_shutdown_room_block_peek(self): + def test_shutdown_room_block_peek(self) -> None: """Test that a world_readable room can no longer be peeked into after it has been shut down. Members will be moved to a new room and will receive a message. @@ -400,7 +400,7 @@ class DeleteRoomTestCase(unittest.HomeserverTestCase): # Assert we can no longer peek into the room self._assert_peek(self.room_id, expect_code=HTTPStatus.FORBIDDEN) - def _is_blocked(self, room_id, expect=True): + def _is_blocked(self, room_id: str, expect: bool = True) -> None: """Assert that the room is blocked or not""" d = self.store.is_room_blocked(room_id) if expect: @@ -408,17 +408,17 @@ class DeleteRoomTestCase(unittest.HomeserverTestCase): else: self.assertIsNone(self.get_success(d)) - def _has_no_members(self, room_id): + def _has_no_members(self, room_id: str) -> None: """Assert there is now no longer anyone in the room""" users_in_room = self.get_success(self.store.get_users_in_room(room_id)) self.assertEqual([], users_in_room) - def _is_member(self, room_id, user_id): + def _is_member(self, room_id: str, user_id: str) -> None: """Test that user is member of the room""" users_in_room = self.get_success(self.store.get_users_in_room(room_id)) self.assertIn(user_id, users_in_room) - def _is_purged(self, room_id): + def _is_purged(self, room_id: str) -> None: """Test that the following tables have been purged of all rows related to the room.""" for table in PURGE_TABLES: count = self.get_success( @@ -432,7 +432,7 @@ class DeleteRoomTestCase(unittest.HomeserverTestCase): self.assertEqual(count, 0, msg=f"Rows not purged in {table}") - def _assert_peek(self, room_id, expect_code): + def _assert_peek(self, room_id: str, expect_code: int) -> None: """Assert that the admin user can (or cannot) peek into the room.""" url = "rooms/%s/initialSync" % (room_id,) @@ -492,7 +492,7 @@ class DeleteRoomV2TestCase(unittest.HomeserverTestCase): ("GET", "/_synapse/admin/v2/rooms/delete_status/%s"), ] ) - def test_requester_is_no_admin(self, method: str, url: str): + def test_requester_is_no_admin(self, method: str, url: str) -> None: """ If the user is not a server admin, an error HTTPStatus.FORBIDDEN is returned. """ @@ -507,7 +507,7 @@ class DeleteRoomV2TestCase(unittest.HomeserverTestCase): self.assertEqual(HTTPStatus.FORBIDDEN, channel.code, msg=channel.json_body) self.assertEqual(Codes.FORBIDDEN, channel.json_body["errcode"]) - def test_room_does_not_exist(self): + def test_room_does_not_exist(self) -> None: """ Check that unknown rooms/server return 200 @@ -544,7 +544,7 @@ class DeleteRoomV2TestCase(unittest.HomeserverTestCase): ("GET", "/_synapse/admin/v2/rooms/%s/delete_status"), ] ) - def test_room_is_not_valid(self, method: str, url: str): + def test_room_is_not_valid(self, method: str, url: str) -> None: """ Check that invalid room names, return an error HTTPStatus.BAD_REQUEST. """ @@ -562,7 +562,7 @@ class DeleteRoomV2TestCase(unittest.HomeserverTestCase): channel.json_body["error"], ) - def test_new_room_user_does_not_exist(self): + def test_new_room_user_does_not_exist(self) -> None: """ Tests that the user ID must be from local server but it does not have to exist. """ @@ -580,7 +580,7 @@ class DeleteRoomV2TestCase(unittest.HomeserverTestCase): self._test_result(delete_id, self.other_user, expect_new_room=True) - def test_new_room_user_is_not_local(self): + def test_new_room_user_is_not_local(self) -> None: """ Check that only local users can create new room to move members. """ @@ -598,7 +598,7 @@ class DeleteRoomV2TestCase(unittest.HomeserverTestCase): channel.json_body["error"], ) - def test_block_is_not_bool(self): + def test_block_is_not_bool(self) -> None: """ If parameter `block` is not boolean, return an error """ @@ -613,7 +613,7 @@ class DeleteRoomV2TestCase(unittest.HomeserverTestCase): self.assertEqual(HTTPStatus.BAD_REQUEST, channel.code, msg=channel.json_body) self.assertEqual(Codes.BAD_JSON, channel.json_body["errcode"]) - def test_purge_is_not_bool(self): + def test_purge_is_not_bool(self) -> None: """ If parameter `purge` is not boolean, return an error """ @@ -628,7 +628,7 @@ class DeleteRoomV2TestCase(unittest.HomeserverTestCase): self.assertEqual(HTTPStatus.BAD_REQUEST, channel.code, msg=channel.json_body) self.assertEqual(Codes.BAD_JSON, channel.json_body["errcode"]) - def test_delete_expired_status(self): + def test_delete_expired_status(self) -> None: """Test that the task status is removed after expiration.""" # first task, do not purge, that we can create a second task @@ -699,7 +699,7 @@ class DeleteRoomV2TestCase(unittest.HomeserverTestCase): self.assertEqual(HTTPStatus.NOT_FOUND, channel.code, msg=channel.json_body) self.assertEqual(Codes.NOT_FOUND, channel.json_body["errcode"]) - def test_delete_same_room_twice(self): + def test_delete_same_room_twice(self) -> None: """Test that the call for delete a room at second time gives an exception.""" body = {"new_room_user_id": self.admin_user} @@ -743,7 +743,7 @@ class DeleteRoomV2TestCase(unittest.HomeserverTestCase): expect_new_room=True, ) - def test_purge_room_and_block(self): + def test_purge_room_and_block(self) -> None: """Test to purge a room and block it. Members will not be moved to a new room and will not receive a message. """ @@ -774,7 +774,7 @@ class DeleteRoomV2TestCase(unittest.HomeserverTestCase): self._is_blocked(self.room_id, expect=True) self._has_no_members(self.room_id) - def test_purge_room_and_not_block(self): + def test_purge_room_and_not_block(self) -> None: """Test to purge a room and do not block it. Members will not be moved to a new room and will not receive a message. """ @@ -805,7 +805,7 @@ class DeleteRoomV2TestCase(unittest.HomeserverTestCase): self._is_blocked(self.room_id, expect=False) self._has_no_members(self.room_id) - def test_block_room_and_not_purge(self): + def test_block_room_and_not_purge(self) -> None: """Test to block a room without purging it. Members will not be moved to a new room and will not receive a message. The room will not be purged. @@ -838,7 +838,7 @@ class DeleteRoomV2TestCase(unittest.HomeserverTestCase): self._is_blocked(self.room_id, expect=True) self._has_no_members(self.room_id) - def test_shutdown_room_consent(self): + def test_shutdown_room_consent(self) -> None: """Test that we can shutdown rooms with local users who have not yet accepted the privacy policy. This used to fail when we tried to force part the user from the old room. @@ -899,7 +899,7 @@ class DeleteRoomV2TestCase(unittest.HomeserverTestCase): self._is_purged(self.room_id) self._has_no_members(self.room_id) - def test_shutdown_room_block_peek(self): + def test_shutdown_room_block_peek(self) -> None: """Test that a world_readable room can no longer be peeked into after it has been shut down. Members will be moved to a new room and will receive a message. -- cgit 1.5.1 From 7a1cefc6e37aa583647f2804c9d9c9765712c59a Mon Sep 17 00:00:00 2001 From: Dirk Klimpel <5740567+dklimpel@users.noreply.github.com> Date: Wed, 5 Jan 2022 12:49:06 +0100 Subject: Add admin API to get users' account data (#11664) Co-authored-by: reivilibre --- changelog.d/11664.feature | 1 + docs/admin_api/user_admin_api.md | 75 +++++++++++++++++++++++++++++++++ synapse/rest/admin/__init__.py | 2 + synapse/rest/admin/users.py | 30 ++++++++++++++ tests/rest/admin/test_user.py | 90 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 198 insertions(+) create mode 100644 changelog.d/11664.feature (limited to 'tests/rest/admin') diff --git a/changelog.d/11664.feature b/changelog.d/11664.feature new file mode 100644 index 0000000000..df81783c66 --- /dev/null +++ b/changelog.d/11664.feature @@ -0,0 +1 @@ +Add admin API to get users' account data. \ No newline at end of file diff --git a/docs/admin_api/user_admin_api.md b/docs/admin_api/user_admin_api.md index ba574d795f..74933d2fcf 100644 --- a/docs/admin_api/user_admin_api.md +++ b/docs/admin_api/user_admin_api.md @@ -480,6 +480,81 @@ The following fields are returned in the JSON response body: - `joined_rooms` - An array of `room_id`. - `total` - Number of rooms. +## Account Data +Gets information about account data for a specific `user_id`. + +The API is: + +``` +GET /_synapse/admin/v1/users//accountdata +``` + +A response body like the following is returned: + +```json +{ + "account_data": { + "global": { + "m.secret_storage.key.LmIGHTg5W": { + "algorithm": "m.secret_storage.v1.aes-hmac-sha2", + "iv": "fwjNZatxg==", + "mac": "eWh9kNnLWZUNOgnc=" + }, + "im.vector.hide_profile": { + "hide_profile": true + }, + "org.matrix.preview_urls": { + "disable": false + }, + "im.vector.riot.breadcrumb_rooms": { + "rooms": [ + "!LxcBDAsDUVAfJDEo:matrix.org", + "!MAhRxqasbItjOqxu:matrix.org" + ] + }, + "m.accepted_terms": { + "accepted": [ + "https://example.org/somewhere/privacy-1.2-en.html", + "https://example.org/somewhere/terms-2.0-en.html" + ] + }, + "im.vector.setting.breadcrumbs": { + "recent_rooms": [ + "!MAhRxqasbItqxuEt:matrix.org", + "!ZtSaPCawyWtxiImy:matrix.org" + ] + } + }, + "rooms": { + "!GUdfZSHUJibpiVqHYd:matrix.org": { + "m.fully_read": { + "event_id": "$156334540fYIhZ:matrix.org" + } + }, + "!tOZwOOiqwCYQkLhV:matrix.org": { + "m.fully_read": { + "event_id": "$xjsIyp4_NaVl2yPvIZs_k1Jl8tsC_Sp23wjqXPno" + } + } + } + } +} +``` + +**Parameters** + +The following parameters should be set in the URL: + +- `user_id` - fully qualified: for example, `@user:server.com`. + +**Response** + +The following fields are returned in the JSON response body: + +- `account_data` - A map containing the account data for the user + - `global` - A map containing the global account data for the user + - `rooms` - A map containing the account data per room for the user + ## User media ### List media uploaded by a user diff --git a/synapse/rest/admin/__init__.py b/synapse/rest/admin/__init__.py index 701c609c12..465e06772b 100644 --- a/synapse/rest/admin/__init__.py +++ b/synapse/rest/admin/__init__.py @@ -69,6 +69,7 @@ from synapse.rest.admin.server_notice_servlet import SendServerNoticeServlet from synapse.rest.admin.statistics import UserMediaStatisticsRestServlet from synapse.rest.admin.username_available import UsernameAvailableRestServlet from synapse.rest.admin.users import ( + AccountDataRestServlet, AccountValidityRenewServlet, DeactivateAccountRestServlet, PushersRestServlet, @@ -255,6 +256,7 @@ def register_servlets(hs: "HomeServer", http_server: HttpServer) -> None: UserMediaStatisticsRestServlet(hs).register(http_server) EventReportDetailRestServlet(hs).register(http_server) EventReportsRestServlet(hs).register(http_server) + AccountDataRestServlet(hs).register(http_server) PushersRestServlet(hs).register(http_server) MakeRoomAdminRestServlet(hs).register(http_server) ShadowBanRestServlet(hs).register(http_server) diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index db678da4cf..78e795c347 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -1121,3 +1121,33 @@ class RateLimitRestServlet(RestServlet): await self.store.delete_ratelimit_for_user(user_id) return HTTPStatus.OK, {} + + +class AccountDataRestServlet(RestServlet): + """Retrieve the given user's account data""" + + PATTERNS = admin_patterns("/users/(?P[^/]*)/accountdata") + + def __init__(self, hs: "HomeServer"): + self._auth = hs.get_auth() + self._store = hs.get_datastore() + self._is_mine_id = hs.is_mine_id + + async def on_GET( + self, request: SynapseRequest, user_id: str + ) -> Tuple[int, JsonDict]: + await assert_requester_is_admin(self._auth, request) + + if not self._is_mine_id(user_id): + raise SynapseError(HTTPStatus.BAD_REQUEST, "Can only look up local users") + + if not await self._store.get_user_by_id(user_id): + raise NotFoundError("User not found") + + global_data, by_room_data = await self._store.get_account_data_for_user(user_id) + return HTTPStatus.OK, { + "account_data": { + "global": global_data, + "rooms": by_room_data, + }, + } diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index eea675991c..e0b9fe8e91 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -3883,3 +3883,93 @@ class RateLimitTestCase(unittest.HomeserverTestCase): self.assertEqual(HTTPStatus.OK, channel.code, msg=channel.json_body) self.assertNotIn("messages_per_second", channel.json_body) self.assertNotIn("burst_count", channel.json_body) + + +class AccountDataTestCase(unittest.HomeserverTestCase): + + servlets = [ + synapse.rest.admin.register_servlets, + login.register_servlets, + ] + + def prepare(self, reactor, clock, hs) -> None: + 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 = f"/_synapse/admin/v1/users/{self.other_user}/accountdata" + + def test_no_auth(self) -> None: + """Try to get information of a user without authentication.""" + channel = self.make_request("GET", self.url, {}) + + self.assertEqual(HTTPStatus.UNAUTHORIZED, channel.code, msg=channel.json_body) + self.assertEqual(Codes.MISSING_TOKEN, channel.json_body["errcode"]) + + def test_requester_is_no_admin(self) -> None: + """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(HTTPStatus.FORBIDDEN, channel.code, msg=channel.json_body) + self.assertEqual(Codes.FORBIDDEN, channel.json_body["errcode"]) + + def test_user_does_not_exist(self) -> None: + """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(HTTPStatus.NOT_FOUND, channel.code, msg=channel.json_body) + self.assertEqual(Codes.NOT_FOUND, channel.json_body["errcode"]) + + def test_user_is_not_local(self) -> None: + """Tests that a lookup for a user that is not a local returns a 400""" + url = "/_synapse/admin/v1/users/@unknown_person:unknown_domain/accountdata" + + channel = self.make_request( + "GET", + url, + access_token=self.admin_user_tok, + ) + + self.assertEqual(HTTPStatus.BAD_REQUEST, channel.code, msg=channel.json_body) + self.assertEqual("Can only look up local users", channel.json_body["error"]) + + def test_success(self) -> None: + """Request account data should succeed for an admin.""" + + # add account data + self.get_success( + self.store.add_account_data_for_user(self.other_user, "m.global", {"a": 1}) + ) + self.get_success( + self.store.add_account_data_to_room( + self.other_user, "test_room", "m.per_room", {"b": 2} + ) + ) + + channel = self.make_request( + "GET", + self.url, + access_token=self.admin_user_tok, + ) + self.assertEqual(HTTPStatus.OK, channel.code, msg=channel.json_body) + self.assertEqual( + {"a": 1}, channel.json_body["account_data"]["global"]["m.global"] + ) + self.assertEqual( + {"b": 2}, + channel.json_body["account_data"]["rooms"]["test_room"]["m.per_room"], + ) -- cgit 1.5.1