From 38e391e95fe5258bc6d2467332dc9cd44ce6ba52 Mon Sep 17 00:00:00 2001 From: Jon Janzen Date: Sat, 6 May 2023 20:20:00 -0700 Subject: [PATCH] Refs #31949 -- Made @sensitive_variables/sensitive_post_parameters decorators to work with async functions. Co-authored-by: Mariusz Felisiak --- django/views/debug.py | 46 ++++++++---- django/views/decorators/debug.py | 90 +++++++++++++++++----- docs/howto/error-reporting.txt | 8 ++ docs/releases/5.0.txt | 6 +- tests/view_tests/tests/test_debug.py | 108 ++++++++++++++++++++++++++- tests/view_tests/views.py | 83 ++++++++++++++++++++ 6 files changed, 303 insertions(+), 38 deletions(-) diff --git a/django/views/debug.py b/django/views/debug.py index 53b4125716..c1265bfe6b 100644 --- a/django/views/debug.py +++ b/django/views/debug.py @@ -1,4 +1,5 @@ import functools +import inspect import itertools import re import sys @@ -17,6 +18,7 @@ from django.utils.encoding import force_str from django.utils.module_loading import import_string from django.utils.regex_helper import _lazy_re_compile from django.utils.version import PY311, get_docs_version +from django.views.decorators.debug import coroutine_functions_to_sensitive_variables # Minimal Django templates engine to render the error templates # regardless of the project's TEMPLATES setting. Templates are @@ -239,21 +241,37 @@ class SafeExceptionReporterFilter: Replace the values of variables marked as sensitive with stars (*********). """ - # Loop through the frame's callers to see if the sensitive_variables - # decorator was used. - current_frame = tb_frame.f_back sensitive_variables = None - while current_frame is not None: - if ( - current_frame.f_code.co_name == "sensitive_variables_wrapper" - and "sensitive_variables_wrapper" in current_frame.f_locals - ): - # The sensitive_variables decorator was used, so we take note - # of the sensitive variables' names. - wrapper = current_frame.f_locals["sensitive_variables_wrapper"] - sensitive_variables = getattr(wrapper, "sensitive_variables", None) - break - current_frame = current_frame.f_back + + # Coroutines don't have a proper `f_back` so they need to be inspected + # separately. Handle this by stashing the registered sensitive + # variables in a global dict indexed by `hash(file_path:line_number)`. + if ( + tb_frame.f_code.co_flags & inspect.CO_COROUTINE != 0 + and tb_frame.f_code.co_name != "sensitive_variables_wrapper" + ): + key = hash( + f"{tb_frame.f_code.co_filename}:{tb_frame.f_code.co_firstlineno}" + ) + sensitive_variables = coroutine_functions_to_sensitive_variables.get( + key, None + ) + + if sensitive_variables is None: + # Loop through the frame's callers to see if the + # sensitive_variables decorator was used. + current_frame = tb_frame + while current_frame is not None: + if ( + current_frame.f_code.co_name == "sensitive_variables_wrapper" + and "sensitive_variables_wrapper" in current_frame.f_locals + ): + # The sensitive_variables decorator was used, so take note + # of the sensitive variables' names. + wrapper = current_frame.f_locals["sensitive_variables_wrapper"] + sensitive_variables = getattr(wrapper, "sensitive_variables", None) + break + current_frame = current_frame.f_back cleansed = {} if self.is_active(request) and sensitive_variables: diff --git a/django/views/decorators/debug.py b/django/views/decorators/debug.py index 5578a4995f..2d5d1ebd83 100644 --- a/django/views/decorators/debug.py +++ b/django/views/decorators/debug.py @@ -1,7 +1,12 @@ +import inspect from functools import wraps +from asgiref.sync import iscoroutinefunction + from django.http import HttpRequest +coroutine_functions_to_sensitive_variables = {} + def sensitive_variables(*variables): """ @@ -33,13 +38,42 @@ def sensitive_variables(*variables): ) def decorator(func): - @wraps(func) - def sensitive_variables_wrapper(*func_args, **func_kwargs): - if variables: - sensitive_variables_wrapper.sensitive_variables = variables + if iscoroutinefunction(func): + + @wraps(func) + async def sensitive_variables_wrapper(*func_args, **func_kwargs): + return await func(*func_args, **func_kwargs) + + wrapped_func = func + while getattr(wrapped_func, "__wrapped__", None) is not None: + wrapped_func = wrapped_func.__wrapped__ + + try: + file_path = inspect.getfile(wrapped_func) + _, first_file_line = inspect.getsourcelines(wrapped_func) + except TypeError: # Raises for builtins or native functions. + raise ValueError( + f"{func.__name__} cannot safely be wrapped by " + "@sensitive_variables, make it either non-async or defined in a " + "Python file (not a builtin or from a native extension)." + ) else: - sensitive_variables_wrapper.sensitive_variables = "__ALL__" - return func(*func_args, **func_kwargs) + key = hash(f"{file_path}:{first_file_line}") + + if variables: + coroutine_functions_to_sensitive_variables[key] = variables + else: + coroutine_functions_to_sensitive_variables[key] = "__ALL__" + + else: + + @wraps(func) + def sensitive_variables_wrapper(*func_args, **func_kwargs): + if variables: + sensitive_variables_wrapper.sensitive_variables = variables + else: + sensitive_variables_wrapper.sensitive_variables = "__ALL__" + return func(*func_args, **func_kwargs) return sensitive_variables_wrapper @@ -77,19 +111,37 @@ def sensitive_post_parameters(*parameters): ) def decorator(view): - @wraps(view) - def sensitive_post_parameters_wrapper(request, *args, **kwargs): - if not isinstance(request, HttpRequest): - raise TypeError( - "sensitive_post_parameters didn't receive an HttpRequest " - "object. If you are decorating a classmethod, make sure " - "to use @method_decorator." - ) - if parameters: - request.sensitive_post_parameters = parameters - else: - request.sensitive_post_parameters = "__ALL__" - return view(request, *args, **kwargs) + if iscoroutinefunction(view): + + @wraps(view) + async def sensitive_post_parameters_wrapper(request, *args, **kwargs): + if not isinstance(request, HttpRequest): + raise TypeError( + "sensitive_post_parameters didn't receive an HttpRequest " + "object. If you are decorating a classmethod, make sure to use " + "@method_decorator." + ) + if parameters: + request.sensitive_post_parameters = parameters + else: + request.sensitive_post_parameters = "__ALL__" + return await view(request, *args, **kwargs) + + else: + + @wraps(view) + def sensitive_post_parameters_wrapper(request, *args, **kwargs): + if not isinstance(request, HttpRequest): + raise TypeError( + "sensitive_post_parameters didn't receive an HttpRequest " + "object. If you are decorating a classmethod, make sure to use " + "@method_decorator." + ) + if parameters: + request.sensitive_post_parameters = parameters + else: + request.sensitive_post_parameters = "__ALL__" + return view(request, *args, **kwargs) return sensitive_post_parameters_wrapper diff --git a/docs/howto/error-reporting.txt b/docs/howto/error-reporting.txt index fd465ecf6b..875e56a51d 100644 --- a/docs/howto/error-reporting.txt +++ b/docs/howto/error-reporting.txt @@ -205,6 +205,10 @@ filtered out of error reports in a production environment (that is, where exception reporting, and consider implementing a :ref:`custom filter ` if necessary. + .. versionchanged:: 5.0 + + Support for wrapping ``async`` functions was added. + .. function:: sensitive_post_parameters(*parameters) If one of your views receives an :class:`~django.http.HttpRequest` object @@ -245,6 +249,10 @@ filtered out of error reports in a production environment (that is, where ``user_change_password`` in the ``auth`` admin) to prevent the leaking of sensitive information such as user passwords. + .. versionchanged:: 5.0 + + Support for wrapping ``async`` functions was added. + .. _custom-error-reports: Custom error reports diff --git a/docs/releases/5.0.txt b/docs/releases/5.0.txt index 0184dad12c..588aa23ca1 100644 --- a/docs/releases/5.0.txt +++ b/docs/releases/5.0.txt @@ -241,6 +241,8 @@ Decorators * :func:`~django.views.decorators.cache.cache_control` * :func:`~django.views.decorators.cache.never_cache` * :func:`~django.views.decorators.common.no_append_slash` + * :func:`~django.views.decorators.debug.sensitive_variables` + * :func:`~django.views.decorators.debug.sensitive_post_parameters` * ``xframe_options_deny()`` * ``xframe_options_sameorigin()`` * ``xframe_options_exempt()`` @@ -253,7 +255,9 @@ Email Error Reporting ~~~~~~~~~~~~~~~ -* ... +* :func:`~django.views.decorators.debug.sensitive_variables` and + :func:`~django.views.decorators.debug.sensitive_post_parameters` can now be + used with asynchronous functions. File Storage ~~~~~~~~~~~~ diff --git a/tests/view_tests/tests/test_debug.py b/tests/view_tests/tests/test_debug.py index 15e69f6811..65f9db89bf 100644 --- a/tests/view_tests/tests/test_debug.py +++ b/tests/view_tests/tests/test_debug.py @@ -9,6 +9,8 @@ from io import StringIO from pathlib import Path from unittest import mock, skipIf, skipUnless +from asgiref.sync import async_to_sync, iscoroutinefunction + from django.core import mail from django.core.files.uploadedfile import SimpleUploadedFile from django.db import DatabaseError, connection @@ -39,6 +41,10 @@ from django.views.debug import ( from django.views.decorators.debug import sensitive_post_parameters, sensitive_variables from ..views import ( + async_sensitive_method_view, + async_sensitive_method_view_nested, + async_sensitive_view, + async_sensitive_view_nested, custom_exception_reporter_filter_view, index_page, multivalue_dict_key_error, @@ -1351,7 +1357,10 @@ class ExceptionReportTestMixin: Asserts that potentially sensitive info are displayed in the response. """ request = self.rf.post("/some_url/", self.breakfast_data) - response = view(request) + if iscoroutinefunction(view): + response = async_to_sync(view)(request) + else: + response = view(request) if check_for_vars: # All variables are shown. self.assertContains(response, "cooked_eggs", status_code=500) @@ -1371,7 +1380,10 @@ class ExceptionReportTestMixin: Asserts that certain sensitive info are not displayed in the response. """ request = self.rf.post("/some_url/", self.breakfast_data) - response = view(request) + if iscoroutinefunction(view): + response = async_to_sync(view)(request) + else: + response = view(request) if check_for_vars: # Non-sensitive variable's name and value are shown. self.assertContains(response, "cooked_eggs", status_code=500) @@ -1418,7 +1430,10 @@ class ExceptionReportTestMixin: with self.settings(ADMINS=[("Admin", "admin@fattie-breakie.com")]): mail.outbox = [] # Empty outbox request = self.rf.post("/some_url/", self.breakfast_data) - view(request) + if iscoroutinefunction(view): + async_to_sync(view)(request) + else: + view(request) self.assertEqual(len(mail.outbox), 1) email = mail.outbox[0] @@ -1451,7 +1466,10 @@ class ExceptionReportTestMixin: with self.settings(ADMINS=[("Admin", "admin@fattie-breakie.com")]): mail.outbox = [] # Empty outbox request = self.rf.post("/some_url/", self.breakfast_data) - view(request) + if iscoroutinefunction(view): + async_to_sync(view)(request) + else: + view(request) self.assertEqual(len(mail.outbox), 1) email = mail.outbox[0] @@ -1543,6 +1561,24 @@ class ExceptionReporterFilterTests( self.verify_safe_response(sensitive_view) self.verify_safe_email(sensitive_view) + def test_async_sensitive_request(self): + with self.settings(DEBUG=True): + self.verify_unsafe_response(async_sensitive_view) + self.verify_unsafe_email(async_sensitive_view) + + with self.settings(DEBUG=False): + self.verify_safe_response(async_sensitive_view) + self.verify_safe_email(async_sensitive_view) + + def test_async_sensitive_nested_request(self): + with self.settings(DEBUG=True): + self.verify_unsafe_response(async_sensitive_view_nested) + self.verify_unsafe_email(async_sensitive_view_nested) + + with self.settings(DEBUG=False): + self.verify_safe_response(async_sensitive_view_nested) + self.verify_safe_email(async_sensitive_view_nested) + def test_paranoid_request(self): """ No POST parameters and frame variables can be seen in the @@ -1598,6 +1634,46 @@ class ExceptionReporterFilterTests( ) self.verify_safe_email(sensitive_method_view, check_for_POST_params=False) + def test_async_sensitive_method(self): + """ + The sensitive_variables decorator works with async object methods. + """ + with self.settings(DEBUG=True): + self.verify_unsafe_response( + async_sensitive_method_view, check_for_POST_params=False + ) + self.verify_unsafe_email( + async_sensitive_method_view, check_for_POST_params=False + ) + + with self.settings(DEBUG=False): + self.verify_safe_response( + async_sensitive_method_view, check_for_POST_params=False + ) + self.verify_safe_email( + async_sensitive_method_view, check_for_POST_params=False + ) + + def test_async_sensitive_method_nested(self): + """ + The sensitive_variables decorator works with async object methods. + """ + with self.settings(DEBUG=True): + self.verify_unsafe_response( + async_sensitive_method_view_nested, check_for_POST_params=False + ) + self.verify_unsafe_email( + async_sensitive_method_view_nested, check_for_POST_params=False + ) + + with self.settings(DEBUG=False): + self.verify_safe_response( + async_sensitive_method_view_nested, check_for_POST_params=False + ) + self.verify_safe_email( + async_sensitive_method_view_nested, check_for_POST_params=False + ) + def test_sensitive_function_arguments(self): """ Sensitive variables don't leak in the sensitive_variables decorator's @@ -1890,6 +1966,30 @@ class NonHTMLResponseExceptionReporterFilter( with self.settings(DEBUG=False): self.verify_safe_response(sensitive_view, check_for_vars=False) + def test_async_sensitive_request(self): + """ + Sensitive POST parameters cannot be seen in the default + error reports for sensitive requests. + """ + with self.settings(DEBUG=True): + self.verify_unsafe_response(async_sensitive_view, check_for_vars=False) + + with self.settings(DEBUG=False): + self.verify_safe_response(async_sensitive_view, check_for_vars=False) + + def test_async_sensitive_request_nested(self): + """ + Sensitive POST parameters cannot be seen in the default + error reports for sensitive requests. + """ + with self.settings(DEBUG=True): + self.verify_unsafe_response( + async_sensitive_view_nested, check_for_vars=False + ) + + with self.settings(DEBUG=False): + self.verify_safe_response(async_sensitive_view_nested, check_for_vars=False) + def test_paranoid_request(self): """ No POST parameters can be seen in the default error reports diff --git a/tests/view_tests/views.py b/tests/view_tests/views.py index a9eeee3cd2..9eb7a352d6 100644 --- a/tests/view_tests/views.py +++ b/tests/view_tests/views.py @@ -178,6 +178,46 @@ def sensitive_view(request): return technical_500_response(request, *exc_info) +@sensitive_variables("sauce") +@sensitive_post_parameters("bacon-key", "sausage-key") +async def async_sensitive_view(request): + # Do not just use plain strings for the variables' values in the code so + # that the tests don't return false positives when the function's source is + # displayed in the exception report. + cooked_eggs = "".join(["s", "c", "r", "a", "m", "b", "l", "e", "d"]) # NOQA + sauce = "".join( # NOQA + ["w", "o", "r", "c", "e", "s", "t", "e", "r", "s", "h", "i", "r", "e"] + ) + try: + raise Exception + except Exception: + exc_info = sys.exc_info() + send_log(request, exc_info) + return technical_500_response(request, *exc_info) + + +@sensitive_variables("sauce") +@sensitive_post_parameters("bacon-key", "sausage-key") +async def async_sensitive_function(request): + # Do not just use plain strings for the variables' values in the code so + # that the tests don't return false positives when the function's source is + # displayed in the exception report. + cooked_eggs = "".join(["s", "c", "r", "a", "m", "b", "l", "e", "d"]) # NOQA + sauce = "".join( # NOQA + ["w", "o", "r", "c", "e", "s", "t", "e", "r", "s", "h", "i", "r", "e"] + ) + raise Exception + + +async def async_sensitive_view_nested(request): + try: + await async_sensitive_function(request) + except Exception: + exc_info = sys.exc_info() + send_log(request, exc_info) + return technical_500_response(request, *exc_info) + + @sensitive_variables() @sensitive_post_parameters() def paranoid_view(request): @@ -309,11 +349,54 @@ class Klass: send_log(request, exc_info) return technical_500_response(request, *exc_info) + @sensitive_variables("sauce") + async def async_method(self, request): + # Do not just use plain strings for the variables' values in the code + # so that the tests don't return false positives when the function's + # source is displayed in the exception report. + cooked_eggs = "".join(["s", "c", "r", "a", "m", "b", "l", "e", "d"]) # NOQA + sauce = "".join( # NOQA + ["w", "o", "r", "c", "e", "s", "t", "e", "r", "s", "h", "i", "r", "e"] + ) + try: + raise Exception + except Exception: + exc_info = sys.exc_info() + send_log(request, exc_info) + return technical_500_response(request, *exc_info) + + @sensitive_variables("sauce") + async def _async_method_inner(self, request): + # Do not just use plain strings for the variables' values in the code + # so that the tests don't return false positives when the function's + # source is displayed in the exception report. + cooked_eggs = "".join(["s", "c", "r", "a", "m", "b", "l", "e", "d"]) # NOQA + sauce = "".join( # NOQA + ["w", "o", "r", "c", "e", "s", "t", "e", "r", "s", "h", "i", "r", "e"] + ) + raise Exception + + async def async_method_nested(self, request): + try: + await self._async_method_inner(request) + except Exception: + exc_info = sys.exc_info() + send_log(request, exc_info) + return technical_500_response(request, *exc_info) + def sensitive_method_view(request): return Klass().method(request) +async def async_sensitive_method_view(request): + return await Klass().async_method(request) + + +async def async_sensitive_method_view_nested(request): + return await Klass().async_method_nested(request) + + @sensitive_variables("sauce") @sensitive_post_parameters("bacon-key", "sausage-key") def multivalue_dict_key_error(request):