From 50066c6753a287fc5682911b37646ebbfdaed20e Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Tue, 27 Jun 2017 18:56:35 +0100 Subject: [PATCH] Prevent template creation or update w/o permission --- app/template/rest.py | 21 +++++- tests/app/template/test_rest.py | 109 ++++++++++++++++++++++++++------ 2 files changed, 108 insertions(+), 22 deletions(-) diff --git a/app/template/rest.py b/app/template/rest.py index a9bd44f1f..ac7e12d5b 100644 --- a/app/template/rest.py +++ b/app/template/rest.py @@ -15,7 +15,8 @@ from app.dao.templates_dao import ( ) from notifications_utils.template import SMSMessageTemplate from app.dao.services_dao import dao_fetch_service_by_id -from app.models import SMS_TYPE +from app.dao.service_permissions_dao import dao_fetch_service_permissions +from app.models import SMS_TYPE, EMAIL_TYPE from app.schemas import (template_schema, template_history_schema) template_blueprint = Blueprint('template', __name__, url_prefix='/service//template') @@ -36,10 +37,26 @@ def _content_count_greater_than_limit(content, template_type): return template.content_count > current_app.config.get('SMS_CHAR_COUNT_LIMIT') +def _has_service_permission(template_type, action, permissions): + if template_type not in [p.permission for p in permissions]: + template_type_text = template_type + if template_type == SMS_TYPE: + template_type_text = 'text message' + message = 'Cannot {action} {type} templates'.format( + action=action, type=template_type_text) + errors = {'content': [message]} + raise InvalidRequest(errors, status_code=400) + + @template_blueprint.route('', methods=['POST']) def create_template(service_id): fetched_service = dao_fetch_service_by_id(service_id=service_id) + # permissions needs to be placed here otherwise marshmallow will intefere with versioning + permissions = fetched_service.permissions new_template = template_schema.load(request.get_json()).data + + _has_service_permission(new_template.template_type, 'create', permissions) + new_template.service = fetched_service over_limit = _content_count_greater_than_limit(new_template.content, new_template.template_type) if over_limit: @@ -56,6 +73,8 @@ def create_template(service_id): def update_template(service_id, template_id): fetched_template = dao_get_template_by_id_and_service_id(template_id=template_id, service_id=service_id) + _has_service_permission(fetched_template.template_type, 'update', fetched_template.service.permissions) + data = request.get_json() # if redacting, don't update anything else diff --git a/tests/app/template/test_rest.py b/tests/app/template/test_rest.py index 80c30d71b..8ce72bd40 100644 --- a/tests/app/template/test_rest.py +++ b/tests/app/template/test_rest.py @@ -7,26 +7,34 @@ from datetime import datetime, timedelta import pytest from freezegun import freeze_time -from app.models import Template +from app.models import Template, SMS_TYPE, EMAIL_TYPE, LETTER_TYPE from app.dao.templates_dao import dao_get_template_by_id, dao_redact_template from tests import create_authorization_header -from tests.app.conftest import sample_template as create_sample_template +from tests.app.conftest import ( + sample_template as create_sample_template, + sample_template_without_sms_permission, + sample_template_without_email_permission +) +from tests.app.db import create_service + +from app.dao.templates_dao import dao_get_template_by_id @pytest.mark.parametrize('template_type, subject', [ - ('sms', None), - ('email', 'subject'), - ('letter', 'subject'), + (SMS_TYPE, None), + (EMAIL_TYPE, 'subject'), + (LETTER_TYPE, 'subject'), ]) def test_should_create_a_new_template_for_a_service( - client, sample_user, sample_service, template_type, subject + client, sample_user, template_type, subject ): + service = create_service(service_permissions=[template_type]) data = { 'name': 'my template', 'template_type': template_type, 'content': 'template content', - 'service': str(sample_service.id), + 'service': str(service.id), 'created_by': str(sample_user.id) } if subject: @@ -35,7 +43,7 @@ def test_should_create_a_new_template_for_a_service( auth_header = create_authorization_header() response = client.post( - '/service/{}/template'.format(sample_service.id), + '/service/{}/template'.format(service.id), headers=[('Content-Type', 'application/json'), auth_header], data=data ) @@ -44,7 +52,7 @@ def test_should_create_a_new_template_for_a_service( assert json_resp['data']['name'] == 'my template' assert json_resp['data']['template_type'] == template_type assert json_resp['data']['content'] == 'template content' - assert json_resp['data']['service'] == str(sample_service.id) + assert json_resp['data']['service'] == str(service.id) assert json_resp['data']['id'] assert json_resp['data']['version'] == 1 assert json_resp['data']['process_type'] == 'normal' @@ -59,10 +67,10 @@ def test_should_create_a_new_template_for_a_service( assert sorted(json_resp['data']) == sorted(template_schema.dump(template).data) -def test_should_be_error_if_service_does_not_exist_on_create(client, sample_user, fake_uuid): +def test_should_raise_error_if_service_does_not_exist_on_create(client, sample_user, fake_uuid): data = { 'name': 'my template', - 'template_type': 'sms', + 'template_type': SMS_TYPE, 'content': 'template content', 'service': fake_uuid, 'created_by': str(sample_user.id) @@ -81,11 +89,70 @@ def test_should_be_error_if_service_does_not_exist_on_create(client, sample_user assert json_resp['message'] == 'No result found' +@pytest.mark.parametrize('permissions, template_type, subject, expected_error', [ + ([EMAIL_TYPE], SMS_TYPE, None, 'Cannot create text message templates'), + ([SMS_TYPE], EMAIL_TYPE, 'subject', 'Cannot create email templates'), +]) +def test_should_raise_error_on_create_if_no_permission( + client, sample_user, permissions, template_type, subject, expected_error): + service = create_service(service_permissions=permissions) + data = { + 'name': 'my template', + 'template_type': template_type, + 'content': 'template content', + 'service': str(service.id), + 'created_by': str(sample_user.id) + } + if subject: + data.update({'subject': subject}) + + data = json.dumps(data) + auth_header = create_authorization_header() + + response = client.post( + '/service/{}/template'.format(service.id), + headers=[('Content-Type', 'application/json'), auth_header], + data=data + ) + json_resp = json.loads(response.get_data(as_text=True)) + assert response.status_code == 400 + assert json_resp['result'] == 'error' + assert json_resp['message'] == {'content': [expected_error]} + + +@pytest.mark.parametrize('template_factory, expected_error', [ + (sample_template_without_sms_permission, 'Cannot update text message templates'), + (sample_template_without_email_permission, 'Cannot update email templates'), +]) +def test_should_be_error_on_update_if_no_permission( + client, sample_user, template_factory, expected_error, notify_db, notify_db_session): + template_without_permission = template_factory(notify_db, notify_db_session) + data = { + 'content': 'new template content', + 'created_by': str(sample_user.id) + } + + data = json.dumps(data) + auth_header = create_authorization_header() + + update_response = client.post( + '/service/{}/template/{}'.format( + template_without_permission.service_id, template_without_permission.id), + headers=[('Content-Type', 'application/json'), auth_header], + data=data + ) + + json_resp = json.loads(update_response.get_data(as_text=True)) + assert update_response.status_code == 400 + assert json_resp['result'] == 'error' + assert json_resp['message'] == {'content': [expected_error]} + + def test_should_error_if_created_by_missing(client, sample_user, sample_service): service_id = str(sample_service.id) data = { 'name': 'my template', - 'template_type': 'sms', + 'template_type': SMS_TYPE, 'content': 'template content', 'service': service_id } @@ -120,7 +187,7 @@ def test_should_be_error_if_service_does_not_exist_on_update(client, fake_uuid): assert json_resp['message'] == 'No result found' -@pytest.mark.parametrize('template_type', ['email', 'letter']) +@pytest.mark.parametrize('template_type', [EMAIL_TYPE, LETTER_TYPE]) def test_must_have_a_subject_on_an_email_or_letter_template(client, sample_user, sample_service, template_type): data = { 'name': 'my template', @@ -194,7 +261,7 @@ def test_should_be_able_to_archive_template(client, sample_template): def test_should_be_able_to_get_all_templates_for_a_service(client, sample_user, sample_service): data = { 'name': 'my template 1', - 'template_type': 'email', + 'template_type': EMAIL_TYPE, 'subject': 'subject 1', 'content': 'template content', 'service': str(sample_service.id), @@ -203,7 +270,7 @@ def test_should_be_able_to_get_all_templates_for_a_service(client, sample_user, data_1 = json.dumps(data) data = { 'name': 'my template 2', - 'template_type': 'email', + 'template_type': EMAIL_TYPE, 'subject': 'subject 2', 'content': 'template content', 'service': str(sample_service.id), @@ -271,7 +338,7 @@ def test_should_get_only_templates_for_that_service(client, sample_user, service data = { 'name': 'my template 2', - 'template_type': 'email', + 'template_type': EMAIL_TYPE, 'subject': 'subject 2', 'content': 'template content', 'service': str(service_1.id), @@ -310,17 +377,17 @@ def test_should_get_only_templates_for_that_service(client, sample_user, service ( 'about your ((thing))', 'hello ((name)) we’ve received your ((thing))', - 'email' + EMAIL_TYPE ), ( None, 'hello ((name)) we’ve received your ((thing))', - 'sms' + SMS_TYPE ), ( 'about your ((thing))', 'hello ((name)) we’ve received your ((thing))', - 'letter' + LETTER_TYPE ) ] ) @@ -401,7 +468,7 @@ def test_should_preview_a_single_template( ): template = create_sample_template( - notify_db, notify_db.session, subject_line=subject, content=content, template_type='email' + notify_db, notify_db.session, subject_line=subject, content=content, template_type=EMAIL_TYPE ) response = client.get( @@ -455,7 +522,7 @@ def test_create_400_for_over_limit_content(client, notify_api, sample_user, samp content = ''.join(random.choice(string.ascii_uppercase + string.digits) for _ in range(limit + 1)) data = { 'name': 'too big template', - 'template_type': 'sms', + 'template_type': SMS_TYPE, 'content': content, 'service': str(sample_service.id), 'created_by': str(sample_user.id)