From 5d6e5a42e800ccc2d3c8a6dbca7cc5bd095cc6c0 Mon Sep 17 00:00:00 2001
From: Joseph Schorr <josephschorr@users.noreply.github.com>
Date: Thu, 14 Apr 2016 15:04:32 -0400
Subject: [PATCH] Add delete logging and tests for logging

---
 endpoints/key_server.py | 10 +++++++++
 test/test_endpoints.py  | 49 ++++++++++++++++++++++++-----------------
 2 files changed, 39 insertions(+), 20 deletions(-)

diff --git a/endpoints/key_server.py b/endpoints/key_server.py
index b164f03f3..cc4873a60 100644
--- a/endpoints/key_server.py
+++ b/endpoints/key_server.py
@@ -199,6 +199,16 @@ def delete_service_key(service, kid):
     except data.model.ServiceKeyDoesNotExist:
       abort(404)
 
+    key_log_metadata = {
+      'kid': kid,
+      'signer_kid': signer_key.kid,
+      'service': service,
+      'name': signer_key.name,
+      'user_agent': request.headers.get('User-Agent'),
+      'ip': request.remote_addr,
+    }
+
+    log_action('service_key_delete', None, metadata=key_log_metadata, ip=request.remote_addr)
     return make_response('', 204)
 
   abort(403)
diff --git a/test/test_endpoints.py b/test/test_endpoints.py
index 10ba2c29c..615dc5ec7 100644
--- a/test/test_endpoints.py
+++ b/test/test_endpoints.py
@@ -21,6 +21,7 @@ from endpoints.api import api, api_bp
 from endpoints.api.user import Signin
 from endpoints.web import web as web_bp
 from initdb import setup_database_for_testing, finished_database_for_testing
+from test.helpers import assert_action_logged
 
 
 try:
@@ -205,8 +206,9 @@ class KeyServerTestCase(EndpointTestCase):
     jwkset = py_json.loads(rv)
 
     # Make sure the unapproved key isn't returned in our results
+    self.assertTrue(len(jwkset['keys']) > 0)
     for jwk in jwkset['keys']:
-      self.assertTrue(jwk != unapproved_key.jwk)
+      self.assertNotEquals(jwk, unapproved_key.jwk)
 
   def test_get_service_key(self):
     # 200 for an approved key
@@ -232,19 +234,25 @@ class KeyServerTestCase(EndpointTestCase):
     token = jwt.encode(payload, private_key.exportKey('PEM'), 'RS256')
 
     # Publish a new key
-    self.putResponse('key_server.put_service_key', service='sample_service', kid='kid420',
-                     headers={
-                       'Authorization': 'Bearer %s' % token,
-                       'Content-Type': 'application/json',
-                     }, data=jwk, expected_code=202)
+    with assert_action_logged('service_key_create'):
+      self.putResponse('key_server.put_service_key', service='sample_service', kid='kid420',
+                       headers={
+                        'Authorization': 'Bearer %s' % token,
+                        'Content-Type': 'application/json',
+                       }, data=jwk, expected_code=202)
+
+    # Ensure that the key exists but is unapproved.
+    self.getResponse('key_server.get_service_key', service='sample_service', kid='kid420',
+                     expected_code=409)
 
     # Rotate that new key
-    token = jwt.encode(payload, private_key.exportKey('PEM'), 'RS256', headers={'kid': 'kid420'})
-    self.putResponse('key_server.put_service_key', service='sample_service', kid='kid6969',
-                     headers={
-                       'Authorization': 'Bearer %s' % token,
-                       'Content-Type': 'application/json',
-                     }, data=jwk, expected_code=200)
+    with assert_action_logged('service_key_rotate'):
+      token = jwt.encode(payload, private_key.exportKey('PEM'), 'RS256', headers={'kid': 'kid420'})
+      self.putResponse('key_server.put_service_key', service='sample_service', kid='kid6969',
+                       headers={
+                         'Authorization': 'Bearer %s' % token,
+                         'Content-Type': 'application/json',
+                       }, data=jwk, expected_code=200)
 
     # Rotation should only work when signed by the previous key
     private_key = RSA.generate(2048)
@@ -256,6 +264,7 @@ class KeyServerTestCase(EndpointTestCase):
                        'Content-Type': 'application/json',
                      }, data=jwk, expected_code=403)
 
+
   def test_delete_service_key(self):
     # No Authorization header should yield a 400
     self.deleteResponse('key_server.delete_service_key', expected_code=400,
@@ -271,9 +280,10 @@ class KeyServerTestCase(EndpointTestCase):
                        headers={'kid': 'kid123'})
 
     # Using the credentials of our approved key, delete our unapproved key
-    self.deleteResponse('key_server.delete_service_key',
-                        headers={'Authorization': 'Bearer %s' % token},
-                        expected_code=204, service='sample_service', kid='kid321')
+    with assert_action_logged('service_key_delete'):
+      self.deleteResponse('key_server.delete_service_key',
+                          headers={'Authorization': 'Bearer %s' % token},
+                          expected_code=204, service='sample_service', kid='kid321')
 
     # Attempt to delete a key signed by a key from a different service
     bad_token = jwt.encode(self._get_test_jwt_payload(), private_key.exportKey('PEM'), 'RS256',
@@ -283,11 +293,10 @@ class KeyServerTestCase(EndpointTestCase):
                         expected_code=403, service='sample_service', kid='kid123')
 
     # Delete a self-signed, approved key
-    self.deleteResponse('key_server.delete_service_key',
-                        headers={'Authorization': 'Bearer %s' % token},
-                        expected_code=204, service='sample_service', kid='kid123')
-
-
+    with assert_action_logged('service_key_delete'):
+      self.deleteResponse('key_server.delete_service_key',
+                          headers={'Authorization': 'Bearer %s' % token},
+                          expected_code=204, service='sample_service', kid='kid123')
 
 
 if __name__ == '__main__':