From 664c81e8b7525bfaa5c3a7620f2f831ade0754a2 Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Wed, 6 Feb 2019 17:47:22 -0500 Subject: [PATCH 1/6] return proper error codes for some 404s --- synapse/handlers/e2e_room_keys.py | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/synapse/handlers/e2e_room_keys.py b/synapse/handlers/e2e_room_keys.py index 42b040375f0c..c5d7bf0c29c9 100644 --- a/synapse/handlers/e2e_room_keys.py +++ b/synapse/handlers/e2e_room_keys.py @@ -267,7 +267,7 @@ def get_version_info(self, user_id, version=None): version(str): Optional; if None gives the most recent version otherwise a historical one. Raises: - StoreError: code 404 if the requested backup version doesn't exist + NotFoundError: if the requested backup version doesn't exist Returns: A deferred of a info dict that gives the info about the new version. @@ -279,7 +279,13 @@ def get_version_info(self, user_id, version=None): """ with (yield self._upload_linearizer.queue(user_id)): - res = yield self.store.get_e2e_room_keys_version_info(user_id, version) + try: + res = yield self.store.get_e2e_room_keys_version_info(user_id, version) + except StoreError as e: + if e.code == 404: + raise NotFoundError("Unknown backup version") + else: + raise defer.returnValue(res) @defer.inlineCallbacks @@ -290,8 +296,14 @@ def delete_version(self, user_id, version=None): user_id(str): the user whose current backup version we're deleting version(str): the version id of the backup being deleted Raises: - StoreError: code 404 if this backup version doesn't exist + NotFoundError: if this backup version doesn't exist """ with (yield self._upload_linearizer.queue(user_id)): - yield self.store.delete_e2e_room_keys_version(user_id, version) + try: + yield self.store.delete_e2e_room_keys_version(user_id, version) + except StoreError as e: + if e.code == 404: + raise NotFoundError("Unknown backup version") + else: + raise From 82486371738a130322f3a1829bc4b49f79c1a3e4 Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Wed, 6 Feb 2019 17:57:10 -0500 Subject: [PATCH 2/6] add new endpoint to update backup versions --- synapse/handlers/e2e_room_keys.py | 34 ++++++++++++++++++++++- synapse/rest/client/v2_alpha/room_keys.py | 33 ++++++++++++++++++++++ synapse/storage/e2e_room_keys.py | 21 ++++++++++++++ 3 files changed, 87 insertions(+), 1 deletion(-) diff --git a/synapse/handlers/e2e_room_keys.py b/synapse/handlers/e2e_room_keys.py index c5d7bf0c29c9..e0e5ece7470d 100644 --- a/synapse/handlers/e2e_room_keys.py +++ b/synapse/handlers/e2e_room_keys.py @@ -19,7 +19,8 @@ from twisted.internet import defer -from synapse.api.errors import NotFoundError, RoomKeysVersionError, StoreError +from synapse.api.errors import Codes, NotFoundError, RoomKeysVersionError, \ + StoreError, SynapseError from synapse.util.async_helpers import Linearizer logger = logging.getLogger(__name__) @@ -307,3 +308,34 @@ def delete_version(self, user_id, version=None): raise NotFoundError("Unknown backup version") else: raise + + @defer.inlineCallbacks + def update_version(self, user_id, version, version_info): + """Update the info about a given version of the user's backup + + Args: + user_id(str): the user whose current backup version we're updating + version(str): the backup version we're updating + version_info(dict): the new information about the backup + Raises: + NotFoundError: if the requested backup version doesn't exist + Returns: + A deferred of an empty dict. + """ + try: + old_info = yield self.store.get_e2e_room_keys_version_info(user_id, version) + except StoreError as e: + if e.code == 404: + raise NotFoundError("Unknown backup version") + else: + raise + if old_info["algorithm"] != version_info["algorithm"]: + raise SynapseError( + 400, + "Algorithm does not match", + Codes.INVALID_PARAM + ) + + yield self.store.update_e2e_room_keys_version(user_id, version, version_info) + + defer.returnValue({}) diff --git a/synapse/rest/client/v2_alpha/room_keys.py b/synapse/rest/client/v2_alpha/room_keys.py index ab3f1bd21a2c..1c39d2af1c0b 100644 --- a/synapse/rest/client/v2_alpha/room_keys.py +++ b/synapse/rest/client/v2_alpha/room_keys.py @@ -380,6 +380,39 @@ def on_DELETE(self, request, version): ) defer.returnValue((200, {})) + @defer.inlineCallbacks + def on_PUT(self, request, version): + """ + Update the information about a given version of the user's room_keys backup. + + POST /room_keys/version/12345 HTTP/1.1 + Content-Type: application/json + { + "algorithm": "m.megolm_backup.v1", + "auth_data": { + "public_key": "abcdefg", + "signatures": { + "ed25519:something": "hijklmnop" + } + } + } + + HTTP/1.1 200 OK + Content-Type: application/json + {} + """ + requester = yield self.auth.get_user_by_req(request, allow_guest=False) + user_id = requester.user.to_string() + info = parse_json_object_from_request(request) + + if version is None: + raise SynapseError(400, "No version specified to update", Codes.MISSING_PARAM) + + yield self.e2e_room_keys_handler.update_version( + user_id, version, info + ) + defer.returnValue((200, {})) + def register_servlets(hs, http_server): RoomKeysServlet(hs).register(http_server) diff --git a/synapse/storage/e2e_room_keys.py b/synapse/storage/e2e_room_keys.py index 45cebe61d1fa..9a3aec759e75 100644 --- a/synapse/storage/e2e_room_keys.py +++ b/synapse/storage/e2e_room_keys.py @@ -298,6 +298,27 @@ def _create_e2e_room_keys_version_txn(txn): "create_e2e_room_keys_version_txn", _create_e2e_room_keys_version_txn ) + def update_e2e_room_keys_version(self, user_id, version, info): + """Update a given backup version + + Args: + user_id(str): the user whose backup version we're updating + version(str): the version ID of the backup version we're updating + info(dict): the new backup version info to store + """ + + return self._simple_update( + table="e2e_room_keys_versions", + keyvalues={ + "user_id": user_id, + "version": version, + }, + updatevalues={ + "auth_data": json.dumps(info["auth_data"]), + }, + desc="update_e2e_room_keys_version" + ) + def delete_e2e_room_keys_version(self, user_id, version=None): """Delete a given backup version of the user's room keys. Doesn't delete their actual key data. From 9ff620a518739bfe7417bac20365e4acf9c5906e Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Wed, 6 Feb 2019 21:32:52 -0500 Subject: [PATCH 3/6] fix import to make isort happy --- synapse/handlers/e2e_room_keys.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/synapse/handlers/e2e_room_keys.py b/synapse/handlers/e2e_room_keys.py index e0e5ece7470d..6c43c9db7644 100644 --- a/synapse/handlers/e2e_room_keys.py +++ b/synapse/handlers/e2e_room_keys.py @@ -19,8 +19,13 @@ from twisted.internet import defer -from synapse.api.errors import Codes, NotFoundError, RoomKeysVersionError, \ - StoreError, SynapseError +from synapse.api.errors import ( + Codes, + NotFoundError, + RoomKeysVersionError, + StoreError, + SynapseError +) from synapse.util.async_helpers import Linearizer logger = logging.getLogger(__name__) From 51b73be63bd6931018f98ea00f6b723c6196219a Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Wed, 6 Feb 2019 21:39:56 -0500 Subject: [PATCH 4/6] add changelog entry --- changelog.d/4580.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/4580.feature diff --git a/changelog.d/4580.feature b/changelog.d/4580.feature new file mode 100644 index 000000000000..a2a5a77dbebe --- /dev/null +++ b/changelog.d/4580.feature @@ -0,0 +1 @@ +Add ability to update backup versions \ No newline at end of file From d9e424bf649c1d4ba7b9da7fd64db84cda389e11 Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Wed, 6 Feb 2019 22:18:41 -0500 Subject: [PATCH 5/6] re-try to make isort happy --- synapse/handlers/e2e_room_keys.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/handlers/e2e_room_keys.py b/synapse/handlers/e2e_room_keys.py index 6c43c9db7644..546050f8e7fc 100644 --- a/synapse/handlers/e2e_room_keys.py +++ b/synapse/handlers/e2e_room_keys.py @@ -24,7 +24,7 @@ NotFoundError, RoomKeysVersionError, StoreError, - SynapseError + SynapseError, ) from synapse.util.async_helpers import Linearizer From afae8442b56d2e2466812916654396341038c38c Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Fri, 8 Feb 2019 01:32:45 -0500 Subject: [PATCH 6/6] make sure version is in body and wrap in linearizer queue also add tests --- synapse/handlers/e2e_room_keys.py | 37 ++++++++---- synapse/rest/client/v2_alpha/room_keys.py | 3 +- tests/handlers/test_e2e_room_keys.py | 72 +++++++++++++++++++++++ 3 files changed, 100 insertions(+), 12 deletions(-) diff --git a/synapse/handlers/e2e_room_keys.py b/synapse/handlers/e2e_room_keys.py index 546050f8e7fc..7bc174070e6c 100644 --- a/synapse/handlers/e2e_room_keys.py +++ b/synapse/handlers/e2e_room_keys.py @@ -327,20 +327,35 @@ def update_version(self, user_id, version, version_info): Returns: A deferred of an empty dict. """ - try: - old_info = yield self.store.get_e2e_room_keys_version_info(user_id, version) - except StoreError as e: - if e.code == 404: - raise NotFoundError("Unknown backup version") - else: - raise - if old_info["algorithm"] != version_info["algorithm"]: + if "version" not in version_info: + raise SynapseError( + 400, + "Missing version in body", + Codes.MISSING_PARAM + ) + if version_info["version"] != version: raise SynapseError( 400, - "Algorithm does not match", + "Version in body does not match", Codes.INVALID_PARAM ) + with (yield self._upload_linearizer.queue(user_id)): + try: + old_info = yield self.store.get_e2e_room_keys_version_info( + user_id, version + ) + except StoreError as e: + if e.code == 404: + raise NotFoundError("Unknown backup version") + else: + raise + if old_info["algorithm"] != version_info["algorithm"]: + raise SynapseError( + 400, + "Algorithm does not match", + Codes.INVALID_PARAM + ) - yield self.store.update_e2e_room_keys_version(user_id, version, version_info) + yield self.store.update_e2e_room_keys_version(user_id, version, version_info) - defer.returnValue({}) + defer.returnValue({}) diff --git a/synapse/rest/client/v2_alpha/room_keys.py b/synapse/rest/client/v2_alpha/room_keys.py index 1c39d2af1c0b..220a0de30bf6 100644 --- a/synapse/rest/client/v2_alpha/room_keys.py +++ b/synapse/rest/client/v2_alpha/room_keys.py @@ -394,7 +394,8 @@ def on_PUT(self, request, version): "signatures": { "ed25519:something": "hijklmnop" } - } + }, + "version": "42" } HTTP/1.1 200 OK diff --git a/tests/handlers/test_e2e_room_keys.py b/tests/handlers/test_e2e_room_keys.py index c8994f416e12..1c49bbbc3c51 100644 --- a/tests/handlers/test_e2e_room_keys.py +++ b/tests/handlers/test_e2e_room_keys.py @@ -125,6 +125,78 @@ def test_create_version(self): "auth_data": "second_version_auth_data", }) + @defer.inlineCallbacks + def test_update_version(self): + """Check that we can update versions. + """ + version = yield self.handler.create_version(self.local_user, { + "algorithm": "m.megolm_backup.v1", + "auth_data": "first_version_auth_data", + }) + self.assertEqual(version, "1") + + res = yield self.handler.update_version(self.local_user, version, { + "algorithm": "m.megolm_backup.v1", + "auth_data": "revised_first_version_auth_data", + "version": version + }) + self.assertDictEqual(res, {}) + + # check we can retrieve it as the current version + res = yield self.handler.get_version_info(self.local_user) + self.assertDictEqual(res, { + "algorithm": "m.megolm_backup.v1", + "auth_data": "revised_first_version_auth_data", + "version": version + }) + + @defer.inlineCallbacks + def test_update_missing_version(self): + """Check that we get a 404 on updating nonexistent versions + """ + res = None + try: + yield self.handler.update_version(self.local_user, "1", { + "algorithm": "m.megolm_backup.v1", + "auth_data": "revised_first_version_auth_data", + "version": "1" + }) + except errors.SynapseError as e: + res = e.code + self.assertEqual(res, 404) + + @defer.inlineCallbacks + def test_update_bad_version(self): + """Check that we get a 400 if the version in the body is missing or + doesn't match + """ + version = yield self.handler.create_version(self.local_user, { + "algorithm": "m.megolm_backup.v1", + "auth_data": "first_version_auth_data", + }) + self.assertEqual(version, "1") + + res = None + try: + yield self.handler.update_version(self.local_user, version, { + "algorithm": "m.megolm_backup.v1", + "auth_data": "revised_first_version_auth_data" + }) + except errors.SynapseError as e: + res = e.code + self.assertEqual(res, 400) + + res = None + try: + yield self.handler.update_version(self.local_user, version, { + "algorithm": "m.megolm_backup.v1", + "auth_data": "revised_first_version_auth_data", + "version": "incorrect" + }) + except errors.SynapseError as e: + res = e.code + self.assertEqual(res, 400) + @defer.inlineCallbacks def test_delete_missing_version(self): """Check that we get a 404 on deleting nonexistent versions