launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:py3-signing-proxy into launchpad:master.

Commit message:
lp.services.signing.proxy: Fix up bytes vs. text for Python 3

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/399081

base64 operates on bytes while json generally operates on text, so a good deal of explicit encoding and decoding is needed.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:py3-signing-proxy into launchpad:master.

diff --git a/lib/lp/services/signing/proxy.py b/lib/lp/services/signing/proxy.py
index 8d78e65..2cb5393 100644
--- a/lib/lp/services/signing/proxy.py
+++ b/lib/lp/services/signing/proxy.py
@@ -64,7 +64,8 @@ class SigningServiceClient:
     def _decryptResponseJson(self, response, response_nonce):
         box = Box(self.private_key, self.service_public_key)
         return json.loads(box.decrypt(
-            response.content, response_nonce, encoder=Base64Encoder))
+            response.content, response_nonce,
+            encoder=Base64Encoder).decode("UTF-8"))
 
     def _requestJson(self, path, method="GET", encrypt=False, **kwargs):
         """Helper method to do an HTTP request and get back a json from  the
@@ -138,8 +139,9 @@ class SigningServiceClient:
         return {
             "Content-Type": "application/x-boxed-json",
             "X-Client-Public-Key": config.signing.client_public_key,
-            "X-Nonce": base64.b64encode(nonce),
-            "X-Response-Nonce": base64.b64encode(response_nonce),
+            "X-Nonce": base64.b64encode(nonce).decode("UTF-8"),
+            "X-Response-Nonce": (
+                base64.b64encode(response_nonce).decode("UTF-8")),
             }
 
     def _encryptPayload(self, nonce, message):
@@ -178,7 +180,8 @@ class SigningServiceClient:
             "/generate", "POST", encrypt=True, json=payload)
         return {
             "fingerprint": ret["fingerprint"],
-            "public-key": base64.b64decode(ret["public-key"])}
+            "public-key": base64.b64decode(ret["public-key"].encode("UTF-8")),
+            }
 
     def sign(self, key_type, fingerprint, message_name, message, mode):
         valid_modes = {SigningMode.ATTACHED, SigningMode.DETACHED}
@@ -199,8 +202,10 @@ class SigningServiceClient:
 
         ret = self._requestJson("/sign", "POST", encrypt=True, json=payload)
         return {
-            "public-key": base64.b64decode(ret["public-key"]),
-            "signed-message": base64.b64decode(ret["signed-message"])}
+            "public-key": base64.b64decode(ret["public-key"].encode("UTF-8")),
+            "signed-message": base64.b64decode(
+                ret["signed-message"].encode("UTF-8")),
+            }
 
     def inject(self, key_type, private_key, public_key, description,
                created_at):
diff --git a/lib/lp/services/signing/tests/test_proxy.py b/lib/lp/services/signing/tests/test_proxy.py
index 4405fe7..5f7fe7e 100644
--- a/lib/lp/services/signing/tests/test_proxy.py
+++ b/lib/lp/services/signing/tests/test_proxy.py
@@ -69,11 +69,11 @@ class SigningServiceResponseFactory:
 
         self.generated_public_key = PrivateKey.generate().public_key
         self.b64_generated_public_key = base64.b64encode(
-            bytes(self.generated_public_key))
+            bytes(self.generated_public_key)).decode("UTF-8")
         self.generated_fingerprint = (
             u'338D218488DFD597D8FCB9C328C3E9D9ADA16CEE')
 
-        self.signed_msg_template = "%s::signed!"
+        self.signed_msg_template = b"%d::signed!"
 
     @classmethod
     def getUrl(cls, path):
@@ -86,7 +86,8 @@ class SigningServiceResponseFactory:
         lp-signing would do."""
         box = Box(self.service_private_key, self.client_public_key)
         return box.encrypt(
-            json.dumps(data), nonce, encoder=Base64Encoder).ciphertext
+            json.dumps(data).encode("UTF-8"), nonce,
+            encoder=Base64Encoder).ciphertext
 
     def _decryptPayload(self, value):
         """Decrypt a payload we encrypted.
@@ -137,18 +138,18 @@ class SigningServiceResponseFactory:
 
         responses.add(
             responses.GET, self.getUrl("/service-key"),
-            json={"service-key": self.b64_service_public_key.decode('utf8')},
+            json={"service-key": self.b64_service_public_key},
             status=200)
 
         responses.add(
             responses.POST, self.getUrl("/nonce"),
-            json={"nonce": self.b64_nonce.decode('utf8')}, status=201)
+            json={"nonce": self.b64_nonce}, status=201)
 
         responses.add(
             responses.POST, self.getUrl("/generate"),
             body=self._encryptPayload({
                 'fingerprint': self.generated_fingerprint,
-                'public-key': self.b64_generated_public_key.decode('utf8')
+                'public-key': self.b64_generated_public_key,
             }, nonce=self.response_nonce),
             status=201)
 
@@ -169,9 +170,9 @@ class SigningServiceResponseFactory:
         def sign_callback(request):
             call_counts['/sign'] += 1
             signed_msg = self.signed_msg_template % call_counts['/sign']
-            signed = base64.b64encode(signed_msg.encode('utf8'))
+            signed = base64.b64encode(signed_msg)
             data = {'signed-message': signed.decode('utf8'),
-                    'public-key': self.b64_generated_public_key.decode('utf8')}
+                    'public-key': self.b64_generated_public_key}
             return 201, {}, self._encryptPayload(data, self.response_nonce)
 
         responses.add_callback(
@@ -222,7 +223,7 @@ class SigningServiceProxyTest(TestCaseWithFactory, TestWithFixtures):
         self.assertIsInstance(key, PublicKey)
         self.assertEqual(
             key.encode(Base64Encoder),
-            self.response_factory.b64_service_public_key)
+            self.response_factory.b64_service_public_key.encode("UTF-8"))
 
         # Checks that the HTTP call was made
         self.assertEqual(1, len(responses.calls))
@@ -241,7 +242,8 @@ class SigningServiceProxyTest(TestCaseWithFactory, TestWithFixtures):
         nonce = signing.getNonce()
 
         self.assertEqual(
-            base64.b64encode(nonce), self.response_factory.b64_nonce)
+            base64.b64encode(nonce),
+            self.response_factory.b64_nonce.encode("UTF-8"))
 
         # Checks that the HTTP call was made
         self.assertEqual(1, len(responses.calls))
@@ -422,7 +424,7 @@ class SigningServiceProxyTest(TestCaseWithFactory, TestWithFixtures):
         key_type = SigningKeyType.KMOD
         mode = SigningMode.DETACHED
         message_name = 'my test msg'
-        message = 'this is the message content'
+        message = b'this is the message content'
 
         signing = getUtility(ISigningServiceClient)
         data = signing.sign(
diff --git a/lib/lp/services/signing/tests/test_signingkey.py b/lib/lp/services/signing/tests/test_signingkey.py
index 00714f9..03e0a74 100644
--- a/lib/lp/services/signing/tests/test_signingkey.py
+++ b/lib/lp/services/signing/tests/test_signingkey.py
@@ -88,7 +88,7 @@ class TestSigningKey(TestCaseWithFactory, TestWithFixtures):
         self.assertEqual(
             self.signing_service.generated_fingerprint, db_key.fingerprint)
         self.assertEqual(
-            self.signing_service.b64_generated_public_key,
+            self.signing_service.b64_generated_public_key.encode("UTF-8"),
             base64.b64encode(db_key.public_key))
         self.assertEqual("this is my key", db_key.description)