[Alerting] Added proper check to see if TLS is required using security API for apiKeys: areAPIKeysEnabled (#97317)

* replaced xpack usage calls with the security API areAPIKeysEnabled * fixed typechecks Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
2021-04-19 09:04:34 -07:00 · 2021-04-19 09:04:34 -07:00 · 5141f598dc
parent 4ab22f9336
commit 5141f598dc
7 changed files with 42 additions and 188 deletions
--- a/x-pack/plugins/alerting/server/plugin.ts
+++ b/x-pack/plugins/alerting/server/plugin.ts
@ -364,7 +364,7 @@ export class AlertingPlugin {
  }

  private createRouteHandlerContext = (
-    core: CoreSetup
+    core: CoreSetup<AlertingPluginsStart, unknown>
  ): IContextProvider<AlertingRequestHandlerContext, 'alerting'> => {
    const { alertTypeRegistry, alertsClientFactory } = this;
    return async function alertsRouteHandlerContext(context, request) {
@ -376,6 +376,10 @@ export class AlertingPlugin {
        listTypes: alertTypeRegistry!.list.bind(alertTypeRegistry!),
        getFrameworkHealth: async () =>
          await getHealth(savedObjects.createInternalRepository(['alert'])),
+        areApiKeysEnabled: async () => {
+          const [, { security }] = await core.getStartServices();
+          return security?.authc.apiKeys.areAPIKeysEnabled() ?? false;
+        },
      };
    };
  };
--- a/x-pack/plugins/alerting/server/routes/_mock_handler_arguments.ts
+++ b/x-pack/plugins/alerting/server/routes/_mock_handler_arguments.ts
@ -8,26 +8,23 @@
 import { KibanaRequest, KibanaResponseFactory } from 'kibana/server';
 import { identity } from 'lodash';
 import type { MethodKeysOf } from '@kbn/utility-types';
-// eslint-disable-next-line @kbn/eslint/no-restricted-paths
-import { ScopedClusterClientMock } from '../../../../../src/core/server/elasticsearch/client/mocks';
 import { httpServerMock } from '../../../../../src/core/server/mocks';
 import { alertsClientMock, AlertsClientMock } from '../alerts_client.mock';
 import { AlertsHealth, AlertType } from '../../common';
-import { elasticsearchServiceMock } from '../../../../../src/core/server/mocks';
 import type { AlertingRequestHandlerContext } from '../types';

 export function mockHandlerArguments(
  {
    alertsClient = alertsClientMock.create(),
    listTypes: listTypesRes = [],
-    esClient = elasticsearchServiceMock.createScopedClusterClient(),
    getFrameworkHealth,
+    areApiKeysEnabled,
  }: {
    alertsClient?: AlertsClientMock;
    listTypes?: AlertType[];
-    esClient?: jest.Mocked<ScopedClusterClientMock>;
    getFrameworkHealth?: jest.MockInstance<Promise<AlertsHealth>, []> &
      (() => Promise<AlertsHealth>);
+    areApiKeysEnabled?: () => Promise<boolean>;
  },
  req: unknown,
  res?: Array<MethodKeysOf<KibanaResponseFactory>>
@ -39,13 +36,13 @@ export function mockHandlerArguments(
  const listTypes = jest.fn(() => listTypesRes);
  return [
    ({
-      core: { elasticsearch: { client: esClient } },
      alerting: {
        listTypes,
        getAlertsClient() {
          return alertsClient || alertsClientMock.create();
        },
        getFrameworkHealth,
+        areApiKeysEnabled: areApiKeysEnabled ? areApiKeysEnabled : () => Promise.resolve(true),
      },
    } as unknown) as AlertingRequestHandlerContext,
    req as KibanaRequest<unknown, unknown, unknown>,
--- a/x-pack/plugins/alerting/server/routes/health.test.ts
+++ b/x-pack/plugins/alerting/server/routes/health.test.ts
@ -8,15 +8,12 @@
 import { healthRoute } from './health';
 import { httpServiceMock } from 'src/core/server/mocks';
 import { mockHandlerArguments } from './_mock_handler_arguments';
-import { elasticsearchServiceMock } from '../../../../../src/core/server/mocks';
 import { verifyApiAccess } from '../lib/license_api_access';
 import { licenseStateMock } from '../lib/license_state.mock';
 import { encryptedSavedObjectsMock } from '../../../encrypted_saved_objects/server/mocks';
 import { alertsClientMock } from '../alerts_client.mock';
 import { HealthStatus } from '../types';
 import { alertsMock } from '../mocks';
-// eslint-disable-next-line @kbn/eslint/no-restricted-paths
-import { elasticsearchClientMock } from '../../../../../src/core/server/elasticsearch/client/mocks';
 const alertsClient = alertsClientMock.create();

 jest.mock('../lib/license_api_access.ts', () => ({
@ -65,25 +62,11 @@ describe('healthRoute', () => {
    healthRoute(router, licenseState, encryptedSavedObjects);
    const [, handler] = router.get.mock.calls[0];

-    const esClient = elasticsearchServiceMock.createScopedClusterClient();
-    esClient.asInternalUser.transport.request.mockReturnValue(
-      elasticsearchClientMock.createSuccessTransportRequestPromise({})
-    );
-
-    const [context, req, res] = mockHandlerArguments({ esClient, alertsClient }, {}, ['ok']);
+    const [context, req, res] = mockHandlerArguments({ alertsClient }, {}, ['ok']);

    await handler(context, req, res);

    expect(verifyApiAccess).toHaveBeenCalledWith(licenseState);
-
-    expect(esClient.asInternalUser.transport.request.mock.calls[0]).toMatchInlineSnapshot(`
-      Array [
-        Object {
-          "method": "GET",
-          "path": "/_xpack/usage",
-        },
-      ]
-    `);
  });

  it('evaluates whether Encrypted Saved Objects is missing encryption key', async () => {
@ -94,13 +77,8 @@ describe('healthRoute', () => {
    healthRoute(router, licenseState, encryptedSavedObjects);
    const [, handler] = router.get.mock.calls[0];

-    const esClient = elasticsearchServiceMock.createScopedClusterClient();
-    esClient.asInternalUser.transport.request.mockReturnValue(
-      elasticsearchClientMock.createSuccessTransportRequestPromise({})
-    );
-
    const [context, req, res] = mockHandlerArguments(
-      { esClient, alertsClient, getFrameworkHealth: alerting.getFrameworkHealth },
+      { alertsClient, getFrameworkHealth: alerting.getFrameworkHealth },
      {},
      ['ok']
    );
@ -135,13 +113,8 @@ describe('healthRoute', () => {
    healthRoute(router, licenseState, encryptedSavedObjects);
    const [, handler] = router.get.mock.calls[0];

-    const esClient = elasticsearchServiceMock.createScopedClusterClient();
-    esClient.asInternalUser.transport.request.mockReturnValue(
-      elasticsearchClientMock.createSuccessTransportRequestPromise({})
-    );
-
    const [context, req, res] = mockHandlerArguments(
-      { esClient, alertsClient, getFrameworkHealth: alerting.getFrameworkHealth },
+      { alertsClient, getFrameworkHealth: alerting.getFrameworkHealth },
      {},
      ['ok']
    );
@ -176,13 +149,8 @@ describe('healthRoute', () => {
    healthRoute(router, licenseState, encryptedSavedObjects);
    const [, handler] = router.get.mock.calls[0];

-    const esClient = elasticsearchServiceMock.createScopedClusterClient();
-    esClient.asInternalUser.transport.request.mockReturnValue(
-      elasticsearchClientMock.createSuccessTransportRequestPromise({ security: {} })
-    );
-
    const [context, req, res] = mockHandlerArguments(
-      { esClient, alertsClient, getFrameworkHealth: alerting.getFrameworkHealth },
+      { alertsClient, getFrameworkHealth: alerting.getFrameworkHealth },
      {},
      ['ok']
    );
@ -217,13 +185,12 @@ describe('healthRoute', () => {
    healthRoute(router, licenseState, encryptedSavedObjects);
    const [, handler] = router.get.mock.calls[0];

-    const esClient = elasticsearchServiceMock.createScopedClusterClient();
-    esClient.asInternalUser.transport.request.mockReturnValue(
-      elasticsearchClientMock.createSuccessTransportRequestPromise({ security: { enabled: true } })
-    );
-
    const [context, req, res] = mockHandlerArguments(
-      { esClient, alertsClient, getFrameworkHealth: alerting.getFrameworkHealth },
+      {
+        alertsClient,
+        getFrameworkHealth: alerting.getFrameworkHealth,
+        areApiKeysEnabled: () => Promise.resolve(false),
+      },
      {},
      ['ok']
    );
@ -258,15 +225,12 @@ describe('healthRoute', () => {
    healthRoute(router, licenseState, encryptedSavedObjects);
    const [, handler] = router.get.mock.calls[0];

-    const esClient = elasticsearchServiceMock.createScopedClusterClient();
-    esClient.asInternalUser.transport.request.mockReturnValue(
-      elasticsearchClientMock.createSuccessTransportRequestPromise({
-        security: { enabled: true, ssl: {} },
-      })
-    );
-
    const [context, req, res] = mockHandlerArguments(
-      { esClient, alertsClient, getFrameworkHealth: alerting.getFrameworkHealth },
+      {
+        alertsClient,
+        getFrameworkHealth: alerting.getFrameworkHealth,
+        areApiKeysEnabled: () => Promise.resolve(false),
+      },
      {},
      ['ok']
    );
@ -301,15 +265,8 @@ describe('healthRoute', () => {
    healthRoute(router, licenseState, encryptedSavedObjects);
    const [, handler] = router.get.mock.calls[0];

-    const esClient = elasticsearchServiceMock.createScopedClusterClient();
-    esClient.asInternalUser.transport.request.mockReturnValue(
-      elasticsearchClientMock.createSuccessTransportRequestPromise({
-        security: { enabled: true, ssl: { http: { enabled: true } } },
-      })
-    );
-
    const [context, req, res] = mockHandlerArguments(
-      { esClient, alertsClient, getFrameworkHealth: alerting.getFrameworkHealth },
+      { alertsClient, getFrameworkHealth: alerting.getFrameworkHealth },
      {},
      ['ok']
    );
--- a/x-pack/plugins/alerting/server/routes/health.ts
+++ b/x-pack/plugins/alerting/server/routes/health.ts
@ -5,7 +5,6 @@
 * 2.0.
 */

-import { ApiResponse } from '@elastic/elasticsearch';
 import { IRouter } from 'kibana/server';
 import { ILicenseState } from '../lib';
 import { EncryptedSavedObjectsPluginSetup } from '../../../encrypted_saved_objects/server';
@ -16,17 +15,6 @@ import {
  AlertingFrameworkHealth,
 } from '../types';

-interface XPackUsageSecurity {
-  security?: {
-    enabled?: boolean;
-    ssl?: {
-      http?: {
-        enabled?: boolean;
-      };
-    };
-  };
-}
-
 const rewriteBodyRes: RewriteResponseCase<AlertingFrameworkHealth> = ({
  isSufficientlySecure,
  hasPermanentEncryptionKey,
@ -56,23 +44,11 @@ export const healthRoute = (
    router.handleLegacyErrors(
      verifyAccessAndContext(licenseState, async function (context, req, res) {
        try {
-          const {
-            body: {
-              security: {
-                enabled: isSecurityEnabled = false,
-                ssl: { http: { enabled: isTLSEnabled = false } = {} } = {},
-              } = {},
-            },
-          }: ApiResponse<XPackUsageSecurity> = await context.core.elasticsearch.client.asInternalUser.transport // Do not augment with such input. // `transport.request` is potentially unsafe when combined with untrusted user input.
-            .request({
-              method: 'GET',
-              path: '/_xpack/usage',
-            });
-
+          const areApiKeysEnabled = await context.alerting.areApiKeysEnabled();
          const alertingFrameworkHeath = await context.alerting.getFrameworkHealth();

          const frameworkHealth: AlertingFrameworkHealth = {
-            isSufficientlySecure: !isSecurityEnabled || (isSecurityEnabled && isTLSEnabled),
+            isSufficientlySecure: areApiKeysEnabled,
            hasPermanentEncryptionKey: encryptedSavedObjects.canEncrypt,
            alertingFrameworkHeath,
          };
--- a/x-pack/plugins/alerting/server/routes/legacy/health.test.ts
+++ b/x-pack/plugins/alerting/server/routes/legacy/health.test.ts
@ -8,8 +8,6 @@
 import { healthRoute } from './health';
 import { httpServiceMock } from 'src/core/server/mocks';
 import { mockHandlerArguments } from './../_mock_handler_arguments';
-import { elasticsearchServiceMock } from '../../../../../../src/core/server/mocks';
-import { verifyApiAccess } from '../../lib/license_api_access';
 import { licenseStateMock } from '../../lib/license_state.mock';
 import { encryptedSavedObjectsMock } from '../../../../encrypted_saved_objects/server/mocks';
 import { alertsClientMock } from '../../alerts_client.mock';
@ -55,35 +53,6 @@ describe('healthRoute', () => {
    expect(config.path).toMatchInlineSnapshot(`"/api/alerts/_health"`);
  });

-  it('queries the usage api', async () => {
-    const router = httpServiceMock.createRouter();
-
-    const licenseState = licenseStateMock.create();
-    const encryptedSavedObjects = encryptedSavedObjectsMock.createSetup({ canEncrypt: true });
-    healthRoute(router, licenseState, encryptedSavedObjects);
-    const [, handler] = router.get.mock.calls[0];
-
-    const esClient = elasticsearchServiceMock.createScopedClusterClient();
-    esClient.asInternalUser.transport.request.mockReturnValue(
-      elasticsearchServiceMock.createSuccessTransportRequestPromise({})
-    );
-
-    const [context, req, res] = mockHandlerArguments({ esClient, alertsClient }, {}, ['ok']);
-
-    await handler(context, req, res);
-
-    expect(verifyApiAccess).toHaveBeenCalledWith(licenseState);
-
-    expect(esClient.asInternalUser.transport.request.mock.calls[0]).toMatchInlineSnapshot(`
-      Array [
-        Object {
-          "method": "GET",
-          "path": "/_xpack/usage",
-        },
-      ]
-    `);
-  });
-
  it('evaluates whether Encrypted Saved Objects is missing encryption key', async () => {
    const router = httpServiceMock.createRouter();

@ -92,13 +61,8 @@ describe('healthRoute', () => {
    healthRoute(router, licenseState, encryptedSavedObjects);
    const [, handler] = router.get.mock.calls[0];

-    const esClient = elasticsearchServiceMock.createScopedClusterClient();
-    esClient.asInternalUser.transport.request.mockReturnValue(
-      elasticsearchServiceMock.createSuccessTransportRequestPromise({})
-    );
-
    const [context, req, res] = mockHandlerArguments(
-      { esClient, alertsClient, getFrameworkHealth: alerting.getFrameworkHealth },
+      { alertsClient, getFrameworkHealth: alerting.getFrameworkHealth },
      {},
      ['ok']
    );
@ -133,13 +97,8 @@ describe('healthRoute', () => {
    healthRoute(router, licenseState, encryptedSavedObjects);
    const [, handler] = router.get.mock.calls[0];

-    const esClient = elasticsearchServiceMock.createScopedClusterClient();
-    esClient.asInternalUser.transport.request.mockReturnValue(
-      elasticsearchServiceMock.createSuccessTransportRequestPromise({})
-    );
-
    const [context, req, res] = mockHandlerArguments(
-      { esClient, alertsClient, getFrameworkHealth: alerting.getFrameworkHealth },
+      { alertsClient, getFrameworkHealth: alerting.getFrameworkHealth },
      {},
      ['ok']
    );
@ -174,13 +133,8 @@ describe('healthRoute', () => {
    healthRoute(router, licenseState, encryptedSavedObjects);
    const [, handler] = router.get.mock.calls[0];

-    const esClient = elasticsearchServiceMock.createScopedClusterClient();
-    esClient.asInternalUser.transport.request.mockReturnValue(
-      elasticsearchServiceMock.createSuccessTransportRequestPromise({})
-    );
-
    const [context, req, res] = mockHandlerArguments(
-      { esClient, alertsClient, getFrameworkHealth: alerting.getFrameworkHealth },
+      { alertsClient, getFrameworkHealth: alerting.getFrameworkHealth },
      {},
      ['ok']
    );
@ -215,13 +169,12 @@ describe('healthRoute', () => {
    healthRoute(router, licenseState, encryptedSavedObjects);
    const [, handler] = router.get.mock.calls[0];

-    const esClient = elasticsearchServiceMock.createScopedClusterClient();
-    esClient.asInternalUser.transport.request.mockReturnValue(
-      elasticsearchServiceMock.createSuccessTransportRequestPromise({ security: { enabled: true } })
-    );
-
    const [context, req, res] = mockHandlerArguments(
-      { esClient, alertsClient, getFrameworkHealth: alerting.getFrameworkHealth },
+      {
+        alertsClient,
+        getFrameworkHealth: alerting.getFrameworkHealth,
+        areApiKeysEnabled: () => Promise.resolve(false),
+      },
      {},
      ['ok']
    );
@ -256,15 +209,12 @@ describe('healthRoute', () => {
    healthRoute(router, licenseState, encryptedSavedObjects);
    const [, handler] = router.get.mock.calls[0];

-    const esClient = elasticsearchServiceMock.createScopedClusterClient();
-    esClient.asInternalUser.transport.request.mockReturnValue(
-      elasticsearchServiceMock.createSuccessTransportRequestPromise({
-        security: { enabled: true, ssl: {} },
-      })
-    );
-
    const [context, req, res] = mockHandlerArguments(
-      { esClient, alertsClient, getFrameworkHealth: alerting.getFrameworkHealth },
+      {
+        alertsClient,
+        getFrameworkHealth: alerting.getFrameworkHealth,
+        areApiKeysEnabled: () => Promise.resolve(false),
+      },
      {},
      ['ok']
    );
@ -299,15 +249,8 @@ describe('healthRoute', () => {
    healthRoute(router, licenseState, encryptedSavedObjects);
    const [, handler] = router.get.mock.calls[0];

-    const esClient = elasticsearchServiceMock.createScopedClusterClient();
-    esClient.asInternalUser.transport.request.mockReturnValue(
-      elasticsearchServiceMock.createSuccessTransportRequestPromise({
-        security: { enabled: true, ssl: { http: { enabled: true } } },
-      })
-    );
-
    const [context, req, res] = mockHandlerArguments(
-      { esClient, alertsClient, getFrameworkHealth: alerting.getFrameworkHealth },
+      { alertsClient, getFrameworkHealth: alerting.getFrameworkHealth },
      {},
      ['ok']
    );
--- a/x-pack/plugins/alerting/server/routes/legacy/health.ts
+++ b/x-pack/plugins/alerting/server/routes/legacy/health.ts
@ -5,24 +5,12 @@
 * 2.0.
 */

-import { ApiResponse } from '@elastic/elasticsearch';
 import type { AlertingRouter } from '../../types';
 import { ILicenseState } from '../../lib/license_state';
 import { verifyApiAccess } from '../../lib/license_api_access';
 import { AlertingFrameworkHealth } from '../../types';
 import { EncryptedSavedObjectsPluginSetup } from '../../../../encrypted_saved_objects/server';

-interface XPackUsageSecurity {
-  security?: {
-    enabled?: boolean;
-    ssl?: {
-      http?: {
-        enabled?: boolean;
-      };
-    };
-  };
-}
-
 export function healthRoute(
  router: AlertingRouter,
  licenseState: ILicenseState,
@ -39,23 +27,11 @@ export function healthRoute(
        return res.badRequest({ body: 'RouteHandlerContext is not registered for alerting' });
      }
      try {
-        const {
-          body: {
-            security: {
-              enabled: isSecurityEnabled = false,
-              ssl: { http: { enabled: isTLSEnabled = false } = {} } = {},
-            } = {},
-          },
-        }: ApiResponse<XPackUsageSecurity> = await context.core.elasticsearch.client.asInternalUser.transport // Do not augment with such input. // `transport.request` is potentially unsafe when combined with untrusted user input.
-          .request({
-            method: 'GET',
-            path: '/_xpack/usage',
-          });
-
        const alertingFrameworkHeath = await context.alerting.getFrameworkHealth();
+        const areApiKeysEnabled = await context.alerting.areApiKeysEnabled();

        const frameworkHealth: AlertingFrameworkHealth = {
-          isSufficientlySecure: !isSecurityEnabled || (isSecurityEnabled && isTLSEnabled),
+          isSufficientlySecure: areApiKeysEnabled,
          hasPermanentEncryptionKey: encryptedSavedObjects.canEncrypt,
          alertingFrameworkHeath,
        };
--- a/x-pack/plugins/alerting/server/types.ts
+++ b/x-pack/plugins/alerting/server/types.ts
@ -46,6 +46,7 @@ export interface AlertingApiRequestHandlerContext {
  getAlertsClient: () => AlertsClient;
  listTypes: AlertTypeRegistry['list'];
  getFrameworkHealth: () => Promise<AlertsHealth>;
+  areApiKeysEnabled: () => Promise<boolean>;
 }

 /**