From 885a41e6f558eabf6854b24f013cff434cdbfef4 Mon Sep 17 00:00:00 2001
From: Jimmy Zelinskie <jimmyzelinskie@gmail.com>
Date: Fri, 1 Apr 2016 15:48:31 -0400
Subject: [PATCH] key server: misc fixes to make jwtproxy work

---
 data/model/service_keys.py | 11 ++++++-----
 endpoints/api/superuser.py |  3 +--
 endpoints/key_server.py    |  1 +
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/data/model/service_keys.py b/data/model/service_keys.py
index 868c9bea9..5286c0199 100644
--- a/data/model/service_keys.py
+++ b/data/model/service_keys.py
@@ -1,6 +1,8 @@
 from calendar import timegm
 from datetime import datetime, timedelta
 
+from peewee import JOIN_LEFT_OUTER
+
 from app import app
 from data.database import db_for_update, User, ServiceKey, ServiceKeyApproval
 from data.model import ServiceKeyDoesNotExist, ServiceKeyAlreadyApproved, db_transaction
@@ -39,8 +41,6 @@ def _notify_superusers(key):
 
   superusers = User.select().where(User.username << app.config['SUPER_USERS'])
   for superuser in superusers:
-    # TODO(jzelinskie): create notification type in the database migration
-    # I already put it in initdb
     create_notification('service_key_submitted', superuser, metadata=notification_metadata,
                         lookup_path='/service_key_approval/{0}'.format(key.kid))
 
@@ -94,14 +94,15 @@ def delete_service_key(service, kid):
   _gc_expired(service)
 
 
-def approve_service_key(kid, approver, approval_type):
+def approve_service_key(kid, approver, approval_type, notes=''):
   try:
     with db_transaction():
       key = db_for_update(ServiceKey.select().where(ServiceKey.kid == kid)).get()
       if key.approval is not None:
         raise ServiceKeyAlreadyApproved
 
-      approval = ServiceKeyApproval.create(approver=approver, approval_type=approval_type)
+      approval = ServiceKeyApproval.create(approver=approver, approval_type=approval_type,
+                                           notes=notes)
       key.approval = approval
       key.save()
   except ServiceKey.DoesNotExist:
@@ -112,7 +113,7 @@ def approve_service_key(kid, approver, approval_type):
 
 
 def _list_service_keys_query(kid=None, service=None, approved_only=False):
-  query = ServiceKey.select().join(ServiceKeyApproval)
+  query = ServiceKey.select().join(ServiceKeyApproval, JOIN_LEFT_OUTER)
 
   if approved_only:
     query = query.where(~(ServiceKey.approval >> None))
diff --git a/endpoints/api/superuser.py b/endpoints/api/superuser.py
index 8dcccb79a..a0248f68c 100644
--- a/endpoints/api/superuser.py
+++ b/endpoints/api/superuser.py
@@ -637,7 +637,6 @@ class SuperUserServiceKeyApproval(ApiResource):
   @verify_not_prod
   @nickname('approveServiceKey')
   @require_scope(scopes.SUPERUSER)
-  @validate_json_request('ApproveServiceKey')
   def put(self, kid):
     if SuperUserPermission().can():
       approver = get_authenticated_user()
@@ -648,6 +647,6 @@ class SuperUserServiceKeyApproval(ApiResource):
       except model.ServiceKeyAlreadyApproved:
         pass
 
-      make_response('', 200)
+      return make_response('', 200)
 
     abort(403)
diff --git a/endpoints/key_server.py b/endpoints/key_server.py
index ce140b689..1d88c3689 100644
--- a/endpoints/key_server.py
+++ b/endpoints/key_server.py
@@ -83,6 +83,7 @@ def list_service_keys(service):
 
 @key_server.route('/services/<service>/keys/<kid>', methods=['GET'])
 def get_service_key(service, kid):
+  logger.debug(kid)
   try:
     key = data.model.service_keys.get_service_key(kid)
   except data.model.ServiceKeyDoesNotExist: