Various fixes from PR review

apps/cli/pisa_cli.py

@@ -63,7 +63,7 @@ def is_appointment_signature_valid(appointment, signature, pk):
     try:
         sig_bytes = unhexlify(signature.encode('utf-8'))
         data = json.dumps(appointment, sort_keys=True, separators=(',', ':')).encode("utf-8")
-        pisa_public_key.verify(sig_bytes, data, ec.ECDSA(hashes.SHA256()))
+        pk.verify(sig_bytes, data, ec.ECDSA(hashes.SHA256()))
         return True
     except InvalidSignature:
         return False
@@ -72,15 +72,11 @@ def is_appointment_signature_valid(appointment, signature, pk):
 # Makes sure that the folder APPOINTMENTS_FOLDER_NAME exists, then saves the appointment and signature in it.
 def save_signed_appointment(appointment, signature):
     # Create the appointments directory if it doesn't already exist
-    try:
-        os.makedirs(APPOINTMENTS_FOLDER_NAME)
-    except FileExistsError:
-        # directory already exists, this is fine
-        pass
+    os.makedirs(APPOINTMENTS_FOLDER_NAME, exist_ok=True)
 
-    timestamp = int(time.time()*1000)
+    timestamp = int(time.time())
     locator = appointment['locator']
-    uuid = uuid4()  # prevent filename collisions
+    uuid = uuid4().hex  # prevent filename collisions
     filename = "{}/appointment-{}-{}-{}.json".format(APPOINTMENTS_FOLDER_NAME, timestamp, locator, uuid)
     data = {"appointment": appointment, "signature": signature}
 
@@ -110,7 +106,7 @@ def add_appointment(args):
 
             try:
                 with open(fin) as f:
-                    appointment_data = json.load(open(fin))
+                    appointment_data = json.load(f)
             except IOError as e:
                 logger.error("I/O error({}): {}".format(e.errno, e.strerror))
                 return False

test/apps/cli/test_pisa_cli.py

@@ -15,6 +15,7 @@ from test.unit.conftest import get_random_value_hex
 # TODO: should find a way of doing without this
 from apps.cli.pisa_cli import build_appointment
 
+# dummy keys for the tests
 pisa_sk = ec.generate_private_key(ec.SECP256K1, default_backend())
 pisa_pk = pisa_sk.public_key()
 
@@ -52,11 +53,18 @@ def test_is_appointment_signature_valid():
     assert not pisa_cli.is_appointment_signature_valid(dummy_appointment, other_signature, pisa_pk)
 
 
+def get_dummy_pisa_pk():
+    return pisa_pk
+
+
 @responses.activate
-def test_add_appointment():
+def test_add_appointment(monkeypatch):
     # Simulate a request to add_appointment for dummy_appointment, make sure that the right endpoint is requested
     # and the return value is True
 
+    # make sure the test uses the right dummy key instead of loading it from disk
+    monkeypatch.setattr(pisa_cli, "load_pisa_public_key", get_dummy_pisa_pk)
+
     response = {"locator": dummy_appointment["locator"], "signature": sign_appointment(pisa_sk, dummy_appointment)}
 
     request_url = "http://{}/".format(pisa_endpoint)
@@ -71,9 +79,13 @@ def test_add_appointment():
 
 
 @responses.activate
-def test_add_appointment_with_invalid_signature():
+def test_add_appointment_with_invalid_signature(monkeypatch):
     # Simulate a request to add_appointment for dummy_appointment, but sign with a different key,
     # make sure that the right endpoint is requested, but the return value is False
+
+    # make sure the test uses the right dummy key instead of loading it from disk
+    monkeypatch.setattr(pisa_cli, "load_pisa_public_key", get_dummy_pisa_pk)
+
     response = {
         "locator": dummy_appointment["locator"],
         "signature": sign_appointment(other_sk, dummy_appointment),  # signing with a different key