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.

Sign up for GitHub

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] [Exceptions] Fix Exception Auto-populate from Alert actions #159908

Merged
merged 53 commits into from
Jun 28, 2023
Merged

[Security Solution] [Exceptions] Fix Exception Auto-populate from Alert actions #159908

Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
53 commits
Select commit Hold shift + click to select a range
e123658
populate the rule exception with the alert highlighted fields
WafaaNasr Jun 5, 2023
d618d0a
Merge branch 'main' of https://github.com/elastic/kibana into 6405-au…
WafaaNasr Jun 5, 2023
702f330
add initial tests for the helper methods
WafaaNasr Jun 7, 2023
8cf53cb
Merge branch 'main' of https://github.com/elastic/kibana into 6405-au…
WafaaNasr Jun 8, 2023
ee9d078
handle fieldValue entry in case of array
WafaaNasr Jun 8, 2023
9156746
Merge branch 'main' of https://github.com/elastic/kibana into 6405-au…
WafaaNasr Jun 9, 2023
115eff7
add tests
WafaaNasr Jun 9, 2023
a88527c
Merge branch 'main' of https://github.com/elastic/kibana into 6405-au…
WafaaNasr Jun 12, 2023
033842c
add cypress test
WafaaNasr Jun 12, 2023
94283e4
Merge branch 'main' of https://github.com/elastic/kibana into 6405-au…
WafaaNasr Jun 12, 2023
a0cca84
fix comment assertion
WafaaNasr Jun 12, 2023
d6da91a
address comments
WafaaNasr Jun 13, 2023
78aedc3
fix stale state of exceptionListItems
WafaaNasr Jun 13, 2023
4daff25
Merge branch 'main' of https://github.com/elastic/kibana into 6405-au…
WafaaNasr Jun 13, 2023
9519b5b
Merge branch 'main' of https://github.com/elastic/kibana into 6405-au…
WafaaNasr Jun 14, 2023
e78b92a
Merge branch 'main' of https://github.com/elastic/kibana into 6405-au…
WafaaNasr Jun 14, 2023
ace134a
revert back changes until see if cypress is trusted
WafaaNasr Jun 14, 2023
58f350a
Merge branch 'main' of https://github.com/elastic/kibana into fix-end…
WafaaNasr Jun 15, 2023
aaf86d3
fix autopopulate
WafaaNasr Jun 15, 2023
54e0588
Merge branch 'main' of https://github.com/elastic/kibana into fix-end…
WafaaNasr Jun 19, 2023
4932445
Merge branch 'main' of https://github.com/elastic/kibana into fix-end…
WafaaNasr Jun 19, 2023
20aa5bb
Merge branch 'main' of https://github.com/elastic/kibana into fix-end…
WafaaNasr Jun 19, 2023
0bc6b56
try to unskip validations
WafaaNasr Jun 19, 2023
87829b1
Merge branch 'main' of https://github.com/elastic/kibana into fix-end…
WafaaNasr Jun 19, 2023
0aa23bb
Merge branch 'main' of https://github.com/elastic/kibana into fix-end…
WafaaNasr Jun 19, 2023
d8c5c32
Merge branch 'main' of https://github.com/elastic/kibana into fix-end…
WafaaNasr Jun 20, 2023
20a7a46
skip the validations
WafaaNasr Jun 20, 2023
7f11193
Merge branch 'main' of https://github.com/elastic/kibana into fix-end…
WafaaNasr Jun 20, 2023
24ae54f
add meta prop to the exceptionItems on update
WafaaNasr Jun 20, 2023
4375245
Merge branch 'main' of https://github.com/elastic/kibana into fix-end…
WafaaNasr Jun 21, 2023
86f8b19
Merge branch 'main' of https://github.com/elastic/kibana into fix-end…
WafaaNasr Jun 21, 2023
9c684cf
Merge branch 'main' of https://github.com/elastic/kibana into fix-end…
WafaaNasr Jun 21, 2023
21c404c
fix reseting the exceptionListItems
WafaaNasr Jun 21, 2023
d0780b0
add check for eventfilters flyout
WafaaNasr Jun 21, 2023
471a6ce
Merge branch 'main' of https://github.com/elastic/kibana into fix-end…
WafaaNasr Jun 22, 2023
2e280fd
add cypress test to validate change exception item multiple times wit…
WafaaNasr Jun 23, 2023
1b28f82
add cypress case for deleting all prefilled fields
WafaaNasr Jun 23, 2023
1382430
Merge branch 'main' of https://github.com/elastic/kibana into fix-end…
WafaaNasr Jun 23, 2023
94b4d88
Merge branch 'main' of https://github.com/elastic/kibana into fix-end…
WafaaNasr Jun 23, 2023
f676400
Merge branch 'main' into fix-endpointexception-autopopulateAlertActions
WafaaNasr Jun 23, 2023
0468b11
Merge branch 'main' of https://github.com/elastic/kibana into fix-end…
WafaaNasr Jun 26, 2023
76d41f9
fix comment open default status
WafaaNasr Jun 26, 2023
ab3b222
Merge branch 'fix-endpointexception-autopopulateAlertActions' of http…
WafaaNasr Jun 26, 2023
823fbd9
Merge branch 'main' of https://github.com/elastic/kibana into fix-end…
WafaaNasr Jun 26, 2023
3e8a70d
add timeout to add_exception_btn
WafaaNasr Jun 26, 2023
37a9f68
Merge branch 'main' of https://github.com/elastic/kibana into fix-end…
WafaaNasr Jun 26, 2023
cbaf0ec
remove unused memo var
WafaaNasr Jun 26, 2023
e5cae86
separate opening alert summary and click add exception
WafaaNasr Jun 26, 2023
7843461
remove extra check from addexception
WafaaNasr Jun 26, 2023
046574c
Merge branch 'main' of https://github.com/elastic/kibana into fix-end…
WafaaNasr Jun 26, 2023
f147868
Merge branch 'main' into fix-endpointexception-autopopulateAlertActions
kibanamachine Jun 27, 2023
6d0c1d4
Merge branch 'main' of https://github.com/elastic/kibana into fix-end…
WafaaNasr Jun 28, 2023
eab421e
update translation to include id in the exception comment
WafaaNasr Jun 28, 2023
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
15 changes: 11 additions & 4 deletions x-pack/plugins/lists/public/exceptions/components/builder/exception_items_renderer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import React, { useCallback, useEffect, useMemo, useReducer } from 'react';
import React, { useCallback, useEffect, useMemo, useReducer, useState } from 'react';
import { EuiFlexGroup, EuiFlexItem, EuiSpacer } from '@elastic/eui';
import styled from 'styled-components';
import { HttpStart } from '@kbn/core/public';
Expand Down Expand Up @@ -34,6 +34,7 @@ import {
} from '@kbn/securitysolution-list-utils';
import { DataViewBase } from '@kbn/es-query';
import type { AutocompleteStart } from '@kbn/unified-search-plugin/public';
import deepEqual from 'fast-deep-equal';

