Merge pull request #772 from alphagov/fix-v2-uri

Fix uri validation in v2 responses
This commit is contained in:
imdadahad
2016-12-16 15:53:26 +00:00
committed by GitHub
5 changed files with 169 additions and 81 deletions

View File

@@ -42,4 +42,27 @@ def build_error_message(errors):
def __format_message(e): def __format_message(e):
return e.message.replace("'", "") if not e.cause else "{} {}".format(e.path[0], e.cause.message) def get_path(e):
error_path = None
try:
error_path = e.path.popleft()
# no need to catch IndexError exception explicity as
# error_path is None if e.path has no items
finally:
return error_path
def get_error_message(e):
error_message = None
try:
error_message = e.cause.message
except AttributeError:
error_message = e.message
finally:
return error_message.replace("'", '')
path = get_path(e)
message = get_error_message(e)
if path:
return "{} {}".format(path, message)
else:
return "{}".format(message)

View File

@@ -10,7 +10,7 @@ template = {
"properties": { "properties": {
"id": uuid, "id": uuid,
"version": {"type": "integer"}, "version": {"type": "integer"},
"uri": {"type": "string"} "uri": {"type": "string", "format": "uri"}
}, },
"required": ["id", "version", "uri"] "required": ["id", "version", "uri"]
} }
@@ -163,7 +163,7 @@ post_sms_response = {
"id": uuid, "id": uuid,
"reference": {"type": ["string", "null"]}, "reference": {"type": ["string", "null"]},
"content": sms_content, "content": sms_content,
"uri": {"type": "string"}, "uri": {"type": "string", "format": "uri"},
"template": template "template": template
}, },
"required": ["id", "content", "uri", "template"] "required": ["id", "content", "uri", "template"]
@@ -206,7 +206,7 @@ post_email_response = {
"id": uuid, "id": uuid,
"reference": {"type": ["string", "null"]}, "reference": {"type": ["string", "null"]},
"content": email_content, "content": email_content,
"uri": {"type": "string"}, "uri": {"type": "string", "format": "uri"},
"template": template "template": template
}, },
"required": ["id", "content", "uri", "template"] "required": ["id", "content", "uri", "template"]
@@ -218,7 +218,7 @@ def create_post_sms_response_from_notification(notification, body, from_number,
"reference": notification.client_reference, "reference": notification.client_reference,
"content": {'body': body, "content": {'body': body,
'from_number': from_number}, 'from_number': from_number},
"uri": "{}/v2/notifications/{}".format(url_root, str(notification.id)), "uri": "{}v2/notifications/{}".format(url_root, str(notification.id)),
"template": __create_template_from_notification(notification=notification, url_root=url_root) "template": __create_template_from_notification(notification=notification, url_root=url_root)
} }
@@ -232,7 +232,7 @@ def create_post_email_response_from_notification(notification, content, subject,
"body": content, "body": content,
"subject": subject "subject": subject
}, },
"uri": "{}/v2/notifications/{}".format(url_root, str(notification.id)), "uri": "{}v2/notifications/{}".format(url_root, str(notification.id)),
"template": __create_template_from_notification(notification=notification, url_root=url_root) "template": __create_template_from_notification(notification=notification, url_root=url_root)
} }
@@ -241,5 +241,5 @@ def __create_template_from_notification(notification, url_root):
return { return {
"id": notification.template_id, "id": notification.template_id,
"version": notification.template_version, "version": notification.template_version,
"uri": "{}/v2/templates/{}".format(url_root, str(notification.template_id)) "uri": "{}v2/templates/{}".format(url_root, str(notification.template_id))
} }

View File

@@ -109,7 +109,6 @@ def test_should_send_personalised_template_to_correct_email_provider_and_persist
assert '<!DOCTYPE html' in app.aws_ses_client.send_email.call_args[1]['html_body'] assert '<!DOCTYPE html' in app.aws_ses_client.send_email.call_args[1]['html_body']
notification = Notification.query.filter_by(id=db_notification.id).one() notification = Notification.query.filter_by(id=db_notification.id).one()
assert notification.status == 'sending' assert notification.status == 'sending'
assert notification.sent_at <= datetime.utcnow() assert notification.sent_at <= datetime.utcnow()
assert notification.sent_by == 'ses' assert notification.sent_by == 'ses'

