[Django] #28699: Problem with CSRF in Django 1.11.6
Django
-----------------------------------------+------------------------
Reporter: stephanm | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 1.11
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------+------------------------
I have a problem with csrf protection starting with django 1.11.6
(django 1.11.5 has not this problem).
I am doing all time exactly what is explained in
https://docs.djangoproject.com/en/1.11/howto/auth-remote-user/
My settings:
{{{#!python
MIDDLEWARE = [
"django.contrib.sessions.middleware.SessionMiddleware",
"django.middleware.locale.LocaleMiddleware",
"django.middleware.common.CommonMiddleware",
"django.middleware.csrf.CsrfViewMiddleware",
"django.contrib.auth.middleware.AuthenticationMiddleware",
# "django.contrib.auth.middleware.RemoteUserMiddleware",
# own middleware because behind proxy we get HTTP_REMOTE_USER
# instead of REMOTE_USER
"lib.auth.middleware.RemoteUserMiddlewareProxy",
"django.contrib.messages.middleware.MessageMiddleware",
"django.middleware.clickjacking.XFrameOptionsMiddleware",
]
AUTHENTICATION_BACKENDS = [
# "django.contrib.auth.backends.RemoteUserBackend",
"lib.auth.backends.RemoteUserBackendTooling",
# default is:
"django.contrib.auth.backends.ModelBackend",
]
}}}
{{{#!python
# content of lib.auth.middleware.RemoteUserMiddlewareProxy
from django.contrib.auth.middleware import RemoteUserMiddleware
class RemoteUserMiddlewareProxy(RemoteUserMiddleware):
header = "HTTP_REMOTE_USER"
}}}
{{{#!python
# content of lib.auth.backends.RemoteUserBackendTooling
from django.contrib.auth.backends import RemoteUserBackend
class RemoteUserBackendTooling(RemoteUserBackend):
create_unknown_user = False
def clean_username(self, username):
"""
Performs any cleaning on the "username" prior to using it to get
or
create the user object. Returns the cleaned username.
By default, returns the username unchanged.
"""
if username.startswith("IT\\"):
username = username[3:]
return username
"
}}}
My C# Application does a login by using the normal
"django.contrib.auth.backends.ModelBackend", not using the REMOTE_USER !
It calls a function in my views.py:
{{{#!python
def auth_login_json(request):
# code...
# POST data with user & password
}}}
Now the csrf protection fails with an http error 403
(only with django 1.11.6, ... django 1.11.5 works)
I found two possibilities to make it work again:
1. In MIDDLEWARE, comment out
"lib.auth.middleware.RemoteUserMiddlewareProxy"
but the other remote user login functionality is gone.
2. I add @csrf_exempt to auth_login_json function like this:
{{{#!python
@csrf_exempt
def auth_login_json(request):
# code...
# POST data with user & password
}}}
Reading the changelog
https://docs.djangoproject.com/en/1.11/releases/1.11.6/
I suppose this behaviour change comes with
https://code.djangoproject.com/ticket/28488
My question: Was I wrong all the years or is this a bug?
--
Ticket URL: <https://code.djangoproject.com/ticket/28699>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: stephanm | Owner: nobody
Type: Bug | Status: new
Component: CSRF | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* cc: Florian Apolloner (added)
* type: Uncategorized => Bug
* component: Uncategorized => CSRF
--
Ticket URL: <https://code.djangoproject.com/ticket/28699#comment:1>
Django
Reporter: stephanm | Owner: nobody
Type: Bug | Status: new
Component: CSRF | Version: 1.11
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Florian Apolloner):
Can your share your code/setup? I do not see anything obvious -- your C#
app should always have gotten an CSRF error, or did it include a csrf
token?
--
Ticket URL: <https://code.djangoproject.com/ticket/28699#comment:2>
Django
Reporter: stephanm | Owner: nobody
Type: Bug | Status: new
Component: CSRF | Version: 1.11
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Florian Apolloner):
Actually I might have an idea, can you check if commenting out
https://github.com/django/django/blob/4d60261b2a77460b4c127c3d832518b95e11a0ac/django/contrib/auth/__init__.py#L128
fixes the issue? This seems to be caused by the `auth.login` call from the
RemoteUserMiddleware which then resets tokens :/
--
Ticket URL: <https://code.djangoproject.com/ticket/28699#comment:3>
Django
Reporter: stephanm | Owner: nobody
Type: Bug | Status: new
Component: CSRF | Version: 1.11
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by stephanm):
Hi Florian,
just commented out the "rotate_token(request)" line in login as you told
me.
**Now it works again.**
Perhaps I am doing something wrong too, I didn't understand exactly the
csrf workflow.
I use Apache on Windows with a plugin which allows me to use NTLM as
Single Sign On.
My django runs as reverse proxy and gets the remote_user from apache,
which is intended for the normal users which come with their browsers.
But my c# application does a normal login.
Is there some howto explainig how an external program c#
should login, showing when and how the csrf tokens
appears in the cookies during the HTTP conversation
and what of them should be taken?
Thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/28699#comment:4>
Django
Reporter: stephanm | Owner: nobody
Type: Bug | Status: new
Component: CSRF | Version: 1.11
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Florian Apolloner):
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted
Comment:
Ok, thanks -- given this I can reproduce it. This is a bug in Django
(kinda), but probably a hard one to fix :(
--
Ticket URL: <https://code.djangoproject.com/ticket/28699#comment:5>
Django
Reporter: stephanm | Owner: nobody
Type: Bug | Status: new
Component: CSRF | Version: 1.11
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Florian Apolloner):
Actually I am still not sure what and why is happening here. How does your
C# app login exactly? Ie where from does it get the csrf token and is the
C# app affected by the single sign on stuff?
--
Ticket URL: <https://code.djangoproject.com/ticket/28699#comment:6>
Django
Reporter: stephanm | Owner: nobody
Type: Bug | Status: new
Component: CSRF | Version: 1.11
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Florian Apolloner):
Please restore the original Django 1.11.6 and move
```
"django.contrib.auth.middleware.AuthenticationMiddleware",
"lib.auth.middleware.RemoteUserMiddlewareProxy",
```
before the CSRF middleware. The issue should be gone then, which will
probably mean that the fix will just be a documentation fix.
--
Ticket URL: <https://code.djangoproject.com/ticket/28699#comment:7>
Django
Reporter: stephanm | Owner: nobody
Type: Bug | Status: new
Component: CSRF | Version: 1.11
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by stephanm):
Hi,
I restored the original Django 1.11.6 and moved the lines you mentioned
*before* before the CSRF middleware and I can confirm that **it works
now**!
Concerning my C# Application, It calls the following
function in my views.py with a GET call to
get the carf_token:
{{{#!python
def auth_get_csrf_token_json(request):
token = csrf(request)
csrf_token = str(token["csrf_token"]) # ab django 1.5
response = JsonResponse({"dataType": "csrf", "data": {"csrftoken":
csrf_token}})
# I set the cookie in the past but it seems not necessary
##response.set_cookie("csrftoken", csrf_token)
return response
}}}
Note:
- I send back the csrf token in as json data but in
my C# app I use the csrf token which is in the **cookie**.
- One strange thing I didn't understand: the csrf token
in the returned json data and in the cookie are **different**
Honestly I was never sure where to get this **initial** csrf token
to be able to POST my login data.
So I did my experiments until I found this solution which worked for me
(some times ago).
--
Ticket URL: <https://code.djangoproject.com/ticket/28699#comment:8>
Django
Reporter: stephanm | Owner: nobody
Type: Bug | Status: new
Component: CSRF | Version: 1.11
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Florian Apolloner):
> One strange thing I didn't understand: the csrf token in the returned
json data and in the cookie are different
Yes, the token changes every request to account for BREACH style attacks.
you have to take the first half of it and xor it to the second one
(basically) to get the constant "secret" behind it which is reused during
the requests.
As for your code:
{{{
from django.middleware.csrf import get_token
get_token(request)
}}}
in your view should be enough, Django will take care of setting the cookie
etc accordingly.
--
Ticket URL: <https://code.djangoproject.com/ticket/28699#comment:9>
Django
Reporter: stephanm | Owner: nobody
Type: Bug | Status: new
Component: CSRF | Version: 1.11
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by stephanm):
Replying to [comment:7 Florian Apolloner]:
> ...
> before the CSRF middleware. The issue should be gone then, which will
probably mean that the fix will just be a documentation fix.
If the documentation fix is about to place AuthenticationMiddleware
before the CsrfViewMiddleware in the MIDDLEWARE setting then you
will have to do more than only changing the docs:
- You have to move AuthenticationMiddleware before the CsrfViewMiddleware
in the docs:\\
https://docs.djangoproject.com/en/1.11/topics/http/middleware
/#activating-middleware
- You will have to change: `django-admin startproject`
so that it generates the appropriate middleware ordering.
- You will have to change the middleware-ordering docs in:\\
https://docs.djangoproject.com/en/1.11/ref/middleware/#middleware-
ordering
- You will have to tell everybody that their settings.MIDDLEWARE has to
be modified, otherwise some functionality may be broken
- modify perhaps some other places in the docs i missed ...
Is it really only a documentation fix?
--
Ticket URL: <https://code.djangoproject.com/ticket/28699#comment:10>
Django
Reporter: stephanm | Owner: nobody
Type: Bug | Status: new
Component: CSRF | Version: 1.11
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Florian Apolloner):
It only affects the ''RemoteUserMiddleware'', which is not enabled by
default.
--
Ticket URL: <https://code.djangoproject.com/ticket/28699#comment:11>
Django
Reporter: stephanm | Owner: nobody
Type: Bug | Status: new
Component: CSRF | Version: 1.11
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by stephanm):
Aha ... so, the fix will be whats mentioned in comment:7,
the move of django.contrib.auth.middleware.AuthenticationMiddleware
and the RemoteUserMiddleware... ? (plus fixes in the docs of course)
Right? Or do you plan to do other changes?
I ask this, so I could fix my code **now**.
--
Ticket URL: <https://code.djangoproject.com/ticket/28699#comment:12>
Django
Reporter: stephanm | Owner: nobody
Type: Bug | Status: new
Component: CSRF | Version: 1.11
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Florian Apolloner):
Yes, something along those lines will be the final fix. I need to think
about it a bit more though, cannot gurantee if or what I missed.
--
Ticket URL: <https://code.djangoproject.com/ticket/28699#comment:13>
Django
1.11.6
-------------------------------+------------------------------------
Reporter: stephanm | Owner: nobody
Type: Bug | Status: new
Component: Documentation | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* component: CSRF => Documentation
* severity: Release blocker => Normal
--
Ticket URL: <https://code.djangoproject.com/ticket/28699#comment:14>
Django
1.11.6
-------------------------------+------------------------------------
Type: Bug | Status: assigned
Component: Documentation | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Rodrigo):
* owner: nobody => Rodrigo
* status: new => assigned
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/10722 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/28699#comment:15>
Django
1.11.6
-------------------------------+------------------------------------
Reporter: stephanm | Owner: Rodrigo
Type: Bug | Status: assigned
Component: Documentation | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Carlton Gibson):
The PR here looks fine, re the discussion, but I'm missing a crucial part
in my understanding here: I can't see how you're meant to make a
successful CSRF check in the **same request** as a `REMOTE_USER` login...
As I read it, if `RemoteUserMiddleware` is first then:
* `RemoteUserMiddleware.process_request()` calls `auth.login()` which
calls `rotate_token()`:
{{{
def rotate_token(request):
"""
Change the CSRF token in use for a request - should be done on login
for security purposes.
"""
request.META.update({
"CSRF_COOKIE_USED": True,
"CSRF_COOKIE": _get_new_csrf_token(),
})
request.csrf_cookie_needs_reset = True
}}}
* But then `CsrfViewMiddleware.process_request()` calls `_get_token()`
which fetches the **old** `CSRF_COOKIE`, from either the session or the
cookie, and
[https://github.com/django/django/blob/4d60261b2a77460b4c127c3d832518b95e11a0ac/django/middleware/csrf.py#L201
resets it on `request.META`].
Unless I missed something, this is **negating the `rotate_token()` call**.
Is that correct, or have I misread it? Given the docstring in
`rotate_token()` isn't this a no-no?
On the other hand, if `CsrfViewMiddleware` is first then the
`rotate_token()` call will replace whatever `CSRF_COOKIE` was previously
set, and so the actual CSRF check in `process_view()` will necessarily
fail. (As we must be seeing here.)
--
Ticket URL: <https://code.djangoproject.com/ticket/28699#comment:16>
Django
1.11.6
-------------------------------+------------------------------------
Reporter: stephanm | Owner: Rodrigo
Type: Bug | Status: assigned
Component: Documentation | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* cc: Carlton Gibson (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/28699#comment:17>
Django
1.11.6
-------------------------------+------------------------------------
Reporter: stephanm | Owner: Rodrigo
Type: Bug | Status: assigned
Component: Documentation | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Florian Apolloner):
Replying to [comment:16 Carlton Gibson]:
> Unless I missed something, this is **negating the `rotate_token()`
call**. Is that correct, or have I misread it? Given the docstring in
`rotate_token()` isn't this a no-no? If so, we can't recommend this.
Yes, this seems correct
> On the other hand, if `CsrfViewMiddleware` is first then the
`rotate_token()` call will replace whatever `CSRF_COOKIE` was previously
set, and so the actual CSRF check in `process_view()` will necessarily
fail. (As we must be seeing here.)
Also correct. I wonder if my patch in #28488 actually made the situation
worse (speaks for the complexity of the middleware :/)
--
Ticket URL: <https://code.djangoproject.com/ticket/28699#comment:18>
Django
1.11.6
-------------------------------+------------------------------------
Reporter: stephanm | Owner: Rodrigo
Type: Bug | Status: assigned
Component: Documentation | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* needs_better_patch: 0 => 1
Comment:
Okay. Thank you for following up Florian. I will dig dipper given your
confirmation of my first reading.
--
Ticket URL: <https://code.djangoproject.com/ticket/28699#comment:19>
Django
1.11.6
-------------------------------+------------------------------------
Reporter: stephanm | Owner: Rodrigo
Type: Bug | Status: assigned
Component: Documentation | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Comment (by Carlton Gibson):
Right, some time later...
I think that prior to c4c128d67c7dc2830631c6859a204c9d259f1fb1 (for
#28488) there was actually a security issue in the way
`CsrfViewMiddleware` worked.
It would call `_get_token()` in `process_view()`, resetting the CSRF token
to the one submitted in the request, even though `rotate_token()` had
previously been
called during `login()` by the `RemoteUserMiddleware`.
This is equivalent to the `CsrfViewMiddleware` second case discussed
above. It allows the Login + CSRF check in the single request but is not
safe.
As such, that the behaviour changed slightly cannot be considered a
regression. (It should never have worked.)
**Possibly** `RemoteUserMiddleware` could be adjusted to defer rotating
the CSRF token (until say `process_response()`), but anything in that
ball-park is highly sensitive, and probably not worth the price of
admission.
For this ticket I think documenting that remote user auth will require two
requests — one to login, on to submit further data passing CSRF — is the
best we can do.
--
Ticket URL: <https://code.djangoproject.com/ticket/28699#comment:20>
Django
requests.
-------------------------------+------------------------------------
Reporter: stephanm | Owner: Rodrigo
Type: Bug | Status: assigned
Component: Documentation | Version: 1.11
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* keywords: => remote user
* needs_better_patch: 1 => 0
* has_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/28699#comment:21>
Django
requests.
-------------------------------+------------------------------------
Reporter: stephanm | Owner: Rodrigo
Type: Bug | Status: assigned
Component: Documentation | Version: 1.11
Severity: Normal | Resolution:
Keywords: remote user | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Carlton Gibson):
> For this ticket I think documenting that remote user auth will require
two requests — one to login, on to submit further data passing CSRF — is
the best we can do.
Actually, I'm not exactly sure what to say here. Thinking about it,
exactly the same considerations apply to all login. You'd have to take
special measures to login a user and submit additional form data, whilst
also checking CSRF, in a single request, even if you were using session
based authentication with the model backend. (You'd write a view to do it,
manually calling `login()` yourself...)
I'm kind of inclined towards `wontfix` for that reason...
--
Ticket URL: <https://code.djangoproject.com/ticket/28699#comment:22>
Django
requests.
-------------------------------+------------------------------------
Reporter: stephanm | Owner: Rodrigo
Type: Bug | Status: assigned
Component: Documentation | Version: 1.11
Severity: Normal | Resolution:
Keywords: remote user | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Rodrigo):
Replying to [comment:22 Carlton Gibson]:
> > For this ticket I think documenting that remote user auth will require
two requests — one to login, on to submit further data passing CSRF — is
the best we can do.
>
> Actually, I'm not exactly sure what to say here. Thinking about it,
exactly the same considerations apply to all login. You'd have to take
special measures to login a user and submit additional form data, whilst
also checking CSRF, in a single request, even if you were using session
based authentication with the model backend. (You'd write a view to do it,
manually calling `login()` yourself...)
>
> I'm kind of inclined towards `wontfix` for that reason...
I am not sure if I am understanding, there will be always one extra
request for getting the CSRF token before POSTing anything. The
rotate_token() is called only by login(), the RemoteUserMiddleware calls
it only when user is not auth'ed (otherwise it just return) - which as you
said before, it will be negated later by CSRF.process_view().
If this would have worked as expected, the documentation should say
something like "After a login, the client needs to update the CSRF one
more time to keep posting in further requests - you may return it in your
login view to save one request", which applies to all logins - as you
said.
In this case, as it is overridden, there is no need, because the view
would be executed anyway and the data procesed - though without the
rotate_token() protection.
A fix for this that comes into my mind would be a "hook" on login to
deffer the token rotation and set a "user_has_logged" flag if REMOTE_USER
is enabled and then catch it later - as you said - to trigger the
rotation. This way it would behave as other backends.
Otherwise, it should be documented that token rotation on login is not
functioning for the REMOTE_USER backend, so beware! :)
--
Ticket URL: <https://code.djangoproject.com/ticket/28699#comment:23>
Django
requests.
-------------------------------+------------------------------------
Reporter: stephanm | Owner: Rodrigo
Type: Bug | Status: assigned
Component: Documentation | Version: 1.11
Severity: Normal | Resolution:
Keywords: remote user | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"94469504706b494877b6bb45a979bcb81c7fd7be" 94469504]:
{{{
#!CommitTicketReference repository=""
revision="94469504706b494877b6bb45a979bcb81c7fd7be"
Refs #28699 -- Clarified CSRF middleware ordering in relation to
RemoteUserMiddleware.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28699#comment:24>
Django
requests.
-------------------------------+------------------------------------
Reporter: stephanm | Owner: Rodrigo
Type: Bug | Status: assigned
Component: Documentation | Version: 1.11
Severity: Normal | Resolution:
Keywords: remote user | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"b0b98fcacf929ed190b2fa5fb56f595d4fc518ff" b0b98fca]:
{{{
#!CommitTicketReference repository=""
revision="b0b98fcacf929ed190b2fa5fb56f595d4fc518ff"
[3.0.x] Refs #28699 -- Clarified CSRF middleware ordering in relation to
RemoteUserMiddleware.
Backport of 94469504706b494877b6bb45a979bcb81c7fd7be from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28699#comment:25>
Django
requests.
-------------------------------+------------------------------------
Reporter: stephanm | Owner: Rodrigo
Type: Bug | Status: assigned
Component: Documentation | Version: 1.11
Severity: Normal | Resolution:
Keywords: remote user | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"09013aae13b008359c054d5e4252844ebdb5aa57" 09013aa]:
{{{
#!CommitTicketReference repository=""
revision="09013aae13b008359c054d5e4252844ebdb5aa57"
[2.2.x] Refs #28699 -- Clarified CSRF middleware ordering in relation to
RemoteUserMiddleware.
Backport of 94469504706b494877b6bb45a979bcb81c7fd7be from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28699#comment:26>
Django
------------------------------+------------------------------------
Reporter: stephanm | Owner: Rodrigo
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: remote user | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Rodrigo):
* has_patch: 0 => 1
* component: Documentation => contrib.auth
Comment:
I think I got it! :)
First I implemented the deferred token rotation as Carlton suggested. I
introduced a backend setting called "CSRF_DEFER_TOKEN_ROTATION" to the
REMOTE_USER backend, then modified the login function to check if the
backend has that option do not rotate the token and defer it, then catch
it on CSRF's process_response, not REMOTE_USER's.
It worked. But... I then realized that every backend should have that
option because it's CSRF MW responsability to do that, not the login
function, which lead me to realize that it's kind of a break of the
separation of concerns, the login function as stands before is tied to the
CSRF MW and if CSRF MW is not enabled, the django login would not work,
making it not loosely coupled with the MW.
Then, the solution is to delegate entirely the token rotation to the CSRF
MW and do it at the end of the request-response cycle, so if there is a
valid token, process the request and then rotate the token for the next
request.
After trying different options, like aCSRF_TOKEN_ROTATION_ON_LOGIN general
setting, the best and minimal solution I found is just reuse the previous
defined vars, add another flag for rotation - csrf_token_rotation - and
just delegate it to the MW if the MW is enabled.
By doing this, token rotation for the REMOTE_USER backend should be fixed
without the need of reordering.
I removed the CSRF_TOKEN_ROTATION_ON_LOGIN setting to disable the feature,
but as the rotation works as expected, it has a very limited application
(only troubleshooting comes to my mind) though I can re-submit it later if
you consider it.
[https://github.com/django/django/pull/11919 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/28699#comment:27>
Django
------------------------------+------------------------------------
Reporter: stephanm | Owner: Rodrigo
Type: Bug | Status: assigned
Component: contrib.auth | Version: 1.11
Severity: Normal | Resolution:
Keywords: remote user | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
------------------------------+------------------------------------
* needs_tests: 0 => 1
Comment:
[https://github.com/django/django/pull/12402 New PR] that suggests using
`CsrfViewMiddleware._get_token()` in `process_view()`. (This was how the
token was fetched prior to c4c128d67c7dc2830631c6859a204c9d259f1fb1.) If
there's no issue with that, then it looks feasible.
Needs tests.
--
Ticket URL: <https://code.djangoproject.com/ticket/28699#comment:28>
Django
Reporter: stephanm | Owner: Colton
| Hicks
Type: Bug | Status: assigned
Component: contrib.auth | Version: 1.11
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* owner: Rodrigo => Colton Hicks
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/28699#comment:29>
Django
-------------------------------------+-------------------------------------
Reporter: stephanm | Owner: Colton
| Hicks
Component: contrib.auth | Version: 1.11
Severity: Normal | Resolution: fixed
Keywords: remote user | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"f283ffaa84ef0a558eb466b8fc3fae7e6fbb547c" f283ffa]:
{{{
#!CommitTicketReference repository=""
revision="f283ffaa84ef0a558eb466b8fc3fae7e6fbb547c"
Fixed #28699 -- Fixed CSRF validation with remote user middleware.
Ensured process_view() always accesses the CSRF token from the session
or cookie, rather than the request, as rotate_token() may have been called
by an authentication middleware during the process_request() phase.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28699#comment:30>