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

rest-explorer doesn't work behind path-mapping proxy #2285

Closed
mgabeler-lee-6rs opened this issue Jan 25, 2019 · 3 comments · Fixed by #3133
Closed

rest-explorer doesn't work behind path-mapping proxy #2285

mgabeler-lee-6rs opened this issue Jan 25, 2019 · 3 comments · Fixed by #3133

Comments

@mgabeler-lee-6rs
Copy link
Contributor

Description / Steps to reproduce / Feature proposal

The rest-explorer component doesn't work if the app is behind a proxy that does path mapping -- e.g. external requests made to https://example.com/servicename/api-endpoint, which is mapped to http://localhost:3000/api-endpoint in a docker container or the like. There is no way to make the explorer component or the external explorer redirect work in this environment that I can find.

Current Behavior

In the normal setup, the explorer tells the client to load /openapi.json, which doesn't work, because it needs to load https://example.com/servicename/openapi.json. The external redirect has the same problem.

Expected Behavior

  1. It should be possible to configure things such that the openapi.json correctly references paths when behind a proxy that inserts a path prefix
  2. It should be possible to configure the rest-explorer component to work in this context too

Other Notes

This is loosely related to #1702 but that only dealt with host/port issues, not path issues.

This is more closely related to #914

Internally we do not want to use the externally hosted API explorer, but the issues with the base path in the API spec serving mean that I couldn't make that work either even if I wanted to.

Workaround & possible fix?

I was able to work around this, but it is not pretty:

  1. Set rest.openApiSpec.servers to reflect the proxy path mapping -- this seems to be roughly equivalent to the LB2 config item, and seems reasonable
  2. Override the ExplorerController class with a local sub-class that patches its private openApiSpecUrl member to be aware of the rest.openApiSpec.servers value, and setup the app binding to use this class instead of the normal one. Similarly also had to patch up the indexRedirect method to be aware of this path prefix.

The second item is rather icky given the class I'm extending is not exported, and the member I'm modifying is private.

However, making the normal ExplorerController aware of the openApiSpec.servers list seems like it might be a reasonable fix for this? However doing that "for real" has some issues I'm not sure how to address:

  1. What if there are multiple servers defined? Which one should it use? Using the first one is a simple answer, but it's not at all clear to me that it is the correct answer, esp. since I'm not clear on any precedence rules that might be implied by order in that list.
  2. What if openApiSpec.setServersFromRequest is enabled? The logic that handles that is not available to the ExplorerController, and it's not clear to me how that should interact with this concern. On the other hand, the way that feature works would not play nice with a proxy that inserts path components, so maybe that is a moot question?
@bajtos
Copy link
Member

bajtos commented Jan 25, 2019

Set rest.openApiSpec.servers to reflect the proxy path mapping -- this seems to be roughly equivalent to the LB2 config item, and seems reasonable

This looks like the right solution to me, what downsides do you see yourself? When a LB4 app is running behind a path-mapping proxy, there is no way for LB4 runtime to figure out the URL where the API is publicly available. That's why we are providing the configuration option rest.openApiSpec.servers. Just make sure to disable rest.openApiSpec.setServersFromRequest as well!

However, making the normal ExplorerController aware of the openApiSpec.servers list seems like it might be a reasonable fix for this.

Yes please, that makes a lot of sense to me!

I think it was an oversight on my side that the initial implementation assumes the LB4 app is running at a root path.

https://github.com/strongloop/loopback-next/blob/84202bb2a777fc2aaaa69328feaf3022073a1927/packages/rest-explorer/src/rest-explorer.controller.ts#L51-L59

What if there are multiple servers defined? Which one should it use? Using the first one is a simple answer, but it's not at all clear to me that it is the correct answer, esp. since I'm not clear on any precedence rules that might be implied by order in that list.

We can improve in increments. Let's start with using the first server entry for the first iteration and wait until there are users asking for more complex rule.

What if openApiSpec.setServersFromRequest is enabled? The logic that handles that is not available to the ExplorerController, and it's not clear to me how that should interact with this concern. On the other hand, the way that feature works would not play nice with a proxy that inserts path components, so maybe that is a moot question?

As I understand the matter, setServersFromRequest is not compatible with path-inserting proxies, so as you wrote yourself, this should not be a concern.

@mgabeler-lee-6rs
Copy link
Contributor Author

Thank you for the feedback! In this case I'll put together a PR to implement the servers awareness in the rest-explorer 😃

@mgabeler-lee-6rs
Copy link
Contributor Author

mgabeler-lee-6rs commented Jun 12, 2019

The prior path turned out to be complicated ... overly so. Replacing with new PR #3133 that takes a simpler path of having the explorer add an additional endpoint for the OAS spec that it can reference via a relative path that is constant in relation to the explorer itself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants