From 147d7aac9b1dcd6f311f260246ff8878ac70fedd Mon Sep 17 00:00:00 2001 From: Luke Elmers Date: Thu, 4 Nov 2021 15:29:50 -0600 Subject: [PATCH 01/10] Adds validations for SO types & enforces them on create. --- src/core/server/index.ts | 4 + src/core/server/saved_objects/index.ts | 7 + .../saved_objects/saved_objects_service.ts | 16 +- .../service/lib/repository.test.ts | 47 ++- .../saved_objects/service/lib/repository.ts | 75 +++- src/core/server/saved_objects/types.ts | 15 +- .../server/saved_objects/validation/index.ts | 15 + .../integration_tests/validator.test.ts | 322 ++++++++++++++++++ .../validation/validator.test.ts | 87 +++++ .../saved_objects/validation/validator.ts | 129 +++++++ .../validation/validator_error.ts | 23 ++ 11 files changed, 719 insertions(+), 21 deletions(-) create mode 100644 src/core/server/saved_objects/validation/index.ts create mode 100644 src/core/server/saved_objects/validation/integration_tests/validator.test.ts create mode 100644 src/core/server/saved_objects/validation/validator.test.ts create mode 100644 src/core/server/saved_objects/validation/validator.ts create mode 100644 src/core/server/saved_objects/validation/validator_error.ts diff --git a/src/core/server/index.ts b/src/core/server/index.ts index bb91b9f9be98f2..bef5f2f29e62e6 100644 --- a/src/core/server/index.ts +++ b/src/core/server/index.ts @@ -362,6 +362,10 @@ export type { SavedObjectsImportSimpleWarning, SavedObjectsImportActionRequiredWarning, SavedObjectsImportWarning, + SavedObjectsValidationError, + SavedObjectsValidationFunction, + SavedObjectsValidationMap, + SavedObjectsValidationSpec, } from './saved_objects'; export type { diff --git a/src/core/server/saved_objects/index.ts b/src/core/server/saved_objects/index.ts index b689b348e9e4c6..21044c4e0e138e 100644 --- a/src/core/server/saved_objects/index.ts +++ b/src/core/server/saved_objects/index.ts @@ -93,6 +93,13 @@ export type { SavedObjectTypeExcludeFromUpgradeFilterHook, } from './types'; +export type { + SavedObjectsValidationMap, + SavedObjectsValidationSpec, + SavedObjectsValidationFunction, +} from './validation'; +export { SavedObjectsValidationError } from './validation'; + export { savedObjectsConfig, savedObjectsMigrationConfig } from './saved_objects_config'; export { SavedObjectTypeRegistry } from './saved_objects_type_registry'; export type { ISavedObjectTypeRegistry } from './saved_objects_type_registry'; diff --git a/src/core/server/saved_objects/saved_objects_service.ts b/src/core/server/saved_objects/saved_objects_service.ts index a55f370c7ca225..fef20937c9cfe9 100644 --- a/src/core/server/saved_objects/saved_objects_service.ts +++ b/src/core/server/saved_objects/saved_objects_service.ts @@ -29,7 +29,12 @@ import { SavedObjectConfig, } from './saved_objects_config'; import { KibanaRequest, InternalHttpServiceSetup } from '../http'; -import { SavedObjectsClientContract, SavedObjectsType, SavedObjectStatusMeta } from './types'; +import { + SavedObjectsClientContract, + SavedObjectsType, + SavedObjectStatusMeta, + SavedObjectAttributes, +} from './types'; import { ISavedObjectsRepository, SavedObjectsRepository } from './service/lib/repository'; import { SavedObjectsClientFactoryProvider, @@ -112,6 +117,7 @@ export interface SavedObjectsServiceSetup { * // src/plugins/my_plugin/server/saved_objects/my_type.ts * import { SavedObjectsType } from 'src/core/server'; * import * as migrations from './migrations'; + * import * as schemas from './schemas'; * * export const myType: SavedObjectsType = { * name: 'MyType', @@ -131,6 +137,10 @@ export interface SavedObjectsServiceSetup { * '2.0.0': migrations.migrateToV2, * '2.1.0': migrations.migrateToV2_1 * }, + * schemas: { + * '2.0.0': schemas.v2, + * '2.1.0': schemas.v2_1, + * }, * }; * * // src/plugins/my_plugin/server/plugin.ts @@ -144,7 +154,9 @@ export interface SavedObjectsServiceSetup { * } * ``` */ - registerType: (type: SavedObjectsType) => void; + registerType: ( + type: SavedObjectsType + ) => void; /** * Returns the default index used for saved objects. diff --git a/src/core/server/saved_objects/service/lib/repository.test.ts b/src/core/server/saved_objects/service/lib/repository.test.ts index 46a532cdefef4c..43b424a240ec37 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.ts +++ b/src/core/server/saved_objects/service/lib/repository.test.ts @@ -22,6 +22,7 @@ import { import type { Payload } from '@hapi/boom'; import * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey'; +import { schema } from '@kbn/config-schema'; import { SavedObjectsType, SavedObject, @@ -225,7 +226,16 @@ describe('SavedObjectsRepository', () => { const registry = new SavedObjectTypeRegistry(); registry.registerType(createType('config')); registry.registerType(createType('index-pattern')); - registry.registerType(createType('dashboard')); + registry.registerType( + createType('dashboard', { + schemas: { + '8.0.0-testing': schema.object({ + title: schema.maybe(schema.string()), + otherField: schema.maybe(schema.string()), + }), + }, + }) + ); registry.registerType(createType(CUSTOM_INDEX_TYPE, { indexPattern: 'custom' })); registry.registerType(createType(NAMESPACE_AGNOSTIC_TYPE, { namespaceType: 'agnostic' })); registry.registerType(createType(MULTI_NAMESPACE_TYPE, { namespaceType: 'multiple' })); @@ -971,6 +981,32 @@ describe('SavedObjectsRepository', () => { }; await bulkCreateError(obj3, true, expectedErrorResult); }); + + it(`returns errors for any bulk objects with invalid schemas`, async () => { + const response = getMockBulkCreateResponse([obj3]); + client.bulk.mockResolvedValueOnce( + elasticsearchClientMock.createSuccessTransportRequestPromise(response) + ); + + const result = await savedObjectsRepository.bulkCreate([ + obj3, + // @ts-expect-error - Title should be a string and is intentionally malformed for testing + { ...obj3, id: 'three-again', attributes: { title: 123 } }, + ]); + expect(client.bulk).toHaveBeenCalledTimes(1); // only called once for the valid object + expect(result.saved_objects).toEqual( + expect.arrayContaining([ + expect.objectContaining(obj3), + expect.objectContaining({ + error: new Error( + '[title]: expected value of type [string] but got [number]: Bad Request' + ), + id: 'three-again', + type: 'dashboard', + }), + ]) + ); + }); }); describe('migration', () => { @@ -2530,6 +2566,15 @@ describe('SavedObjectsRepository', () => { expect(client.create).not.toHaveBeenCalled(); }); + it(`throws when schema validation fails`, async () => { + await expect( + savedObjectsRepository.create('dashboard', { title: 123 }) + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"[title]: expected value of type [string] but got [number]: Bad Request"` + ); + expect(client.create).not.toHaveBeenCalled(); + }); + it(`throws when there is a conflict from preflightCheckForCreate`, async () => { mockPreflightCheckForCreate.mockResolvedValueOnce([ { type: MULTI_NAMESPACE_ISOLATED_TYPE, id, error: { type: 'unresolvableConflict' } }, // error type and metadata dont matter diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index 9be58f1b718613..432726fdbfffcc 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -63,6 +63,7 @@ import { SavedObjectsMigrationVersion, MutatingOperationRefreshSetting, } from '../../types'; +import { SavedObjectsTypeValidator } from '../../validation'; import { ISavedObjectTypeRegistry } from '../../saved_objects_type_registry'; import { internalBulkResolve, InternalBulkResolveError } from './internal_bulk_resolve'; import { validateConvertFilterToKueryNode } from './filter_utils'; @@ -363,7 +364,15 @@ export class SavedObjectsRepository { ...(Array.isArray(references) && { references }), }); - const raw = this._serializer.savedObjectToRaw(migrated as SavedObjectSanitizedDoc); + /** + * If a validation has been registered for this type, we run it against the migrated attributes. + * This is an imperfect solution because malformed attributes could have already caused the + * migration to fail, but it's the best we can do without devising a way to run validations + * inside the migration algorithm itself. + */ + this.validateObjectAttributes(type, migrated as SavedObjectSanitizedDoc); + + const raw = this._serializer.savedObjectToRaw(migrated as SavedObjectSanitizedDoc); const requestParams = { id: raw._id, @@ -514,22 +523,41 @@ export class SavedObjectsRepository { versionProperties = getExpectedVersionProperties(version); } + const migrated = this._migrator.migrateDocument({ + id: object.id, + type: object.type, + attributes: object.attributes, + migrationVersion: object.migrationVersion, + ...(savedObjectNamespace && { namespace: savedObjectNamespace }), + ...(savedObjectNamespaces && { namespaces: savedObjectNamespaces }), + updated_at: time, + references: object.references || [], + originId: object.originId, + }) as SavedObjectSanitizedDoc; + + /** + * If a validation has been registered for this type, we run it against the migrated attributes. + * This is an imperfect solution because malformed attributes could have already caused the + * migration to fail, but it's the best we can do without devising a way to run validations + * inside the migration algorithm itself. + */ + try { + this.validateObjectAttributes(object.type, migrated); + } catch (error) { + return { + tag: 'Left', + value: { + id: object.id, + type: object.type, + error, + }, + }; + } + const expectedResult = { esRequestIndex: bulkRequestIndexCounter++, requestedId: object.id, - rawMigratedDoc: this._serializer.savedObjectToRaw( - this._migrator.migrateDocument({ - id: object.id, - type: object.type, - attributes: object.attributes, - migrationVersion: object.migrationVersion, - ...(savedObjectNamespace && { namespace: savedObjectNamespace }), - ...(savedObjectNamespaces && { namespaces: savedObjectNamespaces }), - updated_at: time, - references: object.references || [], - originId: object.originId, - }) as SavedObjectSanitizedDoc - ), + rawMigratedDoc: this._serializer.savedObjectToRaw(migrated), }; bulkCreateParams.push( @@ -2196,6 +2224,25 @@ export class SavedObjectsRepository { ); } } + + /** Validate a migrated doc against the registered saved object type's schema. */ + private validateObjectAttributes(type: string, doc: SavedObjectSanitizedDoc) { + const savedObjectType = this._registry.getType(type); + if (!savedObjectType?.schemas) { + return; + } + + const validator = new SavedObjectsTypeValidator({ + type, + validationMap: savedObjectType.schemas, + }); + + try { + validator.validate(this._migrator.kibanaVersion, doc.attributes); + } catch (error) { + throw SavedObjectsErrorHelpers.createBadRequestError(error.message); + } + } } /** diff --git a/src/core/server/saved_objects/types.ts b/src/core/server/saved_objects/types.ts index 68040d9c6e0038..09cf12f61d2814 100644 --- a/src/core/server/saved_objects/types.ts +++ b/src/core/server/saved_objects/types.ts @@ -12,6 +12,7 @@ import { SavedObjectsTypeMappingDefinition } from './mappings'; import { SavedObjectMigrationMap } from './migrations'; import { SavedObjectsExportTransform } from './export'; import { SavedObjectsImportHook } from './import/types'; +import { SavedObjectsValidationMap } from './validation'; export type { SavedObjectsImportResponse, @@ -28,7 +29,7 @@ export type { SavedObjectsImportWarning, } from './import/types'; -import { SavedObject } from '../../types'; +import { SavedObject, SavedObjectAttributes } from '../../types'; import { ElasticsearchClient } from '../elasticsearch'; type KueryNode = any; @@ -250,11 +251,9 @@ export type SavedObjectsClientContract = Pick { +export interface SavedObjectsType { /** * The name of the type, which is also used as the internal id. */ @@ -291,6 +290,14 @@ export interface SavedObjectsType { * An optional map of {@link SavedObjectMigrationFn | migrations} or a function returning a map of {@link SavedObjectMigrationFn | migrations} to be used to migrate the type. */ migrations?: SavedObjectMigrationMap | (() => SavedObjectMigrationMap); + /** + * An optional schema that can be used to validate the attributes of the type. + * + * When provided, calls to {@link SavedObjectsClient.create | create} will be validated against this schema. + * + * See {@link SavedObjectsValidationMap} for more details. + */ + schemas?: SavedObjectsValidationMap | (() => SavedObjectsValidationMap); /** * If defined, objects of this type will be converted to a 'multiple' or 'multiple-isolated' namespace type when migrating to this * version. diff --git a/src/core/server/saved_objects/validation/index.ts b/src/core/server/saved_objects/validation/index.ts new file mode 100644 index 00000000000000..5c5c9e238fd990 --- /dev/null +++ b/src/core/server/saved_objects/validation/index.ts @@ -0,0 +1,15 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +export { SavedObjectsTypeValidator } from './validator'; +export type { + SavedObjectsValidationMap, + SavedObjectsValidationSpec, + SavedObjectsValidationFunction, +} from './validator'; +export { SavedObjectsValidationError } from './validator_error'; diff --git a/src/core/server/saved_objects/validation/integration_tests/validator.test.ts b/src/core/server/saved_objects/validation/integration_tests/validator.test.ts new file mode 100644 index 00000000000000..0743114585e65e --- /dev/null +++ b/src/core/server/saved_objects/validation/integration_tests/validator.test.ts @@ -0,0 +1,322 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import Path from 'path'; +import Fs from 'fs'; +import Util from 'util'; +import { Env } from '@kbn/config'; +import { schema } from '@kbn/config-schema'; +import { REPO_ROOT } from '@kbn/dev-utils'; +import { SavedObjectsType } from '../../types'; +import { ISavedObjectsRepository } from '../../service/lib'; +import { getEnvOptions } from '../../../config/mocks'; +import { InternalCoreSetup, InternalCoreStart } from '../../../internal_types'; +import { Root } from '../../../root'; +import * as kbnTestServer from '../../../../test_helpers/kbn_server'; + +const kibanaVersion = Env.createDefault(REPO_ROOT, getEnvOptions()).packageInfo.version; +const logFilePath = Path.join(__dirname, 'saved_object_type_validation.log'); + +const asyncUnlink = Util.promisify(Fs.unlink); + +async function removeLogFile() { + // ignore errors if it doesn't exist + await asyncUnlink(logFilePath).catch(() => void 0); +} + +function createRoot() { + return kbnTestServer.createRootWithCorePlugins( + { + migrations: { + skip: false, + }, + logging: { + appenders: { + file: { + type: 'file', + fileName: logFilePath, + layout: { + type: 'json', + }, + }, + }, + loggers: [ + { + name: 'root', + appenders: ['file'], + }, + ], + }, + }, + { + oss: true, + } + ); +} + +// To keep this suite from running too long, we are only setting up ES once and registering +// a handful of SO types to test different scenarios. This means we need to take care when +// adding new tests, as ES will not be cleaned up in between each test run. +const savedObjectTypes: SavedObjectsType[] = [ + { + name: 'schema-using-kbn-config', + hidden: false, + mappings: { + properties: { + a: { type: 'integer' }, + b: { type: 'text' }, + }, + }, + migrations: { + [kibanaVersion]: (doc) => doc, + }, + namespaceType: 'agnostic', + schemas: { + [kibanaVersion]: schema.object({ + a: schema.number(), + b: schema.string(), + }), + }, + }, + { + name: 'schema-using-custom-function', + hidden: false, + mappings: { + properties: { + a: { type: 'integer' }, + b: { type: 'text' }, + }, + }, + migrations: { + [kibanaVersion]: (doc) => doc, + }, + namespaceType: 'agnostic', + schemas: { + [kibanaVersion]: (data) => { + if (typeof data.a !== 'number') { + throw new Error(`[a]: expected value of type [number] but got [${typeof data.a}]`); + } + if (typeof data.b !== 'string') { + throw new Error(`[b]: expected value of type [string] but got [${typeof data.b}]`); + } + }, + }, + }, + { + name: 'no-schema', + hidden: false, + mappings: { + properties: { + a: { type: 'integer' }, + b: { type: 'text' }, + }, + }, + migrations: { + [kibanaVersion]: (doc) => doc, + }, + namespaceType: 'agnostic', + }, + { + name: 'migration-error', + hidden: false, + mappings: { + properties: { + a: { type: 'integer' }, + b: { type: 'text' }, + }, + }, + migrations: { + [kibanaVersion]: (doc) => { + throw new Error('migration error'); // intentionally create a migration error + }, + }, + namespaceType: 'agnostic', + schemas: { + [kibanaVersion]: schema.object({ + a: schema.number(), + b: schema.string(), + }), + }, + }, +]; + +describe('validates saved object types when a schema is provided', () => { + let esServer: kbnTestServer.TestElasticsearchUtils; + let root: Root; + let coreSetup: InternalCoreSetup; + let coreStart: InternalCoreStart; + let savedObjectsClient: ISavedObjectsRepository; + + beforeAll(async () => { + await removeLogFile(); + + const { startES } = kbnTestServer.createTestServers({ + adjustTimeout: (t: number) => jest.setTimeout(t), + settings: { + es: { + license: 'basic', + }, + }, + }); + + root = createRoot(); + esServer = await startES(); + + await root.preboot(); + coreSetup = await root.setup(); + + savedObjectTypes.forEach((type) => coreSetup.savedObjects.registerType(type)); + + coreStart = await root.start(); + savedObjectsClient = coreStart.savedObjects.createInternalRepository(); + }); + + afterAll(async () => { + if (root) { + await root.shutdown(); + } + if (esServer) { + await esServer.stop(); + } + + await new Promise((resolve) => setTimeout(resolve, 10000)); + }); + + it('does nothing when no schema is provided', async () => { + const { attributes } = await savedObjectsClient.create( + 'no-schema', + { + a: 1, + b: 'heya', + }, + { migrationVersion: { bar: '7.16.0' } } + ); + expect(attributes).toEqual( + expect.objectContaining({ + a: 1, + b: 'heya', + }) + ); + }); + + it('is superseded by migration errors and does not run if a migration fails', async () => { + expect(async () => { + await savedObjectsClient.create( + 'migration-error', + { + a: 1, + b: 2, // invalid, would throw validation error if migration didn't fail first + }, + { migrationVersion: { foo: '7.16.0' } } + ); + }).rejects.toThrowErrorMatchingInlineSnapshot( + `"Migration function for version 8.1.0 threw an error"` + ); + }); + + it('returns validation errors with bulkCreate', async () => { + const validObj = { + type: 'schema-using-kbn-config', + id: 'bulk-valid', + attributes: { + a: 1, + b: 'heya', + }, + }; + const invalidObj = { + type: 'schema-using-kbn-config', + id: 'bulk-invalid', + attributes: { + a: 'oops', + b: 'heya', + }, + }; + + // @ts-expect-error - The invalidObj is intentionally malformed for testing + const results = await savedObjectsClient.bulkCreate([validObj, invalidObj]); + + expect(results.saved_objects).toEqual( + expect.arrayContaining([ + expect.objectContaining(validObj), + expect.objectContaining({ + error: new Error('[a]: expected value of type [number] but got [string]: Bad Request'), + id: 'bulk-invalid', + type: 'schema-using-kbn-config', + }), + ]) + ); + }); + + describe('when validating with a config schema', () => { + it('throws when an invalid attribute is provided', async () => { + expect(async () => { + await savedObjectsClient.create( + 'schema-using-kbn-config', + { + a: 1, + b: 2, + }, + { migrationVersion: { foo: '7.16.0' } } + ); + }).rejects.toThrowErrorMatchingInlineSnapshot( + `"[b]: expected value of type [string] but got [number]: Bad Request"` + ); + }); + + it('does not throw when valid attributes are provided', async () => { + const { attributes } = await savedObjectsClient.create( + 'schema-using-kbn-config', + { + a: 1, + b: 'heya', + }, + { migrationVersion: { foo: '7.16.0' } } + ); + expect(attributes).toEqual( + expect.objectContaining({ + a: 1, + b: 'heya', + }) + ); + }); + }); + + describe('when validating with a custom function', () => { + it('throws when an invalid attribute is provided', async () => { + expect(async () => { + await savedObjectsClient.create( + 'schema-using-custom-function', + { + a: 1, + b: 2, + }, + { migrationVersion: { bar: '7.16.0' } } + ); + }).rejects.toThrowErrorMatchingInlineSnapshot( + `"[b]: expected value of type [string] but got [number]: Bad Request"` + ); + }); + + it('does not throw when valid attributes are provided', async () => { + const { attributes } = await savedObjectsClient.create( + 'schema-using-custom-function', + { + a: 1, + b: 'heya', + }, + { migrationVersion: { bar: '7.16.0' } } + ); + expect(attributes).toEqual( + expect.objectContaining({ + a: 1, + b: 'heya', + }) + ); + }); + }); +}); diff --git a/src/core/server/saved_objects/validation/validator.test.ts b/src/core/server/saved_objects/validation/validator.test.ts new file mode 100644 index 00000000000000..6ebfba1b32ea27 --- /dev/null +++ b/src/core/server/saved_objects/validation/validator.test.ts @@ -0,0 +1,87 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { schema } from '@kbn/config-schema'; +import { SavedObjectsTypeValidator, SavedObjectsValidationMap } from './'; + +describe('Saved Objects type validator', () => { + let validator: SavedObjectsTypeValidator; + + const type = 'my-type'; + const validationMap: SavedObjectsValidationMap = { + '1.0.0': (data) => { + if (typeof data.foo !== 'string') { + throw new Error(`[foo]: expected value of type [string] but got [${typeof data.foo}]`); + } + }, + '2.0.0': schema.object({ + foo: schema.string(), + }), + }; + + beforeEach(() => { + validator = new SavedObjectsTypeValidator({ type, validationMap }); + }); + + it('should throw if an invalid mapping is provided', () => { + const malformedValidationMap = { + '1.0.0': ['oops'], + } as unknown as SavedObjectsValidationMap; + validator = new SavedObjectsTypeValidator({ type, validationMap: malformedValidationMap }); + + const data = { foo: false }; + expect(() => validator.validate('1.0.0', data)).toThrowErrorMatchingInlineSnapshot( + `"The 1.0.0 validation for saved object of type [my-type] is malformed."` + ); + }); + + it('should do nothing if no matching validation could be found', () => { + const data = { foo: false }; + expect(validator.validate('3.0.0', data)).toBeUndefined(); + }); + + it('should not allow custom functions to mutate data', () => { + const data = { foo: 'not mutated' }; + + const mutatingValidationMap: SavedObjectsValidationMap = { + '1.0.0': (d) => (d.foo = 'mutated'), + }; + validator = new SavedObjectsTypeValidator({ type, validationMap: mutatingValidationMap }); + + expect(validator.validate('1.0.0', data)).toBeUndefined(); + expect(data.foo).toBe('not mutated'); + }); + + describe('with valid values', () => { + it('should work with a custom function', () => { + const data = { foo: 'hi' }; + expect(() => validator.validate('1.0.0', data)).not.toThrowError(); + }); + + it('should work with a schema', () => { + const data = { foo: 'hi' }; + expect(() => validator.validate('2.0.0', data)).not.toThrowError(); + }); + }); + + describe('with invalid values', () => { + it('should work with a custom function', () => { + const data = { foo: false }; + expect(() => validator.validate('1.0.0', data)).toThrowErrorMatchingInlineSnapshot( + `"[foo]: expected value of type [string] but got [boolean]"` + ); + }); + + it('should work with a schema', () => { + const data = { foo: false }; + expect(() => validator.validate('2.0.0', data)).toThrowErrorMatchingInlineSnapshot( + `"[foo]: expected value of type [string] but got [boolean]"` + ); + }); + }); +}); diff --git a/src/core/server/saved_objects/validation/validator.ts b/src/core/server/saved_objects/validation/validator.ts new file mode 100644 index 00000000000000..77d0a0de00b6e6 --- /dev/null +++ b/src/core/server/saved_objects/validation/validator.ts @@ -0,0 +1,129 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { ValidationError, ObjectType, isConfigSchema } from '@kbn/config-schema'; +import { SavedObjectAttributes } from '../../types'; +import { SavedObjectsValidationError } from './validator_error'; + +/** + * The custom validation function if @kbn/config-schema is not a valid solution for your specific plugin requirements. + * + * @example + * The validation should look something like: + * ```typescript + * const myAttributesValidation: SavedObjectsValidationFunction = (data) => { + * if (typeof data.bar !== 'string') { + * throw new Error(`[bar]: expected value of type [string] but got [${typeof data.bar}]`); + * } + * } + * ``` + * + * @public + */ +export type SavedObjectsValidationFunction< + A extends SavedObjectAttributes = SavedObjectAttributes +> = (data: A) => void; + +/** + * Allowed property validation options: either @kbn/config-schema validations or custom validation functions. + * + * See {@link SavedObjectsValidationFunction} for custom validation. + * + * @public + */ +export type SavedObjectsValidationSpec = + | ObjectType + | SavedObjectsValidationFunction; + +/** + * A map of {@link SavedObjectsValidationSpec | validation specs} to be used for a given type. + * The map's keys must be valid semver versions. + * + * Any time you change the schema of a {@link SavedObjectsType}, you should add a new entry + * to this map for the Kibana version the change was introduced in. + * + * @example + * ```typescript + * const validationMap: SavedObjectValidationMap = { + * '1.0.0': schema.object({ + * foo: schema.string(), + * }), + * '1.1.0': schema.object({ + * foo: schema.oneOf([schema.string(), schema.boolean()]), + * }), + * '2.1.0': (data) => { + * if (typeof data.bar !== 'string') { + * throw new Error(`[bar]: expected value of type [string] but got [${typeof data.bar}]`); + * } + * if (typeof data.foo !== 'string' && typeof data.foo !== 'boolean') { + * throw new Error(`[foo]: expected value of type [string,boolean] but got [${typeof data.foo}]`); + * } + * } + * } + * ``` + * + * @public + */ +export interface SavedObjectsValidationMap< + A extends SavedObjectAttributes = SavedObjectAttributes +> { + [version: string]: SavedObjectsValidationSpec; +} + +/** + * Helper class that takes a {@link SavedObjectsValidationMap} and runs validations for a + * given type based on the provided Kibana version. + * + * @internal + */ +export class SavedObjectsTypeValidator { + private readonly type: string; + private readonly validationMap: SavedObjectsValidationMap; + + constructor({ + type, + validationMap, + }: { + type: string; + validationMap: SavedObjectsValidationMap | (() => SavedObjectsValidationMap); + }) { + this.type = type; + this.validationMap = typeof validationMap === 'function' ? validationMap() : validationMap; + } + + public validate(kibanaVersion: string, data: A): void { + const validationRule = this.validationMap[kibanaVersion]; + if (!validationRule) { + return; // no matching validation rule could be found; proceed without validating + } + + if (isConfigSchema(validationRule)) { + validationRule.validate(data, {}); + } else if (isValidationFunction(validationRule)) { + this.validateFunction(data, validationRule); + } else { + throw new ValidationError( + new SavedObjectsValidationError( + `The ${kibanaVersion} validation for saved object of type [${this.type}] is malformed.` + ) + ); + } + } + + private validateFunction(data: A, validateFn: SavedObjectsValidationFunction) { + try { + validateFn({ ...data }); // shallow clone to prevent mutation + } catch (err) { + throw new ValidationError(new SavedObjectsValidationError(err)); + } + } +} + +function isValidationFunction(fn: any): fn is SavedObjectsValidationFunction { + return typeof fn === 'function'; +} diff --git a/src/core/server/saved_objects/validation/validator_error.ts b/src/core/server/saved_objects/validation/validator_error.ts new file mode 100644 index 00000000000000..6cd5538c153c90 --- /dev/null +++ b/src/core/server/saved_objects/validation/validator_error.ts @@ -0,0 +1,23 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { SchemaTypeError } from '@kbn/config-schema'; + +/** + * Error to return when the validation is not successful. + * @public + */ +export class SavedObjectsValidationError extends SchemaTypeError { + constructor(error: Error | string, path: string[] = []) { + super(error, path); + + // Set the prototype explicitly, see: + // https://github.com/Microsoft/TypeScript/wiki/Breaking-Changes#extending-built-ins-like-error-array-and-map-may-no-longer-work + Object.setPrototypeOf(this, SavedObjectsValidationError.prototype); + } +} From 8833c886f2edf0cf0d307ac2a4fc0f866c4b11e1 Mon Sep 17 00:00:00 2001 From: Luke Elmers Date: Thu, 18 Nov 2021 15:37:11 -0700 Subject: [PATCH 02/10] Update generated docs. --- .../core/server/kibana-plugin-core-server.md | 4 ++ ...in-core-server.savedobjectsservicesetup.md | 2 +- ...r.savedobjectsservicesetup.registertype.md | 7 +++- ...ana-plugin-core-server.savedobjectstype.md | 8 ++-- ...in-core-server.savedobjectstype.schemas.md | 17 +++++++++ ...vedobjectsvalidationerror._constructor_.md | 21 ++++++++++ ...core-server.savedobjectsvalidationerror.md | 21 ++++++++++ ...e-server.savedobjectsvalidationfunction.md | 26 +++++++++++++ ...n-core-server.savedobjectsvalidationmap.md | 38 +++++++++++++++++++ ...-core-server.savedobjectsvalidationspec.md | 15 ++++++++ src/core/server/server.api.md | 22 ++++++++++- 11 files changed, 172 insertions(+), 9 deletions(-) create mode 100644 docs/development/core/server/kibana-plugin-core-server.savedobjectstype.schemas.md create mode 100644 docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationerror._constructor_.md create mode 100644 docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationerror.md create mode 100644 docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationfunction.md create mode 100644 docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationmap.md create mode 100644 docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationspec.md diff --git a/docs/development/core/server/kibana-plugin-core-server.md b/docs/development/core/server/kibana-plugin-core-server.md index 17c2ab736044ed..b575a6a9c64325 100644 --- a/docs/development/core/server/kibana-plugin-core-server.md +++ b/docs/development/core/server/kibana-plugin-core-server.md @@ -31,6 +31,7 @@ The plugin integrates with the core system via lifecycle events: `setup` | [SavedObjectsRepository](./kibana-plugin-core-server.savedobjectsrepository.md) | | | [SavedObjectsSerializer](./kibana-plugin-core-server.savedobjectsserializer.md) | A serializer that can be used to manually convert [raw](./kibana-plugin-core-server.savedobjectsrawdoc.md) or [sanitized](./kibana-plugin-core-server.savedobjectsanitizeddoc.md) documents to the other kind. | | [SavedObjectsUtils](./kibana-plugin-core-server.savedobjectsutils.md) | | +| [SavedObjectsValidationError](./kibana-plugin-core-server.savedobjectsvalidationerror.md) | Error to return when the validation is not successful. | | [SavedObjectTypeRegistry](./kibana-plugin-core-server.savedobjecttyperegistry.md) | Registry holding information about all the registered [saved object types](./kibana-plugin-core-server.savedobjectstype.md). | ## Enumerations @@ -216,6 +217,7 @@ The plugin integrates with the core system via lifecycle events: `setup` | [SavedObjectsUpdateObjectsSpacesResponseObject](./kibana-plugin-core-server.savedobjectsupdateobjectsspacesresponseobject.md) | Details about a specific object's update result. | | [SavedObjectsUpdateOptions](./kibana-plugin-core-server.savedobjectsupdateoptions.md) | | | [SavedObjectsUpdateResponse](./kibana-plugin-core-server.savedobjectsupdateresponse.md) | | +| [SavedObjectsValidationMap](./kibana-plugin-core-server.savedobjectsvalidationmap.md) | A map of [validation specs](./kibana-plugin-core-server.savedobjectsvalidationspec.md) to be used for a given type. The map's keys must be valid semver versions.Any time you change the schema of a [SavedObjectsType](./kibana-plugin-core-server.savedobjectstype.md), you should add a new entry to this map for the Kibana version the change was introduced in. | | [SearchResponse](./kibana-plugin-core-server.searchresponse.md) | | | [ServiceStatus](./kibana-plugin-core-server.servicestatus.md) | The current status of a service at a point in time. | | [SessionCookieValidationResult](./kibana-plugin-core-server.sessioncookievalidationresult.md) | Return type from a function to validate cookie contents. | @@ -318,6 +320,8 @@ The plugin integrates with the core system via lifecycle events: `setup` | [SavedObjectsImportHook](./kibana-plugin-core-server.savedobjectsimporthook.md) | A hook associated with a specific saved object type, that will be invoked during the import process. The hook will have access to the objects of the registered type.Currently, the only supported feature for import hooks is to return warnings to be displayed in the UI when the import succeeds. The only interactions the hook can have with the import process is via the hook's response. Mutating the objects inside the hook's code will have no effect. | | [SavedObjectsImportWarning](./kibana-plugin-core-server.savedobjectsimportwarning.md) | Composite type of all the possible types of import warnings.See [SavedObjectsImportSimpleWarning](./kibana-plugin-core-server.savedobjectsimportsimplewarning.md) and [SavedObjectsImportActionRequiredWarning](./kibana-plugin-core-server.savedobjectsimportactionrequiredwarning.md) for more details. | | [SavedObjectsNamespaceType](./kibana-plugin-core-server.savedobjectsnamespacetype.md) | The namespace type dictates how a saved object can be interacted in relation to namespaces. Each type is mutually exclusive: \* single (default): This type of saved object is namespace-isolated, e.g., it exists in only one namespace. \* multiple: This type of saved object is shareable, e.g., it can exist in one or more namespaces. \* multiple-isolated: This type of saved object is namespace-isolated, e.g., it exists in only one namespace, but object IDs must be unique across all namespaces. This is intended to be an intermediate step when objects with a "single" namespace type are being converted to a "multiple" namespace type. In other words, objects with a "multiple-isolated" namespace type will be \*share-capable\*, but will not actually be shareable until the namespace type is changed to "multiple". \* agnostic: This type of saved object is global. | +| [SavedObjectsValidationFunction](./kibana-plugin-core-server.savedobjectsvalidationfunction.md) | The custom validation function if @kbn/config-schema is not a valid solution for your specific plugin requirements. | +| [SavedObjectsValidationSpec](./kibana-plugin-core-server.savedobjectsvalidationspec.md) | Allowed property validation options: either @kbn/config-schema validations or custom validation functions.See [SavedObjectsValidationFunction](./kibana-plugin-core-server.savedobjectsvalidationfunction.md) for custom validation. | | [SavedObjectTypeExcludeFromUpgradeFilterHook](./kibana-plugin-core-server.savedobjecttypeexcludefromupgradefilterhook.md) | If defined, allows a type to run a search query and return a query filter that may match any documents which may be excluded from the next migration upgrade process. Useful for cleaning up large numbers of old documents which are no longer needed and may slow the migration process.If this hook fails, the migration will proceed without these documents having been filtered out, so this should not be used as a guarantee that these documents have been deleted.Experimental and subject to change | | [SavedObjectUnsanitizedDoc](./kibana-plugin-core-server.savedobjectunsanitizeddoc.md) | Describes Saved Object documents from Kibana < 7.0.0 which don't have a references root property defined. This type should only be used in migrations. | | [ScopeableRequest](./kibana-plugin-core-server.scopeablerequest.md) | A user credentials container. It accommodates the necessary auth credentials to impersonate the current user.See [KibanaRequest](./kibana-plugin-core-server.kibanarequest.md). | diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectsservicesetup.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectsservicesetup.md index 28afaacce7ce63..d9f7b888338fc3 100644 --- a/docs/development/core/server/kibana-plugin-core-server.savedobjectsservicesetup.md +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectsservicesetup.md @@ -51,6 +51,6 @@ export class Plugin() { | --- | --- | --- | | [addClientWrapper](./kibana-plugin-core-server.savedobjectsservicesetup.addclientwrapper.md) | (priority: number, id: string, factory: SavedObjectsClientWrapperFactory) => void | Add a [client wrapper factory](./kibana-plugin-core-server.savedobjectsclientwrapperfactory.md) with the given priority. | | [getKibanaIndex](./kibana-plugin-core-server.savedobjectsservicesetup.getkibanaindex.md) | () => string | Returns the default index used for saved objects. | -| [registerType](./kibana-plugin-core-server.savedobjectsservicesetup.registertype.md) | <Attributes = any>(type: SavedObjectsType<Attributes>) => void | Register a [savedObjects type](./kibana-plugin-core-server.savedobjectstype.md) definition.See the [mappings format](./kibana-plugin-core-server.savedobjectstypemappingdefinition.md) and [migration format](./kibana-plugin-core-server.savedobjectmigrationmap.md) for more details about these. | +| [registerType](./kibana-plugin-core-server.savedobjectsservicesetup.registertype.md) | <Attributes extends SavedObjectAttributes = any>(type: SavedObjectsType<Attributes>) => void | Register a [savedObjects type](./kibana-plugin-core-server.savedobjectstype.md) definition.See the [mappings format](./kibana-plugin-core-server.savedobjectstypemappingdefinition.md) and [migration format](./kibana-plugin-core-server.savedobjectmigrationmap.md) for more details about these. | | [setClientFactoryProvider](./kibana-plugin-core-server.savedobjectsservicesetup.setclientfactoryprovider.md) | (clientFactoryProvider: SavedObjectsClientFactoryProvider) => void | Set the default [factory provider](./kibana-plugin-core-server.savedobjectsclientfactoryprovider.md) for creating Saved Objects clients. Only one provider can be set, subsequent calls to this method will fail. | diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectsservicesetup.registertype.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectsservicesetup.registertype.md index 3085224fdaa6b3..afe3098ef1813b 100644 --- a/docs/development/core/server/kibana-plugin-core-server.savedobjectsservicesetup.registertype.md +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectsservicesetup.registertype.md @@ -11,7 +11,7 @@ See the [mappings format](./kibana-plugin-core-server.savedobjectstypemappingdef Signature: ```typescript -registerType: (type: SavedObjectsType) => void; +registerType: (type: SavedObjectsType) => void; ``` ## Example @@ -21,6 +21,7 @@ registerType: (type: SavedObjectsType) => void; // src/plugins/my_plugin/server/saved_objects/my_type.ts import { SavedObjectsType } from 'src/core/server'; import * as migrations from './migrations'; +import * as schemas from './schemas'; export const myType: SavedObjectsType = { name: 'MyType', @@ -40,6 +41,10 @@ export const myType: SavedObjectsType = { '2.0.0': migrations.migrateToV2, '2.1.0': migrations.migrateToV2_1 }, + schemas: { + '2.0.0': schemas.v2, + '2.1.0': schemas.v2_1, + }, }; // src/plugins/my_plugin/server/plugin.ts diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectstype.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectstype.md index 3c76a898d06f66..be26649dd50acc 100644 --- a/docs/development/core/server/kibana-plugin-core-server.savedobjectstype.md +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectstype.md @@ -4,16 +4,13 @@ ## SavedObjectsType interface + Signature: ```typescript -export interface SavedObjectsType +export interface SavedObjectsType ``` -## Remarks - -This is only internal for now, and will only be public when we expose the registerType API - ## Properties | Property | Type | Description | @@ -57,4 +54,5 @@ Note: migration function(s) can be optionally specified for any of these version | [migrations?](./kibana-plugin-core-server.savedobjectstype.migrations.md) | SavedObjectMigrationMap \| (() => SavedObjectMigrationMap) | (Optional) An optional map of [migrations](./kibana-plugin-core-server.savedobjectmigrationfn.md) or a function returning a map of [migrations](./kibana-plugin-core-server.savedobjectmigrationfn.md) to be used to migrate the type. | | [name](./kibana-plugin-core-server.savedobjectstype.name.md) | string | The name of the type, which is also used as the internal id. | | [namespaceType](./kibana-plugin-core-server.savedobjectstype.namespacetype.md) | SavedObjectsNamespaceType | The [namespace type](./kibana-plugin-core-server.savedobjectsnamespacetype.md) for the type. | +| [schemas?](./kibana-plugin-core-server.savedobjectstype.schemas.md) | SavedObjectsValidationMap<Attributes> \| (() => SavedObjectsValidationMap<Attributes>) | (Optional) An optional schema that can be used to validate the attributes of the type.When provided, calls to [create](./kibana-plugin-core-server.savedobjectsclient.create.md) will be validated against this schema.See [SavedObjectsValidationMap](./kibana-plugin-core-server.savedobjectsvalidationmap.md) for more details. | diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectstype.schemas.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectstype.schemas.md new file mode 100644 index 00000000000000..78fb3d033b2ef0 --- /dev/null +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectstype.schemas.md @@ -0,0 +1,17 @@ + + +[Home](./index.md) > [kibana-plugin-core-server](./kibana-plugin-core-server.md) > [SavedObjectsType](./kibana-plugin-core-server.savedobjectstype.md) > [schemas](./kibana-plugin-core-server.savedobjectstype.schemas.md) + +## SavedObjectsType.schemas property + +An optional schema that can be used to validate the attributes of the type. + +When provided, calls to [create](./kibana-plugin-core-server.savedobjectsclient.create.md) will be validated against this schema. + +See [SavedObjectsValidationMap](./kibana-plugin-core-server.savedobjectsvalidationmap.md) for more details. + +Signature: + +```typescript +schemas?: SavedObjectsValidationMap | (() => SavedObjectsValidationMap); +``` diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationerror._constructor_.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationerror._constructor_.md new file mode 100644 index 00000000000000..c3867cc095a5ee --- /dev/null +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationerror._constructor_.md @@ -0,0 +1,21 @@ + + +[Home](./index.md) > [kibana-plugin-core-server](./kibana-plugin-core-server.md) > [SavedObjectsValidationError](./kibana-plugin-core-server.savedobjectsvalidationerror.md) > [(constructor)](./kibana-plugin-core-server.savedobjectsvalidationerror._constructor_.md) + +## SavedObjectsValidationError.(constructor) + +Constructs a new instance of the `SavedObjectsValidationError` class + +Signature: + +```typescript +constructor(error: Error | string, path?: string[]); +``` + +## Parameters + +| Parameter | Type | Description | +| --- | --- | --- | +| error | Error \| string | | +| path | string\[\] | | + diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationerror.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationerror.md new file mode 100644 index 00000000000000..cc6d0ae8d73e14 --- /dev/null +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationerror.md @@ -0,0 +1,21 @@ + + +[Home](./index.md) > [kibana-plugin-core-server](./kibana-plugin-core-server.md) > [SavedObjectsValidationError](./kibana-plugin-core-server.savedobjectsvalidationerror.md) + +## SavedObjectsValidationError class + +Error to return when the validation is not successful. + +Signature: + +```typescript +export declare class SavedObjectsValidationError extends SchemaTypeError +``` +Extends: SchemaTypeError + +## Constructors + +| Constructor | Modifiers | Description | +| --- | --- | --- | +| [(constructor)(error, path)](./kibana-plugin-core-server.savedobjectsvalidationerror._constructor_.md) | | Constructs a new instance of the SavedObjectsValidationError class | + diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationfunction.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationfunction.md new file mode 100644 index 00000000000000..7fef39f7971f06 --- /dev/null +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationfunction.md @@ -0,0 +1,26 @@ + + +[Home](./index.md) > [kibana-plugin-core-server](./kibana-plugin-core-server.md) > [SavedObjectsValidationFunction](./kibana-plugin-core-server.savedobjectsvalidationfunction.md) + +## SavedObjectsValidationFunction type + +The custom validation function if @kbn/config-schema is not a valid solution for your specific plugin requirements. + +Signature: + +```typescript +export declare type SavedObjectsValidationFunction = (data: A) => void; +``` + +## Example + +The validation should look something like: + +```typescript +const myAttributesValidation: SavedObjectsValidationFunction = (data) => { + if (typeof data.bar !== 'string') { + throw new Error(`[bar]: expected value of type [string] but got [${typeof data.bar}]`); + } +} +``` + diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationmap.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationmap.md new file mode 100644 index 00000000000000..508fcca97019e4 --- /dev/null +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationmap.md @@ -0,0 +1,38 @@ + + +[Home](./index.md) > [kibana-plugin-core-server](./kibana-plugin-core-server.md) > [SavedObjectsValidationMap](./kibana-plugin-core-server.savedobjectsvalidationmap.md) + +## SavedObjectsValidationMap interface + +A map of [validation specs](./kibana-plugin-core-server.savedobjectsvalidationspec.md) to be used for a given type. The map's keys must be valid semver versions. + +Any time you change the schema of a [SavedObjectsType](./kibana-plugin-core-server.savedobjectstype.md), you should add a new entry to this map for the Kibana version the change was introduced in. + +Signature: + +```typescript +export interface SavedObjectsValidationMap +``` + +## Example + + +```typescript +const validationMap: SavedObjectValidationMap = { + '1.0.0': schema.object({ + foo: schema.string(), + }), + '1.1.0': schema.object({ + foo: schema.oneOf([schema.string(), schema.boolean()]), + }), + '2.1.0': (data) => { + if (typeof data.bar !== 'string') { + throw new Error(`[bar]: expected value of type [string] but got [${typeof data.bar}]`); + } + if (typeof data.foo !== 'string' && typeof data.foo !== 'boolean') { + throw new Error(`[foo]: expected value of type [string,boolean] but got [${typeof data.foo}]`); + } + } +} +``` + diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationspec.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationspec.md new file mode 100644 index 00000000000000..b69867d5939e78 --- /dev/null +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationspec.md @@ -0,0 +1,15 @@ + + +[Home](./index.md) > [kibana-plugin-core-server](./kibana-plugin-core-server.md) > [SavedObjectsValidationSpec](./kibana-plugin-core-server.savedobjectsvalidationspec.md) + +## SavedObjectsValidationSpec type + +Allowed property validation options: either @kbn/config-schema validations or custom validation functions. + +See [SavedObjectsValidationFunction](./kibana-plugin-core-server.savedobjectsvalidationfunction.md) for custom validation. + +Signature: + +```typescript +export declare type SavedObjectsValidationSpec = ObjectType | SavedObjectsValidationFunction; +``` diff --git a/src/core/server/server.api.md b/src/core/server/server.api.md index 18d1e479dddee8..2a3adf64f3b5a5 100644 --- a/src/core/server/server.api.md +++ b/src/core/server/server.api.md @@ -2711,7 +2711,7 @@ export class SavedObjectsSerializer { export interface SavedObjectsServiceSetup { addClientWrapper: (priority: number, id: string, factory: SavedObjectsClientWrapperFactory) => void; getKibanaIndex: () => string; - registerType: (type: SavedObjectsType) => void; + registerType: (type: SavedObjectsType) => void; setClientFactoryProvider: (clientFactoryProvider: SavedObjectsClientFactoryProvider) => void; } @@ -2737,7 +2737,7 @@ export interface SavedObjectStatusMeta { } // @public (undocumented) -export interface SavedObjectsType { +export interface SavedObjectsType { convertToAliasScript?: string; convertToMultiNamespaceTypeVersion?: string; excludeOnUpgrade?: SavedObjectTypeExcludeFromUpgradeFilterHook; @@ -2748,6 +2748,7 @@ export interface SavedObjectsType { migrations?: SavedObjectMigrationMap | (() => SavedObjectMigrationMap); name: string; namespaceType: SavedObjectsNamespaceType; + schemas?: SavedObjectsValidationMap | (() => SavedObjectsValidationMap); } // @public @@ -2830,6 +2831,23 @@ export class SavedObjectsUtils { static namespaceStringToId: (namespace: string) => string | undefined; } +// @public +export class SavedObjectsValidationError extends SchemaTypeError { + constructor(error: Error | string, path?: string[]); +} + +// @public +export type SavedObjectsValidationFunction = (data: A) => void; + +// @public +export interface SavedObjectsValidationMap { + // (undocumented) + [version: string]: SavedObjectsValidationSpec; +} + +// @public +export type SavedObjectsValidationSpec = ObjectType | SavedObjectsValidationFunction; + // Warning: (ae-extra-release-tag) The doc comment should not contain more than one release tag // // @public From aa94b0e2e1933bdf06198429932b11b39a8e79b7 Mon Sep 17 00:00:00 2001 From: Luke Elmers Date: Fri, 3 Dec 2021 16:50:40 -0700 Subject: [PATCH 03/10] Address initial feedback. --- .../server/http/router/validator/types.ts | 127 ++++++++++++++++++ .../saved_objects/service/lib/repository.ts | 9 +- src/core/server/saved_objects/types.ts | 6 +- .../server/saved_objects/validation/index.ts | 4 +- .../integration_tests/validator.test.ts | 14 +- .../server/saved_objects/validation/types.ts | 70 ++++++++++ .../validation/validator.test.ts | 68 ++++++---- .../saved_objects/validation/validator.ts | 103 +++----------- 8 files changed, 280 insertions(+), 121 deletions(-) create mode 100644 src/core/server/http/router/validator/types.ts create mode 100644 src/core/server/saved_objects/validation/types.ts diff --git a/src/core/server/http/router/validator/types.ts b/src/core/server/http/router/validator/types.ts new file mode 100644 index 00000000000000..2dac2454d6dcab --- /dev/null +++ b/src/core/server/http/router/validator/types.ts @@ -0,0 +1,127 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { ValidationError, ObjectType, Type, schema, isConfigSchema } from '@kbn/config-schema'; +import { Stream } from 'stream'; +import { RouteValidationError } from './validator_error'; + +/** + * Validation result factory to be used in the custom validation function to return the valid data or validation errors + * + * See {@link RouteValidationFunction}. + * + * @public + */ +export interface RouteValidationResultFactory { + ok: (value: T) => { value: T }; + badRequest: (error: Error | string, path?: string[]) => { error: RouteValidationError }; +} + +/** + * The custom validation function if @kbn/config-schema is not a valid solution for your specific plugin requirements. + * + * @example + * + * The validation should look something like: + * ```typescript + * interface MyExpectedBody { + * bar: string; + * baz: number; + * } + * + * const myBodyValidation: RouteValidationFunction = (data, validationResult) => { + * const { ok, badRequest } = validationResult; + * const { bar, baz } = data || {}; + * if (typeof bar === 'string' && typeof baz === 'number') { + * return ok({ bar, baz }); + * } else { + * return badRequest('Wrong payload', ['body']); + * } + * } + * ``` + * + * @public + */ +export type RouteValidationFunction = ( + data: any, + validationResult: RouteValidationResultFactory +) => + | { + value: T; + error?: never; + } + | { + value?: never; + error: RouteValidationError; + }; + +/** + * Allowed property validation options: either @kbn/config-schema validations or custom validation functions + * + * See {@link RouteValidationFunction} for custom validation. + * + * @public + */ +export type RouteValidationSpec = ObjectType | Type | RouteValidationFunction; + +// Ugly as hell but we need this conditional typing to have proper type inference +type RouteValidationResultType | undefined> = NonNullable< + T extends RouteValidationFunction + ? ReturnType['value'] + : T extends Type + ? T['type'] + : undefined +>; + +/** + * The configuration object to the RouteValidator class. + * Set `params`, `query` and/or `body` to specify the validation logic to follow for that property. + * + * @public + */ +export interface RouteValidatorConfig { + /** + * Validation logic for the URL params + * @public + */ + params?: RouteValidationSpec

; + /** + * Validation logic for the Query params + * @public + */ + query?: RouteValidationSpec; + /** + * Validation logic for the body payload + * @public + */ + body?: RouteValidationSpec; +} + +/** + * Additional options for the RouteValidator class to modify its default behaviour. + * + * @public + */ +export interface RouteValidatorOptions { + /** + * Set the `unsafe` config to avoid running some additional internal *safe* validations on top of your custom validation + * @public + */ + unsafe?: { + params?: boolean; + query?: boolean; + body?: boolean; + }; +} + +/** + * Route validations config and options merged into one object + * @public + */ +export type RouteValidatorFullConfig = RouteValidatorConfig & + RouteValidatorOptions; diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index 432726fdbfffcc..29a92c80252189 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -370,7 +370,7 @@ export class SavedObjectsRepository { * migration to fail, but it's the best we can do without devising a way to run validations * inside the migration algorithm itself. */ - this.validateObjectAttributes(type, migrated as SavedObjectSanitizedDoc); + this.validateObjectAttributes(type, migrated as SavedObjectSanitizedDoc); const raw = this._serializer.savedObjectToRaw(migrated as SavedObjectSanitizedDoc); @@ -542,7 +542,7 @@ export class SavedObjectsRepository { * inside the migration algorithm itself. */ try { - this.validateObjectAttributes(object.type, migrated); + this.validateObjectAttributes(object.type, migrated); } catch (error) { return { tag: 'Left', @@ -2226,19 +2226,20 @@ export class SavedObjectsRepository { } /** Validate a migrated doc against the registered saved object type's schema. */ - private validateObjectAttributes(type: string, doc: SavedObjectSanitizedDoc) { + private validateObjectAttributes(type: string, doc: SavedObjectSanitizedDoc) { const savedObjectType = this._registry.getType(type); if (!savedObjectType?.schemas) { return; } const validator = new SavedObjectsTypeValidator({ + logger: this._logger.get('type-validator'), type, validationMap: savedObjectType.schemas, }); try { - validator.validate(this._migrator.kibanaVersion, doc.attributes); + validator.validate(this._migrator.kibanaVersion, doc); } catch (error) { throw SavedObjectsErrorHelpers.createBadRequestError(error.message); } diff --git a/src/core/server/saved_objects/types.ts b/src/core/server/saved_objects/types.ts index 09cf12f61d2814..5194931ad8a4ef 100644 --- a/src/core/server/saved_objects/types.ts +++ b/src/core/server/saved_objects/types.ts @@ -29,7 +29,7 @@ export type { SavedObjectsImportWarning, } from './import/types'; -import { SavedObject, SavedObjectAttributes } from '../../types'; +import { SavedObject } from '../../types'; import { ElasticsearchClient } from '../elasticsearch'; type KueryNode = any; @@ -253,7 +253,7 @@ export type SavedObjectsNamespaceType = 'single' | 'multiple' | 'multiple-isolat /** * @public */ -export interface SavedObjectsType { +export interface SavedObjectsType { /** * The name of the type, which is also used as the internal id. */ @@ -297,7 +297,7 @@ export interface SavedObjectsType | (() => SavedObjectsValidationMap); + schemas?: SavedObjectsValidationMap | (() => SavedObjectsValidationMap); /** * If defined, objects of this type will be converted to a 'multiple' or 'multiple-isolated' namespace type when migrating to this * version. diff --git a/src/core/server/saved_objects/validation/index.ts b/src/core/server/saved_objects/validation/index.ts index 5c5c9e238fd990..5311e2207250d1 100644 --- a/src/core/server/saved_objects/validation/index.ts +++ b/src/core/server/saved_objects/validation/index.ts @@ -6,10 +6,10 @@ * Side Public License, v 1. */ -export { SavedObjectsTypeValidator } from './validator'; export type { SavedObjectsValidationMap, SavedObjectsValidationSpec, SavedObjectsValidationFunction, -} from './validator'; +} from './types'; +export { SavedObjectsTypeValidator } from './validator'; export { SavedObjectsValidationError } from './validator_error'; diff --git a/src/core/server/saved_objects/validation/integration_tests/validator.test.ts b/src/core/server/saved_objects/validation/integration_tests/validator.test.ts index 0743114585e65e..698368fae495eb 100644 --- a/src/core/server/saved_objects/validation/integration_tests/validator.test.ts +++ b/src/core/server/saved_objects/validation/integration_tests/validator.test.ts @@ -59,6 +59,10 @@ function createRoot() { ); } +function isRecord(obj: unknown): obj is Record { + return typeof obj === 'object' && obj !== null && !Array.isArray(obj); +} + // To keep this suite from running too long, we are only setting up ES once and registering // a handful of SO types to test different scenarios. This means we need to take care when // adding new tests, as ES will not be cleaned up in between each test run. @@ -97,12 +101,12 @@ const savedObjectTypes: SavedObjectsType[] = [ }, namespaceType: 'agnostic', schemas: { - [kibanaVersion]: (data) => { - if (typeof data.a !== 'number') { - throw new Error(`[a]: expected value of type [number] but got [${typeof data.a}]`); + [kibanaVersion]: ({ attributes }) => { + if (isRecord(attributes) && typeof attributes.a !== 'number') { + throw new Error(`[a]: expected value of type [number] but got [${typeof attributes.a}]`); } - if (typeof data.b !== 'string') { - throw new Error(`[b]: expected value of type [string] but got [${typeof data.b}]`); + if (isRecord(attributes) && typeof attributes.b !== 'string') { + throw new Error(`[b]: expected value of type [string] but got [${typeof attributes.b}]`); } }, }, diff --git a/src/core/server/saved_objects/validation/types.ts b/src/core/server/saved_objects/validation/types.ts new file mode 100644 index 00000000000000..86215bd956d17f --- /dev/null +++ b/src/core/server/saved_objects/validation/types.ts @@ -0,0 +1,70 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { ObjectType } from '@kbn/config-schema'; + +/** + * The custom validation function if @kbn/config-schema is not a valid solution for your specific plugin requirements. + * + * Be careful not to mutate the provided attributes. + * + * @example + * The validation should look something like: + * ```typescript + * const myAttributesValidation: SavedObjectsValidationFunction = ({ attributes }) => { + * if (typeof attributes.bar !== 'string') { + * throw new Error(`[bar]: expected value of type [string] but got [${typeof attributes.bar}]`); + * } + * } + * ``` + * + * @public + */ +export type SavedObjectsValidationFunction = (data: { attributes: unknown }) => void; + +/** + * Allowed property validation options: either @kbn/config-schema validations or custom validation functions. + * + * See {@link SavedObjectsValidationFunction} for custom validation. + * + * @public + */ +export type SavedObjectsValidationSpec = ObjectType | SavedObjectsValidationFunction; + +/** + * A map of {@link SavedObjectsValidationSpec | validation specs} to be used for a given type. + * The map's keys must be valid semver versions. + * + * Any time you change the schema of a {@link SavedObjectsType}, you should add a new entry + * to this map for the Kibana version the change was introduced in. + * + * @example + * ```typescript + * const validationMap: SavedObjectValidationMap = { + * '1.0.0': schema.object({ + * foo: schema.string(), + * }), + * '1.1.0': schema.object({ + * foo: schema.oneOf([schema.string(), schema.boolean()]), + * }), + * '2.1.0': ({ attributes }) => { + * if (typeof attributes.bar !== 'string') { + * throw new Error(`[bar]: expected value of type [string] but got [${typeof data.bar}]`); + * } + * if (typeof attributes.foo !== 'string' && typeof attributes.foo !== 'boolean') { + * throw new Error(`[foo]: expected value of type [string,boolean] but got [${typeof attributes.foo}]`); + * } + * } + * } + * ``` + * + * @public + */ +export interface SavedObjectsValidationMap { + [version: string]: SavedObjectsValidationSpec; +} diff --git a/src/core/server/saved_objects/validation/validator.test.ts b/src/core/server/saved_objects/validation/validator.test.ts index 6ebfba1b32ea27..9af959f903caff 100644 --- a/src/core/server/saved_objects/validation/validator.test.ts +++ b/src/core/server/saved_objects/validation/validator.test.ts @@ -8,15 +8,24 @@ import { schema } from '@kbn/config-schema'; import { SavedObjectsTypeValidator, SavedObjectsValidationMap } from './'; +import { SavedObjectSanitizedDoc } from '../serialization'; +import { loggerMock, MockedLogger } from '../../logging/logger.mock'; + +function isRecord(obj: unknown): obj is Record { + return typeof obj === 'object' && obj !== null && !Array.isArray(obj); +} describe('Saved Objects type validator', () => { let validator: SavedObjectsTypeValidator; + let logger: MockedLogger; const type = 'my-type'; const validationMap: SavedObjectsValidationMap = { - '1.0.0': (data) => { - if (typeof data.foo !== 'string') { - throw new Error(`[foo]: expected value of type [string] but got [${typeof data.foo}]`); + '1.0.0': ({ attributes }) => { + if (isRecord(attributes) && typeof attributes.foo !== 'string') { + throw new Error( + `[foo]: expected value of type [string] but got [${typeof attributes.foo}]` + ); } }, '2.0.0': schema.object({ @@ -24,61 +33,70 @@ describe('Saved Objects type validator', () => { }), }; + const createMockObject = (attributes: Record): SavedObjectSanitizedDoc => ({ + attributes, + id: 'test-id', + references: [], + type, + }); + beforeEach(() => { - validator = new SavedObjectsTypeValidator({ type, validationMap }); + logger = loggerMock.create(); + validator = new SavedObjectsTypeValidator({ logger, type, validationMap }); + }); + + afterEach(() => { + jest.clearAllMocks(); }); - it('should throw if an invalid mapping is provided', () => { + it('should silently skip validation if an invalid mapping is provided', () => { const malformedValidationMap = { '1.0.0': ['oops'], } as unknown as SavedObjectsValidationMap; - validator = new SavedObjectsTypeValidator({ type, validationMap: malformedValidationMap }); + validator = new SavedObjectsTypeValidator({ + logger, + type, + validationMap: malformedValidationMap, + }); - const data = { foo: false }; - expect(() => validator.validate('1.0.0', data)).toThrowErrorMatchingInlineSnapshot( - `"The 1.0.0 validation for saved object of type [my-type] is malformed."` - ); + const data = createMockObject({ foo: false }); + expect(() => validator.validate('1.0.0', data)).not.toThrowError(); }); it('should do nothing if no matching validation could be found', () => { - const data = { foo: false }; + const data = createMockObject({ foo: false }); expect(validator.validate('3.0.0', data)).toBeUndefined(); + expect(logger.debug).not.toHaveBeenCalled(); }); - it('should not allow custom functions to mutate data', () => { - const data = { foo: 'not mutated' }; - - const mutatingValidationMap: SavedObjectsValidationMap = { - '1.0.0': (d) => (d.foo = 'mutated'), - }; - validator = new SavedObjectsTypeValidator({ type, validationMap: mutatingValidationMap }); - - expect(validator.validate('1.0.0', data)).toBeUndefined(); - expect(data.foo).toBe('not mutated'); + it('should log when attempting to perform validation', () => { + const data = createMockObject({ foo: 'hi' }); + validator.validate('1.0.0', data); + expect(logger.debug).toHaveBeenCalledTimes(1); }); describe('with valid values', () => { it('should work with a custom function', () => { - const data = { foo: 'hi' }; + const data = createMockObject({ foo: 'hi' }); expect(() => validator.validate('1.0.0', data)).not.toThrowError(); }); it('should work with a schema', () => { - const data = { foo: 'hi' }; + const data = createMockObject({ foo: 'hi' }); expect(() => validator.validate('2.0.0', data)).not.toThrowError(); }); }); describe('with invalid values', () => { it('should work with a custom function', () => { - const data = { foo: false }; + const data = createMockObject({ foo: false }); expect(() => validator.validate('1.0.0', data)).toThrowErrorMatchingInlineSnapshot( `"[foo]: expected value of type [string] but got [boolean]"` ); }); it('should work with a schema', () => { - const data = { foo: false }; + const data = createMockObject({ foo: false }); expect(() => validator.validate('2.0.0', data)).toThrowErrorMatchingInlineSnapshot( `"[foo]: expected value of type [string] but got [boolean]"` ); diff --git a/src/core/server/saved_objects/validation/validator.ts b/src/core/server/saved_objects/validation/validator.ts index 77d0a0de00b6e6..f168f69f05444f 100644 --- a/src/core/server/saved_objects/validation/validator.ts +++ b/src/core/server/saved_objects/validation/validator.ts @@ -6,74 +6,11 @@ * Side Public License, v 1. */ -import { ValidationError, ObjectType, isConfigSchema } from '@kbn/config-schema'; -import { SavedObjectAttributes } from '../../types'; +import { ValidationError, isConfigSchema } from '@kbn/config-schema'; import { SavedObjectsValidationError } from './validator_error'; - -/** - * The custom validation function if @kbn/config-schema is not a valid solution for your specific plugin requirements. - * - * @example - * The validation should look something like: - * ```typescript - * const myAttributesValidation: SavedObjectsValidationFunction = (data) => { - * if (typeof data.bar !== 'string') { - * throw new Error(`[bar]: expected value of type [string] but got [${typeof data.bar}]`); - * } - * } - * ``` - * - * @public - */ -export type SavedObjectsValidationFunction< - A extends SavedObjectAttributes = SavedObjectAttributes -> = (data: A) => void; - -/** - * Allowed property validation options: either @kbn/config-schema validations or custom validation functions. - * - * See {@link SavedObjectsValidationFunction} for custom validation. - * - * @public - */ -export type SavedObjectsValidationSpec = - | ObjectType - | SavedObjectsValidationFunction; - -/** - * A map of {@link SavedObjectsValidationSpec | validation specs} to be used for a given type. - * The map's keys must be valid semver versions. - * - * Any time you change the schema of a {@link SavedObjectsType}, you should add a new entry - * to this map for the Kibana version the change was introduced in. - * - * @example - * ```typescript - * const validationMap: SavedObjectValidationMap = { - * '1.0.0': schema.object({ - * foo: schema.string(), - * }), - * '1.1.0': schema.object({ - * foo: schema.oneOf([schema.string(), schema.boolean()]), - * }), - * '2.1.0': (data) => { - * if (typeof data.bar !== 'string') { - * throw new Error(`[bar]: expected value of type [string] but got [${typeof data.bar}]`); - * } - * if (typeof data.foo !== 'string' && typeof data.foo !== 'boolean') { - * throw new Error(`[foo]: expected value of type [string,boolean] but got [${typeof data.foo}]`); - * } - * } - * } - * ``` - * - * @public - */ -export interface SavedObjectsValidationMap< - A extends SavedObjectAttributes = SavedObjectAttributes -> { - [version: string]: SavedObjectsValidationSpec; -} +import { SavedObjectsValidationMap, SavedObjectsValidationFunction } from './types'; +import { SavedObjectSanitizedDoc } from '../serialization'; +import { Logger } from '../../logging'; /** * Helper class that takes a {@link SavedObjectsValidationMap} and runs validations for a @@ -81,49 +18,51 @@ export interface SavedObjectsValidationMap< * * @internal */ -export class SavedObjectsTypeValidator { +export class SavedObjectsTypeValidator { + private readonly log: Logger; private readonly type: string; - private readonly validationMap: SavedObjectsValidationMap; + private readonly validationMap: SavedObjectsValidationMap; constructor({ + logger, type, validationMap, }: { + logger: Logger; type: string; - validationMap: SavedObjectsValidationMap | (() => SavedObjectsValidationMap); + validationMap: SavedObjectsValidationMap | (() => SavedObjectsValidationMap); }) { + this.log = logger; this.type = type; this.validationMap = typeof validationMap === 'function' ? validationMap() : validationMap; } - public validate(kibanaVersion: string, data: A): void { - const validationRule = this.validationMap[kibanaVersion]; + public validate(objectVersion: string, data: SavedObjectSanitizedDoc): void { + const validationRule = this.validationMap[objectVersion]; if (!validationRule) { return; // no matching validation rule could be found; proceed without validating } + this.log.debug(`Validating object of type [${this.type}] against version [${objectVersion}]`); if (isConfigSchema(validationRule)) { - validationRule.validate(data, {}); + validationRule.validate(data.attributes); } else if (isValidationFunction(validationRule)) { this.validateFunction(data, validationRule); - } else { - throw new ValidationError( - new SavedObjectsValidationError( - `The ${kibanaVersion} validation for saved object of type [${this.type}] is malformed.` - ) - ); } } - private validateFunction(data: A, validateFn: SavedObjectsValidationFunction) { + private validateFunction( + data: SavedObjectSanitizedDoc, + validateFn: SavedObjectsValidationFunction + ) { try { - validateFn({ ...data }); // shallow clone to prevent mutation + validateFn({ attributes: data.attributes }); } catch (err) { throw new ValidationError(new SavedObjectsValidationError(err)); } } } -function isValidationFunction(fn: any): fn is SavedObjectsValidationFunction { +function isValidationFunction(fn: unknown): fn is SavedObjectsValidationFunction { return typeof fn === 'function'; } From aa4375543c8621572c50321047d1c551ec06eed1 Mon Sep 17 00:00:00 2001 From: Luke Elmers Date: Fri, 10 Dec 2021 14:32:08 -0700 Subject: [PATCH 04/10] Add validation for root-level properties. --- .../service/lib/repository.test.ts | 4 +- .../integration_tests/validator.test.ts | 16 ++++-- .../saved_objects/validation/schema.test.ts | 51 +++++++++++++++++++ .../server/saved_objects/validation/schema.ts | 49 ++++++++++++++++++ .../validation/validator.test.ts | 36 ++++++++++++- .../saved_objects/validation/validator.ts | 10 ++-- 6 files changed, 154 insertions(+), 12 deletions(-) create mode 100644 src/core/server/saved_objects/validation/schema.test.ts create mode 100644 src/core/server/saved_objects/validation/schema.ts diff --git a/src/core/server/saved_objects/service/lib/repository.test.ts b/src/core/server/saved_objects/service/lib/repository.test.ts index 43b424a240ec37..4a6848810ffddd 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.ts +++ b/src/core/server/saved_objects/service/lib/repository.test.ts @@ -999,7 +999,7 @@ describe('SavedObjectsRepository', () => { expect.objectContaining(obj3), expect.objectContaining({ error: new Error( - '[title]: expected value of type [string] but got [number]: Bad Request' + '[attributes.title]: expected value of type [string] but got [number]: Bad Request' ), id: 'three-again', type: 'dashboard', @@ -2570,7 +2570,7 @@ describe('SavedObjectsRepository', () => { await expect( savedObjectsRepository.create('dashboard', { title: 123 }) ).rejects.toThrowErrorMatchingInlineSnapshot( - `"[title]: expected value of type [string] but got [number]: Bad Request"` + `"[attributes.title]: expected value of type [string] but got [number]: Bad Request"` ); expect(client.create).not.toHaveBeenCalled(); }); diff --git a/src/core/server/saved_objects/validation/integration_tests/validator.test.ts b/src/core/server/saved_objects/validation/integration_tests/validator.test.ts index 698368fae495eb..4ad20390d0c653 100644 --- a/src/core/server/saved_objects/validation/integration_tests/validator.test.ts +++ b/src/core/server/saved_objects/validation/integration_tests/validator.test.ts @@ -103,10 +103,14 @@ const savedObjectTypes: SavedObjectsType[] = [ schemas: { [kibanaVersion]: ({ attributes }) => { if (isRecord(attributes) && typeof attributes.a !== 'number') { - throw new Error(`[a]: expected value of type [number] but got [${typeof attributes.a}]`); + throw new Error( + `[attributes.a]: expected value of type [number] but got [${typeof attributes.a}]` + ); } if (isRecord(attributes) && typeof attributes.b !== 'string') { - throw new Error(`[b]: expected value of type [string] but got [${typeof attributes.b}]`); + throw new Error( + `[attributes.b]: expected value of type [string] but got [${typeof attributes.b}]` + ); } }, }, @@ -248,7 +252,9 @@ describe('validates saved object types when a schema is provided', () => { expect.arrayContaining([ expect.objectContaining(validObj), expect.objectContaining({ - error: new Error('[a]: expected value of type [number] but got [string]: Bad Request'), + error: new Error( + '[attributes.a]: expected value of type [number] but got [string]: Bad Request' + ), id: 'bulk-invalid', type: 'schema-using-kbn-config', }), @@ -268,7 +274,7 @@ describe('validates saved object types when a schema is provided', () => { { migrationVersion: { foo: '7.16.0' } } ); }).rejects.toThrowErrorMatchingInlineSnapshot( - `"[b]: expected value of type [string] but got [number]: Bad Request"` + `"[attributes.b]: expected value of type [string] but got [number]: Bad Request"` ); }); @@ -302,7 +308,7 @@ describe('validates saved object types when a schema is provided', () => { { migrationVersion: { bar: '7.16.0' } } ); }).rejects.toThrowErrorMatchingInlineSnapshot( - `"[b]: expected value of type [string] but got [number]: Bad Request"` + `"[attributes.b]: expected value of type [string] but got [number]: Bad Request"` ); }); diff --git a/src/core/server/saved_objects/validation/schema.test.ts b/src/core/server/saved_objects/validation/schema.test.ts new file mode 100644 index 00000000000000..2e9b5c30d1a3a0 --- /dev/null +++ b/src/core/server/saved_objects/validation/schema.test.ts @@ -0,0 +1,51 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { schema } from '@kbn/config-schema'; +import { SavedObjectsValidationMap } from './'; +import { SavedObjectSanitizedDoc } from '../serialization'; +import { createSavedObjectSanitizedDocSchema } from './schema'; + +describe('Saved Objects type validation schema', () => { + const type = 'my-type'; + const validationMap: SavedObjectsValidationMap = { + '1.0.0': (data) => data, + '2.0.0': schema.object({ + foo: schema.string(), + }), + }; + + const createMockObject = (attributes: unknown): SavedObjectSanitizedDoc => ({ + attributes, + id: 'test-id', + references: [], + type, + }); + + it('should use generic validation on attributes if a config schema is not provided', () => { + const objectSchema = createSavedObjectSanitizedDocSchema(validationMap['1.0.0']); + const data = createMockObject({ foo: false }); + expect(() => objectSchema.validate(data)).not.toThrowError(); + }); + + it('should fail if attributes do not match generic validation', () => { + const objectSchema = createSavedObjectSanitizedDocSchema(validationMap['2.0.0']); + const data = createMockObject('oops'); + expect(() => objectSchema.validate(data)).toThrowErrorMatchingInlineSnapshot( + `"[attributes]: could not parse object value from json input"` + ); + }); + + it('should use config schema if provided', () => { + const objectSchema = createSavedObjectSanitizedDocSchema(validationMap['2.0.0']); + const data = createMockObject({ foo: false }); + expect(() => objectSchema.validate(data)).toThrowErrorMatchingInlineSnapshot( + `"[attributes.foo]: expected value of type [string] but got [boolean]"` + ); + }); +}); diff --git a/src/core/server/saved_objects/validation/schema.ts b/src/core/server/saved_objects/validation/schema.ts new file mode 100644 index 00000000000000..b5fca3ecaff72a --- /dev/null +++ b/src/core/server/saved_objects/validation/schema.ts @@ -0,0 +1,49 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { isConfigSchema, schema, Type } from '@kbn/config-schema'; +import { SavedObjectsValidationSpec } from './types'; +import { SavedObjectSanitizedDoc } from '../serialization'; + +// We convert `SavedObjectSanitizedDoc` to its validation schema representation +// to ensure that we don't forget to keep the schema up-to-date. TS will complain +// if we update `SavedObjectSanitizedDoc` without making changes below. +type SavedObjectSanitizedDocSchema = { + [K in keyof Required]: Type; +}; + +/** + * Returns a validation schema representing a {@link SavedObjectSanitizedDoc}. + * + * If a {@link SavedObjectsValidationSpec} is provided and the spec is a + * config schema, that validation will be used for the object's `attributes`. + * Otherwise, it falls back to a more generic validation. + * + * @internal + */ +export const createSavedObjectSanitizedDocSchema = (rule?: SavedObjectsValidationSpec) => + schema.object({ + attributes: isConfigSchema(rule) ? rule : schema.object({}, { unknowns: 'allow' }), + id: schema.string(), + type: schema.string(), + references: schema.arrayOf( + schema.object({ + name: schema.string(), + type: schema.string(), + id: schema.string(), + }), + { defaultValue: [] } + ), + namespace: schema.maybe(schema.string()), + namespaces: schema.maybe(schema.arrayOf(schema.string())), + migrationVersion: schema.maybe(schema.object({}, { unknowns: 'allow' })), + coreMigrationVersion: schema.maybe(schema.string()), + updated_at: schema.maybe(schema.string()), + version: schema.maybe(schema.string()), + originId: schema.maybe(schema.string()), + }); diff --git a/src/core/server/saved_objects/validation/validator.test.ts b/src/core/server/saved_objects/validation/validator.test.ts index 9af959f903caff..40e272764a84bd 100644 --- a/src/core/server/saved_objects/validation/validator.test.ts +++ b/src/core/server/saved_objects/validation/validator.test.ts @@ -98,8 +98,42 @@ describe('Saved Objects type validator', () => { it('should work with a schema', () => { const data = createMockObject({ foo: false }); expect(() => validator.validate('2.0.0', data)).toThrowErrorMatchingInlineSnapshot( - `"[foo]: expected value of type [string] but got [boolean]"` + `"[attributes.foo]: expected value of type [string] but got [boolean]"` + ); + }); + + it('should create an error if fields other than attributes are malformed', () => { + const data = createMockObject({ foo: 'hi' }); + // @ts-expect-error Intentionally malformed object + data.updated_at = false; + expect(() => validator.validate('1.0.0', data)).toThrowErrorMatchingInlineSnapshot( + `"[updated_at]: expected value of type [string] but got [boolean]"` ); + expect(() => validator.validate('2.0.0', data)).toThrowErrorMatchingInlineSnapshot( + `"[updated_at]: expected value of type [string] but got [boolean]"` + ); + }); + }); + + describe('when the validation map is a function', () => { + const fnValidationMap: SavedObjectsValidationMap = { + '1.0.0': () => validationMap['1.0.0'], + '2.0.0': () => validationMap['2.0.0'], + }; + validator = new SavedObjectsTypeValidator({ + logger, + type, + validationMap: fnValidationMap, + }); + + it('should work with a custom function', () => { + const data = createMockObject({ foo: 'hi' }); + expect(() => validator.validate('1.0.0', data)).not.toThrowError(); + }); + + it('should work with a schema', () => { + const data = createMockObject({ foo: 'hi' }); + expect(() => validator.validate('2.0.0', data)).not.toThrowError(); }); }); }); diff --git a/src/core/server/saved_objects/validation/validator.ts b/src/core/server/saved_objects/validation/validator.ts index f168f69f05444f..ed839ab25ae8d9 100644 --- a/src/core/server/saved_objects/validation/validator.ts +++ b/src/core/server/saved_objects/validation/validator.ts @@ -7,8 +7,9 @@ */ import { ValidationError, isConfigSchema } from '@kbn/config-schema'; -import { SavedObjectsValidationError } from './validator_error'; +import { createSavedObjectSanitizedDocSchema } from './schema'; import { SavedObjectsValidationMap, SavedObjectsValidationFunction } from './types'; +import { SavedObjectsValidationError } from './validator_error'; import { SavedObjectSanitizedDoc } from '../serialization'; import { Logger } from '../../logging'; @@ -44,9 +45,10 @@ export class SavedObjectsTypeValidator { } this.log.debug(`Validating object of type [${this.type}] against version [${objectVersion}]`); - if (isConfigSchema(validationRule)) { - validationRule.validate(data.attributes); - } else if (isValidationFunction(validationRule)) { + const validationSchema = createSavedObjectSanitizedDocSchema(validationRule); + validationSchema.validate(data); + + if (!isConfigSchema(validationRule) && isValidationFunction(validationRule)) { this.validateFunction(data, validationRule); } } From dff697e4fa8c1224e13e6b3c8d68d47ee8517105 Mon Sep 17 00:00:00 2001 From: Luke Elmers Date: Mon, 13 Dec 2021 15:07:18 -0700 Subject: [PATCH 05/10] Remove busted file that was accidentally committed. --- .../server/http/router/validator/types.ts | 127 ------------------ 1 file changed, 127 deletions(-) delete mode 100644 src/core/server/http/router/validator/types.ts diff --git a/src/core/server/http/router/validator/types.ts b/src/core/server/http/router/validator/types.ts deleted file mode 100644 index 2dac2454d6dcab..00000000000000 --- a/src/core/server/http/router/validator/types.ts +++ /dev/null @@ -1,127 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0 and the Server Side Public License, v 1; you may not use this file except - * in compliance with, at your election, the Elastic License 2.0 or the Server - * Side Public License, v 1. - */ - -import { ValidationError, ObjectType, Type, schema, isConfigSchema } from '@kbn/config-schema'; -import { Stream } from 'stream'; -import { RouteValidationError } from './validator_error'; - -/** - * Validation result factory to be used in the custom validation function to return the valid data or validation errors - * - * See {@link RouteValidationFunction}. - * - * @public - */ -export interface RouteValidationResultFactory { - ok: (value: T) => { value: T }; - badRequest: (error: Error | string, path?: string[]) => { error: RouteValidationError }; -} - -/** - * The custom validation function if @kbn/config-schema is not a valid solution for your specific plugin requirements. - * - * @example - * - * The validation should look something like: - * ```typescript - * interface MyExpectedBody { - * bar: string; - * baz: number; - * } - * - * const myBodyValidation: RouteValidationFunction = (data, validationResult) => { - * const { ok, badRequest } = validationResult; - * const { bar, baz } = data || {}; - * if (typeof bar === 'string' && typeof baz === 'number') { - * return ok({ bar, baz }); - * } else { - * return badRequest('Wrong payload', ['body']); - * } - * } - * ``` - * - * @public - */ -export type RouteValidationFunction = ( - data: any, - validationResult: RouteValidationResultFactory -) => - | { - value: T; - error?: never; - } - | { - value?: never; - error: RouteValidationError; - }; - -/** - * Allowed property validation options: either @kbn/config-schema validations or custom validation functions - * - * See {@link RouteValidationFunction} for custom validation. - * - * @public - */ -export type RouteValidationSpec = ObjectType | Type | RouteValidationFunction; - -// Ugly as hell but we need this conditional typing to have proper type inference -type RouteValidationResultType | undefined> = NonNullable< - T extends RouteValidationFunction - ? ReturnType['value'] - : T extends Type - ? T['type'] - : undefined ->; - -/** - * The configuration object to the RouteValidator class. - * Set `params`, `query` and/or `body` to specify the validation logic to follow for that property. - * - * @public - */ -export interface RouteValidatorConfig { - /** - * Validation logic for the URL params - * @public - */ - params?: RouteValidationSpec

; - /** - * Validation logic for the Query params - * @public - */ - query?: RouteValidationSpec; - /** - * Validation logic for the body payload - * @public - */ - body?: RouteValidationSpec; -} - -/** - * Additional options for the RouteValidator class to modify its default behaviour. - * - * @public - */ -export interface RouteValidatorOptions { - /** - * Set the `unsafe` config to avoid running some additional internal *safe* validations on top of your custom validation - * @public - */ - unsafe?: { - params?: boolean; - query?: boolean; - body?: boolean; - }; -} - -/** - * Route validations config and options merged into one object - * @public - */ -export type RouteValidatorFullConfig = RouteValidatorConfig & - RouteValidatorOptions; From cd7843044b0c773500dfdd97255b3a5ac5b5e969 Mon Sep 17 00:00:00 2001 From: Luke Elmers Date: Mon, 13 Dec 2021 15:07:36 -0700 Subject: [PATCH 06/10] Fix broken import. --- .../validation/integration_tests/validator.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/server/saved_objects/validation/integration_tests/validator.test.ts b/src/core/server/saved_objects/validation/integration_tests/validator.test.ts index 4ad20390d0c653..23fb0e69173658 100644 --- a/src/core/server/saved_objects/validation/integration_tests/validator.test.ts +++ b/src/core/server/saved_objects/validation/integration_tests/validator.test.ts @@ -11,7 +11,7 @@ import Fs from 'fs'; import Util from 'util'; import { Env } from '@kbn/config'; import { schema } from '@kbn/config-schema'; -import { REPO_ROOT } from '@kbn/dev-utils'; +import { REPO_ROOT } from '@kbn/utils'; import { SavedObjectsType } from '../../types'; import { ISavedObjectsRepository } from '../../service/lib'; import { getEnvOptions } from '../../../config/mocks'; From c593b1f33f4a3b81da8d719677e3a87fc714e87c Mon Sep 17 00:00:00 2001 From: Luke Elmers Date: Mon, 13 Dec 2021 15:07:52 -0700 Subject: [PATCH 07/10] Update generated docs. --- .../core/server/kibana-plugin-core-server.md | 2 +- .../kibana-plugin-core-server.savedobjectstype.md | 4 ++-- ...-plugin-core-server.savedobjectstype.schemas.md | 2 +- ...n-core-server.savedobjectsvalidationfunction.md | 12 ++++++++---- ...plugin-core-server.savedobjectsvalidationmap.md | 10 +++++----- ...lugin-core-server.savedobjectsvalidationspec.md | 2 +- src/core/server/server.api.md | 14 ++++++++------ 7 files changed, 26 insertions(+), 20 deletions(-) diff --git a/docs/development/core/server/kibana-plugin-core-server.md b/docs/development/core/server/kibana-plugin-core-server.md index 667d056af0a554..59e63eab323134 100644 --- a/docs/development/core/server/kibana-plugin-core-server.md +++ b/docs/development/core/server/kibana-plugin-core-server.md @@ -322,7 +322,7 @@ The plugin integrates with the core system via lifecycle events: `setup` | [SavedObjectsImportHook](./kibana-plugin-core-server.savedobjectsimporthook.md) | A hook associated with a specific saved object type, that will be invoked during the import process. The hook will have access to the objects of the registered type.Currently, the only supported feature for import hooks is to return warnings to be displayed in the UI when the import succeeds. The only interactions the hook can have with the import process is via the hook's response. Mutating the objects inside the hook's code will have no effect. | | [SavedObjectsImportWarning](./kibana-plugin-core-server.savedobjectsimportwarning.md) | Composite type of all the possible types of import warnings.See [SavedObjectsImportSimpleWarning](./kibana-plugin-core-server.savedobjectsimportsimplewarning.md) and [SavedObjectsImportActionRequiredWarning](./kibana-plugin-core-server.savedobjectsimportactionrequiredwarning.md) for more details. | | [SavedObjectsNamespaceType](./kibana-plugin-core-server.savedobjectsnamespacetype.md) | The namespace type dictates how a saved object can be interacted in relation to namespaces. Each type is mutually exclusive: \* single (default): This type of saved object is namespace-isolated, e.g., it exists in only one namespace. \* multiple: This type of saved object is shareable, e.g., it can exist in one or more namespaces. \* multiple-isolated: This type of saved object is namespace-isolated, e.g., it exists in only one namespace, but object IDs must be unique across all namespaces. This is intended to be an intermediate step when objects with a "single" namespace type are being converted to a "multiple" namespace type. In other words, objects with a "multiple-isolated" namespace type will be \*share-capable\*, but will not actually be shareable until the namespace type is changed to "multiple". \* agnostic: This type of saved object is global. | -| [SavedObjectsValidationFunction](./kibana-plugin-core-server.savedobjectsvalidationfunction.md) | The custom validation function if @kbn/config-schema is not a valid solution for your specific plugin requirements. | +| [SavedObjectsValidationFunction](./kibana-plugin-core-server.savedobjectsvalidationfunction.md) | The custom validation function if @kbn/config-schema is not a valid solution for your specific plugin requirements.Be careful not to mutate the provided attributes. | | [SavedObjectsValidationSpec](./kibana-plugin-core-server.savedobjectsvalidationspec.md) | Allowed property validation options: either @kbn/config-schema validations or custom validation functions.See [SavedObjectsValidationFunction](./kibana-plugin-core-server.savedobjectsvalidationfunction.md) for custom validation. | | [SavedObjectTypeExcludeFromUpgradeFilterHook](./kibana-plugin-core-server.savedobjecttypeexcludefromupgradefilterhook.md) | If defined, allows a type to run a search query and return a query filter that may match any documents which may be excluded from the next migration upgrade process. Useful for cleaning up large numbers of old documents which are no longer needed and may slow the migration process.If this hook fails, the migration will proceed without these documents having been filtered out, so this should not be used as a guarantee that these documents have been deleted.Experimental and subject to change | | [SavedObjectUnsanitizedDoc](./kibana-plugin-core-server.savedobjectunsanitizeddoc.md) | Describes Saved Object documents from Kibana < 7.0.0 which don't have a references root property defined. This type should only be used in migrations. | diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectstype.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectstype.md index be26649dd50acc..cd6e724685777a 100644 --- a/docs/development/core/server/kibana-plugin-core-server.savedobjectstype.md +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectstype.md @@ -8,7 +8,7 @@ Signature: ```typescript -export interface SavedObjectsType +export interface SavedObjectsType ``` ## Properties @@ -54,5 +54,5 @@ Note: migration function(s) can be optionally specified for any of these version | [migrations?](./kibana-plugin-core-server.savedobjectstype.migrations.md) | SavedObjectMigrationMap \| (() => SavedObjectMigrationMap) | (Optional) An optional map of [migrations](./kibana-plugin-core-server.savedobjectmigrationfn.md) or a function returning a map of [migrations](./kibana-plugin-core-server.savedobjectmigrationfn.md) to be used to migrate the type. | | [name](./kibana-plugin-core-server.savedobjectstype.name.md) | string | The name of the type, which is also used as the internal id. | | [namespaceType](./kibana-plugin-core-server.savedobjectstype.namespacetype.md) | SavedObjectsNamespaceType | The [namespace type](./kibana-plugin-core-server.savedobjectsnamespacetype.md) for the type. | -| [schemas?](./kibana-plugin-core-server.savedobjectstype.schemas.md) | SavedObjectsValidationMap<Attributes> \| (() => SavedObjectsValidationMap<Attributes>) | (Optional) An optional schema that can be used to validate the attributes of the type.When provided, calls to [create](./kibana-plugin-core-server.savedobjectsclient.create.md) will be validated against this schema.See [SavedObjectsValidationMap](./kibana-plugin-core-server.savedobjectsvalidationmap.md) for more details. | +| [schemas?](./kibana-plugin-core-server.savedobjectstype.schemas.md) | SavedObjectsValidationMap \| (() => SavedObjectsValidationMap) | (Optional) An optional schema that can be used to validate the attributes of the type.When provided, calls to [create](./kibana-plugin-core-server.savedobjectsclient.create.md) will be validated against this schema.See [SavedObjectsValidationMap](./kibana-plugin-core-server.savedobjectsvalidationmap.md) for more details. | diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectstype.schemas.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectstype.schemas.md index 78fb3d033b2ef0..466d8e361f9953 100644 --- a/docs/development/core/server/kibana-plugin-core-server.savedobjectstype.schemas.md +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectstype.schemas.md @@ -13,5 +13,5 @@ See [SavedObjectsValidationMap](./kibana-plugin-core-server.savedobjectsvalidati Signature: ```typescript -schemas?: SavedObjectsValidationMap | (() => SavedObjectsValidationMap); +schemas?: SavedObjectsValidationMap | (() => SavedObjectsValidationMap); ``` diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationfunction.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationfunction.md index 7fef39f7971f06..58fda4f141f25f 100644 --- a/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationfunction.md +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationfunction.md @@ -6,10 +6,14 @@ The custom validation function if @kbn/config-schema is not a valid solution for your specific plugin requirements. +Be careful not to mutate the provided attributes. + Signature: ```typescript -export declare type SavedObjectsValidationFunction = (data: A) => void; +export declare type SavedObjectsValidationFunction = (data: { + attributes: unknown; +}) => void; ``` ## Example @@ -17,9 +21,9 @@ export declare type SavedObjectsValidationFunction { - if (typeof data.bar !== 'string') { - throw new Error(`[bar]: expected value of type [string] but got [${typeof data.bar}]`); +const myAttributesValidation: SavedObjectsValidationFunction = ({ attributes }) => { + if (typeof attributes.bar !== 'string') { + throw new Error(`[bar]: expected value of type [string] but got [${typeof attributes.bar}]`); } } ``` diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationmap.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationmap.md index 508fcca97019e4..bb3e2b4c7c71e9 100644 --- a/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationmap.md +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationmap.md @@ -11,7 +11,7 @@ Any time you change the schema of a [SavedObjectsType](./kibana-plugin-core-serv Signature: ```typescript -export interface SavedObjectsValidationMap +export interface SavedObjectsValidationMap ``` ## Example @@ -25,12 +25,12 @@ const validationMap: SavedObjectValidationMap = { '1.1.0': schema.object({ foo: schema.oneOf([schema.string(), schema.boolean()]), }), - '2.1.0': (data) => { - if (typeof data.bar !== 'string') { + '2.1.0': ({ attributes }) => { + if (typeof attributes.bar !== 'string') { throw new Error(`[bar]: expected value of type [string] but got [${typeof data.bar}]`); } - if (typeof data.foo !== 'string' && typeof data.foo !== 'boolean') { - throw new Error(`[foo]: expected value of type [string,boolean] but got [${typeof data.foo}]`); + if (typeof attributes.foo !== 'string' && typeof attributes.foo !== 'boolean') { + throw new Error(`[foo]: expected value of type [string,boolean] but got [${typeof attributes.foo}]`); } } } diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationspec.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationspec.md index b69867d5939e78..00a098aa1076f1 100644 --- a/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationspec.md +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationspec.md @@ -11,5 +11,5 @@ See [SavedObjectsValidationFunction](./kibana-plugin-core-server.savedobjectsval Signature: ```typescript -export declare type SavedObjectsValidationSpec = ObjectType | SavedObjectsValidationFunction; +export declare type SavedObjectsValidationSpec = ObjectType | SavedObjectsValidationFunction; ``` diff --git a/src/core/server/server.api.md b/src/core/server/server.api.md index 198a21aa15f260..798a29b6022680 100644 --- a/src/core/server/server.api.md +++ b/src/core/server/server.api.md @@ -2771,7 +2771,7 @@ export interface SavedObjectStatusMeta { } // @public (undocumented) -export interface SavedObjectsType { +export interface SavedObjectsType { convertToAliasScript?: string; convertToMultiNamespaceTypeVersion?: string; excludeOnUpgrade?: SavedObjectTypeExcludeFromUpgradeFilterHook; @@ -2782,7 +2782,7 @@ export interface SavedObjectsType SavedObjectMigrationMap); name: string; namespaceType: SavedObjectsNamespaceType; - schemas?: SavedObjectsValidationMap | (() => SavedObjectsValidationMap); + schemas?: SavedObjectsValidationMap | (() => SavedObjectsValidationMap); } // @public @@ -2871,16 +2871,18 @@ export class SavedObjectsValidationError extends SchemaTypeError { } // @public -export type SavedObjectsValidationFunction = (data: A) => void; +export type SavedObjectsValidationFunction = (data: { + attributes: unknown; +}) => void; // @public -export interface SavedObjectsValidationMap { +export interface SavedObjectsValidationMap { // (undocumented) - [version: string]: SavedObjectsValidationSpec; + [version: string]: SavedObjectsValidationSpec; } // @public -export type SavedObjectsValidationSpec = ObjectType | SavedObjectsValidationFunction; +export type SavedObjectsValidationSpec = ObjectType | SavedObjectsValidationFunction; // Warning: (ae-extra-release-tag) The doc comment should not contain more than one release tag // From 46b0cfa1f58b6f839f753c31990943b3a0e7dfa7 Mon Sep 17 00:00:00 2001 From: Luke Elmers Date: Tue, 28 Dec 2021 16:22:10 -0700 Subject: [PATCH 08/10] Remove support for custom validation functions. --- src/core/server/index.ts | 2 - src/core/server/saved_objects/index.ts | 7 +- .../server/saved_objects/validation/index.ts | 7 +- .../integration_tests/validator.test.ts | 66 ----------- .../saved_objects/validation/schema.test.ts | 25 ++--- .../server/saved_objects/validation/schema.ts | 13 +-- .../server/saved_objects/validation/types.ts | 40 ++----- .../validation/validator.test.ts | 104 +++++------------- .../saved_objects/validation/validator.ts | 31 ++---- .../validation/validator_error.ts | 23 ---- 10 files changed, 62 insertions(+), 256 deletions(-) delete mode 100644 src/core/server/saved_objects/validation/validator_error.ts diff --git a/src/core/server/index.ts b/src/core/server/index.ts index c757361a7acbaa..2f2f78da7274eb 100644 --- a/src/core/server/index.ts +++ b/src/core/server/index.ts @@ -363,8 +363,6 @@ export type { SavedObjectsImportSimpleWarning, SavedObjectsImportActionRequiredWarning, SavedObjectsImportWarning, - SavedObjectsValidationError, - SavedObjectsValidationFunction, SavedObjectsValidationMap, SavedObjectsValidationSpec, } from './saved_objects'; diff --git a/src/core/server/saved_objects/index.ts b/src/core/server/saved_objects/index.ts index 21044c4e0e138e..99b21a1a99c7ce 100644 --- a/src/core/server/saved_objects/index.ts +++ b/src/core/server/saved_objects/index.ts @@ -93,12 +93,7 @@ export type { SavedObjectTypeExcludeFromUpgradeFilterHook, } from './types'; -export type { - SavedObjectsValidationMap, - SavedObjectsValidationSpec, - SavedObjectsValidationFunction, -} from './validation'; -export { SavedObjectsValidationError } from './validation'; +export type { SavedObjectsValidationMap, SavedObjectsValidationSpec } from './validation'; export { savedObjectsConfig, savedObjectsMigrationConfig } from './saved_objects_config'; export { SavedObjectTypeRegistry } from './saved_objects_type_registry'; diff --git a/src/core/server/saved_objects/validation/index.ts b/src/core/server/saved_objects/validation/index.ts index 5311e2207250d1..f18b4e467a75e4 100644 --- a/src/core/server/saved_objects/validation/index.ts +++ b/src/core/server/saved_objects/validation/index.ts @@ -6,10 +6,5 @@ * Side Public License, v 1. */ -export type { - SavedObjectsValidationMap, - SavedObjectsValidationSpec, - SavedObjectsValidationFunction, -} from './types'; +export type { SavedObjectsValidationMap, SavedObjectsValidationSpec } from './types'; export { SavedObjectsTypeValidator } from './validator'; -export { SavedObjectsValidationError } from './validator_error'; diff --git a/src/core/server/saved_objects/validation/integration_tests/validator.test.ts b/src/core/server/saved_objects/validation/integration_tests/validator.test.ts index 23fb0e69173658..c87b8dd5301a82 100644 --- a/src/core/server/saved_objects/validation/integration_tests/validator.test.ts +++ b/src/core/server/saved_objects/validation/integration_tests/validator.test.ts @@ -59,10 +59,6 @@ function createRoot() { ); } -function isRecord(obj: unknown): obj is Record { - return typeof obj === 'object' && obj !== null && !Array.isArray(obj); -} - // To keep this suite from running too long, we are only setting up ES once and registering // a handful of SO types to test different scenarios. This means we need to take care when // adding new tests, as ES will not be cleaned up in between each test run. @@ -87,34 +83,6 @@ const savedObjectTypes: SavedObjectsType[] = [ }), }, }, - { - name: 'schema-using-custom-function', - hidden: false, - mappings: { - properties: { - a: { type: 'integer' }, - b: { type: 'text' }, - }, - }, - migrations: { - [kibanaVersion]: (doc) => doc, - }, - namespaceType: 'agnostic', - schemas: { - [kibanaVersion]: ({ attributes }) => { - if (isRecord(attributes) && typeof attributes.a !== 'number') { - throw new Error( - `[attributes.a]: expected value of type [number] but got [${typeof attributes.a}]` - ); - } - if (isRecord(attributes) && typeof attributes.b !== 'string') { - throw new Error( - `[attributes.b]: expected value of type [string] but got [${typeof attributes.b}]` - ); - } - }, - }, - }, { name: 'no-schema', hidden: false, @@ -295,38 +263,4 @@ describe('validates saved object types when a schema is provided', () => { ); }); }); - - describe('when validating with a custom function', () => { - it('throws when an invalid attribute is provided', async () => { - expect(async () => { - await savedObjectsClient.create( - 'schema-using-custom-function', - { - a: 1, - b: 2, - }, - { migrationVersion: { bar: '7.16.0' } } - ); - }).rejects.toThrowErrorMatchingInlineSnapshot( - `"[attributes.b]: expected value of type [string] but got [number]: Bad Request"` - ); - }); - - it('does not throw when valid attributes are provided', async () => { - const { attributes } = await savedObjectsClient.create( - 'schema-using-custom-function', - { - a: 1, - b: 'heya', - }, - { migrationVersion: { bar: '7.16.0' } } - ); - expect(attributes).toEqual( - expect.objectContaining({ - a: 1, - b: 'heya', - }) - ); - }); - }); }); diff --git a/src/core/server/saved_objects/validation/schema.test.ts b/src/core/server/saved_objects/validation/schema.test.ts index 2e9b5c30d1a3a0..3cb07705e430d9 100644 --- a/src/core/server/saved_objects/validation/schema.test.ts +++ b/src/core/server/saved_objects/validation/schema.test.ts @@ -14,8 +14,7 @@ import { createSavedObjectSanitizedDocSchema } from './schema'; describe('Saved Objects type validation schema', () => { const type = 'my-type'; const validationMap: SavedObjectsValidationMap = { - '1.0.0': (data) => data, - '2.0.0': schema.object({ + '1.0.0': schema.object({ foo: schema.string(), }), }; @@ -27,25 +26,25 @@ describe('Saved Objects type validation schema', () => { type, }); - it('should use generic validation on attributes if a config schema is not provided', () => { + it('should validate attributes based on provided spec', () => { const objectSchema = createSavedObjectSanitizedDocSchema(validationMap['1.0.0']); - const data = createMockObject({ foo: false }); + const data = createMockObject({ foo: 'heya' }); expect(() => objectSchema.validate(data)).not.toThrowError(); }); - it('should fail if attributes do not match generic validation', () => { - const objectSchema = createSavedObjectSanitizedDocSchema(validationMap['2.0.0']); - const data = createMockObject('oops'); + it('should fail if invalid attributes are provided', () => { + const objectSchema = createSavedObjectSanitizedDocSchema(validationMap['1.0.0']); + const data = createMockObject({ foo: false }); expect(() => objectSchema.validate(data)).toThrowErrorMatchingInlineSnapshot( - `"[attributes]: could not parse object value from json input"` + `"[attributes.foo]: expected value of type [string] but got [boolean]"` ); }); - it('should use config schema if provided', () => { - const objectSchema = createSavedObjectSanitizedDocSchema(validationMap['2.0.0']); - const data = createMockObject({ foo: false }); - expect(() => objectSchema.validate(data)).toThrowErrorMatchingInlineSnapshot( - `"[attributes.foo]: expected value of type [string] but got [boolean]"` + it('should fail if top-level properties are invalid', () => { + const objectSchema = createSavedObjectSanitizedDocSchema(validationMap['1.0.0']); + const data = createMockObject({ foo: 'heya' }); + expect(() => objectSchema.validate({ ...data, id: false })).toThrowErrorMatchingInlineSnapshot( + `"[id]: expected value of type [string] but got [boolean]"` ); }); }); diff --git a/src/core/server/saved_objects/validation/schema.ts b/src/core/server/saved_objects/validation/schema.ts index b5fca3ecaff72a..27cef4ebb5bfe3 100644 --- a/src/core/server/saved_objects/validation/schema.ts +++ b/src/core/server/saved_objects/validation/schema.ts @@ -6,7 +6,7 @@ * Side Public License, v 1. */ -import { isConfigSchema, schema, Type } from '@kbn/config-schema'; +import { schema, Type } from '@kbn/config-schema'; import { SavedObjectsValidationSpec } from './types'; import { SavedObjectSanitizedDoc } from '../serialization'; @@ -18,17 +18,14 @@ type SavedObjectSanitizedDocSchema = { }; /** - * Returns a validation schema representing a {@link SavedObjectSanitizedDoc}. - * - * If a {@link SavedObjectsValidationSpec} is provided and the spec is a - * config schema, that validation will be used for the object's `attributes`. - * Otherwise, it falls back to a more generic validation. + * Takes a {@link SavedObjectsValidationSpec} and returns a full schema representing + * a {@link SavedObjectSanitizedDoc}, with the spec applied to the object's `attributes`. * * @internal */ -export const createSavedObjectSanitizedDocSchema = (rule?: SavedObjectsValidationSpec) => +export const createSavedObjectSanitizedDocSchema = (rule: SavedObjectsValidationSpec) => schema.object({ - attributes: isConfigSchema(rule) ? rule : schema.object({}, { unknowns: 'allow' }), + attributes: rule, id: schema.string(), type: schema.string(), references: schema.arrayOf( diff --git a/src/core/server/saved_objects/validation/types.ts b/src/core/server/saved_objects/validation/types.ts index 86215bd956d17f..3cfa0c01ae8e55 100644 --- a/src/core/server/saved_objects/validation/types.ts +++ b/src/core/server/saved_objects/validation/types.ts @@ -8,25 +8,6 @@ import { ObjectType } from '@kbn/config-schema'; -/** - * The custom validation function if @kbn/config-schema is not a valid solution for your specific plugin requirements. - * - * Be careful not to mutate the provided attributes. - * - * @example - * The validation should look something like: - * ```typescript - * const myAttributesValidation: SavedObjectsValidationFunction = ({ attributes }) => { - * if (typeof attributes.bar !== 'string') { - * throw new Error(`[bar]: expected value of type [string] but got [${typeof attributes.bar}]`); - * } - * } - * ``` - * - * @public - */ -export type SavedObjectsValidationFunction = (data: { attributes: unknown }) => void; - /** * Allowed property validation options: either @kbn/config-schema validations or custom validation functions. * @@ -34,7 +15,7 @@ export type SavedObjectsValidationFunction = (data: { attributes: unknown }) => * * @public */ -export type SavedObjectsValidationSpec = ObjectType | SavedObjectsValidationFunction; +export type SavedObjectsValidationSpec = ObjectType; /** * A map of {@link SavedObjectsValidationSpec | validation specs} to be used for a given type. @@ -49,17 +30,16 @@ export type SavedObjectsValidationSpec = ObjectType | SavedObjectsValidationFunc * '1.0.0': schema.object({ * foo: schema.string(), * }), - * '1.1.0': schema.object({ - * foo: schema.oneOf([schema.string(), schema.boolean()]), + * '2.0.0': schema.object({ + * foo: schema.string({ + * minLength: 2, + * validate(value) { + * if (!/^[a-z]+$/.test(value)) { + * return 'must be lowercase letters only'; + * } + * } + * }), * }), - * '2.1.0': ({ attributes }) => { - * if (typeof attributes.bar !== 'string') { - * throw new Error(`[bar]: expected value of type [string] but got [${typeof data.bar}]`); - * } - * if (typeof attributes.foo !== 'string' && typeof attributes.foo !== 'boolean') { - * throw new Error(`[foo]: expected value of type [string,boolean] but got [${typeof attributes.foo}]`); - * } - * } * } * ``` * diff --git a/src/core/server/saved_objects/validation/validator.test.ts b/src/core/server/saved_objects/validation/validator.test.ts index 40e272764a84bd..ae1d56b60b2412 100644 --- a/src/core/server/saved_objects/validation/validator.test.ts +++ b/src/core/server/saved_objects/validation/validator.test.ts @@ -11,24 +11,13 @@ import { SavedObjectsTypeValidator, SavedObjectsValidationMap } from './'; import { SavedObjectSanitizedDoc } from '../serialization'; import { loggerMock, MockedLogger } from '../../logging/logger.mock'; -function isRecord(obj: unknown): obj is Record { - return typeof obj === 'object' && obj !== null && !Array.isArray(obj); -} - describe('Saved Objects type validator', () => { let validator: SavedObjectsTypeValidator; let logger: MockedLogger; const type = 'my-type'; const validationMap: SavedObjectsValidationMap = { - '1.0.0': ({ attributes }) => { - if (isRecord(attributes) && typeof attributes.foo !== 'string') { - throw new Error( - `[foo]: expected value of type [string] but got [${typeof attributes.foo}]` - ); - } - }, - '2.0.0': schema.object({ + '1.0.0': schema.object({ foo: schema.string(), }), }; @@ -49,91 +38,48 @@ describe('Saved Objects type validator', () => { jest.clearAllMocks(); }); - it('should silently skip validation if an invalid mapping is provided', () => { - const malformedValidationMap = { - '1.0.0': ['oops'], - } as unknown as SavedObjectsValidationMap; - validator = new SavedObjectsTypeValidator({ - logger, - type, - validationMap: malformedValidationMap, - }); - - const data = createMockObject({ foo: false }); - expect(() => validator.validate('1.0.0', data)).not.toThrowError(); - }); - it('should do nothing if no matching validation could be found', () => { const data = createMockObject({ foo: false }); expect(validator.validate('3.0.0', data)).toBeUndefined(); expect(logger.debug).not.toHaveBeenCalled(); }); - it('should log when attempting to perform validation', () => { - const data = createMockObject({ foo: 'hi' }); - validator.validate('1.0.0', data); - expect(logger.debug).toHaveBeenCalledTimes(1); + it('should log when a validation fails', () => { + const data = createMockObject({ foo: false }); + expect(() => validator.validate('1.0.0', data)).toThrowError(); + expect(logger.warn).toHaveBeenCalledTimes(1); }); - describe('with valid values', () => { - it('should work with a custom function', () => { - const data = createMockObject({ foo: 'hi' }); - expect(() => validator.validate('1.0.0', data)).not.toThrowError(); - }); - - it('should work with a schema', () => { - const data = createMockObject({ foo: 'hi' }); - expect(() => validator.validate('2.0.0', data)).not.toThrowError(); - }); + it('should work when given valid values', () => { + const data = createMockObject({ foo: 'hi' }); + expect(() => validator.validate('1.0.0', data)).not.toThrowError(); }); - describe('with invalid values', () => { - it('should work with a custom function', () => { - const data = createMockObject({ foo: false }); - expect(() => validator.validate('1.0.0', data)).toThrowErrorMatchingInlineSnapshot( - `"[foo]: expected value of type [string] but got [boolean]"` - ); - }); - - it('should work with a schema', () => { - const data = createMockObject({ foo: false }); - expect(() => validator.validate('2.0.0', data)).toThrowErrorMatchingInlineSnapshot( - `"[attributes.foo]: expected value of type [string] but got [boolean]"` - ); - }); + it('should throw an error when given invalid values', () => { + const data = createMockObject({ foo: false }); + expect(() => validator.validate('1.0.0', data)).toThrowErrorMatchingInlineSnapshot( + `"[attributes.foo]: expected value of type [string] but got [boolean]"` + ); + }); - it('should create an error if fields other than attributes are malformed', () => { - const data = createMockObject({ foo: 'hi' }); - // @ts-expect-error Intentionally malformed object - data.updated_at = false; - expect(() => validator.validate('1.0.0', data)).toThrowErrorMatchingInlineSnapshot( - `"[updated_at]: expected value of type [string] but got [boolean]"` - ); - expect(() => validator.validate('2.0.0', data)).toThrowErrorMatchingInlineSnapshot( - `"[updated_at]: expected value of type [string] but got [boolean]"` - ); - }); + it('should throw an error if fields other than attributes are malformed', () => { + const data = createMockObject({ foo: 'hi' }); + // @ts-expect-error Intentionally malformed object + data.updated_at = false; + expect(() => validator.validate('1.0.0', data)).toThrowErrorMatchingInlineSnapshot( + `"[updated_at]: expected value of type [string] but got [boolean]"` + ); }); - describe('when the validation map is a function', () => { - const fnValidationMap: SavedObjectsValidationMap = { - '1.0.0': () => validationMap['1.0.0'], - '2.0.0': () => validationMap['2.0.0'], - }; + it('works when the validation map is a function', () => { + const fnValidationMap: () => SavedObjectsValidationMap = () => validationMap; validator = new SavedObjectsTypeValidator({ logger, type, validationMap: fnValidationMap, }); - it('should work with a custom function', () => { - const data = createMockObject({ foo: 'hi' }); - expect(() => validator.validate('1.0.0', data)).not.toThrowError(); - }); - - it('should work with a schema', () => { - const data = createMockObject({ foo: 'hi' }); - expect(() => validator.validate('2.0.0', data)).not.toThrowError(); - }); + const data = createMockObject({ foo: 'hi' }); + expect(() => validator.validate('1.0.0', data)).not.toThrowError(); }); }); diff --git a/src/core/server/saved_objects/validation/validator.ts b/src/core/server/saved_objects/validation/validator.ts index ed839ab25ae8d9..e358a435dc0271 100644 --- a/src/core/server/saved_objects/validation/validator.ts +++ b/src/core/server/saved_objects/validation/validator.ts @@ -6,10 +6,8 @@ * Side Public License, v 1. */ -import { ValidationError, isConfigSchema } from '@kbn/config-schema'; import { createSavedObjectSanitizedDocSchema } from './schema'; -import { SavedObjectsValidationMap, SavedObjectsValidationFunction } from './types'; -import { SavedObjectsValidationError } from './validator_error'; +import { SavedObjectsValidationMap } from './types'; import { SavedObjectSanitizedDoc } from '../serialization'; import { Logger } from '../../logging'; @@ -44,27 +42,14 @@ export class SavedObjectsTypeValidator { return; // no matching validation rule could be found; proceed without validating } - this.log.debug(`Validating object of type [${this.type}] against version [${objectVersion}]`); - const validationSchema = createSavedObjectSanitizedDocSchema(validationRule); - validationSchema.validate(data); - - if (!isConfigSchema(validationRule) && isValidationFunction(validationRule)) { - this.validateFunction(data, validationRule); - } - } - - private validateFunction( - data: SavedObjectSanitizedDoc, - validateFn: SavedObjectsValidationFunction - ) { try { - validateFn({ attributes: data.attributes }); - } catch (err) { - throw new ValidationError(new SavedObjectsValidationError(err)); + const validationSchema = createSavedObjectSanitizedDocSchema(validationRule); + validationSchema.validate(data); + } catch (e) { + this.log.warn( + `Error validating object of type [${this.type}] against version [${objectVersion}]` + ); + throw e; } } } - -function isValidationFunction(fn: unknown): fn is SavedObjectsValidationFunction { - return typeof fn === 'function'; -} diff --git a/src/core/server/saved_objects/validation/validator_error.ts b/src/core/server/saved_objects/validation/validator_error.ts deleted file mode 100644 index 6cd5538c153c90..00000000000000 --- a/src/core/server/saved_objects/validation/validator_error.ts +++ /dev/null @@ -1,23 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0 and the Server Side Public License, v 1; you may not use this file except - * in compliance with, at your election, the Elastic License 2.0 or the Server - * Side Public License, v 1. - */ - -import { SchemaTypeError } from '@kbn/config-schema'; - -/** - * Error to return when the validation is not successful. - * @public - */ -export class SavedObjectsValidationError extends SchemaTypeError { - constructor(error: Error | string, path: string[] = []) { - super(error, path); - - // Set the prototype explicitly, see: - // https://github.com/Microsoft/TypeScript/wiki/Breaking-Changes#extending-built-ins-like-error-array-and-map-may-no-longer-work - Object.setPrototypeOf(this, SavedObjectsValidationError.prototype); - } -} From c37b5988cdd7ddeca7af74826000f25705791c5e Mon Sep 17 00:00:00 2001 From: Luke Elmers Date: Tue, 28 Dec 2021 16:23:07 -0700 Subject: [PATCH 09/10] Update generated docs. --- .../core/server/kibana-plugin-core-server.md | 4 +-- ...vedobjectsvalidationerror._constructor_.md | 21 ------------- ...core-server.savedobjectsvalidationerror.md | 21 ------------- ...e-server.savedobjectsvalidationfunction.md | 30 ------------------- ...n-core-server.savedobjectsvalidationmap.md | 19 ++++++------ ...-core-server.savedobjectsvalidationspec.md | 4 +-- src/core/server/server.api.md | 14 ++------- 7 files changed, 15 insertions(+), 98 deletions(-) delete mode 100644 docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationerror._constructor_.md delete mode 100644 docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationerror.md delete mode 100644 docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationfunction.md diff --git a/docs/development/core/server/kibana-plugin-core-server.md b/docs/development/core/server/kibana-plugin-core-server.md index 59e63eab323134..798b7090c5ec20 100644 --- a/docs/development/core/server/kibana-plugin-core-server.md +++ b/docs/development/core/server/kibana-plugin-core-server.md @@ -31,7 +31,6 @@ The plugin integrates with the core system via lifecycle events: `setup` | [SavedObjectsRepository](./kibana-plugin-core-server.savedobjectsrepository.md) | | | [SavedObjectsSerializer](./kibana-plugin-core-server.savedobjectsserializer.md) | A serializer that can be used to manually convert [raw](./kibana-plugin-core-server.savedobjectsrawdoc.md) or [sanitized](./kibana-plugin-core-server.savedobjectsanitizeddoc.md) documents to the other kind. | | [SavedObjectsUtils](./kibana-plugin-core-server.savedobjectsutils.md) | | -| [SavedObjectsValidationError](./kibana-plugin-core-server.savedobjectsvalidationerror.md) | Error to return when the validation is not successful. | | [SavedObjectTypeRegistry](./kibana-plugin-core-server.savedobjecttyperegistry.md) | Registry holding information about all the registered [saved object types](./kibana-plugin-core-server.savedobjectstype.md). | ## Enumerations @@ -322,8 +321,7 @@ The plugin integrates with the core system via lifecycle events: `setup` | [SavedObjectsImportHook](./kibana-plugin-core-server.savedobjectsimporthook.md) | A hook associated with a specific saved object type, that will be invoked during the import process. The hook will have access to the objects of the registered type.Currently, the only supported feature for import hooks is to return warnings to be displayed in the UI when the import succeeds. The only interactions the hook can have with the import process is via the hook's response. Mutating the objects inside the hook's code will have no effect. | | [SavedObjectsImportWarning](./kibana-plugin-core-server.savedobjectsimportwarning.md) | Composite type of all the possible types of import warnings.See [SavedObjectsImportSimpleWarning](./kibana-plugin-core-server.savedobjectsimportsimplewarning.md) and [SavedObjectsImportActionRequiredWarning](./kibana-plugin-core-server.savedobjectsimportactionrequiredwarning.md) for more details. | | [SavedObjectsNamespaceType](./kibana-plugin-core-server.savedobjectsnamespacetype.md) | The namespace type dictates how a saved object can be interacted in relation to namespaces. Each type is mutually exclusive: \* single (default): This type of saved object is namespace-isolated, e.g., it exists in only one namespace. \* multiple: This type of saved object is shareable, e.g., it can exist in one or more namespaces. \* multiple-isolated: This type of saved object is namespace-isolated, e.g., it exists in only one namespace, but object IDs must be unique across all namespaces. This is intended to be an intermediate step when objects with a "single" namespace type are being converted to a "multiple" namespace type. In other words, objects with a "multiple-isolated" namespace type will be \*share-capable\*, but will not actually be shareable until the namespace type is changed to "multiple". \* agnostic: This type of saved object is global. | -| [SavedObjectsValidationFunction](./kibana-plugin-core-server.savedobjectsvalidationfunction.md) | The custom validation function if @kbn/config-schema is not a valid solution for your specific plugin requirements.Be careful not to mutate the provided attributes. | -| [SavedObjectsValidationSpec](./kibana-plugin-core-server.savedobjectsvalidationspec.md) | Allowed property validation options: either @kbn/config-schema validations or custom validation functions.See [SavedObjectsValidationFunction](./kibana-plugin-core-server.savedobjectsvalidationfunction.md) for custom validation. | +| [SavedObjectsValidationSpec](./kibana-plugin-core-server.savedobjectsvalidationspec.md) | Allowed property validation options: either @kbn/config-schema validations or custom validation functions.See for custom validation. | | [SavedObjectTypeExcludeFromUpgradeFilterHook](./kibana-plugin-core-server.savedobjecttypeexcludefromupgradefilterhook.md) | If defined, allows a type to run a search query and return a query filter that may match any documents which may be excluded from the next migration upgrade process. Useful for cleaning up large numbers of old documents which are no longer needed and may slow the migration process.If this hook fails, the migration will proceed without these documents having been filtered out, so this should not be used as a guarantee that these documents have been deleted.Experimental and subject to change | | [SavedObjectUnsanitizedDoc](./kibana-plugin-core-server.savedobjectunsanitizeddoc.md) | Describes Saved Object documents from Kibana < 7.0.0 which don't have a references root property defined. This type should only be used in migrations. | | [ScopeableRequest](./kibana-plugin-core-server.scopeablerequest.md) | A user credentials container. It accommodates the necessary auth credentials to impersonate the current user.See [KibanaRequest](./kibana-plugin-core-server.kibanarequest.md). | diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationerror._constructor_.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationerror._constructor_.md deleted file mode 100644 index c3867cc095a5ee..00000000000000 --- a/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationerror._constructor_.md +++ /dev/null @@ -1,21 +0,0 @@ - - -[Home](./index.md) > [kibana-plugin-core-server](./kibana-plugin-core-server.md) > [SavedObjectsValidationError](./kibana-plugin-core-server.savedobjectsvalidationerror.md) > [(constructor)](./kibana-plugin-core-server.savedobjectsvalidationerror._constructor_.md) - -## SavedObjectsValidationError.(constructor) - -Constructs a new instance of the `SavedObjectsValidationError` class - -Signature: - -```typescript -constructor(error: Error | string, path?: string[]); -``` - -## Parameters - -| Parameter | Type | Description | -| --- | --- | --- | -| error | Error \| string | | -| path | string\[\] | | - diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationerror.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationerror.md deleted file mode 100644 index cc6d0ae8d73e14..00000000000000 --- a/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationerror.md +++ /dev/null @@ -1,21 +0,0 @@ - - -[Home](./index.md) > [kibana-plugin-core-server](./kibana-plugin-core-server.md) > [SavedObjectsValidationError](./kibana-plugin-core-server.savedobjectsvalidationerror.md) - -## SavedObjectsValidationError class - -Error to return when the validation is not successful. - -Signature: - -```typescript -export declare class SavedObjectsValidationError extends SchemaTypeError -``` -Extends: SchemaTypeError - -## Constructors - -| Constructor | Modifiers | Description | -| --- | --- | --- | -| [(constructor)(error, path)](./kibana-plugin-core-server.savedobjectsvalidationerror._constructor_.md) | | Constructs a new instance of the SavedObjectsValidationError class | - diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationfunction.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationfunction.md deleted file mode 100644 index 58fda4f141f25f..00000000000000 --- a/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationfunction.md +++ /dev/null @@ -1,30 +0,0 @@ - - -[Home](./index.md) > [kibana-plugin-core-server](./kibana-plugin-core-server.md) > [SavedObjectsValidationFunction](./kibana-plugin-core-server.savedobjectsvalidationfunction.md) - -## SavedObjectsValidationFunction type - -The custom validation function if @kbn/config-schema is not a valid solution for your specific plugin requirements. - -Be careful not to mutate the provided attributes. - -Signature: - -```typescript -export declare type SavedObjectsValidationFunction = (data: { - attributes: unknown; -}) => void; -``` - -## Example - -The validation should look something like: - -```typescript -const myAttributesValidation: SavedObjectsValidationFunction = ({ attributes }) => { - if (typeof attributes.bar !== 'string') { - throw new Error(`[bar]: expected value of type [string] but got [${typeof attributes.bar}]`); - } -} -``` - diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationmap.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationmap.md index bb3e2b4c7c71e9..7306b8ab2ce3db 100644 --- a/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationmap.md +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationmap.md @@ -22,17 +22,16 @@ const validationMap: SavedObjectValidationMap = { '1.0.0': schema.object({ foo: schema.string(), }), - '1.1.0': schema.object({ - foo: schema.oneOf([schema.string(), schema.boolean()]), + '2.0.0': schema.object({ + foo: schema.string({ + minLength: 2, + validate(value) { + if (!/^[a-z]+$/.test(value)) { + return 'must be lowercase letters only'; + } + } + }), }), - '2.1.0': ({ attributes }) => { - if (typeof attributes.bar !== 'string') { - throw new Error(`[bar]: expected value of type [string] but got [${typeof data.bar}]`); - } - if (typeof attributes.foo !== 'string' && typeof attributes.foo !== 'boolean') { - throw new Error(`[foo]: expected value of type [string,boolean] but got [${typeof attributes.foo}]`); - } - } } ``` diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationspec.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationspec.md index 00a098aa1076f1..fb13a6a26d8494 100644 --- a/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationspec.md +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationspec.md @@ -6,10 +6,10 @@ Allowed property validation options: either @kbn/config-schema validations or custom validation functions. -See [SavedObjectsValidationFunction](./kibana-plugin-core-server.savedobjectsvalidationfunction.md) for custom validation. +See for custom validation. Signature: ```typescript -export declare type SavedObjectsValidationSpec = ObjectType | SavedObjectsValidationFunction; +export declare type SavedObjectsValidationSpec = ObjectType; ``` diff --git a/src/core/server/server.api.md b/src/core/server/server.api.md index 2e097350cd0b1a..e6e7023b388d98 100644 --- a/src/core/server/server.api.md +++ b/src/core/server/server.api.md @@ -2867,24 +2867,16 @@ export class SavedObjectsUtils { static namespaceStringToId: (namespace: string) => string | undefined; } -// @public -export class SavedObjectsValidationError extends SchemaTypeError { - constructor(error: Error | string, path?: string[]); -} - -// @public -export type SavedObjectsValidationFunction = (data: { - attributes: unknown; -}) => void; - // @public export interface SavedObjectsValidationMap { // (undocumented) [version: string]: SavedObjectsValidationSpec; } +// Warning: (ae-unresolved-link) The @link reference could not be resolved: The package "kibana" does not have an export "SavedObjectsValidationFunction" +// // @public -export type SavedObjectsValidationSpec = ObjectType | SavedObjectsValidationFunction; +export type SavedObjectsValidationSpec = ObjectType; // Warning: (ae-extra-release-tag) The doc comment should not contain more than one release tag // From 5ed7501a88e9319b93a9b5cbd8e2bcb379c13d1e Mon Sep 17 00:00:00 2001 From: Luke Elmers Date: Tue, 4 Jan 2022 20:49:59 -0700 Subject: [PATCH 10/10] Address feedback. --- .../core/server/kibana-plugin-core-server.md | 2 +- ...n-core-server.savedobjectsvalidationmap.md | 2 +- ...-core-server.savedobjectsvalidationspec.md | 4 +-- .../service/lib/repository.test.ts | 22 +++++++------- .../integration_tests/validator.test.ts | 22 +++++++------- .../saved_objects/validation/schema.test.ts | 29 +++++++++++++++++++ .../server/saved_objects/validation/schema.ts | 6 ++-- .../server/saved_objects/validation/types.ts | 6 ++-- src/core/server/server.api.md | 2 -- 9 files changed, 57 insertions(+), 38 deletions(-) diff --git a/docs/development/core/server/kibana-plugin-core-server.md b/docs/development/core/server/kibana-plugin-core-server.md index 798b7090c5ec20..6b0f65861ad5b0 100644 --- a/docs/development/core/server/kibana-plugin-core-server.md +++ b/docs/development/core/server/kibana-plugin-core-server.md @@ -321,7 +321,7 @@ The plugin integrates with the core system via lifecycle events: `setup` | [SavedObjectsImportHook](./kibana-plugin-core-server.savedobjectsimporthook.md) | A hook associated with a specific saved object type, that will be invoked during the import process. The hook will have access to the objects of the registered type.Currently, the only supported feature for import hooks is to return warnings to be displayed in the UI when the import succeeds. The only interactions the hook can have with the import process is via the hook's response. Mutating the objects inside the hook's code will have no effect. | | [SavedObjectsImportWarning](./kibana-plugin-core-server.savedobjectsimportwarning.md) | Composite type of all the possible types of import warnings.See [SavedObjectsImportSimpleWarning](./kibana-plugin-core-server.savedobjectsimportsimplewarning.md) and [SavedObjectsImportActionRequiredWarning](./kibana-plugin-core-server.savedobjectsimportactionrequiredwarning.md) for more details. | | [SavedObjectsNamespaceType](./kibana-plugin-core-server.savedobjectsnamespacetype.md) | The namespace type dictates how a saved object can be interacted in relation to namespaces. Each type is mutually exclusive: \* single (default): This type of saved object is namespace-isolated, e.g., it exists in only one namespace. \* multiple: This type of saved object is shareable, e.g., it can exist in one or more namespaces. \* multiple-isolated: This type of saved object is namespace-isolated, e.g., it exists in only one namespace, but object IDs must be unique across all namespaces. This is intended to be an intermediate step when objects with a "single" namespace type are being converted to a "multiple" namespace type. In other words, objects with a "multiple-isolated" namespace type will be \*share-capable\*, but will not actually be shareable until the namespace type is changed to "multiple". \* agnostic: This type of saved object is global. | -| [SavedObjectsValidationSpec](./kibana-plugin-core-server.savedobjectsvalidationspec.md) | Allowed property validation options: either @kbn/config-schema validations or custom validation functions.See for custom validation. | +| [SavedObjectsValidationSpec](./kibana-plugin-core-server.savedobjectsvalidationspec.md) | Allows for validating properties using @kbn/config-schema validations. | | [SavedObjectTypeExcludeFromUpgradeFilterHook](./kibana-plugin-core-server.savedobjecttypeexcludefromupgradefilterhook.md) | If defined, allows a type to run a search query and return a query filter that may match any documents which may be excluded from the next migration upgrade process. Useful for cleaning up large numbers of old documents which are no longer needed and may slow the migration process.If this hook fails, the migration will proceed without these documents having been filtered out, so this should not be used as a guarantee that these documents have been deleted.Experimental and subject to change | | [SavedObjectUnsanitizedDoc](./kibana-plugin-core-server.savedobjectunsanitizeddoc.md) | Describes Saved Object documents from Kibana < 7.0.0 which don't have a references root property defined. This type should only be used in migrations. | | [ScopeableRequest](./kibana-plugin-core-server.scopeablerequest.md) | A user credentials container. It accommodates the necessary auth credentials to impersonate the current user.See [KibanaRequest](./kibana-plugin-core-server.kibanarequest.md). | diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationmap.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationmap.md index 7306b8ab2ce3db..3716db5c3d9ac3 100644 --- a/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationmap.md +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationmap.md @@ -18,7 +18,7 @@ export interface SavedObjectsValidationMap ```typescript -const validationMap: SavedObjectValidationMap = { +const validationMap: SavedObjectsValidationMap = { '1.0.0': schema.object({ foo: schema.string(), }), diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationspec.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationspec.md index fb13a6a26d8494..55938fb4b4b6c8 100644 --- a/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationspec.md +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectsvalidationspec.md @@ -4,9 +4,7 @@ ## SavedObjectsValidationSpec type -Allowed property validation options: either @kbn/config-schema validations or custom validation functions. - -See for custom validation. +Allows for validating properties using @kbn/config-schema validations. Signature: diff --git a/src/core/server/saved_objects/service/lib/repository.test.ts b/src/core/server/saved_objects/service/lib/repository.test.ts index 2a48e746563bc5..41a284764b0ea9 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.ts +++ b/src/core/server/saved_objects/service/lib/repository.test.ts @@ -994,18 +994,16 @@ describe('SavedObjectsRepository', () => { { ...obj3, id: 'three-again', attributes: { title: 123 } }, ]); expect(client.bulk).toHaveBeenCalledTimes(1); // only called once for the valid object - expect(result.saved_objects).toEqual( - expect.arrayContaining([ - expect.objectContaining(obj3), - expect.objectContaining({ - error: new Error( - '[attributes.title]: expected value of type [string] but got [number]: Bad Request' - ), - id: 'three-again', - type: 'dashboard', - }), - ]) - ); + expect(result.saved_objects).toEqual([ + expect.objectContaining(obj3), + expect.objectContaining({ + error: new Error( + '[attributes.title]: expected value of type [string] but got [number]: Bad Request' + ), + id: 'three-again', + type: 'dashboard', + }), + ]); }); }); diff --git a/src/core/server/saved_objects/validation/integration_tests/validator.test.ts b/src/core/server/saved_objects/validation/integration_tests/validator.test.ts index c87b8dd5301a82..d47b42e36ce812 100644 --- a/src/core/server/saved_objects/validation/integration_tests/validator.test.ts +++ b/src/core/server/saved_objects/validation/integration_tests/validator.test.ts @@ -216,18 +216,16 @@ describe('validates saved object types when a schema is provided', () => { // @ts-expect-error - The invalidObj is intentionally malformed for testing const results = await savedObjectsClient.bulkCreate([validObj, invalidObj]); - expect(results.saved_objects).toEqual( - expect.arrayContaining([ - expect.objectContaining(validObj), - expect.objectContaining({ - error: new Error( - '[attributes.a]: expected value of type [number] but got [string]: Bad Request' - ), - id: 'bulk-invalid', - type: 'schema-using-kbn-config', - }), - ]) - ); + expect(results.saved_objects).toEqual([ + expect.objectContaining(validObj), + expect.objectContaining({ + error: new Error( + '[attributes.a]: expected value of type [number] but got [string]: Bad Request' + ), + id: 'bulk-invalid', + type: 'schema-using-kbn-config', + }), + ]); }); describe('when validating with a config schema', () => { diff --git a/src/core/server/saved_objects/validation/schema.test.ts b/src/core/server/saved_objects/validation/schema.test.ts index 3cb07705e430d9..7b75dd2cec4fbe 100644 --- a/src/core/server/saved_objects/validation/schema.test.ts +++ b/src/core/server/saved_objects/validation/schema.test.ts @@ -40,6 +40,35 @@ describe('Saved Objects type validation schema', () => { ); }); + it('should validate top-level properties', () => { + const objectSchema = createSavedObjectSanitizedDocSchema(validationMap['1.0.0']); + const data = createMockObject({ foo: 'heya' }); + + expect(() => + objectSchema.validate({ + ...data, + id: 'abc-123', + type: 'dashboard', + references: [ + { + name: 'ref_0', + type: 'visualization', + id: '123', + }, + ], + namespace: 'a', + namespaces: ['a', 'b'], + migrationVersion: { + dashboard: '1.0.0', + }, + coreMigrationVersion: '1.0.0', + updated_at: '2022-01-05T03:17:07.183Z', + version: '2', + originId: 'def-456', + }) + ).not.toThrowError(); + }); + it('should fail if top-level properties are invalid', () => { const objectSchema = createSavedObjectSanitizedDocSchema(validationMap['1.0.0']); const data = createMockObject({ foo: 'heya' }); diff --git a/src/core/server/saved_objects/validation/schema.ts b/src/core/server/saved_objects/validation/schema.ts index 27cef4ebb5bfe3..e6e125b6288ed2 100644 --- a/src/core/server/saved_objects/validation/schema.ts +++ b/src/core/server/saved_objects/validation/schema.ts @@ -23,9 +23,9 @@ type SavedObjectSanitizedDocSchema = { * * @internal */ -export const createSavedObjectSanitizedDocSchema = (rule: SavedObjectsValidationSpec) => +export const createSavedObjectSanitizedDocSchema = (attributesSchema: SavedObjectsValidationSpec) => schema.object({ - attributes: rule, + attributes: attributesSchema, id: schema.string(), type: schema.string(), references: schema.arrayOf( @@ -38,7 +38,7 @@ export const createSavedObjectSanitizedDocSchema = (rule: SavedObjectsValidation ), namespace: schema.maybe(schema.string()), namespaces: schema.maybe(schema.arrayOf(schema.string())), - migrationVersion: schema.maybe(schema.object({}, { unknowns: 'allow' })), + migrationVersion: schema.maybe(schema.recordOf(schema.string(), schema.string())), coreMigrationVersion: schema.maybe(schema.string()), updated_at: schema.maybe(schema.string()), version: schema.maybe(schema.string()), diff --git a/src/core/server/saved_objects/validation/types.ts b/src/core/server/saved_objects/validation/types.ts index 3cfa0c01ae8e55..bf5d8fc355ade7 100644 --- a/src/core/server/saved_objects/validation/types.ts +++ b/src/core/server/saved_objects/validation/types.ts @@ -9,9 +9,7 @@ import { ObjectType } from '@kbn/config-schema'; /** - * Allowed property validation options: either @kbn/config-schema validations or custom validation functions. - * - * See {@link SavedObjectsValidationFunction} for custom validation. + * Allows for validating properties using @kbn/config-schema validations. * * @public */ @@ -26,7 +24,7 @@ export type SavedObjectsValidationSpec = ObjectType; * * @example * ```typescript - * const validationMap: SavedObjectValidationMap = { + * const validationMap: SavedObjectsValidationMap = { * '1.0.0': schema.object({ * foo: schema.string(), * }), diff --git a/src/core/server/server.api.md b/src/core/server/server.api.md index e6e7023b388d98..9cd5486ecf67b5 100644 --- a/src/core/server/server.api.md +++ b/src/core/server/server.api.md @@ -2873,8 +2873,6 @@ export interface SavedObjectsValidationMap { [version: string]: SavedObjectsValidationSpec; } -// Warning: (ae-unresolved-link) The @link reference could not be resolved: The package "kibana" does not have an export "SavedObjectsValidationFunction" -// // @public export type SavedObjectsValidationSpec = ObjectType;