Added support for Content Security Policy, Rebase #330 #704
Conversation
|
This is a rebase of #330 |
| @@ -35,7 +35,7 @@ | |||
| <dt><strong>{{ key|escape }}</strong></dt> | |||
| <dd> | |||
| <div class="djTemplateShowContextDiv"><a class="djTemplateShowContext"><span class="toggleArrow">▶</span> {% trans "Toggle context" %}</a></div> | |||
| <div class="djTemplateHideContextDiv" style="display:none;"><code>{{ value|escape }}</code></div> | |||
| <div class="djTemplateHideContextDiv" hidden><code>{{ value|escape }}</code></div> | |||
There was a problem hiding this comment.
Is there a reason why you didn't use hidden="hidden" here and on line 22?
|
This looks pretty good. Could you please add a |
graingert
force-pushed
the
csp
branch
2 times, most recently
from
60f57c3
to
a966351
Compare
|
Done, I've also prefixed the CSS class 'highlighted' |
|
@graingert Can you please rebase again? The PR contains merge conflicts. |
Conflicts: debug_toolbar/templates/debug_toolbar/panels/profiling.html
|
re-based, also killed that other "hidden" |
|
@tim-schilling @thedrow poke |
|
I tried setting up a test application with a content security policy enabled and I had to add |
|
Yes that's correct. Although if you set a custom JQuery URL it will be whatever the origin of that URL is. |
|
I was doing some more testing and whenever I click on the SQL tab, it generates the unsafe-eval error. Can you duplicate that issue? |
|
@tim-schilling Can you paste me the full error? Do you have your test project on GitHub? |
|
I've repeated the issue. We need to allow |
|
Yes, I think we should merge this PR before it needs rebasing again. @graingert Fortunately there isn't a lot of activity, only @tim-schilling and myself merge patches. I think it would be helpful to add guidelines to the contributing documentation so future changes don't break CSP inadventently. I'm not sure if it could be tested automatically e.g. with Selenium? @tim-schilling As far as I am concerned, I don't mind merging the patch as is and iterating. |
|
I meant to respond earlier. I decided that I'd merge it but not document it
until the unsafe-eval directive isn't needed. I don't think we can say the
toolbar supports a content security policy until that directive is no
longer needed.
|
Closes PR #704. This is the first part of making the toolbar support a content security policy. With this change a user would need to set the script-src directive to contain 'self' 'unsafe-eval' and ajax.googleapis.com, set the img-src directive to contain 'self' and data:. In the future the unsafe-eval value will no longer be needed.
|
I rebased your changes on master, and merged them in with commit 8b29127. Thank you for the PR @graingert! |
Closes PR jazzband#704. This is the first part of making the toolbar support a content security policy. With this change a user would need to set the script-src directive to contain 'self' 'unsafe-eval' and ajax.googleapis.com, set the img-src directive to contain 'self' and data:. In the future the unsafe-eval value will no longer be needed.
I'm deploying the experimental `Feature-Policy` on my site. I tried disabling the `sync-script` policy, which blocks HTML-parse-blocking JavaScript, and django-debug-toolbar broke because its script tags aren't async or deferred. There's a bit more information about this feature, and a demo at https://feature-policy-demos.appspot.com/sync-script.html?on I've added `defer` to all the `<script>` tags, which makes them execute after HTML parsing has finished, in order. There was also one inline script for redirects which I've extracted - this would break for `sync-script` but also for use of `Content-Security-Policy` blocking inline JavaScript, which has been supported since jazzband#704 but it seems this was missed. Tested by using the `example` app, redirect interception worked and every panel loaded and looked right.
No description provided.