Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[9.0] [Response Ops][Cases] Cases with empty string assignees throwing error (#209973) #210733

Merged
merged 1 commit into from
Feb 12, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,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,
});
};

// FLAKY: https://github.com/elastic/kibana/issues/192739
describe.skip('AllCasesListGeneric', () => {
const onRowClick = jest.fn();
Expand All @@ -113,48 +151,18 @@ describe.skip('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(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,29 @@ describe('update', () => {
Operations.updateCase,
]);
});

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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,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.
Expand Down Expand Up @@ -384,7 +385,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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,22 @@ describe('create', () => {
})
);
});

it('should filter out empty assignees', async () => {
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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

/**
Expand All @@ -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;

Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}),
};
};
Original file line number Diff line number Diff line change
@@ -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,
});
});
});
};
Original file line number Diff line number Diff line change
@@ -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 }]);
});
});
};
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,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
Expand Down