Opened 15 years ago

Closed 13 years ago

Last modified 13 years ago

#11223 closed (fixed)

logout view of authentication broken

Reported by: Terrell Smith Owned by: Brandon M Height
Component: contrib.auth Version: dev
Severity: Keywords:
Cc: miracle2k, bmheight@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

In the documentation for the authentication logout view it says that the redirect_field_name, which defaults to 'next', will override the value for next_page. This is not the case. The redirect_field_name works fine when next_page is not defined, but if next_page is defined then its value we be used even if you pass in a value with the redirect_field_name.

For example the following works fine and correctly redirect to http://www.example.com/foo/

route: url(r'^logout/$', 'django.contrib.auth.views.logout', {} , name='logout')
or
route: url(r'^logout/$', 'django.contrib.auth.views.logout', {'redirect_field_name': 'next'}, name='logout')
and
url: http://www.example.com/logout/?next=/foo/

But this doesn't and redirects the http://www.example.com/

route: url(r'^logout/$', 'django.contrib.auth.views.logout', {'next_page': '/'}, name='logout')
or
route: url(r'^logout/$', 'django.contrib.auth.views.logout', {'next_page': '/', 'redirect_field_name': 'next'}, name='logout')
and
url: http://www.example.com/logout/?next=/foo/

Shouldn't the function in '.../django/contrib/auth/views.py' be something like.

def logout(request, next_page=None, template_name='registration/logged_out.html', redirect_field_name=REDIRECT_FIELD_NAME):
    "Logs out the user and displays 'You are logged out' message."
    from django.contrib.auth import logout
    logout(request)
    redirect_to = request.REQUEST.get(redirect_field_name, '')
    if redirect_to:
        return HttpResponseRedirect(redirect_to)
    elif next_page is None:
        return render_to_response(template_name, 
                                  {'title': _('Logged out')}, 
                                  context_instance=RequestContext(request))
    else:
        # Redirect to this page until the session has been cleared.
        return HttpResponseRedirect(next_page or request.path)

Attachments (2)

views.py.diff (1.6 KB ) - added by Brandon M Height 13 years ago.
views_with_tests.diff (2.6 KB ) - added by Brandon M Height 13 years ago.
Patch w/ Tests

Download all attachments as: .zip

Change History (12)

comment:1 by Karen Tracey, 15 years ago

Triage Stage: UnreviewedAccepted

Not sure whether it is the doc or the code that needs to be fixed here. redirect_field_name for the logout view was added in r10332. The documentation was added in r10371 without, as near as I can tell, a ticket (r10332 fixed #10460, a brief scan of the tickets with higher numbers than that listed as being fixed by r10371 doesn't show any that mention auth logout, unless I missed it). I don't know where that doc came from but it doesn't match the code.

It would seem to be simpler and less risky at this point to just change the doc to reflect the code, and say redirect_field_name is only used when next_page is None. I don't see any great advantage to changing the implementation to what the doc says?

in reply to:  1 comment:2 by Terrell Smith, 15 years ago

The reason I ran into this bug is because I wanted the documentated implementation. I wanted my logout to redirect to the value of 'next_page', in this case my home page, if no value for 'next' is passed through the url instead of going to a logout template. I understand that I could define a different logout template and code a redirection there, but that would be a much like DRY approach. Ticket #10460 wanted to make the logout work more like the login view and right now it doesn't since 'redirect_field_name' doesn't override 'next_page' unlike the login where 'redirect_field_name' overrides settings.LOGIN_REDIRECT_URL.

comment:3 by miracle2k, 14 years ago

Cc: miracle2k added

comment:4 by anonymous, 14 years ago

I was about to submit the same bug. I think the right way is the one documented and proposed in this ticket.This will solve a lot of problems. Ticket #12405 is related to the same problem, although with a different approach. I think the best is to chose one of the proposed solution and solve this problem.

by Brandon M Height, 13 years ago

Attachment: views.py.diff added

comment:5 by Brandon M Height, 13 years ago

Has patch: set
Needs documentation: set
Needs tests: set

Added a patch to resolve the issue but it stills needs tests written as well as the Documentation modification.

comment:6 by Brandon M Height, 13 years ago

Owner: changed from nobody to Brandon M Height
Status: newassigned

by Brandon M Height, 13 years ago

Attachment: views_with_tests.diff added

Patch w/ Tests

comment:7 by Brandon M Height, 13 years ago

Needs tests: unset

in reply to:  5 comment:8 by Brandon M Height, 13 years ago

Cc: bmheight@… added
Needs documentation: unset

Replying to lasko:

Added a patch to resolve the issue but it stills needs tests written as well as the Documentation modification.

The attached patch/test implements the documentation as described.
http://docs.djangoproject.com/en/dev/topics/auth/#django.contrib.auth.views.logout

The behavior implements redirect_field_name overriding next_page if the given GET parameter is passed.
No new documentation should be needed.
I have verified the patch against my build and the tests pass with flying colors.

Someone please review the patch and tests and then we can move forward with this.

comment:9 by Jannis Leidel, 13 years ago

Resolution: fixed
Status: assignedclosed

In [15706]:

Fixed #11223 -- Fixed logout view to use the 'next' GET parameter correctly as described in the docs, while only allowing redirection to the same host.

comment:10 by Jannis Leidel, 13 years ago

In [15707]:

[1.2.X] Fixed #11223 -- Fixed logout view to use the 'next' GET parameter correctly as described in the docs, while only allowing redirection to the same host.

Backport from trunk (r15706).

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