From 27bedec7fafe1a540901bc98606763dbce9900c7 Mon Sep 17 00:00:00 2001 From: David Teller Date: Tue, 15 Dec 2020 16:45:44 +0100 Subject: [PATCH 1/5] Resolves #8947 - /_synapse/admin/v1/users//joined_rooms also works for remote users As far as I can tell, the code simply filters off remote users but calls code that works for both local and remote users. Signed-off-by: David Teller --- changelog.d/8948.feature | 1 + docs/admin_api/user_admin_api.rst | 4 +++ synapse/rest/admin/users.py | 10 +++--- tests/rest/admin/test_user.py | 57 ++++++++++++++++++++++++------- 4 files changed, 54 insertions(+), 18 deletions(-) create mode 100644 changelog.d/8948.feature diff --git a/changelog.d/8948.feature b/changelog.d/8948.feature new file mode 100644 index 000000000000..3b06cbfa222a --- /dev/null +++ b/changelog.d/8948.feature @@ -0,0 +1 @@ +Update `/_synapse/admin/v1/users//joined_rooms` to work for both local and remote users. diff --git a/docs/admin_api/user_admin_api.rst b/docs/admin_api/user_admin_api.rst index e4d6f8203b7c..5e8a43c9006d 100644 --- a/docs/admin_api/user_admin_api.rst +++ b/docs/admin_api/user_admin_api.rst @@ -337,6 +337,10 @@ A response body like the following is returned: "total": 2 } +The server returns the list of rooms of which the user and the server +are member. If the user is local, that's all the rooms of which the +user is member. + **Parameters** The following parameters should be set in the URL: diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index 6658c2da5666..ee023bb5c450 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -714,12 +714,10 @@ def __init__(self, hs): async def on_GET(self, request, user_id): await assert_requester_is_admin(self.auth, request) - if not self.is_mine(UserID.from_string(user_id)): - raise SynapseError(400, "Can only lookup local users") - - user = await self.store.get_user_by_id(user_id) - if user is None: - raise NotFoundError("Unknown user") + if self.is_mine(UserID.from_string(user_id)): + # Let's get rid of unknown local users quickly. + if await self.store.get_user_by_id(user_id) is None: + raise NotFoundError("Unknown user") room_ids = await self.store.get_rooms_for_user(user_id) ret = {"joined_rooms": list(room_ids), "total": len(room_ids)} diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index 9b2e4765f69f..c82cbe037d9e 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -25,6 +25,7 @@ import synapse.rest.admin from synapse.api.constants import UserTypes 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 @@ -166,7 +167,7 @@ def test_register_correct_nonce(self): "mac": want_mac, } ) - channel = self.make_request("POST", self.url, body.encode("utf8")) + request, channel = self.make_request("POST", self.url, body.encode("utf8")) self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) self.assertEqual("@bob:test", channel.json_body["user_id"]) @@ -1244,17 +1245,6 @@ def test_user_does_not_exist(self): self.assertEqual(404, channel.code, msg=channel.json_body) self.assertEqual(Codes.NOT_FOUND, channel.json_body["errcode"]) - def test_user_is_not_local(self): - """ - Tests that a lookup for a user that is not a local returns a 400 - """ - url = "/_synapse/admin/v1/users/@unknown_person:unknown_domain/joined_rooms" - - channel = self.make_request("GET", url, access_token=self.admin_user_tok,) - - self.assertEqual(400, channel.code, msg=channel.json_body) - self.assertEqual("Can only lookup local users", channel.json_body["error"]) - def test_no_memberships(self): """ Tests that a normal lookup for rooms is successfully @@ -1284,6 +1274,49 @@ def test_get_rooms(self): self.assertEqual(number_rooms, channel.json_body["total"]) self.assertEqual(number_rooms, len(channel.json_body["joined_rooms"])) + def test_get_rooms_with_nonlocal_user(self): + """ + Tests that a normal lookup for rooms is successful with a non-local user + """ + + other_user_tok = self.login("user", "pass") + event_builder_factory = self.hs.get_event_builder_factory() + event_creation_handler = self.hs.get_event_creation_handler() + storage = self.hs.get_storage() + + # Create two rooms, one with a local user only and one with both a local + # and remote user. + self.helper.create_room_as(self.other_user, tok=other_user_tok) + local_and_remote_room_id = self.helper.create_room_as( + self.other_user, tok=other_user_tok + ) + + # Add a remote user to the room. + builder = event_builder_factory.for_room_version( + RoomVersions.V1, + { + "type": "m.room.member", + "sender": "@joiner:remote_hs", + "state_key": "@joiner:remote_hs", + "room_id": local_and_remote_room_id, + "content": {"membership": "join"}, + }, + ) + + event, context = self.get_success( + event_creation_handler.create_new_client_event(builder) + ) + + self.get_success(storage.persistence.persist_event(event, context)) + + # Now get rooms + url = "/_synapse/admin/v1/users/@joiner:remote_hs/joined_rooms" + channel = self.make_request("GET", url, access_token=self.admin_user_tok,) + + self.assertEqual(200, channel.code, msg=channel.json_body) + self.assertEqual(1, channel.json_body["total"]) + self.assertEqual([local_and_remote_room_id], channel.json_body["joined_rooms"]) + class PushersRestTestCase(unittest.HomeserverTestCase): From 382407202dd9834efea25f396b42fc4b588e10a0 Mon Sep 17 00:00:00 2001 From: David Teller Date: Thu, 7 Jan 2021 10:00:04 +0100 Subject: [PATCH 2/5] FIXUP: Tests --- tests/rest/admin/test_user.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index c82cbe037d9e..c52b97a9728c 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -167,7 +167,7 @@ def test_register_correct_nonce(self): "mac": want_mac, } ) - request, channel = self.make_request("POST", self.url, body.encode("utf8")) + channel = self.make_request("POST", self.url, body.encode("utf8")) self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) self.assertEqual("@bob:test", channel.json_body["user_id"]) From 93410799135a3be19a964a8f864b00da60afe35e Mon Sep 17 00:00:00 2001 From: David Teller Date: Thu, 7 Jan 2021 15:13:07 +0100 Subject: [PATCH 3/5] FIXUP: Remove special error NotFoundError --- synapse/rest/admin/users.py | 5 ----- tests/rest/admin/test_user.py | 10 ---------- 2 files changed, 15 deletions(-) diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index ee023bb5c450..f8a73e7d9d72 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -714,11 +714,6 @@ def __init__(self, hs): async def on_GET(self, request, user_id): await assert_requester_is_admin(self.auth, request) - if self.is_mine(UserID.from_string(user_id)): - # Let's get rid of unknown local users quickly. - if await self.store.get_user_by_id(user_id) is None: - raise NotFoundError("Unknown user") - room_ids = await self.store.get_rooms_for_user(user_id) ret = {"joined_rooms": list(room_ids), "total": len(room_ids)} return 200, ret diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index c52b97a9728c..08d2a6e4a872 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -1235,16 +1235,6 @@ def test_requester_is_no_admin(self): self.assertEqual(403, int(channel.result["code"]), msg=channel.result["body"]) self.assertEqual(Codes.FORBIDDEN, channel.json_body["errcode"]) - def test_user_does_not_exist(self): - """ - Tests that a lookup for a user that does not exist returns a 404 - """ - url = "/_synapse/admin/v1/users/@unknown_person:test/joined_rooms" - channel = self.make_request("GET", url, access_token=self.admin_user_tok,) - - self.assertEqual(404, channel.code, msg=channel.json_body) - self.assertEqual(Codes.NOT_FOUND, channel.json_body["errcode"]) - def test_no_memberships(self): """ Tests that a normal lookup for rooms is successfully From 5b3f0d6b846f0f50a13e960ae1290a239cf8a201 Mon Sep 17 00:00:00 2001 From: David Teller Date: Thu, 7 Jan 2021 18:56:05 +0100 Subject: [PATCH 4/5] FIXUP: Tests --- tests/rest/admin/test_user.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index 08d2a6e4a872..2a2cdaa64033 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -1235,6 +1235,29 @@ def test_requester_is_no_admin(self): self.assertEqual(403, int(channel.result["code"]), msg=channel.result["body"]) self.assertEqual(Codes.FORBIDDEN, channel.json_body["errcode"]) + def test_user_does_not_exist(self): + """ + Tests that a lookup for a user that does not exist returns an empty list + """ + url = "/_synapse/admin/v1/users/@unknown_person:test/joined_rooms" + channel = self.make_request("GET", url, access_token=self.admin_user_tok,) + + self.assertEqual(200, channel.code, msg=channel.json_body) + self.assertEqual(0, channel.json_body["total"]) + self.assertEqual(0, len(channel.json_body["joined_rooms"])) + + def test_user_is_not_local(self): + """ + Tests that a lookup for a user that is not a local and participates in no conversation returns an empty list + """ + url = "/_synapse/admin/v1/users/@unknown_person:unknown_domain/joined_rooms" + + channel = self.make_request("GET", url, access_token=self.admin_user_tok,) + + self.assertEqual(200, channel.code, msg=channel.json_body) + self.assertEqual(0, channel.json_body["total"]) + self.assertEqual(0, len(channel.json_body["joined_rooms"])) + def test_no_memberships(self): """ Tests that a normal lookup for rooms is successfully From 3b590fc026470dbc7d557f485a5f904d9fb33acd Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 11 Jan 2021 10:11:47 -0500 Subject: [PATCH 5/5] Update docs. --- docs/admin_api/user_admin_api.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/admin_api/user_admin_api.rst b/docs/admin_api/user_admin_api.rst index 5e8a43c9006d..3115951e1fc0 100644 --- a/docs/admin_api/user_admin_api.rst +++ b/docs/admin_api/user_admin_api.rst @@ -338,8 +338,8 @@ A response body like the following is returned: } The server returns the list of rooms of which the user and the server -are member. If the user is local, that's all the rooms of which the -user is member. +are member. If the user is local, all the rooms of which the user is +member are returned. **Parameters**