Add Content-Security-Policy-Report-Only header #3024
Conversation
|
|
||
| def content_security_policy_tween(request): | ||
| resp = handler(request) | ||
| resp.headers["Content-Security-Policy-Report-Only"] = policy.format(request=request) |
There was a problem hiding this comment.
I'd suggest adding a csp.report_only setting that can be set with a header -- CSP_REPORT_ONLY, say, and then toggling the name of the header (Content-Security-Policy vs Content-Security-Policy-Report-Only) on the basis of that setting.
|
Looks good. Let's start with something like: "font-src": ["'self'", "fonts.gstatic.com"],
"report-uri": [config.registry.settings.get("csp.report_uri")],
"script-src": ["'self'"],
"style-src": ["'self'", "fonts.googleapis.com"],and see what we get. @robertknight sound about right? |
Current coverage is
|
| @@ -24,6 +24,7 @@ def settings_from_environment(): | |||
| _setup_client(settings) | |||
| _setup_statsd(settings) | |||
| _setup_websocket(settings) | |||
| _setup_csp(settings) | |||
There was a problem hiding this comment.
The config module has changed substantially in #3035 so this will need rebasing. Looks like the options you've added will map straightforwardly, though.
finitud
force-pushed
the
aw/csp
branch
2 times, most recently
from
ae3e56c
to
8dc39fd
Compare
| from h import tweens | ||
|
|
||
|
|
||
| class DummyRegistry(): |
There was a problem hiding this comment.
pyramid.testing.DummyRequest should already have a registry, so you probably don't need this.
|
|
||
| response = tween(request) | ||
|
|
||
| assert 'report-uri localhost' in response.headers['Content-Security-Policy'] |
There was a problem hiding this comment.
It might be good to have a test for the value of the header as well. Just to mitigate future bugs in the code that generates the value string of this header. As far as I understand the user experience could completely break when CSP is enabled and the header value is not what we expect it to be.
The easiest would be to set some specific CSP policy in the test and compare it to the value string we expect it to generate.
There was a problem hiding this comment.
I've made this one more strict ('==' instead of 'in') and added one more that tests for a more complex header.
|
LGTM. |
Add Content-Security-Policy-Report-Only header
https://trello.com/c/r2MISk7g/267-deploy-csp-for-web-application