diff --git a/app/dao/api_key_dao.py b/app/dao/api_key_dao.py index a44adf472..2b7e040ff 100644 --- a/app/dao/api_key_dao.py +++ b/app/dao/api_key_dao.py @@ -1,4 +1,6 @@ import uuid +from datetime import datetime + from flask import current_app from itsdangerous import URLSafeSerializer @@ -13,17 +15,19 @@ from app.dao.dao_utils import ( @transactional @version_class(ApiKey) -def save_model_api_key(api_key, update_dict={}): - if update_dict: - update_dict.pop('id', None) - for key, value in update_dict.items(): - setattr(api_key, key, value) - db.session.add(api_key) - else: - if not api_key.id: - api_key.id = uuid.uuid4() # must be set now so version history model can use same id - api_key.secret = _generate_secret() - db.session.add(api_key) +def save_model_api_key(api_key): + if not api_key.id: + api_key.id = uuid.uuid4() # must be set now so version history model can use same id + api_key.secret = _generate_secret() + db.session.add(api_key) + + +@transactional +@version_class(ApiKey) +def expire_api_key(service_id, api_key_id): + api_key = ApiKey.query.filter_by(id=api_key_id, service_id=service_id).one() + api_key.expiry_date = datetime.utcnow() + db.session.add(api_key) def get_model_api_keys(service_id, id=None): @@ -49,9 +53,8 @@ def get_unsigned_secret(key_id): return _get_secret(api_key.secret) -def _generate_secret(token=None): - if not token: - token = uuid.uuid4() +def _generate_secret(): + token = uuid.uuid4() serializer = URLSafeSerializer(current_app.config.get('SECRET_KEY')) return serializer.dumps(str(token), current_app.config.get('DANGEROUS_SALT')) diff --git a/app/service/rest.py b/app/service/rest.py index 1b01ec9a1..d4b4519b7 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -14,8 +14,8 @@ from sqlalchemy.orm.exc import NoResultFound from app.dao.api_key_dao import ( save_model_api_key, get_model_api_keys, - get_unsigned_secret -) + get_unsigned_secret, + expire_api_key) from app.dao.services_dao import ( dao_fetch_service_by_id_and_user, dao_fetch_service_by_id, @@ -95,6 +95,7 @@ def update_service(service_id): return jsonify(data=service_schema.dump(fetched_service).data), 200 +# is this used. @service.route('//api-key', methods=['POST']) def renew_api_key(service_id=None): fetched_service = dao_fetch_service_by_id(service_id=service_id) @@ -107,8 +108,7 @@ def renew_api_key(service_id=None): @service.route('//api-key/revoke/', methods=['POST']) def revoke_api_key(service_id, api_key_id): - service_api_key = get_model_api_keys(service_id=service_id, id=api_key_id) - save_model_api_key(service_api_key, update_dict={'expiry_date': datetime.utcnow()}) + expire_api_key(service_id=service_id, api_key_id=api_key_id) return jsonify(), 202 diff --git a/tests/app/authentication/test_authentication.py b/tests/app/authentication/test_authentication.py index ef1236a90..d632208a1 100644 --- a/tests/app/authentication/test_authentication.py +++ b/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)}) diff --git a/tests/app/dao/test_api_key_dao.py b/tests/app/dao/test_api_key_dao.py index 2c2cd6857..e6a673ba7 100644 --- a/tests/app/dao/test_api_key_dao.py +++ b/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 diff --git a/tests/app/service/test_api_key_endpoints.py b/tests/app/service/test_api_key_endpoints.py index b6fc82332..c673dfc30 100644 --- a/tests/app/service/test_api_key_endpoints.py +++ b/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