🐛[#68] add CSP headers for DRF spectacular schema #69
Conversation
Coperh
force-pushed
the
feature/68-CSP-for-drf-spectacular
branch
4 times, most recently
from
74d7e26
to
471daf6
Compare
Coperh
force-pushed
the
feature/68-CSP-for-drf-spectacular
branch
from
471daf6
to
157bc99
Compare
open_api_framework/conf/base.py
Outdated
| CSP_DEFAULT_SRC = [ | ||
| "'self'", | ||
| ] + config( | ||
| CSP_DEFAULT_SRC = ["'self'", "'unsafe-inline'"] + config( |
There was a problem hiding this comment.
Do we have to add unsafe-inline to the default? I'd prefer to add it only to the specific directives that need it (probably script-src)
| @@ -213,6 +213,7 @@ | |||
| "vng_api_common", | |||
| "notifications_api_common", | |||
| "drf_spectacular", | |||
| "drf_spectacular_sidecar", | |||
There was a problem hiding this comment.
I haven't used this package before, so I'm not sure how it works, but what does it do in this case? If I check the github it contains the static assets, but we're still fetching them via CDNs right now.
I think it would be best if we use these self hosted assets, by changing the SPECTACULAR_SETTINGS (https://github.com/tfranzel/drf-spectacular?tab=readme-ov-file#self-contained-ui-installation), and then we can probably remove the changes to the CSP directives
There was a problem hiding this comment.
It's supposed to be self contained but stil has CSP errors without these links added.
There was a problem hiding this comment.
@stevenbal I thought so, but it looks even using sidecar required some csp tweaking:
https://drf-spectacular.readthedocs.io/en/latest/faq.html#solution-for-redoc
Coperh
force-pushed
the
feature/68-CSP-for-drf-spectacular
branch
from
9fef862
to
91abb7a
Compare
open_api_framework/conf/base.py
Outdated
| help_text="Extra ``img-src`` sources for CSP other than ``CSP_DEFAULT_SRC``.", | ||
| CSP_IMG_SRC = ( | ||
| CSP_DEFAULT_SRC | ||
| + ["data:", "cdn.redoc.ly", "cdn.jsdelivr.net"] # used by DRF spectacular |
There was a problem hiding this comment.
Hmmm It looks like you use two solutions in the same time:
sidecarto load assets locally- whitelist redoc CDN for CSP
I think one solution should be enough. If you want to whitelist it, there is no need in sidecar if I understand it correctly
There was a problem hiding this comment.
Removed CDN links, does cause issues for the redoc logo which they do not include for "quick rebranding"
OZ does use raw.githubusercontent.com
There was a problem hiding this comment.
I've read their solution for REDOC specifically and yeah, you were right, you need this CDN for the images
Sorry for my first comment. Could you please bring the setting back
Coperh
force-pushed
the
feature/68-CSP-for-drf-spectacular
branch
3 times, most recently
from
9bd596d
to
855f7b5
Compare
| @@ -213,6 +213,7 @@ | |||
| "vng_api_common", | |||
| "notifications_api_common", | |||
| "drf_spectacular", | |||
| "drf_spectacular_sidecar", | |||
There was a problem hiding this comment.
@stevenbal I thought so, but it looks even using sidecar required some csp tweaking:
https://drf-spectacular.readthedocs.io/en/latest/faq.html#solution-for-redoc
Coperh
force-pushed
the
feature/68-CSP-for-drf-spectacular
branch
from
855f7b5
to
45bef6b
Compare
Fixes #68