[Django] #33522: Unexpected behaviour when logging in
Django
-------------------------------------+-------------------------------------
Reporter: Michael | Owner: nobody
Type: Bug | Status: new
Component: | Version: 4.0
contrib.auth | Keywords: authentication log
Severity: Normal | in CSRF token
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Say a user opens two tabs to the same site, both show the login page.
The user logs in on Tab A, does some things, after a while changes to Tab
B, sees the login page, and instictively clicks login (their details pre-
populated due to auto fill). They will then get a surprising CSRF error.
I thing this is because when they logged in on Tab A, it deleted the
anonymous session, and replaced it with a new session, which means the
CSRF token from Tab B fails.
Would it not be better to not delete the anomyous session when one logs
in, or upgrade the session (instead of deleting it)?
--
Ticket URL: <https://code.djangoproject.com/ticket/33522>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: Michael | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: authentication log | Triage Stage:
in CSRF token | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
> Say a user opens two tabs to the same site, both show the login page.
>
> The user logs in on Tab A, does some things, after a while changes to Tab
> B, sees the login page, and instictively clicks login (their details pre-
> populated due to auto fill). They will then get a surprising CSRF error.
>
> I thing this is because when they logged in on Tab A, it deleted the
> anonymous session, and replaced it with a new session, which means the
> CSRF token from Tab B fails.
>
> Would it not be better to not delete the anomyous session when one logs
> in, or upgrade the session (instead of deleting it)?
New description:
Say a user opens two tabs to the same site, both show the login page.
The user logs in on Tab A, does some things, after a while changes to Tab
B, sees the login page, and instictively clicks login (their details pre-
populated due to auto fill). They will then get a surprising CSRF error.
I thing this is because when they logged in on Tab A, it deleted the
anonymous session, and replaced it with a new session, which means the
CSRF token from Tab B fails.
Would it not be better to not delete the anomyous session when one logs
in, or upgrade the session (instead of deleting it)?
Another idea is making the login view `csrf_exempt`, then we won't have
these csrf errors.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/33522#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: Michael | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: authentication log | Triage Stage:
in CSRF token | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
> Say a user opens two tabs to the same site, both show the login page.
>
> The user logs in on Tab A, does some things, after a while changes to Tab
> B, sees the login page, and instictively clicks login (their details pre-
> populated due to auto fill). They will then get a surprising CSRF error.
>
> I thing this is because when they logged in on Tab A, it deleted the
> anonymous session, and replaced it with a new session, which means the
> CSRF token from Tab B fails.
>
> Would it not be better to not delete the anomyous session when one logs
> in, or upgrade the session (instead of deleting it)?
>
> Another idea is making the login view `csrf_exempt`, then we won't have
> these csrf errors.
New description:
Say a user opens two tabs to the same site, both show the login page.
The user logs in on Tab A, does some things, after a while changes to Tab
B, sees the login page, and instictively clicks login (their details pre-
populated due to auto fill). They will then get a surprising CSRF error.
I thing this is because when they logged in on Tab A, it deleted the
anonymous session, and replaced it with a new session, which means the
CSRF token from Tab B fails.
Would it not be better to not delete the anomyous session when one logs
in, or upgrade the session (instead of deleting it)?
--
--
Ticket URL: <https://code.djangoproject.com/ticket/33522#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: Michael | Owner: nobody
Component: contrib.auth | Version: 4.0
Severity: Normal | Resolution:
Keywords: authentication log | Triage Stage:
in CSRF token | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* type: Bug => New feature
--
Ticket URL: <https://code.djangoproject.com/ticket/33522#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: Michael | Owner: nobody
Component: contrib.auth | Version: 4.0
Severity: Normal | Resolution: invalid
Keywords: authentication log | Triage Stage:
in CSRF token | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => invalid
Comment:
It seems like pre-login session token, and creating a new session is
required for security due to my research, so this suggestion is invalid.
I think the solution is to create a custom CSRF view that if there is a
CSRF failure, you redirect the user to the page they want to see in the
first place, and it will now work cause they already logged in.
--
Ticket URL: <https://code.djangoproject.com/ticket/33522#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Michael | Owner: nobody
Component: contrib.auth | Version: 4.0
Severity: Normal | Resolution: invalid
Keywords: authentication log | Triage Stage:
in CSRF token | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Michael):
Here is the solution I posted if anyone else has this issue:
https://stackoverflow.com/a/71174489/5506400
--
Ticket URL: <https://code.djangoproject.com/ticket/33522#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: Michael | Owner: nobody
Component: contrib.auth | Version: 4.0
Severity: Normal | Resolution:
in CSRF token | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: closed => new
* resolution: invalid =>
Comment:
Thinking about this some more, what if the CSRF middleware got the token
from the cookie, instead of from the form data? This would solve the CSRF
errors that pop up every now and then.
--
Ticket URL: <https://code.djangoproject.com/ticket/33522#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Michael | Owner: nobody
Component: contrib.auth | Version: 4.0
Severity: Normal | Resolution:
Keywords: authentication log | Triage Stage:
in CSRF token | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Florian Apolloner, Shai Berger (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/33522#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Michael | Owner: nobody
Component: contrib.auth | Version: 4.0
Severity: Normal | Resolution: invalid
Keywords: authentication log | Triage Stage:
in CSRF token | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => invalid
Comment:
Others can comment, but this looks like exactly expected behaviour to me.
I'm not sure we can, or it's worth trying to work around.
I don't see that we're ever going to be able to update forms rendered
before a login with a fresh CSRF token. (Some JS maybe, but it's not clear
that's going to be 100% safe ever.)
> ... what if the CSRF middleware got the token from the cookie, instead
of from the form data?
This would totally defeat the point no? I get you to submit a form from my
evil site to yours; your browser sends the cookie; the only thing that
saves you is that I don't know the right token to add to the form data.
--
Ticket URL: <https://code.djangoproject.com/ticket/33522#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: Michael | Owner: nobody
Component: contrib.auth | Version: 4.0
Severity: Normal | Resolution: invalid
Keywords: authentication log | Triage Stage:
in CSRF token | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Florian Apolloner):
> Thinking about this some more, what if the CSRF middleware got the token
from the cookie, instead of from the form data? This would solve the CSRF
errors that pop up every now and then.
Then it would compare the cookie against the cookie providing no security
:) It has to compare the cookie (something the attacker cannot control) to
the POST data (or header as fallback).
--
Ticket URL: <https://code.djangoproject.com/ticket/33522#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: Michael | Owner: nobody
Component: contrib.auth | Version: 4.0
Severity: Normal | Resolution: invalid
Keywords: authentication log | Triage Stage:
in CSRF token | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Michael):
Sorry yes that would be pointless.
With the "SameSite: Lax", and some JavaScript to set the X-CSRFToken
header, and not use GZIP for HTML responses (I think the masked CSRF Token
is to counter BREACH attacks), I think it would mitigate CSRF token
errors, and be just as secure. I understand the Django admin site is not
keen on JavaScript, so its up to custom code if one prefers that.
--
Ticket URL: <https://code.djangoproject.com/ticket/33522#comment:10>