search

lincolnloop / django-production

production settings for Django
MIT License
91 stars 3 forks source link

CSRF_TRUSTED_ORIGINS? #3

Open wsvincent opened 1 year ago

wsvincent commented 1 year ago

CSRF_TRUSTED_ORIGINS needs to be set for Django 4.0+ if you want any POST requests, aka the Django admin and any other form, to work in production.

Maybe something in settings.py or elsewhere to at least alert the user to this need?

ipmb commented 1 year ago

If I'm reading correctly, this would only be if you're making requests from a different domain or via something on the same domain that doesn't include the referer header, is that correct? I would imagine any traditional browser requests would have the host and referer headers match so this wouldn't be necessary?

If that's the case, you'd use this when you are making POSTs via JS and/or from a different domain? I might be missing how this affects the default Django admin.

My initial thought is that we could add it like this:

CSRF_TRUSTED_ORIGINS = env.list("CSRF_TRUSTED_ORIGINS", default=[])

Does that seem reasonable?

carltongibson commented 1 year ago

Modern browsers will set the Origin and this will be checked in preference to referer.

However, the point about subdomains is correct: if Origin matches the request host then this list is not consulted. This is the basic case, so an empty list seems a good default. (Indeed it is the default.)

(I’m slightly sceptical about setting list values from string env vars, but I guess that’s a distraction at this point.)

ipmb commented 1 year ago

Thanks for the additional insight!

I'm happy to bite on the distraction 😆 ... What is your concern about converting CSVs to a list? I can see it being an issue if you have commas in the string, but that wouldn't be the case for domains.

Pydantic requires valid JSON to make this conversion, but that feels a little over the top to me.

carltongibson commented 1 year ago

What is your concern...

I gives me the 'eebies™ 😜

I'm sure it's fine (again ™ :) but once I'm passing more than a simple scalar I feel much happier with more than just strings. (If I had a full answer here I'd be pushing it, so do feel free to carry on regardless 🙂)

carltongibson commented 1 year ago

I gives me the 'eebies™ 😜

In the cold light of morning, the issue is (as ever with these things) How quickly does an error show up? — the trouble with string values is typos go undetected until they trigger an error, and then there's a long debugging process. I'd rather catch those when defining the input values.

... a distraction at this point.

I don't think it's something to worry about here. Simple works in most cases.