Refactor the api_key_dao.

The only update we should be doing to an api key is to expire/revoke the api key.
Removed the update_dict from the the save method.
Added an expire_api_key method that only updates the api key with an expiry date.

tests/app/authentication/test_authentication.py

@@ -2,7 +2,7 @@ import uuid
 from datetime import datetime, timedelta
 from notifications_python_client.authentication import create_jwt_token
 from flask import json, current_app
-from app.dao.api_key_dao import get_unsigned_secrets, save_model_api_key, get_unsigned_secret
+from app.dao.api_key_dao import get_unsigned_secrets, save_model_api_key, get_unsigned_secret, expire_api_key
 from app.models import ApiKey
 
 
@@ -161,13 +161,7 @@ def test_authentication_returns_token_expired_when_service_uses_expired_key_and_
             token = create_jwt_token(
                 secret=get_unsigned_secret(expired_api_key.id),
                 client_id=str(sample_api_key.service_id))
-            # expire the key
-            expire_the_key = {'id': expired_api_key.id,
-                              'service': sample_api_key.service,
-                              'name': 'expired_key',
-                              'expiry_date': datetime.utcnow() + timedelta(hours=-2),
-                              'created_by': sample_api_key.created_by}
-            save_model_api_key(expired_api_key, expire_the_key)
+            expire_api_key(service_id=sample_api_key.service_id, api_key_id=expired_api_key.id)
             response = client.get(
                 '/service',
                 headers={'Authorization': 'Bearer {}'.format(token)})

tests/app/dao/test_api_key_dao.py

@@ -8,17 +8,16 @@ from app.dao.api_key_dao import (save_model_api_key,
                                  get_unsigned_secrets,
                                  get_unsigned_secret,
                                  _generate_secret,
-                                 _get_secret)
+                                 _get_secret, expire_api_key)
 from app.models import ApiKey
 
 
-def test_secret_is_signed_and_can_be_read_again(notify_api):
-    import uuid
+def test_secret_is_signed_and_can_be_read_again(notify_api, mocker):
     with notify_api.test_request_context():
-        token = str(uuid.uuid4())
-        signed_secret = _generate_secret(token=token)
-        assert token == _get_secret(signed_secret)
-        assert signed_secret != token
+        mocker.patch("uuid.uuid4", return_value='some_uuid')
+        signed_secret = _generate_secret()
+        assert 'some_uuid' == _get_secret(signed_secret)
+        assert signed_secret != 'some_uuid'
 
 
 def test_save_api_key_should_create_new_api_key_and_history(notify_api,
@@ -45,20 +44,18 @@ def test_expire_api_key_should_update_the_api_key_and_create_history_record(noti
                                                                             notify_db,
                                                                             notify_db_session,
                                                                             sample_api_key):
-    now = datetime.utcnow()
-    saved_api_key = get_model_api_keys(service_id=sample_api_key.service_id, id=sample_api_key.id)
-    save_model_api_key(saved_api_key, update_dict={'id': saved_api_key.id, 'expiry_date': now})
+    expire_api_key(service_id=sample_api_key.service_id, api_key_id=sample_api_key.id)
     all_api_keys = get_model_api_keys(service_id=sample_api_key.service_id)
     assert len(all_api_keys) == 1
-    assert all_api_keys[0].expiry_date == now
-    assert all_api_keys[0].secret == saved_api_key.secret
-    assert all_api_keys[0].id == saved_api_key.id
-    assert all_api_keys[0].service_id == saved_api_key.service_id
+    assert all_api_keys[0].expiry_date <= datetime.utcnow()
+    assert all_api_keys[0].secret == sample_api_key.secret
+    assert all_api_keys[0].id == sample_api_key.id
+    assert all_api_keys[0].service_id == sample_api_key.service_id
 
-    all_history = saved_api_key.get_history_model().query.all()
+    all_history = sample_api_key.get_history_model().query.all()
     assert len(all_history) == 2
-    assert all_history[0].id == saved_api_key.id
-    assert all_history[1].id == saved_api_key.id
+    assert all_history[0].id == sample_api_key.id
+    assert all_history[1].id == sample_api_key.id
     sorted_all_history = sorted(all_history, key=lambda hist: hist.version)
     sorted_all_history[0].version = 1
     sorted_all_history[1].version = 2

tests/app/service/test_api_key_endpoints.py

@@ -3,7 +3,7 @@ from datetime import timedelta, datetime
 
 from flask import url_for
 from app.models import ApiKey
-from app.dao.api_key_dao import save_model_api_key
+from app.dao.api_key_dao import save_model_api_key, expire_api_key
 from tests import create_authorization_header
 from tests.app.conftest import sample_api_key as create_sample_api_key
 from tests.app.conftest import sample_service as create_sample_service
@@ -97,7 +97,7 @@ def test_get_api_keys_should_return_all_keys_for_service(notify_api, notify_db,
             # this service already has one key, add two more, one expired
             create_sample_api_key(notify_db, notify_db_session, service=sample_api_key.service)
             one_to_expire = create_sample_api_key(notify_db, notify_db_session, service=sample_api_key.service)
-            save_model_api_key(one_to_expire, update_dict={'expiry_date': datetime.utcnow()})
+            expire_api_key(service_id=one_to_expire.service_id, api_key_id=one_to_expire.id)
 
             assert ApiKey.query.count() == 4