From d755a98b8438c10f3cff61303ceb1fe16d414e9b Mon Sep 17 00:00:00 2001 From: Sarah Boyce <42296566+sarahboyce@users.noreply.github.com> Date: Fri, 17 Jan 2025 17:44:25 +0100 Subject: [PATCH] Fixed #35959 -- Displayed password reset button in admin only when user has sufficient permissions. This change ensures that the "Reset password" button in the admin is shown only when the user has the necessary permission to perform a password change operation. It reuses the password hashing rendering logic in `display_for_field` to show the appropriate read-only widget for users with view-only access. --- django/contrib/admin/helpers.py | 6 ------ django/contrib/admin/utils.py | 6 +++++- django/contrib/auth/forms.py | 1 - tests/admin_utils/tests.py | 25 +++++++++++++++++++++++++ tests/auth_tests/test_views.py | 6 +++++- 5 files changed, 35 insertions(+), 9 deletions(-) diff --git a/django/contrib/admin/helpers.py b/django/contrib/admin/helpers.py index 51450d1d9e..969167f0e2 100644 --- a/django/contrib/admin/helpers.py +++ b/django/contrib/admin/helpers.py @@ -276,12 +276,6 @@ class AdminReadonlyField: except (AttributeError, ValueError, ObjectDoesNotExist): result_repr = self.empty_value_display else: - if field in self.form.fields: - widget = self.form[field].field.widget - # This isn't elegant but suffices for contrib.auth's - # ReadOnlyPasswordHashWidget. - if getattr(widget, "read_only", False): - return widget.render(field, value) if f is None: if getattr(attr, "boolean", False): result_repr = _boolean_icon(value) diff --git a/django/contrib/admin/utils.py b/django/contrib/admin/utils.py index cc8ca5604d..eec93fa4be 100644 --- a/django/contrib/admin/utils.py +++ b/django/contrib/admin/utils.py @@ -5,6 +5,8 @@ from collections import defaultdict from functools import reduce from operator import or_ +from django.contrib.auth import get_user_model +from django.contrib.auth.templatetags.auth import render_password_as_hash from django.core.exceptions import FieldDoesNotExist from django.core.validators import EMPTY_VALUES from django.db import models, router @@ -429,7 +431,9 @@ def help_text_for_field(name, model): def display_for_field(value, field, empty_value_display, avoid_link=False): from django.contrib.admin.templatetags.admin_list import _boolean_icon - if getattr(field, "flatchoices", None): + if field.name == "password" and field.model == get_user_model(): + return render_password_as_hash(value) + elif getattr(field, "flatchoices", None): try: return dict(field.flatchoices).get(value, empty_value_display) except TypeError: diff --git a/django/contrib/auth/forms.py b/django/contrib/auth/forms.py index 7b0b3833b8..2214e134d0 100644 --- a/django/contrib/auth/forms.py +++ b/django/contrib/auth/forms.py @@ -34,7 +34,6 @@ def _unicode_ci_compare(s1, s2): class ReadOnlyPasswordHashWidget(forms.Widget): template_name = "auth/widgets/read_only_password_hash.html" - read_only = True def get_context(self, name, value, attrs): context = super().get_context(name, value, attrs) diff --git a/tests/admin_utils/tests.py b/tests/admin_utils/tests.py index 77d6655290..6d165637e7 100644 --- a/tests/admin_utils/tests.py +++ b/tests/admin_utils/tests.py @@ -17,9 +17,12 @@ from django.contrib.admin.utils import ( lookup_field, quote, ) +from django.contrib.auth.models import User +from django.contrib.auth.templatetags.auth import render_password_as_hash from django.core.validators import EMPTY_VALUES from django.db import DEFAULT_DB_ALIAS, models from django.test import SimpleTestCase, TestCase, override_settings +from django.test.utils import isolate_apps from django.utils.formats import localize from django.utils.safestring import mark_safe @@ -238,6 +241,28 @@ class UtilsTests(SimpleTestCase): ) self.assertEqual(display_value, "12,345") + @isolate_apps("admin_utils") + def test_display_for_field_password_name_not_user_model(self): + class PasswordModel(models.Model): + password = models.CharField(max_length=200) + + password_field = PasswordModel._meta.get_field("password") + display_value = display_for_field("test", password_field, self.empty_value) + self.assertEqual(display_value, "test") + + def test_password_display_for_field_user_model(self): + password_field = User._meta.get_field("password") + for password in [ + "invalid", + "md5$zjIiKM8EiyfXEGiexlQRw4$a59a82cf344546e7bc09cb5f2246370a", + "!b7pk7RNudAXGTNLK6fW5YnBCLVE6UUmeoJJYQHaO", + ]: + with self.subTest(password=password): + display_value = display_for_field( + password, password_field, self.empty_value + ) + self.assertEqual(display_value, render_password_as_hash(password)) + def test_list_display_for_value(self): display_value = display_for_value([1, 2, 3], self.empty_value) self.assertEqual(display_value, "1, 2, 3") diff --git a/tests/auth_tests/test_views.py b/tests/auth_tests/test_views.py index 156520ebf7..c8f0be1be7 100644 --- a/tests/auth_tests/test_views.py +++ b/tests/auth_tests/test_views.py @@ -1703,7 +1703,7 @@ class ChangelistTests(MessagesTestMixin, AuthViewsTestCase): ) algo, salt, hash_string = u.password.split("$") self.assertContains(response, '
testclient
') - # ReadOnlyPasswordHashWidget is used to render the field. + # The password value is hashed. self.assertContains( response, "algorithm: %s\n\n" @@ -1716,6 +1716,10 @@ class ChangelistTests(MessagesTestMixin, AuthViewsTestCase): ), html=True, ) + self.assertNotContains( + response, + 'Reset password', + ) # Value in POST data is ignored. data = self.get_user_data(u) data["password"] = "shouldnotchange"