Skip to content

Commit

Permalink
[ResponseOps][Rules] Remove unintended internal Find routes API with …
Browse files Browse the repository at this point in the history
…public access (#193757)

## Summary

Fixes #192957 

Removes the `internal/_find` route from public access by moving the
hard-coded `options` into the route builder functions.

---------

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
  • Loading branch information
Zacqary and elasticmachine authored Oct 23, 2024
1 parent 75a6406 commit 196caba
Show file tree
Hide file tree
Showing 19 changed files with 1,450 additions and 387 deletions.
3 changes: 2 additions & 1 deletion x-pack/plugins/alerting/server/routes/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ import { deleteRuleRoute } from './rule/apis/delete/delete_rule_route';
import { aggregateRulesRoute } from './rule/apis/aggregate/aggregate_rules_route';
import { disableRuleRoute } from './rule/apis/disable/disable_rule_route';
import { enableRuleRoute } from './rule/apis/enable/enable_rule_route';
import { findRulesRoute, findInternalRulesRoute } from './rule/apis/find/find_rules_route';
import { findRulesRoute } from './rule/apis/find/find_rules_route';
import { findInternalRulesRoute } from './rule/apis/find/find_internal_rules_route';
import { getRuleAlertSummaryRoute } from './get_rule_alert_summary';
import { getRuleExecutionLogRoute } from './get_rule_execution_log';
import { getGlobalExecutionLogRoute } from './get_global_execution_logs';
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
* 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 { httpServiceMock } from '@kbn/core/server/mocks';
import { licenseStateMock } from '../../../../lib/license_state.mock';
import { findInternalRulesRoute } from './find_internal_rules_route';

jest.mock('../../../../lib/license_api_access', () => ({
verifyApiAccess: jest.fn(),
}));

jest.mock('../../../lib/track_legacy_terminology', () => ({
trackLegacyTerminology: jest.fn(),
}));

beforeEach(() => {
jest.resetAllMocks();
});

describe('findInternalRulesRoute', () => {
it('registers the route without public access', async () => {
const licenseState = licenseStateMock.create();
const router = httpServiceMock.createRouter();

findInternalRulesRoute(router, licenseState);
expect(router.post).toHaveBeenCalledWith(
expect.not.objectContaining({
options: expect.objectContaining({ access: 'public' }),
}),
expect.any(Function)
);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
/*
* 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 { IRouter } from '@kbn/core/server';
import { UsageCounter } from '@kbn/usage-collection-plugin/server';
import type {
FindRulesRequestQueryV1,
FindRulesResponseV1,
} from '../../../../../common/routes/rule/apis/find';
import { findRulesRequestQuerySchemaV1 } from '../../../../../common/routes/rule/apis/find';
import { RuleParamsV1 } from '../../../../../common/routes/rule/response';
import { ILicenseState } from '../../../../lib';
import {
AlertingRequestHandlerContext,
INTERNAL_ALERTING_API_FIND_RULES_PATH,
} from '../../../../types';
import { verifyAccessAndContext } from '../../../lib';
import { trackLegacyTerminology } from '../../../lib/track_legacy_terminology';
import { transformFindRulesBodyV1, transformFindRulesResponseV1 } from './transforms';

export const findInternalRulesRoute = (
router: IRouter<AlertingRequestHandlerContext>,
licenseState: ILicenseState,
usageCounter?: UsageCounter
) => {
router.post(
{
path: INTERNAL_ALERTING_API_FIND_RULES_PATH,
options: { access: 'internal' },
validate: {
body: findRulesRequestQuerySchemaV1,
},
},
router.handleLegacyErrors(
verifyAccessAndContext(licenseState, async function (context, req, res) {
const rulesClient = (await context.alerting).getRulesClient();

const body: FindRulesRequestQueryV1 = req.body;

trackLegacyTerminology(
[req.body.search, req.body.search_fields, req.body.sort_field].filter(
Boolean
) as string[],
usageCounter
);

const options = transformFindRulesBodyV1({
...body,
has_reference: body.has_reference || undefined,
search_fields: searchFieldsAsArray(body.search_fields),
});

if (req.body.fields) {
usageCounter?.incrementCounter({
counterName: `alertingFieldsUsage`,
counterType: 'alertingFieldsUsage',
incrementBy: 1,
});
}

const findResult = await rulesClient.find({
options,
excludeFromPublicApi: false,
includeSnoozeData: true,
});

const responseBody: FindRulesResponseV1<RuleParamsV1>['body'] =
transformFindRulesResponseV1<RuleParamsV1>(findResult, options.fields);

return res.ok({
body: responseBody,
});
})
)
);
};

function searchFieldsAsArray(searchFields: string | string[] | undefined): string[] | undefined {
if (!searchFields) {
return;
}
return Array.isArray(searchFields) ? searchFields : [searchFields];
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,18 @@ beforeEach(() => {
});

describe('findRulesRoute', () => {
it('registers the route with public access', async () => {
const licenseState = licenseStateMock.create();
const router = httpServiceMock.createRouter();

findRulesRoute(router, licenseState);
expect(router.get).toHaveBeenCalledWith(
expect.objectContaining({
options: expect.objectContaining({ access: 'public' }),
}),
expect.any(Function)
);
});
it('finds rules with proper parameters', async () => {
const licenseState = licenseStateMock.create();
const router = httpServiceMock.createRouter();
Expand Down
116 changes: 11 additions & 105 deletions x-pack/plugins/alerting/server/routes/rule/apis/find/find_rules_route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,40 +7,26 @@

import { IRouter } from '@kbn/core/server';
import { UsageCounter } from '@kbn/usage-collection-plugin/server';
import { ILicenseState } from '../../../../lib';
import { verifyAccessAndContext } from '../../../lib';
import { findRulesRequestQuerySchemaV1 } from '../../../../../common/routes/rule/apis/find';
import type {
FindRulesRequestQueryV1,
FindRulesResponseV1,
} from '../../../../../common/routes/rule/apis/find';
import { findRulesRequestQuerySchemaV1 } from '../../../../../common/routes/rule/apis/find';
import { RuleParamsV1, ruleResponseSchemaV1 } from '../../../../../common/routes/rule/response';
import {
AlertingRequestHandlerContext,
BASE_ALERTING_API_PATH,
INTERNAL_ALERTING_API_FIND_RULES_PATH,
} from '../../../../types';
import { ILicenseState } from '../../../../lib';
import { AlertingRequestHandlerContext, BASE_ALERTING_API_PATH } from '../../../../types';
import { verifyAccessAndContext } from '../../../lib';
import { trackLegacyTerminology } from '../../../lib/track_legacy_terminology';
import { transformFindRulesBodyV1, transformFindRulesResponseV1 } from './transforms';

interface BuildFindRulesRouteParams {
licenseState: ILicenseState;
path: string;
router: IRouter<AlertingRequestHandlerContext>;
excludeFromPublicApi?: boolean;
usageCounter?: UsageCounter;
}

const buildFindRulesRoute = ({
licenseState,
path,
router,
excludeFromPublicApi = false,
usageCounter,
}: BuildFindRulesRouteParams) => {
export const findRulesRoute = (
router: IRouter<AlertingRequestHandlerContext>,
licenseState: ILicenseState,
usageCounter?: UsageCounter
) => {
router.get(
{
path,
path: `${BASE_ALERTING_API_PATH}/rules/_find`,
options: {
access: 'public',
summary: 'Get information about rules',
Expand Down Expand Up @@ -91,7 +77,7 @@ const buildFindRulesRoute = ({

const findResult = await rulesClient.find({
options,
excludeFromPublicApi,
excludeFromPublicApi: true,
includeSnoozeData: true,
});

Expand All @@ -104,86 +90,6 @@ const buildFindRulesRoute = ({
})
)
);
if (path === INTERNAL_ALERTING_API_FIND_RULES_PATH) {
router.post(
{
path,
options: { access: 'internal' },
validate: {
body: findRulesRequestQuerySchemaV1,
},
},
router.handleLegacyErrors(
verifyAccessAndContext(licenseState, async function (context, req, res) {
const rulesClient = (await context.alerting).getRulesClient();

const body: FindRulesRequestQueryV1 = req.body;

trackLegacyTerminology(
[req.body.search, req.body.search_fields, req.body.sort_field].filter(
Boolean
) as string[],
usageCounter
);

const options = transformFindRulesBodyV1({
...body,
has_reference: body.has_reference || undefined,
search_fields: searchFieldsAsArray(body.search_fields),
});

if (req.body.fields) {
usageCounter?.incrementCounter({
counterName: `alertingFieldsUsage`,
counterType: 'alertingFieldsUsage',
incrementBy: 1,
});
}

const findResult = await rulesClient.find({
options,
excludeFromPublicApi,
includeSnoozeData: true,
});

const responseBody: FindRulesResponseV1<RuleParamsV1>['body'] =
transformFindRulesResponseV1<RuleParamsV1>(findResult, options.fields);

return res.ok({
body: responseBody,
});
})
)
);
}
};

export const findRulesRoute = (
router: IRouter<AlertingRequestHandlerContext>,
licenseState: ILicenseState,
usageCounter?: UsageCounter
) => {
buildFindRulesRoute({
excludeFromPublicApi: true,
licenseState,
path: `${BASE_ALERTING_API_PATH}/rules/_find`,
router,
usageCounter,
});
};

export const findInternalRulesRoute = (
router: IRouter<AlertingRequestHandlerContext>,
licenseState: ILicenseState,
usageCounter?: UsageCounter
) => {
buildFindRulesRoute({
excludeFromPublicApi: false,
licenseState,
path: INTERNAL_ALERTING_API_FIND_RULES_PATH,
router,
usageCounter,
});
};

function searchFieldsAsArray(searchFields: string | string[] | undefined): string[] | undefined {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -854,9 +854,7 @@ describe('Detections Rules API', () => {
expect(fetchMock).toHaveBeenCalledWith(
expect.any(String),
expect.objectContaining({
query: expect.objectContaining({
filter: 'alert.id:"alert:id1" or alert.id:"alert:id2"',
}),
body: '{"filter":"alert.id:\\"alert:id1\\" or alert.id:\\"alert:id2\\"","fields":"[\\"muteAll\\",\\"activeSnoozes\\",\\"isSnoozedUntil\\",\\"snoozeSchedule\\"]","per_page":2}',
})
);
});
Expand All @@ -867,9 +865,7 @@ describe('Detections Rules API', () => {
expect(fetchMock).toHaveBeenCalledWith(
expect.any(String),
expect.objectContaining({
query: expect.objectContaining({
per_page: 2,
}),
body: '{"filter":"alert.id:\\"alert:id1\\" or alert.id:\\"alert:id2\\"","fields":"[\\"muteAll\\",\\"activeSnoozes\\",\\"isSnoozedUntil\\",\\"snoozeSchedule\\"]","per_page":2}',
})
);
});
Expand All @@ -880,14 +876,7 @@ describe('Detections Rules API', () => {
expect(fetchMock).toHaveBeenCalledWith(
expect.any(String),
expect.objectContaining({
query: expect.objectContaining({
fields: JSON.stringify([
'muteAll',
'activeSnoozes',
'isSnoozedUntil',
'snoozeSchedule',
]),
}),
body: '{"filter":"alert.id:\\"alert:id1\\" or alert.id:\\"alert:id2\\"","fields":"[\\"muteAll\\",\\"activeSnoozes\\",\\"isSnoozedUntil\\",\\"snoozeSchedule\\"]","per_page":2}',
})
);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,12 +241,12 @@ export const fetchRulesSnoozeSettings = async ({
const response = await KibanaServices.get().http.fetch<RulesSnoozeSettingsBatchResponse>(
INTERNAL_ALERTING_API_FIND_RULES_PATH,
{
method: 'GET',
query: {
method: 'POST',
body: JSON.stringify({
filter: ids.map((x) => `alert.id:"alert:${x}"`).join(' or '),
fields: JSON.stringify(['muteAll', 'activeSnoozes', 'isSnoozedUntil', 'snoozeSchedule']),
per_page: ids.length,
},
}),
signal,
}
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import type { RulesSnoozeSettingsMap } from '../../logic';
import { fetchRulesSnoozeSettings } from '../api';
import { DEFAULT_QUERY_OPTIONS } from './constants';

const FETCH_RULE_SNOOZE_SETTINGS_QUERY_KEY = ['GET', INTERNAL_ALERTING_API_FIND_RULES_PATH];
const FETCH_RULE_SNOOZE_SETTINGS_QUERY_KEY = ['POST', INTERNAL_ALERTING_API_FIND_RULES_PATH];

/**
* A wrapper around useQuery provides default values to the underlying query,
Expand Down
Loading

0 comments on commit 196caba

Please sign in to comment.