From 4c71ec29f422626cefbc91de1d95b939b6510cde Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 17 Aug 2022 19:03:05 +0100 Subject: [PATCH 1/7] Expose last_seen_user_agent for users if experimental option is enabled This also exposes the stable version of the field to the: * GET /_synapse/admin/v2/users//devices * GET /_synapse/admin/v2/users//devices/ Admin APIs. --- synapse/config/experimental.py | 3 +++ synapse/handlers/device.py | 9 ++++++++- synapse/rest/client/devices.py | 27 +++++++++++++++++++++++++++ 3 files changed, 38 insertions(+), 1 deletion(-) diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index 7d17c958bb94..c1ff41753994 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -90,3 +90,6 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: # MSC3848: Introduce errcodes for specific event sending failures self.msc3848_enabled: bool = experimental.get("msc3848_enabled", False) + + # MSC3852: Expose last seen user agent field on /_matrix/client/v3/devices. + self.msc3852_enabled: bool = experimental.get("msc3852_enabled", False) diff --git a/synapse/handlers/device.py b/synapse/handlers/device.py index 1a8379854ce1..f5c586f65785 100644 --- a/synapse/handlers/device.py +++ b/synapse/handlers/device.py @@ -74,6 +74,7 @@ def __init__(self, hs: "HomeServer"): self._state_storage = hs.get_storage_controllers().state self._auth_handler = hs.get_auth_handler() self.server_name = hs.hostname + self._msc3852_enabled = hs.config.experimental.msc3852_enabled @trace async def get_devices_by_user(self, user_id: str) -> List[JsonDict]: @@ -747,7 +748,13 @@ def _update_device_from_client_ips( device: JsonDict, client_ips: Mapping[Tuple[str, str], Mapping[str, Any]] ) -> None: ip = client_ips.get((device["user_id"], device["device_id"]), {}) - device.update({"last_seen_ts": ip.get("last_seen"), "last_seen_ip": ip.get("ip")}) + device.update( + { + "last_seen_user_agent": ip.get("user_agent"), + "last_seen_ts": ip.get("last_seen"), + "last_seen_ip": ip.get("ip"), + } + ) class DeviceListUpdater: diff --git a/synapse/rest/client/devices.py b/synapse/rest/client/devices.py index 6fab102437f7..ed6ce78d4771 100644 --- a/synapse/rest/client/devices.py +++ b/synapse/rest/client/devices.py @@ -42,12 +42,26 @@ def __init__(self, hs: "HomeServer"): self.hs = hs self.auth = hs.get_auth() self.device_handler = hs.get_device_handler() + self._msc3852_enabled = hs.config.experimental.msc3852_enabled async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]: requester = await self.auth.get_user_by_req(request, allow_guest=True) devices = await self.device_handler.get_devices_by_user( requester.user.to_string() ) + + # If MSC3852 is disabled, then the "last_seen_user_agent" field will be + # removed from each device. If it is enabled, then the field name will + # be replaced by the unstable identifier. + # + # When MSC3852 is accepted, this block of code can just be removed to + # expose "last_seen_user_agent" to clients. + for device in devices: + last_seen_user_agent = device["last_seen_user_agent"] + del device["last_seen_user_agent"] + if self._msc3852_enabled: + device["org.matrix.msc3852.last_seen_user_agent"] = last_seen_user_agent + return 200, {"devices": devices} @@ -108,6 +122,7 @@ def __init__(self, hs: "HomeServer"): self.auth = hs.get_auth() self.device_handler = hs.get_device_handler() self.auth_handler = hs.get_auth_handler() + self._msc3852_enabled = hs.config.experimental.msc3852_enabled async def on_GET( self, request: SynapseRequest, device_id: str @@ -118,6 +133,18 @@ async def on_GET( ) if device is None: raise NotFoundError("No device found") + + # If MSC3852 is disabled, then the "last_seen_user_agent" field will be + # removed from each device. If it is enabled, then the field name will + # be replaced by the unstable identifier. + # + # When MSC3852 is accepted, this block of code can just be removed to + # expose "last_seen_user_agent" to clients. + last_seen_user_agent = device["last_seen_user_agent"] + del device["last_seen_user_agent"] + if self._msc3852_enabled: + device["org.matrix.msc3852.last_seen_user_agent"] = last_seen_user_agent + return 200, device @interactive_auth_handler From 2e94efe503b49b8a14a50be5379238fb53e717b0 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 17 Aug 2022 19:07:04 +0100 Subject: [PATCH 2/7] Add 'last_seen_user_agent' resp fields to Admin API documentation --- docs/admin_api/user_admin_api.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/admin_api/user_admin_api.md b/docs/admin_api/user_admin_api.md index 0871cfebf566..c1ca0c8a64d9 100644 --- a/docs/admin_api/user_admin_api.md +++ b/docs/admin_api/user_admin_api.md @@ -753,6 +753,7 @@ A response body like the following is returned: "device_id": "QBUAZIFURK", "display_name": "android", "last_seen_ip": "1.2.3.4", + "last_seen_user_agent": "Mozilla/5.0 (X11; Linux x86_64; rv:103.0) Gecko/20100101 Firefox/103.0", "last_seen_ts": 1474491775024, "user_id": "" }, @@ -760,6 +761,7 @@ A response body like the following is returned: "device_id": "AUIECTSRND", "display_name": "ios", "last_seen_ip": "1.2.3.5", + "last_seen_user_agent": "Mozilla/5.0 (X11; Linux x86_64; rv:103.0) Gecko/20100101 Firefox/103.0", "last_seen_ts": 1474491775025, "user_id": "" } @@ -786,6 +788,8 @@ The following fields are returned in the JSON response body: Absent if no name has been set. - `last_seen_ip` - The IP address where this device was last seen. (May be a few minutes out of date, for efficiency reasons). + - `last_seen_user_agent` - The user agent of the device when it was last seen. + (May be a few minutes out of date, for efficiency reasons). - `last_seen_ts` - The timestamp (in milliseconds since the unix epoch) when this devices was last seen. (May be a few minutes out of date, for efficiency reasons). - `user_id` - Owner of device. @@ -837,6 +841,7 @@ A response body like the following is returned: "device_id": "", "display_name": "android", "last_seen_ip": "1.2.3.4", + "last_seen_user_agent": "Mozilla/5.0 (X11; Linux x86_64; rv:103.0) Gecko/20100101 Firefox/103.0", "last_seen_ts": 1474491775024, "user_id": "" } @@ -858,6 +863,8 @@ The following fields are returned in the JSON response body: Absent if no name has been set. - `last_seen_ip` - The IP address where this device was last seen. (May be a few minutes out of date, for efficiency reasons). + - `last_seen_user_agent` - The user agent of the device when it was last seen. + (May be a few minutes out of date, for efficiency reasons). - `last_seen_ts` - The timestamp (in milliseconds since the unix epoch) when this devices was last seen. (May be a few minutes out of date, for efficiency reasons). - `user_id` - Owner of device. From 3a1f89f61930322455fdf584a221da6f11da565d Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 18 Aug 2022 17:58:04 +0100 Subject: [PATCH 3/7] Add an 'additional_request_fields' arg to login test helper So that we can specify an initial device name in the next commit --- tests/unittest.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/unittest.py b/tests/unittest.py index bec4a3d02396..975b0a23a7b7 100644 --- a/tests/unittest.py +++ b/tests/unittest.py @@ -677,14 +677,29 @@ def login( username: str, password: str, device_id: Optional[str] = None, + additional_request_fields: Optional[Dict[str, str]] = None, custom_headers: Optional[Iterable[CustomHeaderType]] = None, ) -> str: """ Log in a user, and get an access token. Requires the Login API be registered. + + Args: + username: The localpart to assign to the new user. + password: The password to assign to the new user. + device_id: An optional device ID to assign to the new device created during + login. + additional_request_fields: A dictionary containing any additional /login + request fields and their values. + custom_headers: Custom HTTP headers and values to add to the /login request. + + Returns: + The newly registered user's Matrix ID. """ body = {"type": "m.login.password", "user": username, "password": password} if device_id: body["device_id"] = device_id + if additional_request_fields: + body.update(additional_request_fields) channel = self.make_request( "POST", From ff6ec45c2509d56b43f362132823ea77a94ab240 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 18 Aug 2022 17:58:21 +0100 Subject: [PATCH 4/7] Add a unit test for listing device information from the Admin API --- tests/rest/admin/test_user.py | 92 ++++++++++++++++++++++++++++++++++- 1 file changed, 91 insertions(+), 1 deletion(-) diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index 411e4ec00557..ea64f96e6ac3 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -1,4 +1,4 @@ -# Copyright 2018-2021 The Matrix.org Foundation C.I.C. +# Copyright 2018-2022 The Matrix.org Foundation C.I.C. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -904,6 +904,96 @@ def _create_users(self, number_users: int) -> None: ) +class UserDevicesTestCase(unittest.HomeserverTestCase): + """ + Tests user device management-related Admin APIs. + """ + + servlets = [ + synapse.rest.admin.register_servlets, + login.register_servlets, + sync.register_servlets, + ] + + def prepare( + self, reactor: MemoryReactor, clock: Clock, homeserver: HomeServer + ) -> None: + # Set up an Admin user to query the Admin API with. + self.admin_user_id = self.register_user("admin", "pass", admin=True) + self.admin_user_token = self.login("admin", "pass") + + # Set up a test user to query the devices of. + self.other_user_device_id = "TESTDEVICEID" + self.other_user_device_display_name = "My Test Device" + self.other_user_client_ip = "1.2.3.4" + self.other_user_user_agent = "EquestriaTechnology/123.0" + + self.other_user_id = self.register_user("user", "pass", displayname="User1") + self.other_user_token = self.login( + "user", + "pass", + device_id=self.other_user_device_id, + additional_request_fields={ + "initial_device_display_name": self.other_user_device_display_name, + }, + ) + + # Have ther "other user" make a request so that the "last_seen_*" fields are + # populated in the tests below. + channel = self.make_request( + "GET", + "/_matrix/client/v3/sync", + access_token=self.other_user_token, + client_ip=self.other_user_client_ip, + custom_headers=[ + ("User-Agent", self.other_user_user_agent), + ], + ) + self.assertEqual(200, channel.code, msg=channel.json_body) + + def test_list_user_devices(self) -> None: + """ + Tests that a user's devices and attributes are listed correctly via the Admin API. + """ + # Request all devices of "other user" + channel = self.make_request( + "GET", + f"/_synapse/admin/v2/users/{self.other_user_id}/devices", + access_token=self.admin_user_token, + ) + self.assertEqual(200, channel.code, msg=channel.json_body) + + # Double-check we got the single device expected + user_devices = channel.json_body["devices"] + self.assertEqual(len(user_devices), 1) + self.assertEqual(channel.json_body["total"], 1) + + # Check that all the attributes of the device reported are as expected. + self._validate_attributes_of_device_response(user_devices[0]) + + # Request just a single device for "other user" by its ID + channel = self.make_request( + "GET", + f"/_synapse/admin/v2/users/{self.other_user_id}/devices/" + f"{self.other_user_device_id}", + access_token=self.admin_user_token, + ) + self.assertEqual(200, channel.code, msg=channel.json_body) + + # Check that all the attributes of the device reported are as expected. + self._validate_attributes_of_device_response(channel.json_body) + + def _validate_attributes_of_device_response(self, response: JsonDict) -> None: + # Check that all device expected attributes are present + self.assertEqual(response["user_id"], self.other_user_id) + self.assertEqual(response["device_id"], self.other_user_device_id) + self.assertEqual(response["display_name"], self.other_user_device_display_name) + self.assertEqual(response["last_seen_ip"], self.other_user_client_ip) + self.assertEqual(response["last_seen_user_agent"], self.other_user_user_agent) + self.assertTrue(isinstance(response["last_seen_ts"], int)) + self.assertGreater(response["last_seen_ts"], 0) + + class DeactivateAccountTestCase(unittest.HomeserverTestCase): servlets = [ From 0410e3f11f3ef8c63151cf6758b192d19645be14 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 18 Aug 2022 18:11:34 +0100 Subject: [PATCH 5/7] changelogs --- changelog.d/13549.feature | 1 + changelog.d/13549.misc | 1 + 2 files changed, 2 insertions(+) create mode 100644 changelog.d/13549.feature create mode 100644 changelog.d/13549.misc diff --git a/changelog.d/13549.feature b/changelog.d/13549.feature new file mode 100644 index 000000000000..b6a726789cc7 --- /dev/null +++ b/changelog.d/13549.feature @@ -0,0 +1 @@ +Add an experimental implementation for [MSC3852](https://github.com/matrix-org/matrix-spec-proposals/pull/3852). \ No newline at end of file diff --git a/changelog.d/13549.misc b/changelog.d/13549.misc new file mode 100644 index 000000000000..5b4303e87e94 --- /dev/null +++ b/changelog.d/13549.misc @@ -0,0 +1 @@ +Allow specifying additional request fields when using the `HomeServerTestCase.login` helper method. \ No newline at end of file From f6edde4d53dcd1987913e317e28b2cb894594658 Mon Sep 17 00:00:00 2001 From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Date: Fri, 19 Aug 2022 16:45:19 +0100 Subject: [PATCH 6/7] Update tests/rest/admin/test_user.py Co-authored-by: Sean Quah <8349537+squahtx@users.noreply.github.com> --- 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 ea64f96e6ac3..11a57919a8ef 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -938,7 +938,7 @@ def prepare( }, ) - # Have ther "other user" make a request so that the "last_seen_*" fields are + # Have the "other user" make a request so that the "last_seen_*" fields are # populated in the tests below. channel = self.make_request( "GET", From 4fccfdaf76df66df5273b3b835ac4ec987e2bb45 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 19 Aug 2022 16:46:13 +0100 Subject: [PATCH 7/7] Utilise assertIsInstance --- 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 11a57919a8ef..1afd082707c2 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -990,7 +990,7 @@ def _validate_attributes_of_device_response(self, response: JsonDict) -> None: self.assertEqual(response["display_name"], self.other_user_device_display_name) self.assertEqual(response["last_seen_ip"], self.other_user_client_ip) self.assertEqual(response["last_seen_user_agent"], self.other_user_user_agent) - self.assertTrue(isinstance(response["last_seen_ts"], int)) + self.assertIsInstance(response["last_seen_ts"], int) self.assertGreater(response["last_seen_ts"], 0)