From 008187d4408171a1c7a73b0e622dce1f5b723be0 Mon Sep 17 00:00:00 2001 From: Sandra G Date: Fri, 6 Dec 2024 10:58:20 -0500 Subject: [PATCH] [Data Usage] add error handling and tests for privilege related errors (#203006) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - handling of 2 error cases to error handler - `security_exception` due to lack of privileges. Metering api will respond when one of the following isn't available as a user privilege `monitor,view_index_metadata,manage,all`. - `index_not_found_exception`. Metering api will respond with this when no indices exist for the privileges it has access to or when no indices are found. - api integration tests for data_streams route for the following cases - returns no data streams when there are none it has access to and responds with appropriate message - returns no data streams without necessary privileges and responds with appropriate message - returns data streams when user only has access to a subset of indices with necessary privileges - functional tests for same as above. these remain skipped due to not being able to create data streams picked up by metering api since we implemented filtering out zero storage size data streams, but useful for local testing with some code changes. ### `security_exception` view Screenshot 2024-12-04 at 1 14 10 PM ### `index_not_found_exception` view Screenshot 2024-12-04 at 1 13 13 PM --------- Co-authored-by: Ashokaditya Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> --- .../data_usage/server/common/errors.ts | 2 +- x-pack/plugins/data_usage/server/errors.ts | 30 ++++ .../data_usage/server/routes/error_handler.ts | 18 +- .../routes/internal/data_streams.test.ts | 35 +++- .../routes/internal/data_streams_handler.ts | 7 + .../routes/internal/usage_metrics.test.ts | 2 +- .../data_usage/server/services/autoops_api.ts | 2 +- .../data_usage/server/services/errors.ts | 10 -- .../data_usage/server/services/index.ts | 2 +- .../test_suites/common/data_usage/index.ts | 1 + .../tests/data_streams_privileges.ts | 155 ++++++++++++++++++ .../common/data_usage/privileges.ts | 101 +++++++++++- 12 files changed, 345 insertions(+), 20 deletions(-) create mode 100644 x-pack/plugins/data_usage/server/errors.ts delete mode 100644 x-pack/plugins/data_usage/server/services/errors.ts create mode 100644 x-pack/test_serverless/api_integration/test_suites/common/data_usage/tests/data_streams_privileges.ts diff --git a/x-pack/plugins/data_usage/server/common/errors.ts b/x-pack/plugins/data_usage/server/common/errors.ts index 7a43a10108be1..2b583570058e9 100644 --- a/x-pack/plugins/data_usage/server/common/errors.ts +++ b/x-pack/plugins/data_usage/server/common/errors.ts @@ -5,7 +5,7 @@ * 2.0. */ -export class BaseError extends Error { +export class DataUsageError extends Error { constructor(message: string, public readonly meta?: MetaType) { super(message); // For debugging - capture name of subclasses diff --git a/x-pack/plugins/data_usage/server/errors.ts b/x-pack/plugins/data_usage/server/errors.ts new file mode 100644 index 0000000000000..17363afe5da31 --- /dev/null +++ b/x-pack/plugins/data_usage/server/errors.ts @@ -0,0 +1,30 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +/* eslint-disable max-classes-per-file */ + +import { DataUsageError } from './common/errors'; + +export class NotFoundError extends DataUsageError {} + +export class AutoOpsError extends DataUsageError {} + +export class NoPrivilegeMeteringError extends DataUsageError { + constructor() { + super( + 'You do not have the necessary privileges to access data stream statistics. Please contact your administrator.' + ); + } +} + +export class NoIndicesMeteringError extends DataUsageError { + constructor() { + super( + 'No data streams or indices are available for the current user. Ensure that the data streams or indices you are authorized to access have been created and contain data. If you believe this is an error, please contact your administrator.' + ); + } +} diff --git a/x-pack/plugins/data_usage/server/routes/error_handler.ts b/x-pack/plugins/data_usage/server/routes/error_handler.ts index b889d12674db5..055b09bb71b9e 100644 --- a/x-pack/plugins/data_usage/server/routes/error_handler.ts +++ b/x-pack/plugins/data_usage/server/routes/error_handler.ts @@ -7,10 +7,12 @@ import type { IKibanaResponse, KibanaResponseFactory, Logger } from '@kbn/core/server'; import { CustomHttpRequestError } from '../utils/custom_http_request_error'; -import { BaseError } from '../common/errors'; -import { AutoOpsError } from '../services/errors'; - -export class NotFoundError extends BaseError {} +import { + AutoOpsError, + NoPrivilegeMeteringError, + NoIndicesMeteringError, + NotFoundError, +} from '../errors'; /** * Default Data Usage Routes error handler @@ -43,6 +45,14 @@ export const errorHandler = ( return res.notFound({ body: error }); } + if (error instanceof NoPrivilegeMeteringError) { + return res.forbidden({ body: error }); + } + + if (error instanceof NoIndicesMeteringError) { + return res.notFound({ body: error }); + } + // Kibana CORE will take care of `500` errors when the handler `throw`'s, including logging the error throw error; }; diff --git a/x-pack/plugins/data_usage/server/routes/internal/data_streams.test.ts b/x-pack/plugins/data_usage/server/routes/internal/data_streams.test.ts index 374c4b9c82e7e..9efe61bd75118 100644 --- a/x-pack/plugins/data_usage/server/routes/internal/data_streams.test.ts +++ b/x-pack/plugins/data_usage/server/routes/internal/data_streams.test.ts @@ -19,6 +19,7 @@ import { DATA_USAGE_DATA_STREAMS_API_ROUTE } from '../../../common'; import { createMockedDataUsageContext } from '../../mocks'; import { getMeteringStats } from '../../utils/get_metering_stats'; import { CustomHttpRequestError } from '../../utils'; +import { NoIndicesMeteringError, NoPrivilegeMeteringError } from '../../errors'; jest.mock('../../utils/get_metering_stats'); const mockGetMeteringStats = getMeteringStats as jest.Mock; @@ -126,7 +127,7 @@ describe('registerDataStreamsRoute', () => { }); }); - it('should return correct error if metering stats request fails', async () => { + it('should return correct error if metering stats request fails with an unknown error', async () => { // using custom error for test here to avoid having to import the actual error class mockGetMeteringStats.mockRejectedValue( new CustomHttpRequestError('Error getting metring stats!') @@ -144,6 +145,38 @@ describe('registerDataStreamsRoute', () => { }); }); + it('should return `not found` error if metering stats request fails when no indices', async () => { + mockGetMeteringStats.mockRejectedValue( + new Error(JSON.stringify({ message: 'index_not_found_exception' })) + ); + const mockRequest = httpServerMock.createKibanaRequest({ body: {} }); + const mockResponse = httpServerMock.createResponseFactory(); + const mockRouter = mockCore.http.createRouter.mock.results[0].value; + const [[, handler]] = mockRouter.versioned.get.mock.results[0].value.addVersion.mock.calls; + await handler(context, mockRequest, mockResponse); + + expect(mockResponse.notFound).toHaveBeenCalledTimes(1); + expect(mockResponse.notFound).toHaveBeenCalledWith({ + body: new NoIndicesMeteringError(), + }); + }); + + it('should return `forbidden` error if metering stats request fails with privileges error', async () => { + mockGetMeteringStats.mockRejectedValue( + new Error(JSON.stringify({ message: 'security_exception' })) + ); + const mockRequest = httpServerMock.createKibanaRequest({ body: {} }); + const mockResponse = httpServerMock.createResponseFactory(); + const mockRouter = mockCore.http.createRouter.mock.results[0].value; + const [[, handler]] = mockRouter.versioned.get.mock.results[0].value.addVersion.mock.calls; + await handler(context, mockRequest, mockResponse); + + expect(mockResponse.forbidden).toHaveBeenCalledTimes(1); + expect(mockResponse.forbidden).toHaveBeenCalledWith({ + body: new NoPrivilegeMeteringError(), + }); + }); + it.each([ ['no datastreams', {}, []], ['empty array', { datastreams: [] }, []], diff --git a/x-pack/plugins/data_usage/server/routes/internal/data_streams_handler.ts b/x-pack/plugins/data_usage/server/routes/internal/data_streams_handler.ts index 726aa157050f8..28967b9a0ee4a 100644 --- a/x-pack/plugins/data_usage/server/routes/internal/data_streams_handler.ts +++ b/x-pack/plugins/data_usage/server/routes/internal/data_streams_handler.ts @@ -10,6 +10,7 @@ import { DataUsageContext, DataUsageRequestHandlerContext } from '../../types'; import { errorHandler } from '../error_handler'; import { getMeteringStats } from '../../utils/get_metering_stats'; import { DataStreamsRequestQuery } from '../../../common/rest_types/data_streams'; +import { NoIndicesMeteringError, NoPrivilegeMeteringError } from '../../errors'; export const getDataStreamsHandler = ( dataUsageContext: DataUsageContext @@ -45,6 +46,12 @@ export const getDataStreamsHandler = ( body, }); } catch (error) { + if (error.message.includes('security_exception')) { + return errorHandler(logger, response, new NoPrivilegeMeteringError()); + } else if (error.message.includes('index_not_found_exception')) { + return errorHandler(logger, response, new NoIndicesMeteringError()); + } + return errorHandler(logger, response, error); } }; diff --git a/x-pack/plugins/data_usage/server/routes/internal/usage_metrics.test.ts b/x-pack/plugins/data_usage/server/routes/internal/usage_metrics.test.ts index f2bccd6d9c6b0..e6f98a97f0e93 100644 --- a/x-pack/plugins/data_usage/server/routes/internal/usage_metrics.test.ts +++ b/x-pack/plugins/data_usage/server/routes/internal/usage_metrics.test.ts @@ -18,7 +18,7 @@ import type { import { DATA_USAGE_METRICS_API_ROUTE } from '../../../common'; import { createMockedDataUsageContext } from '../../mocks'; import { CustomHttpRequestError } from '../../utils'; -import { AutoOpsError } from '../../services/errors'; +import { AutoOpsError } from '../../errors'; import { transformToUTCtime } from '../../../common/utils'; const timeRange = { diff --git a/x-pack/plugins/data_usage/server/services/autoops_api.ts b/x-pack/plugins/data_usage/server/services/autoops_api.ts index 8f371d3004def..0fb9009bb95a5 100644 --- a/x-pack/plugins/data_usage/server/services/autoops_api.ts +++ b/x-pack/plugins/data_usage/server/services/autoops_api.ts @@ -21,7 +21,7 @@ import { type UsageMetricsRequestBody, } from '../../common/rest_types'; import { AutoOpsConfig } from '../types'; -import { AutoOpsError } from './errors'; +import { AutoOpsError } from '../errors'; import { appContextService } from './app_context'; const AUTO_OPS_REQUEST_FAILED_PREFIX = '[AutoOps API] Request failed'; diff --git a/x-pack/plugins/data_usage/server/services/errors.ts b/x-pack/plugins/data_usage/server/services/errors.ts deleted file mode 100644 index 0574e2a3c75fb..0000000000000 --- a/x-pack/plugins/data_usage/server/services/errors.ts +++ /dev/null @@ -1,10 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -import { BaseError } from '../common/errors'; - -export class AutoOpsError extends BaseError {} diff --git a/x-pack/plugins/data_usage/server/services/index.ts b/x-pack/plugins/data_usage/server/services/index.ts index 56e449c8a5679..cf7a24e5ccba5 100644 --- a/x-pack/plugins/data_usage/server/services/index.ts +++ b/x-pack/plugins/data_usage/server/services/index.ts @@ -7,7 +7,7 @@ import { ValidationError } from '@kbn/config-schema'; import { Logger } from '@kbn/logging'; import type { MetricTypes } from '../../common/rest_types'; -import { AutoOpsError } from './errors'; +import { AutoOpsError } from '../errors'; import { AutoOpsAPIService } from './autoops_api'; export class DataUsageService { diff --git a/x-pack/test_serverless/api_integration/test_suites/common/data_usage/index.ts b/x-pack/test_serverless/api_integration/test_suites/common/data_usage/index.ts index cf31e1885d1d5..b0236e8dab722 100644 --- a/x-pack/test_serverless/api_integration/test_suites/common/data_usage/index.ts +++ b/x-pack/test_serverless/api_integration/test_suites/common/data_usage/index.ts @@ -11,6 +11,7 @@ export default function ({ loadTestFile }: FtrProviderContext) { describe('Serverless Data Usage APIs', function () { this.tags(['esGate']); + loadTestFile(require.resolve('./tests/data_streams_privileges')); loadTestFile(require.resolve('./tests/data_streams')); loadTestFile(require.resolve('./tests/metrics')); }); diff --git a/x-pack/test_serverless/api_integration/test_suites/common/data_usage/tests/data_streams_privileges.ts b/x-pack/test_serverless/api_integration/test_suites/common/data_usage/tests/data_streams_privileges.ts new file mode 100644 index 0000000000000..9ea213b6d94ea --- /dev/null +++ b/x-pack/test_serverless/api_integration/test_suites/common/data_usage/tests/data_streams_privileges.ts @@ -0,0 +1,155 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import expect from '@kbn/expect'; +import { DataStreamsResponseBodySchemaBody } from '@kbn/data-usage-plugin/common/rest_types'; +import { DATA_USAGE_DATA_STREAMS_API_ROUTE } from '@kbn/data-usage-plugin/common'; +import type { RoleCredentials } from '@kbn/ftr-common-functional-services'; +import { + NoIndicesMeteringError, + NoPrivilegeMeteringError, +} from '@kbn/data-usage-plugin/server/errors'; +import { FtrProviderContext } from '../../../../ftr_provider_context'; + +export default function ({ getService }: FtrProviderContext) { + const svlDatastreamsHelpers = getService('svlDatastreamsHelpers'); + const svlCommonApi = getService('svlCommonApi'); + const samlAuth = getService('samlAuth'); + const supertestWithoutAuth = getService('supertestWithoutAuth'); + const testDataStreamName = 'test-data-stream'; + const otherTestDataStreamName = 'other-test-data-stream'; + let roleAuthc: RoleCredentials; + + describe('privileges with custom roles', function () { + // custom role testing is not supported in MKI + // the metering api which this route calls requires one of: monitor,view_index_metadata,manage,all + this.tags(['skipSvlOblt', 'skipMKI']); + before(async () => { + await svlDatastreamsHelpers.createDataStream(testDataStreamName); + await svlDatastreamsHelpers.createDataStream(otherTestDataStreamName); + }); + after(async () => { + await svlDatastreamsHelpers.deleteDataStream(testDataStreamName); + await svlDatastreamsHelpers.deleteDataStream(otherTestDataStreamName); + }); + afterEach(async () => { + await samlAuth.invalidateM2mApiKeyWithRoleScope(roleAuthc); + await samlAuth.deleteCustomRole(); + }); + it('returns all data streams for indices with necessary privileges', async () => { + await samlAuth.setCustomRole({ + elasticsearch: { + indices: [{ names: ['*'], privileges: ['all'] }], + }, + kibana: [ + { + base: ['all'], + feature: {}, + spaces: ['*'], + }, + ], + }); + roleAuthc = await samlAuth.createM2mApiKeyWithRoleScope('customRole'); + const res = await supertestWithoutAuth + .get(DATA_USAGE_DATA_STREAMS_API_ROUTE) + .query({ includeZeroStorage: true }) + .set(svlCommonApi.getInternalRequestHeader()) + .set(roleAuthc.apiKeyHeader) + .set('elastic-api-version', '1'); + + const dataStreams: DataStreamsResponseBodySchemaBody = res.body; + const foundTestDataStream = dataStreams.find((stream) => stream.name === testDataStreamName); + const foundTestDataStream2 = dataStreams.find( + (stream) => stream.name === otherTestDataStreamName + ); + expect(res.statusCode).to.be(200); + expect(foundTestDataStream?.name).to.be(testDataStreamName); + expect(foundTestDataStream2?.name).to.be(otherTestDataStreamName); + }); + it('returns data streams for only a subset of indices with necessary privileges', async () => { + await samlAuth.setCustomRole({ + elasticsearch: { + indices: [{ names: ['test-data-stream*'], privileges: ['all'] }], + }, + kibana: [ + { + base: ['all'], + feature: {}, + spaces: ['*'], + }, + ], + }); + roleAuthc = await samlAuth.createM2mApiKeyWithRoleScope('customRole'); + const res = await supertestWithoutAuth + .get(DATA_USAGE_DATA_STREAMS_API_ROUTE) + .query({ includeZeroStorage: true }) + .set(svlCommonApi.getInternalRequestHeader()) + .set(roleAuthc.apiKeyHeader) + .set('elastic-api-version', '1'); + + const dataStreams: DataStreamsResponseBodySchemaBody = res.body; + const foundTestDataStream = dataStreams.find((stream) => stream.name === testDataStreamName); + const dataStreamNoPermission = dataStreams.find( + (stream) => stream.name === otherTestDataStreamName + ); + + expect(res.statusCode).to.be(200); + expect(foundTestDataStream?.name).to.be(testDataStreamName); + expect(dataStreamNoPermission?.name).to.be(undefined); + }); + + it('returns no data streams without necessary privileges', async () => { + await samlAuth.setCustomRole({ + elasticsearch: { + indices: [{ names: ['*'], privileges: ['write'] }], + }, + kibana: [ + { + base: ['all'], + feature: {}, + spaces: ['*'], + }, + ], + }); + roleAuthc = await samlAuth.createM2mApiKeyWithRoleScope('customRole'); + const res = await supertestWithoutAuth + .get(DATA_USAGE_DATA_STREAMS_API_ROUTE) + .query({ includeZeroStorage: true }) + .set(svlCommonApi.getInternalRequestHeader()) + .set(roleAuthc.apiKeyHeader) + .set('elastic-api-version', '1'); + + expect(res.statusCode).to.be(403); + expect(res.body.message).to.contain(new NoPrivilegeMeteringError().message); + }); + + it('returns no data streams when there are none it has access to', async () => { + await samlAuth.setCustomRole({ + elasticsearch: { + indices: [{ names: ['none*'], privileges: ['all'] }], + }, + kibana: [ + { + base: ['all'], + feature: {}, + spaces: ['*'], + }, + ], + }); + roleAuthc = await samlAuth.createM2mApiKeyWithRoleScope('customRole'); + const res = await supertestWithoutAuth + .get(DATA_USAGE_DATA_STREAMS_API_ROUTE) + .query({ includeZeroStorage: true }) + .set(svlCommonApi.getInternalRequestHeader()) + .set(roleAuthc.apiKeyHeader) + .set('elastic-api-version', '1'); + + expect(res.statusCode).to.be(404); + expect(res.body.message).to.contain(new NoIndicesMeteringError().message); + }); + }); +} diff --git a/x-pack/test_serverless/functional/test_suites/common/data_usage/privileges.ts b/x-pack/test_serverless/functional/test_suites/common/data_usage/privileges.ts index e926b43aedfc4..4efcdd2586a85 100644 --- a/x-pack/test_serverless/functional/test_suites/common/data_usage/privileges.ts +++ b/x-pack/test_serverless/functional/test_suites/common/data_usage/privileges.ts @@ -5,6 +5,11 @@ * 2.0. */ +import expect from '@kbn/expect'; +import { + NoIndicesMeteringError, + NoPrivilegeMeteringError, +} from '@kbn/data-usage-plugin/server/errors'; import { FtrProviderContext } from '../../../ftr_provider_context'; export default ({ getPageObjects, getService }: FtrProviderContext) => { @@ -12,7 +17,9 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => { const testSubjects = getService('testSubjects'); const samlAuth = getService('samlAuth'); const retry = getService('retry'); + const es = getService('es'); const dataUsageAppUrl = 'management/data/data_usage'; + const toasts = getService('toasts'); const navigateAndVerify = async (expectedVisible: boolean) => { await pageObjects.common.navigateToApp('management'); @@ -32,6 +39,32 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => { }; describe('privileges', function () { + before(async () => { + await es.indices.putIndexTemplate({ + name: 'test-datastream', + body: { + index_patterns: ['test-datastream'], + data_stream: {}, + priority: 200, + }, + }); + + await es.indices.createDataStream({ name: 'test-datastream' }); + await es.indices.putIndexTemplate({ + name: 'no-permission-test-datastream', + body: { + index_patterns: ['no-permission-test-datastream'], + data_stream: {}, + priority: 200, + }, + }); + + await es.indices.createDataStream({ name: 'no-permission-test-datastream' }); + }); + after(async () => { + await es.indices.deleteDataStream({ name: 'test-datastream' }); + await es.indices.deleteDataStream({ name: 'no-permission-test-datastream' }); + }); it('renders for the admin role', async () => { await pageObjects.svlCommonPage.loginAsAdmin(); await navigateAndVerify(true); @@ -63,7 +96,7 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => { afterEach(async () => { await samlAuth.deleteCustomRole(); }); - it('renders with a custom role that has the monitor cluster privilege', async () => { + it('renders with a custom role that has the privileges cluster: monitor and indices all', async () => { await samlAuth.setCustomRole({ elasticsearch: { cluster: ['monitor'], @@ -97,6 +130,72 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => { await pageObjects.svlCommonPage.loginWithCustomRole(); await navigateAndVerify(false); }); + + describe.skip('with custom role and data streams', function () { + // skip in all environments. requires a code change to the data_streams route + // to allow zero storage data streams to not be filtered out, but useful for testing. + // the api integration tests can pass a flag to get around this case but we can't in the UI. + // metering api requires one of: monitor,view_index_metadata,manage,all + it('does not load data streams without necessary index privilege for any index', async () => { + await samlAuth.setCustomRole({ + elasticsearch: { + cluster: ['monitor'], + indices: [{ names: ['*'], privileges: ['read'] }], + }, + kibana: [ + { + base: ['all'], + feature: {}, + spaces: ['*'], + }, + ], + }); + await pageObjects.svlCommonPage.loginWithCustomRole(); + await navigateAndVerify(true); + const toastContent = await toasts.getContentByIndex(1); + expect(toastContent).to.contain(NoPrivilegeMeteringError); + }); + + it('does load data streams with necessary index privilege for some indices', async () => { + await samlAuth.setCustomRole({ + elasticsearch: { + cluster: ['monitor'], + indices: [ + { names: ['test-datastream*'], privileges: ['all'] }, + { names: ['.*'], privileges: ['read'] }, + ], + }, + kibana: [ + { + base: ['all'], + feature: {}, + spaces: ['*'], + }, + ], + }); + await pageObjects.svlCommonPage.loginWithCustomRole(); + await navigateAndVerify(true); + }); + it('handles error when no data streams that it has permission to exist (index_not_found_exception)', async () => { + await samlAuth.setCustomRole({ + elasticsearch: { + cluster: ['monitor'], + indices: [{ names: ['none*'], privileges: ['all'] }], + }, + kibana: [ + { + base: ['all'], + feature: {}, + spaces: ['*'], + }, + ], + }); + await pageObjects.svlCommonPage.loginWithCustomRole(); + await navigateAndVerify(true); + const toastContent = await toasts.getContentByIndex(1); + expect(toastContent).to.contain(NoIndicesMeteringError); + }); + }); }); }); };