From e9482c7fe19b125e4aba5b3dfe999ada32b8ee34 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Thu, 23 Jun 2016 16:45:20 +0100 Subject: [PATCH] add new key_type table * single-column static data table that currently contains two types: 'normal' and 'team' * key_type foreign-keyed from api_keys - must be not null - existing rows set to 'normal' * key_type foreign-keyed from notifications - nullable - existing rows set to null * api_key foreign-keyed from notifications - nullable - existing rows set to null --- app/models.py | 14 +++++ app/schemas.py | 1 + .../0032_notification_created_status.py | 2 +- migrations/versions/0033_api_key_type.py | 59 +++++++++++++++++++ tests/__init__.py | 9 ++- .../app/authentication/test_authentication.py | 17 ++++-- tests/app/conftest.py | 8 ++- tests/app/dao/test_api_key_dao.py | 11 ++-- tests/app/service/test_api_key_endpoints.py | 25 +++++--- tests/conftest.py | 2 +- 10 files changed, 123 insertions(+), 25 deletions(-) create mode 100644 migrations/versions/0033_api_key_type.py diff --git a/app/models.py b/app/models.py index cca4b7d32..b660bd90c 100644 --- a/app/models.py +++ b/app/models.py @@ -113,6 +113,7 @@ class ApiKey(db.Model, Versioned): secret = db.Column(db.String(255), unique=True, nullable=False) service_id = db.Column(UUID(as_uuid=True), db.ForeignKey('services.id'), index=True, nullable=False) service = db.relationship('Service', backref=db.backref('api_keys', lazy='dynamic')) + key_type = db.Column(db.String(255), db.ForeignKey('key_types.name'), index=True, nullable=False) expiry_date = db.Column(db.DateTime) created_at = db.Column( db.DateTime, @@ -134,6 +135,16 @@ class ApiKey(db.Model, Versioned): ) +KEY_TYPE_NORMAL = 'normal' +KEY_TYPE_TEAM = 'team' + + +class KeyTypes(db.Model): + __tablename__ = 'key_types' + + name = db.Column(db.String(255), primary_key=True) + + class NotificationStatistics(db.Model): __tablename__ = 'notification_statistics' @@ -329,6 +340,9 @@ class Notification(db.Model): template_id = db.Column(UUID(as_uuid=True), db.ForeignKey('templates.id'), index=True, unique=False) template = db.relationship('Template') template_version = db.Column(db.Integer, nullable=False) + api_key_id = db.Column(UUID(as_uuid=True), db.ForeignKey('api_keys.id'), index=True, unique=False) + api_key = db.relationship('ApiKey') + key_type = db.Column(db.String, db.ForeignKey('key_types.name'), index=True, unique=False) content_char_count = db.Column(db.Integer, nullable=True) created_at = db.Column( db.DateTime, diff --git a/app/schemas.py b/app/schemas.py index 23e99fb5a..13b0fb026 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -165,6 +165,7 @@ class NotificationsStatisticsSchema(BaseSchema): class ApiKeySchema(BaseSchema): created_by = field_for(models.ApiKey, 'created_by', required=True) + key_type = field_for(models.ApiKey, 'key_type', required=True) class Meta: model = models.ApiKey diff --git a/migrations/versions/0032_notification_created_status.py b/migrations/versions/0032_notification_created_status.py index 484f7884f..d12b39f28 100644 --- a/migrations/versions/0032_notification_created_status.py +++ b/migrations/versions/0032_notification_created_status.py @@ -47,4 +47,4 @@ def downgrade(): op.get_bind() op.execute('DROP TYPE notify_status_type') op.alter_column('notifications', 'status', nullable=False) - ### end Alembic commands ### \ No newline at end of file + ### end Alembic commands ### diff --git a/migrations/versions/0033_api_key_type.py b/migrations/versions/0033_api_key_type.py new file mode 100644 index 000000000..ebcb072ef --- /dev/null +++ b/migrations/versions/0033_api_key_type.py @@ -0,0 +1,59 @@ +"""empty message + +Revision ID: 0033_api_key_type +Revises: 0032_notification_created_status +Create Date: 2016-06-24 12:02:10.915817 + +""" + +# revision identifiers, used by Alembic. +revision = '0033_api_key_type' +down_revision = '0032_notification_created_status' + +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +def upgrade(): + ### commands auto generated by Alembic - please adjust! ### + op.create_table('key_types', + sa.Column('name', sa.String(length=255), nullable=False), + sa.PrimaryKeyConstraint('name') + ) + op.add_column('api_keys', sa.Column('key_type', sa.String(length=255), nullable=True)) + op.add_column('api_keys_history', sa.Column('key_type', sa.String(length=255), nullable=True)) + op.add_column('notifications', sa.Column('api_key_id', postgresql.UUID(as_uuid=True), nullable=True)) + op.add_column('notifications', sa.Column('key_type', sa.String(length=255), nullable=True)) + + op.create_index(op.f('ix_api_keys_key_type'), 'api_keys', ['key_type'], unique=False) + op.create_index(op.f('ix_api_keys_history_key_type'), 'api_keys_history', ['key_type'], unique=False) + op.create_index(op.f('ix_notifications_api_key_id'), 'notifications', ['api_key_id'], unique=False) + op.create_index(op.f('ix_notifications_key_type'), 'notifications', ['key_type'], unique=False) + op.create_foreign_key(None, 'api_keys', 'key_types', ['key_type'], ['name']) + op.create_foreign_key(None, 'notifications', 'api_keys', ['api_key_id'], ['id']) + op.create_foreign_key(None, 'notifications', 'key_types', ['key_type'], ['name']) + + op.execute("insert into key_types values ('normal'), ('team')") + op.execute("update api_keys set key_type = 'normal'") + op.execute("update api_keys_history set key_type = 'normal'") + + op.alter_column('api_keys', 'key_type', nullable=False) + op.alter_column('api_keys_history', 'key_type', nullable=False) + ### end Alembic commands ### + + +def downgrade(): + ### commands auto generated by Alembic - please adjust! ### + op.drop_constraint('notifications_key_type_fkey', 'notifications', type_='foreignkey') + op.drop_constraint('notifications_api_key_id_fkey', 'notifications', type_='foreignkey') + op.drop_index(op.f('ix_notifications_key_type'), table_name='notifications') + op.drop_index(op.f('ix_notifications_api_key_id'), table_name='notifications') + op.drop_column('notifications', 'key_type') + op.drop_column('notifications', 'api_key_id') + op.drop_index(op.f('ix_api_keys_history_key_type'), table_name='api_keys_history') + op.drop_column('api_keys_history', 'key_type') + op.drop_constraint('api_keys_key_type_fkey', 'api_keys', type_='foreignkey') + op.drop_index(op.f('ix_api_keys_key_type'), table_name='api_keys') + op.drop_column('api_keys', 'key_type') + op.drop_table('key_types') + ### end Alembic commands ### diff --git a/tests/__init__.py b/tests/__init__.py index af6e98e77..d8ee29d26 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -1,7 +1,7 @@ import uuid from flask import current_app from notifications_python_client.authentication import create_jwt_token -from app.models import ApiKey +from app.models import ApiKey, KEY_TYPE_NORMAL from app.dao.api_key_dao import (get_unsigned_secrets, save_model_api_key) from app.dao.services_dao import dao_fetch_service_by_id @@ -14,7 +14,12 @@ def create_authorization_header(service_id=None): secret = secrets[0] else: service = dao_fetch_service_by_id(service_id) - data = {'service': service, 'name': uuid.uuid4(), 'created_by': service.created_by} + data = { + 'service': service, + 'name': uuid.uuid4(), + 'created_by': service.created_by, + 'key_type': KEY_TYPE_NORMAL + } api_key = ApiKey(**data) save_model_api_key(api_key) secret = get_unsigned_secrets(service_id)[0] diff --git a/tests/app/authentication/test_authentication.py b/tests/app/authentication/test_authentication.py index d632208a1..12d7e4681 100644 --- a/tests/app/authentication/test_authentication.py +++ b/tests/app/authentication/test_authentication.py @@ -3,7 +3,7 @@ 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, expire_api_key -from app.models import ApiKey +from app.models import ApiKey, KEY_TYPE_NORMAL def test_should_not_allow_request_with_no_token(notify_api): @@ -78,7 +78,8 @@ def test_should_allow_valid_token_when_service_has_multiple_keys(notify_api, sam with notify_api.test_client() as client: data = {'service': sample_api_key.service, 'name': 'some key name', - 'created_by': sample_api_key.created_by + 'created_by': sample_api_key.created_by, + 'key_type': KEY_TYPE_NORMAL } api_key = ApiKey(**data) save_model_api_key(api_key) @@ -121,13 +122,15 @@ def test_authentication_passes_when_service_has_multiple_keys_some_expired( expired_key_data = {'service': sample_api_key.service, 'name': 'expired_key', 'expiry_date': datetime.utcnow(), - 'created_by': sample_api_key.created_by + 'created_by': sample_api_key.created_by, + 'key_type': KEY_TYPE_NORMAL } expired_key = ApiKey(**expired_key_data) save_model_api_key(expired_key) another_key = {'service': sample_api_key.service, 'name': 'another_key', - 'created_by': sample_api_key.created_by + 'created_by': sample_api_key.created_by, + 'key_type': KEY_TYPE_NORMAL } api_key = ApiKey(**another_key) save_model_api_key(api_key) @@ -148,13 +151,15 @@ def test_authentication_returns_token_expired_when_service_uses_expired_key_and_ with notify_api.test_client() as client: expired_key = {'service': sample_api_key.service, 'name': 'expired_key', - 'created_by': sample_api_key.created_by + 'created_by': sample_api_key.created_by, + 'key_type': KEY_TYPE_NORMAL } expired_api_key = ApiKey(**expired_key) save_model_api_key(expired_api_key) another_key = {'service': sample_api_key.service, 'name': 'another_key', - 'created_by': sample_api_key.created_by + 'created_by': sample_api_key.created_by, + 'key_type': KEY_TYPE_NORMAL } api_key = ApiKey(**another_key) save_model_api_key(api_key) diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 8fe1a1e83..8d1b1fed2 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -18,7 +18,8 @@ from app.models import ( Permission, ProviderStatistics, ProviderDetails, - NotificationStatistics) + NotificationStatistics, + KEY_TYPE_NORMAL) from app.dao.users_dao import (save_model_user, create_user_code, create_secret_code) from app.dao.services_dao import (dao_create_service, dao_add_user_to_service) from app.dao.templates_dao import dao_create_template @@ -229,10 +230,11 @@ def sample_email_template_with_placeholders(notify_db, notify_db_session): @pytest.fixture(scope='function') def sample_api_key(notify_db, notify_db_session, - service=None): + service=None, + key_type=KEY_TYPE_NORMAL): if service is None: service = sample_service(notify_db, notify_db_session) - data = {'service': service, 'name': uuid.uuid4(), 'created_by': service.created_by} + data = {'service': service, 'name': uuid.uuid4(), 'created_by': service.created_by, 'key_type': key_type} api_key = ApiKey(**data) save_model_api_key(api_key) return api_key diff --git a/tests/app/dao/test_api_key_dao.py b/tests/app/dao/test_api_key_dao.py index e6a673ba7..58505a812 100644 --- a/tests/app/dao/test_api_key_dao.py +++ b/tests/app/dao/test_api_key_dao.py @@ -9,7 +9,7 @@ from app.dao.api_key_dao import (save_model_api_key, get_unsigned_secret, _generate_secret, _get_secret, expire_api_key) -from app.models import ApiKey +from app.models import ApiKey, KEY_TYPE_NORMAL def test_secret_is_signed_and_can_be_read_again(notify_api, mocker): @@ -26,7 +26,8 @@ def test_save_api_key_should_create_new_api_key_and_history(notify_api, sample_service): api_key = ApiKey(**{'service': sample_service, 'name': sample_service.name, - 'created_by': sample_service.created_by}) + 'created_by': sample_service.created_by, + 'key_type': KEY_TYPE_NORMAL}) save_model_api_key(api_key) all_api_keys = get_model_api_keys(service_id=sample_service.id) @@ -105,7 +106,8 @@ def test_should_not_allow_duplicate_key_names_per_service(notify_api, api_key = ApiKey(**{'id': fake_uuid, 'service': sample_api_key.service, 'name': sample_api_key.name, - 'created_by': sample_api_key.created_by}) + 'created_by': sample_api_key.created_by, + 'key_type': KEY_TYPE_NORMAL}) try: save_model_api_key(api_key) fail("should throw IntegrityError") @@ -122,7 +124,8 @@ def test_save_api_key_should_not_create_new_service_history(notify_api, notify_d api_key = ApiKey(**{'service': sample_service, 'name': sample_service.name, - 'created_by': sample_service.created_by}) + 'created_by': sample_service.created_by, + 'key_type': KEY_TYPE_NORMAL}) save_model_api_key(api_key) assert Service.get_history_model().query.count() == 1 diff --git a/tests/app/service/test_api_key_endpoints.py b/tests/app/service/test_api_key_endpoints.py index 045a37a30..4714de018 100644 --- a/tests/app/service/test_api_key_endpoints.py +++ b/tests/app/service/test_api_key_endpoints.py @@ -1,9 +1,9 @@ import json -from datetime import timedelta, datetime +from datetime import datetime from flask import url_for -from app.models import ApiKey -from app.dao.api_key_dao import save_model_api_key, expire_api_key +from app.models import ApiKey, KEY_TYPE_NORMAL +from app.dao.api_key_dao import 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 @@ -15,13 +15,17 @@ def test_api_key_should_create_new_api_key_for_service(notify_api, notify_db, sample_service): with notify_api.test_request_context(): with notify_api.test_client() as client: - data = {'name': 'some secret name', 'created_by': str(sample_service.created_by.id)} + data = { + 'name': 'some secret name', + 'created_by': str(sample_service.created_by.id), + 'key_type': KEY_TYPE_NORMAL + } auth_header = create_authorization_header() response = client.post(url_for('service.create_api_key', service_id=sample_service.id), data=json.dumps(data), headers=[('Content-Type', 'application/json'), auth_header]) assert response.status_code == 201 - assert response.get_data is not None + assert 'data' in json.loads(response.get_data(as_text=True)) saved_api_key = ApiKey.query.filter_by(service_id=sample_service.id).first() assert saved_api_key.service_id == sample_service.id assert saved_api_key.name == 'some secret name' @@ -60,20 +64,25 @@ def test_api_key_should_create_multiple_new_api_key_for_service(notify_api, noti with notify_api.test_request_context(): with notify_api.test_client() as client: assert ApiKey.query.count() == 0 - data = {'name': 'some secret name', 'created_by': str(sample_service.created_by.id)} + data = { + 'name': 'some secret name', + 'created_by': str(sample_service.created_by.id), + 'key_type': KEY_TYPE_NORMAL + } auth_header = create_authorization_header() response = client.post(url_for('service.create_api_key', service_id=sample_service.id), data=json.dumps(data), headers=[('Content-Type', 'application/json'), auth_header]) assert response.status_code == 201 assert ApiKey.query.count() == 1 - data = {'name': 'another secret name', 'created_by': str(sample_service.created_by.id)} + + data['name'] = 'another secret name' auth_header = create_authorization_header() response2 = client.post(url_for('service.create_api_key', service_id=sample_service.id), data=json.dumps(data), headers=[('Content-Type', 'application/json'), auth_header]) assert response2.status_code == 201 - assert response2.get_data != response.get_data + assert json.loads(response.get_data(as_text=True)) != json.loads(response2.get_data(as_text=True)) assert ApiKey.query.count() == 2 diff --git a/tests/conftest.py b/tests/conftest.py index 2499360dd..04777f4fc 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -50,7 +50,7 @@ def notify_db_session(request, notify_db): def teardown(): notify_db.session.remove() for tbl in reversed(notify_db.metadata.sorted_tables): - if tbl.name not in ["provider_details"]: + if tbl.name not in ["provider_details", "key_types"]: notify_db.engine.execute(tbl.delete()) notify_db.session.commit()