Skip to content

Commit

Permalink
[SIEM][CASE] Fix bug when connector is deleted. (#65876)
Browse files Browse the repository at this point in the history
  • Loading branch information
cnasikas committed May 15, 2020
1 parent a3ce65e commit d174897
Show file tree
Hide file tree
Showing 15 changed files with 193 additions and 41 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export const getActions = (): FindActionResult[] => [
referencedByCount: 0,
},
{
id: 'd611af27-3532-4da9-8034-271fee81d634',
id: '123',
actionTypeId: '.servicenow',
name: 'ServiceNow',
config: {
Expand Down
21 changes: 18 additions & 3 deletions x-pack/plugins/case/server/routes/api/cases/push_case.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,23 @@ export function initPushCaseUserActionApi({
async (context, request, response) => {
try {
const client = context.core.savedObjects.client;
const actionsClient = await context.actions?.getActionsClient();

const caseId = request.params.case_id;
const query = pipe(
CaseExternalServiceRequestRt.decode(request.body),
fold(throwErrors(Boom.badRequest), identity)
);

if (actionsClient == null) {
throw Boom.notFound('Action client have not been found');
}

const { username, full_name, email } = await caseService.getUser({ request, response });

const pushedDate = new Date().toISOString();

const [myCase, myCaseConfigure, totalCommentsFindByCases] = await Promise.all([
const [myCase, myCaseConfigure, totalCommentsFindByCases, connectors] = await Promise.all([
caseService.getCase({
client,
caseId: request.params.case_id,
Expand All @@ -60,6 +68,7 @@ export function initPushCaseUserActionApi({
perPage: 1,
},
}),
actionsClient.getAll(),
]);

if (myCase.attributes.status === 'closed') {
Expand All @@ -85,9 +94,15 @@ export function initPushCaseUserActionApi({
};

const caseConfigureConnectorId = getConnectorId(myCaseConfigure);

// old case may not have new attribute connector_id, so we default to the configured system
const updateConnectorId =
myCase.attributes.connector_id == null ? { connector_id: caseConfigureConnectorId } : {};
const updateConnectorId = {
connector_id: myCase.attributes.connector_id ?? caseConfigureConnectorId,
};

if (!connectors.some(connector => connector.id === updateConnectorId.connector_id)) {
throw Boom.notFound('Connector not found or set to none');
}

const [updatedCase, updatedComments] = await Promise.all([
caseService.patchCase({
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugins/siem/cypress/integration/cases.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import {
ACTION,
CASE_DETAILS_DESCRIPTION,
CASE_DETAILS_PAGE_TITLE,
CASE_DETAILS_PUSH_AS_SERVICE_NOW_BTN,
CASE_DETAILS_PUSH_TO_EXTERNAL_SERVICE_BTN,
CASE_DETAILS_STATUS,
CASE_DETAILS_TAGS,
CASE_DETAILS_TIMELINE_MARKDOWN,
Expand Down Expand Up @@ -102,7 +102,7 @@ describe('Cases', () => {
.eq(PARTICIPANTS)
.should('have.text', case1.reporter);
cy.get(CASE_DETAILS_TAGS).should('have.text', expectedTags);
cy.get(CASE_DETAILS_PUSH_AS_SERVICE_NOW_BTN).should('have.attr', 'disabled');
cy.get(CASE_DETAILS_PUSH_TO_EXTERNAL_SERVICE_BTN).should('have.attr', 'disabled');
cy.get(CASE_DETAILS_TIMELINE_MARKDOWN).then($element => {
const timelineLink = $element.prop('href').match(/http(s?):\/\/\w*:\w*(\S*)/)[0];
openCaseTimeline(timelineLink);
Expand Down
3 changes: 2 additions & 1 deletion x-pack/plugins/siem/cypress/screens/case_details.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ export const CASE_DETAILS_DESCRIPTION = '[data-test-subj="markdown-root"]';

export const CASE_DETAILS_PAGE_TITLE = '[data-test-subj="header-page-title"]';

export const CASE_DETAILS_PUSH_AS_SERVICE_NOW_BTN = '[data-test-subj="push-to-external-service"]';
export const CASE_DETAILS_PUSH_TO_EXTERNAL_SERVICE_BTN =
'[data-test-subj="push-to-external-service"]';

export const CASE_DETAILS_STATUS = '[data-test-subj="case-view-status"]';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,27 @@ import { useUpdateCase } from '../../containers/use_update_case';
import { useGetCase } from '../../containers/use_get_case';
import { useGetCaseUserActions } from '../../containers/use_get_case_user_actions';
import { wait } from '../../../common/lib/helpers';
import { usePushToService } from '../use_push_to_service';

import { useConnectors } from '../../containers/configure/use_connectors';
import { connectorsMock } from '../../containers/configure/mock';

import { usePostPushToService } from '../../containers/use_post_push_to_service';

jest.mock('../../containers/use_update_case');
jest.mock('../../containers/use_get_case_user_actions');
jest.mock('../../containers/use_get_case');
jest.mock('../use_push_to_service');
jest.mock('../../containers/configure/use_connectors');
jest.mock('../../containers/use_post_push_to_service');

const useUpdateCaseMock = useUpdateCase as jest.Mock;
const useGetCaseUserActionsMock = useGetCaseUserActions as jest.Mock;
const usePushToServiceMock = usePushToService as jest.Mock;
const useConnectorsMock = useConnectors as jest.Mock;
const usePostPushToServiceMock = usePostPushToService as jest.Mock;

export const caseProps: CaseProps = {
caseId: basicCase.id,
userCanCrud: true,
caseData: basicCase,
caseData: { ...basicCase, connectorId: 'servicenow-2' },
fetchCase: jest.fn(),
updateCase: jest.fn(),
};
Expand All @@ -42,6 +50,8 @@ describe('CaseView ', () => {
const fetchCaseUserActions = jest.fn();
const fetchCase = jest.fn();
const updateCase = jest.fn();
const postPushToService = jest.fn();

const data = caseProps.caseData;
const defaultGetCase = {
isLoading: false,
Expand Down Expand Up @@ -85,18 +95,8 @@ describe('CaseView ', () => {
useUpdateCaseMock.mockImplementation(() => defaultUpdateCaseState);
jest.spyOn(routeData, 'useLocation').mockReturnValue(mockLocation);
useGetCaseUserActionsMock.mockImplementation(() => defaultUseGetCaseUserActions);
usePushToServiceMock.mockImplementation(({ updateCase: updateCaseMockCall }) => ({
pushButton: (
<button
data-test-subj="mock-button"
onClick={() => updateCaseMockCall(caseProps.caseData)}
type="button"
>
{'Hello Button'}
</button>
),
pushCallouts: null,
}));
usePostPushToServiceMock.mockImplementation(() => ({ isLoading: false, postPushToService }));
useConnectorsMock.mockImplementation(() => ({ connectors: connectorsMock, isLoading: false }));
});

it('should render CaseComponent', async () => {
Expand Down Expand Up @@ -328,27 +328,32 @@ describe('CaseView ', () => {
...defaultUseGetCaseUserActions,
hasDataToPush: true,
}));

const wrapper = mount(
<TestProviders>
<Router history={mockHistory}>
<CaseComponent {...{ ...caseProps, updateCase }} />
</Router>
</TestProviders>
);

await wait();

expect(
wrapper
.find('[data-test-subj="has-data-to-push-button"]')
.first()
.exists()
).toBeTruthy();

wrapper
.find('[data-test-subj="mock-button"]')
.find('[data-test-subj="push-to-external-service"]')
.first()
.simulate('click');

wrapper.update();
await wait();
expect(updateCase).toBeCalledWith(caseProps.caseData);
expect(fetchCaseUserActions).toBeCalledWith(caseProps.caseData.id);

expect(postPushToService).toHaveBeenCalled();
});

it('should return null if error', () => {
Expand Down Expand Up @@ -429,4 +434,32 @@ describe('CaseView ', () => {
expect(fetchCaseUserActions).toBeCalledWith(caseProps.caseData.id);
expect(fetchCase).toBeCalled();
});

it('should disable the push button when connector is invalid', () => {
useGetCaseUserActionsMock.mockImplementation(() => ({
...defaultUseGetCaseUserActions,
hasDataToPush: true,
}));

const wrapper = mount(
<TestProviders>
<Router history={mockHistory}>
<CaseComponent
{...{
...caseProps,
updateCase,
caseData: { ...caseProps.caseData, connectorId: 'not-exist' },
}}
/>
</Router>
</TestProviders>
);

expect(
wrapper
.find('button[data-test-subj="push-to-external-service"]')
.first()
.prop('disabled')
).toBeTruthy();
});
});
10 changes: 6 additions & 4 deletions x-pack/plugins/siem/public/cases/components/case_view/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -163,10 +163,11 @@ export const CaseComponent = React.memo<CaseProps>(
);

const { loading: isLoadingConnectors, connectors } = useConnectors();
const caseConnectorName = useMemo(
() => connectors.find(c => c.id === caseData.connectorId)?.name ?? 'none',
[connectors, caseData.connectorId]
);

const [caseConnectorName, isValidConnector] = useMemo(() => {
const connector = connectors.find(c => c.id === caseData.connectorId);
return [connector?.name ?? 'none', !!connector];
}, [connectors, caseData.connectorId]);

const currentExternalIncident = useMemo(
() =>
Expand All @@ -185,6 +186,7 @@ export const CaseComponent = React.memo<CaseProps>(
connectors,
updateCase: handleUpdateCase,
userCanCrud,
isValidConnector,
});

const onSubmitConnector = useCallback(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@ describe('usePushToService', () => {
connectors: connectorsMock,
updateCase,
userCanCrud: true,
isValidConnector: true,
};

beforeEach(() => {
jest.resetAllMocks();
(usePostPushToService as jest.Mock).mockImplementation(() => mockPostPush);
Expand All @@ -55,6 +57,7 @@ describe('usePushToService', () => {
actionLicense,
}));
});

it('push case button posts the push with correct args', async () => {
await act(async () => {
const { result, waitForNextUpdate } = renderHook<UsePushToService, ReturnUsePushToService>(
Expand All @@ -75,6 +78,7 @@ describe('usePushToService', () => {
expect(result.current.pushCallouts).toBeNull();
});
});

it('Displays message when user does not have premium license', async () => {
(useGetActionLicense as jest.Mock).mockImplementation(() => ({
isLoading: false,
Expand All @@ -96,6 +100,7 @@ describe('usePushToService', () => {
expect(errorsMsg[0].title).toEqual(getLicenseError().title);
});
});

it('Displays message when user does not have case enabled in config', async () => {
(useGetActionLicense as jest.Mock).mockImplementation(() => ({
isLoading: false,
Expand All @@ -117,6 +122,7 @@ describe('usePushToService', () => {
expect(errorsMsg[0].title).toEqual(getKibanaConfigError().title);
});
});

it('Displays message when user does not have a connector configured', async () => {
await act(async () => {
const { result, waitForNextUpdate } = renderHook<UsePushToService, ReturnUsePushToService>(
Expand All @@ -135,6 +141,27 @@ describe('usePushToService', () => {
expect(errorsMsg[0].title).toEqual(i18n.PUSH_DISABLE_BY_NO_CASE_CONFIG_TITLE);
});
});

it('Displays message when connector is deleted', async () => {
await act(async () => {
const { result, waitForNextUpdate } = renderHook<UsePushToService, ReturnUsePushToService>(
() =>
usePushToService({
...defaultArgs,
caseConnectorId: 'not-exist',
isValidConnector: false,
}),
{
wrapper: ({ children }) => <TestProviders> {children}</TestProviders>,
}
);
await waitForNextUpdate();
const errorsMsg = result.current.pushCallouts?.props.messages;
expect(errorsMsg).toHaveLength(1);
expect(errorsMsg[0].title).toEqual(i18n.PUSH_DISABLE_BY_NO_CASE_CONFIG_TITLE);
});
});

it('Displays message when case is closed', async () => {
await act(async () => {
const { result, waitForNextUpdate } = renderHook<UsePushToService, ReturnUsePushToService>(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export interface UsePushToService {
connectors: Connector[];
updateCase: (newCase: Case) => void;
userCanCrud: boolean;
isValidConnector: boolean;
}

export interface ReturnUsePushToService {
Expand All @@ -45,6 +46,7 @@ export const usePushToService = ({
connectors,
updateCase,
userCanCrud,
isValidConnector,
}: UsePushToService): ReturnUsePushToService => {
const urlSearch = useGetUrlSearch(navTabs.case);

Expand Down Expand Up @@ -77,7 +79,7 @@ export const usePushToService = ({
description: (
<FormattedMessage
defaultMessage="To open and update cases in external systems, you must configure a {link}."
id="xpack.siem.case.caseView.pushToServiceDisableByNoCaseConfigDescription"
id="xpack.siem.case.caseView.pushToServiceDisableByNoConnectors"
values={{
link: (
<EuiLink href={getConfigureCasesUrl(urlSearch)} target="_blank">
Expand All @@ -97,7 +99,20 @@ export const usePushToService = ({
description: (
<FormattedMessage
defaultMessage="To open and update cases in external systems, you must select an external incident management system for this case."
id="xpack.siem.case.caseView.pushToServiceDisableByNoCaseConfigDesc"
id="xpack.siem.case.caseView.pushToServiceDisableByNoCaseConfigDescription"
/>
),
},
];
} else if (!isValidConnector && !loadingLicense) {
errors = [
...errors,
{
title: i18n.PUSH_DISABLE_BY_NO_CASE_CONFIG_TITLE,
description: (
<FormattedMessage
defaultMessage="The connector used to send updates to external service has been deleted. To update cases in external systems, select a different connector or create a new one."
id="xpack.siem.case.caseView.pushToServiceDisableByInvalidConnector"
/>
),
},
Expand Down Expand Up @@ -130,7 +145,9 @@ export const usePushToService = ({
fill
iconType="importAction"
onClick={handlePushToService}
disabled={isLoading || loadingLicense || errorsMsg.length > 0 || !userCanCrud}
disabled={
isLoading || loadingLicense || errorsMsg.length > 0 || !userCanCrud || !isValidConnector
}
isLoading={isLoading}
>
{caseServices[caseConnectorId]
Expand All @@ -147,6 +164,7 @@ export const usePushToService = ({
isLoading,
loadingLicense,
userCanCrud,
isValidConnector,
]);

const objToReturn = useMemo(() => {
Expand Down
Loading

0 comments on commit d174897

Please sign in to comment.