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

Pass incoming request headers to Elasticsearch #6221

Closed
skearns64 opened this issue Feb 12, 2016 · 21 comments
Closed

Pass incoming request headers to Elasticsearch #6221

skearns64 opened this issue Feb 12, 2016 · 21 comments

Comments

@skearns64
Copy link

There are some cases where it would be valuable to have the Kibana backend preserve incoming headers and pass them on to Elasticsearch. The idea is that the headers that the browser sends to the Kibana backend would be attached to the requests that Kibana makes to Elasticsearch on behalf of the user.

I'm sure there are lots of ways to go about it, but a few approaches came to mind: allow configuring specific headers that should be passed through, or simply pass all headers through, either by default or with a setting.

@rashidkpc
Copy link
Contributor

I'd avoid passing them all.

This will need to be made configurable in any case as we log headers and don't want to log say, custom auth headers: epixa@21a6660

@TinLe
Copy link

TinLe commented Feb 12, 2016

Definitely need to be configurable. There are headers that we do not want anywhere near the log :)

Maybe something like a whitelist or blacklist to keep it simple.

@skearns64
Copy link
Author

Great point! I forgot we were logging all the headers, except auth, by default.

In my mind, these are two related, but independent issues. One is making sure certain headers (either listed by name, or *?) get attached to the requests to ES, and the other is making sure we can control what gets logged. I opened #6231 to track the ability to configure which headers are filtered out of the logs.

@PhaedrusTheGreek
Copy link
Contributor

@rashidkpc @skearns64 @tbragin Could it also be possible to set the auth header by URL query string parameter? It seem like setting request headers in an IFrame is not trivial.

@skearns64 skearns64 changed the title Pass incoming request headers to the Elasticsearch Pass incoming request headers to Elasticsearch Mar 18, 2016
@blazerguns
Copy link

This is very important feature for us. Any ETA on this?

@tbragin tbragin added P1 and removed P2 labels Mar 24, 2016
@rashidkpc rashidkpc assigned lukasolson and unassigned lukasolson Mar 31, 2016
@ycombinator
Copy link
Contributor

Might want to keep tabs on the work being done in #6896 as well.

@rashidkpc
Copy link
Contributor

@ycombinator I think with #6896 having been merged, this can be closed yeah?

@ycombinator
Copy link
Contributor

Yes, indeed! Resolved by #6896.

@skearns64
Copy link
Author

@ycombinator - this doesn't appear to be tagged with version info.. which versions is this in?

@ycombinator
Copy link
Contributor

@skearns64 Fixed, based on tags on PR. Thanks!

@skearns64
Copy link
Author

thx @ycombinator! is this a candidate for backport to 4.x?

cc @epixa

@ycombinator
Copy link
Contributor

@skearns64 It is not a candidate for backport to 4.x because it would be a breaking change for anyone relying on the 4.x behavior of all headers being sent along.

@skearns64
Copy link
Author

@ycombinator - Hm.. It was my understanding that NO headers were being passed though, other than Basic Auth in 4.x, which is why we had to make a change in order for SSO systems to send headers through to ES in the first place?

@ycombinator
Copy link
Contributor

ycombinator commented Jun 23, 2016

@skearns64 Looking at the code — specifically this line that was removed as part of the PR that resolved this issue: https://github.com/elastic/kibana/pull/6896/files#diff-f36b4e729966a4e9bce9d19d9137b3bcL19 — it would appear that all headers were being passed through. Its been a while, though, so I think it would be best to check the actual behavior in 4.x. I'll update this issue with what I find.

@ycombinator
Copy link
Contributor

@skearns64 I just verified using 4.4.2 that all headers are being proxied through to Elasticsearch:

Request made from Kibana browser -> Kibana server:

screen shot 2016-06-23 at 9 27 39 am

Request proxied from Kibana server -> Elasticsearch:

screen shot 2016-06-23 at 9 27 48 am

Notice, for instance, the kbn-version version header which gets proxied over from the first request.

@skearns64
Copy link
Author

In the proxy, you're exactly right, but in call_with_request: https://github.com/elastic/kibana/pull/6896/files#diff-f5784d211125172afccd1e7ea1e596feL9 it looks like it looks like it was only passing through the authorization header, right?

If I'm reading that right, then we had inconsistent behavior before, which is a bug, so I interpret it as a bugfix, not a breaking change :)

@ycombinator
Copy link
Contributor

ycombinator commented Jun 23, 2016

You're right, call_with_request does indeed only pass through the Authorization header, if it is set. So there are two ways to make calls to ES from Kibana - the proxy or call_with_request. IF we are to backport, I'd recommend we only backport the call_with_request change, as that won't be breaking. Thoughts?

@skearns64
Copy link
Author

I'm not sure we should backport, I'd like @epixa to weigh in on relative priority, but if we do backport only the call_with_request, I think it would resolve my issue in 4.x - both the proxy and call_with_request would pass through all headers.

(another alternative would be to backport it all, but set the default to pass through all headers, so it's consistent, but non-breaking)

@ycombinator
Copy link
Contributor

ycombinator commented Jun 23, 2016

(another alternative would be to backport it all, but set the default to pass through all headers, so it's consistent, but non-breaking)

Yeah, that could be an option as well - but I wonder if that'll cause confusion where the default will change to almost the complete opposite from 4.x (pass through all headers by default) to 5.0 (pass through only the Authorization header by default).

Agreed on getting @epixa's thoughts on the backporting.

@epixa
Copy link
Contributor

epixa commented Jun 28, 2016

This is really a discussion about an entirely separate 4.x change rather than a backport, so I've created a new issue for making the behavior consistent in 4.x: #7564

@ycombinator
Copy link
Contributor

@skearns64 @tbragin @sherry-ger We have reverted the change in 4.x that would allow all headers to be passed through via callWithRequest. For details on why, see the discussion starting here: #7564 (comment)

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

No branches or pull requests

10 participants