Skip to content

Commit

Permalink
[Security Solution][Exceptions] - Improve UX for missing exception li…
Browse files Browse the repository at this point in the history 
…st associated with rule (#75898)

## Summary

**Current behavior:**
  - **Scenario 1:** User is in the exceptions viewer flow, they select to edit an exception item, but the list the item is associated with has since been deleted (let's say by another user) - a user is able to open modal to edit exception item and on save, an error toaster shows but no information is given to the user to indicate the issue.
  - **Scenario 2:** User exports rules from space 'X' and imports into space 'Y'. The exception lists associated with their newly imported rules do not exist in space 'Y' - a user goes to add an exception item and gets a modal with an error, unable to add any exceptions. 
  - **Workaround:** current workaround exists only via API - user would need to remove the exception list from their rule via API

**New behavior:**
  - **Scenario 1:** User is still able to oped edit modal, but on save they see an error explaining that the associated exception list does not exist and prompts them to remove the exception list --> now they're able to add exceptions to their rule
  - **Scenario 2:** User navigates to exceptions after importing their rule, tries to add  exception, modal pops up with error informing them that they need to remove association to missing exception list, button prompts them to do so --> now can continue adding exceptions to rule
  • Loading branch information
@yctercero
yctercero authored Aug 26, 2020
1 parent 4e1b1b5 commit b9c8201
Show file tree
Hide file tree
Showing 13 changed files with 706 additions and 54 deletions.
90 changes: 64 additions & 26 deletions ...ugins/security_solution/public/common/components/exceptions/add_exception_modal/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import {
EuiCheckbox,
EuiSpacer,
EuiFormRow,
EuiCallOut,
EuiText,
} from '@elastic/eui';
import { Status } from '../../../../../common/detection_engine/schemas/common/schemas';
Expand All @@ -28,13 +27,15 @@ import {
ExceptionListType,
} from '../../../../../public/lists_plugin_deps';
import * as i18n from './translations';
import * as sharedI18n from '../translations';
import { TimelineNonEcsData, Ecs } from '../../../../graphql/types';
import { useAppToasts } from '../../../hooks/use_app_toasts';
import { useKibana } from '../../../lib/kibana';
import { ExceptionBuilderComponent } from '../builder';
import { Loader } from '../../loader';
import { useAddOrUpdateException } from '../use_add_exception';
import { useSignalIndex } from '../../../../detections/containers/detection_engine/alerts/use_signal_index';
import { useRuleAsync } from '../../../../detections/containers/detection_engine/rules/use_rule_async';
import { useFetchOrCreateRuleExceptionList } from '../use_fetch_or_create_rule_exception_list';
import { AddExceptionComments } from '../add_exception_comments';
import {
Expand All @@ -46,6 +47,7 @@ import {
entryHasNonEcsType,
getMappedNonEcsValue,
} from '../helpers';
import { ErrorInfo, ErrorCallout } from '../error_callout';
import { useFetchIndexPatterns } from '../../../../detections/containers/detection_engine/rules';

export interface AddExceptionModalBaseProps {
Expand Down Expand Up @@ -107,13 +109,14 @@ export const AddExceptionModal = memo(function AddExceptionModal({
}: AddExceptionModalProps) {
const { http } = useKibana().services;
const [comment, setComment] = useState('');
const { rule: maybeRule } = useRuleAsync(ruleId);
const [shouldCloseAlert, setShouldCloseAlert] = useState(false);
const [shouldBulkCloseAlert, setShouldBulkCloseAlert] = useState(false);
const [shouldDisableBulkClose, setShouldDisableBulkClose] = useState(false);
const [exceptionItemsToAdd, setExceptionItemsToAdd] = useState<
Array<ExceptionListItemSchema | CreateExceptionListItemSchema>
>([]);
const [fetchOrCreateListError, setFetchOrCreateListError] = useState(false);
const [fetchOrCreateListError, setFetchOrCreateListError] = useState<ErrorInfo | null>(null);
const { addError, addSuccess } = useAppToasts();
const { loading: isSignalIndexLoading, signalIndexName } = useSignalIndex();
const [
Expand Down Expand Up @@ -164,17 +167,41 @@ export const AddExceptionModal = memo(function AddExceptionModal({
},
[onRuleChange]
);
const onFetchOrCreateExceptionListError = useCallback(
(error: Error) => {
setFetchOrCreateListError(true);

const handleDissasociationSuccess = useCallback(
(id: string): void => {
handleRuleChange(true);
addSuccess(sharedI18n.DISSASOCIATE_LIST_SUCCESS(id));
onCancel();
},
[handleRuleChange, addSuccess, onCancel]
);

const handleDissasociationError = useCallback(
(error: Error): void => {
addError(error, { title: sharedI18n.DISSASOCIATE_EXCEPTION_LIST_ERROR });
onCancel();
},
[addError, onCancel]
);

const handleFetchOrCreateExceptionListError = useCallback(
(error: Error, statusCode: number | null, message: string | null) => {
setFetchOrCreateListError({
reason: error.message,
code: statusCode,
details: message,
listListId: null,
});
},
[setFetchOrCreateListError]
);

const [isLoadingExceptionList, ruleExceptionList] = useFetchOrCreateRuleExceptionList({
http,
ruleId,
exceptionListType,
onError: onFetchOrCreateExceptionListError,
onError: handleFetchOrCreateExceptionListError,
onSuccess: handleRuleChange,
});

Expand Down Expand Up @@ -279,7 +306,9 @@ export const AddExceptionModal = memo(function AddExceptionModal({
]);

const isSubmitButtonDisabled = useMemo(
() => fetchOrCreateListError || exceptionItemsToAdd.every((item) => item.entries.length === 0),
() =>
fetchOrCreateListError != null ||
exceptionItemsToAdd.every((item) => item.entries.length === 0),
[fetchOrCreateListError, exceptionItemsToAdd]
);

Expand All @@ -295,19 +324,27 @@ export const AddExceptionModal = memo(function AddExceptionModal({
</ModalHeaderSubtitle>
</ModalHeader>

{fetchOrCreateListError === true && (
<EuiCallOut title={i18n.ADD_EXCEPTION_FETCH_ERROR_TITLE} color="danger" iconType="alert">
<p>{i18n.ADD_EXCEPTION_FETCH_ERROR}</p>
</EuiCallOut>
{fetchOrCreateListError != null && (
<EuiModalFooter>
<ErrorCallout
http={http}
errorInfo={fetchOrCreateListError}
rule={maybeRule}
onCancel={onCancel}
onSuccess={handleDissasociationSuccess}
onError={handleDissasociationError}
data-test-subj="addExceptionModalErrorCallout"
/>
</EuiModalFooter>
)}
{fetchOrCreateListError === false &&
{fetchOrCreateListError == null &&
(isLoadingExceptionList ||
isIndexPatternLoading ||
isSignalIndexLoading ||
isSignalIndexPatternLoading) && (
<Loader data-test-subj="loadingAddExceptionModal" size="xl" />
)}
{fetchOrCreateListError === false &&
{fetchOrCreateListError == null &&
!isSignalIndexLoading &&
!isSignalIndexPatternLoading &&
!isLoadingExceptionList &&
Expand Down Expand Up @@ -377,20 +414,21 @@ export const AddExceptionModal = memo(function AddExceptionModal({
</ModalBodySection>
</>
)}
{fetchOrCreateListError == null && (
<EuiModalFooter>
<EuiButtonEmpty onClick={onCancel}>{i18n.CANCEL}</EuiButtonEmpty>

<EuiModalFooter>
<EuiButtonEmpty onClick={onCancel}>{i18n.CANCEL}</EuiButtonEmpty>

<EuiButton
data-test-subj="add-exception-confirm-button"
onClick={onAddExceptionConfirm}
isLoading={addExceptionIsLoading}
isDisabled={isSubmitButtonDisabled}
fill
>
{i18n.ADD_EXCEPTION}
</EuiButton>
</EuiModalFooter>
<EuiButton
data-test-subj="add-exception-confirm-button"
onClick={onAddExceptionConfirm}
isLoading={addExceptionIsLoading}
isDisabled={isSubmitButtonDisabled}
fill
>
{i18n.ADD_EXCEPTION}
</EuiButton>
</EuiModalFooter>
)}
</Modal>
</EuiOverlayMask>
);
Expand Down
5 changes: 5 additions & 0 deletions ...security_solution/public/common/components/exceptions/edit_exception_modal/index.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ describe('When the edit exception modal is opened', () => {
<ThemeProvider theme={() => ({ eui: euiLightVars, darkMode: false })}>
<EditExceptionModal
ruleIndices={[]}
ruleId="123"
ruleName={ruleName}
exceptionListType={'endpoint'}
onCancel={jest.fn()}
Expand Down Expand Up @@ -105,6 +106,7 @@ describe('When the edit exception modal is opened', () => {
<ThemeProvider theme={() => ({ eui: euiLightVars, darkMode: false })}>
<EditExceptionModal
ruleIndices={['filebeat-*']}
ruleId="123"
ruleName={ruleName}
exceptionListType={'endpoint'}
onCancel={jest.fn()}
Expand Down Expand Up @@ -147,6 +149,7 @@ describe('When the edit exception modal is opened', () => {
<ThemeProvider theme={() => ({ eui: euiLightVars, darkMode: false })}>
<EditExceptionModal
ruleIndices={['filebeat-*']}
ruleId="123"
ruleName={ruleName}
exceptionListType={'endpoint'}
onCancel={jest.fn()}
Expand Down Expand Up @@ -190,6 +193,7 @@ describe('When the edit exception modal is opened', () => {
<ThemeProvider theme={() => ({ eui: euiLightVars, darkMode: false })}>
<EditExceptionModal
ruleIndices={['filebeat-*']}
ruleId="123"
ruleName={ruleName}
exceptionListType={'detection'}
onCancel={jest.fn()}
Expand Down Expand Up @@ -229,6 +233,7 @@ describe('When the edit exception modal is opened', () => {
<ThemeProvider theme={() => ({ eui: euiLightVars, darkMode: false })}>
<EditExceptionModal
ruleIndices={['filebeat-*']}
ruleId="123"
ruleName={ruleName}
exceptionListType={'detection'}
onCancel={jest.fn()}
Expand Down
91 changes: 68 additions & 23 deletions ...gins/security_solution/public/common/components/exceptions/edit_exception_modal/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,14 @@ import {
} from '@elastic/eui';
import { useFetchIndexPatterns } from '../../../../detections/containers/detection_engine/rules';
import { useSignalIndex } from '../../../../detections/containers/detection_engine/alerts/use_signal_index';
import { useRuleAsync } from '../../../../detections/containers/detection_engine/rules/use_rule_async';
import {
ExceptionListItemSchema,
CreateExceptionListItemSchema,
ExceptionListType,
} from '../../../../../public/lists_plugin_deps';
import * as i18n from './translations';
import * as sharedI18n from '../translations';
import { useKibana } from '../../../lib/kibana';
import { useAppToasts } from '../../../hooks/use_app_toasts';
import { ExceptionBuilderComponent } from '../builder';
Expand All @@ -43,14 +45,17 @@ import {
lowercaseHashValues,
} from '../helpers';
import { Loader } from '../../loader';
import { ErrorInfo, ErrorCallout } from '../error_callout';

interface EditExceptionModalProps {
ruleName: string;
ruleId: string;
ruleIndices: string[];
exceptionItem: ExceptionListItemSchema;
exceptionListType: ExceptionListType;
onCancel: () => void;
onConfirm: () => void;
onRuleChange?: () => void;
}

const Modal = styled(EuiModal)`
Expand Down Expand Up @@ -83,14 +88,18 @@ const ModalBodySection = styled.section`

export const EditExceptionModal = memo(function EditExceptionModal({
ruleName,
ruleId,
ruleIndices,
exceptionItem,
exceptionListType,
onCancel,
onConfirm,
onRuleChange,
}: EditExceptionModalProps) {
const { http } = useKibana().services;
const [comment, setComment] = useState('');
const { rule: maybeRule } = useRuleAsync(ruleId);
const [updateError, setUpdateError] = useState<ErrorInfo | null>(null);
const [hasVersionConflict, setHasVersionConflict] = useState(false);
const [shouldBulkCloseAlert, setShouldBulkCloseAlert] = useState(false);
const [shouldDisableBulkClose, setShouldDisableBulkClose] = useState(false);
Expand All @@ -108,27 +117,53 @@ export const EditExceptionModal = memo(function EditExceptionModal({
'rules'
);

const onError = useCallback(
(error) => {
const handleExceptionUpdateError = useCallback(
(error: Error, statusCode: number | null, message: string | null) => {
if (error.message.includes('Conflict')) {
setHasVersionConflict(true);
} else {
addError(error, { title: i18n.EDIT_EXCEPTION_ERROR });
onCancel();
setUpdateError({
reason: error.message,
code: statusCode,
details: message,
listListId: exceptionItem.list_id,
});
}
},
[setUpdateError, setHasVersionConflict, exceptionItem.list_id]
);

const handleDissasociationSuccess = useCallback(
(id: string): void => {
addSuccess(sharedI18n.DISSASOCIATE_LIST_SUCCESS(id));

if (onRuleChange) {
onRuleChange();
}

onCancel();
},
[addSuccess, onCancel, onRuleChange]
);

const handleDissasociationError = useCallback(
(error: Error): void => {
addError(error, { title: sharedI18n.DISSASOCIATE_EXCEPTION_LIST_ERROR });
onCancel();
},
[addError, onCancel]
);
const onSuccess = useCallback(() => {

const handleExceptionUpdateSuccess = useCallback((): void => {
addSuccess(i18n.EDIT_EXCEPTION_SUCCESS);
onConfirm();
}, [addSuccess, onConfirm]);

const [{ isLoading: addExceptionIsLoading }, addOrUpdateExceptionItems] = useAddOrUpdateException(
{
http,
onSuccess,
onError,
onSuccess: handleExceptionUpdateSuccess,
onError: handleExceptionUpdateError,
}
);

Expand Down Expand Up @@ -222,11 +257,9 @@ export const EditExceptionModal = memo(function EditExceptionModal({
{ruleName}
</ModalHeaderSubtitle>
</ModalHeader>

{(addExceptionIsLoading || isIndexPatternLoading || isSignalIndexLoading) && (
<Loader data-test-subj="loadingEditExceptionModal" size="xl" />
)}

{!isSignalIndexLoading && !addExceptionIsLoading && !isIndexPatternLoading && (
<>
<ModalBodySection className="builder-section">
Expand Down Expand Up @@ -280,28 +313,40 @@ export const EditExceptionModal = memo(function EditExceptionModal({
</ModalBodySection>
</>
)}

{updateError != null && (
<ModalBodySection>
<ErrorCallout
http={http}
errorInfo={updateError}
rule={maybeRule}
onCancel={onCancel}
onSuccess={handleDissasociationSuccess}
onError={handleDissasociationError}
/>
</ModalBodySection>
)}
{hasVersionConflict && (
<ModalBodySection>
<EuiCallOut title={i18n.VERSION_CONFLICT_ERROR_TITLE} color="danger" iconType="alert">
<p>{i18n.VERSION_CONFLICT_ERROR_DESCRIPTION}</p>
</EuiCallOut>
</ModalBodySection>
)}
{updateError == null && (
<EuiModalFooter>
<EuiButtonEmpty onClick={onCancel}>{i18n.CANCEL}</EuiButtonEmpty>

<EuiModalFooter>
<EuiButtonEmpty onClick={onCancel}>{i18n.CANCEL}</EuiButtonEmpty>

<EuiButton
data-test-subj="edit-exception-confirm-button"
onClick={onEditExceptionConfirm}
isLoading={addExceptionIsLoading}
isDisabled={isSubmitButtonDisabled}
fill
>
{i18n.EDIT_EXCEPTION_SAVE_BUTTON}
</EuiButton>
</EuiModalFooter>
<EuiButton
data-test-subj="edit-exception-confirm-button"
onClick={onEditExceptionConfirm}
isLoading={addExceptionIsLoading}
isDisabled={isSubmitButtonDisabled}
fill
>
{i18n.EDIT_EXCEPTION_SAVE_BUTTON}
</EuiButton>
</EuiModalFooter>
)}
</Modal>
</EuiOverlayMask>
);
Expand Down
Loading

0 comments on commit b9c8201

Please sign in to comment.