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

Request object should provide original url after url re-writing #80485

Closed
mshustov opened this issue Oct 14, 2020 · 4 comments · Fixed by #80810
Closed

Request object should provide original url after url re-writing #80485

mshustov opened this issue Oct 14, 2020 · 4 comments · Fixed by #80810
Assignees
Labels
enhancement New value added to drive a business result Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc

Comments

@mshustov
Copy link
Contributor

Kibana Platform allows plugins to rewrite URL with onPreRouting interceptor.

rewriteUrl(url: string): OnPreRoutingResult {

There is no way to get the original url at the moment. The platform should support this case - otherwise, plugins have to implement error-prone workarounds #74640 (comment)

@mshustov mshustov added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc enhancement New value added to drive a business result labels Oct 14, 2020
@mshustov mshustov self-assigned this Oct 14, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform (Team:Platform)

@thomheymann thomheymann linked a pull request Oct 14, 2020 that will close this issue
4 tasks
@thomheymann thomheymann removed a link to a pull request Oct 14, 2020
4 tasks
@thomheymann thomheymann mentioned this issue Oct 14, 2020
4 tasks
@mshustov
Copy link
Contributor Author

mshustov commented Oct 16, 2020

@thomheymann I didn't find hapi provides this information. Before adding this on the platform level, I want to clarify a couple of questions:

  • What if we pass an original URL via a request header?
    What part of url do you need? only pathname, I suppose? would it suffices x-kibana-rewritten-pathname: /s/space_name/.... Should you need to access other url parts (port, host, etc.), we can add them as x-kibana-rewritten-port, etc. similarly to the same manner as h2o2 provides the original address
  'x-forwarded-for': '127.0.0.1',
  'x-forwarded-port': '60666',
  'x-forwarded-proto': 'http',
  'x-forwarded-host': 'localhost:5601',
  • an incoming request url can be rewritten several times. Do you need an original pathname, I suppose? So the header shouldn't be overridden if presents.

@thomheymann
Copy link
Contributor

I don't know for sure what the platform APIs allow you to rewrite but if plugin developers can only change the pathname, then that is all I need to get the original url. Do the query params stay intact if a plugin rewrites the pathname?

I get that adding a headers is a good approach if an external system does the url rewriting but think that if it's Kibana itself I'd prefer if that would be exposed on the url/request object itself. You've got much more context though so happy to go with your suggestion.

@mshustov
Copy link
Contributor Author

mshustov commented Oct 16, 2020

if plugin developers can only change the pathname, then that is all I need to get the original url

No, a plugin passes the final URL, therefore, it might change any part of it, including query. originalUrl-like property should be a serialized URL object then.

I get that adding a headers is a good approach if an external system does the url rewriting but think that if it's Kibana itself I'd prefer if that would be exposed on the url/request object itself. Ok, it might be better to

I don't want to add a custom property to URL object. Adding to KibanaRequest object sounds acceptable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New value added to drive a business result Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants