From df421f869feb747256bfa329efbbf8c30c603e0e Mon Sep 17 00:00:00 2001 From: ymao1 Date: Mon, 2 Aug 2021 10:37:30 -0400 Subject: [PATCH] [Alerting] Fix health check to allow access to alerting when ES security 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 <42973632+kibanamachine@users.noreply.github.com> --- docs/user/alerting/alerting-setup.asciidoc | 1 + .../alerting/server/lib/license_state.mock.ts | 1 + .../alerting/server/lib/license_state.test.ts | 36 ++++++++ .../alerting/server/lib/license_state.ts | 9 ++ .../alerting/server/routes/health.test.ts | 87 ++++++++----------- .../plugins/alerting/server/routes/health.ts | 13 ++- .../server/routes/legacy/health.test.ts | 76 ++++++---------- .../alerting/server/routes/legacy/health.ts | 12 ++- .../application/components/health_check.tsx | 2 +- 9 files changed, 136 insertions(+), 101 deletions(-) diff --git a/docs/user/alerting/alerting-setup.asciidoc b/docs/user/alerting/alerting-setup.asciidoc index 2ae5160069f0a..4cd26dbc13e4d 100644 --- 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 <>: * You must enable Transport Layer Security (TLS) for communication <>. {kib} alerting uses <> 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 index 3932ce9329824..1521a1cf25da9 100644 --- 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 index 6cfe368245842..e20acafbab314 100644 --- 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; + 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 index 837fecde11659..9f6fd1b292af8 100644 --- 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 index 7c00b04ae7ef3..b8e023e4f4d1b 100644 --- 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 index 96016ccc45472..fa09213dada3a 100644 --- 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, }; diff --git a/x-pack/plugins/alerting/server/routes/legacy/health.test.ts b/x-pack/plugins/alerting/server/routes/legacy/health.test.ts index 57bfdc29024f6..ed8e6763d66b7 100644 --- a/x-pack/plugins/alerting/server/routes/legacy/health.test.ts +++ b/x-pack/plugins/alerting/server/routes/legacy/health.test.ts @@ -62,7 +62,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'] ); @@ -89,52 +93,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]; - - const [context, req, res] = mockHandlerArguments( - { rulesClient, getFrameworkHealth: alerting.getFrameworkHealth }, - {}, - ['ok'] - ); - - expect(await handler(context, req, res)).toStrictEqual({ - body: { - alertingFrameworkHeath: { - decryptionHealth: { - status: HealthStatus.OK, - timestamp: currentDate, - }, - executionHealth: { - status: HealthStatus.OK, - timestamp: currentDate, - }, - readHealth: { - status: HealthStatus.OK, - timestamp: currentDate, - }, - }, - hasPermanentEncryptionKey: true, - isSufficientlySecure: true, - }, - }); - }); - - it('evaluates missing security http info from the usage api to mean that the security plugin is disbled', async () => { - const router = httpServiceMock.createRouter(); + licenseState.getIsSecurityEnabled.mockReturnValueOnce(null); - 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'] ); @@ -156,16 +129,17 @@ describe('healthRoute', () => { }, }, hasPermanentEncryptionKey: true, - isSufficientlySecure: true, + isSufficientlySecure: 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]; @@ -196,16 +170,17 @@ describe('healthRoute', () => { }, }, hasPermanentEncryptionKey: true, - isSufficientlySecure: false, + isSufficientlySecure: 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]; @@ -241,16 +216,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/legacy/health.ts b/x-pack/plugins/alerting/server/routes/legacy/health.ts index 206a74c2ea636..03a574ca62c33 100644 --- a/x-pack/plugins/alerting/server/routes/legacy/health.ts +++ b/x-pack/plugins/alerting/server/routes/legacy/health.ts @@ -27,11 +27,21 @@ export function healthRoute( return res.badRequest({ body: 'RouteHandlerContext is not registered for alerting' }); } try { + const isEsSecurityEnabled: boolean | null = licenseState.getIsSecurityEnabled(); const alertingFrameworkHeath = await context.alerting.getFrameworkHealth(); const areApiKeysEnabled = await context.alerting.areApiKeysEnabled(); + 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, }; diff --git a/x-pack/plugins/triggers_actions_ui/public/application/components/health_check.tsx b/x-pack/plugins/triggers_actions_ui/public/application/components/health_check.tsx index 762526dfd7fa7..f990e12ed76e5 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/components/health_check.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/components/health_check.tsx @@ -153,7 +153,7 @@ const TlsError = ({ docLinks, className }: PromptErrorProps) => (

}