diff --git a/README.md b/README.md index 6e0f3b741..23dd3de7a 100644 --- a/README.md +++ b/README.md @@ -4,52 +4,49 @@ # notifications-admin -Application to handle the admin functions of the notifications application. +GOV.UK Notify admin application. ## Features of this application - - Register users - - Register services - - Download CSV for an email or SMS batch + - Register and manage users + - Create and manage services + - Send batch emails and SMS by uploading a CSV - Show history of notifications - - Reports ## First-time setup -You need [Node](http://nodejs.org/) which will also get you [NPM](npmjs.org), -Node's package management tool. +Languages needed +- Python 3 +- [Node](http://nodejs.org/) 5.0.0 or greater ```shell brew install node ``` -n is a tool for managing different versions of node. The following installs n -and uses the latest version of node. +[NPM](npmjs.org) is Node's package management tool. `n` is a tool for managing +different versions of Node. The following installs `n` and uses the latest +version of Node. ```shell npm install -g n n latest npm rebuild node-sass ``` -The frontend dependencies are managed using NPM and Bower. To install or update -*all the things*, run -```shell - npm install - npm run build -``` - The app runs within a virtual environment. To [install virtualenv](https://virtualenv.readthedocs.org/en/latest/installation.html), run ```shell [sudo] pip install virtualenv ``` -To make a virtualenv for this app, run +Make a virtual environment for this app: ```shell mkvirtualenv -p /usr/local/bin/python3 notifications-admin - pip install -r requirements.txt +``` + +Install dependencies and build the frontend assets: +```shell ./scripts/bootstrap.sh ``` -## Building the frontend +## Rebuilding the frontend assets If you want the front end assets to re-compile on changes, leave this running in a separate terminal from the app @@ -64,10 +61,4 @@ in a separate terminal from the app ./scripts/run_app.sh ``` -Then visit [localhost:6012](localhost:6012) - -## Domain model - -All the domain models are defined in the -[models.py](https://github.com/alphagov/notifications-admin/blob/master/app/models.py) -file. +Then visit [localhost:6012](http://localhost:6012) diff --git a/app/__init__.py b/app/__init__.py index b9e070318..3f8e61989 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -14,8 +14,8 @@ from app.notify_client.api_client import NotificationsAdminAPIClient from app.notify_client.api_key_api_client import ApiKeyApiClient from app.notify_client.user_api_client import UserApiClient from app.notify_client.job_api_client import JobApiClient +from app.notify_client.notification_api_client import NotificationApiClient from app.notify_client.status_api_client import StatusApiClient -from app.notify_client.permission_api_client import PermissionApiClient from app.notify_client.invite_api_client import InviteApiClient from app.its_dangerous_session import ItsdangerousSessionInterface from app.asset_fingerprinter import AssetFingerprinter @@ -31,10 +31,10 @@ notifications_api_client = NotificationsAdminAPIClient() user_api_client = UserApiClient() api_key_api_client = ApiKeyApiClient() job_api_client = JobApiClient() +notification_api_client = NotificationApiClient() status_api_client = StatusApiClient() invite_api_client = InviteApiClient() asset_fingerprinter = AssetFingerprinter() -permission_api_client = PermissionApiClient() def create_app(config_name, config_overrides=None): @@ -50,8 +50,8 @@ def create_app(config_name, config_overrides=None): user_api_client.init_app(application) api_key_api_client.init_app(application) job_api_client.init_app(application) + notification_api_client.init_app(application) status_api_client.init_app(application) - permission_api_client.init_app(application) invite_api_client.init_app(application) login_manager.init_app(application) @@ -70,6 +70,7 @@ def create_app(config_name, config_overrides=None): application.add_template_filter(nl2br) application.add_template_filter(format_datetime) + application.add_template_filter(format_time) application.add_template_filter(syntax_highlight_json) application.add_template_filter(valid_phone_number) @@ -146,6 +147,12 @@ def format_datetime(date): return native.strftime('%A %d %B %Y at %H:%M') +def format_time(date): + date = dateutil.parser.parse(date) + native = date.replace(tzinfo=None) + return native.strftime('%H:%M') + + def valid_phone_number(phone_number): try: validate_phone_number(phone_number) diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py index 9469fa810..bb0013a63 100644 --- a/app/main/views/jobs.py +++ b/app/main/views/jobs.py @@ -10,7 +10,7 @@ from flask_login import login_required from notifications_python_client.errors import HTTPError from utils.template import Template -from app import job_api_client +from app import job_api_client, notification_api_client from app.main import main from app.main.dao import templates_dao from app.main.dao import services_dao @@ -41,28 +41,27 @@ def view_job(service_id, job_id): service = services_dao.get_service_by_id_or_404(service_id) try: job = job_api_client.get_job(service_id, job_id)['data'] - messages = [] + notifications = notification_api_client.get_notifications_for_service(service_id, job_id) + finished = job['status'] == 'finished' return render_template( 'views/job.html', - messages=messages, + notifications=notifications['notifications'], counts={ - 'total': len(messages), - 'delivered': len([ - message for message in messages if message['status'] == 'Delivered' - ]), - 'failed': len([ - message for message in messages if message['status'] == 'Failed' - ]) + 'queued': 0 if finished else job['notification_count'], + 'sent': job['notification_count'] if finished else 0, + 'failed': 0 }, + uploaded_at=job['created_at'], + finished_at=job['updated_at'] if finished else None, cost=u'£0.00', uploaded_file_name=job['original_file_name'], - uploaded_file_time=job['created_at'], template=Template( templates_dao.get_service_template_or_404(service_id, job['template'])['data'], prefix=service['name'] ), service_id=service_id, - service=service + from_name=service['name'], + job_id=job_id ) except HTTPError as e: if e.status_code == 404: diff --git a/app/main/views/send.py b/app/main/views/send.py index 3b3e8d19e..7ff1629fb 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -26,15 +26,31 @@ from app.main.uploader import ( from app.main.dao import templates_dao from app.main.dao import services_dao from app import job_api_client -from app.utils import validate_recipient, InvalidPhoneError, InvalidEmailError +from app.utils import ( + validate_recipient, InvalidPhoneError, InvalidEmailError, user_has_permissions) from utils.process_csv import first_column_heading -page_headings = { + +send_messages_page_headings = { 'email': 'Send emails', 'sms': 'Send text messages' } +manage_templates_page_headings = { + 'email': 'Manage templates', + 'sms': 'Manage templates' +} + + +def get_page_headings(template_type): + # User has manage_service role + if current_user.has_permissions(['send_texts', 'send_emails', 'send_letters']): + return send_messages_page_headings[template_type] + else: + return manage_templates_page_headings[template_type] + + @main.route("/services//send/letters", methods=['GET']) def letters_stub(service_id): return render_template( @@ -43,6 +59,8 @@ def letters_stub(service_id): @main.route("/services//send/", methods=['GET']) +@login_required +@user_has_permissions('send_texts', 'send_emails', 'send_letters', 'manage_templates', or_=True) def choose_template(service_id, template_type): service = services_dao.get_service_by_id_or_404(service_id) @@ -66,7 +84,7 @@ def choose_template(service_id, template_type): if template['template_type'] == template_type ], template_type=template_type, - page_heading=page_headings[template_type], + page_heading=get_page_headings(template_type), service=service, has_jobs=len(jobs), service_id=service_id @@ -75,6 +93,7 @@ def choose_template(service_id, template_type): @main.route("/services//send/", methods=['GET', 'POST']) @login_required +@user_has_permissions('send_texts', 'send_emails', 'send_letters') def send_messages(service_id, template_id): form = CsvUploadForm() @@ -112,6 +131,7 @@ def send_messages(service_id, template_id): @main.route("/services//send/.csv", methods=['GET']) @login_required +@user_has_permissions('send_texts', 'send_emails', 'send_letters', 'manage_templates', or_=True) def get_example_csv(service_id, template_id): template = Template(templates_dao.get_service_template_or_404(service_id, template_id)['data']) output = io.StringIO() @@ -131,6 +151,7 @@ def get_example_csv(service_id, template_id): @main.route("/services//send//to-self", methods=['GET']) @login_required +@user_has_permissions('send_texts', 'send_emails', 'send_letters') def send_message_to_self(service_id, template_id): template = Template(templates_dao.get_service_template_or_404(service_id, template_id)['data']) output = io.StringIO() @@ -166,6 +187,7 @@ def send_message_to_self(service_id, template_id): @main.route("/services//check/", methods=['GET', 'POST']) @login_required +@user_has_permissions('send_texts', 'send_emails', 'send_letters') def check_messages(service_id, upload_id): upload_data = session['upload_data'] @@ -189,7 +211,7 @@ def check_messages(service_id, upload_id): 'views/check.html', upload_result=upload_result, template=template, - page_heading=page_headings[template.template_type], + page_heading=get_page_headings(template.template_type), column_headers=[first_column_heading[template.template_type]] + list(template.placeholders_as_markup), original_file_name=upload_data.get('original_file_name'), service_id=service_id, diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 12b68f307..b813b4f5a 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -3,6 +3,7 @@ from flask import ( from flask_login import (login_required, current_user) from app.main import main +from app.utils import user_has_permissions from app.main.dao.services_dao import ( get_service_by_id, delete_service, update_service) from app.main.dao.users_dao import verify_password @@ -12,6 +13,7 @@ from notifications_python_client.errors import HTTPError @main.route("/services//service-settings") @login_required +@user_has_permissions('manage_settings') def service_settings(service_id): try: service = get_service_by_id(service_id)['data'] @@ -29,6 +31,7 @@ def service_settings(service_id): @main.route("/services//service-settings/name", methods=['GET', 'POST']) @login_required +@user_has_permissions('manage_settings') def service_name_change(service_id): try: service = get_service_by_id(service_id)['data'] @@ -53,6 +56,7 @@ def service_name_change(service_id): @main.route("/services//service-settings/name/confirm", methods=['GET', 'POST']) @login_required +@user_has_permissions('manage_settings') def service_name_change_confirm(service_id): try: service = get_service_by_id(service_id)['data'] @@ -82,6 +86,7 @@ def service_name_change_confirm(service_id): @main.route("/services//service-settings/request-to-go-live", methods=['GET', 'POST']) @login_required +@user_has_permissions('manage_settings') def service_request_to_go_live(service_id): try: service = get_service_by_id(service_id)['data'] @@ -104,6 +109,7 @@ def service_request_to_go_live(service_id): @main.route("/services//service-settings/status", methods=['GET', 'POST']) @login_required +@user_has_permissions('manage_settings') def service_status_change(service_id): try: service = get_service_by_id(service_id)['data'] @@ -125,6 +131,7 @@ def service_status_change(service_id): @main.route("/services//service-settings/status/confirm", methods=['GET', 'POST']) @login_required +@user_has_permissions('manage_settings') def service_status_change_confirm(service_id): try: service = get_service_by_id(service_id)['data'] @@ -153,6 +160,7 @@ def service_status_change_confirm(service_id): @main.route("/services//service-settings/delete", methods=['GET', 'POST']) @login_required +@user_has_permissions('manage_settings') def service_delete(service_id): try: service = get_service_by_id(service_id)['data'] @@ -174,6 +182,7 @@ def service_delete(service_id): @main.route("/services//service-settings/delete/confirm", methods=['GET', 'POST']) @login_required +@user_has_permissions('manage_settings') def service_delete_confirm(service_id): try: service = get_service_by_id(service_id)['data'] diff --git a/app/main/views/templates.py b/app/main/views/templates.py index 2c29ba46a..0b7119e9d 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -2,6 +2,7 @@ from flask import request, render_template, redirect, url_for, flash, abort from flask_login import login_required from app.main import main +from app.utils import user_has_permissions from app.main.forms import SMSTemplateForm, EmailTemplateForm from app.main.dao import templates_dao as tdao from app.main.dao import services_dao as sdao @@ -15,6 +16,7 @@ form_objects = { @main.route("/services//templates/add-", methods=['GET', 'POST']) @login_required +@user_has_permissions('manage_templates') def add_service_template(service_id, template_type): service = sdao.get_service_by_id_or_404(service_id) @@ -46,6 +48,7 @@ def add_service_template(service_id, template_type): @main.route("/services//templates/", methods=['GET', 'POST']) @login_required +@user_has_permissions('manage_templates') def edit_service_template(service_id, template_id): template = tdao.get_service_template_or_404(service_id, template_id)['data'] template['template_content'] = template['content'] @@ -73,6 +76,7 @@ def edit_service_template(service_id, template_id): @main.route("/services//templates//delete", methods=['GET', 'POST']) @login_required +@user_has_permissions('manage_templates') def delete_service_template(service_id, template_id): template = tdao.get_service_template_or_404(service_id, template_id)['data'] diff --git a/app/notify_client/models.py b/app/notify_client/models.py index 8c51666eb..df8d41306 100644 --- a/app/notify_client/models.py +++ b/app/notify_client/models.py @@ -1,4 +1,5 @@ from flask.ext.login import (UserMixin, login_fresh) +from flask import session class User(UserMixin): @@ -81,8 +82,14 @@ class User(UserMixin): def permissions(self, permissions): raise AttributeError("Read only property") - def has_permissions(self, service_id, permissions): - return True + def has_permissions(self, permissions, service_id=None, or_=False): + if service_id is None: + service_id = session.get('service_id', '') + if service_id in self._permissions: + if or_: + return any([x in self._permissions[service_id] for x in permissions]) + return set(self._permissions[service_id]) > set(permissions) + return False @property def failed_login_count(self): diff --git a/app/notify_client/notification_api_client.py b/app/notify_client/notification_api_client.py new file mode 100644 index 000000000..fe253669a --- /dev/null +++ b/app/notify_client/notification_api_client.py @@ -0,0 +1,37 @@ +from notifications_python_client.base import BaseAPIClient + + +class NotificationApiClient(BaseAPIClient): + def __init__(self, base_url=None, client_id=None, secret=None): + super(self.__class__, self).__init__(base_url=base_url or 'base_url', + client_id=client_id or 'client_id', + secret=secret or 'secret') + + def init_app(self, app): + self.base_url = app.config['API_HOST_NAME'] + self.client_id = app.config['ADMIN_CLIENT_USER_NAME'] + self.secret = app.config['ADMIN_CLIENT_SECRET'] + + def get_all_notifications(self, page=None): + params = {} + if page is not None: + params['page'] = page + return self.get( + url='/notifications', + params=params + ) + + def get_notifications_for_service(self, service_id, job_id=None, page=None): + params = {} + if page is not None: + params['page'] = page + if job_id: + return self.get( + url='/service/{}/job/{}/notifications'.format(service_id, job_id), + params=params + ) + else: + return self.get( + url='/service/{}/notifications'.format(service_id), + params=params + ) diff --git a/app/notify_client/permission_api_client.py b/app/notify_client/permission_api_client.py deleted file mode 100644 index a112251af..000000000 --- a/app/notify_client/permission_api_client.py +++ /dev/null @@ -1,25 +0,0 @@ -import uuid - -from notifications_python_client.base import BaseAPIClient - - -class PermissionApiClient(BaseAPIClient): - def __init__(self, base_url=None, client_id=None, secret=None): - super(self.__class__, self).__init__(base_url=base_url or 'base_url', - client_id=client_id or 'client_id', - secret=secret or 'secret') - - def init_app(self, app): - self.base_url = app.config['API_HOST_NAME'] - self.client_id = app.config['ADMIN_CLIENT_USER_NAME'] - self.secret = app.config['ADMIN_CLIENT_SECRET'] - - def delete_permission(self, permission_id): - return self.delete(url='/permission/{}'.format(permission_id))['data'] - - def create_permission(self, permission, user_id, service_id): - return self.post( - url='/permission', - data={'permission': permission, - 'user': user_id, - 'service': service_id})['data'] diff --git a/app/notify_client/user_api_client.py b/app/notify_client/user_api_client.py index 6b3450503..d825f079f 100644 --- a/app/notify_client/user_api_client.py +++ b/app/notify_client/user_api_client.py @@ -98,3 +98,8 @@ class UserApiClient(BaseAPIClient): endpoint = '/service/{}/users/{}'.format(service_id, user_id) resp = self.post(endpoint, data={}) return User(resp['data'], max_failed_login_count=self.max_failed_login_count) + + def set_user_permissions(self, user_id, service_id, permissions): + data = [{'permission': x} for x in permissions] + endpoint = '/user/{}/service/{}/permission'.format(user_id, service_id) + resp = self.post(endpoint, data=data) diff --git a/app/templates/main_nav.html b/app/templates/main_nav.html index 216bec23c..3f2727e40 100644 --- a/app/templates/main_nav.html +++ b/app/templates/main_nav.html @@ -2,20 +2,26 @@

{{ session.get('service_name', 'Service') }}

- {% if current_user.has_permissions(session.get('service_id', ''), ['send_messages']) %} + {% if current_user.has_permissions(['send_texts', 'send_emails', 'send_letters']) %} + {% elif current_user.has_permissions(['manage_templates']) %} + {% endif %} - {% if current_user.has_permissions(session.get('service_id', ''), ['manage_service']) %} + {% if current_user.has_permissions(['manage_users', 'manage_settings']) %} {% endif %} - {% if current_user.has_permissions(session.get('service_id', ''), ['manage_api_keys']) %} + {% if current_user.has_permissions(['manage_api_keys', 'access_developer_docs']) %}
  • API keys
  • Developer documentation
    • diff --git a/app/templates/views/choose-template.html b/app/templates/views/choose-template.html index 7fb87b827..00b4cbc2f 100644 --- a/app/templates/views/choose-template.html +++ b/app/templates/views/choose-template.html @@ -16,6 +16,7 @@ {% if templates %} {% if not has_jobs %} + {% if current_user.has_permissions(['send_texts', 'send_emails', 'send_letters'], or_=True) %} {{ banner( """ Send yourself a test message @@ -23,6 +24,7 @@ subhead='Next step', type="tip" )}} + {% endif %} {% endif %}
    {% for template in templates %} @@ -39,9 +41,13 @@
    + {% if current_user.has_permissions(['send_texts', 'send_emails', 'send_letters']) %} Add recipients Send yourself a test + {% endif %} + {% if current_user.has_permissions(['manage_templates']) %} Edit template + {% endif %}
    {% endfor %} @@ -49,7 +55,9 @@ {% endif %}

    + {% if current_user.has_permissions(['manage_templates']) %} Add a new template + {% endif %}

    diff --git a/app/templates/views/job.html b/app/templates/views/job.html index 416da439e..63346c7ea 100644 --- a/app/templates/views/job.html +++ b/app/templates/views/job.html @@ -19,7 +19,7 @@
    {{ sms_message( - template, + template.formatted_as_markup, )}}
    @@ -28,23 +28,29 @@ template.subject, template, from_address='{}@notifications.service.gov.uk'.format(service.email_from), - from_name=service.name + from_name=from_name )}} {% endif %} -

    - Started {{ uploaded_file_time|format_datetime }} -

    + {% if finished_at %} +

    + Finished {{ finished_at|format_datetime }} +

    + {% else %} +

    + Started {{ uploaded_at|format_datetime }} +

    + {% endif %}
    • {{ big_number( - 0, 'queued' + counts.queued, 'queued' )}}
    • {{ big_number( - 1, 'sent' + counts.sent, 'sent' )}}
    • @@ -60,35 +66,32 @@
    - {% call(item) list_table( - [ - {'row': 1, 'phone': '+447700 900995', 'template': template['name'], 'status': 'sent'} - ], - caption=uploaded_file_name, - caption_visible=False, - empty_message="Messages go here", - field_headings=[ - 'Row', - 'Recipient', - 'Template', - right_aligned_field_heading('Status') - ] - ) %} - {% call field() %} - {{ item.row }}. - {% endcall %} - {% call field() %} - {{item.phone[:3]}} •••• •••••• - {% endcall %} - {% call field() %} - {{item.template}} - {% endcall %} - {% call field( - align='right', - status='error' if item.status == 'Failed' else 'default' + + {% if notifications %} + {% call(item) list_table( + notifications, + caption=uploaded_file_name, + caption_visible=False, + empty_message="Messages go here", + field_headings=[ + 'Recipient', + right_aligned_field_heading('Status') + ] ) %} - {{ item.status }} + {% call field() %} + {{ item.to }} + {% endcall %} + {% call field( + align='right', + status='error' if item.status == 'Failed' else 'default' + ) %} + {{ item.status|title }} at {{ item.sent_at|format_time }} + {% endcall %} {% endcall %} - {% endcall %} + {% else %} +

    + Refresh +

    + {% endif %} {% endblock %} diff --git a/app/utils.py b/app/utils.py index 0fd38f808..11eb18876 100644 --- a/app/utils.py +++ b/app/utils.py @@ -96,14 +96,12 @@ def validate_recipient(row, template_type): }[template_type](get_recipient_from_row(row, template_type)) -def user_has_permissions(*permissions): +def user_has_permissions(*permissions, or_=False): def wrap(func): @wraps(func) def wrap_func(*args, **kwargs): - # We are making the assumption that the user is logged in. from flask_login import current_user - service_id = session.get('service_id', '') - if current_user and current_user.has_permissions(service_id, permissions): + if current_user and current_user.has_permissions(permissions, or_=or_): return func(*args, **kwargs) else: abort(403) diff --git a/scripts/bootstrap.sh b/scripts/bootstrap.sh index cd3ffb715..82274e091 100755 --- a/scripts/bootstrap.sh +++ b/scripts/bootstrap.sh @@ -27,3 +27,5 @@ fi # Install Python development dependencies pip3 install -r requirements_for_test.txt + +npm install && npm run build diff --git a/tests/__init__.py b/tests/__init__.py index 57e3984fb..f5028b65b 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -109,6 +109,18 @@ def job_json(): 'bucket_name': 'service-1-{}-notify'.format(job_id), 'file_name': '{}.csv'.format(job_id), 'created_at': created_at, - 'notification_count': 1 + 'notification_count': 1, + 'status': '' } return data + + +def notification_json(): + import datetime + data = { + 'notifications': [{ + 'sent_at': str(datetime.datetime.now().time()) + } for i in range(5)], + 'links': {} + } + return data diff --git a/tests/app/main/notify_client/test_notification_client.py b/tests/app/main/notify_client/test_notification_client.py new file mode 100644 index 000000000..08db26780 --- /dev/null +++ b/tests/app/main/notify_client/test_notification_client.py @@ -0,0 +1,41 @@ +import pytest +from app.notify_client.notification_api_client import NotificationApiClient + + +def test_client_gets_notifications(mocker): + + mock_get = mocker.patch('app.notify_client.notification_api_client.NotificationApiClient.get') + NotificationApiClient().get_all_notifications() + mock_get.assert_called_once_with(url='/notifications', params={}) + + +def test_client_gets_notifications_with_page(mocker): + + mock_get = mocker.patch('app.notify_client.notification_api_client.NotificationApiClient.get') + NotificationApiClient().get_all_notifications(page=99) + mock_get.assert_called_once_with(url='/notifications', params={'page': 99}) + + +@pytest.mark.parametrize("arguments,expected_call", [ + ( + {}, + {'url': '/service/abcd1234/notifications', 'params': {}} + ), + ( + {'page': 99}, + {'url': '/service/abcd1234/notifications', 'params': {'page': 99}} + ), + ( + {'job_id': 'efgh5678'}, + {'url': '/service/abcd1234/job/efgh5678/notifications', 'params': {}} + ), + ( + {'job_id': 'efgh5678', 'page': 48}, + {'url': '/service/abcd1234/job/efgh5678/notifications', 'params': {'page': 48}} + ) +]) +def test_client_gets_notifications_for_service_and_job_by_page(mocker, arguments, expected_call): + + mock_get = mocker.patch('app.notify_client.notification_api_client.NotificationApiClient.get') + NotificationApiClient().get_notifications_for_service('abcd1234', **arguments) + mock_get.assert_called_once_with(**expected_call) diff --git a/tests/app/main/test_utils.py b/tests/app/main/test_utils.py index 67262c360..95b435f6e 100644 --- a/tests/app/main/test_utils.py +++ b/tests/app/main/test_utils.py @@ -6,18 +6,53 @@ from app.main.views.index import index from werkzeug.exceptions import Forbidden -# def test_user_has_permissions(app_, -# api_user_active, -# mock_get_user, -# mock_get_user_by_email, -# mock_login): -# with app_.test_request_context(): -# with app_.test_client() as client: -# client.login(api_user_active) -# decorator = user_has_permissions('something') -# decorated_index = decorator(index) -# try: -# response = decorated_index() -# pytest.fail("Failed to throw a forbidden exception") -# except Forbidden: -# pass +def test_user_has_permissions_on_endpoint_fail(app_, + api_user_active, + mock_login, + mock_get_user_with_permissions): + with app_.test_request_context(): + with app_.test_client() as client: + client.login(api_user_active) + decorator = user_has_permissions('something') + decorated_index = decorator(index) + try: + response = decorated_index() + pytest.fail("Failed to throw a forbidden exception") + except Forbidden: + pass + + +def test_user_has_permissions_success(app_, + api_user_active, + mock_login, + mock_get_user_with_permissions): + with app_.test_request_context(): + with app_.test_client() as client: + client.login(api_user_active) + decorator = user_has_permissions('manage_users') + decorated_index = decorator(index) + response = decorated_index() + + +def test_user_has_permissions_or(app_, + api_user_active, + mock_login, + mock_get_user_with_permissions): + with app_.test_request_context(): + with app_.test_client() as client: + client.login(api_user_active) + decorator = user_has_permissions('something', 'manage_users', or_=True) + decorated_index = decorator(index) + response = decorated_index() + + +def test_user_has_permissions_multiple(app_, + api_user_active, + mock_login, + mock_get_user_with_permissions): + with app_.test_request_context(): + with app_.test_client() as client: + client.login(api_user_active) + decorator = user_has_permissions('manage_templates', 'manage_users') + decorated_index = decorator(index) + response = decorated_index() diff --git a/tests/app/main/views/test_jobs.py b/tests/app/main/views/test_jobs.py index c3147555f..8a8ff0aff 100644 --- a/tests/app/main/views/test_jobs.py +++ b/tests/app/main/views/test_jobs.py @@ -30,7 +30,8 @@ def test_should_show_page_for_one_job(app_, mock_get_service, mock_get_service_template, job_data, - mock_get_job): + mock_get_job, + mock_get_notifications): service_id = job_data['service'] job_id = job_data['id'] file_name = job_data['original_file_name'] @@ -41,4 +42,6 @@ def test_should_show_page_for_one_job(app_, response = client.get(url_for('main.view_job', service_id=service_id, job_id=job_id)) assert response.status_code == 200 - assert file_name in response.get_data(as_text=True) + content = response.get_data(as_text=True) + assert "Test Service: Your vehicle tax is about to expire" in content + assert file_name in content diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index c9f892c69..f3e194009 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -17,7 +17,8 @@ def test_choose_template( mock_get_service, mock_check_verify_code, mock_get_service_templates, - mock_get_jobs + mock_get_jobs, + mock_has_permissions ): with app_.test_request_context(): with app_.test_client() as client: @@ -40,7 +41,8 @@ def test_upload_empty_csvfile_returns_to_upload_page( mock_get_service, mock_get_service_templates, mock_check_verify_code, - mock_get_service_template + mock_get_service_template, + mock_has_permissions ): with app_.test_request_context(): with app_.test_client() as client: @@ -64,7 +66,8 @@ def test_upload_csvfile_with_invalid_phone_shows_check_page_with_errors( mock_login, mock_get_service, mock_get_service_template, - mock_s3_upload + mock_s3_upload, + mock_has_permissions ): contents = 'phone number,name\n+44 123,test1\n+44 456,test2' @@ -95,7 +98,8 @@ def test_upload_csvfile_removes_empty_lines_and_trailing_commas( mock_login, mock_get_service, mock_get_service_template, - mock_s3_upload + mock_s3_upload, + mock_has_permissions ): contents = 'phone number,name,,,\n++44 7700 900981,test1,,,\n+44 7700 900981,test2,,,\n ,,, \n ,,, \t \t \n' @@ -127,7 +131,8 @@ def test_send_test_message_to_self( mock_login, mock_get_service, mock_get_service_template, - mock_s3_upload + mock_s3_upload, + mock_has_permissions ): expected_data = {'data': ['phone number', '+4412341234'], 'file_name': 'Test run'} @@ -151,7 +156,8 @@ def test_send_test_message_to_self( mock_login, mock_get_service, mock_get_service_email_template, - mock_s3_upload + mock_s3_upload, + mock_has_permissions ): expected_data = {'data': ['email address', 'test@user.gov.uk'], 'file_name': 'Test run'} @@ -174,7 +180,8 @@ def test_download_example_csv( api_user_active, mock_login, mock_get_service, - mock_get_service_template + mock_get_service_template, + mock_has_permissions ): with app_.test_request_context(): @@ -196,7 +203,8 @@ def test_upload_csvfile_with_valid_phone_shows_all_numbers( mock_login, mock_get_service, mock_get_service_template, - mock_s3_upload + mock_s3_upload, + mock_has_permissions ): contents = 'phone number\n+44 7700 900981\n+44 7700 900982\n+44 7700 900983\n+44 7700 900984\n+44 7700 900985\n+44 7700 900986' # noqa @@ -234,8 +242,10 @@ def test_create_job_should_call_api( job_data, mock_create_job, mock_get_job, + mock_get_notifications, mock_get_service, - mock_get_service_template + mock_get_service_template, + mock_has_permissions ): service_id = service_one['id'] diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 045cc4c86..174935f2a 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -6,7 +6,8 @@ def test_should_show_overview(app_, mock_get_service, mock_get_user, mock_get_user_by_email, - mock_login): + mock_login, + mock_has_permissions): with app_.test_request_context(): with app_.test_client() as client: client.login(api_user_active) @@ -25,7 +26,8 @@ def test_should_show_service_name(app_, mock_get_service, mock_get_user, mock_get_user_by_email, - mock_login): + mock_login, + mock_has_permissions): with app_.test_request_context(): with app_.test_client() as client: client.login(api_user_active) @@ -44,7 +46,8 @@ def test_should_redirect_after_change_service_name(app_, mock_get_service, mock_get_user, mock_get_user_by_email, - mock_login): + mock_login, + mock_has_permissions): with app_.test_request_context(): with app_.test_client() as client: client.login(api_user_active) @@ -64,7 +67,8 @@ def test_should_show_service_name_confirmation(app_, mock_get_service, mock_get_user, mock_get_user_by_email, - mock_login): + mock_login, + mock_has_permissions): with app_.test_request_context(): with app_.test_client() as client: client.login(api_user_active) @@ -85,7 +89,8 @@ def test_should_redirect_after_service_name_confirmation(app_, mock_get_user, mock_get_user_by_email, mock_login, - mock_verify_password): + mock_verify_password, + mock_has_permissions): with app_.test_request_context(): with app_.test_client() as client: client.login(api_user_active) @@ -110,7 +115,8 @@ def test_should_show_request_to_go_live(app_, mock_get_service, mock_get_user, mock_get_user_by_email, - mock_login): + mock_login, + mock_has_permissions): with app_.test_request_context(): with app_.test_client() as client: client.login(api_user_active) @@ -130,7 +136,8 @@ def test_should_redirect_after_request_to_go_live(app_, mock_update_service, mock_get_user, mock_get_user_by_email, - mock_login): + mock_login, + mock_has_permissions): with app_.test_request_context(): with app_.test_client() as client: client.login(api_user_active) @@ -151,7 +158,8 @@ def test_should_show_status_page(app_, mock_get_service, mock_get_user, mock_get_user_by_email, - mock_login): + mock_login, + mock_has_permissions): with app_.test_request_context(): with app_.test_client() as client: client.login(api_user_active) @@ -170,7 +178,8 @@ def test_should_show_redirect_after_status_change(app_, mock_get_service, mock_get_user, mock_get_user_by_email, - mock_login): + mock_login, + mock_has_permissions): with app_.test_request_context(): with app_.test_client() as client: client.login(api_user_active) @@ -190,7 +199,8 @@ def test_should_show_status_confirmation(app_, mock_get_service, mock_get_user, mock_get_user_by_email, - mock_login): + mock_login, + mock_has_permissions): with app_.test_request_context(): with app_.test_client() as client: client.login(api_user_active) @@ -211,7 +221,8 @@ def test_should_redirect_after_status_confirmation(app_, mock_get_user, mock_get_user_by_email, mock_login, - mock_verify_password): + mock_verify_password, + mock_has_permissions): with app_.test_request_context(): with app_.test_client() as client: client.login(api_user_active) @@ -232,7 +243,8 @@ def test_should_show_delete_page(app_, mock_get_service, mock_get_user, mock_get_user_by_email, - mock_login): + mock_login, + mock_has_permissions): with app_.test_request_context(): with app_.test_client() as client: client.login(api_user_active) @@ -250,7 +262,8 @@ def test_should_show_redirect_after_deleting_service(app_, mock_get_service, mock_get_user, mock_get_user_by_email, - mock_login): + mock_login, + mock_has_permissions): with app_.test_request_context(): with app_.test_client() as client: client.login(api_user_active) @@ -269,7 +282,8 @@ def test_should_show_delete_confirmation(app_, mock_get_service, mock_get_user, mock_get_user_by_email, - mock_login): + mock_login, + mock_has_permissions): with app_.test_request_context(): with app_.test_client() as client: client.login(api_user_active) @@ -289,7 +303,8 @@ def test_should_redirect_delete_confirmation(app_, mock_get_user, mock_get_user_by_email, mock_login, - mock_verify_password): + mock_verify_password, + mock_has_permissions): with app_.test_request_context(): with app_.test_client() as client: client.login(api_user_active) diff --git a/tests/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py index 1ec43d502..cff04bde1 100644 --- a/tests/app/main/views/test_templates.py +++ b/tests/app/main/views/test_templates.py @@ -9,7 +9,8 @@ def test_should_show_page_for_one_templates(app_, mock_get_service_template, mock_get_user, mock_get_user_by_email, - mock_login): + mock_login, + mock_has_permissions): with app_.test_request_context(): with app_.test_client() as client: client.login(api_user_active) @@ -33,7 +34,8 @@ def test_should_redirect_when_saving_a_template(app_, mock_update_service_template, mock_get_user, mock_get_user_by_email, - mock_login): + mock_login, + mock_has_permissions): with app_.test_request_context(): with app_.test_client() as client: client.login(api_user_active) @@ -65,7 +67,8 @@ def test_should_show_delete_template_page(app_, mock_get_service_template, mock_get_user, mock_get_user_by_email, - mock_login): + mock_login, + mock_has_permissions): with app_.test_request_context(): with app_.test_client() as client: client.login(api_user_active) @@ -91,7 +94,8 @@ def test_should_redirect_when_deleting_a_template(app_, mock_delete_service_template, mock_get_user, mock_get_user_by_email, - mock_login): + mock_login, + mock_has_permissions): with app_.test_request_context(): with app_.test_client() as client: client.login(api_user_active) diff --git a/tests/conftest.py b/tests/conftest.py index 152621b2f..2779f849d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -10,6 +10,7 @@ from . import ( template_json, api_key_json, job_json, + notification_json, invite_json ) from app.notify_client.models import ( @@ -316,6 +317,15 @@ def mock_get_user_by_email(mocker, api_user_active): return mocker.patch('app.user_api_client.get_user_by_email', side_effect=_get_user) +@pytest.fixture(scope='function') +def mock_get_user_with_permissions(mocker, api_user_active): + def _get_user(id): + api_user_active._permissions[''] = ['manage_users', 'manage_templates', 'manage_settings'] + return api_user_active + return mocker.patch( + 'app.user_api_client.get_user', side_effect=_get_user) + + @pytest.fixture(scope='function') def mock_dont_get_user_by_email(mocker): @@ -532,9 +542,19 @@ def mock_get_jobs(mocker): return mocker.patch('app.job_api_client.get_job', side_effect=_get_jobs) +@pytest.fixture(scope='function') +def mock_get_notifications(mocker): + def _get_notifications(service_id, job_id): + return notification_json() + return mocker.patch( + 'app.notification_api_client.get_notifications_for_service', + side_effect=_get_notifications + ) + + @pytest.fixture(scope='function') def mock_has_permissions(mocker): - def _has_permission(service_id, permissions): + def _has_permission(permissions, service_id=None, or_=False): return True return mocker.patch( 'app.notify_client.user_api_client.User.has_permissions',