From 3b272e3d8dea65571933d9f97d68a493c12ca102 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 30 Oct 2017 11:48:44 +0000 Subject: [PATCH 01/10] Standardise analytics code on failwhale Missed this page when doing 62fcc2429fe01f81937d06ddfd513766542f4e78 --- paas-failwhale/index.html | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/paas-failwhale/index.html b/paas-failwhale/index.html index 4db6b5b71..1082941d9 100644 --- a/paas-failwhale/index.html +++ b/paas-failwhale/index.html @@ -132,7 +132,13 @@ m=s.getElementsByTagName(o)[0];a.async=1;a.src=g;m.parentNode.insertBefore(a,m) })(window,document,'script','//www.google-analytics.com/analytics.js','ga'); ga('create', 'UA-75215134-1', 'auto'); - ga('send', 'pageview'); + ga('set', 'anonymizeIp', true); + ga('set', 'displayFeaturesTask', null); + ga('set', 'transport', 'beacon'); + page = (window.location.pathname + window.location.search).replace( + /[a-f0-9]{8}-?[a-f0-9]{4}-?4[a-f0-9]{3}-?[89ab][a-f0-9]{3}-?[a-f0-9]{12}/g, '…' + ) + ga('send', 'pageview', page); From 4e721c95cedbcbed0c5353f8e23a3f21daf509d3 Mon Sep 17 00:00:00 2001 From: chrisw Date: Tue, 24 Oct 2017 15:37:44 +0100 Subject: [PATCH 02/10] Added Multiple SMS sender functionality --- app/main/forms.py | 14 ++ app/main/views/service_settings.py | 116 ++++++++- app/notify_client/inbound_number_client.py | 6 +- app/notify_client/service_api_client.py | 41 +++- app/templates/views/service-settings.html | 27 +-- .../service-settings/set-inbound-number.html | 35 +++ .../service-settings/sms-sender/add.html | 31 +++ .../service-settings/sms-sender/edit.html | 44 ++++ .../views/service-settings/sms-senders.html | 49 ++++ .../test_inbound_sms_setting.py | 179 ++++---------- .../test_service_setting_permissions.py | 3 +- tests/app/main/views/test_service_settings.py | 229 ++++++++++++++---- tests/conftest.py | 93 ++++++- 13 files changed, 638 insertions(+), 229 deletions(-) create mode 100644 app/templates/views/service-settings/set-inbound-number.html create mode 100644 app/templates/views/service-settings/sms-sender/add.html create mode 100644 app/templates/views/service-settings/sms-sender/edit.html create mode 100644 app/templates/views/service-settings/sms-senders.html diff --git a/app/main/forms.py b/app/main/forms.py index 33712c932..82c62288e 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -505,6 +505,7 @@ class ServiceSmsSender(Form): Length(max=11, message="Enter 11 characters or fewer") ] ) + is_default = BooleanField("Make this text message sender the default") def validate_sms_sender(self, field): if field.data and not re.match(r'^[a-zA-Z0-9\s]+$', field.data): @@ -674,6 +675,19 @@ class PasswordFieldShowHasContent(StringField): widget = widgets.PasswordInput(hide_value=False) +class ServiceInboundNumberForm(Form): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.inbound_number.choices = kwargs['inbound_number_choices'] + + inbound_number = RadioField( + "Select your inbound number", + validators=[ + DataRequired("Option must be selected") + ] + ) + + class ServiceInboundApiForm(Form): url = StringField("Inbound sms url", validators=[DataRequired(message='Can’t be empty'), diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 63eb21c31..2bc91599a 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -28,6 +28,7 @@ from app.main.forms import ( RenameServiceForm, RequestToGoLiveForm, ServiceReplyToEmailForm, + ServiceInboundNumberForm, ServiceSmsSender, ServiceLetterContactBlockForm, ServiceBrandingOrg, @@ -82,6 +83,11 @@ def service_settings(service_id): default_letter_contact_block = next( (Field(x['contact_block'], html='escape') for x in letter_contact_details if x['is_default']), "Not set" ) + sms_senders = service_api_client.get_sms_senders(service_id) + sms_sender_count = len(sms_senders) + default_sms_sender = next( + (Field(x['sms_sender'], html='escape') for x in sms_senders if x['is_default']), "None" + ) return render_template( 'views/service-settings.html', organisation=organisation, @@ -94,7 +100,9 @@ def service_settings(service_id): default_reply_to_email_address=default_reply_to_email_address, reply_to_email_address_count=reply_to_email_address_count, default_letter_contact_block=default_letter_contact_block, - letter_contact_details_count=letter_contact_details_count + letter_contact_details_count=letter_contact_details_count, + default_sms_sender=default_sms_sender, + sms_sender_count=sms_sender_count ) @@ -438,22 +446,34 @@ def service_set_sms_sender(service_id): form=form) -@main.route("/services//service-settings/set-inbound-number", methods=['GET']) +@main.route("/services//service-settings/set-inbound-number", methods=['GET', 'POST']) @login_required @user_has_permissions('manage_settings', admin_override=True) def service_set_inbound_number(service_id): - switch_service_permissions(current_service['id'], 'inbound_sms') - set_inbound_sms = request.args.get('set_inbound_sms', False) - try: - if set_inbound_sms == 'True': - inbound_number_client.activate_inbound_sms_service(service_id) - return redirect(url_for('.service_settings', service_id=service_id)) - else: - inbound_number_client.deactivate_inbound_sms_permission(service_id=service_id) - return redirect(url_for('.service_set_sms_sender', service_id=service_id)) - except HTTPError as e: + available_inbound_numbers = inbound_number_client.get_available_inbound_sms_numbers() + service_has_inbound_number = inbound_number_client.get_inbound_sms_number_for_service(service_id)['data'] != {} + inbound_numbers_value_and_label = [ + (number['id'], number['number']) for number in available_inbound_numbers['data'] + ] + no_available_numbers = available_inbound_numbers['data'] == [] + form = ServiceInboundNumberForm( + inbound_number_choices=inbound_numbers_value_and_label + ) + if form.validate_on_submit(): + service_api_client.add_sms_sender( + current_service['id'], + sms_sender=form.inbound_number.data, + is_default=True, + inbound_number_id=form.inbound_number.data + ) switch_service_permissions(current_service['id'], 'inbound_sms') - raise e + return redirect(url_for('.service_settings', service_id=service_id)) + return render_template( + 'views/service-settings/set-inbound-number.html', + form=form, + no_available_numbers=no_available_numbers, + service_has_inbound_number=service_has_inbound_number + ) @main.route("/services//service-settings/set-sms", methods=['GET']) @@ -559,6 +579,76 @@ def service_edit_letter_contact(service_id, letter_contact_id): letter_contact_id=letter_contact_block['id']) +@main.route("/services//service-settings/sms-sender", methods=['GET']) +@login_required +@user_has_permissions('manage_settings', admin_override=True) +def service_sms_senders(service_id): + + def attach_hint(sender): + hints = [] + if sender['is_default']: + hints += ["default"] + if sender['inbound_number_id']: + hints += ["recieves replies"] + if hints: + sender['hint'] = "(" + " and ".join(hints) + ")" + + sms_senders = service_api_client.get_sms_senders(service_id) + + for sender in sms_senders: + attach_hint(sender) + + return render_template( + 'views/service-settings/sms-senders.html', + sms_senders=sms_senders + ) + + +@main.route("/services//service-settings/sms-sender/add", methods=['GET', 'POST']) +@login_required +@user_has_permissions('manage_settings', admin_override=True) +def service_add_sms_sender(service_id): + form = ServiceSmsSender() + sms_sender_count = len(service_api_client.get_sms_senders(service_id)) + first_sms_sender = sms_sender_count == 0 + if form.validate_on_submit(): + service_api_client.add_sms_sender( + current_service['id'], + sms_sender=form.sms_sender.data.replace('\r', '') or None, + is_default=first_sms_sender if first_sms_sender else form.is_default.data + ) + return redirect(url_for('.service_sms_senders', service_id=service_id)) + return render_template( + 'views/service-settings/sms-sender/add.html', + form=form, + first_sms_sender=first_sms_sender) + + +@main.route("/services//service-settings/sms-sender//edit", methods=['GET', 'POST']) +@login_required +@user_has_permissions('manage_settings', admin_override=True) +def service_edit_sms_sender(service_id, sms_sender_id): + sms_sender = service_api_client.get_sms_sender(service_id, sms_sender_id) + form = ServiceSmsSender() + form.sms_sender.data = sms_sender['sms_sender'] + is_inbound_number = True if sms_sender['inbound_number_id'] else False + if request.method == 'GET': + form.is_default.data = sms_sender['is_default'] + if form.validate_on_submit(): + service_api_client.update_sms_sender( + current_service['id'], + sms_sender_id=sms_sender_id, + sms_sender=form.sms_sender.data.replace('\r', ''), + is_default=True if sms_sender['is_default'] else form.is_default.data + ) + return redirect(url_for('.service_sms_senders', service_id=service_id)) + return render_template( + 'views/service-settings/sms-sender/edit.html', + form=form, + sms_sender_id=sms_sender['id'], + is_inbound_number=is_inbound_number) + + @main.route("/services//service-settings/set-letter-contact-block", methods=['GET', 'POST']) @login_required @user_has_permissions('manage_settings', admin_override=True) diff --git a/app/notify_client/inbound_number_client.py b/app/notify_client/inbound_number_client.py index a12890974..662f93fea 100644 --- a/app/notify_client/inbound_number_client.py +++ b/app/notify_client/inbound_number_client.py @@ -11,6 +11,9 @@ class InboundNumberClient(NotifyAdminAPIClient): self.service_id = app.config['ADMIN_CLIENT_USER_NAME'] self.api_key = app.config['ADMIN_CLIENT_SECRET'] + def get_available_inbound_sms_numbers(self): + return self.get(url='/inbound-number/available') + def get_all_inbound_sms_number_service(self): return self.get('/inbound-number') @@ -19,6 +22,3 @@ class InboundNumberClient(NotifyAdminAPIClient): def activate_inbound_sms_service(self, service_id): return self.post(url='/inbound-number/service/{}'.format(service_id), data={}) - - def deactivate_inbound_sms_permission(self, service_id): - return self.post(url='/inbound-number/service/{}/off'.format(service_id), data={}) diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index 7713ccb00..e374487d5 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -331,19 +331,10 @@ class ServiceAPIClient(NotifyAdminAPIClient): ) def get_letter_contacts(self, service_id): - return self.get( - "/service/{}/letter-contact".format( - service_id - ) - ) + return self.get("/service/{}/letter-contact".format(service_id)) def get_letter_contact(self, service_id, letter_contact_id): - return self.get( - "/service/{}/letter-contact/{}".format( - service_id, - letter_contact_id - ) - ) + return self.get("/service/{}/letter-contact/{}".format(service_id, letter_contact_id)) def add_letter_contact(self, service_id, contact_block, is_default=False): return self.post( @@ -369,6 +360,34 @@ class ServiceAPIClient(NotifyAdminAPIClient): def get_aggregate_platform_stats(self, params_dict=None): return self.get("/service/platform-stats", params=params_dict) + def get_sms_senders(self, service_id): + return self.get( + "/service/{}/sms-sender".format(service_id) + ) + + def get_sms_sender(self, service_id, sms_sender_id): + return self.get( + "/service/{}/sms-sender/{}".format(service_id, sms_sender_id) + ) + + def add_sms_sender(self, service_id, sms_sender, is_default=False, inbound_number_id=None): + data = { + "sms_sender": sms_sender, + "is_default": is_default + } + if inbound_number_id: + data["inbound_number_id"] = inbound_number_id + return self.post("/service/{}/sms-sender".format(service_id), data=data) + + def update_sms_sender(self, service_id, sms_sender_id, sms_sender, is_default=False): + return self.post( + "/service/{}/sms-sender/{}".format(service_id, sms_sender_id), + data={ + "sms_sender": sms_sender, + "is_default": is_default + } + ) + class ServicesBrowsableItem(BrowsableItem): @property diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html index 6e7f63739..4e279f043 100644 --- a/app/templates/views/service-settings.html +++ b/app/templates/views/service-settings.html @@ -80,19 +80,16 @@ {% if 'sms' in current_service.permissions %} {% call row() %} - {{ text_field('Text message sender') }} - {% if can_receive_inbound %} - {{ text_field(inbound_number) }} - {% else %} - {{ text_field(current_service.sms_sender) }} - {% endif %} - {% if (current_user.has_permissions([], admin_override=True) or not can_receive_inbound) and not can_receive_inbound %} - {{ edit_field('Change', url_for('.service_set_sms_sender', service_id=current_service.id, set_inbound_sms=False)) }} - {% else %} - {{ text_field('') }} - {% endif %} - + {% call field(status='default' if default_sms_sender == "None" else '') %} + {{ default_sms_sender | string | nl2br | safe if default_sms_sender else 'None'}} + {% if sms_sender_count > 1 %} +
+ {{ '…and %d more' | format(sms_sender_count - 1) }} +
+ {% endif %} + {% endcall %} + {{ edit_field('Manage' if sms_sender_count else 'Change', url_for('.service_sms_senders', service_id=current_service.id)) }} {% endcall %} {% call row() %} @@ -250,11 +247,7 @@ {% if 'sms' in current_service.permissions %}
  • - {% if can_receive_inbound %} - - Stop inbound sms - - {% else %} + {% if not can_receive_inbound %} Allow inbound sms diff --git a/app/templates/views/service-settings/set-inbound-number.html b/app/templates/views/service-settings/set-inbound-number.html new file mode 100644 index 000000000..72a11b5b8 --- /dev/null +++ b/app/templates/views/service-settings/set-inbound-number.html @@ -0,0 +1,35 @@ +{% extends "withnav_template.html" %} +{% from "components/textbox.html" import textbox %} +{% from "components/page-footer.html" import page_footer %} +{% from "components/radios.html" import radios%} + +{% block service_page_title %} + Set Inbound Number +{% endblock %} + +{% block maincolumn_content %} +

    Set Inbound Number

    + {% if service_has_inbound_number %} +

    This service already has an inbound number

    + {{ page_footer( + back_link=url_for('.service_settings', service_id=current_service.id), + back_link_text='Back to settings' + ) }} + {% elif no_available_numbers %} +

    No available inbound numbers

    + {{ page_footer( + back_link=url_for('.service_settings', service_id=current_service.id), + back_link_text='Back to settings' + ) }} + {% else %} +
    + {{ radios(form.inbound_number) }} + {{ page_footer( + 'Save', + back_link=url_for('.service_settings', service_id=current_service.id), + back_link_text='Back' + ) }} +
    + {% endif %} + +{% endblock %} \ No newline at end of file diff --git a/app/templates/views/service-settings/sms-sender/add.html b/app/templates/views/service-settings/sms-sender/add.html new file mode 100644 index 000000000..e670d67b7 --- /dev/null +++ b/app/templates/views/service-settings/sms-sender/add.html @@ -0,0 +1,31 @@ +{% extends "withnav_template.html" %} +{% from "components/textbox.html" import textbox %} +{% from "components/checkbox.html" import checkbox %} +{% from "components/page-footer.html" import page_footer %} + +{% block service_page_title %} + Add text message sender +{% endblock %} + +{% block maincolumn_content %} + +

    Add text message sender

    +
    + {{ textbox( + form.sms_sender, + width='1-4', + hint='Up to 11 characters, letters, numbers and spaces only' + ) }} + {% if not first_sms_sender %} +
    + {{ checkbox(form.is_default) }} +
    + {% endif %} + {{ page_footer( + 'Save', + back_link=url_for('.service_sms_senders', service_id=current_service.id), + back_link_text='Back' + ) }} +
    + +{% endblock %} \ No newline at end of file diff --git a/app/templates/views/service-settings/sms-sender/edit.html b/app/templates/views/service-settings/sms-sender/edit.html new file mode 100644 index 000000000..81857c2cd --- /dev/null +++ b/app/templates/views/service-settings/sms-sender/edit.html @@ -0,0 +1,44 @@ +{% extends "withnav_template.html" %} +{% from "components/textbox.html" import textbox %} +{% from "components/checkbox.html" import checkbox %} +{% from "components/page-footer.html" import page_footer %} + +{% block service_page_title %} + Edit text message sender +{% endblock %} + +{% block maincolumn_content %} + +

    + Edit text message sender +

    +
    + {% if is_inbound_number %} +

    + {{ form.sms_sender.data }} + This phone number receives replies and can’t be changed +

    + {% else %} + {{ textbox( + form.sms_sender, + width='1-4', + hint='Up to 11 characters, letters, numbers and spaces only' + ) }} + {% endif %} + {% if form.is_default.data %} +

    + This is currently your text message sender for {{ current_service.name }} +

    + {% else %} +
    + {{ checkbox(form.is_default) }} +
    + {% endif %} + {{ page_footer( + 'Save', + back_link=None if request.args.get('from_template') else url_for('.service_sms_senders', service_id=current_service.id), + back_link_text='Back' + ) }} +
    + +{% endblock %} \ No newline at end of file diff --git a/app/templates/views/service-settings/sms-senders.html b/app/templates/views/service-settings/sms-senders.html new file mode 100644 index 000000000..ee11c3e4c --- /dev/null +++ b/app/templates/views/service-settings/sms-senders.html @@ -0,0 +1,49 @@ +{% extends "withnav_template.html" %} +{% from "components/api-key.html" import api_key %} +{% from "components/page-footer.html" import page_footer %} +{% from "components/table.html" import row_group, row, text_field, edit_field, field, boolean_field, list_table %} + +{% block service_page_title %} + Text message senders +{% endblock %} + +{% block maincolumn_content %} +
    +
    +

    + Text message senders +

    +
    +
    + Add text message sender +
    +
    +
    + {% if not sms_senders %} +
    + You haven’t added any sms senders yet +
    + {% endif %} + {% for item in sms_senders %} +
    +

    + {{ item.sms_sender }} + {% if item.hint %} +   + + {{ item.hint }} + + {% endif %} +

    + + {% if sms_senders|length > 1 %} + {{ api_key(item.id, thing="ID") }} + {% endif %} +
    + {% endfor %} +
    +{% endblock %} diff --git a/tests/app/main/views/service_settings/test_inbound_sms_setting.py b/tests/app/main/views/service_settings/test_inbound_sms_setting.py index 730436f38..768b2da5c 100644 --- a/tests/app/main/views/service_settings/test_inbound_sms_setting.py +++ b/tests/app/main/views/service_settings/test_inbound_sms_setting.py @@ -1,78 +1,64 @@ import app -import pytest -from bs4 import BeautifulSoup from flask import url_for -from notifications_python_client.errors import HTTPError +from tests.conftest import normalize_spaces -def test_set_text_message_sender( - logged_in_client, - mock_update_service, - service_one +def test_set_inbound_sms_sets_a_number_for_service( + logged_in_client, + mock_add_sms_sender, + multiple_available_inbound_numbers, + service_one, + fake_uuid, + mock_no_inbound_number_for_service, + mocker ): - data = {"sms_sender": "elevenchars"} - response = logged_in_client.post(url_for('main.service_set_sms_sender', service_id=service_one['id']), - data=data) - assert response.status_code == 302 - assert response.location == url_for('main.service_settings', service_id=service_one['id'], _external=True) + mocker.patch('app.service_api_client.update_service_with_properties') + data = { + "inbound_number": "781d9c60-7a7e-46b7-9896-7b045b992fa5", + } - mock_update_service.assert_called_with( + response = logged_in_client.post( + url_for('main.service_set_inbound_number', service_id=service_one['id']), + data=data + ) + + assert response.status_code == 302 + mock_add_sms_sender.assert_called_once_with( service_one['id'], - sms_sender="elevenchars" + sms_sender="781d9c60-7a7e-46b7-9896-7b045b992fa5", + is_default=True, + inbound_number_id="781d9c60-7a7e-46b7-9896-7b045b992fa5" ) -def test_get_inbound_number_in_service_settings( - logged_in_client, - mock_update_service, - mock_get_letter_organisations, - single_reply_to_email_address, - single_letter_contact_block, +def test_set_inbound_sms_when_no_available_inbound_numbers( + client_request, service_one, + no_available_inbound_numbers, + mock_no_inbound_number_for_service, mocker ): - mocker_get_inbound_number_fun = mocker.patch( - 'app.inbound_number_client.get_inbound_sms_number_for_service', - return_value={'data': {'number': '077777777', 'id': 'some_uuid'}}) + page = client_request.get( + 'main.service_set_inbound_number', + service_id=service_one['id'] + ) - response = logged_in_client.get(url_for('main.service_settings', service_id=service_one['id'])) - assert response.status_code == 200 - mocker_get_inbound_number_fun.assert_called_once_with(service_one['id']) - - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - element = page.find('span', {"id": "077777777"}) - assert not element + assert normalize_spaces(page.select_one('main p').text) == "No available inbound numbers" -def test_allow_inbound_sms_sets_a_number_for_service( - logged_in_client, - service_one, - mocker +def test_set_inbound_sms_when_service_already_has_sms( + client_request, + service_one, + multiple_available_inbound_numbers, + mock_get_inbound_number_for_service, ): - mocker.patch('app.service_api_client.update_service_with_properties') - mock_activate_inbound_sms = mocker.patch('app.inbound_number_client.activate_inbound_sms_service') - response = logged_in_client.get(url_for('main.service_set_inbound_number', - service_id=service_one['id'], - set_inbound_sms=True)) + page = client_request.get( + 'main.service_set_inbound_number', + service_id=service_one['id'] + ) - assert response.status_code == 302 - assert response.location == url_for('main.service_settings', service_id=service_one['id'], _external=True) - mock_activate_inbound_sms.assert_called_once_with(service_one['id']) - - -def test_allow_inbound_sms_returns_400_if_no_numbers_available( - logged_in_client, - service_one, - mocker -): - mock_switch_service = mocker.patch('app.service_api_client.update_service_with_properties') - mock_activate_inbound = mocker.patch('app.inbound_number_client.activate_inbound_sms_service', - side_effect=HTTPError) - logged_in_client.get( - url_for('main.service_set_inbound_number', service_id=service_one['id'], set_inbound_sms='True')) - mock_activate_inbound.assert_called_once_with(service_one['id']) - assert mock_switch_service.call_count == 2 + assert normalize_spaces(page.select_one('main p').text) == "This service already has an inbound number" def test_set_text_message_sender_and_inbound_sms_permission_exists_return_403( @@ -92,84 +78,3 @@ def test_set_text_message_sender_and_inbound_sms_permission_exists_return_403( assert not update_service_mock.called assert app.current_service['permissions'] == ['inbound_sms'] - - -def test_turn_inbound_sms_off( - logged_in_client, - service_one, - mocker -): - service_one['permissions'] = ['inbound_sms'] - update_service_mock = mocker.patch('app.service_api_client.update_service', - return_value=service_one) - mock_deactivate_inbound = mocker.patch('app.inbound_number_client.deactivate_inbound_sms_permission') - - response = logged_in_client.get(url_for('main.service_set_inbound_number', service_id=service_one['id'], - set_inbound_sms=False)) - assert response.status_code == 302 - assert response.location == url_for('main.service_set_sms_sender', service_id=service_one['id'], _external=True) - - assert app.current_service['permissions'] == [] - mock_deactivate_inbound.assert_called_once_with(service_id=service_one['id']) - assert update_service_mock.called - - -def test_set_text_message_sender_and_not_inbound_sms( - logged_in_client, - service_one, - mocker -): - service_one['permissions'] = [] - update_service_mock = mocker.patch('app.service_api_client.update_service', - return_value=service_one) - - data = {"sms_sender": "elevenchars"} - response = logged_in_client.post(url_for('main.service_set_sms_sender', service_id=service_one['id'], - set_inbound_sms=False), - data=data) - assert response.status_code == 302 - assert response.location == url_for('main.service_settings', service_id=service_one['id'], _external=True) - - update_service_mock.assert_called_with( - service_one['id'], - sms_sender="elevenchars" - ) - assert app.current_service['permissions'] == [] - - -@pytest.mark.parametrize('content, expected_error', [ - ("", "Can’t be empty"), - ("twelvecharss", "Enter 11 characters or fewer"), - (".", "Use letters and numbers only") -]) -def test_set_text_message_sender_validation( - logged_in_client, - mock_update_service, - service_one, - content, - expected_error, -): - response = logged_in_client.post(url_for( - 'main.service_set_sms_sender', - service_id=service_one['id']), - data={"sms_sender": content}, - follow_redirects=True - ) - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - - assert response.status_code == 200 - assert page.select(".error-message")[0].text.strip() == expected_error - assert not mock_update_service.called - - -def test_if_sms_sender_set_then_form_populated( - logged_in_client, - service_one, - mock_get_inbound_number_for_service -): - service_one['sms_sender'] = 'elevenchars' - response = logged_in_client.get(url_for('main.service_set_sms_sender', service_id=service_one['id'])) - - assert response.status_code == 200 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - assert page.find(id='sms_sender')['value'] == 'elevenchars' diff --git a/tests/app/main/views/service_settings/test_service_setting_permissions.py b/tests/app/main/views/service_settings/test_service_setting_permissions.py index e5c60e3f4..0002e23bc 100644 --- a/tests/app/main/views/service_settings/test_service_setting_permissions.py +++ b/tests/app/main/views/service_settings/test_service_setting_permissions.py @@ -14,6 +14,7 @@ def get_service_settings_page( mock_get_letter_organisations, no_reply_to_email_addresses, no_letter_contact_blocks, + single_sms_sender, ): platform_admin_request = client_request_factory(logged_in_platform_admin_client) return functools.partial(platform_admin_request.get, 'main.service_settings', service_id=service_one['id']) @@ -35,7 +36,6 @@ def get_service_settings_page( ({'permissions': ['sms']}, '.service_switch_can_send_sms', {}, 'Stop sending sms'), ({'permissions': []}, '.service_switch_can_send_sms', {}, 'Allow to send sms'), - ({'permissions': ['sms', 'inbound_sms']}, '.service_set_inbound_number', {'set_inbound_sms': False}, 'Stop inbound sms'), # noqa ({'permissions': ['sms']}, '.service_set_inbound_number', {'set_inbound_sms': True}, 'Allow inbound sms'), ({'active': True}, '.archive_service', {}, 'Archive service'), @@ -87,6 +87,7 @@ def test_normal_user_doesnt_see_any_toggle_buttons( mock_get_letter_organisations, no_reply_to_email_addresses, no_letter_contact_blocks, + single_sms_sender, ): page = client_request.get('main.service_settings', service_id=service_one['id']) toggles = page.find('a', {'class': 'button'}) diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index e07b27215..4ae91e081 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -14,15 +14,20 @@ from tests.conftest import ( active_user_with_permissions, platform_admin_user, normalize_spaces, - no_reply_to_email_addresses, multiple_reply_to_email_addresses, multiple_letter_contact_blocks, + multiple_sms_senders, + no_reply_to_email_addresses, no_letter_contact_blocks, + no_sms_senders, get_default_reply_to_email_address, get_non_default_reply_to_email_address, get_default_letter_contact_block, get_non_default_letter_contact_block, - SERVICE_ONE_ID, + get_default_sms_sender, + get_non_default_sms_sender, + get_inbound_number_sms_sender, + SERVICE_ONE_ID ) @@ -38,7 +43,7 @@ from tests.conftest import ( 'Label Value Action', 'Send text messages On Change', - 'Text message sender GOVUK Change', + 'Text message sender GOVUK Manage', 'International text messages Off Change', 'Receive text messages Off Change', @@ -57,7 +62,7 @@ from tests.conftest import ( 'Label Value Action', 'Send text messages On Change', - 'Text message sender GOVUK Change', + 'Text message sender GOVUK Manage', 'International text messages Off Change', 'Receive text messages Off Change', @@ -80,6 +85,7 @@ def test_should_show_overview( mock_get_letter_organisations, no_reply_to_email_addresses, no_letter_contact_blocks, + single_sms_sender, user, expected_rows, mock_get_inbound_number_for_service @@ -112,7 +118,7 @@ def test_should_show_overview( 'Label Value Action', 'Send text messages On Change', - 'Text message sender 0781239871', + 'Text message sender GOVUK Manage', 'International text messages On Change', 'Receive text messages On Change', 'API endpoint for received text messages Not set Change', @@ -131,7 +137,7 @@ def test_should_show_overview( 'Label Value Action', 'Send text messages On Change', - 'Text message sender GOVUK Change', + 'Text message sender GOVUK Manage', 'International text messages Off Change', 'Receive text messages Off Change', @@ -147,6 +153,7 @@ def test_should_show_overview_for_service_with_more_things_set( service_one, single_reply_to_email_address, single_letter_contact_block, + single_sms_sender, mock_get_organisation, mock_get_letter_organisations, mock_get_inbound_number_for_service, @@ -173,12 +180,13 @@ def test_service_settings_show_elided_api_url_if_needed( service_one, mock_get_letter_organisations, single_reply_to_email_address, + single_sms_sender, single_letter_contact_block, mocker, fake_uuid, url, elided_url, - mock_get_inbound_number_for_service, + mock_get_inbound_number_for_service ): service_one['permissions'] = ['sms', 'email', 'inbound_sms'] service_one['inbound_api'] = [fake_uuid] @@ -216,32 +224,13 @@ def test_if_cant_send_letters_then_cant_see_letter_contact_block( assert 'Letter contact block' not in response.get_data(as_text=True) -def test_if_can_receive_inbound_then_cant_change_sms_sender( - logged_in_client, - service_one, - mock_get_letter_organisations, - single_reply_to_email_address, - single_letter_contact_block, - mock_get_inbound_number_for_service -): - service_one['permissions'] = ['email', 'sms', 'inbound_sms'] - response = logged_in_client.get(url_for( - 'main.service_settings', service_id=service_one['id'] - )) - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - rows_as_text = [" ".join(row.text.split()) for row in page.find_all('tr')] - assert 'Text message sender 0781239871 Change' not in rows_as_text - assert url_for('main.service_request_to_go_live', service_id=service_one['id'], - set_inbound_sms=False) not in response.get_data(as_text=True) - assert '0781239871' in response.get_data(as_text=True) - - def test_letter_contact_block_shows_none_if_not_set( logged_in_client, service_one, mocker, single_reply_to_email_address, no_letter_contact_blocks, + single_sms_sender, mock_get_letter_organisations, mock_get_inbound_number_for_service ): @@ -261,6 +250,7 @@ def test_escapes_letter_contact_block( service_one, mocker, single_reply_to_email_address, + single_sms_sender, injected_letter_contact_block, mock_get_letter_organisations, mock_get_inbound_number_for_service @@ -312,6 +302,7 @@ def test_show_restricted_service( mock_get_letter_organisations, single_reply_to_email_address, single_letter_contact_block, + single_sms_sender, mock_get_inbound_number_for_service ): response = logged_in_client.get(url_for('main.service_settings', service_id=service_one['id'])) @@ -345,6 +336,7 @@ def test_show_live_service( mock_get_live_service, single_reply_to_email_address, single_letter_contact_block, + single_sms_sender, mock_get_letter_organisations, mock_get_inbound_number_for_service ): @@ -471,6 +463,7 @@ def test_should_redirect_after_request_to_go_live( active_user_with_permissions, single_reply_to_email_address, single_letter_contact_block, + single_sms_sender, mock_get_letter_organisations, mock_get_inbound_number_for_service, ): @@ -569,6 +562,7 @@ def test_route_permissions( service_one, single_reply_to_email_address, single_letter_contact_block, + single_sms_sender, mock_get_letter_organisations, route, mock_get_inbound_number_for_service @@ -628,6 +622,7 @@ def test_route_for_platform_admin( service_one, single_reply_to_email_address, single_letter_contact_block, + single_sms_sender, mock_get_letter_organisations, route, mock_get_inbound_number_for_service @@ -701,7 +696,8 @@ def test_and_more_hint_appears_on_settings_with_more_than_just_a_single_sender( mock_get_letter_organisations, mock_get_inbound_number_for_service, multiple_reply_to_email_addresses, - multiple_letter_contact_blocks + multiple_letter_contact_blocks, + multiple_sms_senders ): service_one['permissions'] = ['email', 'sms', 'letter'] @@ -710,22 +706,26 @@ def test_and_more_hint_appears_on_settings_with_more_than_just_a_single_sender( service_id=service_one['id'] ) - assert normalize_spaces( - page.select('tbody tr')[2].text - ) == "Email reply to addresses test@example.com …and 2 more Manage" - assert normalize_spaces( - page.select('tbody tr')[8].text - ) == "Sender addresses 1 Example Street …and 2 more Manage" + def get_row(page, index): + return normalize_spaces( + page.select('tbody tr')[index].text + ) + + assert get_row(page, 2) == "Email reply to addresses test@example.com …and 2 more Manage" + assert get_row(page, 4) == "Text message sender Example …and 2 more Manage" + assert get_row(page, 8) == "Sender addresses 1 Example Street …and 2 more Manage" @pytest.mark.parametrize('sender_list_page, expected_output', [ ('main.service_email_reply_to', 'test@example.com (default) Change'), ('main.service_letter_contact_details', '1 Example Street (default) Change'), + ('main.service_sms_senders', 'GOVUK (default) Change') ]) def test_api_ids_dont_show_on_option_pages_with_a_single_sender( client_request, single_reply_to_email_address, single_letter_contact_block, + single_sms_sender, sender_list_page, expected_output ): @@ -758,6 +758,12 @@ def test_api_ids_dont_show_on_option_pages_with_a_single_sender( '1 Example Street (default) Change 1234', '2 Example Street Change 5678', '3 Example Street Change 9457' + ), ( + 'main.service_sms_senders', + multiple_sms_senders, + 'Example (default and recieves replies) Change 1234', + 'Example 2 Change 5678', + 'Example 3 Change 9457' ), ] ) @@ -796,6 +802,11 @@ def test_default_option_shows_for_default_sender( no_letter_contact_blocks, 'You haven’t added any letter contact details yet' ), + ( + 'main.service_sms_senders', + no_sms_senders, + 'You haven’t added any sms senders yet' + ), ]) def test_no_senders_message_shows( client_request, @@ -822,7 +833,7 @@ def test_no_senders_message_shows( ('testtest', 'Enter a valid email address'), ('test@hello.com', 'Enter a government email address. If you think you should have access contact us') ]) -def test_incorrect_reply_to_email_address( +def test_incorrect_reply_to_email_address_input( reply_to_input, expected_error, client_request, @@ -842,7 +853,7 @@ def test_incorrect_reply_to_email_address( ('', 'Can’t be empty'), ('1 \n 2 \n 3 \n 4 \n 5 \n 6 \n 7 \n 8 \n 9 \n 0 \n a', 'Contains 11 lines, maximum is 10') ]) -def test_incorrect_letter_contact_block( +def test_incorrect_letter_contact_block_input( contact_block_input, expected_error, client_request, @@ -858,6 +869,26 @@ def test_incorrect_letter_contact_block( assert normalize_spaces(page.select_one('.error-message').text) == expected_error +@pytest.mark.parametrize('sms_sender_input, expected_error', [ + ('', 'Can’t be empty'), + ('abcdefghijkhgkg', 'Enter 11 characters or fewer') +]) +def test_incorrect_sms_sender_input( + sms_sender_input, + expected_error, + client_request, + no_sms_senders +): + page = client_request.post( + 'main.service_add_sms_sender', + service_id=SERVICE_ONE_ID, + _data={'sms_sender': sms_sender_input}, + _expected_status=200 + ) + + assert normalize_spaces(page.select_one('.error-message').text) == expected_error + + @pytest.mark.parametrize('fixture, data, api_default_args', [ (no_reply_to_email_addresses, {}, True), (multiple_reply_to_email_addresses, {}, False), @@ -914,6 +945,34 @@ def test_add_letter_contact( ) +@pytest.mark.parametrize('fixture, data, api_default_args', [ + (no_sms_senders, {}, True), + (multiple_sms_senders, {}, False), + (multiple_sms_senders, {"is_default": "y"}, True) +]) +def test_add_sms_sender( + fixture, + data, + api_default_args, + mocker, + client_request, + mock_add_sms_sender +): + fixture(mocker) + data['sms_sender'] = "Example" + client_request.post( + 'main.service_add_sms_sender', + service_id=SERVICE_ONE_ID, + _data=data + ) + + mock_add_sms_sender.assert_called_once_with( + SERVICE_ONE_ID, + sms_sender="Example", + is_default=api_default_args + ) + + @pytest.mark.parametrize('sender_page, fixture, checkbox_present', [ ('main.service_add_email_reply_to', no_reply_to_email_addresses, False), ('main.service_add_email_reply_to', multiple_reply_to_email_addresses, True), @@ -1000,6 +1059,37 @@ def test_edit_letter_contact_block( ) +@pytest.mark.parametrize('fixture, data, api_default_args', [ + (get_default_sms_sender, {"is_default": "y"}, True), + (get_default_sms_sender, {}, True), + (get_non_default_sms_sender, {}, False), + (get_non_default_sms_sender, {"is_default": "y"}, True) +]) +def test_edit_sms_sender( + fixture, + data, + api_default_args, + mocker, + fake_uuid, + client_request, + mock_update_sms_sender +): + fixture(mocker) + client_request.post( + 'main.service_edit_sms_sender', + service_id=SERVICE_ONE_ID, + sms_sender_id=fake_uuid, + _data=data + ) + + mock_update_sms_sender.assert_called_once_with( + SERVICE_ONE_ID, + sms_sender_id=fake_uuid, + sms_sender="GOVUK", + is_default=api_default_args + ) + + @pytest.mark.parametrize('sender_page, fixture, default_message, params, checkbox_present', [ ( 'main.service_edit_email_reply_to', @@ -1028,6 +1118,20 @@ def test_edit_letter_contact_block( 'This is the default contact details for service one letters', 'letter_contact_id', True + ), + ( + 'main.service_edit_sms_sender', + get_default_sms_sender, + 'This is currently your text message sender for service one', + 'sms_sender_id', + False + ), + ( + 'main.service_edit_sms_sender', + get_non_default_sms_sender, + 'This is currently your text message sender for service one', + 'sms_sender_id', + True ) ]) def test_default_box_shows_on_non_default_sender_details_while_editing( @@ -1059,6 +1163,34 @@ def test_default_box_shows_on_non_default_sender_details_while_editing( ) +@pytest.mark.parametrize('fixture, hide_textbox, fixture_sender_id', [ + (get_inbound_number_sms_sender, True, '1234'), + (get_default_sms_sender, False, '1234'), +]) +def test_inbound_sms_sender_is_not_editable( + client_request, + service_one, + fake_uuid, + fixture, + hide_textbox, + fixture_sender_id, + mocker +): + fixture(mocker) + + page = client_request.get( + '.service_edit_sms_sender', + service_id=SERVICE_ONE_ID, + sms_sender_id=fixture_sender_id, + ) + + assert (page.select_one('main input[name="sms_sender"]') is None) == hide_textbox + if hide_textbox: + assert normalize_spaces( + page.select_one('form[method="post"] p').text + ) == "GOVUK This phone number receives replies and can’t be changed" + + def test_switch_service_to_research_mode( logged_in_platform_admin_client, platform_admin_user, @@ -1107,7 +1239,8 @@ def test_shows_research_mode_indicator( mock_get_letter_organisations, single_reply_to_email_address, single_letter_contact_block, - mock_get_inbound_number_for_service, + single_sms_sender, + mock_get_inbound_number_for_service ): service_one['research_mode'] = True mocker.patch('app.service_api_client.update_service_with_properties', return_value=service_one) @@ -1126,7 +1259,8 @@ def test_does_not_show_research_mode_indicator( mock_get_letter_organisations, single_reply_to_email_address, single_letter_contact_block, - mock_get_inbound_number_for_service, + single_sms_sender, + mock_get_inbound_number_for_service ): response = logged_in_client.get(url_for('main.service_settings', service_id=service_one['id'])) assert response.status_code == 200 @@ -1692,8 +1826,9 @@ def test_archive_service_prompts_user( mocker, single_reply_to_email_address, single_letter_contact_block, + single_sms_sender, mock_get_letter_organisations, - mock_get_inbound_number_for_service, + mock_get_inbound_number_for_service ): mocked_fn = mocker.patch('app.service_api_client.post') @@ -1710,8 +1845,9 @@ def test_cant_archive_inactive_service( service_one, single_reply_to_email_address, single_letter_contact_block, + single_sms_sender, mock_get_letter_organisations, - mock_get_inbound_number_for_service, + mock_get_inbound_number_for_service ): service_one['active'] = False @@ -1743,8 +1879,9 @@ def test_suspend_service_prompts_user( mocker, single_reply_to_email_address, single_letter_contact_block, + single_sms_sender, mock_get_letter_organisations, - mock_get_inbound_number_for_service, + mock_get_inbound_number_for_service ): mocked_fn = mocker.patch('app.service_api_client.post') @@ -1762,8 +1899,9 @@ def test_cant_suspend_inactive_service( service_one, single_reply_to_email_address, single_letter_contact_block, + single_sms_sender, mock_get_letter_organisations, - mock_get_inbound_number_for_service, + mock_get_inbound_number_for_service ): service_one['active'] = False @@ -1797,9 +1935,10 @@ def test_resume_service_prompts_user( service_one, single_reply_to_email_address, single_letter_contact_block, + single_sms_sender, mocker, mock_get_letter_organisations, - mock_get_inbound_number_for_service, + mock_get_inbound_number_for_service ): service_one['active'] = False mocked_fn = mocker.patch('app.service_api_client.post') @@ -1818,8 +1957,9 @@ def test_cant_resume_active_service( service_one, single_reply_to_email_address, single_letter_contact_block, + single_sms_sender, mock_get_letter_organisations, - mock_get_inbound_number_for_service, + mock_get_inbound_number_for_service ): response = logged_in_platform_admin_client.get(url_for('main.service_settings', service_id=service_one['id'])) @@ -1880,6 +2020,7 @@ def test_service_settings_when_inbound_number_is_not_set( service_one, single_reply_to_email_address, single_letter_contact_block, + single_sms_sender, mocker, mock_get_letter_organisations, ): diff --git a/tests/conftest.py b/tests/conftest.py index 38ad538a6..d3a2a1a7d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -280,6 +280,7 @@ def multiple_sms_senders(mocker): 'sms_sender': 'Example', 'is_default': True, 'created_at': datetime.utcnow(), + 'inbound_number_id': '1234', 'updated_at': None }, { 'id': '5678', @@ -287,6 +288,7 @@ def multiple_sms_senders(mocker): 'sms_sender': 'Example 2', 'is_default': False, 'created_at': datetime.utcnow(), + 'inbound_number_id': None, 'updated_at': None }, { 'id': '9457', @@ -294,6 +296,7 @@ def multiple_sms_senders(mocker): 'sms_sender': 'Example 3', 'is_default': False, 'created_at': datetime.utcnow(), + 'inbound_number_id': None, 'updated_at': None } ] @@ -316,9 +319,10 @@ def single_sms_sender(mocker): { 'id': '1234', 'service_id': service_id, - 'sms_sender': 'Example', + 'sms_sender': 'GOVUK', 'is_default': True, 'created_at': datetime.utcnow(), + 'inbound_number_id': None, 'updated_at': None } ] @@ -332,9 +336,10 @@ def get_default_sms_sender(mocker): return { 'id': '1234', 'service_id': service_id, - 'sms_sender': 'Example', + 'sms_sender': 'GOVUK', 'is_default': True, 'created_at': datetime.utcnow(), + 'inbound_number_id': None, 'updated_at': None } @@ -347,15 +352,90 @@ def get_non_default_sms_sender(mocker): return { 'id': '1234', 'service_id': service_id, - 'service_id': service_id, + 'sms_sender': 'GOVUK', 'is_default': False, 'created_at': datetime.utcnow(), + 'inbound_number_id': None, 'updated_at': None } return mocker.patch('app.service_api_client.get_sms_sender', side_effect=_get) +@pytest.fixture(scope='function') +def get_inbound_number_sms_sender(mocker): + def _get(service_id, sms_sender_id): + return { + 'id': '1234', + 'service_id': service_id, + 'sms_sender': 'GOVUK', + 'is_default': False, + 'created_at': datetime.utcnow(), + 'inbound_number_id': '1234', + 'updated_at': None + } + + return mocker.patch('app.service_api_client.get_sms_sender', side_effect=_get) + + +@pytest.fixture(scope='function') +def mock_add_sms_sender(mocker): + def _add_sms_sender(service_id, sms_sender, is_default=False, inbound_number_id=None): + return + + return mocker.patch('app.service_api_client.add_sms_sender', side_effect=_add_sms_sender) + + +@pytest.fixture(scope='function') +def mock_update_sms_sender(mocker): + def _update_sms_sender(service_id, sms_sender_id, sms_sender, is_default=False): + return + + return mocker.patch('app.service_api_client.update_sms_sender', side_effect=_update_sms_sender) + + +@pytest.fixture(scope='function') +def multiple_available_inbound_numbers(mocker): + def _get(): + return {'data': [ + { + 'active': True, + 'created_at': '2017-10-18T16:57:14.154185Z', + 'id': '781d9c60-7a7e-46b7-9896-7b045b992fa7', + 'number': '0712121214', + 'provider': 'mmg', + 'service': None, + 'updated_at': None + }, { + 'active': True, + 'created_at': '2017-10-18T16:57:22.585806Z', + 'id': '781d9c60-7a7e-46b7-9896-7b045b992fa5', + 'number': '0712121215', + 'provider': 'mmg', + 'service': None, + 'updated_at': None + }, { + 'active': True, + 'created_at': '2017-10-18T16:57:38.585806Z', + 'id': '781d9c61-7a7e-46b7-9896-7b045b992fa5', + 'number': '0712121216', + 'provider': 'mmg', + 'service': None, + 'updated_at': None + } + ]} + + return mocker.patch('app.inbound_number_client.get_available_inbound_sms_numbers', side_effect=_get) + + +@pytest.fixture(scope='function') +def no_available_inbound_numbers(mocker): + def _get(): + return {'data': []} + + return mocker.patch('app.inbound_number_client.get_available_inbound_sms_numbers', side_effect=_get) + + @pytest.fixture(scope='function') def fake_uuid(): return sample_uuid() @@ -1638,6 +1718,13 @@ def mock_get_inbound_number_for_service(mocker): return_value={'data': {'number': '0781239871'}}) +@pytest.fixture(scope='function') +def mock_no_inbound_number_for_service(mocker): + return mocker.patch( + 'app.inbound_number_client.get_inbound_sms_number_for_service', + return_value={'data': {}}) + + @pytest.fixture(scope='function') def mock_has_permissions(mocker): def _has_permission(permissions=None, any_=False, admin_override=False): From 2ec3c26b2e03c0c154fdb6a7b4cbf126d5cb6846 Mon Sep 17 00:00:00 2001 From: pyup-bot Date: Mon, 30 Oct 2017 14:25:43 +0000 Subject: [PATCH 03/10] Update pytz from 2017.2 to 2017.3 --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 1f87c1bdb..27ed7fe20 100644 --- a/requirements.txt +++ b/requirements.txt @@ -11,7 +11,7 @@ pyexcel-io==0.5.3 pyexcel-xls==0.5.2 pyexcel-xlsx==0.5.2 pyexcel-ods3==0.5.2 -pytz==2017.2 +pytz==2017.3 gunicorn==19.7.1 whitenoise==3.3.1 #manages static assets From d02cd67b0dde033dc7fbef70e372ae035bba373f Mon Sep 17 00:00:00 2001 From: chrisw Date: Mon, 30 Oct 2017 14:30:43 +0000 Subject: [PATCH 04/10] Fixed broken edit functionality --- app/main/forms.py | 6 ++++- app/main/views/service_settings.py | 27 +++++++++++-------- .../service-settings/sms-sender/edit.html | 4 +-- tests/app/main/test_validators.py | 4 +-- tests/app/main/views/test_service_settings.py | 12 ++++----- 5 files changed, 31 insertions(+), 22 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index 82c62288e..75d15fb16 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -497,7 +497,7 @@ class ServiceReplyToEmailForm(Form): is_default = BooleanField("Make this email address the default") -class ServiceSmsSender(Form): +class ServiceSmsSenderForm(Form): sms_sender = StringField( 'Text message sender', validators=[ @@ -512,6 +512,10 @@ class ServiceSmsSender(Form): raise ValidationError('Use letters and numbers only') +class ServiceEditInboundNumberForm(Form): + is_default = BooleanField("Make this text message sender the default") + + class ServiceLetterContactBlockForm(Form): letter_contact_block = TextAreaField( validators=[ diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 2bc91599a..4c2cef034 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -29,7 +29,7 @@ from app.main.forms import ( RequestToGoLiveForm, ServiceReplyToEmailForm, ServiceInboundNumberForm, - ServiceSmsSender, + ServiceSmsSenderForm, ServiceLetterContactBlockForm, ServiceBrandingOrg, LetterBranding, @@ -37,6 +37,7 @@ from app.main.forms import ( InternationalSMSForm, OrganisationTypeForm, FreeSMSAllowance, + ServiceEditInboundNumberForm, ) from app import user_api_client, current_service, organisations_client, inbound_number_client from notifications_utils.formatters import formatted_list @@ -428,7 +429,7 @@ def service_edit_email_reply_to(service_id, reply_to_email_id): @login_required @user_has_permissions('manage_settings', admin_override=True) def service_set_sms_sender(service_id): - form = ServiceSmsSender() + form = ServiceSmsSenderForm() if form.validate_on_submit(): if 'inbound_sms' in current_service['permissions']: abort(403) @@ -608,7 +609,7 @@ def service_sms_senders(service_id): @login_required @user_has_permissions('manage_settings', admin_override=True) def service_add_sms_sender(service_id): - form = ServiceSmsSender() + form = ServiceSmsSenderForm() sms_sender_count = len(service_api_client.get_sms_senders(service_id)) first_sms_sender = sms_sender_count == 0 if form.validate_on_submit(): @@ -629,24 +630,28 @@ def service_add_sms_sender(service_id): @user_has_permissions('manage_settings', admin_override=True) def service_edit_sms_sender(service_id, sms_sender_id): sms_sender = service_api_client.get_sms_sender(service_id, sms_sender_id) - form = ServiceSmsSender() - form.sms_sender.data = sms_sender['sms_sender'] - is_inbound_number = True if sms_sender['inbound_number_id'] else False - if request.method == 'GET': - form.is_default.data = sms_sender['is_default'] + is_inbound_number = sms_sender['inbound_number_id'] + if is_inbound_number: + form = ServiceEditInboundNumberForm(is_default=sms_sender['is_default']) + else: + form = ServiceSmsSenderForm(**sms_sender) + if form.validate_on_submit(): service_api_client.update_sms_sender( current_service['id'], sms_sender_id=sms_sender_id, - sms_sender=form.sms_sender.data.replace('\r', ''), + sms_sender=sms_sender['sms_sender'] if is_inbound_number else form.sms_sender.data.replace('\r', ''), is_default=True if sms_sender['is_default'] else form.is_default.data ) return redirect(url_for('.service_sms_senders', service_id=service_id)) + + form.is_default.data = sms_sender['is_default'] return render_template( 'views/service-settings/sms-sender/edit.html', form=form, - sms_sender_id=sms_sender['id'], - is_inbound_number=is_inbound_number) + sms_sender=sms_sender, + inbound_number=is_inbound_number + ) @main.route("/services//service-settings/set-letter-contact-block", methods=['GET', 'POST']) diff --git a/app/templates/views/service-settings/sms-sender/edit.html b/app/templates/views/service-settings/sms-sender/edit.html index 81857c2cd..cccc8ef12 100644 --- a/app/templates/views/service-settings/sms-sender/edit.html +++ b/app/templates/views/service-settings/sms-sender/edit.html @@ -13,9 +13,9 @@ Edit text message sender
    - {% if is_inbound_number %} + {% if inbound_number %}

    - {{ form.sms_sender.data }} + {{ sms_sender.sms_sender }} This phone number receives replies and can’t be changed

    {% else %} diff --git a/tests/app/main/test_validators.py b/tests/app/main/test_validators.py index 7b70e74dd..663d7b438 100644 --- a/tests/app/main/test_validators.py +++ b/tests/app/main/test_validators.py @@ -1,5 +1,5 @@ import pytest -from app.main.forms import RegisterUserForm, ServiceSmsSender +from app.main.forms import RegisterUserForm, ServiceSmsSenderForm from app.main.validators import ValidGovEmail, NoCommasInPlaceHolders, OnlyGSMCharacters from wtforms import ValidationError from unittest.mock import Mock @@ -184,7 +184,7 @@ def test_sms_sender_form_validation( client, mock_get_user_by_email, ): - form = ServiceSmsSender() + form = ServiceSmsSenderForm() form.sms_sender.data = 'elevenchars' form.validate() diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 4ae91e081..3a7cc9884 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -1060,10 +1060,10 @@ def test_edit_letter_contact_block( @pytest.mark.parametrize('fixture, data, api_default_args', [ - (get_default_sms_sender, {"is_default": "y"}, True), - (get_default_sms_sender, {}, True), - (get_non_default_sms_sender, {}, False), - (get_non_default_sms_sender, {"is_default": "y"}, True) + (get_default_sms_sender, {"is_default": "y", "sms_sender": "test"}, True), + (get_default_sms_sender, {"sms_sender": "test"}, True), + (get_non_default_sms_sender, {"sms_sender": "test"}, False), + (get_non_default_sms_sender, {"is_default": "y", "sms_sender": "test"}, True) ]) def test_edit_sms_sender( fixture, @@ -1085,7 +1085,7 @@ def test_edit_sms_sender( mock_update_sms_sender.assert_called_once_with( SERVICE_ONE_ID, sms_sender_id=fake_uuid, - sms_sender="GOVUK", + sms_sender="test", is_default=api_default_args ) @@ -1184,7 +1184,7 @@ def test_inbound_sms_sender_is_not_editable( sms_sender_id=fixture_sender_id, ) - assert (page.select_one('main input[name="sms_sender"]') is None) == hide_textbox + assert bool(page.find('input', attrs={'name': "sms_sender"})) != hide_textbox if hide_textbox: assert normalize_spaces( page.select_one('form[method="post"] p').text From 273b864dce9d0048ac8fccb370fd98ab2f242cc5 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Mon, 30 Oct 2017 17:38:50 +0000 Subject: [PATCH 05/10] add auth_type default to InvitedUser object we unpack the api invited user rest endpoint results straight into the InvitedUser object, so we should make sure that any fields added to the api response are mentioned here --- app/notify_client/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/notify_client/models.py b/app/notify_client/models.py index 0ca6d8991..bedbbd051 100644 --- a/app/notify_client/models.py +++ b/app/notify_client/models.py @@ -141,7 +141,7 @@ class User(UserMixin): class InvitedUser(object): - def __init__(self, id, service, from_user, email_address, permissions, status, created_at): + def __init__(self, id, service, from_user, email_address, permissions, status, created_at, auth_type=None): self.id = id self.service = str(service) self.from_user = from_user From 061ef3dddc6f38b5c80901e2942f240b1a36f63e Mon Sep 17 00:00:00 2001 From: Pete Herlihy Date: Tue, 31 Oct 2017 12:27:34 +0000 Subject: [PATCH 06/10] 98 to 100 services..... woo hoo.... Also corrected the org count to 44 as HM Passports Office isn't a separate org from Home Office. --- app/templates/views/signedout.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/templates/views/signedout.html b/app/templates/views/signedout.html index a9d05fa34..e45a032eb 100644 --- a/app/templates/views/signedout.html +++ b/app/templates/views/signedout.html @@ -116,12 +116,12 @@
    -
    98
    +
    100
    services
    -
    45
    +
    44
    organisations
    From fafb8dc75b14116106d1708a1c22a0c07405963a Mon Sep 17 00:00:00 2001 From: Pete Herlihy Date: Wed, 1 Nov 2017 14:50:15 +0000 Subject: [PATCH 07/10] 100-102 for GOV.UK Email and SSCSA --- app/templates/views/signedout.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/templates/views/signedout.html b/app/templates/views/signedout.html index e45a032eb..607352fb7 100644 --- a/app/templates/views/signedout.html +++ b/app/templates/views/signedout.html @@ -116,7 +116,7 @@
    -
    100
    +
    102
    services
    From aff9d473233e6e3dbeb21a4f68092651ef0670f8 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Wed, 1 Nov 2017 14:39:14 +0000 Subject: [PATCH 08/10] don't hit API when checking new account email-token we currently store new account email verify tokens in the database, and check against that to work out if they've expired. But we don't need to do that, tokens have their own timing mechanism. So lets just use that, and free up the database to do other things. Also, standardised the forgot password, change email, and new account email verification timeouts to all be an hour, from the config val 'EMAIL_EXPIRY_SECONDS' --- app/config.py | 3 +- app/main/views/new_password.py | 8 ++-- app/main/views/verify.py | 50 ++++++++++------------- tests/app/main/views/test_new_password.py | 13 +++--- tests/app/main/views/test_verify.py | 37 ++++------------- 5 files changed, 41 insertions(+), 70 deletions(-) diff --git a/app/config.py b/app/config.py index 2cd1bc3fb..dce3d4104 100644 --- a/app/config.py +++ b/app/config.py @@ -41,7 +41,7 @@ class Config(object): 'local': 25000, 'nhs': 25000, } - EMAIL_EXPIRY_SECONDS = 3600 * 24 * 7 # one week + EMAIL_EXPIRY_SECONDS = 3600 # 1 hour HEADER_COLOUR = '#FFBF47' # $yellow HTTP_PROTOCOL = 'http' MAX_FAILED_LOGIN_COUNT = 10 @@ -56,7 +56,6 @@ class Config(object): SHOW_STYLEGUIDE = True # TODO: move to utils SMS_CHAR_COUNT_LIMIT = 459 - TOKEN_MAX_AGE_SECONDS = 3600 WTF_CSRF_ENABLED = True WTF_CSRF_TIME_LIMIT = None CSV_UPLOAD_BUCKET_NAME = 'local-notifications-csv-upload' diff --git a/app/main/views/new_password.py b/app/main/views/new_password.py index c615c5ea8..84f5051d8 100644 --- a/app/main/views/new_password.py +++ b/app/main/views/new_password.py @@ -1,20 +1,20 @@ +from datetime import datetime import json from flask import (render_template, url_for, redirect, flash, session, current_app) from itsdangerous import SignatureExpired +from notifications_utils.url_safe_token import check_token +from app import user_api_client from app.main import main from app.main.forms import NewPasswordForm -from datetime import datetime -from app import user_api_client @main.route('/new-password/', methods=['GET', 'POST']) def new_password(token): - from notifications_utils.url_safe_token import check_token try: token_data = check_token(token, current_app.config['SECRET_KEY'], current_app.config['DANGEROUS_SALT'], - current_app.config['TOKEN_MAX_AGE_SECONDS']) + current_app.config['EMAIL_EXPIRY_SECONDS']) except SignatureExpired: flash('The link in the email we sent you has expired. Enter your email address to resend.') return redirect(url_for('.forgot_password')) diff --git a/app/main/views/verify.py b/app/main/views/verify.py index a3163a538..e94af163e 100644 --- a/app/main/views/verify.py +++ b/app/main/views/verify.py @@ -50,34 +50,26 @@ def verify(): @main.route('/verify-email/') def verify_email(token): try: - token_data = check_token(token, - current_app.config['SECRET_KEY'], - current_app.config['DANGEROUS_SALT'], - current_app.config['EMAIL_EXPIRY_SECONDS']) - - token_data = json.loads(token_data) - verified = user_api_client.check_verify_code(token_data['user_id'], token_data['secret_code'], 'email') - user = user_api_client.get_user(token_data['user_id']) - if not user: - abort(404) - - if user.is_active: - flash("That verification link has expired.") - return redirect(url_for('main.sign_in')) - - session['user_details'] = {"email": user.email_address, "id": user.id} - if verified[0]: - user_api_client.send_verify_code(user.id, 'sms', user.mobile_number) - return redirect('verify') - else: - if verified[1] == 'Code has expired': - flash("The link in the email we sent you has expired. We've sent you a new one.") - return redirect(url_for('main.resend_email_verification')) - else: - message = "There was a problem verifying your account. Error message: '{}'".format(verified[1]) - flash(message) - return redirect(url_for('main.index')) - + token_data = check_token( + token, + current_app.config['SECRET_KEY'], + current_app.config['DANGEROUS_SALT'], + current_app.config['EMAIL_EXPIRY_SECONDS'] + ) except SignatureExpired: - flash('The link in the email we sent you has expired') + flash("The link in the email we sent you has expired. We've sent you a new one.") return redirect(url_for('main.resend_email_verification')) + + # token contains json blob of format: {'user_id': '...', 'secret_code': '...'} (secret_code is unused) + token_data = json.loads(token_data) + user = user_api_client.get_user(token_data['user_id']) + if not user: + abort(404) + + if user.is_active: + flash("That verification link has expired.") + return redirect(url_for('main.sign_in')) + + session['user_details'] = {"email": user.email_address, "id": user.id} + user_api_client.send_verify_code(user.id, 'sms', user.mobile_number) + return redirect('verify') diff --git a/tests/app/main/views/test_new_password.py b/tests/app/main/views/test_new_password.py index e18601970..6efbeec87 100644 --- a/tests/app/main/views/test_new_password.py +++ b/tests/app/main/views/test_new_password.py @@ -1,6 +1,7 @@ import json from datetime import datetime +from itsdangerous import SignatureExpired from flask import url_for from notifications_utils.url_safe_token import generate_token @@ -69,13 +70,13 @@ def test_should_redirect_index_if_user_has_already_changed_password( def test_should_redirect_to_forgot_password_with_flash_message_when_token_is_expired( app_, client, - mock_get_user_by_email_request_password_reset, mock_login, + mocker ): - app_.config['TOKEN_MAX_AGE_SECONDS'] = -1000 - user = mock_get_user_by_email_request_password_reset.return_value - token = generate_token(user.email_address, app_.config['SECRET_KEY'], app_.config['DANGEROUS_SALT']) - response = client.post(url_for('.new_password', token=token), data={'new_password': 'a-new_password'}) + mocker.patch('app.main.views.new_password.check_token', side_effect=SignatureExpired('expired')) + token = generate_token('foo@bar.com', app_.config['SECRET_KEY'], app_.config['DANGEROUS_SALT']) + + response = client.get(url_for('.new_password', token=token)) + assert response.status_code == 302 assert response.location == url_for('.forgot_password', _external=True) - app_.config['TOKEN_MAX_AGE_SECONDS'] = 3600 diff --git a/tests/app/main/views/test_verify.py b/tests/app/main/views/test_verify.py index ce27250b3..5cda6794f 100644 --- a/tests/app/main/views/test_verify.py +++ b/tests/app/main/views/test_verify.py @@ -1,6 +1,7 @@ import uuid import json +from itsdangerous import SignatureExpired from flask import url_for from bs4 import BeautifulSoup @@ -97,7 +98,7 @@ def test_verify_email_redirects_to_verify_if_token_valid( mock_send_verify_code, mock_check_verify_code, ): - token_data = {"user_id": api_user_pending.id, "secret_code": 12345} + token_data = {"user_id": api_user_pending.id, "secret_code": 'UNUSED'} mocker.patch('app.main.views.verify.check_token', return_value=json.dumps(token_data)) with client.session_transaction() as session: @@ -108,39 +109,20 @@ def test_verify_email_redirects_to_verify_if_token_valid( assert response.status_code == 302 assert response.location == url_for('main.verify', _external=True) + assert not mock_check_verify_code.called + mock_send_verify_code.assert_called_once_with(api_user_pending.id, 'sms', api_user_pending.mobile_number) + + with client.session_transaction() as session: + assert session['user_details'] == {'email': api_user_pending.email_address, 'id': api_user_pending.id} + def test_verify_email_redirects_to_email_sent_if_token_expired( client, mocker, api_user_pending, - mock_check_verify_code, ): - from itsdangerous import SignatureExpired mocker.patch('app.main.views.verify.check_token', side_effect=SignatureExpired('expired')) - with client.session_transaction() as session: - session['user_details'] = {'email_address': api_user_pending.email_address, 'id': api_user_pending.id} - - response = client.get(url_for('main.verify_email', token='notreal')) - - assert response.status_code == 302 - assert response.location == url_for('main.resend_email_verification', _external=True) - - -def test_verify_email_redirects_to_email_sent_if_token_used( - client, - mocker, - api_user_pending, - mock_get_user_pending, - mock_send_verify_code, - mock_check_verify_code_code_expired, -): - from itsdangerous import SignatureExpired - mocker.patch('app.main.views.verify.check_token', side_effect=SignatureExpired('expired')) - - with client.session_transaction() as session: - session['user_details'] = {'email_address': api_user_pending.email_address, 'id': api_user_pending.id} - response = client.get(url_for('main.verify_email', token='notreal')) assert response.status_code == 302 @@ -158,9 +140,6 @@ def test_verify_email_redirects_to_sign_in_if_user_active( token_data = {"user_id": api_user_active.id, "secret_code": 12345} mocker.patch('app.main.views.verify.check_token', return_value=json.dumps(token_data)) - with client.session_transaction() as session: - session['user_details'] = {'email_address': api_user_active.email_address, 'id': api_user_active.id} - response = client.get(url_for('main.verify_email', token='notreal'), follow_redirects=True) page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') assert page.h1.text == 'Sign in' From 19f731ec0754bfb8e36b61eff06c576afa59c802 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Wed, 1 Nov 2017 15:47:05 +0000 Subject: [PATCH 09/10] add error handler that catches invalid tokens, and returns 404 --- app/__init__.py | 10 +++++++++- tests/app/main/test_errorhandlers.py | 17 +++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/app/__init__.py b/app/__init__.py index 34aa8523e..ce425152b 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -5,6 +5,7 @@ from time import monotonic import itertools import ago +from itsdangerous import BadSignature from flask import ( Flask, session, @@ -13,7 +14,8 @@ from flask import ( current_app, request, g, - url_for + url_for, + flash ) from flask._compat import string_types from flask.globals import _lookup_req_object, _request_ctx_stack @@ -492,6 +494,12 @@ def register_errorhandlers(application): raise error return _error_response(500) + @application.errorhandler(BadSignature) + def handle_bad_token(error): + # if someone has a malformed token + flash('There’s something wrong with the link you’ve used.') + return _error_response(404) + def setup_event_handlers(): from flask_login import user_logged_in diff --git a/tests/app/main/test_errorhandlers.py b/tests/app/main/test_errorhandlers.py index f9185a1d1..92d712259 100644 --- a/tests/app/main/test_errorhandlers.py +++ b/tests/app/main/test_errorhandlers.py @@ -1,3 +1,4 @@ +import pytest from bs4 import BeautifulSoup @@ -6,3 +7,19 @@ def test_bad_url_returns_page_not_found(client): assert response.status_code == 404 page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') assert page.h1.string.strip() == 'Page could not be found' + + +@pytest.mark.parametrize('url', [ + '/invitation/MALFORMED_TOKEN', + '/new-password/MALFORMED_TOKEN', + '/user-profile/email/confirm/MALFORMED_TOKEN', + '/verify-email/MALFORMED_TOKEN' +]) +def test_malformed_token_returns_page_not_found(client, url): + response = client.get(url) + + assert response.status_code == 404 + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + assert page.h1.string.strip() == 'Page could not be found' + flash_banner = page.find('div', class_='banner-dangerous').string.strip() + assert flash_banner == "There’s something wrong with the link you’ve used." From 9eb5e6a532c5d7438ec36fc5458644450d45a444 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Wed, 1 Nov 2017 16:02:05 +0000 Subject: [PATCH 10/10] make sure invite tokens still check token on admin for error handler to kick in --- app/__init__.py | 2 +- app/main/views/invites.py | 15 ++++++++++----- tests/app/main/test_errorhandlers.py | 4 ++-- tests/app/main/views/test_accept_invite.py | 20 +++++++++++++++++--- 4 files changed, 30 insertions(+), 11 deletions(-) diff --git a/app/__init__.py b/app/__init__.py index ce425152b..4de099f5c 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -440,7 +440,7 @@ def useful_headers_after_request(response): return response -def register_errorhandlers(application): +def register_errorhandlers(application): # noqa (C901 too complex) def _error_response(error_code): application.logger.exception('Admin app errored with %s', error_code) resp = make_response(render_template("error/{0}.html".format(error_code)), error_code) diff --git a/app/main/views/invites.py b/app/main/views/invites.py index 41cbc8c72..af3d0eede 100644 --- a/app/main/views/invites.py +++ b/app/main/views/invites.py @@ -4,24 +4,29 @@ from flask import ( session, flash, render_template, - abort + abort, + current_app ) from markupsafe import Markup +from notifications_utils.url_safe_token import check_token +from flask_login import current_user from app.main import main - from app import ( invite_api_client, user_api_client, service_api_client ) -from flask_login import current_user - @main.route("/invitation/") def accept_invite(token): - + check_token( + token, + current_app.config['SECRET_KEY'], + current_app.config['DANGEROUS_SALT'], + current_app.config['EMAIL_EXPIRY_SECONDS'] + ) invited_user = invite_api_client.check_token(token) if not current_user.is_anonymous and current_user.email_address != invited_user.email_address: diff --git a/tests/app/main/test_errorhandlers.py b/tests/app/main/test_errorhandlers.py index 92d712259..93bbfe3e1 100644 --- a/tests/app/main/test_errorhandlers.py +++ b/tests/app/main/test_errorhandlers.py @@ -15,8 +15,8 @@ def test_bad_url_returns_page_not_found(client): '/user-profile/email/confirm/MALFORMED_TOKEN', '/verify-email/MALFORMED_TOKEN' ]) -def test_malformed_token_returns_page_not_found(client, url): - response = client.get(url) +def test_malformed_token_returns_page_not_found(logged_in_client, url): + response = logged_in_client.get(url) assert response.status_code == 404 page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') diff --git a/tests/app/main/views/test_accept_invite.py b/tests/app/main/views/test_accept_invite.py index aa7b14da7..fe4058f66 100644 --- a/tests/app/main/views/test_accept_invite.py +++ b/tests/app/main/views/test_accept_invite.py @@ -13,14 +13,14 @@ def test_existing_user_accept_invite_calls_api_and_redirects_to_dashboard( client, service_one, api_user_active, - sample_invite, - mock_get_service, mock_check_invite_token, mock_get_user_by_email, mock_get_users_by_service, mock_accept_invite, mock_add_user_to_service, + mocker, ): + mocker.patch('app.main.views.invites.check_token') expected_service = service_one['id'] expected_redirect_location = 'http://localhost/services/{}/dashboard'.format(expected_service) @@ -47,8 +47,8 @@ def test_existing_user_with_no_permissions_accept_invite( mock_get_user_by_email, mock_get_users_by_service, mock_add_user_to_service, - mock_get_service, ): + mocker.patch('app.main.views.invites.check_token') expected_service = service_one['id'] sample_invite['permissions'] = '' @@ -67,6 +67,7 @@ def test_if_existing_user_accepts_twice_they_redirect_to_sign_in( sample_invite, mock_get_service, ): + mocker.patch('app.main.views.invites.check_token') sample_invite['status'] = 'accepted' invite = InvitedUser(**sample_invite) @@ -93,6 +94,7 @@ def test_existing_user_of_service_get_redirected_to_signin( mock_get_user_by_email, mock_accept_invite, ): + mocker.patch('app.main.views.invites.check_token') sample_invite['email_address'] = api_user_active.email_address invite = InvitedUser(**sample_invite) mocker.patch('app.invite_api_client.check_token', return_value=invite) @@ -122,7 +124,9 @@ def test_existing_signed_out_user_accept_invite_redirects_to_sign_in( mock_add_user_to_service, mock_accept_invite, mock_get_service, + mocker, ): + mocker.patch('app.main.views.invites.check_token') expected_service = service_one['id'] expected_permissions = ['send_messages', 'manage_service', 'manage_api_keys'] @@ -153,7 +157,9 @@ def test_new_user_accept_invite_calls_api_and_redirects_to_registration( mock_add_user_to_service, mock_get_users_by_service, mock_get_service, + mocker, ): + mocker.patch('app.main.views.invites.check_token') expected_redirect_location = 'http://localhost/register-from-invite' @@ -174,7 +180,9 @@ def test_new_user_accept_invite_calls_api_and_views_registration_page( mock_add_user_to_service, mock_get_users_by_service, mock_get_service, + mocker, ): + mocker.patch('app.main.views.invites.check_token') response = client.get(url_for('main.accept_invite', token='thisisnotarealtoken'), follow_redirects=True) @@ -209,6 +217,7 @@ def test_cancelled_invited_user_accepts_invited_redirect_to_cancelled_invitation mock_get_user, mock_get_service, ): + mocker.patch('app.main.views.invites.check_token') cancelled_invitation = create_sample_invite(mocker, service_one, status='cancelled') mock_check_token_invite(mocker, cancelled_invitation) response = client.get(url_for('main.accept_invite', token='thisisnotarealtoken')) @@ -233,7 +242,9 @@ def test_new_user_accept_invite_completes_new_registration_redirects_to_verify( mock_get_users_by_service, mock_add_user_to_service, mock_get_service, + mocker, ): + mocker.patch('app.main.views.invites.check_token') expected_service = service_one['id'] expected_email = sample_invite['email_address'] @@ -282,6 +293,7 @@ def test_signed_in_existing_user_cannot_use_anothers_invite( mock_accept_invite, mock_get_service, ): + mocker.patch('app.main.views.invites.check_token') invite = InvitedUser(**sample_invite) mocker.patch('app.invite_api_client.check_token', return_value=invite) mocker.patch('app.user_api_client.get_users_for_service', return_value=[api_user_active]) @@ -322,7 +334,9 @@ def test_new_invited_user_verifies_and_added_to_service( mock_get_users_by_service, mock_get_detailed_service, mock_get_usage, + mocker, ): + mocker.patch('app.main.views.invites.check_token') # visit accept token page response = client.get(url_for('main.accept_invite', token='thisisnotarealtoken'))