Check that the request payload data is valid json.

By adding `force=True` to request.get_json() the mime type is ignore. If the data is not valid json the method will return a `BadRequestError` we catch that and throw our own error with a clear error message "Invalid JSON supplied in POST data".
If the json is valid return the json data or an empty dict if None is passed in.

This PR improves the error messages if the json is invalid, previously, the error message was "None object type" message which is not very helpful.
This commit is contained in:
Rebecca Law
2019-11-21 15:21:18 +00:00
parent c78a5d8536
commit 5d6886242b
6 changed files with 60 additions and 27 deletions

View File

@@ -7,8 +7,6 @@ from jsonschema import (Draft7Validator, ValidationError, FormatChecker)
from notifications_utils.recipients import (validate_phone_number, validate_email_address, InvalidPhoneError, from notifications_utils.recipients import (validate_phone_number, validate_email_address, InvalidPhoneError,
InvalidEmailError) InvalidEmailError)
from app.v2.errors import BadRequestError
format_checker = FormatChecker() format_checker = FormatChecker()
@@ -57,9 +55,6 @@ def validate_schema_date_with_hour(instance):
def validate(json_to_validate, schema): def validate(json_to_validate, schema):
if json_to_validate is None:
raise BadRequestError(message="Request body is empty.",
status_code=400)
validator = Draft7Validator(schema, format_checker=format_checker) validator = Draft7Validator(schema, format_checker=format_checker)
errors = list(validator.iter_errors(json_to_validate)) errors = list(validator.iter_errors(json_to_validate))
if errors.__len__() > 0: if errors.__len__() > 0:

View File

@@ -3,7 +3,6 @@ import functools
from flask import request, jsonify, current_app, abort from flask import request, jsonify, current_app, abort
from notifications_utils.recipients import try_validate_and_format_phone_number from notifications_utils.recipients import try_validate_and_format_phone_number
from werkzeug.exceptions import BadRequest
from app import api_user, authenticated_service, notify_celery, document_download_client from app import api_user, authenticated_service, notify_celery, document_download_client
from app.celery.letters_pdf_tasks import create_letters_pdf, process_virus_scan_passed from app.celery.letters_pdf_tasks import create_letters_pdf, process_virus_scan_passed
@@ -58,6 +57,7 @@ from app.v2.notifications.notification_schemas import (
post_letter_request, post_letter_request,
post_precompiled_letter_request post_precompiled_letter_request
) )
from app.v2.utils import get_valid_json
@v2_notification_blueprint.route('/{}'.format(LETTER_TYPE), methods=['POST']) @v2_notification_blueprint.route('/{}'.format(LETTER_TYPE), methods=['POST'])
@@ -173,15 +173,6 @@ def post_notification(notification_type):
return jsonify(resp), 201 return jsonify(resp), 201
def get_valid_json():
try:
request_json = request.get_json(force=True)
except BadRequest:
raise BadRequestError(message="Invalid JSON supplied in POST data",
status_code=400)
return request_json
def process_sms_or_email_notification(*, form, notification_type, api_key, template, service, reply_to_text=None): def process_sms_or_email_notification(*, form, notification_type, api_key, template, service, reply_to_text=None):
form_send_to = form['email_address'] if notification_type == EMAIL_TYPE else form['phone_number'] form_send_to = form['email_address'] if notification_type == EMAIL_TYPE else form['phone_number']

View File

@@ -6,17 +6,21 @@ from app.schema_validation import validate
from app.utils import get_template_instance from app.utils import get_template_instance
from app.v2.errors import BadRequestError from app.v2.errors import BadRequestError
from app.v2.template import v2_template_blueprint from app.v2.template import v2_template_blueprint
from app.v2.template.template_schemas import post_template_preview_request, create_post_template_preview_response from app.v2.template.template_schemas import (
post_template_preview_request,
create_post_template_preview_response
)
from app.v2.utils import get_valid_json
@v2_template_blueprint.route("/<template_id>/preview", methods=['POST']) @v2_template_blueprint.route("/<template_id>/preview", methods=['POST'])
def post_template_preview(template_id): def post_template_preview(template_id):
if not request.content_type or request.content_type != 'application/json': # If the payload is is empty if there is no personalisation in the template.
raise BadRequestError(message="Content-Type header is not set to application/json.", _data = request.get_data(as_text=True)
status_code=400) if not _data:
_data = request.get_json()
if _data is None:
_data = {} _data = {}
else:
_data = get_valid_json()
_data['id'] = template_id _data['id'] = template_id

13
app/v2/utils.py Normal file
View File

@@ -0,0 +1,13 @@
from flask import request
from app.v2.errors import BadRequestError
from werkzeug.exceptions import BadRequest
def get_valid_json():
try:
request_json = request.get_json(force=True)
except BadRequest:
raise BadRequestError(message="Invalid JSON supplied in POST data",
status_code=400)
return request_json or {}

View File

@@ -931,4 +931,7 @@ def test_post_email_notification_when_data_is_empty_returns_400(
) )
error_msg = json.loads(response.get_data(as_text=True))["errors"][0]["message"] error_msg = json.loads(response.get_data(as_text=True))["errors"][0]["message"]
assert response.status_code == 400 assert response.status_code == 400
assert error_msg == 'Request body is empty.' if notification_type == 'sms':
assert error_msg == 'phone_number is a required property'
else:
assert error_msg == 'email_address is a required property'

View File

@@ -154,13 +154,40 @@ def test_post_template_with_non_existent_template_id_returns_404(client, fake_uu
} }
def test_post_template_without_content_header(client, sample_template): def test_post_template_returns_200_without_personalisation(client, sample_template):
response = client.post(
path='/v2/template/{}/preview'.format(sample_template.id),
data=None,
headers=[('Content-Type', 'application/json'),
create_authorization_header(service_id=sample_template.service_id)]
)
assert response.status_code == 200
def test_post_template_returns_200_without_personalisation_and_missing_content_header(client, sample_template):
response = client.post(
path='/v2/template/{}/preview'.format(sample_template.id),
data=None,
headers=[create_authorization_header(service_id=sample_template.service_id)]
)
assert response.status_code == 200
def test_post_template_returns_200_without_personalisation_as_valid_json_and_missing_content_header(
client, sample_template
):
response = client.post(
path='/v2/template/{}/preview'.format(sample_template.id),
data=json.dumps(None),
headers=[create_authorization_header(service_id=sample_template.service_id)]
)
assert response.status_code == 200
def test_post_template_returns_200_with_valid_json_and_missing_content_header(client, sample_template):
response = client.post( response = client.post(
path='/v2/template/{}/preview'.format(sample_template.id), path='/v2/template/{}/preview'.format(sample_template.id),
data=json.dumps(valid_personalisation), data=json.dumps(valid_personalisation),
headers=[create_authorization_header(service_id=sample_template.service_id)] headers=[create_authorization_header(service_id=sample_template.service_id)]
) )
assert response.status_code == 200
assert response.status_code == 400
json_response = json.loads(response.get_data(as_text=True))
assert json_response['errors'][0]['message'] == 'Content-Type header is not set to application/json.'