-
Notifications
You must be signed in to change notification settings - Fork 8.1k
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
[EDR Workflows] Automated Actions in more rule types #191874
Changes from 1 commit
5fb4a70
96c92ed
2b342da
0db366d
e9b272c
45c6f9a
a07a932
90990df
5c3701b
3a983f9
ac7010c
62f3045
e3cef1a
28a93d2
f566043
e878208
4e3be88
4bdb90c
9453127
a73360b
8cd270d
f989c37
64dffe1
3e46918
3977af5
eac4b45
cab1f84
618982b
3fc2bcf
708905a
28e37f2
a27a0dd
07a573c
16a534c
2f79a3d
6f0e36e
c1536b9
81fc160
6a39a14
ebf458d
ee68d55
1dc5cf4
412e456
69d9cd0
d787d0a
e2680e9
6215ded
87c0c8b
2b7c7f4
ade46fd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,7 +17,7 @@ import type { | |
import { UseArray } from '@kbn/es-ui-shared-plugin/static/forms/hook_form_lib'; | ||
import type { Type } from '@kbn/securitysolution-io-ts-alerting-types'; | ||
import type { RuleObjectId } from '../../../../../common/api/detection_engine/model/rule_schema'; | ||
import { isQueryRule } from '../../../../../common/detection_engine/utils'; | ||
import { isQueryRule, isEsqlRule, isEqlRule } from '../../../../../common/detection_engine/utils'; | ||
import { ResponseActionsForm } from '../../../rule_response_actions/response_actions_form'; | ||
import type { | ||
RuleStepProps, | ||
|
@@ -101,7 +101,7 @@ const StepRuleActionsComponent: FC<StepRuleActionsProps> = ({ | |
[actionMessageParams, summaryActionMessageParams] | ||
); | ||
const displayResponseActionsOptions = useMemo(() => { | ||
if (isQueryRule(ruleType)) { | ||
if (isQueryRule(ruleType) || isEsqlRule(ruleType) || isEqlRule(ruleType)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The information about which rule type supports response actions lays down here. It might be hard to get why these rule types support response actions while the others don't. As an intermediate simple improvement is to create helper function like The better option is to add response actions to all rule types. Is there something blocking from that approach? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks Maxim, good point! I am afraid we cannot call response actions against all the rules types because in order to do so we need to know what agent the alert is referring to. Do you know if any other alerts (from other rules than Query, E(S)QL contain There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For now I'll adjust to use |
||
return ( | ||
<UseArray path="responseActions" initialNumberOfItems={0}> | ||
{ResponseActionsForm} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,9 @@ | ||
/* | ||
* 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. | ||
* 2.0 and the Server Side Public License, v 1; you may not use this file except | ||
* in compliance with, at your election, the Elastic License 2.0 or the Server | ||
* Side Public License, v 1. | ||
*/ | ||
|
||
import { | ||
|
@@ -12,6 +13,7 @@ import { | |
tryAddingDisabledResponseAction, | ||
validateAvailableCommands, | ||
visitRuleActions, | ||
fillUpNewEsqlRule, | ||
} from '../../tasks/response_actions'; | ||
import { cleanupRule, generateRandomStringName, loadRule } from '../../tasks/api_fixtures'; | ||
import { ResponseActionTypesEnum } from '../../../../../common/api/detection_engine'; | ||
|
@@ -202,6 +204,31 @@ describe( | |
}); | ||
}); | ||
|
||
describe('User should be able to add response action to ESQL rule', () => { | ||
let ruleId: string; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like |
||
const [ruleName, ruleDescription] = generateRandomStringName(2); | ||
|
||
beforeEach(() => { | ||
login(ROLE.soc_manager); | ||
}); | ||
afterEach(() => { | ||
if (ruleId) { | ||
cleanupRule(ruleId); | ||
} | ||
}); | ||
|
||
it('create and save endpoint response action inside of a rule', () => { | ||
fillUpNewEsqlRule(ruleName, ruleDescription); | ||
addEndpointResponseAction(); | ||
focusAndOpenCommandDropdown(); | ||
validateAvailableCommands(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we could keep it procedural by extracting commands below to functions? |
||
cy.getByTestSubj(`command-type-isolate`).click(); | ||
|
||
cy.getByTestSubj('create-enabled-false').click(); | ||
cy.contains(`${ruleName} was created`); | ||
}); | ||
}); | ||
|
||
describe('User should not see endpoint action when no rbac', () => { | ||
const [ruleName, ruleDescription] = generateRandomStringName(2); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,9 @@ | ||
/* | ||
* 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. | ||
* 2.0 and the Server Side Public License, v 1; you may not use this file except | ||
* in compliance with, at your election, the Elastic License 2.0 or the Server | ||
* Side Public License, v 1. | ||
*/ | ||
|
||
import { inputConsoleCommand, submitCommand } from './response_console'; | ||
|
@@ -69,6 +70,28 @@ export const fillUpNewRule = (name = 'Test', description = 'Test') => { | |
cy.getByTestSubj('about-continue').click(); | ||
cy.getByTestSubj('schedule-continue').click(); | ||
}; | ||
|
||
export const fillUpNewEsqlRule = (name = 'Test', description = 'Test') => { | ||
loadPage('app/security/rules/management'); | ||
cy.getByTestSubj('create-new-rule').click(); | ||
cy.getByTestSubj('stepDefineRule').within(() => { | ||
cy.getByTestSubj('esqlRuleType').click(); | ||
cy.getByTestSubj('globalQueryBar').first().click(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we locate it in any other manner than going for the |
||
cy.getByTestSubj('kibanaCodeEditor').type( | ||
'FROM logs-* METADATA _index, _id {backspace}{enter}' | ||
); | ||
}); | ||
cy.getByTestSubj('define-continue').click(); | ||
cy.getByTestSubj('detectionEngineStepAboutRuleName').within(() => { | ||
cy.getByTestSubj('input').type(name); | ||
}); | ||
cy.getByTestSubj('detectionEngineStepAboutRuleDescription').within(() => { | ||
cy.getByTestSubj('input').type(description); | ||
}); | ||
cy.getByTestSubj('about-continue').click(); | ||
cy.getByTestSubj('schedule-continue').click(); | ||
}; | ||
|
||
export const visitRuleActions = (ruleId: string) => { | ||
loadPage(`app/security/rules/id/${ruleId}/edit`); | ||
cy.getByTestSubj('edit-rule-actions-tab').should('exist'); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -189,7 +189,7 @@ describe('Update rule route', () => { | |
}); | ||
const defaultAction = getResponseAction(); | ||
|
||
test('is successful', async () => { | ||
test('is successful for query rule', async () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It could be transformed into |
||
const request = requestMock.create({ | ||
method: 'post', | ||
path: DETECTION_ENGINE_RULES_URL, | ||
|
@@ -203,6 +203,34 @@ describe('Update rule route', () => { | |
expect(response.status).toEqual(200); | ||
}); | ||
|
||
test('is successful for esql rule', async () => { | ||
const request = requestMock.create({ | ||
method: 'post', | ||
path: DETECTION_ENGINE_RULES_URL, | ||
body: { | ||
...getCreateEsqlRulesSchemaMock('rule-2'), | ||
response_actions: [defaultAction], | ||
}, | ||
}); | ||
|
||
const response = await server.inject(request, requestContextMock.convertContext(context)); | ||
expect(response.status).toEqual(200); | ||
}); | ||
|
||
test('is successful for eql rule', async () => { | ||
const request = requestMock.create({ | ||
method: 'post', | ||
path: DETECTION_ENGINE_RULES_URL, | ||
body: { | ||
...getCreateEqlRuleSchemaMock('rule-3'), | ||
response_actions: [defaultAction], | ||
}, | ||
}); | ||
|
||
const response = await server.inject(request, requestContextMock.convertContext(context)); | ||
expect(response.status).toEqual(200); | ||
}); | ||
|
||
test('fails when isolate rbac is set to false', async () => { | ||
(context.securitySolution.getEndpointAuthz as jest.Mock).mockReturnValue(() => ({ | ||
canIsolateHost: jest.fn().mockReturnValue(false), | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: @tomsonpl Adding spaces is unnecessary. It seems your eslint YAML rules are slightly different and it leads to bigger diff.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I will revert 👍