Merge pull request #482 from alphagov/ris-api-error-handlers

add common error handlers for "api" flask apps

dmutils/__init__.py

@@ -2,4 +2,4 @@
 from .flask_init import init_app, init_manager
 
 
-__version__ = '45.1.1'
+__version__ = '45.2.0'

dmutils/authentication.py

@@ -0,0 +1,25 @@
+from werkzeug.exceptions import Unauthorized
+
+
+class UnauthorizedWWWAuthenticate(Unauthorized):
+    """
+    This is a near verbatim copy of an improvement to an as-yet-unreleased upstream version of werkzeug that allows
+    us to specify a www_authenticate argument containing the contents of that field. We should get rid of this once
+    we're able to upgrade past that version.
+
+    From werkzeug 8ed5b3f9a285eca756c3ab33f8c370a88eab3842
+    """
+    def __init__(self, www_authenticate=None, description=None):
+        super().__init__(description=description)
+        if not isinstance(www_authenticate, (tuple, list)):
+            www_authenticate = (www_authenticate,)
+        self.www_authenticate = www_authenticate
+
+    def get_headers(self, environ=None):
+        headers = super().get_headers(environ)
+        if self.www_authenticate:
+            headers.append((
+                'WWW-Authenticate',
+                ', '.join([str(x) for x in self.www_authenticate])
+            ))
+        return headers

dmutils/errors/__init__.py

@@ -0,0 +1,2 @@
+# the contents of .frontend are exposed as the top-level module for backwards compatibility
+from dmutils.errors.frontend import *  # noqa

dmutils/errors/api.py

@@ -0,0 +1,29 @@
+import json
+
+from flask import current_app, jsonify
+from werkzeug.exceptions import BadRequest
+
+
+class ValidationError(ValueError):
+    @property
+    def message(self):
+        return self.args[0]
+
+
+def json_error_handler(e):
+    try:
+        # initially we'll try and assume this is an HTTPException of some sort.  for the most part, the default
+        # HTTPExceptions render themselves in the desired way if returned as a response. the only change we want to
+        # make is to enclose the error description in json.
+        response = e.get_response()
+        response.set_data(json.dumps({"error": e.description}))
+        response.mimetype = current_app.config["JSONIFY_MIMETYPE"]
+
+        return response
+    except Exception:
+        # either `e` wasn't an HTTPException or something went wrong in trying to jsonify it
+        return jsonify(error="Internal error"), 500
+
+
+def validation_error_handler(validation_error):
+    return json_error_handler(BadRequest(description=validation_error.message))

dmutils/flask_init.py

@@ -1,7 +1,33 @@
+from collections import OrderedDict
 import os
-from . import config, logging, proxy_fix, request_id, formats, filters, errors
+from types import MappingProxyType
+
+from dmutils import config, logging, proxy_fix, request_id, formats, filters
+from dmutils.errors import api as api_errors, frontend as fe_errors
 from flask_script import Manager, Server
 from flask_wtf.csrf import CSRFError
+from werkzeug.exceptions import default_exceptions
+
+
+frontend_error_handlers = MappingProxyType(OrderedDict((
+    (CSRFError, fe_errors.csrf_handler,),
+    (400, fe_errors.render_error_page,),
+    (401, fe_errors.redirect_to_login,),
+    (403, fe_errors.redirect_to_login,),
+    (404, fe_errors.render_error_page,),
+    (410, fe_errors.render_error_page,),
+    (503, fe_errors.render_error_page,),
+    (500, fe_errors.render_error_page,),
+)))
+
+
+api_error_handlers = MappingProxyType(OrderedDict(
+    (
+        (api_errors.ValidationError, api_errors.validation_error_handler,),
+    ) + tuple(
+        (code, api_errors.json_error_handler) for code in default_exceptions
+    ),
+))
 
 
 def init_app(
@@ -12,6 +38,7 @@ def init_app(
         db=None,
         login_manager=None,
         search_api_client=None,
+        error_handlers=frontend_error_handlers,
 ):
 
     application.config.from_object(config_object)
@@ -61,15 +88,8 @@ def inject_global_template_variables():
             pluralize=pluralize,
             **(application.config['BASE_TEMPLATE_DATA'] or {}))
 
-    # Register error handlers for CSRF errors and common error status codes
-    application.register_error_handler(CSRFError, errors.csrf_handler)
-    application.register_error_handler(400, errors.render_error_page)
-    application.register_error_handler(401, errors.redirect_to_login)
-    application.register_error_handler(403, errors.redirect_to_login)
-    application.register_error_handler(404, errors.render_error_page)
-    application.register_error_handler(410, errors.render_error_page)
-    application.register_error_handler(503, errors.render_error_page)
-    application.register_error_handler(500, errors.render_error_page)
+    for exc_or_code, handler in error_handlers.items():
+        application.register_error_handler(exc_or_code, handler)
 
 
 def pluralize(count, singular, plural):

tests/test_errors.py

@@ -1,3 +1,4 @@
+import json
 import mock
 import pytest
 
