Skip to content

Commit

Permalink
[Security Solution] DetectionRulesClient: various refactorings (#18…
Browse files Browse the repository at this point in the history
…4954)

**Partially addresses: #184364

## Summary
This PR contains various smaller-scale refactorings for the recently
added `DetectionsRuleClient`.

**Changes**:
- Renamed `DetectionRulesClient` containing directory from
`rule_management` to `detection_rules_client`
- Moved `DetectionRulesClient` methods into the
`detection_rules_client/methods` dir
- Moved the TS interface of `DetectionRulesClient` into a separate file
`detection_rules_client_interface.ts`
- Simplified `importRule` method parameters
- Added memoization to `getDetectionRulesClient`

---------

Co-authored-by: Georgii Gorbachev <banderror@gmail.com>
  • Loading branch information
nikitaindik and banderror authored Jun 7, 2024
1 parent 87f6c1d commit 41c34e0
Show file tree
Hide file tree
Showing 48 changed files with 519 additions and 512 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { MAX_RULES_TO_UPDATE_IN_PARALLEL } from '../../../../../../common/consta
import { initPromisePool } from '../../../../../utils/promise_pool';
import { withSecuritySpan } from '../../../../../utils/with_security_span';
import type { PrebuiltRuleAsset } from '../../model/rule_assets/prebuilt_rule_asset';
import type { IDetectionRulesClient } from '../../../rule_management/logic/rule_management/detection_rules_client';
import type { IDetectionRulesClient } from '../../../rule_management/logic/detection_rules_client/detection_rules_client_interface';

