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

4.1.3 does not allow configuration via query parameters #156

Closed
mplanchant opened this issue Jan 11, 2022 · 25 comments
Closed

4.1.3 does not allow configuration via query parameters #156

mplanchant opened this issue Jan 11, 2022 · 25 comments

Comments

@mplanchant
Copy link

This upstream change means that it is no longer possible to pass configuration as query parameters https://github.com/swagger-api/swagger-ui/releases/tag/v4.1.3.

So URLs like http://localhost:8080/my-api/docs/index.html?url=/my-api/openapi3.json now display the default Petstore API.

Is there a way of setting queryConfigEnabled to true when using the webjar (https://github.com/swagger-api/swagger-ui/blob/master/docs/usage/configuration.md#core)?

@jamesward
Copy link
Member

Oh interesting. I wonder if we should add a swagger-config.yaml into the WebJar with this config. Think that would work?

@mplanchant
Copy link
Author

mplanchant commented Jan 12, 2022

So queryConfigEnabled would be permanently be set to true? That would certainly make the webjar usable for us.

@jamesward
Copy link
Member

4.1.3-1 will be released shortly and we will see if that fixes this.

@cachescrubber
Copy link

Simply adding the swagger-config.yaml is not sufficient. Since the swager-ui webjar is using the dist
installation, editing the index.html would be necessary.

Besides the technical detail - the upstream project decided to disable parameter overrides via url query params as a response to a security advisory - I personally think we should not overrule this decision in a downstream distribution.

A solution could be to modify the swagger-ui/index.html to request a user-provided configuration from the same host, via the configUrl parameter. A Problem could be to find the right path, working with all kind of different contextRoots, servlet path, etc).

As a workaround, I copied the index.html to my server root, editing the parameters as required and sourcing the assets (js, css, etc) via the webjar.

    <script src="webjars/swagger-ui/swagger-ui-bundle.js" charset="UTF-8"> </script>

@jamesward
Copy link
Member

Thanks @cachescrubber for the details on that. I would really rather not modify the upstream code. That becomes a maintenance burden. I wonder what other options we have?

@cachescrubber
Copy link

Not many I think. The springdoc project is transforming the index.html on the fly, if I understand the code correctly.

@datagitlies
Copy link

datagitlies commented Jan 24, 2022

I'll chime in to say I did what @cachescrubber did (below):

I copied the index.html to my server root, editing the parameters as required and sourcing the assets (js, css, etc) via the webjar.

EDIT: I also agree that the webjar should not override the setting from a security perspective.

@datagitlies
Copy link

FYI

User Interface (UI) Misrepresentation of Critical Information
Affecting org.webjars:swagger-ui package, versions [0,4.1.3)

https://security.snyk.io/vuln/SNYK-JAVA-ORGWEBJARS-2314887

@drewmacmac
Copy link

drewmacmac commented Feb 1, 2022

After two days of trying to figure this out I finally found this thread. I was eventually able to get the workaround described by @cachescrubber to work locally after some trial and error

For future people who aren't running static assets through Play yet, the trick was to put the swagger.html file in a new directory tree: ./public/lib/swagger-ui/swagger.html. This allowed me to keep the HTML file as is without recoding the js/css references.

Then add the route entry:

