From c6b8b3ce8cd96cca8332b6cc0f2fa7bf9b6d16eb Mon Sep 17 00:00:00 2001
From: Jimmy Zelinskie <jimmyzelinskie@gmail.com>
Date: Wed, 30 Mar 2016 13:20:35 -0400
Subject: [PATCH] service_keys: s/get_keys/list_keys

---
 data/model/service_keys.py | 14 +++++++-------
 endpoints/api/superuser.py |  6 ++++--
 endpoints/key_server.py    | 21 +++++++++++++--------
 3 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/data/model/service_keys.py b/data/model/service_keys.py
index a07526e53..82a3cdcb6 100644
--- a/data/model/service_keys.py
+++ b/data/model/service_keys.py
@@ -110,7 +110,7 @@ def approve_service_key(kid, approver, approval_type):
   _gc_expired(key.service)
 
 
-def _get_service_keys_query(kid=None, service=None, approved_only=False):
+def _list_service_keys_query(kid=None, service=None, approved_only=False):
   query = ServiceKey.select()
 
   if approved_only:
@@ -127,13 +127,13 @@ def _get_service_keys_query(kid=None, service=None, approved_only=False):
   return query
 
 
-def get_keys():
-  return list(_get_service_keys_query())
+def list_keys():
+  return list(_list_service_keys_query())
 
 
-def get_service_keys(service):
-  return list(_get_service_keys_query(service=service, approved_only=True))
+def list_service_keys(service):
+  return list(_list_service_keys_query(service=service, approved_only=True))
 
 
-def get_service_key(kid):
-  return _get_service_keys_query(kid=kid).get()
+def get_service_key(kid, service=None):
+  return _list_service_keys_query(kid=kid, service=service).get()
diff --git a/endpoints/api/superuser.py b/endpoints/api/superuser.py
index da54bbd48..065d17709 100644
--- a/endpoints/api/superuser.py
+++ b/endpoints/api/superuser.py
@@ -514,7 +514,7 @@ class SuperUserServiceKeyManagement(ApiResource):
   @require_scope(scopes.SUPERUSER)
   def get(self):
     if SuperUserPermission().can():
-      return jsonify(model.service_keys.get_keys())
+      return jsonify(model.service_keys.list_keys())
     abort(403)
 
   @verify_not_prod
@@ -526,7 +526,9 @@ class SuperUserServiceKeyManagement(ApiResource):
       body = request.get_json()
 
       expiration_date = body.get('expiration', None)
-      if expiration_date is not None and expiration_date != '':
+      if expiration_date == '':
+        expiration_date = None
+      if expiration_date is not None:
         try:
           expiration_date = datetime.utcfromtimestamp(float(expiration_date))
         except ValueError:
diff --git a/endpoints/key_server.py b/endpoints/key_server.py
index 452b280d0..451d8ed15 100644
--- a/endpoints/key_server.py
+++ b/endpoints/key_server.py
@@ -51,16 +51,16 @@ def _signer_kid(encoded_jwt):
   return decoded_jwt.get('signer_kid', None)
 
 
-def _signer_key(signer_kid):
+def _signer_key(service, signer_kid):
   try:
-    return data.model.service_keys.get_service_key(signer_kid)
+    return data.model.service_keys.get_service_key(signer_kid, service=service)
   except data.model.ServiceKeyDoesNotExist:
     abort(403)
 
 
 @key_server.route('/services/<service>/keys', methods=['GET'])
-def get_service_keys(service):
-  keys = data.model.service_keys.get_service_keys(service)
+def list_service_keys(service):
+  keys = data.model.service_keys.list_service_keys(service)
   return jsonify({'keys': [key.jwk for key in keys]})
 
 
@@ -100,14 +100,14 @@ def put_service_keys(service, kid):
   metadata = {'ip': request.remote_addr}
   signer_kid = _signer_kid(encoded_jwt)
 
-  if kid == signer_kid:
+  if kid == signer_kid or signer_kid == '':
     # The key is self-signed. Create a new instance and await approval.
     _validate_jwt(encoded_jwt, jwk, service)
     data.model.service_keys.create_service_key('', kid, service, jwk, metadata, expiration_date)
     return make_response('', 202)
 
   metadata.update({'created_by': 'Key Rotation'})
-  signer_key = _signer_key(signer_kid)
+  signer_key = _signer_key(service, signer_kid)
   signer_jwk = signer_key.jwk
   if signer_key.service != service:
     abort(403)
@@ -129,14 +129,19 @@ def delete_service_key(service, kid):
     abort(400)
 
   signer_kid = _signer_kid(encoded_jwt)
-  signer_key = _signer_key(signer_kid)
+  signer_key = _signer_key(service, signer_kid)
 
-  if (kid == signer_kid) or (signer_key.approval is not None):
+  self_signed = kid == signer_kid or signer_kid == ''
+  approved_key_for_service = signer_key.approval is not None
+
+  if self_signed or approved_key_for_service:
     _validate_jwt(encoded_jwt, signer_key.jwk, service)
+
     try:
       data.model.service_keys.delete_service_key(service, kid)
     except data.model.ServiceKeyDoesNotExist:
       abort(404)
+
     return make_response('', 200)
 
   abort(403)