From e759ab61e8a22235c45192750b536612a6c2cb8a Mon Sep 17 00:00:00 2001 From: Victor Shyba Date: Wed, 27 Jan 2016 21:35:43 -0300 Subject: [refactor] isolate requests Isolate requests lib related code and update docstrings. --- src/leap/keymanager/__init__.py | 112 +++++++++++++-------------- src/leap/keymanager/tests/test_keymanager.py | 8 +- 2 files changed, 56 insertions(+), 64 deletions(-) diff --git a/src/leap/keymanager/__init__.py b/src/leap/keymanager/__init__.py index 06128c0..6413e9e 100644 --- a/src/leap/keymanager/__init__.py +++ b/src/leap/keymanager/__init__.py @@ -187,35 +187,43 @@ class KeyManager(object): self._wrapper_map).pop() @defer.inlineCallbacks - def _get(self, uri, data=None): + def _get_json(self, address): """ Send a GET request to C{uri} containing C{data}. :param uri: The URI of the request. :type uri: str - :param data: The body of the request. - :type data: dict, str or file - :return: The response to the request. - :rtype: requests.Response + :return: A deferred that will be fired with the GET response + :rtype: Deferred """ leap_assert( self._ca_cert_path is not None, 'We need the CA certificate path!') - res = yield threads.deferToThread(self._fetcher.get, uri, data=data, - verify=self._ca_cert_path) - # Nickserver now returns 404 for key not found and 500 for - # other cases (like key too small), so we are skipping this - # check for the time being - # res.raise_for_status() - + try: + uri, data = self._nickserver_uri, {'address': address} + res = yield threads.deferToThread(self._fetcher.get, uri, + data=data, + verify=self._ca_cert_path) + res.raise_for_status() + except requests.exceptions.HTTPError as e: + if e.response.status_code == 404: + raise KeyNotFound(address) + else: + raise KeyNotFound(e.message) + logger.warning("HTTP error retrieving key: %r" % (e,)) + logger.warning("%s" % (res.content,)) + except Exception as e: + raise KeyNotFound(e.message) + logger.warning("Error retrieving key: %r" % (e,)) # Responses are now text/plain, although it's json anyway, but # this will fail when it shouldn't # leap_assert( # res.headers['content-type'].startswith('application/json'), # 'Content-type is not JSON.') - defer.returnValue(res) + defer.returnValue(res.json()) + @defer.inlineCallbacks def _get_with_combined_ca_bundle(self, uri, data=None): """ Send a GET request to C{uri} containing C{data}. @@ -228,12 +236,18 @@ class KeyManager(object): :param data: The body of the request. :type data: dict, str or file - :return: The response to the request. - :rtype: requests.Response + :return: A deferred that will be fired with the GET response + :rtype: Deferred """ - return threads.deferToThread(self._fetcher.get, - uri, data=data, - verify=self._combined_ca_bundle) + try: + res = yield threads.deferToThread(self._fetcher.get, uri, + verify=self._combined_ca_bundle) + except Exception as e: + logger.warning("There was a problem fetching key: %s" % (e,)) + raise KeyNotFound(uri) + if not res.ok: + raise KeyNotFound(uri) + defer.returnValue(res.content) @defer.inlineCallbacks def _put(self, uri, data=None): @@ -249,8 +263,8 @@ class KeyManager(object): :param data: The body of the request. :type data: dict, str or file - :return: The response to the request. - :rtype: requests.Response + :return: A deferred that will be fired when PUT request finishes + :rtype: Deferred """ leap_assert( self._ca_cert_path is not None, @@ -284,38 +298,22 @@ class KeyManager(object): """ # request keys from the nickserver - d = defer.succeed(None) - res = None - try: - res = yield self._get(self._nickserver_uri, {'address': address}) - res.raise_for_status() - server_keys = res.json() - - # insert keys in local database - if self.OPENPGP_KEY in server_keys: - # nicknym server is authoritative for its own domain, - # for other domains the key might come from key servers. - validation_level = ValidationLevels.Weak_Chain - _, domain = _split_email(address) - if (domain == _get_domain(self._nickserver_uri)): - validation_level = ValidationLevels.Provider_Trust - - d = self.put_raw_key( - server_keys['openpgp'], - OpenPGPKey, - address=address, - validation=validation_level) - except requests.exceptions.HTTPError as e: - if e.response.status_code == 404: - d = defer.fail(KeyNotFound(address)) - else: - d = defer.fail(KeyNotFound(e.message)) - logger.warning("HTTP error retrieving key: %r" % (e,)) - logger.warning("%s" % (res.content,)) - except Exception as e: - d = defer.fail(KeyNotFound(e.message)) - logger.warning("Error retrieving key: %r" % (e,)) - yield d + server_keys = yield self._get_json(address) + + # insert keys in local database + if self.OPENPGP_KEY in server_keys: + # nicknym server is authoritative for its own domain, + # for other domains the key might come from key servers. + validation_level = ValidationLevels.Weak_Chain + _, domain = _split_email(address) + if (domain == _get_domain(self._nickserver_uri)): + validation_level = ValidationLevels.Provider_Trust + + yield self.put_raw_key( + server_keys['openpgp'], + OpenPGPKey, + address=address, + validation=validation_level) # # key management @@ -861,16 +859,10 @@ class KeyManager(object): _keys = self._wrapper_map[ktype] logger.info("Fetch key for %s from %s" % (address, uri)) - try: - res = yield self._get_with_combined_ca_bundle(uri) - except Exception as e: - logger.warning("There was a problem fetching key: %s" % (e,)) - raise KeyNotFound(uri) - if not res.ok: - raise KeyNotFound(uri) + ascii_content = yield self._get_with_combined_ca_bundle(uri) # XXX parse binary keys - pubkey, _ = _keys.parse_ascii_key(res.content) + pubkey, _ = _keys.parse_ascii_key(ascii_content) if pubkey is None: raise KeyNotFound(uri) diff --git a/src/leap/keymanager/tests/test_keymanager.py b/src/leap/keymanager/tests/test_keymanager.py index 856d6da..afcbd5f 100644 --- a/src/leap/keymanager/tests/test_keymanager.py +++ b/src/leap/keymanager/tests/test_keymanager.py @@ -342,7 +342,7 @@ class KeyManagerKeyManagementTestCase(KeyManagerWithSoledadTestCase): yield km.fetch_key(ADDRESS_OTHER, REMOTE_KEY_URL, OpenPGPKey) - get_mock.assert_called_once_with(REMOTE_KEY_URL, data=None, + get_mock.assert_called_once_with(REMOTE_KEY_URL, verify=ca_bundle.where()) @inlineCallbacks @@ -353,7 +353,7 @@ class KeyManagerKeyManagementTestCase(KeyManagerWithSoledadTestCase): yield km.fetch_key(ADDRESS_OTHER, REMOTE_KEY_URL, OpenPGPKey) - get_mock.assert_called_once_with(REMOTE_KEY_URL, data=None, + get_mock.assert_called_once_with(REMOTE_KEY_URL, verify=ca_bundle.where()) @inlineCallbacks @@ -364,7 +364,7 @@ class KeyManagerKeyManagementTestCase(KeyManagerWithSoledadTestCase): yield km.fetch_key(ADDRESS_OTHER, REMOTE_KEY_URL, OpenPGPKey) - get_mock.assert_called_once_with(REMOTE_KEY_URL, data=None, + get_mock.assert_called_once_with(REMOTE_KEY_URL, verify=ca_bundle.where()) @inlineCallbacks @@ -383,7 +383,7 @@ class KeyManagerKeyManagementTestCase(KeyManagerWithSoledadTestCase): yield km.fetch_key(ADDRESS_OTHER, REMOTE_KEY_URL, OpenPGPKey) # assert that combined bundle file is passed to get call - get_mock.assert_called_once_with(REMOTE_KEY_URL, data=None, + get_mock.assert_called_once_with(REMOTE_KEY_URL, verify=tmp_output.name) # assert that files got appended -- cgit v1.2.3