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 0a7eab8d1..8ef213c8c 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, missing=models.KEY_TYPE_NORMAL) class Meta: model = models.ApiKey diff --git a/app/service/rest.py b/app/service/rest.py index d4b4519b7..e94107260 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -95,9 +95,8 @@ 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): +def create_api_key(service_id=None): fetched_service = dao_fetch_service_by_id(service_id=service_id) valid_api_key = api_key_schema.load(request.get_json()).data valid_api_key.service = fetched_service 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 8773218fc..28f4a9df1 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..233966b4f 100644 --- a/tests/app/dao/test_api_key_dao.py +++ b/tests/app/dao/test_api_key_dao.py @@ -1,6 +1,7 @@ from datetime import datetime -from pytest import fail +import pytest +from sqlalchemy.exc import IntegrityError from sqlalchemy.orm.exc import NoResultFound from app.dao.api_key_dao import (save_model_api_key, @@ -9,7 +10,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): @@ -20,13 +21,11 @@ def test_secret_is_signed_and_can_be_read_again(notify_api, mocker): assert signed_secret != 'some_uuid' -def test_save_api_key_should_create_new_api_key_and_history(notify_api, - notify_db, - notify_db_session, - sample_service): +def test_save_api_key_should_create_new_api_key_and_history(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) @@ -41,8 +40,6 @@ def test_save_api_key_should_create_new_api_key_and_history(notify_api, def test_expire_api_key_should_update_the_api_key_and_create_history_record(notify_api, - notify_db, - notify_db_session, sample_api_key): 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) @@ -61,16 +58,9 @@ def test_expire_api_key_should_update_the_api_key_and_create_history_record(noti sorted_all_history[1].version = 2 -def test_get_api_key_should_raise_exception_when_api_key_does_not_exist(notify_api, - notify_db, - notify_db_session, - sample_service, - fake_uuid): - try: +def test_get_api_key_should_raise_exception_when_api_key_does_not_exist(sample_service, fake_uuid): + with pytest.raises(NoResultFound): get_model_api_keys(sample_service.id, id=fake_uuid) - fail("Should have thrown a NoResultFound exception") - except NoResultFound: - pass def test_should_return_api_key_for_service(notify_api, notify_db, notify_db_session, sample_api_key): @@ -78,43 +68,30 @@ def test_should_return_api_key_for_service(notify_api, notify_db, notify_db_sess assert api_key == sample_api_key -def test_should_return_unsigned_api_keys_for_service_id(notify_api, - notify_db, - notify_db_session, - sample_api_key): +def test_should_return_unsigned_api_keys_for_service_id(sample_api_key): unsigned_api_key = get_unsigned_secrets(sample_api_key.service_id) assert len(unsigned_api_key) == 1 assert sample_api_key.secret != unsigned_api_key[0] assert unsigned_api_key[0] == _get_secret(sample_api_key.secret) -def test_get_unsigned_secret_returns_key(notify_api, - notify_db, - notify_db_session, - sample_api_key): +def test_get_unsigned_secret_returns_key(sample_api_key): unsigned_api_key = get_unsigned_secret(sample_api_key.id) assert sample_api_key.secret != unsigned_api_key assert unsigned_api_key == _get_secret(sample_api_key.secret) -def test_should_not_allow_duplicate_key_names_per_service(notify_api, - notify_db, - notify_db_session, - sample_api_key, - fake_uuid): +def test_should_not_allow_duplicate_key_names_per_service(sample_api_key, fake_uuid): api_key = ApiKey(**{'id': fake_uuid, 'service': sample_api_key.service, 'name': sample_api_key.name, - 'created_by': sample_api_key.created_by}) - try: + 'created_by': sample_api_key.created_by, + 'key_type': KEY_TYPE_NORMAL}) + with pytest.raises(IntegrityError): save_model_api_key(api_key) - fail("should throw IntegrityError") - except: - pass -def test_save_api_key_should_not_create_new_service_history(notify_api, notify_db, notify_db_session, sample_service): - +def test_save_api_key_should_not_create_new_service_history(sample_service): from app.models import Service assert Service.query.count() == 1 @@ -122,7 +99,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 c673dfc30..35c354553 100644 --- a/tests/app/service/test_api_key_endpoints.py +++ b/tests/app/service/test_api_key_endpoints.py @@ -1,46 +1,60 @@ import json -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, 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 from tests.app.conftest import sample_user as create_user -def test_api_key_should_create_new_api_key_for_service(notify_api, notify_db, - notify_db_session, - sample_service): +def test_api_key_should_create_new_api_key_for_service(notify_api, 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.renew_api_key', service_id=sample_service.id), + 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' -def test_api_key_should_return_error_when_service_does_not_exist(notify_api, notify_db, notify_db_session, - sample_service): +def test_api_key_should_return_error_when_service_does_not_exist(notify_api, sample_service): with notify_api.test_request_context(): with notify_api.test_client() as client: import uuid missing_service_id = uuid.uuid4() auth_header = create_authorization_header() - response = client.post(url_for('service.renew_api_key', service_id=missing_service_id), + response = client.post(url_for('service.create_api_key', service_id=missing_service_id), headers=[('Content-Type', 'application/json'), auth_header]) assert response.status_code == 404 -def test_revoke_should_expire_api_key_for_service(notify_api, notify_db, notify_db_session, - sample_api_key): +def test_create_api_key_should_set_default_key_type_of_normal(notify_api, sample_service): + with notify_api.test_request_context(), notify_api.test_client() as client: + data = { + 'name': 'some secret name', + 'created_by': str(sample_service.created_by.id) + } + 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.one().key_type == KEY_TYPE_NORMAL + + +def test_revoke_should_expire_api_key_for_service(notify_api, sample_api_key): with notify_api.test_request_context(): with notify_api.test_client() as client: assert ApiKey.query.count() == 1 @@ -54,26 +68,29 @@ def test_revoke_should_expire_api_key_for_service(notify_api, notify_db, notify_ assert api_keys_for_service.expiry_date is not None -def test_api_key_should_create_multiple_new_api_key_for_service(notify_api, notify_db, - notify_db_session, - sample_service): +def test_api_key_should_create_multiple_new_api_key_for_service(notify_api, sample_service): 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.renew_api_key', service_id=sample_service.id), + 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.renew_api_key', service_id=sample_service.id), + 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 @@ -110,9 +127,7 @@ def test_get_api_keys_should_return_all_keys_for_service(notify_api, notify_db, assert len(json_resp['apiKeys']) == 3 -def test_get_api_keys_should_return_one_key_for_service(notify_api, notify_db, - notify_db_session, - sample_api_key): +def test_get_api_keys_should_return_one_key_for_service(notify_api, sample_api_key): with notify_api.test_request_context(): with notify_api.test_client() as client: auth_header = create_authorization_header() diff --git a/tests/app/service/test_url_for.py b/tests/app/service/test_url_for.py index 286b949d1..c0c44cd1d 100644 --- a/tests/app/service/test_url_for.py +++ b/tests/app/service/test_url_for.py @@ -34,7 +34,7 @@ def test_url_for_update_service(notify_api): assert str(url) == '/service/{}'.format(service_id) -def test_url_for_renew_api_key(notify_api): +def test_url_for_create_api_key(notify_api): with notify_api.test_request_context(): - url = url_for('service.renew_api_key', service_id=service_id) + url = url_for('service.create_api_key', service_id=service_id) assert str(url) == '/service/{}/api-key'.format(service_id) 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()