[Security Solution] [Case] Fixes "Case connector cannot be updated when created with a wrong field" (#87223)

This commit is contained in:
Steph Milovic 2021-01-05 15:33:36 -07:00 committed by GitHub
parent e954306786
commit b0ba4f47ab
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
19 changed files with 331 additions and 65 deletions

View file

@ -583,6 +583,19 @@ describe('Jira service', () => {
'[Action][Jira]: Unable to get capabilities. Error: An error has occurred. Reason: Could not get capabilities'
);
});
test('it should throw an auth error', async () => {
requestMock.mockImplementation(() => {
const error = new Error('An error has occurred');
// @ts-ignore this can happen!
error.response = { data: 'Unauthorized' };
throw error;
});
await expect(service.getCapabilities()).rejects.toThrow(
'[Action][Jira]: Unable to get capabilities. Error: An error has occurred. Reason: Unauthorized'
);
});
});
describe('getIssueTypes', () => {

View file

@ -102,10 +102,14 @@ export const createExternalService = (
return fields;
};
const createErrorMessage = (errorResponse: ResponseError | null | undefined): string => {
const createErrorMessage = (errorResponse: ResponseError | string | null | undefined): string => {
if (errorResponse == null) {
return '';
}
if (typeof errorResponse === 'string') {
// Jira error.response.data can be string!!
return errorResponse;
}
const { errorMessages, errors } = errorResponse;

View file

@ -41,6 +41,7 @@ export const CaseConfigureResponseRt = rt.intersection([
ConnectorMappingsRt,
rt.type({
version: rt.string,
error: rt.union([rt.string, rt.null]),
}),
]);

View file

@ -19,6 +19,7 @@ import { initGetCaseConfigure } from './get_configure';
import { CASE_CONFIGURE_URL } from '../../../../../common/constants';
import { mappings } from '../../../../client/configure/mock';
import { ConnectorTypes } from '../../../../../common/api/connectors';
import { CaseClient } from '../../../../client';
describe('GET configuration', () => {
let routeHandler: RequestHandler<any, any, any>;
@ -43,6 +44,7 @@ describe('GET configuration', () => {
expect(res.status).toEqual(200);
expect(res.payload).toEqual({
...mockCaseConfigure[0].attributes,
error: null,
mappings: mappings[ConnectorTypes.jira],
version: mockCaseConfigure[0].version,
});
@ -77,6 +79,7 @@ describe('GET configuration', () => {
email: 'testemail@elastic.co',
username: 'elastic',
},
error: null,
mappings: mappings[ConnectorTypes.jira],
updated_at: '2020-04-09T09:43:51.778Z',
updated_by: {
@ -122,4 +125,40 @@ describe('GET configuration', () => {
expect(res.status).toEqual(404);
expect(res.payload.isBoom).toEqual(true);
});
it('returns an error when mappings request throws', async () => {
const req = httpServerMock.createKibanaRequest({
path: CASE_CONFIGURE_URL,
method: 'get',
});
const context = await createRouteContext(
createMockSavedObjectsRepository({
caseConfigureSavedObject: mockCaseConfigure,
caseMappingsSavedObject: [],
})
);
const mockThrowContext = {
...context,
case: {
...context.case,
getCaseClient: () =>
({
...context?.case?.getCaseClient(),
getMappings: () => {
throw new Error();
},
} as CaseClient),
},
};
const res = await routeHandler(mockThrowContext, req, kibanaResponseFactory);
expect(res.status).toEqual(200);
expect(res.payload).toEqual({
...mockCaseConfigure[0].attributes,
error: 'Error connecting to My connector 3 instance',
mappings: [],
version: mockCaseConfigure[0].version,
});
});
});

View file

@ -19,6 +19,7 @@ export function initGetCaseConfigure({ caseConfigureService, router }: RouteDeps
},
async (context, request, response) => {
try {
let error = null;
const client = context.core.savedObjects.client;
const myCaseConfigure = await caseConfigureService.find({ client });
@ -35,12 +36,18 @@ export function initGetCaseConfigure({ caseConfigureService, router }: RouteDeps
if (actionsClient == null) {
throw Boom.notFound('Action client have not been found');
}
mappings = await caseClient.getMappings({
actionsClient,
caseClient,
connectorId: connector.id,
connectorType: connector.type,
});
try {
mappings = await caseClient.getMappings({
actionsClient,
caseClient,
connectorId: connector.id,
connectorType: connector.type,
});
} catch (e) {
error = e.isBoom
? e.output.payload.message
: `Error connecting to ${connector.name} instance`;
}
}
return response.ok({
@ -51,6 +58,7 @@ export function initGetCaseConfigure({ caseConfigureService, router }: RouteDeps
connector: transformESConnectorToCaseConnector(connector),
mappings,
version: myCaseConfigure.saved_objects[0].version ?? '',
error,
})
: {},
});

View file

@ -18,6 +18,7 @@ import { mockCaseConfigure } from '../../__fixtures__/mock_saved_objects';
import { initPatchCaseConfigure } from './patch_configure';
import { CASE_CONFIGURE_URL } from '../../../../../common/constants';
import { ConnectorTypes } from '../../../../../common/api/connectors';
import { CaseClient } from '../../../../client';
describe('PATCH configuration', () => {
let routeHandler: RequestHandler<any, any, any>;
@ -135,6 +136,52 @@ describe('PATCH configuration', () => {
);
});
it('patch configuration with error message for getMappings throw', async () => {
const req = httpServerMock.createKibanaRequest({
path: CASE_CONFIGURE_URL,
method: 'patch',
body: {
closure_type: 'close-by-pushing',
connector: {
id: 'connector-new',
name: 'New connector',
type: '.jira',
fields: null,
},
version: mockCaseConfigure[0].version,
},
});
const context = await createRouteContext(
createMockSavedObjectsRepository({
caseConfigureSavedObject: mockCaseConfigure,
caseMappingsSavedObject: [],
})
);
const mockThrowContext = {
...context,
case: {
...context.case,
getCaseClient: () =>
({
...context?.case?.getCaseClient(),
getMappings: () => {
throw new Error();
},
} as CaseClient),
},
};
const res = await routeHandler(mockThrowContext, req, kibanaResponseFactory);
expect(res.status).toEqual(200);
expect(res.payload).toEqual(
expect.objectContaining({
mappings: [],
error: 'Error connecting to New connector instance',
})
);
});
it('throw error when configuration have not being created', async () => {
const req = httpServerMock.createKibanaRequest({
path: CASE_CONFIGURE_URL,

View file

@ -33,6 +33,7 @@ export function initPatchCaseConfigure({ caseConfigureService, caseService, rout
},
async (context, request, response) => {
try {
let error = null;
const client = context.core.savedObjects.client;
const query = pipe(
CasesConfigurePatchRt.decode(request.body),
@ -68,12 +69,18 @@ export function initPatchCaseConfigure({ caseConfigureService, caseService, rout
if (actionsClient == null) {
throw Boom.notFound('Action client have not been found');
}
mappings = await caseClient.getMappings({
actionsClient,
caseClient,
connectorId: connector.id,
connectorType: connector.type,
});
try {
mappings = await caseClient.getMappings({
actionsClient,
caseClient,
connectorId: connector.id,
connectorType: connector.type,
});
} catch (e) {
error = e.isBoom
? e.output.payload.message
: `Error connecting to ${connector.name} instance`;
}
}
const patch = await caseConfigureService.patch({
client,
@ -96,6 +103,7 @@ export function initPatchCaseConfigure({ caseConfigureService, caseService, rout
),
mappings,
version: patch.version ?? '',
error,
}),
});
} catch (error) {

View file

@ -19,6 +19,7 @@ import { initPostCaseConfigure } from './post_configure';
import { newConfiguration } from '../../__mocks__/request_responses';
import { CASE_CONFIGURE_URL } from '../../../../../common/constants';
import { ConnectorTypes } from '../../../../../common/api/connectors';
import { CaseClient } from '../../../../client';
describe('POST configuration', () => {
let routeHandler: RequestHandler<any, any, any>;
@ -64,6 +65,43 @@ describe('POST configuration', () => {
})
);
});
it('create configuration with error message for getMappings throw', async () => {
const req = httpServerMock.createKibanaRequest({
path: CASE_CONFIGURE_URL,
method: 'post',
body: newConfiguration,
});
const context = await createRouteContext(
createMockSavedObjectsRepository({
caseConfigureSavedObject: mockCaseConfigure,
caseMappingsSavedObject: [],
})
);
const mockThrowContext = {
...context,
case: {
...context.case,
getCaseClient: () =>
({
...context?.case?.getCaseClient(),
getMappings: () => {
throw new Error();
},
} as CaseClient),
},
};
const res = await routeHandler(mockThrowContext, req, kibanaResponseFactory);
expect(res.status).toEqual(200);
expect(res.payload).toEqual(
expect.objectContaining({
mappings: [],
error: 'Error connecting to My connector 2 instance',
})
);
});
it('create configuration without authentication', async () => {
routeHandler = await createRoute(initPostCaseConfigure, 'post', true);

View file

@ -13,6 +13,7 @@ import {
CasesConfigureRequestRt,
CaseConfigureResponseRt,
throwErrors,
ConnectorMappingsAttributes,
} from '../../../../../common/api';
import { RouteDeps } from '../../types';
import { wrapError, escapeHatch } from '../../utils';
@ -32,6 +33,7 @@ export function initPostCaseConfigure({ caseConfigureService, caseService, route
},
async (context, request, response) => {
try {
let error = null;
if (!context.case) {
throw Boom.badRequest('RouteHandlerContext is not registered for cases');
}
@ -58,12 +60,19 @@ export function initPostCaseConfigure({ caseConfigureService, caseService, route
const { email, full_name, username } = await caseService.getUser({ request, response });
const creationDate = new Date().toISOString();
const mappings = await caseClient.getMappings({
actionsClient,
caseClient,
connectorId: query.connector.id,
connectorType: query.connector.type,
});
let mappings: ConnectorMappingsAttributes[] = [];
try {
mappings = await caseClient.getMappings({
actionsClient,
caseClient,
connectorId: query.connector.id,
connectorType: query.connector.type,
});
} catch (e) {
error = e.isBoom
? e.output.payload.message
: `Error connecting to ${query.connector.name} instance`;
}
const post = await caseConfigureService.post({
client,
attributes: {
@ -83,6 +92,7 @@ export function initPostCaseConfigure({ caseConfigureService, caseService, route
connector: transformESConnectorToCaseConnector(post.attributes.connector),
mappings,
version: post.version ?? '',
error,
}),
});
} catch (error) {

View file

@ -29,6 +29,7 @@ describe('Cases connectors', () => {
closure_type: 'close-by-user',
created_at: '2020-12-01T16:28:09.219Z',
created_by: { email: null, full_name: null, username: 'elastic' },
error: null,
updated_at: null,
updated_by: null,
mappings: [
@ -47,6 +48,23 @@ describe('Cases connectors', () => {
res.send(200, { ...configureResult, connector });
});
}).as('saveConnector');
cy.intercept('GET', '/api/cases/configure', (req) => {
req.reply((res) => {
const resBody =
res.body.version != null
? {
...res.body,
error: null,
mappings: [
{ source: 'title', target: 'short_description', action_type: 'overwrite' },
{ source: 'description', target: 'description', action_type: 'overwrite' },
{ source: 'comments', target: 'comments', action_type: 'append' },
],
}
: res.body;
res.send(200, resBody);
});
});
});
it('Configures a new connector', () => {

View file

@ -72,16 +72,17 @@ const ConfigureCasesComponent: React.FC<ConfigureCasesComponentProps> = ({ userC
mappings,
persistLoading,
persistCaseConfigure,
refetchCaseConfigure,
setConnector,
setClosureType,
} = useCaseConfigure();
const { loading: isLoadingConnectors, connectors, refetchConnectors } = useConnectors();
// ActionsConnectorsContextProvider reloadConnectors prop expects a Promise<void>.
// TODO: Fix it if reloadConnectors type change.
// eslint-disable-next-line react-hooks/exhaustive-deps
const reloadConnectors = useCallback(async () => refetchConnectors(), []);
const onConnectorUpdate = useCallback(async () => {
refetchConnectors();
refetchCaseConfigure();
}, [refetchCaseConfigure, refetchConnectors]);
const isLoadingAny = isLoadingConnectors || persistLoading || loadingCaseConfigure;
const updateConnectorDisabled = isLoadingAny || !connectorIsValid || connector.id === 'none';
const onClickUpdateConnector = useCallback(() => {
@ -92,9 +93,7 @@ const ConfigureCasesComponent: React.FC<ConfigureCasesComponentProps> = ({ userC
setAddFlyoutVisibility,
]);
const onCloseEditFlyout = useCallback(() => setEditFlyoutVisibility(false), [
setEditFlyoutVisibility,
]);
const onCloseEditFlyout = useCallback(() => setEditFlyoutVisibility(false), []);
const onChangeConnector = useCallback(
(id: string) => {
@ -156,7 +155,7 @@ const ConfigureCasesComponent: React.FC<ConfigureCasesComponentProps> = ({ userC
consumer: 'case',
onClose: onCloseAddFlyout,
actionTypes,
reloadConnectors,
reloadConnectors: onConnectorUpdate,
}),
// eslint-disable-next-line react-hooks/exhaustive-deps
[]
@ -169,7 +168,7 @@ const ConfigureCasesComponent: React.FC<ConfigureCasesComponentProps> = ({ userC
initialConnector: editedConnectorItem,
consumer: 'case',
onClose: onCloseEditFlyout,
reloadConnectors,
reloadConnectors: onConnectorUpdate,
})
: null,
// eslint-disable-next-line react-hooks/exhaustive-deps

View file

@ -5,15 +5,13 @@
*/
import React from 'react';
import { mount, ReactWrapper } from 'enzyme';
import { mount } from 'enzyme';
import { TestProviders } from '../../../common/mock';
import { Mapping, MappingProps } from './mapping';
import { mappings } from './__mock__';
describe('Mapping', () => {
let wrapper: ReactWrapper;
const setEditFlyoutVisibility = jest.fn();
const props: MappingProps = {
connectorActionTypeId: '.servicenow',
isLoading: false,
@ -22,39 +20,27 @@ describe('Mapping', () => {
beforeEach(() => {
jest.clearAllMocks();
wrapper = mount(<Mapping {...props} />, { wrappingComponent: TestProviders });
});
test('it shows mapping form group', () => {
const wrapper = mount(<Mapping {...props} />, { wrappingComponent: TestProviders });
expect(wrapper.find('[data-test-subj="static-mappings"]').first().exists()).toBe(true);
});
afterEach(() => {
wrapper.unmount();
test('correctly maps fields', () => {
const wrapper = mount(<Mapping {...props} />, { wrappingComponent: TestProviders });
expect(wrapper.find('[data-test-subj="field-mapping-source"] code').first().text()).toBe(
'title'
);
expect(wrapper.find('[data-test-subj="field-mapping-target"] code').first().text()).toBe(
'short_description'
);
});
describe('Common', () => {
test('it shows mapping form group', () => {
expect(wrapper.find('[data-test-subj="static-mappings"]').first().exists()).toBe(true);
});
test('correctly maps fields', () => {
expect(wrapper.find('[data-test-subj="field-mapping-source"] code').first().text()).toBe(
'title'
);
expect(wrapper.find('[data-test-subj="field-mapping-target"] code').first().text()).toBe(
'short_description'
);
});
// skipping until next PR
test.skip('it shows the update button', () => {
expect(
wrapper.find('[data-test-subj="case-mappings-update-connector-button"]').first().exists()
).toBe(true);
});
test.skip('it triggers update flyout', () => {
expect(setEditFlyoutVisibility).not.toHaveBeenCalled();
wrapper
.find('button[data-test-subj="case-mappings-update-connector-button"]')
.first()
.simulate('click');
expect(setEditFlyoutVisibility).toHaveBeenCalled();
test('displays connection warning when isLoading: false and mappings: []', () => {
const wrapper = mount(<Mapping {...{ ...props, mappings: [] }} />, {
wrappingComponent: TestProviders,
});
expect(wrapper.find('[data-test-subj="field-mapping-desc"]').first().text()).toBe(
'Field mappings require an established connection to ServiceNow. Please check your connection credentials.'
);
});
});

View file

@ -8,6 +8,7 @@ import React, { useMemo } from 'react';
import { EuiFlexGroup, EuiFlexItem, EuiText, EuiTextColor } from '@elastic/eui';
import { TextColor } from '@elastic/eui/src/components/text/text_color';
import * as i18n from './translations';
import { FieldMapping } from './field_mapping';
@ -28,13 +29,20 @@ const MappingComponent: React.FC<MappingProps> = ({
const selectedConnector = useMemo(() => connectorsConfiguration[connectorActionTypeId], [
connectorActionTypeId,
]);
const fieldMappingDesc: { desc: string; color: TextColor } = useMemo(
() =>
mappings.length > 0 || isLoading
? { desc: i18n.FIELD_MAPPING_DESC(selectedConnector.name), color: 'subdued' }
: { desc: i18n.FIELD_MAPPING_DESC_ERR(selectedConnector.name), color: 'danger' },
[isLoading, mappings.length, selectedConnector.name]
);
return (
<EuiFlexGroup direction="column" gutterSize="none">
<EuiFlexItem grow={false}>
<EuiText size="xs">
<h4>{i18n.FIELD_MAPPING_TITLE(selectedConnector.name)}</h4>
<EuiTextColor color="subdued">
{i18n.FIELD_MAPPING_DESC(selectedConnector.name)}
<EuiTextColor data-test-subj="field-mapping-desc" color={fieldMappingDesc.color}>
{fieldMappingDesc.desc}
</EuiTextColor>
</EuiText>
</EuiFlexItem>

View file

@ -94,6 +94,14 @@ export const FIELD_MAPPING_DESC = (thirdPartyName: string): string => {
'Map Security Case fields to { thirdPartyName } fields when pushing data to { thirdPartyName }. Field mappings require an established connection to { thirdPartyName }.',
});
};
export const FIELD_MAPPING_DESC_ERR = (thirdPartyName: string): string => {
return i18n.translate('xpack.securitySolution.case.configureCases.fieldMappingDescErr', {
values: { thirdPartyName },
defaultMessage:
'Field mappings require an established connection to { thirdPartyName }. Please check your connection credentials.',
});
};
export const EDIT_FIELD_MAPPING_TITLE = (thirdPartyName: string): string => {
return i18n.translate('xpack.securitySolution.case.configureCases.editFieldMappingTitle', {
values: { thirdPartyName },

View file

@ -70,6 +70,7 @@ export const caseConfigurationResposeMock: CasesConfigureResponse = {
fields: null,
},
closure_type: 'close-by-pushing',
error: null,
mappings: [],
updated_at: '2020-04-06T14:03:18.657Z',
updated_by: { username: 'elastic', full_name: 'Elastic', email: 'elastic@elastic.co' },
@ -96,6 +97,7 @@ export const caseConfigurationCamelCaseResponseMock: CaseConfigure = {
fields: null,
},
closureType: 'close-by-pushing',
error: null,
mappings: [],
updatedAt: '2020-04-06T14:03:18.657Z',
updatedBy: { username: 'elastic', fullName: 'Elastic', email: 'elastic@elastic.co' },

View file

@ -28,6 +28,7 @@ export interface CaseConfigure {
connector: CasesConfigure['connector'];
createdAt: string;
createdBy: ElasticUser;
error: string | null;
mappings: CaseConnectorMapping[];
updatedAt: string;
updatedBy: ElasticUser;

View file

@ -16,7 +16,14 @@ import * as api from './api';
import { ConnectorTypes } from '../../../../../case/common/api/connectors';
jest.mock('./api');
const mockErrorToToaster = jest.fn();
jest.mock('../../../common/components/toasters', () => {
const original = jest.requireActual('../../../common/components/toasters');
return {
...original,
errorToToaster: () => mockErrorToToaster(),
};
});
const configuration: ConnectorConfiguration = {
connector: {
id: '456',
@ -156,6 +163,7 @@ describe('useConfigure', () => {
);
await waitForNextUpdate();
await waitForNextUpdate();
expect(mockErrorToToaster).not.toHaveBeenCalled();
result.current.persistCaseConfigure(configuration);
@ -165,6 +173,60 @@ describe('useConfigure', () => {
});
});
test('Displays error when present - getCaseConfigure', async () => {
const spyOnGetCaseConfigure = jest.spyOn(api, 'getCaseConfigure');
spyOnGetCaseConfigure.mockImplementation(() =>
Promise.resolve({
...caseConfigurationCamelCaseResponseMock,
error: 'uh oh homeboy',
version: '',
})
);
await act(async () => {
const { waitForNextUpdate } = renderHook<string, ReturnUseCaseConfigure>(() =>
useCaseConfigure()
);
await waitForNextUpdate();
await waitForNextUpdate();
expect(mockErrorToToaster).toHaveBeenCalled();
});
});
test('Displays error when present - postCaseConfigure', async () => {
// When there is no version, a configuration is created. Otherwise is updated.
const spyOnGetCaseConfigure = jest.spyOn(api, 'getCaseConfigure');
spyOnGetCaseConfigure.mockImplementation(() =>
Promise.resolve({
...caseConfigurationCamelCaseResponseMock,
version: '',
})
);
const spyOnPostCaseConfigure = jest.spyOn(api, 'postCaseConfigure');
spyOnPostCaseConfigure.mockImplementation(() =>
Promise.resolve({
...caseConfigurationCamelCaseResponseMock,
...configuration,
error: 'uh oh homeboy',
})
);
await act(async () => {
const { result, waitForNextUpdate } = renderHook<string, ReturnUseCaseConfigure>(() =>
useCaseConfigure()
);
await waitForNextUpdate();
await waitForNextUpdate();
expect(mockErrorToToaster).not.toHaveBeenCalled();
result.current.persistCaseConfigure(configuration);
expect(mockErrorToToaster).not.toHaveBeenCalled();
await waitForNextUpdate();
expect(mockErrorToToaster).toHaveBeenCalled();
});
});
test('save case configuration - patchCaseConfigure', async () => {
const spyOnPatchCaseConfigure = jest.spyOn(api, 'patchCaseConfigure');
spyOnPatchCaseConfigure.mockImplementation(() =>

View file

@ -235,6 +235,13 @@ export const useCaseConfigure = (): ReturnUseCaseConfigure => {
});
}
}
if (res.error != null) {
errorToToaster({
dispatchToaster,
error: new Error(res.error),
title: i18n.ERROR_TITLE,
});
}
}
setLoading(false);
}
@ -295,7 +302,13 @@ export const useCaseConfigure = (): ReturnUseCaseConfigure => {
},
});
}
if (res.error != null) {
errorToToaster({
dispatchToaster,
error: new Error(res.error),
title: i18n.ERROR_TITLE,
});
}
displaySuccessToast(i18n.SUCCESS_CONFIGURE, dispatchToaster);
setPersistLoading(false);
}

View file

@ -32,6 +32,7 @@ export const getConfiguration = ({
export const getConfigurationOutput = (update = false): Partial<CasesConfigureResponse> => {
return {
...getConfiguration(),
error: null,
mappings: [],
created_by: { email: null, full_name: null, username: 'elastic' },
updated_by: update ? { email: null, full_name: null, username: 'elastic' } : null,