Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Security Solution] [Detections] Adds support for system actions (and cases action) to detection rules #183937

Merged
merged 74 commits into from
Jul 26, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
74 commits
Select commit Hold shift + click to select a range
2cfed78
working system actions for cases for create rule route security
dhurley14 May 21, 2024
a2bb5ba
merge with main
dhurley14 Jun 17, 2024
7082de6
fix merge conflict, re-delete file
dhurley14 Jun 17, 2024
5e137b3
create rule with system actions cases works after the rules client re…
dhurley14 Jun 17, 2024
3600f32
isntantiate actions client within detection rules client
dhurley14 Jun 19, 2024
7140b48
Merge branch 'main' into system-actions-security
dhurley14 Jun 19, 2024
33cebd7
Merge branch 'main' into system-actions-security
dhurley14 Jun 19, 2024
2a91ec0
Merge branch 'main' into system-actions-security
dhurley14 Jun 19, 2024
4d73f76
working rule export
dhurley14 Jun 25, 2024
2ef4df5
import rule with system action working
dhurley14 Jun 25, 2024
62b6ec8
adds support for update with system actions
dhurley14 Jun 25, 2024
67e406b
adds support for system actions in patch, cleanup types
dhurley14 Jun 26, 2024
5e61574
small fix with partition logic
dhurley14 Jun 26, 2024
7c72634
small cleanup and notes for cleanup / investigation tomorrow
dhurley14 Jun 27, 2024
7f7a4ba
remove unused import
dhurley14 Jun 27, 2024
098194d
small updates and cleanup
dhurley14 Jun 27, 2024
fdce619
fixes duplicate rule unit tests
dhurley14 Jun 27, 2024
8d75153
fixes createCustomeRule unit test
dhurley14 Jun 27, 2024
0599aa2
fixes unit test mocks for detection rules client
dhurley14 Jun 27, 2024
3c8a799
fixes types on UI
dhurley14 Jun 27, 2024
1d02e49
partition on action.id, not action_type_id
dhurley14 Jun 28, 2024
0b121bd
merge with main
dhurley14 Jun 28, 2024
b697826
fixes duplicate rule with system actions
dhurley14 Jun 28, 2024
43a779f
fixes duplicate rule functionality
dhurley14 Jun 28, 2024
ed4b400
updates tests, render cases title for system action on rule details page
dhurley14 Jun 28, 2024
033fe41
removes console statements
dhurley14 Jul 1, 2024
b3f5498
remove comment
dhurley14 Jul 1, 2024
caa8702
more cleanup
dhurley14 Jul 1, 2024
73202f0
Merge remote-tracking branch 'upstream/main' into system-actions-secu…
dhurley14 Jul 1, 2024
6a6f397
Merge branch 'main' into system-actions-security
dhurley14 Jul 2, 2024
6e16ab0
Merge remote-tracking branch 'upstream/main' into system-actions-secu…
dhurley14 Jul 2, 2024
1b35aeb
fixes issue where if actions were not present in the update (i.e. pat…
dhurley14 Jul 2, 2024
9eeb2e6
remove test cases looking for 'group' property, updates actionsClient…
dhurley14 Jul 2, 2024
56c8b03
Merge branch 'main' into system-actions-security
dhurley14 Jul 2, 2024
06efd06
Merge branch 'main' into system-actions-security
dhurley14 Jul 2, 2024
c250336
remove hardcoded string value for siem feature id in cases action
dhurley14 Jul 3, 2024
dfb239e
updates rules client to use named parameters
dhurley14 Jul 3, 2024
1fbafc3
named params in context factor, adds test for migrate legacy actions,…
dhurley14 Jul 3, 2024
0e42109
fix mistyped parameter for rule create / edit form
dhurley14 Jul 3, 2024
7254095
adds tests for validating changes made to convertCreateAPIToInternalS…
dhurley14 Jul 3, 2024
77e351b
fixes create_custom_rule test mock
dhurley14 Jul 3, 2024
0e4f045
updates detection rules client patch and update() unit tests
dhurley14 Jul 3, 2024
683504b
make system actions optional on the internal create and update types
dhurley14 Jul 3, 2024
57a4a77
adds wrappers around partition and array.map -> transformRuleToAlertA…
dhurley14 Jul 3, 2024
e36a295
adds FTR test which creates a rule and case system action and checks …
dhurley14 Jul 3, 2024
ed6f8c7
forgot to add the actual test case
dhurley14 Jul 5, 2024
69ba82b
fixes rule edit bulk actions -> set and add actions
dhurley14 Jul 5, 2024
475915e
fixes tests and type checker
dhurley14 Jul 5, 2024
3b8e874
Merge remote-tracking branch 'upstream/main' into system-actions-secu…
dhurley14 Jul 5, 2024
bf2d39b
undo ftr test
dhurley14 Jul 9, 2024
9e8eda5
merge with main
dhurley14 Jul 15, 2024
9e67428
Merge remote-tracking branch 'upstream/main' into system-actions-secu…
dhurley14 Jul 15, 2024
5827e1e
change boolean name and import, move ftr test to actions
dhurley14 Jul 16, 2024
ee5ae09
Merge remote-tracking branch 'upstream/main' into system-actions-secu…
dhurley14 Jul 16, 2024
cc33a01
adds condition in cypress test to ensure complete tier cases action i…
dhurley14 Jul 16, 2024
dbf4db2
fix unit tests
dhurley14 Jul 17, 2024
991e10a
fixes ftr test for adding case system action to rule
dhurley14 Jul 17, 2024
374309c
undo before and after fn changes from testing
dhurley14 Jul 17, 2024
da2217f
Merge remote-tracking branch 'upstream/main' into system-actions-secu…
dhurley14 Jul 18, 2024
fee7dbb
remove unused function
dhurley14 Jul 18, 2024
bd7c725
adds comment to logic in notification action component
dhurley14 Jul 18, 2024
604a755
[CI] Auto-commit changed files from 'yarn openapi:bundle'
kibanamachine Jul 18, 2024
bc9efa2
merge with main
dhurley14 Jul 19, 2024
09716a2
Merge branch 'system-actions-security' of github.com:dhurley14/kibana…
dhurley14 Jul 19, 2024
f2e919d
merge with main
dhurley14 Jul 24, 2024
8de96fa
Update x-pack/plugins/security_solution/public/detection_engine/rule_…
dhurley14 Jul 24, 2024
19748e3
adds check to ensure cases action is visible in serverless complete tier
dhurley14 Jul 24, 2024
851e19a
move function parameters so actionsClient is last
dhurley14 Jul 24, 2024
3848196
adds unit tests for changes to bulkEditActionToRulesClientOperation
dhurley14 Jul 24, 2024
8d517b4
fixes prettier problem from code suggestion commit in github
dhurley14 Jul 24, 2024
739a6a3
Merge branch 'main' into system-actions-security
dhurley14 Jul 25, 2024
3903f00
adds frequency message for cases system action
dhurley14 Jul 25, 2024
872477e
Merge branch 'system-actions-security' of github.com:dhurley14/kibana…
dhurley14 Jul 26, 2024
256b6ea
remove comment, delete whitespace
dhurley14 Jul 26, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,10 @@ export const createRuleRoute = (router: SecuritySolutionPluginRouter): void => {
'lists',
'actions',
]);

