diff --git a/.jshintrc b/.jshintrc new file mode 100644 index 000000000..368bcf310 --- /dev/null +++ b/.jshintrc @@ -0,0 +1 @@ +{"esversion": 6, "esnext": false} diff --git a/app/__init__.py b/app/__init__.py index 34aa8523e..c45ad9a84 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 @@ -404,10 +406,23 @@ def load_service_before_request(): if '/static/' in request.url: _request_ctx_stack.top.service = None return - service_id = request.view_args.get('service_id', session.get('service_id')) if request.view_args \ - else session.get('service_id') if _request_ctx_stack.top is not None: - _request_ctx_stack.top.service = service_api_client.get_service(service_id)['data'] if service_id else None + _request_ctx_stack.top.service = None + + if request.view_args: + service_id = request.view_args.get('service_id', session.get('service_id')) + else: + service_id = session.get('service_id') + + if service_id: + try: + _request_ctx_stack.top.service = service_api_client.get_service(service_id)['data'] + except HTTPError as exc: + # if service id isn't real, then 404 rather than 500ing later because we expect service to be set + if exc.status_code == 404: + abort(404) + else: + raise def save_service_after_request(response): @@ -426,6 +441,7 @@ def useful_headers_after_request(response): response.headers.add('Content-Security-Policy', ( "default-src 'self' 'unsafe-inline';" "script-src 'self' *.google-analytics.com 'unsafe-inline' 'unsafe-eval' data:;" + "connect-src 'self' *.google-analytics.com;" "object-src 'self';" "font-src 'self' data:;" "img-src 'self' *.google-analytics.com *.notifications.service.gov.uk {} data:;" @@ -438,7 +454,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) @@ -492,6 +508,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/assets/stylesheets/components/tick-cross.scss b/app/assets/stylesheets/components/tick-cross.scss index 4f80e08b3..4cf3f1151 100644 --- a/app/assets/stylesheets/components/tick-cross.scss +++ b/app/assets/stylesheets/components/tick-cross.scss @@ -63,6 +63,11 @@ right: -135px; } + &-hint { + color: #6F777B; + padding-top: 5px; + } + } } diff --git a/app/config.py b/app/config.py index 2cd1bc3fb..125b91834 100644 --- a/app/config.py +++ b/app/config.py @@ -41,7 +41,8 @@ class Config(object): 'local': 25000, 'nhs': 25000, } - EMAIL_EXPIRY_SECONDS = 3600 * 24 * 7 # one week + EMAIL_EXPIRY_SECONDS = 3600 # 1 hour + INVITATION_EXPIRY_SECONDS = 3600 * 24 * 2 # 2 days - also set on api HEADER_COLOUR = '#FFBF47' # $yellow HTTP_PROTOCOL = 'http' MAX_FAILED_LOGIN_COUNT = 10 @@ -56,7 +57,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 75d15fb16..5c3c389ca 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -186,6 +186,14 @@ class PermissionsForm(Form): manage_templates = BooleanField("Add and edit templates") manage_service = BooleanField("Modify this service and its team") manage_api_keys = BooleanField("Create and revoke API keys") + login_authentication = RadioField( + 'Sign in using', + choices=[ + ('sms_auth', 'Text message code'), + ('email_auth', 'Email link'), + ], + validators=[DataRequired()] + ) class InviteUserForm(PermissionsForm): @@ -693,10 +701,10 @@ class ServiceInboundNumberForm(Form): class ServiceInboundApiForm(Form): - url = StringField("Inbound sms url", + url = StringField("Callback URL", validators=[DataRequired(message='Can’t be empty'), Regexp(regex="^https.*", - message='Must be a valid https url')] + message='Must be a valid https URL')] ) bearer_token = PasswordFieldShowHasContent("Bearer token", validators=[DataRequired(message='Can’t be empty'), @@ -713,6 +721,16 @@ class InternationalSMSForm(Form): ) +class SMSPrefixForm(Form): + enabled = RadioField( + '', + choices=[ + ('on', 'On'), + ('off', 'Off'), + ], + ) + + def get_placeholder_form_instance( placeholder_name, dict_to_populate_from, diff --git a/app/main/views/index.py b/app/main/views/index.py index 412a8dbe4..24061b3cc 100644 --- a/app/main/views/index.py +++ b/app/main/views/index.py @@ -135,3 +135,8 @@ def using_notify(): @main.route('/information-risk-management') def information_risk_management(): return render_template('views/information-risk-management.html') + + +@main.route('/callbacks') +def callbacks(): + return render_template('views/callbacks.html') diff --git a/app/main/views/invites.py b/app/main/views/invites.py index 41cbc8c72..4f1c88f7a 100644 --- a/app/main/views/invites.py +++ b/app/main/views/invites.py @@ -4,23 +4,37 @@ from flask import ( session, flash, render_template, - abort + abort, + current_app ) +from itsdangerous import SignatureExpired 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): + try: + check_token( + token, + current_app.config['SECRET_KEY'], + current_app.config['DANGEROUS_SALT'], + current_app.config['INVITATION_EXPIRY_SECONDS'] + ) + except SignatureExpired: + errors = [ + 'Your invitation to GOV.UK Notify has expired. ' + 'Please ask the person that invited you to send you another one' + ] + return render_template("error/400.html", message=errors), 400 invited_user = invite_api_client.check_token(token) diff --git a/app/main/views/manage_users.py b/app/main/views/manage_users.py index 8eae948b3..40e1dc047 100644 --- a/app/main/views/manage_users.py +++ b/app/main/views/manage_users.py @@ -19,7 +19,7 @@ from app.main.forms import ( InviteUserForm, PermissionsForm ) -from app import (user_api_client, service_api_client, invite_api_client) +from app import (user_api_client, current_service, service_api_client, invite_api_client) from app.utils import user_has_permissions @@ -38,6 +38,7 @@ def manage_users(service_id): users = user_api_client.get_users_for_service(service_id=service_id) invited_users = [invite for invite in invite_api_client.get_invites_for_service(service_id=service_id) if invite.status != 'accepted'] + return render_template( 'views/manage-users.html', users=users, @@ -51,7 +52,13 @@ def manage_users(service_id): @user_has_permissions('manage_users', admin_override=True) def invite_user(service_id): - form = InviteUserForm(invalid_email_address=current_user.email_address) + form = InviteUserForm( + invalid_email_address=current_user.email_address + ) + + service_has_email_auth = 'email_auth' in current_service['permissions'] + if not service_has_email_auth: + form.login_authentication.data = 'sms_auth' if form.validate_on_submit(): email_address = form.email_address.data @@ -60,7 +67,8 @@ def invite_user(service_id): current_user.id, service_id, email_address, - permissions + permissions, + form.login_authentication.data ) flash('Invite sent to {}'.format(invited_user.email_address), 'default_with_tick') @@ -68,7 +76,8 @@ def invite_user(service_id): return render_template( 'views/invite-user.html', - form=form + form=form, + service_has_email_auth=service_has_email_auth ) @@ -76,26 +85,35 @@ def invite_user(service_id): @login_required @user_has_permissions('manage_users', admin_override=True) def edit_user_permissions(service_id, user_id): + service_has_email_auth = 'email_auth' in current_service['permissions'] # TODO we should probably using the service id here in the get user # call as well. eg. /user/?&service=service_id user = user_api_client.get_user(user_id) - # Need to make the email address read only, or a disabled field? - # Do it through the template or the form class? - form = PermissionsForm(**{ - role: user.has_permissions(permissions=permissions) for role, permissions in roles.items() - }) + user_has_no_mobile_number = user.mobile_number is None + + form = PermissionsForm( + **{role: user.has_permissions(permissions=permissions) for role, permissions in roles.items()}, + login_authentication=user.auth_type + ) if form.validate_on_submit(): user_api_client.set_user_permissions( user_id, service_id, permissions=set(get_permissions_from_form(form)), ) + if service_has_email_auth: + user_api_client.update_user_attribute( + user_id, + auth_type=form.login_authentication.data + ) return redirect(url_for('.manage_users', service_id=service_id)) return render_template( 'views/edit-user-permissions.html', user=user, - form=form + form=form, + service_has_email_auth=service_has_email_auth, + user_has_no_mobile_number=user_has_no_mobile_number ) 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/platform_admin.py b/app/main/views/platform_admin.py index 24195fd07..5db76551f 100644 --- a/app/main/views/platform_admin.py +++ b/app/main/views/platform_admin.py @@ -113,7 +113,6 @@ def create_global_stats(services): 'requested': 0 } } - for service in services: for msg_type, status in itertools.product(('sms', 'email', 'letter'), ('delivered', 'failed', 'requested')): stats[msg_type][status] += service['statistics'][msg_type][status] diff --git a/app/main/views/send.py b/app/main/views/send.py index 5c8555970..7835c9955 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -178,7 +178,7 @@ def set_sender(service_id, template_id): template = service_api_client.get_service_template(service_id, template_id)['data'] - if template['template_type'] != 'email': + if template['template_type'] == 'letter': return redirect_to_one_off sender_details = get_sender_details(service_id, template['template_type']) @@ -192,7 +192,12 @@ def set_sender(service_id, template_id): sender_choices=sender_context['value_and_label'], sender_label=sender_context['description'] ) - option_hints = {sender_context['default_id']: 'Default'} + option_hints = {sender_context['default_id']: '(Default)', + } + if sender_context.get('receives_text_message', None): + option_hints.update({sender_context['receives_text_message']: '(Receives replies)'}) + if sender_context.get('default_and_receives', None): + option_hints = {sender_context['default_and_receives']: '(Default and receives replies)'} if form.validate_on_submit(): session['sender_id'] = form.sender.data @@ -220,12 +225,24 @@ def get_sender_context(sender_details, template_type): 'title': 'Choose sender address', 'description': 'Select an address that recipients can reply to', 'field_name': 'contact_block' + }, + 'sms': { + 'title': 'Chose text message sender', + 'description': 'Select a text message sender that the recipients can reply to', + 'field_name': 'sms_sender' } }[template_type] sender_format = context['field_name'] context['default_id'] = next(sender['id'] for sender in sender_details if sender['is_default']) + if template_type == 'sms': + inbound = [sender['id'] for sender in sender_details if sender['inbound_number_id']] + if inbound: + context['receives_text_message'] = next(iter(inbound)) + if context['default_id'] == context.get('receives_text_message', None): + context['default_and_receives'] = context['default_id'] + context['value_and_label'] = [(sender['id'], sender[sender_format]) for sender in sender_details] return context @@ -233,7 +250,8 @@ def get_sender_context(sender_details, template_type): def get_sender_details(service_id, template_type): api_call = { 'email': service_api_client.get_reply_to_email_addresses, - 'letter': service_api_client.get_letter_contacts + 'letter': service_api_client.get_letter_contacts, + 'sms': service_api_client.get_sms_senders }[template_type] return api_call(service_id) @@ -248,7 +266,7 @@ def send_test(service_id, template_id): session['send_test_letter_page_count'] = None db_template = service_api_client.get_service_template(service_id, template_id)['data'] - if db_template['template_type'] != 'email': + if db_template['template_type'] == 'letter': session['sender_id'] = None if email_or_sms_not_enabled(db_template['template_type'], current_service['permissions']): @@ -311,7 +329,12 @@ def send_test_step(service_id, template_id, step_index): if not session.get('send_test_letter_page_count'): session['send_test_letter_page_count'] = get_page_count_for_letter(db_template) - + email_reply_to = None + sms_sender = None + if db_template['template_type'] == 'email': + email_reply_to = get_email_reply_to_address_from_session(service_id) + elif db_template['template_type'] == 'sms': + sms_sender = get_sms_sender_from_session(service_id) template = get_template( db_template, current_service, @@ -324,7 +347,8 @@ def send_test_step(service_id, template_id, step_index): filetype='png', ), page_count=session['send_test_letter_page_count'], - email_reply_to=get_email_reply_to_address_from_session(service_id), + email_reply_to=email_reply_to, + sms_sender=sms_sender ) placeholders = fields_to_fill_in( @@ -449,6 +473,12 @@ def _check_messages(service_id, template_type, upload_id, letters_as_pdf=False): remaining_messages = (current_service['message_limit'] - sum(stat['requested'] for stat in statistics.values())) contents = s3download(service_id, upload_id) + email_reply_to = None + sms_sender = None + if template_type == 'email': + email_reply_to = get_email_reply_to_address_from_session(service_id) + elif template_type == 'sms': + sms_sender = get_sms_sender_from_session(service_id) template = get_template( service_api_client.get_service_template( service_id, @@ -463,7 +493,8 @@ def _check_messages(service_id, template_type, upload_id, letters_as_pdf=False): upload_id=upload_id, filetype='png', ) if not letters_as_pdf else None, - email_reply_to=get_email_reply_to_address_from_session(service_id), + email_reply_to=email_reply_to, + sms_sender=sms_sender ) recipients = RecipientCSV( contents, @@ -723,12 +754,18 @@ def check_notification(service_id, template_id): def _check_notification(service_id, template_id, exception=None): db_template = service_api_client.get_service_template(service_id, template_id)['data'] - + email_reply_to = None + sms_sender = None + if db_template['template_type'] == 'email': + email_reply_to = get_email_reply_to_address_from_session(service_id) + elif db_template['template_type'] == 'sms': + sms_sender = get_sms_sender_from_session(service_id) template = get_template( db_template, current_service, show_recipient=True, - email_reply_to=get_email_reply_to_address_from_session(service_id), + email_reply_to=email_reply_to, + sms_sender=sms_sender ) # go back to start of process @@ -814,3 +851,10 @@ def get_email_reply_to_address_from_session(service_id): return service_api_client.get_reply_to_email_address( service_id, session['sender_id'] )['email_address'] + + +def get_sms_sender_from_session(service_id): + if session.get('sender_id'): + return service_api_client.get_sms_sender( + service_id=service_id, sms_sender_id=session['sender_id'] + )['sms_sender'] diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index ed89ada2c..3306cdcbc 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -38,6 +38,7 @@ from app.main.forms import ( OrganisationTypeForm, FreeSMSAllowance, ServiceEditInboundNumberForm, + SMSPrefixForm, ) from app import user_api_client, current_service, organisations_client, inbound_number_client, billing_api_client from notifications_utils.formatters import formatted_list @@ -108,7 +109,9 @@ def service_settings(service_id): letter_contact_details_count=letter_contact_details_count, default_sms_sender=default_sms_sender, sms_sender_count=sms_sender_count, - free_sms_fragment_limit=free_sms_fragment_limit + free_sms_fragment_limit=free_sms_fragment_limit, + prefix_sms_with_service_name=current_service['prefix_sms_with_service_name'], + ) @@ -491,6 +494,30 @@ def service_set_sms(service_id): ) +@main.route("/services//service-settings/sms-prefix", methods=['GET', 'POST']) +@login_required +@user_has_permissions('manage_settings', admin_override=True) +def service_set_sms_prefix(service_id): + + form = SMSPrefixForm(enabled=( + 'on' if current_service['prefix_sms_with_service_name'] else 'off' + )) + + form.enabled.label.text = 'Start all text messages with ‘{}:’'.format(current_service['name']) + + if form.validate_on_submit(): + service_api_client.update_service( + current_service['id'], + prefix_sms=(form.enabled.data == 'on') + ) + return redirect(url_for('.service_settings', service_id=service_id)) + + return render_template( + 'views/service-settings/sms-prefix.html', + form=form + ) + + @main.route("/services//service-settings/set-international-sms", methods=['GET', 'POST']) @login_required @user_has_permissions('manage_settings', admin_override=True) @@ -595,7 +622,7 @@ def service_sms_senders(service_id): if sender['is_default']: hints += ["default"] if sender['inbound_number_id']: - hints += ["recieves replies"] + hints += ["receives replies"] if hints: sender['hint'] = "(" + " and ".join(hints) + ")" 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/invite_api_client.py b/app/notify_client/invite_api_client.py index eae4fc2d6..352aaef77 100644 --- a/app/notify_client/invite_api_client.py +++ b/app/notify_client/invite_api_client.py @@ -12,12 +12,13 @@ class InviteApiClient(NotifyAdminAPIClient): self.service_id = app.config['ADMIN_CLIENT_USER_NAME'] self.api_key = app.config['ADMIN_CLIENT_SECRET'] - def create_invite(self, invite_from_id, service_id, email_address, permissions): + def create_invite(self, invite_from_id, service_id, email_address, permissions, auth_type): data = { 'service': str(service_id), 'email_address': email_address, 'from_user': invite_from_id, - 'permissions': permissions + 'permissions': permissions, + 'auth_type': auth_type } data = _attach_current_user(data) resp = self.post(url='/service/{}/invite'.format(service_id), data=data) diff --git a/app/notify_client/models.py b/app/notify_client/models.py index bedbbd051..fe9775a36 100644 --- a/app/notify_client/models.py +++ b/app/notify_client/models.py @@ -10,6 +10,7 @@ class User(UserMixin): self._mobile_number = fields.get('mobile_number') self._password_changed_at = fields.get('password_changed_at') self._permissions = fields.get('permissions') + self._auth_type = fields.get('auth_type') self._failed_login_count = fields.get('failed_login_count') self._state = fields.get('state') self.max_failed_login_count = max_failed_login_count @@ -108,6 +109,14 @@ class User(UserMixin): return set(self._permissions[service_id]) >= set(permissions) return False + @property + def auth_type(self): + return self._auth_type + + @auth_type.setter + def auth_type(self, auth_type): + self._auth_type = auth_type + @property def failed_login_count(self): return self._failed_login_count @@ -155,6 +164,7 @@ class InvitedUser(object): self.permissions = [] self.status = status self.created_at = created_at + self.auth_type = auth_type def has_permissions(self, permissions): return set(self.permissions) > set(permissions) @@ -164,10 +174,12 @@ class InvitedUser(object): self.service, self.from_user, self.email_address, + self.auth_type, self.status) == (other.id, other.service, other.from_user, other.email_address, + other.auth_type, other.status)) def serialize(self, permissions_as_string=False): @@ -176,7 +188,8 @@ class InvitedUser(object): 'from_user': self.from_user, 'email_address': self.email_address, 'status': self.status, - 'created_at': str(self.created_at) + 'created_at': str(self.created_at), + 'auth_type': self.auth_type } if permissions_as_string: data['permissions'] = ','.join(self.permissions) diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index e374487d5..2d7b1975b 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -107,6 +107,7 @@ class ServiceAPIClient(NotifyAdminAPIClient): 'permissions', 'organisation_type', 'free_sms_fragment_limit', + 'prefix_sms', } if disallowed_attributes: raise TypeError('Not allowed to update service attributes: {}'.format( diff --git a/app/notify_client/user_api_client.py b/app/notify_client/user_api_client.py index 887339fab..3e3170689 100644 --- a/app/notify_client/user_api_client.py +++ b/app/notify_client/user_api_client.py @@ -6,7 +6,8 @@ from app.notify_client.models import User ALLOWED_ATTRIBUTES = { 'name', 'email_address', - 'mobile_number' + 'mobile_number', + 'auth_type', } diff --git a/app/templates/views/callbacks.html b/app/templates/views/callbacks.html new file mode 100644 index 000000000..38ec51669 --- /dev/null +++ b/app/templates/views/callbacks.html @@ -0,0 +1,63 @@ +{% from "components/table.html" import mapping_table, row, text_field, edit_field, field, row_heading%} +{% extends "withoutnav_template.html" %} + +{% block per_page_title %} + Callbacks +{% endblock %} + +{% block maincolumn_content %} + +
+
+

