-
Notifications
You must be signed in to change notification settings - Fork 415
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
Override Content Security Policy for Swagger UI Compat #263
Conversation
This closes rswag#174. The Content Security Policy being served by Rails might be incompatible with what Swagger UI needs. This is true of the default CSP that Rails provides and even if it wasn't, a developer might configure their CSP to be incompatible with the Swagger UI. The ensure the Swagger UI works it makes the most sense to have it provide it's own CSP. This will allow us to ensure we have all the permissions that UI needs while at the same time having no more than that. The CSP I propose is: default-src 'self'; img-src 'self' data: https://online.swagger.io; font-src 'self' https://fonts.gstatic.com; style-src 'self' 'unsafe-inline' https://fonts.googleapis.com; script-src 'self' 'unsafe-inline'; The main sticking points are the `unsafe-inline` options for both styles and scripts. This is needed because the `index.html` file[1] has both inline styles and scripts. This is a permission most applications would be hesitant to enable on their app. Therefore if we ONLY enable it in this context we can allow the rest of the Rails app to be as locked down as the developer desires. Since we are overriding the CSP we might as well do the most restrictive config that still works with the Swagger UI. For example the default CSP in Rails allows any font, image, style or script as long as it is hosted on a HTTPS server. It also allows data URLs for fonts and images. For the Swagger UI we only need to allow data urls for images (the Swagger logo). The only 3rd party image is on https://online.swagger.io (for the validator image). Finally we need to allow fonts and styles to some Google sites for the Google fonts. I suggest future PRs work on removing the inline styles and scripts from the `index.html` file. This will allow us to lock down the permissions further by removing the dangerous `unsafe-inline` permission. For now my main goal was compatibility with the current functionality. Since the `index.html` is based on an upstream system I wasn't sure if it was best to tackle that there. Also the upgrade to Swagger 3 probably plays into this as that file will likely change so perhaps we want until Swagger 3 is working with RSwag before we work on locking down `index.html`. 1. https://github.com/rswag/rswag/blob/master/rswag-ui/lib/rswag/ui/index.erb
Recently merged PR #187 allows custom headers in the API section, I think a combination of these approaches would be best, providing a default CSP with the opportunity to overwrite it. Will use this as a base for adding custom headers at a later time, or you can do so yourself. Thank you for your contribution to the project so far. |
Would that hook work for this? It is the delivery of the index page that needs the CSP which is a different middleware (api middleware vs ui middleware). |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If the issue is still relevant to you, please leave a comment stating so to keep the issue from being closed. Thank you for your contributions. |
The logo has always been a `data` img but there used to be a validator image at the bottom that doesn't seem to be there any longer so removing it as an allowed source.
Since there was interest in this PR again on #174 I have updated my branch to merge with the latest in
|
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.
🎉
I've just created a git tag for the release and a discussion for it here: #561 |
@BookOfGreg - You mentioned over on #174 that you were open to a fix for this so I went ahead and put together a PR. This should provide that impl. When testing I realized that my
unsafe-eval
from #174 was not really needed. It was my Vue plugin giving me a false positive so the CSP for this PR is a bit tighter in terms of permissions.This closes #174.
The Content Security Policy being served by Rails might be incompatible with what Swagger UI needs. This is true of the default CSP that Rails provides and even if it wasn't, a developer might configure their CSP to be incompatible with the Swagger UI.
The ensure the Swagger UI works it makes the most sense to have it provide it's own CSP. This will allow us to ensure we have all the permissions that UI needs while at the same time having no more than that.
The CSP I propose is:
The main sticking points are the
unsafe-inline
options for both styles and scripts. This is needed because theindex.html
file has both inline styles and scripts. This is a permission most applications would be hesitant to enable on their app. Therefore if we ONLY enable it in this context we can allow the rest of the Rails app to be as locked down as the developer desires.Since we are overriding the CSP we might as well do the most restrictive config that still works with the Swagger UI. For example the default CSP in Rails allows any font, image, style or script as long as it is hosted on a HTTPS server. It also allows data URLs for fonts and images.
For the Swagger UI we only need to allow data urls for images (the Swagger logo). The only 3rd party image is on https://online.swagger.io (for the validator image). Finally we need to allow fonts and styles to some Google sites for the Google fonts.
I suggest future PRs work on removing the inline styles and scripts from the
index.html
file. This will allow us to lock down the permissions further by removing the dangerousunsafe-inline
permission. For now my main goal was compatibility with the current functionality. Since theindex.html
is based on an upstream system I wasn't sure if it was best to tackle that there. Also the upgrade to Swagger 3 probably plays into this as that file will likely change so perhaps we wait until Swagger 3 is working with RSwag before we work on locking downindex.html
.