Deprecating logout via GET

[René Fleschenberg]

Hi everyone,

there seems to be consensus that logging the client out on GET requests
to the logout view is not great. Clients may try to prefetch links (this
came up on IRC today). Attackers might annoy users by logging them out
with embedded links to the logout URL.

The reason this was not changed yet is backwards compatibility.

I'd like to propose deprecating logging out via GET with the usual
deprecation cycle, so that we have the possibility to stop supporting it
in Django 4.0.

In my opinion, this is a fairly easy change for Django users to make
upon a Django update. Some changes to Django itself will also have to be
made (the admin templates), but I think it should be possible to do
those before the 4.0 release. And even if that doesn't happen, emitting
a DeprecationWarning in 3.1+ wouldn't harm, right?

What do you think?

I went ahead and created a PR for this:
https://github.com/django/django/pull/12504

--
René Fleschenberg

[Adam Johnson]

 > The reason this was not changed yet is backwards compatibility.

Do you have any mailing list / ticket links as reference?

It should be noted that the popular allauth already doesn’t allow logout by GET (by default). 

Personally I’m in favour.

--
You received this message because you are subscribed to the Google Groups "Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/9baf0189-fc7a-80a0-6543-559c2b2b186a%40fleschenberg.net.

--

Adam

[René Fleschenberg]

Hi,

On 2/28/20 12:12 AM, Adam Johnson wrote:
>  > The reason this was not changed yet is backwards compatibility.
>
> Do you have any mailing list / ticket links as reference?

Sorry, I forgot to link them here. The main ticket seems to be
https://code.djangoproject.com/ticket/15619.

Mailing list dicussions are at
https://groups.google.com/forum/#!topic/django-developers/MmFzCq8oB5I/discussion
and
https://groups.google.com/forum/#!topic/django-developers/ax95u_f82D4/discussion.

--
René Fleschenberg

[Adam Johnson]

Wow and first ticket referenced in one of those conversations was 12 years ago: https://code.djangoproject.com/ticket/7989

Seems like a of a no-brainer at this point with general support from other core devs in the past :)

--
You received this message because you are subscribed to the Google Groups "Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/f6c73570-5276-d789-db00-c162457ac5bb%40fleschenberg.net.

--

Adam

[Maher, Brian]

I’m on the other side of the fence – I don’t think this is a necessary change.

 

The examples given in historic tickets are not worthy of a change like this in my opinion. For example, take the example regarding a bot crawling a website, and losing its session by crawling the logout link – if you were crawling any website using a bot with an authenticated session, you would instruct the bot to not crawl the logout links. In 15 years of web development, I’ve yet to ever find a bot using an authenticated session.

 

Are any current browsers dumb enough to prefetch logout links these days? I would assume that most prefetch algorithms are smart enough to not pre-fetch these.

 

I have also seen the argument floated around that it’s not “correctly restful” to modify the state (session data) via a GET request. I’d say that sessions themselves are not restful by nature.

 

I just don’t see the benefit, in return for breaking practically every logout button on every installation of Django around.

 

B

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAMyDDM3r_68yowcjGnr9kBPSFwcfHnP6VKgsmuBsBfk3c7rJ%3Dg%40mail.gmail.com.

[Florian Apolloner]

On Friday, February 28, 2020 at 5:08:07 PM UTC+1, Maher, Brian wrote:

Are any current browsers dumb enough to prefetch logout links these days? I would assume that most prefetch algorithms are smart enough to not pre-fetch these.

Not sure what heuristics browsers use.

I have also seen the argument floated around that it’s not “correctly restful” to modify the state (session data) via a GET request. I’d say that sessions themselves are not restful by nature.

It has nothing do to with restful, logout by GET is not CSRF protected…

I just don’t see the benefit, in return for breaking practically every logout button on every installation of Django around.

Not necessarily, if you GET the logout page an intermediary page can be displayed (though that would need a new template).

<img src="some_page/admin/logout"/> is currently a perfectly nice way to perform a logout against your django installation if you are viewing this google group in the same browser ;)

[René Fleschenberg]

Hi,

On 2/28/20 5:04 PM, 'Maher, Brian' via Django developers (Contributions

to Django itself) wrote:
> Are any current browsers dumb enough to prefetch logout links these
> days? I would assume that most prefetch algorithms are smart enough to
> not pre-fetch these.

We not only have to consider browsers, but also current and future
frontend frameworks.

I don't know how often people run into this in practice, but yesterday
someone mentioned on IRC that they got caught by this while
experimenting with some JS preloading. I also vaguely remember having
seen reports on similar issues by others over the years (maybe not all
of them for Django, though).

You can also find tickets like
https://bugs.chromium.org/p/chromium/issues/detail?id=86175. To sum up,
I don't think this is a purely academical problem.

> I just don’t see the benefit, in return for breaking practically every
> logout button on every installation of Django around.

If I write a logout button myself, I use POST nowadays. The logout view
has supported POST for a while now. But I don't know how widespread POST
vs GET is for this in user code overall.

I don't think this is a particularly painful adjustment to make on a
major version upgrade. Maybe we can provide a specific example in the
release notes. IMO, in the end it is better to make users go through
this at some point in time than to maintain the undesired behavior forever.

--
René Fleschenberg

[אורי‎]

I'm interested: Google, Gmail, Facebook, Instagram, Twitter: How do they use logout? POST or GET?

אורי

u...@speedy.net

--
You received this message because you are subscribed to the Google Groups "Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/9baf0189-fc7a-80a0-6543-559c2b2b186a%40fleschenberg.net.

[Adam Johnson]

Google (=Gmail): GET, but with a security token in the URL

Facebook: POST