import { AndOrBadge } from '../and_or_badge';

Expand Down Expand Up @@ -131,6 +132,9 @@ export const ExceptionBuilderComponent = ({
disableNested: isNestedDisabled,
disableOr: isOrDisabled,
});
const [currentExceptionListItems, setCurrentExceptionListItems] = useState<
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe using useRef would be more preferable here as it would not trigger another re-render

  const exceptionsRef = useRef<ExceptionsBuilderExceptionItem[]>([]);

  useEffect(() => {
    if (exceptionListItems.length > 0 && !deepEqual(exceptionListItems, exceptionsRef.current)) {
      exceptionsRef.current = exceptionListItems;
      setUpdateExceptions(exceptionListItems);
    }
  }, [exceptionListItems, setUpdateExceptions]);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried it but it didn't work, and I am not sure if it will as the previous reason :(

Upon further investigation, I discovered that modifying the entries of the Exception and subsequently changing another field causes a reset to the initial entries. This behavior occurs because altering the entries (conditions) does not trigger a corresponding change in the exceptionListItems.

ExceptionsBuilderExceptionItem[]
>([]);

const {
addNested,
Expand Down Expand Up @@ -395,11 +399,14 @@ export const ExceptionBuilderComponent = ({
}, [exceptions, handleAddNewExceptionItem]);

useEffect(() => {
if (exceptionListItems.length > 0) {
if (
exceptionListItems.length > 0 &&
!deepEqual(exceptionListItems, currentExceptionListItems)
) {
setCurrentExceptionListItems(exceptionListItems);
setUpdateExceptions(exceptionListItems);
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);
}, [currentExceptionListItems, exceptionListItems, setUpdateExceptions]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Noticed an issue, when added new exception item and then typed name.
Exception item has disappeared. I suspect it's because it got reset when name changed (probably because exceptionListItems has another reference)

Screen.Recording.2023-06-21.at.12.46.07.mov

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, a similar issue was identified by Yara.

Upon further investigation, I discovered that modifying the entries of the Exception and subsequently changing another field causes a reset to the initial entries. This behavior occurs because altering the entries (conditions) does not trigger a corresponding change in the exceptionListItems.


return (
<EuiFlexGroup gutterSize="s" direction="column" data-test-subj="exceptionsBuilderWrapper">
Expand Down
1 change: 1 addition & 0 deletions .../plugins/security_solution/public/management/pages/event_filters/view/components/form.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,7 @@ export const EventFiltersForm: React.FC<ArtifactFormComponentProps & { allowSele
comments: exception?.comments ?? [],
os_types: exception?.os_types ?? [OperatingSystem.WINDOWS],
tags: exception?.tags ?? [],
meta: exception.meta,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this change related to the bug fix or something else you found?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is related to the Event Filters component
The component was throwing the below issue, when we updated useEffect with the deps
image

}
: exception;
const hasValidConditions =
Expand Down