From 3f0ff53158cc07b481c701077357d9d09254845b Mon Sep 17 00:00:00 2001 From: Dirk Klimpel <5740567+dklimpel@users.noreply.github.com> Date: Wed, 25 Nov 2020 22:26:11 +0100 Subject: [PATCH] Remove deprecated `/_matrix/client/*/admin` endpoints (#8785) These are now only available via `/_synapse/admin/v1`. --- UPGRADE.rst | 22 ++++ changelog.d/8785.removal | 1 + docs/admin_api/user_admin_api.rst | 7 ++ synapse/_scripts/register_new_matrix_user.py | 2 +- synapse/rest/admin/__init__.py | 12 +- synapse/rest/admin/_base.py | 22 ---- synapse/rest/admin/groups.py | 7 +- synapse/rest/admin/media.py | 15 +-- synapse/rest/admin/rooms.py | 3 +- synapse/rest/admin/users.py | 25 +++-- tests/rest/admin/test_admin.py | 2 +- tests/rest/admin/test_room.py | 4 +- tests/rest/admin/test_user.py | 110 ++++++++++++++++++- tests/rest/client/v2_alpha/test_register.py | 6 +- tests/storage/test_client_ips.py | 2 +- tests/unittest.py | 4 +- 16 files changed, 176 insertions(+), 68 deletions(-) create mode 100644 changelog.d/8785.removal diff --git a/UPGRADE.rst b/UPGRADE.rst index 4de1bb5841..6825b567e9 100644 --- a/UPGRADE.rst +++ b/UPGRADE.rst @@ -105,6 +105,28 @@ shown below: return {"localpart": localpart} +Removal historical Synapse Admin API +------------------------------------ + +Historically, the Synapse Admin API has been accessible under: + +* ``/_matrix/client/api/v1/admin`` +* ``/_matrix/client/unstable/admin`` +* ``/_matrix/client/r0/admin`` +* ``/_synapse/admin/v1`` + +The endpoints with ``/_matrix/client/*`` prefixes have been removed as of v1.24.0. +The Admin API is now only accessible under: + +* ``/_synapse/admin/v1`` + +The only exception is the `/admin/whois` endpoint, which is +`also available via the client-server API `_. + +The deprecation of the old endpoints was announced with Synapse 1.20.0 (released +on 2020-09-22) and makes it easier for homeserver admins to lock down external +access to the Admin API endpoints. + Upgrading to v1.23.0 ==================== diff --git a/changelog.d/8785.removal b/changelog.d/8785.removal new file mode 100644 index 0000000000..ee8ee32598 --- /dev/null +++ b/changelog.d/8785.removal @@ -0,0 +1 @@ +Remove old `/_matrix/client/*/admin` endpoints which was deprecated since Synapse 1.20.0. \ 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 84863296e3..1473a3d4e3 100644 --- a/docs/admin_api/user_admin_api.rst +++ b/docs/admin_api/user_admin_api.rst @@ -176,6 +176,13 @@ The api is:: GET /_synapse/admin/v1/whois/ +and:: + + GET /_matrix/client/r0/admin/whois/ + +See also: `Client Server API Whois +`_ + To use it, you will need to authenticate by providing an ``access_token`` for a server admin: see `README.rst `_. diff --git a/synapse/_scripts/register_new_matrix_user.py b/synapse/_scripts/register_new_matrix_user.py index da0996edbc..d37ccccd5b 100644 --- a/synapse/_scripts/register_new_matrix_user.py +++ b/synapse/_scripts/register_new_matrix_user.py @@ -37,7 +37,7 @@ def request_registration( exit=sys.exit, ): - url = "%s/_matrix/client/r0/admin/register" % (server_location,) + url = "%s/_synapse/admin/v1/register" % (server_location,) # Get the nonce r = requests.get(url, verify=False) diff --git a/synapse/rest/admin/__init__.py b/synapse/rest/admin/__init__.py index 7a3a5c46ca..55ddebb4fe 100644 --- a/synapse/rest/admin/__init__.py +++ b/synapse/rest/admin/__init__.py @@ -21,11 +21,7 @@ import synapse from synapse.api.errors import Codes, NotFoundError, SynapseError from synapse.http.server import JsonResource from synapse.http.servlet import RestServlet, parse_json_object_from_request -from synapse.rest.admin._base import ( - admin_patterns, - assert_requester_is_admin, - historical_admin_path_patterns, -) +from synapse.rest.admin._base import admin_patterns, assert_requester_is_admin from synapse.rest.admin.devices import ( DeleteDevicesRestServlet, DeviceRestServlet, @@ -84,7 +80,7 @@ class VersionServlet(RestServlet): class PurgeHistoryRestServlet(RestServlet): - PATTERNS = historical_admin_path_patterns( + PATTERNS = admin_patterns( "/purge_history/(?P[^/]*)(/(?P[^/]+))?" ) @@ -169,9 +165,7 @@ class PurgeHistoryRestServlet(RestServlet): class PurgeHistoryStatusRestServlet(RestServlet): - PATTERNS = historical_admin_path_patterns( - "/purge_history_status/(?P[^/]+)" - ) + PATTERNS = admin_patterns("/purge_history_status/(?P[^/]+)") def __init__(self, hs): """ diff --git a/synapse/rest/admin/_base.py b/synapse/rest/admin/_base.py index db9fea263a..e09234c644 100644 --- a/synapse/rest/admin/_base.py +++ b/synapse/rest/admin/_base.py @@ -22,28 +22,6 @@ from synapse.api.errors import AuthError from synapse.types import UserID -def historical_admin_path_patterns(path_regex): - """Returns the list of patterns for an admin endpoint, including historical ones - - This is a backwards-compatibility hack. Previously, the Admin API was exposed at - various paths under /_matrix/client. This function returns a list of patterns - matching those paths (as well as the new one), so that existing scripts which rely - on the endpoints being available there are not broken. - - Note that this should only be used for existing endpoints: new ones should just - register for the /_synapse/admin path. - """ - return [ - re.compile(prefix + path_regex) - for prefix in ( - "^/_synapse/admin/v1", - "^/_matrix/client/api/v1/admin", - "^/_matrix/client/unstable/admin", - "^/_matrix/client/r0/admin", - ) - ] - - def admin_patterns(path_regex: str, version: str = "v1"): """Returns the list of patterns for an admin endpoint diff --git a/synapse/rest/admin/groups.py b/synapse/rest/admin/groups.py index 0b54ca09f4..d0c86b204a 100644 --- a/synapse/rest/admin/groups.py +++ b/synapse/rest/admin/groups.py @@ -16,10 +16,7 @@ import logging from synapse.api.errors import SynapseError from synapse.http.servlet import RestServlet -from synapse.rest.admin._base import ( - assert_user_is_admin, - historical_admin_path_patterns, -) +from synapse.rest.admin._base import admin_patterns, assert_user_is_admin logger = logging.getLogger(__name__) @@ -28,7 +25,7 @@ class DeleteGroupAdminRestServlet(RestServlet): """Allows deleting of local groups """ - PATTERNS = historical_admin_path_patterns("/delete_group/(?P[^/]*)") + PATTERNS = admin_patterns("/delete_group/(?P[^/]*)") def __init__(self, hs): self.group_server = hs.get_groups_server_handler() diff --git a/synapse/rest/admin/media.py b/synapse/rest/admin/media.py index ba50cb876d..c82b4f87d6 100644 --- a/synapse/rest/admin/media.py +++ b/synapse/rest/admin/media.py @@ -22,7 +22,6 @@ from synapse.rest.admin._base import ( admin_patterns, assert_requester_is_admin, assert_user_is_admin, - historical_admin_path_patterns, ) logger = logging.getLogger(__name__) @@ -34,10 +33,10 @@ class QuarantineMediaInRoom(RestServlet): """ PATTERNS = ( - historical_admin_path_patterns("/room/(?P[^/]+)/media/quarantine") + admin_patterns("/room/(?P[^/]+)/media/quarantine") + # This path kept around for legacy reasons - historical_admin_path_patterns("/quarantine_media/(?P[^/]+)") + admin_patterns("/quarantine_media/(?P[^/]+)") ) def __init__(self, hs): @@ -63,9 +62,7 @@ class QuarantineMediaByUser(RestServlet): this server. """ - PATTERNS = historical_admin_path_patterns( - "/user/(?P[^/]+)/media/quarantine" - ) + PATTERNS = admin_patterns("/user/(?P[^/]+)/media/quarantine") def __init__(self, hs): self.store = hs.get_datastore() @@ -90,7 +87,7 @@ class QuarantineMediaByID(RestServlet): it via this server. """ - PATTERNS = historical_admin_path_patterns( + PATTERNS = admin_patterns( "/media/quarantine/(?P[^/]+)/(?P[^/]+)" ) @@ -116,7 +113,7 @@ class ListMediaInRoom(RestServlet): """Lists all of the media in a given room. """ - PATTERNS = historical_admin_path_patterns("/room/(?P[^/]+)/media") + PATTERNS = admin_patterns("/room/(?P[^/]+)/media") def __init__(self, hs): self.store = hs.get_datastore() @@ -134,7 +131,7 @@ class ListMediaInRoom(RestServlet): class PurgeMediaCacheRestServlet(RestServlet): - PATTERNS = historical_admin_path_patterns("/purge_media_cache") + PATTERNS = admin_patterns("/purge_media_cache") def __init__(self, hs): self.media_repository = hs.get_media_repository() diff --git a/synapse/rest/admin/rooms.py b/synapse/rest/admin/rooms.py index ee345e12ce..353151169a 100644 --- a/synapse/rest/admin/rooms.py +++ b/synapse/rest/admin/rooms.py @@ -29,7 +29,6 @@ from synapse.rest.admin._base import ( admin_patterns, assert_requester_is_admin, assert_user_is_admin, - historical_admin_path_patterns, ) from synapse.storage.databases.main.room import RoomSortOrder from synapse.types import RoomAlias, RoomID, UserID, create_requester @@ -44,7 +43,7 @@ class ShutdownRoomRestServlet(RestServlet): joined to the new room. """ - PATTERNS = historical_admin_path_patterns("/shutdown_room/(?P[^/]+)") + PATTERNS = admin_patterns("/shutdown_room/(?P[^/]+)") def __init__(self, hs): self.hs = hs diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index fa8d8e6d91..b0ff5e1ead 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -33,8 +33,8 @@ from synapse.rest.admin._base import ( admin_patterns, assert_requester_is_admin, assert_user_is_admin, - historical_admin_path_patterns, ) +from synapse.rest.client.v2_alpha._base import client_patterns from synapse.types import JsonDict, UserID if TYPE_CHECKING: @@ -55,7 +55,7 @@ _GET_PUSHERS_ALLOWED_KEYS = { class UsersRestServlet(RestServlet): - PATTERNS = historical_admin_path_patterns("/users/(?P[^/]*)$") + PATTERNS = admin_patterns("/users/(?P[^/]*)$") def __init__(self, hs): self.hs = hs @@ -338,7 +338,7 @@ class UserRegisterServlet(RestServlet): nonce to the time it was generated, in int seconds. """ - PATTERNS = historical_admin_path_patterns("/register") + PATTERNS = admin_patterns("/register") NONCE_TIMEOUT = 60 def __init__(self, hs): @@ -461,7 +461,14 @@ class UserRegisterServlet(RestServlet): class WhoisRestServlet(RestServlet): - PATTERNS = historical_admin_path_patterns("/whois/(?P[^/]*)") + path_regex = "/whois/(?P[^/]*)$" + PATTERNS = ( + admin_patterns(path_regex) + + + # URL for spec reason + # https://matrix.org/docs/spec/client_server/r0.6.1#get-matrix-client-r0-admin-whois-userid + client_patterns("/admin" + path_regex, v1=True) + ) def __init__(self, hs): self.hs = hs @@ -485,7 +492,7 @@ class WhoisRestServlet(RestServlet): class DeactivateAccountRestServlet(RestServlet): - PATTERNS = historical_admin_path_patterns("/deactivate/(?P[^/]*)") + PATTERNS = admin_patterns("/deactivate/(?P[^/]*)") def __init__(self, hs): self._deactivate_account_handler = hs.get_deactivate_account_handler() @@ -516,7 +523,7 @@ class DeactivateAccountRestServlet(RestServlet): class AccountValidityRenewServlet(RestServlet): - PATTERNS = historical_admin_path_patterns("/account_validity/validity$") + PATTERNS = admin_patterns("/account_validity/validity$") def __init__(self, hs): """ @@ -559,9 +566,7 @@ class ResetPasswordRestServlet(RestServlet): 200 OK with empty object if success otherwise an error. """ - PATTERNS = historical_admin_path_patterns( - "/reset_password/(?P[^/]*)" - ) + PATTERNS = admin_patterns("/reset_password/(?P[^/]*)") def __init__(self, hs): self.store = hs.get_datastore() @@ -603,7 +608,7 @@ class SearchUsersRestServlet(RestServlet): 200 OK with json object {list[dict[str, Any]], count} or empty object. """ - PATTERNS = historical_admin_path_patterns("/search_users/(?P[^/]*)") + PATTERNS = admin_patterns("/search_users/(?P[^/]*)") def __init__(self, hs): self.hs = hs diff --git a/tests/rest/admin/test_admin.py b/tests/rest/admin/test_admin.py index 898e43411e..4f76f8f768 100644 --- a/tests/rest/admin/test_admin.py +++ b/tests/rest/admin/test_admin.py @@ -100,7 +100,7 @@ class DeleteGroupTestCase(unittest.HomeserverTestCase): self.assertIn(group_id, self._get_groups_user_is_in(self.other_user_token)) # Now delete the group - url = "/admin/delete_group/" + group_id + url = "/_synapse/admin/v1/delete_group/" + group_id request, channel = self.make_request( "POST", url.encode("ascii"), diff --git a/tests/rest/admin/test_room.py b/tests/rest/admin/test_room.py index 54824a5410..46933a0493 100644 --- a/tests/rest/admin/test_room.py +++ b/tests/rest/admin/test_room.py @@ -78,7 +78,7 @@ class ShutdownRoomTestCase(unittest.HomeserverTestCase): ) # Test that the admin can still send shutdown - url = "admin/shutdown_room/" + room_id + url = "/_synapse/admin/v1/shutdown_room/" + room_id request, channel = self.make_request( "POST", url.encode("ascii"), @@ -112,7 +112,7 @@ class ShutdownRoomTestCase(unittest.HomeserverTestCase): self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) # Test that the admin can still send shutdown - url = "admin/shutdown_room/" + room_id + url = "/_synapse/admin/v1/shutdown_room/" + room_id request, channel = self.make_request( "POST", url.encode("ascii"), diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index 9661af7e79..54d46f4bd3 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -41,7 +41,7 @@ class UserRegisterTestCase(unittest.HomeserverTestCase): def make_homeserver(self, reactor, clock): - self.url = "/_matrix/client/r0/admin/register" + self.url = "/_synapse/admin/v1/register" self.registration_handler = Mock() self.identity_handler = Mock() @@ -1768,3 +1768,111 @@ class UserTokenRestTestCase(unittest.HomeserverTestCase): # though the MAU limit would stop the user doing so. puppet_token = self._get_token() self.helper.join(room_id, user=self.other_user, tok=puppet_token) + + +class WhoisRestTestCase(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.url1 = "/_synapse/admin/v1/whois/%s" % urllib.parse.quote(self.other_user) + self.url2 = "/_matrix/client/r0/admin/whois/%s" % urllib.parse.quote( + self.other_user + ) + + def test_no_auth(self): + """ + Try to get information of an user without authentication. + """ + request, channel = self.make_request("GET", self.url1, b"{}") + self.assertEqual(401, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(Codes.MISSING_TOKEN, channel.json_body["errcode"]) + + request, channel = self.make_request("GET", self.url2, 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_not_admin(self): + """ + If the user is not a server admin, an error is returned. + """ + self.register_user("user2", "pass") + other_user2_token = self.login("user2", "pass") + + request, channel = self.make_request( + "GET", self.url1, access_token=other_user2_token, + ) + self.assertEqual(403, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(Codes.FORBIDDEN, channel.json_body["errcode"]) + + request, channel = self.make_request( + "GET", self.url2, access_token=other_user2_token, + ) + self.assertEqual(403, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(Codes.FORBIDDEN, 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 + """ + url1 = "/_synapse/admin/v1/whois/@unknown_person:unknown_domain" + url2 = "/_matrix/client/r0/admin/whois/@unknown_person:unknown_domain" + + request, channel = self.make_request( + "GET", url1, access_token=self.admin_user_tok, + ) + self.assertEqual(400, channel.code, msg=channel.json_body) + self.assertEqual("Can only whois a local user", channel.json_body["error"]) + + request, channel = self.make_request( + "GET", url2, access_token=self.admin_user_tok, + ) + self.assertEqual(400, channel.code, msg=channel.json_body) + self.assertEqual("Can only whois a local user", channel.json_body["error"]) + + def test_get_whois_admin(self): + """ + The lookup should succeed for an admin. + """ + request, channel = self.make_request( + "GET", self.url1, access_token=self.admin_user_tok, + ) + self.assertEqual(200, channel.code, msg=channel.json_body) + self.assertEqual(self.other_user, channel.json_body["user_id"]) + self.assertIn("devices", channel.json_body) + + request, channel = self.make_request( + "GET", self.url2, access_token=self.admin_user_tok, + ) + self.assertEqual(200, channel.code, msg=channel.json_body) + self.assertEqual(self.other_user, channel.json_body["user_id"]) + self.assertIn("devices", channel.json_body) + + def test_get_whois_user(self): + """ + The lookup should succeed for a normal user looking up their own information. + """ + other_user_token = self.login("user", "pass") + + request, channel = self.make_request( + "GET", self.url1, access_token=other_user_token, + ) + self.assertEqual(200, channel.code, msg=channel.json_body) + self.assertEqual(self.other_user, channel.json_body["user_id"]) + self.assertIn("devices", channel.json_body) + + request, channel = self.make_request( + "GET", self.url2, access_token=other_user_token, + ) + self.assertEqual(200, channel.code, msg=channel.json_body) + self.assertEqual(self.other_user, channel.json_body["user_id"]) + self.assertIn("devices", channel.json_body) diff --git a/tests/rest/client/v2_alpha/test_register.py b/tests/rest/client/v2_alpha/test_register.py index 88923fcea4..699a40c3df 100644 --- a/tests/rest/client/v2_alpha/test_register.py +++ b/tests/rest/client/v2_alpha/test_register.py @@ -342,7 +342,7 @@ class AccountValidityTestCase(unittest.HomeserverTestCase): self.register_user("admin", "adminpassword", admin=True) admin_tok = self.login("admin", "adminpassword") - url = "/_matrix/client/unstable/admin/account_validity/validity" + url = "/_synapse/admin/v1/account_validity/validity" params = {"user_id": user_id} request_data = json.dumps(params) request, channel = self.make_request( @@ -362,7 +362,7 @@ class AccountValidityTestCase(unittest.HomeserverTestCase): self.register_user("admin", "adminpassword", admin=True) admin_tok = self.login("admin", "adminpassword") - url = "/_matrix/client/unstable/admin/account_validity/validity" + url = "/_synapse/admin/v1/account_validity/validity" params = { "user_id": user_id, "expiration_ts": 0, @@ -389,7 +389,7 @@ class AccountValidityTestCase(unittest.HomeserverTestCase): self.register_user("admin", "adminpassword", admin=True) admin_tok = self.login("admin", "adminpassword") - url = "/_matrix/client/unstable/admin/account_validity/validity" + url = "/_synapse/admin/v1/account_validity/validity" params = { "user_id": user_id, "expiration_ts": 0, diff --git a/tests/storage/test_client_ips.py b/tests/storage/test_client_ips.py index 6bdde1a2ba..a69117c5a9 100644 --- a/tests/storage/test_client_ips.py +++ b/tests/storage/test_client_ips.py @@ -416,7 +416,7 @@ class ClientIpAuthTestCase(unittest.HomeserverTestCase): self.reactor, self.site, "GET", - "/_matrix/client/r0/admin/users/" + self.user_id, + "/_synapse/admin/v1/users/" + self.user_id, access_token=access_token, custom_headers=headers1.items(), **make_request_args, diff --git a/tests/unittest.py b/tests/unittest.py index c7c889c405..a9d59e31f7 100644 --- a/tests/unittest.py +++ b/tests/unittest.py @@ -554,7 +554,7 @@ class HomeserverTestCase(TestCase): self.hs.config.registration_shared_secret = "shared" # Create the user - request, channel = self.make_request("GET", "/_matrix/client/r0/admin/register") + request, channel = self.make_request("GET", "/_synapse/admin/v1/register") self.assertEqual(channel.code, 200, msg=channel.result) nonce = channel.json_body["nonce"] @@ -580,7 +580,7 @@ class HomeserverTestCase(TestCase): } ) request, channel = self.make_request( - "POST", "/_matrix/client/r0/admin/register", body.encode("utf8") + "POST", "/_synapse/admin/v1/register", body.encode("utf8") ) self.assertEqual(channel.code, 200, channel.json_body)