diff --git a/data/model/service_keys.py b/data/model/service_keys.py index deae1b95c..5de5b0301 100644 --- a/data/model/service_keys.py +++ b/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): - expired_ttl = timedelta(seconds=config.app_config['EXPIRED_SERVICE_KEY_TTL_SEC']) - return ((ServiceKey.service == service) & - (ServiceKey.expiration_date <= (datetime.utcnow() - expired_ttl))) +def _stale_expired_keys_service_clause(service): + return ((ServiceKey.service == service) & _stale_expired_keys_clause()) +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 diff --git a/endpoints/key_server.py b/endpoints/key_server.py index 62adcb0d4..3839ffc5e 100644 --- a/endpoints/key_server.py +++ b/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 = timedelta(days=365) - if key.expiration_date is not None: - lifetime = key.expiration_date - key.created_date - resp.cache_control.max_age = lifetime.seconds - + lifetime = min(timedelta(days=1), ((key.expiration_date or datetime.max) - datetime.utcnow())) + resp.cache_control.max_age = max(0, lifetime.total_seconds()) return resp diff --git a/initdb.py b/initdb.py index 6c3492504..871ef22e5 100644 --- a/initdb.py +++ b/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'}) diff --git a/test/test_endpoints.py b/test/test_endpoints.py index e1486ce3d..45c381587 100644 --- a/test/test_endpoints.py +++ b/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',