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

Allow UserProfileSettingsClient to be created on Security Plugin Setup #170172

Merged

Conversation

kc13greiner
Copy link
Contributor

@kc13greiner kc13greiner commented Oct 30, 2023

Summary

Closes #155949

The Security Plugin provides a UserProfileSettings client to the core UserSettingsService on Setup.

With the first implementation of Dark Mode, the client was not set until the security UserSettingService was started, which violates the setup/start contract best practices.

Now the client is provided during security plugin setup and the security UserSettingService will be set on start.

Testing

Dark Mode should still work as it did before:

  • Login as elastic

  • Navigate to User Profile by clicking the Avatar menu > Edit Profile

  • Choose Dark Mode, Save, and Refresh

  • Bonus test: double check Adv. Settings Dark Mode vs User Profiles Dark Mode

…n setup and set with UserSettingServiceStart on Start to adhere to the setup/start best practices
@kc13greiner kc13greiner added chore release_note:skip Skip the PR/issue when compiling release notes Feature:Security/User Profile v8.12.0 v8.11.1 Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! labels Oct 30, 2023
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #2 / endpoint endpoint list when initially navigating to page when there is data, "before all" hook for "finds page title"
  • [job] [logs] FTR Configs #21 / Serverless Common UI - Home Page home page "before all" hook for "has project header"

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@kc13greiner kc13greiner marked this pull request as ready for review November 1, 2023 18:23
@kc13greiner kc13greiner requested a review from a team as a code owner November 1, 2023 18:23
@elasticmachine
Copy link
Contributor

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

@jeramysoucy jeramysoucy self-requested a review November 1, 2023 22:21
Copy link
Contributor

@jeramysoucy jeramysoucy left a comment

Choose a reason for hiding this comment

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

LGTM

@kc13greiner kc13greiner merged commit 195ef3d into elastic:main Nov 2, 2023
28 checks passed
@kc13greiner kc13greiner deleted the feature/fix_user_settings_client_start branch November 2, 2023 14:51
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 2, 2023
elastic#170172)

## Summary

Closes elastic#155949

The Security Plugin provides a UserProfileSettings client to the core
UserSettingsService on Setup.

With the first implementation of Dark Mode, the client was not set until
the security UserSettingService was started, which violates the
setup/start contract best practices.

Now the client is provided during security plugin setup and the security
UserSettingService will be set on start.

## Testing

Dark Mode should still work as it did before:

- Login as elastic
- Navigate to User Profile by clicking the Avatar menu > Edit Profile
- Choose Dark Mode, Save, and Refresh

- Bonus test: double check Adv. Settings Dark Mode vs User Profiles Dark
Mode

