[bug] make nicknym return key data

There was a mismatch on the return values of Nicknym class and the
expectations. As most of the code was expecting an OpenPGPKey, but it
was returning the json as it came from the nickserver. Now is just
returning the key data.

5 files changed, 41 insertions, 51 deletions

diff --git a/src/leap/bitmask/core/mail_services.py b/src/leap/bitmask/core/mail_services.py
index 5337b313..31c27364 100644
--- a/src/leap/bitmask/core/mail_services.py
+++ b/src/leap/bitmask/core/mail_services.py
@@ -298,10 +298,9 @@ class KeymanagerContainer(Container):
         try:
             remote = yield keymanager._nicknym.fetch_key_with_address(userid)
         except Exception:
-            remote = {}
+            remote = ""
 
-        if (keymanager.OPENPGP_KEY not in remote or
-                key.key_data != remote[KeyManager.OPENPGP_KEY]):
+        if key.key_data != remote:
             yield keymanager.send_key()
 
     def _set_status(self, address, status, error=None, keys=None):
diff --git a/src/leap/bitmask/keymanager/__init__.py b/src/leap/bitmask/keymanager/__init__.py
index bc0c2185..45b7e582 100644
--- a/src/leap/bitmask/keymanager/__init__.py
+++ b/src/leap/bitmask/keymanager/__init__.py
@@ -18,16 +18,12 @@
 Key Manager is a Nicknym agent for LEAP client.
 """
 import fileinput
-import json
-import sys
 import tempfile
 
 from urlparse import urlparse
 
 from twisted.logger import Logger
 from twisted.internet import defer, task, reactor
-from twisted.web import client
-from twisted.web._responses import NOT_FOUND
 
 from leap.common import ca_bundle
 from leap.common.http import HTTPClient
@@ -49,9 +45,6 @@ class KeyManager(object):
 
     log = Logger()
 
-    OPENPGP_KEY = 'openpgp'
-    PUBKEY_KEY = "user[public_key]"
-
     def __init__(self, address, nickserver_uri, soledad, token=None,
                  ca_cert_path=None, api_uri=None, api_version=None, uid=None,
                  gpgbinary=None, combined_ca_bundle=None):
@@ -203,23 +196,17 @@ class KeyManager(object):
         :rtype: Deferred
 
         """
-        server_keys = yield self._nicknym.fetch_key_with_address(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[self.OPENPGP_KEY],
-                address=address,
-                validation=validation_level)
-        else:
-            raise KeyNotFound("No openpgp key found")
+        raw_key = yield self._nicknym.fetch_key_with_address(address)
+
+        # 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(
+            raw_key, address=address, validation=validation_level)
 
     def get_key(self, address, private=False, fetch_remote=True):
         """
diff --git a/src/leap/bitmask/keymanager/nicknym.py b/src/leap/bitmask/keymanager/nicknym.py
index cbc4e25d..5342a7e4 100644
--- a/src/leap/bitmask/keymanager/nicknym.py
+++ b/src/leap/bitmask/keymanager/nicknym.py
@@ -38,6 +38,7 @@ class Nicknym(object):
 
     log = Logger()
 
+    OPENPGP_KEY = 'openpgp'
     PUBKEY_KEY = "user[public_key]"
 
     def __init__(self, nickserver_uri, ca_cert_path, token):
@@ -99,12 +100,13 @@ class Nicknym(object):
         :param uri: The URI of the request.
         :type uri: str
 
-        :return: A deferred that will be fired with GET content as json (dict)
+        :return: A deferred that will be fired with GET content as raw key str
         :rtype: Deferred
         """
         try:
             content = yield self._fetch_and_handle_404_from_nicknym(uri)
             json_content = json.loads(content)
+            key = json_content[self.OPENPGP_KEY]
         except KeyNotFound:
             raise
         except IOError as e:
@@ -114,15 +116,12 @@ class Nicknym(object):
         except ValueError as v:
             self.log.warn('Invalid JSON data from key: %s' % (uri,))
             raise KeyNotFound(v.message + ' - ' + uri), None, sys.exc_info()[2]
+        except KeyError:
+            raise KeyNotFound("No openpgp key found")
         except Exception as e:
             self.log.warn('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(json_content)
+        defer.returnValue(key)
 
     def _fetch_and_handle_404_from_nicknym(self, uri):
         """
@@ -166,9 +165,8 @@ class Nicknym(object):
         :param address: The address bound to the keys.
         :type address: str
 
-        :return: A Deferred which fires when the key is in the storage,
-                 or which fails with KeyNotFound if the key was not found on
-                 nickserver.
+        :return: A Deferred with the raw key, or which fails with KeyNotFound
+                 if the key was not found on nickserver.
         :rtype: Deferred
 
         """
@@ -183,9 +181,8 @@ class Nicknym(object):
         :param fingerprint: The fingerprint bound to the keys.
         :type fingerprint: str
 
-        :return: A Deferred which fires when the key is in the storage,
-                 or which fails with KeyNotFound if the key was not found on
-                 nickserver.
+        :return: A Deferred with the raw key, or which fails with KeyNotFound
+                 if the key was not found on nickserver.
         :rtype: Deferred
 
         """
diff --git a/src/leap/bitmask/keymanager/refresher.py b/src/leap/bitmask/keymanager/refresher.py
index d89a7508..9ccc81e1 100644
--- a/src/leap/bitmask/keymanager/refresher.py
+++ b/src/leap/bitmask/keymanager/refresher.py
@@ -106,17 +106,24 @@ class RandomRefreshPublicKey(object):
         if old_key is None:
             defer.returnValue(None)
 
-        old_updated_key = yield self._keymanger._nicknym.\
+        updated_key_data = yield self._keymanger._nicknym.\
             fetch_key_with_fingerprint(old_key.fingerprint)
+        updated_key, _ = self._openpgp.parse_key(updated_key_data,
+                                                 old_key.address)
 
-        if old_updated_key.fingerprint != old_key.fingerprint:
+        if updated_key.fingerprint != old_key.fingerprint:
             self.log.error(
                 ERROR_UNEQUAL_FINGERPRINTS % (
-                    old_key.fingerprint, old_updated_key.fingerprint))
+                    old_key.fingerprint, updated_key.fingerprint))
             defer.returnValue(None)
 
-        yield self._maybe_unactivate_key(old_updated_key)
-        yield self._openpgp.put_key(old_updated_key)
+        updated_key.validation = old_key.validation
+        updated_key.last_audited_at = old_key.last_audited_at
+        updated_key.encr_used = old_key.encr_used
+        updated_key.sign_used = old_key.sign_used
+
+        yield self._maybe_unactivate_key(updated_key)
+        yield self._openpgp.put_key(updated_key)
 
         # No new fetch by address needed, bc that will happen before sending an
         # email could be discussed since fetching before sending an email
diff --git a/tests/integration/keymanager/test_refresher.py b/tests/integration/keymanager/test_refresher.py
index 24ad4b05..47a27b4b 100644
--- a/tests/integration/keymanager/test_refresher.py
+++ b/tests/integration/keymanager/test_refresher.py
@@ -32,7 +32,7 @@ from leap.bitmask.keymanager.refresher import RandomRefreshPublicKey, \
     ERROR_UNEQUAL_FINGERPRINTS
 from leap.bitmask.keymanager.testing import KeyManagerWithSoledadTestCase
 
-from common import KEY_FINGERPRINT
+from common import KEY_FINGERPRINT, PUBLIC_KEY_2, KEY_FINGERPRINT_2
 
 ANOTHER_FP = 'ANOTHERFINGERPRINT'
 
@@ -76,13 +76,13 @@ class RandomRefreshPublicKeyTestCase(KeyManagerWithSoledadTestCase):
                     fingerprint=KEY_FINGERPRINT)))
 
             km._nicknym.fetch_key_with_fingerprint = \
-                Mock(return_value=defer.succeed(OpenPGPKey(
-                    fingerprint=ANOTHER_FP)))
+                Mock(return_value=defer.succeed(PUBLIC_KEY_2))
 
             yield rf.maybe_refresh_key()
 
-            mock_logger_error.assert_called_with(ERROR_UNEQUAL_FINGERPRINTS %
-                                                 (KEY_FINGERPRINT, ANOTHER_FP))
+            mock_logger_error.assert_called_with(
+                ERROR_UNEQUAL_FINGERPRINTS %
+                (KEY_FINGERPRINT, KEY_FINGERPRINT_2))
 
     @defer.inlineCallbacks
     def test_put_new_key_in_local_storage(self):
@@ -95,7 +95,7 @@ class RandomRefreshPublicKeyTestCase(KeyManagerWithSoledadTestCase):
             OpenPGPKey(fingerprint=KEY_FINGERPRINT)))
 
         km._nicknym.fetch_key_with_fingerprint = Mock(
-            return_value=defer.succeed(OpenPGPKey(fingerprint=ANOTHER_FP)))
+            return_value=defer.succeed(PUBLIC_KEY_2))
 
         yield rf.maybe_refresh_key()