Various background API calls extend session idle timeout

[jportner]

Kibana version: 7.10 (not yet released)

Original install method (e.g. download page, yum, from source, etc.): from source

Describe the bug: The session idle timeout should log users out after a period of inactivity. This happens both on the client side and the server side. However, depending upon what page you are viewing, API calls periodically occur in the background, which potentially extends the user's session indefinitely. This includes, but is not limited to:

• POST /api/ui_metric/report (UI metrics collection -- runs on all pages, but only once it seems? update 2021/02/05: this actually seems to run every 15 minutes or so, even if the user is completely idle and hasn't clicked anything)
• POST /api/index_management/indices/reload (Index Management page)
• POST /api/index_lifecycle_management/policies?withIndices=true (Index Management page)
• GET /api/remote_clusters (Remote Clusters page)
• (7.11 and newer) GET /api/saved_objects_tagging/tags (all pages?)

Steps to reproduce:

1. Set a low session idle timeout value, example: xpack.security.session.idleTimeout: 75000
2. Start Kibana and log in, and navigate to the Index Management page
3. Open your browser's developer tools and view the "Network" tab
4. Observe the session timeout warning toast that pops up in 10 seconds
5. Observe that, at some point in the next minute, an API call to report is made, and the warning toast disappears

Expected behavior: Unattended background actions should not extend the user's session.

Any additional context: These API calls should use the kbn-system-request header to indicate that it was not a user request and should not extend the session.

[elasticmachine]

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

[jportner]

@elastic/kibana-platform it'll be extremely time consuming to attempt to audit every single page that's making background API calls and change them to use the system request header. Even then, we'll be sure to have this problem again when new pages are added and new API calls are used. How do you feel about changing the fetch utility's asSystemRequest option to default to true?

kibana/src/core/public/http/fetch.ts

Lines 118 to 151 in 82a6ddb

	private createRequest(options: HttpFetchOptionsWithPath): Request {
	// Merge and destructure options out that are not applicable to the Fetch API.
	const {
	query,
	prependBasePath: shouldPrependBasePath,
	asResponse,
	asSystemRequest,
	...fetchOptions
	} = {
	method: 'GET',
	credentials: 'same-origin',
	prependBasePath: true,
	...options,
	// options can pass an `undefined` Content-Type to erase the default value.
	// however we can't pass it to `fetch` as it will send an `Content-Type: Undefined` header
	headers: removedUndefined({
	'Content-Type': 'application/json',
	...options.headers,
	'kbn-version': this.params.kibanaVersion,
	}),
	};
	const url = format({
	pathname: shouldPrependBasePath ? this.params.basePath.prepend(options.path) : options.path,
	query: removedUndefined(query),
	});
	// Make sure the system request header is only present if `asSystemRequest` is true.
	if (asSystemRequest) {
	fetchOptions.headers['kbn-system-request'] = 'true';
	}
	return new Request(url, fetchOptions as RequestInit);
	}

I think we can more easily undertake a one-time audit of pages and change them to use asSystemRequest: false as needed. CC @elastic/kibana-security

[mshustov]

How do you feel about changing the fetch utility's asSystemRequest option to default to true?

That wouldn't solve the problem completely - we still have to audit all places of use to ensure that the correct header is set. Otherwise, it will worsen UX by unexpected user logout. Note that decision whether a call is system is context-dependent and might be different for the same API, which makes audit by an external team (security or platform) even harder.
I haven't been here when asSystemRequest was introduced. Have we ever considered using other indicators - user actions in UI, for example? I don't see any discussions in #8047 From the issue, it seems to have been introduced for different purposes.

[legrego]

Have we ever considered using other indicators - user actions in UI, for example? I don't see any discussions in #8047 From the issue, it seems to have been introduced for different purposes.

We have a similar issue here, with a similar lack of discussion: #18751.

I agree that relying on this "system request" feature is problematic for determining user activity. I think it would be great if we could use other mechanisms for figuring out if a session is "active" or not. For example, we could use the Page Visibility API to determine if the tab is active or not. If the tab is active, then we can use additional signals (TBD?) to determine activity. If the tab is inactive, then that's a good indication that the session is not actively being used (barring things like background searches).

[jportner]

That wouldn't solve the problem completely - we still have to audit all places of use to ensure that the correct header is set. Otherwise, it will worsen UX by unexpected user logout.

Yes, I agree it doesn't solve the problem completely. It's certainly a trade-off.

If we change the default to isSystemRequest:true, we risk false negatives for detecting user activity; the impact is that the user may see the "Your session will time out soon" toast more often. Conversely, if we leave today's behavior in place, we risk false positives for detecting user activity; the impact is that the user's session may stay open indefinitely, which leaves a much greater exposure window for session hijacking attacks.

Considering that session hijacking is the core concern of anyone who chooses to use the idle timeout setting, I feel pretty strongly that we should err on the side of caution for this behavior.

Note that decision whether a call is system is context-dependent and might be different for the same API, which makes audit by an external team (security or platform) even harder.

Agreed, we could use this opportunity to educate code owners.

I agree that relying on this "system request" feature is problematic for determining user activity. I think it would be great if we could use other mechanisms for figuring out if a session is "active" or not.

So, are you thinking we could make the suggested change in addition to adding these new signals?

[legrego]

So, are you thinking we could make the suggested change in addition to adding these new signals?

I don't have a concrete suggestion just yet 😬. My fear is that continuing to rely on asSystemRequest is going to become a continuous game of whack-a-mole, no matter which direction we take for the default. We have a hard time writing a proper functional tests for session timeout today, and we can't possibly have discrete timeout tests for each page of our growing list of solutions.

If we were starting fresh, I think having discrete functions to call would make engineers consider this more carefully -- for example, on the server we have callAsInternalUser and callAsCurrentUser, which makes the distinction quite clear.

Compare that to the client-side, where we have a set of shared functions which you can optionally call as a "normal" or "system" request. The fact that asSystemRequest is an optional parameter makes this very easy to overlook.

We could update these functions to make asSystemRequest a required argument, but that will touch a lot of files.

If we change the default to isSystemRequest:true, we risk false negatives for detecting user activity; the impact is that the user may see the "Your session will time out soon" toast more often.

I don't expect that solution teams will keep this in mind, despite education efforts. I think this would lead to a poor user experience, since this is something that is likely to introduce session bugs fairly frequently as new requests are added. I agree this is the more secure approach, but I'm not yet convinced that the tradeoff is worth it. Perhaps this is something we can consider alongside 8.0 since we'll have sensible defaults in place for session timeouts?

[legrego]

We could update these functions to make asSystemRequest a required argument, but that will touch a lot of files.

I looked into this briefly, and I'm not sure it will be adequate. core.http.fetch is consumed by other common shared services, such as the saved objects client. The SOC has no way of knowing if the caller is making a system request vs a user-initiated request, and extending the SOC API to include this flag feels very wrong to me.

[legrego]

I wonder if we could take queues from input events:

• mousemove
• mousedown
• keypress
• touchmove
• scroll

If we don't detect any of these after a certain amount of time, then we could consider the session inactive. This won't cover all cases though, since I expect that dashboards with auto-refresh set won't want to have their session expired just because that session is being used passively instead of actively. We could detect if Kibana is in full-screen mode, and never expire those sessions.

This is consistent with how computers detect activity today: no input == no activity, unless consuming media or similar.

[pgayvallet]

How do you plan on forwarding that 'activity' information from the client to the backend? Would you be performing a call at a given interval to the backend (in the case there actually WAS user activity), and use this new endpoint as the sole indicator for session refresh?

[legrego]

How do you plan on forwarding that 'activity' information from the client to the backend? Would you be performing a call at a given interval to the backend (in the case there actually WAS user activity), and use this new endpoint as the sole indicator for session refresh?

Yeah that's exactly what I had in mind. Do you foresee issues with this approach?

[pgayvallet]

No, it sounds good to me.

[jportner]

I updated the issue description, it seems that the call to /api/ui_metric/_report is actually on some sort of interval, and as of 7.11 the client also makes repeated calls to /api/saved_objects_tagging/tags. With this in mind, probably all pages in Kibana are affected, and we really need to change these requests to use the kbn-system-request header until we have a better solution for determining user activity. I hope to submit a PR for this very soon.

[legrego]

@thomheymann and I discussed this issue a bit today -- we have an idea for a middle ground that will help to mitigate this, but not completely solve.

All of our supported browsers support the Page Visibility API, which allows us to detect if the tab is active or in the background.

If we detect that the tab is in the background (or the window is minimized), then we can treat all API calls as "system requests" by default (unless the call explicitly opts out of this via asSystemRequest: false). This means that a rogue API call will only extend the session idle timeout of the tab is currently active.

If we combine this approach with a solution for client-side activity (see #18751, or my comment above), then I think we will have a much more accurate system for detecting session in/activity