(cherry picked from commit 195ef3d)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.11

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

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Nov 2, 2023
…in Setup (#170172) (#170429)

# Backport

This will backport the following commits from `main` to `8.11`:
- [Allow UserProfileSettingsClient to be created on Security Plugin
Setup (#170172)](#170172)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT
[{"author":{"name":"Kurt","email":"kc13greiner@users.noreply.github.com"},"sourceCommit":{"committedDate":"2023-11-02T14:51:43Z","message":"Allow
UserProfileSettingsClient to be created on Security Plugin Setup
(#170172)\n\n## Summary\r\n\r\nCloses
https://github.com/elastic/kibana/issues/155949\r\n\r\nThe Security
Plugin provides a UserProfileSettings client to the
core\r\nUserSettingsService on Setup.\r\n\r\nWith the first
implementation of Dark Mode, the client was not set until\r\nthe
security UserSettingService was started, which violates
the\r\nsetup/start contract best practices.\r\n\r\nNow the client is
provided during security plugin setup and the
security\r\nUserSettingService will be set on start.\r\n\r\n##
Testing\r\n\r\nDark Mode should still work as it did before:\r\n\r\n-
Login as elastic\r\n- Navigate to User Profile by clicking the Avatar
menu > Edit Profile\r\n- Choose Dark Mode, Save, and Refresh\r\n\r\n-
Bonus test: double check Adv. Settings Dark Mode vs User Profiles
Dark\r\nMode","sha":"195ef3dc2bd08fad54a0ea8278abffb4f39d39bc","branchLabelMapping":{"^v8.12.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["chore","Team:Security","release_note:skip","Feature:Security/User
Profile","v8.12.0","v8.11.1"],"number":170172,"url":"https://github.com/elastic/kibana/pull/170172","mergeCommit":{"message":"Allow
UserProfileSettingsClient to be created on Security Plugin Setup
(#170172)\n\n## Summary\r\n\r\nCloses
https://github.com/elastic/kibana/issues/155949\r\n\r\nThe Security
Plugin provides a UserProfileSettings client to the
core\r\nUserSettingsService on Setup.\r\n\r\nWith the first
implementation of Dark Mode, the client was not set until\r\nthe
security UserSettingService was started, which violates
the\r\nsetup/start contract best practices.\r\n\r\nNow the client is
provided during security plugin setup and the
security\r\nUserSettingService will be set on start.\r\n\r\n##
Testing\r\n\r\nDark Mode should still work as it did before:\r\n\r\n-
Login as elastic\r\n- Navigate to User Profile by clicking the Avatar
menu > Edit Profile\r\n- Choose Dark Mode, Save, and Refresh\r\n\r\n-
Bonus test: double check Adv. Settings Dark Mode vs User Profiles
Dark\r\nMode","sha":"195ef3dc2bd08fad54a0ea8278abffb4f39d39bc"}},"sourceBranch":"main","suggestedTargetBranches":["8.11"],"targetPullRequestStates":[{"branch":"main","label":"v8.12.0","labelRegex":"^v8.12.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/170172","number":170172,"mergeCommit":{"message":"Allow
UserProfileSettingsClient to be created on Security Plugin Setup
(#170172)\n\n## Summary\r\n\r\nCloses
https://github.com/elastic/kibana/issues/155949\r\n\r\nThe Security
Plugin provides a UserProfileSettings client to the
core\r\nUserSettingsService on Setup.\r\n\r\nWith the first
implementation of Dark Mode, the client was not set until\r\nthe
security UserSettingService was started, which violates
the\r\nsetup/start contract best practices.\r\n\r\nNow the client is
provided during security plugin setup and the
security\r\nUserSettingService will be set on start.\r\n\r\n##
Testing\r\n\r\nDark Mode should still work as it did before:\r\n\r\n-
Login as elastic\r\n- Navigate to User Profile by clicking the Avatar
menu > Edit Profile\r\n- Choose Dark Mode, Save, and Refresh\r\n\r\n-
Bonus test: double check Adv. Settings Dark Mode vs User Profiles
Dark\r\nMode","sha":"195ef3dc2bd08fad54a0ea8278abffb4f39d39bc"}},{"branch":"8.11","label":"v8.11.1","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Kurt <kc13greiner@users.noreply.github.com>
delanni pushed a commit to delanni/kibana that referenced this pull request Nov 6, 2023
elastic#170172)

## Summary

Closes elastic#155949

The Security Plugin provides a UserProfileSettings client to the core
UserSettingsService on Setup.

With the first implementation of Dark Mode, the client was not set until
the security UserSettingService was started, which violates the
setup/start contract best practices.

Now the client is provided during security plugin setup and the security
UserSettingService will be set on start.

## Testing

Dark Mode should still work as it did before:

- Login as elastic
- Navigate to User Profile by clicking the Avatar menu > Edit Profile
- Choose Dark Mode, Save, and Refresh

- Bonus test: double check Adv. Settings Dark Mode vs User Profiles Dark
Mode
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Feature:Security/User Profile release_note:skip Skip the PR/issue when compiling release notes Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! v8.11.0 v8.11.1 v8.12.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UserSettingsService setup() method should not be called after start() has been initiated
5 participants