-
Notifications
You must be signed in to change notification settings - Fork 570
Removed inline javascript and style code to support Content Security Policy #382
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
Conversation
adds explicit Content Security Policy header setting which are needed to load DebugKit correctly (fonts.googleapis.com and fonts.gstatic.com)
webroot/js/debug_kit.js
Outdated
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.
These PHP tags look wrong.
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.
This probably has to be elem.getAttribute("data-id");.
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.
Oh yeah, holy ... 😄 Interesting that it is working nevertheless. ^^ I will fix it.
Edith says: Ok, changed that. A new data attribute at the script tag is created for the toolbar id and is gotten by elem.getAttribute("data-id") like Marlinc said already. 😉
…ut of a data attribute now (based on code review cakephp#382 (diff))
|
So I noticed a few regressions that will need to be figured out before this can be merged:
Other than these issues the changes are looking good. |
|
@markstory I fixed Syntax highlighting as we can just separate all styles in a css file and load the css file in the To be honest ... as it is in an own iframe with it's own CSP restrictions and as it is just supposed to be used in development ... I would suggest just to allow Maybe you want to know what is definitely needed to support CSP then. I will comment it in the code. But in general: Everything what is done in the original page. |
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.
[NEEDED] This is absolutely needed for CSP support inside the original page.
|
I think the current state where debug kit can be loaded into a csp page, but does not use csp itself is a good approach to take. 👍 |
Unsets the Content-Security-Policy "inside" the iframe. The iframe will be loaded with user's CSP, but doesn't need any CSP inside the iframe as it will just be used in development. Force unset, as it could be set anywhere else in the user's original application.
|
Ok .. I just changed it so the CSP header will be force unset for the possibility that the user set the CSP header somewhere else in the original application (e.g. directly in bootstrap). |
Removed inline javascript and style code to support Content Security Policy
Based on the issue #7511 in cakephp repo, I removed inline javascript and style code and replace them with code in separated js resp. css files.
So DebugKit is (hopefully) fully supported for Content Security Policy. DebugKit also set its own Content-Security-Policy, so it should be guaranteed that everything can be loaded by DebugKit.