diff --git a/app/main/forms.py b/app/main/forms.py index f8db6e577..d9afad2f1 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -9,6 +9,7 @@ from notifications_utils.recipients import ( ) from notifications_utils.columns import Columns from wtforms import ( + widgets, validators, StringField, PasswordField, @@ -651,6 +652,21 @@ class PlaceholderForm(Form): pass +class PasswordFieldShowHasContent(StringField): + widget = widgets.PasswordInput(hide_value=False) + + +class ServiceInboundApiForm(Form): + url = StringField("Inbound sms url", + validators=[DataRequired(message='Can’t be empty'), + Regexp(regex="^https.*", + message='Must be a valid https url')] + ) + bearer_token = PasswordFieldShowHasContent("Bearer token", + validators=[DataRequired(message='Can’t be empty'), + Length(min=10, message='Must be at least 10 characters')]) + + def get_placeholder_form_instance( placeholder_name, dict_to_populate_from, diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index d1667bfa7..85e02c12c 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -1,3 +1,5 @@ +from urllib.parse import urlparse + import requests from flask import ( render_template, @@ -30,10 +32,21 @@ from app.main.forms import ( ServiceLetterContactBlock, ServiceBrandingOrg, LetterBranding, -) + ServiceInboundApiForm) from app import user_api_client, current_service, organisations_client +dummy_bearer_token = 'bearer_token_set' + + +def get_inbound_api(): + if current_service['inbound_api']: + return service_api_client.get_service_inbound_api( + current_service['id'], + current_service.get('inbound_api')[0] + ) + + @main.route("/services//service-settings") @login_required @user_has_permissions('manage_settings', admin_override=True) @@ -43,6 +56,15 @@ def service_settings(service_id): organisation = organisations_client.get_organisation(current_service['organisation'])['organisation'] else: organisation = None + + inbound_api = get_inbound_api() + if inbound_api: + parsed_url = urlparse(inbound_api.get('url')) if inbound_api else '' + inbound_api_url = '{uri.scheme}://{uri.netloc}{elide_token}'.format( + uri=parsed_url, elide_token='...' if parsed_url.path else '') + else: + inbound_api_url = '' + return render_template( 'views/service-settings.html', organisation=organisation, @@ -50,6 +72,7 @@ def service_settings(service_id): current_service.get('dvla_organisation', '001') ), can_receive_inbound=('inbound_sms' in current_service['permissions']), + inbound_api_url=inbound_api_url, letter_contact_block=Field(current_service['letter_contact_block'], html='escape') ) @@ -268,6 +291,7 @@ def service_set_reply_to_email(service_id): @user_has_permissions('manage_settings', admin_override=True) def service_set_sms_sender(service_id): form = ServiceSmsSender() + if form.validate_on_submit(): set_inbound_sms = request.args.get('set_inbound_sms', False) if set_inbound_sms == 'True': @@ -410,3 +434,41 @@ def get_branding_as_dict(organisations): 'colour': organisation['colour'] } for organisation in organisations } + + +@main.route("/services//service-settings/set-inbound-api", methods=['GET', 'POST']) +@login_required +@user_has_permissions('manage_settings', admin_override=True) +def service_set_inbound_api(service_id): + if 'inbound_sms' not in current_service['permissions']: + abort(403) + + inbound_api = get_inbound_api() + form = ServiceInboundApiForm( + url=inbound_api.get('url') if inbound_api else '', + bearer_token=dummy_bearer_token if inbound_api else '' + ) + + if form.validate_on_submit(): + if inbound_api: + if inbound_api.get('url') != form.url.data or form.bearer_token.data != dummy_bearer_token: + service_api_client.update_service_inbound_api( + service_id, + url=form.url.data, + bearer_token=form.bearer_token.data if form.bearer_token.data != dummy_bearer_token else '', + user_id=current_user.id, + inbound_api_id=inbound_api.get('id') + ) + else: + service_api_client.create_service_inbound_api( + service_id, + url=form.url.data, + bearer_token=form.bearer_token.data, + user_id=current_user.id + ) + return redirect(url_for('.service_settings', service_id=service_id)) + + return render_template( + 'views/service-settings/set-inbound-api.html', + form=form, + ) diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index bbc4fdf81..73b324924 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -266,6 +266,30 @@ class ServiceAPIClient(NotifyAdminAPIClient): '/service/{}/inbound-sms/summary'.format(service_id) ) + def create_service_inbound_api(self, service_id, url, bearer_token, user_id): + data = { + "url": url, + "bearer_token": bearer_token, + "updated_by_id": user_id + } + return self.post("/service/{}/inbound-api".format(service_id), data) + + def update_service_inbound_api(self, service_id, url, bearer_token, user_id, inbound_api_id): + data = { + "url": url, + "updated_by_id": user_id + } + if bearer_token: + data['bearer_token'] = bearer_token + return self.post("/service/{}/inbound-api/{}".format(service_id, inbound_api_id), data) + + def get_service_inbound_api(self, service_id, inbound_sms_api_id): + return self.get( + "/service/{}/inbound-api/{}".format( + service_id, inbound_sms_api_id + ) + )['data'] + class ServicesBrowsableItem(BrowsableItem): @property diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html index 60fcf1f3e..52f053cfa 100644 --- a/app/templates/views/service-settings.html +++ b/app/templates/views/service-settings.html @@ -57,6 +57,17 @@ {{ edit_field('Change', url_for('.service_set_inbound_sms', service_id=current_service.id)) }} {% endcall %} + {% if can_receive_inbound %} + {% call row() %} + {{ text_field('API endpoint for received text messages') }} + {{ text_field( + 'None' if not inbound_api_url else inbound_api_url, + status='' if inbound_api_url else 'default' + ) }} + {{ edit_field('Change', url_for('.service_set_inbound_api', service_id=current_service.id)) }} + {% endcall %} + {% endif %} + {% call row() %} {{ text_field('Letters') }} {{ boolean_field(current_service.can_send_letters) }} @@ -72,6 +83,7 @@ {{ edit_field('Change', url_for('.service_set_letter_contact_block', service_id=current_service.id)) }} {% endcall %} {% endif %} + {% endcall %} diff --git a/app/templates/views/service-settings/set-inbound-api.html b/app/templates/views/service-settings/set-inbound-api.html new file mode 100644 index 000000000..0609ff126 --- /dev/null +++ b/app/templates/views/service-settings/set-inbound-api.html @@ -0,0 +1,38 @@ +{% extends "withnav_template.html" %} +{% from "components/textbox.html" import textbox %} +{% from "components/page-footer.html" import page_footer %} + +{% block service_page_title %} + Inbound api +{% endblock %} + +{% block maincolumn_content %} +
+
+

