From 52ddb79781f5527d351a5d231c61e5ad09d86d13 Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Fri, 24 Jul 2020 17:49:57 -0400 Subject: [PATCH 01/25] initial version of dehydration --- synapse/handlers/device.py | 78 ++++++++++- synapse/rest/client/v1/login.py | 55 ++++++++ synapse/rest/client/v2_alpha/keys.py | 31 ++-- synapse/storage/data_stores/main/devices.py | 132 +++++++++++++++++- .../main/schema/delta/58/11dehydration.sql | 30 ++++ tests/rest/client/v1/test_login.py | 69 +++++++++ 6 files changed, 375 insertions(+), 20 deletions(-) create mode 100644 synapse/storage/data_stores/main/schema/delta/58/11dehydration.sql diff --git a/synapse/handlers/device.py b/synapse/handlers/device.py index db417d60deb4..614b10ca9ef3 100644 --- a/synapse/handlers/device.py +++ b/synapse/handlers/device.py @@ -14,8 +14,9 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +import json import logging -from typing import Any, Dict, List, Optional +from typing import Any, Dict, List, Optional, Tuple from synapse.api import errors from synapse.api.constants import EventTypes @@ -28,6 +29,7 @@ from synapse.logging.opentracing import log_kv, set_tag, trace from synapse.metrics.background_process_metrics import run_as_background_process from synapse.types import ( + JsonDict, RoomStreamToken, get_domain_from_id, get_verify_key_from_cross_signing_key, @@ -489,6 +491,80 @@ async def user_left_room(self, user, room_id): # receive device updates. Mark this in DB. await self.store.mark_remote_user_device_list_as_unsubscribed(user_id) + async def store_dehydrated_device( + self, user_id: str, device_data: str, + initial_device_display_name: Optional[str] = None) -> str: + device_id = await self.check_device_registered( + user_id, None, initial_device_display_name, + ) + old_device_id = await self.store.store_dehydrated_device(user_id, device_id, device_data) + if old_device_id is not None: + await self.delete_device(user_id, old_device_id) + return device_id + + async def get_dehydrated_device(self, user_id: str) -> Tuple[str, str]: + return await self.store.get_dehydrated_device(user_id) + + async def get_dehydration_token(self, user_id: str, device_id: str, login_submission: JsonDict) -> str: + return await self.store.create_dehydration_token(user_id, device_id, json.dumps(login_submission)) + + async def rehydrate_device(self, token: str) -> dict: + # FIXME: if can't find token, return 404 + token_info = await self.store.clear_dehydration_token(token, True) + + # normally, the constructor would do self.registration_handler = + # self.hs.get_registration_handler(), but doing that results in a + # circular dependency in the handlers. So do this for now + registration_handler = self.hs.get_registration_handler() + + if token_info["dehydrated"]: + # create access token for dehydrated device + initial_display_name = None # FIXME: get display name from login submission? + device_id, access_token = await registration_handler.register_device( + token_info.get("user_id"), token_info.get("device_id"), initial_display_name + ) + + return { + "user_id": token_info.get("user_id"), + "access_token": access_token, + "home_server": self.hs.hostname, + "device_id": device_id, + } + + else: + # create device and access token from original login submission + login_submission = token_info.get("login_submission") + device_id = login_submission.get("device_id") + initial_display_name = login_submission.get("initial_device_display_name") + device_id, access_token = await registration_handler.register_device( + token_info.get("user_id"), device_id, initial_display_name + ) + + return { + "user_id": token.info.get("user_id"), + "access_token": access_token, + "home_server": self.hs.hostname, + "device_id": device_id, + } + + async def cancel_rehydrate(self, token: str) -> dict: + # FIXME: if can't find token, return 404 + token_info = await self.store.clear_dehydration_token(token) + # create device and access token from original login submission + login_submission = token_info.get("login_submission") + device_id = login_submission.get("device_id") + initial_display_name = login_submission.get("initial_device_display_name") + device_id, access_token = await self.registration_handler.register_device( + token_info.get("user_id"), device_id, initial_display_name + ) + + return { + "user_id": token_info.get("user_id"), + "access_token": access_token, + "home_server": self.hs.hostname, + "device_id": device_id, + } + def _update_device_from_client_ips(device, client_ips): ip = client_ips.get((device["user_id"], device["device_id"]), {}) diff --git a/synapse/rest/client/v1/login.py b/synapse/rest/client/v1/login.py index 379f668d6f8a..3e6da34de95d 100644 --- a/synapse/rest/client/v1/login.py +++ b/synapse/rest/client/v1/login.py @@ -103,6 +103,7 @@ def __init__(self, hs): self.oidc_enabled = hs.config.oidc_enabled self.auth_handler = self.hs.get_auth_handler() + self.device_handler = hs.get_device_handler() self.registration_handler = hs.get_registration_handler() self.handlers = hs.get_handlers() self._well_known_builder = WellKnownBuilder(hs) @@ -339,6 +340,22 @@ async def _complete_login( ) user_id = canonical_uid + if login_submission.get("org.matrix.msc2697.restore_device"): + device_id, dehydrated_device = await self.device_handler.get_dehydrated_device(user_id) + if dehydrated_device: + token = await self.device_handler.get_dehydration_token(user_id, device_id, login_submission) + result = { + "user_id": user_id, + "home_server": self.hs.hostname, + "device_data": dehydrated_device, + "device_id": device_id, + "dehydration_token": token, + } + + # FIXME: call callback? + + return result + device_id = login_submission.get("device_id") initial_display_name = login_submission.get("initial_device_display_name") device_id, access_token = await self.registration_handler.register_device( @@ -401,6 +418,42 @@ async def _do_jwt_login(self, login_submission: JsonDict) -> Dict[str, str]: return result +class RestoreDeviceServlet(RestServlet): + PATTERNS = client_patterns("/org.matrix.msc26997/restore_device") + + def __init__(self, hs): + super(RestoreDeviceServlet, self).__init__() + self.hs = hs + self.device_handler = hs.get_device_handler() + + async def on_POST(self, request: SynapseRequest): + submission = parse_json_object_from_request(request) + + if submission.get("rehydrate"): + return 200, await self.device_handler.rehydrate_device(submission.get("dehydration_token")) + else: + return 200, await self.device_handler.cancel_rehydrate(submission.get("dehydration_token")) + + +class StoreDeviceServlet(RestServlet): + PATTERNS = client_patterns("/org.matrix.msc2697/device/dehydrate") + + def __init__(self, hs): + super(StoreDeviceServlet, self).__init__() + self.hs = hs + self.auth = hs.get_auth() + self.device_handler = hs.get_device_handler() + + async def on_POST(self, request: SynapseRequest): + submission = parse_json_object_from_request(request) + requester = await self.auth.get_user_by_req(request) + + device_id = await self.device_handler.store_dehydrated_device( + requester.user.to_string(), submission.get("device_data") + ) + return 200, {"device_id": device_id} + + class BaseSSORedirectServlet(RestServlet): """Common base class for /login/sso/redirect impls""" @@ -499,6 +552,8 @@ async def get_sso_url( def register_servlets(hs, http_server): LoginRestServlet(hs).register(http_server) + RestoreDeviceServlet(hs).register(http_server) + StoreDeviceServlet(hs).register(http_server) if hs.config.cas_enabled: CasRedirectServlet(hs).register(http_server) CasTicketServlet(hs).register(http_server) diff --git a/synapse/rest/client/v2_alpha/keys.py b/synapse/rest/client/v2_alpha/keys.py index 24bb090822a7..fa18db19463a 100644 --- a/synapse/rest/client/v2_alpha/keys.py +++ b/synapse/rest/client/v2_alpha/keys.py @@ -67,6 +67,7 @@ def __init__(self, hs): super(KeyUploadServlet, self).__init__() self.auth = hs.get_auth() self.e2e_keys_handler = hs.get_e2e_keys_handler() + self.device_handler = hs.get_device_handler() @trace(opname="upload_keys") async def on_POST(self, request, device_id): @@ -78,20 +79,22 @@ async def on_POST(self, request, device_id): # passing the device_id here is deprecated; however, we allow it # for now for compatibility with older clients. if requester.device_id is not None and device_id != requester.device_id: - set_tag("error", True) - log_kv( - { - "message": "Client uploading keys for a different device", - "logged_in_id": requester.device_id, - "key_being_uploaded": device_id, - } - ) - logger.warning( - "Client uploading keys for a different device " - "(logged in as %s, uploading for %s)", - requester.device_id, - device_id, - ) + dehydrated_device_id, _ = await self.device_handler.get_dehydrated_device(user_id) + if device_id != dehydrated_device_id: + set_tag("error", True) + log_kv( + { + "message": "Client uploading keys for a different device", + "logged_in_id": requester.device_id, + "key_being_uploaded": device_id, + } + ) + logger.warning( + "Client uploading keys for a different device " + "(logged in as %s, uploading for %s)", + requester.device_id, + device_id, + ) else: device_id = requester.device_id diff --git a/synapse/storage/data_stores/main/devices.py b/synapse/storage/data_stores/main/devices.py index 45581a65004e..c16ed922ae42 100644 --- a/synapse/storage/data_stores/main/devices.py +++ b/synapse/storage/data_stores/main/devices.py @@ -43,7 +43,7 @@ cachedList, ) from synapse.util.iterutils import batch_iter -from synapse.util.stringutils import shortstr +from synapse.util.stringutils import random_string, shortstr logger = logging.getLogger(__name__) @@ -728,6 +728,129 @@ def _mark_remote_user_device_list_as_unsubscribed_txn(txn): _mark_remote_user_device_list_as_unsubscribed_txn, ) + async def get_dehydrated_device(self, user_id: str) -> Tuple[str, str]: + row = await self.db.simple_select_one( + table="dehydrated_devices", + keyvalues={"user_id": user_id}, + retcols=["device_id", "device_data"], + allow_none=True, + ) + return (row["device_id"], row["device_data"]) if row else (None, None) + + def _store_dehydrated_device_txn( + self, txn, user_id: str, device_id: str, device_data: str + ) -> Optional[str]: + old_device_id = self.db.simple_select_one_onecol_txn( + txn, + table="dehydrated_devices", + keyvalues={"user_id": user_id}, + retcol="device_id", + allow_none=True, + ) + if old_device_id is None: + self.db.simple_insert_txn( + txn, + table="dehydrated_devices", + values={ + "user_id": user_id, + "device_id": device_id, + "device_data": device_data, + }, + ) + else: + self.db.simple_update_txn( + txn, + table="dehydrated_devices", + keyvalues={"user_id", user_id}, + updatevalues={ + "device_id": device_id, + "device_data": device_data, + }, + ) + return old_device_id + + async def store_dehydrated_device( + self, user_id: str, device_id: str, device_data: str + ) -> Optional[str]: + return await self.db.runInteraction( + "store_dehydrated_device_txn", + self._store_dehydrated_device_txn, + user_id, device_id, device_data, + ) + + async def create_dehydration_token( + self, user_id: str, device_id: str, login_submission: str + ) -> str: + # FIXME: expire any old tokens + + attempts = 0 + while attempts < 5: + token = random_string(24) + + try: + await self.db.simple_insert( + table="dehydration_token", + values={ + "token": token, + "user_id": user_id, + "device_id": device_id, + "login_submission": login_submission, + "creation_time": self.hs.get_clock().time_msec(), + }, + desc="create_dehydration_token", + ) + return token + except self.db.engine.module.IntegrityError: + attempts += 1 + raise StoreError(500, "Couldn't generate a token.") + + def _clear_dehydration_token_txn(self, txn, token: str, dehydrate: bool) -> dict: + token_info = self.db.simple_select_one_txn( + txn, + "dehydration_token", + { + "token": token, + }, + ["user_id", "device_id", "login_submission"], + ) + self.db.simple_delete_one_txn( + txn, + "dehydration_token", + { + "token": token, + }, + ) + + if dehydrate: + device = self.db.simple_select_one_txn( + txn, + "dehydrated_devices", + {"user_id": token_info["user_id"]}, + ["device_id", "device_data"], + allow_none=True, + ) + if device and device["device_id"] == token_info["device_id"]: + count = self.db.simple_delete_txn( + txn, + "dehydrated_devices", + { + "user_id": token_info["user_id"], + "device_id": token_info["device_id"], + }, + ) + if count != 0: + token_info["dehydrated"] = True + + return token_info + + async def clear_dehydration_token(self, token: str, dehydrate: bool) -> dict: + return await self.db.runInteraction( + "get_users_whose_devices_changed", + self._clear_dehydration_token_txn, + token, + dehydrate, + ) + class DeviceBackgroundUpdateStore(SQLBaseStore): def __init__(self, database: Database, db_conn, hs): @@ -865,8 +988,7 @@ def __init__(self, database: Database, db_conn, hs): self._clock.looping_call(self._prune_old_outbound_device_pokes, 60 * 60 * 1000) - @defer.inlineCallbacks - def store_device(self, user_id, device_id, initial_device_display_name): + async def store_device(self, user_id, device_id, initial_device_display_name): """Ensure the given device is known; add it to the store if not Args: @@ -885,7 +1007,7 @@ def store_device(self, user_id, device_id, initial_device_display_name): return False try: - inserted = yield self.db.simple_insert( + inserted = await self.db.simple_insert( "devices", values={ "user_id": user_id, @@ -899,7 +1021,7 @@ def store_device(self, user_id, device_id, initial_device_display_name): if not inserted: # if the device already exists, check if it's a real device, or # if the device ID is reserved by something else - hidden = yield self.db.simple_select_one_onecol( + hidden = await self.db.simple_select_one_onecol( "devices", keyvalues={"user_id": user_id, "device_id": device_id}, retcol="hidden", diff --git a/synapse/storage/data_stores/main/schema/delta/58/11dehydration.sql b/synapse/storage/data_stores/main/schema/delta/58/11dehydration.sql new file mode 100644 index 000000000000..be5e8a47129d --- /dev/null +++ b/synapse/storage/data_stores/main/schema/delta/58/11dehydration.sql @@ -0,0 +1,30 @@ +/* Copyright 2020 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +CREATE TABLE IF NOT EXISTS dehydrated_devices( + user_id TEXT NOT NULL PRIMARY KEY, + device_id TEXT NOT NULL, + device_data TEXT NOT NULL +); + +CREATE TABLE IF NOT EXISTS dehydration_token( + token TEXT NOT NULL PRIMARY KEY, + user_id TEXT NOT NULL, + device_id TEXT NOT NULL, + login_submission TEXT NOT NULL, + creation_time BIGINT NOT NULL +); + +-- FIXME: index on creation_time to expire old tokens diff --git a/tests/rest/client/v1/test_login.py b/tests/rest/client/v1/test_login.py index db52725cfe73..cafa581be7c8 100644 --- a/tests/rest/client/v1/test_login.py +++ b/tests/rest/client/v1/test_login.py @@ -754,3 +754,72 @@ def test_login_jwt_invalid_signature(self): channel.json_body["error"], "JWT validation failed: Signature verification failed", ) + + +class DehydrationTestCase(unittest.HomeserverTestCase): + + servlets = [ + synapse.rest.admin.register_servlets_for_client_rest_resource, + login.register_servlets, + logout.register_servlets, + devices.register_servlets, + ] + + def make_homeserver(self, reactor, clock): + self.hs = self.setup_test_homeserver() + self.hs.config.enable_registration = True + self.hs.config.registrations_require_3pid = [] + self.hs.config.auto_join_rooms = [] + self.hs.config.enable_registration_captcha = False + + return self.hs + + def test_dehydrate_and_rehydrate_device(self): + self.register_user("kermit", "monkey") + access_token = self.login("kermit", "monkey") + + # dehydrate a device + params = json.dumps({ + "device_data": "foobar" + }) + request, channel = self.make_request( + b"POST", b"/_matrix/client/unstable/org.matrix.msc2697/device/dehydrate", + params, + access_token=access_token + ) + self.render(request) + self.assertEquals(channel.code, 200, channel.result) + dehydrated_device_id = channel.json_body["device_id"] + + # Log out + request, channel = self.make_request( + b"POST", "/logout", access_token=access_token + ) + self.render(request) + + # log in, requesting a dehydrated device + params = json.dumps({ + "type": "m.login.password", + "user": "kermit", + "password": "monkey", + "org.matrix.msc2697.restore_device": True, + }) + request, channel = self.make_request( + "POST", "/_matrix/client/r0/login", params + ) + self.render(request) + self.assertEqual(channel.code, 200, channel.result) + self.assertEqual(channel.json_body["device_data"], "foobar") + self.assertEqual(channel.json_body["device_id"], dehydrated_device_id) + dehydration_token = channel.json_body["dehydration_token"] + + params = json.dumps({ + "rehydrate": True, + "dehydration_token": dehydration_token + }) + request, channel = self.make_request( + "POST", "/_matrix/client/unstable/org.matrix.msc2697/restore_device", params + ) + self.render(request) + self.assertEqual(channel.code, 200, channel.result) + self.assertEqual(channel.json_body["device_id"], dehydrated_device_id) From e60a99deb6ce7c83d693c0f17f620c2a072eee11 Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Mon, 27 Jul 2020 16:48:32 -0400 Subject: [PATCH 02/25] run black --- synapse/handlers/device.py | 27 +++++++++++++++------ synapse/rest/client/v1/login.py | 23 +++++++++++++++--- synapse/rest/client/v2_alpha/keys.py | 5 +++- synapse/storage/data_stores/main/devices.py | 25 +++++++------------ 4 files changed, 52 insertions(+), 28 deletions(-) diff --git a/synapse/handlers/device.py b/synapse/handlers/device.py index 614b10ca9ef3..0923466db48b 100644 --- a/synapse/handlers/device.py +++ b/synapse/handlers/device.py @@ -492,12 +492,17 @@ async def user_left_room(self, user, room_id): await self.store.mark_remote_user_device_list_as_unsubscribed(user_id) async def store_dehydrated_device( - self, user_id: str, device_data: str, - initial_device_display_name: Optional[str] = None) -> str: + self, + user_id: str, + device_data: str, + initial_device_display_name: Optional[str] = None, + ) -> str: device_id = await self.check_device_registered( user_id, None, initial_device_display_name, ) - old_device_id = await self.store.store_dehydrated_device(user_id, device_id, device_data) + old_device_id = await self.store.store_dehydrated_device( + user_id, device_id, device_data + ) if old_device_id is not None: await self.delete_device(user_id, old_device_id) return device_id @@ -505,8 +510,12 @@ async def store_dehydrated_device( async def get_dehydrated_device(self, user_id: str) -> Tuple[str, str]: return await self.store.get_dehydrated_device(user_id) - async def get_dehydration_token(self, user_id: str, device_id: str, login_submission: JsonDict) -> str: - return await self.store.create_dehydration_token(user_id, device_id, json.dumps(login_submission)) + async def get_dehydration_token( + self, user_id: str, device_id: str, login_submission: JsonDict + ) -> str: + return await self.store.create_dehydration_token( + user_id, device_id, json.dumps(login_submission) + ) async def rehydrate_device(self, token: str) -> dict: # FIXME: if can't find token, return 404 @@ -519,9 +528,13 @@ async def rehydrate_device(self, token: str) -> dict: if token_info["dehydrated"]: # create access token for dehydrated device - initial_display_name = None # FIXME: get display name from login submission? + initial_display_name = ( + None # FIXME: get display name from login submission? + ) device_id, access_token = await registration_handler.register_device( - token_info.get("user_id"), token_info.get("device_id"), initial_display_name + token_info.get("user_id"), + token_info.get("device_id"), + initial_display_name, ) return { diff --git a/synapse/rest/client/v1/login.py b/synapse/rest/client/v1/login.py index 3e6da34de95d..6a03ccf40e4c 100644 --- a/synapse/rest/client/v1/login.py +++ b/synapse/rest/client/v1/login.py @@ -341,9 +341,14 @@ async def _complete_login( user_id = canonical_uid if login_submission.get("org.matrix.msc2697.restore_device"): - device_id, dehydrated_device = await self.device_handler.get_dehydrated_device(user_id) + ( + device_id, + dehydrated_device, + ) = await self.device_handler.get_dehydrated_device(user_id) if dehydrated_device: - token = await self.device_handler.get_dehydration_token(user_id, device_id, login_submission) + token = await self.device_handler.get_dehydration_token( + user_id, device_id, login_submission + ) result = { "user_id": user_id, "home_server": self.hs.hostname, @@ -430,9 +435,19 @@ async def on_POST(self, request: SynapseRequest): submission = parse_json_object_from_request(request) if submission.get("rehydrate"): - return 200, await self.device_handler.rehydrate_device(submission.get("dehydration_token")) + return ( + 200, + await self.device_handler.rehydrate_device( + submission.get("dehydration_token") + ), + ) else: - return 200, await self.device_handler.cancel_rehydrate(submission.get("dehydration_token")) + return ( + 200, + await self.device_handler.cancel_rehydrate( + submission.get("dehydration_token") + ), + ) class StoreDeviceServlet(RestServlet): diff --git a/synapse/rest/client/v2_alpha/keys.py b/synapse/rest/client/v2_alpha/keys.py index fa18db19463a..b86c8f598bb8 100644 --- a/synapse/rest/client/v2_alpha/keys.py +++ b/synapse/rest/client/v2_alpha/keys.py @@ -79,7 +79,10 @@ async def on_POST(self, request, device_id): # passing the device_id here is deprecated; however, we allow it # for now for compatibility with older clients. if requester.device_id is not None and device_id != requester.device_id: - dehydrated_device_id, _ = await self.device_handler.get_dehydrated_device(user_id) + ( + dehydrated_device_id, + _, + ) = await self.device_handler.get_dehydrated_device(user_id) if device_id != dehydrated_device_id: set_tag("error", True) log_kv( diff --git a/synapse/storage/data_stores/main/devices.py b/synapse/storage/data_stores/main/devices.py index c16ed922ae42..8555d47ad4f6 100644 --- a/synapse/storage/data_stores/main/devices.py +++ b/synapse/storage/data_stores/main/devices.py @@ -738,7 +738,7 @@ async def get_dehydrated_device(self, user_id: str) -> Tuple[str, str]: return (row["device_id"], row["device_data"]) if row else (None, None) def _store_dehydrated_device_txn( - self, txn, user_id: str, device_id: str, device_data: str + self, txn, user_id: str, device_id: str, device_data: str ) -> Optional[str]: old_device_id = self.db.simple_select_one_onecol_txn( txn, @@ -762,24 +762,23 @@ def _store_dehydrated_device_txn( txn, table="dehydrated_devices", keyvalues={"user_id", user_id}, - updatevalues={ - "device_id": device_id, - "device_data": device_data, - }, + updatevalues={"device_id": device_id, "device_data": device_data,}, ) return old_device_id async def store_dehydrated_device( - self, user_id: str, device_id: str, device_data: str + self, user_id: str, device_id: str, device_data: str ) -> Optional[str]: return await self.db.runInteraction( "store_dehydrated_device_txn", self._store_dehydrated_device_txn, - user_id, device_id, device_data, + user_id, + device_id, + device_data, ) async def create_dehydration_token( - self, user_id: str, device_id: str, login_submission: str + self, user_id: str, device_id: str, login_submission: str ) -> str: # FIXME: expire any old tokens @@ -808,17 +807,11 @@ def _clear_dehydration_token_txn(self, txn, token: str, dehydrate: bool) -> dict token_info = self.db.simple_select_one_txn( txn, "dehydration_token", - { - "token": token, - }, + {"token": token,}, ["user_id", "device_id", "login_submission"], ) self.db.simple_delete_one_txn( - txn, - "dehydration_token", - { - "token": token, - }, + txn, "dehydration_token", {"token": token,}, ) if dehydrate: From 0f9e402e166326186323e1a9acd27377031227ca Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Mon, 27 Jul 2020 16:49:29 -0400 Subject: [PATCH 03/25] add changelog --- changelog.d/7955.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/7955.feature diff --git a/changelog.d/7955.feature b/changelog.d/7955.feature new file mode 100644 index 000000000000..d6d04619e5ef --- /dev/null +++ b/changelog.d/7955.feature @@ -0,0 +1 @@ +Add support for device dehydration. From b59bc664e6d200a17ce2d0bb03863b80b359768b Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Tue, 4 Aug 2020 16:10:49 -0400 Subject: [PATCH 04/25] minor fixes --- synapse/handlers/device.py | 2 +- synapse/rest/client/v1/login.py | 2 +- synapse/storage/data_stores/main/devices.py | 3 ++- tests/rest/client/v1/test_login.py | 30 +++++++++------------ 4 files changed, 17 insertions(+), 20 deletions(-) diff --git a/synapse/handlers/device.py b/synapse/handlers/device.py index 0923466db48b..8995be8446d1 100644 --- a/synapse/handlers/device.py +++ b/synapse/handlers/device.py @@ -562,7 +562,7 @@ async def rehydrate_device(self, token: str) -> dict: async def cancel_rehydrate(self, token: str) -> dict: # FIXME: if can't find token, return 404 - token_info = await self.store.clear_dehydration_token(token) + token_info = await self.store.clear_dehydration_token(token, False) # create device and access token from original login submission login_submission = token_info.get("login_submission") device_id = login_submission.get("device_id") diff --git a/synapse/rest/client/v1/login.py b/synapse/rest/client/v1/login.py index 6a03ccf40e4c..9e6748d4d198 100644 --- a/synapse/rest/client/v1/login.py +++ b/synapse/rest/client/v1/login.py @@ -424,7 +424,7 @@ async def _do_jwt_login(self, login_submission: JsonDict) -> Dict[str, str]: class RestoreDeviceServlet(RestServlet): - PATTERNS = client_patterns("/org.matrix.msc26997/restore_device") + PATTERNS = client_patterns("/org.matrix.msc2697/restore_device") def __init__(self, hs): super(RestoreDeviceServlet, self).__init__() diff --git a/synapse/storage/data_stores/main/devices.py b/synapse/storage/data_stores/main/devices.py index 8555d47ad4f6..0ad4846463bb 100644 --- a/synapse/storage/data_stores/main/devices.py +++ b/synapse/storage/data_stores/main/devices.py @@ -729,6 +729,7 @@ def _mark_remote_user_device_list_as_unsubscribed_txn(txn): ) async def get_dehydrated_device(self, user_id: str) -> Tuple[str, str]: + # FIXME: make sure device ID still exists in devices table row = await self.db.simple_select_one( table="dehydrated_devices", keyvalues={"user_id": user_id}, @@ -761,7 +762,7 @@ def _store_dehydrated_device_txn( self.db.simple_update_txn( txn, table="dehydrated_devices", - keyvalues={"user_id", user_id}, + keyvalues={"user_id": user_id}, updatevalues={"device_id": device_id, "device_data": device_data,}, ) return old_device_id diff --git a/tests/rest/client/v1/test_login.py b/tests/rest/client/v1/test_login.py index cafa581be7c8..d0c3f40e78be 100644 --- a/tests/rest/client/v1/test_login.py +++ b/tests/rest/client/v1/test_login.py @@ -779,13 +779,12 @@ def test_dehydrate_and_rehydrate_device(self): access_token = self.login("kermit", "monkey") # dehydrate a device - params = json.dumps({ - "device_data": "foobar" - }) + params = json.dumps({"device_data": "foobar"}) request, channel = self.make_request( - b"POST", b"/_matrix/client/unstable/org.matrix.msc2697/device/dehydrate", + b"POST", + b"/_matrix/client/unstable/org.matrix.msc2697/device/dehydrate", params, - access_token=access_token + access_token=access_token, ) self.render(request) self.assertEquals(channel.code, 200, channel.result) @@ -798,25 +797,22 @@ def test_dehydrate_and_rehydrate_device(self): self.render(request) # log in, requesting a dehydrated device - params = json.dumps({ - "type": "m.login.password", - "user": "kermit", - "password": "monkey", - "org.matrix.msc2697.restore_device": True, - }) - request, channel = self.make_request( - "POST", "/_matrix/client/r0/login", params + params = json.dumps( + { + "type": "m.login.password", + "user": "kermit", + "password": "monkey", + "org.matrix.msc2697.restore_device": True, + } ) + request, channel = self.make_request("POST", "/_matrix/client/r0/login", params) self.render(request) self.assertEqual(channel.code, 200, channel.result) self.assertEqual(channel.json_body["device_data"], "foobar") self.assertEqual(channel.json_body["device_id"], dehydrated_device_id) dehydration_token = channel.json_body["dehydration_token"] - params = json.dumps({ - "rehydrate": True, - "dehydration_token": dehydration_token - }) + params = json.dumps({"rehydrate": True, "dehydration_token": dehydration_token}) request, channel = self.make_request( "POST", "/_matrix/client/unstable/org.matrix.msc2697/restore_device", params ) From 96d9fc3410036e1ca23b9a1d7788c6f972f5325f Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Tue, 4 Aug 2020 16:18:44 -0400 Subject: [PATCH 05/25] maybe this will make lint happy? --- synapse/storage/data_stores/main/devices.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/synapse/storage/data_stores/main/devices.py b/synapse/storage/data_stores/main/devices.py index 0ad4846463bb..d8bfd9e113dc 100644 --- a/synapse/storage/data_stores/main/devices.py +++ b/synapse/storage/data_stores/main/devices.py @@ -763,7 +763,7 @@ def _store_dehydrated_device_txn( txn, table="dehydrated_devices", keyvalues={"user_id": user_id}, - updatevalues={"device_id": device_id, "device_data": device_data,}, + updatevalues={"device_id": device_id, "device_data": device_data}, ) return old_device_id @@ -808,11 +808,11 @@ def _clear_dehydration_token_txn(self, txn, token: str, dehydrate: bool) -> dict token_info = self.db.simple_select_one_txn( txn, "dehydration_token", - {"token": token,}, + {"token": token}, ["user_id", "device_id", "login_submission"], ) self.db.simple_delete_one_txn( - txn, "dehydration_token", {"token": token,}, + txn, "dehydration_token", {"token": token}, ) if dehydrate: From c7c8f2822d252019376f73e48107f35a23baebb3 Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Mon, 17 Aug 2020 18:13:11 -0400 Subject: [PATCH 06/25] newer version of dehydration proposal, add doc improvements and other minor fixes --- changelog.d/7955.feature | 2 +- synapse/handlers/device.py | 59 +++++++++++++--- synapse/rest/client/v1/login.py | 70 +++++++++++++++---- synapse/storage/databases/main/devices.py | 67 +++++++++++++++--- .../storage/databases/main/end_to_end_keys.py | 5 ++ 5 files changed, 171 insertions(+), 32 deletions(-) diff --git a/changelog.d/7955.feature b/changelog.d/7955.feature index d6d04619e5ef..7d726046fe91 100644 --- a/changelog.d/7955.feature +++ b/changelog.d/7955.feature @@ -1 +1 @@ -Add support for device dehydration. +Add support for device dehydration. (MSC2697) diff --git a/synapse/handlers/device.py b/synapse/handlers/device.py index 8995be8446d1..e1fd39356c4c 100644 --- a/synapse/handlers/device.py +++ b/synapse/handlers/device.py @@ -494,9 +494,19 @@ async def user_left_room(self, user, room_id): async def store_dehydrated_device( self, user_id: str, - device_data: str, + device_data: JsonDict, initial_device_display_name: Optional[str] = None, ) -> str: + """Store a dehydrated device for a user. If the user had a previous + dehydrated device, it is removed. + + Args: + user_id: the user that we are storing the device for + device_data: the dehydrated device information + initial_device_display_name: The display name to use for the device + Returns: + device id of the dehydrated device + """ device_id = await self.check_device_registered( user_id, None, initial_device_display_name, ) @@ -507,17 +517,43 @@ async def store_dehydrated_device( await self.delete_device(user_id, old_device_id) return device_id - async def get_dehydrated_device(self, user_id: str) -> Tuple[str, str]: + async def get_dehydrated_device(self, user_id: str) -> Tuple[str, JsonDict]: + """Retrieve the information for a dehydrated device. + + Args: + user_id: the user whose dehydrated device we are looking for + Returns: + a tuple whose first item is the device ID, and the second item is + the dehydrated device information + """ return await self.store.get_dehydrated_device(user_id) - async def get_dehydration_token( + async def create_dehydration_token( self, user_id: str, device_id: str, login_submission: JsonDict ) -> str: + """Create a token for a client to fulfill a dehydration request. + + Args: + user_id: the user that we are creating the token for + device_id: the device ID for the dehydrated device. This is to + ensure that the device still exists when the user tells us + they want to use the dehydrated device. + login_submission: the contents of the login request. + Returns: + the dehydration token + """ return await self.store.create_dehydration_token( - user_id, device_id, json.dumps(login_submission) + user_id, device_id, login_submission ) async def rehydrate_device(self, token: str) -> dict: + """Process a rehydration request from the user. + + Args: + token: the dehydration token + Returns: + the login result, including the user's access token and device ID + """ # FIXME: if can't find token, return 404 token_info = await self.store.clear_dehydration_token(token, True) @@ -538,7 +574,7 @@ async def rehydrate_device(self, token: str) -> dict: ) return { - "user_id": token_info.get("user_id"), + "user_id": token_info["user_id"], "access_token": access_token, "home_server": self.hs.hostname, "device_id": device_id, @@ -546,7 +582,7 @@ async def rehydrate_device(self, token: str) -> dict: else: # create device and access token from original login submission - login_submission = token_info.get("login_submission") + login_submission = token_info["login_submission"] device_id = login_submission.get("device_id") initial_display_name = login_submission.get("initial_device_display_name") device_id, access_token = await registration_handler.register_device( @@ -554,17 +590,24 @@ async def rehydrate_device(self, token: str) -> dict: ) return { - "user_id": token.info.get("user_id"), + "user_id": token.info["user_id"], "access_token": access_token, "home_server": self.hs.hostname, "device_id": device_id, } async def cancel_rehydrate(self, token: str) -> dict: + """Cancel a rehydration request from the user and complete the user's login. + + Args: + token: the dehydration token + Returns: + the login result, including the user's access token and device ID + """ # FIXME: if can't find token, return 404 token_info = await self.store.clear_dehydration_token(token, False) # create device and access token from original login submission - login_submission = token_info.get("login_submission") + login_submission = token_info["login_submission"] device_id = login_submission.get("device_id") initial_display_name = login_submission.get("initial_device_display_name") device_id, access_token = await self.registration_handler.register_device( diff --git a/synapse/rest/client/v1/login.py b/synapse/rest/client/v1/login.py index 9e6748d4d198..68fece986bcd 100644 --- a/synapse/rest/client/v1/login.py +++ b/synapse/rest/client/v1/login.py @@ -341,12 +341,14 @@ async def _complete_login( user_id = canonical_uid if login_submission.get("org.matrix.msc2697.restore_device"): + # user requested to rehydrate a device, so check if there they have + # a dehydrated device, and if so, allow them to try to rehydrate it ( device_id, dehydrated_device, ) = await self.device_handler.get_dehydrated_device(user_id) if dehydrated_device: - token = await self.device_handler.get_dehydration_token( + token = await self.device_handler.create_dehydration_token( user_id, device_id, login_submission ) result = { @@ -424,33 +426,75 @@ async def _do_jwt_login(self, login_submission: JsonDict) -> Dict[str, str]: class RestoreDeviceServlet(RestServlet): + """Complete a rehydration request, either by letting the client use the + dehydrated device, or by creating a new device for the user. + + POST /org.matrix.msc2697/restore_device + Content-Type: application/json + + { + "rehydrate": true, + "dehydration_token": "an_opaque_token" + } + + HTTP/1.1 200 OK + Content-Type: application/json + + { // same format as the result from a /login request + "user_id": "@alice:example.org", + "device_id": "dehydrated_device", + "access_token": "another_opaque_token" + } + + """ + PATTERNS = client_patterns("/org.matrix.msc2697/restore_device") def __init__(self, hs): super(RestoreDeviceServlet, self).__init__() self.hs = hs self.device_handler = hs.get_device_handler() + self._well_known_builder = WellKnownBuilder(hs) async def on_POST(self, request: SynapseRequest): submission = parse_json_object_from_request(request) if submission.get("rehydrate"): - return ( - 200, - await self.device_handler.rehydrate_device( - submission.get("dehydration_token") - ), + result = await self.device_handler.rehydrate_device( + submission["dehydration_token"] ) else: - return ( - 200, - await self.device_handler.cancel_rehydrate( - submission.get("dehydration_token") - ), + result = await self.device_handler.cancel_rehydrate( + submission["dehydration_token"] ) + well_known_data = self._well_known_builder.get_well_known() + if well_known_data: + result["well_known"] = well_known_data + return (200, result) class StoreDeviceServlet(RestServlet): + """Store a dehydrated device. + + POST /org.matrix.msc2697/device/dehydrate + Content-Type: application/json + + { + "device_data": { + "algorithm": "m.dehydration.v1.olm", + "account": "dehydrated_device" + } + } + + HTTP/1.1 200 OK + Content-Type: application/json + + { + "device_id": "dehydrated_device_id" + } + + """ + PATTERNS = client_patterns("/org.matrix.msc2697/device/dehydrate") def __init__(self, hs): @@ -464,7 +508,9 @@ async def on_POST(self, request: SynapseRequest): requester = await self.auth.get_user_by_req(request) device_id = await self.device_handler.store_dehydrated_device( - requester.user.to_string(), submission.get("device_data") + requester.user.to_string(), + submission["device_data"], + submission.get("initial_device_display_name", None) ) return 200, {"device_id": device_id} diff --git a/synapse/storage/databases/main/devices.py b/synapse/storage/databases/main/devices.py index eb9d772d6ebf..d6c6f0ac3497 100644 --- a/synapse/storage/databases/main/devices.py +++ b/synapse/storage/databases/main/devices.py @@ -35,7 +35,7 @@ LoggingTransaction, make_tuple_comparison_clause, ) -from synapse.types import Collection, get_verify_key_from_cross_signing_key +from synapse.types import Collection, JsonDict, get_verify_key_from_cross_signing_key from synapse.util.caches.descriptors import ( Cache, cached, @@ -728,7 +728,15 @@ def _mark_remote_user_device_list_as_unsubscribed_txn(txn): _mark_remote_user_device_list_as_unsubscribed_txn, ) - async def get_dehydrated_device(self, user_id: str) -> Tuple[str, str]: + async def get_dehydrated_device(self, user_id: str) -> Tuple[str, JsonDict]: + """Retrieve the information for a dehydrated device. + + Args: + user_id: the user whose dehydrated device we are looking for + Returns: + a tuple whose first item is the device ID, and the second item is + the dehydrated device information + """ # FIXME: make sure device ID still exists in devices table row = await self.db_pool.simple_select_one( table="dehydrated_devices", @@ -736,7 +744,7 @@ async def get_dehydrated_device(self, user_id: str) -> Tuple[str, str]: retcols=["device_id", "device_data"], allow_none=True, ) - return (row["device_id"], row["device_data"]) if row else (None, None) + return (row["device_id"], json.loads(row["device_data"])) if row else (None, None) def _store_dehydrated_device_txn( self, txn, user_id: str, device_id: str, device_data: str @@ -768,19 +776,39 @@ def _store_dehydrated_device_txn( return old_device_id async def store_dehydrated_device( - self, user_id: str, device_id: str, device_data: str + self, user_id: str, device_id: str, device_data: JsonDict ) -> Optional[str]: + """Store a dehydrated device for a user. + + Args: + user_id: the user that we are storing the device for + device_data: the dehydrated device information + initial_device_display_name: The display name to use for the device + Returns: + device id of the user's previous dehydrated device, if any + """ return await self.db_pool.runInteraction( "store_dehydrated_device_txn", self._store_dehydrated_device_txn, user_id, device_id, - device_data, + json.dumps(device_data), ) async def create_dehydration_token( - self, user_id: str, device_id: str, login_submission: str + self, user_id: str, device_id: str, login_submission: JsonDict ) -> str: + """Create a token for a client to fulfill a dehydration request. + + Args: + user_id: the user that we are creating the token for + device_id: the device ID for the dehydrated device. This is to + ensure that the device still exists when the user tells us + they want to use the dehydrated device. + login_submission: the contents of the login request. + Returns: + the dehydration token + """ # FIXME: expire any old tokens attempts = 0 @@ -794,7 +822,7 @@ async def create_dehydration_token( "token": token, "user_id": user_id, "device_id": device_id, - "login_submission": login_submission, + "login_submission": json.dumps(login_submission), "creation_time": self.hs.get_clock().time_msec(), }, desc="create_dehydration_token", @@ -814,16 +842,18 @@ def _clear_dehydration_token_txn(self, txn, token: str, dehydrate: bool) -> dict self.db_pool.simple_delete_one_txn( txn, "dehydration_token", {"token": token}, ) + token_info["login_submission"] = json.loads(token_info["login_submission"]) if dehydrate: - device = self.db_pool.simple_select_one_txn( + device_id = self.db_pool.simple_select_one_onecol_txn( txn, "dehydrated_devices", - {"user_id": token_info["user_id"]}, - ["device_id", "device_data"], + keyvalues={"user_id": token_info["user_id"]}, + retcol="device_id", allow_none=True, ) - if device and device["device_id"] == token_info["device_id"]: + token_info["dehydrated"] = False + if device_id == token_info["device_id"]: count = self.db_pool.simple_delete_txn( txn, "dehydrated_devices", @@ -838,6 +868,21 @@ def _clear_dehydration_token_txn(self, txn, token: str, dehydrate: bool) -> dict return token_info async def clear_dehydration_token(self, token: str, dehydrate: bool) -> dict: + """Use a dehydration token. If the client wishes to use the dehydrated + device, it will also remove the dehydrated device. + + Args: + token: the dehydration token + dehydrate: whether the client wishes to use the dehydrated device + Returns: + A dict giving the information related to the token. It will have + the following properties: + - user_id: the user associated from the token + - device_id: the ID of the dehydrated device + - login_submission: the original submission to /login + - dehydrated: (only present if the "dehydrate" parameter is True). + Whether the dehydrated device can be used by the client. + """ return await self.db_pool.runInteraction( "get_users_whose_devices_changed", self._clear_dehydration_token_txn, diff --git a/synapse/storage/databases/main/end_to_end_keys.py b/synapse/storage/databases/main/end_to_end_keys.py index 40354b8304dd..23f04a4887e3 100644 --- a/synapse/storage/databases/main/end_to_end_keys.py +++ b/synapse/storage/databases/main/end_to_end_keys.py @@ -641,6 +641,11 @@ def delete_e2e_keys_by_device_txn(txn): self._invalidate_cache_and_stream( txn, self.count_e2e_one_time_keys, (user_id, device_id) ) + self.db_pool.simple_delete_txn( + txn, + table="dehydrated_devices", + keyvalues={"user_id": user_id, "device_id": device_id}, + ) return self.db_pool.runInteraction( "delete_e2e_keys_by_device", delete_e2e_keys_by_device_txn From b7398f74891c3b302360258a1954c731298ea52d Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Thu, 3 Sep 2020 16:34:06 -0400 Subject: [PATCH 07/25] fix nonexistent reference --- synapse/handlers/device.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/synapse/handlers/device.py b/synapse/handlers/device.py index e1fd39356c4c..7c809b27f0d8 100644 --- a/synapse/handlers/device.py +++ b/synapse/handlers/device.py @@ -610,7 +610,8 @@ async def cancel_rehydrate(self, token: str) -> dict: login_submission = token_info["login_submission"] device_id = login_submission.get("device_id") initial_display_name = login_submission.get("initial_device_display_name") - device_id, access_token = await self.registration_handler.register_device( + registration_handler = self.hs.get_registration_handler() + device_id, access_token = await registration_handler.register_device( token_info.get("user_id"), device_id, initial_display_name ) From 355f1234b502a558d92fa8f37485403d84a9c6cf Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Tue, 22 Sep 2020 18:10:24 -0400 Subject: [PATCH 08/25] implement alternative dehydration API --- synapse/handlers/device.py | 103 +++--------------- synapse/rest/client/v1/login.py | 87 +++++++-------- synapse/storage/databases/main/devices.py | 101 ++--------------- .../storage/databases/main/registration.py | 14 +++ tests/rest/client/v1/test_login.py | 65 ----------- 5 files changed, 81 insertions(+), 289 deletions(-) diff --git a/synapse/handlers/device.py b/synapse/handlers/device.py index 7c809b27f0d8..dd5a5d826e40 100644 --- a/synapse/handlers/device.py +++ b/synapse/handlers/device.py @@ -528,99 +528,28 @@ async def get_dehydrated_device(self, user_id: str) -> Tuple[str, JsonDict]: """ return await self.store.get_dehydrated_device(user_id) - async def create_dehydration_token( - self, user_id: str, device_id: str, login_submission: JsonDict - ) -> str: - """Create a token for a client to fulfill a dehydration request. - - Args: - user_id: the user that we are creating the token for - device_id: the device ID for the dehydrated device. This is to - ensure that the device still exists when the user tells us - they want to use the dehydrated device. - login_submission: the contents of the login request. - Returns: - the dehydration token - """ - return await self.store.create_dehydration_token( - user_id, device_id, login_submission - ) - - async def rehydrate_device(self, token: str) -> dict: + async def rehydrate_device( + self, user_id: str, access_token: str, device_id: str + ) -> dict: """Process a rehydration request from the user. Args: - token: the dehydration token + user_id: the user who is rehydrating the device + access_token: the access token used for the request + device_id: the ID of the device that will be rehydrated Returns: - the login result, including the user's access token and device ID + a dict containing {"success": True} """ - # FIXME: if can't find token, return 404 - token_info = await self.store.clear_dehydration_token(token, True) - - # normally, the constructor would do self.registration_handler = - # self.hs.get_registration_handler(), but doing that results in a - # circular dependency in the handlers. So do this for now - registration_handler = self.hs.get_registration_handler() - - if token_info["dehydrated"]: - # create access token for dehydrated device - initial_display_name = ( - None # FIXME: get display name from login submission? - ) - device_id, access_token = await registration_handler.register_device( - token_info.get("user_id"), - token_info.get("device_id"), - initial_display_name, - ) - - return { - "user_id": token_info["user_id"], - "access_token": access_token, - "home_server": self.hs.hostname, - "device_id": device_id, - } - + success = await self.store.remove_dehydrated_device(user_id, device_id) + + if success: + # If the dehydrated device was successfully deleted (the device ID + # matched the stored dehydrated device), then modify the access + # token to use the dehydrated device's ID + await self.store.set_device_for_access_token(access_token, device_id) + return {"success": True} else: - # create device and access token from original login submission - login_submission = token_info["login_submission"] - device_id = login_submission.get("device_id") - initial_display_name = login_submission.get("initial_device_display_name") - device_id, access_token = await registration_handler.register_device( - token_info.get("user_id"), device_id, initial_display_name - ) - - return { - "user_id": token.info["user_id"], - "access_token": access_token, - "home_server": self.hs.hostname, - "device_id": device_id, - } - - async def cancel_rehydrate(self, token: str) -> dict: - """Cancel a rehydration request from the user and complete the user's login. - - Args: - token: the dehydration token - Returns: - the login result, including the user's access token and device ID - """ - # FIXME: if can't find token, return 404 - token_info = await self.store.clear_dehydration_token(token, False) - # create device and access token from original login submission - login_submission = token_info["login_submission"] - device_id = login_submission.get("device_id") - initial_display_name = login_submission.get("initial_device_display_name") - registration_handler = self.hs.get_registration_handler() - device_id, access_token = await registration_handler.register_device( - token_info.get("user_id"), device_id, initial_display_name - ) - - return { - "user_id": token_info.get("user_id"), - "access_token": access_token, - "home_server": self.hs.hostname, - "device_id": device_id, - } + raise errors.NotFoundError() def _update_device_from_client_ips(device, client_ips): diff --git a/synapse/rest/client/v1/login.py b/synapse/rest/client/v1/login.py index 68fece986bcd..3b1affcaa02d 100644 --- a/synapse/rest/client/v1/login.py +++ b/synapse/rest/client/v1/login.py @@ -16,7 +16,7 @@ import logging from typing import Awaitable, Callable, Dict, Optional -from synapse.api.errors import Codes, LoginError, SynapseError +from synapse.api.errors import Codes, LoginError, NotFoundError, SynapseError from synapse.api.ratelimiting import Ratelimiter from synapse.http.server import finish_request from synapse.http.servlet import ( @@ -340,29 +340,6 @@ async def _complete_login( ) user_id = canonical_uid - if login_submission.get("org.matrix.msc2697.restore_device"): - # user requested to rehydrate a device, so check if there they have - # a dehydrated device, and if so, allow them to try to rehydrate it - ( - device_id, - dehydrated_device, - ) = await self.device_handler.get_dehydrated_device(user_id) - if dehydrated_device: - token = await self.device_handler.create_dehydration_token( - user_id, device_id, login_submission - ) - result = { - "user_id": user_id, - "home_server": self.hs.hostname, - "device_data": dehydrated_device, - "device_id": device_id, - "dehydration_token": token, - } - - # FIXME: call callback? - - return result - device_id = login_submission.get("device_id") initial_display_name = login_submission.get("initial_device_display_name") device_id, access_token = await self.registration_handler.register_device( @@ -426,50 +403,68 @@ async def _do_jwt_login(self, login_submission: JsonDict) -> Dict[str, str]: class RestoreDeviceServlet(RestServlet): - """Complete a rehydration request, either by letting the client use the - dehydrated device, or by creating a new device for the user. + # FIXME: move this to v2_alpha/devices.py. This is kept here for now to + # make it easier to compare with the other PR. + """Request or complete a rehydration request. - POST /org.matrix.msc2697/restore_device + GET /org.matrix.msc2697.v2/restore_device + + HTTP/1.1 200 OK Content-Type: application/json { - "rehydrate": true, - "dehydration_token": "an_opaque_token" + "device_id": "dehydrated_device_id", + "device_data": {"device": "data"} + } + + POST /org.matrix.msc2697.v2/restore_device + Content-Type: application/json + + { + "device_id": "dehydrated_device_id" } HTTP/1.1 200 OK Content-Type: application/json - { // same format as the result from a /login request - "user_id": "@alice:example.org", - "device_id": "dehydrated_device", - "access_token": "another_opaque_token" + { + "success": true, } """ - PATTERNS = client_patterns("/org.matrix.msc2697/restore_device") + PATTERNS = client_patterns("/org.matrix.msc2697.v2/restore_device") def __init__(self, hs): super(RestoreDeviceServlet, self).__init__() self.hs = hs + self.auth = hs.get_auth() self.device_handler = hs.get_device_handler() self._well_known_builder = WellKnownBuilder(hs) + async def on_GET(self, request: SynapseRequest): + requester = await self.auth.get_user_by_req(request, allow_guest=True) + ( + device_id, + dehydrated_device, + ) = await self.device_handler.get_dehydrated_device(requester.user.to_string()) + if dehydrated_device: + result = {"device_id": device_id, "device_data": dehydrated_device} + return (200, result) + else: + raise NotFoundError() + async def on_POST(self, request: SynapseRequest): + requester = await self.auth.get_user_by_req(request, allow_guest=True) + submission = parse_json_object_from_request(request) - if submission.get("rehydrate"): - result = await self.device_handler.rehydrate_device( - submission["dehydration_token"] - ) - else: - result = await self.device_handler.cancel_rehydrate( - submission["dehydration_token"] - ) - well_known_data = self._well_known_builder.get_well_known() - if well_known_data: - result["well_known"] = well_known_data + result = await self.device_handler.rehydrate_device( + requester.user.to_string(), + self.auth.get_access_token_from_request(request), + submission["device_id"], + ) + return (200, result) @@ -510,7 +505,7 @@ async def on_POST(self, request: SynapseRequest): device_id = await self.device_handler.store_dehydrated_device( requester.user.to_string(), submission["device_data"], - submission.get("initial_device_display_name", None) + submission.get("initial_device_display_name", None), ) return 200, {"device_id": device_id} diff --git a/synapse/storage/databases/main/devices.py b/synapse/storage/databases/main/devices.py index d6c6f0ac3497..94603dd72515 100644 --- a/synapse/storage/databases/main/devices.py +++ b/synapse/storage/databases/main/devices.py @@ -744,7 +744,9 @@ async def get_dehydrated_device(self, user_id: str) -> Tuple[str, JsonDict]: retcols=["device_id", "device_data"], allow_none=True, ) - return (row["device_id"], json.loads(row["device_data"])) if row else (None, None) + return ( + (row["device_id"], json.loads(row["device_data"])) if row else (None, None) + ) def _store_dehydrated_device_txn( self, txn, user_id: str, device_id: str, device_data: str @@ -795,100 +797,17 @@ async def store_dehydrated_device( json.dumps(device_data), ) - async def create_dehydration_token( - self, user_id: str, device_id: str, login_submission: JsonDict - ) -> str: - """Create a token for a client to fulfill a dehydration request. - - Args: - user_id: the user that we are creating the token for - device_id: the device ID for the dehydrated device. This is to - ensure that the device still exists when the user tells us - they want to use the dehydrated device. - login_submission: the contents of the login request. - Returns: - the dehydration token - """ - # FIXME: expire any old tokens - - attempts = 0 - while attempts < 5: - token = random_string(24) - - try: - await self.db_pool.simple_insert( - table="dehydration_token", - values={ - "token": token, - "user_id": user_id, - "device_id": device_id, - "login_submission": json.dumps(login_submission), - "creation_time": self.hs.get_clock().time_msec(), - }, - desc="create_dehydration_token", - ) - return token - except self.db_pool.engine.module.IntegrityError: - attempts += 1 - raise StoreError(500, "Couldn't generate a token.") - - def _clear_dehydration_token_txn(self, txn, token: str, dehydrate: bool) -> dict: - token_info = self.db_pool.simple_select_one_txn( - txn, - "dehydration_token", - {"token": token}, - ["user_id", "device_id", "login_submission"], - ) - self.db_pool.simple_delete_one_txn( - txn, "dehydration_token", {"token": token}, - ) - token_info["login_submission"] = json.loads(token_info["login_submission"]) - - if dehydrate: - device_id = self.db_pool.simple_select_one_onecol_txn( - txn, - "dehydrated_devices", - keyvalues={"user_id": token_info["user_id"]}, - retcol="device_id", - allow_none=True, - ) - token_info["dehydrated"] = False - if device_id == token_info["device_id"]: - count = self.db_pool.simple_delete_txn( - txn, - "dehydrated_devices", - { - "user_id": token_info["user_id"], - "device_id": token_info["device_id"], - }, - ) - if count != 0: - token_info["dehydrated"] = True - - return token_info - - async def clear_dehydration_token(self, token: str, dehydrate: bool) -> dict: - """Use a dehydration token. If the client wishes to use the dehydrated - device, it will also remove the dehydrated device. + async def remove_dehydrated_device(self, user_id: str, device_id: str) -> bool: + """Remove a dehydrated device. Args: - token: the dehydration token - dehydrate: whether the client wishes to use the dehydrated device - Returns: - A dict giving the information related to the token. It will have - the following properties: - - user_id: the user associated from the token - - device_id: the ID of the dehydrated device - - login_submission: the original submission to /login - - dehydrated: (only present if the "dehydrate" parameter is True). - Whether the dehydrated device can be used by the client. + user_id: the user that the dehydrated device belongs to + device_id: the ID of the dehydrated device """ - return await self.db_pool.runInteraction( - "get_users_whose_devices_changed", - self._clear_dehydration_token_txn, - token, - dehydrate, + count = self.db_pool.simple_delete( + "dehydrated_devices", {"user_id": user_id, "device_id": device_id,}, ) + return count >= 1 class DeviceBackgroundUpdateStore(SQLBaseStore): diff --git a/synapse/storage/databases/main/registration.py b/synapse/storage/databases/main/registration.py index f618629e0901..a60d14e64b9a 100644 --- a/synapse/storage/databases/main/registration.py +++ b/synapse/storage/databases/main/registration.py @@ -965,6 +965,20 @@ def add_access_token_to_user(self, user_id, token, device_id, valid_until_ms): desc="add_access_token_to_user", ) + async def set_device_for_access_token(self, token, device_id): + """Sets the device ID associated with an access token. + + Args: + token (str): The access token to modify. + device_id (str): The new device ID. + """ + + # FIXME: invalidate caches + + return await self.db_pool.simple_update( + "access_tokens", {"token": token}, {"device_id": device_id} + ) + def register_user( self, user_id, diff --git a/tests/rest/client/v1/test_login.py b/tests/rest/client/v1/test_login.py index d0c3f40e78be..db52725cfe73 100644 --- a/tests/rest/client/v1/test_login.py +++ b/tests/rest/client/v1/test_login.py @@ -754,68 +754,3 @@ def test_login_jwt_invalid_signature(self): channel.json_body["error"], "JWT validation failed: Signature verification failed", ) - - -class DehydrationTestCase(unittest.HomeserverTestCase): - - servlets = [ - synapse.rest.admin.register_servlets_for_client_rest_resource, - login.register_servlets, - logout.register_servlets, - devices.register_servlets, - ] - - def make_homeserver(self, reactor, clock): - self.hs = self.setup_test_homeserver() - self.hs.config.enable_registration = True - self.hs.config.registrations_require_3pid = [] - self.hs.config.auto_join_rooms = [] - self.hs.config.enable_registration_captcha = False - - return self.hs - - def test_dehydrate_and_rehydrate_device(self): - self.register_user("kermit", "monkey") - access_token = self.login("kermit", "monkey") - - # dehydrate a device - params = json.dumps({"device_data": "foobar"}) - request, channel = self.make_request( - b"POST", - b"/_matrix/client/unstable/org.matrix.msc2697/device/dehydrate", - params, - access_token=access_token, - ) - self.render(request) - self.assertEquals(channel.code, 200, channel.result) - dehydrated_device_id = channel.json_body["device_id"] - - # Log out - request, channel = self.make_request( - b"POST", "/logout", access_token=access_token - ) - self.render(request) - - # log in, requesting a dehydrated device - params = json.dumps( - { - "type": "m.login.password", - "user": "kermit", - "password": "monkey", - "org.matrix.msc2697.restore_device": True, - } - ) - request, channel = self.make_request("POST", "/_matrix/client/r0/login", params) - self.render(request) - self.assertEqual(channel.code, 200, channel.result) - self.assertEqual(channel.json_body["device_data"], "foobar") - self.assertEqual(channel.json_body["device_id"], dehydrated_device_id) - dehydration_token = channel.json_body["dehydration_token"] - - params = json.dumps({"rehydrate": True, "dehydration_token": dehydration_token}) - request, channel = self.make_request( - "POST", "/_matrix/client/unstable/org.matrix.msc2697/restore_device", params - ) - self.render(request) - self.assertEqual(channel.code, 200, channel.result) - self.assertEqual(channel.json_body["device_id"], dehydrated_device_id) From 037c20101770b649a468185b0159fb60ac7369f1 Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Tue, 22 Sep 2020 18:25:01 -0400 Subject: [PATCH 09/25] remove unneeded table --- .../databases/main/schema/delta/58/11dehydration.sql | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/synapse/storage/databases/main/schema/delta/58/11dehydration.sql b/synapse/storage/databases/main/schema/delta/58/11dehydration.sql index be5e8a47129d..a1e256aa8103 100644 --- a/synapse/storage/databases/main/schema/delta/58/11dehydration.sql +++ b/synapse/storage/databases/main/schema/delta/58/11dehydration.sql @@ -18,13 +18,3 @@ CREATE TABLE IF NOT EXISTS dehydrated_devices( device_id TEXT NOT NULL, device_data TEXT NOT NULL ); - -CREATE TABLE IF NOT EXISTS dehydration_token( - token TEXT NOT NULL PRIMARY KEY, - user_id TEXT NOT NULL, - device_id TEXT NOT NULL, - login_submission TEXT NOT NULL, - creation_time BIGINT NOT NULL -); - --- FIXME: index on creation_time to expire old tokens From a84e491213aeda4990817c9ce72a0cd9d86255c1 Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Mon, 28 Sep 2020 23:26:47 -0400 Subject: [PATCH 10/25] move rest endpoint to devices and tweak endpoint names --- synapse/rest/client/v1/login.py | 113 +----------------------- synapse/rest/client/v2_alpha/devices.py | 112 +++++++++++++++++++++++ 2 files changed, 113 insertions(+), 112 deletions(-) diff --git a/synapse/rest/client/v1/login.py b/synapse/rest/client/v1/login.py index 3b1affcaa02d..379f668d6f8a 100644 --- a/synapse/rest/client/v1/login.py +++ b/synapse/rest/client/v1/login.py @@ -16,7 +16,7 @@ import logging from typing import Awaitable, Callable, Dict, Optional -from synapse.api.errors import Codes, LoginError, NotFoundError, SynapseError +from synapse.api.errors import Codes, LoginError, SynapseError from synapse.api.ratelimiting import Ratelimiter from synapse.http.server import finish_request from synapse.http.servlet import ( @@ -103,7 +103,6 @@ def __init__(self, hs): self.oidc_enabled = hs.config.oidc_enabled self.auth_handler = self.hs.get_auth_handler() - self.device_handler = hs.get_device_handler() self.registration_handler = hs.get_registration_handler() self.handlers = hs.get_handlers() self._well_known_builder = WellKnownBuilder(hs) @@ -402,114 +401,6 @@ async def _do_jwt_login(self, login_submission: JsonDict) -> Dict[str, str]: return result -class RestoreDeviceServlet(RestServlet): - # FIXME: move this to v2_alpha/devices.py. This is kept here for now to - # make it easier to compare with the other PR. - """Request or complete a rehydration request. - - GET /org.matrix.msc2697.v2/restore_device - - HTTP/1.1 200 OK - Content-Type: application/json - - { - "device_id": "dehydrated_device_id", - "device_data": {"device": "data"} - } - - POST /org.matrix.msc2697.v2/restore_device - Content-Type: application/json - - { - "device_id": "dehydrated_device_id" - } - - HTTP/1.1 200 OK - Content-Type: application/json - - { - "success": true, - } - - """ - - PATTERNS = client_patterns("/org.matrix.msc2697.v2/restore_device") - - def __init__(self, hs): - super(RestoreDeviceServlet, self).__init__() - self.hs = hs - self.auth = hs.get_auth() - self.device_handler = hs.get_device_handler() - self._well_known_builder = WellKnownBuilder(hs) - - async def on_GET(self, request: SynapseRequest): - requester = await self.auth.get_user_by_req(request, allow_guest=True) - ( - device_id, - dehydrated_device, - ) = await self.device_handler.get_dehydrated_device(requester.user.to_string()) - if dehydrated_device: - result = {"device_id": device_id, "device_data": dehydrated_device} - return (200, result) - else: - raise NotFoundError() - - async def on_POST(self, request: SynapseRequest): - requester = await self.auth.get_user_by_req(request, allow_guest=True) - - submission = parse_json_object_from_request(request) - - result = await self.device_handler.rehydrate_device( - requester.user.to_string(), - self.auth.get_access_token_from_request(request), - submission["device_id"], - ) - - return (200, result) - - -class StoreDeviceServlet(RestServlet): - """Store a dehydrated device. - - POST /org.matrix.msc2697/device/dehydrate - Content-Type: application/json - - { - "device_data": { - "algorithm": "m.dehydration.v1.olm", - "account": "dehydrated_device" - } - } - - HTTP/1.1 200 OK - Content-Type: application/json - - { - "device_id": "dehydrated_device_id" - } - - """ - - PATTERNS = client_patterns("/org.matrix.msc2697/device/dehydrate") - - def __init__(self, hs): - super(StoreDeviceServlet, self).__init__() - self.hs = hs - self.auth = hs.get_auth() - self.device_handler = hs.get_device_handler() - - async def on_POST(self, request: SynapseRequest): - submission = parse_json_object_from_request(request) - requester = await self.auth.get_user_by_req(request) - - device_id = await self.device_handler.store_dehydrated_device( - requester.user.to_string(), - submission["device_data"], - submission.get("initial_device_display_name", None), - ) - return 200, {"device_id": device_id} - - class BaseSSORedirectServlet(RestServlet): """Common base class for /login/sso/redirect impls""" @@ -608,8 +499,6 @@ async def get_sso_url( def register_servlets(hs, http_server): LoginRestServlet(hs).register(http_server) - RestoreDeviceServlet(hs).register(http_server) - StoreDeviceServlet(hs).register(http_server) if hs.config.cas_enabled: CasRedirectServlet(hs).register(http_server) CasTicketServlet(hs).register(http_server) diff --git a/synapse/rest/client/v2_alpha/devices.py b/synapse/rest/client/v2_alpha/devices.py index c0714fcfb105..62909d490ac8 100644 --- a/synapse/rest/client/v2_alpha/devices.py +++ b/synapse/rest/client/v2_alpha/devices.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- # Copyright 2015, 2016 OpenMarket Ltd +# Copyright 2020 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. @@ -21,6 +22,7 @@ assert_params_in_dict, parse_json_object_from_request, ) +from synapse.http.site import SynapseRequest from ._base import client_patterns, interactive_auth_handler @@ -151,7 +153,117 @@ async def on_PUT(self, request, device_id): return 200, {} +class DehydratedDeviceServlet(RestServlet): + """Request or complete a rehydration request. + + GET /org.matrix.msc2697.v2/dehydrated_device + + HTTP/1.1 200 OK + Content-Type: application/json + + { + "device_id": "dehydrated_device_id", + "device_data": { + "algorithm": "org.matrix.msc2697.v1.dehydration.v1.olm", + "account": "dehydrated_device" + } + } + + PUT /org.matrix.msc2697/dehydrated_device + Content-Type: application/json + + { + "device_data": { + "algorithm": "org.matrix.msc2697.v1.dehydration.v1.olm", + "account": "dehydrated_device" + } + } + + HTTP/1.1 200 OK + Content-Type: application/json + + { + "device_id": "dehydrated_device_id" + } + + """ + + PATTERNS = client_patterns("/org.matrix.msc2697.v2/restore_device") + + def __init__(self, hs): + super(DehydratedDeviceServlet, self).__init__() + self.hs = hs + self.auth = hs.get_auth() + self.device_handler = hs.get_device_handler() + + async def on_GET(self, request: SynapseRequest): + requester = await self.auth.get_user_by_req(request, allow_guest=True) + ( + device_id, + dehydrated_device, + ) = await self.device_handler.get_dehydrated_device(requester.user.to_string()) + if dehydrated_device: + result = {"device_id": device_id, "device_data": dehydrated_device} + return (200, result) + else: + raise errors.NotFoundError() + + async def on_PUT(self, request: SynapseRequest): + submission = parse_json_object_from_request(request) + requester = await self.auth.get_user_by_req(request) + + device_id = await self.device_handler.store_dehydrated_device( + requester.user.to_string(), + submission["device_data"], + submission.get("initial_device_display_name", None), + ) + return 200, {"device_id": device_id} + + +class ClaimDehydratedDeviceServlet(RestServlet): + """Store a dehydrated device. + + POST /org.matrix.msc2697.v2/dehydrated_device/claim + Content-Type: application/json + + { + "device_id": "dehydrated_device_id" + } + + HTTP/1.1 200 OK + Content-Type: application/json + + { + "success": true, + } + + """ + + PATTERNS = client_patterns("/org.matrix.msc2697.v2/dehydrated_device/claim") + + def __init__(self, hs): + super(ClaimDehydratedDeviceServlet, self).__init__() + self.hs = hs + self.auth = hs.get_auth() + self.device_handler = hs.get_device_handler() + + async def on_POST(self, request: SynapseRequest): + requester = await self.auth.get_user_by_req(request, allow_guest=True) + + submission = parse_json_object_from_request(request) + + result = await self.device_handler.rehydrate_device( + requester.user.to_string(), + self.auth.get_access_token_from_request(request), + submission["device_id"], + ) + + return (200, result) + + def register_servlets(hs, http_server): DeleteDevicesRestServlet(hs).register(http_server) DevicesRestServlet(hs).register(http_server) DeviceRestServlet(hs).register(http_server) + DehydratedDeviceServlet(hs).register(http_server) + ClaimDehydratedDeviceServlet(hs).register(http_server) From 24405c79610b7ca95f077b7678c35fe3240d88d1 Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Wed, 30 Sep 2020 12:50:05 -0400 Subject: [PATCH 11/25] some fixes --- synapse/rest/client/v2_alpha/devices.py | 2 +- synapse/storage/databases/main/devices.py | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/synapse/rest/client/v2_alpha/devices.py b/synapse/rest/client/v2_alpha/devices.py index 62909d490ac8..329b3efc6f68 100644 --- a/synapse/rest/client/v2_alpha/devices.py +++ b/synapse/rest/client/v2_alpha/devices.py @@ -188,7 +188,7 @@ class DehydratedDeviceServlet(RestServlet): """ - PATTERNS = client_patterns("/org.matrix.msc2697.v2/restore_device") + PATTERNS = client_patterns("/org.matrix.msc2697.v2/dehydrated_device") def __init__(self, hs): super(DehydratedDeviceServlet, self).__init__() diff --git a/synapse/storage/databases/main/devices.py b/synapse/storage/databases/main/devices.py index 94603dd72515..e5d19b64b055 100644 --- a/synapse/storage/databases/main/devices.py +++ b/synapse/storage/databases/main/devices.py @@ -804,8 +804,9 @@ async def remove_dehydrated_device(self, user_id: str, device_id: str) -> bool: user_id: the user that the dehydrated device belongs to device_id: the ID of the dehydrated device """ - count = self.db_pool.simple_delete( - "dehydrated_devices", {"user_id": user_id, "device_id": device_id,}, + count = await self.db_pool.simple_delete( + "dehydrated_devices", {"user_id": user_id, "device_id": device_id}, + desc="remove_dehydrated_device" ) return count >= 1 From bb7c73273cf0ba7977f14c8a7649bfb3814c764b Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Wed, 30 Sep 2020 12:50:18 -0400 Subject: [PATCH 12/25] invalidate cache and clear old device when setting new device ID --- synapse/handlers/device.py | 10 ++++-- .../storage/databases/main/registration.py | 31 ++++++++++++++++--- 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/synapse/handlers/device.py b/synapse/handlers/device.py index dd5a5d826e40..59b14010dd8a 100644 --- a/synapse/handlers/device.py +++ b/synapse/handlers/device.py @@ -545,8 +545,14 @@ async def rehydrate_device( if success: # If the dehydrated device was successfully deleted (the device ID # matched the stored dehydrated device), then modify the access - # token to use the dehydrated device's ID - await self.store.set_device_for_access_token(access_token, device_id) + # token to use the dehydrated device's ID and destroy the old device ID + old_device_id = await self.store.set_device_for_access_token(access_token, device_id) + await self.store.delete_device(user_id, old_device_id) + await self.store.delete_e2e_keys_by_device(user_id=user_id, device_id=old_device_id) + + # tell everyone that the old device is gone + await self.notify_device_update(user_id, [old_device_id]) + return {"success": True} else: raise errors.NotFoundError() diff --git a/synapse/storage/databases/main/registration.py b/synapse/storage/databases/main/registration.py index a60d14e64b9a..ef0773686a21 100644 --- a/synapse/storage/databases/main/registration.py +++ b/synapse/storage/databases/main/registration.py @@ -965,18 +965,39 @@ def add_access_token_to_user(self, user_id, token, device_id, valid_until_ms): desc="add_access_token_to_user", ) - async def set_device_for_access_token(self, token, device_id): + def _set_device_for_access_token_txn( + self, txn, token: str, device_id: str + ) -> str: + old_device_id = self.db_pool.simple_select_one_onecol_txn( + txn, "access_tokens", {"token": token}, "device_id" + ) + + self.db_pool.simple_update_txn( + txn, + "access_tokens", {"token": token}, {"device_id": device_id} + ) + + self._invalidate_cache_and_stream( + txn, self.get_user_by_access_token, (token,) + ) + + return old_device_id + + async def set_device_for_access_token(self, token, device_id) -> str: """Sets the device ID associated with an access token. Args: token (str): The access token to modify. device_id (str): The new device ID. + Returns: + The old device ID associated with the access token. """ - # FIXME: invalidate caches - - return await self.db_pool.simple_update( - "access_tokens", {"token": token}, {"device_id": device_id} + return await self.db_pool.runInteraction( + "set_device_for_access_token", + self._set_device_for_access_token_txn, + token, + device_id, ) def register_user( From 6110a6ad40ec8c18ff7d77b063b951b88f52c248 Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Wed, 30 Sep 2020 12:51:19 -0400 Subject: [PATCH 13/25] run black --- synapse/handlers/device.py | 8 ++++++-- synapse/storage/databases/main/devices.py | 5 +++-- synapse/storage/databases/main/registration.py | 11 +++-------- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/synapse/handlers/device.py b/synapse/handlers/device.py index 59b14010dd8a..0b2878f7be60 100644 --- a/synapse/handlers/device.py +++ b/synapse/handlers/device.py @@ -546,9 +546,13 @@ async def rehydrate_device( # If the dehydrated device was successfully deleted (the device ID # matched the stored dehydrated device), then modify the access # token to use the dehydrated device's ID and destroy the old device ID - old_device_id = await self.store.set_device_for_access_token(access_token, device_id) + old_device_id = await self.store.set_device_for_access_token( + access_token, device_id + ) await self.store.delete_device(user_id, old_device_id) - await self.store.delete_e2e_keys_by_device(user_id=user_id, device_id=old_device_id) + await self.store.delete_e2e_keys_by_device( + user_id=user_id, device_id=old_device_id + ) # tell everyone that the old device is gone await self.notify_device_update(user_id, [old_device_id]) diff --git a/synapse/storage/databases/main/devices.py b/synapse/storage/databases/main/devices.py index e5d19b64b055..e048d4461ec8 100644 --- a/synapse/storage/databases/main/devices.py +++ b/synapse/storage/databases/main/devices.py @@ -805,8 +805,9 @@ async def remove_dehydrated_device(self, user_id: str, device_id: str) -> bool: device_id: the ID of the dehydrated device """ count = await self.db_pool.simple_delete( - "dehydrated_devices", {"user_id": user_id, "device_id": device_id}, - desc="remove_dehydrated_device" + "dehydrated_devices", + {"user_id": user_id, "device_id": device_id}, + desc="remove_dehydrated_device", ) return count >= 1 diff --git a/synapse/storage/databases/main/registration.py b/synapse/storage/databases/main/registration.py index ef0773686a21..a07f21b8fb89 100644 --- a/synapse/storage/databases/main/registration.py +++ b/synapse/storage/databases/main/registration.py @@ -965,21 +965,16 @@ def add_access_token_to_user(self, user_id, token, device_id, valid_until_ms): desc="add_access_token_to_user", ) - def _set_device_for_access_token_txn( - self, txn, token: str, device_id: str - ) -> str: + def _set_device_for_access_token_txn(self, txn, token: str, device_id: str) -> str: old_device_id = self.db_pool.simple_select_one_onecol_txn( txn, "access_tokens", {"token": token}, "device_id" ) self.db_pool.simple_update_txn( - txn, - "access_tokens", {"token": token}, {"device_id": device_id} + txn, "access_tokens", {"token": token}, {"device_id": device_id} ) - self._invalidate_cache_and_stream( - txn, self.get_user_by_access_token, (token,) - ) + self._invalidate_cache_and_stream(txn, self.get_user_by_access_token, (token,)) return old_device_id From 7029d9730b6c98d4b21ee150819da66301103c71 Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Wed, 30 Sep 2020 16:27:32 -0400 Subject: [PATCH 14/25] add unit tests --- tests/handlers/test_device.py | 76 +++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/tests/handlers/test_device.py b/tests/handlers/test_device.py index 6aa322bf3ac8..eedcb6d18b90 100644 --- a/tests/handlers/test_device.py +++ b/tests/handlers/test_device.py @@ -213,3 +213,79 @@ def _record_user( ) ) self.reactor.advance(1000) + + +class DehydrationTestCase(unittest.HomeserverTestCase): + def make_homeserver(self, reactor, clock): + hs = self.setup_test_homeserver("server", http_client=None) + self.handler = hs.get_device_handler() + self.registration = hs.get_registration_handler() + self.auth = hs.get_auth() + self.store = hs.get_datastore() + return hs + + def test_dehydrate_and_rehydrate_device(self): + user_id = "@boris:dehydration" + + self.get_success(self.store.register_user(user_id, "foobar")) + + # First check if we can store and fetch a dehydrated device + stored_dehydrated_device_id = self.get_success( + self.handler.store_dehydrated_device( + user_id=user_id, + device_data={"device_data": {"foo": "bar"}}, + initial_device_display_name="dehydrated device", + ) + ) + + retrieved_device_id, device_data = self.get_success( + self.handler.get_dehydrated_device(user_id=user_id) + ) + + self.assertEqual(retrieved_device_id, stored_dehydrated_device_id) + self.assertEqual(device_data, {"device_data": {"foo": "bar"}}) + + # Create a new login for the user and dehydrated the device + device_id, access_token = self.get_success( + self.registration.register_device( + user_id=user_id, device_id=None, initial_display_name=None, + ) + ) + + # Trying to claim a nonexistent device should throw an error + self.get_failure( + self.handler.rehydrate_device( + user_id=user_id, + access_token=access_token, + device_id="not the right device ID", + ), + synapse.api.errors.NotFoundError, + ) + + # dehydrating the right devices should succeed and change our device ID + # to the dehydrated device's ID + res = self.get_success( + self.handler.rehydrate_device( + user_id=user_id, + access_token=access_token, + device_id=retrieved_device_id, + ) + ) + + self.assertEqual(res, {"success": True}) + + # make sure that our device ID has changed + user_info = self.get_success(self.auth.get_user_by_access_token(access_token)) + + self.assertEqual(user_info["device_id"], retrieved_device_id) + + # make sure that the device ID that we were initially assigned no longer exists + self.get_failure( + self.handler.get_device(user_id, device_id), + synapse.api.errors.NotFoundError, + ) + + # make sure that there's no device available for dehydrating now + ret = self.get_success(self.handler.get_dehydrated_device(user_id=user_id)) + + self.assertEqual(ret, (None, None)) From 90f42add433b01240f54f50363e2c4b7b5a66042 Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Wed, 30 Sep 2020 16:33:42 -0400 Subject: [PATCH 15/25] lint and fix changelog --- changelog.d/7955.feature | 1 - changelog.d/8380.feature | 1 + synapse/handlers/device.py | 1 - synapse/storage/databases/main/devices.py | 2 +- 4 files changed, 2 insertions(+), 3 deletions(-) delete mode 100644 changelog.d/7955.feature create mode 100644 changelog.d/8380.feature diff --git a/changelog.d/7955.feature b/changelog.d/7955.feature deleted file mode 100644 index 7d726046fe91..000000000000 --- a/changelog.d/7955.feature +++ /dev/null @@ -1 +0,0 @@ -Add support for device dehydration. (MSC2697) diff --git a/changelog.d/8380.feature b/changelog.d/8380.feature new file mode 100644 index 000000000000..937a4de83803 --- /dev/null +++ b/changelog.d/8380.feature @@ -0,0 +1 @@ +Add support for device dehydration (MSC2697). diff --git a/synapse/handlers/device.py b/synapse/handlers/device.py index 0b2878f7be60..87066a529d2b 100644 --- a/synapse/handlers/device.py +++ b/synapse/handlers/device.py @@ -14,7 +14,6 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -import json import logging from typing import Any, Dict, List, Optional, Tuple diff --git a/synapse/storage/databases/main/devices.py b/synapse/storage/databases/main/devices.py index e048d4461ec8..b08471d986fe 100644 --- a/synapse/storage/databases/main/devices.py +++ b/synapse/storage/databases/main/devices.py @@ -43,7 +43,7 @@ cachedList, ) from synapse.util.iterutils import batch_iter -from synapse.util.stringutils import random_string, shortstr +from synapse.util.stringutils import shortstr logger = logging.getLogger(__name__) From f63d6c0421c0546a1895009cf95e487aa2ca41ef Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Wed, 30 Sep 2020 17:05:08 -0400 Subject: [PATCH 16/25] adjust copyright headers --- synapse/handlers/device.py | 2 +- synapse/rest/client/v2_alpha/keys.py | 1 + synapse/storage/databases/main/devices.py | 2 +- synapse/storage/databases/main/end_to_end_keys.py | 2 +- synapse/storage/databases/main/registration.py | 2 +- tests/handlers/test_device.py | 1 + 6 files changed, 6 insertions(+), 4 deletions(-) diff --git a/synapse/handlers/device.py b/synapse/handlers/device.py index 45b609688935..1c2bf2988662 100644 --- a/synapse/handlers/device.py +++ b/synapse/handlers/device.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- # Copyright 2016 OpenMarket Ltd # Copyright 2019 New Vector Ltd -# Copyright 2019 The Matrix.org Foundation C.I.C. +# Copyright 2019,2020 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. diff --git a/synapse/rest/client/v2_alpha/keys.py b/synapse/rest/client/v2_alpha/keys.py index 3ca444395365..d92e84844089 100644 --- a/synapse/rest/client/v2_alpha/keys.py +++ b/synapse/rest/client/v2_alpha/keys.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- # Copyright 2015, 2016 OpenMarket Ltd # Copyright 2019 New Vector Ltd +# Copyright 2020 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. diff --git a/synapse/storage/databases/main/devices.py b/synapse/storage/databases/main/devices.py index 9c93df86918c..0bd27cea7ca6 100644 --- a/synapse/storage/databases/main/devices.py +++ b/synapse/storage/databases/main/devices.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- # Copyright 2016 OpenMarket Ltd # Copyright 2019 New Vector Ltd -# Copyright 2019 The Matrix.org Foundation C.I.C. +# Copyright 2019,2020 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. diff --git a/synapse/storage/databases/main/end_to_end_keys.py b/synapse/storage/databases/main/end_to_end_keys.py index f0636561615c..e3e5f2e3b6d3 100644 --- a/synapse/storage/databases/main/end_to_end_keys.py +++ b/synapse/storage/databases/main/end_to_end_keys.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- # Copyright 2015, 2016 OpenMarket Ltd # Copyright 2019 New Vector Ltd -# Copyright 2019 The Matrix.org Foundation C.I.C. +# Copyright 2019,2020 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. diff --git a/synapse/storage/databases/main/registration.py b/synapse/storage/databases/main/registration.py index c4a2b31c22a6..aa167f5d4646 100644 --- a/synapse/storage/databases/main/registration.py +++ b/synapse/storage/databases/main/registration.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- # Copyright 2014-2016 OpenMarket Ltd # Copyright 2017-2018 New Vector Ltd -# Copyright 2019 The Matrix.org Foundation C.I.C. +# Copyright 2019,2020 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. diff --git a/tests/handlers/test_device.py b/tests/handlers/test_device.py index 1c69acaba2ea..b423ac56e64a 100644 --- a/tests/handlers/test_device.py +++ b/tests/handlers/test_device.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- # Copyright 2016 OpenMarket Ltd # Copyright 2018 New Vector Ltd +# Copyright 2020 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. From 2b30fc732d0400b99c35f9471b9cb5350b759fc6 Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Wed, 30 Sep 2020 17:14:48 -0400 Subject: [PATCH 17/25] black --- synapse/storage/databases/main/devices.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/devices.py b/synapse/storage/databases/main/devices.py index 0bd27cea7ca6..52860d967158 100644 --- a/synapse/storage/databases/main/devices.py +++ b/synapse/storage/databases/main/devices.py @@ -715,7 +715,9 @@ async def get_dehydrated_device(self, user_id: str) -> Tuple[str, JsonDict]: allow_none=True, ) return ( - (row["device_id"], json_decoder.decode(row["device_data"])) if row else (None, None) + (row["device_id"], json_decoder.decode(row["device_data"])) + if row + else (None, None) ) def _store_dehydrated_device_txn( From 8e5ef1655c859fc7f0bd11ace0d3e9f811baa7bb Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Fri, 2 Oct 2020 15:56:16 -0400 Subject: [PATCH 18/25] Apply suggestions from code review Co-authored-by: Patrick Cloke --- changelog.d/8380.feature | 2 +- synapse/storage/databases/main/registration.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/changelog.d/8380.feature b/changelog.d/8380.feature index 937a4de83803..05ccea19dce4 100644 --- a/changelog.d/8380.feature +++ b/changelog.d/8380.feature @@ -1 +1 @@ -Add support for device dehydration (MSC2697). +Add support for device dehydration ([MSC2697](https://github.com/matrix-org/matrix-doc/pull/2697)). diff --git a/synapse/storage/databases/main/registration.py b/synapse/storage/databases/main/registration.py index aa167f5d4646..16ba5457403a 100644 --- a/synapse/storage/databases/main/registration.py +++ b/synapse/storage/databases/main/registration.py @@ -977,12 +977,12 @@ def _set_device_for_access_token_txn(self, txn, token: str, device_id: str) -> s return old_device_id - async def set_device_for_access_token(self, token, device_id) -> str: + async def set_device_for_access_token(self, token: str, device_id: str) -> str: """Sets the device ID associated with an access token. Args: - token (str): The access token to modify. - device_id (str): The new device ID. + token: The access token to modify. + device_id: The new device ID. Returns: The old device ID associated with the access token. """ From 59354775e0f0135744fb4628f966a01b3dfc5cff Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Fri, 2 Oct 2020 16:52:22 -0400 Subject: [PATCH 19/25] apply suggestions from review --- synapse/handlers/device.py | 4 +- synapse/rest/client/v2_alpha/devices.py | 42 ++++++++++++++----- synapse/rest/client/v2_alpha/keys.py | 14 +++---- synapse/storage/databases/main/devices.py | 33 +++++---------- .../main/schema/delta/58/11dehydration.sql | 2 +- tests/handlers/test_device.py | 2 +- 6 files changed, 55 insertions(+), 42 deletions(-) diff --git a/synapse/handlers/device.py b/synapse/handlers/device.py index 1c2bf2988662..260901a2d13d 100644 --- a/synapse/handlers/device.py +++ b/synapse/handlers/device.py @@ -532,7 +532,9 @@ async def store_dehydrated_device( await self.delete_device(user_id, old_device_id) return device_id - async def get_dehydrated_device(self, user_id: str) -> Tuple[str, JsonDict]: + async def get_dehydrated_device( + self, user_id: str + ) -> Optional[Tuple[str, JsonDict]]: """Retrieve the information for a dehydrated device. Args: diff --git a/synapse/rest/client/v2_alpha/devices.py b/synapse/rest/client/v2_alpha/devices.py index 59ab009472a2..6fa07db50818 100644 --- a/synapse/rest/client/v2_alpha/devices.py +++ b/synapse/rest/client/v2_alpha/devices.py @@ -188,7 +188,7 @@ class DehydratedDeviceServlet(RestServlet): """ - PATTERNS = client_patterns("/org.matrix.msc2697.v2/dehydrated_device") + PATTERNS = client_patterns("/org.matrix.msc2697.v2/dehydrated_device", releases=()) def __init__(self, hs): super(DehydratedDeviceServlet, self).__init__() @@ -197,21 +197,32 @@ def __init__(self, hs): self.device_handler = hs.get_device_handler() async def on_GET(self, request: SynapseRequest): - requester = await self.auth.get_user_by_req(request, allow_guest=True) - ( - device_id, - dehydrated_device, - ) = await self.device_handler.get_dehydrated_device(requester.user.to_string()) - if dehydrated_device: + requester = await self.auth.get_user_by_req(request) + dehydrated_device = await self.device_handler.get_dehydrated_device( + requester.user.to_string() + ) + if dehydrated_device is not None: + (device_id, device_data) = dehydrated_device result = {"device_id": device_id, "device_data": dehydrated_device} return (200, result) else: - raise errors.NotFoundError() + raise errors.NotFoundError("No dehydrated device available") async def on_PUT(self, request: SynapseRequest): submission = parse_json_object_from_request(request) requester = await self.auth.get_user_by_req(request) + if "device_data" not in submission: + raise errors.SynapseError( + 400, "device_data missing", errcode=errors.Codes.MISSING_PARAM, + ) + elif isinstance(submission["device_id"], dict): + raise errors.SynapseError( + 400, + "device_data must be an object", + errcode=errors.Codes.INVALID_PARAM, + ) + device_id = await self.device_handler.store_dehydrated_device( requester.user.to_string(), submission["device_data"], @@ -239,7 +250,9 @@ class ClaimDehydratedDeviceServlet(RestServlet): """ - PATTERNS = client_patterns("/org.matrix.msc2697.v2/dehydrated_device/claim") + PATTERNS = client_patterns( + "/org.matrix.msc2697.v2/dehydrated_device/claim", releases=() + ) def __init__(self, hs): super(ClaimDehydratedDeviceServlet, self).__init__() @@ -248,10 +261,19 @@ def __init__(self, hs): self.device_handler = hs.get_device_handler() async def on_POST(self, request: SynapseRequest): - requester = await self.auth.get_user_by_req(request, allow_guest=True) + requester = await self.auth.get_user_by_req(request) submission = parse_json_object_from_request(request) + if "device_id" not in submission: + raise errors.SynapseError( + 400, "device_id missing", errcode=errors.Codes.MISSING_PARAM, + ) + elif isinstance(submission["device_id"], str): + raise errors.SynapseError( + 400, "device_id must be a string", errcode=errors.Codes.INVALID_PARAM, + ) + result = await self.device_handler.rehydrate_device( requester.user.to_string(), self.auth.get_access_token_from_request(request), diff --git a/synapse/rest/client/v2_alpha/keys.py b/synapse/rest/client/v2_alpha/keys.py index d92e84844089..16f3bf3a26d1 100644 --- a/synapse/rest/client/v2_alpha/keys.py +++ b/synapse/rest/client/v2_alpha/keys.py @@ -77,14 +77,14 @@ async def on_POST(self, request, device_id): body = parse_json_object_from_request(request) if device_id is not None: - # passing the device_id here is deprecated; however, we allow it - # for now for compatibility with older clients. + # passing the device_id here should only be done for setting keys + # for dehydrated devices; however, we allow it for now for + # compatibility with older clients. if requester.device_id is not None and device_id != requester.device_id: - ( - dehydrated_device_id, - _, - ) = await self.device_handler.get_dehydrated_device(user_id) - if device_id != dehydrated_device_id: + dehydrated_device = await self.device_handler.get_dehydrated_device( + user_id + ) + if dehydrated_device is not None and device_id != dehydrated_device[0]: set_tag("error", True) log_kv( { diff --git a/synapse/storage/databases/main/devices.py b/synapse/storage/databases/main/devices.py index 52860d967158..317d6cde955d 100644 --- a/synapse/storage/databases/main/devices.py +++ b/synapse/storage/databases/main/devices.py @@ -698,7 +698,9 @@ def _mark_remote_user_device_list_as_unsubscribed_txn(txn): _mark_remote_user_device_list_as_unsubscribed_txn, ) - async def get_dehydrated_device(self, user_id: str) -> Tuple[str, JsonDict]: + async def get_dehydrated_device( + self, user_id: str + ) -> Optional[Tuple[str, JsonDict]]: """Retrieve the information for a dehydrated device. Args: @@ -715,9 +717,7 @@ async def get_dehydrated_device(self, user_id: str) -> Tuple[str, JsonDict]: allow_none=True, ) return ( - (row["device_id"], json_decoder.decode(row["device_data"])) - if row - else (None, None) + (row["device_id"], json_decoder.decode(row["device_data"])) if row else None ) def _store_dehydrated_device_txn( @@ -730,23 +730,12 @@ def _store_dehydrated_device_txn( retcol="device_id", allow_none=True, ) - if old_device_id is None: - self.db_pool.simple_insert_txn( - txn, - table="dehydrated_devices", - values={ - "user_id": user_id, - "device_id": device_id, - "device_data": device_data, - }, - ) - else: - self.db_pool.simple_update_txn( - txn, - table="dehydrated_devices", - keyvalues={"user_id": user_id}, - updatevalues={"device_id": device_id, "device_data": device_data}, - ) + self.db_pool.simple_upsert_txn( + txn, + table="dehydrated_devices", + keyvalues={"user_id": user_id}, + values={"device_id": device_id, "device_data": device_data}, + ) return old_device_id async def store_dehydrated_device( @@ -756,8 +745,8 @@ async def store_dehydrated_device( Args: user_id: the user that we are storing the device for + device_id: the ID of the dehydrated device device_data: the dehydrated device information - initial_device_display_name: The display name to use for the device Returns: device id of the user's previous dehydrated device, if any """ diff --git a/synapse/storage/databases/main/schema/delta/58/11dehydration.sql b/synapse/storage/databases/main/schema/delta/58/11dehydration.sql index a1e256aa8103..7851a0a825e4 100644 --- a/synapse/storage/databases/main/schema/delta/58/11dehydration.sql +++ b/synapse/storage/databases/main/schema/delta/58/11dehydration.sql @@ -16,5 +16,5 @@ CREATE TABLE IF NOT EXISTS dehydrated_devices( user_id TEXT NOT NULL PRIMARY KEY, device_id TEXT NOT NULL, - device_data TEXT NOT NULL + device_data TEXT NOT NULL -- JSON-encoded client-defined data ); diff --git a/tests/handlers/test_device.py b/tests/handlers/test_device.py index b423ac56e64a..e27689705625 100644 --- a/tests/handlers/test_device.py +++ b/tests/handlers/test_device.py @@ -300,4 +300,4 @@ def test_dehydrate_and_rehydrate_device(self): # make sure that there's no device available for dehydrating now ret = self.get_success(self.handler.get_dehydrated_device(user_id=user_id)) - self.assertEqual(ret, (None, None)) + self.assertEqual(ret, None) From e69835e04e354936c0b681e87e7635c3702cd2d4 Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Mon, 5 Oct 2020 12:30:23 -0400 Subject: [PATCH 20/25] Apply suggestions from code review Co-authored-by: Patrick Cloke --- synapse/rest/client/v2_alpha/devices.py | 4 ++-- synapse/rest/client/v2_alpha/keys.py | 4 ++-- tests/handlers/test_device.py | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/synapse/rest/client/v2_alpha/devices.py b/synapse/rest/client/v2_alpha/devices.py index 6fa07db50818..15ca685677a6 100644 --- a/synapse/rest/client/v2_alpha/devices.py +++ b/synapse/rest/client/v2_alpha/devices.py @@ -216,7 +216,7 @@ async def on_PUT(self, request: SynapseRequest): raise errors.SynapseError( 400, "device_data missing", errcode=errors.Codes.MISSING_PARAM, ) - elif isinstance(submission["device_id"], dict): + elif not isinstance(submission["device_id"], dict): raise errors.SynapseError( 400, "device_data must be an object", @@ -269,7 +269,7 @@ async def on_POST(self, request: SynapseRequest): raise errors.SynapseError( 400, "device_id missing", errcode=errors.Codes.MISSING_PARAM, ) - elif isinstance(submission["device_id"], str): + elif not isinstance(submission["device_id"], str): raise errors.SynapseError( 400, "device_id must be a string", errcode=errors.Codes.INVALID_PARAM, ) diff --git a/synapse/rest/client/v2_alpha/keys.py b/synapse/rest/client/v2_alpha/keys.py index 16f3bf3a26d1..b91996c7387d 100644 --- a/synapse/rest/client/v2_alpha/keys.py +++ b/synapse/rest/client/v2_alpha/keys.py @@ -77,8 +77,8 @@ async def on_POST(self, request, device_id): body = parse_json_object_from_request(request) if device_id is not None: - # passing the device_id here should only be done for setting keys - # for dehydrated devices; however, we allow it for now for + # Providing the device_id should only be done for setting keys + # for dehydrated devices; however, we allow it for any device for # compatibility with older clients. if requester.device_id is not None and device_id != requester.device_id: dehydrated_device = await self.device_handler.get_dehydrated_device( diff --git a/tests/handlers/test_device.py b/tests/handlers/test_device.py index e27689705625..110a1c9a3d44 100644 --- a/tests/handlers/test_device.py +++ b/tests/handlers/test_device.py @@ -300,4 +300,4 @@ def test_dehydrate_and_rehydrate_device(self): # make sure that there's no device available for dehydrating now ret = self.get_success(self.handler.get_dehydrated_device(user_id=user_id)) - self.assertEqual(ret, None) + self.assertIsNone(ret) From b934fde8dd5b3c5dcb978bde452d7ea301415139 Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Mon, 5 Oct 2020 16:46:26 -0400 Subject: [PATCH 21/25] fix a couple incorrect names --- synapse/rest/client/v2_alpha/devices.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/rest/client/v2_alpha/devices.py b/synapse/rest/client/v2_alpha/devices.py index 15ca685677a6..79fe20ae9310 100644 --- a/synapse/rest/client/v2_alpha/devices.py +++ b/synapse/rest/client/v2_alpha/devices.py @@ -203,7 +203,7 @@ async def on_GET(self, request: SynapseRequest): ) if dehydrated_device is not None: (device_id, device_data) = dehydrated_device - result = {"device_id": device_id, "device_data": dehydrated_device} + result = {"device_id": device_id, "device_data": device_data} return (200, result) else: raise errors.NotFoundError("No dehydrated device available") @@ -216,7 +216,7 @@ async def on_PUT(self, request: SynapseRequest): raise errors.SynapseError( 400, "device_data missing", errcode=errors.Codes.MISSING_PARAM, ) - elif not isinstance(submission["device_id"], dict): + elif not isinstance(submission["device_data"], dict): raise errors.SynapseError( 400, "device_data must be an object", From 0b34ac0d705bdaf1f559b71b71deda8fa1599985 Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Mon, 5 Oct 2020 18:13:24 -0400 Subject: [PATCH 22/25] update display name when dehydrating device --- synapse/handlers/device.py | 11 ++++++++--- tests/handlers/test_device.py | 7 ++++++- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/synapse/handlers/device.py b/synapse/handlers/device.py index 260901a2d13d..3a7d3deed88b 100644 --- a/synapse/handlers/device.py +++ b/synapse/handlers/device.py @@ -562,17 +562,22 @@ async def rehydrate_device( if success: # If the dehydrated device was successfully deleted (the device ID # matched the stored dehydrated device), then modify the access - # token to use the dehydrated device's ID and destroy the old device ID + # token to use the dehydrated device's ID and copy the old device + # display name to the dehydrated device, and destroy the old device + # ID old_device_id = await self.store.set_device_for_access_token( access_token, device_id ) + old_device = await self.store.get_device(user_id, old_device_id) + await self.store.update_device(user_id, device_id, old_device["display_name"]) await self.store.delete_device(user_id, old_device_id) await self.store.delete_e2e_keys_by_device( user_id=user_id, device_id=old_device_id ) - # tell everyone that the old device is gone - await self.notify_device_update(user_id, [old_device_id]) + # tell everyone that the old device is gone and that the dehydrated + # device has a new display name + await self.notify_device_update(user_id, [old_device_id, device_id]) return {"success": True} else: diff --git a/tests/handlers/test_device.py b/tests/handlers/test_device.py index 110a1c9a3d44..4512c513111c 100644 --- a/tests/handlers/test_device.py +++ b/tests/handlers/test_device.py @@ -260,7 +260,7 @@ def test_dehydrate_and_rehydrate_device(self): # Create a new login for the user and dehydrated the device device_id, access_token = self.get_success( self.registration.register_device( - user_id=user_id, device_id=None, initial_display_name=None, + user_id=user_id, device_id=None, initial_display_name="new device", ) ) @@ -291,6 +291,11 @@ def test_dehydrate_and_rehydrate_device(self): self.assertEqual(user_info["device_id"], retrieved_device_id) + # make sure the device has the display name that was set from the login + res = self.get_success(self.handler.get_device(user_id, retrieved_device_id)) + + self.assertEqual(res["display_name"], "new device") + # make sure that the device ID that we were initially assigned no longer exists self.get_failure( self.handler.get_device(user_id, device_id), From a87cb40c9edab6958241c4c433ea160dcf9aa21e Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Mon, 5 Oct 2020 18:19:27 -0400 Subject: [PATCH 23/25] black --- synapse/handlers/device.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/synapse/handlers/device.py b/synapse/handlers/device.py index 3a7d3deed88b..1db7a38d38b1 100644 --- a/synapse/handlers/device.py +++ b/synapse/handlers/device.py @@ -569,7 +569,9 @@ async def rehydrate_device( access_token, device_id ) old_device = await self.store.get_device(user_id, old_device_id) - await self.store.update_device(user_id, device_id, old_device["display_name"]) + await self.store.update_device( + user_id, device_id, old_device["display_name"] + ) await self.store.delete_device(user_id, old_device_id) await self.store.delete_e2e_keys_by_device( user_id=user_id, device_id=old_device_id From 2a10d7a5e5b4ae42cb54d5630b6616289ff646e6 Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Mon, 5 Oct 2020 19:36:36 -0400 Subject: [PATCH 24/25] fix comments --- synapse/rest/client/v2_alpha/devices.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/rest/client/v2_alpha/devices.py b/synapse/rest/client/v2_alpha/devices.py index 79fe20ae9310..201678a7c85b 100644 --- a/synapse/rest/client/v2_alpha/devices.py +++ b/synapse/rest/client/v2_alpha/devices.py @@ -154,7 +154,7 @@ async def on_PUT(self, request, device_id): class DehydratedDeviceServlet(RestServlet): - """Request or complete a rehydration request. + """Retrieve or store a dehydrated device. GET /org.matrix.msc2697.v2/dehydrated_device @@ -232,7 +232,7 @@ async def on_PUT(self, request: SynapseRequest): class ClaimDehydratedDeviceServlet(RestServlet): - """Store a dehydrated device. + """Claim a dehydrated device. POST /org.matrix.msc2697.v2/dehydrated_device/claim Content-Type: application/json From f37f6a0a340d0620d9c964b14cbfa5d46d33c847 Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Tue, 6 Oct 2020 13:25:24 -0400 Subject: [PATCH 25/25] apply changes from review --- synapse/handlers/device.py | 46 ++++++++++++------------- synapse/rest/client/v2_alpha/devices.py | 4 +-- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/synapse/handlers/device.py b/synapse/handlers/device.py index 1db7a38d38b1..e883ed1e37ea 100644 --- a/synapse/handlers/device.py +++ b/synapse/handlers/device.py @@ -559,31 +559,31 @@ async def rehydrate_device( """ success = await self.store.remove_dehydrated_device(user_id, device_id) - if success: - # If the dehydrated device was successfully deleted (the device ID - # matched the stored dehydrated device), then modify the access - # token to use the dehydrated device's ID and copy the old device - # display name to the dehydrated device, and destroy the old device - # ID - old_device_id = await self.store.set_device_for_access_token( - access_token, device_id - ) - old_device = await self.store.get_device(user_id, old_device_id) - await self.store.update_device( - user_id, device_id, old_device["display_name"] - ) - await self.store.delete_device(user_id, old_device_id) - await self.store.delete_e2e_keys_by_device( - user_id=user_id, device_id=old_device_id - ) + if not success: + raise errors.NotFoundError() - # tell everyone that the old device is gone and that the dehydrated - # device has a new display name - await self.notify_device_update(user_id, [old_device_id, device_id]) + # If the dehydrated device was successfully deleted (the device ID + # matched the stored dehydrated device), then modify the access + # token to use the dehydrated device's ID and copy the old device + # display name to the dehydrated device, and destroy the old device + # ID + old_device_id = await self.store.set_device_for_access_token( + access_token, device_id + ) + old_device = await self.store.get_device(user_id, old_device_id) + await self.store.update_device(user_id, device_id, old_device["display_name"]) + # can't call self.delete_device because that will clobber the + # access token so call the storage layer directly + await self.store.delete_device(user_id, old_device_id) + await self.store.delete_e2e_keys_by_device( + user_id=user_id, device_id=old_device_id + ) - return {"success": True} - else: - raise errors.NotFoundError() + # tell everyone that the old device is gone and that the dehydrated + # device has a new display name + await self.notify_device_update(user_id, [old_device_id, device_id]) + + return {"success": True} def _update_device_from_client_ips(device, client_ips): diff --git a/synapse/rest/client/v2_alpha/devices.py b/synapse/rest/client/v2_alpha/devices.py index 201678a7c85b..af117cb27c16 100644 --- a/synapse/rest/client/v2_alpha/devices.py +++ b/synapse/rest/client/v2_alpha/devices.py @@ -191,7 +191,7 @@ class DehydratedDeviceServlet(RestServlet): PATTERNS = client_patterns("/org.matrix.msc2697.v2/dehydrated_device", releases=()) def __init__(self, hs): - super(DehydratedDeviceServlet, self).__init__() + super().__init__() self.hs = hs self.auth = hs.get_auth() self.device_handler = hs.get_device_handler() @@ -255,7 +255,7 @@ class ClaimDehydratedDeviceServlet(RestServlet): ) def __init__(self, hs): - super(ClaimDehydratedDeviceServlet, self).__init__() + super().__init__() self.hs = hs self.auth = hs.get_auth() self.device_handler = hs.get_device_handler()