Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Support rendering openapi and swagger documents #26802

Draft
wants to merge 22 commits into
base: main
Choose a base branch
from
Draft

Support rendering openapi and swagger documents #26802

wants to merge 22 commits into from

Conversation

silverwind
Copy link
Member

@silverwind silverwind commented Aug 29, 2023 •
edited
Loading

Continuation of #25824
Fixes: #20852

Current status: working in iframe including light/dark, but I am considering moving it to first-party code as the current security attributes are already essentially the same outcome as first-party code.

Screenshot 2023-08-29 at 22 00 06

lunny and others added 19 commits July 11, 2023 16:10
@lunny
Support render opanapi v2.0
c5ac9d0
@lunny
Use dynamically created style sheet urls
6ba74f4
@lunny
Merge branch 'main' of github.com:go-gitea/gitea into lunny/support_r…
218eebc 
…ender_openapi
@lunny
fix cors
bc23c08
@lunny
Fix bug
0e14ef3
@lunny
use a standalone page to render openapi
5aefd2e
@lunny
Merge branch 'main' into lunny/support_render_openapi
c6e25e5
@lunny
Revert unnecessary change
568a29a
@lunny
revert unnecessary change
8e3dd7e
@lunny
Fix test
e5d1149
@lunny
Merge branch 'main' into lunny/support_render_openapi
60c4a04
@lunny
add comment for i18n
767ea4e
@silverwind
Merge branch 'main' into lunny/support_render_openapi
8d70d57
@silverwind
make it work for for openapi 3.x
897be68
@silverwind
make it work for yaml specs as well
d866b87
@silverwind
remove defaultModelRendering (does nothing), misc refactors
0ed4b33
@silverwind
fetch api accepts URLs as well
c24c14a
@silverwind
Merge branch 'main' into lunny/support_render_openapi
1041279
@silverwind
make iframe work
d3f28e5
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Aug 29, 2023
@silverwind silverwind mentioned this pull request Aug 29, 2023
Closed
@silverwind silverwind added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Aug 29, 2023
@silverwind silverwind added this to the 1.21.0 milestone Aug 29, 2023
@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 29, 2023
@silverwind
Copy link
Member Author

silverwind commented Aug 29, 2023 •
edited
Loading

If we could pass the yaml/json into the iframe HTML as a data-attribute, we could do away with the very insecure allow-same-origin sandbox attribute on the iframe and avoid having to to the raw request entirely. I think this is the solution to strife for as it will be sufficiently secure.

@lunny can you help with that from backend side? Essentially we just need to encode the json/yaml into data-source here. If you could do that, I can do the rest in the JS.

@silverwind silverwind marked this pull request as draft August 29, 2023 23:43
@lunny lunny modified the milestones: 1.21.0, 1.22.0 Sep 21, 2023
@silverwind silverwind mentioned this pull request Sep 23, 2023
Closed
@lunny lunny modified the milestones: 1.22.0, 1.23.0 Mar 4, 2024
@techknowlogick
Copy link
Member

Essentially we just need to encode the json/yaml into data-source here

To clarify, before the page is rendered, go would fetch the yaml or js from the file contents (up to some reasonable size limit to ensure that memory isn't fully consumed), set the contents as a template variable and then insert it into the attribute that you linked? And the iframe and such would still be used, but the allow-same-origin could be removed since it's not doing an additional HTTP call?

If yes, I think that's probably doable.

@silverwind
Merge remote-tracking branch 'origin/main' into support_render_openapi
ff8a5b3 
* origin/main: (1540 commits)
  Add `interface{}` to `any` replacement to `make fmt`, exclude `*.pb.go` (go-gitea#30461)
  Fix network error when open/close organization/individual projects and redirect to project page (go-gitea#30387)
  Avoid losing token when updating mirror settings (go-gitea#30429)
  Fix label rendering (go-gitea#30456)
  Add comment for ContainsRedirectURI about the exact match (go-gitea#30457)
  Update JS and PY deps, lock eslint and related plugins (go-gitea#30452)
  Refactor cache and disable go-chi cache (go-gitea#30417)
  Fix admin notice view-detail (go-gitea#30450)
  Fix mirror error when mirror repo is empty (go-gitea#30432)
  Add `/public/assets/img/webpack` to ignore files again (go-gitea#30451)
  Lock a few tool dependencies to major versions (go-gitea#30439)
  Fix commit status cache which missed target_url (go-gitea#30426)
  Remove jQuery from the commit graph (except Fomantic) (go-gitea#30395)
  Fix rename branch 500 when the target branch is deleted but exist in database (go-gitea#30430)
  Limit the max line length when parsing git grep output (go-gitea#30418)
  Change the default maxPerPage for gitbucket (go-gitea#30392)
  Fix the spacing issue in the Project view (go-gitea#30415)
  Add commit status summary table to reduce query from commit status table (go-gitea#30223)
  Split `issue edit` code from `repo-legacy.js` into its own file (go-gitea#30419)
  Check the token's owner and repository when registering a runner (go-gitea#30406)
  ...
@github-actions github-actions bot added modifies/api This PR adds API routes or modifies them modifies/go Pull requests that update Go code modifies/js modifies/dependencies labels Apr 13, 2024
@silverwind
Copy link
Member Author

Essentially we just need to encode the json/yaml into data-source here

To clarify, before the page is rendered, go would fetch the yaml or js from the file contents (up to some reasonable size limit to ensure that memory isn't fully consumed), set the contents as a template variable and then insert it into the attribute that you linked? And the iframe and such would still be used, but the allow-same-origin could be removed since it's not doing an additional HTTP call?

If yes, I think that's probably doable.

Yes, with the yaml/json passed into the iframe HTML, we avoid having to deal with CORS because no more fetch request is done from the iframe.

BTW, If I remove the allow-same-origin now, I get this:

image

@silverwind
Copy link
Member Author

@wxiaoguang maybe you can help me with the backend part described in #26802 (comment). This PR is otherwise ready and working but I would really like to eliminate this fetch request and the insecure iframe allow-same-origin sandbox attribute.

@wxiaoguang
Copy link
Contributor

I guess I wouldn't have enough time to look into it soon (maybe I could try before 1.23 freezing).

Some of my thoughts:

  1. Maybe it could use "postMessage" to communicate, then no CORS trick is needed.
  2. Be careful about the changed renderer behavior to avoid XSS problems.

@silverwind
Copy link
Member Author

Maybe it could use "postMessage" to communicate, then no CORS trick is needed.

Hmm yes, postMessage from parent to child window could work. Still I would prefer the render variant I guess.

@lunny lunny modified the milestones: 1.23.0, 1.24.0 Sep 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. modifies/api This PR adds API routes or modifies them modifies/dependencies modifies/go Pull requests that update Go code modifies/js size/L Denotes a PR that changes 100-499 lines, ignoring generated files. type/feature Completely new functionality. Can only be merged if feature freeze is not active.
Projects
None yet
Milestone
1.24.0
Development

Successfully merging this pull request may close these issues.

OpenAPI Viewer
5 participants
@silverwind @techknowlogick @wxiaoguang @lunny @GiteaBot