[Django] #35959: Admin "Change password" Button Visible with Only "Can view user" Permission

[Django]

#35959: Admin "Change password" Button Visible with Only "Can view user" Permission
-------------------------------------+-------------------------------------
Reporter: devnamdev2003 | Type: Bug
Status: new | Component:
| contrib.admin
Version: 5.1 | Severity: Normal
Keywords: Permissions, Admin | Triage Stage:
Interface, Change Password, View | Unreviewed
User, Permission Bug |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
There seems to be a bug (or design oversight) in the Django admin panel
where the "**Change password**" button is visible to users who only have
the **Can view user permission**. According to Django's permission model,
a user who can only view users should not have access to modifying user
details, including changing the password.

Steps to reproduce:

1. Create a superuser and a test user.
2. Grant the test user only the Can view user permission.
3. Log in to the admin panel as the test user.
4. Navigate to the user change page for any user.
5. Observe that the "Change password" button is visible despite the user
having no permission to change user details.
--
Ticket URL: <https://code.djangoproject.com/ticket/35959>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#35959: Admin "Change password" Button Visible with Only "Can view user" Permission
-------------------------------------+-------------------------------------

Reporter: devnamdev2003 | Owner: (none)

Type: Bug | Status: new
Component: contrib.admin | Version: 5.1

Severity: Normal | Resolution:

Keywords: Permissions, Admin | Triage Stage:
Interface, Change Password, View | Unreviewed
User, Permission Bug |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------

Changes (by devnamdev2003):

* Attachment "Screenshot 2024-12-01 112851.png" added.

[Django]

#35959: Admin "Change password" Button Visible with Only "Can view user" Permission
-------------------------------------+-------------------------------------
Reporter: devnamdev2003 | Owner: (none)
Type: Bug | Status: new
Component: contrib.admin | Version: 5.1
Severity: Normal | Resolution:
Keywords: Permissions, Admin | Triage Stage:
Interface, Change Password, View | Unreviewed
User, Permission Bug |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
Changes (by devnamdev2003):

* Attachment "image-20241201-113218.png" added.

[Django]

#35959: Admin "Change password" Button Visible with Only "Can view user" Permission
-------------------------------------+-------------------------------------

Reporter: Dev Namdev | Owner: (none)

Type: Bug | Status: new
Component: contrib.admin | Version: 5.1
Severity: Normal | Resolution:

Keywords: Permissions, Admin | Triage Stage: Accepted
Interface, Change Password, View |

User, Permission Bug |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------

Changes (by Sarah Boyce):

* stage: Unreviewed => Accepted

Comment:

Thank you for the report
Although this widget was updated in #34977, this behavior existed prior to
that so not a release blocker
--
Ticket URL: <https://code.djangoproject.com/ticket/35959#comment:1>

[Django]

#35959: Admin "Change password" Button Visible with Only "Can view user" Permission
-------------------------------------+-------------------------------------

Reporter: Dev Namdev | Owner: Brock
| Smickley
Type: Bug | Status: assigned

Component: contrib.admin | Version: 5.1
Severity: Normal | Resolution:
Keywords: Permissions, Admin | Triage Stage: Accepted
Interface, Change Password, View |
User, Permission Bug |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------

Changes (by Brock Smickley):

* owner: (none) => Brock Smickley
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/35959#comment:2>

[Django]

#35959: Admin "Change password" Button Visible with Only "Can view user" Permission
-------------------------------------+-------------------------------------
Reporter: Dev Namdev | Owner: Brock
| Smickley
Type: Bug | Status: assigned
Component: contrib.admin | Version: 5.1
Severity: Normal | Resolution:
Keywords: Permissions, Admin | Triage Stage: Accepted
Interface, Change Password, View |
User, Permission Bug |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------

Comment (by Brock Smickley):

