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

Content-Security-Policy fixes #2723

Merged
merged 5 commits into from
Nov 27, 2016
Merged

Content-Security-Policy fixes #2723

merged 5 commits into from
Nov 27, 2016

Conversation

koenpunt
Copy link
Collaborator

@koenpunt koenpunt commented Oct 15, 2016

To make Chosen compatible with restrictive Content-Security-Policies, I've updated the way we apply inline styles to elements.

The HTML used to build the select is now defined in AbstractChosen.

I also updated the documentation pages with a CSP header as an example.

fixes #2423
related #2575

@koenpunt koenpunt changed the title Csp fixes Content-Security-Policy fixes Oct 15, 2016
@koenpunt koenpunt force-pushed the csp-fixes branch 4 times, most recently from 40c63fc to 7834a3c Compare October 15, 2016 12:48
@koenpunt
Copy link
Collaborator Author

ping ✨ @harvesthq/chosen-developers @stof

@@ -6,9 +6,25 @@
<link rel="stylesheet" href="docsupport/style.css">
<link rel="stylesheet" href="docsupport/prism.css">
<link rel="stylesheet" href="chosen.css">

<meta http-equiv="Content-Security-Policy" content="default-src 'self'; script-src 'self' https://ajax.googleapis.com; style-src 'self' 'sha256-MzlZlLruPmzLm/yy8z6yCpmyo5QSe/SURdcVrrJx880='; img-src 'self' data:">
Copy link
Collaborator

Choose a reason for hiding this comment

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

using a nonce rather than a hash may make maintenance easier, by avoiding to recompute the hash when updating the demo CSS (which will be forgotten for sure)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Even though calculating the hash is easy, because the correct one is logged to the console when using an incorrect hash, the forgetting part is true.

But then again, a nonce isn't safe if not regenerated on every request. Not that there's really a security risk at this page, I still think we shouldn't advertise the less secure option.

Maybe we can move the CSP additions to the build process of the documentation, and leave out the header in the HTML here.

Or validate the HTML in the build process, if possible.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Or better yet, we can just move the rules to docsupport/style.css

Copy link
Collaborator

Choose a reason for hiding this comment

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

as we already have a CSS file for the doc, this is indeed the best way

</div>
"""

get_no_results_template: (terms) ->
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is not really returning a template. It is rendering a template

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah thought of that. Same applies for get_multi/get_single. Maybe a suffix of _html is better.

@koenpunt koenpunt force-pushed the csp-fixes branch 2 times, most recently from 18a0871 to f6db804 Compare October 20, 2016 15:31
when the Content-Security-Policy doesn't include 'unsafe-inline',
setting the style attribute directly is not allowed.
remove inline style from search input field
@koenpunt koenpunt merged commit 56ec9dd into harvesthq:master Nov 27, 2016
@koenpunt koenpunt deleted the csp-fixes branch November 27, 2016 19:42
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.

Inline styles and Content Security Policy
2 participants