Skip to content

Commit

Permalink
[Security Solution][Artifacts][Trusted Apps] Wildcard warning with IS…
Browse files Browse the repository at this point in the history
… operator for trusted apps creation/editing (elastic#175356)

## Summary

- [x] Adds updated warning messaging for trusted apps entries that use
wildcards `*?` with the "IS" operator
- [x] Three different warnings: callout, individual entry item warnings
and a final confirmation modal when the user tries to add a trusted app
with ineffective IS / wildcard combination etnry.
- [x] Unit tests

# Screenshots
<img width="829" alt="image"
src="https://github.com/elastic/kibana/assets/56409205/c7beec62-a249-4535-ac0b-34f9be57f542">
<img width="1649" alt="image"
src="https://github.com/elastic/kibana/assets/56409205/22f38f1b-7e6b-4b69-8d03-4d74d8674fa6">

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
2 people authored and CoenWarmer committed Feb 15, 2024
1 parent 9349f1b commit 7c5a668
Show file tree
Hide file tree
Showing 12 changed files with 330 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,4 @@ export * from './src/types';
export * from './src/list_header';
export * from './src/header_menu';
export * from './src/generate_linked_rules_menu_item';
export * from './src/wildcard_with_wrong_operator_callout';
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/*
* 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 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 React from 'react';
import { FormattedMessage } from '@kbn/i18n-react';
import { i18n } from '@kbn/i18n';

import { EuiCallOut } from '@elastic/eui';

export const WildCardWithWrongOperatorCallout = () => {
return (
<EuiCallOut
title={i18n.translate('exceptionList-components.wildcardWithWrongOperatorCallout.title', {
defaultMessage: 'Please review your entries',
})}
iconType="warning"
color="warning"
size="s"
data-test-subj="wildcardWithWrongOperatorCallout"
>
<p>
<FormattedMessage
id="exceptionList-components.wildcardWithWrongOperatorCallout.body"
defaultMessage="Using a '*' or a '?' in the value with the 'IS' operator can make the entry ineffective. {operator} to '{matches}' to ensure wildcards run properly."
values={{
operator: (
<strong>
{i18n.translate(
'exceptionList-components.wildcardWithWrongOperatorCallout.changeTheOperator',
{ defaultMessage: 'Change the operator' }
)}
</strong>
),
matches: (
<strong>
{i18n.translate(
'exceptionList-components.wildcardWithWrongOperatorCallout.matches',
{ defaultMessage: 'matches' }
)}
</strong>
),
}}
/>
</p>
</EuiCallOut>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
"@kbn/securitysolution-autocomplete",
"@kbn/ui-theme",
"@kbn/i18n",
"@kbn/i18n-react",
],
"exclude": [
"target/**/*",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
hasSimpleExecutableName,
OperatingSystem,
ConditionEntryField,
hasWildcardAndInvalidOperator,
validatePotentialWildcardInput,
validateFilePathInput,
validateWildcardInput,
Expand Down Expand Up @@ -128,6 +129,21 @@ describe('validateFilePathInput', () => {
});
});

describe('Wildcard and invalid operator', () => {
it('should return TRUE when operator is not "WILDCARD" and value contains a wildcard', () => {
expect(hasWildcardAndInvalidOperator({ operator: 'match', value: 'asdf*' })).toEqual(true);
});
it('should return FALSE when operator is not "WILDCARD" and value does not contain a wildcard', () => {
expect(hasWildcardAndInvalidOperator({ operator: 'match', value: 'asdf' })).toEqual(false);
});
it('should return FALSE when operator is "WILDCARD" and value contains a wildcard', () => {
expect(hasWildcardAndInvalidOperator({ operator: 'wildcard', value: 'asdf*' })).toEqual(false);
});
it('should return FALSE when operator is "WILDCARD" and value does not contain a wildcard', () => {
expect(hasWildcardAndInvalidOperator({ operator: 'wildcard', value: 'asdf' })).toEqual(false);
});
});

