CSRF Middlware and usage of request attributes (META, csrf_cookie_needs_reset)

[Florian Apolloner]

Hi there,

I am considering rewriting and (hopefully) simplifying the CSRF middleware. While looking through the code I realized that we put stuff into request.META as well as attributes on the request object itself (csrf_cookie_needs_reset) for instance. Is there any reason why we do not stick to one format?

Or more generally put: When should middlewares write into META as opposed to a attribute.

Cheers,

Florian

[Adam Johnson]

It looks like it dates back to the Django 1.1 refactor and extension in https://github.com/django/django/commit/8e70cef9b67433edd70935dcc30c621d1e7fc0a0 ticket #9977, for which the referred wiki page ( https://code.djangoproject.com/wiki/CsrfProtection) is still up too with rationales about the design. I can't find any discussion about request.META vs attributes though, but since the move in the commit was from two middlewares to a unified one, maybe it was to do with that, since request.META is always there whilst attributes might not be if the middleware aren't set up properly? It seems like it could just be oversight. I would think that moving would require a standard deprecation period.

--
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 post to this group, send email to django-d...@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/43f6d8da-c66c-4100-a6d6-e85d4cef3684%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--

Adam

[Florian Apolloner]

Hi Adam,

On Saturday, December 29, 2018 at 7:59:44 PM UTC+1, Adam Johnson wrote:

since request.META is always there whilst attributes might not be if the middleware aren't set up properly?

request.META is always there, but the keys inside there might not be. In that sense you have to do "key in request.META" like we do with "hasattr(request, key)" if you are not sure on whether process_request of the middleware did actually set that stuff.

 

It seems like it could just be oversight. I would think that moving would require a standard deprecation period.

Unless we documented the attributes/keys I'd consider it an implementation detail of the middleware.

Cheers,

Florian

[Adam Johnson]

Unless we documented the attributes/keys I'd consider it an implementation detail of the middleware. 

Fair enough, I think I'm becoming a bit too conservative :)

--

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 post to this group, send email to django-d...@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/fa76d5eb-68c6-4f9d-859d-3eb48011ae23%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--

Adam

[Luke Plant]

Hi Florian,

My own instincts would be steer away from writing to request.META for most things, because request.META also contains things from the environment and indeed from the user request. You really don't want an attacker to be able to set an HTTP header and bypass security controls or directly influence anything which is supposed to be from application/framework code.

Of course, all arbitrary HTTP request headers are mapped to `request.META['HTTP_ …`, while some specific ones have special mappings,  but I'd rather not have to think about that kind of thing when doing security audits. With attributes it is always very clear where they have come from.

In addition, looking at the docs for request.META it is confusing to users if there are things in there that have not originated directly from the request.

As the original author of the CSRF stuff, I have no idea now why I used request.META for some things, if indeed it was me who did it that way — attributes seems much better for those usages.

Luke

--
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 post to this group, send email to django-d...@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/43f6d8da-c66c-4100-a6d6-e85d4cef3684%40googlegroups.com.

[Adam Johnson]

Thanks Luke for your look-again-later self code review :)

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/9fb674dc-df9a-0c86-8e93-3465b8a596bb%40cantab.net.

For more options, visit https://groups.google.com/d/optout.

--

Adam