Fixed #15619 – Added CSRF protection in logout view.

[KrzysiekJ]

Logout is now performed only for POST requests.
GET requests return confirmation page.

Based on patches from ashchristopher and vzima.

https://code.djangoproject.com/ticket/15619

[ashchristopher]

I originally wrote this patch, but was lost in the transition to GitHub (my bad) - one thing that I think we should have is to do POST via javascript so that currently functionality is maintained - if JS is not enabled, or the link is opened in a new tab, then the confirmation page is shown.

Just my thoughts.

[KrzysiekJ]

I don’t think that punishing users for not enabling JavaScript is a good thing, so we should probably leave the plain HTML form by default and replace it with a link when JavaScript is enabled. However the only advantage we would gain is that users would be able to log out in a new tab, which seems to be a rather uncommon use case. In most cases, when user logs out, he doesn’t want to leave open tabs showing what he was doing before.

Also the inability to submit a form to a new tab is a general issue which maybe, if at all, should be solved at the browser level. I don’t see any reason (except some sort of backwards compatibility) why we should make a workaround only in this particular place. For example, why not allow submitting search form in admin to a new tab? So, unless some of the core devs express their support, I’m probably not going to implement this.

[ziima]

I finally finished my rebase, but you were a bit faster #1963

It differs in a few things we should merge, on of which is missed to fix the logout link in password_change_* templates.

[ziima]

As for JS, I'm also for logout form. I only have one comment which is that logout button shouldn't look like a link. I hate elements which looks like a link but can't be opened in a new tab.

[KrzysiekJ]

I’ve updated the patch and incorporated your changes to the password_change_* templates. There are still a few differences – for example, I’ve removed dummy “post” input from the form and have just got rid of XHTML-style backslashes in input tags. I’ve also changed “logout” (noun) in the submit button to “log out” (verb).

As for the logout button look, styling it differently will probably require some design decisions, so maybe it would be better to do it via a separate ticket.

[ziima]

It looks like a logout button style will be an issue. I propose to split this patch into two. This one which changes the logout function and second which changes the logout link. It would be troubling if this patch got stuck because of style of logout link/button.

For the patch: You should check the diff in docs. I'm not very experienced in writing docs for django, but the changes in AdminSite should be mentioned as well. You haven't also fixed the arguments of logout function in docs. At last, I'm not sure how the information about changes between versions works, but I tried to note them.

[KrzysiekJ]

Thanks for the review, I’ve added the missing documentation. About the version notifications: I’m not really sure whether they add more value or noise in this case, so I did not add them. However I may do it if they get more support.

[collinanderson]

https://groups.google.com/d/topic/django-developers/MmFzCq8oB5I/discussion

[timgraham]

Please send a new PR if someone can update this and include the concerns mentioned in the mailing list thread that Collin linked above.