Merge branch 'feat/defer_requests' into develop

2 files changed, 138 insertions, 140 deletions

diff --git a/src/leap/keymanager/__init__.py b/src/leap/keymanager/__init__.py
index c7886e0..7e4d30e 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
 
@@ -61,6 +63,7 @@ from twisted.internet import defer
 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
 
@@ -141,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
@@ -179,40 +184,55 @@ 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()
 
-    def _get(self, uri, data=None):
+    @defer.inlineCallbacks
+    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 data: The body of the request.
-        :type data: dict, str or file
+        :param address: The URI of the request.
+        :type address: str
 
-        :return: The response to the request.
-        :rtype: requests.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!')
-        res = 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 = 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" % (content,))
+            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]
 
+        except Exception as e:
+            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.')
-        return res
+        defer.returnValue(json_content)
 
+    @defer.inlineCallbacks
     def _get_with_combined_ca_bundle(self, uri, data=None):
         """
         Send a GET request to C{uri} containing C{data}.
@@ -225,12 +245,19 @@ 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 self._fetcher.get(
-            uri, data=data, verify=self._combined_ca_bundle)
+        try:
+            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 content:
+            raise KeyNotFound(uri)
+        defer.returnValue(content)
 
+    @defer.inlineCallbacks
     def _put(self, uri, data=None):
         """
         Send a PUT request to C{uri} containing C{data}.
@@ -244,23 +271,31 @@ 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,
-            'We need the CA certificate path!')
-        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})
-        # assert that the response is valid
-        res.raise_for_status()
-        return 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
     def _fetch_keys_from_server(self, address):
         """
         Fetch keys bound to address from nickserver and insert them in
@@ -276,38 +311,22 @@ class KeyManager(object):
 
         """
         # request keys from the nickserver
-        d = defer.succeed(None)
-        res = None
-        try:
-            res = 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,))
-        return d
+        server_keys = yield self._get_key_from_nicknym(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
@@ -339,8 +358,11 @@ class KeyManager(object):
                 self._api_uri,
                 self._api_version,
                 self._uid)
-            self._put(uri, data)
-            emit_async(catalog.KEYMANAGER_DONE_UPLOADING_KEYS, self._address)
+            d = self._put(uri, data)
+            d.addCallback(lambda _:
+                          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 +844,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):
         """
@@ -851,21 +874,15 @@ class KeyManager(object):
         _keys = self._wrapper_map[ktype]
 
         logger.info("Fetch key for %s from %s" % (address, uri))
-        try:
-            res = 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))
-        if not res.ok:
-            return defer.fail(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:
-            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):
         """
diff --git a/src/leap/keymanager/tests/test_keymanager.py b/src/leap/keymanager/tests/test_keymanager.py
index 856d6da..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, data=None,
-                                         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, data=None,
-                                         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, data=None,
-                                         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, data=None,
-                                                 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