-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Add support for calculating CSP hashes of inline scripts #1371
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
644b1b1
Add support for calculating CSP hashes of inline scripts
anders-kiaer 6881805
Update tests/integration/test_csp.py
alexcjohnson 6df88f7
Merge branch 'dev' into dash_inline_js_csp
alexcjohnson c62e51b
black
alexcjohnson eb20cfe
noise
alexcjohnson File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,3 +10,4 @@ cryptography==3.0 | |
requests[security]==2.21.0 | ||
beautifulsoup4==4.8.2 | ||
waitress==1.4.3 | ||
flask-talisman==0.7.0 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
import contextlib | ||
|
||
import pytest | ||
import flask_talisman | ||
from selenium.common.exceptions import NoSuchElementException | ||
|
||
import dash | ||
import dash_core_components as dcc | ||
import dash_html_components as html | ||
from dash.dependencies import Input, Output | ||
|
||
|
||
@contextlib.contextmanager | ||
def does_not_raise(): | ||
yield | ||
|
||
|
||
@pytest.mark.parametrize( | ||
"add_hashes, hash_algorithm, expectation", | ||
[ | ||
(False, None, pytest.raises(NoSuchElementException)), | ||
(True, "sha256", does_not_raise()), | ||
(True, "sha384", does_not_raise()), | ||
(True, "sha512", does_not_raise()), | ||
(True, "sha999", pytest.raises(ValueError)), | ||
], | ||
) | ||
def test_incs001_csp_hashes_inline_scripts( | ||
dash_duo, add_hashes, hash_algorithm, expectation | ||
): | ||
app = dash.Dash(__name__) | ||
|
||
app.layout = html.Div( | ||
[dcc.Input(id="input_element", type="text"), html.Div(id="output_element")] | ||
) | ||
|
||
app.clientside_callback( | ||
""" | ||
function(input) { | ||
return input; | ||
} | ||
""", | ||
Output("output_element", "children"), | ||
[Input("input_element", "value")], | ||
) | ||
|
||
with expectation: | ||
csp = { | ||
"default-src": "'self'", | ||
"script-src": ["'self'"] | ||
+ (app.csp_hashes(hash_algorithm) if add_hashes else []), | ||
} | ||
|
||
flask_talisman.Talisman( | ||
app.server, content_security_policy=csp, force_https=False | ||
) | ||
|
||
dash_duo.start_server(app) | ||
|
||
dash_duo.find_element("#input_element").send_keys("xyz") | ||
assert dash_duo.wait_for_element("#output_element").text == "xyz" |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@anders-kiaer this looks great, and fantastic tests - I love
does_not_raise
, hadn't seen that trick before but I can see it coming in handy elsewhere.The name is a little long, would it be ambiguous if we just call it
csp_hashes
orscript_hashes
?Can you add a bit of usage example into the docstring here? Something like:
Am I understanding correctly that this is what you'd normally do?
I haven't used
flask_talisman
before, but I presume that it works just as well (and with no changes to your code) withgunicorn
as it does withrun_server
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree, it is currently a bit long. I think both of the two alternatives should be ok. If there is any ambiguity, that probably is:
csp_hashes
: CSP hashes are, generally, used both for inlinestyles
and inlinescripts
. So from the name alone it is not clear if it is script hashes, style hashes or both.However, in Dash framework context (since dynamic style attributes from Python side are set in a CSP compatible way through React, see Separate CSS file on build dash-core-components#753 (comment)), only
script
hashes should be necessary, which reduces the risk of the ambiguity. There are of course Dash components out there that needs to do changes similar to Separate CSS file on build dash-core-components#753, but that is on the different custom/standard Dash components to solve/fix in theirwebpack
build setup, not something concerning the Dash framework.script_hashes
: Solves the style vs script ambiguity, but it does not indicate that the hashes are in "CSP format" and ready to go ('{hash algorithm}-{base 64 hash}'
, including the''
).I like
csp_hashes
best of the two 👍 If there appears e.g. astyle
hash use case later, for some reason, this can be solved with a natural non-breaking extension with proper default value, e.g.app.csp_hashes(hash_algorithm, source="scripts")
.Yes and yes 🙂
gunicorn
passes by default the (CSP) headers through. E.g. with that example CSP configuration in the suggested docstring, Dash users will get 🔒A+
rating out of the box on Mozilla observatory.