[SO Tagging] Avoid fetching tags when user doesn't have the permission

[pgayvallet]

At the moment, the SO tagging plugin fetches the tags at a fixed interval (15mins by default), for every users, as long as we're authenticated (meaning: not on anonymous pages).

However, to have read permission on the tag type, a user must have access to at least one Kibana feature / application that uses tags (dashboard, visualization...) , or to the savedObjectsManagement feature.

When trying to fetch tags for a user without such permission, an unauthorized error is thrown, and is visible both in the server logs, and in the request's response.

[2023-07-17T16:36:10.786-04:00][ERROR][savedobjects-service.repository.point-in-time-finder] Failed to open PIT for types [tag]

We should avoid such errors, either by:

• detecting when a user doesn't have the appropriate permission from the client-side and then not fetching the tags at all
• detecting when a user doesn't have the appropriate permission from the server-side and return an empty list instead of fetching them
• detect/identify such errors from the server-side when trying to fetch the tags, trap them, and return an empty list instead.

Ihmo the second option would be the best one (as the third option would not get rid of the error in the logs, given it's thrown from a lower layer)

Technical pointers

pooling mechanism on the client side

kibana/x-pack/plugins/saved_objects_tagging/public/plugin.ts

Lines 71 to 84 in 3730dd0

	this.tagCache = new TagsCache({
	refreshHandler: () => this.tagClient!.getAll({ asSystemRequest: true }),
	refreshInterval: this.config.cacheRefreshInterval,
	});
	this.tagClient = new TagsClient({ http, changeListener: this.tagCache });
	this.assignmentService = new TagAssignmentService({ http });
	// do not fetch tags on anonymous page
	if (!http.anonymousPaths.isAnonymous(window.location.pathname)) {
	// we don't need to wait for this to resolve.
	this.tagCache.initialize().catch(() => {
	// cache is resilient to initial load failure. We just need to catch to avoid unhandled promise rejection
	});
	}

the route

kibana/x-pack/plugins/saved_objects_tagging/server/routes/tags/get_all_tags.ts

Lines 11 to 18 in a02c00b

	router.get(
	{
	path: '/api/saved_objects_tagging/tags',
	validate: {},
	},
	router.handleLegacyErrors(async (ctx, req, res) => {
	const { tagsClient } = await ctx.tags;
	const tags = await tagsClient.getAll();

the service opening the PIT

kibana/x-pack/plugins/saved_objects_tagging/server/services/tags/tags_client.ts

Lines 51 to 55 in c6d4fb5

	public async getAll() {
	const pitFinder = this.soClient.createPointInTimeFinder<TagAttributes>({
	type: this.type,
	perPage: 1000,
	});

the error that surfaces when we fetch the tags on behalf of a user without the correct permissions

kibana/packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/point_in_time_finder.ts

Lines 130 to 133 in 9408552

	if (e.output?.statusCode !== 404) {
	this.#log.error(`Failed to open PIT for types [${this.#findOptions.type}]`);
	throw e;
	}

[elasticmachine]

Pinging @elastic/appex-sharedux (Team:SharedUX)

[clintandrewhall]

@pgayvallet I'm actually a fan of "option one": we should have a server and client-side check for if the person has access to tagging functionality. We'd want to hide the UX, prevent the request, and throw if the API endpoint is hit under those circumstances. Is there something I'm missing?

[pgayvallet]

Is there something I'm missing?

I think the major problem here is that we don't have a way to perform such authz / capabilities check from the client-side (at least that I'm aware of, but @elastic/kibana-security would need to confirm).

[legrego]

However, to have read permission on the tag type, a user must have access to at least one Kibana feature / application that uses tags (dashboard, visualization...) , or to the savedObjectsManagement feature.

Do we consider tags to be a fundamental feature of the platform (like the config & telemetry saved objects), or is there a reason for us to gate access based on individual features? If we treat it like the config object, then we can sidestep this issue altogether by automatically granting access to read tags to all authorized Kibana users, but still gate write access based on the existing privileges.

[pgayvallet]

by automatically granting access to read tags to all authorized Kibana users

@legrego IIRC, we did not do that at the time because the security team (Joe? Brandon?) discouraged us from doing so (but it's been a long time, I could remember wrong).

But yes, I think we can consider tags to be a "platform feature", and granting read access automatically to all users may be a easy solution to solve the problem.

[legrego]

IIRC, we did not do that at the time because the security team (Joe? Brandon?) discouraged us from doing so (but it's been a long time, I could remember wrong).

I unfortunately can't remember either, and I very well may have been one of the voices in this conversation.

But yes, I think we can consider tags to be a "platform feature", and granting read access automatically to all users may be a easy solution to solve the problem.

I don't think that this would pose any meaningful risk, but I'll ping the rest of @elastic/kibana-security for further input

[azasypkin]

I don't think that this would pose any meaningful risk, but I'll ping the rest of @elastic/kibana-security for further input

I can't think of any risk as well. For what it's worth, I tried to dig into the original discussion about the Tagging permission model, but couldn't find anything specific about this topic: #74571 (comment).

[legrego]

If we move forward with this, the automated privilege grants are maintained within the features plugin. We'd want to add the tags SO type to the read arrays, alongside the config and url types:

kibana/x-pack/plugins/features/server/feature_registry.ts

Lines 115 to 139 in 02a43a1

	function applyAutomaticAllPrivilegeGrants(
	...allPrivileges: Array<FeatureKibanaPrivileges | undefined>
	) {
	allPrivileges.forEach((allPrivilege) => {
	if (allPrivilege) {
	allPrivilege.savedObject.all = uniq([...allPrivilege.savedObject.all, 'telemetry']);
	allPrivilege.savedObject.read = uniq([...allPrivilege.savedObject.read, 'config', 'url']);
	}
	});
	}
	function applyAutomaticReadPrivilegeGrants(
	...readPrivileges: Array<FeatureKibanaPrivileges | undefined>
	) {
	readPrivileges.forEach((readPrivilege) => {
	if (readPrivilege) {
	readPrivilege.savedObject.read = uniq([
	...readPrivilege.savedObject.read,
	'config',
	'telemetry',
	'url',
	]);
	}
	});
	}

[ijardillier]

Any news about this issue ?
We have alerting on Kibana errors and this problem often generates alerts :-(
Thanks

[petrklapka]

@vadimkibana - are you planning on wrapping this up or can we re-assign to someone else for quick attention?