[Cases] Restrict searchFields, sortField and remove rootSearchFields from find_cases API (#162245)

## Summary

This PR modifies` find_cases API` with below changes
- [x] Restrict `searchFields` only to the `title` and the `description`
- [x] Restrict `sortField` to: `title`, `category`, `createdAt`,
`updatedAt`, `status`, and `severity`
- [x] Remove `rootSearchFields`

### Checklist

Delete any items that are not applicable to this PR.

- [x]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

### For maintainers

- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
This commit is contained in:
Janki Salvi 2023-07-24 09:24:05 +02:00 committed by GitHub
parent 800f8dded3
commit 0af4e7e113
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
14 changed files with 176 additions and 162 deletions

View file

@ -5,6 +5,7 @@
* 2.0.
*/
import { PathReporter } from 'io-ts/lib/PathReporter';
import { ConnectorTypes } from '../../types/domain/connector/v1';
import {
RelatedCaseInfoRt,
@ -25,6 +26,8 @@ import {
CasesRt,
CasesFindResponseRt,
CaseResolveResponseRt,
CasesFindRequestSearchFieldsRt,
CasesFindRequestSortFieldsRt,
} from './case';
import { CommentType } from './comment';
import { CaseStatuses } from './status';
@ -336,11 +339,10 @@ describe('Case', () => {
page: '1',
perPage: '10',
search: 'search text',
searchFields: 'closed_by.username',
rootSearchFields: ['_id'],
searchFields: ['title', 'description'],
to: '1w',
sortOrder: 'desc',
sortField: 'created_at',
sortField: 'createdAt',
owner: 'cases',
};
@ -361,6 +363,58 @@ describe('Case', () => {
right: { ...defaultRequest, page: 1, perPage: 10 },
});
});
const searchFields = Object.keys(CasesFindRequestSearchFieldsRt.keys);
it.each(searchFields)('succeeds with %s as searchFields', (field) => {
const query = CasesFindRequestRt.decode({ ...defaultRequest, searchFields: field });
expect(query).toStrictEqual({
_tag: 'Right',
right: { ...defaultRequest, searchFields: field, page: 1, perPage: 10 },
});
});
const sortFields = Object.keys(CasesFindRequestSortFieldsRt.keys);
it.each(sortFields)('succeeds with %s as sortField', (sortField) => {
const query = CasesFindRequestRt.decode({ ...defaultRequest, sortField });
expect(query).toStrictEqual({
_tag: 'Right',
right: { ...defaultRequest, sortField, page: 1, perPage: 10 },
});
});
it('removes rootSearchField when passed', () => {
expect(
PathReporter.report(
CasesFindRequestRt.decode({ ...defaultRequest, rootSearchField: ['foobar'] })
)
).toContain('No errors!');
});
describe('errors', () => {
it('throws error when invalid searchField passed', () => {
expect(
PathReporter.report(
CasesFindRequestRt.decode({ ...defaultRequest, searchFields: 'foobar' })
)
).not.toContain('No errors!');
});
it('throws error when invalid sortField passed', () => {
expect(
PathReporter.report(CasesFindRequestRt.decode({ ...defaultRequest, sortField: 'foobar' }))
).not.toContain('No errors!');
});
it('succeeds when valid parameters passed', () => {
expect(PathReporter.report(CasesFindRequestRt.decode(defaultRequest))).toContain(
'No errors!'
);
});
});
});
describe('CasesByAlertIDRequestRt', () => {

View file

@ -205,32 +205,19 @@ export const CasePostRequestRt = rt.intersection([
),
]);
const CasesFindRequestSearchFieldsRt = rt.keyof({
'closed_by.username': null,
'closed_by.full_name': null,
'closed_by.email': null,
'closed_by.profile_uid': null,
'created_by.username': null,
'created_by.full_name': null,
'created_by.email': null,
'created_by.profile_uid': null,
export const CasesFindRequestSearchFieldsRt = rt.keyof({
description: null,
'connector.name': null,
'connector.type': null,
'external_service.pushed_by.username': null,
'external_service.pushed_by.full_name': null,
'external_service.pushed_by.email': null,
'external_service.pushed_by.profile_uid': null,
'external_service.connector_name': null,
'external_service.external_id': null,
'external_service.external_title': null,
'external_service.external_url': null,
title: null,
'title.keyword': null,
'updated_by.username': null,
'updated_by.full_name': null,
'updated_by.email': null,
'updated_by.profile_uid': null,
});
export const CasesFindRequestSortFieldsRt = rt.keyof({
title: null,
category: null,
createdAt: null,
updatedAt: null,
closedAt: null,
status: null,
severity: null,
});
export const CasesFindRequestRt = rt.intersection([
@ -307,15 +294,11 @@ export const CasesFindRequestRt = rt.intersection([
rt.array(CasesFindRequestSearchFieldsRt),
CasesFindRequestSearchFieldsRt,
]),
/**
* The root fields to perform the simple_query_string parsed query against
*/
rootSearchFields: rt.array(rt.string),
/**
* The field to use for sorting the found objects.
*
*/
sortField: rt.string,
sortField: CasesFindRequestSortFieldsRt,
/**
* The order to sort by
*/
@ -551,6 +534,7 @@ export type Cases = rt.TypeOf<typeof CasesRt>;
export type CasesDeleteRequest = rt.TypeOf<typeof CasesDeleteRequestRt>;
export type CasesByAlertIDRequest = rt.TypeOf<typeof CasesByAlertIDRequestRt>;
export type CasesFindRequest = rt.TypeOf<typeof CasesFindRequestRt>;
export type CasesFindRequestSortFields = rt.TypeOf<typeof CasesFindRequestSortFieldsRt>;
export type CasesFindResponse = rt.TypeOf<typeof CasesFindResponseRt>;
export type CasePatchRequest = rt.TypeOf<typeof CasePatchRequestRt>;
export type CasesPatchRequest = rt.TypeOf<typeof CasesPatchRequestRt>;

View file

@ -12,18 +12,26 @@
"url": "https://www.elastic.co/licensing/elastic-license"
}
},
"tags": [
{
"name": "cases",
"description": "Case APIs enable you to open and track issues."
}
],
"servers": [
{
"url": "http://localhost:5601",
"description": "local"
}
],
"security": [
{
"basicAuth": []
},
{
"apiKeyAuth": []
}
],
"tags": [
{
"name": "cases",
"description": "Case APIs enable you to open and track issues."
}
],
"paths": {
"/api/cases": {
"post": {
@ -3977,7 +3985,12 @@
"type": "string",
"enum": [
"createdAt",
"updatedAt"
"updatedAt",
"closedAt",
"title",
"category",
"status",
"severity"
],
"default": "createdAt"
},
@ -5286,31 +5299,8 @@
"type": "string",
"description": "The fields to perform the `simple_query_string` parsed query against.",
"enum": [
"closed_by.username",
"closed_by.full_name",
"closed_by.email",
"closed_by.profile_uid",
"created_by.username",
"created_by.full_name",
"created_by.email",
"created_by.profile_uid",
"description",
"connector.name",
"connector.type",
"external_service.pushed_by.username",
"external_service.pushed_by.full_name",
"external_service.pushed_by.email",
"external_service.pushed_by.profile_uid",
"external_service.connector_name",
"external_service.external_id",
"external_service.external_title",
"external_service.external_url",
"title",
"title.keyword",
"updated_by.username",
"updated_by.full_name",
"updated_by.email",
"updated_by.profile_uid"
"title"
]
},
"closure_types": {
@ -7151,13 +7141,5 @@
]
}
}
},
"security": [
{
"basicAuth": []
},
{
"apiKeyAuth": []
}
]
}
}