struggling with this one but I think I figured out how to test it!
{{{#!diff
diff --git a/tests/auth_tests/test_views.py
b/tests/auth_tests/test_views.py
index 98fdfe79b7..6e1ebc2b3b 100644
--- a/tests/auth_tests/test_views.py
+++'''' b/tests/auth_tests/test_views.py
@@ -1704,6 +1704,7 @@ class ChangelistTests(MessagesTestMixin,
AuthViewsTestCase):
),
html=True,
)
+ self.assertNotContains(response, '<a class="button"
href="../password/">Reset password</a>')
# Value in POST data is ignored.
data = self.get_user_data(u)
data["password"] = "shouldnotchange"
}}}
Note to self for later: I should also probably test to make sure that the
button ''does'' show for users ''with'' permission.
--
Ticket URL: <https://code.djangoproject.com/ticket/35959#comment:3>

[Django]

#35959: Admin "Change password" Button Visible with Only "Can view user" Permission
-------------------------------------+-------------------------------------
Reporter: Dev Namdev | Owner: Brock
| Smickley
Type: Bug | Status: assigned
Component: contrib.admin | Version: 5.1
Severity: Normal | Resolution:
Keywords: Permissions, Admin | Triage Stage: Accepted
Interface, Change Password, View |
User, Permission Bug |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
Comment (by Brock Smickley):

I speculate that the solution involves extracting the user's permission
and passing it to the context in `ReadOnlyPasswordHashWidget`, but I'm
struggling to figure out how to do that. I was trying to figure out a way
to use the [https://docs.djangoproject.com/en/5.1/ref/templates/api
/#django-contrib-auth-context-processors-auth auth function] from
`context_processors.py` but the template isn't rendered by a request,
right?
--
Ticket URL: <https://code.djangoproject.com/ticket/35959#comment:4>

[Django]

#35959: Admin "Change password" Button Visible with Only "Can view user" Permission
-------------------------------------+-------------------------------------
Reporter: Dev Namdev | Owner: Brock
| Smickley
Type: Bug | Status: assigned
Component: contrib.admin | Version: 5.1
Severity: Normal | Resolution:
Keywords: Permissions, Admin | Triage Stage: Accepted
Interface, Change Password, View |
User, Permission Bug |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------

Comment (by Sarah Boyce):

This is actually quite similar to #33171
I think maybe if the user has view only permission the password field
shouldn't be available (maybe updating `get_fieldsets`)
--
Ticket URL: <https://code.djangoproject.com/ticket/35959#comment:5>

[Django]

#35959: Admin "Change password" Button Visible with Only "Can view user" Permission
-------------------------------------+-------------------------------------
Reporter: Dev Namdev | Owner: Brock
| Smickley
Type: Bug | Status: assigned
Component: contrib.admin | Version: 5.1
Severity: Normal | Resolution:
Keywords: Permissions, Admin | Triage Stage: Accepted
Interface, Change Password, View |
User, Permission Bug |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------

Comment (by Brock Smickley):

Replying to [comment:5 Sarah Boyce]:

> This is actually quite similar to #33171
> I think maybe if the user has view only permission the password field
shouldn't be available (maybe updating `get_fieldsets`)

ok, I understand how to check the user's permissions from `get_fieldsets`
but then how do I omit the password field from there? I tried messing
around with the `exclude` option as well as the raw `fieldsets` tuple but
I couldn't figure anything out. is there an example of something like this
already in the codebase?
--
Ticket URL: <https://code.djangoproject.com/ticket/35959#comment:6>

[Django]

#35959: Admin "Change password" Button Visible with Only "Can view user" Permission
-------------------------------------+-------------------------------------
Reporter: Dev Namdev | Owner: Brock
| Smickley
Type: Bug | Status: assigned
Component: contrib.admin | Version: 5.1
Severity: Normal | Resolution:
Keywords: Permissions, Admin | Triage Stage: Accepted
Interface, Change Password, View |
User, Permission Bug |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------

Comment (by Sarah Boyce):

{{{#!diff
--- a/django/contrib/auth/admin.py
+++ b/django/contrib/auth/admin.py
@@ -1,3 +1,4 @@
+import copy
from django.conf import settings
from django.contrib import admin, messages
from django.contrib.admin.options import IS_POPUP_VAR
@@ -82,10 +83,24 @@ class UserAdmin(admin.ModelAdmin):
"user_permissions",
)

+ @staticmethod
+ def _remove_fields_from_fieldsets(fieldsets, fields):
+ fieldset_without_fields = []
+ for fieldset_name, fieldset in copy.deepcopy(fieldsets):
+ fieldset["fields"] = [f for f in fieldset["fields"] if f not
in fields]
+ fieldset_without_fields.append((fieldset_name, fieldset))
+ return fieldset_without_fields
+
def get_fieldsets(self, request, obj=None):
if not obj:
return self.add_fieldsets
- return super().get_fieldsets(request, obj)
+ fieldsets = super().get_fieldsets(request, obj)
+ if not self.has_change_permission(request, obj):
+ return self._remove_fields_from_fieldsets(
+ fieldsets=fieldsets,
+ fields=["password"]
+ )
+ return fieldsets

def get_form(self, request, obj=None, **kwargs):
"""
diff --git a/tests/auth_tests/test_views.py
b/tests/auth_tests/test_views.py
index 98fdfe79b7..e9ae523293 100644
--- a/tests/auth_tests/test_views.py
+++ b/tests/auth_tests/test_views.py
@@ -1692,7 +1692,7 @@ class ChangelistTests(MessagesTestMixin,
AuthViewsTestCase):
algo, salt, hash_string = u.password.split("$")
self.assertContains(response, '<div
class="readonly">testclient</div>')
# ReadOnlyPasswordHashWidget is used to render the field.
- self.assertContains(
+ self.assertNotContains(
response,
"<strong>algorithm</strong>: <bdi>%s</bdi>\n\n"
"<strong>salt</strong>:
<bdi>%s********************</bdi>\n\n"

@@ -1704,6 +1704,7 @@ class ChangelistTests(MessagesTestMixin,
AuthViewsTestCase):
),
html=True,
)
+ self.assertNotContains(response,'<a class="button"
href="../password/">Reset password</a>')
# Value in POST data is ignored.
data = self.get_user_data(u)
}}}

Something like this maybe?
--
Ticket URL: <https://code.djangoproject.com/ticket/35959#comment:7>

[Django]

#35959: Admin "Change password" Button Visible with Only "Can view user" Permission
-------------------------------------+-------------------------------------
Reporter: Dev Namdev | Owner: Brock
| Smickley
Type: Bug | Status: assigned
Component: contrib.admin | Version: 5.1
Severity: Normal | Resolution:
Keywords: Permissions, Admin | Triage Stage: Accepted
Interface, Change Password, View |
User, Permission Bug |

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------

Changes (by Brock Smickley):

* has_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/35959#comment:8>