Skip to content

Commit

Permalink
Fixed #34977 -- Improved accessibility in the UserChangeForm by repla…
Browse files Browse the repository at this point in the history
…cing the reset password link with a button.

Co-authored-by: Natalia <124304+nessita@users.noreply.github.com>
  • Loading branch information
fsbraun and nessita committed Mar 27, 2024
1 parent d658a31 commit 944745a
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 22 deletions.
20 changes: 10 additions & 10 deletions django/contrib/auth/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,9 @@ class ReadOnlyPasswordHashWidget(forms.Widget):

def get_context(self, name, value, attrs):
context = super().get_context(name, value, attrs)
usable_password = value and not value.startswith(UNUSABLE_PASSWORD_PREFIX)
summary = []
if not value or value.startswith(UNUSABLE_PASSWORD_PREFIX):
summary.append({"label": gettext("No password set.")})
else:
if usable_password:
try:
hasher = identify_hasher(value)
except ValueError:
Expand All @@ -53,7 +52,12 @@ def get_context(self, name, value, attrs):
else:
for key, value_ in hasher.safe_summary(value).items():
summary.append({"label": gettext(key), "value": value_})
else:
summary.append({"label": gettext("No password set.")})
context["summary"] = summary
context["button_label"] = (
_("Reset password") if usable_password else _("Set password")
)
return context

def id_for_label(self, id_):
Expand Down Expand Up @@ -253,9 +257,8 @@ class UserChangeForm(forms.ModelForm):
password = ReadOnlyPasswordHashField(
label=_("Password"),
help_text=_(
"Raw passwords are not stored, so there is no way to see this "
"user’s password, but you can change or unset the password using "
'<a href="{}">this form</a>.'
"Raw passwords are not stored, so there is no way to see "
"the user’s password."
),
)

Expand All @@ -271,11 +274,8 @@ def __init__(self, *args, **kwargs):
if self.instance and not self.instance.has_usable_password():
password.help_text = _(
"Enable password-based authentication for this user by setting a "
'password using <a href="{}">this form</a>.'
"password."
)
password.help_text = password.help_text.format(
f"../../{self.instance.pk}/password/"
)
user_permissions = self.fields.get("user_permissions")
if user_permissions:
user_permissions.queryset = user_permissions.queryset.select_related(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
<div{% include 'django/forms/widgets/attrs.html' %}>
{% for entry in summary %}
<strong>{{ entry.label }}</strong>{% if entry.value %}: <bdi>{{ entry.value }}</bdi>{% endif %}
{% endfor %}
<p>
{% for entry in summary %}
<strong>{{ entry.label }}</strong>{% if entry.value %}: <bdi>{{ entry.value }}</bdi>{% endif %}
{% endfor %}
</p>
<p><a class="button" href="{{ password_url|default:"../password/" }}">{{ button_label }}</a></p>
</div>
6 changes: 6 additions & 0 deletions docs/releases/5.1.txt
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,12 @@ Minor features
:func:`~.django.contrib.auth.decorators.user_passes_test` decorators now
support wrapping asynchronous view functions.

* ``ReadOnlyPasswordHashWidget`` now includes a button to reset the user's
password, which replaces the link previously embedded in the
``ReadOnlyPasswordHashField``'s help text, improving the overall
accessibility of the
:class:`~django.contrib.auth.forms.UserChangeForm`.

:mod:`django.contrib.contenttypes`
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Expand Down
25 changes: 18 additions & 7 deletions tests/auth_tests/test_forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -1023,34 +1023,42 @@ def test_bug_19349_bound_password_field(self):
self.assertEqual(form.initial["password"], form["password"].value())

@override_settings(ROOT_URLCONF="auth_tests.urls_admin")
def test_link_to_password_reset_in_helptext_via_to_field(self):
def test_link_to_password_reset_in_user_change_form(self):
cases = [
(
"testclient",
'you can change or unset the password using <a href="(.*?)">',
"Raw passwords are not stored, so there is no way to see "
"the user’s password.",
"Reset password",
),
(
"unusable_password",
"Enable password-based authentication for this user by setting "
'a password using <a href="(.*?)">this form</a>.',
"Enable password-based authentication for this user by setting a "
"password.",
"Set password",
),
]
for username, expected_help_text in cases:
password_reset_link = r'<a class="button" href="([^"]*)">([^<]*)</a>'
for username, expected_help_text, expected_button_label in cases:
with self.subTest(username=username):
user = User.objects.get(username=username)
form = UserChangeForm(data={}, instance=user)
password_help_text = form.fields["password"].help_text
matches = re.search(expected_help_text, password_help_text)
self.assertEqual(password_help_text, expected_help_text)

matches = re.search(password_reset_link, form.as_p())
self.assertIsNotNone(matches)
self.assertEqual(len(matches.groups()), 2)
url_prefix = f"admin:{user._meta.app_label}_{user._meta.model_name}"
# URL to UserChangeForm in admin via to_field (instead of pk).
user_change_url = reverse(f"{url_prefix}_change", args=(user.username,))
user_change_url = reverse(f"{url_prefix}_change", args=(user.pk,))
joined_url = urllib.parse.urljoin(user_change_url, matches.group(1))

pw_change_url = reverse(
f"{url_prefix}_password_change", args=(user.pk,)
)
self.assertEqual(joined_url, pw_change_url)
self.assertEqual(matches.group(2), expected_button_label)

def test_custom_form(self):
class CustomUserChangeForm(UserChangeForm):
Expand Down Expand Up @@ -1345,11 +1353,14 @@ def test_render(self):
self.assertHTMLEqual(
widget.render("name", value, {"id": "id_password"}),
'<div id="id_password">'
" <p>"
" <strong>algorithm</strong>: <bdi>pbkdf2_sha256</bdi>"
" <strong>iterations</strong>: <bdi>100000</bdi>"
" <strong>salt</strong>: <bdi>a6Pucb******</bdi>"
" <strong>hash</strong>: "
" <bdi>WmCkn9**************************************</bdi>"
" </p>"
' <p><a class="button" href="../password/">Reset password</a></p>'
"</div>",
)

Expand Down
4 changes: 2 additions & 2 deletions tests/auth_tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -1442,7 +1442,7 @@ def test_user_with_usable_password_change_password(self):
response = self.client.get(user_change_url)
# Test the link inside password field help_text.
rel_link = re.search(
r'change or unset the password using <a href="([^"]*)">this form</a>',
r'<a class="button" href="([^"]*)">Reset password</a>',
response.content.decode(),
)[1]
self.assertEqual(urljoin(user_change_url, rel_link), password_change_url)
Expand Down Expand Up @@ -1538,7 +1538,7 @@ def test_user_with_unusable_password_change_password(self):
response = self.client.get(user_change_url)
# Test the link inside password field help_text.
rel_link = re.search(
r'by setting a password using <a href="([^"]*)">this form</a>',
r'<a class="button" href="([^"]*)">Set password</a>',
response.content.decode(),
)[1]
self.assertEqual(urljoin(user_change_url, rel_link), password_change_url)
Expand Down

0 comments on commit 944745a

Please sign in to comment.