From 09e6e6198b0b7a05aec06513f5ce5ac290f2e46a Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Fri, 23 Sep 2016 15:46:48 +0100 Subject: [PATCH 1/2] Fixing a bug that allows a sms notification to be sent with an email template and vice versa. This has been resolved for the post notifications endpoint --- app/notifications/rest.py | 11 +++++-- .../rest/test_send_notification.py | 33 +++++++++++++++++-- 2 files changed, 38 insertions(+), 6 deletions(-) diff --git a/app/notifications/rest.py b/app/notifications/rest.py index d47f9f27c5..233ceb0654 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -205,6 +205,8 @@ def send_notification(notification_type): notification, errors = ( sms_template_notification_schema if notification_type == SMS_TYPE else email_notification_schema ).load(request.get_json()) + if errors: + raise InvalidRequest(errors, status_code=400) if all((api_user.key_type != KEY_TYPE_TEST, service.restricted)): service_stats = sum(row.count for row in dao_fetch_todays_stats_for_service(service.id)) @@ -212,13 +214,16 @@ def send_notification(notification_type): error = 'Exceeded send limits ({}) for today'.format(service.message_limit) raise InvalidRequest(error, status_code=429) - if errors: - raise InvalidRequest(errors, status_code=400) - template = templates_dao.dao_get_template_by_id_and_service_id( template_id=notification['template'], service_id=service_id ) + + if notification_type != template.template_type: + raise InvalidRequest("{0} template is not suitable for a {1} notification".format(template.template_type, + notification_type), + status_code=400) + errors = unarchived_template_schema.validate({'archived': template.archived}) if errors: raise InvalidRequest(errors, status_code=400) diff --git a/tests/app/notifications/rest/test_send_notification.py b/tests/app/notifications/rest/test_send_notification.py index 0b5e5eaf06..59aa8754c6 100644 --- a/tests/app/notifications/rest/test_send_notification.py +++ b/tests/app/notifications/rest/test_send_notification.py @@ -1,16 +1,13 @@ -import uuid from datetime import datetime import random import string import pytest -from unittest.mock import ANY from flask import (json, current_app) from freezegun import freeze_time from notifications_python_client.authentication import create_jwt_token import app -from app import encryption from app.dao import notifications_dao from app.models import ApiKey, KEY_TYPE_TEAM, KEY_TYPE_TEST, Notification, NotificationHistory from app.dao.templates_dao import dao_get_all_templates_for_service, dao_update_template @@ -999,3 +996,33 @@ def test_should_not_persist_notification_or_send_sms_if_simulated_number( assert response.status_code == 201 apply_async.assert_not_called() assert Notification.query.count() == 0 + + +@pytest.mark.parametrize( + 'notification_type, template_type, to', [ + ('email', 'sms', 'notify@digital.cabinet-office.gov.uk'), + ('sms', 'email', '+447700900986') + ]) +def test_should_error_if_notification_type_does_not_match_template_type( + client, + notify_db, + notify_db_session, + template_type, + notification_type, + to +): + template = create_sample_template(notify_db, notify_db_session, template_type=template_type) + data = { + 'to': to, + 'template': template.id + } + auth_header = create_authorization_header(service_id=template.service_id) + response = client.post("/notifications/{}".format(notification_type), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) + + assert response.status_code == 400 + json_resp = json.loads(response.get_data(as_text=True)) + assert json_resp['result'] == 'error' + assert '{0} template is not suitable for a {1} notification'.format(template_type, notification_type) \ + in json_resp['message'] From 0ef43ab1dcca43d7abc8657fbce140f7b89f3678 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Fri, 23 Sep 2016 16:03:11 +0100 Subject: [PATCH 2/2] Fix the wording on the message a little bit. --- app/notifications/rest.py | 4 ++-- tests/app/notifications/rest/test_send_notification.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/notifications/rest.py b/app/notifications/rest.py index 233ceb0654..c960a6aef5 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -220,8 +220,8 @@ def send_notification(notification_type): ) if notification_type != template.template_type: - raise InvalidRequest("{0} template is not suitable for a {1} notification".format(template.template_type, - notification_type), + raise InvalidRequest("{0} template is not suitable for {1} notification".format(template.template_type, + notification_type), status_code=400) errors = unarchived_template_schema.validate({'archived': template.archived}) diff --git a/tests/app/notifications/rest/test_send_notification.py b/tests/app/notifications/rest/test_send_notification.py index 59aa8754c6..2d7cd43abe 100644 --- a/tests/app/notifications/rest/test_send_notification.py +++ b/tests/app/notifications/rest/test_send_notification.py @@ -1024,5 +1024,5 @@ def test_should_error_if_notification_type_does_not_match_template_type( assert response.status_code == 400 json_resp = json.loads(response.get_data(as_text=True)) assert json_resp['result'] == 'error' - assert '{0} template is not suitable for a {1} notification'.format(template_type, notification_type) \ + assert '{0} template is not suitable for {1} notification'.format(template_type, notification_type) \ in json_resp['message']