From f2e2363a50e0fed04953ccfa09b60dafc4529948 Mon Sep 17 00:00:00 2001 From: Alice Heaton <aliceh@aptivate.org> Date: Wed, 29 Jul 2015 16:49:28 +0100 Subject: [PATCH] Allow widgets to display error messages in the front end by raising a WidgetError (while other exceptions only display a generic message so as to avoid potential security leak) --- .../dashboard/templatetags/render_widget.py | 6 +- .../website/dashboard/tests/widget_tests.py | 67 ++++++++++++++++++- django/website/dashboard/widget_pool.py | 11 +++ 3 files changed, 82 insertions(+), 2 deletions(-) diff --git a/django/website/dashboard/templatetags/render_widget.py b/django/website/dashboard/templatetags/render_widget.py index e9de818e..175ea4a9 100644 --- a/django/website/dashboard/templatetags/render_widget.py +++ b/django/website/dashboard/templatetags/render_widget.py @@ -4,7 +4,7 @@ from django import template from django.template.loader import render_to_string from django.utils.translation import ugettext_lazy as _ -from dashboard.widget_pool import get_widget, MissingWidgetType +from dashboard.widget_pool import get_widget, MissingWidgetType, WidgetError logger = logging.getLogger(__name__) @@ -53,6 +53,10 @@ def render_widget(widget_instance): context = widget.get_context_data(**settings) except AttributeError: context = {} + except WidgetError as e: + template_name = 'dashboard/widget-error.html' + context = {} + context['error'] = str(e) except Exception as e: logger.exception('Error while fetching widget context data: %s', e) template_name = 'dashboard/widget-error.html' diff --git a/django/website/dashboard/tests/widget_tests.py b/django/website/dashboard/tests/widget_tests.py index 2b69bc33..fed0612b 100644 --- a/django/website/dashboard/tests/widget_tests.py +++ b/django/website/dashboard/tests/widget_tests.py @@ -3,7 +3,7 @@ from mock import patch from django.test import TestCase from dashboard.templatetags.render_widget import render_widget -from dashboard.widget_pool import register_widget, get_widget +from dashboard.widget_pool import register_widget, get_widget, WidgetError class TestWidget(object): @@ -38,6 +38,26 @@ class TestWidgetNoTemplateName(object): return {} +class TestWidgetRaisesException(object): + """ A test widget which raises a generic exception in + get_context_data + """ + template_name = 'something.hmtl' + + def get_context_data(self, **kwargs): + raise Exception('message raised from get_context_data') + + +class TestWidgetRaisesWidgetError(object): + """ A test widget which raises a WidgetError in + get_context_data + """ + template_name = 'something.html' + + def get_context_data(self, **kwargs): + raise WidgetError('message raised from get_context_data') + + class MockWidgetInstance(object): """ A Mock class to represent a widget instance @@ -152,3 +172,48 @@ class WidgetPoolTestCase(TestCase): self.assertEqual( template_name, 'dashboard/widget-error.html' ) + + def test_render_widget_exception_includes_generic_message(self): + """ Test that a widget which raises a generic exception will + display a generic error message, not the content of the + exception + """ + test_widget = TestWidgetRaisesException() + register_widget('test-widget', test_widget) + widget_instance = MockWidgetInstance('test-widget') + with patch(self.render_to_string_method) as mock: + render_widget(widget_instance) + template_name = self.get_mock_render_to_string_parameter( + mock, 'template_name' + ) + context = self.get_mock_render_to_string_parameter( + mock, 'context' + ) + self.assertEqual( + template_name, 'dashboard/widget-error.html' + ) + self.assertEqual( + context['error'], 'Widget error. See error logs.' + ) + + def test_render_widget_widgeterror_exception_includes_error_message(self): + """ Test that a widget which raises a WidgetError exception will + display the error message provided in the exception. + """ + test_widget = TestWidgetRaisesWidgetError() + register_widget('test-widget', test_widget) + widget_instance = MockWidgetInstance('test-widget') + with patch(self.render_to_string_method) as mock: + render_widget(widget_instance) + template_name = self.get_mock_render_to_string_parameter( + mock, 'template_name' + ) + context = self.get_mock_render_to_string_parameter( + mock, 'context' + ) + self.assertEqual( + template_name, 'dashboard/widget-error.html' + ) + self.assertEqual( + str(context['error']), 'message raised from get_context_data' + ) diff --git a/django/website/dashboard/widget_pool.py b/django/website/dashboard/widget_pool.py index 98cc08bc..83bc4157 100644 --- a/django/website/dashboard/widget_pool.py +++ b/django/website/dashboard/widget_pool.py @@ -6,6 +6,17 @@ class MissingWidgetType(Exception): pass +class WidgetError(Exception): + """ Exception that can be raised from widget types + in get_context_data. + + The error message will be displayed to the end + user, so should not contain debug or sensisitve + information + """ + pass + + def register_widget(name, widget): """ Register a new widget type -- GitLab