export const createPrebuiltRules = (
detectionRulesClient: IDetectionRulesClient,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { MAX_RULES_TO_UPDATE_IN_PARALLEL } from '../../../../../../common/consta
import { initPromisePool } from '../../../../../utils/promise_pool';
import { withSecuritySpan } from '../../../../../utils/with_security_span';
import type { PrebuiltRuleAsset } from '../../model/rule_assets/prebuilt_rule_asset';
import type { IDetectionRulesClient } from '../../../rule_management/logic/rule_management/detection_rules_client';
import type { IDetectionRulesClient } from '../../../rule_management/logic/detection_rules_client/detection_rules_client_interface';

/**
* Upgrades existing prebuilt rules given a set of rules and output index.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ import { riskEngineDataClientMock } from '../../../entity_analytics/risk_engine/
import { riskScoreDataClientMock } from '../../../entity_analytics/risk_score/risk_score_data_client.mock';
import { assetCriticalityDataClientMock } from '../../../entity_analytics/asset_criticality/asset_criticality_data_client.mock';
import { auditLoggerMock } from '@kbn/security-plugin/server/audit/mocks';
import { detectionRulesClientMock } from '../../rule_management/logic/rule_management/__mocks__/detection_rules_client';
import { detectionRulesClientMock } from '../../rule_management/logic/detection_rules_client/__mocks__/detection_rules_client';

export const createMockClients = () => {
const core = coreMock.createRequestHandlerContext();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,13 @@ import {
CreateRuleExceptionsRequestParams,
} from '../../../../../../common/api/detection_engine/rule_exceptions';

import { readRules } from '../../../rule_management/logic/rule_management/read_rules';
import { readRules } from '../../../rule_management/logic/detection_rules_client/read_rules';
import { checkDefaultRuleExceptionListReferences } from '../../../rule_management/logic/exceptions/check_for_default_rule_exception_list';
import type { RuleParams } from '../../../rule_schema';
import type { SecuritySolutionPluginRouter } from '../../../../../types';
import { buildSiemResponse } from '../../../routes/utils';
import { buildRouteValidation } from '../../../../../utils/build_validation/route_validation';
import type { IDetectionRulesClient } from '../../../rule_management/logic/rule_management/detection_rules_client';
import type { IDetectionRulesClient } from '../../../rule_management/logic/detection_rules_client/detection_rules_client_interface';

export const createRuleExceptionsRoute = (router: SecuritySolutionPluginRouter) => {
router.versioned
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { MAX_RULES_TO_UPDATE_IN_PARALLEL } from '../../../../../../../common/con
import type { PromisePoolOutcome } from '../../../../../../utils/promise_pool';
import { initPromisePool } from '../../../../../../utils/promise_pool';
import type { RuleAlertType } from '../../../../rule_schema';
import { readRules } from '../../../logic/rule_management/read_rules';
import { readRules } from '../../../logic/detection_rules_client/read_rules';
import { findRules } from '../../../logic/search/find_rules';
import { MAX_RULES_TO_PROCESS_TOTAL } from './route';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@ import {
getBulkDisableRuleActionSchemaMock,
} from '../../../../../../../common/api/detection_engine/rule_management/mocks';
import { loggingSystemMock } from '@kbn/core/server/mocks';
import { readRules } from '../../../logic/rule_management/read_rules';
import { readRules } from '../../../logic/detection_rules_client/read_rules';

jest.mock('../../../../../machine_learning/authz');
jest.mock('../../../logic/rule_management/read_rules', () => ({ readRules: jest.fn() }));
jest.mock('../../../logic/detection_rules_client/read_rules', () => ({ readRules: jest.fn() }));

describe('Perform bulk action route', () => {
const readRulesMock = readRules as jest.Mock;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {
} from '../../../../../../../common/api/detection_engine/rule_management';

import type { SecuritySolutionPluginRouter } from '../../../../../../types';
import { readRules } from '../../../logic/rule_management/read_rules';
import { readRules } from '../../../logic/detection_rules_client/read_rules';
import { getDuplicates } from './get_duplicates';
import { transformValidateBulkError } from '../../../utils/validate';
import { buildRouteValidationWithZod } from '../../../../../../utils/build_validation/route_validation';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import {
createBulkErrorObject,
transformBulkError,
} from '../../../../routes/utils';
import { readRules } from '../../../logic/rule_management/read_rules';
import { readRules } from '../../../logic/detection_rules_client/read_rules';
import { getIdBulkError } from '../../../utils/utils';
import { transformValidateBulkError } from '../../../utils/validate';
import { getDeprecatedBulkEndpointHeader, logDeprecatedBulkEndpoint } from '../../deprecation';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import type { SecuritySolutionPluginRouter } from '../../../../../../types';
import { transformBulkError, buildSiemResponse } from '../../../../routes/utils';
import { getIdBulkError } from '../../../utils/utils';
import { transformValidateBulkError } from '../../../utils/validate';
import { readRules } from '../../../logic/rule_management/read_rules';
import { readRules } from '../../../logic/detection_rules_client/read_rules';
import { getDeprecatedBulkEndpointHeader, logDeprecatedBulkEndpoint } from '../../deprecation';
import { validateRuleDefaultExceptionList } from '../../../logic/exceptions/validate_rule_default_exception_list';
import { validateRulesWithDuplicatedDefaultExceptionsList } from '../../../logic/exceptions/validate_rules_with_duplicated_default_exceptions_list';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import {
buildSiemResponse,
createBulkErrorObject,
} from '../../../../routes/utils';
import { readRules } from '../../../logic/rule_management/read_rules';
import { readRules } from '../../../logic/detection_rules_client/read_rules';
import { getDeprecatedBulkEndpointHeader, logDeprecatedBulkEndpoint } from '../../deprecation';
import { validateRuleDefaultExceptionList } from '../../../logic/exceptions/validate_rule_default_exception_list';
import { validateRulesWithDuplicatedDefaultExceptionsList } from '../../../logic/exceptions/validate_rules_with_duplicated_default_exceptions_list';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { DETECTION_ENGINE_RULES_URL } from '../../../../../../../common/constant
import type { SecuritySolutionPluginRouter } from '../../../../../../types';
import { buildRouteValidationWithZod } from '../../../../../../utils/build_validation/route_validation';
import { buildSiemResponse } from '../../../../routes/utils';
import { readRules } from '../../../logic/rule_management/read_rules';
import { readRules } from '../../../logic/detection_rules_client/read_rules';
import { checkDefaultRuleExceptionListReferences } from '../../../logic/exceptions/check_for_default_rule_exception_list';
import { validateRuleDefaultExceptionList } from '../../../logic/exceptions/validate_rule_default_exception_list';
import { transformValidate, validateResponseActionsPermissions } from '../../../utils/validate';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { DETECTION_ENGINE_RULES_URL } from '../../../../../../../common/constant
import type { SecuritySolutionPluginRouter } from '../../../../../../types';
import { buildRouteValidationWithZod } from '../../../../../../utils/build_validation/route_validation';
import { buildSiemResponse } from '../../../../routes/utils';
import { readRules } from '../../../logic/rule_management/read_rules';
import { readRules } from '../../../logic/detection_rules_client/read_rules';
import { getIdError, transform } from '../../../utils/utils';

export const deleteRuleRoute = (router: SecuritySolutionPluginRouter) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { DETECTION_ENGINE_RULES_URL } from '../../../../../../../common/constant
import type { SecuritySolutionPluginRouter } from '../../../../../../types';
import { buildRouteValidationWithZod } from '../../../../../../utils/build_validation/route_validation';
import { buildSiemResponse } from '../../../../routes/utils';
import { readRules } from '../../../logic/rule_management/read_rules';
import { readRules } from '../../../logic/detection_rules_client/read_rules';
import { checkDefaultRuleExceptionListReferences } from '../../../logic/exceptions/check_for_default_rule_exception_list';
import { validateRuleDefaultExceptionList } from '../../../logic/exceptions/validate_rule_default_exception_list';
import { getIdError } from '../../../utils/utils';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { DETECTION_ENGINE_RULES_URL } from '../../../../../../../common/constant
import type { SecuritySolutionPluginRouter } from '../../../../../../types';
import { buildRouteValidationWithZod } from '../../../../../../utils/build_validation/route_validation';
import { buildSiemResponse } from '../../../../routes/utils';
import { readRules } from '../../../logic/rule_management/read_rules';
import { readRules } from '../../../logic/detection_rules_client/read_rules';
import { getIdError, transform } from '../../../utils/utils';

export const readRuleRoute = (router: SecuritySolutionPluginRouter, logger: Logger) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { DETECTION_ENGINE_RULES_URL } from '../../../../../../../common/constant
import type { SecuritySolutionPluginRouter } from '../../../../../../types';
import { buildRouteValidationWithZod } from '../../../../../../utils/build_validation/route_validation';
import { buildSiemResponse } from '../../../../routes/utils';
import { readRules } from '../../../logic/rule_management/read_rules';
import { readRules } from '../../../logic/detection_rules_client/read_rules';
import { checkDefaultRuleExceptionListReferences } from '../../../logic/exceptions/check_for_default_rule_exception_list';
import { validateRuleDefaultExceptionList } from '../../../logic/exceptions/validate_rule_default_exception_list';
import { getIdError } from '../../../utils/utils';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import type { IDetectionRulesClient } from '../detection_rules_client';
import type { IDetectionRulesClient } from '../detection_rules_client_interface';

export type DetectionRulesClientMock = jest.Mocked<IDetectionRulesClient>;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { DEFAULT_INDICATOR_SOURCE_PATH } from '../../../../../../common/constant
import { buildMlAuthz } from '../../../../machine_learning/authz';
import { throwAuthzError } from '../../../../machine_learning/validation';
import { createDetectionRulesClient } from './detection_rules_client';
import type { IDetectionRulesClient } from './detection_rules_client';
import type { IDetectionRulesClient } from './detection_rules_client_interface';

jest.mock('../../../../machine_learning/authz');
jest.mock('../../../../machine_learning/validation');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { DEFAULT_INDICATOR_SOURCE_PATH } from '../../../../../../common/constant
import { buildMlAuthz } from '../../../../machine_learning/authz';
import { throwAuthzError } from '../../../../machine_learning/validation';
import { createDetectionRulesClient } from './detection_rules_client';
import type { IDetectionRulesClient } from './detection_rules_client';
import type { IDetectionRulesClient } from './detection_rules_client_interface';

jest.mock('../../../../machine_learning/authz');
jest.mock('../../../../machine_learning/validation');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import { rulesClientMock } from '@kbn/alerting-plugin/server/mocks';
import { buildMlAuthz } from '../../../../machine_learning/authz';
import { createDetectionRulesClient } from './detection_rules_client';
import type { IDetectionRulesClient } from './detection_rules_client';
import type { IDetectionRulesClient } from './detection_rules_client_interface';

jest.mock('../../../../machine_learning/authz');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { getQueryRuleParams } from '../../../rule_schema/mocks';
import { buildMlAuthz } from '../../../../machine_learning/authz';
import { throwAuthzError } from '../../../../machine_learning/validation';
import { createDetectionRulesClient } from './detection_rules_client';
import type { IDetectionRulesClient } from './detection_rules_client';
import type { IDetectionRulesClient } from './detection_rules_client_interface';

jest.mock('../../../../machine_learning/authz');
jest.mock('../../../../machine_learning/validation');
Expand Down Expand Up @@ -50,7 +50,7 @@ describe('DetectionRulesClient.importRule', () => {
await detectionRulesClient.importRule({
ruleToImport,
overwriteRules: true,
options: { allowMissingConnectorSecrets },
allowMissingConnectorSecrets,
});

expect(rulesClient.create).toHaveBeenCalledWith(
Expand Down Expand Up @@ -78,7 +78,7 @@ describe('DetectionRulesClient.importRule', () => {
detectionRulesClient.importRule({
ruleToImport,
overwriteRules: true,
options: { allowMissingConnectorSecrets },
allowMissingConnectorSecrets,
})
).rejects.toThrow('mocked MLAuth error');

Expand All @@ -92,7 +92,7 @@ describe('DetectionRulesClient.importRule', () => {
await detectionRulesClient.importRule({
ruleToImport,
overwriteRules: true,
options: { allowMissingConnectorSecrets },
allowMissingConnectorSecrets,
});

expect(rulesClient.update).toHaveBeenCalledWith(
Expand Down Expand Up @@ -133,7 +133,7 @@ describe('DetectionRulesClient.importRule', () => {
timestamp_override: undefined,
},
overwriteRules: true,
options: { allowMissingConnectorSecrets },
allowMissingConnectorSecrets,
});

expect(rulesClient.create).not.toHaveBeenCalled();
Expand All @@ -154,7 +154,7 @@ describe('DetectionRulesClient.importRule', () => {
detectionRulesClient.importRule({
ruleToImport,
overwriteRules: false,
options: { allowMissingConnectorSecrets },
allowMissingConnectorSecrets,
})
).rejects.toMatchObject({
error: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { readRules } from './read_rules';
import { buildMlAuthz } from '../../../../machine_learning/authz';
import { throwAuthzError } from '../../../../machine_learning/validation';
import { createDetectionRulesClient } from './detection_rules_client';
import type { IDetectionRulesClient } from './detection_rules_client';
import type { IDetectionRulesClient } from './detection_rules_client_interface';

jest.mock('../../../../machine_learning/authz');
jest.mock('../../../../machine_learning/validation');
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
/*
* 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 type { RulesClient } from '@kbn/alerting-plugin/server';
import type { MlAuthz } from '../../../../machine_learning/authz';

import type { RuleAlertType } from '../../../rule_schema';
import type {
IDetectionRulesClient,
CreateCustomRuleArgs,
CreatePrebuiltRuleArgs,
UpdateRuleArgs,
PatchRuleArgs,
DeleteRuleArgs,
UpgradePrebuiltRuleArgs,
ImportRuleArgs,
} from './detection_rules_client_interface';

import { createCustomRule } from './methods/create_custom_rule';
import { createPrebuiltRule } from './methods/create_prebuilt_rule';
import { updateRule } from './methods/update_rule';
import { patchRule } from './methods/patch_rule';
import { deleteRule } from './methods/delete_rule';
import { upgradePrebuiltRule } from './methods/upgrade_prebuilt_rule';
import { importRule } from './methods/import_rule';

import { withSecuritySpan } from '../../../../../utils/with_security_span';

export const createDetectionRulesClient = (
rulesClient: RulesClient,
mlAuthz: MlAuthz
): IDetectionRulesClient => ({
async createCustomRule(args: CreateCustomRuleArgs): Promise<RuleAlertType> {
return withSecuritySpan('DetectionRulesClient.createCustomRule', async () => {
return createCustomRule(rulesClient, args, mlAuthz);
});
},

async createPrebuiltRule(args: CreatePrebuiltRuleArgs): Promise<RuleAlertType> {
return withSecuritySpan('DetectionRulesClient.createPrebuiltRule', async () => {
return createPrebuiltRule(rulesClient, args, mlAuthz);
});
},

async updateRule(args: UpdateRuleArgs): Promise<RuleAlertType> {
return withSecuritySpan('DetectionRulesClient.updateRule', async () => {
return updateRule(rulesClient, args, mlAuthz);
});
},

async patchRule(args: PatchRuleArgs): Promise<RuleAlertType> {
return withSecuritySpan('DetectionRulesClient.patchRule', async () => {
return patchRule(rulesClient, args, mlAuthz);
});
},

async deleteRule(args: DeleteRuleArgs): Promise<void> {
return withSecuritySpan('DetectionRulesClient.deleteRule', async () => {
return deleteRule(rulesClient, args);
});
},

async upgradePrebuiltRule(args: UpgradePrebuiltRuleArgs): Promise<RuleAlertType> {
return withSecuritySpan('DetectionRulesClient.upgradePrebuiltRule', async () => {
return upgradePrebuiltRule(rulesClient, args, mlAuthz);
});
},

async importRule(args: ImportRuleArgs): Promise<RuleAlertType> {
return withSecuritySpan('DetectionRulesClient.importRule', async () => {
return importRule(rulesClient, args, mlAuthz);
});
},
});
Loading

0 comments on commit 41c34e0

Please sign in to comment.