Browse Source

Refs #23004 -- Allowed exception reporter filters to customize settings filtering.

Thanks to Tim Graham for the original implementation idea.

Co-authored-by: Daniel Maxson <dmaxson@ccpgames.com>
Carlton Gibson 5 years ago
parent
commit
581ba5a948
4 changed files with 152 additions and 78 deletions
  1. 44 49
      django/views/debug.py
  2. 32 11
      docs/howto/error-reporting.txt
  3. 11 0
      docs/releases/3.1.txt
  4. 65 18
      tests/view_tests/tests/test_debug.py

+ 44 - 49
django/views/debug.py

@@ -25,10 +25,6 @@ DEBUG_ENGINE = Engine(
     libraries={'i18n': 'django.templatetags.i18n'},
 )
 
-HIDDEN_SETTINGS = _lazy_re_compile('API|TOKEN|KEY|SECRET|PASS|SIGNATURE', flags=re.IGNORECASE)
-
-CLEANSED_SUBSTITUTE = '********************'
-
 CURRENT_DIR = Path(__file__).parent
 
 
@@ -46,42 +42,6 @@ class CallableSettingWrapper:
         return repr(self._wrapped)
 
 
-def cleanse_setting(key, value):
-    """
-    Cleanse an individual setting key/value of sensitive content. If the value
-    is a dictionary, recursively cleanse the keys in that dictionary.
-    """
-    try:
-        if HIDDEN_SETTINGS.search(key):
-            cleansed = CLEANSED_SUBSTITUTE
-        else:
-            if isinstance(value, dict):
-                cleansed = {k: cleanse_setting(k, v) for k, v in value.items()}
-            else:
-                cleansed = value
-    except TypeError:
-        # If the key isn't regex-able, just return as-is.
-        cleansed = value
-
-    if callable(cleansed):
-        # For fixing #21345 and #23070
-        cleansed = CallableSettingWrapper(cleansed)
-
-    return cleansed
-
-
-def get_safe_settings():
-    """
-    Return a dictionary of the settings module with values of sensitive
-    settings replaced with stars (*********).
-    """
-    settings_dict = {}
-    for k in dir(settings):
-        if k.isupper():
-            settings_dict[k] = cleanse_setting(k, getattr(settings, k))
-    return settings_dict
-
-
 def technical_500_response(request, exc_type, exc_value, tb, status_code=500):
     """
     Create a technical server error response. The last three arguments are
@@ -128,6 +88,40 @@ class SafeExceptionReporterFilter(ExceptionReporterFilter):
     Use annotations made by the sensitive_post_parameters and
     sensitive_variables decorators to filter out sensitive information.
     """
