Code

Opened 17 months ago

Closed 5 months ago

#19830 closed Cleanup/optimization (duplicate)

staff_member_required decorator should use 'admin.site.login_form' and not

Reported by: rene@… Owned by: nobody
Component: contrib.admin Version: master
Severity: Normal Keywords: staff_member_required authentication_form template_name
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The staff_member_required decorator in django.contrib.admin.views checks if the user is logged in and is staff. If not it will display the admin login page instead of the decorated view.

If the user is not logged in and not staff, a 'defaults' dictionary is build with the following keys: 'template_name', 'authentication_form' and 'extra_context'.

The 'template_name' refers to the template to use for the login screen.
The 'authentication_form' refers to the form class to use to handle the login form.

The two values are now: 'admin/login.html' and 'AdminAuthenticationForm'. See the code block below:

defaults = {
            'template_name': 'admin/login.html',
            'authentication_form': AdminAuthenticationForm,
            'extra_context': {
                'title': _('Log in'),
                'app_path': request.get_full_path(),
                REDIRECT_FIELD_NAME: request.get_full_path(),
            },
        }

According to this documentation: https://docs.djangoproject.com/en/1.4/ref/contrib/admin/#root-and-login-templates a developer can provide it's own login-template and own login-form class to handle 'admin' logins.

Should the 'staff_member_required' decorator not refer to these two settings instead of the hard-coded values in the current implementation?
Like this:

defaults = {
            'template_name':  site.login_template or 'admin/login.html',
            'authentication_form': site.login_form or AdminAuthenticationForm,
            'extra_context': {
                'title': _('Log in'),
                'app_path': request.get_full_path(),
                REDIRECT_FIELD_NAME: request.get_full_path(),
            },
        }

These is a problem in the above codeblock. The 'site' variable does not exists. I am not sure how to get a reference to the 'admin' site object.

Attachments (0)

Change History (3)

comment:1 Changed 17 months ago by claudep

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted
  • Version changed from 1.4 to master

I think that instead of login, the decorator should use the redirect_to_login view, unless I've missed something...

comment:2 Changed 16 months ago by anonymous

The biggest issue I see here is dealing with multiple admin sites.

comment:3 Changed 5 months ago by claudep

  • Resolution set to duplicate
  • Status changed from new to closed

See ticket #21911, staff_member_required implementation changed to use user_passes_test (which itself uses redirect_to_login).

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.