CsrfViewMiddleware's request.META["CSRF_COOKIE_USED"] and request.csrf_cookie_needs_reset can be combined

[Chris Jerdonek]

In CsrfViewMiddleware, request.META["CSRF_COOKIE_USED"] and request.csrf_cookie_needs_reset are both used for the same purpose. Namely, they are inspected inside CsrfViewMiddleware.process_response() to determine whether a cookie should be sent (though the logic in the method is currently buggy): ​https://github.com/django/django/blob/6f60fa97b0b501ef7cc77e16392654bf27ec8db3/django/middleware/csrf.py#L440-L445

Combining these two things would simplify CsrfViewMiddleware. This could be done after #32902, which fixes the bugginess mentioned above.

My suggestion would be to replace both of these with a single request.META key of request.META["CSRF_COOKIE_NEEDS_RESET"]. The reason is that the request.META dict is more visible and easier to debug than a custom request attribute, and it pairs more nicely with request.META["CSRF_COOKIE"]. Also, using the current key of "CSRF_COOKIE_USED" would be misleading because there are cases where the cookie is queued for reset even if it hasn't been used in the request.

[Chris Jerdonek]

As additional background and for future reference, request.csrf_processing_done is the only other custom request attribute used by CsrfViewMiddleware: ​https://github.com/django/django/blob/6f60fa97b0b501ef7cc77e16392654bf27ec8db3/django/middleware/csrf.py#L191

[Mariusz Felisiak]

Tentatively accepted. csrf_cookie_needs_reset is undocumented API but I found projects that use it, so we should at least add a short release note.

Florian, Shai, can you take a look? Your opinions would be greatly appreciated. I've checked an extensive discussion in #20869 and ​PR but couldn't find any rationale for this duality.

[Florian Apolloner]

I'd love to get rid of one of those. I tried in the past and failed :D I do not have strong feelings on the request attr vs the META dict. Somehow META feels wrong, what is META supposed to be used for? http-headers and wsgi attributes, or should we also stuff our things in there?

[Chris Jerdonek]

I do not have strong feelings on the request attr vs the META dict. Somehow META feels wrong, what is META supposed to be used for?

Thanks, Florian. One advantage META has over a custom attribute is that its contents automatically get displayed in the debug view under "Request information" (which isn't currently true for custom attributes). So developers are able to see a snapshot of the CSRF "state" more easily when an error occurs. This is what I was referring to in part when I said above it's "more visible and easier to debug." Also, as long as we're using META to store CSRF_COOKIE, I think it makes sense to store the related values alongside. If we're considering moving CSRF_COOKIE out of there at some point, it's another story. Finally, while the debug view and tools could be updated to display certain custom attributes, it would create more friction when changing the attributes because the tools would also need to be updated.

[Chris Jerdonek]

So people coming across this ticket know, I'm waiting for ticket #32902 to be resolved before submitting a PR for this ticket. One reason is that my PR for that ticket adds some extra test cases, so it will be good to have those test cases in place before making the change considered here.

[Chris Jerdonek]

​https://github.com/django/django/pull/14688

[Mariusz Felisiak <felisiak.mariusz@…>]

In 6ebf931d:

Fixed #32916 -- Combined request.METACSRF_COOKIE_USED and request.csrf_cookie_needs_reset.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 7c30bdb:

Refs #32916 -- Replaced request.csrf_cookie_needs_reset with request.METACSRF_COOKIE_NEEDS_UPDATE.