From e0fbcb0dc67054714fdf4db8e1783d1a496e765e Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Thu, 29 Jun 2017 11:13:32 +0100 Subject: [PATCH] Add permission check in for v2 post notification --- app/notifications/validators.py | 19 +++++++--- app/template/rest.py | 6 +-- app/v2/notifications/post_notifications.py | 11 ++++-- .../notifications/test_post_notifications.py | 38 +++++++++++++++++-- 4 files changed, 58 insertions(+), 16 deletions(-) diff --git a/app/notifications/validators.py b/app/notifications/validators.py index f171e746e..f35982b68 100644 --- a/app/notifications/validators.py +++ b/app/notifications/validators.py @@ -73,6 +73,19 @@ def service_can_send_to_recipient(send_to, key_type, service): raise BadRequestError(message=message) +def service_has_permission(service, permission): + if permission not in [p.permission for p in service.permissions]: + action = 'send' + permission_text = permission + 's' + if permission == SMS_TYPE: + permission_text = 'text messages' + elif permission == SCHEDULE_NOTIFICATIONS: + action = 'schedule' + permission_text = "notifications (this feature is invite-only)" + + raise BadRequestError(message="Cannot {} {}".format(action, permission_text)) + + def validate_and_format_recipient(send_to, key_type, service, notification_type): service_can_send_to_recipient(send_to, key_type, service) @@ -98,12 +111,6 @@ def check_sms_content_char_count(content_count): raise BadRequestError(message=message) -def service_can_schedule_notification(service, scheduled_for): - if scheduled_for: - if SCHEDULE_NOTIFICATIONS not in [p.permission for p in service.permissions]: - raise BadRequestError(message="Cannot schedule notifications (this feature is invite-only)") - - def validate_template(template_id, personalisation, service, notification_type): try: template = templates_dao.dao_get_template_by_id_and_service_id( diff --git a/app/template/rest.py b/app/template/rest.py index b15401f1a..c82e99eec 100644 --- a/app/template/rest.py +++ b/app/template/rest.py @@ -37,7 +37,7 @@ 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): +def _service_has_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: @@ -53,7 +53,7 @@ def create_template(service_id): permissions = fetched_service.permissions new_template = template_schema.load(request.get_json()).data - _has_service_permission(new_template.template_type, 'Create', permissions) + _service_has_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) @@ -71,7 +71,7 @@ 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) + _service_has_permission(fetched_template.template_type, 'Update', fetched_template.service.permissions) data = request.get_json() diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index 01e25ea20..e4b1ee52e 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -2,7 +2,7 @@ from flask import request, jsonify, current_app from app import api_user, authenticated_service from app.config import QueueNames -from app.models import SMS_TYPE, EMAIL_TYPE, PRIORITY +from app.models import SMS_TYPE, EMAIL_TYPE, PRIORITY, SCHEDULE_NOTIFICATIONS from app.notifications.process_notifications import ( persist_notification, send_notification_to_queue, @@ -11,7 +11,7 @@ from app.notifications.process_notifications import ( from app.notifications.validators import ( validate_and_format_recipient, check_rate_limiting, - service_can_schedule_notification, + service_has_permission, validate_template ) from app.schema_validation import validate @@ -30,8 +30,11 @@ def post_notification(notification_type): else: form = validate(request.get_json(), post_sms_request) + service_has_permission(authenticated_service, notification_type) + scheduled_for = form.get("scheduled_for", None) - service_can_schedule_notification(authenticated_service, scheduled_for) + if scheduled_for: + service_has_permission(authenticated_service, SCHEDULE_NOTIFICATIONS) check_rate_limiting(authenticated_service, api_user) @@ -45,7 +48,7 @@ def post_notification(notification_type): form['template_id'], form.get('personalisation', {}), authenticated_service, - notification_type + notification_type, ) # Do not persist or send notification to the queue if it is a simulated recipient diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index 464f3a456..aea003b7e 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -12,7 +12,11 @@ from flask import json, current_app from app.models import Notification from app.v2.errors import RateLimitError from tests import create_authorization_header -from tests.app.conftest import sample_template as create_sample_template, sample_service +from tests.app.conftest import ( + sample_template as create_sample_template, sample_service, + sample_template_without_sms_permission, sample_template_without_email_permission +) + from tests.app.db import create_service, create_template @@ -54,7 +58,7 @@ def test_post_sms_notification_returns_201(client, sample_template_with_placehol @pytest.mark.parametrize("notification_type, key_send_to, send_to", [("sms", "phone_number", "+447700900855"), ("email", "email_address", "sample@email.com")]) -def test_post_sms_notification_returns_404_and_missing_template(client, sample_service, +def test_post_sms_notification_returns_400_and_missing_template(client, sample_service, notification_type, key_send_to, send_to): data = { key_send_to: send_to, @@ -308,9 +312,37 @@ def test_post_sms_notification_returns_400_if_not_allowed_to_send_int_sms(client ] +@pytest.mark.parametrize('template_factory,expected_error', [ + (sample_template_without_sms_permission, 'Cannot send text messages'), + (sample_template_without_email_permission, 'Cannot send emails')]) +def test_post_sms_notification_returns_400_if_not_allowed_to_send_notification( + client, template_factory, expected_error, notify_db, notify_db_session): + sample_template_without_permission = template_factory(notify_db, notify_db_session) + data = { + 'phone_number': '07700 900000', + 'email_address': 'someone@test.com', + 'template_id': sample_template_without_permission.id + } + auth_header = create_authorization_header(service_id=sample_template_without_permission.service.id) + + response = client.post( + path='/v2/notifications/{}'.format(sample_template_without_permission.template_type), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) + + assert response.status_code == 400 + assert response.headers['Content-type'] == 'application/json' + + error_json = json.loads(response.get_data(as_text=True)) + assert error_json['status_code'] == 400 + assert error_json['errors'] == [ + {"error": "BadRequestError", "message": expected_error} + ] + + def test_post_sms_notification_returns_201_if_allowed_to_send_int_sms(notify_db, notify_db_session, client, mocker): - service = sample_service(notify_db, notify_db_session, permissions=[INTERNATIONAL_SMS_TYPE]) + service = sample_service(notify_db, notify_db_session, permissions=[SMS_TYPE, INTERNATIONAL_SMS_TYPE]) template = create_sample_template(notify_db, notify_db_session, service=service) mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async')