From 90ce69a61815377a00dd4866d5af8da5530c971d Mon Sep 17 00:00:00 2001 From: Aleh Zasypkin Date: Wed, 18 Mar 2020 14:06:59 +0100 Subject: [PATCH] [7.x] Enforce `required` presence for value/key validation of `recordOf` and `mapOf`. (#60491) --- .../kbn-config-schema/src/internals/index.ts | 12 ++++++++---- .../src/types/map_of_type.test.ts | 18 ++++++++++++++++++ .../kbn-config-schema/src/types/map_type.ts | 5 ++++- .../src/types/record_of_type.test.ts | 18 ++++++++++++++++++ .../kbn-config-schema/src/types/record_type.ts | 5 ++++- .../roles/model/put_payload.test.ts | 2 +- 6 files changed, 53 insertions(+), 7 deletions(-) diff --git a/packages/kbn-config-schema/src/internals/index.ts b/packages/kbn-config-schema/src/internals/index.ts index 8f5d09e5b8b49..f84e14d2f741d 100644 --- a/packages/kbn-config-schema/src/internals/index.ts +++ b/packages/kbn-config-schema/src/internals/index.ts @@ -314,7 +314,8 @@ export const internals = Joi.extend([ for (const [entryKey, entryValue] of value) { const { value: validatedEntryKey, error: keyError } = Joi.validate( entryKey, - params.key + params.key, + { presence: 'required' } ); if (keyError) { @@ -323,7 +324,8 @@ export const internals = Joi.extend([ const { value: validatedEntryValue, error: valueError } = Joi.validate( entryValue, - params.value + params.value, + { presence: 'required' } ); if (valueError) { @@ -374,7 +376,8 @@ export const internals = Joi.extend([ for (const [entryKey, entryValue] of Object.entries(value)) { const { value: validatedEntryKey, error: keyError } = Joi.validate( entryKey, - params.key + params.key, + { presence: 'required' } ); if (keyError) { @@ -383,7 +386,8 @@ export const internals = Joi.extend([ const { value: validatedEntryValue, error: valueError } = Joi.validate( entryValue, - params.value + params.value, + { presence: 'required' } ); if (valueError) { diff --git a/packages/kbn-config-schema/src/types/map_of_type.test.ts b/packages/kbn-config-schema/src/types/map_of_type.test.ts index b015f51bdc8ad..1c5a227ef0fac 100644 --- a/packages/kbn-config-schema/src/types/map_of_type.test.ts +++ b/packages/kbn-config-schema/src/types/map_of_type.test.ts @@ -159,6 +159,24 @@ test('object within mapOf', () => { expect(type.validate(value)).toEqual(expected); }); +test('enforces required object fields within mapOf', () => { + const type = schema.mapOf( + schema.string(), + schema.object({ + bar: schema.object({ + baz: schema.number(), + }), + }) + ); + const value = { + foo: {}, + }; + + expect(() => type.validate(value)).toThrowErrorMatchingInlineSnapshot( + `"[foo.bar.baz]: expected value of type [number] but got [undefined]"` + ); +}); + test('error preserves full path', () => { const type = schema.object({ grandParentKey: schema.object({ diff --git a/packages/kbn-config-schema/src/types/map_type.ts b/packages/kbn-config-schema/src/types/map_type.ts index 231c3726ae9d5..6da664bf95616 100644 --- a/packages/kbn-config-schema/src/types/map_type.ts +++ b/packages/kbn-config-schema/src/types/map_type.ts @@ -57,7 +57,10 @@ export class MapOfType extends Type> { path.length, 0, // If `key` validation failed, let's stress that to make error more obvious. - type === 'map.key' ? `key("${entryKey}")` : entryKey.toString() + type === 'map.key' ? `key("${entryKey}")` : entryKey.toString(), + // Error could have happened deep inside value/key schema and error message should + // include full path. + ...(reason instanceof SchemaTypeError ? reason.path : []) ); return reason instanceof SchemaTypesError diff --git a/packages/kbn-config-schema/src/types/record_of_type.test.ts b/packages/kbn-config-schema/src/types/record_of_type.test.ts index ef15e7b0f6ad6..aee7dde71c3e4 100644 --- a/packages/kbn-config-schema/src/types/record_of_type.test.ts +++ b/packages/kbn-config-schema/src/types/record_of_type.test.ts @@ -159,6 +159,24 @@ test('object within recordOf', () => { expect(type.validate(value)).toEqual({ foo: { bar: 123 } }); }); +test('enforces required object fields within recordOf', () => { + const type = schema.recordOf( + schema.string(), + schema.object({ + bar: schema.object({ + baz: schema.number(), + }), + }) + ); + const value = { + foo: {}, + }; + + expect(() => type.validate(value)).toThrowErrorMatchingInlineSnapshot( + `"[foo.bar.baz]: expected value of type [number] but got [undefined]"` + ); +}); + test('error preserves full path', () => { const type = schema.object({ grandParentKey: schema.object({ diff --git a/packages/kbn-config-schema/src/types/record_type.ts b/packages/kbn-config-schema/src/types/record_type.ts index c6d4b4d71b4f1..ef9e70cbabc08 100644 --- a/packages/kbn-config-schema/src/types/record_type.ts +++ b/packages/kbn-config-schema/src/types/record_type.ts @@ -49,7 +49,10 @@ export class RecordOfType extends Type> { path.length, 0, // If `key` validation failed, let's stress that to make error more obvious. - type === 'record.key' ? `key("${entryKey}")` : entryKey.toString() + type === 'record.key' ? `key("${entryKey}")` : entryKey.toString(), + // Error could have happened deep inside value/key schema and error message should + // include full path. + ...(reason instanceof SchemaTypeError ? reason.path : []) ); return reason instanceof SchemaTypesError diff --git a/x-pack/plugins/security/server/routes/authorization/roles/model/put_payload.test.ts b/x-pack/plugins/security/server/routes/authorization/roles/model/put_payload.test.ts index acde73dcd8190..eedd63e228523 100644 --- a/x-pack/plugins/security/server/routes/authorization/roles/model/put_payload.test.ts +++ b/x-pack/plugins/security/server/routes/authorization/roles/model/put_payload.test.ts @@ -28,7 +28,7 @@ describe('Put payload schema', () => { kibana: [{ feature: { foo: ['!foo'] } }], }) ).toThrowErrorMatchingInlineSnapshot( - `"[kibana.0.feature.foo]: only a-z, A-Z, 0-9, '_', and '-' are allowed"` + `"[kibana.0.feature.foo.0]: only a-z, A-Z, 0-9, '_', and '-' are allowed"` ); });