From b186cad0462f6e0b55eb83e8961c0b3e1fec8cb0 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 13 Jun 2017 15:27:13 +0100 Subject: [PATCH 1/6] Add a new table to store the api information for a service inbound sms message. Including: - url to push the inbound sms to - bearer_token to be added to the header of the request. The services will be expected to manage these properties. --- app/dao/service_inbound_api_dao.py | 10 ++++++ app/models.py | 13 ++++++++ .../versions/0098_service_inbound_api.py | 31 +++++++++++++++++++ .../app/dao/test_save_service_inbound_api.py | 20 ++++++++++++ 4 files changed, 74 insertions(+) create mode 100644 app/dao/service_inbound_api_dao.py create mode 100644 migrations/versions/0098_service_inbound_api.py create mode 100644 tests/app/dao/test_save_service_inbound_api.py diff --git a/app/dao/service_inbound_api_dao.py b/app/dao/service_inbound_api_dao.py new file mode 100644 index 000000000..447bbdf8a --- /dev/null +++ b/app/dao/service_inbound_api_dao.py @@ -0,0 +1,10 @@ +from app import db +from app.authentication.utils import generate_secret +from app.dao.dao_utils import transactional + + +@transactional +def save_service_inbound_api(service_inbound_api): + + service_inbound_api.bearer_token = generate_secret(service_inbound_api.bearer_token) + db.session.add(service_inbound_api) diff --git a/app/models.py b/app/models.py index 4b8213a4a..d5f8831dc 100644 --- a/app/models.py +++ b/app/models.py @@ -295,6 +295,19 @@ class ServiceWhitelist(db.Model): return 'Recipient {} of type: {}'.format(self.recipient, self.recipient_type) +class ServiceInboundApi(db.Model): + __tablename__ = 'service_inbound_api' + id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) + service_id = db.Column(UUID(as_uuid=True), db.ForeignKey('services.id'), index=True, nullable=False) + service = db.relationship('Service') + url = db.Column(db.String(255), nullable=False) + bearer_token = db.Column(db.String(255), nullable=False) + + @property + def unsigned_bearer_token(self): + return get_secret(self.bearer_token) + + class ApiKey(db.Model, Versioned): __tablename__ = 'api_keys' diff --git a/migrations/versions/0098_service_inbound_api.py b/migrations/versions/0098_service_inbound_api.py new file mode 100644 index 000000000..223ef1de8 --- /dev/null +++ b/migrations/versions/0098_service_inbound_api.py @@ -0,0 +1,31 @@ +"""empty message + +Revision ID: 0098_service_inbound_api +Revises: 0097_notnull_inbound_provider +Create Date: 2017-06-13 15:02:33.609656 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +revision = '0098_service_inbound_api' +down_revision = '0097_notnull_inbound_provider' + + +def upgrade(): + op.create_table( + 'service_inbound_api', + sa.Column('id', postgresql.UUID(as_uuid=True), nullable=False), + sa.Column('service_id', postgresql.UUID(as_uuid=True), nullable=False), + sa.Column('url', sa.String, nullable=False), + sa.Column('bearer_token', sa.String, nullable=False), + + sa.ForeignKeyConstraint(['service_id'], ['services.id'], ), + sa.PrimaryKeyConstraint('id') + ) + op.create_index(op.f('ix_service_inbound_api_id'), 'service_inbound_api', ['service_id'], unique=False) + + +def downgrade(): + op.drop_table("service_inbound_api") diff --git a/tests/app/dao/test_save_service_inbound_api.py b/tests/app/dao/test_save_service_inbound_api.py new file mode 100644 index 000000000..f24222fe9 --- /dev/null +++ b/tests/app/dao/test_save_service_inbound_api.py @@ -0,0 +1,20 @@ +from app.dao.service_inbound_api_dao import save_service_inbound_api +from app.models import ServiceInboundApi + + +def test_save_service_inbound_api(sample_service): + service_inbound_api = ServiceInboundApi( + service_id=sample_service.id, + url="https::/some_service/inbound_messages", + bearer_token="some_unique_string" + ) + + save_service_inbound_api(service_inbound_api) + + results = ServiceInboundApi.query.all() + assert len(results) == 1 + assert results[0].id + assert results[0].service_id == sample_service.id + assert results[0].url == "https::/some_service/inbound_messages" + assert results[0].unsigned_bearer_token == "some_unique_string" + assert results[0].bearer_token != "some_unique_string" From 3fdd180515c3acf25ef3c2b799baa06c718a2a11 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 13 Jun 2017 18:00:25 +0100 Subject: [PATCH 2/6] New endpoint to set ServiceInboundApi data. --- app/service/rest.py | 12 ++++++++- .../app/dao/test_save_service_inbound_api.py | 16 ++++++++++++ tests/app/service/test_rest.py | 25 ++++++++++++++++--- 3 files changed, 49 insertions(+), 4 deletions(-) diff --git a/app/service/rest.py b/app/service/rest.py index 5c0dfae6a..d1f60d56f 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -20,6 +20,7 @@ from app.dao.api_key_dao import ( expire_api_key) from app.dao.date_util import get_financial_year from app.dao.notification_usage_dao import get_total_billable_units_for_sent_sms_notifications_in_date_range +from app.dao.service_inbound_api_dao import save_service_inbound_api from app.dao.services_dao import ( dao_fetch_service_by_id, dao_fetch_all_services, @@ -49,7 +50,7 @@ from app.errors import ( InvalidRequest, register_errors ) -from app.models import Service +from app.models import Service, ServiceInboundApi from app.service import statistics from app.service.utils import get_whitelist_objects from app.service.sender import send_notification_to_service_users @@ -531,3 +532,12 @@ def get_yearly_monthly_usage(service_id): return json.dumps(json_results) except TypeError: return jsonify(result='error', message='No valid year provided'), 400 + + +@service_blueprint.route('//inbound-api', methods=['POST']) +def set_service_inbound_api(service_id): + data = request.get_json() + data["service_id"] = service_id + save_service_inbound_api(ServiceInboundApi(**data)) + + return jsonify(data="Service inbound api data saved"), 201 diff --git a/tests/app/dao/test_save_service_inbound_api.py b/tests/app/dao/test_save_service_inbound_api.py index f24222fe9..3c6f03b9e 100644 --- a/tests/app/dao/test_save_service_inbound_api.py +++ b/tests/app/dao/test_save_service_inbound_api.py @@ -1,3 +1,8 @@ +import uuid + +import pytest +from sqlalchemy.exc import SQLAlchemyError + from app.dao.service_inbound_api_dao import save_service_inbound_api from app.models import ServiceInboundApi @@ -18,3 +23,14 @@ def test_save_service_inbound_api(sample_service): assert results[0].url == "https::/some_service/inbound_messages" assert results[0].unsigned_bearer_token == "some_unique_string" assert results[0].bearer_token != "some_unique_string" + + +def test_save_service_inbound_api_fails_if_service_doesnot_exist(notify_db, notify_db_session): + service_inbound_api = ServiceInboundApi( + service_id=uuid.uuid4(), + url="https::/some_service/inbound_messages", + bearer_token="some_unique_string" + ) + + with pytest.raises(SQLAlchemyError): + save_service_inbound_api(service_inbound_api) diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 70622bac1..8b7f6f575 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -14,8 +14,8 @@ from app.models import ( Organisation, Rate, Service, ServicePermission, User, KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST, EMAIL_TYPE, SMS_TYPE, LETTER_TYPE, INTERNATIONAL_SMS_TYPE, INBOUND_SMS_TYPE, - DVLA_ORG_LAND_REGISTRY -) + DVLA_ORG_LAND_REGISTRY, + ServiceInboundApi) from tests import create_authorization_header from tests.app.db import create_template from tests.app.conftest import ( @@ -27,7 +27,6 @@ from tests.app.conftest import ( ) from tests.app.db import create_user -from tests.conftest import set_config_values def test_get_service_list(client, service_factory): @@ -2149,3 +2148,23 @@ def test_search_for_notification_by_to_field_returns_content( assert notifications[0]['id'] == str(notification.id) assert notifications[0]['to'] == '+447700900855' assert notifications[0]['body'] == 'Hello Foo\nYour thing is due soon' + + +def test_set_service_inbound_api(client, sample_service): + data = { + "url": "https://some_service/inbound-sms", + "bearer_token": "some-unique-string" + } + response = client.post( + '/service/{}/inbound-api'.format(sample_service.id), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), create_authorization_header()] + ) + assert response.status_code == 201 + + api_data = ServiceInboundApi.query.all() + assert len(api_data) == 1 + assert api_data[0].service_id == sample_service.id + assert api_data[0].url == "https://some_service/inbound-sms" + assert api_data[0].unsigned_bearer_token == "some-unique-string" + assert api_data[0].bearer_token != "some-unique-string" From 828d5cd493b292d569a552a341c7b1712b79ded8 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 15 Jun 2017 11:32:51 +0100 Subject: [PATCH 3/6] New table to store the inbound api information for a service. The table is versioned. There is a new endpoint to create the inbound api and one to update it. --- app/dao/service_inbound_api_dao.py | 22 +++- app/models.py | 17 ++- app/schema_validation/__init__.py | 10 +- app/schema_validation/definitions.py | 10 ++ app/service/rest.py | 35 +++++- app/service/service_inbound_api_schema.py | 27 ++++ .../versions/0098_service_inbound_api.py | 44 +++++-- .../app/dao/test_save_service_inbound_api.py | 36 ------ tests/app/dao/test_service_inbound_api_dao.py | 116 ++++++++++++++++++ tests/app/db.py | 19 ++- tests/app/service/test_rest.py | 73 +++++++++-- tests/app/service/test_schema.py | 45 +++++++ 12 files changed, 384 insertions(+), 70 deletions(-) create mode 100644 app/service/service_inbound_api_schema.py delete mode 100644 tests/app/dao/test_save_service_inbound_api.py create mode 100644 tests/app/dao/test_service_inbound_api_dao.py create mode 100644 tests/app/service/test_schema.py diff --git a/app/dao/service_inbound_api_dao.py b/app/dao/service_inbound_api_dao.py index 447bbdf8a..db694b653 100644 --- a/app/dao/service_inbound_api_dao.py +++ b/app/dao/service_inbound_api_dao.py @@ -1,10 +1,26 @@ -from app import db +from datetime import datetime + +from app import db, create_uuid from app.authentication.utils import generate_secret -from app.dao.dao_utils import transactional +from app.dao.dao_utils import transactional, version_class +from app.models import ServiceInboundApi @transactional +@version_class(ServiceInboundApi) def save_service_inbound_api(service_inbound_api): - + service_inbound_api.id = create_uuid() + service_inbound_api.created_at == datetime.utcnow() service_inbound_api.bearer_token = generate_secret(service_inbound_api.bearer_token) db.session.add(service_inbound_api) + + +@transactional +@version_class(ServiceInboundApi) +def reset_service_inbound_api(service_inbound_api): + db.session.add(service_inbound_api) + + +def get_service_inbound_api(service_inbound_api_id, service_id): + return ServiceInboundApi.query.filter_by(id=service_inbound_api_id, + service_id=service_id).one() diff --git a/app/models.py b/app/models.py index d5f8831dc..ae057f6e6 100644 --- a/app/models.py +++ b/app/models.py @@ -295,18 +295,33 @@ class ServiceWhitelist(db.Model): return 'Recipient {} of type: {}'.format(self.recipient, self.recipient_type) -class ServiceInboundApi(db.Model): +class ServiceInboundApi(db.Model, Versioned): __tablename__ = 'service_inbound_api' id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) service_id = db.Column(UUID(as_uuid=True), db.ForeignKey('services.id'), index=True, nullable=False) service = db.relationship('Service') url = db.Column(db.String(255), nullable=False) bearer_token = db.Column(db.String(255), nullable=False) + created_at = db.Column(db.DateTime, default=datetime.datetime.utcnow, nullable=False) + updated_at = db.Column(db.DateTime, nullable=True) + updated_by = db.relationship('User') + updated_by_id = db.Column(UUID(as_uuid=True), db.ForeignKey('users.id'), index=True, nullable=False) @property def unsigned_bearer_token(self): return get_secret(self.bearer_token) + def serialize(self): + return { + "id": self.id, + "service_id": self.service_id, + "url": self.url, + "bearer_token": self.bearer_token, + "updated_by_id": self.updated_by_id, + "created_at": self.created_at.strftime(DATETIME_FORMAT), + "updated_at": self.updated_at.strftime(DATETIME_FORMAT) if self.updated_at else None + } + class ApiKey(db.Model, Versioned): __tablename__ = 'api_keys' diff --git a/app/schema_validation/__init__.py b/app/schema_validation/__init__.py index 9202458eb..4a443391f 100644 --- a/app/schema_validation/__init__.py +++ b/app/schema_validation/__init__.py @@ -53,12 +53,20 @@ def build_error_message(errors): fields.append({"error": "ValidationError", "message": field}) message = { "status_code": 400, - "errors": fields + "errors": unique_errors(fields) } return json.dumps(message) +def unique_errors(dups): + unique = [] + for x in dups: + if x not in unique: + unique.append(x) + return unique + + def __format_message(e): def get_path(e): error_path = None diff --git a/app/schema_validation/definitions.py b/app/schema_validation/definitions.py index 3841b64a8..f0fdc9356 100644 --- a/app/schema_validation/definitions.py +++ b/app/schema_validation/definitions.py @@ -18,3 +18,13 @@ personalisation = { "code": "1001", # yet to be implemented "link": "link to our error documentation not yet implemented" } + + +https_url = { + "type": "string", + "format": "uri", + "pattern": "^https.*", + "validationMessage": "is not a valid https url", + "code": "1001", # yet to be implemented + "link": "link to our error documentation not yet implemented" +} diff --git a/app/service/rest.py b/app/service/rest.py index d1f60d56f..4f4da5371 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -11,6 +11,7 @@ from flask import ( from sqlalchemy.orm.exc import NoResultFound from app import redis_store +from app.authentication.utils import generate_secret from app.dao import notification_usage_dao, notifications_dao from app.dao.dao_utils import dao_rollback from app.dao.api_key_dao import ( @@ -20,7 +21,11 @@ from app.dao.api_key_dao import ( expire_api_key) from app.dao.date_util import get_financial_year from app.dao.notification_usage_dao import get_total_billable_units_for_sent_sms_notifications_in_date_range -from app.dao.service_inbound_api_dao import save_service_inbound_api +from app.dao.service_inbound_api_dao import ( + save_service_inbound_api, + reset_service_inbound_api, + get_service_inbound_api +) from app.dao.services_dao import ( dao_fetch_service_by_id, dao_fetch_all_services, @@ -51,7 +56,9 @@ from app.errors import ( register_errors ) from app.models import Service, ServiceInboundApi +from app.schema_validation import validate from app.service import statistics +from app.service.service_inbound_api_schema import service_inbound_api, update_service_inbound_api_schema from app.service.utils import get_whitelist_objects from app.service.sender import send_notification_to_service_users from app.schemas import ( @@ -535,9 +542,29 @@ def get_yearly_monthly_usage(service_id): @service_blueprint.route('//inbound-api', methods=['POST']) -def set_service_inbound_api(service_id): +def create_service_inbound_api(service_id): data = request.get_json() + validate(data, service_inbound_api) data["service_id"] = service_id - save_service_inbound_api(ServiceInboundApi(**data)) + inbound_api = ServiceInboundApi(**data) + save_service_inbound_api(inbound_api) - return jsonify(data="Service inbound api data saved"), 201 + return jsonify(data=inbound_api.serialize()), 201 + + +@service_blueprint.route('//inbound-api/', methods=['POST']) +def update_service_inbound_api(service_id, id): + data = request.get_json() + validate(data, update_service_inbound_api_schema) + + to_update = get_service_inbound_api(id, service_id) + + if data.get("url", None): + to_update.url = data["url"] + if data.get("bearer_token", None): + to_update.bearer_token = generate_secret(data["bearer_token"]) + to_update.updated_by_id = data["updated_by_id"] + to_update.updated_at = datetime.utcnow() + + reset_service_inbound_api(to_update) + return jsonify(data=to_update.serialize()), 200 diff --git a/app/service/service_inbound_api_schema.py b/app/service/service_inbound_api_schema.py new file mode 100644 index 000000000..88978c949 --- /dev/null +++ b/app/service/service_inbound_api_schema.py @@ -0,0 +1,27 @@ +from app.schema_validation.definitions import uuid, https_url + +service_inbound_api = { + "$schema": "http://json-schema.org/draft-04/schema#", + "description": "POST service inbound api schema", + "type": "object", + "title": "Create service inbound api", + "properties": { + "url": https_url, + "bearer_token": {"type": "string", "minLength": 10}, + "updated_by_id": uuid + }, + "required": ["url", "bearer_token", "updated_by_id"] +} + +update_service_inbound_api_schema = { + "$schema": "http://json-schema.org/draft-04/schema#", + "description": "POST service inbound api schema", + "type": "object", + "title": "Create service inbound api", + "properties": { + "url": https_url, + "bearer_token": {"type": "string", "minLength": 10}, + "updated_by_id": uuid + }, + "required": ["updated_by_id"] +} diff --git a/migrations/versions/0098_service_inbound_api.py b/migrations/versions/0098_service_inbound_api.py index 223ef1de8..3dd84b64b 100644 --- a/migrations/versions/0098_service_inbound_api.py +++ b/migrations/versions/0098_service_inbound_api.py @@ -14,18 +14,40 @@ down_revision = '0097_notnull_inbound_provider' def upgrade(): - op.create_table( - 'service_inbound_api', - sa.Column('id', postgresql.UUID(as_uuid=True), nullable=False), - sa.Column('service_id', postgresql.UUID(as_uuid=True), nullable=False), - sa.Column('url', sa.String, nullable=False), - sa.Column('bearer_token', sa.String, nullable=False), - - sa.ForeignKeyConstraint(['service_id'], ['services.id'], ), - sa.PrimaryKeyConstraint('id') + op.create_table('service_inbound_api_history', + sa.Column('id', postgresql.UUID(as_uuid=True), nullable=False), + sa.Column('service_id', postgresql.UUID(as_uuid=True), nullable=False), + sa.Column('url', sa.String(length=255), nullable=False), + sa.Column('bearer_token', sa.String(length=255), nullable=False), + sa.Column('created_at', sa.DateTime(), nullable=False), + sa.Column('updated_at', sa.DateTime(), nullable=True), + sa.Column('updated_by_id', postgresql.UUID(as_uuid=True), nullable=False), + sa.Column('version', sa.Integer(), autoincrement=False, nullable=False), + sa.PrimaryKeyConstraint('id', 'version') ) - op.create_index(op.f('ix_service_inbound_api_id'), 'service_inbound_api', ['service_id'], unique=False) + op.create_index(op.f('ix_service_inbound_api_history_service_id'), 'service_inbound_api_history', ['service_id'], unique=False) + op.create_index(op.f('ix_service_inbound_api_history_updated_by_id'), 'service_inbound_api_history', ['updated_by_id'], unique=False) + op.create_table('service_inbound_api', + sa.Column('id', postgresql.UUID(as_uuid=True), nullable=False), + sa.Column('service_id', postgresql.UUID(as_uuid=True), nullable=False), + sa.Column('url', sa.String(length=255), nullable=False), + sa.Column('bearer_token', sa.String(length=255), nullable=False), + sa.Column('created_at', sa.DateTime(), nullable=False), + sa.Column('updated_at', sa.DateTime(), nullable=True), + sa.Column('updated_by_id', postgresql.UUID(as_uuid=True), nullable=False), + sa.Column('version', sa.Integer(), nullable=False), + sa.ForeignKeyConstraint(['service_id'], ['services.id'], ), + sa.ForeignKeyConstraint(['updated_by_id'], ['users.id'], ), + sa.PrimaryKeyConstraint('id') + ) + op.create_index(op.f('ix_service_inbound_api_service_id'), 'service_inbound_api', ['service_id'], unique=False) + op.create_index(op.f('ix_service_inbound_api_updated_by_id'), 'service_inbound_api', ['updated_by_id'], unique=False) def downgrade(): - op.drop_table("service_inbound_api") + op.drop_index(op.f('ix_service_inbound_api_updated_by_id'), table_name='service_inbound_api') + op.drop_index(op.f('ix_service_inbound_api_service_id'), table_name='service_inbound_api') + op.drop_table('service_inbound_api') + op.drop_index(op.f('ix_service_inbound_api_history_updated_by_id'), table_name='service_inbound_api_history') + op.drop_index(op.f('ix_service_inbound_api_history_service_id'), table_name='service_inbound_api_history') + op.drop_table('service_inbound_api_history') \ No newline at end of file diff --git a/tests/app/dao/test_save_service_inbound_api.py b/tests/app/dao/test_save_service_inbound_api.py deleted file mode 100644 index 3c6f03b9e..000000000 --- a/tests/app/dao/test_save_service_inbound_api.py +++ /dev/null @@ -1,36 +0,0 @@ -import uuid - -import pytest -from sqlalchemy.exc import SQLAlchemyError - -from app.dao.service_inbound_api_dao import save_service_inbound_api -from app.models import ServiceInboundApi - - -def test_save_service_inbound_api(sample_service): - service_inbound_api = ServiceInboundApi( - service_id=sample_service.id, - url="https::/some_service/inbound_messages", - bearer_token="some_unique_string" - ) - - save_service_inbound_api(service_inbound_api) - - results = ServiceInboundApi.query.all() - assert len(results) == 1 - assert results[0].id - assert results[0].service_id == sample_service.id - assert results[0].url == "https::/some_service/inbound_messages" - assert results[0].unsigned_bearer_token == "some_unique_string" - assert results[0].bearer_token != "some_unique_string" - - -def test_save_service_inbound_api_fails_if_service_doesnot_exist(notify_db, notify_db_session): - service_inbound_api = ServiceInboundApi( - service_id=uuid.uuid4(), - url="https::/some_service/inbound_messages", - bearer_token="some_unique_string" - ) - - with pytest.raises(SQLAlchemyError): - save_service_inbound_api(service_inbound_api) diff --git a/tests/app/dao/test_service_inbound_api_dao.py b/tests/app/dao/test_service_inbound_api_dao.py new file mode 100644 index 000000000..a362425cc --- /dev/null +++ b/tests/app/dao/test_service_inbound_api_dao.py @@ -0,0 +1,116 @@ +import uuid + +import pytest +from sqlalchemy.exc import SQLAlchemyError + +from app.authentication.utils import get_secret +from app.dao.service_inbound_api_dao import save_service_inbound_api, reset_service_inbound_api, \ + get_service_inbound_api +from app.models import ServiceInboundApi + + +def test_save_service_inbound_api(sample_service): + service_inbound_api = ServiceInboundApi( + service_id=sample_service.id, + url="https::/some_service/inbound_messages", + bearer_token="some_unique_string", + updated_by_id=sample_service.users[0].id + ) + + save_service_inbound_api(service_inbound_api) + + results = ServiceInboundApi.query.all() + assert len(results) == 1 + inbound_api = results[0] + assert inbound_api.id + assert inbound_api.service_id == sample_service.id + assert inbound_api.updated_by_id == sample_service.users[0].id + assert inbound_api.url == "https::/some_service/inbound_messages" + assert inbound_api.unsigned_bearer_token == "some_unique_string" + assert inbound_api.bearer_token != "some_unique_string" + assert not inbound_api.updated_at + + versioned = ServiceInboundApi.get_history_model().query.filter_by(id=inbound_api.id).one() + assert versioned.id == inbound_api.id + assert versioned.service_id == sample_service.id + assert versioned.updated_by_id == sample_service.users[0].id + assert versioned.url == "https::/some_service/inbound_messages" + assert versioned.bearer_token != "some_unique_string" + assert not versioned.updated_at + assert versioned.version == 1 + + +def test_save_service_inbound_api_fails_if_service_does_not_exist(notify_db, notify_db_session): + service_inbound_api = ServiceInboundApi( + service_id=uuid.uuid4(), + url="https::/some_service/inbound_messages", + bearer_token="some_unique_string", + updated_by_id=uuid.uuid4() + ) + + with pytest.raises(SQLAlchemyError): + save_service_inbound_api(service_inbound_api) + + +def test_update_service_inbound_api(sample_service): + service_inbound_api = ServiceInboundApi( + service_id=sample_service.id, + url="https::/some_service/inbound_messages", + bearer_token="some_unique_string", + updated_by_id=sample_service.users[0].id + ) + + save_service_inbound_api(service_inbound_api) + results = ServiceInboundApi.query.all() + assert len(results) == 1 + saved_inbound_api = results[0] + + updated_inbound_api = saved_inbound_api + updated_inbound_api.url = "https::/some_service/changed_url" + + reset_service_inbound_api(updated_inbound_api) + updated_results = ServiceInboundApi.query.all() + assert len(updated_results) == 1 + updated = updated_results[0] + assert updated.id + assert updated.service_id == sample_service.id + assert updated.updated_by_id == sample_service.users[0].id + assert updated.url == "https::/some_service/changed_url" + assert updated.unsigned_bearer_token == "some_unique_string" + assert updated.bearer_token != "some_unique_string" + assert updated.updated_at + + versioned_results = ServiceInboundApi.get_history_model().query.filter_by(id=saved_inbound_api.id).all() + assert len(versioned_results) == 2 + for x in versioned_results: + if x.version == 1: + assert x.url == "https::/some_service/inbound_messages" + assert not x.updated_at + elif x.version == 2: + assert x.url == "https::/some_service/changed_url" + assert x.updated_at + else: + pytest.fail("version should not exist") + assert x.id + assert x.service_id == sample_service.id + assert x.updated_by_id == sample_service.users[0].id + assert get_secret(x.bearer_token) == "some_unique_string" + + +def test_get_service_inbound_api(sample_service): + service_inbound_api = ServiceInboundApi( + service_id=sample_service.id, + url="https::/some_service/inbound_messages", + bearer_token="some_unique_string", + updated_by_id=sample_service.users[0].id + ) + save_service_inbound_api(service_inbound_api) + + inbound_api = get_service_inbound_api(service_inbound_api.id, sample_service.id) + assert inbound_api.id + assert inbound_api.service_id == sample_service.id + assert inbound_api.updated_by_id == sample_service.users[0].id + assert inbound_api.url == "https::/some_service/inbound_messages" + assert inbound_api.unsigned_bearer_token == "some_unique_string" + assert inbound_api.bearer_token != "some_unique_string" + assert not inbound_api.updated_at diff --git a/tests/app/db.py b/tests/app/db.py index 6fff1612b..c212a6612 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -2,10 +2,9 @@ from datetime import datetime import uuid -from app.dao.inbound_sms_dao import dao_create_inbound_sms from app.dao.jobs_dao import dao_create_job +from app.dao.service_inbound_api_dao import save_service_inbound_api from app.models import ( - InboundSms, Service, User, Template, @@ -17,7 +16,7 @@ from app.models import ( EMAIL_TYPE, SMS_TYPE, KEY_TYPE_NORMAL, -) + ServiceInboundApi) from app.dao.users_dao import save_model_user from app.dao.notifications_dao import dao_create_notification, dao_created_scheduled_notification from app.dao.templates_dao import dao_create_template @@ -212,3 +211,17 @@ def create_inbound_sms( ) dao_create_inbound_sms(inbound) return inbound + + +def create_service_inbound_api( + service, + url="https://something.com", + bearer_token="some_super_secret", +): + service_inbound_api = ServiceInboundApi(service_id=service.id, + url=url, + bearer_token=bearer_token, + updated_by_id=service.users[0].id + ) + save_service_inbound_api(service_inbound_api) + return service_inbound_api diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 8b7f6f575..678745e42 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -8,16 +8,17 @@ import pytest from flask import url_for, current_app from freezegun import freeze_time +from app.authentication.utils import get_secret from app.dao.users_dao import save_model_user from app.dao.services_dao import dao_remove_user_from_service from app.models import ( Organisation, Rate, Service, ServicePermission, User, KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST, EMAIL_TYPE, SMS_TYPE, LETTER_TYPE, INTERNATIONAL_SMS_TYPE, INBOUND_SMS_TYPE, - DVLA_ORG_LAND_REGISTRY, - ServiceInboundApi) + DVLA_ORG_LAND_REGISTRY +) from tests import create_authorization_header -from tests.app.db import create_template +from tests.app.db import create_template, create_service_inbound_api from tests.app.conftest import ( sample_service as create_service, sample_user_service_permission as create_user_service_permission, @@ -2150,10 +2151,11 @@ def test_search_for_notification_by_to_field_returns_content( assert notifications[0]['body'] == 'Hello Foo\nYour thing is due soon' -def test_set_service_inbound_api(client, sample_service): +def test_create_service_inbound_api(client, sample_service): data = { "url": "https://some_service/inbound-sms", - "bearer_token": "some-unique-string" + "bearer_token": "some-unique-string", + "updated_by_id": str(sample_service.users[0].id) } response = client.post( '/service/{}/inbound-api'.format(sample_service.id), @@ -2162,9 +2164,58 @@ def test_set_service_inbound_api(client, sample_service): ) assert response.status_code == 201 - api_data = ServiceInboundApi.query.all() - assert len(api_data) == 1 - assert api_data[0].service_id == sample_service.id - assert api_data[0].url == "https://some_service/inbound-sms" - assert api_data[0].unsigned_bearer_token == "some-unique-string" - assert api_data[0].bearer_token != "some-unique-string" + resp_json = json.loads(response.get_data(as_text=True))["data"] + assert resp_json["id"] + assert resp_json["service_id"] == str(sample_service.id) + assert resp_json["url"] == "https://some_service/inbound-sms" + assert resp_json["bearer_token"] != "some-unique-string" # returned encrypted + assert resp_json["updated_by_id"] == str(sample_service.users[0].id) + assert resp_json["created_at"] + assert not resp_json["updated_at"] + + +def test_set_service_inbound_api_raises_500_when_service_does_not_exist(client): + data = { + "url": "https://some_service/inbound-sms", + "bearer_token": "some-unique-string", + "updated_by_id": str(uuid.uuid4()) + } + response = client.post( + '/service/{}/inbound-api'.format(uuid.uuid4()), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), create_authorization_header()] + ) + assert response.status_code == 500 + + +def test_update_service_inbound_api_updates_url(client, sample_service): + service_inbound_api = create_service_inbound_api(service=sample_service, + url="https://original_url.com") + + data = { + "url": "https://another_url.com", + "updated_by_id": str(sample_service.users[0].id) + } + response = client.post("/service/{}/inbound-api/{}".format(sample_service.id, service_inbound_api.id), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), create_authorization_header()]) + assert response.status_code == 200 + resp_json = json.loads(response.get_data(as_text=True))["data"] + assert resp_json["url"] == "https://another_url.com" + assert service_inbound_api.url == "https://another_url.com" + + +def test_update_service_inbound_api_updates_bearer_token(client, sample_service): + service_inbound_api = create_service_inbound_api(service=sample_service, + bearer_token="some_super_secret") + data = { + "bearer_token": "different_token", + "updated_by_id": str(sample_service.users[0].id) + } + response = client.post("/service/{}/inbound-api/{}".format(sample_service.id, service_inbound_api.id), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), create_authorization_header()]) + assert response.status_code == 200 + resp_json = json.loads(response.get_data(as_text=True))["data"] + assert get_secret(resp_json["bearer_token"]) == "different_token" + assert service_inbound_api.unsigned_bearer_token == "different_token" diff --git a/tests/app/service/test_schema.py b/tests/app/service/test_schema.py new file mode 100644 index 000000000..23fa186de --- /dev/null +++ b/tests/app/service/test_schema.py @@ -0,0 +1,45 @@ +import json +import uuid + +import pytest +from jsonschema import ValidationError + +from app.schema_validation import validate +from app.service.service_inbound_api_schema import service_inbound_api + + +def test_service_inbound_api_schema_validates(): + under_test = {"url": "https://some_url.for_service", + "bearer_token": "something_ten_chars", + "updated_by_id": str(uuid.uuid4()) + } + + validated = validate(under_test, service_inbound_api) + assert validated == under_test + + +@pytest.mark.parametrize("url", ["not a url", "https not a url", "http://valid.com"]) +def test_service_inbound_api_schema_errors_for_url_not_valid_url(url): + under_test = {"url": url, + "bearer_token": "something_ten_chars", + "updated_by_id": str(uuid.uuid4()) + } + + with pytest.raises(ValidationError) as e: + validate(under_test, service_inbound_api) + errors = json.loads(str(e.value)).get('errors') + assert len(errors) == 1 + assert errors[0]['message'] == "url is not a valid https url" + + +def test_service_inbound_api_schema_bearer_token_under_ten_char(): + under_test = {"url": "https://some_url.for_service", + "bearer_token": "shorty", + "updated_by_id": str(uuid.uuid4()) + } + + with pytest.raises(ValidationError) as e: + validate(under_test, service_inbound_api) + errors = json.loads(str(e.value)).get('errors') + assert len(errors) == 1 + assert errors[0]['message'] == "bearer_token shorty is too short" From effb99dca86c3b290058b935e9ddf90e5fab2c2f Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 15 Jun 2017 16:19:12 +0100 Subject: [PATCH 4/6] Add fetch request for service inbound api. Add unique constraint on service_id for service_inbound_api. --- app/dao/service_inbound_api_dao.py | 2 +- app/models.py | 10 +++---- app/service/rest.py | 27 ++++++++++++++++--- .../versions/0098_service_inbound_api.py | 13 +++++---- tests/app/service/test_rest.py | 10 +++++++ 5 files changed, 47 insertions(+), 15 deletions(-) diff --git a/app/dao/service_inbound_api_dao.py b/app/dao/service_inbound_api_dao.py index db694b653..53691f56d 100644 --- a/app/dao/service_inbound_api_dao.py +++ b/app/dao/service_inbound_api_dao.py @@ -23,4 +23,4 @@ def reset_service_inbound_api(service_inbound_api): def get_service_inbound_api(service_inbound_api_id, service_id): return ServiceInboundApi.query.filter_by(id=service_inbound_api_id, - service_id=service_id).one() + service_id=service_id).first() diff --git a/app/models.py b/app/models.py index ae057f6e6..baa03dc42 100644 --- a/app/models.py +++ b/app/models.py @@ -298,8 +298,8 @@ class ServiceWhitelist(db.Model): class ServiceInboundApi(db.Model, Versioned): __tablename__ = 'service_inbound_api' id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) - service_id = db.Column(UUID(as_uuid=True), db.ForeignKey('services.id'), index=True, nullable=False) - service = db.relationship('Service') + service_id = db.Column(UUID(as_uuid=True), db.ForeignKey('services.id'), index=True, nullable=False, unique=True) + service = db.relationship('Service', backref='inbound_api') url = db.Column(db.String(255), nullable=False) bearer_token = db.Column(db.String(255), nullable=False) created_at = db.Column(db.DateTime, default=datetime.datetime.utcnow, nullable=False) @@ -313,11 +313,11 @@ class ServiceInboundApi(db.Model, Versioned): def serialize(self): return { - "id": self.id, - "service_id": self.service_id, + "id": str(self.id), + "service_id": str(self.service_id), "url": self.url, "bearer_token": self.bearer_token, - "updated_by_id": self.updated_by_id, + "updated_by_id": str(self.updated_by_id), "created_at": self.created_at.strftime(DATETIME_FORMAT), "updated_at": self.updated_at.strftime(DATETIME_FORMAT) if self.updated_at else None } diff --git a/app/service/rest.py b/app/service/rest.py index 4f4da5371..5fe831f35 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -8,6 +8,7 @@ from flask import ( current_app, Blueprint ) +from sqlalchemy.exc import SQLAlchemyError from sqlalchemy.orm.exc import NoResultFound from app import redis_store @@ -547,17 +548,28 @@ def create_service_inbound_api(service_id): validate(data, service_inbound_api) data["service_id"] = service_id inbound_api = ServiceInboundApi(**data) - save_service_inbound_api(inbound_api) + try: + save_service_inbound_api(inbound_api) + except SQLAlchemyError as e: + if hasattr(e, 'orig') and hasattr(e.orig, 'pgerror')and e.orig.pgerror\ + and ('duplicate key value violates unique constraint "ix_service_inbound_api_service_id"' + in e.orig.pgerror): + return jsonify( + result='error', + message={'name': ["You can only have one URL and bearer token for your service."]} + ), 400 + else: + raise e return jsonify(data=inbound_api.serialize()), 201 -@service_blueprint.route('//inbound-api/', methods=['POST']) -def update_service_inbound_api(service_id, id): +@service_blueprint.route('//inbound-api/', methods=['POST']) +def update_service_inbound_api(service_id, inbound_api_id): data = request.get_json() validate(data, update_service_inbound_api_schema) - to_update = get_service_inbound_api(id, service_id) + to_update = get_service_inbound_api(inbound_api_id, service_id) if data.get("url", None): to_update.url = data["url"] @@ -568,3 +580,10 @@ def update_service_inbound_api(service_id, id): reset_service_inbound_api(to_update) return jsonify(data=to_update.serialize()), 200 + + +@service_blueprint.route('//inbound-api/', methods=["GET"]) +def fetch_service_inbound_api(service_id, inbound_api_id): + inbound_api = get_service_inbound_api(inbound_api_id, service_id) + + return jsonify(data=inbound_api.serialize()), 200 diff --git a/migrations/versions/0098_service_inbound_api.py b/migrations/versions/0098_service_inbound_api.py index 3dd84b64b..1d0a54b6d 100644 --- a/migrations/versions/0098_service_inbound_api.py +++ b/migrations/versions/0098_service_inbound_api.py @@ -25,8 +25,10 @@ def upgrade(): sa.Column('version', sa.Integer(), autoincrement=False, nullable=False), sa.PrimaryKeyConstraint('id', 'version') ) - op.create_index(op.f('ix_service_inbound_api_history_service_id'), 'service_inbound_api_history', ['service_id'], unique=False) - op.create_index(op.f('ix_service_inbound_api_history_updated_by_id'), 'service_inbound_api_history', ['updated_by_id'], unique=False) + op.create_index(op.f('ix_service_inbound_api_history_service_id'), 'service_inbound_api_history', ['service_id'], + unique=False) + op.create_index(op.f('ix_service_inbound_api_history_updated_by_id'), 'service_inbound_api_history', + ['updated_by_id'], unique=False) op.create_table('service_inbound_api', sa.Column('id', postgresql.UUID(as_uuid=True), nullable=False), sa.Column('service_id', postgresql.UUID(as_uuid=True), nullable=False), @@ -35,13 +37,14 @@ def upgrade(): sa.Column('created_at', sa.DateTime(), nullable=False), sa.Column('updated_at', sa.DateTime(), nullable=True), sa.Column('updated_by_id', postgresql.UUID(as_uuid=True), nullable=False), - sa.Column('version', sa.Integer(), nullable=False), + sa.Column('version', sa.Integer(), nullable=False),\ sa.ForeignKeyConstraint(['service_id'], ['services.id'], ), sa.ForeignKeyConstraint(['updated_by_id'], ['users.id'], ), sa.PrimaryKeyConstraint('id') ) - op.create_index(op.f('ix_service_inbound_api_service_id'), 'service_inbound_api', ['service_id'], unique=False) - op.create_index(op.f('ix_service_inbound_api_updated_by_id'), 'service_inbound_api', ['updated_by_id'], unique=False) + op.create_index(op.f('ix_service_inbound_api_service_id'), 'service_inbound_api', ['service_id'], unique=True) + op.create_index(op.f('ix_service_inbound_api_updated_by_id'), 'service_inbound_api', ['updated_by_id'], + unique=False) def downgrade(): diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 678745e42..17bedcc77 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -2219,3 +2219,13 @@ def test_update_service_inbound_api_updates_bearer_token(client, sample_service) resp_json = json.loads(response.get_data(as_text=True))["data"] assert get_secret(resp_json["bearer_token"]) == "different_token" assert service_inbound_api.unsigned_bearer_token == "different_token" + + +def test_fetch_service_inbound_api(client, sample_service): + service_inbound_api = create_service_inbound_api(service=sample_service) + + response = client.get("/service/{}/inbound-api/{}".format(sample_service.id, service_inbound_api.id), + headers=[create_authorization_header()]) + + assert response.status_code == 200 + assert json.loads(response.get_data(as_text=True))["data"] == service_inbound_api.serialize() From 6202da7dea7c0c39e84c72feb6c7e49384304dca Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 19 Jun 2017 12:25:05 +0100 Subject: [PATCH 5/6] Update model to remove the string length restriction. Moved logic to the dao from the endpoint. --- app/dao/service_inbound_api_dao.py | 9 +++- app/models.py | 4 +- app/service/rest.py | 39 ++++++++------- .../versions/0098_service_inbound_api.py | 8 ++-- tests/app/dao/test_service_inbound_api_dao.py | 47 ++++++++++--------- tests/app/service/test_rest.py | 5 +- 6 files changed, 63 insertions(+), 49 deletions(-) diff --git a/app/dao/service_inbound_api_dao.py b/app/dao/service_inbound_api_dao.py index 53691f56d..8acf0070d 100644 --- a/app/dao/service_inbound_api_dao.py +++ b/app/dao/service_inbound_api_dao.py @@ -17,7 +17,14 @@ def save_service_inbound_api(service_inbound_api): @transactional @version_class(ServiceInboundApi) -def reset_service_inbound_api(service_inbound_api): +def reset_service_inbound_api(service_inbound_api, updated_by_id, url=None, bearer_token=None): + if url: + service_inbound_api.url = url + if bearer_token: + service_inbound_api.bearer_token = generate_secret(bearer_token) + service_inbound_api.updated_by_id = updated_by_id + service_inbound_api.updated_at = datetime.utcnow() + db.session.add(service_inbound_api) diff --git a/app/models.py b/app/models.py index baa03dc42..9c69d897e 100644 --- a/app/models.py +++ b/app/models.py @@ -300,8 +300,8 @@ class ServiceInboundApi(db.Model, Versioned): id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) service_id = db.Column(UUID(as_uuid=True), db.ForeignKey('services.id'), index=True, nullable=False, unique=True) service = db.relationship('Service', backref='inbound_api') - url = db.Column(db.String(255), nullable=False) - bearer_token = db.Column(db.String(255), nullable=False) + url = db.Column(db.String(), nullable=False) + bearer_token = db.Column(db.String(), nullable=False) created_at = db.Column(db.DateTime, default=datetime.datetime.utcnow, nullable=False) updated_at = db.Column(db.DateTime, nullable=True) updated_by = db.relationship('User') diff --git a/app/service/rest.py b/app/service/rest.py index 5fe831f35..b46cdbd83 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -551,15 +551,7 @@ def create_service_inbound_api(service_id): try: save_service_inbound_api(inbound_api) except SQLAlchemyError as e: - if hasattr(e, 'orig') and hasattr(e.orig, 'pgerror')and e.orig.pgerror\ - and ('duplicate key value violates unique constraint "ix_service_inbound_api_service_id"' - in e.orig.pgerror): - return jsonify( - result='error', - message={'name': ["You can only have one URL and bearer token for your service."]} - ), 400 - else: - raise e + return handle_sql_errror(e) return jsonify(data=inbound_api.serialize()), 201 @@ -571,14 +563,10 @@ def update_service_inbound_api(service_id, inbound_api_id): to_update = get_service_inbound_api(inbound_api_id, service_id) - if data.get("url", None): - to_update.url = data["url"] - if data.get("bearer_token", None): - to_update.bearer_token = generate_secret(data["bearer_token"]) - to_update.updated_by_id = data["updated_by_id"] - to_update.updated_at = datetime.utcnow() - - reset_service_inbound_api(to_update) + reset_service_inbound_api(service_inbound_api=to_update, + updated_by_id=data["updated_by_id"], + url=data.get("url", None), + bearer_token=data.get("bearer_token", None)) return jsonify(data=to_update.serialize()), 200 @@ -587,3 +575,20 @@ def fetch_service_inbound_api(service_id, inbound_api_id): inbound_api = get_service_inbound_api(inbound_api_id, service_id) return jsonify(data=inbound_api.serialize()), 200 + + +def handle_sql_errror(e): + if hasattr(e, 'orig') and hasattr(e.orig, 'pgerror') and e.orig.pgerror \ + and ('duplicate key value violates unique constraint "ix_service_inbound_api_service_id"' + in e.orig.pgerror): + return jsonify( + result='error', + message={'name': ["You can only have one URL and bearer token for your service."]} + ), 400 + elif hasattr(e, 'orig') and hasattr(e.orig, 'pgerror') and e.orig.pgerror \ + and ('insert or update on table "service_inbound_api" violates ' + 'foreign key constraint "service_inbound_api_service_id_fkey"' + in e.orig.pgerror): + return jsonify(result='error', message="No result found"), 404 + else: + raise e diff --git a/migrations/versions/0098_service_inbound_api.py b/migrations/versions/0098_service_inbound_api.py index 1d0a54b6d..3c2f96a43 100644 --- a/migrations/versions/0098_service_inbound_api.py +++ b/migrations/versions/0098_service_inbound_api.py @@ -17,8 +17,8 @@ def upgrade(): op.create_table('service_inbound_api_history', sa.Column('id', postgresql.UUID(as_uuid=True), nullable=False), sa.Column('service_id', postgresql.UUID(as_uuid=True), nullable=False), - sa.Column('url', sa.String(length=255), nullable=False), - sa.Column('bearer_token', sa.String(length=255), nullable=False), + sa.Column('url', sa.String(), nullable=False), + sa.Column('bearer_token', sa.String(), nullable=False), sa.Column('created_at', sa.DateTime(), nullable=False), sa.Column('updated_at', sa.DateTime(), nullable=True), sa.Column('updated_by_id', postgresql.UUID(as_uuid=True), nullable=False), @@ -32,8 +32,8 @@ def upgrade(): op.create_table('service_inbound_api', sa.Column('id', postgresql.UUID(as_uuid=True), nullable=False), sa.Column('service_id', postgresql.UUID(as_uuid=True), nullable=False), - sa.Column('url', sa.String(length=255), nullable=False), - sa.Column('bearer_token', sa.String(length=255), nullable=False), + sa.Column('url', sa.String(), nullable=False), + sa.Column('bearer_token', sa.String(), nullable=False), sa.Column('created_at', sa.DateTime(), nullable=False), sa.Column('updated_at', sa.DateTime(), nullable=True), sa.Column('updated_by_id', postgresql.UUID(as_uuid=True), nullable=False), diff --git a/tests/app/dao/test_service_inbound_api_dao.py b/tests/app/dao/test_service_inbound_api_dao.py index a362425cc..dde053427 100644 --- a/tests/app/dao/test_service_inbound_api_dao.py +++ b/tests/app/dao/test_service_inbound_api_dao.py @@ -4,15 +4,18 @@ import pytest from sqlalchemy.exc import SQLAlchemyError from app.authentication.utils import get_secret -from app.dao.service_inbound_api_dao import save_service_inbound_api, reset_service_inbound_api, \ +from app.dao.service_inbound_api_dao import ( + save_service_inbound_api, + reset_service_inbound_api, get_service_inbound_api +) from app.models import ServiceInboundApi def test_save_service_inbound_api(sample_service): service_inbound_api = ServiceInboundApi( service_id=sample_service.id, - url="https::/some_service/inbound_messages", + url="https://some_service/inbound_messages", bearer_token="some_unique_string", updated_by_id=sample_service.users[0].id ) @@ -22,28 +25,28 @@ def test_save_service_inbound_api(sample_service): results = ServiceInboundApi.query.all() assert len(results) == 1 inbound_api = results[0] - assert inbound_api.id + assert inbound_api.id is not None assert inbound_api.service_id == sample_service.id assert inbound_api.updated_by_id == sample_service.users[0].id - assert inbound_api.url == "https::/some_service/inbound_messages" + assert inbound_api.url == "https://some_service/inbound_messages" assert inbound_api.unsigned_bearer_token == "some_unique_string" assert inbound_api.bearer_token != "some_unique_string" - assert not inbound_api.updated_at + assert inbound_api.updated_at is None versioned = ServiceInboundApi.get_history_model().query.filter_by(id=inbound_api.id).one() assert versioned.id == inbound_api.id assert versioned.service_id == sample_service.id assert versioned.updated_by_id == sample_service.users[0].id - assert versioned.url == "https::/some_service/inbound_messages" + assert versioned.url == "https://some_service/inbound_messages" assert versioned.bearer_token != "some_unique_string" - assert not versioned.updated_at + assert versioned.updated_at is None assert versioned.version == 1 def test_save_service_inbound_api_fails_if_service_does_not_exist(notify_db, notify_db_session): service_inbound_api = ServiceInboundApi( service_id=uuid.uuid4(), - url="https::/some_service/inbound_messages", + url="https://some_service/inbound_messages", bearer_token="some_unique_string", updated_by_id=uuid.uuid4() ) @@ -55,7 +58,7 @@ def test_save_service_inbound_api_fails_if_service_does_not_exist(notify_db, not def test_update_service_inbound_api(sample_service): service_inbound_api = ServiceInboundApi( service_id=sample_service.id, - url="https::/some_service/inbound_messages", + url="https://some_service/inbound_messages", bearer_token="some_unique_string", updated_by_id=sample_service.users[0].id ) @@ -65,33 +68,31 @@ def test_update_service_inbound_api(sample_service): assert len(results) == 1 saved_inbound_api = results[0] - updated_inbound_api = saved_inbound_api - updated_inbound_api.url = "https::/some_service/changed_url" - - reset_service_inbound_api(updated_inbound_api) + reset_service_inbound_api(saved_inbound_api, updated_by_id=sample_service.users[0].id, + url="https://some_service/changed_url") updated_results = ServiceInboundApi.query.all() assert len(updated_results) == 1 updated = updated_results[0] - assert updated.id + assert updated.id is not None assert updated.service_id == sample_service.id assert updated.updated_by_id == sample_service.users[0].id - assert updated.url == "https::/some_service/changed_url" + assert updated.url == "https://some_service/changed_url" assert updated.unsigned_bearer_token == "some_unique_string" assert updated.bearer_token != "some_unique_string" - assert updated.updated_at + assert updated.updated_at is not None versioned_results = ServiceInboundApi.get_history_model().query.filter_by(id=saved_inbound_api.id).all() assert len(versioned_results) == 2 for x in versioned_results: if x.version == 1: - assert x.url == "https::/some_service/inbound_messages" + assert x.url == "https://some_service/inbound_messages" assert not x.updated_at elif x.version == 2: - assert x.url == "https::/some_service/changed_url" + assert x.url == "https://some_service/changed_url" assert x.updated_at else: pytest.fail("version should not exist") - assert x.id + assert x.id is not None assert x.service_id == sample_service.id assert x.updated_by_id == sample_service.users[0].id assert get_secret(x.bearer_token) == "some_unique_string" @@ -100,17 +101,17 @@ def test_update_service_inbound_api(sample_service): def test_get_service_inbound_api(sample_service): service_inbound_api = ServiceInboundApi( service_id=sample_service.id, - url="https::/some_service/inbound_messages", + url="https://some_service/inbound_messages", bearer_token="some_unique_string", updated_by_id=sample_service.users[0].id ) save_service_inbound_api(service_inbound_api) inbound_api = get_service_inbound_api(service_inbound_api.id, sample_service.id) - assert inbound_api.id + assert inbound_api.id is not None assert inbound_api.service_id == sample_service.id assert inbound_api.updated_by_id == sample_service.users[0].id - assert inbound_api.url == "https::/some_service/inbound_messages" + assert inbound_api.url == "https://some_service/inbound_messages" assert inbound_api.unsigned_bearer_token == "some_unique_string" assert inbound_api.bearer_token != "some_unique_string" - assert not inbound_api.updated_at + assert inbound_api.updated_at is None diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 17bedcc77..a0ec13621 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -2174,7 +2174,7 @@ def test_create_service_inbound_api(client, sample_service): assert not resp_json["updated_at"] -def test_set_service_inbound_api_raises_500_when_service_does_not_exist(client): +def test_set_service_inbound_api_raises_404_when_service_does_not_exist(client): data = { "url": "https://some_service/inbound-sms", "bearer_token": "some-unique-string", @@ -2185,7 +2185,8 @@ def test_set_service_inbound_api_raises_500_when_service_does_not_exist(client): data=json.dumps(data), headers=[('Content-Type', 'application/json'), create_authorization_header()] ) - assert response.status_code == 500 + assert response.status_code == 404 + assert json.loads(response.get_data(as_text=True))['message'] == 'No result found' def test_update_service_inbound_api_updates_url(client, sample_service): From 3a66027d6a669fef9336e9289626e22372d8d18a Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 19 Jun 2017 14:32:22 +0100 Subject: [PATCH 6/6] Refactor ApiKeys.secret and ServiceInboundApi.bearer_token to use the same encryption method and get rid of the duplicate code. --- app/authentication/auth.py | 2 +- app/authentication/utils.py | 12 -------- app/dao/api_key_dao.py | 9 +++--- app/dao/service_inbound_api_dao.py | 5 ++-- app/models.py | 28 +++++++++++++------ app/schemas.py | 2 +- app/service/rest.py | 1 - tests/__init__.py | 4 +-- tests/app/authentication/test_utils.py | 8 ------ tests/app/dao/test_api_key_dao.py | 10 +++---- tests/app/dao/test_service_inbound_api_dao.py | 18 ++++++------ .../rest/test_send_notification.py | 24 ++++++---------- tests/app/notifications/test_rest.py | 2 +- .../test_POST_notification.py | 2 -- tests/app/service/test_rest.py | 7 ++--- 15 files changed, 55 insertions(+), 79 deletions(-) delete mode 100644 app/authentication/utils.py delete mode 100644 tests/app/authentication/test_utils.py diff --git a/app/authentication/auth.py b/app/authentication/auth.py index 77fba3d95..81f7f81a4 100644 --- a/app/authentication/auth.py +++ b/app/authentication/auth.py @@ -73,7 +73,7 @@ def requires_auth(): for api_key in service.api_keys: try: - get_decode_errors(auth_token, api_key.unsigned_secret) + get_decode_errors(auth_token, api_key.secret) except TokenDecodeError: continue diff --git a/app/authentication/utils.py b/app/authentication/utils.py deleted file mode 100644 index fa10f44b5..000000000 --- a/app/authentication/utils.py +++ /dev/null @@ -1,12 +0,0 @@ -from flask import current_app -from itsdangerous import URLSafeSerializer - - -def get_secret(secret): - serializer = URLSafeSerializer(current_app.config.get('SECRET_KEY')) - return serializer.loads(secret, salt=current_app.config.get('DANGEROUS_SALT')) - - -def generate_secret(token): - serializer = URLSafeSerializer(current_app.config.get('SECRET_KEY')) - return serializer.dumps(str(token), current_app.config.get('DANGEROUS_SALT')) diff --git a/app/dao/api_key_dao.py b/app/dao/api_key_dao.py index 04299c9a7..8d32a10a5 100644 --- a/app/dao/api_key_dao.py +++ b/app/dao/api_key_dao.py @@ -1,14 +1,13 @@ import uuid from datetime import datetime -from app import db +from app import db, encryption from app.models import ApiKey from app.dao.dao_utils import ( transactional, version_class ) -from app.authentication.utils import generate_secret @transactional @@ -16,7 +15,7 @@ from app.authentication.utils import generate_secret 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(uuid.uuid4()) + api_key.secret = uuid.uuid4() db.session.add(api_key) @@ -39,7 +38,7 @@ def get_unsigned_secrets(service_id): This method can only be exposed to the Authentication of the api calls. """ api_keys = ApiKey.query.filter_by(service_id=service_id, expiry_date=None).all() - keys = [x.unsigned_secret for x in api_keys] + keys = [x.secret for x in api_keys] return keys @@ -48,4 +47,4 @@ def get_unsigned_secret(key_id): This method can only be exposed to the Authentication of the api calls. """ api_key = ApiKey.query.filter_by(id=key_id, expiry_date=None).one() - return api_key.unsigned_secret + return api_key.secret diff --git a/app/dao/service_inbound_api_dao.py b/app/dao/service_inbound_api_dao.py index 8acf0070d..bc26db584 100644 --- a/app/dao/service_inbound_api_dao.py +++ b/app/dao/service_inbound_api_dao.py @@ -1,7 +1,6 @@ from datetime import datetime from app import db, create_uuid -from app.authentication.utils import generate_secret from app.dao.dao_utils import transactional, version_class from app.models import ServiceInboundApi @@ -11,7 +10,7 @@ from app.models import ServiceInboundApi def save_service_inbound_api(service_inbound_api): service_inbound_api.id = create_uuid() service_inbound_api.created_at == datetime.utcnow() - service_inbound_api.bearer_token = generate_secret(service_inbound_api.bearer_token) + service_inbound_api.bearer_token = service_inbound_api.bearer_token db.session.add(service_inbound_api) @@ -21,7 +20,7 @@ def reset_service_inbound_api(service_inbound_api, updated_by_id, url=None, bear if url: service_inbound_api.url = url if bearer_token: - service_inbound_api.bearer_token = generate_secret(bearer_token) + service_inbound_api.bearer_token = bearer_token service_inbound_api.updated_by_id = updated_by_id service_inbound_api.updated_at = datetime.utcnow() diff --git a/app/models.py b/app/models.py index 9c69d897e..18aab6629 100644 --- a/app/models.py +++ b/app/models.py @@ -22,7 +22,6 @@ from app.encryption import ( hashpw, check_hash ) -from app.authentication.utils import get_secret from app import ( db, encryption, @@ -301,22 +300,28 @@ class ServiceInboundApi(db.Model, Versioned): service_id = db.Column(UUID(as_uuid=True), db.ForeignKey('services.id'), index=True, nullable=False, unique=True) service = db.relationship('Service', backref='inbound_api') url = db.Column(db.String(), nullable=False) - bearer_token = db.Column(db.String(), nullable=False) + _bearer_token = db.Column("bearer_token", db.String(), nullable=False) created_at = db.Column(db.DateTime, default=datetime.datetime.utcnow, nullable=False) updated_at = db.Column(db.DateTime, nullable=True) updated_by = db.relationship('User') updated_by_id = db.Column(UUID(as_uuid=True), db.ForeignKey('users.id'), index=True, nullable=False) @property - def unsigned_bearer_token(self): - return get_secret(self.bearer_token) + def bearer_token(self): + if self._bearer_token: + return encryption.decrypt(self._bearer_token) + return None + + @bearer_token.setter + def bearer_token(self, bearer_token): + if bearer_token: + self._bearer_token = encryption.encrypt(str(bearer_token)) def serialize(self): return { "id": str(self.id), "service_id": str(self.service_id), "url": self.url, - "bearer_token": self.bearer_token, "updated_by_id": str(self.updated_by_id), "created_at": self.created_at.strftime(DATETIME_FORMAT), "updated_at": self.updated_at.strftime(DATETIME_FORMAT) if self.updated_at else None @@ -328,7 +333,7 @@ class ApiKey(db.Model, Versioned): id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) name = db.Column(db.String(255), nullable=False) - secret = db.Column(db.String(255), unique=True, nullable=False) + _secret = db.Column("secret", 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='api_keys') key_type = db.Column(db.String(255), db.ForeignKey('key_types.name'), index=True, nullable=False) @@ -353,8 +358,15 @@ class ApiKey(db.Model, Versioned): ) @property - def unsigned_secret(self): - return get_secret(self.secret) + def secret(self): + if self._secret: + return encryption.decrypt(self._secret) + return None + + @secret.setter + def secret(self, secret): + if secret: + self._secret = encryption.encrypt(str(secret)) KEY_TYPE_NORMAL = 'normal' diff --git a/app/schemas.py b/app/schemas.py index 7971300b7..bb8aad235 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -338,7 +338,7 @@ class ApiKeySchema(BaseSchema): class Meta: model = models.ApiKey - exclude = ("service", "secret") + exclude = ("service", "_secret") strict = True diff --git a/app/service/rest.py b/app/service/rest.py index b46cdbd83..31d553b12 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -12,7 +12,6 @@ from sqlalchemy.exc import SQLAlchemyError from sqlalchemy.orm.exc import NoResultFound from app import redis_store -from app.authentication.utils import generate_secret from app.dao import notification_usage_dao, notifications_dao from app.dao.dao_utils import dao_rollback from app.dao.api_key_dao import ( diff --git a/tests/__init__.py b/tests/__init__.py index a40d209cf..93990cdfe 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -11,7 +11,7 @@ def create_authorization_header(service_id=None, key_type=KEY_TYPE_NORMAL): client_id = str(service_id) secrets = ApiKey.query.filter_by(service_id=service_id, key_type=key_type).all() if secrets: - secret = secrets[0].unsigned_secret + secret = secrets[0].secret else: service = dao_fetch_service_by_id(service_id) data = { @@ -22,7 +22,7 @@ def create_authorization_header(service_id=None, key_type=KEY_TYPE_NORMAL): } api_key = ApiKey(**data) save_model_api_key(api_key) - secret = api_key.unsigned_secret + secret = api_key.secret else: client_id = current_app.config.get('ADMIN_CLIENT_USER_NAME') diff --git a/tests/app/authentication/test_utils.py b/tests/app/authentication/test_utils.py deleted file mode 100644 index c99ed734c..000000000 --- a/tests/app/authentication/test_utils.py +++ /dev/null @@ -1,8 +0,0 @@ -from app.authentication.utils import generate_secret, get_secret - - -def test_secret_is_signed_and_can_be_read_again(notify_api): - with notify_api.test_request_context(): - signed_secret = generate_secret('some_uuid') - assert signed_secret != 'some_uuid' - assert 'some_uuid' == get_secret(signed_secret) diff --git a/tests/app/dao/test_api_key_dao.py b/tests/app/dao/test_api_key_dao.py index 555770125..aad7861a9 100644 --- a/tests/app/dao/test_api_key_dao.py +++ b/tests/app/dao/test_api_key_dao.py @@ -4,7 +4,7 @@ import pytest from sqlalchemy.exc import IntegrityError from sqlalchemy.orm.exc import NoResultFound -from app.authentication.utils import get_secret +from app import encryption from app.dao.api_key_dao import (save_model_api_key, get_model_api_keys, get_unsigned_secrets, @@ -63,14 +63,14 @@ def test_should_return_api_key_for_service(notify_api, notify_db, notify_db_sess 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) + assert sample_api_key._secret != unsigned_api_key[0] + assert unsigned_api_key[0] == sample_api_key.secret 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) + assert sample_api_key._secret != unsigned_api_key + assert unsigned_api_key == sample_api_key.secret def test_should_not_allow_duplicate_key_names_per_service(sample_api_key, fake_uuid): diff --git a/tests/app/dao/test_service_inbound_api_dao.py b/tests/app/dao/test_service_inbound_api_dao.py index dde053427..68b804d23 100644 --- a/tests/app/dao/test_service_inbound_api_dao.py +++ b/tests/app/dao/test_service_inbound_api_dao.py @@ -3,7 +3,7 @@ import uuid import pytest from sqlalchemy.exc import SQLAlchemyError -from app.authentication.utils import get_secret +from app import encryption from app.dao.service_inbound_api_dao import ( save_service_inbound_api, reset_service_inbound_api, @@ -29,8 +29,8 @@ def test_save_service_inbound_api(sample_service): assert inbound_api.service_id == sample_service.id assert inbound_api.updated_by_id == sample_service.users[0].id assert inbound_api.url == "https://some_service/inbound_messages" - assert inbound_api.unsigned_bearer_token == "some_unique_string" - assert inbound_api.bearer_token != "some_unique_string" + assert inbound_api.bearer_token == "some_unique_string" + assert inbound_api._bearer_token != "some_unique_string" assert inbound_api.updated_at is None versioned = ServiceInboundApi.get_history_model().query.filter_by(id=inbound_api.id).one() @@ -38,7 +38,7 @@ def test_save_service_inbound_api(sample_service): assert versioned.service_id == sample_service.id assert versioned.updated_by_id == sample_service.users[0].id assert versioned.url == "https://some_service/inbound_messages" - assert versioned.bearer_token != "some_unique_string" + assert encryption.decrypt(versioned._bearer_token) == "some_unique_string" assert versioned.updated_at is None assert versioned.version == 1 @@ -77,8 +77,8 @@ def test_update_service_inbound_api(sample_service): assert updated.service_id == sample_service.id assert updated.updated_by_id == sample_service.users[0].id assert updated.url == "https://some_service/changed_url" - assert updated.unsigned_bearer_token == "some_unique_string" - assert updated.bearer_token != "some_unique_string" + assert updated.bearer_token == "some_unique_string" + assert updated._bearer_token != "some_unique_string" assert updated.updated_at is not None versioned_results = ServiceInboundApi.get_history_model().query.filter_by(id=saved_inbound_api.id).all() @@ -95,7 +95,7 @@ def test_update_service_inbound_api(sample_service): assert x.id is not None assert x.service_id == sample_service.id assert x.updated_by_id == sample_service.users[0].id - assert get_secret(x.bearer_token) == "some_unique_string" + assert encryption.decrypt(x._bearer_token) == "some_unique_string" def test_get_service_inbound_api(sample_service): @@ -112,6 +112,6 @@ def test_get_service_inbound_api(sample_service): assert inbound_api.service_id == sample_service.id assert inbound_api.updated_by_id == sample_service.users[0].id assert inbound_api.url == "https://some_service/inbound_messages" - assert inbound_api.unsigned_bearer_token == "some_unique_string" - assert inbound_api.bearer_token != "some_unique_string" + assert inbound_api.bearer_token == "some_unique_string" + assert inbound_api._bearer_token != "some_unique_string" assert inbound_api.updated_at is None diff --git a/tests/app/notifications/rest/test_send_notification.py b/tests/app/notifications/rest/test_send_notification.py index b491e3d7f..6a9fe1f2d 100644 --- a/tests/app/notifications/rest/test_send_notification.py +++ b/tests/app/notifications/rest/test_send_notification.py @@ -321,7 +321,6 @@ def test_should_allow_valid_sms_notification(notify_api, sample_template, mocker with notify_api.test_request_context(): with notify_api.test_client() as client: mocked = mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') - mocker.patch('app.encryption.encrypt', return_value="something_encrypted") data = { 'to': '07700 900 855', @@ -374,7 +373,6 @@ def test_should_allow_valid_email_notification(notify_api, sample_email_template with notify_api.test_request_context(): with notify_api.test_client() as client: mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') - mocker.patch('app.encryption.encrypt', return_value="something_encrypted") data = { 'to': 'ok@ok.com', @@ -411,7 +409,6 @@ def test_should_block_api_call_if_over_day_limit_for_live_service( with notify_api.test_request_context(): with notify_api.test_client() as client: mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') - mocker.patch('app.encryption.encrypt', return_value="something_encrypted") service = create_sample_service(notify_db, notify_db_session, limit=1, restricted=False) email_template = create_sample_email_template(notify_db, notify_db_session, service=service) @@ -443,7 +440,6 @@ def test_should_block_api_call_if_over_day_limit_for_restricted_service( with notify_api.test_request_context(): with notify_api.test_client() as client: mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') - mocker.patch('app.encryption.encrypt', return_value="something_encrypted") service = create_sample_service(notify_db, notify_db_session, limit=1, restricted=True) email_template = create_sample_email_template(notify_db, notify_db_session, service=service) @@ -479,7 +475,6 @@ def test_should_allow_api_call_if_under_day_limit_regardless_of_type( with notify_api.test_request_context(): with notify_api.test_client() as client: mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') - mocker.patch('app.encryption.encrypt', return_value="something_encrypted") service = create_sample_service(notify_db, notify_db_session, limit=2, restricted=restricted) email_template = create_sample_email_template(notify_db, notify_db_session, service=service) @@ -586,7 +581,7 @@ def test_should_send_email_if_team_api_key_and_a_service_user(notify_api, sample created_by=sample_email_template.created_by, key_type=KEY_TYPE_TEAM) save_model_api_key(api_key) - auth_header = create_jwt_token(secret=api_key.unsigned_secret, client_id=str(api_key.service_id)) + auth_header = create_jwt_token(secret=api_key.secret, client_id=str(api_key.service_id)) response = client.post( path='/notifications/email', @@ -619,7 +614,7 @@ def test_should_send_sms_to_anyone_with_test_key( key_type=KEY_TYPE_TEST ) save_model_api_key(api_key) - auth_header = create_jwt_token(secret=api_key.unsigned_secret, client_id=str(api_key.service_id)) + auth_header = create_jwt_token(secret=api_key.secret, client_id=str(api_key.service_id)) response = client.post( path='/notifications/sms', @@ -654,7 +649,7 @@ def test_should_send_email_to_anyone_with_test_key( key_type=KEY_TYPE_TEST ) save_model_api_key(api_key) - auth_header = create_jwt_token(secret=api_key.unsigned_secret, client_id=str(api_key.service_id)) + auth_header = create_jwt_token(secret=api_key.secret, client_id=str(api_key.service_id)) response = client.post( path='/notifications/email', @@ -682,7 +677,7 @@ def test_should_send_sms_if_team_api_key_and_a_service_user(notify_api, sample_t created_by=sample_template.created_by, key_type=KEY_TYPE_TEAM) save_model_api_key(api_key) - auth_header = create_jwt_token(secret=api_key.unsigned_secret, client_id=str(api_key.service_id)) + auth_header = create_jwt_token(secret=api_key.secret, client_id=str(api_key.service_id)) response = client.post( path='/notifications/sms', @@ -715,7 +710,7 @@ def test_should_persist_notification(notify_api, sample_template, created_by=template.created_by, key_type=KEY_TYPE_TEAM) save_model_api_key(api_key) - auth_header = create_jwt_token(secret=api_key.unsigned_secret, client_id=str(api_key.service_id)) + auth_header = create_jwt_token(secret=api_key.secret, client_id=str(api_key.service_id)) response = client.post( path='/notifications/{}'.format(template_type), @@ -758,7 +753,7 @@ def test_should_delete_notification_and_return_error_if_sqs_fails( created_by=template.created_by, key_type=KEY_TYPE_TEAM) save_model_api_key(api_key) - auth_header = create_jwt_token(secret=api_key.unsigned_secret, client_id=str(api_key.service_id)) + auth_header = create_jwt_token(secret=api_key.secret, client_id=str(api_key.service_id)) response = client.post( path='/notifications/{}'.format(template_type), @@ -862,7 +857,7 @@ def test_should_not_send_notification_to_non_whitelist_recipient_in_trial_mode( } api_key = create_sample_api_key(notify_db, notify_db_session, service, key_type=key_type) - auth_header = create_jwt_token(secret=api_key.unsigned_secret, client_id=str(api_key.service_id)) + auth_header = create_jwt_token(secret=api_key.secret, client_id=str(api_key.service_id)) response = client.post( path='/notifications/{}'.format(notification_type), @@ -923,7 +918,7 @@ def test_should_send_notification_to_whitelist_recipient( } sample_key = create_sample_api_key(notify_db, notify_db_session, service, key_type=key_type) - auth_header = create_jwt_token(secret=sample_key.unsigned_secret, client_id=str(sample_key.service_id)) + auth_header = create_jwt_token(secret=sample_key.secret, client_id=str(sample_key.service_id)) response = client.post( path='/notifications/{}'.format(notification_type), @@ -1101,7 +1096,6 @@ def test_returns_a_429_limit_exceeded_if_rate_limit_exceeded( def test_should_allow_store_original_number_on_sms_notification(client, sample_template, mocker): mocked = mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') - mocker.patch('app.encryption.encrypt', return_value="something_encrypted") data = { 'to': '+(44) 7700-900 855', @@ -1128,7 +1122,6 @@ def test_should_allow_store_original_number_on_sms_notification(client, sample_t def test_should_not_allow_international_number_on_sms_notification(client, sample_template, mocker): mocked = mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') - mocker.patch('app.encryption.encrypt', return_value="something_encrypted") data = { 'to': '20-12-1234-1234', @@ -1151,7 +1144,6 @@ def test_should_not_allow_international_number_on_sms_notification(client, sampl def test_should_allow_international_number_on_sms_notification(client, notify_db, notify_db_session, mocker): mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') - mocker.patch('app.encryption.encrypt', return_value="something_encrypted") service = sample_service(notify_db, notify_db_session, can_send_international_sms=True) template = create_sample_template(notify_db, notify_db_session, service=service) diff --git a/tests/app/notifications/test_rest.py b/tests/app/notifications/test_rest.py index 9a9fcae3d..811fc1290 100644 --- a/tests/app/notifications/test_rest.py +++ b/tests/app/notifications/test_rest.py @@ -741,5 +741,5 @@ def test_get_notification_selects_correct_template_for_personalisation(client, def _create_auth_header_from_key(api_key): - token = create_jwt_token(secret=api_key.unsigned_secret, client_id=str(api_key.service_id)) + token = create_jwt_token(secret=api_key.secret, client_id=str(api_key.service_id)) return [('Authorization', 'Bearer {}'.format(token))] diff --git a/tests/app/public_contracts/test_POST_notification.py b/tests/app/public_contracts/test_POST_notification.py index 3acd5cf5d..058cf3194 100644 --- a/tests/app/public_contracts/test_POST_notification.py +++ b/tests/app/public_contracts/test_POST_notification.py @@ -21,7 +21,6 @@ def _post_notification(client, template, url, to): def test_post_sms_contract(client, mocker, sample_template): mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') - mocker.patch('app.encryption.encrypt', return_value="something_encrypted") response_json = return_json_from_response(_post_notification( client, sample_template, url='/notifications/sms', to='07700 900 855' @@ -31,7 +30,6 @@ def test_post_sms_contract(client, mocker, sample_template): def test_post_email_contract(client, mocker, sample_email_template): mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') - mocker.patch('app.encryption.encrypt', return_value="something_encrypted") response_json = return_json_from_response(_post_notification( client, sample_email_template, url='/notifications/email', to='foo@bar.com' diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index a0ec13621..3293e69fd 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -8,7 +8,7 @@ import pytest from flask import url_for, current_app from freezegun import freeze_time -from app.authentication.utils import get_secret +from app import encryption from app.dao.users_dao import save_model_user from app.dao.services_dao import dao_remove_user_from_service from app.models import ( @@ -2168,7 +2168,6 @@ def test_create_service_inbound_api(client, sample_service): assert resp_json["id"] assert resp_json["service_id"] == str(sample_service.id) assert resp_json["url"] == "https://some_service/inbound-sms" - assert resp_json["bearer_token"] != "some-unique-string" # returned encrypted assert resp_json["updated_by_id"] == str(sample_service.users[0].id) assert resp_json["created_at"] assert not resp_json["updated_at"] @@ -2217,9 +2216,7 @@ def test_update_service_inbound_api_updates_bearer_token(client, sample_service) data=json.dumps(data), headers=[('Content-Type', 'application/json'), create_authorization_header()]) assert response.status_code == 200 - resp_json = json.loads(response.get_data(as_text=True))["data"] - assert get_secret(resp_json["bearer_token"]) == "different_token" - assert service_inbound_api.unsigned_bearer_token == "different_token" + assert service_inbound_api.bearer_token == "different_token" def test_fetch_service_inbound_api(client, sample_service):