From dd7531deb4c83a2711b0d4bc4871acea425eb8cf Mon Sep 17 00:00:00 2001 From: Mikhail Shustov Date: Mon, 16 Mar 2020 14:30:20 +0100 Subject: [PATCH] Add UiSettings validation & Kibana default route redirection (#59694) * add schema to ui settings params * add validation for defaults and overrides * validate in ui settings client * ui settings routes validation * clean up tests * use schema for defaultRoutes * move URL redirection to NP * fix spaces test * update docs * update kbn pm * fix karma test * fix tests * address comments * get rid of getDEfaultRoute * regen docs * fix tests * fix enter-spaces test * validate on relative url format * update i18n * fix enter-spoace test * move relative url validation to utils * add CoreApp containing application logic * extract public uiSettings params in a separate type * make schema required * update docs --- ...in-core-public.iuisettingsclient.getall.md | 2 +- ...na-plugin-core-public.iuisettingsclient.md | 2 +- .../core/public/kibana-plugin-core-public.md | 1 + ...ugin-core-public.publicuisettingsparams.md | 13 ++ ...ana-plugin-core-public.uisettingsparams.md | 5 +- ...gin-core-public.uisettingsparams.schema.md | 11 ++ ...ugin-core-public.uisettingsparams.value.md | 2 +- ...-server.iuisettingsclient.getregistered.md | 2 +- ...na-plugin-core-server.iuisettingsclient.md | 2 +- .../core/server/kibana-plugin-core-server.md | 1 + ...ugin-core-server.publicuisettingsparams.md | 13 ++ ...ana-plugin-core-server.uisettingsparams.md | 5 +- ...gin-core-server.uisettingsparams.schema.md | 11 ++ ...ugin-core-server.uisettingsparams.value.md | 2 +- .../src/errors/validation_error.ts | 3 + .../src/kbn_client/kbn_client_ui_settings.ts | 4 +- packages/kbn-pm/dist/index.js | 4 +- src/core/public/index.ts | 2 +- src/core/public/public.api.md | 16 +- src/core/public/types.ts | 1 + .../ui_settings_api.test.ts.snap | 8 +- src/core/public/ui_settings/types.ts | 6 +- .../ui_settings/ui_settings_api.test.ts | 2 +- .../public/ui_settings/ui_settings_api.ts | 10 +- .../public/ui_settings/ui_settings_client.ts | 8 +- src/core/server/core_app/core_app.ts | 52 ++++++ src/core/server/core_app/index.ts | 20 +++ .../default_route_provider_config.test.ts | 90 ++++++++++ src/core/server/index.ts | 1 + src/core/server/server.api.md | 11 +- src/core/server/server.ts | 15 +- src/core/server/ui_settings/index.ts | 1 + .../integration_tests/routes.test.ts | 66 ++++++++ src/core/server/ui_settings/routes/set.ts | 4 +- .../server/ui_settings/routes/set_many.ts | 4 +- src/core/server/ui_settings/types.ts | 5 +- .../ui_settings/ui_settings_client.test.ts | 159 ++++++++++++++++-- .../server/ui_settings/ui_settings_client.ts | 73 +++++--- .../ui_settings/ui_settings_service.test.ts | 42 +++++ .../server/ui_settings/ui_settings_service.ts | 20 +++ src/core/types/ui_settings.ts | 19 ++- src/core/utils/url.test.ts | 16 +- src/core/utils/url.ts | 16 ++ .../kibana/ui_setting_defaults.js | 24 ++- .../tests_bundle/tests_entry_template.js | 11 +- src/legacy/server/http/index.js | 11 -- .../default_route_provider.test.ts | 87 ---------- .../default_route_provider_config.test.ts | 54 ------ .../http/setup_default_route_provider.ts | 74 -------- src/legacy/server/kbn_server.d.ts | 1 - .../management_app/advanced_settings.test.tsx | 10 +- .../lib/to_editable_config.test.ts | 6 +- .../management_app/lib/to_editable_config.ts | 4 +- .../public/management_app/types.ts | 11 +- src/plugins/data/public/public.api.md | 2 +- src/test_utils/kbn_server.ts | 1 + test/api_integration/apis/index.js | 1 - .../ui_settings_plugin/server/plugin.ts | 3 +- .../spaces/server/routes/views/enter_space.ts | 8 +- .../translations/translations/ja-JP.json | 1 - .../translations/translations/zh-CN.json | 1 - .../functional/apps/spaces/enter_space.ts | 32 ++-- .../es_archives/spaces/enter_space/data.json | 4 +- 63 files changed, 724 insertions(+), 372 deletions(-) create mode 100644 docs/development/core/public/kibana-plugin-core-public.publicuisettingsparams.md create mode 100644 docs/development/core/public/kibana-plugin-core-public.uisettingsparams.schema.md create mode 100644 docs/development/core/server/kibana-plugin-core-server.publicuisettingsparams.md create mode 100644 docs/development/core/server/kibana-plugin-core-server.uisettingsparams.schema.md create mode 100644 src/core/server/core_app/core_app.ts create mode 100644 src/core/server/core_app/index.ts create mode 100644 src/core/server/core_app/integration_tests/default_route_provider_config.test.ts create mode 100644 src/core/server/ui_settings/integration_tests/routes.test.ts delete mode 100644 src/legacy/server/http/integration_tests/default_route_provider.test.ts delete mode 100644 src/legacy/server/http/integration_tests/default_route_provider_config.test.ts delete mode 100644 src/legacy/server/http/setup_default_route_provider.ts diff --git a/docs/development/core/public/kibana-plugin-core-public.iuisettingsclient.getall.md b/docs/development/core/public/kibana-plugin-core-public.iuisettingsclient.getall.md index 805ac57b2fb9a..004979977376e 100644 --- a/docs/development/core/public/kibana-plugin-core-public.iuisettingsclient.getall.md +++ b/docs/development/core/public/kibana-plugin-core-public.iuisettingsclient.getall.md @@ -9,5 +9,5 @@ Gets the metadata about all uiSettings, including the type, default value, and u Signature: ```typescript -getAll: () => Readonly>; +getAll: () => Readonly>; ``` diff --git a/docs/development/core/public/kibana-plugin-core-public.iuisettingsclient.md b/docs/development/core/public/kibana-plugin-core-public.iuisettingsclient.md index da566ed25cff5..87ef5784a6c6d 100644 --- a/docs/development/core/public/kibana-plugin-core-public.iuisettingsclient.md +++ b/docs/development/core/public/kibana-plugin-core-public.iuisettingsclient.md @@ -18,7 +18,7 @@ export interface IUiSettingsClient | --- | --- | --- | | [get](./kibana-plugin-core-public.iuisettingsclient.get.md) | <T = any>(key: string, defaultOverride?: T) => T | Gets the value for a specific uiSetting. If this setting has no user-defined value then the defaultOverride parameter is returned (and parsed if setting is of type "json" or "number). If the parameter is not defined and the key is not registered by any plugin then an error is thrown, otherwise reads the default value defined by a plugin. | | [get$](./kibana-plugin-core-public.iuisettingsclient.get_.md) | <T = any>(key: string, defaultOverride?: T) => Observable<T> | Gets an observable of the current value for a config key, and all updates to that config key in the future. Providing a defaultOverride argument behaves the same as it does in \#get() | -| [getAll](./kibana-plugin-core-public.iuisettingsclient.getall.md) | () => Readonly<Record<string, UiSettingsParams & UserProvidedValues>> | Gets the metadata about all uiSettings, including the type, default value, and user value for each key. | +| [getAll](./kibana-plugin-core-public.iuisettingsclient.getall.md) | () => Readonly<Record<string, PublicUiSettingsParams & UserProvidedValues>> | Gets the metadata about all uiSettings, including the type, default value, and user value for each key. | | [getSaved$](./kibana-plugin-core-public.iuisettingsclient.getsaved_.md) | <T = any>() => Observable<{
key: string;
newValue: T;
oldValue: T;
}> | Returns an Observable that notifies subscribers of each update to the uiSettings, including the key, newValue, and oldValue of the setting that changed. | | [getUpdate$](./kibana-plugin-core-public.iuisettingsclient.getupdate_.md) | <T = any>() => Observable<{
key: string;
newValue: T;
oldValue: T;
}> | Returns an Observable that notifies subscribers of each update to the uiSettings, including the key, newValue, and oldValue of the setting that changed. | | [getUpdateErrors$](./kibana-plugin-core-public.iuisettingsclient.getupdateerrors_.md) | () => Observable<Error> | Returns an Observable that notifies subscribers of each error while trying to update the settings, containing the actual Error class. | diff --git a/docs/development/core/public/kibana-plugin-core-public.md b/docs/development/core/public/kibana-plugin-core-public.md index bafc2eb3a4bc9..a9fbaa25ea150 100644 --- a/docs/development/core/public/kibana-plugin-core-public.md +++ b/docs/development/core/public/kibana-plugin-core-public.md @@ -147,6 +147,7 @@ The plugin integrates with the core system via lifecycle events: `setup` | [MountPoint](./kibana-plugin-core-public.mountpoint.md) | A function that should mount DOM content inside the provided container element and return a handler to unmount it. | | [PluginInitializer](./kibana-plugin-core-public.plugininitializer.md) | The plugin export at the root of a plugin's public directory should conform to this interface. | | [PluginOpaqueId](./kibana-plugin-core-public.pluginopaqueid.md) | | +| [PublicUiSettingsParams](./kibana-plugin-core-public.publicuisettingsparams.md) | A sub-set of [UiSettingsParams](./kibana-plugin-core-public.uisettingsparams.md) exposed to the client-side. | | [RecursiveReadonly](./kibana-plugin-core-public.recursivereadonly.md) | | | [SavedObjectAttribute](./kibana-plugin-core-public.savedobjectattribute.md) | Type definition for a Saved Object attribute value | | [SavedObjectAttributeSingle](./kibana-plugin-core-public.savedobjectattributesingle.md) | Don't use this type, it's simply a helper type for [SavedObjectAttribute](./kibana-plugin-core-public.savedobjectattribute.md) | diff --git a/docs/development/core/public/kibana-plugin-core-public.publicuisettingsparams.md b/docs/development/core/public/kibana-plugin-core-public.publicuisettingsparams.md new file mode 100644 index 0000000000000..678a69289ff23 --- /dev/null +++ b/docs/development/core/public/kibana-plugin-core-public.publicuisettingsparams.md @@ -0,0 +1,13 @@ + + +[Home](./index.md) > [kibana-plugin-core-public](./kibana-plugin-core-public.md) > [PublicUiSettingsParams](./kibana-plugin-core-public.publicuisettingsparams.md) + +## PublicUiSettingsParams type + +A sub-set of [UiSettingsParams](./kibana-plugin-core-public.uisettingsparams.md) exposed to the client-side. + +Signature: + +```typescript +export declare type PublicUiSettingsParams = Omit; +``` diff --git a/docs/development/core/public/kibana-plugin-core-public.uisettingsparams.md b/docs/development/core/public/kibana-plugin-core-public.uisettingsparams.md index 00f1c0f0deca5..e7facb4a109cd 100644 --- a/docs/development/core/public/kibana-plugin-core-public.uisettingsparams.md +++ b/docs/development/core/public/kibana-plugin-core-public.uisettingsparams.md @@ -9,7 +9,7 @@ UiSettings parameters defined by the plugins. Signature: ```typescript -export interface UiSettingsParams +export interface UiSettingsParams ``` ## Properties @@ -24,7 +24,8 @@ export interface UiSettingsParams | [options](./kibana-plugin-core-public.uisettingsparams.options.md) | string[] | array of permitted values for this setting | | [readonly](./kibana-plugin-core-public.uisettingsparams.readonly.md) | boolean | a flag indicating that value cannot be changed | | [requiresPageReload](./kibana-plugin-core-public.uisettingsparams.requirespagereload.md) | boolean | a flag indicating whether new value applying requires page reloading | +| [schema](./kibana-plugin-core-public.uisettingsparams.schema.md) | Type<T> | | | [type](./kibana-plugin-core-public.uisettingsparams.type.md) | UiSettingsType | defines a type of UI element [UiSettingsType](./kibana-plugin-core-public.uisettingstype.md) | | [validation](./kibana-plugin-core-public.uisettingsparams.validation.md) | ImageValidation | StringValidation | | -| [value](./kibana-plugin-core-public.uisettingsparams.value.md) | SavedObjectAttribute | default value to fall back to if a user doesn't provide any | +| [value](./kibana-plugin-core-public.uisettingsparams.value.md) | T | default value to fall back to if a user doesn't provide any | diff --git a/docs/development/core/public/kibana-plugin-core-public.uisettingsparams.schema.md b/docs/development/core/public/kibana-plugin-core-public.uisettingsparams.schema.md new file mode 100644 index 0000000000000..f90d5161f96a9 --- /dev/null +++ b/docs/development/core/public/kibana-plugin-core-public.uisettingsparams.schema.md @@ -0,0 +1,11 @@ + + +[Home](./index.md) > [kibana-plugin-core-public](./kibana-plugin-core-public.md) > [UiSettingsParams](./kibana-plugin-core-public.uisettingsparams.md) > [schema](./kibana-plugin-core-public.uisettingsparams.schema.md) + +## UiSettingsParams.schema property + +Signature: + +```typescript +schema: Type; +``` diff --git a/docs/development/core/public/kibana-plugin-core-public.uisettingsparams.value.md b/docs/development/core/public/kibana-plugin-core-public.uisettingsparams.value.md index 8775588290d70..2740f169eeecb 100644 --- a/docs/development/core/public/kibana-plugin-core-public.uisettingsparams.value.md +++ b/docs/development/core/public/kibana-plugin-core-public.uisettingsparams.value.md @@ -9,5 +9,5 @@ default value to fall back to if a user doesn't provide any Signature: ```typescript -value?: SavedObjectAttribute; +value?: T; ``` diff --git a/docs/development/core/server/kibana-plugin-core-server.iuisettingsclient.getregistered.md b/docs/development/core/server/kibana-plugin-core-server.iuisettingsclient.getregistered.md index 2ca6b4cbe1589..71a2bbf88472e 100644 --- a/docs/development/core/server/kibana-plugin-core-server.iuisettingsclient.getregistered.md +++ b/docs/development/core/server/kibana-plugin-core-server.iuisettingsclient.getregistered.md @@ -9,5 +9,5 @@ Returns registered uiSettings values [UiSettingsParams](./kibana-plugin-core-ser Signature: ```typescript -getRegistered: () => Readonly>; +getRegistered: () => Readonly>; ``` diff --git a/docs/development/core/server/kibana-plugin-core-server.iuisettingsclient.md b/docs/development/core/server/kibana-plugin-core-server.iuisettingsclient.md index 42fcc81419cbe..af99b5e5bb215 100644 --- a/docs/development/core/server/kibana-plugin-core-server.iuisettingsclient.md +++ b/docs/development/core/server/kibana-plugin-core-server.iuisettingsclient.md @@ -18,7 +18,7 @@ export interface IUiSettingsClient | --- | --- | --- | | [get](./kibana-plugin-core-server.iuisettingsclient.get.md) | <T = any>(key: string) => Promise<T> | Retrieves uiSettings values set by the user with fallbacks to default values if not specified. | | [getAll](./kibana-plugin-core-server.iuisettingsclient.getall.md) | <T = any>() => Promise<Record<string, T>> | Retrieves a set of all uiSettings values set by the user with fallbacks to default values if not specified. | -| [getRegistered](./kibana-plugin-core-server.iuisettingsclient.getregistered.md) | () => Readonly<Record<string, UiSettingsParams>> | Returns registered uiSettings values [UiSettingsParams](./kibana-plugin-core-server.uisettingsparams.md) | +| [getRegistered](./kibana-plugin-core-server.iuisettingsclient.getregistered.md) | () => Readonly<Record<string, PublicUiSettingsParams>> | Returns registered uiSettings values [UiSettingsParams](./kibana-plugin-core-server.uisettingsparams.md) | | [getUserProvided](./kibana-plugin-core-server.iuisettingsclient.getuserprovided.md) | <T = any>() => Promise<Record<string, UserProvidedValues<T>>> | Retrieves a set of all uiSettings values set by the user. | | [isOverridden](./kibana-plugin-core-server.iuisettingsclient.isoverridden.md) | (key: string) => boolean | Shows whether the uiSettings value set by the user. | | [remove](./kibana-plugin-core-server.iuisettingsclient.remove.md) | (key: string) => Promise<void> | Removes uiSettings value by key. | diff --git a/docs/development/core/server/kibana-plugin-core-server.md b/docs/development/core/server/kibana-plugin-core-server.md index 8a88329031e1f..54cf496b2d6af 100644 --- a/docs/development/core/server/kibana-plugin-core-server.md +++ b/docs/development/core/server/kibana-plugin-core-server.md @@ -232,6 +232,7 @@ The plugin integrates with the core system via lifecycle events: `setup` | [PluginInitializer](./kibana-plugin-core-server.plugininitializer.md) | The plugin export at the root of a plugin's server directory should conform to this interface. | | [PluginName](./kibana-plugin-core-server.pluginname.md) | Dedicated type for plugin name/id that is supposed to make Map/Set/Arrays that use it as a key or value more obvious. | | [PluginOpaqueId](./kibana-plugin-core-server.pluginopaqueid.md) | | +| [PublicUiSettingsParams](./kibana-plugin-core-server.publicuisettingsparams.md) | A sub-set of [UiSettingsParams](./kibana-plugin-core-server.uisettingsparams.md) exposed to the client-side. | | [RecursiveReadonly](./kibana-plugin-core-server.recursivereadonly.md) | | | [RedirectResponseOptions](./kibana-plugin-core-server.redirectresponseoptions.md) | HTTP response parameters for redirection response | | [RequestHandler](./kibana-plugin-core-server.requesthandler.md) | A function executed when route path matched requested resource path. Request handler is expected to return a result of one of [KibanaResponseFactory](./kibana-plugin-core-server.kibanaresponsefactory.md) functions. | diff --git a/docs/development/core/server/kibana-plugin-core-server.publicuisettingsparams.md b/docs/development/core/server/kibana-plugin-core-server.publicuisettingsparams.md new file mode 100644 index 0000000000000..4ccc91fbe1f74 --- /dev/null +++ b/docs/development/core/server/kibana-plugin-core-server.publicuisettingsparams.md @@ -0,0 +1,13 @@ + + +[Home](./index.md) > [kibana-plugin-core-server](./kibana-plugin-core-server.md) > [PublicUiSettingsParams](./kibana-plugin-core-server.publicuisettingsparams.md) + +## PublicUiSettingsParams type + +A sub-set of [UiSettingsParams](./kibana-plugin-core-server.uisettingsparams.md) exposed to the client-side. + +Signature: + +```typescript +export declare type PublicUiSettingsParams = Omit; +``` diff --git a/docs/development/core/server/kibana-plugin-core-server.uisettingsparams.md b/docs/development/core/server/kibana-plugin-core-server.uisettingsparams.md index fe6e5d956f3e2..f134decb5102b 100644 --- a/docs/development/core/server/kibana-plugin-core-server.uisettingsparams.md +++ b/docs/development/core/server/kibana-plugin-core-server.uisettingsparams.md @@ -9,7 +9,7 @@ UiSettings parameters defined by the plugins. Signature: ```typescript -export interface UiSettingsParams +export interface UiSettingsParams ``` ## Properties @@ -24,7 +24,8 @@ export interface UiSettingsParams | [options](./kibana-plugin-core-server.uisettingsparams.options.md) | string[] | array of permitted values for this setting | | [readonly](./kibana-plugin-core-server.uisettingsparams.readonly.md) | boolean | a flag indicating that value cannot be changed | | [requiresPageReload](./kibana-plugin-core-server.uisettingsparams.requirespagereload.md) | boolean | a flag indicating whether new value applying requires page reloading | +| [schema](./kibana-plugin-core-server.uisettingsparams.schema.md) | Type<T> | | | [type](./kibana-plugin-core-server.uisettingsparams.type.md) | UiSettingsType | defines a type of UI element [UiSettingsType](./kibana-plugin-core-server.uisettingstype.md) | | [validation](./kibana-plugin-core-server.uisettingsparams.validation.md) | ImageValidation | StringValidation | | -| [value](./kibana-plugin-core-server.uisettingsparams.value.md) | SavedObjectAttribute | default value to fall back to if a user doesn't provide any | +| [value](./kibana-plugin-core-server.uisettingsparams.value.md) | T | default value to fall back to if a user doesn't provide any | diff --git a/docs/development/core/server/kibana-plugin-core-server.uisettingsparams.schema.md b/docs/development/core/server/kibana-plugin-core-server.uisettingsparams.schema.md new file mode 100644 index 0000000000000..f181fbd309b7f --- /dev/null +++ b/docs/development/core/server/kibana-plugin-core-server.uisettingsparams.schema.md @@ -0,0 +1,11 @@ + + +[Home](./index.md) > [kibana-plugin-core-server](./kibana-plugin-core-server.md) > [UiSettingsParams](./kibana-plugin-core-server.uisettingsparams.md) > [schema](./kibana-plugin-core-server.uisettingsparams.schema.md) + +## UiSettingsParams.schema property + +Signature: + +```typescript +schema: Type; +``` diff --git a/docs/development/core/server/kibana-plugin-core-server.uisettingsparams.value.md b/docs/development/core/server/kibana-plugin-core-server.uisettingsparams.value.md index ca00cd0cd6396..78c8f0c8fcf8d 100644 --- a/docs/development/core/server/kibana-plugin-core-server.uisettingsparams.value.md +++ b/docs/development/core/server/kibana-plugin-core-server.uisettingsparams.value.md @@ -9,5 +9,5 @@ default value to fall back to if a user doesn't provide any Signature: ```typescript -value?: SavedObjectAttribute; +value?: T; ``` diff --git a/packages/kbn-config-schema/src/errors/validation_error.ts b/packages/kbn-config-schema/src/errors/validation_error.ts index d688d022da85c..2a4f887bc4349 100644 --- a/packages/kbn-config-schema/src/errors/validation_error.ts +++ b/packages/kbn-config-schema/src/errors/validation_error.ts @@ -44,5 +44,8 @@ export class ValidationError extends SchemaError { constructor(error: SchemaTypeError, namespace?: string) { super(ValidationError.extractMessage(error, namespace), error); + + // https://github.com/Microsoft/TypeScript/wiki/Breaking-Changes#extending-built-ins-like-error-array-and-map-may-no-longer-work + Object.setPrototypeOf(this, ValidationError.prototype); } } diff --git a/packages/kbn-dev-utils/src/kbn_client/kbn_client_ui_settings.ts b/packages/kbn-dev-utils/src/kbn_client/kbn_client_ui_settings.ts index ad01dea624c3c..dbfa87e70032b 100644 --- a/packages/kbn-dev-utils/src/kbn_client/kbn_client_ui_settings.ts +++ b/packages/kbn-dev-utils/src/kbn_client/kbn_client_ui_settings.ts @@ -67,7 +67,7 @@ export class KbnClientUiSettings { * Replace all uiSettings with the `doc` values, `doc` is merged * with some defaults */ - async replace(doc: UiSettingValues) { + async replace(doc: UiSettingValues, { retries = 5 }: { retries?: number } = {}) { this.log.debug('replacing kibana config doc: %j', doc); const changes: Record = { @@ -85,7 +85,7 @@ export class KbnClientUiSettings { method: 'POST', path: '/api/kibana/settings', body: { changes }, - retries: 5, + retries, }); } diff --git a/packages/kbn-pm/dist/index.js b/packages/kbn-pm/dist/index.js index fe0491870e4bd..338d489300526 100644 --- a/packages/kbn-pm/dist/index.js +++ b/packages/kbn-pm/dist/index.js @@ -43920,7 +43920,7 @@ class KbnClientUiSettings { * Replace all uiSettings with the `doc` values, `doc` is merged * with some defaults */ - async replace(doc) { + async replace(doc, { retries = 5 } = {}) { this.log.debug('replacing kibana config doc: %j', doc); const changes = { ...this.defaults, @@ -43935,7 +43935,7 @@ class KbnClientUiSettings { method: 'POST', path: '/api/kibana/settings', body: { changes }, - retries: 5, + retries, }); } /** diff --git a/src/core/public/index.ts b/src/core/public/index.ts index 483d4dbfdf7c5..0ff044878afa9 100644 --- a/src/core/public/index.ts +++ b/src/core/public/index.ts @@ -171,7 +171,7 @@ export { ErrorToastOptions, } from './notifications'; -export { MountPoint, UnmountCallback } from './types'; +export { MountPoint, UnmountCallback, PublicUiSettingsParams } from './types'; /** * Core services exposed to the `Plugin` setup lifecycle diff --git a/src/core/public/public.api.md b/src/core/public/public.api.md index 69668176a397e..46667230edc3b 100644 --- a/src/core/public/public.api.md +++ b/src/core/public/public.api.md @@ -16,10 +16,11 @@ import { Location } from 'history'; import { LocationDescriptorObject } from 'history'; import { MaybePromise } from '@kbn/utility-types'; import { Observable } from 'rxjs'; +import { PublicUiSettingsParams as PublicUiSettingsParams_2 } from 'src/core/server/types'; import React from 'react'; import * as Rx from 'rxjs'; import { ShallowPromise } from '@kbn/utility-types'; -import { UiSettingsParams as UiSettingsParams_2 } from 'src/core/server/types'; +import { Type } from '@kbn/config-schema'; import { UnregisterCallback } from 'history'; import { UserProvidedValues as UserProvidedValues_2 } from 'src/core/server/types'; @@ -784,7 +785,7 @@ export type IToasts = Pick(key: string, defaultOverride?: T) => Observable; get: (key: string, defaultOverride?: T) => T; - getAll: () => Readonly>; + getAll: () => Readonly>; getSaved$: () => Observable<{ key: string; newValue: T; @@ -933,6 +934,9 @@ export interface PluginInitializerContext // @public (undocumented) export type PluginOpaqueId = symbol; +// @public +export type PublicUiSettingsParams = Omit; + // Warning: (ae-forgotten-export) The symbol "RecursiveReadonlyArray" needs to be exported by the entry point index.d.ts // // @public (undocumented) @@ -1291,7 +1295,7 @@ export type ToastsSetup = IToasts; export type ToastsStart = IToasts; // @public -export interface UiSettingsParams { +export interface UiSettingsParams { category?: string[]; // Warning: (ae-forgotten-export) The symbol "DeprecationSettings" needs to be exported by the entry point index.d.ts deprecation?: DeprecationSettings; @@ -1301,16 +1305,18 @@ export interface UiSettingsParams { options?: string[]; readonly?: boolean; requiresPageReload?: boolean; + // (undocumented) + schema: Type; type?: UiSettingsType; // (undocumented) validation?: ImageValidation | StringValidation; - value?: SavedObjectAttribute; + value?: T; } // @public (undocumented) export interface UiSettingsState { // (undocumented) - [key: string]: UiSettingsParams_2 & UserProvidedValues_2; + [key: string]: PublicUiSettingsParams_2 & UserProvidedValues_2; } // @public diff --git a/src/core/public/types.ts b/src/core/public/types.ts index 267a9e9f7e014..26f1e46836378 100644 --- a/src/core/public/types.ts +++ b/src/core/public/types.ts @@ -19,6 +19,7 @@ export { UiSettingsParams, + PublicUiSettingsParams, UserProvidedValues, UiSettingsType, ImageValidation, diff --git a/src/core/public/ui_settings/__snapshots__/ui_settings_api.test.ts.snap b/src/core/public/ui_settings/__snapshots__/ui_settings_api.test.ts.snap index cd55c77526d52..b737c04a5f269 100644 --- a/src/core/public/ui_settings/__snapshots__/ui_settings_api.test.ts.snap +++ b/src/core/public/ui_settings/__snapshots__/ui_settings_api.test.ts.snap @@ -84,21 +84,21 @@ Array [ exports[`#batchSet rejects all promises for batched requests that fail: promise rejections 1`] = ` Array [ Object { - "error": [Error: Request failed with status code: 400], + "error": [Error: invalid], "isRejected": true, }, Object { - "error": [Error: Request failed with status code: 400], + "error": [Error: invalid], "isRejected": true, }, Object { - "error": [Error: Request failed with status code: 400], + "error": [Error: invalid], "isRejected": true, }, ] `; -exports[`#batchSet rejects on 301 1`] = `"Request failed with status code: 301"`; +exports[`#batchSet rejects on 301 1`] = `"Moved Permanently"`; exports[`#batchSet rejects on 404 response 1`] = `"Request failed with status code: 404"`; diff --git a/src/core/public/ui_settings/types.ts b/src/core/public/ui_settings/types.ts index 19fd91924f247..d92c033ae8c8c 100644 --- a/src/core/public/ui_settings/types.ts +++ b/src/core/public/ui_settings/types.ts @@ -18,11 +18,11 @@ */ import { Observable } from 'rxjs'; -import { UiSettingsParams, UserProvidedValues } from 'src/core/server/types'; +import { PublicUiSettingsParams, UserProvidedValues } from 'src/core/server/types'; /** @public */ export interface UiSettingsState { - [key: string]: UiSettingsParams & UserProvidedValues; + [key: string]: PublicUiSettingsParams & UserProvidedValues; } /** @@ -53,7 +53,7 @@ export interface IUiSettingsClient { * Gets the metadata about all uiSettings, including the type, default value, and user value * for each key. */ - getAll: () => Readonly>; + getAll: () => Readonly>; /** * Sets the value for a uiSetting. If the setting is not registered by any plugin diff --git a/src/core/public/ui_settings/ui_settings_api.test.ts b/src/core/public/ui_settings/ui_settings_api.test.ts index 1170c42cea704..9a462e0541347 100644 --- a/src/core/public/ui_settings/ui_settings_api.test.ts +++ b/src/core/public/ui_settings/ui_settings_api.test.ts @@ -148,7 +148,7 @@ describe('#batchSet', () => { '*', { status: 400, - body: 'invalid', + body: { message: 'invalid' }, }, { overwriteRoutes: false, diff --git a/src/core/public/ui_settings/ui_settings_api.ts b/src/core/public/ui_settings/ui_settings_api.ts index 33b43107acf1b..c5efced0a41e3 100644 --- a/src/core/public/ui_settings/ui_settings_api.ts +++ b/src/core/public/ui_settings/ui_settings_api.ts @@ -152,10 +152,14 @@ export class UiSettingsApi { }, }); } catch (err) { - if (err.response && err.response.status >= 300) { - throw new Error(`Request failed with status code: ${err.response.status}`); + if (err.response) { + if (err.response.status === 400) { + throw new Error(err.body.message); + } + if (err.response.status > 400) { + throw new Error(`Request failed with status code: ${err.response.status}`); + } } - throw err; } finally { this.loadingCount$.next(this.loadingCount$.getValue() - 1); diff --git a/src/core/public/ui_settings/ui_settings_client.ts b/src/core/public/ui_settings/ui_settings_client.ts index f0071ed08435c..f5596b1bc34fc 100644 --- a/src/core/public/ui_settings/ui_settings_client.ts +++ b/src/core/public/ui_settings/ui_settings_client.ts @@ -21,14 +21,14 @@ import { cloneDeep, defaultsDeep } from 'lodash'; import { Observable, Subject, concat, defer, of } from 'rxjs'; import { filter, map } from 'rxjs/operators'; -import { UiSettingsParams, UserProvidedValues } from 'src/core/server/types'; +import { UserProvidedValues, PublicUiSettingsParams } from 'src/core/server/types'; import { IUiSettingsClient, UiSettingsState } from './types'; import { UiSettingsApi } from './ui_settings_api'; interface UiSettingsClientParams { api: UiSettingsApi; - defaults: Record; + defaults: Record; initialSettings?: UiSettingsState; done$: Observable; } @@ -39,8 +39,8 @@ export class UiSettingsClient implements IUiSettingsClient { private readonly updateErrors$ = new Subject(); private readonly api: UiSettingsApi; - private readonly defaults: Record; - private cache: Record; + private readonly defaults: Record; + private cache: Record; constructor(params: UiSettingsClientParams) { this.api = params.api; diff --git a/src/core/server/core_app/core_app.ts b/src/core/server/core_app/core_app.ts new file mode 100644 index 0000000000000..2f8c85f47a76e --- /dev/null +++ b/src/core/server/core_app/core_app.ts @@ -0,0 +1,52 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { InternalCoreSetup } from '../internal_types'; +import { CoreContext } from '../core_context'; +import { Logger } from '../logging'; + +/** @internal */ +export class CoreApp { + private readonly logger: Logger; + constructor(core: CoreContext) { + this.logger = core.logger.get('core-app'); + } + setup(coreSetup: InternalCoreSetup) { + this.logger.debug('Setting up core app.'); + this.registerDefaultRoutes(coreSetup); + } + + private registerDefaultRoutes(coreSetup: InternalCoreSetup) { + const httpSetup = coreSetup.http; + const router = httpSetup.createRouter('/'); + router.get({ path: '/', validate: false }, async (context, req, res) => { + const defaultRoute = await context.core.uiSettings.client.get('defaultRoute'); + const basePath = httpSetup.basePath.get(req); + const url = `${basePath}${defaultRoute}`; + + return res.redirected({ + headers: { + location: url, + }, + }); + }); + router.get({ path: '/core', validate: false }, async (context, req, res) => + res.ok({ body: { version: '0.0.1' } }) + ); + } +} diff --git a/src/core/server/core_app/index.ts b/src/core/server/core_app/index.ts new file mode 100644 index 0000000000000..342ed43f1ff8a --- /dev/null +++ b/src/core/server/core_app/index.ts @@ -0,0 +1,20 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +export { CoreApp } from './core_app'; diff --git a/src/core/server/core_app/integration_tests/default_route_provider_config.test.ts b/src/core/server/core_app/integration_tests/default_route_provider_config.test.ts new file mode 100644 index 0000000000000..221e6fa42471c --- /dev/null +++ b/src/core/server/core_app/integration_tests/default_route_provider_config.test.ts @@ -0,0 +1,90 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import * as kbnTestServer from '../../../../test_utils/kbn_server'; +import { Root } from '../../root'; + +const { startES } = kbnTestServer.createTestServers({ + adjustTimeout: (t: number) => jest.setTimeout(t), +}); +let esServer: kbnTestServer.TestElasticsearchUtils; + +describe('default route provider', () => { + let root: Root; + + beforeAll(async () => { + esServer = await startES(); + root = kbnTestServer.createRootWithCorePlugins({ + server: { + basePath: '/hello', + }, + }); + + await root.setup(); + await root.start(); + }); + + afterAll(async () => { + await esServer.stop(); + await root.shutdown(); + }); + + it('redirects to the configured default route respecting basePath', async function() { + const { status, header } = await kbnTestServer.request.get(root, '/'); + + expect(status).toEqual(302); + expect(header).toMatchObject({ + location: '/hello/app/kibana', + }); + }); + + it('ignores invalid values', async function() { + const invalidRoutes = [ + 'http://not-your-kibana.com', + '///example.com', + '//example.com', + ' //example.com', + ]; + + for (const url of invalidRoutes) { + await kbnTestServer.request + .post(root, '/api/kibana/settings/defaultRoute') + .send({ value: url }) + .expect(400); + } + + const { status, header } = await kbnTestServer.request.get(root, '/'); + expect(status).toEqual(302); + expect(header).toMatchObject({ + location: '/hello/app/kibana', + }); + }); + + it('consumes valid values', async function() { + await kbnTestServer.request + .post(root, '/api/kibana/settings/defaultRoute') + .send({ value: '/valid' }) + .expect(200); + + const { status, header } = await kbnTestServer.request.get(root, '/'); + expect(status).toEqual(302); + expect(header).toMatchObject({ + location: '/hello/valid', + }); + }); +}); diff --git a/src/core/server/index.ts b/src/core/server/index.ts index 4a1ac8988e4e5..89fee92a7ef02 100644 --- a/src/core/server/index.ts +++ b/src/core/server/index.ts @@ -250,6 +250,7 @@ export { export { IUiSettingsClient, UiSettingsParams, + PublicUiSettingsParams, UiSettingsType, UiSettingsServiceSetup, UiSettingsServiceStart, diff --git a/src/core/server/server.api.md b/src/core/server/server.api.md index 9cd0c26ea2497..84fe081adaae6 100644 --- a/src/core/server/server.api.md +++ b/src/core/server/server.api.md @@ -998,7 +998,7 @@ export interface IScopedRenderingClient { export interface IUiSettingsClient { get: (key: string) => Promise; getAll: () => Promise>; - getRegistered: () => Readonly>; + getRegistered: () => Readonly>; getUserProvided: () => Promise>>; isOverridden: (key: string) => boolean; remove: (key: string) => Promise; @@ -1443,6 +1443,9 @@ export interface PluginsServiceStart { contracts: Map; } +// @public +export type PublicUiSettingsParams = Omit; + // Warning: (ae-forgotten-export) The symbol "RecursiveReadonlyArray" needs to be exported by the entry point index.d.ts // // @public (undocumented) @@ -2284,7 +2287,7 @@ export interface StringValidationRegexString { } // @public -export interface UiSettingsParams { +export interface UiSettingsParams { category?: string[]; deprecation?: DeprecationSettings; description?: string; @@ -2293,10 +2296,12 @@ export interface UiSettingsParams { options?: string[]; readonly?: boolean; requiresPageReload?: boolean; + // (undocumented) + schema: Type; type?: UiSettingsType; // (undocumented) validation?: ImageValidation | StringValidation; - value?: SavedObjectAttribute; + value?: T; } // @public (undocumented) diff --git a/src/core/server/server.ts b/src/core/server/server.ts index 2402504f717ca..09a1328f346d8 100644 --- a/src/core/server/server.ts +++ b/src/core/server/server.ts @@ -26,8 +26,9 @@ import { RawConfigurationProvider, coreDeprecationProvider, } from './config'; +import { CoreApp } from './core_app'; import { ElasticsearchService } from './elasticsearch'; -import { HttpService, InternalHttpServiceSetup } from './http'; +import { HttpService } from './http'; import { RenderingService, RenderingServiceSetup } from './rendering'; import { LegacyService, ensureValidConfiguration } from './legacy'; import { Logger, LoggerFactory } from './logging'; @@ -69,6 +70,7 @@ export class Server { private readonly uiSettings: UiSettingsService; private readonly uuid: UuidService; private readonly metrics: MetricsService; + private readonly coreApp: CoreApp; private coreStart?: InternalCoreStart; @@ -92,6 +94,7 @@ export class Server { this.capabilities = new CapabilitiesService(core); this.uuid = new UuidService(core); this.metrics = new MetricsService(core); + this.coreApp = new CoreApp(core); } public async setup() { @@ -122,8 +125,6 @@ export class Server { context: contextServiceSetup, }); - this.registerDefaultRoute(httpSetup); - const capabilitiesSetup = this.capabilities.setup({ http: httpSetup }); const elasticsearchServiceSetup = await this.elasticsearch.setup({ @@ -168,6 +169,7 @@ export class Server { }); this.registerCoreContext(coreSetup, renderingSetup); + this.coreApp.setup(coreSetup); return coreSetup; } @@ -216,13 +218,6 @@ export class Server { await this.metrics.stop(); } - private registerDefaultRoute(httpSetup: InternalHttpServiceSetup) { - const router = httpSetup.createRouter('/core'); - router.get({ path: '/', validate: false }, async (context, req, res) => - res.ok({ body: { version: '0.0.1' } }) - ); - } - private registerCoreContext(coreSetup: InternalCoreSetup, rendering: RenderingServiceSetup) { coreSetup.http.registerRouteHandlerContext( coreId, diff --git a/src/core/server/ui_settings/index.ts b/src/core/server/ui_settings/index.ts index ddb66df3ffcbe..b11f398d59fa8 100644 --- a/src/core/server/ui_settings/index.ts +++ b/src/core/server/ui_settings/index.ts @@ -27,6 +27,7 @@ export { UiSettingsServiceStart, IUiSettingsClient, UiSettingsParams, + PublicUiSettingsParams, InternalUiSettingsServiceSetup, InternalUiSettingsServiceStart, UiSettingsType, diff --git a/src/core/server/ui_settings/integration_tests/routes.test.ts b/src/core/server/ui_settings/integration_tests/routes.test.ts new file mode 100644 index 0000000000000..c1261bc7c1350 --- /dev/null +++ b/src/core/server/ui_settings/integration_tests/routes.test.ts @@ -0,0 +1,66 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { schema } from '@kbn/config-schema'; +import * as kbnTestServer from '../../../../test_utils/kbn_server'; + +describe('ui settings service', () => { + describe('routes', () => { + let root: ReturnType; + beforeAll(async () => { + root = kbnTestServer.createRoot(); + + const { uiSettings } = await root.setup(); + uiSettings.register({ + custom: { + value: '42', + schema: schema.string(), + }, + }); + + await root.start(); + }, 30000); + afterAll(async () => await root.shutdown()); + + describe('set', () => { + it('validates value', async () => { + const response = await kbnTestServer.request + .post(root, '/api/kibana/settings/custom') + .send({ value: 100 }) + .expect(400); + + expect(response.body.message).toBe( + '[validation [custom]]: expected value of type [string] but got [number]' + ); + }); + }); + describe('set many', () => { + it('validates value', async () => { + const response = await kbnTestServer.request + .post(root, '/api/kibana/settings') + .send({ changes: { custom: 100, foo: 'bar' } }) + .expect(400); + + expect(response.body.message).toBe( + '[validation [custom]]: expected value of type [string] but got [number]' + ); + }); + }); + }); +}); diff --git a/src/core/server/ui_settings/routes/set.ts b/src/core/server/ui_settings/routes/set.ts index 51ad256b51335..e5158e274245c 100644 --- a/src/core/server/ui_settings/routes/set.ts +++ b/src/core/server/ui_settings/routes/set.ts @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { schema } from '@kbn/config-schema'; +import { schema, ValidationError } from '@kbn/config-schema'; import { IRouter } from '../../http'; import { SavedObjectsErrorHelpers } from '../../saved_objects'; @@ -56,7 +56,7 @@ export function registerSetRoute(router: IRouter) { }); } - if (error instanceof CannotOverrideError) { + if (error instanceof CannotOverrideError || error instanceof ValidationError) { return response.badRequest({ body: error }); } diff --git a/src/core/server/ui_settings/routes/set_many.ts b/src/core/server/ui_settings/routes/set_many.ts index 3794eba004bee..5623c3fe11b80 100644 --- a/src/core/server/ui_settings/routes/set_many.ts +++ b/src/core/server/ui_settings/routes/set_many.ts @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { schema } from '@kbn/config-schema'; +import { schema, ValidationError } from '@kbn/config-schema'; import { IRouter } from '../../http'; import { SavedObjectsErrorHelpers } from '../../saved_objects'; @@ -50,7 +50,7 @@ export function registerSetManyRoute(router: IRouter) { }); } - if (error instanceof CannotOverrideError) { + if (error instanceof CannotOverrideError || error instanceof ValidationError) { return response.badRequest({ body: error }); } diff --git a/src/core/server/ui_settings/types.ts b/src/core/server/ui_settings/types.ts index f3eb1f5a6859c..076e1de4458d7 100644 --- a/src/core/server/ui_settings/types.ts +++ b/src/core/server/ui_settings/types.ts @@ -17,9 +17,10 @@ * under the License. */ import { SavedObjectsClientContract } from '../saved_objects/types'; -import { UiSettingsParams, UserProvidedValues } from '../../types'; +import { UiSettingsParams, UserProvidedValues, PublicUiSettingsParams } from '../../types'; export { UiSettingsParams, + PublicUiSettingsParams, StringValidationRegexString, StringValidationRegex, StringValidation, @@ -41,7 +42,7 @@ export interface IUiSettingsClient { /** * Returns registered uiSettings values {@link UiSettingsParams} */ - getRegistered: () => Readonly>; + getRegistered: () => Readonly>; /** * Retrieves uiSettings values set by the user with fallbacks to default values if not specified. */ diff --git a/src/core/server/ui_settings/ui_settings_client.test.ts b/src/core/server/ui_settings/ui_settings_client.test.ts index b8aa57291dccf..4ce33eed267a3 100644 --- a/src/core/server/ui_settings/ui_settings_client.test.ts +++ b/src/core/server/ui_settings/ui_settings_client.test.ts @@ -18,6 +18,7 @@ */ import Chance from 'chance'; +import { schema } from '@kbn/config-schema'; import { loggingServiceMock } from '../logging/logging_service.mock'; import { createOrUpgradeSavedConfigMock } from './create_or_upgrade_saved_config/create_or_upgrade_saved_config.test.mock'; @@ -145,6 +146,22 @@ describe('ui settings', () => { expect(error.message).toBe('Unable to update "foo" because it is overridden'); } }); + + it('validates value if a schema presents', async () => { + const defaults = { foo: { schema: schema.string() } }; + const { uiSettings, savedObjectsClient } = setup({ defaults }); + + await expect( + uiSettings.setMany({ + bar: 2, + foo: 1, + }) + ).rejects.toMatchInlineSnapshot( + `[Error: [validation [foo]]: expected value of type [string] but got [number]]` + ); + + expect(savedObjectsClient.update).toHaveBeenCalledTimes(0); + }); }); describe('#set()', () => { @@ -163,6 +180,17 @@ describe('ui settings', () => { }); }); + it('validates value if a schema presents', async () => { + const defaults = { foo: { schema: schema.string() } }; + const { uiSettings, savedObjectsClient } = setup({ defaults }); + + await expect(uiSettings.set('foo', 1)).rejects.toMatchInlineSnapshot( + `[Error: [validation [foo]]: expected value of type [string] but got [number]]` + ); + + expect(savedObjectsClient.update).toHaveBeenCalledTimes(0); + }); + it('throws CannotOverrideError if the key is overridden', async () => { const { uiSettings } = setup({ overrides: { @@ -193,6 +221,20 @@ describe('ui settings', () => { expect(savedObjectsClient.update).toHaveBeenCalledWith(TYPE, ID, { one: null }); }); + it('does not fail validation', async () => { + const defaults = { + foo: { + schema: schema.string(), + value: '1', + }, + }; + const { uiSettings, savedObjectsClient } = setup({ defaults }); + + await uiSettings.remove('foo'); + + expect(savedObjectsClient.update).toHaveBeenCalledTimes(1); + }); + it('throws CannotOverrideError if the key is overridden', async () => { const { uiSettings } = setup({ overrides: { @@ -235,6 +277,20 @@ describe('ui settings', () => { }); }); + it('does not fail validation', async () => { + const defaults = { + foo: { + schema: schema.string(), + value: '1', + }, + }; + const { uiSettings, savedObjectsClient } = setup({ defaults }); + + await uiSettings.removeMany(['foo', 'bar']); + + expect(savedObjectsClient.update).toHaveBeenCalledTimes(1); + }); + it('throws CannotOverrideError if any key is overridden', async () => { const { uiSettings } = setup({ overrides: { @@ -256,7 +312,13 @@ describe('ui settings', () => { const value = chance.word(); const defaults = { key: { value } }; const { uiSettings } = setup({ defaults }); - expect(uiSettings.getRegistered()).toBe(defaults); + expect(uiSettings.getRegistered()).toEqual(defaults); + }); + it('does not leak validation schema outside', () => { + const value = chance.word(); + const defaults = { key: { value, schema: schema.string() } }; + const { uiSettings } = setup({ defaults }); + expect(uiSettings.getRegistered()).toStrictEqual({ key: { value } }); }); }); @@ -274,7 +336,7 @@ describe('ui settings', () => { const { uiSettings } = setup({ esDocSource }); const result = await uiSettings.getUserProvided(); - expect(result).toEqual({ + expect(result).toStrictEqual({ user: { userValue: 'customized', }, @@ -286,7 +348,7 @@ describe('ui settings', () => { const { uiSettings } = setup({ esDocSource }); const result = await uiSettings.getUserProvided(); - expect(result).toEqual({ + expect(result).toStrictEqual({ user: { userValue: 'customized', }, @@ -296,6 +358,32 @@ describe('ui settings', () => { }); }); + it('ignores user-configured value if it fails validation', async () => { + const esDocSource = { user: 'foo', id: 'bar' }; + const defaults = { + id: { + value: 42, + schema: schema.number(), + }, + }; + const { uiSettings } = setup({ esDocSource, defaults }); + const result = await uiSettings.getUserProvided(); + + expect(result).toStrictEqual({ + user: { + userValue: 'foo', + }, + }); + + expect(loggingServiceMock.collect(logger).warn).toMatchInlineSnapshot(` + Array [ + Array [ + "Ignore invalid UiSettings value. Error: [validation [id]]: expected value of type [number] but got [string].", + ], + ] + `); + }); + it('automatically creates the savedConfig if it is missing and returns empty object', async () => { const { uiSettings, savedObjectsClient, createOrUpgradeSavedConfig } = setup(); savedObjectsClient.get = jest @@ -303,7 +391,7 @@ describe('ui settings', () => { .mockRejectedValueOnce(SavedObjectsClient.errors.createGenericNotFoundError()) .mockResolvedValueOnce({ attributes: {} }); - expect(await uiSettings.getUserProvided()).toEqual({}); + expect(await uiSettings.getUserProvided()).toStrictEqual({}); expect(savedObjectsClient.get).toHaveBeenCalledTimes(2); @@ -320,7 +408,7 @@ describe('ui settings', () => { SavedObjectsClient.errors.createGenericNotFoundError() ); - expect(await uiSettings.getUserProvided()).toEqual({ foo: { userValue: 'bar ' } }); + expect(await uiSettings.getUserProvided()).toStrictEqual({ foo: { userValue: 'bar ' } }); }); it('returns an empty object on Forbidden responses', async () => { @@ -329,7 +417,7 @@ describe('ui settings', () => { const error = SavedObjectsClient.errors.decorateForbiddenError(new Error()); savedObjectsClient.get.mockRejectedValue(error); - expect(await uiSettings.getUserProvided()).toEqual({}); + expect(await uiSettings.getUserProvided()).toStrictEqual({}); expect(createOrUpgradeSavedConfig).toHaveBeenCalledTimes(0); }); @@ -339,7 +427,7 @@ describe('ui settings', () => { const error = SavedObjectsClient.errors.decorateEsUnavailableError(new Error()); savedObjectsClient.get.mockRejectedValue(error); - expect(await uiSettings.getUserProvided()).toEqual({}); + expect(await uiSettings.getUserProvided()).toStrictEqual({}); expect(createOrUpgradeSavedConfig).toHaveBeenCalledTimes(0); }); @@ -382,7 +470,7 @@ describe('ui settings', () => { }; const { uiSettings } = setup({ esDocSource, overrides }); - expect(await uiSettings.getUserProvided()).toEqual({ + expect(await uiSettings.getUserProvided()).toStrictEqual({ user: { userValue: 'customized', }, @@ -404,15 +492,40 @@ describe('ui settings', () => { expect(savedObjectsClient.get).toHaveBeenCalledWith(TYPE, ID); }); - it(`returns defaults when es doc is empty`, async () => { + it('returns defaults when es doc is empty', async () => { const esDocSource = {}; const defaults = { foo: { value: 'bar' } }; const { uiSettings } = setup({ esDocSource, defaults }); - expect(await uiSettings.getAll()).toEqual({ + expect(await uiSettings.getAll()).toStrictEqual({ foo: 'bar', }); }); + it('ignores user-configured value if it fails validation', async () => { + const esDocSource = { user: 'foo', id: 'bar' }; + const defaults = { + id: { + value: 42, + schema: schema.number(), + }, + }; + const { uiSettings } = setup({ esDocSource, defaults }); + const result = await uiSettings.getAll(); + + expect(result).toStrictEqual({ + id: 42, + user: 'foo', + }); + + expect(loggingServiceMock.collect(logger).warn).toMatchInlineSnapshot(` + Array [ + Array [ + "Ignore invalid UiSettings value. Error: [validation [id]]: expected value of type [number] but got [string].", + ], + ] + `); + }); + it(`merges user values, including ones without defaults, into key value pairs`, async () => { const esDocSource = { foo: 'user-override', @@ -427,7 +540,7 @@ describe('ui settings', () => { const { uiSettings } = setup({ esDocSource, defaults }); - expect(await uiSettings.getAll()).toEqual({ + expect(await uiSettings.getAll()).toStrictEqual({ foo: 'user-override', bar: 'user-provided', }); @@ -451,7 +564,7 @@ describe('ui settings', () => { const { uiSettings } = setup({ esDocSource, defaults, overrides }); - expect(await uiSettings.getAll()).toEqual({ + expect(await uiSettings.getAll()).toStrictEqual({ foo: 'bax', bar: 'user-provided', }); @@ -518,6 +631,28 @@ describe('ui settings', () => { expect(await uiSettings.get('dateFormat')).toBe('foo'); }); + + it('returns the default value if user-configured value fails validation', async () => { + const esDocSource = { id: 'bar' }; + const defaults = { + id: { + value: 42, + schema: schema.number(), + }, + }; + + const { uiSettings } = setup({ esDocSource, defaults }); + + expect(await uiSettings.get('id')).toBe(42); + + expect(loggingServiceMock.collect(logger).warn).toMatchInlineSnapshot(` + Array [ + Array [ + "Ignore invalid UiSettings value. Error: [validation [id]]: expected value of type [number] but got [string].", + ], + ] + `); + }); }); describe('#isOverridden()', () => { diff --git a/src/core/server/ui_settings/ui_settings_client.ts b/src/core/server/ui_settings/ui_settings_client.ts index a7e55d2b2da65..76c8284175f11 100644 --- a/src/core/server/ui_settings/ui_settings_client.ts +++ b/src/core/server/ui_settings/ui_settings_client.ts @@ -16,13 +16,13 @@ * specific language governing permissions and limitations * under the License. */ -import { defaultsDeep } from 'lodash'; +import { defaultsDeep, omit } from 'lodash'; import { SavedObjectsErrorHelpers } from '../saved_objects'; import { SavedObjectsClientContract } from '../saved_objects/types'; import { Logger } from '../logging'; import { createOrUpgradeSavedConfig } from './create_or_upgrade_saved_config'; -import { IUiSettingsClient, UiSettingsParams } from './types'; +import { IUiSettingsClient, UiSettingsParams, PublicUiSettingsParams } from './types'; import { CannotOverrideError } from './ui_settings_errors'; export interface UiSettingsServiceOptions { @@ -40,14 +40,14 @@ interface ReadOptions { autoCreateOrUpgradeIfMissing?: boolean; } -interface UserProvidedValue { +interface UserProvidedValue { userValue?: T; isOverridden?: boolean; } type UiSettingsRawValue = UiSettingsParams & UserProvidedValue; -type UserProvided = Record>; +type UserProvided = Record>; type UiSettingsRaw = Record; export class UiSettingsClient implements IUiSettingsClient { @@ -72,7 +72,11 @@ export class UiSettingsClient implements IUiSettingsClient { } getRegistered() { - return this.defaults; + const copiedDefaults: Record = {}; + for (const [key, value] of Object.entries(this.defaults)) { + copiedDefaults[key] = omit(value, 'schema'); + } + return copiedDefaults; } async get(key: string): Promise { @@ -90,29 +94,21 @@ export class UiSettingsClient implements IUiSettingsClient { }, {} as Record); } - async getUserProvided(): Promise> { - const userProvided: UserProvided = {}; - - // write the userValue for each key stored in the saved object that is not overridden - for (const [key, userValue] of Object.entries(await this.read())) { - if (userValue !== null && !this.isOverridden(key)) { - userProvided[key] = { - userValue, - }; - } - } + async getUserProvided(): Promise> { + const userProvided: UserProvided = this.onReadHook(await this.read()); // write all overridden keys, dropping the userValue is override is null and // adding keys for overrides that are not in saved object - for (const [key, userValue] of Object.entries(this.overrides)) { + for (const [key, value] of Object.entries(this.overrides)) { userProvided[key] = - userValue === null ? { isOverridden: true } : { isOverridden: true, userValue }; + value === null ? { isOverridden: true } : { isOverridden: true, userValue: value }; } return userProvided; } async setMany(changes: Record) { + this.onWriteHook(changes); await this.write({ changes }); } @@ -147,6 +143,43 @@ export class UiSettingsClient implements IUiSettingsClient { return defaultsDeep(userProvided, this.defaults); } + private validateKey(key: string, value: unknown) { + const definition = this.defaults[key]; + if (value === null || definition === undefined) return; + if (definition.schema) { + definition.schema.validate(value, {}, `validation [${key}]`); + } + } + + private onWriteHook(changes: Record) { + for (const key of Object.keys(changes)) { + this.assertUpdateAllowed(key); + } + + for (const [key, value] of Object.entries(changes)) { + this.validateKey(key, value); + } + } + + private onReadHook(values: Record) { + // write the userValue for each key stored in the saved object that is not overridden + // validate value read from saved objects as it can be changed via SO API + const filteredValues: UserProvided = {}; + for (const [key, userValue] of Object.entries(values)) { + if (userValue === null || this.isOverridden(key)) continue; + try { + this.validateKey(key, userValue); + filteredValues[key] = { + userValue: userValue as T, + }; + } catch (error) { + this.log.warn(`Ignore invalid UiSettings value. ${error}.`); + } + } + + return filteredValues; + } + private async write({ changes, autoCreateOrUpgradeIfMissing = true, @@ -154,10 +187,6 @@ export class UiSettingsClient implements IUiSettingsClient { changes: Record; autoCreateOrUpgradeIfMissing?: boolean; }) { - for (const key of Object.keys(changes)) { - this.assertUpdateAllowed(key); - } - try { await this.savedObjectsClient.update(this.type, this.id, changes); } catch (error) { diff --git a/src/core/server/ui_settings/ui_settings_service.test.ts b/src/core/server/ui_settings/ui_settings_service.test.ts index 11766713b3be0..08400f56ad281 100644 --- a/src/core/server/ui_settings/ui_settings_service.test.ts +++ b/src/core/server/ui_settings/ui_settings_service.test.ts @@ -17,6 +17,8 @@ * under the License. */ import { BehaviorSubject } from 'rxjs'; +import { schema } from '@kbn/config-schema'; + import { MockUiSettingsClientConstructor } from './ui_settings_service.test.mock'; import { UiSettingsService, SetupDeps } from './ui_settings_service'; import { httpServiceMock } from '../http/http_service.mock'; @@ -35,6 +37,7 @@ const defaults = { value: 'bar', category: [], description: '', + schema: schema.string(), }, }; @@ -104,6 +107,45 @@ describe('uiSettings', () => { }); describe('#start', () => { + describe('validation', () => { + it('validates registered definitions', async () => { + const { register } = await service.setup(setupDeps); + register({ + custom: { + value: 42, + schema: schema.string(), + }, + }); + + await expect(service.start()).rejects.toMatchInlineSnapshot( + `[Error: [ui settings defaults [custom]]: expected value of type [string] but got [number]]` + ); + }); + + it('validates overrides', async () => { + const coreContext = mockCoreContext.create(); + coreContext.configService.atPath.mockReturnValueOnce( + new BehaviorSubject({ + overrides: { + custom: 42, + }, + }) + ); + const customizedService = new UiSettingsService(coreContext); + const { register } = await customizedService.setup(setupDeps); + register({ + custom: { + value: '42', + schema: schema.string(), + }, + }); + + await expect(customizedService.start()).rejects.toMatchInlineSnapshot( + `[Error: [ui settings overrides [custom]]: expected value of type [string] but got [number]]` + ); + }); + }); + describe('#asScopedToClient', () => { it('passes saved object type "config" to UiSettingsClient', async () => { await service.setup(setupDeps); diff --git a/src/core/server/ui_settings/ui_settings_service.ts b/src/core/server/ui_settings/ui_settings_service.ts index de2cc9d510e0c..83e66cf6dd06d 100644 --- a/src/core/server/ui_settings/ui_settings_service.ts +++ b/src/core/server/ui_settings/ui_settings_service.ts @@ -70,6 +70,9 @@ export class UiSettingsService } public async start(): Promise { + this.validatesDefinitions(); + this.validatesOverrides(); + return { asScopedToClient: this.getScopedClientFactory(), }; @@ -101,4 +104,21 @@ export class UiSettingsService this.uiSettingsDefaults.set(key, value); }); } + + private validatesDefinitions() { + for (const [key, definition] of this.uiSettingsDefaults) { + if (definition.schema) { + definition.schema.validate(definition.value, {}, `ui settings defaults [${key}]`); + } + } + } + + private validatesOverrides() { + for (const [key, value] of Object.entries(this.overrides)) { + const definition = this.uiSettingsDefaults.get(key); + if (definition?.schema) { + definition.schema.validate(value, {}, `ui settings overrides [${key}]`); + } + } + } } diff --git a/src/core/types/ui_settings.ts b/src/core/types/ui_settings.ts index eccd3f9616af0..ed1076b571960 100644 --- a/src/core/types/ui_settings.ts +++ b/src/core/types/ui_settings.ts @@ -16,8 +16,7 @@ * specific language governing permissions and limitations * under the License. */ - -import { SavedObjectAttribute } from './saved_objects'; +import { Type } from '@kbn/config-schema'; /** * UI element type to represent the settings. @@ -49,11 +48,11 @@ export interface DeprecationSettings { * UiSettings parameters defined by the plugins. * @public * */ -export interface UiSettingsParams { +export interface UiSettingsParams { /** title in the UI */ name?: string; /** default value to fall back to if a user doesn't provide any */ - value?: SavedObjectAttribute; + value?: T; /** description provided to a user in UI */ description?: string; /** used to group the configured setting in the UI */ @@ -73,10 +72,22 @@ export interface UiSettingsParams { /* * Allows defining a custom validation applicable to value change on the client. * @deprecated + * Use schema instead. */ validation?: ImageValidation | StringValidation; + /* + * Value validation schema + * Used to validate value on write and read. + */ + schema: Type; } +/** + * A sub-set of {@link UiSettingsParams} exposed to the client-side. + * @public + * */ +export type PublicUiSettingsParams = Omit; + /** * Allows regex objects or a regex string * @public diff --git a/src/core/utils/url.test.ts b/src/core/utils/url.test.ts index 3c35ba44455bc..419c0cda2b8cb 100644 --- a/src/core/utils/url.test.ts +++ b/src/core/utils/url.test.ts @@ -17,7 +17,7 @@ * under the License. */ -import { modifyUrl } from './url'; +import { modifyUrl, isRelativeUrl } from './url'; describe('modifyUrl()', () => { test('throws an error with invalid input', () => { @@ -69,3 +69,17 @@ describe('modifyUrl()', () => { ).toEqual('mail:localhost'); }); }); + +describe('isRelativeUrl()', () => { + test('returns "true" for a relative URL', () => { + expect(isRelativeUrl('good')).toBe(true); + expect(isRelativeUrl('/good')).toBe(true); + expect(isRelativeUrl('/good/even/better')).toBe(true); + }); + test('returns "false" for a non-relative URL', () => { + expect(isRelativeUrl('http://evil.com')).toBe(false); + expect(isRelativeUrl('//evil.com')).toBe(false); + expect(isRelativeUrl('///evil.com')).toBe(false); + expect(isRelativeUrl(' //evil.com')).toBe(false); + }); +}); diff --git a/src/core/utils/url.ts b/src/core/utils/url.ts index 31de7e1814038..c2bf80ce3f86f 100644 --- a/src/core/utils/url.ts +++ b/src/core/utils/url.ts @@ -99,3 +99,19 @@ export function modifyUrl( slashes: modifiedParts.slashes, } as UrlObject); } + +export function isRelativeUrl(candidatePath: string) { + // validate that `candidatePath` is not attempting a redirect to somewhere + // outside of this Kibana install + const all = parseUrl(candidatePath, false /* parseQueryString */, true /* slashesDenoteHost */); + const { protocol, hostname, port } = all; + // We should explicitly compare `protocol`, `port` and `hostname` to null to make sure these are not + // detected in the URL at all. For example `hostname` can be empty string for Node URL parser, but + // browser (because of various bwc reasons) processes URL differently (e.g. `///abc.com` - for browser + // hostname is `abc.com`, but for Node hostname is an empty string i.e. everything between schema (`//`) + // and the first slash that belongs to path. + if (protocol !== null || hostname !== null || port !== null) { + return false; + } + return true; +} diff --git a/src/legacy/core_plugins/kibana/ui_setting_defaults.js b/src/legacy/core_plugins/kibana/ui_setting_defaults.js index c0628b72c2ce7..85b1956f45333 100644 --- a/src/legacy/core_plugins/kibana/ui_setting_defaults.js +++ b/src/legacy/core_plugins/kibana/ui_setting_defaults.js @@ -16,11 +16,13 @@ * specific language governing permissions and limitations * under the License. */ - import moment from 'moment-timezone'; import numeralLanguages from '@elastic/numeral/languages'; import { i18n } from '@kbn/i18n'; +import { schema } from '@kbn/config-schema'; + import { DEFAULT_QUERY_LANGUAGE } from '../../../plugins/data/common'; +import { isRelativeUrl } from '../../../core/utils'; export function getUiSettingDefaults() { const weekdays = moment.weekdays().slice(); @@ -67,17 +69,23 @@ export function getUiSettingDefaults() { defaultMessage: 'Default route', }), value: '/app/kibana', - validation: { - regexString: '^/', - message: i18n.translate('kbn.advancedSettings.defaultRoute.defaultRouteValidationMessage', { - defaultMessage: 'The route must start with a slash ("/")', - }), - }, + schema: schema.string({ + validate(value) { + if (!value.startsWith('/') || !isRelativeUrl(value)) { + return i18n.translate( + 'kbn.advancedSettings.defaultRoute.defaultRouteIsRelativeValidationMessage', + { + defaultMessage: 'Must be a relative URL.', + } + ); + } + }, + }), description: i18n.translate('kbn.advancedSettings.defaultRoute.defaultRouteText', { defaultMessage: 'This setting specifies the default route when opening Kibana. ' + 'You can use this setting to modify the landing page when opening Kibana. ' + - 'The route must start with a slash ("/").', + 'The route must be a relative URL.', }), }, 'query:queryString:options': { diff --git a/src/legacy/core_plugins/tests_bundle/tests_entry_template.js b/src/legacy/core_plugins/tests_bundle/tests_entry_template.js index 57adf730f3dd9..3e3dc284671da 100644 --- a/src/legacy/core_plugins/tests_bundle/tests_entry_template.js +++ b/src/legacy/core_plugins/tests_bundle/tests_entry_template.js @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ - +import { Type } from '@kbn/config-schema'; import pkg from '../../../../package.json'; export const createTestEntryTemplate = defaultUiSettings => bundle => ` @@ -87,7 +87,14 @@ const coreSystem = new CoreSystem({ buildNum: 1234, devMode: true, uiSettings: { - defaults: ${JSON.stringify(defaultUiSettings, null, 2) + defaults: ${JSON.stringify( + defaultUiSettings, + (key, value) => { + if (value instanceof Type) return null; + return value; + }, + 2 + ) .split('\n') .join('\n ')}, user: {} diff --git a/src/legacy/server/http/index.js b/src/legacy/server/http/index.js index 265d71e95b301..d616afb533d0a 100644 --- a/src/legacy/server/http/index.js +++ b/src/legacy/server/http/index.js @@ -24,15 +24,12 @@ import Boom from 'boom'; import { registerHapiPlugins } from './register_hapi_plugins'; import { setupBasePathProvider } from './setup_base_path_provider'; -import { setupDefaultRouteProvider } from './setup_default_route_provider'; export default async function(kbnServer, server, config) { server = kbnServer.server; setupBasePathProvider(kbnServer); - setupDefaultRouteProvider(server); - await registerHapiPlugins(server); // provide a simple way to expose static directories @@ -60,14 +57,6 @@ export default async function(kbnServer, server, config) { }); }); - server.route({ - path: '/', - method: 'GET', - async handler(req, h) { - return h.redirect(await req.getDefaultRoute()); - }, - }); - server.route({ method: 'GET', path: '/{p*}', diff --git a/src/legacy/server/http/integration_tests/default_route_provider.test.ts b/src/legacy/server/http/integration_tests/default_route_provider.test.ts deleted file mode 100644 index d91438d904558..0000000000000 --- a/src/legacy/server/http/integration_tests/default_route_provider.test.ts +++ /dev/null @@ -1,87 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -jest.mock('../../../ui/ui_settings/ui_settings_mixin', () => { - return jest.fn(); -}); - -import * as kbnTestServer from '../../../../test_utils/kbn_server'; -// eslint-disable-next-line @kbn/eslint/no-restricted-paths -import { Root } from '../../../../core/server/root'; - -let mockDefaultRouteSetting: any = ''; - -describe('default route provider', () => { - let root: Root; - beforeAll(async () => { - root = kbnTestServer.createRoot({ migrations: { skip: true } }); - - await root.setup(); - await root.start(); - - const kbnServer = kbnTestServer.getKbnServer(root); - - kbnServer.server.decorate('request', 'getUiSettingsService', function() { - return { - get: (key: string) => { - if (key === 'defaultRoute') { - return Promise.resolve(mockDefaultRouteSetting); - } - throw Error(`unsupported ui setting: ${key}`); - }, - getRegistered: () => { - return { - defaultRoute: { - value: '/app/kibana', - }, - }; - }, - }; - }); - }, 30000); - - afterAll(async () => await root.shutdown()); - - it('redirects to the configured default route', async function() { - mockDefaultRouteSetting = '/app/some/default/route'; - - const { status, header } = await kbnTestServer.request.get(root, '/'); - expect(status).toEqual(302); - expect(header).toMatchObject({ - location: '/app/some/default/route', - }); - }); - - const invalidRoutes = [ - 'http://not-your-kibana.com', - '///example.com', - '//example.com', - ' //example.com', - ]; - for (const route of invalidRoutes) { - it(`falls back to /app/kibana when the configured route (${route}) is not a valid relative path`, async function() { - mockDefaultRouteSetting = route; - - const { status, header } = await kbnTestServer.request.get(root, '/'); - expect(status).toEqual(302); - expect(header).toMatchObject({ - location: '/app/kibana', - }); - }); - } -}); diff --git a/src/legacy/server/http/integration_tests/default_route_provider_config.test.ts b/src/legacy/server/http/integration_tests/default_route_provider_config.test.ts deleted file mode 100644 index 8365941cbeb10..0000000000000 --- a/src/legacy/server/http/integration_tests/default_route_provider_config.test.ts +++ /dev/null @@ -1,54 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -import * as kbnTestServer from '../../../../test_utils/kbn_server'; -// eslint-disable-next-line @kbn/eslint/no-restricted-paths -import { Root } from '../../../../core/server/root'; - -describe('default route provider', () => { - let root: Root; - - afterEach(async () => await root.shutdown()); - - it('redirects to the configured default route', async function() { - root = kbnTestServer.createRoot({ - server: { - defaultRoute: '/app/some/default/route', - }, - migrations: { skip: true }, - }); - - await root.setup(); - await root.start(); - - const kbnServer = kbnTestServer.getKbnServer(root); - - kbnServer.server.decorate('request', 'getSavedObjectsClient', function() { - return { - get: (type: string, id: string) => ({ attributes: {} }), - }; - }); - - const { status, header } = await kbnTestServer.request.get(root, '/'); - - expect(status).toEqual(302); - expect(header).toMatchObject({ - location: '/app/some/default/route', - }); - }); -}); diff --git a/src/legacy/server/http/setup_default_route_provider.ts b/src/legacy/server/http/setup_default_route_provider.ts deleted file mode 100644 index 9a580dd1c59bd..0000000000000 --- a/src/legacy/server/http/setup_default_route_provider.ts +++ /dev/null @@ -1,74 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import { Legacy } from 'kibana'; -import { parse } from 'url'; - -export function setupDefaultRouteProvider(server: Legacy.Server) { - server.decorate('request', 'getDefaultRoute', async function() { - // @ts-ignore - const request: Legacy.Request = this; - - const serverBasePath: string = server.config().get('server.basePath'); - - const uiSettings = request.getUiSettingsService(); - - const defaultRoute = await uiSettings.get('defaultRoute'); - const qualifiedDefaultRoute = `${request.getBasePath()}${defaultRoute}`; - - if (isRelativePath(qualifiedDefaultRoute, serverBasePath)) { - return qualifiedDefaultRoute; - } else { - server.log( - ['http', 'warn'], - `Ignoring configured default route of '${defaultRoute}', as it is malformed.` - ); - - const fallbackRoute = uiSettings.getRegistered().defaultRoute.value; - - const qualifiedFallbackRoute = `${request.getBasePath()}${fallbackRoute}`; - return qualifiedFallbackRoute; - } - }); - - function isRelativePath(candidatePath: string, basePath = '') { - // validate that `candidatePath` is not attempting a redirect to somewhere - // outside of this Kibana install - const { protocol, hostname, port, pathname } = parse( - candidatePath, - false /* parseQueryString */, - true /* slashesDenoteHost */ - ); - - // We should explicitly compare `protocol`, `port` and `hostname` to null to make sure these are not - // detected in the URL at all. For example `hostname` can be empty string for Node URL parser, but - // browser (because of various bwc reasons) processes URL differently (e.g. `///abc.com` - for browser - // hostname is `abc.com`, but for Node hostname is an empty string i.e. everything between schema (`//`) - // and the first slash that belongs to path. - if (protocol !== null || hostname !== null || port !== null) { - return false; - } - - if (!String(pathname).startsWith(basePath)) { - return false; - } - - return true; - } -} diff --git a/src/legacy/server/kbn_server.d.ts b/src/legacy/server/kbn_server.d.ts index 68b5a63871372..9952b345fa06f 100644 --- a/src/legacy/server/kbn_server.d.ts +++ b/src/legacy/server/kbn_server.d.ts @@ -92,7 +92,6 @@ declare module 'hapi' { interface Request { getSavedObjectsClient(options?: SavedObjectsClientProviderOptions): SavedObjectsClientContract; getBasePath(): string; - getDefaultRoute(): Promise; getUiSettingsService(): IUiSettingsClient; } diff --git a/src/plugins/advanced_settings/public/management_app/advanced_settings.test.tsx b/src/plugins/advanced_settings/public/management_app/advanced_settings.test.tsx index 7a2ab648ec258..6103041cf0a4c 100644 --- a/src/plugins/advanced_settings/public/management_app/advanced_settings.test.tsx +++ b/src/plugins/advanced_settings/public/management_app/advanced_settings.test.tsx @@ -22,7 +22,11 @@ import { Observable } from 'rxjs'; import { ReactWrapper } from 'enzyme'; import { mountWithI18nProvider } from 'test_utils/enzyme_helpers'; import dedent from 'dedent'; -import { UiSettingsParams, UserProvidedValues, UiSettingsType } from '../../../../core/public'; +import { + PublicUiSettingsParams, + UserProvidedValues, + UiSettingsType, +} from '../../../../core/public'; import { FieldSetting } from './types'; import { AdvancedSettingsComponent } from './advanced_settings'; import { notificationServiceMock, docLinksServiceMock } from '../../../../core/public/mocks'; @@ -68,7 +72,7 @@ function mockConfig() { remove: (key: string) => Promise.resolve(true), isCustom: (key: string) => false, isOverridden: (key: string) => Boolean(config.getAll()[key].isOverridden), - getRegistered: () => ({} as Readonly>), + getRegistered: () => ({} as Readonly>), overrideLocalDefault: (key: string, value: any) => {}, getUpdate$: () => new Observable<{ @@ -89,7 +93,7 @@ function mockConfig() { getUpdateErrors$: () => new Observable(), get: (key: string, defaultOverride?: any): any => config.getAll()[key] || defaultOverride, get$: (key: string) => new Observable(config.get(key)), - getAll: (): Readonly> => { + getAll: (): Readonly> => { return { 'test:array:setting': { ...defaultConfig, diff --git a/src/plugins/advanced_settings/public/management_app/lib/to_editable_config.test.ts b/src/plugins/advanced_settings/public/management_app/lib/to_editable_config.test.ts index 881a2eb003cc8..7ac9b281eb99a 100644 --- a/src/plugins/advanced_settings/public/management_app/lib/to_editable_config.test.ts +++ b/src/plugins/advanced_settings/public/management_app/lib/to_editable_config.test.ts @@ -17,7 +17,7 @@ * under the License. */ -import { UiSettingsParams, StringValidationRegex } from 'src/core/public'; +import { PublicUiSettingsParams, StringValidationRegex } from 'src/core/public'; import expect from '@kbn/expect'; import { toEditableConfig } from './to_editable_config'; @@ -30,7 +30,7 @@ function invoke({ name = 'woah', value = 'forreal', }: { - def?: UiSettingsParams & { isOverridden?: boolean }; + def?: PublicUiSettingsParams & { isOverridden?: boolean }; name?: string; value?: any; }) { @@ -55,7 +55,7 @@ describe('Settings', function() { }); describe('when given a setting definition object', function() { - let def: UiSettingsParams & { isOverridden?: boolean }; + let def: PublicUiSettingsParams & { isOverridden?: boolean }; beforeEach(function() { def = { value: 'the original', diff --git a/src/plugins/advanced_settings/public/management_app/lib/to_editable_config.ts b/src/plugins/advanced_settings/public/management_app/lib/to_editable_config.ts index 2c27d72f7f645..406bc35f826e8 100644 --- a/src/plugins/advanced_settings/public/management_app/lib/to_editable_config.ts +++ b/src/plugins/advanced_settings/public/management_app/lib/to_editable_config.ts @@ -18,7 +18,7 @@ */ import { - UiSettingsParams, + PublicUiSettingsParams, UserProvidedValues, StringValidationRegexString, SavedObjectAttribute, @@ -40,7 +40,7 @@ export function toEditableConfig({ isCustom, isOverridden, }: { - def: UiSettingsParams & UserProvidedValues; + def: PublicUiSettingsParams & UserProvidedValues; name: string; value: SavedObjectAttribute; isCustom: boolean; diff --git a/src/plugins/advanced_settings/public/management_app/types.ts b/src/plugins/advanced_settings/public/management_app/types.ts index d44a05ce36f5d..ee9b9b0535b79 100644 --- a/src/plugins/advanced_settings/public/management_app/types.ts +++ b/src/plugins/advanced_settings/public/management_app/types.ts @@ -17,17 +17,12 @@ * under the License. */ -import { - UiSettingsType, - StringValidation, - ImageValidation, - SavedObjectAttribute, -} from '../../../../core/public'; +import { UiSettingsType, StringValidation, ImageValidation } from '../../../../core/public'; export interface FieldSetting { displayName: string; name: string; - value: SavedObjectAttribute; + value: unknown; description?: string; options?: string[]; optionLabels?: Record; @@ -36,7 +31,7 @@ export interface FieldSetting { category: string[]; ariaName: string; isOverridden: boolean; - defVal: SavedObjectAttribute; + defVal: unknown; isCustom: boolean; validation?: StringValidation | ImageValidation; readOnly?: boolean; diff --git a/src/plugins/data/public/public.api.md b/src/plugins/data/public/public.api.md index 333b13eedc17a..783411bbf27e2 100644 --- a/src/plugins/data/public/public.api.md +++ b/src/plugins/data/public/public.api.md @@ -38,6 +38,7 @@ import { Observable } from 'rxjs'; import { Plugin as Plugin_2 } from 'src/core/public'; import { PluginInitializerContext as PluginInitializerContext_2 } from 'src/core/public'; import { PopoverAnchorPosition } from '@elastic/eui'; +import { PublicUiSettingsParams } from 'src/core/server/types'; import React from 'react'; import * as React_2 from 'react'; import { Required } from '@kbn/utility-types'; @@ -49,7 +50,6 @@ import { SearchResponse as SearchResponse_2 } from 'elasticsearch'; import { SimpleSavedObject } from 'src/core/public'; import { UiActionsSetup } from 'src/plugins/ui_actions/public'; import { UiActionsStart } from 'src/plugins/ui_actions/public'; -import { UiSettingsParams } from 'src/core/server/types'; import { Unit } from '@elastic/datemath'; import { UnregisterCallback } from 'history'; import { UserProvidedValues } from 'src/core/server/types'; diff --git a/src/test_utils/kbn_server.ts b/src/test_utils/kbn_server.ts index f4c3ecd8243ce..12f7eb5a0a043 100644 --- a/src/test_utils/kbn_server.ts +++ b/src/test_utils/kbn_server.ts @@ -50,6 +50,7 @@ const DEFAULTS_SETTINGS = { logging: { silent: true }, plugins: {}, optimize: { enabled: false }, + migrations: { skip: true }, }; const DEFAULT_SETTINGS_WITH_CORE_PLUGINS = { diff --git a/test/api_integration/apis/index.js b/test/api_integration/apis/index.js index 8cdbbf8e74a3d..57e9120773f33 100644 --- a/test/api_integration/apis/index.js +++ b/test/api_integration/apis/index.js @@ -33,6 +33,5 @@ export default function({ loadTestFile }) { loadTestFile(require.resolve('./status')); loadTestFile(require.resolve('./stats')); loadTestFile(require.resolve('./ui_metric')); - loadTestFile(require.resolve('./core')); }); } diff --git a/test/plugin_functional/plugins/ui_settings_plugin/server/plugin.ts b/test/plugin_functional/plugins/ui_settings_plugin/server/plugin.ts index c32e8a75d95da..3801d3bbce055 100644 --- a/test/plugin_functional/plugins/ui_settings_plugin/server/plugin.ts +++ b/test/plugin_functional/plugins/ui_settings_plugin/server/plugin.ts @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ - +import { schema } from '@kbn/config-schema'; import { Plugin, CoreSetup } from 'kibana/server'; export class UiSettingsPlugin implements Plugin { @@ -27,6 +27,7 @@ export class UiSettingsPlugin implements Plugin { description: 'just for testing', value: '2', category: ['any'], + schema: schema.string(), }, }); diff --git a/x-pack/legacy/plugins/spaces/server/routes/views/enter_space.ts b/x-pack/legacy/plugins/spaces/server/routes/views/enter_space.ts index 337faa2a18fb6..60ae3a1fa77bb 100644 --- a/x-pack/legacy/plugins/spaces/server/routes/views/enter_space.ts +++ b/x-pack/legacy/plugins/spaces/server/routes/views/enter_space.ts @@ -14,7 +14,13 @@ export function initEnterSpaceView(server: Legacy.Server) { path: ENTER_SPACE_PATH, async handler(request, h) { try { - return h.redirect(await request.getDefaultRoute()); + const uiSettings = request.getUiSettingsService(); + const defaultRoute = await uiSettings.get('defaultRoute'); + + const basePath = server.newPlatform.setup.core.http.basePath.get(request); + const url = `${basePath}${defaultRoute}`; + + return h.redirect(url); } catch (e) { server.log(['spaces', 'error'], `Error navigating to space: ${e}`); return wrapError(e); diff --git a/x-pack/plugins/translations/translations/ja-JP.json b/x-pack/plugins/translations/translations/ja-JP.json index fb0eb6e4bf804..3763020a0b692 100644 --- a/x-pack/plugins/translations/translations/ja-JP.json +++ b/x-pack/plugins/translations/translations/ja-JP.json @@ -833,7 +833,6 @@ "kbn.advancedSettings.defaultIndexTitle": "デフォルトのインデックス", "kbn.advancedSettings.defaultRoute.defaultRouteText": "この設定は、Kibana 起動時のデフォルトのルートを設定します。この設定で、Kibana 起動時のランディングページを変更できます。ルートはスラッシュ (\"/\") で始まる必要があります。", "kbn.advancedSettings.defaultRoute.defaultRouteTitle": "デフォルトのルート", - "kbn.advancedSettings.defaultRoute.defaultRouteValidationMessage": "ルートはスラッシュ (\"/\") で始まる必要があります。", "kbn.advancedSettings.disableAnimationsText": "Kibana UI の不要なアニメーションをオフにします。変更を適用するにはページを更新してください。", "kbn.advancedSettings.disableAnimationsTitle": "アニメーションを無効にする", "kbn.advancedSettings.discover.aggsTermsSizeText": "「可視化」ボタンをクリックした際に、フィールドドロップダウンやディスカバリサイドバーに可視化される用語の数を設定します。", diff --git a/x-pack/plugins/translations/translations/zh-CN.json b/x-pack/plugins/translations/translations/zh-CN.json index 0a9c82afaec1d..0477470a4b8a1 100644 --- a/x-pack/plugins/translations/translations/zh-CN.json +++ b/x-pack/plugins/translations/translations/zh-CN.json @@ -833,7 +833,6 @@ "kbn.advancedSettings.defaultIndexTitle": "默认索引", "kbn.advancedSettings.defaultRoute.defaultRouteText": "此设置指定打开 Kibana 时的默认路由。您可以使用此设置修改打开 Kibana 时的登陆页面。路由必须以正斜杠（“/”）开头。", "kbn.advancedSettings.defaultRoute.defaultRouteTitle": "默认路由", - "kbn.advancedSettings.defaultRoute.defaultRouteValidationMessage": "路由必须以正斜杠（“/”）开头", "kbn.advancedSettings.disableAnimationsText": "在 Kibana UI 中关闭所有没有必要的动画。刷新页面以应用更改。", "kbn.advancedSettings.disableAnimationsTitle": "禁用动画", "kbn.advancedSettings.discover.aggsTermsSizeText": "确定在单击“可视化”按钮时将在发现侧边栏的字段下拉列表中可视化多少个词。", diff --git a/x-pack/test/functional/apps/spaces/enter_space.ts b/x-pack/test/functional/apps/spaces/enter_space.ts index e0b1ec544d460..38220c15cb266 100644 --- a/x-pack/test/functional/apps/spaces/enter_space.ts +++ b/x-pack/test/functional/apps/spaces/enter_space.ts @@ -10,7 +10,6 @@ export default function enterSpaceFunctonalTests({ getPageObjects, }: FtrProviderContext) { const esArchiver = getService('esArchiver'); - const kibanaServer = getService('kibanaServer'); const PageObjects = getPageObjects(['security', 'spaceSelector']); describe('Enter Space', function() { @@ -25,8 +24,8 @@ export default function enterSpaceFunctonalTests({ await PageObjects.security.forceLogout(); }); - it('allows user to navigate to different spaces, respecting the configured default route', async () => { - const spaceId = 'another-space'; + it('falls back to the default home page when the configured default route is malformed', async () => { + const spaceId = 'default'; await PageObjects.security.login(null, null, { expectSpaceSelector: true, @@ -34,22 +33,11 @@ export default function enterSpaceFunctonalTests({ await PageObjects.spaceSelector.clickSpaceCard(spaceId); - await PageObjects.spaceSelector.expectRoute(spaceId, '/app/kibana/#/dashboard'); - - await PageObjects.spaceSelector.openSpacesNav(); - - // change spaces - - await PageObjects.spaceSelector.clickSpaceAvatar('default'); - - await PageObjects.spaceSelector.expectRoute('default', '/app/canvas'); + await PageObjects.spaceSelector.expectHomePage(spaceId); }); - it('falls back to the default home page when the configured default route is malformed', async () => { - await kibanaServer.uiSettings.replace({ defaultRoute: 'http://example.com/evil' }); - - // This test only works with the default space, as other spaces have an enforced relative url of `${serverBasePath}/s/space-id/${defaultRoute}` - const spaceId = 'default'; + it('allows user to navigate to different spaces, respecting the configured default route', async () => { + const spaceId = 'another-space'; await PageObjects.security.login(null, null, { expectSpaceSelector: true, @@ -57,7 +45,15 @@ export default function enterSpaceFunctonalTests({ await PageObjects.spaceSelector.clickSpaceCard(spaceId); - await PageObjects.spaceSelector.expectHomePage(spaceId); + await PageObjects.spaceSelector.expectRoute(spaceId, '/app/canvas'); + + await PageObjects.spaceSelector.openSpacesNav(); + + // change spaces + const newSpaceId = 'default'; + await PageObjects.spaceSelector.clickSpaceAvatar(newSpaceId); + + await PageObjects.spaceSelector.expectHomePage(newSpaceId); }); }); } diff --git a/x-pack/test/functional/es_archives/spaces/enter_space/data.json b/x-pack/test/functional/es_archives/spaces/enter_space/data.json index 462a2a1ee38fe..475fc14e96e6a 100644 --- a/x-pack/test/functional/es_archives/spaces/enter_space/data.json +++ b/x-pack/test/functional/es_archives/spaces/enter_space/data.json @@ -7,7 +7,7 @@ "config": { "buildNum": 8467, "dateFormat:tz": "UTC", - "defaultRoute": "/app/canvas" + "defaultRoute": "http://example.com/evil" }, "type": "config" } @@ -24,7 +24,7 @@ "config": { "buildNum": 8467, "dateFormat:tz": "UTC", - "defaultRoute": "/app/kibana/#dashboard" + "defaultRoute": "/app/canvas" }, "type": "config" }