[Alerting] Fix health check to allow access to alerting when ES secur…

…ity is disabled (#107032) * Using license plugin to check if es security is enabled * Adding unit tests and updating legacy health route * Updating UI copy and docs Co-authored-by: Kibana Machine <[email protected]>
elastic · Aug 2, 2021 · df421f8 · df421f8
1 parent 0c69b10
commit df421f8
Show file tree

Hide file tree

Showing 9 changed files with 136 additions and 101 deletions.
diff --git a/docs/user/alerting/alerting-setup.asciidoc b/docs/user/alerting/alerting-setup.asciidoc
@@ -18,6 +18,7 @@ If you are using an *on-premises* Elastic Stack deployment:
 If you are using an *on-premises* Elastic Stack deployment with <<using-kibana-with-security, *security*>>:
 
 * You must enable Transport Layer Security (TLS) for communication <<configuring-tls-kib-es, between {es} and {kib}>>. {kib} alerting uses <<api-keys, API keys>> to secure background rule checks and actions, and API keys require {ref}/configuring-tls.html#tls-http[TLS on the HTTP interface]. A proxy will not suffice.
+* If you have enabled TLS and are still unable to access Alerting, ensure that you have not {ref}/security-settings.html#api-key-service-settings[explicitly disabled API keys].
 
 [float]
 [[alerting-setup-production]]

diff --git a/x-pack/plugins/alerting/server/lib/license_state.mock.ts b/x-pack/plugins/alerting/server/lib/license_state.mock.ts
@@ -18,6 +18,7 @@ export const createLicenseStateMock = () => {
     checkLicense: jest.fn().mockResolvedValue({
       state: 'valid',
     }),
+    getIsSecurityEnabled: jest.fn(),
     setNotifyUsage: jest.fn(),
   };
   return licenseState;

diff --git a/x-pack/plugins/alerting/server/lib/license_state.test.ts b/x-pack/plugins/alerting/server/lib/license_state.test.ts
@@ -272,6 +272,42 @@ describe('ensureLicenseForAlertType()', () => {
   });
 });
 
+describe('getIsSecurityEnabled()', () => {
+  let license: Subject<ILicense>;
+  let licenseState: ILicenseState;
+  beforeEach(() => {
+    license = new Subject();
+    licenseState = new LicenseState(license);
+  });
+
+  test('should return null when license is not defined', () => {
+    expect(licenseState.getIsSecurityEnabled()).toBeNull();
+  });
+
+  test('should return null when license is unavailable', () => {
+    license.next(createUnavailableLicense());
+    expect(licenseState.getIsSecurityEnabled()).toBeNull();
+  });
+
+  test('should return true if security is enabled', () => {
+    const basicLicense = licensingMock.createLicense({
+      license: { status: 'active', type: 'basic' },
+      features: { security: { isEnabled: true, isAvailable: true } },
+    });
+    license.next(basicLicense);
+    expect(licenseState.getIsSecurityEnabled()).toEqual(true);
+  });
+
+  test('should return false if security is not enabled', () => {
+    const basicLicense = licensingMock.createLicense({
+      license: { status: 'active', type: 'basic' },
+      features: { security: { isEnabled: false, isAvailable: true } },
+    });
+    license.next(basicLicense);
+    expect(licenseState.getIsSecurityEnabled()).toEqual(false);
+  });
+});
+
 function createUnavailableLicense() {
   const unavailableLicense = licensingMock.createLicenseMock();
   unavailableLicense.isAvailable = false;

diff --git a/x-pack/plugins/alerting/server/lib/license_state.ts b/x-pack/plugins/alerting/server/lib/license_state.ts
@@ -55,6 +55,15 @@ export class LicenseState {
     return this.licenseInformation;
   }
 
+  public getIsSecurityEnabled(): boolean | null {
+    if (!this.license || !this.license?.isAvailable) {
+      return null;
+    }
+
+    const { isEnabled } = this.license.getFeature('security');
+    return isEnabled;
+  }
+
   public setNotifyUsage(notifyUsage: LicensingPluginStart['featureUsage']['notifyUsage']) {
     this._notifyUsage = notifyUsage;
   }

diff --git a/x-pack/plugins/alerting/server/routes/health.test.ts b/x-pack/plugins/alerting/server/routes/health.test.ts
@@ -21,7 +21,6 @@ jest.mock('../lib/license_api_access.ts', () => ({
 }));
 
 const alerting = alertsMock.createStart();
-
 const currentDate = new Date().toISOString();
 beforeEach(() => {
   jest.resetAllMocks();
@@ -62,7 +61,15 @@ describe('healthRoute', () => {
     healthRoute(router, licenseState, encryptedSavedObjects);
     const [, handler] = router.get.mock.calls[0];
 
-    const [context, req, res] = mockHandlerArguments({ rulesClient }, {}, ['ok']);
+    const [context, req, res] = mockHandlerArguments(
+      {
+        rulesClient,
+        getFrameworkHealth: alerting.getFrameworkHealth,
+        areApiKeysEnabled: () => Promise.resolve(true),
+      },
+      {},
+      ['ok']
+    );
 
     await handler(context, req, res);
 
@@ -78,7 +85,11 @@ describe('healthRoute', () => {
     const [, handler] = router.get.mock.calls[0];
 
     const [context, req, res] = mockHandlerArguments(
-      { rulesClient, getFrameworkHealth: alerting.getFrameworkHealth },
+      {
+        rulesClient,
+        getFrameworkHealth: alerting.getFrameworkHealth,
+        areApiKeysEnabled: () => Promise.resolve(true),
+      },
       {},
       ['ok']
     );
@@ -105,52 +116,21 @@ describe('healthRoute', () => {
     });
   });
 
-  it('evaluates missing security info from the usage api to mean that the security plugin is disbled', async () => {
+  test('when ES security status cannot be determined from license state, isSufficientlySecure should return false', 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];
+    licenseState.getIsSecurityEnabled.mockReturnValueOnce(null);
 
-    const [context, req, res] = mockHandlerArguments(
-      { rulesClient, getFrameworkHealth: alerting.getFrameworkHealth },
-      {},
-      ['ok']
-    );
-
-    expect(await handler(context, req, res)).toStrictEqual({
-      body: {
-        alerting_framework_heath: {
-          decryption_health: {
-            status: HealthStatus.OK,
-            timestamp: currentDate,
-          },
-          execution_health: {
-            status: HealthStatus.OK,
-            timestamp: currentDate,
-          },
-          read_health: {
-            status: HealthStatus.OK,
-            timestamp: currentDate,
-          },
-        },
-        has_permanent_encryption_key: true,
-        is_sufficiently_secure: true,
-      },
-    });
-  });
-
-  it('evaluates missing security http info from the usage api to mean that the security plugin is disbled', 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 [context, req, res] = mockHandlerArguments(
-      { rulesClient, getFrameworkHealth: alerting.getFrameworkHealth },
+      {
+        rulesClient,
+        getFrameworkHealth: alerting.getFrameworkHealth,
+        areApiKeysEnabled: () => Promise.resolve(true),
+      },
       {},
       ['ok']
     );
@@ -172,16 +152,17 @@ describe('healthRoute', () => {
           },
         },
         has_permanent_encryption_key: true,
-        is_sufficiently_secure: true,
+        is_sufficiently_secure: false,
       },
     });
   });
 
-  it('evaluates security enabled, and missing ssl info from the usage api to mean that the user cannot generate keys', async () => {
+  test('when ES security is disabled, isSufficientlySecure should return true', async () => {
     const router = httpServiceMock.createRouter();
-
     const licenseState = licenseStateMock.create();
     const encryptedSavedObjects = encryptedSavedObjectsMock.createSetup({ canEncrypt: true });
+    licenseState.getIsSecurityEnabled.mockReturnValueOnce(false);
+
     healthRoute(router, licenseState, encryptedSavedObjects);
     const [, handler] = router.get.mock.calls[0];
 
@@ -212,16 +193,17 @@ describe('healthRoute', () => {
           },
         },
         has_permanent_encryption_key: true,
-        is_sufficiently_secure: false,
+        is_sufficiently_secure: true,
       },
     });
   });
 
