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.

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

Update how the JavaScript run function is invoked in oauth2-redirect.html #6393

Conversation

mderriey
Copy link
Contributor

Update how the JavaScript run function is invoked in oauth2-redirect.html to make it easier to allow in CSP.

Description

The change moves away from using an inline event handler <body onload="run()"> to a programmatic event handler.

Motivation and Context

We're embedding Swagger UI in an application that defines a content security policy — see an introduction on MDN if necessary.

One issue we're facing is that the oauth2-redirect.html file uses an inline event handler, <body onload="run()">, which is not supported by Chrome or Edge.
The official Chrome docs mention that those should be rewritten using the addEventListener method.
They also recommend to use the DOMContentLoaded event on the document rather than the load event on the window, as the former "generally triggers more quickly".
See https://developer.chrome.com/extensions/contentSecurityPolicy#:~:text=The%20inline%20event%20handler,more%20quickly

Because of this, we currently have to maintain a slightly modified copy of oauth2-redirect.html file to which we apply this change.
We're hoping that you'll accept that change to be made upstream.

Related issue: #5720

How Has This Been Tested?

We've tested performing an OAuth 2 implicit flow with Chrome, the new Edge, and Firefox after allowing the specific hash of the contents of the <script> tag in our content security policy.

Screenshots (if appropriate):

N/A

Checklist

My PR contains...

  • No code changes (src/ is unmodified: changes to documentation, CI, metadata, etc.)
  • Dependency changes (any modification to dependencies in package.json)
  • Bug fixes (non-breaking change which fixes an issue)
  • Improvements (misc. changes to existing features)
  • Features (non-breaking change which adds functionality)

My changes...

  • are breaking changes to a public API (config options, System API, major UI change, etc).
  • are breaking changes to a private API (Redux, component props, utility functions, etc.).
  • are breaking changes to a developer API (npm script behavior changes, new dev system dependencies, etc).
  • are not breaking changes.

Documentation

  • My changes do not require a change to the project documentation.
  • My changes require a change to the project documentation.
  • If yes to above: I have updated the documentation accordingly.

Automated tests

  • My changes can not or do not need to be tested.
  • My changes can and should be tested by unit and/or integration tests.
  • If yes to above: I have added tests to cover my changes.
  • If yes to above: I have taken care to cover edge cases in my tests.
  • All new and existing tests passed.

mderriey and others added 3 commits September 11, 2020 09:53
…html

We're embedding Swagger UI in an application that defines a content security policy &mdash; see [an introduction on MDN](https://developer.mozilla.org/en-US/docs/Web/HTTP/CSP) if necessary.

One issue we're facing is that the `oauth2-redirect.html` file uses an inline event handler, `<body onload="run()">`, which is not supported by Chrome or Edge.
The official Chrome docs mention that those should be rewritten using the `addEventListener` method.
They also recommend to use the `DOMContentLoaded` event on the document rather than the `load` event on the window, as the former "generally triggers more quickly".
See <https://developer.chrome.com/extensions/contentSecurityPolicy#:~:text=The%20inline%20event%20handler,more%20quickly>

Because of this, we currently have to maintain a slightly modified copy of `oauth2-redirect.html` file to which we apply this change.
We're hoping that you'll accept that change to be made upstream.
We've tested it on Chrome, the new Edge, and Firefox.

Related issue: <swagger-api#5720>
@tim-lai tim-lai merged commit 48ee32f into swagger-api:master Sep 11, 2020
@tim-lai
Copy link
Contributor

tim-lai commented Sep 11, 2020

@mderriey PR merged! Thanks for the contribution!

ypsah added a commit to ypsah/fastapi that referenced this pull request Jul 24, 2022
When using OpenIDConnect and the "authorization_code with PKCE" flow,
swagger-ui will name the flow `authorization_code` rather than
`authorizationCode`. [0]

The old HTML document generated by fastapi only handled the latter
spelling. This meant that trying to authenticate using the swagger UI
would fail with:

```
auth errorError: Bad Request, error: invalid_request, description: Missing parameter: code
```

This commit updates the HTML used by fastapi for the
`docs/oauth2-redirect` using the latest version published by swagger-ui
[1] which brings in a fix for the bug described above.

The rest of the changes relate to improving support for Chrome and Edge.
[2]

[0] https://github.com/swagger-api/swagger-ui/blob/626defc839f80f0d08105cb72b8f6b7d3334db9c/src/core/components/auth/oauth2.jsx#L130
[1] https://github.com/swagger-api/swagger-ui/blob/626defc839f80f0d08105cb72b8f6b7d3334db9c/dev-helpers/oauth2-redirect.html
[2] swagger-api/swagger-ui#6393
ypsah added a commit to ypsah/fastapi that referenced this pull request Jul 24, 2022
When using OpenIDConnect and the "authorization_code with PKCE" flow,
swagger-ui will name the flow `authorization_code` rather than
`authorizationCode`. [0]

The old HTML document generated by fastapi only handled the latter
spelling. This meant that trying to authenticate using the swagger UI
would fail with:

```
auth errorError: Bad Request, error: invalid_request, description: Missing parameter: code
```

This commit updates the HTML used by fastapi for the
`docs/oauth2-redirect` using the latest version published by swagger-ui
[1] which brings in a fix for the bug described above.

The rest of the changes relate to improving support for Chrome and Edge.
[2]

[0] https://github.com/swagger-api/swagger-ui/blob/626defc839f80f0d08105cb72b8f6b7d3334db9c/src/core/components/auth/oauth2.jsx#L130
[1] https://github.com/swagger-api/swagger-ui/blob/626defc839f80f0d08105cb72b8f6b7d3334db9c/dev-helpers/oauth2-redirect.html
[2] swagger-api/swagger-ui#6393
JuanSW18 pushed a commit to Digital-Paw/digital-paw-swagger-ui that referenced this pull request Aug 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants