[Cases] Limit searchFields in the Find Case API query params (#156495)

Fixes #155978

## Summary

- Remove the `fields` parameter from the find cases API 
- Limit the `searchFields` parameter in the find cases API

One of the FTR tests was actually successfully calling the find_cases
API while passing the fields parameter but only certain combinations of
fields would actually work.

Passing single fields would not work.

I just removed the test as we won't support the `fields` param anymore.

---------

Co-authored-by: lcawl <lcawley@elastic.co>
This commit is contained in:
Antonio 2023-05-11 17:04:10 +02:00 committed by GitHub
parent 44a62f84a2
commit 2e0ba63e31
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 177 additions and 131 deletions

View file

@ -612,9 +612,7 @@ Any modifications made to this file will be overwritten.
<div class="param-desc"><span class="param-type">Query Parameter</span> &mdash; Filters the returned cases by assignees. Valid values are <code>none</code> or unique identifiers for the user profiles. These identifiers can be found by using the suggest user profile API. default: null </div><div class="param">defaultSearchOperator (optional)</div>
<div class="param-desc"><span class="param-type">Query Parameter</span> &mdash; The default operator to use for the simple_query_string. default: OR </div><div class="param">fields (optional)</div>
<div class="param-desc"><span class="param-type">Query Parameter</span> &mdash; The fields in the entity to return in the response. default: null </div><div class="param">from (optional)</div>
<div class="param-desc"><span class="param-type">Query Parameter</span> &mdash; The default operator to use for the simple_query_string. default: OR </div><div class="param">from (optional)</div>
<div class="param-desc"><span class="param-type">Query Parameter</span> &mdash; [preview] Returns only cases that were created after a specific date. The date must be specified as a KQL data range or date match expression. This functionality is in technical preview and may be changed or removed in a future release. Elastic will apply best effort to fix any issues, but features in technical preview are not subject to the support SLA of official GA features. default: null </div><div class="param">owner (optional)</div>
@ -2099,6 +2097,7 @@ Any modifications made to this file will be overwritten.
<li><a href="#findCases_200_response"><code>findCases_200_response</code> - </a></li>
<li><a href="#findCases_assignees_parameter"><code>findCases_assignees_parameter</code> - </a></li>
<li><a href="#findCases_owner_parameter"><code>findCases_owner_parameter</code> - </a></li>
<li><a href="#findCases_searchFields_parameter"><code>findCases_searchFields_parameter</code> - </a></li>
<li><a href="#getCaseComment_200_response"><code>getCaseComment_200_response</code> - </a></li>
<li><a href="#getCaseConfiguration_200_response_inner"><code>getCaseConfiguration_200_response_inner</code> - </a></li>
<li><a href="#getCaseConfiguration_200_response_inner_connector"><code>getCaseConfiguration_200_response_inner_connector</code> - </a></li>
@ -2127,6 +2126,7 @@ Any modifications made to this file will be overwritten.
<li><a href="#payload_user_comment"><code>payload_user_comment</code> - </a></li>
<li><a href="#payload_user_comment_comment"><code>payload_user_comment_comment</code> - </a></li>
<li><a href="#rule"><code>rule</code> - Alerting rule</a></li>
<li><a href="#search_fields"><code>search_fields</code> - </a></li>
<li><a href="#setCaseConfiguration_request"><code>setCaseConfiguration_request</code> - </a></li>
<li><a href="#setCaseConfiguration_request_connector"><code>setCaseConfiguration_request_connector</code> - </a></li>
<li><a href="#setCaseConfiguration_request_settings"><code>setCaseConfiguration_request_settings</code> - </a></li>
@ -2604,6 +2604,12 @@ Any modifications made to this file will be overwritten.
<div class="field-items">
</div> <!-- field-items -->
</div>
<div class="model">
<h3><a name="findCases_searchFields_parameter"><code>findCases_searchFields_parameter</code> - </a> <a class="up" href="#__Models">Up</a></h3>
<div class='model-description'></div>
<div class="field-items">
</div> <!-- field-items -->
</div>
<div class="model">
<h3><a name="getCaseComment_200_response"><code>getCaseComment_200_response</code> - </a> <a class="up" href="#__Models">Up</a></h3>
<div class='model-description'></div>
@ -2872,6 +2878,12 @@ Any modifications made to this file will be overwritten.
<div class="param">name (optional)</div><div class="param-desc"><span class="param-type"><a href="#string">String</a></span> The rule name. </div>
</div> <!-- field-items -->
</div>
<div class="model">
<h3><a name="search_fields"><code>search_fields</code> - </a> <a class="up" href="#__Models">Up</a></h3>
<div class='model-description'></div>
<div class="field-items">
</div> <!-- field-items -->
</div>
<div class="model">
<h3><a name="setCaseConfiguration_request"><code>setCaseConfiguration_request</code> - </a> <a class="up" href="#__Models">Up</a></h3>
<div class='model-description'></div>

View file

@ -43,9 +43,6 @@ identifiers can be found by using the
(Optional, string) The default operator to use for the `simple_query_string`.
Defaults to `OR`.
`fields`::
(Optional, array of strings) The fields in the entity to return in the response.
`from`::
(Optional, string) Returns only cases that were created after a specific date. The date must be specified as a <<kuery-query,KQL>> data range or date match expression. preview:[]

View file

@ -164,6 +164,34 @@ 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,
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 CasesFindRequestRt = rt.partial({
/**
* Tags to filter by
@ -189,10 +217,6 @@ export const CasesFindRequestRt = rt.partial({
* Operator to use for the `search` field
*/
defaultSearchOperator: rt.union([rt.literal('AND'), rt.literal('OR')]),
/**
* The fields in the entity to return in the response
*/
fields: rt.union([rt.array(rt.string), rt.string]),
/**
* A KQL date. If used all cases created after (gte) the from date will be returned
*/
@ -212,7 +236,10 @@ export const CasesFindRequestRt = rt.partial({
/**
* The fields to perform the simple_query_string parsed query against
*/
searchFields: rt.union([rt.array(rt.string), rt.string]),
searchFields: rt.union([
rt.array(CasesFindRequestSearchFieldsRt),
CasesFindRequestSearchFieldsRt,
]),
/**
* The root fields to perform the simple_query_string parsed query against
*/

View file

@ -246,17 +246,6 @@
},
"example": "OR"
},
{
"name": "fields",
"in": "query",
"description": "The fields in the entity to return in the response.",
"schema": {
"type": "array",
"items": {
"type": "string"
}
}
},
{
"name": "from",
"in": "query",
@ -323,12 +312,12 @@
"schema": {
"oneOf": [
{
"type": "string"
"$ref": "#/components/schemas/search_fields"
},
{
"type": "array",
"items": {
"type": "string"
"$ref": "#/components/schemas/search_fields"
}
}
]
@ -3464,6 +3453,36 @@
}
}
},
"search_fields": {
"type": "string",
"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`).",

View file

@ -147,13 +147,6 @@ paths:
type: string
default: OR
example: OR
- name: fields
in: query
description: The fields in the entity to return in the response.
schema:
type: array
items:
type: string
- name: from
in: query
description: |
@ -196,10 +189,10 @@ paths:
description: The fields to perform the simple_query_string parsed query against.
schema:
oneOf:
- type: string
- $ref: '#/components/schemas/search_fields'
- type: array
items:
type: string
$ref: '#/components/schemas/search_fields'
- $ref: '#/components/parameters/severity'
- name: sortField
in: query
@ -2289,6 +2282,34 @@ components:
version:
description: The current version of the case. To determine this value, use the get case or find cases APIs.
type: string
search_fields:
type: string
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`).

View file

@ -0,0 +1,27 @@
type: string
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

@ -25,13 +25,6 @@ get:
type: string
default: OR
example: OR
- name: fields
in: query
description: The fields in the entity to return in the response.
schema:
type: array
items:
type: string
- name: from
in: query
description: >
@ -79,10 +72,10 @@ get:
description: The fields to perform the simple_query_string parsed query against.
schema:
oneOf:
- type: string
- $ref: '../components/parameters/search_fields.yaml'
- type: array
items:
type: string
$ref: '../components/parameters/search_fields.yaml'
- $ref: '../components/parameters/severity.yaml'
- name: sortField
in: query

View file

@ -63,4 +63,34 @@ describe('find', () => {
expect(call.caseOptions).not.toHaveProperty('rootSearchFields');
});
});
describe('searchFields errors', () => {
const clientArgs = createCasesClientMockArgs();
beforeEach(() => {
jest.clearAllMocks();
});
it('invalid searchFields with array', async () => {
const searchFields = ['foobar'];
// @ts-expect-error
const findRequest = createCasesClientMockFindRequest({ searchFields });
await expect(find(findRequest, clientArgs)).rejects.toThrow(
'Error: Invalid value "foobar" supplied to "searchFields"'
);
});
it('invalid searchFields with single string', async () => {
const searchFields = 'foobar';
// @ts-expect-error
const findRequest = createCasesClientMockFindRequest({ searchFields });
await expect(find(findRequest, clientArgs)).rejects.toThrow(
'Error: Invalid value "foobar" supplied to "searchFields"'
);
});
});
});

View file

@ -17,7 +17,6 @@ import { CasesFindRequestRt, throwErrors, CasesFindResponseRt, excess } from '..
import { createCaseError } from '../../common/error';
import { asArray, transformCases } from '../../common/utils';
import { constructQueryOptions, constructSearch } from '../utils';
import { includeFieldsRequiredForAuthentication } from '../../authorization/utils';
import { Operations } from '../../authorization';
import type { CasesClientArgs } from '..';
import { LICENSING_CASE_ASSIGNMENT_FEATURE } from '../../common/constants';
@ -41,10 +40,8 @@ export const find = async (
} = clientArgs;
try {
const fields = asArray(params.fields);
const queryParams = pipe(
excess(CasesFindRequestRt).decode({ ...params, fields }),
excess(CasesFindRequestRt).decode({ ...params }),
fold(throwErrors(Boom.badRequest), identity)
);
@ -96,7 +93,6 @@ export const find = async (
...caseQueryOptions,
...caseSearch,
searchFields: asArray(queryParams.searchFields),
fields: includeFieldsRequiredForAuthentication(fields),
},
}),
caseService.getCaseStatusStats({

View file

@ -234,50 +234,6 @@ export default ({ getService }: FtrProviderContext): void => {
expect(cases.count_in_progress_cases).to.eql(1);
});
it('returns the correct fields', async () => {
const postedCase = await createCase(supertest, postCaseReq);
// all fields that contain the UserRt definition must be included here (aka created_by, closed_by, and updated_by)
// see https://github.com/elastic/kibana/issues/139503
const queryFields: Array<keyof Case | Array<keyof Case>> = [
['title', 'created_by', 'closed_by', 'updated_by'],
['title', 'description', 'created_by', 'closed_by', 'updated_by'],
];
for (const fields of queryFields) {
const cases = await findCases({ supertest, query: { fields } });
const fieldsAsArray = Array.isArray(fields) ? fields : [fields];
const expectedValues = fieldsAsArray.reduce(
(theCase, field) => ({
...theCase,
[field]: postedCase[field],
}),
{}
);
expect(cases).to.eql({
...findCasesResp,
total: 1,
cases: [
{
id: postedCase.id,
version: postedCase.version,
external_service: postedCase.external_service,
owner: postedCase.owner,
connector: postedCase.connector,
severity: postedCase.severity,
status: postedCase.status,
comments: [],
totalAlerts: 0,
totalComment: 0,
...expectedValues,
},
],
count_open_cases: 1,
});
}
});
it('sorts by title', async () => {
const case3 = await createCase(supertest, { ...postCaseReq, title: 'c' });
const case2 = await createCase(supertest, { ...postCaseReq, title: 'b' });
@ -810,45 +766,6 @@ export default ({ getService }: FtrProviderContext): void => {
});
}
it('should return the correct cases when trying to exploit RBAC through the search query parameter', async () => {
await Promise.all([
// super user creates a case with owner securitySolutionFixture
createCase(
supertestWithoutAuth,
getPostCaseRequest({ owner: 'securitySolutionFixture' }),
200,
{
user: superUser,
space: 'space1',
}
),
// super user creates a case with owner observabilityFixture
createCase(
supertestWithoutAuth,
getPostCaseRequest({ owner: 'observabilityFixture' }),
200,
{
user: superUser,
space: 'space1',
}
),
]);
const res = await findCases({
supertest: supertestWithoutAuth,
query: {
search: 'securitySolutionFixture observabilityFixture',
searchFields: 'owner',
},
auth: {
user: secOnly,
space: 'space1',
},
});
ensureSavedObjectIsAuthorized(res.cases, 1, ['securitySolutionFixture']);
});
// This test is to prevent a future developer to add the filter attribute without taking into consideration
// the authorizationFilter produced by the cases authorization class
it('should NOT allow to pass a filter query parameter', async () => {
@ -861,6 +778,14 @@ export default ({ getService }: FtrProviderContext): void => {
.expect(400);
});
it('should NOT allow to pass non-valid fields query parameter', async () => {
await findCases({
supertest,
query: { searchFields: ['foobar'], search: 'some search string*' },
expectedHttpCode: 400,
});
});
// This test ensures that the user is not allowed to define the namespaces query param
// so she cannot search across spaces
it('should NOT allow to pass a namespaces query parameter', async () => {
@ -911,7 +836,6 @@ export default ({ getService }: FtrProviderContext): void => {
supertest: supertestWithoutAuth,
query: {
owner: 'securitySolutionFixture',
searchFields: 'owner',
},
auth: {
user: obsSec,