From 5dad0f260ee513fa193b6730626de420999d7657 Mon Sep 17 00:00:00 2001 From: ymao1 Date: Wed, 19 Jan 2022 09:45:19 -0500 Subject: [PATCH] [Connectors] Checking for undefined config and secrets during connector validation (#122696) * Checking for undefined config and secrets during connector validation * Adding functional tests Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit d19a2f3d05875bc6cafaf16fbea9c699bda0a407) --- .../server/lib/action_executor.test.ts | 2 +- .../server/lib/validate_with_schema.test.ts | 57 +++++++++++++++++-- .../server/lib/validate_with_schema.ts | 20 +++++-- .../spaces_only/tests/actions/migrations.ts | 30 ++++++++++ .../functional/es_archives/actions/data.json | 32 +++++++++++ 5 files changed, 132 insertions(+), 9 deletions(-) diff --git a/x-pack/plugins/actions/server/lib/action_executor.test.ts b/x-pack/plugins/actions/server/lib/action_executor.test.ts index 4175649454f71..30d4ed92e03f8 100644 --- a/x-pack/plugins/actions/server/lib/action_executor.test.ts +++ b/x-pack/plugins/actions/server/lib/action_executor.test.ts @@ -308,7 +308,7 @@ test('throws an error when connector is invalid', async () => { actionId: '1', status: 'error', retry: false, - message: `error validating action type connector: error`, + message: `error validating action type connector: config must be defined`, }); }); diff --git a/x-pack/plugins/actions/server/lib/validate_with_schema.test.ts b/x-pack/plugins/actions/server/lib/validate_with_schema.test.ts index 4f0a11252eb48..49db953506d22 100644 --- a/x-pack/plugins/actions/server/lib/validate_with_schema.test.ts +++ b/x-pack/plugins/actions/server/lib/validate_with_schema.test.ts @@ -68,6 +68,7 @@ test('should validate when validators return incoming value', () => { params: selfValidator, config: selfValidator, secrets: selfValidator, + connector: () => null, }, }; @@ -83,7 +84,7 @@ test('should validate when validators return incoming value', () => { result = validateSecrets(actionType, testValue); expect(result).toEqual(testValue); - result = validateConnector(actionType, { config: testValue }); + result = validateConnector(actionType, { config: testValue, secrets: { user: 'test' } }); expect(result).toBeNull(); }); @@ -99,6 +100,7 @@ test('should validate when validators return different values', () => { params: selfValidator, config: selfValidator, secrets: selfValidator, + connector: () => null, }, }; @@ -133,9 +135,7 @@ test('should throw with expected error when validators fail', () => { params: erroringValidator, config: erroringValidator, secrets: erroringValidator, - connector: () => { - return 'test error'; - }, + connector: () => 'test error', }, }; @@ -180,3 +180,52 @@ test('should work with @kbn/config-schema', () => { `"error validating action params: [foo]: expected value of type [string] but got [undefined]"` ); }); + +describe('validateConnectors', () => { + const testValue = { any: ['old', 'thing'] }; + const selfValidator = { validate: (value: Record) => value }; + const actionType: ActionType = { + id: 'foo', + name: 'bar', + minimumLicenseRequired: 'basic', + executor, + validate: { + params: selfValidator, + config: selfValidator, + secrets: selfValidator, + connector: () => null, + }, + }; + + test('should throw error when connector config is null', () => { + expect(() => + validateConnector(actionType, { config: null, secrets: { user: 'test' } }) + ).toThrowErrorMatchingInlineSnapshot( + `"error validating action type connector: config must be defined"` + ); + }); + + test('should throw error when connector config is undefined', () => { + expect(() => + validateConnector(actionType, { config: undefined, secrets: { user: 'test' } }) + ).toThrowErrorMatchingInlineSnapshot( + `"error validating action type connector: config must be defined"` + ); + }); + + test('should throw error when connector secrets is null', () => { + expect(() => + validateConnector(actionType, { config: testValue, secrets: null }) + ).toThrowErrorMatchingInlineSnapshot( + `"error validating action type connector: secrets must be defined"` + ); + }); + + test('should throw error when connector secrets is undefined', () => { + expect(() => + validateConnector(actionType, { config: testValue, secrets: undefined }) + ).toThrowErrorMatchingInlineSnapshot( + `"error validating action type connector: secrets must be defined"` + ); + }); +}); diff --git a/x-pack/plugins/actions/server/lib/validate_with_schema.ts b/x-pack/plugins/actions/server/lib/validate_with_schema.ts index 8ff0a3666c4b7..b8a861566acd2 100644 --- a/x-pack/plugins/actions/server/lib/validate_with_schema.ts +++ b/x-pack/plugins/actions/server/lib/validate_with_schema.ts @@ -42,10 +42,22 @@ export function validateConnector< ExecutorResultData = void >(actionType: ActionType, value: unknown) { if (actionType.validate && actionType.validate.connector) { - const connectorValue = value as { config: Config; secrets: Secrets }; - const result = actionType.validate.connector(connectorValue.config, connectorValue.secrets); - if (result !== null) { - throw Boom.badRequest(`error validating action type connector: ${result}`); + try { + const connectorValue = value as { config: Config; secrets: Secrets }; + + // Connector config and secrets should be defined + if (connectorValue.config == null) { + throw new Error(`config must be defined`); + } + if (connectorValue.secrets == null) { + throw new Error(`secrets must be defined`); + } + const result = actionType.validate.connector(connectorValue.config, connectorValue.secrets); + if (result !== null) { + throw new Error(result); + } + } catch (err) { + throw Boom.badRequest(`error validating action type connector: ${err.message}`); } } return null; diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/actions/migrations.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/actions/migrations.ts index 9b88dace13239..7b28161c18238 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/actions/migrations.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/actions/migrations.ts @@ -82,5 +82,35 @@ export default function createGetTests({ getService }: FtrProviderContext) { expect(connectorWithoutService.body.config).key('service'); expect(connectorWithoutService.body.config.service).to.eql('other'); }); + + it('decryption error during migration', async () => { + const badEmailConnector = await supertest.get( + `${getUrlPrefix(``)}/api/actions/connector/0f8f2810-0a59-11ec-9a7c-fd0c2b83ff7d` + ); + + expect(badEmailConnector.status).to.eql(200); + expect(badEmailConnector.body.secrets).to.eql(undefined); + + const response = await supertest + .post( + `${getUrlPrefix(``)}/api/actions/connector/0f8f2810-0a59-11ec-9a7c-fd0c2b83ff7d/_execute` + ) + .set('kbn-xsrf', 'foo') + .send({ + params: { + message: 'am i working?', + to: ['user@test.com'], + subject: 'test', + }, + }); + + expect(response.status).to.eql(200); + expect(response.body).to.eql({ + connector_id: '0f8f2810-0a59-11ec-9a7c-fd0c2b83ff7d', + status: 'error', + message: `error validating action type connector: secrets must be defined`, + retry: false, + }); + }); }); } diff --git a/x-pack/test/functional/es_archives/actions/data.json b/x-pack/test/functional/es_archives/actions/data.json index 31d10005c0939..79e7920872ab0 100644 --- a/x-pack/test/functional/es_archives/actions/data.json +++ b/x-pack/test/functional/es_archives/actions/data.json @@ -174,3 +174,35 @@ } } } + +{ + "type": "doc", + "value": { + "id": "action:0f8f2810-0a59-11ec-9a7c-fd0c2b83ff7d", + "index": ".kibana_1", + "source": { + "action": { + "actionTypeId" : ".email", + "name" : "test email connector cant decrypt", + "isMissingSecrets" : false, + "config" : { + "hasAuth" : true, + "from" : "me@test.com", + "host" : "some.non.existent.com", + "port" : 465, + "service" : "other", + "secure" : true + }, + "secrets" : "V2EJEtTv3yTFi1kdglhNahasdfnKYWCS+J7aWCJQU+eEqGPZEz6n7G1NsBWoh7IY0FteLTilTteQXyY/Eg3k/7bb0G8Mz+WBZ1mRvUggGTFqgoOptyUsvHoBhv0R/1bCTCabN3Pe88AfnC+VDXqwuMifpmgKEEsKF3H8VONv7TYO02FW" + }, + "migrationVersion": { + "action": "7.14.0" + }, + "coreMigrationVersion" : "7.15.0", + "references": [ + ], + "type": "action", + "updated_at": "2021-08-31T12:43:37.117Z" + } + } +} \ No newline at end of file