GET   /assets/lib/swagger-ui/*file                 controllers.Assets.versioned(path="/public/lib/swagger-ui", file: Asset)

Unfortunately, after deployment I'm seeing some issues, probably because static assets aren't exposed publicly by default at my company, so now I have to figure that out :headache:.

Edit: the fix for ^ was to add this to the Dockerfile:

ADD public public/

Definitely hoping for a solution to this soon!

@jamesward
Copy link
Member

Thanks for the details! One thing to note is that some of these workarounds might depend on classpath ordering which is tricky and can seem like indeterminate behavior when it doesn't work as expected (ie something produced a different classpath order).

@datagitlies
Copy link

Any plans to simply revert cfe7a37 @jamesward ?

@jamesward
Copy link
Member

@datagitlies It sounds like that commit didn't fix anything so yeah, I should revert it. But I also don't think it breaks anything, right?

@drewmacmac
Copy link

As a side note, if re-enabling the query functionality through the URI reintroduces the security vulnerability, I personally would no longer be able to use this library 😢

@datagitlies
Copy link

datagitlies commented Feb 3, 2022

@datagitlies It sounds like that commit didn't fix anything so yeah, I should revert it. But I also don't think it breaks anything, right?

It doesn't break anything because it doesn't work (currently) but I don't want it to accidentally start working somehow in a future release which could open my application to the security vulnerability described. Hence, I asked about reverting the change so the swagger-config.yaml file is no longer present in the webjar.

EDIT: I'm happy to open a pull request - just let me know.

@jamesward
Copy link
Member

Sounds good. I'm releasing 4.4.1-1 with the revert.

@bubbajoe
Copy link

I am still unable to get this working btw.

@papirosko
Copy link

papirosko commented Mar 15, 2022

workaround for playframework with "org.webjars" % "swagger-ui" % "4.5.2"

  1. create patched index.html in src/main/public/swagger/index.html:

<!-- HTML for static distribution bundle build -->
<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <title>Swagger UI</title>
    <link rel="stylesheet" type="text/css" href="./swagger-ui.css" />
    <link rel="icon" type="image/png" href="./favicon-32x32.png" sizes="32x32" />
    <link rel="icon" type="image/png" href="./favicon-16x16.png" sizes="16x16" />
    <style>
        html
        {
            box-sizing: border-box;
            overflow: -moz-scrollbars-vertical;
            overflow-y: scroll;
        }

        *,
        *:before,
        *:after
        {
            box-sizing: inherit;
        }

        body
        {
            margin:0;
            background: #fafafa;
        }
    </style>
</head>

<body>
<div id="swagger-ui"></div>

<script src="./swagger-ui-bundle.js" charset="UTF-8"> </script>
<script src="./swagger-ui-standalone-preset.js" charset="UTF-8"> </script>
<script>
    window.onload = function() {
        // Begin Swagger UI call region
        const ui = SwaggerUIBundle({
            url: "/swagger.json",
            dom_id: '#swagger-ui',
            deepLinking: true,
            presets: [
                SwaggerUIBundle.presets.apis,
                SwaggerUIStandalonePreset
            ],
            plugins: [
                SwaggerUIBundle.plugins.DownloadUrl
            ],
            layout: "StandaloneLayout"
        });
        // End Swagger UI call region

        window.ui = ui;
    };
</script>
</body>
</html>

(i've just changed this line: url: "/swagger.json")

  1. in routes add:
### NoDocs ###
GET        /                                    controllers.Default.redirect(to = "/docs/index.html")

### NoDocs ###
GET        /docs/index.html                     controllers.Assets.at(path = "/public/swagger", file = "index.html")

### NoDocs ###
GET        /docs/*file                          controllers.Assets.at(path = "/public/lib/swagger-ui", file)

### NoDocs ###
GET        /swagger.json                        controllers.Assets.at(path = "/public", file = "swagger.json")

(i use iheartradio for swagger generation, NoDocs is from there)

@datagitlies
Copy link

I'll second what @papirosko said. I just created my own index.html file instead of using the embedded index.html file that's present in the webjar. At that point, I can change the configuration to whatever I like.

@perja12
Copy link

perja12 commented Mar 28, 2022

I solved this by adding swagger-config.yaml in the resource folder (some-project/src/main/resources/swagger-config.yaml) and set url in that file. Also since I'm doing this in Spring I can use placeholder to get server context path. Example of swagger-config.yaml file:

url: /v1/${server.servlet.context-path}/api-docs/

@bademux
Copy link

bademux commented Apr 15, 2022

All this pseudo security movement makes me cry.
It is obvious that injecting 3rd party url into your JS webpage can mislead someone.
How about division by zero? It is dangerous let's ban it for security reasons.
Sorry about being bit emotional here, but I think, that good tool shouldn't suffer from uberprotective decisions made by JS guys
Can we please enable back url param? It is absolutely safe for internal projects, public projects can override index.html whatever they want.
Sorry and thanks.

@bademux
Copy link

bademux commented Apr 21, 2022

My fix for the (nonexisting) problem. Copy and modify index.html from swagger-ui.
Please make sure you copy openapi.yaml too.
Just add it to you build.gradle, and check your build/resources/main/static. I'm sure the same can be done for maven.
ver1: can be applied only for embedded swagger config index (old webjars)

processResources {
    with copySpec {
        def artifact = configurations.compileClasspath.resolvedConfiguration.resolvedArtifacts.find({ it.moduleVersion.id.name == 'swagger-ui' })
        from zipTree(artifact.file).matching { include '**/index.html' }.singleFile
        into 'static'
        filter { line -> line
                .replace('https://petstore.swagger.io/v2/swagger.json', '/openapi.yaml')
                .replace('StandaloneLayout', 'BaseLayout')
                .replace('="./', """="/webjars/swagger-ui/$artifact.moduleVersion.id.version/""")
        }
    }
}

for newer use V2 (upd with lazy artefact resolving):

processResources {
    with copySpec {
        from {
            def artifact = configurations.compileClasspath.resolvedConfiguration.resolvedArtifacts.find({ it.moduleVersion.id.name == 'swagger-ui' })
            zipTree(artifact.file).matching({ include '**/index.html' }).singleFile
        } into 'static'
        filter {
            def artifact = configurations.compileClasspath.resolvedConfiguration.resolvedArtifacts.find({ it.moduleVersion.id.name == 'swagger-ui' })
            it.replace('="./', """="/webjars/swagger-ui/$artifact.moduleVersion.id.version/""").replace('</body>', """
<script> window.onload = function() {
        const ui = SwaggerUIBundle({url: "/openapi.yaml", dom_id: '#swagger-ui',  deepLinking: true,presets: [SwaggerUIBundle.presets.apis, SwaggerUIStandalonePreset ], plugins: [ SwaggerUIBundle.plugins.DownloadUrl  ], layout: "BaseLayout" });
        window.ui = ui;
};</script></body>
""")
        }
    }

}

@jamesward How about adding solutions that are shared in the issue to the readme?

@jamesward
Copy link
Member

Thanks for the workarounds. I'd rather not patch the upstream in the WebJar because that becomes a maintenance burden. Has anyone worked with the Swagger team to see if there is a possible upstream fix to this?

@bademux
Copy link

bademux commented Apr 23, 2022

@jamesward absolutely agree with you! The problem should be solved on swagger UI side or end user side. While I'm not sure about the first, my code snippet solves the problem on end user side, so no changes to original code swagger UI code.
My question was about opening GitHub forum (discussions) or wiki to store HowTo's.

@perja12
Copy link

perja12 commented Apr 26, 2022

What I wrote is not true. Managed to test with some other modifications 🤦.

I solved this by adding swagger-config.yaml in the resource folder (some-project/src/main/resources/swagger-config.yaml) and set url in that file. Also since I'm doing this in Spring I can use placeholder to get server context path. Example of swagger-config.yaml file:

url: /v1/${server.servlet.context-path}/api-docs/

Will try use one of the other workarounds.

@baincd
Copy link

baincd commented Jun 19, 2022

The latest Swagger UI documentation notes that only swagger-initializer.js needs to be overridden (rather than index.html). Looks like that change happened in swagger-api/swagger-ui#7905

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

No branches or pull requests

10 participants