View file

@ -8,12 +8,15 @@ info:
license:
name: Elastic License 2.0
url: https://www.elastic.co/licensing/elastic-license
tags:
- name: cases
description: Case APIs enable you to open and track issues.
servers:
- url: http://localhost:5601
description: local
security:
- basicAuth: []
- apiKeyAuth: []
tags:
- name: cases
description: Case APIs enable you to open and track issues.
paths:
/api/cases:
post:
@ -2427,6 +2430,11 @@ components:
enum:
- createdAt
- updatedAt
- closedAt
- title
- category
- status
- severity
default: createdAt
example: updatedAt
sort_order:
@ -3392,31 +3400,8 @@ components:
type: string
description: The fields to perform the `simple_query_string` parsed query against.
enum:
- closed_by.username
- closed_by.full_name
- closed_by.email
- closed_by.profile_uid
- created_by.username
- created_by.full_name
- created_by.email
- created_by.profile_uid
- description
- connector.name
- connector.type
- external_service.pushed_by.username
- external_service.pushed_by.full_name
- external_service.pushed_by.email
- external_service.pushed_by.profile_uid
- external_service.connector_name
- external_service.external_id
- external_service.external_title
- external_service.external_url
- title
- title.keyword
- updated_by.username
- updated_by.full_name
- updated_by.email
- updated_by.profile_uid
closure_types:
type: string
description: Indicates whether a case is automatically closed when it is pushed to external systems (`close-by-pushing`) or not automatically closed (`close-by-user`).
@ -4768,6 +4753,3 @@ components:
isPreconfigured: false
isDeprecated: false
referencedByCount: 0
security:
- basicAuth: []
- apiKeyAuth: []

View file

