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

add initial content-security-policy #674

Merged
merged 1 commit into from
May 3, 2023
Merged

add initial content-security-policy #674

merged 1 commit into from
May 3, 2023

Conversation

philli-m
Copy link
Contributor

  • use django-csp
  • add csp for dev to find issues

@philli-m philli-m changed the title add initial content-security-policy WIP add initial content-security-policy Apr 26, 2023
@philli-m
Copy link
Contributor Author

philli-m commented Apr 26, 2023

@goapunk sorry i restored but now it says it's mine, sent you screen shots of what occurred

* use django-csp
* add csp for dev to find issues
@goapunk goapunk changed the title WIP add initial content-security-policy add initial content-security-policy May 2, 2023
@goapunk
Copy link
Contributor

goapunk commented May 2, 2023

I added some missing bits, but the error on dev came because the csp here only applies to local testing, therefore it used the strict defaults. Depends on admin

@Rineee
Copy link
Contributor

Rineee commented May 2, 2023

Lgtm. But I have a question: why are adding all of these to dev settings?

@goapunk
Copy link
Contributor

goapunk commented May 3, 2023

Lgtm. But I have a question: why are adding all of these to dev settings?

good question, my thought was that this way we can test them locally and find problems quickly. And for prod we need a slightly different one anyway. I guess we could also put it in base and then overwrite it for prod. Any preference?

@philli-m
Copy link
Contributor Author

philli-m commented May 3, 2023

I added some missing bits, but the error on dev came because the csp here only applies to local testing, therefore it used the strict defaults. Depends on admin

@goapunk so it's dependent on https://github.com/liqd/admin/pull/605 then?

@goapunk
Copy link
Contributor

goapunk commented May 3, 2023

I added some missing bits, but the error on dev came because the csp here only applies to local testing, therefore it used the strict defaults. Depends on admin

@goapunk so it's dependent on liqd/admin#605 then?

yes

@Rineee
Copy link
Contributor

Rineee commented May 3, 2023

Lgtm. But I have a question: why are adding all of these to dev settings?

good question, my thought was that this way we can test them locally and find problems quickly. And for prod we need a slightly different one anyway. I guess we could also put it in base and then overwrite it for prod. Any preference?

Ah ok, thanks for explaining. No, I think it makes sense to leave them in dev then 👍

@Rineee Rineee merged commit c2b9ceb into main May 3, 2023
2 checks passed
@Rineee Rineee deleted the jd-2023-04-add-csp branch May 3, 2023 11:31
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

Successfully merging this pull request may close these issues.

None yet

3 participants