[usageCollection] /api/stats endpoint throw errors for unauth users when asking for extended stats

[pgayvallet]

(yeah, fairly long title)

The /api/stats endpoint from the usage_collection plugin

kibana/src/plugins/usage_collection/server/routes/stats/stats.ts

Lines 53 to 55 in 8a9789b

	router.get(
	{
	path: '/api/stats',

Can eventually accepts unauthenticated requests when the status.allowAnonymous config setting is true

kibana/src/plugins/usage_collection/server/routes/stats/stats.ts

Lines 56 to 57 in 8a9789b

	options: {
	authRequired: !config.allowAnonymous,

However, when asking for extended stats, the handler performs a request against ES using the es-scoped client:

kibana/src/plugins/usage_collection/server/routes/stats/stats.ts

Lines 76 to 81 in 8a9789b

	if (isExtended) {
	const core = await context.core;
	const { asCurrentUser } = core.elasticsearch.client;
	// as of https://github.com/elastic/kibana/pull/151082, usage will always be an empty object.
	const clusterUuid = await getClusterUuid(asCurrentUser);

So when a unauthenticated user accesses the endpoint with the extended: true option, the request against ES fails, causing errors similar to:

security_exception: missing authentication credentials for REST request [/?filter_path=cluster_uuid]

We should decide if it's fine to use the internal client instead of the user-scoped one for this call:

kibana/src/plugins/usage_collection/server/routes/stats/stats.ts

Lines 47 to 51 in 8a9789b

	const getClusterUuid = async (asCurrentUser: ElasticsearchClient): Promise<string> => {
	const body = await asCurrentUser.info({ filter_path: 'cluster_uuid' });
	const { cluster_uuid: uuid } = body;
	return uuid;
	};

Otherwise, we should forbid to use the extended option for anonymous users and throw a more explicit error instead.

cc @azasypkin @lukeelmers

[elasticmachine]

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

[pgayvallet]

Note that @azasypkin checked, and we would be fine using the internal user for this call in term of permissions:

If the Elasticsearch security features are enabled, you must have the monitor or manage cluster privilege to use this API

and kibana_system has the monitor role.

[pgayvallet]

Ihmo we would be fine using the internal user, given the only ES call that is performed is this info({ filter_path: 'cluster_uuid' }) call. I can't think of any risk of performing that call with the internal kibana user on behalf of unauthenticated users.

In that case, I also think we should just always use the internal client (not only for unauth users), as it's less branching and divergence, and makes it easier to test.

[azasypkin]

In that case, I also think we should just always use the internal client (not only for unauth users), as it's less branching and divergence, and makes it easier to test.

I agree. My only request would be to include an !!!IMPORTANT!!!-like comment in the code that would emphasize the need for careful assessment of any changes in the calls we make with the internal ES client and what we return since it can potentially be triggered by an unauthenticated user.

[lukeelmers]

Just noting that this will be a blocker for autoscaling as it is preventing us from getting the metrics we need /cc @Bamieh

It shouldn't prevent us from progressing on the kibana-controller work, but it will block us from testing the whole thing end-to-end.

[pgayvallet]

I just self-assigned the issue. Unless we missed something big, it should be trivial and addressed this week.

We missed the 8.9 FF, but we could eventually still backport it, given it could easily be labeled as bug. @lukeelmers do you think it would be necessary, or are we fine merging it only on 8.10?

[lukeelmers]

We missed the 8.9 FF, but we could eventually still backport it, given it could easily be labeled as bug. @lukeelmers do you think it would be necessary, or are we fine merging it only on 8.10?

No strong feelings on that; most important is getting it into main. I don't think it is particularly high-risk to backport to 8.9, but also the issue has been present long enough that I think it's okay to just have it released in 8.10.