mirror of
https://github.com/GSA/notifications-api.git
synced 2026-02-04 18:31:13 -05:00
Use validation of recipients from utils
This was added to utils in 5914da74f1
This means that:
- we are doing the exact same validation in the API and admin app
- we are actually validating phone numbers for the correct format (hence all the
changes to the tests)
This commit is contained in:
@@ -6,10 +6,7 @@ from . import models
|
|||||||
from app.dao.permissions_dao import permission_dao
|
from app.dao.permissions_dao import permission_dao
|
||||||
from marshmallow import (post_load, ValidationError, validates, validates_schema)
|
from marshmallow import (post_load, ValidationError, validates, validates_schema)
|
||||||
from marshmallow_sqlalchemy import field_for
|
from marshmallow_sqlalchemy import field_for
|
||||||
|
from utils.recipients import validate_email_address, InvalidEmailError, validate_phone_number, InvalidPhoneError
|
||||||
mobile_regex = re.compile("^\\+44[\\d]{10}$")
|
|
||||||
|
|
||||||
email_regex = re.compile("(^[^@^\\s]+@[^@^\\.^\\s]+(\\.[^@^\\.^\\s]*)*\.(.+))")
|
|
||||||
|
|
||||||
|
|
||||||
# TODO I think marshmallow provides a better integration and error handling.
|
# TODO I think marshmallow provides a better integration and error handling.
|
||||||
@@ -128,8 +125,10 @@ class SmsNotificationSchema(NotificationSchema):
|
|||||||
|
|
||||||
@validates('to')
|
@validates('to')
|
||||||
def validate_to(self, value):
|
def validate_to(self, value):
|
||||||
if not mobile_regex.match(value):
|
try:
|
||||||
raise ValidationError('Invalid phone number, must be of format +441234123123')
|
validate_phone_number(value)
|
||||||
|
except InvalidPhoneError as error:
|
||||||
|
raise ValidationError('Invalid phone number: {}'.format(error))
|
||||||
|
|
||||||
|
|
||||||
class EmailNotificationSchema(NotificationSchema):
|
class EmailNotificationSchema(NotificationSchema):
|
||||||
@@ -138,7 +137,9 @@ class EmailNotificationSchema(NotificationSchema):
|
|||||||
|
|
||||||
@validates('to')
|
@validates('to')
|
||||||
def validate_to(self, value):
|
def validate_to(self, value):
|
||||||
if not email_regex.match(value):
|
try:
|
||||||
|
validate_email_address(value)
|
||||||
|
except InvalidEmailError:
|
||||||
raise ValidationError('Invalid email')
|
raise ValidationError('Invalid email')
|
||||||
|
|
||||||
|
|
||||||
@@ -174,7 +175,9 @@ class InvitedUserSchema(BaseSchema):
|
|||||||
|
|
||||||
@validates('email_address')
|
@validates('email_address')
|
||||||
def validate_to(self, value):
|
def validate_to(self, value):
|
||||||
if not email_regex.match(value):
|
try:
|
||||||
|
validate_email_address(value)
|
||||||
|
except InvalidEmailError:
|
||||||
raise ValidationError('Invalid email')
|
raise ValidationError('Invalid email')
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -277,7 +277,7 @@ def sample_notification(notify_db,
|
|||||||
if to_field:
|
if to_field:
|
||||||
to = to_field
|
to = to_field
|
||||||
else:
|
else:
|
||||||
to = '+44709123456'
|
to = '+447700900855'
|
||||||
|
|
||||||
data = {
|
data = {
|
||||||
'id': notification_id,
|
'id': notification_id,
|
||||||
|
|||||||
@@ -24,7 +24,7 @@ def test_get_notification_by_id(notify_api, sample_notification):
|
|||||||
notification = json.loads(response.get_data(as_text=True))['notification']
|
notification = json.loads(response.get_data(as_text=True))['notification']
|
||||||
assert notification['status'] == 'sent'
|
assert notification['status'] == 'sent'
|
||||||
assert notification['template'] == sample_notification.template.id
|
assert notification['template'] == sample_notification.template.id
|
||||||
assert notification['to'] == '+44709123456'
|
assert notification['to'] == '+447700900855'
|
||||||
assert notification['service'] == str(sample_notification.service_id)
|
assert notification['service'] == str(sample_notification.service_id)
|
||||||
assert response.status_code == 200
|
assert response.status_code == 200
|
||||||
|
|
||||||
@@ -63,7 +63,7 @@ def test_get_all_notifications(notify_api, sample_notification):
|
|||||||
notifications = json.loads(response.get_data(as_text=True))
|
notifications = json.loads(response.get_data(as_text=True))
|
||||||
assert notifications['notifications'][0]['status'] == 'sent'
|
assert notifications['notifications'][0]['status'] == 'sent'
|
||||||
assert notifications['notifications'][0]['template'] == sample_notification.template.id
|
assert notifications['notifications'][0]['template'] == sample_notification.template.id
|
||||||
assert notifications['notifications'][0]['to'] == '+44709123456'
|
assert notifications['notifications'][0]['to'] == '+447700900855'
|
||||||
assert notifications['notifications'][0]['service'] == str(sample_notification.service_id)
|
assert notifications['notifications'][0]['service'] == str(sample_notification.service_id)
|
||||||
assert response.status_code == 200
|
assert response.status_code == 200
|
||||||
|
|
||||||
@@ -296,7 +296,7 @@ def test_should_reject_bad_phone_numbers(notify_api, sample_template, mocker):
|
|||||||
|
|
||||||
assert json_resp['result'] == 'error'
|
assert json_resp['result'] == 'error'
|
||||||
assert len(json_resp['message'].keys()) == 1
|
assert len(json_resp['message'].keys()) == 1
|
||||||
assert 'Invalid phone number, must be of format +441234123123' in json_resp['message']['to']
|
assert 'Invalid phone number: Must not contain letters or symbols' in json_resp['message']['to']
|
||||||
assert response.status_code == 400
|
assert response.status_code == 400
|
||||||
|
|
||||||
|
|
||||||
@@ -306,7 +306,7 @@ def test_send_notification_invalid_template_id(notify_api, sample_template, mock
|
|||||||
mocker.patch('app.celery.tasks.send_sms.apply_async')
|
mocker.patch('app.celery.tasks.send_sms.apply_async')
|
||||||
|
|
||||||
data = {
|
data = {
|
||||||
'to': '+441234123123',
|
'to': '+447700900855',
|
||||||
'template': 9999
|
'template': 9999
|
||||||
}
|
}
|
||||||
auth_header = create_authorization_header(
|
auth_header = create_authorization_header(
|
||||||
@@ -337,7 +337,7 @@ def test_send_notification_with_placeholders_replaced(notify_api, sample_templat
|
|||||||
mocker.patch('app.encryption.encrypt', return_value="something_encrypted")
|
mocker.patch('app.encryption.encrypt', return_value="something_encrypted")
|
||||||
|
|
||||||
data = {
|
data = {
|
||||||
'to': '+441234123123',
|
'to': '+447700900855',
|
||||||
'template': sample_template_with_placeholders.id,
|
'template': sample_template_with_placeholders.id,
|
||||||
'personalisation': {
|
'personalisation': {
|
||||||
'name': 'Jo'
|
'name': 'Jo'
|
||||||
@@ -373,7 +373,7 @@ def test_send_notification_with_missing_personalisation(
|
|||||||
mocker.patch('app.celery.tasks.send_sms.apply_async')
|
mocker.patch('app.celery.tasks.send_sms.apply_async')
|
||||||
|
|
||||||
data = {
|
data = {
|
||||||
'to': '+441234123123',
|
'to': '+447700900855',
|
||||||
'template': sample_template_with_placeholders.id,
|
'template': sample_template_with_placeholders.id,
|
||||||
'personalisation': {
|
'personalisation': {
|
||||||
'foo': 'bar'
|
'foo': 'bar'
|
||||||
@@ -405,7 +405,7 @@ def test_send_notification_with_too_much_personalisation_data(
|
|||||||
mocker.patch('app.celery.tasks.send_sms.apply_async')
|
mocker.patch('app.celery.tasks.send_sms.apply_async')
|
||||||
|
|
||||||
data = {
|
data = {
|
||||||
'to': '+441234123123',
|
'to': '+447700900855',
|
||||||
'template': sample_template_with_placeholders.id,
|
'template': sample_template_with_placeholders.id,
|
||||||
'personalisation': {
|
'personalisation': {
|
||||||
'name': 'Jo', 'foo': 'bar'
|
'name': 'Jo', 'foo': 'bar'
|
||||||
@@ -439,7 +439,7 @@ def test_prevents_sending_to_any_mobile_on_restricted_service(notify_api, sample
|
|||||||
).update(
|
).update(
|
||||||
{'restricted': True}
|
{'restricted': True}
|
||||||
)
|
)
|
||||||
invalid_mob = '+449999999999'
|
invalid_mob = '+447700900855'
|
||||||
data = {
|
data = {
|
||||||
'to': invalid_mob,
|
'to': invalid_mob,
|
||||||
'template': sample_template.id
|
'template': sample_template.id
|
||||||
@@ -504,7 +504,7 @@ def test_should_allow_valid_sms_notification(notify_api, sample_template, mocker
|
|||||||
mocker.patch('app.encryption.encrypt', return_value="something_encrypted")
|
mocker.patch('app.encryption.encrypt', return_value="something_encrypted")
|
||||||
|
|
||||||
data = {
|
data = {
|
||||||
'to': '+441234123123',
|
'to': '+447700900855',
|
||||||
'template': sample_template.id
|
'template': sample_template.id
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user