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

Improve usage stats collection - first- and third-party requests #85869

Closed
jportner opened this issue Dec 14, 2020 · 8 comments
Closed

Improve usage stats collection - first- and third-party requests #85869

jportner opened this issue Dec 14, 2020 · 8 comments
Labels
impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc

Comments

@jportner
Copy link
Contributor

jportner commented Dec 14, 2020

#81907 and #85621 added usage stats collection for various APIs. We attempt to differentiate whether a consumer is "first-party" (the Kibana client) or "third-party" (some other integration) by checking for the presence of kbn-version and referer headers. Originally we also checked for origin too, but discovered that not all first-party requests contain this header.

As discussed in #85706 (comment):

  • The vast majority of our docs instruct consumers to use kbn-xsrf, not kbn-version (which is more strict); however, our Reporting docs currently state to use kbn-version.
  • Currently, we only collect usage data for Saved Objects APIs, Saved Objects Management APIs, and the Copy saved objects APIs.
  • We could add a specific header to differentiate between first- and third-party requests, though this may present its own issues:
    • We really don't have any guarantees that other integrations won't attempt to use this header (though they gain nothing by doing so -- it would just mean that our usage data collection is slightly skewed). However, consumers of these experimental APIs might see Kibana using this header and wrongly assume that they need to use it too.
    • Adding a new header might present problems for users with reverse proxies that may strip headers not in an allow-list; this would cause false negative for first-party request detection

Additionally, we added a regex in Core to check for a space identifier in the base path (#85706 (comment)). We could consider changing that to an explicit check.

@jportner jportner added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! labels Dec 14, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-security (Team:Security)

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-core (Team:Core)

@exalate-issue-sync exalate-issue-sync bot added impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort labels Aug 5, 2021
@stacey-gammon
Copy link
Contributor

Since plugins aren't supposed to access other plugin's HTTP APIs directly, what about exposing the API via two paths, one which uses internal, and having the internal plugin leverage the internal path?

@jportner
Copy link
Contributor Author

Since plugins aren't supposed to access other plugin's HTTP APIs directly, what about exposing the API via two paths, one which uses internal, and having the internal plugin leverage the internal path?

Sure we could do that, it does sound like potentially a lot of work just to improve our usage data though.

FWIW, since I opened this issue, all of our docs have been updated to instruct consumers to use kbn-xsrf.

@pgayvallet
Copy link
Contributor

it does sound like potentially a lot of work just to improve our usage data though.

++ On that. Note that it would also requires changes in all FTR api integration suites to run the suites against both endpoints

@mshustov
Copy link
Contributor

mshustov commented Oct 1, 2021

Since plugins aren't supposed to access other plugin's HTTP APIs directly,

What problem do we want to solve? If we want to prevent pluginA from accessing pluginB endpoints, Core can extend http client-side service to include a plugin-specific header (ie, x-kbn-plugin-origin). With that, the server-side http service can reject a request originated from another plugin.
It won't break public API providing server-side http service doesn't reject incoming requests without this special header.

@stacey-gammon
Copy link
Contributor

What problem do we want to solve?

If the plugin is calling these APIs from the server-side, it won't work properly (from my understanding it will fail but in a non-obvious way). If the plugin is attempting to access from the client-side, they lose out on the benefit of type checking. I have a PR out to document these guidelines here: https://github.com/elastic/kibana/pull/113313/files#diff-3c4e9b75014f3ada216e36cecccffe18d918455747274f9bd8a9d6121d24d793R35

it does sound like potentially a lot of work just to improve our usage data though.

Yea, I agree. I was thinking about the APIs that we want to make internal anyway, and deprecate/remove the public versions. In that case, I think it makes sense to maintain both for a period of time to support a slow migration.

@legrego legrego removed Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! EnableJiraSync labels Aug 11, 2022
@pgayvallet
Copy link
Contributor

I don't think we may want this anymore, closing

@pgayvallet pgayvallet closed this as not planned Won't fix, can't repro, duplicate, stale Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc
Projects
None yet
Development

No branches or pull requests

6 participants