#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)
Change History (12)
follow-up: 2 comment:1 by , 15 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 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 , 15 years ago
Cc: | added |
---|
comment:4 by , 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 , 14 years ago
Attachment: | views.py.diff added |
---|
follow-up: 8 comment:5 by , 14 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 , 14 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:7 by , 14 years ago
Needs tests: | unset |
---|
comment:8 by , 14 years ago
Cc: | 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.
Not sure whether it is the doc or the code that needs to be fixed here.
redirect_field_name
for thelogout
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 whennext_page
is None. I don't see any great advantage to changing the implementation to what the doc says?