describe('No Warnings', () => {
it('should not show warnings on non path entries ', () => {
expect(
Expand Down
14 changes: 14 additions & 0 deletions packages/kbn-securitysolution-utils/src/path_validations/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,20 @@ export const validateWildcardInput = (value?: string): string | undefined => {
}
};

export const hasWildcardAndInvalidOperator = ({
operator,
value,
}: {
operator: EntryTypes | TrustedAppEntryTypes;
value: string;
}): boolean => {
if (operator !== 'wildcard' && validateWildcardInput(value)) {
return true;
} else {
return false;
}
};

export const hasSimpleExecutableName = ({
os,
type,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/*
* 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 React, { memo } from 'react';
import {
EuiButton,
EuiButtonEmpty,
EuiModal,
EuiModalBody,
EuiModalFooter,
EuiModalHeader,
EuiModalHeaderTitle,
EuiText,
} from '@elastic/eui';
import { useTestIdGenerator } from '../../../hooks/use_test_id_generator';

interface ConfirmArtifactModalProps {
title: string;
body: string;
confirmButton: string;
cancelButton: string;
onCancel: () => void;
onSuccess: () => void;
'data-test-subj'?: string;
}

export const ArtifactConfirmModal = memo<ConfirmArtifactModalProps>(
({
title,
body,
confirmButton,
cancelButton,
onCancel,
onSuccess,
'data-test-subj': dataTestSubj,
}) => {
const getTestId = useTestIdGenerator(dataTestSubj);

return (
<EuiModal onClose={onCancel} data-test-subj={dataTestSubj}>
<EuiModalHeader data-test-subj={getTestId('header')}>
<EuiModalHeaderTitle>{title}</EuiModalHeaderTitle>
</EuiModalHeader>

<EuiModalBody data-test-subj={getTestId('body')}>
<EuiText>
<p>{body}</p>
</EuiText>
</EuiModalBody>

<EuiModalFooter>
<EuiButtonEmpty onClick={onCancel} data-test-subj={getTestId('cancelButton')}>
{cancelButton}
</EuiButtonEmpty>

<EuiButton fill onClick={onSuccess} data-test-subj={getTestId('submitButton')}>
{confirmButton}
</EuiButton>
</EuiModalFooter>
</EuiModal>
);
}
);
ArtifactConfirmModal.displayName = 'ArtifactConfirmModal';
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,51 @@ describe('When the flyout is opened in the ArtifactListPage component', () => {
expect(location.search).toBe('');
});
});

describe('and there is a confirmModal', () => {
beforeEach(async () => {
const _renderAndWaitForFlyout = render;

// Override renderAndWaitForFlyout to also set the form data as "valid"
render = async (...props) => {
await _renderAndWaitForFlyout(...props);

act(() => {
const lastProps = getLastFormComponentProps();
lastProps.onChange({
item: { ...lastProps.item, name: 'some name' },
isValid: true,
confirmModalLabels: {
title: 'title',
body: 'body',
confirmButton: 'add',
cancelButton: 'cancel',
},
});
});

return renderResult;
};
});

it('should show the warning modal', async () => {
await render();
act(() => {
userEvent.click(renderResult.getByTestId('testPage-flyout-submitButton'));
});
expect(renderResult.getByTestId('artifactConfirmModal')).toBeTruthy();
expect(renderResult.getByTestId('artifactConfirmModal-header').textContent).toEqual(
'title'
);
expect(renderResult.getByTestId('artifactConfirmModal-body').textContent).toEqual('body');
expect(renderResult.getByTestId('artifactConfirmModal-submitButton').textContent).toEqual(
'add'
);
expect(renderResult.getByTestId('artifactConfirmModal-cancelButton').textContent).toEqual(
'cancel'
);
});
});
});

describe('and in Edit mode', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import { useWithArtifactSubmitData } from '../hooks/use_with_artifact_submit_dat
import { useIsArtifactAllowedPerPolicyUsage } from '../hooks/use_is_artifact_allowed_per_policy_usage';
import { useGetArtifact } from '../../../hooks/artifacts';
import type { PolicyData } from '../../../../../common/endpoint/types';
import { ArtifactConfirmModal } from './artifact_confirm_modal';

export const ARTIFACT_FLYOUT_LABELS = Object.freeze({
flyoutEditTitle: i18n.translate('xpack.securitySolution.artifactListPage.flyoutEditTitle', {
Expand Down Expand Up @@ -207,11 +208,12 @@ export const ArtifactFlyout = memo<ArtifactFlyoutProps>(
..._labels,
};
}, [_labels]);
// TODO:PT Refactor internal/external state into the `useEithArtifactSucmitData()` hook
// TODO:PT Refactor internal/external state into the `useWithArtifactSubmitData()` hook
const [externalIsSubmittingData, setExternalIsSubmittingData] = useState<boolean>(false);
const [externalSubmitHandlerError, setExternalSubmitHandlerError] = useState<
IHttpFetchError | undefined
>(undefined);
const [showConfirmModal, setShowConfirmModal] = useState<boolean>(false);

const isEditFlow = urlParams.show === 'edit';
const formMode: ArtifactFormComponentProps['mode'] = isEditFlow ? 'edit' : 'create';
Expand Down Expand Up @@ -270,11 +272,12 @@ export const ArtifactFlyout = memo<ArtifactFlyoutProps>(
}, [isSubmittingData, onClose, setUrlParams, urlParams]);

const handleFormComponentOnChange: ArtifactFormComponentProps['onChange'] = useCallback(
({ item: updatedItem, isValid }) => {
({ item: updatedItem, isValid, confirmModalLabels }) => {
if (isMounted()) {
setFormState({
item: updatedItem,
isValid,
confirmModalLabels,
});
}
},
Expand Down Expand Up @@ -316,10 +319,42 @@ export const ArtifactFlyout = memo<ArtifactFlyoutProps>(
setExternalIsSubmittingData(false);
}
});
} else if (formState.confirmModalLabels) {
setShowConfirmModal(true);
} else {
submitData(formState.item).then(handleSuccess);
}
}, [formMode, formState.item, handleSuccess, isMounted, submitData, submitHandler]);
}, [
formMode,
formState.item,
formState.confirmModalLabels,
handleSuccess,
isMounted,
submitData,
submitHandler,
]);

const confirmModalOnSuccess = useCallback(
() => submitData(formState.item).then(handleSuccess),
[submitData, formState.item, handleSuccess]
);

const confirmModal = useMemo(() => {
if (formState.confirmModalLabels) {
const { title, body, confirmButton, cancelButton } = formState.confirmModalLabels;
return (
<ArtifactConfirmModal
title={title}
body={body}
confirmButton={confirmButton}
cancelButton={cancelButton}
onSuccess={confirmModalOnSuccess}
onCancel={() => setShowConfirmModal(false)}
data-test-subj="artifactConfirmModal"
/>
);
}
}, [formState, confirmModalOnSuccess]);

// If we don't have the actual Artifact data yet for edit (in initialization phase - ex. came in with an
// ID in the url that was not in the list), then retrieve it now
Expand All @@ -342,7 +377,7 @@ export const ArtifactFlyout = memo<ArtifactFlyoutProps>(
isMounted,
]);

// If we got an error while trying ot retrieve the item for edit, then show a toast message
// If we got an error while trying to retrieve the item for edit, then show a toast message
useEffect(() => {
if (isEditFlow && error) {
toasts.addWarning(labels.flyoutEditItemLoadFailure(error?.body?.message || error.message));
Expand All @@ -363,7 +398,6 @@ export const ArtifactFlyout = memo<ArtifactFlyoutProps>(
<h2>{isEditFlow ? labels.flyoutEditTitle : labels.flyoutCreateTitle}</h2>
</EuiTitle>
</EuiFlyoutHeader>

{!isInitializing && showExpiredLicenseBanner && (
<EuiCallOut
title={labels.flyoutDowngradedLicenseTitle}
Expand All @@ -375,7 +409,6 @@ export const ArtifactFlyout = memo<ArtifactFlyoutProps>(
{labels.flyoutDowngradedLicenseDocsInfo(securitySolution)}
</EuiCallOut>
)}

<EuiFlyoutBody>
{isInitializing && <ManagementPageLoader data-test-subj={getTestId('loader')} />}

Expand All @@ -391,7 +424,6 @@ export const ArtifactFlyout = memo<ArtifactFlyoutProps>(
/>
)}
</EuiFlyoutBody>

{!isInitializing && (
<EuiFlyoutFooter>
<EuiFlexGroup justifyContent="spaceBetween">
Expand Down Expand Up @@ -420,6 +452,7 @@ export const ArtifactFlyout = memo<ArtifactFlyoutProps>(
</EuiFlexGroup>
</EuiFlyoutFooter>
)}
{showConfirmModal && confirmModal}
</EuiFlyout>
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,12 @@ export interface ArtifactFormComponentProps {
export interface ArtifactFormComponentOnChangeCallbackProps {
isValid: boolean;
item: ExceptionListItemSchema | CreateExceptionListItemSchema;
confirmModalLabels?: ArtifactConfirmModalLabelProps;
}

export interface ArtifactConfirmModalLabelProps {
title: string;
body: string;
confirmButton: string;
cancelButton: string;
}
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,16 @@ describe('Trusted apps form', () => {
});
});

describe('and a wildcard value is used with the IS operator', () => {
beforeEach(() => render());
it('shows callout warning and help text warning', () => {
setTextFieldValue(getConditionValue(getCondition()), 'somewildcard*');
rerenderWithLatestProps();
expect(renderResult.getByTestId('wildcardWithWrongOperatorCallout')).toBeTruthy();
expect(renderResult.getByText(INPUT_ERRORS.wildcardWithWrongOperatorWarning(0)));
});
});

describe('and all required data passes validation', () => {
it('should call change callback with isValid set to true and contain the new item', () => {
const propsItem: Partial<ArtifactFormComponentProps['item']> = {
Expand Down
Loading

0 comments on commit 7c5a668

Please sign in to comment.