Opened 3 years ago

Closed 21 months ago

#22295 closed Cleanup/optimization (fixed)

admin/base.html only shows #user-tools when user is staff

Reported by: wouter@… Owned by: Tim Graham <timograham@…>
Component: contrib.admin Version: master
Severity: Normal Keywords: user-tools admin base template
Cc: jeffrey@…, tanner Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The build-in Django Admin ships with the admin/base.html template. This template is, among other things, responsible for rendering the #user-tools div that contains the 'log out' and 'change password' buttons. The user tools are only rendered if user.is_active and user.is_staff are True, see:
https://github.com/django/django/blob/2bc51438664b5ffbbd1430b4f9f3307f18b2b9db/django/contrib/admin/templates/admin/base.html#L27

This check makes sure that #user-tools is only rendered when the user is actually authenticated for use of the admin. This is required because the login template (admin/login.html) eventually inherits from admin/base.html. If the check would be omitted, the #user-tools would become visible if the user was yet to be authenticated resulting in a situation where the user could 'log out' without being 'logged in' first.

This check is therefore relevant, but is it the wrong check and breaks inheritance in the following case:

Lets say you want to inherit from django.contrib.admin.sites.BaseSite to create a customized admin for special users that are not necessarily staff members. You can override the BaseSite.has_permission method. Currently this method holds the condition: request.user.is_active and request.user.is_staff . You might change this to request.user.is_active and request.user.is_a_special_user_but_not_staff . This user would now be allowed to access this customised admin without having access to the default admin.

The problem is that the user cannot log out from this special admin because the #user-tools are only rendered if the user is a staff member.

I can think of two solutions:

  1. Use the BaseAdmin.has_permission to do this check
  2. Create a block called user-tools in the template and override this block in the admin/login.html to be empty

In my opinion solution number 2 would be the best approach :-).

Change History (13)

comment:1 Changed 3 years ago by Aymeric Augustin

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset

Yes, option 2 sounds like a better approach if the goal is simply to hide this block.

But what about people who extend this template and rely on the current implementation to hide the block?

Even for small things, it's important to consider backwards compatibility.

comment:2 Changed 2 years ago by Tim Graham

Triage Stage: UnreviewedAccepted

comment:3 Changed 2 years ago by Maxime Turcotte

Owner: changed from nobody to Maxime Turcotte
Status: newassigned

comment:4 Changed 2 years ago by anonymous

Has patch: set

Here's my PR

comment:5 Changed 2 years ago by Tim Graham

Patch needs improvement: set

I left comments for improvement on PR. Please uncheck "Patch needs improvement" when you update it, thanks.

comment:6 Changed 2 years ago by Maxime Turcotte

Owner: Maxime Turcotte deleted
Status: assignednew

comment:7 Changed 22 months ago by tanner

I have updated my PR https://github.com/django/django/pull/3762 to include the requested improvements and also implemented solution 2.

comment:8 Changed 22 months ago by tanner

Cc: tanner added

comment:9 Changed 21 months ago by Tim Graham

Please uncheck "Patch needs improvement" when you update your pull request so the ticket appears in the review queue.

comment:10 Changed 21 months ago by tanner

Patch needs improvement: unset

comment:11 Changed 21 months ago by Tim Graham <timograham@…>

In c5fb34c47ef43fbd54e11fa6e72de326f5453f98:

Documented AdminSite.has_permission(); refs #22295.

comment:12 Changed 21 months ago by Tim Graham

Triage Stage: AcceptedReady for checkin

comment:13 Changed 21 months ago by Tim Graham <timograham@…>

Owner: set to Tim Graham <timograham@…>
Resolution: fixed
Status: newclosed

In 46068d850d8debd3611ed6499d48b9907bf07ef6:

Fixed #22295 -- Replaced permission check for displaying admin user-tools

Note: See TracTickets for help on using tickets.
Back to Top