Quick hack for including csp_nonces from requests into script tags

[karolyi]

This should fix #1723 as it optionally includes the csp_nonce from the django-csp module.

[matthiask]

Thanks. Looks good. I'd like to have some tests for this; django-csp could be made a dependency for the testsuite so that we can actually verify that the code works as intended.

[karolyi]

What bothers me more is that normally, link/style tags would require the nonces too. But since at its current state, 'strict-dynamic'-enabled script-src scripts can't inject styles which a lot of frameworks do), everyone just enables inlining styles and sameorigins in practice.

If this ever changes and someone enables 'strict-dynamic' for style-src, that will override 'self' on style CSPs and every link[rel=stylesheet]/style tag will require a nonce too from that point on.

I might add that preventative measure. The tests can be organized.

[matthiask]

Sure! Feel free to include that.

[karolyi]

So, tests are there, all working.

I'm totally against the enforcing of opinionated formatting on someone else's code (it's like no one is allowed to have their own coding style), but so be it. I can just as much format the code back automatically when doing free and voluntary contributions to other projects.

In fact, I timed the dict() vs {} call with timeit and the latter seemed faster, so as per empirical evidence, I went with that.

Please merge and do a timely release.

[matthiask]

Thanks for your contribution!

We're using code formatters so that we do not have to discuss code style over and over, as do many other projects. Also, we're not being paid for the work we're doing on this project either, free and voluntary here too.

The PR looks good to me at first sight, thanks. I'll take a closer look probably tomorrow.

[tim-schilling]

@matthiask I'm unsure about the type definition changes here. If you're good with them, I'll let it be.

My specific concern is that we don't have a formal policy for them and I'm not sure how this current state improves things. I'd prefer to be consistent in the library so it's easier for folks to navigate around.

[matthiask]

@tim-schilling My general opinion is that type annotations can be noisy, and not really worth it unless a big part of the code base is typed, but we have already started moving on this #1705 so I'm neutral to slightly fine with it.

[karolyi]

For context, I'm using pyright and with these added typings, my test and everything I did, checks out perfectly.

Don't be such an enemy of typing that will point out errors early with static analysis.

[tim-schilling]

But the project doesn't use pyright, so it's possible for this to get out of date and cause confusion down the line.

[karolyi]

You are confusing python typing and static analysis with pyright. The former will be valid even when the latter changes, as it's defined in python itself, and not the module.

There is only one comment in my code related to pyright, since I didn't want to go and add a missinfg type annotation in Django itself. The rest is standard python typing stuff.

[tim-schilling]

Sorry, I shouldn't have even started us down that discussion. Thank you for the PR, this will be great to have.

[tim-schilling]

@matthiask when you have time again, take a look at the revised PR. I'm now good with merging this.

[tim-schilling]

Suggested change

	context: ContextList = response.context # pyright: ignore[reportAttributeAccessIssue]
	nonce = str(context["toolbar"].request.csp_nonce)
	toolbar = list(DebugToolbar._store.values())[0]
	nonce = str(toolbar.request.csp_nonce)

[karolyi]

Same issue here as above.

It might be inconvenient for others, but I can assure you I have to fight with these kinda errors throughout everywhere when working with django, even when having the stubs installed.

It is not perfect but it's the best we have. I have already made the compromise to have my code "autocorrected" (in the sense of might makes right), so I suggest others make compromises here too.

[karolyi]

Whoa, ruff was pretty bold here removing the HttpRequest import :D Hence why everything's failing.

[matthiask]

Thanks!

[karolyi]

@matthiask you're welcome.

Can I have a release please, so I could use the actual official package, instead of my fork?

[matthiask]

@karolyi We're currently in the middle of a GSoC project adding async support to the toolbar, so I'm not sure if we want to do a release in the next few days without some additional testing first.

I generally think releasing quickly and trusting the test suite is the way to go, but I'm a bit wary now because of this larger project.

[karolyi]

I see thanks. I've already set a notification on the releases of this project, so I'll be notified if there is a new one.

Until then, I'll keep using my fork.

[karolyi]

Congratulations on the release. :)

[Zerotask]

When updating from 4.4.6 to 5.0.1 some pages stop working properly (e.g. charts won't be rendered) and in the browser console we get:

When I uncomment CSPMiddleware in our settings, it works again. So this release somehow added csp related issues.

I can't find any information about this breaking change but just a reference in the release notes.

[robhudson]

It's is hard to see the full picture, @Zerotask, but based on that CSP error shown it sounds like the toolbar is using nonces on the script tag but the header doesn't allow nonced scripts. So adding the nonce to the script-src directive in the header may allow it to work again.

Edit:
Issue opened: #2058

[karolyi]

It is kinda offtopic here imo since it's because of your particular CSP setting, I think.

The error lines are descriptive (if you read them a couple times again and again) and should point out the error sufficiently.