-  it('evaluates security enabled, SSL info present but missing http info from the usage api to mean that the user cannot generate keys', async () => {
+  test('when ES security is enabled but user cannot generate api keys, isSufficientlySecure should return false', async () => {
     const router = httpServiceMock.createRouter();
-
     const licenseState = licenseStateMock.create();
     const encryptedSavedObjects = encryptedSavedObjectsMock.createSetup({ canEncrypt: true });
+    licenseState.getIsSecurityEnabled.mockReturnValueOnce(true);
+
     healthRoute(router, licenseState, encryptedSavedObjects);
     const [, handler] = router.get.mock.calls[0];
 
@@ -257,16 +239,21 @@ describe('healthRoute', () => {
     });
   });
 
-  it('evaluates security and tls enabled to mean that the user can generate keys', async () => {
+  test('when ES security is enabled and user can generate api keys, isSufficientlySecure should return true', async () => {
     const router = httpServiceMock.createRouter();
-
     const licenseState = licenseStateMock.create();
     const encryptedSavedObjects = encryptedSavedObjectsMock.createSetup({ canEncrypt: true });
+    licenseState.getIsSecurityEnabled.mockReturnValueOnce(true);
+
     healthRoute(router, licenseState, encryptedSavedObjects);
     const [, handler] = router.get.mock.calls[0];
 
     const [context, req, res] = mockHandlerArguments(
-      { rulesClient, getFrameworkHealth: alerting.getFrameworkHealth },
+      {
+        rulesClient,
+        getFrameworkHealth: alerting.getFrameworkHealth,
+        areApiKeysEnabled: () => Promise.resolve(true),
+      },
       {},
       ['ok']
     );

diff --git a/x-pack/plugins/alerting/server/routes/health.ts b/x-pack/plugins/alerting/server/routes/health.ts
@@ -44,11 +44,22 @@ export const healthRoute = (
     router.handleLegacyErrors(
       verifyAccessAndContext(licenseState, async function (context, req, res) {
         try {
+          const isEsSecurityEnabled: boolean | null = licenseState.getIsSecurityEnabled();
           const areApiKeysEnabled = await context.alerting.areApiKeysEnabled();
           const alertingFrameworkHeath = await context.alerting.getFrameworkHealth();
 
+          let isSufficientlySecure;
+          if (isEsSecurityEnabled === null) {
+            isSufficientlySecure = false;
+          } else {
+            // if isEsSecurityEnabled = true, then areApiKeysEnabled must be true to enable alerting
+            // if isEsSecurityEnabled = false, then it does not matter what areApiKeysEnabled is
+            isSufficientlySecure =
+              !isEsSecurityEnabled || (isEsSecurityEnabled && areApiKeysEnabled);
+          }
+
           const frameworkHealth: AlertingFrameworkHealth = {
-            isSufficientlySecure: areApiKeysEnabled,
+            isSufficientlySecure,
             hasPermanentEncryptionKey: encryptedSavedObjects.canEncrypt,
             alertingFrameworkHeath,
           };