Skip to content

Commit

Permalink
[Security Solution] Implement isCustomized calculation (#186988)
Browse files Browse the repository at this point in the history
**Resolves: #180145
**Resolves: #184364

> [!NOTE]  
> This PR doesn't include `isCustomized` recalculation when bulk editing
rules. This should be addressed separately as it might require first
changing the `RulesClient.bulkEdit` method signature.

## Summary

This PR implements the calculation of `ruleSource.isCustomized` inside
the `DetectionRulesClient`. The recalculation of the `isCustomized`
field is performed on every rule patch and update operation, including
rule upgrades. See the ticket for more information:
#180145 and
`detection_rules_client/mergers/rule_source/calculate_is_customized.ts`
for implementation details.

The `isCustomized` calculation is based on the `calculateRuleFieldsDiff`
method used inside the prebuilt rules domain for calculating changed
fields during rule upgrades. This ensures that the diff calculation
logic is unified and reused to avoid any discrepancies in different
paths of rule management.

The recalculation and saving of the field is done in the following
endpoints:
- **Update Rule** - `PUT /rules`
- **Patch Rule** - `PATCH /rules`
- **Bulk Update Rules** - `PUT /rules/_bulk_update`
- **Bulk Patch Rules** - `PATCH /rules/_bulk_update`
- **Import Rules** - `POST /rules/_import`
- **Perform Rule Upgrade** - `POST /prebuilt_rules/upgrade/_perform`

This PR also partially addresses refactoring mentioned here:
#184364. Namely:
- Splits the rule converters into smaller single-responsibility
functions.
  - Separate methods to convert RuleResponse to AlertingRule and back
  - Separate methods to apply rule patches, updates, or set defaults
  - Separate case converters
- Migrates methods to work with RuleResponse instead of alerting type
wherever possible.
- Adds new methods for fetching rules by id or rule id and deprecates
the `readRules`. Although new methods are not exposed yet in the public
client interface, this is something that needs to be addressed
separately.
  • Loading branch information
xcrzx authored Jul 9, 2024
1 parent 834f8fd commit 045aafc
Show file tree
Hide file tree
Showing 62 changed files with 2,552 additions and 2,040 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,18 @@
*/

import { DEFAULT_INDICATOR_SOURCE_PATH } from '../../../../constants';
import { getListArrayMock } from '../../../../detection_engine/schemas/types/lists.mock';
import type {
EqlRule,
EsqlRule,
MachineLearningRule,
NewTermsRule,
QueryRule,
SavedQueryRule,
SharedResponseProps,
ThreatMatchRule,
ThresholdRule,
} from './rule_schemas.gen';
import { getListArrayMock } from '../../../../detection_engine/schemas/types/lists.mock';

export const ANCHOR_DATE = '2020-02-20T03:57:54.037Z';

Expand Down Expand Up @@ -238,3 +240,27 @@ export const getRulesEqlSchemaMock = (anchorDate: string = ANCHOR_DATE): EqlRule
tiebreaker_field: undefined,
};
};

export const getRulesNewTermsSchemaMock = (anchorDate: string = ANCHOR_DATE): NewTermsRule => {
return {
...getResponseBaseParams(anchorDate),
type: 'new_terms',
query: '*',
language: 'kuery',
new_terms_fields: ['user.name'],
history_window_start: 'now-7d',
};
};

