logout view of authentication broken

[Terrell Smith]

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)

[Karen Tracey]

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?

[Terrell Smith]

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.

[anonymous]

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.

[Brandon M Height]

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

[Brandon M Height]

Patch w/ Tests

[Brandon M Height]

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.

[Jannis Leidel]

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.

[Jannis Leidel]

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).