Code

Opened 3 years ago

Closed 3 years ago

#17296 closed Bug (wontfix)

staff_login_required decorator redirecting to default Login redirect instead of requested

Reported by: ayarshabeer Owned by: nobody
Component: contrib.admin Version: 1.3
Severity: Normal Keywords:
Cc: Triage Stage: Design decision needed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by aaugustin)

While I am trying to access staff_required URL in the application it will shows admin login page but after I given login credential it redirect to default login redirect page (account/profile) instead of my requested staff page.

After I went through the django code I found that in login_required decorator (django/contrib/auth/views) there is checking for host

           ''' netloc = urlparse.urlparse(redirect_to)[1]
'''
            # Use default setting if redirect_to is empty
            if not redirect_to:
                redirect_to = settings.LOGIN_REDIRECT_URL

            # Security check -- don't allow redirection to a different
            # host.
           ''' elif netloc and netloc != request.get_host():
                redirect_to = settings.LOGIN_REDIRECT_URL'''

but this never succeed because staff_required decorator passing redirect_to value as request_full_path() and it doesnot contain host name.

Attachments (1)

17296.patch (588 bytes) - added by aaugustin 3 years ago.

Download all attachments as: .zip

Change History (7)

comment:1 Changed 3 years ago by aaugustin

  • Description modified (diff)
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

(Sort of) fixed formatting.

comment:2 Changed 3 years ago by aaugustin

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

Django doesn't define a staff_required decorator. I assume you're referring of the staff_member_required decorator defined by the admin contrib app, and used by admindocs?

I checked how the admin behaves: if I make an unauthenticated request to http://<mysite>/admin/<app>/<model>/, I'm presented the login form, and then the page I asked for. So I couldn't reproduce the bug at this level. I obtained the same behavior with admindocs.

Then, this decorator isn't documented; it's an internal API. As long as the admin and admindocs app behaves as expected, it's doing its job.

Finally, I don't understand very well your explanation. If the URL is relative, netloc will be the empty string in the piece of code you're quoting:

>>> import urlparse
>>> urlparse.urlparse('/foo/bar')
ParseResult(scheme='', netloc='', path='/foo/bar', params='', query='', fragment='')
>>> urlparse.urlparse('/foo/bar')[1]
''

This means that the elif netloc and ... clause doesn't apply. This clause isn't the reason why you're redirected to LOGIN_REDIRECT_URL.

At this point, I don't have the proof that something's wrong in Django, and I don't have enough information to reproduce your problem, so I'm going to close this ticket.

comment:3 Changed 3 years ago by ayarshabeer

  • Resolution invalid deleted
  • Status changed from closed to reopened

Sorry I explanation was wrong, But issue persist. Please check staff_member_required decorator we can see the code as 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(),
            },
        }
        return login(request, **defaults)

redirect value is setting to extra context and calling the login method present in /admin/auth/views. In login method we can see that redirect_to is taken from request as redirect_to = request.REQUEST.get(redirect_field_name, '') but redirect value is present in extra_context. So value for redirect_to will be empty.
Please verify the same.

EDIT(aaugustin): fixed formatting.

Last edited 3 years ago by aaugustin (previous) (diff)

Changed 3 years ago by aaugustin

comment:4 Changed 3 years ago by aaugustin

  • Triage Stage changed from Unreviewed to Design decision needed

If I understood you correctly, the patch I just attached would resolve your problem. Could you confirm?


The fact that staff_member_required is an undocumented function purely designed for internal use by the admin and admindocs contrib apps still stands.

It contains hardcoded values that make it unsuitable for general use:

            'template_name': 'admin/login.html',
            'authentication_form': AdminAuthenticationForm,

I'm not sure the patch I uploaded wouldn't have side effects, and I'm reluctant to change this function, since it appears to work correctly within the admin and admindocs contrib apps.

If you want a different behavior, you can always implement your own version of this decorator.

comment:5 Changed 3 years ago by aaugustin

  • Component changed from HTTP handling to contrib.admin

comment:6 Changed 3 years ago by aaugustin

  • Resolution set to wontfix
  • Status changed from reopened to closed

I checked on IRC with the last developer who edited this piece of code, here's what he says:

while there is a next parameter in the extra_context, that's purely for rendering the form correctly

I don't think we should use the extra_context for the next parameter, as well

seems kinda mixed up

it's basically a feature request afaiu for a private api

Closing for these reasons.

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.