Skip to content

Resolve user settings once per page render - #291066

Merged
elena-shostak merged 4 commits into
elastic:mainfrom
elena-shostak:fix/single-user-settings-lookup-on-render
Sep 15, 2026
Merged

Resolve user settings once per page render#291066
elena-shostak merged 4 commits into
elastic:mainfrom
elena-shostak:fix/single-user-settings-lookup-on-render

Conversation

@elena-shostak

@elena-shostak elena-shostak commented Sep 15, 2026 •
edited by kibanamachine
Loading

Copy link
Copy Markdown
Contributor

Summary

Fixes a race in server-side page rendering that triggers duplicate user profile activations.

RenderingService.render resolves the current user's dark mode and locale through two separate calls on the user settings service (getUserSettingDarkMode and getUserSettingLocale) inside the same Promise.all. Each call performs its own userProfile.getCurrent lookup. For a request authenticated with Authorization: Basic and no session cookie, getCurrent activates the user profile on every call, so a single page render fired two concurrent activateUserProfile writes for the same profile document. The two writes race on .security-profile-8, and when one loses Elasticsearch returns a 409 version conflict that Kibana logs at ERROR before retrying:

Failed to activate user profile (retries left: 9): version_conflict_engine_exception ...

The retry always succeeds, so this is log noise rather than a functional failure, but it fires on every basic-auth page load (for example, health checks or probes hitting an app URL with credentials). The second lookup was introduced with the per-user locale setting in 9.5.0.

Changes

  • UserSettingsService exposes a single getUserSettings(request) that performs one profile lookup and returns dark mode, locale, and remember-selected-space together. The existing single-value getters delegate to it.
  • RenderingService calls getUserSettings once and reads both values from the result.
  • Mock and tests updated. A new unit test asserts the combined lookup fetches the profile exactly once.

How to verify

Enable plugins.security.user-profile at debug, then request a page with basic credentials and no cookie:

curl -s -o /dev/null -u elastic:changeme -L http://localhost:5601/

Before: two Activating user profile via password grant lines per request. After: one.

Release note

Fixed duplicate user profile activations during page rendering for requests authenticated with basic credentials, which could log spurious version_conflict_engine_exception errors.

Checklist

  • Unit or functional tests were updated or added to match the most common scenarios
  • The PR description includes the appropriate Release Notes section, and the correct release_note:* label is applied per the guidelines
  • Review the backport guidelines and apply applicable backport:* labels.

🤖 Generated with Claude Code

@elena-shostak @claude
Resolve user settings once per page render
dcc89a3 
The rendering service fetched dark mode and locale through two separate
user settings calls inside the same Promise.all. Each call resolves the
current user profile independently, so for requests authenticated with
Basic credentials and no session both calls activated the same profile
concurrently and raced on the .security-profile index, surfacing as 409
version conflicts logged at ERROR.

Expose a single getUserSettings lookup on the user settings service and
use it from the rendering service so a page render performs one profile
lookup.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@elena-shostak elena-shostak added Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// release_note:fix backport:version Backport to applied version labels labels Sep 15, 2026
@kibanamachine kibanamachine added the reviewer:libra PR review with Libra. This disables Claude and Scout reviewers label Sep 15, 2026
@kibanamachine

kibanamachine commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Selected for Libra review

This PR was selected for Libra review as part of the temporary 50% trial.

To opt out permanently, remove the reviewer:libra label. It will not be added again to this PR.

@elena-shostak elena-shostak added backport:all-open Backport to all branches that could still receive a release and removed backport:version Backport to applied version labels labels Sep 15, 2026
@elena-shostak
elena-shostak marked this pull request as ready for review September 15, 2026 14:27
@elena-shostak
elena-shostak requested review from a team as code owners September 15, 2026 14:27
@infra-vault-gh-plugin-prod

infra-vault-gh-plugin-prod Bot commented Sep 15, 2026

Copy link
Copy Markdown

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

kibanamachine
kibanamachine reviewed Sep 15, 2026
View reviewed changes

@kibanamachine kibanamachine left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Libra found 1 issue.

Generated by Libra

Comment thread src/core/packages/rendering/server-internal/src/rendering_service.test.ts
jeramysoucy
jeramysoucy approved these changes Sep 15, 2026
View reviewed changes

@jeramysoucy jeramysoucy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Approving, but before you merge the Libra review is worth looking at: #291066 (comment)

And 1 nit: getUserSettingLocale and getUserSettingRememberSelectedSpace have no production callers left after this. Should these be removed?

elena-shostak and others added 2 commits September 15, 2026 17:58
@elena-shostak @claude
Address PR review: assert getUserSettings call-site invariants and dr…
531d9dc 
…op dead getters

Add rendering_service tests asserting getUserSettings is called exactly once
per authenticated render and not at all for anonymous renders. Remove the
getUserSettingLocale and getUserSettingRememberSelectedSpace methods from the
InternalUserSettingsServiceSetup interface and implementation; they have no
production callers since rendering switched to the combined getUserSettings.
Update the mock and service tests accordingly.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@elena-shostak
Merge branch 'main' into fix/single-user-settings-lookup-on-render
6c8f83f
@elena-shostak
elena-shostak enabled auto-merge (squash) September 15, 2026 16:06
@kibanamachine

kibanamachine commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

✅ unchanged

Test Failures

  • [job] [logs] Jest Tests #6 / CasesWebhookActionConnectorFields renders Step Validation Step 2 is properly validated
  • [job] [logs] Jest Tests #6 / CasesWebhookActionConnectorFields renders Step Validation Step 3 is properly validated
  • [job] [logs] Scout Lane #113 - serverless-observability_complete / default / local-serverless-observability_complete - statusRulePendingThreshold - waits for two consecutive pending evaluations before firing when pendingThreshold is 2
  • [job] [logs] Scout Lane #113 - serverless-observability_complete / default / local-serverless-observability_complete - Synthetics manage rules privilege - lets a read user with can_manage_rules enable default alerting
  • [job] [logs] Scout Lane #23 - stateful-classic / default / local-stateful-classic - Synthetics manage rules privilege - lets a read user with can_manage_rules enable default alerting

History

@elena-shostak
elena-shostak merged commit a4346ae into elastic:main Sep 15, 2026
41 checks passed
@kibanamachine

kibanamachine commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Starting backport for target branches: 8.19, 9.4, 9.5

https://github.com/elastic/kibana/actions/runs/34997779614

@kibanamachine

kibanamachine commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

💔 All backports failed

Status Branch Result
❌ 8.19 Backport failed because of merge conflicts
❌ 9.4 Backport failed because of merge conflicts
❌ 9.5 Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 291066

Questions ?

Please refer to the Backport tool documentation

@elena-shostak elena-shostak mentioned this pull request Sep 15, 2026
Open
@elena-shostak

elena-shostak commented Sep 15, 2026

Copy link
Copy Markdown
Contributor Author

💚 All backports created successfully

Status Branch Result
✅ 9.5

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

@elena-shostak

elena-shostak commented Sep 15, 2026

Copy link
Copy Markdown
Contributor Author

9.5 is the only version we need to backport this

@elena-shostak elena-shostak added backport:version Backport to applied version labels and removed backport:all-open Backport to all branches that could still receive a release labels Sep 15, 2026
@kibanamachine

kibanamachine commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Starting backport for target branches: 9.5

https://github.com/elastic/kibana/actions/runs/35017812513

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

Reviewers

@kibanamachine kibanamachine kibanamachine left review comments
@jesuswr jesuswr jesuswr approved these changes
@jeramysoucy jeramysoucy jeramysoucy approved these changes

Assignees

No one assigned

Labels

backport:version Backport to applied version labels release_note:fix reviewer:libra PR review with Libra. This disables Claude and Scout reviewers Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// v9.5.5 v9.6.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@elena-shostak @kibanamachine @jesuswr @jeramysoucy