Instagram: POST

Twitter: POST

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CABD5YeHh%2BNvu4vtwaD8pC9526bAVpyA1iwo94c5bVSjRUa5eNg%40mail.gmail.com.

--

Adam

[Florian Apolloner]

I found an example on stackoverflow on how we could do this in the admin without JS (with a bit of styling): https://stackoverflow.com/a/33880971 -- I personally would prefer it if we would not need javascript for a fundamental functionality like this.

To unsubscribe from this group and stop receiving emails from it, send an email to django-d...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/9baf0189-fc7a-80a0-6543-559c2b2b186a%40fleschenberg.net.

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.

To unsubscribe from this group and stop receiving emails from it, send an email to django-d...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CABD5YeHh%2BNvu4vtwaD8pC9526bAVpyA1iwo94c5bVSjRUa5eNg%40mail.gmail.com.

--

Adam

[René Fleschenberg]

Hi,

On 2/29/20 5:01 PM, Florian Apolloner wrote:
> I found an example on stackoverflow on how we could do this in the admin
> without JS (with a bit of styling): https://stackoverflow.com/a/33880971
> -- I personally would prefer it if we would not need javascript for a
> fundamental functionality like this.

I found the same example and based
https://github.com/django/django/pull/12505 on it :) If anyone has some
time to review / test it, that would be great.

--
René Fleschenberg

[אורי‎]

Yes, but then hovering on the link doesn't show the logout URL at the bottom of the screen.

אורי

u...@speedy.net

To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/2598e4a0-1b0d-4d34-a807-2b243af72053%40googlegroups.com.

[Adam Johnson]

Yes, but then hovering on the link doesn't show the logout URL at the bottom of the screen.

I don't think this is a concern.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CABD5YeFddZRuh%3DCtDbDD8ExNXDAfKbYL64XtY9_WMyQ8Pz2hLA%40mail.gmail.com.

--

Adam

[Aymeric Augustin]

Hello,

Le dim. 1 mars 2020 à 11:04, Adam Johnson <m...@adamj.eu> a écrit :

Yes, but then hovering on the link doesn't show the logout URL at the bottom of the screen.

I don't think this is a concern. 

If it's just the link preview, yes, I think we can make the trade off.

The more general concern here is accessibility. It would be nice to experiment and make sure the new implementation is usable with a screen reader (for example).

 

--

Aymeric.

[Anna Sidwell]

Is there any particular reason why it shouldn't look like a button instead of a link?

Anna

--

You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.

To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CANE-7mX2vujJQmfeAd1DbS3D3kfj8fwSKTvZSPfvoNJCKRzQ0A%40mail.gmail.com.

[Tim Chase]

On 2020-03-02 18:35, Anna Sidwell wrote:
> Is there any particular reason why it shouldn't look like a button
> instead of a link?

The concern isn't how it looks (with CSS you can make a button look
like a link, or make a link look like a button).

An <a href="…"> does the logout action via a GET (and is the problem
at hand) while a <button>/<submit> does the logout action via the
form method and can be either a GET (which would still a problem) or a
POST (the goal).

-tkc

[René Fleschenberg]

Hi,

I think the question was if we want to retain the current link-like
appearance on changing this to <button> / POST ;)

The reason I see for doing so is that it blends in with the other
user-tools links (view site, change password). But I think Anna raises a
good point. If someone can come up with a nice button-like design, why
not? Maybe a button-like design would actually be a better UI here?

--
René Fleschenberg

[Sam Willis]

Why not have the logout link take the user to a page asking them to confirm the logout, and have it as a POSTed form button from there?

That adds a helpful confirmation page, removes the difficulties of styling a button as a link constantly (or changing the header design to a button).

One downside would be the change in behaviour, people will be used to the logout link immediately logging them out and so may not read the next page...

I’m sure there is literature in best practices for logout confirmation.

[Adam Johnson]

If your suggestion is limited to the admin, I think it would be fine, but it's not necessary. But I don't think there is a compelling reason - there aren't any difficulties with the CSS since Rene has already written it.

If your suggestion is for all logout views, there's no way to enforce it, and anyway we don't want to require users to implement a new view in order to upgrade Django. That can turn a "backend team" task into needing a design and copy writing. We should document the possibility though.

--
You received this message because you are subscribed to the Google Groups "Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/0161eeed-7f9a-4312-9ed5-fc2bafde64ee%40googlegroups.com.

--

Adam

[אורי‎]

Users don't need to confirm a logout. Confirmation is usually when deleting a profile or making something irreversible. Logging out is reversible and therefore doesn't need to be confirmed. Just clicking "logout" should log the user out - whether a regular user or an admin.

אורי

u...@speedy.net

--
You received this message because you are subscribed to the Google Groups "Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/0161eeed-7f9a-4312-9ed5-fc2bafde64ee%40googlegroups.com.

[René Fleschenberg]

Hi,

On 04.03.20 12:13, Sam Willis wrote:
> Why not have the logout link take the user to a page asking them to confirm the logout, and have it as a POSTed form button from there?
>
> That adds a helpful confirmation page, removes the difficulties of styling a button as a link constantly (or changing the header design to a button).
>
> One downside would be the change in behaviour, people will be used to the logout link immediately logging them out and so may not read the next page...

Personally I am fine with an intermediate page or without one.

It would just be nice to reach some decision on this so we can then move
forward with https://github.com/django/django/pull/12505, or file
another PR that introduces the intermediate page.

I agree with Aymeric that some accessibility testing on this would be
good, however.

Regards,
René

[Florian Apolloner]

UX wise the intermediary page is annoying. So if we can just provide some docs and upgrade notes I'd be fine with just dropping the get part.