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

$ref downloading doesn't use requestInterceptor #3763

Closed
scottohara opened this issue Oct 15, 2017 · 9 comments · Fixed by #3830
Closed

$ref downloading doesn't use requestInterceptor #3763

scottohara opened this issue Oct 15, 2017 · 9 comments · Fixed by #3830

Comments

@scottohara
Copy link
Contributor

Q A
Bug or feature request? Bug
Which Swagger/OpenAPI version? 2.0
Which Swagger-UI version? 3.3.2
How did you install Swagger-UI? git checkout, dist dir
Which browser & version? Chrome 63
Which operating system? macOS 10.13

Demonstration API definition

  1. The 'petstore' example from here:
    https://github.com/OAI/OpenAPI-Specification/blob/master/examples/v2.0/json/petstore.json

  2. The 'petstore-separate' example from here:
    https://github.com/OAI/OpenAPI-Specification/tree/master/examples/v2.0/json/petstore-separate

...both hosted on a local server that requires basic authentication.

Configuration (browser query string, constructor, config.yaml)

{
  url: "http://locahost:3000/spec/swagger.json",  /* or http://localhost:3000/petstore.json */
  requestInterceptor: (req) => {
    if (req.loadSpec) {
        req.headers.Authorization = 'Basic ' + btoa("admin:secret");
    }
    return req;
  },
  ...
}

Expected Behavior

In swagger-ui@3.3.2, #3738 fixes a gap where the requestInterceptor() wasn't being invoked for the download of the spec URL.

Current Behavior

I have been testing this feature and can confirm that it works as expected for the initial spec URL.

For example, when loading the simple petstore.json example, the requestInterceptor is invoked and the correct Authorization: Basic <base64(admin:secret)> header is passed in the fetch.

Similarly, for the 'separate' example, the requestInterceptor is invoked on the fetch of spec/swagger.json and the correct Authorization header is passed.

However, when resolving and fetching any the $ref URLs in spec/swagger.json (i.e. spec/parameters.json, spec/Pet.json, spec/NewPet.json and common/Error.json), the requestInterceptor doesn't appear to be invoked; and as a result these sub-fetches fail with a 401 Unauthorized.

Possible Solution

Ensure that requestInterceptor is correctly called for any $ref downloads.

@shockey
Copy link
Contributor

shockey commented Oct 16, 2017

I overlooked this detail 😬 the initial fetch is handled in Swagger-UI, but Swagger-Client handles $ref resolution.

Relevant code is here: https://github.com/swagger-api/swagger-js/blob/master/src/resolver.js#L5

@CarstenRilke
Copy link

@shockey: Thanks for the very fast fixing of #2793 !
Unfortunately, the behavior described here prevents the modular desing of "protected" APIs.
This is a very important design / documentation element for us because we have hundreds of reusable payloads in our interfaces.

Is it foreseeable when the Swagger-Client will support handling the $ref URLs from a protected server?

@shockey
Copy link
Contributor

shockey commented Oct 16, 2017

@CarstenRilke, I can't give an estimate on this - it depends on how much work is involved in handing off the UI's interceptors to the Swagger-Client resolver.

I'll be taking a look at it within the next couple of days, but no estimate on a fix. Stay tuned!

@shockey
Copy link
Contributor

shockey commented Oct 20, 2017

Good news - this is fixed!

The patch will be released tomorrow evening as part of our weekly release.

Thanks for the report, @scottohara!

@scottohara
Copy link
Contributor Author

You do an amazing job, @shockey. Many thanks.

@CarstenRilke
Copy link

Thanks for the fast fix!
I'm really looking forward to the new release.
... an incredible response speed, Thank you!

@scottohara
Copy link
Contributor Author

Have been testing v3.4.0 today...unfortunately I've not yet been able to confirm that this change was successful.

The $ref fetches don't seem to be invoking the requestInterceptor like the main URL fetch does. This can be seen in the screenshots below:

image

The swagger.json URL correctly invokes the requestInterceptor which adds an Authorization header to to the fetch:

image

...but the $ref downloads aren't doing the same (no Authorization header):

image

In the Chrome DevTools debugger, setting a breakpoint in my requestInterceptor I can see it being invoked only for the main swagger.json fetch.

image

@shockey shockey reopened this Oct 26, 2017
@shockey
Copy link
Contributor

shockey commented Oct 26, 2017

@scottohara, I believe you're right. It appears that Swagger-UI is only providing its interceptors at request time, which doesn't give Swagger-Client the chance to use them in the newly-aware-of-interceptors resolver.

My apologies here, I didn't test the client changes against the UI before shipping. I'll look into this today, hopefully I can patch it up for this week's release.

@shockey
Copy link
Contributor

shockey commented Oct 26, 2017

Okay, this should be working now - I tested it on my end before merging #3830.

This will be released tomorrow evening as part of our weekly release.

Thanks everyone!

@lock lock bot locked and limited conversation to collaborators Jul 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants