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

🔧[#42] add HSTS & CSP settings #49

Merged
merged 1 commit into from
Aug 19, 2024
Merged

Conversation

Coperh
Copy link
Contributor

@Coperh Coperh commented Jul 26, 2024

Fixes #42

Again just copy pasted most settings from OpenForms

@Coperh Coperh requested review from annashamray and stevenbal and removed request for annashamray July 26, 2024 10:46
@Coperh Coperh force-pushed the issue/42-headers-for-CSP-and-HSTS branch from eea1e52 to a1815cc Compare August 13, 2024 07:40
# NOTE: make sure values are a tuple or list, and to quote special values like 'self'

# ideally we'd use BASE_URI but it'd have to be lazy or cause issues
CSP_DEFAULT_SRC = [
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried testing this with Open Zaak, but I don't see any of the Content-Security-Policy headers showing up in the response headers. Does it work for you?

Copy link
Contributor Author

@Coperh Coperh Aug 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tested with all the components and can confirm they work now.

Open-zaak does not use the open-api-framework MIDDLEWARE so it is not enabled by default.

CSP_FORM_ACTION = (
config(
"CSP_FORM_ACTION",
default=["\"'self'\"", "https:"]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can remove https: here, in the comment it's mentioned that this is required for eHerkenning, but this doesn't apply for any of the registration components I think? @annashamray

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree

# and the signature component which saves the image drawn on the canvas as data: URI
CSP_IMG_SRC = (
CSP_DEFAULT_SRC
+ ["data:", "https://service.pdok.nl/"]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think service.pdok.nl can be excluded (@annashamray double check, we don't use this anywhere in the registration components, right?)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so, it's kadaster service afaik, which we don't use

@Coperh Coperh force-pushed the issue/42-headers-for-CSP-and-HSTS branch 3 times, most recently from bb115f7 to 874e5cb Compare August 19, 2024 07:17
@Coperh Coperh force-pushed the issue/42-headers-for-CSP-and-HSTS branch from 874e5cb to 7ec82db Compare August 19, 2024 07:23
@Coperh Coperh requested a review from stevenbal August 19, 2024 07:51
Copy link
Contributor

@annashamray annashamray left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tested it with Objects API and can see a response header with CSP

@Coperh Coperh merged commit 802cb98 into main Aug 19, 2024
8 checks passed
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.

Ensure HTTP headers for CSP and HSTS are properly set for the admin
3 participants