[8.16] [Response Ops][Cases] Cases with empty string assignees throwi…

…ng error (#209973) (#210768) # Backport This will backport the following commits from `main` to `8.16`: - [[Response Ops][Cases] Cases with empty string assignees throwing error (#209973)](#209973)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) \n\n### Questions ?\nPlease refer to the [Backport tool\ndocumentation](https://github.com/sqren/backport)\n\n\n\nCo-authored-by: Julian Gernun <[email protected]>"}},{"branch":"8.18","label":"v8.18.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"url":"https://github.com/elastic/kibana/pull/210731","number":210731,"state":"OPEN"},{"branch":"8.16","label":"v8.16.4","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.17","label":"v8.17.2","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"url":"https://github.com/elastic/kibana/pull/210755","number":210755,"state":"OPEN"},{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/209973","number":209973,"mergeCommit":{"message":"[Response Ops][Cases] Cases with empty string assignees throwing error (#209973)\n\n## Summary\nCloses https://github.com/elastic/kibana/issues/209950\n\nTesting steps in referenced issue\n\n## Release note\nFix error message in cases list if case assignee is an empty string","sha":"0897d0878622db9d9747e2bc1292d647c09ad996"}},{"branch":"8.x","label":"v8.19.0","branchLabelMappingKey":"^v8.19.0$","isSourceBranch":false,"url":"https://github.com/elastic/kibana/pull/210732","number":210732,"state":"MERGED","mergeCommit":{"sha":"8b98614e9dbdaaeaebac56d633cd2b7c35af01bd","message":"[8.x] [Response Ops][Cases] Cases with empty string assignees throwing error (#209973) (#210732)\n\n# Backport\n\nThis will backport the following commits from `main` to `8.x`:\n- [[Response Ops][Cases] Cases with empty string assignees throwing\nerror (#209973)](https://github.com/elastic/kibana/pull/209973)\n\n\n\n### Questions ?\nPlease refer to the [Backport tool\ndocumentation](https://github.com/sqren/backport)\n\n\n\nCo-authored-by: Julian Gernun <[email protected]>"}}]}] BACKPORT-->
elastic · Feb 12, 2025 · 1621d92 · 1621d92
1 parent cbde7c5
commit 1621d92
Show file tree

Hide file tree

Showing 11 changed files with 231 additions and 36 deletions.
diff --git a/x-pack/plugins/cases/public/components/all_cases/all_cases_list.test.tsx b/x-pack/plugins/cases/public/components/all_cases/all_cases_list.test.tsx
@@ -92,6 +92,44 @@ const mockKibana = () => {
   } as unknown as ReturnType<typeof useKibana>);
 };
 
+// eslint-disable-next-line prefer-object-spread
+const originalGetComputedStyle = Object.assign({}, window.getComputedStyle);
+
+const restoreGetComputedStyle = () => {
+  Object.defineProperty(window, 'getComputedStyle', originalGetComputedStyle);
+};
+
+const patchGetComputedStyle = () => {
+  // The JSDOM implementation is too slow
+  // Especially for dropdowns that try to position themselves
+  // perf issue - https://github.com/jsdom/jsdom/issues/3234
+  Object.defineProperty(window, 'getComputedStyle', {
+    value: (el: HTMLElement) => {
+      /**
+       * This is based on the jsdom implementation of getComputedStyle
+       * https://github.com/jsdom/jsdom/blob/9dae17bf0ad09042cfccd82e6a9d06d3a615d9f4/lib/jsdom/browser/Window.js#L779-L820
+       *
+       * It is missing global style parsing and will only return styles applied directly to an element.
+       * Will not return styles that are global or from emotion
+       */
+      const declaration = new CSSStyleDeclaration();
+      const { style } = el;
+
+      Array.prototype.forEach.call(style, (property: string) => {
+        declaration.setProperty(
+          property,
+          style.getPropertyValue(property),
+          style.getPropertyPriority(property)
+        );
+      });
+
+      return declaration;
+    },
+    configurable: true,
+    writable: true,
+  });
+};
+
 describe('AllCasesListGeneric', () => {
   const onRowClick = jest.fn();
   const updateCaseProperty = jest.fn();
@@ -114,48 +152,18 @@ describe('AllCasesListGeneric', () => {
   };
 
   const removeMsFromDate = (value: string) => moment(value).format('YYYY-MM-DDTHH:mm:ss[Z]');
-  // eslint-disable-next-line prefer-object-spread
-  const originalGetComputedStyle = Object.assign({}, window.getComputedStyle);
 
   let appMockRenderer: AppMockRenderer;
 
   beforeAll(() => {
-    // The JSDOM implementation is too slow
-    // Especially for dropdowns that try to position themselves
-    // perf issue - https://github.com/jsdom/jsdom/issues/3234
-    Object.defineProperty(window, 'getComputedStyle', {
-      value: (el: HTMLElement) => {
-        /**
-         * This is based on the jsdom implementation of getComputedStyle
-         * https://github.com/jsdom/jsdom/blob/9dae17bf0ad09042cfccd82e6a9d06d3a615d9f4/lib/jsdom/browser/Window.js#L779-L820
-         *
-         * It is missing global style parsing and will only return styles applied directly to an element.
-         * Will not return styles that are global or from emotion
-         */
-        const declaration = new CSSStyleDeclaration();
-        const { style } = el;
-
-        Array.prototype.forEach.call(style, (property: string) => {
-          declaration.setProperty(
-            property,
-            style.getPropertyValue(property),
-            style.getPropertyPriority(property)
-          );
-        });
-
-        return declaration;
-      },
-      configurable: true,
-      writable: true,
-    });
-
+    patchGetComputedStyle();
     mockKibana();
     const actionTypeRegistry = useKibanaMock().services.triggersActionsUi.actionTypeRegistry;
     registerConnectorsToMockActionRegistry(actionTypeRegistry, connectorsMock);
   });
 
   afterAll(() => {
-    Object.defineProperty(window, 'getComputedStyle', originalGetComputedStyle);
+    restoreGetComputedStyle();
   });
 
   beforeEach(() => {

diff --git a/x-pack/plugins/cases/public/containers/user_profiles/api.test.ts b/x-pack/plugins/cases/public/containers/user_profiles/api.test.ts
@@ -66,6 +66,15 @@ describe('User profiles API', () => {
       expect(res).toEqual(userProfiles);
     });
 
+    it('should filter out empty user profiles', async () => {
+      const res = await bulkGetUserProfiles({
+        security,
+        uids: [...userProfilesIds, ''],
+      });
+
+      expect(res).toEqual(userProfiles);
+    });
+
     it('calls bulkGet correctly', async () => {
       await bulkGetUserProfiles({
         security,

diff --git a/x-pack/plugins/cases/public/containers/user_profiles/api.ts b/x-pack/plugins/cases/public/containers/user_profiles/api.ts
@@ -8,6 +8,7 @@
 import type { HttpStart } from '@kbn/core/public';
 import type { UserProfile } from '@kbn/security-plugin/common';
 import type { SecurityPluginStart } from '@kbn/security-plugin/public';
+import { isEmpty } from 'lodash';
 import { INTERNAL_SUGGEST_USER_PROFILES_URL, DEFAULT_USER_SIZE } from '../../../common/constants';
 
 export interface SuggestUserProfilesArgs {
@@ -42,11 +43,12 @@ export const bulkGetUserProfiles = async ({
   security,
   uids,
 }: BulkGetUserProfilesArgs): Promise<UserProfile[]> => {
-  if (uids.length === 0) {
+  const cleanUids: string[] = uids.filter((uid) => !isEmpty(uid));
+  if (cleanUids.length === 0) {
     return [];
   }
 
-  return security.userProfiles.bulkGet({ uids: new Set(uids), dataPath: 'avatar' });
+  return security.userProfiles.bulkGet({ uids: new Set(cleanUids), dataPath: 'avatar' });
 };
 
 export interface GetCurrentUserProfileArgs {

diff --git a/x-pack/plugins/cases/server/client/cases/bulk_update.test.ts b/x-pack/plugins/cases/server/client/cases/bulk_update.test.ts
@@ -314,6 +314,29 @@ describe('update', () => {
         'Failed to update case, ids: [{"id":"mock-id-1","version":"WzAsMV0="}]: Error: The length of the field assignees is too long. Array must be of length <= 10.'
       );
     });
+
+    it('should filter out empty user profiles', async () => {
+      const casesWithEmptyAssignee = {
+        cases: [
+          {
+            ...cases.cases[0],
+            assignees: [{ uid: '' }, { uid: '2' }],
+          },
+        ],
+      };
+      await bulkUpdate(casesWithEmptyAssignee, clientArgs, casesClientMock);
+      expect(clientArgs.services.caseService.patchCases).toHaveBeenCalledWith(
+        expect.objectContaining({
+          cases: expect.arrayContaining([
+            expect.objectContaining({
+              updatedAttributes: expect.objectContaining({
+                assignees: [{ uid: '2' }],
+              }),
+            }),
+          ]),
+        })
+      );
+    });
   });
 
   describe('Category', () => {

diff --git a/x-pack/plugins/cases/server/client/cases/bulk_update.ts b/x-pack/plugins/cases/server/client/cases/bulk_update.ts
@@ -60,6 +60,7 @@ import type {
 import { CasesPatchRequestRt } from '../../../common/types/api';
 import { CasesRt, CaseStatuses, AttachmentType } from '../../../common/types/domain';
 import { validateCustomFields } from './validators';
+import { emptyCasesAssigneesSanitizer } from './sanitizers';
 
 /**
  * Throws an error if any of the requests attempt to update the owner of a case.
@@ -328,7 +329,8 @@ export const bulkUpdate = async (
   } = clientArgs;
 
   try {
-    const query = decodeWithExcessOrThrow(CasesPatchRequestRt)(cases);
+    const rawQuery = decodeWithExcessOrThrow(CasesPatchRequestRt)(cases);
+    const query = emptyCasesAssigneesSanitizer(rawQuery);
     const caseIds = query.cases.map((q) => q.id);
     const myCases = await caseService.getCases({
       caseIds,

diff --git a/x-pack/plugins/cases/server/client/cases/create.test.ts b/x-pack/plugins/cases/server/client/cases/create.test.ts
@@ -116,6 +116,23 @@ describe('create', () => {
         `Failed to create case: Error: In order to assign users to cases, you must be subscribed to an Elastic Platinum license`
       );
     });
+
+    it('should filter out empty assignees', async () => {
+      clientArgs.services.licensingService.isAtLeastPlatinum.mockResolvedValue(true);
+      await create(
+        { ...theCase, assignees: [{ uid: '' }, { uid: '1' }] },
+        clientArgs,
+        casesClientMock
+      );
+
+      expect(clientArgs.services.caseService.createCase).toHaveBeenCalledWith(
+        expect.objectContaining({
+          attributes: expect.objectContaining({
+            assignees: [{ uid: '1' }],
+          }),
+        })
+      );
+    });
   });
 
   describe('Attributes', () => {

diff --git a/x-pack/plugins/cases/server/client/cases/create.ts b/x-pack/plugins/cases/server/client/cases/create.ts
@@ -21,6 +21,7 @@ import type { CasePostRequest } from '../../../common/types/api';
 import { CasePostRequestRt } from '../../../common/types/api';
 import {} from '../utils';
 import { validateCustomFields } from './validators';
+import { emptyCaseAssigneesSanitizer } from './sanitizers';
 import { normalizeCreateCaseRequest } from './utils';
 
 /**
@@ -40,7 +41,8 @@ export const create = async (
   } = clientArgs;
 
   try {
-    const query = decodeWithExcessOrThrow(CasePostRequestRt)(data);
+    const rawQuery = decodeWithExcessOrThrow(CasePostRequestRt)(data);
+    const query = emptyCaseAssigneesSanitizer(rawQuery);
     const configurations = await casesClient.configure.get({ owner: data.owner });
     const customFieldsConfiguration = configurations[0]?.customFields;
 

diff --git a/x-pack/plugins/cases/server/client/cases/sanitizers.ts b/x-pack/plugins/cases/server/client/cases/sanitizers.ts
@@ -0,0 +1,34 @@
+/*
+ * 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 { isEmpty } from 'lodash';
+import type { CaseUserProfile } from '../../../common/types/domain/user/v1';
+
+export const emptyCaseAssigneesSanitizer = <T extends { assignees?: CaseUserProfile[] }>(
+  theCase: T
+): T => {
+  if (isEmpty(theCase.assignees)) {
+    return theCase;
+  }
+
+  return {
+    ...theCase,
+    assignees: theCase.assignees?.filter(({ uid }) => !isEmpty(uid)),
+  };
+};
+
+export const emptyCasesAssigneesSanitizer = <T extends { assignees?: CaseUserProfile[] }>({
+  cases,
+}: {
+  cases: T[];
+}): { cases: T[] } => {
+  return {
+    cases: cases.map((theCase) => {
+      return emptyCaseAssigneesSanitizer(theCase);
+    }),
+  };
+};
diff --git a/x-pack/test/cases_api_integration/security_and_spaces/tests/trial/cases/patch_case.ts b/x-pack/test/cases_api_integration/security_and_spaces/tests/trial/cases/patch_case.ts
@@ -0,0 +1,54 @@
+/*
+ * 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 expect from '@kbn/expect';
+
+import { postCaseReq, postCaseResp } from '../../../../common/lib/mock';
+import {
+  deleteAllCaseItems,
+  createCase,
+  removeServerGeneratedPropertiesFromCase,
+  updateCase,
+} from '../../../../common/lib/api';
+import { FtrProviderContext } from '../../../../common/ftr_provider_context';
+
+export const defaultUser = { email: null, full_name: null, username: 'elastic' };
+// eslint-disable-next-line import/no-default-export
+export default ({ getService }: FtrProviderContext): void => {
+  const supertest = getService('supertest');
+  const es = getService('es');
+
+  describe('patch_case', () => {
+    afterEach(async () => {
+      await deleteAllCaseItems(es);
+    });
+
+    it('should filter out empty assignee.uid values', async () => {
+      const randomUid = '7f3e9d2a-1b8c-4c5f-9e6d-8f2a4b1d3c7e';
+      const postedCase = await createCase(supertest, postCaseReq);
+      const patchedCases = await updateCase({
+        supertest,
+        params: {
+          cases: [
+            {
+              id: postedCase.id,
+              version: postedCase.version,
+              assignees: [{ uid: '' }, { uid: randomUid }],
+            },
+          ],
+        },
+      });
+
+      const data = removeServerGeneratedPropertiesFromCase(patchedCases[0]);
+      expect(data).to.eql({
+        ...postCaseResp(),
+        assignees: [{ uid: randomUid }],
+        updated_by: defaultUser,
+      });
+    });
+  });
+};
diff --git a/x-pack/test/cases_api_integration/security_and_spaces/tests/trial/cases/post_case.ts b/x-pack/test/cases_api_integration/security_and_spaces/tests/trial/cases/post_case.ts
@@ -0,0 +1,42 @@
+/*
+ * 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 expect from '@kbn/expect';
+
+import { getPostCaseRequest } from '../../../../common/lib/mock';
+import {
+  deleteAllCaseItems,
+  createCase,
+  removeServerGeneratedPropertiesFromCase,
+} from '../../../../common/lib/api';
+import { FtrProviderContext } from '../../../../common/ftr_provider_context';
+
+// eslint-disable-next-line import/no-default-export
+export default ({ getService }: FtrProviderContext): void => {
+  const supertest = getService('supertest');
+  const es = getService('es');
+
+  describe('post_case', () => {
+    afterEach(async () => {
+      await deleteAllCaseItems(es);
+    });
+
+    it('should filter out empty assignee.uid values', async () => {
+      const randomUid = '7f3e9d2a-1b8c-4c5f-9e6d-8f2a4b1d3c7e';
+      const createdCase = await createCase(
+        supertest,
+        getPostCaseRequest({
+          assignees: [{ uid: '' }, { uid: randomUid }],
+        })
+      );
+
+      const data = removeServerGeneratedPropertiesFromCase(createdCase);
+
+      expect(data.assignees).to.eql([{ uid: randomUid }]);
+    });
+  });
+};
diff --git a/x-pack/test/cases_api_integration/security_and_spaces/tests/trial/index.ts b/x-pack/test/cases_api_integration/security_and_spaces/tests/trial/index.ts
@@ -32,6 +32,8 @@ export default ({ loadTestFile, getService }: FtrProviderContext): void => {
     loadTestFile(require.resolve('./cases/user_actions/find_user_actions'));
     loadTestFile(require.resolve('./cases/assignees'));
     loadTestFile(require.resolve('./cases/find_cases'));
+    loadTestFile(require.resolve('./cases/post_case'));
+    loadTestFile(require.resolve('./cases/patch_case'));
     loadTestFile(require.resolve('./configure'));
     loadTestFile(require.resolve('./attachments_framework/registered_persistable_state_trial'));
     // sub privileges are only available with a license above basic