export const getRulesThresholdSchemaMock = (anchorDate: string = ANCHOR_DATE): ThresholdRule => {
return {
...getResponseBaseParams(anchorDate),
type: 'threshold',
language: 'kuery',
query: 'user.name: root or user.name: admin',
threshold: {
field: 'some.field',
value: 4,
},
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { createPrebuiltRuleObjectsClient } from '../../logic/rule_objects/prebui
import { fetchRuleVersionsTriad } from '../../logic/rule_versions/fetch_rule_versions_triad';
import type { PrebuiltRuleAsset } from '../../model/rule_assets/prebuilt_rule_asset';
import { getVersionBuckets } from '../../model/rule_versions/get_version_buckets';
import { convertPrebuiltRuleAssetToRuleResponse } from '../../../rule_management/normalization/rule_converters';
import { convertPrebuiltRuleAssetToRuleResponse } from '../../../rule_management/logic/detection_rules_client/converters/convert_prebuilt_rule_asset_to_rule_response';
import { PREBUILT_RULES_OPERATION_SOCKET_TIMEOUT_MS } from '../../constants';

export const reviewRuleInstallationRoute = (router: SecuritySolutionPluginRouter) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import { createPrebuiltRuleAssetsClient } from '../../logic/rule_assets/prebuilt
import { createPrebuiltRuleObjectsClient } from '../../logic/rule_objects/prebuilt_rule_objects_client';
import { fetchRuleVersionsTriad } from '../../logic/rule_versions/fetch_rule_versions_triad';
import { getVersionBuckets } from '../../model/rule_versions/get_version_buckets';
import { convertPrebuiltRuleAssetToRuleResponse } from '../../../rule_management/normalization/rule_converters';
import { convertPrebuiltRuleAssetToRuleResponse } from '../../../rule_management/logic/detection_rules_client/converters/convert_prebuilt_rule_asset_to_rule_response';
import { PREBUILT_RULES_OPERATION_SOCKET_TIMEOUT_MS } from '../../constants';

export const reviewRuleUpgradeRoute = (router: SecuritySolutionPluginRouter) => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/*
* 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.
*/

export const createPrebuiltRuleAssetsClient = () => {
return {
fetchLatestAssets: jest.fn(),
fetchLatestVersions: jest.fn(),
fetchAssetsByVersion: jest.fn(),
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import type {
import { withSecuritySpan } from '../../../../../utils/with_security_span';
import { findRules } from '../../../rule_management/logic/search/find_rules';
import { getExistingPrepackagedRules } from '../../../rule_management/logic/search/get_existing_prepackaged_rules';
import { internalRuleToAPIResponse } from '../../../rule_management/normalization/rule_converters';
import { internalRuleToAPIResponse } from '../../../rule_management/logic/detection_rules_client/converters/internal_rule_to_api_response';

export interface IPrebuiltRuleObjectsClient {
fetchAllInstalledRules(): Promise<RuleResponse[]>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ export const createAndAssociateDefaultExceptionList = async ({
: existingRuleExceptionLists;

await detectionRulesClient.patchRule({
nextParams: {
rulePatch: {
rule_id: rule.params.ruleId,
...rule.params,
exceptions_list: [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import type { BulkActionsDryRunErrCode } from '../../../../../../../common/const
import type { PromisePoolError } from '../../../../../../utils/promise_pool';
import type { RuleAlertType } from '../../../../rule_schema';
import type { DryRunError } from '../../../logic/bulk_actions/dry_run';
import { internalRuleToAPIResponse } from '../../../normalization/rule_converters';
import { internalRuleToAPIResponse } from '../../../logic/detection_rules_client/converters/internal_rule_to_api_response';

const MAX_ERROR_MESSAGE_LENGTH = 1000;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ export const bulkPatchRulesRoute = (router: SecuritySolutionPluginRouter, logger
});

const patchedRule = await detectionRulesClient.patchRule({
nextParams: payloadRule,
rulePatch: payloadRule,
});

return patchedRule;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ export const patchRuleRoute = (router: SecuritySolutionPluginRouter) => {
});

const patchedRule = await detectionRulesClient.patchRule({
nextParams: params,
rulePatch: params,
});

return response.ok({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,7 @@

export * from './api/register_routes';

// TODO: https://github.com/elastic/kibana/pull/142950
// TODO: Revisit and consider moving to the rule_schema subdomain
export {
commonParamsCamelToSnake,
typeSpecificCamelToSnake,
convertCreateAPIToInternalSchema,
} from './normalization/rule_converters';
export { commonParamsCamelToSnake } from './logic/detection_rules_client/converters/common_params_camel_to_snake';
export { typeSpecificCamelToSnake } from './logic/detection_rules_client/converters/type_specific_camel_to_snake';

export { transformFromAlertThrottle, transformToNotifyWhen } from './normalization/rule_actions';
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import type { SanitizedRule } from '@kbn/alerting-plugin/common';
import { SERVER_APP_ID } from '../../../../../../common/constants';
import type { InternalRuleCreate, RuleParams } from '../../../rule_schema';
import { transformToActionFrequency } from '../../normalization/rule_actions';
import { convertImmutableToRuleSource } from '../../normalization/rule_converters';

const DUPLICATE_TITLE = i18n.translate(
'xpack.securitySolution.detectionEngine.rules.cloneRule.duplicateTitle',
Expand Down Expand Up @@ -47,7 +46,9 @@ export const duplicateRule = async ({ rule }: DuplicateRuleParams): Promise<Inte
params: {
...rule.params,
immutable,
ruleSource: convertImmutableToRuleSource(immutable),
ruleSource: {
type: 'internal',
},
ruleId,
relatedIntegrations,
requiredFields,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*
* 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 { getBaseRuleParams } from '../../../../rule_schema/mocks';
import { commonParamsCamelToSnake } from './common_params_camel_to_snake';

describe('commonParamsCamelToSnake', () => {
test('should convert rule_source params to snake case', () => {
const transformedParams = commonParamsCamelToSnake({
...getBaseRuleParams(),
ruleSource: {
type: 'external',
isCustomized: false,
},
});
expect(transformedParams).toEqual(
expect.objectContaining({
rule_source: {
type: 'external',
is_customized: false,
},
})
);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/*
* 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 { convertObjectKeysToSnakeCase } from '../../../../../../utils/object_case_converters';
import type { BaseRuleParams } from '../../../../rule_schema';
import { migrateLegacyInvestigationFields } from '../../../utils/utils';

export const commonParamsCamelToSnake = (params: BaseRuleParams) => {
return {
description: params.description,
risk_score: params.riskScore,
severity: params.severity,
building_block_type: params.buildingBlockType,
namespace: params.namespace,
note: params.note,
license: params.license,
output_index: params.outputIndex,
timeline_id: params.timelineId,
timeline_title: params.timelineTitle,
meta: params.meta,
rule_name_override: params.ruleNameOverride,
timestamp_override: params.timestampOverride,
timestamp_override_fallback_disabled: params.timestampOverrideFallbackDisabled,
investigation_fields: migrateLegacyInvestigationFields(params.investigationFields),
author: params.author,
false_positives: params.falsePositives,
from: params.from,
rule_id: params.ruleId,
max_signals: params.maxSignals,
risk_score_mapping: params.riskScoreMapping,
severity_mapping: params.severityMapping,
threat: params.threat,
to: params.to,
references: params.references,
version: params.version,
exceptions_list: params.exceptionsList,
immutable: params.immutable,
rule_source: convertObjectKeysToSnakeCase(params.ruleSource),
related_integrations: params.relatedIntegrations ?? [],
required_fields: params.requiredFields ?? [],
setup: params.setup ?? '',
};
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* 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 { SanitizedRule } from '@kbn/alerting-plugin/common';
import { stringifyZodError } from '@kbn/zod-helpers';
import { RuleResponse } from '../../../../../../../common/api/detection_engine/model/rule_schema';
import type { RuleParams } from '../../../../rule_schema';
import { internalRuleToAPIResponse } from './internal_rule_to_api_response';
import { RuleResponseValidationError } from '../utils';

export function convertAlertingRuleToRuleResponse(rule: SanitizedRule<RuleParams>): RuleResponse {
const parseResult = RuleResponse.safeParse(internalRuleToAPIResponse(rule));

if (!parseResult.success) {
throw new RuleResponseValidationError({
message: stringifyZodError(parseResult.error),
ruleId: rule.params.ruleId,
});
}

return parseResult.data;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/*
* 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 { v4 as uuidv4 } from 'uuid';
import { RuleResponse } from '../../../../../../../common/api/detection_engine/model/rule_schema';
import { addEcsToRequiredFields } from '../../../utils/utils';
import type { PrebuiltRuleAsset } from '../../../../prebuilt_rules';
import { RULE_DEFAULTS } from '../mergers/apply_rule_defaults';

export const convertPrebuiltRuleAssetToRuleResponse = (
prebuiltRuleAsset: PrebuiltRuleAsset
): RuleResponse => {
const immutable = true;

const ruleResponseSpecificFields = {
id: uuidv4(),
updated_at: new Date().toISOString(),
updated_by: '',
created_at: new Date().toISOString(),
created_by: '',
immutable,
rule_source: {
type: 'external',
is_customized: false,
},
revision: 1,
};

return RuleResponse.parse({
...RULE_DEFAULTS,
...prebuiltRuleAsset,
required_fields: addEcsToRequiredFields(prebuiltRuleAsset.required_fields),
...ruleResponseSpecificFields,
});
};
Loading

0 comments on commit 045aafc

Please sign in to comment.