Skip to content
Merged
Show file tree
Hide file tree
Changes from 38 commits
Commits
Show all changes
56 commits
Select commit Hold shift + click to select a range
8923f8d
[Advanced settings] Add settings allowlist
ElenaStoeva Aug 22, 2023
eb47fb6
Move allowlist APIs to server
ElenaStoeva Aug 23, 2023
1b6cbb6
[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…
kibanamachine Aug 23, 2023
f8dc1bd
Don't allow updating a setting that is not allowlisted
ElenaStoeva Aug 24, 2023
90abe3a
Leverage readonly param and extract settings list into a package
ElenaStoeva Aug 24, 2023
83be210
[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…
kibanamachine Aug 24, 2023
c607959
[CI] Auto-commit changed files from 'node scripts/generate codeowners'
kibanamachine Aug 24, 2023
cd0e954
Expose setupProjectSettings api from serverless plugin
ElenaStoeva Aug 25, 2023
4b2ed8d
Change readonly param and improve settings package readme
ElenaStoeva Aug 25, 2023
96b6d57
Extract search and security settings into packages
ElenaStoeva Aug 29, 2023
4b782a5
Fix types errors
ElenaStoeva Aug 29, 2023
d4c8131
[CI] Auto-commit changed files from 'node scripts/generate codeowners'
kibanamachine Aug 29, 2023
9abdaf9
Merge branch 'main' into advanced_settings/allowlist_for_serverless
ElenaStoeva Aug 30, 2023
e660157
Fix event log tests
ElenaStoeva Aug 30, 2023
eda865e
Merge branch 'main' into advanced_settings/allowlist_for_serverless
kibanamachine Aug 30, 2023
48cb651
Fix alerting test mocks
ElenaStoeva Aug 31, 2023
529f143
Merge branch 'main' into advanced_settings/allowlist_for_serverless
ElenaStoeva Aug 31, 2023
b7a8350
Address CR comments
ElenaStoeva Aug 31, 2023
77481ca
Add setReadonlyMode method and enforcements/validations
ElenaStoeva Sep 1, 2023
9e896ca
[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…
kibanamachine Sep 1, 2023
8da18e3
Add tests for setAllowlist and validateAllowlist
ElenaStoeva Sep 1, 2023
077c7cc
Disable mappings check
ElenaStoeva Sep 4, 2023
69d763c
Merge branch 'main' into advanced_settings/allowlist_for_serverless
ElenaStoeva Sep 4, 2023
905b342
Extract setting ids into a package
ElenaStoeva Sep 4, 2023
d8f4767
[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…
kibanamachine Sep 4, 2023
a2e8920
[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…
kibanamachine Sep 4, 2023
c5c2535
[CI] Auto-commit changed files from 'node scripts/generate codeowners'
kibanamachine Sep 4, 2023
7dfd41a
Fix logic, refactor changes
ElenaStoeva Sep 5, 2023
4e6a8dc
Merge branch 'main' into advanced_settings/allowlist_for_serverless
ElenaStoeva Sep 5, 2023
4db7ca3
Fix actions plugin tests
ElenaStoeva Sep 5, 2023
c3032bf
[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…
kibanamachine Sep 5, 2023
ad880c7
Skip blocking test
ElenaStoeva Sep 5, 2023
67c33ed
Merge branch 'main' into advanced_settings/allowlist_for_serverless
ElenaStoeva Sep 5, 2023
d0665e6
Add tests for applyAllowlist
ElenaStoeva Sep 5, 2023
5da41f8
[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…
kibanamachine Sep 5, 2023
80fdbe8
Add more documentation
ElenaStoeva Sep 5, 2023
f99a6d9
Add serverless plugin test
ElenaStoeva Sep 5, 2023
2b43fe5
Merge branch 'main' into advanced_settings/allowlist_for_serverless
mistic Sep 5, 2023
cfb1646
Fix alerting and event_log tests
ElenaStoeva Sep 5, 2023
a35f47f
Remove unnecessary comments
ElenaStoeva Sep 6, 2023
baf3e79
Fix exporting/importing errors
ElenaStoeva Sep 6, 2023
2f8d2b8
Remove old security setting id
ElenaStoeva Sep 7, 2023
09a4cba
Merge branch 'main' into advanced_settings/allowlist_for_serverless
ElenaStoeva Sep 7, 2023
2f0bf0f
Add functional tests
ElenaStoeva Sep 8, 2023
9e3d630
Merge branch 'advanced_settings/allowlist_for_serverless' of https://…
ElenaStoeva Sep 8, 2023
0e1561a
[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…
kibanamachine Sep 8, 2023
b6e2bb1
Merge branch 'main' into advanced_settings/allowlist_for_serverless
ElenaStoeva Sep 8, 2023
7601a49
Unskip failed tests and disable multi-call check for setAllowlist
ElenaStoeva Sep 8, 2023
8c13f80
[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…
kibanamachine Sep 8, 2023
7eee9a1
Skip test for setAllowlist check
ElenaStoeva Sep 8, 2023
4e4ee30
Fix test descriptions
ElenaStoeva Sep 8, 2023
6bfe3da
Validate allowlist only in development mode
ElenaStoeva Sep 11, 2023
ba8ffec
Merge branch 'main' into advanced_settings/allowlist_for_serverless
ElenaStoeva Sep 11, 2023
efdc13a
[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…
kibanamachine Sep 11, 2023
a272d86
Refactor isDev check
ElenaStoeva Sep 11, 2023
3dfbdb9
Disable Advanced settings app in serverless and skip functional tests
ElenaStoeva Sep 11, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .buildkite/scripts/steps/checks.sh
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,5 @@ export DISABLE_BOOTSTRAP_VALIDATION=false
.buildkite/scripts/steps/checks/test_projects.sh
.buildkite/scripts/steps/checks/test_hardening.sh
.buildkite/scripts/steps/checks/ftr_configs.sh
.buildkite/scripts/steps/checks/saved_objects_compat_changes.sh
# Skipped as blocking https://github.com/elastic/kibana/pull/164471
#.buildkite/scripts/steps/checks/saved_objects_compat_changes.sh
Comment thread
ElenaStoeva marked this conversation as resolved.
Outdated
5 changes: 5 additions & 0 deletions .github/CODEOWNERS
Original file line number Diff line number Diff line change
Expand Up @@ -480,6 +480,7 @@ packages/kbn-managed-vscode-config @elastic/kibana-operations
packages/kbn-managed-vscode-config-cli @elastic/kibana-operations
packages/kbn-management/cards_navigation @elastic/platform-deployment-management
src/plugins/management @elastic/platform-deployment-management
packages/kbn-management/settings/setting_ids @elastic/appex-sharedux @elastic/platform-deployment-management
packages/kbn-management/settings/section_registry @elastic/appex-sharedux @elastic/platform-deployment-management
packages/kbn-management/storybook/config @elastic/platform-deployment-management
test/plugin_functional/plugins/management_test_plugin @elastic/kibana-app-services
Expand Down Expand Up @@ -632,9 +633,13 @@ packages/kbn-securitysolution-utils @elastic/security-detection-engine
packages/kbn-server-http-tools @elastic/kibana-core
packages/kbn-server-route-repository @elastic/apm-ui
x-pack/plugins/serverless @elastic/appex-sharedux
packages/serverless/settings/common @elastic/appex-sharedux @elastic/platform-deployment-management
x-pack/plugins/serverless_observability @elastic/appex-sharedux @elastic/apm-ui
packages/serverless/settings/observability_project @elastic/appex-sharedux @elastic/apm-ui @elastic/platform-deployment-management
packages/serverless/project_switcher @elastic/appex-sharedux
x-pack/plugins/serverless_search @elastic/enterprise-search-frontend
packages/serverless/settings/search_project @elastic/enterprise-search-frontend @elastic/platform-deployment-management
packages/serverless/settings/security_project @elastic/security-solution @elastic/platform-deployment-management
packages/serverless/storybook/config @elastic/appex-sharedux
packages/serverless/types @elastic/appex-sharedux
test/plugin_functional/plugins/session_notifications @elastic/kibana-core
Expand Down
2 changes: 1 addition & 1 deletion config/serverless.yml
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ management.deeplinks.navLinkStatus: visible
xpack.canvas.enabled: false
xpack.cloud_integrations.data_migration.enabled: false
data.search.sessions.enabled: false
advanced_settings.enabled: false
advanced_settings.enabled: true
Comment thread
ElenaStoeva marked this conversation as resolved.
Outdated

# Disable the browser-side functionality that depends on SecurityCheckupGetStateRoutes
xpack.security.showInsecureClusterWarning: false
Expand Down
5 changes: 5 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,7 @@
"@kbn/logstash-plugin": "link:x-pack/plugins/logstash",
"@kbn/management-cards-navigation": "link:packages/kbn-management/cards_navigation",
"@kbn/management-plugin": "link:src/plugins/management",
"@kbn/management-settings-ids": "link:packages/kbn-management/settings/setting_ids",
"@kbn/management-settings-section-registry": "link:packages/kbn-management/settings/section_registry",
"@kbn/management-test-plugin": "link:test/plugin_functional/plugins/management_test_plugin",
"@kbn/mapbox-gl": "link:packages/kbn-mapbox-gl",
Expand Down Expand Up @@ -637,9 +638,13 @@
"@kbn/server-http-tools": "link:packages/kbn-server-http-tools",
"@kbn/server-route-repository": "link:packages/kbn-server-route-repository",
"@kbn/serverless": "link:x-pack/plugins/serverless",
"@kbn/serverless-common-settings": "link:packages/serverless/settings/common",
"@kbn/serverless-observability": "link:x-pack/plugins/serverless_observability",
"@kbn/serverless-observability-settings": "link:packages/serverless/settings/observability_project",
"@kbn/serverless-project-switcher": "link:packages/serverless/project_switcher",
"@kbn/serverless-search": "link:x-pack/plugins/serverless_search",
"@kbn/serverless-search-settings": "link:packages/serverless/settings/search_project",
"@kbn/serverless-security-settings": "link:packages/serverless/settings/security_project",
"@kbn/serverless-types": "link:packages/serverless/types",
"@kbn/session-notifications-plugin": "link:test/plugin_functional/plugins/session_notifications",
"@kbn/session-view-plugin": "link:x-pack/plugins/session_view",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ export function createCoreSetupMock({
const uiSettingsMock = {
register: uiSettingsServiceMock.createSetupContract().register,
registerGlobal: uiSettingsServiceMock.createSetupContract().registerGlobal,
setAllowlist: uiSettingsServiceMock.createSetupContract().setAllowlist,
};

const mock: CoreSetupMockType = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,7 @@ export function createPluginSetupContext<TPlugin, TPluginDependencies>(
uiSettings: {
register: deps.uiSettings.register,
registerGlobal: deps.uiSettings.registerGlobal,
setAllowlist: deps.uiSettings.setAllowlist,
},
userSettings: {
setUserProfileSettings: deps.userSettings.setUserProfileSettings,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,10 @@ You can use \`IUiSettingsClient.get("${key}", defaultValue)\`, which will just r
return this.isDeclared(key) && Boolean(this.cache[key].isOverridden);
}

isStrictReadonly(key: string) {
return this.isDeclared(key) && Boolean(this.cache[key].readonlyMode === 'strict');
}

getUpdate$() {
return this.update$.asObservable();
}
Expand All @@ -130,6 +134,9 @@ You can use \`IUiSettingsClient.get("${key}", defaultValue)\`, which will just r
`Unable to update "${key}" because its value is overridden by the Kibana server`
);
}
if (this.isStrictReadonly(key)) {
throw new Error(`Unable to update "${key}" because this setting is not in the allowlist.`);
}
}

protected abstract update(key: string, newVal: any): Promise<boolean>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,14 @@ export type UiSettingsType =
| 'image'
| 'color';

/**
* Type for the readonly mode of the readonly settings.
* 'strict' indicates that the value cannot be changed through API and is not displayed in the UI
* 'ui' indicates that the value is just not displayed in the UI
* @public
* */
export type ReadonlyModeType = 'strict' | 'ui';
Comment thread
ElenaStoeva marked this conversation as resolved.

/**
* UiSettings deprecation field options.
* @public
Expand Down Expand Up @@ -57,6 +65,8 @@ export interface UiSettingsParams<T = unknown> {
requiresPageReload?: boolean;
/** a flag indicating that value cannot be changed */
readonly?: boolean;
/** a flag indicating the level of restriction of the readonly settings {@link ReadonlyModeType} */
readonlyMode?: ReadonlyModeType;
/**
* a flag indicating that value might contain user sensitive data.
* used by telemetry to mask the value of the setting when sent.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,17 @@ describe('uiSettings', () => {
expect(() => setup.registerGlobal(defaults)).not.toThrow();
});
});

describe('#setAllowlist', () => {
it('throws if setAllowlist is called twice', async () => {
const { setAllowlist } = await service.setup(setupDeps);
setAllowlist(['mySetting']);

expect(() => setAllowlist(['newSetting'])).toThrowErrorMatchingInlineSnapshot(
`"The uiSettings allowlist has already been set up. Instead of calling setAllowlist(), add your settings to packages/serverless/settings"`
);
});
});
});

describe('#start', () => {
Expand Down Expand Up @@ -213,6 +224,59 @@ describe('uiSettings', () => {

await customizedService.start();
});

it('throws when the allowlist contains unregistered settings', async () => {
const { setAllowlist } = await service.setup(setupDeps);
setAllowlist(['mySetting']);

await expect(service.start()).rejects.toMatchInlineSnapshot(
`[Error: The uiSetting with key [mySetting] is in the allowlist but is not registered. Make sure to remove it from the allowlist in /packages/serverless/settings]`
);
});
});

describe('#applyAllowlist', () => {
const settingId = 'mySetting';
const testSetting = {
name: 'My setting',
value: 10,
readonly: true,
schema: schema.number(),
};

it('allowlisted readonly settings have "ui" readonly mode', async () => {
const { register, setAllowlist } = await service.setup(setupDeps);
register({ [settingId]: testSetting });
setAllowlist([settingId]);

const expectedSetting = {
...testSetting,
readonlyMode: 'ui',
};

const start = await service.start();
start.asScopedToClient(savedObjectsClient);
expect(MockUiSettingsClientConstructor.mock.calls[0][0].defaults).toEqual({
[settingId]: expectedSetting,
});
});

it('non-allowlisted settings have "strict" readonly mode', async () => {
const { register, setAllowlist } = await service.setup(setupDeps);
register({ [settingId]: testSetting });
setAllowlist([]);

const expectedSetting = {
...testSetting,
readonlyMode: 'strict',
};

const start = await service.start();
start.asScopedToClient(savedObjectsClient);
expect(MockUiSettingsClientConstructor.mock.calls[0][0].defaults).toEqual({
[settingId]: expectedSetting,
});
});
});

describe('#asScopedToClient', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import type { InternalHttpServiceSetup } from '@kbn/core-http-server-internal';
import type { SavedObjectsClientContract } from '@kbn/core-saved-objects-api-server';
import type { InternalSavedObjectsServiceSetup } from '@kbn/core-saved-objects-server-internal';
import type { UiSettingsParams, UiSettingsScope } from '@kbn/core-ui-settings-common';
import { ReadonlyModeType } from '@kbn/core-ui-settings-common/src/ui_settings';
import { UiSettingsConfigType, uiSettingsConfig as uiConfigDefinition } from './ui_settings_config';
import { UiSettingsClient, UiSettingsClientFactory, UiSettingsGlobalClient } from './clients';
import type {
Expand Down Expand Up @@ -50,6 +51,7 @@ export class UiSettingsService
private readonly uiSettingsDefaults = new Map<string, UiSettingsParams>();
private readonly uiSettingsGlobalDefaults = new Map<string, UiSettingsParams>();
private overrides: Record<string, any> = {};
private allowlist: Set<string> | null = null;

constructor(private readonly coreContext: CoreContext) {
this.log = coreContext.logger.get('ui-settings-service');
Expand Down Expand Up @@ -94,10 +96,16 @@ export class UiSettingsService
return {
register: this.register,
registerGlobal: this.registerGlobal,
setAllowlist: this.setAllowlist,
Comment thread
ElenaStoeva marked this conversation as resolved.
};
}

public async start(): Promise<InternalUiSettingsServiceStart> {
if (this.allowlist) {
this.validateAllowlist();
this.applyAllowlist(this.uiSettingsDefaults, false);
this.applyAllowlist(this.uiSettingsGlobalDefaults, true);
}
this.validatesDefinitions();
this.validatesOverrides();

Expand Down Expand Up @@ -148,6 +156,54 @@ export class UiSettingsService
});
};

private setAllowlist = (keys: string[]) => {
if (this.allowlist) {
throw new Error(
`The uiSettings allowlist has already been set up. Instead of calling setAllowlist(), add your settings to packages/serverless/settings`
);
}
this.allowlist = new Set(keys);
};

private validateAllowlist() {
this.allowlist?.forEach((key) => {
if (!this.uiSettingsDefaults.has(key) && !this.uiSettingsGlobalDefaults.has(key)) {
throw new Error(
`The uiSetting with key [${key}] is in the allowlist but is not registered. Make sure to remove it from the allowlist in /packages/serverless/settings`
Comment thread
ElenaStoeva marked this conversation as resolved.
);
}
});
}

private setReadonlyMode(key: string, mode: ReadonlyModeType, isGlobal: boolean) {
if (isGlobal) {
const definition = this.uiSettingsGlobalDefaults.get(key);
if (definition) {
this.uiSettingsGlobalDefaults.set(key, { ...definition, readonlyMode: mode });
}
} else {
const definition = this.uiSettingsDefaults.get(key);
if (definition) {
this.uiSettingsDefaults.set(key, { ...definition, readonlyMode: mode });
}
}
}

private applyAllowlist(settingsDefaults: Map<string, UiSettingsParams>, isGlobal: boolean) {
for (const [key, definition] of settingsDefaults) {
// Settings in the allowlist that are already read-only should have 'ui' readonly mode
if (this.allowlist?.has(key) && definition.readonly === true) {
this.setReadonlyMode(key, 'ui', isGlobal);
}

// Setting that are not in the allowlist should have 'strict' readonly mode
if (!this.allowlist?.has(key)) {
definition.readonly = true;
this.setReadonlyMode(key, 'strict', isGlobal);
}
}
}

private validatesDefinitions() {
for (const [key, definition] of this.uiSettingsDefaults) {
if (!definition.schema) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ const createSetupMock = () => {
const mocked: jest.Mocked<InternalUiSettingsServiceSetup> = {
register: jest.fn(),
registerGlobal: jest.fn(),
setAllowlist: jest.fn(),
};

return mocked;
Expand Down
13 changes: 13 additions & 0 deletions packages/core/ui-settings/core-ui-settings-server/src/contracts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,19 @@ export interface UiSettingsServiceSetup {
* ```
*/
registerGlobal(settings: Record<string, UiSettingsParams>): void;

/**
* Sets an allowlist of setting keys.
* @param keys
*
* @example
* ```ts
* setup(core: CoreSetup){
* core.uiSettings.setAllowlist(['csv:quoteValues', 'dateFormat:dow']);
* }
* ```
*/
setAllowlist(keys: string[]): void;
}

/** @public */
Expand Down
39 changes: 39 additions & 0 deletions packages/kbn-management/settings/setting_ids/README.mdx
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
---
id: kbn-management/settings/ids
slug: /kbn-management/settings/setting_ids/
title: Setting ID's
description: ID's of all advanced settings.
tags: ['management', 'settings']
date: 2023-09-04
---

This package contains the id's of all advanced settings.

When registering an advanced setting, add its id to this package and import it for use in the setting definition:

**packages/kbn-management/settings/setting_ids/index.ts**

```ts
export const MY_SETTING_ID = 'mySetting';
```

**src/plugins/my_plugin/server/plugin.ts**

```ts
import { MY_SETTING_ID } from '@kbn/management-settings-ids';

export class MyPlugin implements Plugin<object, object> {
public setup(core: CoreSetup, dependencies: SetupDependencies) {
core.uiSettings.register({
[MY_SETTING_ID]: {
name: 'My setting',
value: 10,
schema: schema.number(),
},
...
});

return {};
}
}
```
Loading