View File

@@ -269,7 +269,7 @@ def test_get_all_notifications_filter_by_template_type_invalid_template_type(cli
assert json_response['status_code'] == 400 assert json_response['status_code'] == 400
assert len(json_response['errors']) == 1 assert len(json_response['errors']) == 1
assert json_response['errors'][0]['message'] == "orange is not one of [sms, email, letter]" assert json_response['errors'][0]['message'] == "template_type orange is not one of [sms, email, letter]"
def test_get_all_notifications_filter_by_single_status(client, notify_db, notify_db_session): def test_get_all_notifications_filter_by_single_status(client, notify_db, notify_db_session):
@@ -306,7 +306,7 @@ def test_get_all_notifications_filter_by_status_invalid_status(client, sample_no
assert json_response['status_code'] == 400 assert json_response['status_code'] == 400
assert len(json_response['errors']) == 1 assert len(json_response['errors']) == 1
assert json_response['errors'][0]['message'] == "elephant is not one of [created, sending, delivered, " \ assert json_response['errors'][0]['message'] == "status elephant is not one of [created, sending, delivered, " \
"pending, failed, technical-failure, temporary-failure, permanent-failure]" "pending, failed, technical-failure, temporary-failure, permanent-failure]"

View File

@@ -6,7 +6,10 @@ from jsonschema import ValidationError
from app.v2.notifications.notification_schemas import ( from app.v2.notifications.notification_schemas import (
get_notifications_request, get_notifications_request,
post_sms_request, post_sms_response, post_email_request, post_email_response post_sms_request as post_sms_request_schema,
post_sms_response as post_sms_response_schema,
post_email_request as post_email_request_schema,
post_email_response as post_email_response_schema
) )
from app.schema_validation import validate from app.schema_validation import validate
@@ -23,7 +26,8 @@ def test_get_notifications_request_invalid_statuses(
invalid_statuses, valid_statuses invalid_statuses, valid_statuses
): ):
partial_error_status = "is not one of " \ partial_error_status = "is not one of " \
"[created, sending, delivered, pending, failed, technical-failure, temporary-failure, permanent-failure]" "[created, sending, delivered, pending, failed, " \
"technical-failure, temporary-failure, permanent-failure]"
with pytest.raises(ValidationError) as e: with pytest.raises(ValidationError) as e:
validate({'status': invalid_statuses + valid_statuses}, get_notifications_request) validate({'status': invalid_statuses + valid_statuses}, get_notifications_request)
@@ -31,7 +35,7 @@ def test_get_notifications_request_invalid_statuses(
errors = json.loads(e.value.message).get('errors') errors = json.loads(e.value.message).get('errors')
assert len(errors) == len(invalid_statuses) assert len(errors) == len(invalid_statuses)
for index, value in enumerate(invalid_statuses): for index, value in enumerate(invalid_statuses):
assert errors[index]['message'] == "{} {}".format(value, partial_error_status) assert errors[index]['message'] == "status {} {}".format(value, partial_error_status)
@pytest.mark.parametrize('invalid_template_types, valid_template_types', [ @pytest.mark.parametrize('invalid_template_types, valid_template_types', [
@@ -53,7 +57,7 @@ def test_get_notifications_request_invalid_template_types(
errors = json.loads(e.value.message).get('errors') errors = json.loads(e.value.message).get('errors')
assert len(errors) == len(invalid_template_types) assert len(errors) == len(invalid_template_types)
for index, value in enumerate(invalid_template_types): for index, value in enumerate(invalid_template_types):
assert errors[index]['message'] == "{} {}".format(value, partial_error_template_type) assert errors[index]['message'] == "template_type {} {}".format(value, partial_error_template_type)
def test_get_notifications_request_invalid_statuses_and_template_types(): def test_get_notifications_request_invalid_statuses_and_template_types():
@@ -68,15 +72,15 @@ def test_get_notifications_request_invalid_statuses_and_template_types():
assert len(errors) == 4 assert len(errors) == 4
error_messages = [error['message'] for error in errors] error_messages = [error['message'] for error in errors]
for invalid_status in ["elephant", "giraffe"]: for invalid_status in ["elephant", "giraffe"]:
assert "{} is not one of [created, sending, delivered, " \ assert "status {} is not one of [created, sending, delivered, " \
"pending, failed, technical-failure, temporary-failure, permanent-failure]".format( "pending, failed, technical-failure, temporary-failure, permanent-failure]".format(
invalid_status invalid_status
) in error_messages ) in error_messages
for invalid_template_type in ["orange", "avocado"]: for invalid_template_type in ["orange", "avocado"]:
assert "{} is not one of [sms, email, letter]".format(invalid_template_type) in error_messages assert "template_type {} is not one of [sms, email, letter]" \
.format(invalid_template_type) in error_messages
valid_json = {"phone_number": "07515111111", valid_json = {"phone_number": "07515111111",
@@ -92,13 +96,13 @@ valid_json_with_optionals = {
@pytest.mark.parametrize("input", [valid_json, valid_json_with_optionals]) @pytest.mark.parametrize("input", [valid_json, valid_json_with_optionals])
def test_post_sms_schema_is_valid(input): def test_post_sms_schema_is_valid(input):
assert validate(input, post_sms_request) == input assert validate(input, post_sms_request_schema) == input
def test_post_sms_json_schema_bad_uuid_and_missing_phone_number(): def test_post_sms_json_schema_bad_uuid_and_missing_phone_number():
j = {"template_id": "notUUID"} j = {"template_id": "notUUID"}
with pytest.raises(ValidationError) as e: with pytest.raises(ValidationError) as e:
validate(j, post_sms_request) validate(j, post_sms_request_schema)
error = json.loads(e.value.message) error = json.loads(e.value.message)
assert len(error.keys()) == 2 assert len(error.keys()) == 2
assert error.get('status_code') == 400 assert error.get('status_code') == 400
@@ -117,7 +121,7 @@ def test_post_sms_schema_with_personalisation_that_is_not_a_dict():
"personalisation": "not_a_dict" "personalisation": "not_a_dict"
} }
with pytest.raises(ValidationError) as e: with pytest.raises(ValidationError) as e:
validate(j, post_sms_request) validate(j, post_sms_request_schema)
error = json.loads(e.value.message) error = json.loads(e.value.message)
assert len(error.get('errors')) == 1 assert len(error.get('errors')) == 1
assert error['errors'] == [{'error': 'ValidationError', assert error['errors'] == [{'error': 'ValidationError',
@@ -130,64 +134,60 @@ def test_post_sms_schema_with_personalisation_that_is_not_a_dict():
[('08515111111', 'phone_number Not a UK mobile number'), [('08515111111', 'phone_number Not a UK mobile number'),
('07515111*11', 'phone_number Must not contain letters or symbols'), ('07515111*11', 'phone_number Must not contain letters or symbols'),
('notaphoneumber', 'phone_number Must not contain letters or symbols')]) ('notaphoneumber', 'phone_number Must not contain letters or symbols')])
def test_post_sms_request_invalid_phone_number(invalid_phone_number, err_msg): def test_post_sms_request_schema_invalid_phone_number(invalid_phone_number, err_msg):
j = {"phone_number": invalid_phone_number, j = {"phone_number": invalid_phone_number,
"template_id": str(uuid.uuid4()) "template_id": str(uuid.uuid4())
} }
with pytest.raises(ValidationError) as e: with pytest.raises(ValidationError) as e:
validate(j, post_sms_request) validate(j, post_sms_request_schema)
errors = json.loads(e.value.message).get('errors') errors = json.loads(e.value.message).get('errors')
assert len(errors) == 1 assert len(errors) == 1
assert {"error": "ValidationError", "message": err_msg} == errors[0] assert {"error": "ValidationError", "message": err_msg} == errors[0]
def test_post_sms_request_invalid_phone_number_and_missing_template(): def test_post_sms_request_schema_invalid_phone_number_and_missing_template():
j = {"phone_number": '08515111111', j = {"phone_number": '08515111111',
} }
with pytest.raises(ValidationError) as e: with pytest.raises(ValidationError) as e:
validate(j, post_sms_request) validate(j, post_sms_request_schema)
errors = json.loads(e.value.message).get('errors') errors = json.loads(e.value.message).get('errors')
assert len(errors) == 2 assert len(errors) == 2
assert {"error": "ValidationError", "message": "phone_number Not a UK mobile number"} in errors assert {"error": "ValidationError", "message": "phone_number Not a UK mobile number"} in errors
assert {"error": "ValidationError", "message": "template_id is a required property"} in errors assert {"error": "ValidationError", "message": "template_id is a required property"} in errors
valid_response = { def valid_sms_response():
"id": str(uuid.uuid4()), return {
"content": {"body": "contents of message", "id": str(uuid.uuid4()),
"from_number": "46045"}, "content": {"body": "contents of message",
"uri": "/v2/notifications/id", "from_number": "46045"},
"template": {"id": str(uuid.uuid4()), "uri": "http://notify.api/v2/notifications/id",
"version": 1, "template": {
"uri": "/v2/template/id"} "id": str(uuid.uuid4()),
} "version": 1,
"uri": "http://notify.api/v2/template/id"
valid_response_with_optionals = { }
"id": str(uuid.uuid4()), }
"reference": "reference_from_service",
"content": {"body": "contents of message",
"from_number": "46045"},
"uri": "/v2/notifications/id",
"template": {"id": str(uuid.uuid4()),
"version": 1,
"uri": "/v2/template/id"}
}
@pytest.mark.parametrize('input', [valid_response]) def valid_sms_response_with_optionals():
def test_post_sms_response_schema_is_valid(input): return {
assert validate(input, post_sms_response) == input "id": str(uuid.uuid4()),
"reference": "reference_from_service",
"content": {"body": "contents of message",
"from_number": "46045"},
"uri": "http://notify.api/v2/notifications/id",
"template": {
"id": str(uuid.uuid4()),
"version": 1,
"uri": "http://notify.api/v2/template/id"
}
}
def test_post_sms_response_schema_missing_uri(): @pytest.mark.parametrize('input', [valid_sms_response()])
j = valid_response def test_post_sms_response_schema_schema_is_valid(input):
del j["uri"] assert validate(input, post_sms_response_schema) == input
with pytest.raises(ValidationError) as e:
validate(j, post_sms_response)
error = json.loads(e.value.message)
assert error['status_code'] == 400
assert error['errors'] == [{'error': 'ValidationError',
'message': "uri is a required property"}]
valid_post_email_json = {"email_address": "test@example.gov.uk", valid_post_email_json = {"email_address": "test@example.gov.uk",
@@ -203,43 +203,109 @@ valid_post_email_json_with_optionals = {
@pytest.mark.parametrize("input", [valid_post_email_json, valid_post_email_json_with_optionals]) @pytest.mark.parametrize("input", [valid_post_email_json, valid_post_email_json_with_optionals])
def test_post_email_schema_is_valid(input): def test_post_email_schema_is_valid(input):
assert validate(input, post_email_request) == input assert validate(input, post_email_request_schema) == input
def test_post_email_schema_bad_uuid_and_missing_email_address(): def test_post_email_schema_bad_uuid_and_missing_email_address():
j = {"template_id": "bad_template"} j = {"template_id": "bad_template"}
with pytest.raises(ValidationError): with pytest.raises(ValidationError):
validate(j, post_email_request) validate(j, post_email_request_schema)
def test_post_email_schema_invalid_email_address(): def test_post_email_schema_invalid_email_address():
j = {"template_id": str(uuid.uuid4()), j = {"template_id": str(uuid.uuid4()),
"email_address": "notavalidemail@address"} "email_address": "notavalidemail@address"}
with pytest.raises(ValidationError): with pytest.raises(ValidationError):
validate(j, post_email_request) validate(j, post_email_request_schema)
valid_email_response = {"id": str(uuid.uuid4()), def valid_email_response():
"content": {"body": "the body of the message", return {
"subject": "subject of the message", "id": str(uuid.uuid4()),
"from_email": "service@dig.gov.uk"}, "content": {"body": "the body of the message",
"uri": "/v2/notifications/id", "subject": "subject of the message",
"template": {"id": str(uuid.uuid4()), "from_email": "service@dig.gov.uk"},
"version": 1, "uri": "http://notify.api/v2/notifications/id",
"uri": "/v2/template/id"} "template": {
} "id": str(uuid.uuid4()),
valid_email_response_with_optionals = {"id": str(uuid.uuid4()), "version": 1,
"reference": "some reference", "uri": "http://notify.api/v2/template/id"
"content": {"body": "the body of the message", }
"subject": "subject of the message", }
"from_email": "service@dig.gov.uk"},
"uri": "/v2/notifications/id",
"template": {"id": str(uuid.uuid4()),
"version": 1,
"uri": "/v2/template/id"}
}
@pytest.mark.parametrize("input", [valid_email_response, valid_email_response_with_optionals]) def valid_email_response_with_optionals():
def test_post_email_response(input): return {
assert validate(input, post_email_response) == input "id": str(uuid.uuid4()),
"reference": "some reference",
"content": {"body": "the body of the message",
"subject": "subject of the message",
"from_email": "service@dig.gov.uk"},
"uri": "http://notify.api/v2/notifications/id",
"template": {
"id": str(uuid.uuid4()),
"version": 1,
"uri": "http://notify.api/v2/template/id"
}
}
@pytest.mark.parametrize("input", [valid_email_response(), valid_email_response_with_optionals()])
def test_post_email_response_schema(input):
assert validate(input, post_email_response_schema) == input
@pytest.mark.parametrize('response, schema', [
(valid_email_response(), post_email_response_schema),
(valid_sms_response(), post_sms_response_schema)
])
def test_post_sms_response_schema_missing_uri_raises_validation_error(response, schema):
del response['uri']
with pytest.raises(ValidationError) as e:
validate(response, schema)
error = json.loads(e.value.message)
assert error['status_code'] == 400
assert error['errors'] == [{'error': 'ValidationError',
'message': "uri is a required property"}]
@pytest.mark.parametrize('response, schema', [
(valid_email_response(), post_email_response_schema),
(valid_sms_response(), post_sms_response_schema)
])
def test_post_sms_response_schema_invalid_uri_raises_validation_error(response, schema):
response['uri'] = 'invalid-uri'
with pytest.raises(ValidationError) as e:
validate(response, schema)
error = json.loads(e.value.message)
assert error['status_code'] == 400
assert error['errors'] == [{'error': 'ValidationError',
'message': "uri invalid-uri is not a uri"}]
@pytest.mark.parametrize('response, schema', [
(valid_email_response(), post_email_response_schema),
(valid_sms_response(), post_sms_response_schema)
])
def test_post_sms_response_schema_missing_template_uri_raises_validation_error(response, schema):
del response['template']['uri']
with pytest.raises(ValidationError) as e:
validate(response, schema)
error = json.loads(e.value.message)
assert error['status_code'] == 400
assert error['errors'] == [{'error': 'ValidationError',
'message': "template uri is a required property"}]
@pytest.mark.parametrize('response, schema', [
(valid_email_response(), post_email_response_schema),
(valid_sms_response(), post_sms_response_schema)
])
def test_post_sms_response_schema_invalid_template_uri_raises_validation_error(response, schema):
response['template']['uri'] = 'invalid-uri'
with pytest.raises(ValidationError) as e:
validate(response, schema)
error = json.loads(e.value.message)
assert error['status_code'] == 400
assert error['errors'] == [{'error': 'ValidationError',
'message': "template invalid-uri is not a uri"}]