diff --git a/app/models.py b/app/models.py index cd82a99e5..0465214f4 100644 --- a/app/models.py +++ b/app/models.py @@ -748,7 +748,7 @@ class ApiKey(db.Model, Versioned): created_by_id = db.Column(UUID(as_uuid=True), db.ForeignKey('users.id'), index=True, nullable=False) __table_args__ = ( - UniqueConstraint('service_id', 'name', name='uix_service_to_key_name'), + Index('uix_service_to_key_name', 'service_id', 'name', unique=True, postgresql_where=expiry_date.is_(None)), ) @property diff --git a/migrations/versions/0295_api_key_constraint.py b/migrations/versions/0295_api_key_constraint.py new file mode 100644 index 000000000..12dec7c87 --- /dev/null +++ b/migrations/versions/0295_api_key_constraint.py @@ -0,0 +1,23 @@ +""" + +Revision ID: 0295_api_key_constraint +Revises: 0294_add_verify_reply_to +Create Date: 2019-06-04 13:49:50.685493 + +""" +from alembic import op +import sqlalchemy as sa + +revision = '0295_api_key_constraint' +down_revision = '0294_add_verify_reply_to' + + +def upgrade(): + op.drop_constraint('uix_service_to_key_name', 'api_keys', type_='unique') + op.create_index('uix_service_to_key_name', 'api_keys', ['service_id', 'name'], unique=True, + postgresql_where=sa.text('expiry_date IS NULL')) + + +def downgrade(): + op.drop_index('uix_service_to_key_name', table_name='api_keys') + op.create_unique_constraint('uix_service_to_key_name', 'api_keys', ['service_id', 'name']) diff --git a/tests/app/dao/test_api_key_dao.py b/tests/app/dao/test_api_key_dao.py index 0f6a67526..2c39881da 100644 --- a/tests/app/dao/test_api_key_dao.py +++ b/tests/app/dao/test_api_key_dao.py @@ -4,11 +4,13 @@ import pytest from sqlalchemy.exc import IntegrityError from sqlalchemy.orm.exc import NoResultFound -from app.dao.api_key_dao import (save_model_api_key, - get_model_api_keys, - get_unsigned_secrets, - get_unsigned_secret, - expire_api_key) +from app.dao.api_key_dao import ( + save_model_api_key, + get_model_api_keys, + get_unsigned_secrets, + get_unsigned_secret, + expire_api_key +) from app.models import ApiKey, KEY_TYPE_NORMAL @@ -82,6 +84,22 @@ def test_should_not_allow_duplicate_key_names_per_service(sample_api_key, fake_u save_model_api_key(api_key) +def test_save_api_key_can_create_key_with_same_name_if_other_is_expired(sample_service): + expired_api_key = ApiKey(**{'service': sample_service, + 'name': "normal api key", + 'created_by': sample_service.created_by, + 'key_type': KEY_TYPE_NORMAL, + 'expiry_date': datetime.utcnow()}) + save_model_api_key(expired_api_key) + api_key = ApiKey(**{'service': sample_service, + 'name': "normal api key", + 'created_by': sample_service.created_by, + 'key_type': KEY_TYPE_NORMAL}) + save_model_api_key(api_key) + keys = ApiKey.query.all() + assert len(keys) == 2 + + def test_save_api_key_should_not_create_new_service_history(sample_service): from app.models import Service @@ -99,9 +117,9 @@ def test_save_api_key_should_not_create_new_service_history(sample_service): @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 + sample_service, + days_old, + expected_length ): expired_api_key = ApiKey(**{'service': sample_service, 'name': sample_service.name,