diff --git a/app/__init__.py b/app/__init__.py index c27a4db36..8cf3c7147 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 @@ -451,7 +453,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) @@ -505,6 +507,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/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/forms.py b/app/main/forms.py index 33712c932..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=[ @@ -505,12 +505,17 @@ 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): 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=[ @@ -674,6 +679,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/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/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/service_settings.py b/app/main/views/service_settings.py index 63eb21c31..4c2cef034 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -28,7 +28,8 @@ from app.main.forms import ( RenameServiceForm, RequestToGoLiveForm, ServiceReplyToEmailForm, - ServiceSmsSender, + ServiceInboundNumberForm, + ServiceSmsSenderForm, ServiceLetterContactBlockForm, ServiceBrandingOrg, LetterBranding, @@ -36,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 @@ -82,6 +84,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 +101,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 ) @@ -420,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) @@ -438,22 +447,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 +580,80 @@ 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 = 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(): + 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) + 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=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=sms_sender, + 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/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/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/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 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..cccc8ef12 --- /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 inbound_number %} +

    + {{ sms_sender.sms_sender }} + 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/app/templates/views/signedout.html b/app/templates/views/signedout.html index a9d05fa34..607352fb7 100644 --- a/app/templates/views/signedout.html +++ b/app/templates/views/signedout.html @@ -116,12 +116,12 @@
    -
    98
    +
    102
    services
    -
    45
    +
    44
    organisations
    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); 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 diff --git a/tests/app/main/test_errorhandlers.py b/tests/app/main/test_errorhandlers.py index 62848a5ff..74c8d2b46 100644 --- a/tests/app/main/test_errorhandlers.py +++ b/tests/app/main/test_errorhandlers.py @@ -1,4 +1,5 @@ from flask import Response +import pytest from bs4 import BeautifulSoup from notifications_python_client.errors import HTTPError @@ -21,3 +22,19 @@ def test_load_service_before_request_handles_404(client_request, mocker): ) get_service.assert_called_once_with('00000000-0000-0000-0000-000000000000') + + +@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(logged_in_client, url): + response = logged_in_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." 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/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_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')) 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_service_settings.py b/tests/app/main/views/test_service_settings.py index e07b27215..3a7cc9884 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", "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, + 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="test", + 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 bool(page.find('input', attrs={'name': "sms_sender"})) != 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/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' 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):