+    cleansed_substitute = '********************'
+    hidden_settings = _lazy_re_compile('API|TOKEN|KEY|SECRET|PASS|SIGNATURE', flags=re.I)
+
+    def cleanse_setting(self, key, value):
+        """
+        Cleanse an individual setting key/value of sensitive content. If the
+        value is a dictionary, recursively cleanse the keys in that dictionary.
+        """
+        try:
+            if self.hidden_settings.search(key):
+                cleansed = self.cleansed_substitute
+            elif isinstance(value, dict):
+                cleansed = {k: self.cleanse_setting(k, v) for k, v in value.items()}
+            else:
+                cleansed = value
+        except TypeError:
+            # If the key isn't regex-able, just return as-is.
+            cleansed = value
+
+        if callable(cleansed):
+            cleansed = CallableSettingWrapper(cleansed)
+
+        return cleansed
+
+    def get_safe_settings(self):
+        """
+        Return a dictionary of the settings module with values of sensitive
+        settings replaced with stars (*********).
+        """
+        settings_dict = {}
+        for k in dir(settings):
+            if k.isupper():
+                settings_dict[k] = self.cleanse_setting(k, getattr(settings, k))
+        return settings_dict
 
     def is_active(self, request):
         """
@@ -149,7 +143,7 @@ class SafeExceptionReporterFilter(ExceptionReporterFilter):
             multivaluedict = multivaluedict.copy()
             for param in sensitive_post_parameters:
                 if param in multivaluedict:
-                    multivaluedict[param] = CLEANSED_SUBSTITUTE
+                    multivaluedict[param] = self.cleansed_substitute
         return multivaluedict
 
     def get_post_parameters(self, request):
@@ -166,13 +160,13 @@ class SafeExceptionReporterFilter(ExceptionReporterFilter):
                 if sensitive_post_parameters == '__ALL__':
                     # Cleanse all parameters.
                     for k in cleansed:
-                        cleansed[k] = CLEANSED_SUBSTITUTE
+                        cleansed[k] = self.cleansed_substitute
                     return cleansed
                 else:
                     # Cleanse only the specified parameters.
                     for param in sensitive_post_parameters:
                         if param in cleansed:
-                            cleansed[param] = CLEANSED_SUBSTITUTE
+                            cleansed[param] = self.cleansed_substitute
                     return cleansed
             else:
                 return request.POST
@@ -215,12 +209,12 @@ class SafeExceptionReporterFilter(ExceptionReporterFilter):
             if sensitive_variables == '__ALL__':
                 # Cleanse all variables
                 for name in tb_frame.f_locals:
-                    cleansed[name] = CLEANSED_SUBSTITUTE
+                    cleansed[name] = self.cleansed_substitute
             else:
                 # Cleanse specified variables
                 for name, value in tb_frame.f_locals.items():
                     if name in sensitive_variables:
-                        value = CLEANSED_SUBSTITUTE
+                        value = self.cleansed_substitute
                     else:
                         value = self.cleanse_special_types(request, value)
                     cleansed[name] = value
@@ -236,8 +230,8 @@ class SafeExceptionReporterFilter(ExceptionReporterFilter):
             # the sensitive_variables decorator's frame, in case the variables
             # associated with those arguments were meant to be obfuscated from
             # the decorated function's frame.
-            cleansed['func_args'] = CLEANSED_SUBSTITUTE
-            cleansed['func_kwargs'] = CLEANSED_SUBSTITUTE
+            cleansed['func_args'] = self.cleansed_substitute
+            cleansed['func_kwargs'] = self.cleansed_substitute
 
         return cleansed.items()
 
@@ -304,7 +298,7 @@ class ExceptionReporter:
             'request': self.request,
             'user_str': user_str,
             'filtered_POST_items': list(self.filter.get_post_parameters(self.request).items()),
-            'settings': get_safe_settings(),
+            'settings': self.filter.get_safe_settings(),
             'sys_executable': sys.executable,
             'sys_version_info': '%d.%d.%d' % sys.version_info[0:3],
             'server_time': timezone.now(),
@@ -506,6 +500,7 @@ def technical_404_response(request, exception):
 
     with Path(CURRENT_DIR, 'templates', 'technical_404.html').open(encoding='utf-8') as fh:
         t = DEBUG_ENGINE.from_string(fh.read())
+    reporter_filter = get_default_exception_reporter_filter()
     c = Context({
         'urlconf': urlconf,
         'root_urlconf': settings.ROOT_URLCONF,
@@ -513,7 +508,7 @@ def technical_404_response(request, exception):
         'urlpatterns': tried,
         'reason': str(exception),
         'request': request,
-        'settings': get_safe_settings(),
+        'settings': reporter_filter.get_safe_settings(),
         'raising_view_name': caller,
     })
     return HttpResponseNotFound(t.render(c), content_type='text/html')

+ 32 - 11
docs/howto/error-reporting.txt

@@ -262,25 +262,46 @@ attribute::
 
 Your custom filter class needs to inherit from
 :class:`django.views.debug.SafeExceptionReporterFilter` and may override the
-following methods:
+following attributes and methods:
 
 .. class:: SafeExceptionReporterFilter
 
-.. method:: SafeExceptionReporterFilter.is_active(request)
+    .. attribute:: cleansed_substitute
 
-    Returns ``True`` to activate the filtering operated in the other methods.
-    By default the filter is active if :setting:`DEBUG` is ``False``.
+        .. versionadded:: 3.1
 
-.. method:: SafeExceptionReporterFilter.get_post_parameters(request)
+        The string value to replace sensitive value with. By default it
+        replaces the values of sensitive variables with stars (`**********`).
 
-    Returns the filtered dictionary of POST parameters. By default it replaces
-    the values of sensitive parameters with stars (`**********`).
+    .. attribute:: hidden_settings
 
-.. method:: SafeExceptionReporterFilter.get_traceback_frame_variables(request, tb_frame)
+        .. versionadded:: 3.1
 
-    Returns the filtered dictionary of local variables for the given traceback
-    frame. By default it replaces the values of sensitive variables with stars
-    (`**********`).
+        A compiled regular expression object used to match settings considered
+        as sensitive. By default equivalent to::
+
+            import re
+
+            re.compile(r'API|TOKEN|KEY|SECRET|PASS|SIGNATURE', flags=re.IGNORECASE)
+
+    .. method:: is_active(request)
+
+        Returns ``True`` to activate the filtering in
+        :meth:`get_post_parameters` and :meth:`get_traceback_frame_variables`.
+        By default the filter is active if :setting:`DEBUG` is ``False``. Note
+        that sensitive settings are always filtered, as described in the
+        :setting:`DEBUG` documentation.
+
+    .. method:: get_post_parameters(request)
+
+        Returns the filtered dictionary of POST parameters. Sensitive values
+        are replaced with :attr:`cleansed_substitute`.
+
+    .. method:: get_traceback_frame_variables(request, tb_frame)
+
+        Returns the filtered dictionary of local variables for the given
+        traceback frame. Sensitive values are replaced with
+        :attr:`cleansed_substitute`.
 
 .. seealso::
 

+ 11 - 0
docs/releases/3.1.txt

@@ -158,6 +158,17 @@ Email
 * The :setting:`EMAIL_FILE_PATH` setting, used by the :ref:`file email backend
   <topic-email-file-backend>`, now supports :class:`pathlib.Path`.
 
+Error Reporting
+~~~~~~~~~~~~~~~
+
+* The new :attr:`.SafeExceptionReporterFilter.cleansed_substitute` and
+  :attr:`.SafeExceptionReporterFilter.hidden_settings` attributes allow
+  customization of sensitive settings filtering in exception reports.
+
+* The technical 404 debug view now respects
+  :setting:`DEFAULT_EXCEPTION_REPORTER_FILTER` when applying settings
+  filtering.
+
 File Storage
 ~~~~~~~~~~~~
 

+ 65 - 18
tests/view_tests/tests/test_debug.py

@@ -20,11 +20,13 @@ from django.test.utils import LoggingCaptureMixin
 from django.urls import path, reverse
 from django.urls.converters import IntConverter
 from django.utils.functional import SimpleLazyObject
+from django.utils.regex_helper import _lazy_re_compile
 from django.utils.safestring import mark_safe
 from django.views.debug import (
-    CLEANSED_SUBSTITUTE, CallableSettingWrapper, ExceptionReporter,
-    Path as DebugPath, cleanse_setting, default_urlconf,
-    technical_404_response, technical_500_response,
+    CallableSettingWrapper, ExceptionReporter, Path as DebugPath,
+    SafeExceptionReporterFilter, default_urlconf,
+    get_default_exception_reporter_filter, technical_404_response,
+    technical_500_response,
 )
 from django.views.decorators.debug import (
     sensitive_post_parameters, sensitive_variables,
@@ -1199,6 +1201,66 @@ class ExceptionReporterFilterTests(ExceptionReportTestMixin, LoggingCaptureMixin
                 response = self.client.get('/raises500/')
                 self.assertNotContains(response, 'should not be displayed', status_code=500)
 
+    def test_cleanse_setting_basic(self):
+        reporter_filter = SafeExceptionReporterFilter()
+        self.assertEqual(reporter_filter.cleanse_setting('TEST', 'TEST'), 'TEST')
+        self.assertEqual(
+            reporter_filter.cleanse_setting('PASSWORD', 'super_secret'),
+            reporter_filter.cleansed_substitute,
+        )
+
+    def test_cleanse_setting_ignore_case(self):
+        reporter_filter = SafeExceptionReporterFilter()
+        self.assertEqual(
+            reporter_filter.cleanse_setting('password', 'super_secret'),
+            reporter_filter.cleansed_substitute,
+        )
+
+    def test_cleanse_setting_recurses_in_dictionary(self):
+        reporter_filter = SafeExceptionReporterFilter()
+        initial = {'login': 'cooper', 'password': 'secret'}
+        self.assertEqual(
+            reporter_filter.cleanse_setting('SETTING_NAME', initial),
+            {'login': 'cooper', 'password': reporter_filter.cleansed_substitute},
+        )
+
+
+class CustomExceptionReporterFilter(SafeExceptionReporterFilter):
+    cleansed_substitute = 'XXXXXXXXXXXXXXXXXXXX'
+    hidden_settings = _lazy_re_compile('API|TOKEN|KEY|SECRET|PASS|SIGNATURE|DATABASE_URL', flags=re.I)
+
+
+@override_settings(
+    ROOT_URLCONF='view_tests.urls',
+    DEFAULT_EXCEPTION_REPORTER_FILTER='%s.CustomExceptionReporterFilter' % __name__,
+)
+class CustomExceptionReporterFilterTests(SimpleTestCase):
+    def setUp(self):
+        get_default_exception_reporter_filter.cache_clear()
+
+    def tearDown(self):
+        get_default_exception_reporter_filter.cache_clear()
+
+    def test_setting_allows_custom_subclass(self):
+        self.assertIsInstance(
+            get_default_exception_reporter_filter(),
+            CustomExceptionReporterFilter,
+        )
+
+    def test_cleansed_substitute_override(self):
+        reporter_filter = get_default_exception_reporter_filter()
+        self.assertEqual(
+            reporter_filter.cleanse_setting('password', 'super_secret'),
+            reporter_filter.cleansed_substitute,
+        )
+
+    def test_hidden_settings_override(self):
+        reporter_filter = get_default_exception_reporter_filter()
+        self.assertEqual(
+            reporter_filter.cleanse_setting('database_url', 'super_secret'),
+            reporter_filter.cleansed_substitute,
+        )
+
 
 class AjaxResponseExceptionReporterFilter(ExceptionReportTestMixin, LoggingCaptureMixin, SimpleTestCase):
     """
@@ -1262,21 +1324,6 @@ class AjaxResponseExceptionReporterFilter(ExceptionReportTestMixin, LoggingCaptu
         self.assertEqual(response['Content-Type'], 'text/plain; charset=utf-8')
 
 
-class HelperFunctionTests(SimpleTestCase):
-
-    def test_cleanse_setting_basic(self):
-        self.assertEqual(cleanse_setting('TEST', 'TEST'), 'TEST')
-        self.assertEqual(cleanse_setting('PASSWORD', 'super_secret'), CLEANSED_SUBSTITUTE)
-
-    def test_cleanse_setting_ignore_case(self):
-        self.assertEqual(cleanse_setting('password', 'super_secret'), CLEANSED_SUBSTITUTE)
-
-    def test_cleanse_setting_recurses_in_dictionary(self):
-        initial = {'login': 'cooper', 'password': 'secret'}
-        expected = {'login': 'cooper', 'password': CLEANSED_SUBSTITUTE}
-        self.assertEqual(cleanse_setting('SETTING_NAME', initial), expected)
-
-
 class DecoratorsTests(SimpleTestCase):
     def test_sensitive_variables_not_called(self):
         msg = (