Skip to content

Commit

Permalink
Consolidate unknown key configuration
Browse files Browse the repository at this point in the history
  • Loading branch information
legrego committed Mar 6, 2020
1 parent 12a2cde commit c74167e
Show file tree
Hide file tree
Showing 51 changed files with 98 additions and 124 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ validate: RouteValidatorFullConfig<P, Q, B> | false;

## Remarks

You \*must\* specify a validation schema to be able to read: - url path segments - request query - request body To opt out of validating the request, specify `validate: false`<!-- -->. In this case request params, query, and body will be \*\*empty\*\* objects and have no access to raw values. In some cases you may want to use another validation library. To do this, you need to instruct the `@kbn/config-schema` library to output \*\*non-validated values\*\* with setting schema as `schema.object({}, { allowUnknowns: true })`<!-- -->;
You \*must\* specify a validation schema to be able to read: - url path segments - request query - request body To opt out of validating the request, specify `validate: false`<!-- -->. In this case request params, query, and body will be \*\*empty\*\* objects and have no access to raw values. In some cases you may want to use another validation library. To do this, you need to instruct the `@kbn/config-schema` library to output \*\*non-validated values\*\* with setting schema as `schema.object({}, { unknowns: 'allow' })`<!-- -->;

## Example

Expand Down
4 changes: 2 additions & 2 deletions packages/kbn-config-schema/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ __Output type:__ `{ [K in keyof TProps]: TypeOf<TProps[K]> } as TObject`
__Options:__
* `defaultValue: TObject | Reference<TObject> | (() => TObject)` - defines a default value, see [Default values](#default-values) section for more details.
* `validate: (value: TObject) => string | void` - defines a custom validator function, see [Custom validation](#custom-validation) section for more details.
* `allowUnknowns: boolean` - indicates whether unknown object properties should be allowed. It's `false` by default.
* `unknowns: 'allow' | 'ignore' | 'forbid'` - indicates whether unknown object properties should be allowed, ignored, or forbidden. It's `forbid` by default.

__Usage:__
```typescript
Expand All @@ -250,7 +250,7 @@ const valueSchema = schema.object({
```

__Notes:__
* Using `allowUnknowns` is discouraged and should only be used in exceptional circumstances. Consider using `schema.recordOf()` instead.
* Using `unknowns: 'allow'` is discouraged and should only be used in exceptional circumstances. Consider using `schema.recordOf()` instead.
* Currently `schema.object()` always has a default value of `{}`, but this may change in the near future. Try to not rely on this behaviour and specify default value explicitly or use `schema.maybe()` if the value is optional.
* `schema.object()` also supports a json string as input if it can be safely parsed using `JSON.parse` and if the resulting value is a plain object.

Expand Down
32 changes: 10 additions & 22 deletions packages/kbn-config-schema/src/types/object_type.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -276,10 +276,10 @@ test('individual keys can validated', () => {
);
});

test('allow unknown keys when allowUnknowns = true', () => {
test('allow unknown keys when unknowns = `allow`', () => {
const type = schema.object(
{ foo: schema.string({ defaultValue: 'test' }) },
{ allowUnknowns: true }
{ unknowns: 'allow' }
);

expect(
Expand All @@ -292,10 +292,10 @@ test('allow unknown keys when allowUnknowns = true', () => {
});
});

test('allowUnknowns = true affects only own keys', () => {
test('unknowns = `allow` affects only own keys', () => {
const type = schema.object(
{ foo: schema.object({ bar: schema.string() }) },
{ allowUnknowns: true }
{ unknowns: 'allow' }
);

expect(() =>
Expand All @@ -308,10 +308,10 @@ test('allowUnknowns = true affects only own keys', () => {
).toThrowErrorMatchingInlineSnapshot(`"[foo.baz]: definition for this key is missing"`);
});

test('does not allow unknown keys when allowUnknowns = false', () => {
test('does not allow unknown keys when unknowns = `forbid`', () => {
const type = schema.object(
{ foo: schema.string({ defaultValue: 'test' }) },
{ allowUnknowns: false }
{ unknowns: 'forbid' }
);
expect(() =>
type.validate({
Expand All @@ -320,10 +320,10 @@ test('does not allow unknown keys when allowUnknowns = false', () => {
).toThrowErrorMatchingInlineSnapshot(`"[bar]: definition for this key is missing"`);
});

test('allow and remove unknown keys when ignoreUnknowns = true', () => {
test('allow and remove unknown keys when unknowns = `ignore`', () => {
const type = schema.object(
{ foo: schema.string({ defaultValue: 'test' }) },
{ ignoreUnknowns: true }
{ unknowns: 'ignore' }
);

expect(
Expand All @@ -335,10 +335,10 @@ test('allow and remove unknown keys when ignoreUnknowns = true', () => {
});
});

test('ignoreUnknowns = true affects only own keys', () => {
test('unknowns = `ignore` affects only own keys', () => {
const type = schema.object(
{ foo: schema.object({ bar: schema.string() }) },
{ ignoreUnknowns: true }
{ unknowns: 'ignore' }
);

expect(() =>
Expand All @@ -350,15 +350,3 @@ test('ignoreUnknowns = true affects only own keys', () => {
})
).toThrowErrorMatchingInlineSnapshot(`"[foo.baz]: definition for this key is missing"`);
});

test('does not allow unknown keys when ignoreUnknowns = false', () => {
const type = schema.object(
{ foo: schema.string({ defaultValue: 'test' }) },
{ ignoreUnknowns: false }
);
expect(() =>
type.validate({
bar: 'baz',
})
).toThrowErrorMatchingInlineSnapshot(`"[bar]: definition for this key is missing"`);
});
32 changes: 11 additions & 21 deletions packages/kbn-config-schema/src/types/object_type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,23 +30,16 @@ export type TypeOf<RT extends Type<any>> = RT['type'];
// this might not have perfect _rendering_ output, but it will be typed.
export type ObjectResultType<P extends Props> = Readonly<{ [K in keyof P]: TypeOf<P[K]> }>;

interface AllowUnknowns {
allowUnknowns: true;
ignoreUnknowns?: false;
interface UnknownOptions {
/**
* Options for dealing with unknown keys:
* - allow: unknown keys will be permitted
* - ignore: unknown keys will not fail validation, but will be stripped out
* - forbid (default): unknown keys will fail validation
*/
unknowns?: 'allow' | 'ignore' | 'forbid';
}

interface IgnoreUnknowns {
allowUnknowns?: false;
ignoreUnknowns: true;
}

interface ForbidUnknowns {
allowUnknowns?: false;
ignoreUnknowns?: false;
}
// type check to not permit both to be set to `true` at the same time
type UnknownOptions = AllowUnknowns | IgnoreUnknowns | ForbidUnknowns;

export type ObjectTypeOptions<P extends Props = any> = TypeOptions<
{ [K in keyof P]: TypeOf<P[K]> }
> &
Expand All @@ -55,10 +48,7 @@ export type ObjectTypeOptions<P extends Props = any> = TypeOptions<
export class ObjectType<P extends Props = any> extends Type<ObjectResultType<P>> {
private props: Record<string, AnySchema>;

constructor(
props: P,
{ allowUnknowns = false, ignoreUnknowns = false, ...typeOptions }: ObjectTypeOptions<P> = {}
) {
constructor(props: P, { unknowns = 'forbid', ...typeOptions }: ObjectTypeOptions<P> = {}) {
const schemaKeys = {} as Record<string, AnySchema>;
for (const [key, value] of Object.entries(props)) {
schemaKeys[key] = value.getSchema();
Expand All @@ -68,8 +58,8 @@ export class ObjectType<P extends Props = any> extends Type<ObjectResultType<P>>
.keys(schemaKeys)
.default()
.optional()
.unknown(Boolean(allowUnknowns))
.options({ stripUnknown: { objects: ignoreUnknowns } });
.unknown(unknowns === 'allow')
.options({ stripUnknown: { objects: unknowns === 'ignore' } });

super(schema, typeOptions);
this.props = schemaKeys;
Expand Down
4 changes: 2 additions & 2 deletions src/core/server/http/router/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ export interface RouteConfig<P, Q, B, Method extends RouteMethod> {
* access to raw values.
* In some cases you may want to use another validation library. To do this, you need to
* instruct the `@kbn/config-schema` library to output **non-validated values** with
* setting schema as `schema.object({}, { allowUnknowns: true })`;
* setting schema as `schema.object({}, { unknowns: 'allow' })`;
*
* @example
* ```ts
Expand Down Expand Up @@ -210,7 +210,7 @@ export interface RouteConfig<P, Q, B, Method extends RouteMethod> {
* path: 'path/{id}',
* validate: {
* // handler has access to raw non-validated params in runtime
* params: schema.object({}, { allowUnknowns: true })
* params: schema.object({}, { unknowns: 'allow' })
* },
* },
* (context, req, res,) {
Expand Down
2 changes: 1 addition & 1 deletion src/core/server/http/router/router.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ describe('Router', () => {
{
path: '/',
options: { body: { output: 'file' } } as any, // We explicitly don't support 'file'
validate: { body: schema.object({}, { allowUnknowns: true }) },
validate: { body: schema.object({}, { unknowns: 'allow' }) },
},
(context, req, res) => res.ok({})
)
Expand Down
2 changes: 1 addition & 1 deletion src/core/server/ui_settings/routes/set_many.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { CannotOverrideError } from '../ui_settings_errors';

const validate = {
body: schema.object({
changes: schema.object({}, { allowUnknowns: true }),
changes: schema.object({}, { unknowns: 'allow' }),
}),
};

Expand Down
2 changes: 1 addition & 1 deletion src/core/server/ui_settings/ui_settings_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ const configSchema = schema.object({
})
),
},
{ allowUnknowns: true }
{ unknowns: 'allow' }
),
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,15 @@ export function registerValueSuggestionsRoute(
{
index: schema.string(),
},
{ allowUnknowns: false }
{ unknowns: 'allow' }
),
body: schema.object(
{
field: schema.string(),
query: schema.string(),
boolFilter: schema.maybe(schema.any()),
},
{ allowUnknowns: false }
{ unknowns: 'allow' }
),
},
},
Expand Down
6 changes: 3 additions & 3 deletions src/plugins/data/server/search/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ export function registerSearchRoute(router: IRouter): void {
validate: {
params: schema.object({ strategy: schema.string() }),

query: schema.object({}, { allowUnknowns: true }),
query: schema.object({}, { unknowns: 'allow' }),

body: schema.object({}, { allowUnknowns: true }),
body: schema.object({}, { unknowns: 'allow' }),
},
},
async (context, request, res) => {
Expand Down Expand Up @@ -64,7 +64,7 @@ export function registerSearchRoute(router: IRouter): void {
id: schema.string(),
}),

query: schema.object({}, { allowUnknowns: true }),
query: schema.object({}, { unknowns: 'allow' }),
},
},
async (context, request, res) => {
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/timelion/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export const configSchema = schema.object(
graphiteUrls: schema.maybe(schema.arrayOf(schema.string())),
},
// This option should be removed as soon as we entirely migrate config from legacy Timelion plugin.
{ allowUnknowns: true }
{ unknowns: 'allow' }
);

export type ConfigSchema = TypeOf<typeof configSchema>;
12 changes: 4 additions & 8 deletions src/plugins/timelion/server/routes/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,15 +78,11 @@ export function runRoute(
es: schema.object({
filter: schema.object({
bool: schema.object({
filter: schema.maybe(
schema.arrayOf(schema.object({}, { allowUnknowns: true }))
),
must: schema.maybe(schema.arrayOf(schema.object({}, { allowUnknowns: true }))),
should: schema.maybe(
schema.arrayOf(schema.object({}, { allowUnknowns: true }))
),
filter: schema.maybe(schema.arrayOf(schema.object({}, { unknowns: 'allow' }))),
must: schema.maybe(schema.arrayOf(schema.object({}, { unknowns: 'allow' }))),
should: schema.maybe(schema.arrayOf(schema.object({}, { unknowns: 'allow' }))),
must_not: schema.maybe(
schema.arrayOf(schema.object({}, { allowUnknowns: true }))
schema.arrayOf(schema.object({}, { unknowns: 'allow' }))
),
}),
}),
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/vis_type_timeseries/server/routes/vis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { getVisData, GetVisDataOptions } from '../lib/get_vis_data';
import { visPayloadSchema } from './post_vis_schema';
import { Framework, ValidationTelemetryServiceSetup } from '../index';

const escapeHatch = schema.object({}, { allowUnknowns: true });
const escapeHatch = schema.object({}, { unknowns: 'allow' });

export const visDataRoutes = (
router: IRouter,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export class RenderingPlugin implements Plugin {
{
includeUserSettings: schema.boolean({ defaultValue: true }),
},
{ allowUnknowns: true }
{ unknowns: 'allow' }
),
params: schema.object({
id: schema.maybe(schema.string()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export function registerLicenseRoute(server: Server, legacy: Legacy, xpackInfo:
validate: {
query: schema.object({ acknowledge: schema.string() }),
body: schema.object({
license: schema.object({}, { allowUnknowns: true }),
license: schema.object({}, { unknowns: 'allow' }),
}),
},
},
Expand Down
2 changes: 1 addition & 1 deletion x-pack/legacy/plugins/rollup/server/routes/api/jobs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ export function registerJobsRoute(deps: RouteDependencies, legacy: ServerShim) {
{
id: schema.string(),
},
{ allowUnknowns: true }
{ unknowns: 'allow' }
),
}),
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,13 @@ export const signalRulesAlertType = ({
savedId: schema.nullable(schema.string()),
timelineId: schema.nullable(schema.string()),
timelineTitle: schema.nullable(schema.string()),
meta: schema.nullable(schema.object({}, { allowUnknowns: true })),
meta: schema.nullable(schema.object({}, { unknowns: 'allow' })),
query: schema.nullable(schema.string()),
filters: schema.nullable(schema.arrayOf(schema.object({}, { allowUnknowns: true }))),
filters: schema.nullable(schema.arrayOf(schema.object({}, { unknowns: 'allow' }))),
maxSignals: schema.number({ defaultValue: DEFAULT_MAX_SIGNALS }),
riskScore: schema.number(),
severity: schema.string(),
threat: schema.nullable(schema.arrayOf(schema.object({}, { allowUnknowns: true }))),
threat: schema.nullable(schema.arrayOf(schema.object({}, { unknowns: 'allow' }))),
to: schema.string(),
type: schema.string(),
references: schema.arrayOf(schema.string(), { defaultValue: [] }),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export class KibanaBackendFrameworkAdapter implements FrameworkAdapter {
this.router.post(
{
path: routePath,
validate: { body: configSchema.object({}, { allowUnknowns: true }) },
validate: { body: configSchema.object({}, { unknowns: 'allow' }) },
options: {
tags: ['access:siem'],
},
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/apm/server/routes/create_api/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export function createApi() {
body: bodyRt || t.null
};

const anyObject = schema.object({}, { allowUnknowns: true });
const anyObject = schema.object({}, { unknowns: 'allow' });

(router[routerMethod] as RouteRegistrar<typeof routerMethod>)(
{
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/canvas/server/routes/workpad/update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ export function initializeUpdateWorkpadAssetsRoute(deps: RouteInitializerDeps) {
// ToDo: Currently the validation must be a schema.object
// Because we don't know what keys the assets will have, we have to allow
// unknowns and then validate in the handler
body: schema.object({}, { allowUnknowns: true }),
body: schema.object({}, { unknowns: 'allow' }),
},
options: {
body: {
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/case/server/routes/api/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,4 +134,4 @@ export const sortToSnake = (sortField: string): SortFieldCase => {
}
};

export const escapeHatch = schema.object({}, { allowUnknowns: true });
export const escapeHatch = schema.object({}, { unknowns: 'allow' });
Loading

0 comments on commit c74167e

Please sign in to comment.