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

Add OAuth config and rework URL config #25

Merged
merged 10 commits into from
May 30, 2022

Conversation

drewsilcock
Copy link
Contributor

This adds support for configuring the Swagger UI OAuth2 integration, as per docs here: https://swagger.io/docs/open-source-tools/swagger-ui/usage/oauth2/.

This also slightly reworks the Swagger specification JSON document location configuration. The default value is nil instead of doc.json (this is to make it more clear what the configuration parameter is for and to make it work with the additional field in the config). This fixes the link to the Swagger spec when the path is local on the Swagger UI page (just below the "Base Path").

This is simpler and more robust. The regexp solution, for instance,
served up 'index.html' if you ask for 'index.htmlnotreally'.

The early 404 if the regexp doesn't match is not necessary - the
swaggerFiles handler will return 404 if the filename doesn't match any
swaggerUI content.

Setting the prefix on the handler is also not necessary, as we are
already passing in the filename, so the prefix doesn't need to be
stripped.
This makes it more clear that this config option should only be set if
the Swagger spec document is not expected to be hosted by this
middleware but instead by an external website or service. It also fixes
the link to the JSON document under the base path on the Swagger UI.

Also set the swaggerFiles handler prefix again, as this is required.
@codecov-io
Copy link

codecov-io commented Jan 6, 2020

Codecov Report

Merging #25 (ad70f0b) into master (80f04ba) will not change coverage.
The diff coverage is 100.00%.

❗ Current head ad70f0b differs from pull request most recent head c946bd0. Consider uploading reports for the commit c946bd0 to get more accurate results

@@            Coverage Diff            @@
##            master       #25   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines           75        78    +3     
=========================================
+ Hits            75        78    +3     
Impacted Files Coverage Δ
swagger.go 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 80f04ba...c946bd0. Read the comment docs.

Copy link
Contributor

@ubogdan ubogdan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ubogdan ubogdan requested review from easonlin404 and sdghchj July 27, 2021 19:18
@ubogdan
Copy link
Contributor

ubogdan commented Dec 24, 2021

@drewsilcock, Would you mind fixing the conflicts so we can go forward with it.

@jaesung9507
Copy link

When will this branch be merge into master?

@drewsilcock drewsilcock force-pushed the add-oauth-config branch 2 times, most recently from e5ac771 to 1f4b3dd Compare May 30, 2022 10:01
@drewsilcock
Copy link
Contributor Author

Hi @ubogdan thanks for your patience, I've resolved the merge conflicts now.

Copy link
Contributor

@ubogdan ubogdan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ubogdan ubogdan merged commit b99c764 into swaggo:master May 30, 2022
@ubogdan
Copy link
Contributor

ubogdan commented May 30, 2022

@drewsilcock Thanks for your contribution.

putnap pushed a commit to putnap/http-swagger that referenced this pull request Nov 29, 2024
Adding 0Auth config values based on this PR from echo-swagger:
swaggo/echo-swagger#25
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.

4 participants