Skip to content

Commit

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

# Backport

This will backport the following commits from `main` to `8.16`:
- [[ResponseOps][Rules] Remove unintended internal Find routes API with
public access (elastic#193757)](elastic#193757)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Zacqary Adam
Xeper","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-23T04:33:31Z","message":"[ResponseOps][Rules]
Remove unintended internal Find routes API with public access
(elastic#193757)\n\n## Summary\r\n\r\nFixes elastic#192957 \r\n\r\nRemoves the
`internal/_find` route from public access by moving the\r\nhard-coded
`options` into the route builder
functions.\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine
<[email protected]>","sha":"196cabad9bbd0511219fc7833a62cb8a0bb61514","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:ResponseOps","v9.0.0","Feature:Alerting/RulesFramework","backport:prev-minor","ci:project-deploy-observability","v8.16.0"],"title":"[ResponseOps][Rules]
Remove unintended internal Find routes API with public
access","number":193757,"url":"https://github.com/elastic/kibana/pull/193757","mergeCommit":{"message":"[ResponseOps][Rules]
Remove unintended internal Find routes API with public access
(elastic#193757)\n\n## Summary\r\n\r\nFixes elastic#192957 \r\n\r\nRemoves the
`internal/_find` route from public access by moving the\r\nhard-coded
`options` into the route builder
functions.\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine
<[email protected]>","sha":"196cabad9bbd0511219fc7833a62cb8a0bb61514"}},"sourceBranch":"main","suggestedTargetBranches":["8.16"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/193757","number":193757,"mergeCommit":{"message":"[ResponseOps][Rules]
Remove unintended internal Find routes API with public access
(elastic#193757)\n\n## Summary\r\n\r\nFixes elastic#192957 \r\n\r\nRemoves the
`internal/_find` route from public access by moving the\r\nhard-coded
`options` into the route builder
functions.\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine
<[email protected]>","sha":"196cabad9bbd0511219fc7833a62cb8a0bb61514"}},{"branch":"8.16","label":"v8.16.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Zacqary Adam Xeper <[email protected]>
  • Loading branch information
kibanamachine and Zacqary authored Oct 23, 2024
1 parent 30127c0 commit d77d619
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 d77d619

Please sign in to comment.