Callbacks for received text messages

+ +

+ Text messages you receive can be forwarded to a URL that you specify, using our callback feature. +

+ +

+ Messages are forwarded as they are received. +

+ +

+ To protect your service, we require you to provide a bearer token. We put this token in the authorisation header of the callback requests. +

+ +

+ Once you have ‘receive text messages’ enabled, you can set up your callback on the settings page of your service. +

+ +

+ If you don’t have ‘receive text messages’ enabled for your service, get in touch and we can turn it on for you. +

+ +

Format of the callback

+ +

+ The format of the callback message you receive is JSON. +

+ +
+ {% call mapping_table( + caption='Callback message format', + field_headings=['Key', 'Description', 'Format'], + field_headings_visible=True, + caption_visible=False + ) %} + {% for key, description, format in [ + ('id', 'Notify’s id for the received message', 'UUID'), + ('source_number', 'The phone number the message was sent from', '447700912345'), + ('destination_number', 'The number the message was sent to (your number)', '07700987654'), + ('message', 'The received message', 'Hello Notify!'), + ('date_received', 'The UTC datetime that the message was received by Notify', '2017-05-14T12:15:30.000000Z') + ] %} + {% call row() %} + {% call row_heading() %} {{ key }} {% endcall %} + {{ text_field(description) }} + {{ text_field(format) }} + {% endcall %} + {% endfor %} + {% endcall %} +
+ +{% endblock %} diff --git a/app/templates/views/information-risk-management.html b/app/templates/views/information-risk-management.html index a79480c10..71724b479 100644 --- a/app/templates/views/information-risk-management.html +++ b/app/templates/views/information-risk-management.html @@ -33,7 +33,7 @@
  • formal risk assessments using a methodology based on ISO 27005:2011 and supplemented by reference to NCSC standards and guidance documentation
    • -
  • CHECK-based +
  • CHECK-based IT Health Check (ITHC) testing (annual and on major change)
  • residual risk statement preparation and active management of the risk treatment plan
  • regular updates to the Privacy Impact Assessment
    • diff --git a/app/templates/views/integration_testing.html b/app/templates/views/integration_testing.html index b16855cff..22164abc8 100644 --- a/app/templates/views/integration_testing.html +++ b/app/templates/views/integration_testing.html @@ -50,7 +50,7 @@

    You should revoke and re-create these keys on a regular basis. You can have more than one active key at a time. To revoke a key click the revoke button on the API Key page.

    - +

    You should never send test messages to invalid numbers or addresses using a live key. This will lead to your API key being revoked. @@ -73,7 +73,7 @@ Use a test key to test the performance of your service and its integration with GOV.UK Notify.

    - Test keys don’t send real messages but do generates realistic responses. + Test keys don’t send real messages but do generate realistic responses. There’s no restriction on who you can send to.

    diff --git a/app/templates/views/manage-users.html b/app/templates/views/manage-users.html index 373c5b3a6..9874e045e 100644 --- a/app/templates/views/manage-users.html +++ b/app/templates/views/manage-users.html @@ -63,6 +63,15 @@ user.has_permissions(permissions=['manage_api_keys']), 'Access API keys' ) }} + {% if 'email_auth' in current_service['permissions'] %} +

    + {% if user.auth_type == 'sms_auth' %} + Signs in with a text message code + {% else %} + Signs in with an email link + {% endif %} +
    + {% endif %}
    {% if current_user.has_permissions(['manage_users'], admin_override=True) %} {% if current_user.id != user.id %} @@ -104,6 +113,15 @@ user.has_permissions(permissions=['manage_api_keys']), 'Access API keys' ) }} + {% if 'email_auth' in current_service['permissions'] %} +
    + {% if user.auth_type == 'sms_auth' %} + Signs in with a text message code + {% else %} + Signs in with an email link + {% endif %} +
    + {% endif %}
  • {% if user.status == 'pending' and current_user.has_permissions(['manage_users']) %} diff --git a/app/templates/views/manage-users/permissions.html b/app/templates/views/manage-users/permissions.html index 8427a1b2e..e23eef804 100644 --- a/app/templates/views/manage-users/permissions.html +++ b/app/templates/views/manage-users/permissions.html @@ -1,4 +1,5 @@ {% from "components/checkbox.html" import checkbox %} +{% from "components/radios.html" import radios %}
    @@ -20,3 +21,7 @@
  • who the other team members are
    • + +{% if service_has_email_auth %} + {{ radios(form.login_authentication, disable=['sms_auth' if user_has_no_mobile_number]) }} +{% endif %} \ No newline at end of file diff --git a/app/templates/views/notifications.html b/app/templates/views/notifications.html index 40bf914a9..b76641b34 100644 --- a/app/templates/views/notifications.html +++ b/app/templates/views/notifications.html @@ -25,7 +25,7 @@ action="{{ url_for('.view_notifications', service_id=current_service.id, message_type=message_type) }}" class="grid-row" > -
    +
    {{ textbox( search_form.to, width='1-1', diff --git a/app/templates/views/platform-admin/_global_stats.html b/app/templates/views/platform-admin/_global_stats.html index 0a8f8d688..5cfe1c50b 100644 --- a/app/templates/views/platform-admin/_global_stats.html +++ b/app/templates/views/platform-admin/_global_stats.html @@ -23,8 +23,8 @@
    {{ big_number_with_status( - global_stats.letter.delivered + global_stats.letter.failed, - message_count_label(global_stats.letter.delivered, 'letter'), + global_stats.letter.requested, + message_count_label(global_stats.letter.requested, 'letter'), global_stats.letter.failed, global_stats.letter.failure_rate, global_stats.letter.failure_rate|float > 3, diff --git a/app/templates/views/platform-admin/services.html b/app/templates/views/platform-admin/services.html index d866658d1..a37267499 100644 --- a/app/templates/views/platform-admin/services.html +++ b/app/templates/views/platform-admin/services.html @@ -54,7 +54,7 @@ {% call row() %} {% if not service['active'] %} - {% call field(status='default') %} + {% call field(status='default', border=False) %} archived {% endcall %} {% elif service['research_mode'] %} @@ -62,16 +62,26 @@ research mode {% endcall %} {% elif not service['restricted'] %} - {% call field(status='error') %} + {% call field(status='error', border=False) %} Live {% endcall %} {% else %} - {{ text_field('') }} + {% call field(border=False) %} + {% endcall %} {% endif %} {{ stats_fields('sms', service['stats']) }} {% endcall %} + {% call row() %} + + {% call field(border=False) %} + + {% endcall %} + {{ stats_fields('letter', service['stats']) }} + + {% endcall %} + {% endcall %} {% endfor %} diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html index 7157affcd..528c6a56d 100644 --- a/app/templates/views/service-settings.html +++ b/app/templates/views/service-settings.html @@ -92,6 +92,12 @@ {{ edit_field('Manage' if sms_sender_count else 'Change', url_for('.service_sms_senders', service_id=current_service.id)) }} {% endcall %} + {% call row() %} + {{ text_field('Text messages start with service name') }} + {{ boolean_field(prefix_sms_with_service_name) }} + {{ edit_field('Change', url_for('.service_set_sms_prefix', service_id=current_service.id)) }} + {% endcall %} + {% call row() %} {{ text_field('International text messages') }} {{ boolean_field('international_sms' in current_service.permissions) }} @@ -106,7 +112,7 @@ {% if can_receive_inbound %} {% call row() %} - {{ text_field('API endpoint for received text messages') }} + {{ text_field('Callback URL for received text messages') }} {{ optional_text_field(inbound_api_url) }} {{ edit_field('Change', url_for('.service_set_inbound_api', service_id=current_service.id)) }} {% endcall %} diff --git a/app/templates/views/service-settings/set-inbound-api.html b/app/templates/views/service-settings/set-inbound-api.html index 0609ff126..9530b36d5 100644 --- a/app/templates/views/service-settings/set-inbound-api.html +++ b/app/templates/views/service-settings/set-inbound-api.html @@ -3,27 +3,27 @@ {% from "components/page-footer.html" import page_footer %} {% block service_page_title %} - Inbound api + Callback URL {% endblock %} {% block maincolumn_content %}
    -

    API endpoint for received text messages

    +

    Callback URL for received text messages

    - This is the https url that the inbound SMS messages will be posted to - and the bearer token used in the authorisation header of the request. + Text messages you receive can be forwarded to your systems with our callback feature. + See our documentation on the format of the callback.

    {{ textbox( form.url, width='2-3', - hint='Valid https url' + hint='Valid https URL' ) }} {{ textbox( form.bearer_token, - width='1-4', + width='2-3', hint='At least 10 characters' ) }} {{ page_footer( @@ -35,4 +35,4 @@
    -{% endblock %} \ No newline at end of file +{% endblock %} diff --git a/app/templates/views/service-settings/sms-prefix.html b/app/templates/views/service-settings/sms-prefix.html new file mode 100644 index 000000000..e74508596 --- /dev/null +++ b/app/templates/views/service-settings/sms-prefix.html @@ -0,0 +1,21 @@ +{% extends "withnav_template.html" %} +{% from "components/radios.html" import radios %} +{% from "components/page-footer.html" import page_footer %} + +{% block service_page_title %} + Text messages start with service name +{% endblock %} + +{% block maincolumn_content %} + +

    Text messages start with service name

    + + {{ radios(form.enabled) }} + {{ page_footer( + button_text="Save", + back_link=url_for('.service_settings', service_id=current_service.id), + back_link_text='Back to settings' + ) }} + + +{% endblock %} diff --git a/app/templates/views/signedout.html b/app/templates/views/signedout.html index a9d05fa34..64776c5e6 100644 --- a/app/templates/views/signedout.html +++ b/app/templates/views/signedout.html @@ -116,7 +116,7 @@
    -
    98
    +
    104
    services
    diff --git a/app/utils.py b/app/utils.py index d7ad0ce15..3a69f4486 100644 --- a/app/utils.py +++ b/app/utils.py @@ -272,6 +272,7 @@ def get_template( page_count=1, redact_missing_personalisation=False, email_reply_to=None, + sms_sender=None ): if 'email' == template['template_type']: return EmailPreviewTemplate( @@ -287,7 +288,7 @@ def get_template( return SMSPreviewTemplate( template, prefix=service['name'], - sender=(service['sms_sender'] not in {'GOVUK', None}), + sender=not service['prefix_sms_with_service_name'], show_recipient=show_recipient, redact_missing_personalisation=redact_missing_personalisation, ) diff --git a/gulpfile.babel.js b/gulpfile.babel.js index 9ecc9c15d..872630dfb 100644 --- a/gulpfile.babel.js +++ b/gulpfile.babel.js @@ -138,7 +138,7 @@ gulp.task('lint:sass', () => gulp gulp.task('lint:js', () => gulp .src(paths.src + 'javascripts/**/*.js') - .pipe(plugins.jshint({'esversion': 6, 'esnext': false})) + .pipe(plugins.jshint()) .pipe(plugins.jshint.reporter(stylish)) .pipe(plugins.jshint.reporter('fail')) ); diff --git a/requirements.txt b/requirements.txt index 27ed7fe20..32b11192f 100644 --- a/requirements.txt +++ b/requirements.txt @@ -8,15 +8,15 @@ boto3==1.4.7 blinker==1.4 pyexcel==0.5.6 pyexcel-io==0.5.3 -pyexcel-xls==0.5.2 -pyexcel-xlsx==0.5.2 +pyexcel-xls==0.5.4 +pyexcel-xlsx==0.5.4 pyexcel-ods3==0.5.2 pytz==2017.3 gunicorn==19.7.1 whitenoise==3.3.1 #manages static assets # pin to minor version 3.1.x -notifications-python-client==4.5.0 +notifications-python-client==4.6.0 # PaaS awscli>=1.11,<1.12 diff --git a/tests/__init__.py b/tests/__init__.py index b48e2e33e..f59633f55 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -53,16 +53,19 @@ def service_json( created_at=None, letter_contact_block=None, inbound_api=None, - permissions=['email', 'sms'], + permissions=None, organisation_type='central', free_sms_fragment_limit=250000, + prefix_sms_with_service_name='Treat as None', ): if users is None: users = [] if permissions is None: - permissions = [] + permissions = ['email', 'sms'] if inbound_api is None: inbound_api = [] + if prefix_sms_with_service_name == 'Treat as None': + prefix_sms_with_service_name = (sms_sender == 'GOVUK') return { 'id': id_, 'name': name, @@ -83,6 +86,7 @@ def service_json( 'dvla_organisation': '001', 'permissions': permissions, 'inbound_api': inbound_api, + 'prefix_sms_with_service_name': prefix_sms_with_service_name, } diff --git a/tests/app/main/test_errorhandlers.py b/tests/app/main/test_errorhandlers.py index f9185a1d1..74c8d2b46 100644 --- a/tests/app/main/test_errorhandlers.py +++ b/tests/app/main/test_errorhandlers.py @@ -1,4 +1,7 @@ +from flask import Response +import pytest from bs4 import BeautifulSoup +from notifications_python_client.errors import HTTPError def test_bad_url_returns_page_not_found(client): @@ -6,3 +9,32 @@ 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' + + +def test_load_service_before_request_handles_404(client_request, mocker): + exc = HTTPError(Response(status=404), 'Not found') + get_service = mocker.patch('app.service_api_client.get_service', side_effect=exc) + + client_request.get( + 'main.service_dashboard', + service_id='00000000-0000-0000-0000-000000000000', + _expected_status=404 + ) + + 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/views/test_accept_invite.py b/tests/app/main/views/test_accept_invite.py index aa7b14da7..fd1f28924 100644 --- a/tests/app/main/views/test_accept_invite.py +++ b/tests/app/main/views/test_accept_invite.py @@ -1,6 +1,7 @@ from flask import url_for from bs4 import BeautifulSoup from unittest.mock import ANY +from itsdangerous import SignatureExpired import app @@ -13,14 +14,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 +48,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 +68,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 +95,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 +125,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 +158,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 +181,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 +218,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 +243,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 +294,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 +335,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')) @@ -354,3 +369,21 @@ def test_new_invited_user_verifies_and_added_to_service( raw_html = response.data.decode('utf-8') page = BeautifulSoup(raw_html, 'html.parser') assert page.find('h1').text == 'Dashboard' + + +def test_gives_message_if_token_has_expired( + app_, + client, + mock_check_invite_token, + mocker, +): + check_token = mocker.patch('app.main.views.invites.check_token', side_effect=SignatureExpired('this is too old')) + + response = client.get(url_for('main.accept_invite', token='a really old token')) + raw_html = response.data.decode('utf-8') + page = BeautifulSoup(raw_html, 'html.parser') + + check_token.assert_called_once_with(ANY, ANY, ANY, 3600 * 24 * 2) + assert response.status_code == 400 + assert 'Your invitation to GOV.UK Notify has expired' in page.find('h1').text + assert not mock_check_invite_token.called diff --git a/tests/app/main/views/test_headers.py b/tests/app/main/views/test_headers.py index 130beac20..7e06b3961 100644 --- a/tests/app/main/views/test_headers.py +++ b/tests/app/main/views/test_headers.py @@ -10,6 +10,7 @@ def test_owasp_useful_headers_set(client, mocker): assert response.headers['Content-Security-Policy'] == ( "default-src 'self' 'unsafe-inline';" "script-src 'self' *.google-analytics.com 'unsafe-inline' 'unsafe-eval' data:;" + "connect-src 'self' *.google-analytics.com;" "object-src 'self';" "font-src 'self' data:;" "img-src 'self' *.google-analytics.com *.notifications.service.gov.uk static-logos.test.com data:;" diff --git a/tests/app/main/views/test_index.py b/tests/app/main/views/test_index.py index 46dd31da7..2e46706e9 100644 --- a/tests/app/main/views/test_index.py +++ b/tests/app/main/views/test_index.py @@ -18,7 +18,7 @@ def test_logged_in_user_redirects_to_choose_service( @pytest.mark.parametrize('view', [ 'cookies', 'using_notify', 'pricing', 'terms', 'integration_testing', 'roadmap', - 'features', 'information_risk_management' + 'features', 'information_risk_management', 'callbacks' ]) def test_static_pages( client, diff --git a/tests/app/main/views/test_manage_users.py b/tests/app/main/views/test_manage_users.py index 2766cd032..c54da8a70 100644 --- a/tests/app/main/views/test_manage_users.py +++ b/tests/app/main/views/test_manage_users.py @@ -10,6 +10,7 @@ from tests.conftest import ( SERVICE_ONE_ID, active_user_with_permissions, active_user_view_permissions, + active_user_no_mobile, active_user_manage_template_permission, ) @@ -55,6 +56,94 @@ def test_should_show_overview_page( app.user_api_client.get_users_for_service.assert_called_once_with(service_id=SERVICE_ONE_ID) +@pytest.mark.parametrize('endpoint, extra_args, service_has_email_auth, auth_options_hidden', [ + ( + 'main.edit_user_permissions', + {'user_id': 0}, + True, + False + ), + ( + 'main.edit_user_permissions', + {'user_id': 0}, + False, + True + ), + ( + 'main.invite_user', + {}, + True, + False + ), + ( + 'main.invite_user', + {}, + False, + True + ) +]) +def test_service_with_no_email_auth_hides_auth_type_options( + client_request, + endpoint, + extra_args, + service_has_email_auth, + auth_options_hidden, + service_one +): + if service_has_email_auth: + service_one['permissions'].append('email_auth') + page = client_request.get(endpoint, service_id=service_one['id'], **extra_args) + assert (page.find('input', attrs={"name": "login_authentication"}) is None) == auth_options_hidden + + +@pytest.mark.parametrize('service_has_email_auth, displays_auth_type', [ + (True, True), + (False, False) +]) +def test_manage_users_page_shows_member_auth_type_if_service_has_email_auth_activated( + client_request, + service_has_email_auth, + service_one, + mock_get_users_by_service, + mock_get_invites_for_service, + displays_auth_type +): + if service_has_email_auth: + service_one['permissions'].append('email_auth') + page = client_request.get('main.manage_users', service_id=service_one['id']) + assert bool(page.select_one('.tick-cross-list-hint')) == displays_auth_type + + +@pytest.mark.parametrize('user, sms_option_disabled', [ + ( + active_user_no_mobile, + True, + ), + ( + active_user_with_permissions, + False, + ), +]) +def test_user_with_no_mobile_number_cant_be_set_to_sms_auth( + client_request, + user, + sms_option_disabled, + service_one, + mocker +): + service_one['permissions'].append('email_auth') + test_user = mocker.patch('app.user_api_client.get_user', return_value=user(mocker)) + + page = client_request.get( + 'main.edit_user_permissions', + service_id=service_one['id'], + user_id=test_user.id + ) + + sms_auth_radio_button = page.select_one('input[value="sms_auth"]') + assert sms_auth_radio_button.has_attr("disabled") == sms_option_disabled + + @pytest.mark.parametrize('endpoint, extra_args, expected_checkboxes', [ ( 'main.edit_user_permissions', @@ -167,6 +256,60 @@ def test_edit_some_user_permissions( ) +@pytest.mark.parametrize('auth_type', ['email_auth', 'sms_auth']) +def test_edit_user_permissions_including_authentication_with_email_auth_service( + logged_in_client, + active_user_with_permissions, + mocker, + mock_get_invites_for_service, + mock_set_user_permissions, + mock_update_user_attribute, + service_one, + auth_type +): + service_one['permissions'].append('email_auth') + + response = logged_in_client.post( + url_for( + 'main.edit_user_permissions', + service_id=service_one['id'], + user_id=active_user_with_permissions.id + ), + data={ + 'email_address': active_user_with_permissions.email_address, + 'send_messages': 'y', + 'manage_templates': 'y', + 'manage_service': 'y', + 'manage_api_keys': 'y', + 'login_authentication': auth_type + } + ) + + mock_set_user_permissions.assert_called_with( + str(active_user_with_permissions.id), + service_one['id'], + permissions={ + 'send_texts', + 'send_emails', + 'send_letters', + 'manage_users', + 'manage_templates', + 'manage_settings', + 'manage_api_keys', + 'view_activity' + } + ) + mock_update_user_attribute.assert_called_with( + str(active_user_with_permissions.id), + auth_type=auth_type + ) + + assert response.status_code == 302 + assert response.location == url_for( + 'main.manage_users', service_id=service_one['id'], _external=True + ) + + def test_should_show_page_for_inviting_user( logged_in_client, active_user_with_permissions, @@ -220,7 +363,60 @@ def test_invite_user( app.invite_api_client.create_invite.assert_called_once_with(sample_invite['from_user'], sample_invite['service'], email_address, - expected_permissions) + expected_permissions, + 'sms_auth') + + +@pytest.mark.parametrize('auth_type', [ + ('sms_auth'), + ('email_auth') +]) +@pytest.mark.parametrize('email_address, gov_user', [ + ('test@example.gov.uk', True), + ('test@nonwhitelist.com', False) +]) +def test_invite_user_with_email_auth_service( + logged_in_client, + active_user_with_permissions, + sample_invite, + email_address, + gov_user, + mocker, + service_one, + auth_type +): + service_one['permissions'].append('email_auth') + sample_invite['email_address'] = 'test@example.gov.uk' + + data = [InvitedUser(**sample_invite)] + assert is_gov_user(email_address) == gov_user + mocker.patch('app.invite_api_client.get_invites_for_service', return_value=data) + mocker.patch('app.user_api_client.get_users_for_service', return_value=[active_user_with_permissions]) + mocker.patch('app.invite_api_client.create_invite', return_value=InvitedUser(**sample_invite)) + response = logged_in_client.post( + url_for('main.invite_user', service_id=service_one['id']), + data={'email_address': email_address, + 'send_messages': 'y', + 'manage_templates': 'y', + 'manage_service': 'y', + 'manage_api_keys': 'y', + 'login_authentication': auth_type}, + follow_redirects=True + ) + + assert response.status_code == 200 + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + assert page.h1.string.strip() == 'Team members' + flash_banner = page.find('div', class_='banner-default-with-tick').string.strip() + assert flash_banner == 'Invite sent to test@example.gov.uk' + + expected_permissions = 'manage_api_keys,manage_settings,manage_templates,manage_users,send_emails,send_letters,send_texts,view_activity' # noqa + + app.invite_api_client.create_invite.assert_called_once_with(sample_invite['from_user'], + sample_invite['service'], + email_address, + expected_permissions, + auth_type) def test_cancel_invited_user_cancels_user_invitations( 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_platform_admin.py b/tests/app/main/views/test_platform_admin.py index 07f89079b..62ee4d451 100644 --- a/tests/app/main/views/test_platform_admin.py +++ b/tests/app/main/views/test_platform_admin.py @@ -97,7 +97,7 @@ def test_should_render_platform_admin_page( response = client.get(url_for(endpoint)) assert response.status_code == 200 page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - assert len(page.select('tbody tr')) == expected_services_shown * 2 # one row for SMS, one for email + assert len(page.select('tbody tr')) == expected_services_shown * 3 # one row for SMS, one for email, one for letter mock_get_detailed_services.assert_called_once_with({'detailed': True, 'include_from_test_key': True, 'only_active': False}) @@ -543,7 +543,7 @@ def test_should_show_correct_sent_totals_for_platform_admin( assert email_total == 60 assert sms_total == 40 - assert letter_total == 45 + assert letter_total == 60 @pytest.mark.parametrize('endpoint, restricted, research_mode', [ diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index 82c1a0ef5..5d99b6d27 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -27,6 +27,10 @@ from tests.conftest import ( mock_get_live_service, multiple_reply_to_email_addresses, no_reply_to_email_addresses, + multiple_sms_senders, + no_sms_senders, + multiple_sms_senders_with_diff_default, + multiple_sms_senders_no_inbound ) template_types = ['email', 'sms'] @@ -43,6 +47,12 @@ test_non_spreadsheet_files = glob(path.join('tests', 'non_spreadsheet_files', '* 'Choose where to send replies', 'Select an email address that recipients can reply to' ), + ( + mock_get_service_template, + multiple_sms_senders, + 'Chose text message sender', + 'Select a text message sender that the recipients can reply to' + ) ]) def test_show_correct_title_and_description_for_sender_type( client_request, @@ -67,12 +77,47 @@ def test_show_correct_title_and_description_for_sender_type( assert normalize_spaces(page.select_one('legend').text) == expected_description +@pytest.mark.parametrize('template_mock, sender_data', [ + ( + mock_get_service_email_template, + multiple_reply_to_email_addresses, + ), + ( + mock_get_service_template, + multiple_sms_senders_with_diff_default + ), + ( + mock_get_service_template, + multiple_sms_senders_no_inbound + ) +]) def test_default_sender_is_checked_and_has_hint( client_request, service_one, fake_uuid, - mock_get_service_email_template, - multiple_reply_to_email_addresses + template_mock, + sender_data, + mocker +): + template_mock(mocker) + sender_data(mocker) + page = client_request.get( + '.set_sender', + service_id=service_one['id'], + template_id=fake_uuid + ) + + assert page.select('.multiple-choice input')[0].has_attr('checked') + assert normalize_spaces(page.select_one('.multiple-choice label .block-label-hint').text) == "(Default)" + assert not page.select('.multiple-choice input')[1].has_attr('checked') + + +def test_default_inbound_sender_is_checked_and_has_hint_with_default_and_receives_text( + client_request, + service_one, + fake_uuid, + mock_get_service_template, + multiple_sms_senders ): page = client_request.get( '.set_sender', @@ -81,18 +126,52 @@ def test_default_sender_is_checked_and_has_hint( ) assert page.select('.multiple-choice input')[0].has_attr('checked') - assert normalize_spaces(page.select_one('.multiple-choice label .block-label-hint').text) == "Default" + assert normalize_spaces( + page.select_one('.multiple-choice label .block-label-hint').text) == "(Default and receives replies)" assert not page.select('.multiple-choice input')[1].has_attr('checked') assert not page.select('.multiple-choice input')[2].has_attr('checked') +def test_sms_sender_has_receives_replies_hint( + client_request, + service_one, + fake_uuid, + mock_get_service_template, + multiple_sms_senders +): + page = client_request.get( + '.set_sender', + service_id=service_one['id'], + template_id=fake_uuid + ) + + assert page.select('.multiple-choice input')[0].has_attr('checked') + assert normalize_spaces( + page.select_one('.multiple-choice label .block-label-hint').text) == "(Default and receives replies)" + assert not page.select('.multiple-choice input')[1].has_attr('checked') + assert not page.select('.multiple-choice input')[2].has_attr('checked') + + +@pytest.mark.parametrize('template_mock, sender_data', [ + ( + mock_get_service_email_template, + multiple_reply_to_email_addresses, + ), + ( + mock_get_service_template, + multiple_sms_senders + ) +]) def test_sender_session_is_present_after_selected( logged_in_client, service_one, fake_uuid, - mock_get_service_email_template, - multiple_reply_to_email_addresses + template_mock, + sender_data, + mocker ): + template_mock(mocker) + sender_data(mocker) logged_in_client.post( url_for('.set_sender', service_id=service_one['id'], template_id=fake_uuid), data={'sender': '1234'} @@ -107,6 +186,10 @@ def test_sender_session_is_present_after_selected( mock_get_service_email_template, no_reply_to_email_addresses, ), + ( + mock_get_service_template, + no_sms_senders + ) ]) def test_set_sender_redirects_if_no_sender_data( logged_in_client, diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index cdb33256b..fa7ca8424 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -45,6 +45,7 @@ from freezegun import freeze_time 'Label Value Action', 'Send text messages On Change', 'Text message sender GOVUK Manage', + 'Text messages start with service name On Change', 'International text messages Off Change', 'Receive text messages Off Change', @@ -64,6 +65,7 @@ from freezegun import freeze_time 'Label Value Action', 'Send text messages On Change', 'Text message sender GOVUK Manage', + 'Text messages start with service name On Change', 'International text messages Off Change', 'Receive text messages Off Change', @@ -121,9 +123,10 @@ def test_should_show_overview( 'Label Value Action', 'Send text messages On Change', 'Text message sender GOVUK Manage', + 'Text messages start with service name On Change', 'International text messages On Change', 'Receive text messages On Change', - 'API endpoint for received text messages Not set Change', + 'Callback URL for received text messages Not set Change', 'Label Value Action', 'Send letters Off Change', @@ -140,6 +143,7 @@ def test_should_show_overview( 'Label Value Action', 'Send text messages On Change', 'Text message sender GOVUK Manage', + 'Text messages start with service name On Change', 'International text messages Off Change', 'Receive text messages Off Change', @@ -209,7 +213,7 @@ def test_service_settings_show_elided_api_url_if_needed( non_empty_trs = [tr.find_all('td') for tr in page.find_all('tr') if tr.find_all('td')] api_url = [api_setting[1].text.strip() for api_setting in non_empty_trs - if api_setting[0].text.strip() == 'API endpoint for received text messages'][0] + if api_setting[0].text.strip() == 'Callback URL for received text messages'][0] assert api_url == elided_url assert mocked_get_fn.called is True @@ -715,7 +719,7 @@ def test_and_more_hint_appears_on_settings_with_more_than_just_a_single_sender( 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" + assert get_row(page, 9) == "Sender addresses 1 Example Street …and 2 more Manage" @pytest.mark.parametrize('sender_list_page, expected_output', [ @@ -763,7 +767,7 @@ def test_api_ids_dont_show_on_option_pages_with_a_single_sender( ), ( 'main.service_sms_senders', multiple_sms_senders, - 'Example (default and recieves replies) Change 1234', + 'Example (default and receives replies) Change 1234', 'Example 2 Change 5678', 'Example 3 Change 9457' ), @@ -1274,7 +1278,7 @@ def test_does_not_show_research_mode_indicator( @pytest.mark.parametrize('url, bearer_token, expected_errors', [ ("", "", "Can’t be empty Can’t be empty"), - ("http://not_https.com", "1234567890", "Must be a valid https url"), + ("http://not_https.com", "1234567890", "Must be a valid https URL"), ("https://test.com", "123456789", "Must be at least 10 characters"), ]) def test_set_inbound_api_validation( @@ -2075,3 +2079,46 @@ def test_empty_letter_contact_block_returns_error( page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') error_message = page.find('span', class_='error-message').text.strip() assert error_message == 'Can’t be empty' + + +def test_show_sms_prefixing_setting_page( + client_request, + mock_update_service, +): + page = client_request.get( + 'main.service_set_sms_prefix', service_id=SERVICE_ONE_ID + ) + assert normalize_spaces(page.select_one('legend').text) == ( + 'Start all text messages with ‘service one:’' + ) + radios = page.select('input[type=radio]') + assert len(radios) == 2 + assert radios[0]['value'] == 'on' + assert radios[0]['checked'] == '' + assert radios[1]['value'] == 'off' + with pytest.raises(KeyError): + assert radios[1]['checked'] + + +@pytest.mark.parametrize('post_value, expected_api_argument', [ + ('on', True), + ('off', False), +]) +def test_updates_sms_prefixing( + client_request, + mock_update_service, + post_value, + expected_api_argument, +): + client_request.post( + 'main.service_set_sms_prefix', service_id=SERVICE_ONE_ID, + _data={'enabled': post_value}, + _expected_redirect=url_for( + 'main.service_settings', service_id=SERVICE_ONE_ID, + _external=True + ) + ) + mock_update_service.assert_called_once_with( + service_id=SERVICE_ONE_ID, + prefix_sms=expected_api_argument, + ) diff --git a/tests/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py index 8a11e6283..cc3974357 100644 --- a/tests/app/main/views/test_templates.py +++ b/tests/app/main/views/test_templates.py @@ -335,13 +335,14 @@ def test_should_not_allow_creation_of_a_template_without_correct_permission( @pytest.mark.parametrize('fixture, expected_status_code', [ (mock_get_service_email_template, 200), - (mock_get_service_template, 302), + (mock_get_service_template, 200), (mock_get_service_letter_template, 302), ]) -def test_should_redirect_to_one_off_if_template_type_is_not_email( +def test_should_redirect_to_one_off_if_template_type_is_letter( logged_in_client, active_user_with_permissions, multiple_reply_to_email_addresses, + multiple_sms_senders, service_one, fake_uuid, mocker, 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 101bfef4f..a2966b7fa 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -304,6 +304,66 @@ def multiple_sms_senders(mocker): return mocker.patch('app.service_api_client.get_sms_senders', side_effect=_get) +@pytest.fixture(scope='function') +def multiple_sms_senders_with_diff_default(mocker): + def _get(service_id): + return [ + { + 'id': '1234', + 'service_id': service_id, + 'sms_sender': 'Example', + 'is_default': True, + 'created_at': datetime.utcnow(), + 'inbound_number_id': None, + 'updated_at': None + }, { + 'id': '5678', + 'service_id': service_id, + 'sms_sender': 'Example 2', + 'is_default': False, + 'created_at': datetime.utcnow(), + 'inbound_number_id': None, + 'updated_at': None + }, { + 'id': '9457', + 'service_id': service_id, + 'sms_sender': 'Example 3', + 'is_default': False, + 'created_at': datetime.utcnow(), + 'inbound_number_id': '12354', + 'updated_at': None + } + ] + + return mocker.patch('app.service_api_client.get_sms_senders', side_effect=_get) + + +@pytest.fixture(scope='function') +def multiple_sms_senders_no_inbound(mocker): + def _get(service_id): + return [ + { + 'id': '1234', + 'service_id': service_id, + 'sms_sender': 'Example', + 'is_default': True, + 'created_at': datetime.utcnow(), + 'inbound_number_id': None, + 'updated_at': None + }, { + 'id': '5678', + 'service_id': service_id, + 'sms_sender': 'Example 2', + 'is_default': False, + 'created_at': datetime.utcnow(), + 'inbound_number_id': None, + 'updated_at': None + } + ] + + return mocker.patch('app.service_api_client.get_sms_senders', side_effect=_get) + + @pytest.fixture(scope='function') def no_sms_senders(mocker): def _get(service_id): @@ -1003,7 +1063,8 @@ def platform_admin_user(fake_uuid): 'manage_settings', 'manage_api_keys', 'view_activity']}, - 'platform_admin': True + 'platform_admin': True, + 'auth_type': 'sms_auth' } user = User(user_data) return user @@ -1021,6 +1082,7 @@ def api_user_active(fake_uuid, email_address='test@user.gov.uk'): 'failed_login_count': 0, 'permissions': {}, 'platform_admin': False, + 'auth_type': 'sms_auth', 'password_changed_at': str(datetime.utcnow()) } user = User(user_data) @@ -1039,6 +1101,7 @@ def api_nongov_user_active(fake_uuid): 'failed_login_count': 0, 'permissions': {}, 'platform_admin': False, + 'auth_type': 'sms_auth', 'password_changed_at': str(datetime.utcnow()) } user = User(user_data) @@ -1065,7 +1128,35 @@ def active_user_with_permissions(fake_uuid): 'manage_settings', 'manage_api_keys', 'view_activity']}, - 'platform_admin': False + 'platform_admin': False, + 'auth_type': 'sms_auth' + } + user = User(user_data) + return user + + +@pytest.fixture(scope='function') +def active_user_no_mobile(fake_uuid): + from app.notify_client.user_api_client import User + + user_data = {'id': fake_uuid, + 'name': 'Test User', + 'password': 'somepassword', + 'password_changed_at': str(datetime.utcnow()), + 'email_address': 'test@user.gov.uk', + 'mobile_number': None, + 'state': 'active', + 'failed_login_count': 0, + 'permissions': {SERVICE_ONE_ID: ['send_texts', + 'send_emails', + 'send_letters', + 'manage_users', + 'manage_templates', + 'manage_settings', + 'manage_api_keys', + 'view_activity']}, + 'platform_admin': False, + 'auth_type': 'email_auth' } user = User(user_data) return user @@ -1084,7 +1175,8 @@ def active_user_view_permissions(fake_uuid): 'state': 'active', 'failed_login_count': 0, 'permissions': {SERVICE_ONE_ID: ['view_activity']}, - 'platform_admin': False + 'platform_admin': False, + 'auth_type': 'sms_auth' } user = User(user_data) return user @@ -1107,7 +1199,8 @@ def active_user_manage_template_permission(fake_uuid): 'manage_templates', 'view_activity', ]}, - 'platform_admin': False + 'platform_admin': False, + 'auth_type': 'sms_auth' } user = User(user_data) return user @@ -1123,7 +1216,8 @@ def api_user_locked(fake_uuid): 'mobile_number': '07700 900762', 'state': 'active', 'failed_login_count': 5, - 'permissions': {} + 'permissions': {}, + 'auth_type': 'sms_auth' } user = User(user_data) return user @@ -1140,7 +1234,8 @@ def api_user_request_password_reset(fake_uuid): 'state': 'active', 'failed_login_count': 5, 'permissions': {}, - 'password_changed_at': None + 'password_changed_at': None, + 'auth_type': 'sms_auth' } user = User(user_data) return user @@ -1157,6 +1252,7 @@ def api_user_changed_password(fake_uuid): 'state': 'active', 'failed_login_count': 5, 'permissions': {}, + 'auth_type': 'sms_auth', 'password_changed_at': str(datetime.utcnow() + timedelta(minutes=1)) } user = User(user_data) @@ -1753,6 +1849,7 @@ def mock_get_users_by_service(mocker): 'password_changed_at': None, 'name': 'Test User', 'email_address': 'notify@digital.cabinet-office.gov.uk', + 'auth_type': 'sms_auth', 'failed_login_count': 0}] return [User(data[0])]