Skip to content

Commit

Permalink
Refs #28215 -- Marked auth form passwords as sensitive variables.
Browse files Browse the repository at this point in the history
  • Loading branch information
GappleBee authored and sarahboyce committed Nov 15, 2024
1 parent 91c879e commit 037e740
Show file tree
Hide file tree
Showing 2 changed files with 136 additions and 1 deletion.
5 changes: 5 additions & 0 deletions django/contrib/auth/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
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")
Expand Down Expand Up @@ -122,6 +123,7 @@ def create_password_fields(label1=_("Password"), label2=_("Password confirmation
)
return password1, password2

@sensitive_variables("password1", "password2")
def validate_passwords(
self,
password1_field_name="password1",
Expand Down Expand Up @@ -151,6 +153,7 @@ def validate_passwords(
)
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:
Expand Down Expand Up @@ -348,6 +351,7 @@ def __init__(self, request=None, *args, **kwargs):
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")
Expand Down Expand Up @@ -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.
Expand Down
132 changes: 131 additions & 1 deletion tests/auth_tests/test_auth_backends.py
Original file line number Diff line number Diff line change
@@ -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

Expand All @@ -14,19 +15,22 @@
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 (
Expand All @@ -38,6 +42,16 @@
)


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"]
Expand Down Expand Up @@ -1040,6 +1054,15 @@ async def aauthenticate(self, request, username=None, password=None):
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().
Expand Down Expand Up @@ -1127,6 +1150,113 @@ def test_clean_credentials_sensitive_variables(self):
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,
'<tr><td>password</td><td class="code">'
"<pre>&#39;********************&#39;</pre></td></tr>",
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,
'<tr><td>password1</td><td class="code">'
"<pre>&#x27;********************&#x27;</pre></td></tr>",
html=True,
status_code=500,
)

self.assertContains(
response,
'<tr><td>password2</td><td class="code">'
"<pre>&#x27;********************&#x27;</pre></td></tr>",
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,
'<tr><td>password</td><td class="code">'
"<pre>&#x27;********************&#x27;</pre></td></tr>",
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,
'<tr><td>old_password</td><td class="code">'
"<pre>&#x27;********************&#x27;</pre></td></tr>",
html=True,
status_code=500,
)

@override_settings(
AUTHENTICATION_BACKENDS=(
"auth_tests.test_auth_backends.SkippedBackend",
Expand Down

0 comments on commit 037e740

Please sign in to comment.