const actionsClient = ctx.actions.getActionsClient();
const rulesClient = ctx.alerting.getRulesClient();
const detectionRulesClient = ctx.securitySolution.getDetectionRulesClient();
banderror marked this conversation as resolved.
Show resolved Hide resolved
const detectionRulesClient = ctx.securitySolution.getDetectionRulesClient(actionsClient);
const exceptionsClient = ctx.lists?.getExceptionListClient();
const actionsClient = ctx.actions.getActionsClient();

if (request.body.rule_id != null) {
const rule = await readRules({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
*/

import type { RulesClient } from '@kbn/alerting-plugin/server';
import type { ActionsClient } from '@kbn/actions-plugin/server';

import type { MlAuthz } from '../../../../machine_learning/authz';

import type { RuleAlertType } from '../../../rule_schema';
Expand All @@ -31,12 +33,13 @@ import { importRule } from './methods/import_rule';
import { withSecuritySpan } from '../../../../../utils/with_security_span';

export const createDetectionRulesClient = (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is maybe nitpicking, but given that we're dealing with the DetectionRulesClient here, I strongly feel that the actionsClient should not be the first parameter of createDetectionRulesClient. Can we reordering it so?

export const createDetectionRulesClient = (
  rulesClient: RulesClient,
  actionsClient: ActionsClient,
  mlAuthz: MlAuthz
): IDetectionRulesClient => ({
  ...

Same goes with the arguments of all the methods used here. Instead of:

createCustomRule(actionsClient, rulesClient, args, mlAuthz);

This should be:

createCustomRule(rulesClient, args, actionsClient, mlAuthz);

The idea being, the most important dependency for interacting with rules is the rulesClient. Then, we need the arguments to create/modify/delete them. And only after that, additional dependencies like actionsClient and mlAutzh.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dhurley14 Just a heads up that I've refactored this builder function to accept arguments as an object instead of positional arguments in my PR:

interface DetectionRulesClientParams {
  rulesClient: RulesClient;
  savedObjectsClient: SavedObjectsClientContract;
  mlAuthz: MlAuthz;
}

export const createDetectionRulesClient = ({
  rulesClient,
  mlAuthz,
  savedObjectsClient,
}: DetectionRulesClientParams): IDetectionRulesClient => {

I'd propose aligning here, so we don't end up with a lot of conflicts.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be fixed now.

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
import { partition } from 'lodash';

import type { ActionsClient } from '@kbn/actions-plugin/server';
import type { RulesClient } from '@kbn/alerting-plugin/server';
import type { CreateCustomRuleArgs } from '../detection_rules_client_interface';
import type { MlAuthz } from '../../../../../machine_learning/authz';
Expand All @@ -14,14 +16,22 @@ import { convertCreateAPIToInternalSchema } from '../../../normalization/rule_co
import { validateMlAuth } from '../utils';

export const createCustomRule = async (
actionsClient?: ActionsClient,
rulesClient: RulesClient,
args: CreateCustomRuleArgs,
mlAuthz: MlAuthz
): Promise<RuleAlertType> => {
const { params } = args;
await validateMlAuth(mlAuthz, params.type);

const internalRule = convertCreateAPIToInternalSchema(params, { immutable: false });
const params = args.params;
const [oldActions, systemActions] = partition(params.actions, (action) =>
actionsClient.isSystemAction(action.action_type_id)
);
console.log('SYSTEM ACTIONS', systemActions);
console.log('OLD ACTIONS', oldActions);
const internalRule = convertCreateAPIToInternalSchema({
...params,
actions: oldActions,
systemActions,
});
const rule = await rulesClient.create<RuleParams>({
data: internalRule,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ export class RequestContextFactory implements IRequestContextFactory {

getAuditLogger,

getDetectionRulesClient: memoize(() => {
getDetectionRulesClient: memoize((actionsClient) => {
const mlAuthz = buildMlAuthz({
license: licensing.license,
ml: plugins.ml,
Expand All @@ -123,6 +123,7 @@ export class RequestContextFactory implements IRequestContextFactory {
});

return createDetectionRulesClient(
actionsClient,
startPlugins.alerting.getRulesClientWithRequest(request),
mlAuthz
);
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugins/security_solution/server/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import type {
IRouter,
KibanaRequest,
} from '@kbn/core/server';
import type { ActionsApiRequestHandlerContext } from '@kbn/actions-plugin/server';
import type { ActionsApiRequestHandlerContext, ActionsClient } from '@kbn/actions-plugin/server';
import type { AlertingApiRequestHandlerContext } from '@kbn/alerting-plugin/server';
import type { FleetRequestHandlerContext } from '@kbn/fleet-plugin/server';
import type { LicensingApiRequestHandlerContext } from '@kbn/licensing-plugin/server';
Expand Down Expand Up @@ -45,7 +45,7 @@ export interface SecuritySolutionApiRequestHandlerContext {
getAppClient: () => AppClient;
getSpaceId: () => string;
getRuleDataService: () => IRuleDataService;
getDetectionRulesClient: () => IDetectionRulesClient;
getDetectionRulesClient: (actionsClient?: ActionsClient) => IDetectionRulesClient;
dhurley14 marked this conversation as resolved.
Show resolved Hide resolved
getDetectionEngineHealthClient: () => IDetectionEngineHealthClient;
getRuleExecutionLog: () => IRuleExecutionLogForRoutes;
getRacClient: (req: KibanaRequest) => Promise<AlertsClient>;
Expand Down