From cb992ae175d3752466aa7be5109950e66bf8a1da Mon Sep 17 00:00:00 2001 From: Kevin Delemme Date: Wed, 6 Nov 2024 16:26:06 -0500 Subject: [PATCH] Assert user has expected privielges on source index during reset --- .../__snapshots__/reset_slo.test.ts.snap | 22 +++--- .../slo/server/services/reset_slo.test.ts | 75 ++++++++++++------- .../slo/server/services/reset_slo.ts | 15 ++++ 3 files changed, 76 insertions(+), 36 deletions(-) diff --git a/x-pack/plugins/observability_solution/slo/server/services/__snapshots__/reset_slo.test.ts.snap b/x-pack/plugins/observability_solution/slo/server/services/__snapshots__/reset_slo.test.ts.snap index 90690a4989586..ec81df9f08fdd 100644 --- a/x-pack/plugins/observability_solution/slo/server/services/__snapshots__/reset_slo.test.ts.snap +++ b/x-pack/plugins/observability_solution/slo/server/services/__snapshots__/reset_slo.test.ts.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`ResetSLO resets all associated resources 1`] = ` +exports[`ResetSLO happy path resets all associated resources 1`] = ` [MockFunction] { "calls": Array [ Array [ @@ -16,7 +16,7 @@ exports[`ResetSLO resets all associated resources 1`] = ` } `; -exports[`ResetSLO resets all associated resources 2`] = ` +exports[`ResetSLO happy path resets all associated resources 2`] = ` [MockFunction] { "calls": Array [ Array [ @@ -32,7 +32,7 @@ exports[`ResetSLO resets all associated resources 2`] = ` } `; -exports[`ResetSLO resets all associated resources 3`] = ` +exports[`ResetSLO happy path resets all associated resources 3`] = ` [MockFunction] { "calls": Array [ Array [ @@ -48,7 +48,7 @@ exports[`ResetSLO resets all associated resources 3`] = ` } `; -exports[`ResetSLO resets all associated resources 4`] = ` +exports[`ResetSLO happy path resets all associated resources 4`] = ` [MockFunction] { "calls": Array [ Array [ @@ -64,7 +64,7 @@ exports[`ResetSLO resets all associated resources 4`] = ` } `; -exports[`ResetSLO resets all associated resources 5`] = ` +exports[`ResetSLO happy path resets all associated resources 5`] = ` [MockFunction] { "calls": Array [ Array [ @@ -115,7 +115,7 @@ exports[`ResetSLO resets all associated resources 5`] = ` } `; -exports[`ResetSLO resets all associated resources 6`] = ` +exports[`ResetSLO happy path resets all associated resources 6`] = ` [MockFunction] { "calls": Array [ Array [ @@ -178,7 +178,7 @@ exports[`ResetSLO resets all associated resources 6`] = ` } `; -exports[`ResetSLO resets all associated resources 7`] = ` +exports[`ResetSLO happy path resets all associated resources 7`] = ` [MockFunction] { "calls": Array [ Array [ @@ -194,7 +194,7 @@ exports[`ResetSLO resets all associated resources 7`] = ` } `; -exports[`ResetSLO resets all associated resources 8`] = ` +exports[`ResetSLO happy path resets all associated resources 8`] = ` [MockFunction] { "calls": Array [ Array [ @@ -542,7 +542,7 @@ exports[`ResetSLO resets all associated resources 8`] = ` } `; -exports[`ResetSLO resets all associated resources 9`] = ` +exports[`ResetSLO happy path resets all associated resources 9`] = ` [MockFunction] { "calls": Array [ Array [ @@ -605,7 +605,7 @@ exports[`ResetSLO resets all associated resources 9`] = ` } `; -exports[`ResetSLO resets all associated resources 10`] = ` +exports[`ResetSLO happy path resets all associated resources 10`] = ` [MockFunction] { "calls": Array [ Array [ @@ -621,7 +621,7 @@ exports[`ResetSLO resets all associated resources 10`] = ` } `; -exports[`ResetSLO resets all associated resources 11`] = ` +exports[`ResetSLO happy path resets all associated resources 11`] = ` [MockFunction] { "calls": Array [ Array [ diff --git a/x-pack/plugins/observability_solution/slo/server/services/reset_slo.test.ts b/x-pack/plugins/observability_solution/slo/server/services/reset_slo.test.ts index 4e66d992b46cd..ab806f221a888 100644 --- a/x-pack/plugins/observability_solution/slo/server/services/reset_slo.test.ts +++ b/x-pack/plugins/observability_solution/slo/server/services/reset_slo.test.ts @@ -5,15 +5,15 @@ * 2.0. */ -import { ElasticsearchClient } from '@kbn/core/server'; +import { SecurityHasPrivilegesResponse } from '@elastic/elasticsearch/lib/api/types'; import { + ElasticsearchClientMock, elasticsearchServiceMock, httpServiceMock, loggingSystemMock, ScopedClusterClientMock, } from '@kbn/core/server/mocks'; import { MockedLogger } from '@kbn/logging-mocks'; - import { SLO_MODEL_VERSION } from '../../common/constants'; import { createSLO } from './fixtures/slo'; import { @@ -31,7 +31,7 @@ describe('ResetSLO', () => { let mockRepository: jest.Mocked; let mockTransformManager: jest.Mocked; let mockSummaryTransformManager: jest.Mocked; - let mockEsClient: jest.Mocked; + let mockEsClient: ElasticsearchClientMock; let mockScopedClusterClient: ScopedClusterClientMock; let loggerMock: jest.Mocked; let resetSLO: ResetSLO; @@ -60,37 +60,62 @@ describe('ResetSLO', () => { jest.useRealTimers(); }); - it('resets all associated resources', async () => { - const slo = createSLO({ id: 'irrelevant', version: 1 }); - mockRepository.findById.mockResolvedValueOnce(slo); - mockRepository.update.mockImplementation((v) => Promise.resolve(v)); + describe('happy path', () => { + beforeEach(() => { + mockEsClient.security.hasPrivileges.mockResolvedValue({ + has_all_requested: true, + } as SecurityHasPrivilegesResponse); + }); + + it('resets all associated resources', async () => { + const slo = createSLO({ id: 'irrelevant', version: 1 }); + mockRepository.findById.mockResolvedValueOnce(slo); + mockRepository.update.mockImplementation((v) => Promise.resolve(v)); + + await resetSLO.execute(slo.id); + + // delete existing resources and data + expect(mockSummaryTransformManager.stop).toMatchSnapshot(); + expect(mockSummaryTransformManager.uninstall).toMatchSnapshot(); - await resetSLO.execute(slo.id); + expect(mockTransformManager.stop).toMatchSnapshot(); + expect(mockTransformManager.uninstall).toMatchSnapshot(); - // delete existing resources and data - expect(mockSummaryTransformManager.stop).toMatchSnapshot(); - expect(mockSummaryTransformManager.uninstall).toMatchSnapshot(); + expect(mockEsClient.deleteByQuery).toMatchSnapshot(); - expect(mockTransformManager.stop).toMatchSnapshot(); - expect(mockTransformManager.uninstall).toMatchSnapshot(); + // install resources + expect(mockSummaryTransformManager.install).toMatchSnapshot(); + expect(mockSummaryTransformManager.start).toMatchSnapshot(); - expect(mockEsClient.deleteByQuery).toMatchSnapshot(); + expect(mockScopedClusterClient.asSecondaryAuthUser.ingest.putPipeline).toMatchSnapshot(); - // install resources - expect(mockSummaryTransformManager.install).toMatchSnapshot(); - expect(mockSummaryTransformManager.start).toMatchSnapshot(); + expect(mockTransformManager.install).toMatchSnapshot(); + expect(mockTransformManager.start).toMatchSnapshot(); - expect(mockScopedClusterClient.asSecondaryAuthUser.ingest.putPipeline).toMatchSnapshot(); + expect(mockEsClient.index).toMatchSnapshot(); - expect(mockTransformManager.install).toMatchSnapshot(); - expect(mockTransformManager.start).toMatchSnapshot(); + expect(mockRepository.update).toHaveBeenCalledWith({ + ...slo, + version: SLO_MODEL_VERSION, + updatedAt: expect.anything(), + }); + }); + }); + + describe('unhappy path', () => { + beforeEach(() => { + mockEsClient.security.hasPrivileges.mockResolvedValue({ + has_all_requested: false, + } as SecurityHasPrivilegesResponse); + }); - expect(mockEsClient.index).toMatchSnapshot(); + it('throws a SecurityException error when the user does not have the required privileges', async () => { + const slo = createSLO({ id: 'irrelevant', version: 1 }); + mockRepository.findById.mockResolvedValueOnce(slo); - expect(mockRepository.update).toHaveBeenCalledWith({ - ...slo, - version: SLO_MODEL_VERSION, - updatedAt: expect.anything(), + await expect(resetSLO.execute(slo.id)).rejects.toThrowError( + "Missing ['read', 'view_index_metadata'] privileges on the source index [metrics-apm*]" + ); }); }); }); diff --git a/x-pack/plugins/observability_solution/slo/server/services/reset_slo.ts b/x-pack/plugins/observability_solution/slo/server/services/reset_slo.ts index 634f02c8f6f90..d2552ad010604 100644 --- a/x-pack/plugins/observability_solution/slo/server/services/reset_slo.ts +++ b/x-pack/plugins/observability_solution/slo/server/services/reset_slo.ts @@ -23,6 +23,8 @@ import { retryTransientEsErrors } from '../utils/retry'; import { SLORepository } from './slo_repository'; import { createTempSummaryDocument } from './summary_transform_generator/helpers/create_temp_summary'; import { TransformManager } from './transform_manager'; +import { SLODefinition } from '../domain/models'; +import { SecurityException } from '../errors'; export class ResetSLO { constructor( @@ -39,6 +41,8 @@ export class ResetSLO { public async execute(sloId: string) { const slo = await this.repository.findById(sloId); + await this.assertExpectedIndicatorSourceIndexPrivileges(slo); + const summaryTransformId = getSLOSummaryTransformId(slo.id, slo.revision); await this.summaryTransformManager.stop(summaryTransformId); await this.summaryTransformManager.uninstall(summaryTransformId); @@ -113,6 +117,17 @@ export class ResetSLO { return resetSLOResponseSchema.encode(updatedSlo); } + private async assertExpectedIndicatorSourceIndexPrivileges(slo: SLODefinition) { + const privileges = await this.esClient.security.hasPrivileges({ + index: [{ names: slo.indicator.params.index, privileges: ['read', 'view_index_metadata'] }], + }); + if (!privileges.has_all_requested) { + throw new SecurityException( + `Missing ['read', 'view_index_metadata'] privileges on the source index [${slo.indicator.params.index}]` + ); + } + } + /** * Deleting all SLI rollup data matching the sloId. All revision will be deleted in case of * residual documents.