[7.x] Enforce required presence for value/key validation of `record…

…Of` and `mapOf`. (elastic#60491)
watson · Mar 18, 2020 · 90ce69a · 90ce69a
1 parent 5acd83c
commit 90ce69a
Show file tree

Hide file tree

Showing 6 changed files with 53 additions and 7 deletions.
diff --git 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
@@ -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
@@ -57,7 +57,10 @@ export class MapOfType<K, V> extends Type<Map<K, V>> {
           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
@@ -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
@@ -49,7 +49,10 @@ export class RecordOfType<K extends string, V> extends Type<Record<K, V>> {
           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
@@ -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"`
     );
   });