@@ -7,12 +8,15 @@
     BadRequest, Forbidden, NotFound, Gone, InternalServerError, ServiceUnavailable, ImATeapot
 )
 from jinja2.exceptions import TemplateNotFound
-from dmutils.errors import csrf_handler, redirect_to_login, render_error_page
+
+from dmutils.authentication import UnauthorizedWWWAuthenticate
+from dmutils.errors.frontend import csrf_handler, redirect_to_login, render_error_page
+from dmutils.errors.api import json_error_handler, validation_error_handler, ValidationError
 from dmutils.external import external as external_blueprint
 
 
 @pytest.mark.parametrize('user_session', (True, False))
-@mock.patch('dmutils.errors.current_app')
+@mock.patch('dmutils.errors.frontend.current_app')
 def test_csrf_handler_redirects_to_login(current_app, user_session, app):
 
     with app.test_request_context('/'):
@@ -56,7 +60,7 @@ def test_unauthorised_redirects_to_login(app):
     (ServiceUnavailable, 503, 'errors/500.html'),
     (mock.Mock(code=None), 500, 'errors/500.html'),
 ])
-@mock.patch('dmutils.errors.render_template')
+@mock.patch('dmutils.errors.frontend.render_template')
 def test_render_error_page_with_exception(render_template, exception, status_code, expected_template, app):
     with app.test_request_context('/'):
         assert render_error_page(exception()) == (render_template.return_value, status_code)
@@ -72,14 +76,14 @@ def test_render_error_page_with_exception(render_template, exception, status_cod
     (500, 'errors/500.html'),
     (503, 'errors/500.html'),
 ])
-@mock.patch('dmutils.errors.render_template')
+@mock.patch('dmutils.errors.frontend.render_template')
 def test_render_error_page_with_status_code(render_template, status_code, expected_template, app):
     with app.test_request_context('/'):
         assert render_error_page(status_code=status_code) == (render_template.return_value, status_code)
         assert render_template.call_args_list == [mock.call(expected_template, error_message=None)]
 
 
-@mock.patch('dmutils.errors.render_template')
+@mock.patch('dmutils.errors.frontend.render_template')
 def test_render_error_page_with_custom_http_exception(render_template, app):
     class CustomHTTPError(Exception):
         def __init__(self):
@@ -90,14 +94,14 @@ def __init__(self):
         assert render_template.call_args_list == [mock.call('errors/500.html', error_message=None)]
 
 
-@mock.patch('dmutils.errors.render_template')
+@mock.patch('dmutils.errors.frontend.render_template')
 def test_render_error_page_for_unknown_status_code_defaults_to_500(render_template, app):
     with app.test_request_context('/'):
         assert render_error_page(ImATeapot()) == (render_template.return_value, 500)
         assert render_template.call_args_list == [mock.call('errors/500.html', error_message=None)]
 
 
-@mock.patch('dmutils.errors.render_template')
+@mock.patch('dmutils.errors.frontend.render_template')
 def test_render_error_page_falls_back_to_toolkit_templates(render_template, app):
     render_template.side_effect = [TemplateNotFound('Oh dear'), "successful rendering"]
     with app.test_request_context('/'):
@@ -108,7 +112,7 @@ def test_render_error_page_falls_back_to_toolkit_templates(render_template, app)
         ]
 
 
-@mock.patch('dmutils.errors.render_template')
+@mock.patch('dmutils.errors.frontend.render_template')
 def test_render_error_page_passes_error_message_as_context(render_template, app):
     render_template.side_effect = [TemplateNotFound('Oh dear'), "successful rendering"]
     with app.test_request_context('/'):
@@ -117,3 +121,40 @@ def test_render_error_page_passes_error_message_as_context(render_template, app)
             mock.call('errors/500.html', error_message="Hole in Teapot"),
             mock.call('toolkit/errors/500.html', error_message="Hole in Teapot")
         ]
+
+
+def test_api_json_error_handler(app):
+    with app.test_request_context('/'):
+        try:
+            raise ImATeapot("Simply teapot all over me!")
+        except ImATeapot as e:
+            response = json_error_handler(e)
+            assert json.loads(response.get_data()) == {
+                "error": "Simply teapot all over me!",
+            }
+            assert response.status_code == 418
+
+
+def test_api_validation_error_handler(app):
+    with app.test_request_context('/'):
+        try:
+            raise ValidationError("Hippogriff")
+        except ValidationError as e:
+            response = validation_error_handler(e)
+            assert json.loads(response.get_data()) == {
+                "error": "Hippogriff",
+            }
+            assert response.status_code == 400
+
+
+def test_api_unauth(app):
+    with app.test_request_context('/'):
+        try:
+            raise UnauthorizedWWWAuthenticate(www_authenticate="lemur", description="Bogeyman's trick")
+        except UnauthorizedWWWAuthenticate as e:
+            response = json_error_handler(e)
+            assert json.loads(response.get_data()) == {
+                "error": "Bogeyman's trick",
+            }
+            assert response.status_code == 401
+            assert response.headers["www-authenticate"] == "lemur"