From 5ec4829d00bdab01a0a7d2849b43dc7adb42e53d Mon Sep 17 00:00:00 2001
From: Leo Hemsted <leo.hemsted@digital.cabinet-office.gov.uk>
Date: Wed, 10 May 2017 11:04:12 +0100
Subject: [PATCH] fix v2 schema phone/email validation when non-str passed in

jsonschema states:

> A format attribute can generally only validate a given set of
> instance types. If the type of the instance to validate is not in
> this set, validation for this format attribute and instance SHOULD
> succeed.

We were not checking for the type of the input, and our validators were
behaving in unexpected manners (throwing TypeErrors etc etc). Despite
us declaring that the phone_number field is of type `str`, we still
need to make sure the validator passes gracefully, so that the inbuilt
type check can be the bit that catches if someone passes in a non-str
value. We've seen this with people passing in integers instead of strs
for phone numbers. This'll make them receive a nice 400 error
(e.g. "phone_number 12345 is not of type string"), rather than us
having a 500 internal server error
---
 app/schema_validation/__init__.py             |  4 +--
 .../test_notification_schemas.py              | 31 ++++++++++++++-----
 2 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/app/schema_validation/__init__.py b/app/schema_validation/__init__.py
index 72767c881..376315a8a 100644
--- a/app/schema_validation/__init__.py
+++ b/app/schema_validation/__init__.py
@@ -10,13 +10,13 @@ def validate(json_to_validate, schema):
 
     @format_checker.checks('phone_number', raises=InvalidPhoneError)
     def validate_schema_phone_number(instance):
-        if instance is not None:
+        if isinstance(instance, str):
             validate_phone_number(instance, international=True)
         return True
 
     @format_checker.checks('email_address', raises=InvalidEmailError)
     def validate_schema_email_address(instance):
-        if instance is not None:
+        if isinstance(instance, str):
             validate_email_address(instance)
         return True
 
diff --git a/tests/app/v2/notifications/test_notification_schemas.py b/tests/app/v2/notifications/test_notification_schemas.py
index 04c32e1c7..65a03b84d 100644
--- a/tests/app/v2/notifications/test_notification_schemas.py
+++ b/tests/app/v2/notifications/test_notification_schemas.py
@@ -131,10 +131,15 @@ def test_post_sms_schema_with_personalisation_that_is_not_a_dict():
     assert len(error.keys()) == 2
 
 
-@pytest.mark.parametrize('invalid_phone_number, err_msg',
-                         [('08515111111', 'phone_number Not a UK mobile number'),
-                          ('07515111*11', 'phone_number Must not contain letters or symbols'),
-                          ('notaphoneumber', 'phone_number Must not contain letters or symbols')])
+@pytest.mark.parametrize('invalid_phone_number, err_msg', [
+    ('08515111111', 'phone_number Not a UK mobile number'),
+    ('07515111*11', 'phone_number Must not contain letters or symbols'),
+    ('notaphoneumber', 'phone_number Must not contain letters or symbols'),
+    (7700900001, 'phone_number 7700900001 is not of type string'),
+    (None, 'phone_number None is not of type string'),
+    ([], 'phone_number [] is not of type string'),
+    ({}, 'phone_number {} is not of type string'),
+])
 def test_post_sms_request_schema_invalid_phone_number(invalid_phone_number, err_msg):
     j = {"phone_number": invalid_phone_number,
          "template_id": str(uuid.uuid4())
@@ -213,12 +218,22 @@ def test_post_email_schema_bad_uuid_and_missing_email_address():
         validate(j, post_email_request_schema)
 
 
-def test_post_email_schema_invalid_email_address():
-    j = {"template_id": str(uuid.uuid4()),
-         "email_address": "notavalidemail@address"}
-    with pytest.raises(ValidationError):
+@pytest.mark.parametrize('email_address, err_msg', [
+    ('example', 'email_address Not a valid email address'),
+    (12345, 'email_address 12345 is not of type string'),
+    (None, 'email_address None is not of type string'),
+    ([], 'email_address [] is not of type string'),
+    ({}, 'email_address {} is not of type string'),
+])
+def test_post_email_schema_invalid_email_address(email_address, err_msg):
+    j = {"template_id": str(uuid.uuid4()), "email_address": email_address}
+    with pytest.raises(ValidationError) as e:
         validate(j, post_email_request_schema)
 
+    errors = json.loads(str(e.value)).get('errors')
+    assert len(errors) == 1
+    assert {"error": "ValidationError", "message": err_msg} == errors[0]
+
 
 def valid_email_response():
     return {