[Connectors] Checking for undefined config and secrets during connect…

…or validation (#122696) (#123369) * Checking for undefined config and secrets during connector validation * Adding functional tests Co-authored-by: Kibana Machine <[email protected]> (cherry picked from commit d19a2f3) Co-authored-by: ymao1 <[email protected]>
elastic · Jan 19, 2022 · ec0314b · ec0314b
1 parent 3411dc3
commit ec0314b
Show file tree

Hide file tree

Showing 5 changed files with 132 additions and 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
@@ -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
@@ -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<string, unknown>) => 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
@@ -42,10 +42,22 @@ export function validateConnector<
   ExecutorResultData = void
 >(actionType: ActionType<Config, Secrets, Params, ExecutorResultData>, 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
@@ -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: ['[email protected]'],
+            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
@@ -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" : "[email protected]",
+          "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"
+    }
+  }
+}