From 22959861b4323d9a78e0c42a30f66519d37847e8 Mon Sep 17 00:00:00 2001 From: Ying Mao Date: Wed, 17 Nov 2021 14:37:52 -0500 Subject: [PATCH] Adding access check to health API --- .../alerting/server/routes/health.test.ts | 63 ++++++++++++++++++ .../plugins/alerting/server/routes/health.ts | 34 ++++++---- .../server/routes/legacy/health.test.ts | 64 ++++++++++++++++++- .../alerting/server/routes/legacy/health.ts | 34 ++++++---- .../tests/alerting/health.ts | 41 +++++++++--- 5 files changed, 200 insertions(+), 36 deletions(-) diff --git a/x-pack/plugins/alerting/server/routes/health.test.ts b/x-pack/plugins/alerting/server/routes/health.test.ts index b8e023e4f4d1b..9779bcbb5ad07 100644 --- a/x-pack/plugins/alerting/server/routes/health.test.ts +++ b/x-pack/plugins/alerting/server/routes/health.test.ts @@ -14,6 +14,9 @@ import { encryptedSavedObjectsMock } from '../../../encrypted_saved_objects/serv import { rulesClientMock } from '../rules_client.mock'; import { HealthStatus } from '../types'; import { alertsMock } from '../mocks'; +import { RecoveredActionGroup } from '../../common'; +import { RegistryAlertTypeWithAuth } from '../authorization'; + const rulesClient = rulesClientMock.create(); jest.mock('../lib/license_api_access.ts', () => ({ @@ -22,6 +25,33 @@ jest.mock('../lib/license_api_access.ts', () => ({ const alerting = alertsMock.createStart(); const currentDate = new Date().toISOString(); +const ruleTypes = [ + { + id: '1', + name: 'name', + actionGroups: [ + { + id: 'default', + name: 'Default', + }, + ], + defaultActionGroupId: 'default', + minimumLicenseRequired: 'basic', + isExportable: true, + ruleTaskTimeout: '10m', + recoveryActionGroup: RecoveredActionGroup, + authorizedConsumers: {}, + actionVariables: { + context: [], + state: [], + }, + producer: 'test', + enabledInLicense: true, + minimumScheduleInterval: '1m', + defaultScheduleInterval: '10m', + } as RegistryAlertTypeWithAuth, +]; + beforeEach(() => { jest.resetAllMocks(); alerting.getFrameworkHealth.mockResolvedValue({ @@ -42,6 +72,7 @@ beforeEach(() => { describe('healthRoute', () => { it('registers the route', async () => { + rulesClient.listAlertTypes.mockResolvedValueOnce(new Set(ruleTypes)); const router = httpServiceMock.createRouter(); const licenseState = licenseStateMock.create(); @@ -54,6 +85,7 @@ describe('healthRoute', () => { }); it('queries the usage api', async () => { + rulesClient.listAlertTypes.mockResolvedValueOnce(new Set(ruleTypes)); const router = httpServiceMock.createRouter(); const licenseState = licenseStateMock.create(); @@ -76,7 +108,34 @@ describe('healthRoute', () => { expect(verifyApiAccess).toHaveBeenCalledWith(licenseState); }); + it('throws error when user does not have any access to any rule types', async () => { + rulesClient.listAlertTypes.mockResolvedValueOnce(new Set()); + const router = httpServiceMock.createRouter(); + + const licenseState = licenseStateMock.create(); + const encryptedSavedObjects = encryptedSavedObjectsMock.createSetup({ canEncrypt: false }); + healthRoute(router, licenseState, encryptedSavedObjects); + const [, handler] = router.get.mock.calls[0]; + + const [context, req, res] = mockHandlerArguments( + { + rulesClient, + getFrameworkHealth: alerting.getFrameworkHealth, + areApiKeysEnabled: () => Promise.resolve(true), + }, + {}, + ['ok'] + ); + + await handler(context, req, res); + + expect(res.forbidden).toHaveBeenCalledWith({ + body: { message: `Unauthorized to access alerting framework health` }, + }); + }); + it('evaluates whether Encrypted Saved Objects is missing encryption key', async () => { + rulesClient.listAlertTypes.mockResolvedValueOnce(new Set(ruleTypes)); const router = httpServiceMock.createRouter(); const licenseState = licenseStateMock.create(); @@ -117,6 +176,7 @@ describe('healthRoute', () => { }); test('when ES security status cannot be determined from license state, isSufficientlySecure should return false', async () => { + rulesClient.listAlertTypes.mockResolvedValueOnce(new Set(ruleTypes)); const router = httpServiceMock.createRouter(); const licenseState = licenseStateMock.create(); const encryptedSavedObjects = encryptedSavedObjectsMock.createSetup({ canEncrypt: true }); @@ -158,6 +218,7 @@ describe('healthRoute', () => { }); test('when ES security is disabled, isSufficientlySecure should return true', async () => { + rulesClient.listAlertTypes.mockResolvedValueOnce(new Set(ruleTypes)); const router = httpServiceMock.createRouter(); const licenseState = licenseStateMock.create(); const encryptedSavedObjects = encryptedSavedObjectsMock.createSetup({ canEncrypt: true }); @@ -199,6 +260,7 @@ describe('healthRoute', () => { }); test('when ES security is enabled but user cannot generate api keys, isSufficientlySecure should return false', async () => { + rulesClient.listAlertTypes.mockResolvedValueOnce(new Set(ruleTypes)); const router = httpServiceMock.createRouter(); const licenseState = licenseStateMock.create(); const encryptedSavedObjects = encryptedSavedObjectsMock.createSetup({ canEncrypt: true }); @@ -240,6 +302,7 @@ describe('healthRoute', () => { }); test('when ES security is enabled and user can generate api keys, isSufficientlySecure should return true', async () => { + rulesClient.listAlertTypes.mockResolvedValueOnce(new Set(ruleTypes)); const router = httpServiceMock.createRouter(); const licenseState = licenseStateMock.create(); const encryptedSavedObjects = encryptedSavedObjectsMock.createSetup({ canEncrypt: true }); diff --git a/x-pack/plugins/alerting/server/routes/health.ts b/x-pack/plugins/alerting/server/routes/health.ts index 4f3ed2b542611..66f8184e96fce 100644 --- a/x-pack/plugins/alerting/server/routes/health.ts +++ b/x-pack/plugins/alerting/server/routes/health.ts @@ -45,22 +45,30 @@ export const healthRoute = ( router.handleLegacyErrors( verifyAccessAndContext(licenseState, async function (context, req, res) { try { - const alertingFrameworkHeath = await context.alerting.getFrameworkHealth(); + // Verify that user has access to at least one rule type + const ruleTypes = Array.from(await context.alerting.getRulesClient().listAlertTypes()); + if (ruleTypes.length > 0) { + const alertingFrameworkHeath = await context.alerting.getFrameworkHealth(); - const securityHealth = await getSecurityHealth( - async () => (licenseState ? licenseState.getIsSecurityEnabled() : null), - async () => encryptedSavedObjects.canEncrypt, - context.alerting.areApiKeysEnabled - ); + const securityHealth = await getSecurityHealth( + async () => (licenseState ? licenseState.getIsSecurityEnabled() : null), + async () => encryptedSavedObjects.canEncrypt, + context.alerting.areApiKeysEnabled + ); - const frameworkHealth: AlertingFrameworkHealth = { - ...securityHealth, - alertingFrameworkHeath, - }; + const frameworkHealth: AlertingFrameworkHealth = { + ...securityHealth, + alertingFrameworkHeath, + }; - return res.ok({ - body: rewriteBodyRes(frameworkHealth), - }); + return res.ok({ + body: rewriteBodyRes(frameworkHealth), + }); + } else { + return res.forbidden({ + body: { message: `Unauthorized to access alerting framework health` }, + }); + } } catch (error) { return res.badRequest({ body: error }); } 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 59ede356add00..7215723264fe2 100644 --- a/x-pack/plugins/alerting/server/routes/legacy/health.test.ts +++ b/x-pack/plugins/alerting/server/routes/legacy/health.test.ts @@ -11,9 +11,10 @@ import { mockHandlerArguments } from './../_mock_handler_arguments'; import { licenseStateMock } from '../../lib/license_state.mock'; import { encryptedSavedObjectsMock } from '../../../../encrypted_saved_objects/server/mocks'; import { rulesClientMock } from '../../rules_client.mock'; -import { HealthStatus } from '../../types'; +import { HealthStatus, RecoveredActionGroup } from '../../types'; import { alertsMock } from '../../mocks'; import { trackLegacyRouteUsage } from '../../lib/track_legacy_route_usage'; +import { RegistryAlertTypeWithAuth } from '../../authorization'; const rulesClient = rulesClientMock.create(); @@ -28,6 +29,34 @@ jest.mock('../../lib/track_legacy_route_usage', () => ({ const alerting = alertsMock.createStart(); const currentDate = new Date().toISOString(); + +const ruleTypes = [ + { + id: '1', + name: 'name', + actionGroups: [ + { + id: 'default', + name: 'Default', + }, + ], + defaultActionGroupId: 'default', + minimumLicenseRequired: 'basic', + isExportable: true, + ruleTaskTimeout: '10m', + recoveryActionGroup: RecoveredActionGroup, + authorizedConsumers: {}, + actionVariables: { + context: [], + state: [], + }, + producer: 'test', + enabledInLicense: true, + minimumScheduleInterval: '1m', + defaultScheduleInterval: '10m', + } as RegistryAlertTypeWithAuth, +]; + beforeEach(() => { jest.resetAllMocks(); alerting.getFrameworkHealth.mockResolvedValue({ @@ -48,6 +77,7 @@ beforeEach(() => { describe('healthRoute', () => { it('registers the route', async () => { + rulesClient.listAlertTypes.mockResolvedValueOnce(new Set(ruleTypes)); const router = httpServiceMock.createRouter(); const licenseState = licenseStateMock.create(); @@ -59,7 +89,34 @@ describe('healthRoute', () => { expect(config.path).toMatchInlineSnapshot(`"/api/alerts/_health"`); }); + it('throws error when user does not have any access to any rule types', async () => { + rulesClient.listAlertTypes.mockResolvedValueOnce(new Set()); + const router = httpServiceMock.createRouter(); + + const licenseState = licenseStateMock.create(); + const encryptedSavedObjects = encryptedSavedObjectsMock.createSetup({ canEncrypt: false }); + healthRoute(router, licenseState, encryptedSavedObjects); + const [, handler] = router.get.mock.calls[0]; + + const [context, req, res] = mockHandlerArguments( + { + rulesClient, + getFrameworkHealth: alerting.getFrameworkHealth, + areApiKeysEnabled: () => Promise.resolve(true), + }, + {}, + ['ok'] + ); + + await handler(context, req, res); + + expect(res.forbidden).toHaveBeenCalledWith({ + body: { message: `Unauthorized to access alerting framework health` }, + }); + }); + it('evaluates whether Encrypted Saved Objects is missing encryption key', async () => { + rulesClient.listAlertTypes.mockResolvedValueOnce(new Set(ruleTypes)); const router = httpServiceMock.createRouter(); const licenseState = licenseStateMock.create(); @@ -100,6 +157,7 @@ describe('healthRoute', () => { }); test('when ES security status cannot be determined from license state, isSufficientlySecure should return false', async () => { + rulesClient.listAlertTypes.mockResolvedValueOnce(new Set(ruleTypes)); const router = httpServiceMock.createRouter(); const licenseState = licenseStateMock.create(); const encryptedSavedObjects = encryptedSavedObjectsMock.createSetup({ canEncrypt: true }); @@ -141,6 +199,7 @@ describe('healthRoute', () => { }); test('when ES security is disabled, isSufficientlySecure should return true', async () => { + rulesClient.listAlertTypes.mockResolvedValueOnce(new Set(ruleTypes)); const router = httpServiceMock.createRouter(); const licenseState = licenseStateMock.create(); const encryptedSavedObjects = encryptedSavedObjectsMock.createSetup({ canEncrypt: true }); @@ -182,6 +241,7 @@ describe('healthRoute', () => { }); test('when ES security is enabled but user cannot generate api keys, isSufficientlySecure should return false', async () => { + rulesClient.listAlertTypes.mockResolvedValueOnce(new Set(ruleTypes)); const router = httpServiceMock.createRouter(); const licenseState = licenseStateMock.create(); const encryptedSavedObjects = encryptedSavedObjectsMock.createSetup({ canEncrypt: true }); @@ -223,6 +283,7 @@ describe('healthRoute', () => { }); test('when ES security is enabled and user can generate api keys, isSufficientlySecure should return true', async () => { + rulesClient.listAlertTypes.mockResolvedValueOnce(new Set(ruleTypes)); const router = httpServiceMock.createRouter(); const licenseState = licenseStateMock.create(); const encryptedSavedObjects = encryptedSavedObjectsMock.createSetup({ canEncrypt: true }); @@ -264,6 +325,7 @@ describe('healthRoute', () => { }); it('should track every call', async () => { + rulesClient.listAlertTypes.mockResolvedValueOnce(new Set(ruleTypes)); const licenseState = licenseStateMock.create(); const router = httpServiceMock.createRouter(); const encryptedSavedObjects = encryptedSavedObjectsMock.createSetup({ canEncrypt: true }); diff --git a/x-pack/plugins/alerting/server/routes/legacy/health.ts b/x-pack/plugins/alerting/server/routes/legacy/health.ts index abea724b63c6f..ea066220f8980 100644 --- a/x-pack/plugins/alerting/server/routes/legacy/health.ts +++ b/x-pack/plugins/alerting/server/routes/legacy/health.ts @@ -32,22 +32,30 @@ export function healthRoute( } trackLegacyRouteUsage('health', usageCounter); try { - const alertingFrameworkHeath = await context.alerting.getFrameworkHealth(); + // Verify that user has access to at least one rule type + const ruleTypes = Array.from(await context.alerting.getRulesClient().listAlertTypes()); + if (ruleTypes.length > 0) { + const alertingFrameworkHeath = await context.alerting.getFrameworkHealth(); - const securityHealth = await getSecurityHealth( - async () => (licenseState ? licenseState.getIsSecurityEnabled() : null), - async () => encryptedSavedObjects.canEncrypt, - context.alerting.areApiKeysEnabled - ); + const securityHealth = await getSecurityHealth( + async () => (licenseState ? licenseState.getIsSecurityEnabled() : null), + async () => encryptedSavedObjects.canEncrypt, + context.alerting.areApiKeysEnabled + ); - const frameworkHealth: AlertingFrameworkHealth = { - ...securityHealth, - alertingFrameworkHeath, - }; + const frameworkHealth: AlertingFrameworkHealth = { + ...securityHealth, + alertingFrameworkHeath, + }; - return res.ok({ - body: frameworkHealth, - }); + return res.ok({ + body: frameworkHealth, + }); + } else { + return res.forbidden({ + body: { message: `Unauthorized to access alerting framework health` }, + }); + } } catch (error) { return res.badRequest({ body: error }); } diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/health.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/health.ts index 21e5c782d185c..601d663a70759 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/health.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/health.ts @@ -77,11 +77,22 @@ export default function createFindTests({ getService }: FtrProviderContext) { const { body: health } = await supertestWithoutAuth .get(`${getUrlPrefix(space.id)}/api/alerting/_health`) .auth(user.username, user.password); - expect(health.is_sufficiently_secure).to.eql(true); - expect(health.has_permanent_encryption_key).to.eql(true); - expect(health.alerting_framework_heath.decryption_health.status).to.eql('ok'); - expect(health.alerting_framework_heath.execution_health.status).to.eql('ok'); - expect(health.alerting_framework_heath.read_health.status).to.eql('ok'); + switch (scenario.id) { + case 'no_kibana_privileges at space1': + case 'space_1_all at space2': + expect(health).to.eql({ + statusCode: 403, + error: 'Forbidden', + message: 'Unauthorized to access alerting framework health', + }); + break; + default: + expect(health.is_sufficiently_secure).to.eql(true); + expect(health.has_permanent_encryption_key).to.eql(true); + expect(health.alerting_framework_heath.decryption_health.status).to.eql('ok'); + expect(health.alerting_framework_heath.execution_health.status).to.eql('ok'); + expect(health.alerting_framework_heath.read_health.status).to.eql('ok'); + } }); it('should return error when a rule in the default space is failing', async () => { @@ -116,10 +127,22 @@ export default function createFindTests({ getService }: FtrProviderContext) { const { body: health } = await supertestWithoutAuth .get(`${getUrlPrefix(space.id)}/api/alerting/_health`) .auth(user.username, user.password); - expect(health.alerting_framework_heath.execution_health.status).to.eql('warn'); - expect(health.alerting_framework_heath.execution_health.timestamp).to.eql( - ruleInErrorStatus.execution_status.last_execution_date - ); + + switch (scenario.id) { + case 'no_kibana_privileges at space1': + case 'space_1_all at space2': + expect(health).to.eql({ + statusCode: 403, + error: 'Forbidden', + message: 'Unauthorized to access alerting framework health', + }); + break; + default: + expect(health.alerting_framework_heath.execution_health.status).to.eql('warn'); + expect(health.alerting_framework_heath.execution_health.timestamp).to.eql( + ruleInErrorStatus.execution_status.last_execution_date + ); + } }); }); });