Hide expired keys outside of their staleness window

data/model/service_keys.py

@@ -20,11 +20,12 @@ def _expired_keys_clause(service):
   return ((ServiceKey.service == service) &
           (ServiceKey.expiration_date <= datetime.utcnow()))
 
-def _stale_expired_keys_clause(service):
+def _stale_expired_keys_service_clause(service):
-  expired_ttl = timedelta(seconds=config.app_config['EXPIRED_SERVICE_KEY_TTL_SEC'])
+  return ((ServiceKey.service == service) & _stale_expired_keys_clause())
-  return ((ServiceKey.service == service) &
-          (ServiceKey.expiration_date <= (datetime.utcnow() - expired_ttl)))
 
+def _stale_expired_keys_clause():
+  expired_ttl = timedelta(seconds=config.app_config['EXPIRED_SERVICE_KEY_TTL_SEC'])
+  return (ServiceKey.expiration_date <= (datetime.utcnow() - expired_ttl))
 
 def _stale_unapproved_keys_clause(service):
   unapproved_ttl = timedelta(seconds=config.app_config['UNAPPROVED_SERVICE_KEY_TTL_SEC'])
@@ -34,7 +35,7 @@ def _stale_unapproved_keys_clause(service):
 
 
 def _gc_expired(service):
-  ServiceKey.delete().where(_stale_expired_keys_clause(service) |
+  ServiceKey.delete().where(_stale_expired_keys_service_clause(service) |
                             _stale_unapproved_keys_clause(service)).execute()
 
 
@@ -64,12 +65,12 @@ def _notify_superusers(key):
 
 def create_service_key(name, kid, service, jwk, metadata, expiration_date, rotation_duration=None):
   _verify_service_name(service)
+  _gc_expired(service)
 
   key = ServiceKey.create(name=name, kid=kid, service=service, jwk=jwk, metadata=metadata,
                           expiration_date=expiration_date, rotation_duration=rotation_duration)
 
   _notify_superusers(key)
-  _gc_expired(service)
   return key
 
 
@@ -155,9 +156,9 @@ def approve_service_key(kid, approver, approval_type, notes=''):
     raise ServiceKeyDoesNotExist
 
   delete_all_notifications_by_path_prefix('/service_key_approval/{0}'.format(kid))
-  _gc_expired(key.service)
   return key
 
+
 def _list_service_keys_query(kid=None, service=None, approved_only=False):
   query = ServiceKey.select().join(ServiceKeyApproval, JOIN_LEFT_OUTER)
 
@@ -172,6 +173,7 @@ def _list_service_keys_query(kid=None, service=None, approved_only=False):
   if kid is not None:
     query = query.where(ServiceKey.kid == kid)
 
+  query = query.where(~(_stale_expired_keys_clause()) | (ServiceKey.expiration_date >> None))
   return query
 
 

endpoints/key_server.py

@@ -88,16 +88,12 @@ def get_service_key(service, kid):
   if key.approval is None:
     abort(409)
 
-  if key.expiration_date <= datetime.utcnow():
+  if key.expiration_date is not None and key.expiration_date <= datetime.utcnow():
     abort(403)
 
   resp = jsonify(key.jwk)
-
+  lifetime = min(timedelta(days=1), ((key.expiration_date or datetime.max) - datetime.utcnow()))
-  lifetime = timedelta(days=365)
+  resp.cache_control.max_age = max(0, lifetime.total_seconds())
-  if key.expiration_date is not None:
-    lifetime = key.expiration_date - key.created_date
-  resp.cache_control.max_age = lifetime.seconds
-
   return resp
 
 

initdb.py

@@ -163,7 +163,7 @@ def __generate_service_key(kid, name, user, timestamp, approval_type, expiration
 
   if approval_type is not None:
     model.service_keys.approve_service_key(key.kid, user, approval_type,
-                                           notes='The **test** apporval')
+                                           notes='The **test** approval')
 
     key_metadata = {
       'kid': kid,
@@ -667,6 +667,12 @@ def populate_database(minimal=False, with_storage=False):
                          ServiceKeyApprovalType.SUPERUSER, today + timedelta(days=14),
                          service='different_sample_service')
 
+  __generate_service_key('kid6', 'someexpiredkey', new_user_1, week_ago,
+                         ServiceKeyApprovalType.SUPERUSER, today - timedelta(days=1))
+
+  __generate_service_key('kid7', 'somewayexpiredkey', new_user_1, week_ago,
+                         ServiceKeyApprovalType.SUPERUSER, today - timedelta(days=30))
+
   model.log.log_action('org_create_team', org.username, performer=new_user_1,
                        timestamp=week_ago, metadata={'team': 'readers'})
 

test/test_endpoints.py

@@ -202,14 +202,23 @@ class KeyServerTestCase(EndpointTestCase):
 
   def test_list_service_keys(self):
     unapproved_key = model.service_keys.get_service_key(kid='kid3')
+    expired_key = model.service_keys.get_service_key(kid='kid6')
+
     rv = self.getResponse('key_server.list_service_keys', service='sample_service')
     jwkset = py_json.loads(rv)
 
-    # Make sure the unapproved key isn't returned in our results
+    # Make sure the hidden keys are not returned and the visible ones are returned.
     self.assertTrue(len(jwkset['keys']) > 0)
+    expired_key_found = False
     for jwk in jwkset['keys']:
       self.assertNotEquals(jwk, unapproved_key.jwk)
 
+      if expired_key.jwk == jwk:
+        expired_key_found = True
+
+    self.assertTrue(expired_key_found)
+
+
   def test_get_service_key(self):
     # 200 for an approved key
     self.getResponse('key_server.get_service_key', service='sample_service', kid='kid1')
@@ -222,6 +231,14 @@ class KeyServerTestCase(EndpointTestCase):
     self.getResponse('key_server.get_service_key', service='sample_service', kid='kid9999',
                      expected_code=404)
 
+    # 403 for an approved but expired key that is inside of the 2 week window.
+    self.getResponse('key_server.get_service_key', service='sample_service', kid='kid6',
+                     expected_code=403)
+
+    # 404 for an approved, expired key that is outside of the 2 week window.
+    self.getResponse('key_server.get_service_key', service='sample_service', kid='kid7',
+                     expected_code=404)
+
   def test_put_service_key(self):
     # No Authorization header should yield a 400
     self.putResponse('key_server.put_service_key', service='sample_service', kid='kid420',