Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CSRF_TRUSTED_ORIGINS? #3

Open
wsvincent opened this issue Oct 26, 2022 · 5 comments
Open

CSRF_TRUSTED_ORIGINS? #3

wsvincent opened this issue Oct 26, 2022 · 5 comments

Comments

@wsvincent
Copy link

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
Copy link
Member

ipmb commented Oct 26, 2022

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
Copy link

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
Copy link
Member

ipmb commented Oct 26, 2022

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
Copy link

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
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants