From d41df11804ed8401f226cf886aa429bbb59c01df Mon Sep 17 00:00:00 2001 From: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Date: Tue, 16 Feb 2021 15:38:07 -0500 Subject: [PATCH] Further optimize check privileges response validation (#90631) (#91530) Co-authored-by: Larry Gregory --- .../validate_es_response.test.ts.snap | 6 +-- .../authorization/check_privileges.test.ts | 12 ++--- .../authorization/validate_es_response.ts | 45 ++++++++++++------- 3 files changed, 38 insertions(+), 25 deletions(-) diff --git a/x-pack/plugins/security/server/authorization/__snapshots__/validate_es_response.test.ts.snap b/x-pack/plugins/security/server/authorization/__snapshots__/validate_es_response.test.ts.snap index 76d284a21984e..04190fbf5eacd 100644 --- a/x-pack/plugins/security/server/authorization/__snapshots__/validate_es_response.test.ts.snap +++ b/x-pack/plugins/security/server/authorization/__snapshots__/validate_es_response.test.ts.snap @@ -1,12 +1,12 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`validateEsPrivilegeResponse fails validation when an action is malformed in the response 1`] = `"Invalid response received from Elasticsearch has_privilege endpoint. Error: [application.foo-application]: [action3]: expected value of type [boolean] but got [string]"`; +exports[`validateEsPrivilegeResponse fails validation when an action is malformed in the response 1`] = `"Invalid response received from Elasticsearch has_privilege endpoint. Error: [application.foo-application]: expected value of type [boolean] but got [string]"`; -exports[`validateEsPrivilegeResponse fails validation when an action is missing in the response 1`] = `"Invalid response received from Elasticsearch has_privilege endpoint. Error: [application.foo-application]: [action2]: expected value of type [boolean] but got [undefined]"`; +exports[`validateEsPrivilegeResponse fails validation when an action is missing in the response 1`] = `"Invalid response received from Elasticsearch has_privilege endpoint. Error: [application.foo-application]: Payload did not match expected actions"`; exports[`validateEsPrivilegeResponse fails validation when an expected resource property is missing from the response 1`] = `"Invalid response received from Elasticsearch has_privilege endpoint. Error: [application.foo-application]: Payload did not match expected resources"`; -exports[`validateEsPrivilegeResponse fails validation when an extra action is present in the response 1`] = `"Invalid response received from Elasticsearch has_privilege endpoint. Error: [application.foo-application]: [action4]: definition for this key is missing"`; +exports[`validateEsPrivilegeResponse fails validation when an extra action is present in the response 1`] = `"Invalid response received from Elasticsearch has_privilege endpoint. Error: [application.foo-application]: Payload did not match expected actions"`; exports[`validateEsPrivilegeResponse fails validation when an extra application is present in the response 1`] = `"Invalid response received from Elasticsearch has_privilege endpoint. Error: [application.otherApplication]: definition for this key is missing"`; diff --git a/x-pack/plugins/security/server/authorization/check_privileges.test.ts b/x-pack/plugins/security/server/authorization/check_privileges.test.ts index 93f5efed58fb8..5bca46f22a512 100644 --- a/x-pack/plugins/security/server/authorization/check_privileges.test.ts +++ b/x-pack/plugins/security/server/authorization/check_privileges.test.ts @@ -316,7 +316,7 @@ describe('#atSpace', () => { }, }); expect(result).toMatchInlineSnapshot( - `[Error: Invalid response received from Elasticsearch has_privilege endpoint. Error: [application.kibana-our_application]: [saved_object:bar-type/get]: definition for this key is missing]` + `[Error: Invalid response received from Elasticsearch has_privilege endpoint. Error: [application.kibana-our_application]: Payload did not match expected actions]` ); }); @@ -338,7 +338,7 @@ describe('#atSpace', () => { }, }); expect(result).toMatchInlineSnapshot( - `[Error: Invalid response received from Elasticsearch has_privilege endpoint. Error: [application.kibana-our_application]: [saved_object:foo-type/get]: expected value of type [boolean] but got [undefined]]` + `[Error: Invalid response received from Elasticsearch has_privilege endpoint. Error: [application.kibana-our_application]: Payload did not match expected actions]` ); }); }); @@ -1092,7 +1092,7 @@ describe('#atSpaces', () => { }, }); expect(result).toMatchInlineSnapshot( - `[Error: Invalid response received from Elasticsearch has_privilege endpoint. Error: [application.kibana-our_application]: [mock-action:version]: expected value of type [boolean] but got [undefined]]` + `[Error: Invalid response received from Elasticsearch has_privilege endpoint. Error: [application.kibana-our_application]: Payload did not match expected actions]` ); }); @@ -2266,7 +2266,7 @@ describe('#globally', () => { }, }); expect(result).toMatchInlineSnapshot( - `[Error: Invalid response received from Elasticsearch has_privilege endpoint. Error: [application.kibana-our_application]: [mock-action:version]: expected value of type [boolean] but got [undefined]]` + `[Error: Invalid response received from Elasticsearch has_privilege endpoint. Error: [application.kibana-our_application]: Payload did not match expected actions]` ); }); @@ -2384,7 +2384,7 @@ describe('#globally', () => { }, }); expect(result).toMatchInlineSnapshot( - `[Error: Invalid response received from Elasticsearch has_privilege endpoint. Error: [application.kibana-our_application]: [saved_object:bar-type/get]: definition for this key is missing]` + `[Error: Invalid response received from Elasticsearch has_privilege endpoint. Error: [application.kibana-our_application]: Payload did not match expected actions]` ); }); @@ -2405,7 +2405,7 @@ describe('#globally', () => { }, }); expect(result).toMatchInlineSnapshot( - `[Error: Invalid response received from Elasticsearch has_privilege endpoint. Error: [application.kibana-our_application]: [saved_object:foo-type/get]: expected value of type [boolean] but got [undefined]]` + `[Error: Invalid response received from Elasticsearch has_privilege endpoint. Error: [application.kibana-our_application]: Payload did not match expected actions]` ); }); }); diff --git a/x-pack/plugins/security/server/authorization/validate_es_response.ts b/x-pack/plugins/security/server/authorization/validate_es_response.ts index 19afaaf035c15..270ff26716e3f 100644 --- a/x-pack/plugins/security/server/authorization/validate_es_response.ts +++ b/x-pack/plugins/security/server/authorization/validate_es_response.ts @@ -8,6 +8,11 @@ import { schema } from '@kbn/config-schema'; import { HasPrivilegesResponse } from './types'; +/** + * Validates an Elasticsearch "Has privileges" response against the expected application, actions, and resources. + * + * Note: the `actions` and `resources` parameters must be unique string arrays; any duplicates will cause validation to fail. + */ export function validateEsPrivilegeResponse( response: HasPrivilegesResponse, application: string, @@ -24,21 +29,29 @@ export function validateEsPrivilegeResponse( return response; } -function buildActionsValidationSchema(actions: string[]) { - return schema.object({ - ...actions.reduce>((acc, action) => { - return { - ...acc, - [action]: schema.boolean(), - }; - }, {}), - }); -} - function buildValidationSchema(application: string, actions: string[], resources: string[]) { - const actionValidationSchema = buildActionsValidationSchema(actions); + const actionValidationSchema = schema.boolean(); + const actionsValidationSchema = schema.object( + {}, + { + unknowns: 'allow', + validate: (value) => { + const actualActions = Object.keys(value).sort(); + if ( + actions.length !== actualActions.length || + ![...actions].sort().every((x, i) => x === actualActions[i]) + ) { + throw new Error('Payload did not match expected actions'); + } + + Object.values(value).forEach((actionResult) => { + actionValidationSchema.validate(actionResult); + }); + }, + } + ); - const resourceValidationSchema = schema.object( + const resourcesValidationSchema = schema.object( {}, { unknowns: 'allow', @@ -46,13 +59,13 @@ function buildValidationSchema(application: string, actions: string[], resources const actualResources = Object.keys(value).sort(); if ( resources.length !== actualResources.length || - !resources.sort().every((x, i) => x === actualResources[i]) + ![...resources].sort().every((x, i) => x === actualResources[i]) ) { throw new Error('Payload did not match expected resources'); } Object.values(value).forEach((actionResult) => { - actionValidationSchema.validate(actionResult); + actionsValidationSchema.validate(actionResult); }); }, } @@ -63,7 +76,7 @@ function buildValidationSchema(application: string, actions: string[], resources has_all_requested: schema.boolean(), cluster: schema.object({}, { unknowns: 'allow' }), application: schema.object({ - [application]: resourceValidationSchema, + [application]: resourcesValidationSchema, }), index: schema.object({}, { unknowns: 'allow' }), });