From 037e740ec56674e69e564b97e1151950757c410d Mon Sep 17 00:00:00 2001 From: GappleBee Date: Mon, 7 Oct 2024 16:09:21 +0200 Subject: [PATCH] Refs #28215 -- Marked auth form passwords as sensitive variables. --- django/contrib/auth/forms.py | 5 + tests/auth_tests/test_auth_backends.py | 132 ++++++++++++++++++++++++- 2 files changed, 136 insertions(+), 1 deletion(-) diff --git a/django/contrib/auth/forms.py b/django/contrib/auth/forms.py index edf672a6e5..093f525245 100644 --- a/django/contrib/auth/forms.py +++ b/django/contrib/auth/forms.py @@ -15,6 +15,7 @@ from django.utils.http import urlsafe_base64_encode from django.utils.text import capfirst from django.utils.translation import gettext from django.utils.translation import gettext_lazy as _ +from django.views.decorators.debug import sensitive_variables UserModel = get_user_model() logger = logging.getLogger("django.contrib.auth") @@ -122,6 +123,7 @@ class SetPasswordMixin: ) return password1, password2 + @sensitive_variables("password1", "password2") def validate_passwords( self, password1_field_name="password1", @@ -151,6 +153,7 @@ class SetPasswordMixin: ) self.add_error(password2_field_name, error) + @sensitive_variables("password") def validate_password_for_user(self, user, password_field_name="password2"): password = self.cleaned_data.get(password_field_name) if password: @@ -348,6 +351,7 @@ class AuthenticationForm(forms.Form): if self.fields["username"].label is None: self.fields["username"].label = capfirst(self.username_field.verbose_name) + @sensitive_variables() def clean(self): username = self.cleaned_data.get("username") password = self.cleaned_data.get("password") @@ -539,6 +543,7 @@ class PasswordChangeForm(SetPasswordForm): field_order = ["old_password", "new_password1", "new_password2"] + @sensitive_variables("old_password") def clean_old_password(self): """ Validate that the old_password field is correct. diff --git a/tests/auth_tests/test_auth_backends.py b/tests/auth_tests/test_auth_backends.py index b612d27ab0..32fb092cf4 100644 --- a/tests/auth_tests/test_auth_backends.py +++ b/tests/auth_tests/test_auth_backends.py @@ -1,6 +1,7 @@ import sys from datetime import date from unittest import mock +from unittest.mock import patch from asgiref.sync import sync_to_async @@ -14,19 +15,22 @@ from django.contrib.auth import ( signals, ) from django.contrib.auth.backends import BaseBackend, ModelBackend +from django.contrib.auth.forms import PasswordChangeForm, SetPasswordForm from django.contrib.auth.hashers import MD5PasswordHasher from django.contrib.auth.models import AnonymousUser, Group, Permission, User from django.contrib.contenttypes.models import ContentType from django.core.exceptions import ImproperlyConfigured, PermissionDenied from django.http import HttpRequest from django.test import ( + Client, RequestFactory, SimpleTestCase, TestCase, modify_settings, override_settings, ) -from django.views.debug import technical_500_response +from django.urls import reverse +from django.views.debug import ExceptionReporter, technical_500_response from django.views.decorators.debug import sensitive_variables from .models import ( @@ -38,6 +42,16 @@ from .models import ( ) +class FilteredExceptionReporter(ExceptionReporter): + def get_traceback_frames(self): + frames = super().get_traceback_frames() + return [ + frame + for frame in frames + if not isinstance(dict(frame["vars"]).get("self"), Client) + ] + + class SimpleBackend(BaseBackend): def get_user_permissions(self, user_obj, obj=None): return ["user_perm"] @@ -1040,6 +1054,15 @@ class TypeErrorBackend: raise TypeError +class TypeErrorValidator: + """ + Always raises a TypeError. + """ + + def validate(self, password=None, user=None): + raise TypeError + + class SkippedBackend: def authenticate(self): # Doesn't accept any credentials so is skipped by authenticate(). @@ -1127,6 +1150,113 @@ class AuthenticateTests(TestCase): status_code=500, ) + @override_settings( + ROOT_URLCONF="django.contrib.auth.urls", + AUTHENTICATION_BACKENDS=["auth_tests.test_auth_backends.TypeErrorBackend"], + ) + def test_login_process_sensitive_variables(self): + try: + self.client.post( + reverse("login"), + dict(username="testusername", password=self.sensitive_password), + ) + except TypeError: + exc_info = sys.exc_info() + + rf = RequestFactory() + with patch("django.views.debug.ExceptionReporter", FilteredExceptionReporter): + response = technical_500_response(rf.get("/"), *exc_info) + + self.assertNotContains(response, self.sensitive_password, status_code=500) + self.assertContains(response, "TypeErrorBackend", status_code=500) + + # AuthenticationForm.clean(). + self.assertContains( + response, + 'password' + "
'********************'
", + html=True, + status_code=500, + ) + + def test_setpasswordform_validate_passwords_sensitive_variables(self): + password_form = SetPasswordForm(AnonymousUser()) + password_form.cleaned_data = { + "password1": self.sensitive_password, + "password2": self.sensitive_password + "2", + } + try: + password_form.validate_passwords() + except ValueError: + exc_info = sys.exc_info() + + rf = RequestFactory() + response = technical_500_response(rf.get("/"), *exc_info) + self.assertNotContains(response, self.sensitive_password, status_code=500) + self.assertNotContains(response, self.sensitive_password + "2", status_code=500) + + self.assertContains( + response, + 'password1' + "
'********************'
", + html=True, + status_code=500, + ) + + self.assertContains( + response, + 'password2' + "
'********************'
", + html=True, + status_code=500, + ) + + @override_settings( + AUTH_PASSWORD_VALIDATORS=[ + {"NAME": __name__ + ".TypeErrorValidator"}, + ] + ) + def test_setpasswordform_validate_password_for_user_sensitive_variables(self): + password_form = SetPasswordForm(AnonymousUser()) + password_form.cleaned_data = {"password2": self.sensitive_password} + try: + password_form.validate_password_for_user(AnonymousUser()) + except TypeError: + exc_info = sys.exc_info() + + rf = RequestFactory() + response = technical_500_response(rf.get("/"), *exc_info) + self.assertNotContains(response, self.sensitive_password, status_code=500) + + self.assertContains( + response, + 'password' + "
'********************'
", + html=True, + status_code=500, + ) + + def test_passwordchangeform_clean_old_password_sensitive_variables(self): + password_form = PasswordChangeForm(User()) + password_form.cleaned_data = {"old_password": self.sensitive_password} + password_form.error_messages = None + try: + password_form.clean_old_password() + except TypeError: + exc_info = sys.exc_info() + + rf = RequestFactory() + response = technical_500_response(rf.get("/"), *exc_info) + self.assertNotContains(response, self.sensitive_password, status_code=500) + + self.assertContains( + response, + 'old_password' + "
'********************'
", + html=True, + status_code=500, + ) + @override_settings( AUTHENTICATION_BACKENDS=( "auth_tests.test_auth_backends.SkippedBackend",