Prevent template creation or update w/o permission

This commit is contained in:
Ken Tsang
2017-06-27 18:56:35 +01:00
committed by venusbb
parent 542bbb2f34
commit 50066c6753
2 changed files with 108 additions and 22 deletions

View File

@@ -15,7 +15,8 @@ from app.dao.templates_dao import (
) )
from notifications_utils.template import SMSMessageTemplate from notifications_utils.template import SMSMessageTemplate
from app.dao.services_dao import dao_fetch_service_by_id 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) from app.schemas import (template_schema, template_history_schema)
template_blueprint = Blueprint('template', __name__, url_prefix='/service/<uuid:service_id>/template') template_blueprint = Blueprint('template', __name__, url_prefix='/service/<uuid:service_id>/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') 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']) @template_blueprint.route('', methods=['POST'])
def create_template(service_id): def create_template(service_id):
fetched_service = dao_fetch_service_by_id(service_id=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 new_template = template_schema.load(request.get_json()).data
_has_service_permission(new_template.template_type, 'create', permissions)
new_template.service = fetched_service new_template.service = fetched_service
over_limit = _content_count_greater_than_limit(new_template.content, new_template.template_type) over_limit = _content_count_greater_than_limit(new_template.content, new_template.template_type)
if over_limit: if over_limit:
@@ -56,6 +73,8 @@ def create_template(service_id):
def update_template(service_id, template_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) 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() data = request.get_json()
# if redacting, don't update anything else # if redacting, don't update anything else

View File

@@ -7,26 +7,34 @@ from datetime import datetime, timedelta
import pytest import pytest
from freezegun import freeze_time 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 app.dao.templates_dao import dao_get_template_by_id, dao_redact_template
from tests import create_authorization_header 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', [ @pytest.mark.parametrize('template_type, subject', [
('sms', None), (SMS_TYPE, None),
('email', 'subject'), (EMAIL_TYPE, 'subject'),
('letter', 'subject'), (LETTER_TYPE, 'subject'),
]) ])
def test_should_create_a_new_template_for_a_service( 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 = { data = {
'name': 'my template', 'name': 'my template',
'template_type': template_type, 'template_type': template_type,
'content': 'template <b>content</b>', 'content': 'template <b>content</b>',
'service': str(sample_service.id), 'service': str(service.id),
'created_by': str(sample_user.id) 'created_by': str(sample_user.id)
} }
if subject: if subject:
@@ -35,7 +43,7 @@ def test_should_create_a_new_template_for_a_service(
auth_header = create_authorization_header() auth_header = create_authorization_header()
response = client.post( response = client.post(
'/service/{}/template'.format(sample_service.id), '/service/{}/template'.format(service.id),
headers=[('Content-Type', 'application/json'), auth_header], headers=[('Content-Type', 'application/json'), auth_header],
data=data 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']['name'] == 'my template'
assert json_resp['data']['template_type'] == template_type assert json_resp['data']['template_type'] == template_type
assert json_resp['data']['content'] == 'template <b>content</b>' assert json_resp['data']['content'] == 'template <b>content</b>'
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']['id']
assert json_resp['data']['version'] == 1 assert json_resp['data']['version'] == 1
assert json_resp['data']['process_type'] == 'normal' 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) 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 = { data = {
'name': 'my template', 'name': 'my template',
'template_type': 'sms', 'template_type': SMS_TYPE,
'content': 'template content', 'content': 'template content',
'service': fake_uuid, 'service': fake_uuid,
'created_by': str(sample_user.id) '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' 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): def test_should_error_if_created_by_missing(client, sample_user, sample_service):
service_id = str(sample_service.id) service_id = str(sample_service.id)
data = { data = {
'name': 'my template', 'name': 'my template',
'template_type': 'sms', 'template_type': SMS_TYPE,
'content': 'template content', 'content': 'template content',
'service': service_id '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' 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): def test_must_have_a_subject_on_an_email_or_letter_template(client, sample_user, sample_service, template_type):
data = { data = {
'name': 'my template', '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): def test_should_be_able_to_get_all_templates_for_a_service(client, sample_user, sample_service):
data = { data = {
'name': 'my template 1', 'name': 'my template 1',
'template_type': 'email', 'template_type': EMAIL_TYPE,
'subject': 'subject 1', 'subject': 'subject 1',
'content': 'template content', 'content': 'template content',
'service': str(sample_service.id), '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_1 = json.dumps(data)
data = { data = {
'name': 'my template 2', 'name': 'my template 2',
'template_type': 'email', 'template_type': EMAIL_TYPE,
'subject': 'subject 2', 'subject': 'subject 2',
'content': 'template content', 'content': 'template content',
'service': str(sample_service.id), 'service': str(sample_service.id),
@@ -271,7 +338,7 @@ def test_should_get_only_templates_for_that_service(client, sample_user, service
data = { data = {
'name': 'my template 2', 'name': 'my template 2',
'template_type': 'email', 'template_type': EMAIL_TYPE,
'subject': 'subject 2', 'subject': 'subject 2',
'content': 'template content', 'content': 'template content',
'service': str(service_1.id), '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))', 'about your ((thing))',
'hello ((name)) weve received your ((thing))', 'hello ((name)) weve received your ((thing))',
'email' EMAIL_TYPE
), ),
( (
None, None,
'hello ((name)) weve received your ((thing))', 'hello ((name)) weve received your ((thing))',
'sms' SMS_TYPE
), ),
( (
'about your ((thing))', 'about your ((thing))',
'hello ((name)) weve received your ((thing))', 'hello ((name)) weve received your ((thing))',
'letter' LETTER_TYPE
) )
] ]
) )
@@ -401,7 +468,7 @@ def test_should_preview_a_single_template(
): ):
template = create_sample_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( 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)) content = ''.join(random.choice(string.ascii_uppercase + string.digits) for _ in range(limit + 1))
data = { data = {
'name': 'too big template', 'name': 'too big template',
'template_type': 'sms', 'template_type': SMS_TYPE,
'content': content, 'content': content,
'service': str(sample_service.id), 'service': str(sample_service.id),
'created_by': str(sample_user.id) 'created_by': str(sample_user.id)