From fcce9b04d0504a88ef6c20b0a3d0605e367a29be Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Mon, 15 May 2023 15:18:00 +0300 Subject: [PATCH 01/13] Encode responses from the cases service --- .../plugins/cases/server/common/types/case.ts | 11 +++ .../cases/server/common/types/saved_object.ts | 20 +++++ .../cases/server/services/cases/index.test.ts | 76 +++++++++++-------- .../cases/server/services/cases/index.ts | 48 +++++++++--- x-pack/plugins/cases/server/services/utils.ts | 17 +++++ 5 files changed, 131 insertions(+), 41 deletions(-) create mode 100644 x-pack/plugins/cases/server/common/types/saved_object.ts create mode 100644 x-pack/plugins/cases/server/services/utils.ts diff --git a/x-pack/plugins/cases/server/common/types/case.ts b/x-pack/plugins/cases/server/common/types/case.ts index 8e425853964c0..6b1d5f9348c91 100644 --- a/x-pack/plugins/cases/server/common/types/case.ts +++ b/x-pack/plugins/cases/server/common/types/case.ts @@ -6,7 +6,9 @@ */ import type { SavedObject } from '@kbn/core-saved-objects-server'; +import { exact, partial } from 'io-ts'; import type { CaseAttributes } from '../../../common/api'; +import { CaseAttributesRt } from '../../../common/api'; import type { ConnectorPersisted } from './connectors'; import type { ExternalServicePersisted } from './external_service'; import type { User, UserProfile } from './user'; @@ -46,7 +48,16 @@ export interface CasePersistedAttributes { updated_by: User | null; } +const caseTransformedAttributesProps = CaseAttributesRt.types.reduce( + (acc, type) => ({ ...acc, ...type.props }), + {} +); + export type CaseTransformedAttributes = CaseAttributes; +export const CaseTransformedAttributesRt = CaseAttributesRt; +export const PartialCaseTransformedAttributesRt = exact( + partial({ ...caseTransformedAttributesProps }) +); export type CaseSavedObject = SavedObject; export type CaseSavedObjectTransformed = SavedObject; diff --git a/x-pack/plugins/cases/server/common/types/saved_object.ts b/x-pack/plugins/cases/server/common/types/saved_object.ts new file mode 100644 index 0000000000000..86070c3832430 --- /dev/null +++ b/x-pack/plugins/cases/server/common/types/saved_object.ts @@ -0,0 +1,20 @@ +/* + * 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 * as rt from 'io-ts'; + +export const createSOType = (attributes: rt.InterfaceType) => { + const requiredFields = rt.strict({ + id: rt.string, + type: rt.string, + attributes, + }); + + const optionalFields = rt.exact(rt.partial({ version: rt.string })); + + return rt.intersection([requiredFields, optionalFields]); +}; diff --git a/x-pack/plugins/cases/server/services/cases/index.test.ts b/x-pack/plugins/cases/server/services/cases/index.test.ts index c0039529ffc91..4258e425b02c8 100644 --- a/x-pack/plugins/cases/server/services/cases/index.test.ts +++ b/x-pack/plugins/cases/server/services/cases/index.test.ts @@ -657,9 +657,7 @@ describe('CasesService', () => { describe('post', () => { it('creates a null external_service field when the attribute was null in the creation parameters', async () => { - unsecuredSavedObjectsClient.create.mockResolvedValue( - {} as SavedObject - ); + unsecuredSavedObjectsClient.create.mockResolvedValue(createCaseSavedObjectResponse()); await service.postNewCase({ attributes: createCasePostParams({ connector: createJiraConnector() }), @@ -672,9 +670,7 @@ describe('CasesService', () => { }); it('includes the creation attributes excluding the connector.id and connector_id', async () => { - unsecuredSavedObjectsClient.create.mockResolvedValue( - {} as SavedObject - ); + unsecuredSavedObjectsClient.create.mockResolvedValue(createCaseSavedObjectResponse()); await service.postNewCase({ attributes: createCasePostParams({ @@ -772,9 +768,7 @@ describe('CasesService', () => { }); it('includes default values for total_alerts and total_comments', async () => { - unsecuredSavedObjectsClient.create.mockResolvedValue( - {} as SavedObject - ); + unsecuredSavedObjectsClient.create.mockResolvedValue(createCaseSavedObjectResponse()); await service.postNewCase({ attributes: createCasePostParams({ @@ -791,9 +785,7 @@ describe('CasesService', () => { }); it('moves the connector.id and connector_id to the references', async () => { - unsecuredSavedObjectsClient.create.mockResolvedValue( - {} as SavedObject - ); + unsecuredSavedObjectsClient.create.mockResolvedValue(createCaseSavedObjectResponse()); await service.postNewCase({ attributes: createCasePostParams({ @@ -822,9 +814,7 @@ describe('CasesService', () => { }); it('sets fields to an empty array when it is not included with the connector', async () => { - unsecuredSavedObjectsClient.create.mockResolvedValue( - {} as SavedObject - ); + unsecuredSavedObjectsClient.create.mockResolvedValue(createCaseSavedObjectResponse()); await service.postNewCase({ attributes: createCasePostParams({ @@ -840,9 +830,7 @@ describe('CasesService', () => { }); it('does not create a reference for a none connector', async () => { - unsecuredSavedObjectsClient.create.mockResolvedValue( - {} as SavedObject - ); + unsecuredSavedObjectsClient.create.mockResolvedValue(createCaseSavedObjectResponse()); await service.postNewCase({ attributes: createCasePostParams({ connector: getNoneCaseConnector() }), @@ -855,9 +843,7 @@ describe('CasesService', () => { }); it('does not create a reference for an external_service field that is null', async () => { - unsecuredSavedObjectsClient.create.mockResolvedValue( - {} as SavedObject - ); + unsecuredSavedObjectsClient.create.mockResolvedValue(createCaseSavedObjectResponse()); await service.postNewCase({ attributes: createCasePostParams({ connector: getNoneCaseConnector() }), @@ -877,9 +863,7 @@ describe('CasesService', () => { ])( 'properly converts "%s" severity to corresponding ES value on creating SO', async (postParamsSeverity, expectedSeverity) => { - unsecuredSavedObjectsClient.create.mockResolvedValue( - {} as SavedObject - ); + unsecuredSavedObjectsClient.create.mockResolvedValue(createCaseSavedObjectResponse()); await service.postNewCase({ attributes: createCasePostParams({ @@ -902,9 +886,7 @@ describe('CasesService', () => { ])( 'properly converts "%s" status to corresponding ES value on creating SO', async (postParamsStatus, expectedStatus) => { - unsecuredSavedObjectsClient.create.mockResolvedValue( - {} as SavedObject - ); + unsecuredSavedObjectsClient.create.mockResolvedValue(createCaseSavedObjectResponse()); await service.postNewCase({ attributes: createCasePostParams({ @@ -1670,6 +1652,12 @@ describe('CasesService', () => { it('defaults to the none connector and null external_services when attributes is undefined', async () => { unsecuredSavedObjectsClient.get.mockResolvedValue({ + ...createCaseSavedObjectResponse(), + attributes: { + ...createCaseSavedObjectResponse().attributes, + external_service: undefined, + connector: undefined, + }, references: [ { id: '1', @@ -1694,8 +1682,9 @@ describe('CasesService', () => { it('returns a null external_services when it is already null', async () => { unsecuredSavedObjectsClient.get.mockResolvedValue({ - attributes: { external_service: null }, - } as SavedObject); + ...createCaseSavedObjectResponse(), + attributes: { ...createCaseSavedObjectResponse().attributes, external_service: null }, + }); const res = await service.getCase({ id: 'a' }); expect(res.attributes.connector).toMatchInlineSnapshot(` @@ -1719,7 +1708,11 @@ describe('CasesService', () => { 'includes the properly converted "%s" severity field in the result', async (internalSeverityValue, expectedSeverity) => { unsecuredSavedObjectsClient.get.mockResolvedValue({ - attributes: { severity: internalSeverityValue }, + ...createCaseSavedObjectResponse(), + attributes: { + ...createCaseSavedObjectResponse().attributes, + severity: internalSeverityValue, + }, } as SavedObject); const res = await service.getCase({ id: 'a' }); @@ -1736,7 +1729,11 @@ describe('CasesService', () => { 'includes the properly converted "%s" status field in the result', async (internalStatusValue, expectedStatus) => { unsecuredSavedObjectsClient.get.mockResolvedValue({ - attributes: { status: internalStatusValue }, + ...createCaseSavedObjectResponse(), + attributes: { + ...createCaseSavedObjectResponse().attributes, + status: internalStatusValue, + }, } as SavedObject); const res = await service.getCase({ id: 'a' }); @@ -1747,7 +1744,9 @@ describe('CasesService', () => { it('does not include total_alerts and total_comments fields in the response', async () => { unsecuredSavedObjectsClient.get.mockResolvedValue({ + ...createCaseSavedObjectResponse(), attributes: { + ...createCaseSavedObjectResponse().attributes, total_alerts: -1, total_comments: -1, }, @@ -1826,4 +1825,19 @@ describe('CasesService', () => { ); }); }); + + describe.only('Encoding responses', () => { + describe('post', () => { + it('encodes correctly', async () => { + unsecuredSavedObjectsClient.create.mockResolvedValue(createCaseSavedObjectResponse()); + + await expect( + service.postNewCase({ + attributes: createCasePostParams({ connector: createJiraConnector() }), + id: '1', + }) + ).resolves.not.toThrow(); + }); + }); + }); }); diff --git a/x-pack/plugins/cases/server/services/cases/index.ts b/x-pack/plugins/cases/server/services/cases/index.ts index 42fd0f03a25e2..0ec8bebd6a200 100644 --- a/x-pack/plugins/cases/server/services/cases/index.ts +++ b/x-pack/plugins/cases/server/services/cases/index.ts @@ -50,7 +50,11 @@ import type { CaseSavedObjectTransformed, CaseTransformedAttributes, } from '../../common/types/case'; -import { CasePersistedStatus } from '../../common/types/case'; +import { + CaseTransformedAttributesRt, + CasePersistedStatus, + PartialCaseTransformedAttributesRt, +} from '../../common/types/case'; import type { GetCaseIdsByAlertIdArgs, GetCaseIdsByAlertIdAggs, @@ -67,6 +71,7 @@ import type { PatchCasesArgs, } from './types'; import type { AttachmentTransformedAttributes } from '../../common/types/attachments'; +import { bulkEncodeSOAttributes } from '../utils'; export class CasesService { private readonly log: Logger; @@ -239,7 +244,7 @@ export class CasesService { public async deleteCase({ id: caseId, refresh }: DeleteCaseArgs) { try { this.log.debug(`Attempting to DELETE case ${caseId}`); - return await this.unsecuredSavedObjectsClient.delete(CASE_SAVED_OBJECT, caseId, { refresh }); + await this.unsecuredSavedObjectsClient.delete(CASE_SAVED_OBJECT, caseId, { refresh }); } catch (error) { this.log.error(`Error on DELETE case ${caseId}: ${error}`); throw error; @@ -255,7 +260,7 @@ export class CasesService { }) { try { this.log.debug(`Attempting to bulk delete case entities ${JSON.stringify(entities)}`); - return await this.unsecuredSavedObjectsClient.bulkDelete(entities, options); + await this.unsecuredSavedObjectsClient.bulkDelete(entities, options); } catch (error) { this.log.error(`Error bulk deleting case entities ${JSON.stringify(entities)}: ${error}`); } @@ -268,7 +273,11 @@ export class CasesService { CASE_SAVED_OBJECT, caseId ); - return transformSavedObjectToExternalModel(caseSavedObject); + + const res = transformSavedObjectToExternalModel(caseSavedObject); + CaseTransformedAttributesRt.encode(res.attributes); + + return res; } catch (error) { this.log.error(`Error on GET case ${caseId}: ${error}`); throw error; @@ -285,9 +294,13 @@ export class CasesService { CASE_SAVED_OBJECT, caseId ); + + const resolvedSO = transformSavedObjectToExternalModel(resolveCaseResult.saved_object); + CaseTransformedAttributesRt.encode(resolvedSO.attributes); + return { ...resolveCaseResult, - saved_object: transformSavedObjectToExternalModel(resolveCaseResult.saved_object), + saved_object: resolvedSO, }; } catch (error) { this.log.error(`Error on resolve case ${caseId}: ${error}`); @@ -304,7 +317,10 @@ export class CasesService { const cases = await this.unsecuredSavedObjectsClient.bulkGet( caseIds.map((caseId) => ({ type: CASE_SAVED_OBJECT, id: caseId, fields })) ); - return transformBulkResponseToExternalModel(cases); + const res = transformBulkResponseToExternalModel(cases); + bulkEncodeSOAttributes(res.saved_objects, CaseTransformedAttributesRt); + + return res; } catch (error) { this.log.error(`Error on GET cases ${caseIds.join(', ')}: ${error}`); throw error; @@ -322,7 +338,10 @@ export class CasesService { type: CASE_SAVED_OBJECT, }); - return transformFindResponseToExternalModel(cases); + const res = transformFindResponseToExternalModel(cases); + bulkEncodeSOAttributes(res.saved_objects, CaseTransformedAttributesRt); + + return res; } catch (error) { this.log.error(`Error on find cases: ${error}`); throw error; @@ -517,7 +536,10 @@ export class CasesService { { id, references: transformedAttributes.referenceHandler.build(), refresh } ); - return transformSavedObjectToExternalModel(createdCase); + const res = transformSavedObjectToExternalModel(createdCase); + CaseTransformedAttributesRt.encode(res.attributes); + + return res; } catch (error) { this.log.error(`Error on POST a new case: ${error}`); throw error; @@ -546,7 +568,10 @@ export class CasesService { } ); - return transformUpdateResponseToExternalModel(updatedCase); + const res = transformUpdateResponseToExternalModel(updatedCase); + PartialCaseTransformedAttributesRt.encode(res.attributes); + + return res; } catch (error) { this.log.error(`Error on UPDATE case ${caseId}: ${error}`); throw error; @@ -576,7 +601,10 @@ export class CasesService { refresh, }); - return transformUpdateResponsesToExternalModels(updatedCases); + const res = transformUpdateResponsesToExternalModels(updatedCases); + bulkEncodeSOAttributes(res.saved_objects, PartialCaseTransformedAttributesRt); + + return res; } catch (error) { this.log.error(`Error on UPDATE case ${cases.map((c) => c.caseId).join(', ')}: ${error}`); throw error; diff --git a/x-pack/plugins/cases/server/services/utils.ts b/x-pack/plugins/cases/server/services/utils.ts new file mode 100644 index 0000000000000..2bf2b025363d7 --- /dev/null +++ b/x-pack/plugins/cases/server/services/utils.ts @@ -0,0 +1,17 @@ +/* + * 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 type { Type } from 'io-ts'; + +export const bulkEncodeSOAttributes = ( + savedObjects: Array<{ attributes: T }>, + type: Type +) => { + for (const so of savedObjects) { + type.encode(so.attributes); + } +}; From 1f7bf4c0bddf9e4edb2a06752d8f89dc553eeb23 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Tue, 16 May 2023 13:39:10 +0300 Subject: [PATCH 02/13] Decode when returning responses fromt the cases service --- .../plugins/cases/server/common/types/case.ts | 17 +- .../cases/server/services/cases/index.test.ts | 378 +++++++++++++++++- .../cases/server/services/cases/index.ts | 59 ++- x-pack/plugins/cases/server/services/utils.ts | 11 +- 4 files changed, 420 insertions(+), 45 deletions(-) diff --git a/x-pack/plugins/cases/server/common/types/case.ts b/x-pack/plugins/cases/server/common/types/case.ts index 6b1d5f9348c91..b620e6c3d8b4b 100644 --- a/x-pack/plugins/cases/server/common/types/case.ts +++ b/x-pack/plugins/cases/server/common/types/case.ts @@ -6,6 +6,7 @@ */ import type { SavedObject } from '@kbn/core-saved-objects-server'; +import type { Type } from 'io-ts'; import { exact, partial } from 'io-ts'; import type { CaseAttributes } from '../../../common/api'; import { CaseAttributesRt } from '../../../common/api'; @@ -48,16 +49,16 @@ export interface CasePersistedAttributes { updated_by: User | null; } -const caseTransformedAttributesProps = CaseAttributesRt.types.reduce( - (acc, type) => ({ ...acc, ...type.props }), - {} -); - export type CaseTransformedAttributes = CaseAttributes; export const CaseTransformedAttributesRt = CaseAttributesRt; -export const PartialCaseTransformedAttributesRt = exact( - partial({ ...caseTransformedAttributesProps }) -); + +export const getPartialCaseTransformedAttributesRt = (): Type> => { + const caseTransformedAttributesProps = CaseAttributesRt.types.reduce( + (acc, type) => ({ ...acc, ...type.props }), + {} + ); + return exact(partial({ ...caseTransformedAttributesProps })); +}; export type CaseSavedObject = SavedObject; export type CaseSavedObjectTransformed = SavedObject; diff --git a/x-pack/plugins/cases/server/services/cases/index.test.ts b/x-pack/plugins/cases/server/services/cases/index.test.ts index 4258e425b02c8..7691c5b13a6b2 100644 --- a/x-pack/plugins/cases/server/services/cases/index.test.ts +++ b/x-pack/plugins/cases/server/services/cases/index.test.ts @@ -13,6 +13,7 @@ * connector.id. */ +import { omit } from 'lodash'; import type { CaseAttributes, CaseConnector, CaseFullExternalService } from '../../../common/api'; import { CaseSeverity, CaseStatuses } from '../../../common/api'; import { CASE_SAVED_OBJECT } from '../../../common/constants'; @@ -44,7 +45,11 @@ import { import { AttachmentService } from '../attachments'; import { PersistableStateAttachmentTypeRegistry } from '../../attachment_framework/persistable_state_registry'; import type { CaseSavedObjectTransformed, CasePersistedAttributes } from '../../common/types/case'; -import { CasePersistedSeverity, CasePersistedStatus } from '../../common/types/case'; +import { + CasePersistedSeverity, + CasePersistedStatus, + CaseTransformedAttributesRt, +} from '../../common/types/case'; const createUpdateSOResponse = ({ connector, @@ -95,6 +100,7 @@ const createFindSO = ( connector?: ESCaseConnectorWithId; externalService?: CaseFullExternalService; overrides?: Partial; + caseId?: string; } = {} ): SavedObjectsFindResult => ({ ...createCaseSavedObjectResponse(params), @@ -910,10 +916,12 @@ describe('CasesService', () => { unsecuredSavedObjectsClient.bulkUpdate.mockResolvedValue({ saved_objects: [ createCaseSavedObjectResponse({ + caseId: '1', connector: createESJiraConnector(), externalService: createExternalService(), }), createCaseSavedObjectResponse({ + caseId: '2', connector: createESJiraConnector({ id: '2' }), externalService: createExternalService({ connector_id: '200' }), }), @@ -969,14 +977,20 @@ describe('CasesService', () => { it('properly converts the severity field to the corresponding external value in the bulkPatch response', async () => { unsecuredSavedObjectsClient.bulkUpdate.mockResolvedValue({ saved_objects: [ - createCaseSavedObjectResponse({ overrides: { severity: CasePersistedSeverity.LOW } }), createCaseSavedObjectResponse({ + caseId: '1', + overrides: { severity: CasePersistedSeverity.LOW }, + }), + createCaseSavedObjectResponse({ + caseId: '2', overrides: { severity: CasePersistedSeverity.MEDIUM }, }), createCaseSavedObjectResponse({ + caseId: '3', overrides: { severity: CasePersistedSeverity.HIGH }, }), createCaseSavedObjectResponse({ + caseId: '4', overrides: { severity: CasePersistedSeverity.CRITICAL }, }), ], @@ -1000,11 +1014,18 @@ describe('CasesService', () => { it('properly converts the status field to the corresponding external value in the bulkPatch response', async () => { unsecuredSavedObjectsClient.bulkUpdate.mockResolvedValue({ saved_objects: [ - createCaseSavedObjectResponse({ overrides: { status: CasePersistedStatus.OPEN } }), createCaseSavedObjectResponse({ + caseId: '1', + overrides: { status: CasePersistedStatus.OPEN }, + }), + createCaseSavedObjectResponse({ + caseId: '2', overrides: { status: CasePersistedStatus.IN_PROGRESS }, }), - createCaseSavedObjectResponse({ overrides: { status: CasePersistedStatus.CLOSED } }), + createCaseSavedObjectResponse({ + caseId: '3', + overrides: { status: CasePersistedStatus.CLOSED }, + }), ], }); @@ -1025,9 +1046,9 @@ describe('CasesService', () => { it('does not include total_alerts and total_comments fields in the response', async () => { unsecuredSavedObjectsClient.bulkUpdate.mockResolvedValue({ saved_objects: [ - createCaseSavedObjectResponse({}), - createCaseSavedObjectResponse({}), - createCaseSavedObjectResponse({}), + createCaseSavedObjectResponse({ caseId: '1' }), + createCaseSavedObjectResponse({ caseId: '2' }), + createCaseSavedObjectResponse({ caseId: '3' }), ], }); @@ -1415,10 +1436,13 @@ describe('CasesService', () => { it('includes the connector.id and connector_id field in the response', async () => { const findMockReturn = createSOFindResponse([ createFindSO({ + caseId: '1', connector: createESJiraConnector(), externalService: createExternalService(), }), - createFindSO(), + createFindSO({ + caseId: '2', + }), ]); unsecuredSavedObjectsClient.find.mockResolvedValue(findMockReturn); @@ -1432,10 +1456,11 @@ describe('CasesService', () => { it('includes the saved object find response fields in the result', async () => { const findMockReturn = createSOFindResponse([ createFindSO({ + caseId: '1', connector: createESJiraConnector(), externalService: createExternalService(), }), - createFindSO(), + createFindSO({ caseId: '2' }), ]); unsecuredSavedObjectsClient.find.mockResolvedValue(findMockReturn); @@ -1459,8 +1484,8 @@ describe('CasesService', () => { 'includes the properly converted "%s" severity field in the result', async (severity, expectedSeverity) => { const findMockReturn = createSOFindResponse([ - createFindSO({ overrides: { severity } }), - createFindSO(), + createFindSO({ caseId: '1', overrides: { severity } }), + createFindSO({ caseId: '2' }), ]); unsecuredSavedObjectsClient.find.mockResolvedValue(findMockReturn); @@ -1477,8 +1502,8 @@ describe('CasesService', () => { 'includes the properly converted "%s" status field in the result', async (status, expectedStatus) => { const findMockReturn = createSOFindResponse([ - createFindSO({ overrides: { status } }), - createFindSO(), + createFindSO({ caseId: '1', overrides: { status } }), + createFindSO({ caseId: '2' }), ]); unsecuredSavedObjectsClient.find.mockResolvedValue(findMockReturn); @@ -1488,7 +1513,10 @@ describe('CasesService', () => { ); it('does not include total_alerts and total_comments fields in the response', async () => { - const findMockReturn = createSOFindResponse([createFindSO(), createFindSO()]); + const findMockReturn = createSOFindResponse([ + createFindSO({ caseId: '1' }), + createFindSO({ caseId: '2' }), + ]); unsecuredSavedObjectsClient.find.mockResolvedValue(findMockReturn); const res = await service.findCases(); @@ -1503,10 +1531,12 @@ describe('CasesService', () => { unsecuredSavedObjectsClient.bulkGet.mockResolvedValue({ saved_objects: [ createCaseSavedObjectResponse({ + caseId: '1', connector: createESJiraConnector(), externalService: createExternalService(), }), createCaseSavedObjectResponse({ + caseId: '2', connector: createESJiraConnector({ id: '2' }), externalService: createExternalService({ connector_id: '200' }), }), @@ -1530,15 +1560,19 @@ describe('CasesService', () => { unsecuredSavedObjectsClient.bulkGet.mockResolvedValue({ saved_objects: [ createCaseSavedObjectResponse({ + caseId: '1', overrides: { severity: CasePersistedSeverity.LOW }, }), createCaseSavedObjectResponse({ + caseId: '2', overrides: { severity: CasePersistedSeverity.MEDIUM }, }), createCaseSavedObjectResponse({ + caseId: '3', overrides: { severity: CasePersistedSeverity.HIGH }, }), createCaseSavedObjectResponse({ + caseId: '4', overrides: { severity: CasePersistedSeverity.CRITICAL }, }), ], @@ -1555,12 +1589,15 @@ describe('CasesService', () => { unsecuredSavedObjectsClient.bulkGet.mockResolvedValue({ saved_objects: [ createCaseSavedObjectResponse({ + caseId: '1', overrides: { status: CasePersistedStatus.OPEN }, }), createCaseSavedObjectResponse({ + caseId: '2', overrides: { status: CasePersistedStatus.IN_PROGRESS }, }), createCaseSavedObjectResponse({ + caseId: '3', overrides: { status: CasePersistedStatus.CLOSED }, }), ], @@ -1575,9 +1612,9 @@ describe('CasesService', () => { it('does not include total_alerts and total_comments fields in the response', async () => { unsecuredSavedObjectsClient.bulkGet.mockResolvedValue({ saved_objects: [ - createCaseSavedObjectResponse({}), - createCaseSavedObjectResponse({}), - createCaseSavedObjectResponse({}), + createCaseSavedObjectResponse({ caseId: '1' }), + createCaseSavedObjectResponse({ caseId: '2' }), + createCaseSavedObjectResponse({ caseId: '3' }), ], }); @@ -1826,7 +1863,178 @@ describe('CasesService', () => { }); }); - describe.only('Encoding responses', () => { + describe('Encoding responses', () => { + const caseTransformedAttributesProps = CaseTransformedAttributesRt.types.reduce( + (acc, type) => ({ ...acc, ...type.props }), + {} + ); + + /** + * Status, severity, connector, and external_service + * are being set to a default value if missing. + * Decode will not throw an error as they are defined. + */ + const attributesToValidateIfMissing = omit( + caseTransformedAttributesProps, + 'status', + 'severity', + 'connector', + 'external_service' + ); + + describe('getCase', () => { + it('encodes correctly', async () => { + unsecuredSavedObjectsClient.get.mockResolvedValue(createCaseSavedObjectResponse()); + + await expect(service.getCase({ id: 'a' })).resolves.not.toThrow(); + }); + + it.each(Object.keys(attributesToValidateIfMissing))( + 'throws if %s is omitted', + async (key) => { + const theCase = createCaseSavedObjectResponse(); + const attributes = omit({ ...theCase.attributes }, key); + unsecuredSavedObjectsClient.get.mockResolvedValue({ ...theCase, attributes }); + + await expect(service.getCase({ id: 'a' })).rejects.toThrow( + `Invalid value "undefined" supplied to "${key}"` + ); + } + ); + + // TODO: Unskip when all types are converted to strict + it.skip('strips out excess attributes', async () => { + const theCase = createCaseSavedObjectResponse(); + const attributes = { ...theCase.attributes, 'not-exists': 'not-exists' }; + unsecuredSavedObjectsClient.get.mockResolvedValue({ ...theCase, attributes }); + + await expect(service.getCase({ id: 'a' })).resolves.toEqual({ attributes }); + }); + }); + + describe('getResolveCase', () => { + it('encodes correctly', async () => { + unsecuredSavedObjectsClient.resolve.mockResolvedValue({ + saved_object: createCaseSavedObjectResponse(), + outcome: 'exactMatch', + }); + + await expect(service.getResolveCase({ id: 'a' })).resolves.not.toThrow(); + }); + + it.each(Object.keys(attributesToValidateIfMissing))( + 'throws if %s is omitted', + async (key) => { + const theCase = createCaseSavedObjectResponse(); + const attributes = omit({ ...theCase.attributes }, key); + unsecuredSavedObjectsClient.resolve.mockResolvedValue({ + saved_object: { ...theCase, attributes }, + outcome: 'exactMatch', + }); + + await expect(service.getResolveCase({ id: 'a' })).rejects.toThrow( + `Invalid value "undefined" supplied to "${key}"` + ); + } + ); + + // TODO: Unskip when all types are converted to strict + it.skip('strips out excess attributes', async () => { + const theCase = createCaseSavedObjectResponse(); + const attributes = { ...theCase.attributes, 'not-exists': 'not-exists' }; + unsecuredSavedObjectsClient.resolve.mockResolvedValue({ + saved_object: { ...theCase, attributes }, + outcome: 'exactMatch', + }); + + await expect(service.getResolveCase({ id: 'a' })).resolves.toEqual({ attributes }); + }); + }); + + describe('getCases', () => { + it('encodes correctly', async () => { + unsecuredSavedObjectsClient.bulkGet.mockResolvedValue({ + saved_objects: [ + createCaseSavedObjectResponse({ caseId: '1' }), + createCaseSavedObjectResponse({ caseId: '2' }), + ], + }); + + await expect(service.getCases({ caseIds: ['a', 'b'] })).resolves.not.toThrow(); + }); + + it.each(Object.keys(attributesToValidateIfMissing))( + 'throws if %s is omitted', + async (key) => { + const theCase = createCaseSavedObjectResponse(); + const attributes = omit({ ...theCase.attributes }, key); + unsecuredSavedObjectsClient.bulkGet.mockResolvedValue({ + saved_objects: [{ ...theCase, attributes }, createCaseSavedObjectResponse()], + }); + + await expect(service.getCases({ caseIds: ['a', 'b'] })).rejects.toThrow( + `Invalid value "undefined" supplied to "${key}"` + ); + } + ); + + // TODO: Unskip when all types are converted to strict + it.skip('strips out excess attributes', async () => { + const theCase = createCaseSavedObjectResponse(); + const attributes = { ...theCase.attributes, 'not-exists': 'not-exists' }; + unsecuredSavedObjectsClient.bulkGet.mockResolvedValue({ + saved_objects: [{ ...theCase, attributes }], + }); + + await expect(service.getCases({ caseIds: ['a', 'b'] })).resolves.toEqual({ attributes }); + }); + }); + + describe('findCases', () => { + it('encodes correctly', async () => { + const findMockReturn = createSOFindResponse([ + createFindSO({ caseId: '1' }), + createFindSO({ caseId: '2' }), + ]); + + unsecuredSavedObjectsClient.find.mockResolvedValue(findMockReturn); + + await expect(service.findCases()).resolves.not.toThrow(); + }); + + it.each(Object.keys(attributesToValidateIfMissing))( + 'throws if %s is omitted', + async (key) => { + const theCase = createCaseSavedObjectResponse(); + const attributes = omit({ ...theCase.attributes }, key); + const findMockReturn = createSOFindResponse([ + { ...theCase, attributes, score: 0 }, + createFindSO(), + ]); + + unsecuredSavedObjectsClient.find.mockResolvedValue(findMockReturn); + + await expect(service.findCases()).rejects.toThrow( + `Invalid value "undefined" supplied to "${key}"` + ); + } + ); + + // TODO: Unskip when all types are converted to strict + it.skip('strips out excess attributes', async () => { + const theCase = createCaseSavedObjectResponse(); + const attributes = { ...theCase.attributes, 'not-exists': 'not-exists' }; + const findMockReturn = createSOFindResponse([ + { ...theCase, attributes, score: 0 }, + createFindSO(), + ]); + + unsecuredSavedObjectsClient.find.mockResolvedValue(findMockReturn); + + await expect(service.findCases()).resolves.toEqual({ attributes }); + }); + }); + describe('post', () => { it('encodes correctly', async () => { unsecuredSavedObjectsClient.create.mockResolvedValue(createCaseSavedObjectResponse()); @@ -1838,6 +2046,140 @@ describe('CasesService', () => { }) ).resolves.not.toThrow(); }); + + it.each(Object.keys(attributesToValidateIfMissing))( + 'throws if %s is omitted', + async (key) => { + const theCase = createCaseSavedObjectResponse(); + const attributes = omit({ ...theCase.attributes }, key); + unsecuredSavedObjectsClient.create.mockResolvedValue({ ...theCase, attributes }); + + await expect( + service.postNewCase({ + attributes: createCasePostParams({ connector: createJiraConnector() }), + id: '1', + }) + ).rejects.toThrow(`Invalid value "undefined" supplied to "${key}"`); + } + ); + + // TODO: Unskip when all types are converted to strict + it.skip('strips out excess attributes', async () => { + const theCase = createCaseSavedObjectResponse(); + const attributes = { ...theCase.attributes, 'not-exists': 'not-exists' }; + unsecuredSavedObjectsClient.create.mockResolvedValue({ ...theCase, attributes }); + + await expect( + service.postNewCase({ + attributes: createCasePostParams({ connector: createJiraConnector() }), + id: '1', + }) + ).resolves.toEqual({ attributes }); + }); + }); + + describe('patchCase', () => { + it('encodes correctly', async () => { + unsecuredSavedObjectsClient.update.mockResolvedValue(createUpdateSOResponse()); + + await expect( + service.patchCase({ + caseId: '1', + updatedAttributes: createCaseUpdateParams({}), + originalCase: {} as CaseSavedObjectTransformed, + }) + ).resolves.not.toThrow(); + }); + + it('strips out excess attributes', async () => { + const theCase = createUpdateSOResponse(); + const attributes = { ...theCase.attributes, 'not-exists': 'not-exists' }; + unsecuredSavedObjectsClient.update.mockResolvedValue({ ...theCase, attributes }); + + await expect( + service.patchCase({ + caseId: '1', + updatedAttributes: {}, + originalCase: {} as CaseSavedObjectTransformed, + }) + ).resolves.toEqual({ + attributes: {}, + id: '1', + references: [], + type: 'cases', + }); + }); + }); + + describe('patchCases', () => { + it('encodes correctly', async () => { + unsecuredSavedObjectsClient.bulkUpdate.mockResolvedValue({ + saved_objects: [ + createCaseSavedObjectResponse({ caseId: '1' }), + createCaseSavedObjectResponse({ caseId: '2' }), + ], + }); + + await expect( + service.patchCases({ + cases: [ + { + caseId: '1', + updatedAttributes: createCasePostParams({ + connector: getNoneCaseConnector(), + }), + originalCase: {} as CaseSavedObjectTransformed, + }, + ], + }) + ).resolves.not.toThrow(); + }); + + it('strips out excess attributes', async () => { + const theCase = createCaseSavedObjectResponse(); + unsecuredSavedObjectsClient.bulkUpdate.mockResolvedValue({ + saved_objects: [ + { ...theCase, attributes: { description: 'update desc', 'not-exists': 'not-exists' } }, + { ...theCase, attributes: { title: 'update title' } }, + ], + }); + + await expect( + service.patchCases({ + cases: [ + { + caseId: '1', + updatedAttributes: { description: 'update desc' }, + originalCase: {} as CaseSavedObjectTransformed, + }, + { + caseId: '2', + updatedAttributes: { title: 'update title' }, + originalCase: {} as CaseSavedObjectTransformed, + }, + ], + }) + ).resolves.toEqual({ + saved_objects: [ + { + attributes: { + title: 'update title', + }, + id: '1', + references: [], + type: 'cases', + }, + { + attributes: { + title: 'update title', + }, + id: '1', + references: [], + type: 'cases', + }, + ], + }); + }); }); }); }); diff --git a/x-pack/plugins/cases/server/services/cases/index.ts b/x-pack/plugins/cases/server/services/cases/index.ts index 0ec8bebd6a200..95fa7af9106fa 100644 --- a/x-pack/plugins/cases/server/services/cases/index.ts +++ b/x-pack/plugins/cases/server/services/cases/index.ts @@ -28,7 +28,7 @@ import { MAX_DOCS_PER_PAGE, } from '../../../common/constants'; import type { Case, User, CaseStatuses } from '../../../common/api'; -import { caseStatuses } from '../../../common/api'; +import { decodeOrThrow, caseStatuses } from '../../../common/api'; import type { SavedObjectFindOptionsKueryNode } from '../../common/types'; import { defaultSortField, flattenCaseSavedObject } from '../../common/utils'; import { DEFAULT_PAGE, DEFAULT_PER_PAGE } from '../../routes/api'; @@ -53,7 +53,7 @@ import type { import { CaseTransformedAttributesRt, CasePersistedStatus, - PartialCaseTransformedAttributesRt, + getPartialCaseTransformedAttributesRt, } from '../../common/types/case'; import type { GetCaseIdsByAlertIdArgs, @@ -71,7 +71,7 @@ import type { PatchCasesArgs, } from './types'; import type { AttachmentTransformedAttributes } from '../../common/types/attachments'; -import { bulkEncodeSOAttributes } from '../utils'; +import { bulkDecodeSOAttributes } from '../utils'; export class CasesService { private readonly log: Logger; @@ -275,9 +275,12 @@ export class CasesService { ); const res = transformSavedObjectToExternalModel(caseSavedObject); - CaseTransformedAttributesRt.encode(res.attributes); + const decodeRes = decodeOrThrow(CaseTransformedAttributesRt)(res.attributes); - return res; + return { + ...res, + attributes: decodeRes, + }; } catch (error) { this.log.error(`Error on GET case ${caseId}: ${error}`); throw error; @@ -296,11 +299,11 @@ export class CasesService { ); const resolvedSO = transformSavedObjectToExternalModel(resolveCaseResult.saved_object); - CaseTransformedAttributesRt.encode(resolvedSO.attributes); + const decodeRes = decodeOrThrow(CaseTransformedAttributesRt)(resolvedSO.attributes); return { ...resolveCaseResult, - saved_object: resolvedSO, + saved_object: { ...resolvedSO, attributes: decodeRes }, }; } catch (error) { this.log.error(`Error on resolve case ${caseId}: ${error}`); @@ -318,9 +321,15 @@ export class CasesService { caseIds.map((caseId) => ({ type: CASE_SAVED_OBJECT, id: caseId, fields })) ); const res = transformBulkResponseToExternalModel(cases); - bulkEncodeSOAttributes(res.saved_objects, CaseTransformedAttributesRt); + const decodeRes = bulkDecodeSOAttributes(res.saved_objects, CaseTransformedAttributesRt); - return res; + return { + ...res, + saved_objects: res.saved_objects.map((so) => ({ + ...so, + attributes: decodeRes.get(so.id) as CaseTransformedAttributes, + })), + }; } catch (error) { this.log.error(`Error on GET cases ${caseIds.join(', ')}: ${error}`); throw error; @@ -339,9 +348,15 @@ export class CasesService { }); const res = transformFindResponseToExternalModel(cases); - bulkEncodeSOAttributes(res.saved_objects, CaseTransformedAttributesRt); + const decodeRes = bulkDecodeSOAttributes(res.saved_objects, CaseTransformedAttributesRt); - return res; + return { + ...res, + saved_objects: res.saved_objects.map((so) => ({ + ...so, + attributes: decodeRes.get(so.id) as CaseTransformedAttributes, + })), + }; } catch (error) { this.log.error(`Error on find cases: ${error}`); throw error; @@ -537,7 +552,7 @@ export class CasesService { ); const res = transformSavedObjectToExternalModel(createdCase); - CaseTransformedAttributesRt.encode(res.attributes); + decodeOrThrow(CaseTransformedAttributesRt)(res.attributes); return res; } catch (error) { @@ -569,9 +584,12 @@ export class CasesService { ); const res = transformUpdateResponseToExternalModel(updatedCase); - PartialCaseTransformedAttributesRt.encode(res.attributes); + const decodeRes = decodeOrThrow(getPartialCaseTransformedAttributesRt())(res.attributes); - return res; + return { + ...res, + attributes: decodeRes, + }; } catch (error) { this.log.error(`Error on UPDATE case ${caseId}: ${error}`); throw error; @@ -602,9 +620,18 @@ export class CasesService { }); const res = transformUpdateResponsesToExternalModels(updatedCases); - bulkEncodeSOAttributes(res.saved_objects, PartialCaseTransformedAttributesRt); + const decodeRes = bulkDecodeSOAttributes( + res.saved_objects, + getPartialCaseTransformedAttributesRt() + ); - return res; + return { + ...res, + saved_objects: res.saved_objects.map((so) => ({ + ...so, + attributes: decodeRes.get(so.id) as CaseTransformedAttributes, + })), + }; } catch (error) { this.log.error(`Error on UPDATE case ${cases.map((c) => c.caseId).join(', ')}: ${error}`); throw error; diff --git a/x-pack/plugins/cases/server/services/utils.ts b/x-pack/plugins/cases/server/services/utils.ts index 2bf2b025363d7..b5b8cac8ac2bc 100644 --- a/x-pack/plugins/cases/server/services/utils.ts +++ b/x-pack/plugins/cases/server/services/utils.ts @@ -6,12 +6,17 @@ */ import type { Type } from 'io-ts'; +import { decodeOrThrow } from '../../common/api'; -export const bulkEncodeSOAttributes = ( - savedObjects: Array<{ attributes: T }>, +export const bulkDecodeSOAttributes = ( + savedObjects: Array<{ id: string; attributes: T }>, type: Type ) => { + const decodeRes = new Map(); + for (const so of savedObjects) { - type.encode(so.attributes); + decodeRes.set(so.id, decodeOrThrow(type)(so.attributes)); } + + return decodeRes; }; From cf4734393f0a9099b849e49d75a509411ec1d6ff Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Tue, 16 May 2023 20:42:36 +0300 Subject: [PATCH 03/13] Fix bug where decoding error responses --- .../cases/server/client/cases/bulk_get.ts | 5 +- x-pack/plugins/cases/server/client/types.ts | 5 +- x-pack/plugins/cases/server/common/types.ts | 3 + x-pack/plugins/cases/server/common/utils.ts | 4 +- .../cases/server/services/cases/index.test.ts | 12 ++- .../cases/server/services/cases/index.ts | 75 ++++++++++++------- .../cases/server/services/cases/transform.ts | 37 --------- 7 files changed, 68 insertions(+), 73 deletions(-) diff --git a/x-pack/plugins/cases/server/client/cases/bulk_get.ts b/x-pack/plugins/cases/server/client/cases/bulk_get.ts index aeccd95ed3bec..29fe604c06c4e 100644 --- a/x-pack/plugins/cases/server/client/cases/bulk_get.ts +++ b/x-pack/plugins/cases/server/client/cases/bulk_get.ts @@ -26,11 +26,12 @@ import { } from '../../../common/api'; import { createCaseError } from '../../common/error'; import { flattenCaseSavedObject } from '../../common/utils'; -import type { CasesClientArgs, SOWithErrors } from '../types'; +import type { CasesClientArgs } from '../types'; import { Operations } from '../../authorization'; import type { CaseSavedObjectTransformed } from '../../common/types/case'; +import type { SOWithErrors } from '../../common/types'; -type CaseSavedObjectWithErrors = SOWithErrors; +type CaseSavedObjectWithErrors = Array>; type BulkGetCase = CasesBulkGetResponse['cases'][number]; /** diff --git a/x-pack/plugins/cases/server/client/types.ts b/x-pack/plugins/cases/server/client/types.ts index 9519d3cb64522..97811f6593bea 100644 --- a/x-pack/plugins/cases/server/client/types.ts +++ b/x-pack/plugins/cases/server/client/types.ts @@ -6,14 +6,13 @@ */ import type { PublicMethodsOf } from '@kbn/utility-types'; -import type { SavedObjectsClientContract, Logger, SavedObject } from '@kbn/core/server'; +import type { SavedObjectsClientContract, Logger } from '@kbn/core/server'; import type { ActionsClient } from '@kbn/actions-plugin/server'; import type { LensServerPluginSetup } from '@kbn/lens-plugin/server'; import type { SecurityPluginStart } from '@kbn/security-plugin/server'; import type { IBasePath } from '@kbn/core-http-browser'; import type { ISavedObjectsSerializer } from '@kbn/core-saved-objects-server'; import type { KueryNode } from '@kbn/es-query'; -import type { SavedObjectError } from '@kbn/core-saved-objects-common'; import type { FileServiceStart } from '@kbn/files-plugin/server'; import type { CasesFindRequest, User } from '../../common/api'; import type { Authorization } from '../authorization/authorization'; @@ -67,5 +66,3 @@ export type CasesFindQueryParams = Partial< 'tags' | 'reporters' | 'status' | 'severity' | 'owner' | 'from' | 'to' | 'assignees' > & { sortByField?: string; authorizationFilter?: KueryNode } >; - -export type SOWithErrors = Array & { error: SavedObjectError }>; diff --git a/x-pack/plugins/cases/server/common/types.ts b/x-pack/plugins/cases/server/common/types.ts index a622126f0b305..c949a8fdfce97 100644 --- a/x-pack/plugins/cases/server/common/types.ts +++ b/x-pack/plugins/cases/server/common/types.ts @@ -7,6 +7,7 @@ import type { SavedObjectsFindOptions } from '@kbn/core-saved-objects-api-server'; import type { SavedObject } from '@kbn/core-saved-objects-server'; +import type { SavedObjectError } from '@kbn/core/types'; import type { KueryNode } from '@kbn/es-query'; import type { CommentAttributes, @@ -49,3 +50,5 @@ export type FileAttachmentRequest = Omit< }; export type AttachmentSavedObject = SavedObject; + +export type SOWithErrors = Omit, 'attributes'> & { error: SavedObjectError }; diff --git a/x-pack/plugins/cases/server/common/utils.ts b/x-pack/plugins/cases/server/common/utils.ts index 75ca980af639b..8ccb18760b348 100644 --- a/x-pack/plugins/cases/server/common/utils.ts +++ b/x-pack/plugins/cases/server/common/utils.ts @@ -24,7 +24,7 @@ import { OWNER_INFO, } from '../../common/constants'; import type { CASE_VIEW_PAGE_TABS } from '../../common/types'; -import type { AlertInfo, FileAttachmentRequest } from './types'; +import type { AlertInfo, FileAttachmentRequest, SOWithErrors } from './types'; import type { CasePostRequest, @@ -456,3 +456,5 @@ export const getCaseViewPath = (params: { return `${basePath}${normalizePath(CASE_VIEW_PATH.replace(':detailName', caseId))}`; }; + +export const isSOError = (so: { error?: unknown }): so is SOWithErrors => so.error != null; diff --git a/x-pack/plugins/cases/server/services/cases/index.test.ts b/x-pack/plugins/cases/server/services/cases/index.test.ts index 7691c5b13a6b2..c0d02b90bd3c6 100644 --- a/x-pack/plugins/cases/server/services/cases/index.test.ts +++ b/x-pack/plugins/cases/server/services/cases/index.test.ts @@ -2139,8 +2139,12 @@ describe('CasesService', () => { const theCase = createCaseSavedObjectResponse(); unsecuredSavedObjectsClient.bulkUpdate.mockResolvedValue({ saved_objects: [ - { ...theCase, attributes: { description: 'update desc', 'not-exists': 'not-exists' } }, - { ...theCase, attributes: { title: 'update title' } }, + { + ...theCase, + id: '1', + attributes: { description: 'update desc', 'not-exists': 'not-exists' }, + }, + { ...theCase, id: '2', attributes: { title: 'update title' } }, ], }); @@ -2163,7 +2167,7 @@ describe('CasesService', () => { saved_objects: [ { attributes: { - title: 'update title', + description: 'update desc', }, id: '1', references: [], @@ -2173,7 +2177,7 @@ describe('CasesService', () => { attributes: { title: 'update title', }, - id: '1', + id: '2', references: [], type: 'cases', }, diff --git a/x-pack/plugins/cases/server/services/cases/index.ts b/x-pack/plugins/cases/server/services/cases/index.ts index 95fa7af9106fa..4cc9a90dece09 100644 --- a/x-pack/plugins/cases/server/services/cases/index.ts +++ b/x-pack/plugins/cases/server/services/cases/index.ts @@ -17,6 +17,7 @@ import type { SavedObjectsFindOptions, SavedObjectsBulkDeleteObject, SavedObjectsBulkDeleteOptions, + SavedObject, } from '@kbn/core/server'; import type * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey'; @@ -29,8 +30,8 @@ import { } from '../../../common/constants'; import type { Case, User, CaseStatuses } from '../../../common/api'; import { decodeOrThrow, caseStatuses } from '../../../common/api'; -import type { SavedObjectFindOptionsKueryNode } from '../../common/types'; -import { defaultSortField, flattenCaseSavedObject } from '../../common/utils'; +import type { SavedObjectFindOptionsKueryNode, SOWithErrors } from '../../common/types'; +import { defaultSortField, flattenCaseSavedObject, isSOError } from '../../common/utils'; import { DEFAULT_PAGE, DEFAULT_PER_PAGE } from '../../routes/api'; import { combineFilters } from '../../client/utils'; import { includeFieldsRequiredForAuthentication } from '../../authorization/utils'; @@ -38,8 +39,6 @@ import { transformSavedObjectToExternalModel, transformAttributesToESModel, transformUpdateResponseToExternalModel, - transformUpdateResponsesToExternalModels, - transformBulkResponseToExternalModel, transformFindResponseToExternalModel, } from './transform'; import type { AttachmentService } from '../attachments'; @@ -320,16 +319,31 @@ export class CasesService { const cases = await this.unsecuredSavedObjectsClient.bulkGet( caseIds.map((caseId) => ({ type: CASE_SAVED_OBJECT, id: caseId, fields })) ); - const res = transformBulkResponseToExternalModel(cases); - const decodeRes = bulkDecodeSOAttributes(res.saved_objects, CaseTransformedAttributesRt); - return { - ...res, - saved_objects: res.saved_objects.map((so) => ({ - ...so, - attributes: decodeRes.get(so.id) as CaseTransformedAttributes, - })), - }; + const res = cases.saved_objects.reduce((acc, theCase) => { + if (isSOError(theCase)) { + acc.push(theCase); + return acc; + } + + const so = Object.assign(theCase, transformSavedObjectToExternalModel(theCase)); + const decodeRes = decodeOrThrow(CaseTransformedAttributesRt)(so.attributes); + const soWithDecodedRes = Object.assign(so, { attributes: decodeRes }); + + acc.push(soWithDecodedRes); + + return acc; + }, [] as Array | SOWithErrors>); + + return Object.assign(cases, { + saved_objects: res, + /** + * The case is needed here because + * the SavedObjectsBulkResponse is wrong. + * It assumes that the attributes exist + * on an error which is not true. + */ + }) as SavedObjectsBulkResponse; } catch (error) { this.log.error(`Error on GET cases ${caseIds.join(', ')}: ${error}`); throw error; @@ -619,19 +633,30 @@ export class CasesService { refresh, }); - const res = transformUpdateResponsesToExternalModels(updatedCases); - const decodeRes = bulkDecodeSOAttributes( - res.saved_objects, - getPartialCaseTransformedAttributesRt() - ); + const res = updatedCases.saved_objects.reduce((acc, theCase) => { + if (isSOError(theCase)) { + acc.push(theCase); + return acc; + } - return { - ...res, - saved_objects: res.saved_objects.map((so) => ({ - ...so, - attributes: decodeRes.get(so.id) as CaseTransformedAttributes, - })), - }; + const so = Object.assign(theCase, transformUpdateResponseToExternalModel(theCase)); + const decodeRes = decodeOrThrow(getPartialCaseTransformedAttributesRt())(so.attributes); + const soWithDecodedRes = Object.assign(so, { attributes: decodeRes }); + + acc.push(soWithDecodedRes); + + return acc; + }, [] as Array | SOWithErrors>); + + return Object.assign(updatedCases, { + saved_objects: res, + /** + * The case is needed here because + * the SavedObjectsBulkUpdateResponse is wrong. + * It assumes that the attributes exist + * on an error which is not true. + */ + }) as SavedObjectsBulkUpdateResponse; } catch (error) { this.log.error(`Error on UPDATE case ${cases.map((c) => c.caseId).join(', ')}: ${error}`); throw error; diff --git a/x-pack/plugins/cases/server/services/cases/transform.ts b/x-pack/plugins/cases/server/services/cases/transform.ts index 5b31bde8107c4..1f8fd4d00ed3b 100644 --- a/x-pack/plugins/cases/server/services/cases/transform.ts +++ b/x-pack/plugins/cases/server/services/cases/transform.ts @@ -10,8 +10,6 @@ import type { SavedObject, SavedObjectReference, - SavedObjectsBulkResponse, - SavedObjectsBulkUpdateResponse, SavedObjectsFindResponse, SavedObjectsUpdateResponse, } from '@kbn/core/server'; @@ -36,18 +34,6 @@ import { ConnectorReferenceHandler } from '../connector_reference_handler'; import type { CasePersistedAttributes, CaseTransformedAttributes } from '../../common/types/case'; import type { ExternalServicePersisted } from '../../common/types/external_service'; -export function transformUpdateResponsesToExternalModels( - response: SavedObjectsBulkUpdateResponse -): SavedObjectsBulkUpdateResponse { - return { - ...response, - saved_objects: response.saved_objects.map((so) => ({ - ...so, - ...transformUpdateResponseToExternalModel(so), - })), - }; -} - export function transformUpdateResponseToExternalModel( updatedCase: SavedObjectsUpdateResponse ): SavedObjectsUpdateResponse { @@ -149,29 +135,6 @@ function buildReferenceHandler( ]); } -/** - * Until Kibana uses typescript 4.3 or higher we'll have to keep these functions separate instead of using an overload - * definition like this: - * - * export function transformArrayResponseToExternalModel( - * response: SavedObjectsBulkResponse | SavedObjectsFindResponse - * ): SavedObjectsBulkResponse | SavedObjectsFindResponse { - * - * See this issue for more details: https://stackoverflow.com/questions/49510832/typescript-how-to-map-over-union-array-type - */ - -export function transformBulkResponseToExternalModel( - response: SavedObjectsBulkResponse -): SavedObjectsBulkResponse { - return { - ...response, - saved_objects: response.saved_objects.map((so) => ({ - ...so, - ...transformSavedObjectToExternalModel(so), - })), - }; -} - export function transformFindResponseToExternalModel( response: SavedObjectsFindResponse ): SavedObjectsFindResponse { From 62924d5984a49e90e47161803ace1b21fbb1a551 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Tue, 16 May 2023 21:20:33 +0300 Subject: [PATCH 04/13] Add unit tests --- .../cases/server/services/cases/index.test.ts | 142 ++++++++++++++++++ 1 file changed, 142 insertions(+) diff --git a/x-pack/plugins/cases/server/services/cases/index.test.ts b/x-pack/plugins/cases/server/services/cases/index.test.ts index c0d02b90bd3c6..72724f228e9ba 100644 --- a/x-pack/plugins/cases/server/services/cases/index.test.ts +++ b/x-pack/plugins/cases/server/services/cases/index.test.ts @@ -1963,6 +1963,93 @@ describe('CasesService', () => { await expect(service.getCases({ caseIds: ['a', 'b'] })).resolves.not.toThrow(); }); + it('do not encodes errors', async () => { + const errorSO = { + ...omit(createCaseSavedObjectResponse({ caseId: '2' }), 'attributes'), + error: { + statusCode: 404, + }, + }; + + unsecuredSavedObjectsClient.bulkGet.mockResolvedValue({ + saved_objects: [ + createCaseSavedObjectResponse({ caseId: '1' }), + // @ts-expect-error: bulkUpdate type expects attributes to be defined + errorSO, + ], + }); + + const res = await service.getCases({ caseIds: ['a', 'b'] }); + + expect(res).toMatchInlineSnapshot(` + Object { + "saved_objects": Array [ + Object { + "attributes": Object { + "assignees": Array [], + "closed_at": null, + "closed_by": null, + "connector": Object { + "fields": null, + "id": "none", + "name": "none", + "type": ".none", + }, + "created_at": "2019-11-25T21:54:48.952Z", + "created_by": Object { + "email": "testemail@elastic.co", + "full_name": "elastic", + "username": "elastic", + }, + "description": "This is a brand new case of a bad meanie defacing data", + "duration": null, + "external_service": Object { + "connector_id": "none", + "connector_name": ".jira", + "external_id": "100", + "external_title": "awesome", + "external_url": "http://www.google.com", + "pushed_at": "2019-11-25T21:54:48.952Z", + "pushed_by": Object { + "email": "testemail@elastic.co", + "full_name": "elastic", + "username": "elastic", + }, + }, + "owner": "securitySolution", + "settings": Object { + "syncAlerts": true, + }, + "severity": "low", + "status": "open", + "tags": Array [ + "defacement", + ], + "title": "Super Bad Security Issue", + "updated_at": "2019-11-25T21:54:48.952Z", + "updated_by": Object { + "email": "testemail@elastic.co", + "full_name": "elastic", + "username": "elastic", + }, + }, + "id": "1", + "references": Array [], + "type": "cases", + }, + Object { + "error": Object { + "statusCode": 404, + }, + "id": "2", + "references": Array [], + "type": "cases", + }, + ], + } + `); + }); + it.each(Object.keys(attributesToValidateIfMissing))( 'throws if %s is omitted', async (key) => { @@ -2135,6 +2222,61 @@ describe('CasesService', () => { ).resolves.not.toThrow(); }); + it('do not encodes errors', async () => { + const errorSO = { + ...omit(createCaseSavedObjectResponse({ caseId: '2' }), 'attributes'), + error: { + statusCode: 404, + }, + }; + + unsecuredSavedObjectsClient.bulkUpdate.mockResolvedValue({ + saved_objects: [ + { + ...createCaseSavedObjectResponse({ caseId: '1' }), + attributes: { description: 'updated desc' }, + }, + // @ts-expect-error: bulkUpdate type expects attributes to be defined + errorSO, + ], + }); + + const res = await service.patchCases({ + cases: [ + { + caseId: '1', + updatedAttributes: createCasePostParams({ + connector: getNoneCaseConnector(), + }), + originalCase: {} as CaseSavedObjectTransformed, + }, + ], + }); + + expect(res).toMatchInlineSnapshot(` + Object { + "saved_objects": Array [ + Object { + "attributes": Object { + "description": "updated desc", + }, + "id": "1", + "references": Array [], + "type": "cases", + }, + Object { + "error": Object { + "statusCode": 404, + }, + "id": "2", + "references": Array [], + "type": "cases", + }, + ], + } + `); + }); + it('strips out excess attributes', async () => { const theCase = createCaseSavedObjectResponse(); unsecuredSavedObjectsClient.bulkUpdate.mockResolvedValue({ From cd57c61e19aedb2dd5eacb57ca8e533dc7683496 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Tue, 16 May 2023 21:21:43 +0300 Subject: [PATCH 05/13] Fix types --- x-pack/plugins/cases/server/client/attachments/bulk_get.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/cases/server/client/attachments/bulk_get.ts b/x-pack/plugins/cases/server/client/attachments/bulk_get.ts index 8970271224252..342cf8af2713e 100644 --- a/x-pack/plugins/cases/server/client/attachments/bulk_get.ts +++ b/x-pack/plugins/cases/server/client/attachments/bulk_get.ts @@ -21,15 +21,15 @@ import { } from '../../../common/api'; import { flattenCommentSavedObjects } from '../../common/utils'; import { createCaseError } from '../../common/error'; -import type { CasesClientArgs, SOWithErrors } from '../types'; +import type { CasesClientArgs } from '../types'; import { Operations } from '../../authorization'; import type { BulkGetArgs } from './types'; import type { BulkOptionalAttributes, OptionalAttributes } from '../../services/attachments/types'; import type { CasesClient } from '../client'; -import type { AttachmentSavedObject } from '../../common/types'; +import type { AttachmentSavedObject, SOWithErrors } from '../../common/types'; import { partitionByCaseAssociation } from '../../common/partitioning'; -type AttachmentSavedObjectWithErrors = SOWithErrors; +type AttachmentSavedObjectWithErrors = Array>; /** * Retrieves multiple attachments by id. From 31e2c8d45cca4a6c8e932b1d702bf20299e53e96 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Tue, 16 May 2023 21:25:14 +0300 Subject: [PATCH 06/13] Remove unecessary code --- .../cases/server/common/types/saved_object.ts | 20 ------------------- 1 file changed, 20 deletions(-) delete mode 100644 x-pack/plugins/cases/server/common/types/saved_object.ts diff --git a/x-pack/plugins/cases/server/common/types/saved_object.ts b/x-pack/plugins/cases/server/common/types/saved_object.ts deleted file mode 100644 index 86070c3832430..0000000000000 --- a/x-pack/plugins/cases/server/common/types/saved_object.ts +++ /dev/null @@ -1,20 +0,0 @@ -/* - * 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 * as rt from 'io-ts'; - -export const createSOType = (attributes: rt.InterfaceType) => { - const requiredFields = rt.strict({ - id: rt.string, - type: rt.string, - attributes, - }); - - const optionalFields = rt.exact(rt.partial({ version: rt.string })); - - return rt.intersection([requiredFields, optionalFields]); -}; From 7378af9dc5f77964310f8f75fdb0f8b3325c6fed Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Tue, 16 May 2023 21:39:06 +0300 Subject: [PATCH 07/13] Add more tests --- .../plugins/cases/server/common/types/case.ts | 3 ++- .../plugins/cases/server/common/utils.test.ts | 11 ++++++++ .../cases/server/services/utils.test.ts | 27 +++++++++++++++++++ 3 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 x-pack/plugins/cases/server/services/utils.test.ts diff --git a/x-pack/plugins/cases/server/common/types/case.ts b/x-pack/plugins/cases/server/common/types/case.ts index b620e6c3d8b4b..a424466ec9aa8 100644 --- a/x-pack/plugins/cases/server/common/types/case.ts +++ b/x-pack/plugins/cases/server/common/types/case.ts @@ -54,9 +54,10 @@ export const CaseTransformedAttributesRt = CaseAttributesRt; export const getPartialCaseTransformedAttributesRt = (): Type> => { const caseTransformedAttributesProps = CaseAttributesRt.types.reduce( - (acc, type) => ({ ...acc, ...type.props }), + (acc, type) => Object.assign(acc, type.props), {} ); + return exact(partial({ ...caseTransformedAttributesProps })); }; diff --git a/x-pack/plugins/cases/server/common/utils.test.ts b/x-pack/plugins/cases/server/common/utils.test.ts index c2ef8281a5887..f5079a04499d7 100644 --- a/x-pack/plugins/cases/server/common/utils.test.ts +++ b/x-pack/plugins/cases/server/common/utils.test.ts @@ -32,6 +32,7 @@ import { transformNewCase, getApplicationRoute, getCaseViewPath, + isSOError, } from './utils'; import { newCase } from '../routes/api/__mocks__/request_responses'; import { CASE_VIEW_PAGE_TABS } from '../../common/types'; @@ -1310,4 +1311,14 @@ describe('common utils', () => { ).toBe('https://example.com/s/test-space/app/security/cases/my-case-id'); }); }); + + describe('isSOError', () => { + it('returns true if the SO is an error', () => { + expect(isSOError({ error: { statusCode: '404' } })).toBe(true); + }); + + it('returns false if the SO is not an error', () => { + expect(isSOError({})).toBe(false); + }); + }); }); diff --git a/x-pack/plugins/cases/server/services/utils.test.ts b/x-pack/plugins/cases/server/services/utils.test.ts new file mode 100644 index 0000000000000..5240e273c9f34 --- /dev/null +++ b/x-pack/plugins/cases/server/services/utils.test.ts @@ -0,0 +1,27 @@ +/* + * 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 * as rt from 'io-ts'; +import { bulkDecodeSOAttributes } from './utils'; + +describe('isSOError', () => { + const typeToTest = rt.type({ foo: rt.string }); + + it('decodes a valid SO correctly', () => { + const savedObjects = [{ id: '1', attributes: { foo: 'test' } }]; + const res = bulkDecodeSOAttributes([{ id: '1', attributes: { foo: 'test' } }], typeToTest); + + expect(res.get('1')).toEqual(savedObjects[0].attributes); + }); + + it('throws an error when SO is not valid', () => { + // @ts-expect-error + expect(() => bulkDecodeSOAttributes([{ id: '1', attributes: {} }], typeToTest)).toThrowError( + 'Invalid value "undefined" supplied to "foo"' + ); + }); +}); From bd10a92828ab7018208fb6d159b98f665921ad6d Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Wed, 17 May 2023 10:28:13 +0300 Subject: [PATCH 08/13] PR feedback --- .../cases/server/common/types/case.test.ts | 71 +++++++++++++++++++ .../cases/server/services/cases/index.ts | 4 +- 2 files changed, 73 insertions(+), 2 deletions(-) create mode 100644 x-pack/plugins/cases/server/common/types/case.test.ts diff --git a/x-pack/plugins/cases/server/common/types/case.test.ts b/x-pack/plugins/cases/server/common/types/case.test.ts new file mode 100644 index 0000000000000..521be071016ac --- /dev/null +++ b/x-pack/plugins/cases/server/common/types/case.test.ts @@ -0,0 +1,71 @@ +/* + * 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 { omit } from 'lodash'; +import { CaseStatuses } from '@kbn/cases-components'; +import { ConnectorTypes, CaseSeverity, SECURITY_SOLUTION_OWNER } from '../../../common'; +import { CaseTransformedAttributesRt, getPartialCaseTransformedAttributesRt } from './case'; + +describe('getPartialCaseTransformedAttributesRt', () => { + const theCaseAttributes = { + closed_at: null, + closed_by: null, + connector: { + id: 'none', + name: 'none', + type: ConnectorTypes.none, + fields: null, + }, + created_at: '2019-11-25T21:54:48.952Z', + created_by: { + full_name: 'elastic', + email: 'testemail@elastic.co', + username: 'elastic', + }, + severity: CaseSeverity.LOW, + duration: null, + description: 'This is a brand new case of a bad meanie defacing data', + external_service: null, + title: 'Super Bad Security Issue', + status: CaseStatuses.open, + tags: ['defacement'], + updated_at: '2019-11-25T21:54:48.952Z', + updated_by: { + full_name: 'elastic', + email: 'testemail@elastic.co', + username: 'elastic', + }, + settings: { + syncAlerts: true, + }, + owner: SECURITY_SOLUTION_OWNER, + assignees: [], + }; + const caseTransformedAttributesProps = CaseTransformedAttributesRt.types.reduce( + (acc, type) => ({ ...acc, ...type.props }), + {} + ); + + const type = getPartialCaseTransformedAttributesRt(); + + it.each(Object.keys(caseTransformedAttributesProps))('does not throw if %s is omitted', (key) => { + const theCase = omit(theCaseAttributes, key); + const decodedRes = type.decode(theCase); + + expect(decodedRes._tag).toEqual('Right'); + // @ts-expect-error: the check above ensures that right exists + expect(decodedRes.right).toEqual(theCase); + }); + + it('removes excess properties', () => { + const decodedRes = type.decode({ description: 'test', 'not-exists': 'excess' }); + + expect(decodedRes._tag).toEqual('Right'); + // @ts-expect-error: the check above ensures that right exists + expect(decodedRes.right).toEqual({ description: 'test' }); + }); +}); diff --git a/x-pack/plugins/cases/server/services/cases/index.ts b/x-pack/plugins/cases/server/services/cases/index.ts index 4cc9a90dece09..798509b695e2b 100644 --- a/x-pack/plugins/cases/server/services/cases/index.ts +++ b/x-pack/plugins/cases/server/services/cases/index.ts @@ -566,9 +566,9 @@ export class CasesService { ); const res = transformSavedObjectToExternalModel(createdCase); - decodeOrThrow(CaseTransformedAttributesRt)(res.attributes); + const decodedRes = decodeOrThrow(CaseTransformedAttributesRt)(res.attributes); - return res; + return { ...res, attributes: decodedRes }; } catch (error) { this.log.error(`Error on POST a new case: ${error}`); throw error; From e9d457e827a2ffddfca2743a7f4d1c0227084195 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Wed, 17 May 2023 11:08:23 +0300 Subject: [PATCH 09/13] Remove fields from bulk get --- x-pack/plugins/cases/common/api/cases/case.ts | 14 +------------- .../cases/server/client/cases/bulk_get.ts | 16 ++++++---------- x-pack/plugins/cases/server/common/types/case.ts | 1 + .../plugins/cases/server/services/cases/index.ts | 3 +-- .../plugins/cases/server/services/cases/types.ts | 1 - .../tests/common/internal/bulk_get_cases.ts | 15 +-------------- 6 files changed, 10 insertions(+), 40 deletions(-) diff --git a/x-pack/plugins/cases/common/api/cases/case.ts b/x-pack/plugins/cases/common/api/cases/case.ts index 39a8c31092b16..2d0d7a2767129 100644 --- a/x-pack/plugins/cases/common/api/cases/case.ts +++ b/x-pack/plugins/cases/common/api/cases/case.ts @@ -360,20 +360,8 @@ export const CasesBulkGetRequestRt = rt.type({ ids: rt.array(rt.string), }); -export const CasesBulkGetResponseFieldsRt = rt.type({ - id: rt.string, - description: rt.string, - title: rt.string, - owner: rt.string, - version: rt.string, - totalComments: rt.number, - status: CaseStatusRt, - created_at: rt.string, - created_by: UserRt, -}); - export const CasesBulkGetResponseRt = rt.type({ - cases: rt.array(CasesBulkGetResponseFieldsRt), + cases: CasesRt, errors: rt.array( rt.type({ error: rt.string, diff --git a/x-pack/plugins/cases/server/client/cases/bulk_get.ts b/x-pack/plugins/cases/server/client/cases/bulk_get.ts index 29fe604c06c4e..49c25d6abfb43 100644 --- a/x-pack/plugins/cases/server/client/cases/bulk_get.ts +++ b/x-pack/plugins/cases/server/client/cases/bulk_get.ts @@ -9,7 +9,7 @@ import Boom from '@hapi/boom'; import { pipe } from 'fp-ts/lib/pipeable'; import { fold } from 'fp-ts/lib/Either'; import { identity } from 'fp-ts/lib/function'; -import { pick, partition } from 'lodash'; +import { partition } from 'lodash'; import { MAX_BULK_GET_CASES } from '../../../common/constants'; import type { @@ -19,7 +19,6 @@ import type { } from '../../../common/api'; import { CasesBulkGetRequestRt, - CasesBulkGetResponseFieldsRt, excess, throwErrors, CasesBulkGetResponseRt, @@ -32,7 +31,6 @@ import type { CaseSavedObjectTransformed } from '../../common/types/case'; import type { SOWithErrors } from '../../common/types'; type CaseSavedObjectWithErrors = Array>; -type BulkGetCase = CasesBulkGetResponse['cases'][number]; /** * Retrieves multiple cases by ids. @@ -48,10 +46,6 @@ export const bulkGet = async ( } = clientArgs; try { - const fields = Object.keys(CasesBulkGetResponseFieldsRt.props).filter( - (field) => !['totalComments', 'id', 'version'].includes(field) - ); - const request = pipe( excess(CasesBulkGetRequestRt).decode(params), fold(throwErrors(Boom.badRequest), identity) @@ -59,7 +53,7 @@ export const bulkGet = async ( throwErrorIfCaseIdsReachTheLimit(request.ids); - const cases = await caseService.getCases({ caseIds: request.ids, fields }); + const cases = await caseService.getCases({ caseIds: request.ids }); const [validCases, soBulkGetErrors] = partition( cases.saved_objects, @@ -77,7 +71,7 @@ export const bulkGet = async ( }); const flattenedCases = authorizedCases.map((theCase) => { - const { userComments } = commentTotals.get(theCase.id) ?? { + const { userComments, alerts } = commentTotals.get(theCase.id) ?? { alerts: 0, userComments: 0, }; @@ -85,11 +79,13 @@ export const bulkGet = async ( const flattenedCase = flattenCaseSavedObject({ savedObject: theCase, totalComment: userComments, + totalAlerts: alerts, }); return { - ...(pick(flattenedCase, [...fields, 'id', 'version']) as BulkGetCase), + ...flattenedCase, totalComments: flattenedCase.totalComment, + totalAlerts: flattenedCase.totalAlerts, }; }); diff --git a/x-pack/plugins/cases/server/common/types/case.ts b/x-pack/plugins/cases/server/common/types/case.ts index a424466ec9aa8..c830abe733a87 100644 --- a/x-pack/plugins/cases/server/common/types/case.ts +++ b/x-pack/plugins/cases/server/common/types/case.ts @@ -50,6 +50,7 @@ export interface CasePersistedAttributes { } export type CaseTransformedAttributes = CaseAttributes; + export const CaseTransformedAttributesRt = CaseAttributesRt; export const getPartialCaseTransformedAttributesRt = (): Type> => { diff --git a/x-pack/plugins/cases/server/services/cases/index.ts b/x-pack/plugins/cases/server/services/cases/index.ts index 798509b695e2b..41f50b3221834 100644 --- a/x-pack/plugins/cases/server/services/cases/index.ts +++ b/x-pack/plugins/cases/server/services/cases/index.ts @@ -312,12 +312,11 @@ export class CasesService { public async getCases({ caseIds, - fields, }: GetCasesArgs): Promise> { try { this.log.debug(`Attempting to GET cases ${caseIds.join(', ')}`); const cases = await this.unsecuredSavedObjectsClient.bulkGet( - caseIds.map((caseId) => ({ type: CASE_SAVED_OBJECT, id: caseId, fields })) + caseIds.map((caseId) => ({ type: CASE_SAVED_OBJECT, id: caseId })) ); const res = cases.saved_objects.reduce((acc, theCase) => { diff --git a/x-pack/plugins/cases/server/services/cases/types.ts b/x-pack/plugins/cases/server/services/cases/types.ts index 3a57fa3eea83a..03ea969069c2c 100644 --- a/x-pack/plugins/cases/server/services/cases/types.ts +++ b/x-pack/plugins/cases/server/services/cases/types.ts @@ -34,7 +34,6 @@ export interface DeleteCaseArgs extends GetCaseArgs, IndexRefresh {} export interface GetCasesArgs { caseIds: string[]; - fields?: string[]; } export interface FindCommentsArgs { diff --git a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/internal/bulk_get_cases.ts b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/internal/bulk_get_cases.ts index 21ce756604b06..34550b1041479 100644 --- a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/internal/bulk_get_cases.ts +++ b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/internal/bulk_get_cases.ts @@ -5,7 +5,6 @@ * 2.0. */ -import { pick } from 'lodash'; import expect from '@kbn/expect'; import { Case, CommentType } from '@kbn/cases-plugin/common'; import { getPostCaseRequest, postCaseReq } from '../../../../common/lib/mock'; @@ -298,18 +297,6 @@ export default ({ getService }: FtrProviderContext): void => { }; const getBulkGetCase = (theCase: Case) => { - const fieldsToPick = [ - 'id', - 'version', - 'owner', - 'title', - 'description', - 'totalComment', - 'status', - 'created_at', - 'created_by', - ]; - - const { totalComment, ...rest } = pick(theCase, fieldsToPick) as Case; + const { totalComment, ...rest } = theCase; return { ...rest, totalComments: totalComment }; }; From 76e3d9e99abd0feea371ee5863480fc348c4c272 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Wed, 17 May 2023 12:49:15 +0300 Subject: [PATCH 10/13] Decode user actions --- .../common/api/cases/user_actions/response.ts | 9 +- .../cases/server/common/types/user_actions.ts | 3 + .../cases/server/services/cases/index.test.ts | 20 +-- .../server/services/user_actions/index.ts | 35 +++- .../user_actions/operations/create.ts | 4 +- .../user_actions/operations/find.test.ts | 160 ++++++++++++++++++ .../services/user_actions/operations/find.ts | 36 +++- .../services/user_actions/test_utils.ts | 8 +- 8 files changed, 246 insertions(+), 29 deletions(-) create mode 100644 x-pack/plugins/cases/server/services/user_actions/operations/find.test.ts diff --git a/x-pack/plugins/cases/common/api/cases/user_actions/response.ts b/x-pack/plugins/cases/common/api/cases/user_actions/response.ts index e4d1e681b1572..edd4533dd6444 100644 --- a/x-pack/plugins/cases/common/api/cases/user_actions/response.ts +++ b/x-pack/plugins/cases/common/api/cases/user_actions/response.ts @@ -69,10 +69,13 @@ const CaseUserActionDeprecatedResponseRt = rt.intersection([ /** * This includes the comment_id but not the action_id or case_id */ -const UserActionAttributes = rt.intersection([CaseUserActionBasicRt, CaseUserActionInjectedIdsRt]); +export const UserActionAttributesRt = rt.intersection([ + CaseUserActionBasicRt, + CaseUserActionInjectedIdsRt, +]); const UserActionRt = rt.intersection([ - UserActionAttributes, + UserActionAttributesRt, rt.type({ id: rt.string, version: rt.string, @@ -93,7 +96,7 @@ export type CaseUserActionsDeprecatedResponse = rt.TypeOf< typeof CaseUserActionsDeprecatedResponseRt >; export type CaseUserActionDeprecatedResponse = rt.TypeOf; -export type UserActionAttributes = rt.TypeOf; +export type UserActionAttributes = rt.TypeOf; /** * This defines the high level category for the user action. Whether the user add, removed, updated something diff --git a/x-pack/plugins/cases/server/common/types/user_actions.ts b/x-pack/plugins/cases/server/common/types/user_actions.ts index 14823f159a408..e1bd08be44323 100644 --- a/x-pack/plugins/cases/server/common/types/user_actions.ts +++ b/x-pack/plugins/cases/server/common/types/user_actions.ts @@ -7,6 +7,7 @@ import type { SavedObject } from '@kbn/core/server'; import type { UserActionAttributes } from '../../../common/api'; +import { UserActionAttributesRt } from '../../../common/api'; import type { User } from './user'; interface UserActionCommonPersistedAttributes { @@ -21,5 +22,7 @@ export interface UserActionPersistedAttributes extends UserActionCommonPersisted payload: Record; } +export const UserActionTransformedAttributesRt = UserActionAttributesRt; + export type UserActionTransformedAttributes = UserActionAttributes; export type UserActionSavedObjectTransformed = SavedObject; diff --git a/x-pack/plugins/cases/server/services/cases/index.test.ts b/x-pack/plugins/cases/server/services/cases/index.test.ts index 72724f228e9ba..4c8f889dd6261 100644 --- a/x-pack/plugins/cases/server/services/cases/index.test.ts +++ b/x-pack/plugins/cases/server/services/cases/index.test.ts @@ -1863,7 +1863,7 @@ describe('CasesService', () => { }); }); - describe('Encoding responses', () => { + describe('Decoding responses', () => { const caseTransformedAttributesProps = CaseTransformedAttributesRt.types.reduce( (acc, type) => ({ ...acc, ...type.props }), {} @@ -1883,7 +1883,7 @@ describe('CasesService', () => { ); describe('getCase', () => { - it('encodes correctly', async () => { + it('decodes correctly', async () => { unsecuredSavedObjectsClient.get.mockResolvedValue(createCaseSavedObjectResponse()); await expect(service.getCase({ id: 'a' })).resolves.not.toThrow(); @@ -1913,7 +1913,7 @@ describe('CasesService', () => { }); describe('getResolveCase', () => { - it('encodes correctly', async () => { + it('decodes correctly', async () => { unsecuredSavedObjectsClient.resolve.mockResolvedValue({ saved_object: createCaseSavedObjectResponse(), outcome: 'exactMatch', @@ -1952,7 +1952,7 @@ describe('CasesService', () => { }); describe('getCases', () => { - it('encodes correctly', async () => { + it('decodes correctly', async () => { unsecuredSavedObjectsClient.bulkGet.mockResolvedValue({ saved_objects: [ createCaseSavedObjectResponse({ caseId: '1' }), @@ -1963,7 +1963,7 @@ describe('CasesService', () => { await expect(service.getCases({ caseIds: ['a', 'b'] })).resolves.not.toThrow(); }); - it('do not encodes errors', async () => { + it('do not decodes errors', async () => { const errorSO = { ...omit(createCaseSavedObjectResponse({ caseId: '2' }), 'attributes'), error: { @@ -2078,7 +2078,7 @@ describe('CasesService', () => { }); describe('findCases', () => { - it('encodes correctly', async () => { + it('decodes correctly', async () => { const findMockReturn = createSOFindResponse([ createFindSO({ caseId: '1' }), createFindSO({ caseId: '2' }), @@ -2123,7 +2123,7 @@ describe('CasesService', () => { }); describe('post', () => { - it('encodes correctly', async () => { + it('decodes correctly', async () => { unsecuredSavedObjectsClient.create.mockResolvedValue(createCaseSavedObjectResponse()); await expect( @@ -2166,7 +2166,7 @@ describe('CasesService', () => { }); describe('patchCase', () => { - it('encodes correctly', async () => { + it('decodes correctly', async () => { unsecuredSavedObjectsClient.update.mockResolvedValue(createUpdateSOResponse()); await expect( @@ -2199,7 +2199,7 @@ describe('CasesService', () => { }); describe('patchCases', () => { - it('encodes correctly', async () => { + it('decodes correctly', async () => { unsecuredSavedObjectsClient.bulkUpdate.mockResolvedValue({ saved_objects: [ createCaseSavedObjectResponse({ caseId: '1' }), @@ -2222,7 +2222,7 @@ describe('CasesService', () => { ).resolves.not.toThrow(); }); - it('do not encodes errors', async () => { + it('do not decodes errors', async () => { const errorSO = { ...omit(createCaseSavedObjectResponse({ caseId: '2' }), 'attributes'), error: { diff --git a/x-pack/plugins/cases/server/services/user_actions/index.ts b/x-pack/plugins/cases/server/services/user_actions/index.ts index 521cee600469b..7960f2f6636c8 100644 --- a/x-pack/plugins/cases/server/services/user_actions/index.ts +++ b/x-pack/plugins/cases/server/services/user_actions/index.ts @@ -10,7 +10,7 @@ import type { SavedObjectsFindResponse, SavedObjectsRawDoc } from '@kbn/core/ser import type * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey'; import type { KueryNode } from '@kbn/es-query'; import type { CaseUserActionDeprecatedResponse } from '../../../common/api'; -import { ActionTypes } from '../../../common/api'; +import { decodeOrThrow, ActionTypes } from '../../../common/api'; import { CASE_SAVED_OBJECT, CASE_USER_ACTION_SAVED_OBJECT, @@ -39,6 +39,7 @@ import type { UserActionPersistedAttributes, UserActionSavedObjectTransformed, } from '../../common/types/user_actions'; +import { UserActionTransformedAttributesRt } from '../../common/types/user_actions'; export class CaseUserActionService { private readonly _creator: UserActionPersister; @@ -208,11 +209,18 @@ export class CaseUserActionService { rawFieldsDoc ); - const fieldsDoc = transformToExternalModel( + const res = transformToExternalModel( doc, this.context.persistableStateAttachmentTypeRegistry ); + const decodeRes = decodeOrThrow(UserActionTransformedAttributesRt)(res.attributes); + + const fieldsDoc = { + ...res, + attributes: decodeRes, + }; + connectorFields.set(connectorId, fieldsDoc); } } @@ -258,10 +266,17 @@ export class CaseUserActionService { return; } - return transformToExternalModel( + const res = transformToExternalModel( userActions.saved_objects[0], this.context.persistableStateAttachmentTypeRegistry ); + + const decodeRes = decodeOrThrow(UserActionTransformedAttributesRt)(res.attributes); + + return { + ...res, + attributes: decodeRes, + }; } catch (error) { this.context.log.error( `Error while retrieving the most recent user action for case id: ${caseId}: ${error}` @@ -334,10 +349,14 @@ export class CaseUserActionService { rawFieldsDoc ); - fieldsDoc = transformToExternalModel( + const res = transformToExternalModel( doc, this.context.persistableStateAttachmentTypeRegistry ); + + const decodeRes = decodeOrThrow(UserActionTransformedAttributesRt)(res.attributes); + + fieldsDoc = { ...res, attributes: decodeRes }; } const pushDocs = this.getPushDocs(connectorInfo.reverse.connectorActivity.buckets.pushInfo); @@ -377,7 +396,13 @@ export class CaseUserActionService { rawPushDoc ); - return transformToExternalModel(doc, this.context.persistableStateAttachmentTypeRegistry); + const res = transformToExternalModel( + doc, + this.context.persistableStateAttachmentTypeRegistry + ); + + const decodeRes = decodeOrThrow(UserActionTransformedAttributesRt)(res.attributes); + return { ...res, attributes: decodeRes }; } } diff --git a/x-pack/plugins/cases/server/services/user_actions/operations/create.ts b/x-pack/plugins/cases/server/services/user_actions/operations/create.ts index d4b5c0d283630..086b0832e6159 100644 --- a/x-pack/plugins/cases/server/services/user_actions/operations/create.ts +++ b/x-pack/plugins/cases/server/services/user_actions/operations/create.ts @@ -325,7 +325,7 @@ export class UserActionPersister { connectorId, attachmentId, refresh, - }: CreateUserActionClient) { + }: CreateUserActionClient): Promise { try { this.context.log.debug(`Attempting to create a user action of type: ${type}`); const userActionBuilder = this.builderFactory.getBuilder(type); @@ -381,7 +381,7 @@ export class UserActionPersister { } } - public async bulkAuditLogCaseDeletion(caseIds: string[]) { + public async bulkAuditLogCaseDeletion(caseIds: string[]): Promise { this.context.log.debug(`Attempting to log bulk case deletion`); for (const id of caseIds) { diff --git a/x-pack/plugins/cases/server/services/user_actions/operations/find.test.ts b/x-pack/plugins/cases/server/services/user_actions/operations/find.test.ts new file mode 100644 index 0000000000000..16a873c784629 --- /dev/null +++ b/x-pack/plugins/cases/server/services/user_actions/operations/find.test.ts @@ -0,0 +1,160 @@ +/* + * 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 { PersistableStateAttachmentTypeRegistry } from '../../../attachment_framework/persistable_state_registry'; +import { savedObjectsClientMock } from '@kbn/core-saved-objects-api-server-mocks'; +import { loggerMock } from '@kbn/logging-mocks'; +import { UserActionFinder } from './find'; +import { createSavedObjectsSerializerMock } from '../../../client/mocks'; +import { auditLoggerMock } from '@kbn/security-plugin/server/audit/mocks'; +import { + createConnectorUserAction, + createUserActionFindSO, + createUserActionSO, +} from '../test_utils'; +import { createSOFindResponse } from '../../test_utils'; +import { omit } from 'lodash'; +import type { SavedObjectsFindResponse } from '@kbn/core/server'; + +describe('UserActionsService: Finder', () => { + const unsecuredSavedObjectsClient = savedObjectsClientMock.create(); + const mockLogger = loggerMock.create(); + const auditMockLocker = auditLoggerMock.create(); + const persistableStateAttachmentTypeRegistry = new PersistableStateAttachmentTypeRegistry(); + const savedObjectsSerializer = createSavedObjectsSerializerMock(); + + const attributesToValidateIfMissing = ['created_at', 'created_by', 'owner', 'action', 'payload']; + + let finder: UserActionFinder; + + beforeEach(() => { + jest.resetAllMocks(); + finder = new UserActionFinder({ + log: mockLogger, + unsecuredSavedObjectsClient, + persistableStateAttachmentTypeRegistry, + savedObjectsSerializer, + auditLogger: auditMockLocker, + }); + }); + + const mockFind = (soFindRes: SavedObjectsFindResponse) => { + unsecuredSavedObjectsClient.find.mockResolvedValue(soFindRes); + }; + + const mockPointInTimeFinder = (soFindRes: SavedObjectsFindResponse) => { + // console.log(soFindRes); + unsecuredSavedObjectsClient.createPointInTimeFinder.mockReturnValue({ + close: jest.fn(), + // @ts-expect-error + find: function* asyncGenerator() { + yield { + ...soFindRes, + }; + }, + }); + }; + + const decodingTests: Array< + [keyof UserActionFinder, (soFindRes: SavedObjectsFindResponse) => void] + > = [ + ['find', mockFind], + ['findStatusChanges', mockPointInTimeFinder], + ]; + + describe('find', () => { + it('sets the comment_id to null if it is omitted', async () => { + const userAction = createUserActionSO(); + const attributes = omit({ ...userAction.attributes }, 'comment_id'); + const soFindRes = createSOFindResponse([{ ...userAction, attributes, score: 0 }]); + mockFind(soFindRes); + + const res = await finder.find({ caseId: '1' }); + const commentId = res.saved_objects[0].attributes.comment_id; + + expect(commentId).toBe(null); + }); + }); + + describe('findStatusChanges', () => { + it('sets the comment_id to null if it is omitted', async () => { + const userAction = createUserActionSO(); + const attributes = omit({ ...userAction.attributes }, 'comment_id'); + const soFindRes = createSOFindResponse([{ ...userAction, attributes, score: 0 }]); + mockPointInTimeFinder(soFindRes); + + const res = await finder.findStatusChanges({ caseId: '1' }); + const commentId = res[0].attributes.comment_id; + + expect(commentId).toBe(null); + }); + }); + + describe.each(decodingTests)('Decoding: %s', (soMethodName, method) => { + it('decodes correctly', async () => { + const userAction = createUserActionSO(); + const soFindRes = createSOFindResponse([createUserActionFindSO(userAction)]); + method(soFindRes); + + await expect(finder[soMethodName]({ caseId: '1' })).resolves.not.toThrow(); + }); + + it.each(attributesToValidateIfMissing)('throws if %s is omitted', async (key) => { + const userAction = createUserActionSO(); + const attributes = omit({ ...userAction.attributes }, key); + const soFindRes = createSOFindResponse([{ ...userAction, attributes, score: 0 }]); + method(soFindRes); + + await expect(finder[soMethodName]({ caseId: '1' })).rejects.toThrow( + `Invalid value "undefined" supplied to "${key}"` + ); + }); + + it('throws if type is omitted', async () => { + const userAction = createUserActionSO(); + const attributes = omit({ ...userAction.attributes }, 'type'); + const soFindRes = createSOFindResponse([{ ...userAction, attributes, score: 0 }]); + method(soFindRes); + + await expect(finder[soMethodName]({ caseId: '1' })).rejects.toThrow(); + }); + + it('throws if missing attributes from the payload', async () => { + const userAction = createUserActionSO(); + const attributes = omit({ ...userAction.attributes }, 'payload.title'); + const soFindRes = createSOFindResponse([{ ...userAction, attributes, score: 0 }]); + method(soFindRes); + + await expect(finder[soMethodName]({ caseId: '1' })).rejects.toThrow( + 'Invalid value "undefined" supplied to "payload,title"' + ); + }); + + it('throws if missing nested attributes from the payload', async () => { + const userAction = createConnectorUserAction(); + const attributes = omit({ ...userAction.attributes }, 'payload.connector.fields.issueType'); + const soFindRes = createSOFindResponse([{ ...userAction, attributes, score: 0 }]); + method(soFindRes); + + await expect(finder[soMethodName]({ caseId: '1' })).rejects.toThrow( + 'Invalid value "undefined" supplied to "payload,connector,fields,issueType",Invalid value "{"priority":"high","parent":"2"}" supplied to "payload,connector,fields"' + ); + }); + + // TODO: Unskip when all types are converted to strict + it.skip('strips out excess attributes', async () => { + const userAction = createUserActionSO(); + const attributes = { ...userAction.attributes, 'not-exists': 'not-exists' }; + const soFindRes = createSOFindResponse([{ ...userAction, attributes, score: 0 }]); + method(soFindRes); + + await expect(finder[soMethodName]({ caseId: '1' })).resolves.toEqual({ + attributes: userAction.attributes, + }); + }); + }); +}); diff --git a/x-pack/plugins/cases/server/services/user_actions/operations/find.ts b/x-pack/plugins/cases/server/services/user_actions/operations/find.ts index c1b3a49ed6507..6c1e597667f19 100644 --- a/x-pack/plugins/cases/server/services/user_actions/operations/find.ts +++ b/x-pack/plugins/cases/server/services/user_actions/operations/find.ts @@ -11,7 +11,7 @@ import type { SavedObjectsFindResponse } from '@kbn/core-saved-objects-api-serve import { DEFAULT_PAGE, DEFAULT_PER_PAGE } from '../../../routes/api'; import { defaultSortField } from '../../../common/utils'; import type { ActionTypeValues, FindTypeField } from '../../../../common/api'; -import { Actions, ActionTypes, CommentType } from '../../../../common/api'; +import { Actions, ActionTypes, CommentType, decodeOrThrow } from '../../../../common/api'; import { CASE_SAVED_OBJECT, CASE_USER_ACTION_SAVED_OBJECT, @@ -26,6 +26,8 @@ import type { UserActionSavedObjectTransformed, UserActionTransformedAttributes, } from '../../../common/types/user_actions'; +import { bulkDecodeSOAttributes } from '../../utils'; +import { UserActionTransformedAttributesRt } from '../../../common/types/user_actions'; export class UserActionFinder { constructor(private readonly context: ServiceContext) {} @@ -54,10 +56,23 @@ export class UserActionFinder { filter: finalFilter, }); - return transformFindResponseToExternalModel( + const res = transformFindResponseToExternalModel( userActions, this.context.persistableStateAttachmentTypeRegistry ); + + const decodeRes = bulkDecodeSOAttributes( + res.saved_objects, + UserActionTransformedAttributesRt + ); + + return { + ...res, + saved_objects: res.saved_objects.map((so) => ({ + ...so, + attributes: decodeRes.get(so.id) as UserActionTransformedAttributes, + })), + }; } catch (error) { this.context.log.error(`Error finding user actions for case id: ${caseId}: ${error}`); throw error; @@ -200,11 +215,22 @@ export class UserActionFinder { ); let userActions: UserActionSavedObjectTransformed[] = []; + for await (const findResults of finder.find()) { userActions = userActions.concat( - findResults.saved_objects.map((so) => - transformToExternalModel(so, this.context.persistableStateAttachmentTypeRegistry) - ) + findResults.saved_objects.map((so) => { + const res = transformToExternalModel( + so, + this.context.persistableStateAttachmentTypeRegistry + ); + + const decodeRes = decodeOrThrow(UserActionTransformedAttributesRt)(res.attributes); + + return { + ...res, + attributes: decodeRes, + }; + }) ); } diff --git a/x-pack/plugins/cases/server/services/user_actions/test_utils.ts b/x-pack/plugins/cases/server/services/user_actions/test_utils.ts index b4dbb114fc2fe..75aa7d593eaed 100644 --- a/x-pack/plugins/cases/server/services/user_actions/test_utils.ts +++ b/x-pack/plugins/cases/server/services/user_actions/test_utils.ts @@ -10,8 +10,8 @@ import type { SavedObjectsFindResponse, SavedObjectsFindResult, } from '@kbn/core-saved-objects-api-server'; -import type { SavedObject, SavedObjectReference } from '@kbn/core-saved-objects-common'; import { omit, get } from 'lodash'; +import type { SavedObject, SavedObjectReference } from '@kbn/core/server'; import { CASE_COMMENT_SAVED_OBJECT, CASE_SAVED_OBJECT, @@ -67,7 +67,7 @@ export const createConnectorUserAction = ( }; export const createUserActionSO = ({ - action, + action = Actions.create, attributesOverrides, commentId, connectorId, @@ -76,7 +76,7 @@ export const createUserActionSO = ({ type, references = [], }: { - action: ActionCategory; + action?: ActionCategory; type?: string; payload?: Record; attributesOverrides?: Partial; @@ -84,7 +84,7 @@ export const createUserActionSO = ({ connectorId?: string; pushedConnectorId?: string; references?: SavedObjectReference[]; -}): SavedObject => { +} = {}): SavedObject => { const defaultParams = { action, created_at: 'abc', From 1f696c78fe6528370d67ac5c4deeab1d048ed868 Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Wed, 17 May 2023 09:57:24 +0000 Subject: [PATCH 11/13] [CI] Auto-commit changed files from 'node scripts/lint_ts_projects --fix' --- x-pack/plugins/cases/tsconfig.json | 1 + 1 file changed, 1 insertion(+) diff --git a/x-pack/plugins/cases/tsconfig.json b/x-pack/plugins/cases/tsconfig.json index 3dd670e260f9d..52e3ef20d1d9f 100644 --- a/x-pack/plugins/cases/tsconfig.json +++ b/x-pack/plugins/cases/tsconfig.json @@ -63,6 +63,7 @@ "@kbn/saved-objects-finder-plugin", "@kbn/saved-objects-management-plugin", "@kbn/utility-types-jest", + "@kbn/core-saved-objects-api-server-mocks", ], "exclude": [ "target/**/*", From ac6fb5e3405e185a7c5653e92690534f446ad10e Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Thu, 18 May 2023 12:34:04 +0300 Subject: [PATCH 12/13] More tests --- .../services/user_actions/index.test.ts | 398 +++++++++++++++++- 1 file changed, 392 insertions(+), 6 deletions(-) diff --git a/x-pack/plugins/cases/server/services/user_actions/index.test.ts b/x-pack/plugins/cases/server/services/user_actions/index.test.ts index 7d2389a7d88d0..fcd1dd0c21119 100644 --- a/x-pack/plugins/cases/server/services/user_actions/index.test.ts +++ b/x-pack/plugins/cases/server/services/user_actions/index.test.ts @@ -5,6 +5,7 @@ * 2.0. */ +import { set, omit } from 'lodash'; import { loggerMock } from '@kbn/logging-mocks'; import { savedObjectsClientMock } from '@kbn/core/server/mocks'; import type { @@ -14,11 +15,14 @@ import type { SavedObjectsUpdateResponse, } from '@kbn/core/server'; import { auditLoggerMock } from '@kbn/security-plugin/server/audit/mocks'; -import type { CaseAttributes } from '../../../common/api'; +import type { + CaseAttributes, + CaseUserActionAttributesWithoutConnectorId, +} from '../../../common/api'; import { Actions, ActionTypes, CaseSeverity, CaseStatuses } from '../../../common/api'; import { SECURITY_SOLUTION_OWNER } from '../../../common/constants'; -import { createCaseSavedObjectResponse } from '../test_utils'; +import { createCaseSavedObjectResponse, createSOFindResponse } from '../test_utils'; import { casePayload, externalService, @@ -33,7 +37,12 @@ import { import { CaseUserActionService } from '.'; import { createPersistableStateAttachmentTypeRegistryMock } from '../../attachment_framework/mocks'; import { serializerMock } from '@kbn/core-saved-objects-base-server-mocks'; -import { createUserActionFindSO, createConnectorUserAction } from './test_utils'; +import { + createUserActionFindSO, + createConnectorUserAction, + createUserActionSO, + pushConnectorUserAction, +} from './test_utils'; describe('CaseUserActionService', () => { const persistableStateAttachmentTypeRegistry = createPersistableStateAttachmentTypeRegistryMock(); @@ -1471,7 +1480,7 @@ describe('CaseUserActionService', () => { jest.clearAllMocks(); }); - it('it returns an empty array if the response is not valid', async () => { + it('returns an empty array if the response is not valid', async () => { const res = await service.getUniqueConnectors({ caseId: '123', }); @@ -1479,7 +1488,7 @@ describe('CaseUserActionService', () => { expect(res).toEqual([]); }); - it('it returns the connectors', async () => { + it('returns the connectors', async () => { unsecuredSavedObjectsClient.find.mockResolvedValue({ ...findResponse, ...aggregationResponse, @@ -1496,7 +1505,7 @@ describe('CaseUserActionService', () => { ]); }); - it('it returns the unique connectors', async () => { + it('returns the unique connectors', async () => { await service.getUniqueConnectors({ caseId: '123', }); @@ -1578,6 +1587,383 @@ describe('CaseUserActionService', () => { ] `); }); + + describe('Decode', () => { + const attributesToValidateIfMissing = [ + 'created_at', + 'created_by', + 'owner', + 'action', + 'payload', + ]; + + const pushes = [{ date: new Date(), connectorId: '123' }]; + + describe('getConnectorFieldsBeforeLatestPush', () => { + const getAggregations = ( + userAction: SavedObject + ) => { + const connectors = set({}, 'servicenow.mostRecent.hits.hits', [userAction]); + + const aggregations = set({}, 'references.connectors.reverse.ids.buckets', connectors); + + return aggregations; + }; + + it('decodes correctly', async () => { + const userAction = createUserActionSO(); + const aggregations = getAggregations(userAction); + const soFindRes = createSOFindResponse([{ ...userAction, score: 0 }]); + + unsecuredSavedObjectsClient.find.mockResolvedValue({ ...soFindRes, aggregations }); + soSerializerMock.rawToSavedObject.mockReturnValue(userAction); + + await expect( + service.getConnectorFieldsBeforeLatestPush('1', pushes) + ).resolves.not.toThrow(); + }); + + it.each(attributesToValidateIfMissing)('throws if %s is omitted', async (key) => { + const userAction = createUserActionSO(); + const attributes = omit({ ...userAction.attributes }, key); + const userActionWithOmittedAttribute = { ...userAction, attributes, score: 0 }; + + // @ts-expect-error: an attribute is missing + const aggregations = getAggregations(userActionWithOmittedAttribute); + const soFindRes = createSOFindResponse([userActionWithOmittedAttribute]); + + unsecuredSavedObjectsClient.find.mockResolvedValue({ ...soFindRes, aggregations }); + soSerializerMock.rawToSavedObject.mockReturnValue(userActionWithOmittedAttribute); + + await expect(service.getConnectorFieldsBeforeLatestPush('1', pushes)).rejects.toThrow( + `Invalid value "undefined" supplied to "${key}"` + ); + }); + + it('throws if missing attributes from the payload', async () => { + const userAction = createUserActionSO(); + const attributes = omit({ ...userAction.attributes }, 'payload.title'); + const userActionWithOmittedAttribute = { ...userAction, attributes, score: 0 }; + + // @ts-expect-error: an attribute is missing + const aggregations = getAggregations(userActionWithOmittedAttribute); + const soFindRes = createSOFindResponse([userActionWithOmittedAttribute]); + + unsecuredSavedObjectsClient.find.mockResolvedValue({ ...soFindRes, aggregations }); + soSerializerMock.rawToSavedObject.mockReturnValue(userActionWithOmittedAttribute); + + await expect(service.getConnectorFieldsBeforeLatestPush('1', pushes)).rejects.toThrow( + 'Invalid value "undefined" supplied to "payload,title"' + ); + }); + + it('throws if missing nested attributes from the payload', async () => { + const userAction = createConnectorUserAction(); + const attributes = omit( + { ...userAction.attributes }, + 'payload.connector.fields.issueType' + ); + const userActionWithOmittedAttribute = { ...userAction, attributes, score: 0 }; + + // @ts-expect-error: an attribute is missing + const aggregations = getAggregations(userActionWithOmittedAttribute); + const soFindRes = createSOFindResponse([userActionWithOmittedAttribute]); + + unsecuredSavedObjectsClient.find.mockResolvedValue({ ...soFindRes, aggregations }); + soSerializerMock.rawToSavedObject.mockReturnValue(userActionWithOmittedAttribute); + + await expect(service.getConnectorFieldsBeforeLatestPush('1', pushes)).rejects.toThrow( + 'Invalid value "undefined" supplied to "payload,connector,fields,issueType",Invalid value "{"priority":"high","parent":"2"}" supplied to "payload,connector,fields"' + ); + }); + + it.skip('strips out excess attributes', async () => { + const userAction = createUserActionSO(); + const attributes = { ...userAction.attributes, 'not-exists': 'not-exists' }; + const userActionWithExtraAttributes = { ...userAction, attributes, score: 0 }; + const aggregations = getAggregations(userActionWithExtraAttributes); + const soFindRes = createSOFindResponse([userActionWithExtraAttributes]); + + unsecuredSavedObjectsClient.find.mockResolvedValue({ ...soFindRes, aggregations }); + soSerializerMock.rawToSavedObject.mockReturnValue(userActionWithExtraAttributes); + + await expect(service.getConnectorFieldsBeforeLatestPush('1', pushes)).resolves.toEqual({ + attributes: userAction.attributes, + }); + }); + }); + + describe('getMostRecentUserAction', () => { + it('decodes correctly', async () => { + const userAction = createUserActionSO(); + const soFindRes = createSOFindResponse([createUserActionFindSO(userAction)]); + unsecuredSavedObjectsClient.find.mockResolvedValue(soFindRes); + + await expect(service.getMostRecentUserAction('123')).resolves.not.toThrow(); + }); + + it.each(attributesToValidateIfMissing)('throws if %s is omitted', async (key) => { + const userAction = createUserActionSO(); + const attributes = omit({ ...userAction.attributes }, key); + const soFindRes = createSOFindResponse([{ ...userAction, attributes, score: 0 }]); + unsecuredSavedObjectsClient.find.mockResolvedValue(soFindRes); + + await expect(service.getMostRecentUserAction('123')).rejects.toThrow( + `Invalid value "undefined" supplied to "${key}"` + ); + }); + + it('throws if missing attributes from the payload', async () => { + const userAction = createUserActionSO(); + const attributes = omit({ ...userAction.attributes }, 'payload.title'); + const soFindRes = createSOFindResponse([{ ...userAction, attributes, score: 0 }]); + unsecuredSavedObjectsClient.find.mockResolvedValue(soFindRes); + + await expect(service.getMostRecentUserAction('123')).rejects.toThrow( + 'Invalid value "undefined" supplied to "payload,title"' + ); + }); + + it('throws if missing nested attributes from the payload', async () => { + const userAction = createConnectorUserAction(); + const attributes = omit( + { ...userAction.attributes }, + 'payload.connector.fields.issueType' + ); + const soFindRes = createSOFindResponse([{ ...userAction, attributes, score: 0 }]); + unsecuredSavedObjectsClient.find.mockResolvedValue(soFindRes); + + await expect(service.getMostRecentUserAction('123')).rejects.toThrow( + 'Invalid value "undefined" supplied to "payload,connector,fields,issueType",Invalid value "{"priority":"high","parent":"2"}" supplied to "payload,connector,fields"' + ); + }); + + // TODO: Unskip when all types are converted to strict + it.skip('strips out excess attributes', async () => { + const userAction = createUserActionSO(); + const attributes = { ...userAction.attributes, 'not-exists': 'not-exists' }; + const soFindRes = createSOFindResponse([{ ...userAction, attributes, score: 0 }]); + unsecuredSavedObjectsClient.find.mockResolvedValue(soFindRes); + + await expect(service.getMostRecentUserAction('123')).resolves.toEqual({ + attributes: userAction.attributes, + }); + }); + }); + + describe('getCaseConnectorInformation', () => { + const getAggregations = ( + userAction: SavedObject, + pushUserAction: SavedObject + ) => { + const changeConnector = set({}, 'mostRecent.hits.hits', [userAction]); + const createCase = set({}, 'mostRecent.hits.hits', []); + const pushInfo = { + mostRecent: set({}, 'hits.hits', [pushUserAction]), + oldest: set({}, 'hits.hits', [pushUserAction]), + }; + + const connectorsBucket = { changeConnector, createCase, pushInfo }; + const connectors = set({}, 'reverse.connectorActivity.buckets', connectorsBucket); + const aggregations = set({}, 'references.connectors.ids.buckets', [connectors]); + + return aggregations; + }; + + it('decodes correctly', async () => { + const userAction = createUserActionSO(); + const pushUserAction = pushConnectorUserAction(); + const aggregations = getAggregations(userAction, pushUserAction); + const soFindRes = createSOFindResponse([{ ...userAction, score: 0 }]); + + unsecuredSavedObjectsClient.find.mockResolvedValue({ ...soFindRes, aggregations }); + soSerializerMock.rawToSavedObject.mockReturnValue(userAction); + + await expect(service.getCaseConnectorInformation('1')).resolves.not.toThrow(); + }); + + describe('Testing userAction', () => { + it.each(attributesToValidateIfMissing)('throws if %s is omitted', async (key) => { + const userAction = createUserActionSO(); + const pushUserAction = pushConnectorUserAction(); + const attributes = omit({ ...userAction.attributes }, key); + const userActionWithOmittedAttribute = { ...userAction, attributes, score: 0 }; + + // @ts-expect-error: an attribute is missing + const aggregations = getAggregations(userActionWithOmittedAttribute, pushUserAction); + const soFindRes = createSOFindResponse([userActionWithOmittedAttribute]); + + unsecuredSavedObjectsClient.find.mockResolvedValue({ ...soFindRes, aggregations }); + soSerializerMock.rawToSavedObject.mockReturnValue(userActionWithOmittedAttribute); + + await expect(service.getCaseConnectorInformation('1')).rejects.toThrow( + `Invalid value "undefined" supplied to "${key}"` + ); + }); + + it('throws if missing attributes from the payload', async () => { + const userAction = createUserActionSO(); + const pushUserAction = pushConnectorUserAction(); + const attributes = omit({ ...userAction.attributes }, 'payload.title'); + const userActionWithOmittedAttribute = { ...userAction, attributes, score: 0 }; + + // @ts-expect-error: an attribute is missing + const aggregations = getAggregations(userActionWithOmittedAttribute, pushUserAction); + const soFindRes = createSOFindResponse([userActionWithOmittedAttribute]); + + unsecuredSavedObjectsClient.find.mockResolvedValue({ ...soFindRes, aggregations }); + soSerializerMock.rawToSavedObject.mockReturnValue(userActionWithOmittedAttribute); + + await expect(service.getCaseConnectorInformation('1')).rejects.toThrow( + 'Invalid value "undefined" supplied to "payload,title"' + ); + }); + + it('throws if missing nested attributes from the payload', async () => { + const userAction = createConnectorUserAction(); + const pushUserAction = pushConnectorUserAction(); + const attributes = omit( + { ...userAction.attributes }, + 'payload.connector.fields.issueType' + ); + const userActionWithOmittedAttribute = { ...userAction, attributes, score: 0 }; + + // @ts-expect-error: an attribute is missing + const aggregations = getAggregations(userActionWithOmittedAttribute, pushUserAction); + const soFindRes = createSOFindResponse([userActionWithOmittedAttribute]); + + unsecuredSavedObjectsClient.find.mockResolvedValue({ ...soFindRes, aggregations }); + soSerializerMock.rawToSavedObject.mockReturnValue(userActionWithOmittedAttribute); + + await expect(service.getCaseConnectorInformation('1')).rejects.toThrow( + 'Invalid value "undefined" supplied to "payload,connector,fields,issueType",Invalid value "{"priority":"high","parent":"2"}" supplied to "payload,connector,fields"' + ); + }); + + it.skip('strips out excess attributes', async () => { + const userAction = createUserActionSO(); + const pushUserAction = pushConnectorUserAction(); + const attributes = { ...userAction.attributes, 'not-exists': 'not-exists' }; + const userActionWithExtraAttributes = { ...userAction, attributes, score: 0 }; + const aggregations = getAggregations(userActionWithExtraAttributes, pushUserAction); + const soFindRes = createSOFindResponse([userActionWithExtraAttributes]); + + unsecuredSavedObjectsClient.find.mockResolvedValue({ ...soFindRes, aggregations }); + soSerializerMock.rawToSavedObject.mockReturnValue(userActionWithExtraAttributes); + + await expect(service.getCaseConnectorInformation('1')).resolves.toEqual({ + attributes: userAction.attributes, + }); + }); + }); + + describe('Testing pushAction', () => { + it.each(attributesToValidateIfMissing)('throws if %s is omitted', async (key) => { + const userAction = createUserActionSO(); + const pushUserAction = pushConnectorUserAction(); + const attributes = omit({ ...pushUserAction.attributes }, key); + const pushActionActionWithOmittedAttribute = { + ...pushUserAction, + attributes, + score: 0, + }; + + const aggregations = getAggregations( + userAction, + // @ts-expect-error: an attribute is missing + pushActionActionWithOmittedAttribute + ); + const soFindRes = createSOFindResponse([{ ...userAction, score: 0 }]); + + unsecuredSavedObjectsClient.find.mockResolvedValue({ ...soFindRes, aggregations }); + soSerializerMock.rawToSavedObject.mockReturnValueOnce(userAction); + soSerializerMock.rawToSavedObject.mockReturnValueOnce( + pushActionActionWithOmittedAttribute + ); + + await expect(service.getCaseConnectorInformation('1')).rejects.toThrow( + `Invalid value "undefined" supplied to "${key}"` + ); + }); + + it('throws if missing attributes from the payload', async () => { + const userAction = createUserActionSO(); + const pushUserAction = pushConnectorUserAction(); + const attributes = omit({ ...pushUserAction.attributes }, 'payload.externalService'); + const pushActionActionWithOmittedAttribute = { + ...pushUserAction, + attributes, + score: 0, + }; + + const aggregations = getAggregations( + userAction, + // @ts-expect-error: an attribute is missing + pushActionActionWithOmittedAttribute + ); + const soFindRes = createSOFindResponse([{ ...userAction, score: 0 }]); + + unsecuredSavedObjectsClient.find.mockResolvedValue({ ...soFindRes, aggregations }); + soSerializerMock.rawToSavedObject.mockReturnValueOnce(userAction); + soSerializerMock.rawToSavedObject.mockReturnValueOnce( + pushActionActionWithOmittedAttribute + ); + + await expect(service.getCaseConnectorInformation('1')).rejects.toThrow( + 'Invalid value "undefined" supplied to "payload,externalService"' + ); + }); + + it('throws if missing nested attributes from the payload', async () => { + const userAction = createUserActionSO(); + const pushUserAction = pushConnectorUserAction(); + const attributes = omit( + { ...pushUserAction.attributes }, + 'payload.externalService.external_id' + ); + const pushActionActionWithOmittedAttribute = { + ...pushUserAction, + attributes, + score: 0, + }; + + const aggregations = getAggregations( + userAction, + // @ts-expect-error: an attribute is missing + pushActionActionWithOmittedAttribute + ); + const soFindRes = createSOFindResponse([{ ...userAction, score: 0 }]); + + unsecuredSavedObjectsClient.find.mockResolvedValue({ ...soFindRes, aggregations }); + soSerializerMock.rawToSavedObject.mockReturnValueOnce(userAction); + soSerializerMock.rawToSavedObject.mockReturnValueOnce( + pushActionActionWithOmittedAttribute + ); + + await expect(service.getCaseConnectorInformation('1')).rejects.toThrow( + 'Invalid value "undefined" supplied to "payload,externalService,external_id"' + ); + }); + + it.skip('strips out excess attributes', async () => { + const userAction = createUserActionSO(); + const pushUserAction = pushConnectorUserAction(); + const attributes = { ...pushUserAction.attributes, 'not-exists': 'not-exists' }; + const pushActionWithExtraAttributes = { ...userAction, attributes, score: 0 }; + const aggregations = getAggregations(userAction, pushActionWithExtraAttributes); + const soFindRes = createSOFindResponse([{ ...userAction, score: 0 }]); + + unsecuredSavedObjectsClient.find.mockResolvedValue({ ...soFindRes, aggregations }); + soSerializerMock.rawToSavedObject.mockReturnValueOnce(userAction); + soSerializerMock.rawToSavedObject.mockReturnValueOnce(pushActionWithExtraAttributes); + + await expect(service.getCaseConnectorInformation('1')).resolves.toEqual({ + attributes: userAction.attributes, + }); + }); + }); + }); + }); }); }); }); From 834183d14463334baea9ac6b5cab2e7ee96fe51b Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Tue, 23 May 2023 22:23:12 +0300 Subject: [PATCH 13/13] PR feedback --- x-pack/plugins/cases/server/services/user_actions/index.ts | 5 ++--- .../server/services/user_actions/operations/find.test.ts | 1 - 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/x-pack/plugins/cases/server/services/user_actions/index.ts b/x-pack/plugins/cases/server/services/user_actions/index.ts index 7960f2f6636c8..15e797df5374f 100644 --- a/x-pack/plugins/cases/server/services/user_actions/index.ts +++ b/x-pack/plugins/cases/server/services/user_actions/index.ts @@ -216,10 +216,9 @@ export class CaseUserActionService { const decodeRes = decodeOrThrow(UserActionTransformedAttributesRt)(res.attributes); - const fieldsDoc = { - ...res, + const fieldsDoc = Object.assign(res, { attributes: decodeRes, - }; + }); connectorFields.set(connectorId, fieldsDoc); } diff --git a/x-pack/plugins/cases/server/services/user_actions/operations/find.test.ts b/x-pack/plugins/cases/server/services/user_actions/operations/find.test.ts index 16a873c784629..9dc4fc2a72425 100644 --- a/x-pack/plugins/cases/server/services/user_actions/operations/find.test.ts +++ b/x-pack/plugins/cases/server/services/user_actions/operations/find.test.ts @@ -47,7 +47,6 @@ describe('UserActionsService: Finder', () => { }; const mockPointInTimeFinder = (soFindRes: SavedObjectsFindResponse) => { - // console.log(soFindRes); unsecuredSavedObjectsClient.createPointInTimeFinder.mockReturnValue({ close: jest.fn(), // @ts-expect-error