@ -1,28 +1,5 @@
type: string
description: The fields to perform the `simple_query_string` parsed query against.
enum:
- closed_by.username
- closed_by.full_name
- closed_by.email
- closed_by.profile_uid
- created_by.username
- created_by.full_name
- created_by.email
- created_by.profile_uid
- description
- connector.name
- connector.type
- external_service.pushed_by.username
- external_service.pushed_by.full_name
- external_service.pushed_by.email
- external_service.pushed_by.profile_uid
- external_service.connector_name
- external_service.external_id
- external_service.external_title
- external_service.external_url
- title
- title.keyword
- updated_by.username
- updated_by.full_name
- updated_by.email
- updated_by.profile_uid

View file

@ -6,5 +6,10 @@ schema:
enum:
- createdAt
- updatedAt
- closedAt
- title
- category
- status
- severity
default: createdAt
example: updatedAt

View file

@ -55,8 +55,7 @@ describe('find', () => {
const call = clientArgs.services.caseService.findCasesGroupedByID.mock.calls[0][0];
expect(call.caseOptions.search).toBe(`"${search}" "cases:${search}"`);
expect(call.caseOptions).toHaveProperty('rootSearchFields');
expect(call.caseOptions.rootSearchFields).toStrictEqual(['_id']);
expect(call.caseOptions).toHaveProperty('rootSearchFields', ['_id']);
});
it('regular search term does not cause rootSearchFields to be appended', async () => {
@ -70,8 +69,16 @@ describe('find', () => {
expect(call.caseOptions.search).toBe(search);
expect(call.caseOptions).not.toHaveProperty('rootSearchFields');
});
});
it('should not have foo:bar attribute in request payload', async () => {
describe('errors', () => {
const clientArgs = createCasesClientMockArgs();
beforeEach(() => {
jest.clearAllMocks();
});
it('when foo:bar attribute in request payload', async () => {
const search = 'sample_text';
const findRequest = createCasesClientMockFindRequest({ search });
await expect(
@ -81,14 +88,6 @@ describe('find', () => {
`"Failed to find cases: {\\"search\\":\\"sample_text\\",\\"searchFields\\":[\\"title\\",\\"description\\"],\\"severity\\":\\"low\\",\\"assignees\\":[],\\"reporters\\":[],\\"status\\":\\"open\\",\\"tags\\":[],\\"owner\\":[],\\"sortField\\":\\"createdAt\\",\\"sortOrder\\":\\"desc\\",\\"foo\\":\\"bar\\"}: Error: invalid keys \\"foo\\""`
);
});
});
describe('errors', () => {
const clientArgs = createCasesClientMockArgs();
beforeEach(() => {
jest.clearAllMocks();
});
it('invalid searchFields with array', async () => {
const searchFields = ['foobar'];
@ -112,6 +111,17 @@ describe('find', () => {
);
});
it('invalid sortField', async () => {
const sortField = 'foobar';
// @ts-expect-error
const findRequest = createCasesClientMockFindRequest({ sortField });
await expect(find(findRequest, clientArgs)).rejects.toThrow(
'Error: Invalid value "foobar" supplied to "sortField"'
);
});
it(`throws an error when the category array has ${MAX_CATEGORY_FILTER_LENGTH} items`, async () => {
const category = Array(MAX_CATEGORY_FILTER_LENGTH + 1).fill('foobar');

View file

@ -79,7 +79,7 @@ export const find = async (
const queryArgs: CasesFindQueryParams = {
tags: queryParams.tags,
reporters: queryParams.reporters,
sortByField: queryParams.sortField,
sortField: queryParams.sortField,
status: queryParams.status,
severity: queryParams.severity,
owner: queryParams.owner,

View file

@ -72,5 +72,6 @@ export type CasesFindQueryParams = Partial<
| 'to'
| 'assignees'
| 'category'
> & { sortByField?: string; authorizationFilter?: KueryNode }
| 'sortField'
> & { authorizationFilter?: KueryNode }
>;

View file

@ -133,14 +133,6 @@ describe('utils', () => {
expect(convertSortField('createdAt')).toBe('created_at');
});
it('transforms created_at correctly', () => {
expect(convertSortField('created_at')).toBe('created_at');
});
it('transforms updated_at correctly', () => {
expect(convertSortField('updated_at')).toBe('updated_at');
});
it('transforms updatedAt correctly', () => {
expect(convertSortField('updatedAt')).toBe('updated_at');
});
@ -149,16 +141,12 @@ describe('utils', () => {
expect(convertSortField('closedAt')).toBe('closed_at');
});
it('transforms closed_at correctly', () => {
expect(convertSortField('closed_at')).toBe('closed_at');
});
it('transforms title correctly', () => {
expect(convertSortField('title')).toBe('title.keyword');
});
it('transforms default correctly', () => {
expect(convertSortField('not-exist')).toBe('created_at');
expect(convertSortField(undefined)).toBe('created_at');
});
});

View file

@ -21,7 +21,7 @@ import type {
CommentRequest,
CaseSeverity,
CommentRequestExternalReferenceType,
CasesFindRequest,
CasesFindRequestSortFields,
} from '../../common/api';
import type { SavedObjectFindOptionsKueryNode } from '../common/types';
import type { CasesFindQueryParams } from './types';
@ -339,7 +339,7 @@ export const constructQueryOptions = ({
reporters,
status,
severity,
sortByField,
sortField,
owner,
authorizationFilter,
from,
@ -349,7 +349,7 @@ export const constructQueryOptions = ({
}: CasesFindQueryParams): SavedObjectFindOptionsKueryNode => {
const tagsFilter = buildFilter({ filters: tags, field: 'tags', operator: 'or' });
const reportersFilter = createReportersFilter(reporters);
const sortField = convertSortField(sortByField);
const sortByField = convertSortField(sortField);
const ownerFilter = buildFilter({ filters: owner, field: OWNER_FIELD, operator: 'or' });
const statusFilter = status != null ? addStatusFilter(status) : undefined;
const severityFilter = severity != null ? addSeverityFilter(severity) : undefined;
@ -370,7 +370,7 @@ export const constructQueryOptions = ({
return {
filter: combineFilterWithAuthorizationFilter(filters, authorizationFilter),
sortField,
sortField: sortByField,
};
};
@ -475,22 +475,21 @@ enum SortFieldCase {
category = 'category',
}
export const convertSortField = (sortField: string | undefined): SortFieldCase => {
export const convertSortField = (
sortField: CasesFindRequestSortFields | undefined
): SortFieldCase => {
switch (sortField) {
case 'status':
return SortFieldCase.status;
case 'createdAt':
case 'created_at':
return SortFieldCase.createdAt;
case 'closedAt':
case 'closed_at':
return SortFieldCase.closedAt;
case 'title':
return SortFieldCase.title;
case 'severity':
return SortFieldCase.severity;
case 'updatedAt':
case 'updated_at':
return SortFieldCase.updatedAt;
case 'category':
return SortFieldCase.category;
@ -503,7 +502,7 @@ export const constructSearch = (
search: string | undefined,
spaceId: string,
savedObjectsSerializer: ISavedObjectsSerializer
): Pick<CasesFindRequest, 'search' | 'rootSearchFields'> | undefined => {
): { search: string; rootSearchFields?: string[] } | undefined => {
if (!search) {
return undefined;
}

View file

@ -77,7 +77,7 @@ describe('useCaseItems', () => {
from: '2020-07-07T08:20:18.966Z',
to: '2020-07-08T08:20:18.966Z',
owner: 'securitySolution',
sortField: 'create_at',
sortField: 'createdAt',
sortOrder: 'desc',
page: 1,
perPage: 4,

View file

@ -57,7 +57,7 @@ export const useCaseItems: UseCaseItems = ({ skip }) => {
from,
to,
owner: APP_ID,
sortField: 'create_at',
sortField: 'createdAt',
sortOrder: 'desc',
page: 1,
perPage: 4,

View file

@ -394,7 +394,7 @@ export default ({ getService }: FtrProviderContext): void => {
expect(cases.cases[0].title).to.equal(uuid);
});
it('should successfully find a case with a valid uuid in title', async () => {
it('should successfully find a case with a valid uuid in description', async () => {
const uuid = uuidv1();
await createCase(supertest, { ...postCaseReq, description: uuid });
@ -459,6 +459,38 @@ export default ({ getService }: FtrProviderContext): void => {
});
}
it('400s when trying to fetch with invalid searchField', async () => {
await findCases({
supertest,
query: { searchFields: 'closed_by.username', search: 'some search string*' },
expectedHttpCode: 400,
});
});
it('400s when trying to fetch with invalid array of searchFields', async () => {
await findCases({
supertest,
query: { searchFields: ['closed_by.username', 'title'], search: 'some search string*' },
expectedHttpCode: 400,
});
});
it('400s when trying to fetch with invalid sortField', async () => {
await findCases({
supertest,
query: { sortField: 'foobar', search: 'some search string*' },
expectedHttpCode: 400,
});
});
it('400s when trying to fetch with rootSearchFields', async () => {
await findCases({
supertest,
query: { rootSearchFields: ['_id'], search: 'some search string*' },
expectedHttpCode: 400,
});
});
it(`400s when perPage > ${MAX_CASES_PER_PAGE} supplied`, async () => {
await findCases({
supertest,