From a4872bd64884cccd85e63cc2cae631f26dbc96c4 Mon Sep 17 00:00:00 2001 From: Victor Shyba Date: Wed, 27 Jan 2016 20:45:00 -0300 Subject: [feat] defer blocking requests calls to thread That's a temporary fix for #6506 This commit adapts code to deal with deferreds coming from calling requests from Twisted. Next step is just to change requests for twisted http client present in leap.common. Unfortunately, this last step will be a bit longer and would be better to have integrations tests to ensure current HTTP behaviour. --- src/leap/keymanager/__init__.py | 42 +++++++++++++++++++++++++---------------- 1 file changed, 26 insertions(+), 16 deletions(-) diff --git a/src/leap/keymanager/__init__.py b/src/leap/keymanager/__init__.py index c7886e0..06128c0 100644 --- a/src/leap/keymanager/__init__.py +++ b/src/leap/keymanager/__init__.py @@ -58,6 +58,7 @@ import logging import requests from twisted.internet import defer +from twisted.internet import threads from urlparse import urlparse from leap.common.check import leap_assert @@ -185,6 +186,7 @@ class KeyManager(object): lambda klass: klass.__name__ == ktype, self._wrapper_map).pop() + @defer.inlineCallbacks def _get(self, uri, data=None): """ Send a GET request to C{uri} containing C{data}. @@ -200,7 +202,8 @@ class KeyManager(object): leap_assert( self._ca_cert_path is not None, 'We need the CA certificate path!') - res = self._fetcher.get(uri, data=data, verify=self._ca_cert_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 @@ -211,7 +214,7 @@ class KeyManager(object): # leap_assert( # res.headers['content-type'].startswith('application/json'), # 'Content-type is not JSON.') - return res + defer.returnValue(res) def _get_with_combined_ca_bundle(self, uri, data=None): """ @@ -228,9 +231,11 @@ class KeyManager(object): :return: The response to the request. :rtype: requests.Response """ - return self._fetcher.get( - uri, data=data, verify=self._combined_ca_bundle) + return threads.deferToThread(self._fetcher.get, + uri, data=data, + verify=self._combined_ca_bundle) + @defer.inlineCallbacks def _put(self, uri, data=None): """ Send a PUT request to C{uri} containing C{data}. @@ -253,14 +258,17 @@ class KeyManager(object): leap_assert( self._token is not None, 'We need a token to interact with webapp!') - res = self._fetcher.put( - uri, data=data, verify=self._ca_cert_path, - headers={'Authorization': 'Token token=%s' % self._token}) + headers = {'Authorization': 'Token token=%s' % self._token} + res = yield threads.deferToThread(self._fetcher.put, + uri, data=data, + verify=self._ca_cert_path, + headers=headers) # assert that the response is valid res.raise_for_status() - return res + defer.returnValue(res) @memoized_method(invalidation=300) + @defer.inlineCallbacks def _fetch_keys_from_server(self, address): """ Fetch keys bound to address from nickserver and insert them in @@ -279,7 +287,7 @@ class KeyManager(object): d = defer.succeed(None) res = None try: - res = self._get(self._nickserver_uri, {'address': address}) + res = yield self._get(self._nickserver_uri, {'address': address}) res.raise_for_status() server_keys = res.json() @@ -307,7 +315,7 @@ class KeyManager(object): except Exception as e: d = defer.fail(KeyNotFound(e.message)) logger.warning("Error retrieving key: %r" % (e,)) - return d + yield d # # key management @@ -339,8 +347,9 @@ class KeyManager(object): self._api_uri, self._api_version, self._uid) - self._put(uri, data) + d = self._put(uri, data) emit_async(catalog.KEYMANAGER_DONE_UPLOADING_KEYS, self._address) + return d d = self.get_key( self._address, ktype, private=False, fetch_remote=False) @@ -822,6 +831,7 @@ class KeyManager(object): d.addCallback(lambda _: self.put_key(privkey, address)) return d + @defer.inlineCallbacks def fetch_key(self, address, uri, ktype, validation=ValidationLevels.Weak_Chain): """ @@ -852,20 +862,20 @@ class KeyManager(object): logger.info("Fetch key for %s from %s" % (address, uri)) try: - res = self._get_with_combined_ca_bundle(uri) + res = yield self._get_with_combined_ca_bundle(uri) except Exception as e: logger.warning("There was a problem fetching key: %s" % (e,)) - return defer.fail(KeyNotFound(uri)) + raise KeyNotFound(uri) if not res.ok: - return defer.fail(KeyNotFound(uri)) + raise KeyNotFound(uri) # XXX parse binary keys pubkey, _ = _keys.parse_ascii_key(res.content) if pubkey is None: - return defer.fail(KeyNotFound(uri)) + raise KeyNotFound(uri) pubkey.validation = validation - return self.put_key(pubkey, address) + yield self.put_key(pubkey, address) def _assert_supported_key_type(self, ktype): """ -- cgit v1.2.3 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 From cb32761d3dc97956085cd34a8f1e2e06432e8141 Mon Sep 17 00:00:00 2001 From: Victor Shyba Date: Wed, 27 Jan 2016 23:18:04 -0300 Subject: [feat] use HTTPClient instead of requests This commit adapts code to use HTTPClient instead of requests. requests library receives a certificate as parameter during requests while HTTPClient recelives a cert only on constructor. In order to have both types (leap cert and commercial certs) working together we introduced two clients on constructor. --- src/leap/keymanager/__init__.py | 63 +++++++++------- src/leap/keymanager/tests/test_keymanager.py | 109 +++++++++++---------------- 2 files changed, 82 insertions(+), 90 deletions(-) diff --git a/src/leap/keymanager/__init__.py b/src/leap/keymanager/__init__.py index 6413e9e..1dcf642 100644 --- a/src/leap/keymanager/__init__.py +++ b/src/leap/keymanager/__init__.py @@ -22,6 +22,8 @@ import fileinput import os import sys import tempfile +import json +import urllib from leap.common import ca_bundle @@ -58,10 +60,10 @@ import logging import requests from twisted.internet import defer -from twisted.internet import threads from urlparse import urlparse from leap.common.check import leap_assert +from leap.common.http import HTTPClient from leap.common.events import emit_async, catalog from leap.common.decorators import memoized_method @@ -142,6 +144,8 @@ class KeyManager(object): # the following are used to perform https requests self._fetcher = requests self._combined_ca_bundle = self._create_combined_bundle_file() + self._async_client = HTTPClient(self._combined_ca_bundle) + self._async_client_pinned = HTTPClient(self._ca_cert_path) # # destructor @@ -201,27 +205,29 @@ class KeyManager(object): self._ca_cert_path is not None, 'We need the CA certificate path!') 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) + uri = self._nickserver_uri + '?address=' + address + content = yield self._async_client_pinned.request(str(uri), 'GET') + json_content = json.loads(content) + except IOError as e: + # FIXME: 404 doesnt raise today, but it wont produce json anyway + # if e.response.status_code == 404: + # raise KeyNotFound(address) logger.warning("HTTP error retrieving key: %r" % (e,)) - logger.warning("%s" % (res.content,)) + logger.warning("%s" % (content,)) + raise KeyNotFound(e), None, sys.exc_info()[2] + except ValueError as v: + logger.warning("Invalid JSON data from key: %s" % (uri,)) + raise KeyNotFound(v.message + ' - ' + uri), None, sys.exc_info()[2] + except Exception as e: - raise KeyNotFound(e.message) logger.warning("Error retrieving key: %r" % (e,)) + raise KeyNotFound(e.message), None, sys.exc_info()[2] # 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.json()) + defer.returnValue(json_content) @defer.inlineCallbacks def _get_with_combined_ca_bundle(self, uri, data=None): @@ -240,14 +246,13 @@ class KeyManager(object): :rtype: Deferred """ try: - res = yield threads.deferToThread(self._fetcher.get, uri, - verify=self._combined_ca_bundle) + content = yield self._async_client.request(str(uri), 'GET') except Exception as e: logger.warning("There was a problem fetching key: %s" % (e,)) raise KeyNotFound(uri) - if not res.ok: + if not content: raise KeyNotFound(uri) - defer.returnValue(res.content) + defer.returnValue(content) @defer.inlineCallbacks def _put(self, uri, data=None): @@ -272,14 +277,20 @@ class KeyManager(object): leap_assert( self._token is not None, 'We need a token to interact with webapp!') - headers = {'Authorization': 'Token token=%s' % self._token} - res = yield threads.deferToThread(self._fetcher.put, - uri, data=data, - verify=self._ca_cert_path, - headers=headers) - # assert that the response is valid - res.raise_for_status() - defer.returnValue(res) + if type(data) == dict: + data = urllib.urlencode(data) + headers = {'Authorization': [str('Token token=%s' % self._token)]} + headers['Content-Type'] = ['application/x-www-form-urlencoded'] + try: + res = yield self._async_client_pinned.request(str(uri), 'PUT', body=str(data), headers=headers) + except Exception as e: + logger.warning("Error uploading key: %r" % (e,)) + raise e + if 'error' in res: + # FIXME: That's a workaround for 500, + # we need to implement a readBody to assert response code + logger.warning("Error uploading key: %r" % (res,)) + raise Exception(res) @memoized_method(invalidation=300) @defer.inlineCallbacks diff --git a/src/leap/keymanager/tests/test_keymanager.py b/src/leap/keymanager/tests/test_keymanager.py index afcbd5f..c3426b6 100644 --- a/src/leap/keymanager/tests/test_keymanager.py +++ b/src/leap/keymanager/tests/test_keymanager.py @@ -21,11 +21,14 @@ Tests for the Key Manager. """ from os import path +import json +import urllib from datetime import datetime import tempfile +import pkg_resources from leap.common import ca_bundle from mock import Mock, MagicMock, patch -from twisted.internet.defer import inlineCallbacks +from twisted.internet import defer from twisted.trial import unittest from leap.keymanager import ( @@ -127,7 +130,7 @@ class KeyManagerUtilTestCase(unittest.TestCase): class KeyManagerKeyManagementTestCase(KeyManagerWithSoledadTestCase): - @inlineCallbacks + @defer.inlineCallbacks def test_get_all_keys_in_db(self): km = self._key_manager() yield km._wrapper_map[OpenPGPKey].put_ascii_key(PRIVATE_KEY, ADDRESS) @@ -142,7 +145,7 @@ class KeyManagerKeyManagementTestCase(KeyManagerWithSoledadTestCase): self.assertTrue(ADDRESS in keys[0].address) self.assertTrue(keys[0].private) - @inlineCallbacks + @defer.inlineCallbacks def test_get_public_key(self): km = self._key_manager() yield km._wrapper_map[OpenPGPKey].put_ascii_key(PRIVATE_KEY, ADDRESS) @@ -155,7 +158,7 @@ class KeyManagerKeyManagementTestCase(KeyManagerWithSoledadTestCase): key.fingerprint.lower(), KEY_FINGERPRINT.lower()) self.assertFalse(key.private) - @inlineCallbacks + @defer.inlineCallbacks def test_get_private_key(self): km = self._key_manager() yield km._wrapper_map[OpenPGPKey].put_ascii_key(PRIVATE_KEY, ADDRESS) @@ -173,7 +176,7 @@ class KeyManagerKeyManagementTestCase(KeyManagerWithSoledadTestCase): d = km.send_key(OpenPGPKey) return self.assertFailure(d, KeyNotFound) - @inlineCallbacks + @defer.inlineCallbacks def test_send_key(self): """ Test that request is well formed when sending keys to server. @@ -181,7 +184,7 @@ class KeyManagerKeyManagementTestCase(KeyManagerWithSoledadTestCase): token = "mytoken" km = self._key_manager(token=token) yield km._wrapper_map[OpenPGPKey].put_ascii_key(PUBLIC_KEY, ADDRESS) - km._fetcher.put = Mock() + km._async_client_pinned.request = Mock(return_value=defer.succeed('')) # the following data will be used on the send km.ca_cert_path = 'capath' km.session_id = 'sessionid' @@ -191,13 +194,15 @@ class KeyManagerKeyManagementTestCase(KeyManagerWithSoledadTestCase): yield km.send_key(OpenPGPKey) # setup expected args pubkey = yield km.get_key(km._address, OpenPGPKey) - data = { + data = urllib.urlencode({ km.PUBKEY_KEY: pubkey.key_data, - } + }) + headers = {'Authorization': [str('Token token=%s' % token)]} + headers['Content-Type'] = ['application/x-www-form-urlencoded'] url = '%s/%s/users/%s.json' % ('apiuri', 'apiver', 'myuid') - km._fetcher.put.assert_called_once_with( - url, data=data, verify='capath', - headers={'Authorization': 'Token token=%s' % token}, + km._async_client_pinned.request.assert_called_once_with( + str(url), 'PUT', body=str(data), + headers=headers ) def test_fetch_keys_from_server(self): @@ -205,19 +210,19 @@ class KeyManagerKeyManagementTestCase(KeyManagerWithSoledadTestCase): Test that the request is well formed when fetching keys from server. """ km = self._key_manager(url=NICKSERVER_URI) + expected_url = NICKSERVER_URI + '?address=' + ADDRESS_2 def verify_the_call(_): - km._fetcher.get.assert_called_once_with( - NICKSERVER_URI, - data={'address': ADDRESS_2}, - verify='cacertpath', + km._async_client_pinned.request.assert_called_once_with( + expected_url, + 'GET', ) d = self._fetch_key(km, ADDRESS_2, PUBLIC_KEY_2) d.addCallback(verify_the_call) return d - @inlineCallbacks + @defer.inlineCallbacks def test_get_key_fetches_from_server(self): """ Test that getting a key successfuly fetches from server. @@ -229,7 +234,7 @@ class KeyManagerKeyManagementTestCase(KeyManagerWithSoledadTestCase): self.assertTrue(ADDRESS in key.address) self.assertEqual(key.validation, ValidationLevels.Provider_Trust) - @inlineCallbacks + @defer.inlineCallbacks def test_get_key_fetches_other_domain(self): """ Test that getting a key successfuly fetches from server. @@ -245,18 +250,10 @@ class KeyManagerKeyManagementTestCase(KeyManagerWithSoledadTestCase): """ :returns: a Deferred that will fire with the OpenPGPKey """ - class Response(object): - status_code = 200 - headers = {'content-type': 'application/json'} - - def json(self): - return {'address': address, 'openpgp': key} - - def raise_for_status(self): - pass + data = json.dumps({'address': address, 'openpgp': key}) # mock the fetcher so it returns the key for ADDRESS_2 - km._fetcher.get = Mock(return_value=Response()) + km._async_client_pinned.request = Mock(return_value=defer.succeed(data)) km.ca_cert_path = 'cacertpath' # try to key get without fetching from server d_fail = km.get_key(address, OpenPGPKey, fetch_remote=False) @@ -265,7 +262,7 @@ class KeyManagerKeyManagementTestCase(KeyManagerWithSoledadTestCase): d.addCallback(lambda _: km.get_key(address, OpenPGPKey)) return d - @inlineCallbacks + @defer.inlineCallbacks def test_put_key_ascii(self): """ Test that putting ascii key works @@ -277,7 +274,7 @@ class KeyManagerKeyManagementTestCase(KeyManagerWithSoledadTestCase): self.assertIsInstance(key, OpenPGPKey) self.assertTrue(ADDRESS in key.address) - @inlineCallbacks + @defer.inlineCallbacks def test_fetch_uri_ascii_key(self): """ Test that fetch key downloads the ascii key and gets included in @@ -285,11 +282,7 @@ class KeyManagerKeyManagementTestCase(KeyManagerWithSoledadTestCase): """ km = self._key_manager() - class Response(object): - ok = True - content = PUBLIC_KEY - - km._fetcher.get = Mock(return_value=Response()) + km._async_client.request = Mock(return_value=defer.succeed(PUBLIC_KEY)) yield km.fetch_key(ADDRESS, "http://site.domain/key", OpenPGPKey) key = yield km.get_key(ADDRESS, OpenPGPKey) @@ -316,25 +309,16 @@ class KeyManagerKeyManagementTestCase(KeyManagerWithSoledadTestCase): """ km = self._key_manager() - class Response(object): - ok = True - content = PUBLIC_KEY - - km._fetcher.get = Mock(return_value=Response()) + km._async_client.request = Mock(return_value=defer.succeed(PUBLIC_KEY)) d = km.fetch_key(ADDRESS_2, "http://site.domain/key", OpenPGPKey) return self.assertFailure(d, KeyAddressMismatch) def _mock_get_response(self, km, body): - class Response(object): - ok = True - content = body - - mock = MagicMock(return_value=Response()) - km._fetcher.get = mock + km._async_client.request = MagicMock(return_value=defer.succeed(body)) - return mock + return km._async_client.request - @inlineCallbacks + @defer.inlineCallbacks def test_fetch_key_uses_ca_bundle_if_none_specified(self): ca_cert_path = None km = self._key_manager(ca_cert_path=ca_cert_path) @@ -342,10 +326,9 @@ class KeyManagerKeyManagementTestCase(KeyManagerWithSoledadTestCase): yield km.fetch_key(ADDRESS_OTHER, REMOTE_KEY_URL, OpenPGPKey) - get_mock.assert_called_once_with(REMOTE_KEY_URL, - verify=ca_bundle.where()) + get_mock.assert_called_once_with(REMOTE_KEY_URL, 'GET') - @inlineCallbacks + @defer.inlineCallbacks def test_fetch_key_uses_ca_bundle_if_empty_string_specified(self): ca_cert_path = '' km = self._key_manager(ca_cert_path=ca_cert_path) @@ -353,10 +336,9 @@ class KeyManagerKeyManagementTestCase(KeyManagerWithSoledadTestCase): yield km.fetch_key(ADDRESS_OTHER, REMOTE_KEY_URL, OpenPGPKey) - get_mock.assert_called_once_with(REMOTE_KEY_URL, - verify=ca_bundle.where()) + get_mock.assert_called_once_with(REMOTE_KEY_URL, 'GET') - @inlineCallbacks + @defer.inlineCallbacks def test_fetch_key_use_default_ca_bundle_if_set_as_ca_cert_path(self): ca_cert_path = ca_bundle.where() km = self._key_manager(ca_cert_path=ca_cert_path) @@ -364,14 +346,14 @@ class KeyManagerKeyManagementTestCase(KeyManagerWithSoledadTestCase): yield km.fetch_key(ADDRESS_OTHER, REMOTE_KEY_URL, OpenPGPKey) - get_mock.assert_called_once_with(REMOTE_KEY_URL, - verify=ca_bundle.where()) + get_mock.assert_called_once_with(REMOTE_KEY_URL, 'GET') - @inlineCallbacks + @defer.inlineCallbacks def test_fetch_uses_combined_ca_bundle_otherwise(self): with tempfile.NamedTemporaryFile() as tmp_input, \ tempfile.NamedTemporaryFile(delete=False) as tmp_output: - ca_content = 'some\ncontent\n' + ca_content = pkg_resources.resource_string('leap.common.testing', + 'cacert.pem') ca_cert_path = tmp_input.name self._dump_to_file(ca_cert_path, ca_content) @@ -383,8 +365,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, - verify=tmp_output.name) + get_mock.assert_called_once_with(REMOTE_KEY_URL, 'GET') # assert that files got appended expected = self._slurp_file(ca_bundle.where()) + ca_content @@ -402,7 +383,7 @@ class KeyManagerKeyManagementTestCase(KeyManagerWithSoledadTestCase): content = f.read() return content - @inlineCallbacks + @defer.inlineCallbacks def test_decrypt_updates_sign_used_for_signer(self): # given km = self._key_manager() @@ -420,7 +401,7 @@ class KeyManagerKeyManagementTestCase(KeyManagerWithSoledadTestCase): # then self.assertEqual(True, key.sign_used) - @inlineCallbacks + @defer.inlineCallbacks def test_decrypt_does_not_update_sign_used_for_recipient(self): # given km = self._key_manager() @@ -445,7 +426,7 @@ class KeyManagerCryptoTestCase(KeyManagerWithSoledadTestCase): RAW_DATA = 'data' - @inlineCallbacks + @defer.inlineCallbacks def test_keymanager_openpgp_encrypt_decrypt(self): km = self._key_manager() # put raw private key @@ -464,7 +445,7 @@ class KeyManagerCryptoTestCase(KeyManagerWithSoledadTestCase): fetch_remote=False) self.assertEqual(signingkey.fingerprint, key.fingerprint) - @inlineCallbacks + @defer.inlineCallbacks def test_keymanager_openpgp_encrypt_decrypt_wrong_sign(self): km = self._key_manager() # put raw keys @@ -481,7 +462,7 @@ class KeyManagerCryptoTestCase(KeyManagerWithSoledadTestCase): self.assertEqual(self.RAW_DATA, rawdata) self.assertTrue(isinstance(signingkey, errors.InvalidSignature)) - @inlineCallbacks + @defer.inlineCallbacks def test_keymanager_openpgp_sign_verify(self): km = self._key_manager() # put raw private keys -- cgit v1.2.3 From 00c0ed0d1b14aff8be0172e03bf26e1c30477af6 Mon Sep 17 00:00:00 2001 From: Victor Shyba Date: Tue, 2 Feb 2016 19:51:36 -0300 Subject: [docs] add docstrings and fixes pep8 Some methods were missing docstrings and some code was exceeding the 80 column limit. Also some asserts arent needed anymore. --- src/leap/keymanager/__init__.py | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/src/leap/keymanager/__init__.py b/src/leap/keymanager/__init__.py index 1dcf642..7e4d30e 100644 --- a/src/leap/keymanager/__init__.py +++ b/src/leap/keymanager/__init__.py @@ -184,26 +184,29 @@ class KeyManager(object): def _key_class_from_type(self, ktype): """ - Return key class from string representation of key type. + Given a class type, return a class + + :param ktype: string representation of a class name + :type ktype: str + + :return: A class with the matching name + :rtype: classobj or type """ return filter( lambda klass: klass.__name__ == ktype, self._wrapper_map).pop() @defer.inlineCallbacks - def _get_json(self, address): + def _get_key_from_nicknym(self, address): """ Send a GET request to C{uri} containing C{data}. - :param uri: The URI of the request. - :type uri: str + :param address: The URI of the request. + :type address: str - :return: A deferred that will be fired with the GET response + :return: A deferred that will be fired with GET content as json (dict) :rtype: Deferred """ - leap_assert( - self._ca_cert_path is not None, - 'We need the CA certificate path!') try: uri = self._nickserver_uri + '?address=' + address content = yield self._async_client_pinned.request(str(uri), 'GET') @@ -214,7 +217,7 @@ class KeyManager(object): # raise KeyNotFound(address) logger.warning("HTTP error retrieving key: %r" % (e,)) logger.warning("%s" % (content,)) - raise KeyNotFound(e), None, sys.exc_info()[2] + raise KeyNotFound(e.message), None, sys.exc_info()[2] except ValueError as v: logger.warning("Invalid JSON data from key: %s" % (uri,)) raise KeyNotFound(v.message + ' - ' + uri), None, sys.exc_info()[2] @@ -271,9 +274,6 @@ class KeyManager(object): :return: A deferred that will be fired when PUT request finishes :rtype: Deferred """ - leap_assert( - self._ca_cert_path is not None, - 'We need the CA certificate path!') leap_assert( self._token is not None, 'We need a token to interact with webapp!') @@ -282,7 +282,9 @@ class KeyManager(object): headers = {'Authorization': [str('Token token=%s' % self._token)]} headers['Content-Type'] = ['application/x-www-form-urlencoded'] try: - res = yield self._async_client_pinned.request(str(uri), 'PUT', body=str(data), headers=headers) + res = yield self._async_client_pinned.request(str(uri), 'PUT', + body=str(data), + headers=headers) except Exception as e: logger.warning("Error uploading key: %r" % (e,)) raise e @@ -309,7 +311,7 @@ class KeyManager(object): """ # request keys from the nickserver - server_keys = yield self._get_json(address) + server_keys = yield self._get_key_from_nicknym(address) # insert keys in local database if self.OPENPGP_KEY in server_keys: @@ -357,7 +359,9 @@ class KeyManager(object): self._api_version, self._uid) d = self._put(uri, data) - emit_async(catalog.KEYMANAGER_DONE_UPLOADING_KEYS, self._address) + d.addCallback(lambda _: + emit_async(catalog.KEYMANAGER_DONE_UPLOADING_KEYS, + self._address)) return d d = self.get_key( -- cgit v1.2.3