Skip to content

Commit

Permalink
[8.11] Allow UserProfileSettingsClient to be created on Security Plug…
Browse files Browse the repository at this point in the history
…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":"[email protected]"},"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 <[email protected]>
  • Loading branch information
kibanamachine and kc13greiner authored Nov 2, 2023
1 parent 1ea2b44 commit 2bfda0e
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 30 deletions.
31 changes: 12 additions & 19 deletions x-pack/plugins/security/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@ export class SecurityPlugin

private readonly userSettingService: UserSettingService;
private userSettingServiceStart?: UserSettingServiceStart;
private userProfileSettingsClient: UserProfileSettingsClient;
private readonly getUserProfileService = () => {
if (!this.userProfileStart) {
throw new Error(`userProfileStart is not registered!`);
Expand Down Expand Up @@ -233,6 +234,10 @@ export class SecurityPlugin
);

this.analyticsService = new AnalyticsService(this.initializerContext.logger.get('analytics'));

this.userProfileSettingsClient = new UserProfileSettingsClient(
this.initializerContext.logger.get('user-settings-client')
);
}

public setup(
Expand All @@ -255,25 +260,12 @@ export class SecurityPlugin
const kibanaIndexName = this.getKibanaIndexName();

// A subset of `start` services we need during `setup`.
const startServicesPromise = core
.getStartServices()
.then(([coreServices, depsServices, startServices]) => ({
elasticsearch: coreServices.elasticsearch,
features: depsServices.features,
userProfiles: startServices.userProfiles,
}));

/**
* Once the UserProfileServiceStart is available, use it to start the SecurityPlugin > UserSettingService.
*
* Then the UserProfileSettingsClient is created with the SecurityPlugin > UserSettingServiceStart and set on
* the Core > UserSettingsServiceSetup
*/
startServicesPromise.then(({ userProfiles }) => {
this.userSettingServiceStart = this.userSettingService.start(userProfiles);
const client = new UserProfileSettingsClient(this.userSettingServiceStart);
core.userSettings.setUserProfileSettings(client);
});
const startServicesPromise = core.getStartServices().then(([coreServices, depsServices]) => ({
elasticsearch: coreServices.elasticsearch,
features: depsServices.features,
}));

core.userSettings.setUserProfileSettings(this.userProfileSettingsClient);

const { license } = this.securityLicenseService.setup({
license$: licensing.license$,
Expand Down Expand Up @@ -408,6 +400,7 @@ export class SecurityPlugin

this.userProfileStart = this.userProfileService.start({ clusterClient, session });
this.userSettingServiceStart = this.userSettingService.start(this.userProfileStart);
this.userProfileSettingsClient.setUserSettingsServiceStart(this.userSettingServiceStart);

// In serverless, we want to redirect users to the list of projects instead of standard "Logged Out" page.
const customLogoutURL =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,28 +5,41 @@
* 2.0.
*/

import { loggingSystemMock } from '@kbn/core/server/mocks';
import type { httpServerMock } from '@kbn/core-http-server-mocks';
import type { Logger } from '@kbn/logging';

import { UserProfileSettingsClient } from './user_profile_settings_client';
import type { UserSettingServiceStart } from './user_setting_service';

describe('UserProfileSettingsClient', () => {
let mockRequest: ReturnType<typeof httpServerMock.createKibanaRequest>;
let client: UserProfileSettingsClient;
let logger: Logger;
let userSettingServiceStart: jest.Mocked<UserSettingServiceStart>;

beforeEach(() => {
const userSettingsServiceStart = {
userSettingServiceStart = {
getCurrentUserProfileSettings: jest.fn(),
} as jest.Mocked<UserSettingServiceStart>;
};

userSettingsServiceStart.getCurrentUserProfileSettings.mockResolvedValue({ darkMode: 'dark' });
userSettingServiceStart.getCurrentUserProfileSettings.mockResolvedValue({ darkMode: 'dark' });

client = new UserProfileSettingsClient(userSettingsServiceStart);
logger = loggingSystemMock.createLogger();
client = new UserProfileSettingsClient(logger);
});

describe('#get', () => {
it('should return user settings', async () => {
it('should return empty before UserSettingServiceStart is set', async () => {
const userSettings = await client.get(mockRequest);
expect(userSettings).toEqual({});
expect(logger.debug).toHaveBeenCalledWith('UserSettingsServiceStart has not been set yet');
});

it('should return user settings after UserSettingServiceStart is set', async () => {
client.setUserSettingsServiceStart(userSettingServiceStart);
const userSettings = await client.get(mockRequest);

expect(userSettings).toEqual({ darkMode: 'dark' });
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,28 +4,42 @@
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import type { Logger } from '@kbn/core/server';
import type { KibanaRequest } from '@kbn/core-http-server';
import type { UserProfileSettingsClientContract } from '@kbn/core-user-settings-server';

import type { UserSettingServiceStart } from './user_setting_service';

/**
* A wrapper client around {@link UserSettingServiceStart} that exposes a method to get the current user's profile
*/
export class UserProfileSettingsClient implements UserProfileSettingsClientContract {
private userSettingsServiceStart: UserSettingServiceStart;
private userSettingServiceStart: UserSettingServiceStart | undefined;
private logger: Logger;

constructor(userSettingsServiceStart: UserSettingServiceStart) {
this.userSettingsServiceStart = userSettingsServiceStart;
constructor(logger: Logger) {
this.logger = logger;
}

/**
* Returns the current user's user profile settings
*
* @param request the KibanaRequest that is required to get the current user and their settings
* @return the User Settings values retrieved from the UserSettingsServiceStart, if it has been set, otherwise,
* default to an empty Record
*/
async get(request: KibanaRequest): Promise<Record<string, string>> {
return await this.userSettingsServiceStart.getCurrentUserProfileSettings(request);
let result: Record<string, string> = {} as Record<string, string>;

if (this.userSettingServiceStart) {
result = await this.userSettingServiceStart.getCurrentUserProfileSettings(request);
} else {
this.logger.debug('UserSettingsServiceStart has not been set yet');
}

return result;
}

setUserSettingsServiceStart(userSettingServiceStart: UserSettingServiceStart) {
this.userSettingServiceStart = userSettingServiceStart;
}
}

0 comments on commit 2bfda0e

Please sign in to comment.