[Cases] Limit perPage param in findComments API (#160042)

## Summary

This PR limits `perPage` param to 100 in `findComments`  API.

### Checklist

- [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

---------

Co-authored-by: lcawl <lcawley@elastic.co>
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
This commit is contained in:
Janki Salvi 2023-06-21 15:00:25 +02:00 committed by GitHub
parent ac5207cca9
commit de3f8fca00
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 66 additions and 16 deletions

View file

@ -556,7 +556,7 @@ Any modifications made to this file will be overwritten.
<div class="param-desc"><span class="param-type">Query Parameter</span> &mdash; The page number to return. default: 1 </div><div class="param">perPage (optional)</div> <div class="param-desc"><span class="param-type">Query Parameter</span> &mdash; The page number to return. default: 1 </div><div class="param">perPage (optional)</div>
<div class="param-desc"><span class="param-type">Query Parameter</span> &mdash; The number of items to return. default: 20 </div><div class="param">sortOrder (optional)</div> <div class="param-desc"><span class="param-type">Query Parameter</span> &mdash; The number of items to return. Limited to 100 items. default: 20 </div><div class="param">sortOrder (optional)</div>
<div class="param-desc"><span class="param-type">Query Parameter</span> &mdash; Determines the sort order. default: desc </div> <div class="param-desc"><span class="param-type">Query Parameter</span> &mdash; Determines the sort order. default: desc </div>
</div> <!-- field-items --> </div> <!-- field-items -->

View file

@ -104,6 +104,7 @@ export const MAX_DOCS_PER_PAGE = 10000 as const;
export const MAX_BULK_GET_ATTACHMENTS = MAX_DOCS_PER_PAGE; export const MAX_BULK_GET_ATTACHMENTS = MAX_DOCS_PER_PAGE;
export const MAX_CONCURRENT_SEARCHES = 10 as const; export const MAX_CONCURRENT_SEARCHES = 10 as const;
export const MAX_BULK_GET_CASES = 1000 as const; export const MAX_BULK_GET_CASES = 1000 as const;
export const MAX_COMMENTS_PER_PAGE = 100 as const;
export const MAX_CATEGORY_FILTER_LENGTH = 100 as const; export const MAX_CATEGORY_FILTER_LENGTH = 100 as const;
/** /**

View file

@ -1852,7 +1852,15 @@
"$ref": "#/components/parameters/page_index" "$ref": "#/components/parameters/page_index"
}, },
{ {
"$ref": "#/components/parameters/page_size" "name": "perPage",
"in": "query",
"description": "The number of items to return. Limited to 100 items.",
"required": false,
"schema": {
"type": "integer",
"default": 20,
"maximum": 100
}
}, },
{ {
"$ref": "#/components/parameters/sort_order" "$ref": "#/components/parameters/sort_order"

View file

@ -1156,7 +1156,14 @@ paths:
parameters: parameters:
- $ref: '#/components/parameters/case_id' - $ref: '#/components/parameters/case_id'
- $ref: '#/components/parameters/page_index' - $ref: '#/components/parameters/page_index'
- $ref: '#/components/parameters/page_size' - name: perPage
in: query
description: The number of items to return. Limited to 100 items.
required: false
schema:
type: integer
default: 20
maximum: 100
- $ref: '#/components/parameters/sort_order' - $ref: '#/components/parameters/sort_order'
- $ref: '#/components/parameters/space_id' - $ref: '#/components/parameters/space_id'
responses: responses:

View file

@ -10,7 +10,14 @@ get:
parameters: parameters:
- $ref: '../components/parameters/case_id.yaml' - $ref: '../components/parameters/case_id.yaml'
- $ref: '../components/parameters/page_index.yaml' - $ref: '../components/parameters/page_index.yaml'
- $ref: '../components/parameters/page_size.yaml' - name: perPage
in: query
description: The number of items to return. Limited to 100 items.
required: false
schema:
type: integer
default: 20
maximum: 100
- $ref: '../components/parameters/sort_order.yaml' - $ref: '../components/parameters/sort_order.yaml'
- $ref: '../components/parameters/space_id.yaml' - $ref: '../components/parameters/space_id.yaml'
responses: responses:

View file

@ -18,12 +18,20 @@ describe('get', () => {
it('Invalid total items results in error', async () => { it('Invalid total items results in error', async () => {
await expect(() => await expect(() =>
findComment({ caseID: 'mock-id', findQueryParams: { page: 2, perPage: 9001 } }, clientArgs) findComment({ caseID: 'mock-id', findQueryParams: { page: 209, perPage: 100 } }, clientArgs)
).rejects.toThrowErrorMatchingInlineSnapshot( ).rejects.toThrowErrorMatchingInlineSnapshot(
`"Failed to find comments case id: mock-id: Error: The number of documents is too high. Paginating through more than 10,000 documents is not possible."` `"Failed to find comments case id: mock-id: Error: The number of documents is too high. Paginating through more than 10,000 documents is not possible."`
); );
}); });
it('Invalid perPage items results in error', async () => {
await expect(() =>
findComment({ caseID: 'mock-id', findQueryParams: { page: 2, perPage: 9001 } }, clientArgs)
).rejects.toThrowErrorMatchingInlineSnapshot(
`"Failed to find comments case id: mock-id: Error: The provided perPage value was too high. The maximum allowed perPage value is 100."`
);
});
it('throws with excess fields', async () => { it('throws with excess fields', async () => {
await expect( await expect(
findComment( findComment(

View file

@ -6,10 +6,13 @@
*/ */
import { validateFindCommentsPagination } from './validators'; import { validateFindCommentsPagination } from './validators';
import { MAX_COMMENTS_PER_PAGE } from '../../../common/constants';
const ERROR_MSG = const ERROR_MSG =
'The number of documents is too high. Paginating through more than 10,000 documents is not possible.'; 'The number of documents is too high. Paginating through more than 10,000 documents is not possible.';
const ERROR_MSG_PER_PAGE = `The provided perPage value was too high. The maximum allowed perPage value is ${MAX_COMMENTS_PER_PAGE}.`;
describe('validators', () => { describe('validators', () => {
describe('validateFindCommentsPagination', () => { describe('validateFindCommentsPagination', () => {
it('does not throw if only page is undefined', () => { it('does not throw if only page is undefined', () => {
@ -20,20 +23,30 @@ describe('validators', () => {
expect(() => validateFindCommentsPagination({ page: 100 })).not.toThrowError(); expect(() => validateFindCommentsPagination({ page: 100 })).not.toThrowError();
}); });
it('does not throw if page and perPage are defined and valid', () => {
expect(() => validateFindCommentsPagination({ page: 2, perPage: 100 })).not.toThrowError();
});
it('returns if page and perPage are undefined', () => { it('returns if page and perPage are undefined', () => {
expect(() => validateFindCommentsPagination({})).not.toThrowError(); expect(() => validateFindCommentsPagination({})).not.toThrowError();
}); });
it('returns if perPage < 0', () => {
expect(() => validateFindCommentsPagination({ perPage: -1 })).not.toThrowError();
});
it('throws if page > 10k', () => { it('throws if page > 10k', () => {
expect(() => validateFindCommentsPagination({ page: 10001 })).toThrow(ERROR_MSG); expect(() => validateFindCommentsPagination({ page: 10001 })).toThrow(ERROR_MSG);
}); });
it('throws if perPage > 10k', () => { it('throws if perPage > 100', () => {
expect(() => validateFindCommentsPagination({ perPage: 10001 })).toThrowError(ERROR_MSG); expect(() =>
validateFindCommentsPagination({ perPage: MAX_COMMENTS_PER_PAGE + 1 })
).toThrowError(ERROR_MSG_PER_PAGE);
}); });
it('throws if page * perPage > 10k', () => { it('throws if page * perPage > 10k', () => {
expect(() => validateFindCommentsPagination({ page: 10, perPage: 1001 })).toThrow(ERROR_MSG); expect(() => validateFindCommentsPagination({ page: 101, perPage: 100 })).toThrow(ERROR_MSG);
}); });
}); });
}); });

View file

@ -6,7 +6,7 @@
*/ */
import Boom from '@hapi/boom'; import Boom from '@hapi/boom';
import { MAX_DOCS_PER_PAGE } from '../../../common/constants'; import { MAX_DOCS_PER_PAGE, MAX_COMMENTS_PER_PAGE } from '../../../common/constants';
import { import {
isCommentRequestTypeExternalReference, isCommentRequestTypeExternalReference,
isCommentRequestTypePersistableState, isCommentRequestTypePersistableState,
@ -51,7 +51,13 @@ export const validateFindCommentsPagination = (params?: FindCommentsQueryParams)
const pageAsNumber = params.page ?? 0; const pageAsNumber = params.page ?? 0;
const perPageAsNumber = params.perPage ?? 0; const perPageAsNumber = params.perPage ?? 0;
if (Math.max(pageAsNumber, perPageAsNumber, pageAsNumber * perPageAsNumber) > MAX_DOCS_PER_PAGE) { if (perPageAsNumber > MAX_COMMENTS_PER_PAGE) {
throw Boom.badRequest(
`The provided perPage value was too high. The maximum allowed perPage value is ${MAX_COMMENTS_PER_PAGE}.`
);
}
if (Math.max(pageAsNumber, pageAsNumber * perPageAsNumber) > MAX_DOCS_PER_PAGE) {
throw Boom.badRequest( throw Boom.badRequest(
'The number of documents is too high. Paginating through more than 10,000 documents is not possible.' 'The number of documents is too high. Paginating through more than 10,000 documents is not possible.'
); );

View file

@ -7,7 +7,7 @@
import expect from '@kbn/expect'; import expect from '@kbn/expect';
import type SuperTest from 'supertest'; import type SuperTest from 'supertest';
import { MAX_DOCS_PER_PAGE } from '@kbn/cases-plugin/common/constants'; import { MAX_COMMENTS_PER_PAGE } from '@kbn/cases-plugin/common/constants';
import { import {
Alerts, Alerts,
createCaseAttachAlertAndDeleteCase, createCaseAttachAlertAndDeleteCase,
@ -170,7 +170,7 @@ export default ({ getService }: FtrProviderContext): void => {
supertest: supertestWithoutAuth, supertest: supertestWithoutAuth,
caseId: postedCase.id, caseId: postedCase.id,
query: { query: {
perPage: MAX_DOCS_PER_PAGE, perPage: MAX_COMMENTS_PER_PAGE,
}, },
}), }),
]); ]);
@ -210,14 +210,14 @@ export default ({ getService }: FtrProviderContext): void => {
supertest: supertestWithoutAuth, supertest: supertestWithoutAuth,
caseId: postedCase1.id, caseId: postedCase1.id,
query: { query: {
perPage: MAX_DOCS_PER_PAGE, perPage: MAX_COMMENTS_PER_PAGE,
}, },
}), }),
findAttachments({ findAttachments({
supertest: supertestWithoutAuth, supertest: supertestWithoutAuth,
caseId: postedCase2.id, caseId: postedCase2.id,
query: { query: {
perPage: MAX_DOCS_PER_PAGE, perPage: MAX_COMMENTS_PER_PAGE,
}, },
}), }),
]); ]);
@ -457,7 +457,7 @@ export default ({ getService }: FtrProviderContext): void => {
supertest: supertestWithoutAuth, supertest: supertestWithoutAuth,
caseId: postedCase.id, caseId: postedCase.id,
query: { query: {
perPage: MAX_DOCS_PER_PAGE, perPage: MAX_COMMENTS_PER_PAGE,
}, },
auth: { user: secAllUser, space: 'space1' }, auth: { user: secAllUser, space: 'space1' },
}), }),

View file

@ -114,7 +114,7 @@ export default ({ getService }: FtrProviderContext): void => {
{ name: 'field is wrong type', queryParams: { perPage: true } }, { name: 'field is wrong type', queryParams: { perPage: true } },
{ name: 'field is unknown', queryParams: { foo: 'bar' } }, { name: 'field is unknown', queryParams: { foo: 'bar' } },
{ name: 'page > 10k', queryParams: { page: 10001 } }, { name: 'page > 10k', queryParams: { page: 10001 } },
{ name: 'perPage > 10k', queryParams: { perPage: 10001 } }, { name: 'perPage > 100', queryParams: { perPage: 101 } },
{ name: 'page * perPage > 10k', queryParams: { page: 2, perPage: 9001 } }, { name: 'page * perPage > 10k', queryParams: { page: 2, perPage: 9001 } },
]) { ]) {
it(`400s when ${errorScenario.name}`, async () => { it(`400s when ${errorScenario.name}`, async () => {