Skip to content

Commit

Permalink
[Data Usage] add error handling and tests for privilege related errors (
Browse files Browse the repository at this point in the history
elastic#203006)

- 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
<img width="1555" alt="Screenshot 2024-12-04 at 1 14 10 PM"
src="https://github.com/user-attachments/assets/241a2eb8-1c77-4592-ba18-b971512e712e">

### `index_not_found_exception` view
<img width="1589" alt="Screenshot 2024-12-04 at 1 13 13 PM"
src="https://github.com/user-attachments/assets/12b68d66-9178-4957-b014-5765be348694">

---------

Co-authored-by: Ashokaditya <ashokaditya@elastic.co>
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
3 people authored and Samiul-TheSoccerFan committed Dec 10, 2024
1 parent 5262649 commit 35446b6
Show file tree
Hide file tree
Showing 12 changed files with 345 additions and 20 deletions.
2 changes: 1 addition & 1 deletion x-pack/plugins/data_usage/server/common/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

export class BaseError<MetaType = unknown> extends Error {
export class DataUsageError<MetaType = unknown> extends Error {
constructor(message: string, public readonly meta?: MetaType) {
super(message);
// For debugging - capture name of subclasses
Expand Down
30 changes: 30 additions & 0 deletions x-pack/plugins/data_usage/server/errors.ts
Original file line number Diff line number Diff line change
@@ -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.'
);
}
}
18 changes: 14 additions & 4 deletions x-pack/plugins/data_usage/server/routes/error_handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -43,6 +45,14 @@ export const errorHandler = <E extends Error>(
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;
};
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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!')
Expand All @@ -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: [] }, []],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/data_usage/server/services/autoops_api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
10 changes: 0 additions & 10 deletions x-pack/plugins/data_usage/server/services/errors.ts

This file was deleted.

2 changes: 1 addition & 1 deletion x-pack/plugins/data_usage/server/services/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'));
});
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
});
});
}
Loading

0 comments on commit 35446b6

Please sign in to comment.