Merge pull request #1758 from alphagov/filter-old-revoked-api-keys

Filter revoked api keys older than 7 days

app/dao/api_key_dao.py

@@ -1,5 +1,5 @@
 import uuid
-from datetime import datetime
+from datetime import datetime, timedelta
 
 from app import db
 from app.models import ApiKey
@@ -9,6 +9,8 @@ from app.dao.dao_utils import (
     version_class
 )
 
+from sqlalchemy import or_, func
+
 
 @transactional
 @version_class(ApiKey)
@@ -30,7 +32,11 @@ def expire_api_key(service_id, api_key_id):
 def get_model_api_keys(service_id, id=None):
     if id:
         return ApiKey.query.filter_by(id=id, service_id=service_id, expiry_date=None).one()
-    return ApiKey.query.filter_by(service_id=service_id).all()
+    seven_days_ago = datetime.utcnow() - timedelta(days=7)
+    return ApiKey.query.filter(
+        or_(ApiKey.expiry_date == None, func.date(ApiKey.expiry_date) > seven_days_ago),  # noqa
+        ApiKey.service_id == service_id
+    ).all()
 
 
 def get_unsigned_secrets(service_id):

tests/app/dao/test_api_key_dao.py

@@ -1,4 +1,4 @@
-from datetime import datetime
+from datetime import datetime, timedelta
 
 import pytest
 from sqlalchemy.exc import IntegrityError
@@ -95,3 +95,21 @@ def test_save_api_key_should_not_create_new_service_history(sample_service):
     save_model_api_key(api_key)
 
     assert Service.get_history_model().query.count() == 1
+
+
+@pytest.mark.parametrize('days_old, expected_length', [(5, 1), (8, 0)])
+def test_should_not_return_revoked_api_keys_older_than_7_days(
+    sample_service,
+    days_old,
+    expected_length
+):
+    expired_api_key = ApiKey(**{'service': sample_service,
+                                'name': sample_service.name,
+                                'created_by': sample_service.created_by,
+                                'key_type': KEY_TYPE_NORMAL,
+                                'expiry_date': datetime.utcnow() - timedelta(days=days_old)})
+    save_model_api_key(expired_api_key)
+
+    all_api_keys = get_model_api_keys(service_id=sample_service.id)
+
+    assert len(all_api_keys) == expected_length