API endpoint for received text messages

+

+ This is the https url that the inbound SMS messages will be posted to + and the bearer token used in the authorisation header of the request. +

+ +
+ {{ textbox( + form.url, + width='2-3', + hint='Valid https url' + ) }} + {{ textbox( + form.bearer_token, + width='1-4', + hint='At least 10 characters' + ) }} + {{ page_footer( + 'Save', + back_link=url_for('.service_settings', service_id=current_service.id), + back_link_text='Back to settings' + ) }} +
+
+
+ +{% endblock %} \ No newline at end of file diff --git a/tests/__init__.py b/tests/__init__.py index 70fbaf095..bbe7dad18 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -56,11 +56,14 @@ def service_json( created_at=None, letter_contact_block=None, permissions=None, + inbound_api=None, ): if users is None: users = [] if permissions is None: permissions = [] + if inbound_api is None: + inbound_api = [] return { 'id': id_, 'name': name, @@ -80,6 +83,7 @@ def service_json( 'letter_contact_block': letter_contact_block, 'dvla_organisation': '001', 'permissions': permissions, + 'inbound_api': inbound_api, } diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 4228d4f8e..ad8f96888 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -7,6 +7,7 @@ from bs4 import BeautifulSoup from werkzeug.exceptions import InternalServerError import app +from app.main.views.service_settings import dummy_bearer_token from app.utils import email_safe from tests import validate_route_permission, service_json from tests.app.test_utils import normalize_spaces @@ -60,6 +61,25 @@ def test_should_show_overview( app.service_api_client.get_service.assert_called_with(service_one['id']) +@pytest.mark.parametrize('permissions, expected_rows', [ + (['email', 'sms', 'inbound_sms'], [ + 'Service name service one Change', + 'Email reply to address test@example.com Change', + 'Text message sender elevenchars', + 'International text messages On Change', + 'Receive text messages On Change', + 'API endpoint for received text messages None Change', + 'Letters Off Change', + ]), + (['email', 'sms'], [ + 'Service name service one Change', + 'Email reply to address test@example.com Change', + 'Text message sender elevenchars Change', + 'International text messages On Change', + 'Receive text messages Off Change', + 'Letters Off Change', + ]), +]) def test_should_show_overview_for_service_with_more_things_set( client, active_user_with_permissions, @@ -67,25 +87,56 @@ def test_should_show_overview_for_service_with_more_things_set( service_with_reply_to_addresses, mock_get_organisation, mock_get_letter_organisations, + permissions, + expected_rows ): client.login(active_user_with_permissions, mocker, service_with_reply_to_addresses) - service_with_reply_to_addresses['permissions'] = ['inbound_sms'] + service_with_reply_to_addresses['permissions'] = permissions service_with_reply_to_addresses['can_send_international_sms'] = True response = client.get(url_for( 'main.service_settings', service_id=service_with_reply_to_addresses['id'] )) page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - for index, row in enumerate([ - 'Service name service one Change', - 'Email reply to address test@example.com Change', - 'Text message sender elevenchars', - 'International text messages On Change', - 'Receive text messages On Change', - 'Letters Off Change', - ]): + for index, row in enumerate(expected_rows): assert row == " ".join(page.find_all('tr')[index + 1].text.split()) +@pytest.mark.parametrize('url, elided_url', [ + ('https://test.url.com/inbound', 'https://test.url.com...'), + ('https://test.url.com/', 'https://test.url.com...'), + ('https://test.url.com', 'https://test.url.com'), +]) +def test_service_settings_show_elided_api_url_if_needed( + logged_in_platform_admin_client, + service_one, + mock_get_letter_organisations, + mocker, + fake_uuid, + url, + elided_url +): + service_one['permissions'] = ['inbound_sms'] + service_one['inbound_api'] = [fake_uuid] + + mocked_get_fn = mocker.patch( + 'app.service_api_client.get', + return_value={'data': {'id': fake_uuid, 'url': url}}) + + response = logged_in_platform_admin_client.get( + url_for( + 'main.service_settings', + service_id=service_one['id'] + ) + ) + assert response.status_code == 200 + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + + non_empty_trs = [tr.find_all('td') for tr in page.find_all('tr') if tr.find_all('td')] + api_url = [api_setting[1].text.strip() for api_setting in non_empty_trs + if api_setting[0].text.strip() == 'API endpoint for received text messages'][0] + assert api_url == elided_url + + def test_if_cant_send_letters_then_cant_see_letter_contact_block( logged_in_client, service_one, @@ -736,6 +787,34 @@ def test_set_text_message_sender_validation( assert not mock_update_service.called +@pytest.mark.parametrize('url, bearer_token, expected_errors', [ + ("", "", "Can’t be empty Can’t be empty"), + ("http://not_https.com", "1234567890", "Must be a valid https url"), + ("https://test.com", "123456789", "Must be at least 10 characters"), +]) +def test_set_inbound_api_validation( + logged_in_client, + mock_update_service, + service_one, + mock_get_letter_organisations, + url, + bearer_token, + expected_errors, +): + service_one['permissions'] = ['inbound_sms'] + response = logged_in_client.post(url_for( + 'main.service_set_inbound_api', + service_id=service_one['id']), + data={"url": url, "bearer_token": bearer_token} + ) + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + error_msgs = ' '.join(msg.text.strip() for msg in page.select(".error-message")) + + assert response.status_code == 200 + assert error_msgs == expected_errors + assert not mock_update_service.called + + def test_if_sms_sender_set_then_form_populated( logged_in_client, service_one, @@ -1008,6 +1087,111 @@ def test_switch_service_disable_international_sms( assert mocked_fn.call_args == call(service_one['id'], {"can_send_international_sms": False}) +def test_set_new_inbound_api_and_valid_bearer_token_calls_create_inbound_api_endpoint( + logged_in_platform_admin_client, + service_one, + mocker, +): + service_one['permissions'] = ['inbound_sms'] + service_one['inbound_api'] = [] + + mocked_post_fn = mocker.patch('app.service_api_client.post', return_value=service_one) + + inbound_api_data = {'url': "https://test.url.com/", 'bearer_token': '1234567890'} + response = logged_in_platform_admin_client.post( + url_for( + 'main.service_set_inbound_api', + service_id=service_one['id'] + ), + data=inbound_api_data + ) + assert response.status_code == 302 + assert response.location == url_for('main.service_settings', service_id=service_one['id'], _external=True) + assert mocked_post_fn.called + + inbound_api_data['updated_by_id'] = service_one['users'][0] + assert mocked_post_fn.call_args == call("/service/{}/inbound-api".format(service_one['id']), inbound_api_data) + + +@pytest.mark.parametrize( + 'inbound_api_data', [ + {'url': "https://test.url.com/inbound", 'bearer_token': dummy_bearer_token}, + {'url': "https://test.url.com/inbound", 'bearer_token': '1234567890'}, + {'url': "https://test.url.com/", 'bearer_token': 'new_1234567890'}, + ] +) +def test_update_inbound_api_and_valid_bearer_token_calls_update_inbound_api_endpoint( + logged_in_platform_admin_client, + service_one, + mocker, + fake_uuid, + inbound_api_data +): + service_one['permissions'] = ['inbound_sms'] + service_one['inbound_api'] = [fake_uuid] + + initial_api_data = {'data': {'id': fake_uuid, 'url': "https://test.url.com/"}} + + mocked_get_fn = mocker.patch('app.service_api_client.get', return_value=initial_api_data) + mocked_post_fn = mocker.patch('app.service_api_client.post', return_value=service_one) + + response = logged_in_platform_admin_client.get( + url_for( + 'main.service_set_inbound_api', + service_id=service_one['id'] + ) + ) + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + + assert page.find('input', {'id': 'url'}).get('value') == initial_api_data['data']['url'] + assert page.find('input', {'id': 'bearer_token'}).get('value') == dummy_bearer_token + + response = logged_in_platform_admin_client.post( + url_for( + 'main.service_set_inbound_api', + service_id=service_one['id'] + ), + data=inbound_api_data + ) + assert response.status_code == 302 + assert response.location == url_for('main.service_settings', service_id=service_one['id'], _external=True) + assert mocked_post_fn.called + + if inbound_api_data['bearer_token'] == dummy_bearer_token: + del inbound_api_data['bearer_token'] + inbound_api_data['updated_by_id'] = service_one['users'][0] + + assert mocked_post_fn.call_args == call( + "/service/{}/inbound-api/{}".format(service_one['id'], fake_uuid), inbound_api_data) + + +def test_save_inbound_api_without_changes_does_not_update_inbound_api( + logged_in_platform_admin_client, + service_one, + mocker, + fake_uuid +): + service_one['permissions'] = ['inbound_sms'] + service_one['inbound_api'] = [fake_uuid] + + initial_api_data = {'data': {'id': fake_uuid, 'url': "https://test.url.com/"}} + inbound_api_data = {'url': initial_api_data['data']['url'], 'bearer_token': dummy_bearer_token} + + mocked_get_fn = mocker.patch('app.service_api_client.get', return_value=initial_api_data) + mocked_post_fn = mocker.patch('app.service_api_client.post', return_value=service_one) + + response = logged_in_platform_admin_client.post( + url_for( + 'main.service_set_inbound_api', + service_id=service_one['id'] + ), + data=inbound_api_data + ) + assert response.status_code == 302 + assert response.location == url_for('main.service_settings', service_id=service_one['id'], _external=True) + assert mocked_post_fn.called is False + + def test_archive_service_after_confirm( logged_in_platform_admin_client, service_one,