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

@@ -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()

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'

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

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"
+}

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('/<uuid:service_id>/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('/<uuid:service_id>/inbound-api/<uuid:id>', 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

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"]
+}

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')

tests/app/dao/test_save_service_inbound_api.py

@@ -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)

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

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

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"

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"