From 8e1f12716461fd31504d12dd197206714816035b Mon Sep 17 00:00:00 2001 From: adcoelho Date: Mon, 3 Jul 2023 16:50:11 +0200 Subject: [PATCH 1/8] Update pagination validation for find cases. Update pagination validation for find user actions. Update pagination validation for find comments. Added unit and e2e tests. Updated the documentation. --- x-pack/plugins/cases/common/api/cases/case.ts | 4 +- .../api/cases/user_actions/operations/find.ts | 4 +- .../plugins/cases/common/constants/index.ts | 2 + .../components/parameters/page_size.yaml | 3 +- ...id}@api@cases@{caseid}@comments@_find.yaml | 9 +-- .../server/client/attachments/validators.ts | 26 ++---- .../cases/server/client/cases/find.test.ts | 22 +++++- .../plugins/cases/server/client/cases/find.ts | 3 + .../server/client/cases/validators.test.ts | 52 ++++++++++++ .../cases/server/client/cases/validators.ts | 19 +++++ .../server/client/user_actions/find.test.ts | 41 ++++++++-- .../cases/server/client/user_actions/find.ts | 3 + .../client/user_actions/validators.test.ts | 54 +++++++++++++ .../server/client/user_actions/validators.ts | 18 +++++ .../cases/server/common/validators.test.ts | 63 +++++++++++++++ .../plugins/cases/server/common/validators.ts | 38 +++++++++ .../tests/common/cases/find_cases.ts | 79 +++++++++++-------- .../tests/common/comments/find_comments.ts | 4 +- .../common/user_actions/find_user_actions.ts | 19 +++++ 19 files changed, 389 insertions(+), 74 deletions(-) create mode 100644 x-pack/plugins/cases/server/client/cases/validators.test.ts create mode 100644 x-pack/plugins/cases/server/client/cases/validators.ts create mode 100644 x-pack/plugins/cases/server/client/user_actions/validators.test.ts create mode 100644 x-pack/plugins/cases/server/client/user_actions/validators.ts create mode 100644 x-pack/plugins/cases/server/common/validators.test.ts create mode 100644 x-pack/plugins/cases/server/common/validators.ts diff --git a/x-pack/plugins/cases/common/api/cases/case.ts b/x-pack/plugins/cases/common/api/cases/case.ts index 9778195465432..47bac0ddf4377 100644 --- a/x-pack/plugins/cases/common/api/cases/case.ts +++ b/x-pack/plugins/cases/common/api/cases/case.ts @@ -284,11 +284,11 @@ export const CasesFindRequestRt = rt.exact( /** * The page of objects to return */ - page: NumberFromString, + page: rt.union([rt.number, NumberFromString]), /** * The number of objects to include in each page */ - perPage: NumberFromString, + perPage: rt.union([rt.number, NumberFromString]), /** * An Elasticsearch simple_query_string */ diff --git a/x-pack/plugins/cases/common/api/cases/user_actions/operations/find.ts b/x-pack/plugins/cases/common/api/cases/user_actions/operations/find.ts index 336c94a7a4fec..ec4843f8ea9ef 100644 --- a/x-pack/plugins/cases/common/api/cases/user_actions/operations/find.ts +++ b/x-pack/plugins/cases/common/api/cases/user_actions/operations/find.ts @@ -30,8 +30,8 @@ export const UserActionFindRequestRt = rt.exact( rt.partial({ types: rt.array(FindTypeFieldRt), sortOrder: rt.union([rt.literal('desc'), rt.literal('asc')]), - page: NumberFromString, - perPage: NumberFromString, + page: rt.union([rt.number, NumberFromString]), + perPage: rt.union([rt.number, NumberFromString]), }) ); diff --git a/x-pack/plugins/cases/common/constants/index.ts b/x-pack/plugins/cases/common/constants/index.ts index b7b4987042495..ffd627c8de865 100644 --- a/x-pack/plugins/cases/common/constants/index.ts +++ b/x-pack/plugins/cases/common/constants/index.ts @@ -105,6 +105,8 @@ export const MAX_BULK_GET_ATTACHMENTS = 100 as const; export const MAX_CONCURRENT_SEARCHES = 10 as const; export const MAX_BULK_GET_CASES = 1000 as const; export const MAX_COMMENTS_PER_PAGE = 100 as const; +export const MAX_CASES_PER_PAGE = 100 as const; +export const MAX_USER_ACTIONS_PER_PAGE = 100 as const; export const MAX_CATEGORY_FILTER_LENGTH = 100 as const; export const MAX_TAGS_FILTER_LENGTH = 100 as const; export const MAX_ASSIGNEES_FILTER_LENGTH = 100 as const; diff --git a/x-pack/plugins/cases/docs/openapi/components/parameters/page_size.yaml b/x-pack/plugins/cases/docs/openapi/components/parameters/page_size.yaml index f59ff5301f9c2..0c78946a39fcf 100644 --- a/x-pack/plugins/cases/docs/openapi/components/parameters/page_size.yaml +++ b/x-pack/plugins/cases/docs/openapi/components/parameters/page_size.yaml @@ -1,7 +1,8 @@ in: query name: perPage -description: The number of items to return. +description: The number of items to return. Limited to 100 items. required: false schema: type: integer default: 20 + maximum: 100 diff --git a/x-pack/plugins/cases/docs/openapi/paths/s@{spaceid}@api@cases@{caseid}@comments@_find.yaml b/x-pack/plugins/cases/docs/openapi/paths/s@{spaceid}@api@cases@{caseid}@comments@_find.yaml index bb43f4dcc0b26..b1dd32a659515 100644 --- a/x-pack/plugins/cases/docs/openapi/paths/s@{spaceid}@api@cases@{caseid}@comments@_find.yaml +++ b/x-pack/plugins/cases/docs/openapi/paths/s@{spaceid}@api@cases@{caseid}@comments@_find.yaml @@ -10,14 +10,7 @@ get: parameters: - $ref: '../components/parameters/case_id.yaml' - $ref: '../components/parameters/page_index.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/page_size.yaml' - $ref: '../components/parameters/sort_order.yaml' - $ref: '../components/parameters/space_id.yaml' responses: diff --git a/x-pack/plugins/cases/server/client/attachments/validators.ts b/x-pack/plugins/cases/server/client/attachments/validators.ts index ea38fa71e702a..50a86c6a63b9f 100644 --- a/x-pack/plugins/cases/server/client/attachments/validators.ts +++ b/x-pack/plugins/cases/server/client/attachments/validators.ts @@ -6,7 +6,7 @@ */ import Boom from '@hapi/boom'; -import { MAX_DOCS_PER_PAGE, MAX_COMMENTS_PER_PAGE } from '../../../common/constants'; +import { MAX_COMMENTS_PER_PAGE } from '../../../common/constants'; import { isCommentRequestTypeExternalReference, isCommentRequestTypePersistableState, @@ -14,6 +14,7 @@ import { import type { CommentRequest, FindCommentsQueryParams } from '../../../common/api'; import type { ExternalReferenceAttachmentTypeRegistry } from '../../attachment_framework/external_reference_registry'; import type { PersistableStateAttachmentTypeRegistry } from '../../attachment_framework/persistable_state_registry'; +import { validatePagination } from '../../common/validators'; export const validateRegisteredAttachments = ({ query, @@ -44,22 +45,9 @@ export const validateRegisteredAttachments = ({ }; export const validateFindCommentsPagination = (params?: FindCommentsQueryParams) => { - if (params?.page == null && params?.perPage == null) { - return; - } - - const pageAsNumber = params.page ?? 0; - const perPageAsNumber = params.perPage ?? 0; - - 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( - 'The number of documents is too high. Paginating through more than 10,000 documents is not possible.' - ); - } + validatePagination({ + page: params?.page, + perPage: params?.perPage, + maxPerPage: MAX_COMMENTS_PER_PAGE, + }); }; diff --git a/x-pack/plugins/cases/server/client/cases/find.test.ts b/x-pack/plugins/cases/server/client/cases/find.test.ts index b7d9d9117ad7e..4ca3eb46ca087 100644 --- a/x-pack/plugins/cases/server/client/cases/find.test.ts +++ b/x-pack/plugins/cases/server/client/cases/find.test.ts @@ -10,6 +10,7 @@ import type { Case } from '../../../common/api'; import { MAX_ASSIGNEES_FILTER_LENGTH, + MAX_CASES_PER_PAGE, MAX_CATEGORY_FILTER_LENGTH, MAX_REPORTERS_FILTER_LENGTH, MAX_TAGS_FILTER_LENGTH, @@ -81,7 +82,7 @@ describe('find', () => { }); }); - describe('searchFields errors', () => { + describe('errors', () => { const clientArgs = createCasesClientMockArgs(); beforeEach(() => { @@ -149,5 +150,24 @@ describe('find', () => { `Error: The length of the field reporters is too long. Array must be of length <= ${MAX_REPORTERS_FILTER_LENGTH}.` ); }); + + it('Invalid total items results in error', async () => { + const findRequest = createCasesClientMockFindRequest({ page: 209, perPage: 100 }); + + await expect(find(findRequest, clientArgs)).rejects.toThrowError( + `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 () => { + const findRequest = createCasesClientMockFindRequest({ + page: 1, + perPage: MAX_CASES_PER_PAGE + 1, + }); + + await expect(find(findRequest, clientArgs)).rejects.toThrowError( + `Error: The provided perPage value was too high. The maximum allowed perPage value is ${MAX_CASES_PER_PAGE}.` + ); + }); }); }); diff --git a/x-pack/plugins/cases/server/client/cases/find.ts b/x-pack/plugins/cases/server/client/cases/find.ts index aa2bb36768207..4f2c74122b2a1 100644 --- a/x-pack/plugins/cases/server/client/cases/find.ts +++ b/x-pack/plugins/cases/server/client/cases/find.ts @@ -16,6 +16,7 @@ import { CasesFindResponseRt, } from '../../../common/api'; +import { validateFindCasesPagination } from './validators'; import { createCaseError } from '../../common/error'; import { asArray, transformCases } from '../../common/utils'; import { constructQueryOptions, constructSearch } from '../utils'; @@ -55,6 +56,8 @@ export const find = async ( try { const queryParams = decodeWithExcessOrThrow(CasesFindRequestRt)(params); + validateFindCasesPagination(queryParams); + throwIfCategoryParamTooLong(queryParams.category); /** diff --git a/x-pack/plugins/cases/server/client/cases/validators.test.ts b/x-pack/plugins/cases/server/client/cases/validators.test.ts new file mode 100644 index 0000000000000..2d0bfbc548628 --- /dev/null +++ b/x-pack/plugins/cases/server/client/cases/validators.test.ts @@ -0,0 +1,52 @@ +/* + * 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 { validateFindCasesPagination } from './validators'; +import { MAX_CASES_PER_PAGE } from '../../../common/constants'; + +const ERROR_MSG = + '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_CASES_PER_PAGE}.`; + +describe('validators', () => { + describe('validateFindCasessPagination', () => { + it('does not throw if only page is undefined', () => { + expect(() => validateFindCasesPagination({ perPage: 100 })).not.toThrowError(); + }); + + it('does not throw if only perPage is undefined', () => { + expect(() => validateFindCasesPagination({ page: 100 })).not.toThrowError(); + }); + + it('does not throw if page and perPage are defined and valid', () => { + expect(() => validateFindCasesPagination({ page: 2, perPage: 100 })).not.toThrowError(); + }); + + it('returns if page and perPage are undefined', () => { + expect(() => validateFindCasesPagination({})).not.toThrowError(); + }); + + it('returns if perPage < 0', () => { + expect(() => validateFindCasesPagination({ perPage: -1 })).not.toThrowError(); + }); + + it('throws if page > 10k', () => { + expect(() => validateFindCasesPagination({ page: 10001 })).toThrow(ERROR_MSG); + }); + + it('throws if perPage > 100', () => { + expect(() => validateFindCasesPagination({ perPage: MAX_CASES_PER_PAGE + 1 })).toThrowError( + ERROR_MSG_PER_PAGE + ); + }); + + it('throws if page * perPage > 10k', () => { + expect(() => validateFindCasesPagination({ page: 101, perPage: 100 })).toThrow(ERROR_MSG); + }); + }); +}); diff --git a/x-pack/plugins/cases/server/client/cases/validators.ts b/x-pack/plugins/cases/server/client/cases/validators.ts new file mode 100644 index 0000000000000..181c4c9f94da2 --- /dev/null +++ b/x-pack/plugins/cases/server/client/cases/validators.ts @@ -0,0 +1,19 @@ +/* + * 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 type { CasesFindRequest } from '../../../common/api'; + +import { MAX_CASES_PER_PAGE } from '../../../common/constants'; +import { validatePagination } from '../../common/validators'; + +export const validateFindCasesPagination = (params?: CasesFindRequest) => { + validatePagination({ + page: params?.page, + perPage: params?.perPage, + maxPerPage: MAX_CASES_PER_PAGE, + }); +}; diff --git a/x-pack/plugins/cases/server/client/user_actions/find.test.ts b/x-pack/plugins/cases/server/client/user_actions/find.test.ts index a8df2782117ce..db042a0f6d41b 100644 --- a/x-pack/plugins/cases/server/client/user_actions/find.test.ts +++ b/x-pack/plugins/cases/server/client/user_actions/find.test.ts @@ -5,11 +5,12 @@ * 2.0. */ +import { MAX_USER_ACTIONS_PER_PAGE } from '../../../common/constants'; import { createMockClient } from '../metrics/test_utils/client'; import { createCasesClientMockArgs } from '../mocks'; import { find } from './find'; -describe('addComment', () => { +describe('findUserActions', () => { const client = createMockClient(); const clientArgs = createCasesClientMockArgs(); @@ -17,10 +18,38 @@ describe('addComment', () => { jest.clearAllMocks(); }); - it('throws with excess fields', async () => { - await expect( - // @ts-expect-error: excess attribute - find({ caseId: 'test-case', params: { foo: 'bar' } }, client, clientArgs) - ).rejects.toThrow('invalid keys "foo"'); + describe('errors', () => { + it('throws with excess fields', async () => { + await expect( + // @ts-expect-error: excess attribute + find({ caseId: 'test-case', params: { foo: 'bar' } }, client, clientArgs) + ).rejects.toThrow('invalid keys "foo"'); + }); + + it('throws when trying to fetch more than 10,000 items', async () => { + await expect( + find({ caseId: 'test-case', params: { page: 209, perPage: 100 } }, client, clientArgs) + ).rejects.toThrow( + 'Error: The number of documents is too high. Paginating through more than 10,000 documents is not possible.' + ); + }); + + it(`throws when perPage > ${MAX_USER_ACTIONS_PER_PAGE}`, async () => { + await expect( + find( + { + caseId: 'test-case', + params: { + page: 1, + perPage: MAX_USER_ACTIONS_PER_PAGE + 1, + }, + }, + client, + clientArgs + ) + ).rejects.toThrow( + `Error: The provided perPage value was too high. The maximum allowed perPage value is ${MAX_USER_ACTIONS_PER_PAGE}.` + ); + }); }); }); diff --git a/x-pack/plugins/cases/server/client/user_actions/find.ts b/x-pack/plugins/cases/server/client/user_actions/find.ts index 713c15fff0d54..6142210a76324 100644 --- a/x-pack/plugins/cases/server/client/user_actions/find.ts +++ b/x-pack/plugins/cases/server/client/user_actions/find.ts @@ -19,6 +19,7 @@ import { createCaseError } from '../../common/error'; import { asArray } from '../../common/utils'; import type { CasesClient } from '../client'; import { decodeOrThrow } from '../../../common/api/runtime_types'; +import { validateFindUserActionsPagination } from './validators'; export const find = async ( { caseId, params }: UserActionFind, @@ -37,6 +38,8 @@ export const find = async ( const queryParams = decodeWithExcessOrThrow(UserActionFindRequestRt)({ ...params, types }); + validateFindUserActionsPagination(queryParams); + const [authorizationFilterRes] = await Promise.all([ authorization.getAuthorizationFilter(Operations.findUserActions), // ensure that we have authorization for reading the case diff --git a/x-pack/plugins/cases/server/client/user_actions/validators.test.ts b/x-pack/plugins/cases/server/client/user_actions/validators.test.ts new file mode 100644 index 0000000000000..405138d7ca8ab --- /dev/null +++ b/x-pack/plugins/cases/server/client/user_actions/validators.test.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 { validateFindUserActionsPagination } from './validators'; +import { MAX_USER_ACTIONS_PER_PAGE } from '../../../common/constants'; + +const ERROR_MSG = + '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_USER_ACTIONS_PER_PAGE}.`; + +describe('validators', () => { + describe('validateFindUserActionsPagination', () => { + it('does not throw if only page is undefined', () => { + expect(() => validateFindUserActionsPagination({ perPage: 100 })).not.toThrowError(); + }); + + it('does not throw if only perPage is undefined', () => { + expect(() => validateFindUserActionsPagination({ page: 100 })).not.toThrowError(); + }); + + it('does not throw if page and perPage are defined and valid', () => { + expect(() => validateFindUserActionsPagination({ page: 2, perPage: 100 })).not.toThrowError(); + }); + + it('returns if page and perPage are undefined', () => { + expect(() => validateFindUserActionsPagination({})).not.toThrowError(); + }); + + it('returns if perPage < 0', () => { + expect(() => validateFindUserActionsPagination({ perPage: -1 })).not.toThrowError(); + }); + + it('throws if page > 10k', () => { + expect(() => validateFindUserActionsPagination({ page: 10001 })).toThrow(ERROR_MSG); + }); + + it('throws if perPage > 100', () => { + expect(() => + validateFindUserActionsPagination({ perPage: MAX_USER_ACTIONS_PER_PAGE + 1 }) + ).toThrowError(ERROR_MSG_PER_PAGE); + }); + + it('throws if page * perPage > 10k', () => { + expect(() => validateFindUserActionsPagination({ page: 101, perPage: 100 })).toThrow( + ERROR_MSG + ); + }); + }); +}); diff --git a/x-pack/plugins/cases/server/client/user_actions/validators.ts b/x-pack/plugins/cases/server/client/user_actions/validators.ts new file mode 100644 index 0000000000000..c011cda32d9a0 --- /dev/null +++ b/x-pack/plugins/cases/server/client/user_actions/validators.ts @@ -0,0 +1,18 @@ +/* + * 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 { MAX_USER_ACTIONS_PER_PAGE } from '../../../common/constants'; +import { validatePagination } from '../../common/validators'; +import type { UserActionFindRequest } from '../../../common/api'; + +export const validateFindUserActionsPagination = (params?: UserActionFindRequest) => { + validatePagination({ + page: params?.page, + perPage: params?.perPage, + maxPerPage: MAX_USER_ACTIONS_PER_PAGE, + }); +}; diff --git a/x-pack/plugins/cases/server/common/validators.test.ts b/x-pack/plugins/cases/server/common/validators.test.ts new file mode 100644 index 0000000000000..d48d4e7d17f38 --- /dev/null +++ b/x-pack/plugins/cases/server/common/validators.test.ts @@ -0,0 +1,63 @@ +/* + * 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 { validatePagination } from './validators'; + +const MAX_PER_PAGE = 666; + +const ERROR_MSG = + '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_PER_PAGE}.`; + +describe('validators', () => { + describe('validatePagination', () => { + it('does not throw if only page is undefined', () => { + expect(() => + validatePagination({ perPage: 100, maxPerPage: MAX_PER_PAGE }) + ).not.toThrowError(); + }); + + it('does not throw if only perPage is undefined', () => { + expect(() => validatePagination({ page: 100, maxPerPage: MAX_PER_PAGE })).not.toThrowError(); + }); + + it('does not throw if page and perPage are defined and valid', () => { + expect(() => + validatePagination({ page: 2, perPage: 100, maxPerPage: MAX_PER_PAGE }) + ).not.toThrowError(); + }); + + it('returns if page and perPage are undefined', () => { + expect(() => validatePagination({ maxPerPage: MAX_PER_PAGE })).not.toThrowError(); + }); + + it('returns if perPage < 0', () => { + expect(() => + validatePagination({ perPage: -1, maxPerPage: MAX_PER_PAGE }) + ).not.toThrowError(); + }); + + it('throws if page > 10k', () => { + expect(() => validatePagination({ page: 10001, maxPerPage: MAX_PER_PAGE })).toThrow( + ERROR_MSG + ); + }); + + it(`throws if perPage > maxPerPage parameter`, () => { + expect(() => + validatePagination({ perPage: MAX_PER_PAGE + 1, maxPerPage: MAX_PER_PAGE }) + ).toThrowError(ERROR_MSG_PER_PAGE); + }); + + it('throws if page * perPage > 10k', () => { + expect(() => + validatePagination({ page: 101, perPage: 100, maxPerPage: MAX_PER_PAGE }) + ).toThrow(ERROR_MSG); + }); + }); +}); diff --git a/x-pack/plugins/cases/server/common/validators.ts b/x-pack/plugins/cases/server/common/validators.ts new file mode 100644 index 0000000000000..cdd66232713d2 --- /dev/null +++ b/x-pack/plugins/cases/server/common/validators.ts @@ -0,0 +1,38 @@ +/* + * 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 Boom from '@hapi/boom'; +import { MAX_DOCS_PER_PAGE } from '../../common/constants'; + +export const validatePagination = ({ + maxPerPage, + page, + perPage, +}: { + maxPerPage: number; + page?: number; + perPage?: number; +}) => { + if (page == null && perPage == null) { + return; + } + + const pageAsNumber = page ?? 0; + const perPageAsNumber = perPage ?? 0; + + if (perPageAsNumber > maxPerPage) { + throw Boom.badRequest( + `The provided perPage value was too high. The maximum allowed perPage value is ${maxPerPage}.` + ); + } + + if (Math.max(pageAsNumber, pageAsNumber * perPageAsNumber) > MAX_DOCS_PER_PAGE) { + throw Boom.badRequest( + 'The number of documents is too high. Paginating through more than 10,000 documents is not possible.' + ); + } +}; diff --git a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/cases/find_cases.ts b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/cases/find_cases.ts index 6875bc043a6c5..f4c044415190f 100644 --- a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/cases/find_cases.ts +++ b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/cases/find_cases.ts @@ -11,6 +11,7 @@ import expect from '@kbn/expect'; import { CASES_URL, MAX_ASSIGNEES_FILTER_LENGTH, + MAX_CASES_PER_PAGE, MAX_CATEGORY_FILTER_LENGTH, MAX_REPORTERS_FILTER_LENGTH, MAX_TAGS_FILTER_LENGTH, @@ -341,39 +342,6 @@ export default ({ getService }: FtrProviderContext): void => { }); }); - describe('errors', () => { - it('unhappy path - 400s when bad query supplied', async () => { - await findCases({ supertest, query: { perPage: true }, expectedHttpCode: 400 }); - }); - - for (const field of ['owner', 'tags', 'severity', 'status']) { - it(`should return a 400 when attempting to query a keyword field [${field}] when using a wildcard query`, async () => { - await findCases({ - supertest, - query: { searchFields: [field], search: 'some search string*' }, - expectedHttpCode: 400, - }); - }); - } - - for (const scenario of [ - { fieldName: 'category', sizeLimit: MAX_CATEGORY_FILTER_LENGTH }, - { fieldName: 'tags', sizeLimit: MAX_TAGS_FILTER_LENGTH }, - { fieldName: 'assignees', sizeLimit: MAX_ASSIGNEES_FILTER_LENGTH }, - { fieldName: 'reporters', sizeLimit: MAX_REPORTERS_FILTER_LENGTH }, - ]) { - it(`unhappy path - 400s when the field ${scenario.fieldName} exceeds the size limit`, async () => { - const value = Array(scenario.sizeLimit + 1).fill('foobar'); - - await findCases({ - supertest, - query: { [scenario.fieldName]: value }, - expectedHttpCode: 400, - }); - }); - } - }); - describe('search and searchField', () => { beforeEach(async () => { await createCase(supertest, postCaseReq); @@ -459,6 +427,51 @@ export default ({ getService }: FtrProviderContext): void => { }); }); + describe('errors', () => { + it('unhappy path - 400s when bad query supplied', async () => { + await findCases({ supertest, query: { perPage: true }, expectedHttpCode: 400 }); + }); + + for (const field of ['owner', 'tags', 'severity', 'status']) { + it(`should return a 400 when attempting to query a keyword field [${field}] when using a wildcard query`, async () => { + await findCases({ + supertest, + query: { searchFields: [field], search: 'some search string*' }, + expectedHttpCode: 400, + }); + }); + } + + for (const scenario of [ + { fieldName: 'category', sizeLimit: MAX_CATEGORY_FILTER_LENGTH }, + { fieldName: 'tags', sizeLimit: MAX_TAGS_FILTER_LENGTH }, + { fieldName: 'assignees', sizeLimit: MAX_ASSIGNEES_FILTER_LENGTH }, + { fieldName: 'reporters', sizeLimit: MAX_REPORTERS_FILTER_LENGTH }, + ]) { + it(`unhappy path - 400s when the field ${scenario.fieldName} exceeds the size limit`, async () => { + const value = Array(scenario.sizeLimit + 1).fill('foobar'); + + await findCases({ + supertest, + query: { [scenario.fieldName]: value }, + expectedHttpCode: 400, + }); + }); + } + + it(`400s when perPage > ${MAX_CASES_PER_PAGE} supplied`, async () => { + await findCases({ + supertest, + query: { perPage: MAX_CASES_PER_PAGE + 1 }, + expectedHttpCode: 400, + }); + }); + + it('400s when trying to fetch more than 10,000 documents', async () => { + await findCases({ supertest, query: { page: 209, perPage: 100 }, expectedHttpCode: 400 }); + }); + }); + describe('alerts', () => { const defaultSignalsIndex = '.siem-signals-default-000001'; const signalID = '4679431ee0ba3209b6fcd60a255a696886fe0a7d18f5375de510ff5b68fa6b78'; diff --git a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/find_comments.ts b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/find_comments.ts index 761959db29f66..0256a0ac1e562 100644 --- a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/find_comments.ts +++ b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/find_comments.ts @@ -7,7 +7,7 @@ import expect from '@kbn/expect'; -import { CASES_URL } from '@kbn/cases-plugin/common/constants'; +import { CASES_URL, MAX_COMMENTS_PER_PAGE } from '@kbn/cases-plugin/common/constants'; import { CommentType } from '@kbn/cases-plugin/common/api'; import { FtrProviderContext } from '../../../../common/ftr_provider_context'; import { @@ -114,7 +114,7 @@ export default ({ getService }: FtrProviderContext): void => { { name: 'field is wrong type', queryParams: { perPage: true } }, { name: 'field is unknown', queryParams: { foo: 'bar' } }, { name: 'page > 10k', queryParams: { page: 10001 } }, - { name: 'perPage > 100', queryParams: { perPage: 101 } }, + { name: 'perPage > 100', queryParams: { perPage: MAX_COMMENTS_PER_PAGE + 1 } }, { name: 'page * perPage > 10k', queryParams: { page: 2, perPage: 9001 } }, ]) { it(`400s when ${errorScenario.name}`, async () => { diff --git a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/user_actions/find_user_actions.ts b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/user_actions/find_user_actions.ts index 28bab81a70bee..8e3cbd543f580 100644 --- a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/user_actions/find_user_actions.ts +++ b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/user_actions/find_user_actions.ts @@ -43,6 +43,7 @@ import { } from '../../../../common/lib/api'; import { FtrProviderContext } from '../../../../common/ftr_provider_context'; +import { MAX_USER_ACTIONS_PER_PAGE } from '@kbn/cases-plugin/common/constants'; // eslint-disable-next-line import/no-default-export export default ({ getService }: FtrProviderContext): void => { @@ -237,6 +238,24 @@ export default ({ getService }: FtrProviderContext): void => { expect(response.userActions[0].type).to.eql('create_case'); expect(response.userActions[0].action).to.eql('create'); }); + + it(`400s when perPage > ${MAX_USER_ACTIONS_PER_PAGE} supplied`, async () => { + await findCaseUserActions({ + caseID: theCase.id, + supertest, + options: { perPage: MAX_USER_ACTIONS_PER_PAGE + 1 }, + expectedHttpCode: 400, + }); + }); + + it('400s when trying to fetch more than 10,000 documents', async () => { + await findCaseUserActions({ + caseID: theCase.id, + supertest, + options: { page: 209, perPage: 100 }, + expectedHttpCode: 400, + }); + }); }); describe('filters using the type query parameter', () => { From 1d1684d718606ee3e6c1fdc2587ac0c1d036385b Mon Sep 17 00:00:00 2001 From: adcoelho Date: Mon, 3 Jul 2023 17:30:21 +0200 Subject: [PATCH 2/8] Updated documentation. --- .../plugins/cases/docs/openapi/bundled.json | 45 ++++++++----------- .../plugins/cases/docs/openapi/bundled.yaml | 24 ++++------ 2 files changed, 28 insertions(+), 41 deletions(-) diff --git a/x-pack/plugins/cases/docs/openapi/bundled.json b/x-pack/plugins/cases/docs/openapi/bundled.json index 4d6905006c08d..c504d966cb9d2 100644 --- a/x-pack/plugins/cases/docs/openapi/bundled.json +++ b/x-pack/plugins/cases/docs/openapi/bundled.json @@ -12,26 +12,18 @@ "url": "https://www.elastic.co/licensing/elastic-license" } }, - "servers": [ - { - "url": "http://localhost:5601", - "description": "local" - } - ], - "security": [ - { - "basicAuth": [] - }, - { - "apiKeyAuth": [] - } - ], "tags": [ { "name": "cases", "description": "Case APIs enable you to open and track issues." } ], + "servers": [ + { + "url": "http://localhost:5601", + "description": "local" + } + ], "paths": { "/api/cases": { "post": { @@ -3415,15 +3407,7 @@ "$ref": "#/components/parameters/page_index" }, { - "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/page_size" }, { "$ref": "#/components/parameters/sort_order" @@ -3916,11 +3900,12 @@ "page_size": { "in": "query", "name": "perPage", - "description": "The number of items to return.", + "description": "The number of items to return. Limited to 100 items.", "required": false, "schema": { "type": "integer", - "default": 20 + "default": 20, + "maximum": 100 } }, "reporters": { @@ -7161,5 +7146,13 @@ ] } } - } + }, + "security": [ + { + "basicAuth": [] + }, + { + "apiKeyAuth": [] + } + ] } \ No newline at end of file diff --git a/x-pack/plugins/cases/docs/openapi/bundled.yaml b/x-pack/plugins/cases/docs/openapi/bundled.yaml index 7608a44f3e45e..d393da4d334a0 100644 --- a/x-pack/plugins/cases/docs/openapi/bundled.yaml +++ b/x-pack/plugins/cases/docs/openapi/bundled.yaml @@ -8,15 +8,12 @@ info: license: name: Elastic License 2.0 url: https://www.elastic.co/licensing/elastic-license -servers: - - url: http://localhost:5601 - description: local -security: - - basicAuth: [] - - apiKeyAuth: [] tags: - name: cases description: Case APIs enable you to open and track issues. +servers: + - url: http://localhost:5601 + description: local paths: /api/cases: post: @@ -2085,14 +2082,7 @@ paths: parameters: - $ref: '#/components/parameters/case_id' - $ref: '#/components/parameters/page_index' - - 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/page_size' - $ref: '#/components/parameters/sort_order' - $ref: '#/components/parameters/space_id' responses: @@ -2382,11 +2372,12 @@ components: page_size: in: query name: perPage - description: The number of items to return. + description: The number of items to return. Limited to 100 items. required: false schema: type: integer default: 20 + maximum: 100 reporters: in: query name: reporters @@ -4772,3 +4763,6 @@ components: isPreconfigured: false isDeprecated: false referencedByCount: 0 +security: + - basicAuth: [] + - apiKeyAuth: [] From 173a9100f4d9eacc2adf4643c7509bf32baf6c57 Mon Sep 17 00:00:00 2001 From: adcoelho Date: Tue, 4 Jul 2023 14:29:48 +0200 Subject: [PATCH 3/8] Create PaginationSchema. Remove code validation for paginations. Tests. --- x-pack/plugins/cases/common/api/cases/case.ts | 214 +++++++++--------- .../cases/common/api/cases/comment/index.ts | 28 +-- .../api/cases/user_actions/operations/find.ts | 16 +- .../plugins/cases/common/schema/index.test.ts | 178 +++++++++++++-- x-pack/plugins/cases/common/schema/index.ts | 39 ++++ x-pack/plugins/cases/common/schema/types.ts | 20 ++ .../server/client/attachments/get.test.ts | 6 +- .../cases/server/client/attachments/get.ts | 3 - .../client/attachments/validators.test.ts | 52 ----- .../server/client/attachments/validators.ts | 12 +- .../cases/server/client/cases/find.test.ts | 5 +- .../plugins/cases/server/client/cases/find.ts | 3 - .../server/client/cases/validators.test.ts | 52 ----- .../cases/server/client/cases/validators.ts | 19 -- .../server/client/user_actions/find.test.ts | 8 +- .../cases/server/client/user_actions/find.ts | 3 - .../client/user_actions/validators.test.ts | 54 ----- .../server/client/user_actions/validators.ts | 18 -- .../cases/server/common/validators.test.ts | 63 ------ .../plugins/cases/server/common/validators.ts | 38 ---- 20 files changed, 354 insertions(+), 477 deletions(-) create mode 100644 x-pack/plugins/cases/common/schema/types.ts delete mode 100644 x-pack/plugins/cases/server/client/attachments/validators.test.ts delete mode 100644 x-pack/plugins/cases/server/client/cases/validators.test.ts delete mode 100644 x-pack/plugins/cases/server/client/cases/validators.ts delete mode 100644 x-pack/plugins/cases/server/client/user_actions/validators.test.ts delete mode 100644 x-pack/plugins/cases/server/client/user_actions/validators.ts delete mode 100644 x-pack/plugins/cases/server/common/validators.test.ts delete mode 100644 x-pack/plugins/cases/server/common/validators.ts diff --git a/x-pack/plugins/cases/common/api/cases/case.ts b/x-pack/plugins/cases/common/api/cases/case.ts index 47bac0ddf4377..6a6cc17a889c6 100644 --- a/x-pack/plugins/cases/common/api/cases/case.ts +++ b/x-pack/plugins/cases/common/api/cases/case.ts @@ -7,13 +7,17 @@ import * as rt from 'io-ts'; -import { NumberFromString } from '../saved_object'; import { UserRt } from '../user'; import { CommentRt } from './comment'; import { CasesStatusResponseRt, CaseStatusRt } from './status'; import { CaseConnectorRt } from '../connectors/connector'; import { CaseAssigneesRt } from './assignee'; -import { limitedArraySchema, limitedStringSchema, NonEmptyString } from '../../schema'; +import { + limitedArraySchema, + limitedStringSchema, + NonEmptyString, + paginationSchema, +} from '../../schema'; import { MAX_DELETE_IDS_LENGTH, MAX_DESCRIPTION_LENGTH, @@ -25,6 +29,7 @@ import { MAX_REPORTERS_FILTER_LENGTH, MAX_TAGS_FILTER_LENGTH, MAX_BULK_GET_CASES, + MAX_CASES_PER_PAGE, } from '../../constants'; export const AttachmentTotalsRt = rt.strict({ @@ -228,108 +233,111 @@ const CasesFindRequestSearchFieldsRt = rt.keyof({ }); export const CasesFindRequestRt = rt.exact( - rt.partial({ - /** - * Tags to filter by - */ - tags: rt.union([ - limitedArraySchema({ - codec: rt.string, - fieldName: 'tags', - min: 0, - max: MAX_TAGS_FILTER_LENGTH, - }), - rt.string, - ]), - /** - * The status of the case (open, closed, in-progress) - */ - status: CaseStatusRt, - /** - * The severity of the case - */ - severity: CaseSeverityRt, - /** - * The uids of the user profiles to filter by - */ - assignees: rt.union([ - limitedArraySchema({ - codec: rt.string, - fieldName: 'assignees', - min: 0, - max: MAX_ASSIGNEES_FILTER_LENGTH, - }), - rt.string, - ]), - /** - * The reporters to filter by - */ - reporters: rt.union([ - limitedArraySchema({ - codec: rt.string, - fieldName: 'reporters', - min: 0, - max: MAX_REPORTERS_FILTER_LENGTH, - }), - rt.string, - ]), - /** - * Operator to use for the `search` field - */ - defaultSearchOperator: rt.union([rt.literal('AND'), rt.literal('OR')]), - /** - * A KQL date. If used all cases created after (gte) the from date will be returned - */ - from: rt.string, - /** - * The page of objects to return - */ - page: rt.union([rt.number, NumberFromString]), - /** - * The number of objects to include in each page - */ - perPage: rt.union([rt.number, NumberFromString]), - /** - * An Elasticsearch simple_query_string - */ - search: rt.string, - /** - * The fields to perform the simple_query_string parsed query against - */ - searchFields: rt.union([ - 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, - /** - * The order to sort by - */ - sortOrder: rt.union([rt.literal('desc'), rt.literal('asc')]), + rt.intersection([ + rt.partial({ + /** + * Tags to filter by + */ + tags: rt.union([ + limitedArraySchema({ + codec: rt.string, + fieldName: 'tags', + min: 0, + max: MAX_TAGS_FILTER_LENGTH, + }), + rt.string, + ]), + /** + * The status of the case (open, closed, in-progress) + */ + status: CaseStatusRt, + /** + * The severity of the case + */ + severity: CaseSeverityRt, + /** + * The uids of the user profiles to filter by + */ + assignees: rt.union([ + limitedArraySchema({ + codec: rt.string, + fieldName: 'assignees', + min: 0, + max: MAX_ASSIGNEES_FILTER_LENGTH, + }), + rt.string, + ]), + /** + * The reporters to filter by + */ + reporters: rt.union([ + limitedArraySchema({ + codec: rt.string, + fieldName: 'reporters', + min: 0, + max: MAX_REPORTERS_FILTER_LENGTH, + }), + rt.string, + ]), + /** + * Operator to use for the `search` field + */ + defaultSearchOperator: rt.union([rt.literal('AND'), rt.literal('OR')]), + /** + * A KQL date. If used all cases created after (gte) the from date will be returned + */ + from: rt.string, + /** + * The page of objects to return + */ + // page: rt.union([rt.number, NumberFromString]), + /** + * The number of objects to include in each page + */ + // perPage: rt.union([rt.number, NumberFromString]), + /** + * An Elasticsearch simple_query_string + */ + search: rt.string, + /** + * The fields to perform the simple_query_string parsed query against + */ + searchFields: rt.union([ + 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, + /** + * The order to sort by + */ + sortOrder: rt.union([rt.literal('desc'), rt.literal('asc')]), - /** - * A KQL date. If used all cases created before (lte) the to date will be returned. - */ - to: rt.string, - /** - * The owner(s) to filter by. The user making the request must have privileges to retrieve cases of that - * ownership or they will be ignored. If no owner is included, then all ownership types will be included in the response - * that the user has access to. - */ + /** + * A KQL date. If used all cases created before (lte) the to date will be returned. + */ + to: rt.string, + /** + * The owner(s) to filter by. The user making the request must have privileges to retrieve cases of that + * ownership or they will be ignored. If no owner is included, then all ownership types will be included in the response + * that the user has access to. + */ - owner: rt.union([rt.array(rt.string), rt.string]), - /** - * The category of the case. - */ - category: rt.union([rt.array(rt.string), rt.string]), - }) + owner: rt.union([rt.array(rt.string), rt.string]), + /** + * The category of the case. + */ + category: rt.union([rt.array(rt.string), rt.string]), + }), + paginationSchema({ maxPerPage: MAX_CASES_PER_PAGE }), + ]) ); export const CasesDeleteRequestRt = limitedArraySchema({ @@ -532,8 +540,8 @@ export type Case = rt.TypeOf; export type CaseResolveResponse = rt.TypeOf; export type Cases = rt.TypeOf; export type CasesDeleteRequest = rt.TypeOf; -export type CasesFindRequest = rt.TypeOf; export type CasesByAlertIDRequest = rt.TypeOf; +export type CasesFindRequest = rt.TypeOf; export type CasesFindResponse = rt.TypeOf; export type CasePatchRequest = rt.TypeOf; export type CasesPatchRequest = rt.TypeOf; diff --git a/x-pack/plugins/cases/common/api/cases/comment/index.ts b/x-pack/plugins/cases/common/api/cases/comment/index.ts index 1662631ae2b79..00254c536127a 100644 --- a/x-pack/plugins/cases/common/api/cases/comment/index.ts +++ b/x-pack/plugins/cases/common/api/cases/comment/index.ts @@ -6,10 +6,9 @@ */ import * as rt from 'io-ts'; -import { MAX_BULK_GET_ATTACHMENTS } from '../../../constants'; -import { limitedArraySchema } from '../../../schema'; +import { MAX_BULK_GET_ATTACHMENTS, MAX_COMMENTS_PER_PAGE } from '../../../constants'; +import { limitedArraySchema, paginationSchema } from '../../../schema'; import { jsonValueRt } from '../../runtime_types'; -import { NumberFromString } from '../../saved_object'; import { UserRt } from '../../user'; @@ -290,20 +289,15 @@ export const CommentsFindResponseRt = rt.strict({ export const CommentsRt = rt.array(CommentRt); export const FindCommentsQueryParamsRt = rt.exact( - rt.partial({ - /** - * The page of objects to return - */ - page: rt.union([rt.number, NumberFromString]), - /** - * The number of objects to return for a page - */ - perPage: rt.union([rt.number, NumberFromString]), - /** - * Order to sort the response - */ - sortOrder: rt.union([rt.literal('desc'), rt.literal('asc')]), - }) + rt.intersection([ + rt.partial({ + /** + * Order to sort the response + */ + sortOrder: rt.union([rt.literal('desc'), rt.literal('asc')]), + }), + paginationSchema({ maxPerPage: MAX_COMMENTS_PER_PAGE }), + ]) ); export const BulkCreateCommentRequestRt = rt.array(CommentRequestRt); diff --git a/x-pack/plugins/cases/common/api/cases/user_actions/operations/find.ts b/x-pack/plugins/cases/common/api/cases/user_actions/operations/find.ts index ec4843f8ea9ef..7406feaed4492 100644 --- a/x-pack/plugins/cases/common/api/cases/user_actions/operations/find.ts +++ b/x-pack/plugins/cases/common/api/cases/user_actions/operations/find.ts @@ -6,9 +6,10 @@ */ import * as rt from 'io-ts'; +import { MAX_USER_ACTIONS_PER_PAGE } from '../../../../constants'; import { UserActionsRt } from '../response'; import { ActionTypes } from '../common'; -import { NumberFromString } from '../../../saved_object'; +import { paginationSchema } from '../../../../schema'; const AdditionalFilterTypes = { action: 'action', @@ -27,12 +28,13 @@ const FindTypeFieldRt = rt.keyof(FindTypes); export type FindTypeField = rt.TypeOf; export const UserActionFindRequestRt = rt.exact( - rt.partial({ - types: rt.array(FindTypeFieldRt), - sortOrder: rt.union([rt.literal('desc'), rt.literal('asc')]), - page: rt.union([rt.number, NumberFromString]), - perPage: rt.union([rt.number, NumberFromString]), - }) + rt.intersection([ + rt.partial({ + types: rt.array(FindTypeFieldRt), + sortOrder: rt.union([rt.literal('desc'), rt.literal('asc')]), + }), + paginationSchema({ maxPerPage: MAX_USER_ACTIONS_PER_PAGE }), + ]) ); export type UserActionFindRequest = rt.TypeOf; diff --git a/x-pack/plugins/cases/common/schema/index.test.ts b/x-pack/plugins/cases/common/schema/index.test.ts index 1a810c8003ff7..06716fcfcaa51 100644 --- a/x-pack/plugins/cases/common/schema/index.test.ts +++ b/x-pack/plugins/cases/common/schema/index.test.ts @@ -7,7 +7,8 @@ import { PathReporter } from 'io-ts/lib/PathReporter'; -import { limitedArraySchema, limitedStringSchema, NonEmptyString } from '.'; +import { limitedArraySchema, limitedStringSchema, NonEmptyString, paginationSchema } from '.'; +import { MAX_DOCS_PER_PAGE } from '../constants'; describe('schema', () => { describe('limitedArraySchema', () => { @@ -19,10 +20,10 @@ describe('schema', () => { limitedArraySchema({ codec: NonEmptyString, fieldName, min: 1, max: 1 }).decode(['']) ) ).toMatchInlineSnapshot(` - Array [ - "string must have length >= 1", - ] - `); + Array [ + "string must have length >= 1", + ] + `); }); it('fails when given an empty array', () => { @@ -31,10 +32,10 @@ describe('schema', () => { limitedArraySchema({ codec: NonEmptyString, fieldName, min: 1, max: 1 }).decode([]) ) ).toMatchInlineSnapshot(` - Array [ - "The length of the field foobar is too short. Array must be of length >= 1.", - ] - `); + Array [ + "The length of the field foobar is too short. Array must be of length >= 1.", + ] + `); }); it('fails when given an array larger than the limit of one item', () => { @@ -46,10 +47,10 @@ describe('schema', () => { ]) ) ).toMatchInlineSnapshot(` - Array [ - "The length of the field foobar is too long. Array must be of length <= 1.", - ] - `); + Array [ + "The length of the field foobar is too long. Array must be of length <= 1.", + ] + `); }); it('succeeds when given an array of 1 item with a non-empty string', () => { @@ -58,10 +59,10 @@ describe('schema', () => { limitedArraySchema({ codec: NonEmptyString, fieldName, min: 1, max: 1 }).decode(['a']) ) ).toMatchInlineSnapshot(` - Array [ - "No errors!", - ] - `); + Array [ + "No errors!", + ] + `); }); it('succeeds when given an array of 0 item with a non-empty string when the min is 0', () => { @@ -70,10 +71,10 @@ describe('schema', () => { limitedArraySchema({ codec: NonEmptyString, fieldName, min: 0, max: 2 }).decode([]) ) ).toMatchInlineSnapshot(` - Array [ - "No errors!", - ] - `); + Array [ + "No errors!", + ] + `); }); }); @@ -84,7 +85,7 @@ describe('schema', () => { expect(PathReporter.report(limitedStringSchema({ fieldName, min: 2, max: 1 }).decode('a'))) .toMatchInlineSnapshot(` Array [ - "The length of the ${fieldName} is too short. The minimum length is 2.", + "The length of the foo is too short. The minimum length is 2.", ] `); }); @@ -93,7 +94,7 @@ describe('schema', () => { expect(PathReporter.report(limitedStringSchema({ fieldName, min: 1, max: 1 }).decode(''))) .toMatchInlineSnapshot(` Array [ - "The ${fieldName} field cannot be an empty string.", + "The foo field cannot be an empty string.", ] `); }); @@ -102,7 +103,7 @@ describe('schema', () => { expect(PathReporter.report(limitedStringSchema({ fieldName, min: 1, max: 1 }).decode(' '))) .toMatchInlineSnapshot(` Array [ - "The ${fieldName} field cannot be an empty string.", + "The foo field cannot be an empty string.", ] `); }); @@ -114,7 +115,7 @@ describe('schema', () => { ) ).toMatchInlineSnapshot(` Array [ - "The length of the ${fieldName} is too long. The maximum length is 5.", + "The length of the foo is too long. The maximum length is 5.", ] `); }); @@ -167,4 +168,131 @@ describe('schema', () => { `); }); }); + + describe('paginationSchema', () => { + it('succeeds when no page or perPage passed', () => { + expect(PathReporter.report(paginationSchema({ maxPerPage: 1 }).decode({}))) + .toMatchInlineSnapshot(` + Array [ + "No errors!", + ] + `); + }); + + it('succeeds when only valid page is passed', () => { + expect(PathReporter.report(paginationSchema({ maxPerPage: 2 }).decode({ page: 0 }))) + .toMatchInlineSnapshot(` + Array [ + "No errors!", + ] + `); + }); + + it('succeeds when only valid perPage is passed', () => { + expect(PathReporter.report(paginationSchema({ maxPerPage: 3 }).decode({ perPage: 1 }))) + .toMatchInlineSnapshot(` + Array [ + "No errors!", + ] + `); + }); + + it('succeeds when page and perPage are passed and valid', () => { + expect( + PathReporter.report(paginationSchema({ maxPerPage: 3 }).decode({ page: 1, perPage: 2 })) + ).toMatchInlineSnapshot(` + Array [ + "No errors!", + ] + `); + }); + + it('fails when page > maxPerPage', () => { + expect(PathReporter.report(paginationSchema({ maxPerPage: 3 }).decode({ perPage: 4 }))) + .toMatchInlineSnapshot(` + Array [ + "The provided perPage value is too high. The maximum allowed perPage value is 3.", + ] + `); + }); + + it(`fails when page > ${MAX_DOCS_PER_PAGE}`, () => { + expect( + PathReporter.report( + paginationSchema({ maxPerPage: 3 }).decode({ page: MAX_DOCS_PER_PAGE + 1 }) + ) + ).toMatchInlineSnapshot(` + Array [ + "The number of documents is too high. Paginating through more than 10000 documents is not possible.", + ] + `); + }); + + it(`fails when page * perPage > ${MAX_DOCS_PER_PAGE}`, () => { + expect( + PathReporter.report( + paginationSchema({ maxPerPage: 3 }).decode({ page: MAX_DOCS_PER_PAGE, perPage: 2 }) + ) + ).toMatchInlineSnapshot(` + Array [ + "The number of documents is too high. Paginating through more than 10000 documents is not possible.", + ] + `); + }); + + it(`fails when page * perPage > ${MAX_DOCS_PER_PAGE}`, () => { + expect( + PathReporter.report( + paginationSchema({ maxPerPage: 3 }).decode({ page: MAX_DOCS_PER_PAGE, perPage: 2 }) + ) + ).toMatchInlineSnapshot(` + Array [ + "The number of documents is too high. Paginating through more than 10000 documents is not possible.", + ] + `); + }); + + it('valid NumberFromString work correctly', () => { + expect( + PathReporter.report(paginationSchema({ maxPerPage: 3 }).decode({ page: '1', perPage: '2' })) + ).toMatchInlineSnapshot(` + Array [ + "No errors!", + ] + `); + }); + + it('invalid NumberFromString work correctly', () => { + expect( + PathReporter.report(paginationSchema({ maxPerPage: 3 }).decode({ page: 'a', perPage: 'b' })) + ).toMatchInlineSnapshot(` + Array [ + "Invalid value \\"a\\" supplied to : Pagination/page: (number | NumberFromString | undefined)/0: number", + "cannot parse to a number", + "Invalid value \\"a\\" supplied to : Pagination/page: (number | NumberFromString | undefined)/2: undefined", + "Invalid value \\"b\\" supplied to : Pagination/perPage: (number | NumberFromString | undefined)/0: number", + "cannot parse to a number", + "Invalid value \\"b\\" supplied to : Pagination/perPage: (number | NumberFromString | undefined)/2: undefined", + ] + `); + }); + + it.skip('fails when page number is negative', () => { + expect(PathReporter.report(paginationSchema({ maxPerPage: 3 }).decode({ page: -1 }))) + .toMatchInlineSnapshot(` + Array [ + "The provided page value is too low. The minimum allowed page value is 0.", + ] + `); + }); + + it.skip('fails when perPage number is negative', () => { + expect(PathReporter.report(paginationSchema({ maxPerPage: 3 }).decode({ perPage: -1 }))) + .toMatchInlineSnapshot(` + Array [ + "The provided perPage value is too low. The minimum allowed perPage value is 0.", + ] + `); + }); + }); }); diff --git a/x-pack/plugins/cases/common/schema/index.ts b/x-pack/plugins/cases/common/schema/index.ts index 9003360ae11b8..bafa80bb1b85e 100644 --- a/x-pack/plugins/cases/common/schema/index.ts +++ b/x-pack/plugins/cases/common/schema/index.ts @@ -8,6 +8,10 @@ import * as rt from 'io-ts'; import { either } from 'fp-ts/lib/Either'; +import { MAX_DOCS_PER_PAGE } from '../constants'; +import type { PartialPaginationType } from './types'; +import { PaginationSchemaRt } from './types'; + export interface LimitedSchemaType { fieldName: string; min: number; @@ -92,3 +96,38 @@ export const limitedArraySchema = ({ }), rt.identity ); + +export const paginationSchema = ({ maxPerPage }: { maxPerPage: number }) => + new rt.PartialType( + 'Pagination', + PaginationSchemaRt.is, + (u, c) => + either.chain(PaginationSchemaRt.validate(u, c), (params) => { + if (params.page == null && params.perPage == null) { + return rt.success(params); + } + + const pageAsNumber = params.page ?? 0; + const perPageAsNumber = params.perPage ?? 0; + + if (perPageAsNumber > maxPerPage) { + return rt.failure( + u, + c, + `The provided perPage value is too high. The maximum allowed perPage value is ${maxPerPage}.` + ); + } + + if (Math.max(pageAsNumber, pageAsNumber * perPageAsNumber) > MAX_DOCS_PER_PAGE) { + return rt.failure( + u, + c, + `The number of documents is too high. Paginating through more than ${MAX_DOCS_PER_PAGE} documents is not possible.` + ); + } + + return rt.success(params); + }), + rt.identity, + undefined + ); diff --git a/x-pack/plugins/cases/common/schema/types.ts b/x-pack/plugins/cases/common/schema/types.ts new file mode 100644 index 0000000000000..267df495ba8c3 --- /dev/null +++ b/x-pack/plugins/cases/common/schema/types.ts @@ -0,0 +1,20 @@ +/* + * 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 * as rt from 'io-ts'; +import { NumberFromString } from '../api/saved_object'; + +const PageTypeRt = rt.union([rt.number, NumberFromString, rt.undefined]); +type PageNumberType = rt.TypeOf; + +export interface PaginationType { + page: PageNumberType; + perPage: PageNumberType; +} + +export const PaginationSchemaRt = rt.partial({ page: PageTypeRt, perPage: PageTypeRt }); +export type PartialPaginationType = Partial; diff --git a/x-pack/plugins/cases/server/client/attachments/get.test.ts b/x-pack/plugins/cases/server/client/attachments/get.test.ts index c2c3423b2388b..fe12fb4b19a3a 100644 --- a/x-pack/plugins/cases/server/client/attachments/get.test.ts +++ b/x-pack/plugins/cases/server/client/attachments/get.test.ts @@ -20,7 +20,7 @@ describe('get', () => { await expect(() => findComment({ caseID: 'mock-id', findQueryParams: { page: 209, perPage: 100 } }, clientArgs) ).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 10000 documents is not possible."` ); }); @@ -28,7 +28,7 @@ describe('get', () => { 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."` + `"Failed to find comments case id: mock-id: Error: The provided perPage value is too high. The maximum allowed perPage value is 100."` ); }); @@ -36,7 +36,7 @@ describe('get', () => { await expect( findComment( // @ts-expect-error: excess attribute - { caseID: 'mock-id', findQueryParams: { page: 2, perPage: 9001, foo: 'bar' } }, + { caseID: 'mock-id', findQueryParams: { foo: 'bar' } }, clientArgs ) ).rejects.toThrowErrorMatchingInlineSnapshot( diff --git a/x-pack/plugins/cases/server/client/attachments/get.ts b/x-pack/plugins/cases/server/client/attachments/get.ts index 62f647434e890..7e4bc311a6572 100644 --- a/x-pack/plugins/cases/server/client/attachments/get.ts +++ b/x-pack/plugins/cases/server/client/attachments/get.ts @@ -39,7 +39,6 @@ import { createCaseError } from '../../common/error'; import { DEFAULT_PAGE, DEFAULT_PER_PAGE } from '../../routes/api'; import { buildFilter, combineFilters } from '../utils'; import { Operations } from '../../authorization'; -import { validateFindCommentsPagination } from './validators'; import { decodeOrThrow } from '../../../common/api/runtime_types'; const normalizeAlertResponse = (alerts: Array>): AlertResponse => @@ -124,8 +123,6 @@ export async function find( try { const queryParams = decodeWithExcessOrThrow(FindCommentsQueryParamsRt)(findQueryParams); - validateFindCommentsPagination(queryParams); - const { filter: authorizationFilter, ensureSavedObjectsAreAuthorized } = await authorization.getAuthorizationFilter(Operations.findComments); diff --git a/x-pack/plugins/cases/server/client/attachments/validators.test.ts b/x-pack/plugins/cases/server/client/attachments/validators.test.ts deleted file mode 100644 index ce6c43a665a10..0000000000000 --- a/x-pack/plugins/cases/server/client/attachments/validators.test.ts +++ /dev/null @@ -1,52 +0,0 @@ -/* - * 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 { validateFindCommentsPagination } from './validators'; -import { MAX_COMMENTS_PER_PAGE } from '../../../common/constants'; - -const ERROR_MSG = - '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('validateFindCommentsPagination', () => { - it('does not throw if only page is undefined', () => { - expect(() => validateFindCommentsPagination({ perPage: 100 })).not.toThrowError(); - }); - - it('does not throw if only perPage is undefined', () => { - 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', () => { - expect(() => validateFindCommentsPagination({})).not.toThrowError(); - }); - - it('returns if perPage < 0', () => { - expect(() => validateFindCommentsPagination({ perPage: -1 })).not.toThrowError(); - }); - - it('throws if page > 10k', () => { - expect(() => validateFindCommentsPagination({ page: 10001 })).toThrow(ERROR_MSG); - }); - - it('throws if perPage > 100', () => { - expect(() => - validateFindCommentsPagination({ perPage: MAX_COMMENTS_PER_PAGE + 1 }) - ).toThrowError(ERROR_MSG_PER_PAGE); - }); - - it('throws if page * perPage > 10k', () => { - expect(() => validateFindCommentsPagination({ page: 101, perPage: 100 })).toThrow(ERROR_MSG); - }); - }); -}); diff --git a/x-pack/plugins/cases/server/client/attachments/validators.ts b/x-pack/plugins/cases/server/client/attachments/validators.ts index 50a86c6a63b9f..4a66d32c31c19 100644 --- a/x-pack/plugins/cases/server/client/attachments/validators.ts +++ b/x-pack/plugins/cases/server/client/attachments/validators.ts @@ -6,15 +6,13 @@ */ import Boom from '@hapi/boom'; -import { MAX_COMMENTS_PER_PAGE } from '../../../common/constants'; import { isCommentRequestTypeExternalReference, isCommentRequestTypePersistableState, } from '../../../common/utils/attachments'; -import type { CommentRequest, FindCommentsQueryParams } from '../../../common/api'; +import type { CommentRequest } from '../../../common/api'; import type { ExternalReferenceAttachmentTypeRegistry } from '../../attachment_framework/external_reference_registry'; import type { PersistableStateAttachmentTypeRegistry } from '../../attachment_framework/persistable_state_registry'; -import { validatePagination } from '../../common/validators'; export const validateRegisteredAttachments = ({ query, @@ -43,11 +41,3 @@ export const validateRegisteredAttachments = ({ ); } }; - -export const validateFindCommentsPagination = (params?: FindCommentsQueryParams) => { - validatePagination({ - page: params?.page, - perPage: params?.perPage, - maxPerPage: MAX_COMMENTS_PER_PAGE, - }); -}; diff --git a/x-pack/plugins/cases/server/client/cases/find.test.ts b/x-pack/plugins/cases/server/client/cases/find.test.ts index 4ca3eb46ca087..988aebd33e2af 100644 --- a/x-pack/plugins/cases/server/client/cases/find.test.ts +++ b/x-pack/plugins/cases/server/client/cases/find.test.ts @@ -12,6 +12,7 @@ import { MAX_ASSIGNEES_FILTER_LENGTH, MAX_CASES_PER_PAGE, MAX_CATEGORY_FILTER_LENGTH, + MAX_DOCS_PER_PAGE, MAX_REPORTERS_FILTER_LENGTH, MAX_TAGS_FILTER_LENGTH, } from '../../../common/constants'; @@ -155,7 +156,7 @@ describe('find', () => { const findRequest = createCasesClientMockFindRequest({ page: 209, perPage: 100 }); await expect(find(findRequest, clientArgs)).rejects.toThrowError( - `Error: The number of documents is too high. Paginating through more than 10,000 documents is not possible.` + `Error: The number of documents is too high. Paginating through more than ${MAX_DOCS_PER_PAGE} documents is not possible.` ); }); @@ -166,7 +167,7 @@ describe('find', () => { }); await expect(find(findRequest, clientArgs)).rejects.toThrowError( - `Error: The provided perPage value was too high. The maximum allowed perPage value is ${MAX_CASES_PER_PAGE}.` + `Error: The provided perPage value is too high. The maximum allowed perPage value is ${MAX_CASES_PER_PAGE}.` ); }); }); diff --git a/x-pack/plugins/cases/server/client/cases/find.ts b/x-pack/plugins/cases/server/client/cases/find.ts index 4f2c74122b2a1..aa2bb36768207 100644 --- a/x-pack/plugins/cases/server/client/cases/find.ts +++ b/x-pack/plugins/cases/server/client/cases/find.ts @@ -16,7 +16,6 @@ import { CasesFindResponseRt, } from '../../../common/api'; -import { validateFindCasesPagination } from './validators'; import { createCaseError } from '../../common/error'; import { asArray, transformCases } from '../../common/utils'; import { constructQueryOptions, constructSearch } from '../utils'; @@ -56,8 +55,6 @@ export const find = async ( try { const queryParams = decodeWithExcessOrThrow(CasesFindRequestRt)(params); - validateFindCasesPagination(queryParams); - throwIfCategoryParamTooLong(queryParams.category); /** diff --git a/x-pack/plugins/cases/server/client/cases/validators.test.ts b/x-pack/plugins/cases/server/client/cases/validators.test.ts deleted file mode 100644 index 2d0bfbc548628..0000000000000 --- a/x-pack/plugins/cases/server/client/cases/validators.test.ts +++ /dev/null @@ -1,52 +0,0 @@ -/* - * 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 { validateFindCasesPagination } from './validators'; -import { MAX_CASES_PER_PAGE } from '../../../common/constants'; - -const ERROR_MSG = - '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_CASES_PER_PAGE}.`; - -describe('validators', () => { - describe('validateFindCasessPagination', () => { - it('does not throw if only page is undefined', () => { - expect(() => validateFindCasesPagination({ perPage: 100 })).not.toThrowError(); - }); - - it('does not throw if only perPage is undefined', () => { - expect(() => validateFindCasesPagination({ page: 100 })).not.toThrowError(); - }); - - it('does not throw if page and perPage are defined and valid', () => { - expect(() => validateFindCasesPagination({ page: 2, perPage: 100 })).not.toThrowError(); - }); - - it('returns if page and perPage are undefined', () => { - expect(() => validateFindCasesPagination({})).not.toThrowError(); - }); - - it('returns if perPage < 0', () => { - expect(() => validateFindCasesPagination({ perPage: -1 })).not.toThrowError(); - }); - - it('throws if page > 10k', () => { - expect(() => validateFindCasesPagination({ page: 10001 })).toThrow(ERROR_MSG); - }); - - it('throws if perPage > 100', () => { - expect(() => validateFindCasesPagination({ perPage: MAX_CASES_PER_PAGE + 1 })).toThrowError( - ERROR_MSG_PER_PAGE - ); - }); - - it('throws if page * perPage > 10k', () => { - expect(() => validateFindCasesPagination({ page: 101, perPage: 100 })).toThrow(ERROR_MSG); - }); - }); -}); diff --git a/x-pack/plugins/cases/server/client/cases/validators.ts b/x-pack/plugins/cases/server/client/cases/validators.ts deleted file mode 100644 index 181c4c9f94da2..0000000000000 --- a/x-pack/plugins/cases/server/client/cases/validators.ts +++ /dev/null @@ -1,19 +0,0 @@ -/* - * 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 type { CasesFindRequest } from '../../../common/api'; - -import { MAX_CASES_PER_PAGE } from '../../../common/constants'; -import { validatePagination } from '../../common/validators'; - -export const validateFindCasesPagination = (params?: CasesFindRequest) => { - validatePagination({ - page: params?.page, - perPage: params?.perPage, - maxPerPage: MAX_CASES_PER_PAGE, - }); -}; diff --git a/x-pack/plugins/cases/server/client/user_actions/find.test.ts b/x-pack/plugins/cases/server/client/user_actions/find.test.ts index db042a0f6d41b..e0655bc1d2d51 100644 --- a/x-pack/plugins/cases/server/client/user_actions/find.test.ts +++ b/x-pack/plugins/cases/server/client/user_actions/find.test.ts @@ -5,7 +5,7 @@ * 2.0. */ -import { MAX_USER_ACTIONS_PER_PAGE } from '../../../common/constants'; +import { MAX_DOCS_PER_PAGE, MAX_USER_ACTIONS_PER_PAGE } from '../../../common/constants'; import { createMockClient } from '../metrics/test_utils/client'; import { createCasesClientMockArgs } from '../mocks'; import { find } from './find'; @@ -26,11 +26,11 @@ describe('findUserActions', () => { ).rejects.toThrow('invalid keys "foo"'); }); - it('throws when trying to fetch more than 10,000 items', async () => { + it(`throws when trying to fetch more than ${MAX_DOCS_PER_PAGE} items`, async () => { await expect( find({ caseId: 'test-case', params: { page: 209, perPage: 100 } }, client, clientArgs) ).rejects.toThrow( - 'Error: The number of documents is too high. Paginating through more than 10,000 documents is not possible.' + `Error: The number of documents is too high. Paginating through more than ${MAX_DOCS_PER_PAGE} documents is not possible.` ); }); @@ -48,7 +48,7 @@ describe('findUserActions', () => { clientArgs ) ).rejects.toThrow( - `Error: The provided perPage value was too high. The maximum allowed perPage value is ${MAX_USER_ACTIONS_PER_PAGE}.` + `Error: The provided perPage value is too high. The maximum allowed perPage value is ${MAX_USER_ACTIONS_PER_PAGE}.` ); }); }); diff --git a/x-pack/plugins/cases/server/client/user_actions/find.ts b/x-pack/plugins/cases/server/client/user_actions/find.ts index 6142210a76324..713c15fff0d54 100644 --- a/x-pack/plugins/cases/server/client/user_actions/find.ts +++ b/x-pack/plugins/cases/server/client/user_actions/find.ts @@ -19,7 +19,6 @@ import { createCaseError } from '../../common/error'; import { asArray } from '../../common/utils'; import type { CasesClient } from '../client'; import { decodeOrThrow } from '../../../common/api/runtime_types'; -import { validateFindUserActionsPagination } from './validators'; export const find = async ( { caseId, params }: UserActionFind, @@ -38,8 +37,6 @@ export const find = async ( const queryParams = decodeWithExcessOrThrow(UserActionFindRequestRt)({ ...params, types }); - validateFindUserActionsPagination(queryParams); - const [authorizationFilterRes] = await Promise.all([ authorization.getAuthorizationFilter(Operations.findUserActions), // ensure that we have authorization for reading the case diff --git a/x-pack/plugins/cases/server/client/user_actions/validators.test.ts b/x-pack/plugins/cases/server/client/user_actions/validators.test.ts deleted file mode 100644 index 405138d7ca8ab..0000000000000 --- a/x-pack/plugins/cases/server/client/user_actions/validators.test.ts +++ /dev/null @@ -1,54 +0,0 @@ -/* - * 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 { validateFindUserActionsPagination } from './validators'; -import { MAX_USER_ACTIONS_PER_PAGE } from '../../../common/constants'; - -const ERROR_MSG = - '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_USER_ACTIONS_PER_PAGE}.`; - -describe('validators', () => { - describe('validateFindUserActionsPagination', () => { - it('does not throw if only page is undefined', () => { - expect(() => validateFindUserActionsPagination({ perPage: 100 })).not.toThrowError(); - }); - - it('does not throw if only perPage is undefined', () => { - expect(() => validateFindUserActionsPagination({ page: 100 })).not.toThrowError(); - }); - - it('does not throw if page and perPage are defined and valid', () => { - expect(() => validateFindUserActionsPagination({ page: 2, perPage: 100 })).not.toThrowError(); - }); - - it('returns if page and perPage are undefined', () => { - expect(() => validateFindUserActionsPagination({})).not.toThrowError(); - }); - - it('returns if perPage < 0', () => { - expect(() => validateFindUserActionsPagination({ perPage: -1 })).not.toThrowError(); - }); - - it('throws if page > 10k', () => { - expect(() => validateFindUserActionsPagination({ page: 10001 })).toThrow(ERROR_MSG); - }); - - it('throws if perPage > 100', () => { - expect(() => - validateFindUserActionsPagination({ perPage: MAX_USER_ACTIONS_PER_PAGE + 1 }) - ).toThrowError(ERROR_MSG_PER_PAGE); - }); - - it('throws if page * perPage > 10k', () => { - expect(() => validateFindUserActionsPagination({ page: 101, perPage: 100 })).toThrow( - ERROR_MSG - ); - }); - }); -}); diff --git a/x-pack/plugins/cases/server/client/user_actions/validators.ts b/x-pack/plugins/cases/server/client/user_actions/validators.ts deleted file mode 100644 index c011cda32d9a0..0000000000000 --- a/x-pack/plugins/cases/server/client/user_actions/validators.ts +++ /dev/null @@ -1,18 +0,0 @@ -/* - * 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 { MAX_USER_ACTIONS_PER_PAGE } from '../../../common/constants'; -import { validatePagination } from '../../common/validators'; -import type { UserActionFindRequest } from '../../../common/api'; - -export const validateFindUserActionsPagination = (params?: UserActionFindRequest) => { - validatePagination({ - page: params?.page, - perPage: params?.perPage, - maxPerPage: MAX_USER_ACTIONS_PER_PAGE, - }); -}; diff --git a/x-pack/plugins/cases/server/common/validators.test.ts b/x-pack/plugins/cases/server/common/validators.test.ts deleted file mode 100644 index d48d4e7d17f38..0000000000000 --- a/x-pack/plugins/cases/server/common/validators.test.ts +++ /dev/null @@ -1,63 +0,0 @@ -/* - * 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 { validatePagination } from './validators'; - -const MAX_PER_PAGE = 666; - -const ERROR_MSG = - '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_PER_PAGE}.`; - -describe('validators', () => { - describe('validatePagination', () => { - it('does not throw if only page is undefined', () => { - expect(() => - validatePagination({ perPage: 100, maxPerPage: MAX_PER_PAGE }) - ).not.toThrowError(); - }); - - it('does not throw if only perPage is undefined', () => { - expect(() => validatePagination({ page: 100, maxPerPage: MAX_PER_PAGE })).not.toThrowError(); - }); - - it('does not throw if page and perPage are defined and valid', () => { - expect(() => - validatePagination({ page: 2, perPage: 100, maxPerPage: MAX_PER_PAGE }) - ).not.toThrowError(); - }); - - it('returns if page and perPage are undefined', () => { - expect(() => validatePagination({ maxPerPage: MAX_PER_PAGE })).not.toThrowError(); - }); - - it('returns if perPage < 0', () => { - expect(() => - validatePagination({ perPage: -1, maxPerPage: MAX_PER_PAGE }) - ).not.toThrowError(); - }); - - it('throws if page > 10k', () => { - expect(() => validatePagination({ page: 10001, maxPerPage: MAX_PER_PAGE })).toThrow( - ERROR_MSG - ); - }); - - it(`throws if perPage > maxPerPage parameter`, () => { - expect(() => - validatePagination({ perPage: MAX_PER_PAGE + 1, maxPerPage: MAX_PER_PAGE }) - ).toThrowError(ERROR_MSG_PER_PAGE); - }); - - it('throws if page * perPage > 10k', () => { - expect(() => - validatePagination({ page: 101, perPage: 100, maxPerPage: MAX_PER_PAGE }) - ).toThrow(ERROR_MSG); - }); - }); -}); diff --git a/x-pack/plugins/cases/server/common/validators.ts b/x-pack/plugins/cases/server/common/validators.ts deleted file mode 100644 index cdd66232713d2..0000000000000 --- a/x-pack/plugins/cases/server/common/validators.ts +++ /dev/null @@ -1,38 +0,0 @@ -/* - * 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 Boom from '@hapi/boom'; -import { MAX_DOCS_PER_PAGE } from '../../common/constants'; - -export const validatePagination = ({ - maxPerPage, - page, - perPage, -}: { - maxPerPage: number; - page?: number; - perPage?: number; -}) => { - if (page == null && perPage == null) { - return; - } - - const pageAsNumber = page ?? 0; - const perPageAsNumber = perPage ?? 0; - - if (perPageAsNumber > maxPerPage) { - throw Boom.badRequest( - `The provided perPage value was too high. The maximum allowed perPage value is ${maxPerPage}.` - ); - } - - if (Math.max(pageAsNumber, pageAsNumber * perPageAsNumber) > MAX_DOCS_PER_PAGE) { - throw Boom.badRequest( - 'The number of documents is too high. Paginating through more than 10,000 documents is not possible.' - ); - } -}; From 41cdde513fbf5260cb4719d7bd070873a83a87e4 Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Tue, 4 Jul 2023 13:19:11 +0000 Subject: [PATCH 4/8] [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix' --- .../tests/common/user_actions/find_user_actions.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/user_actions/find_user_actions.ts b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/user_actions/find_user_actions.ts index 8e3cbd543f580..d5340beea28d9 100644 --- a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/user_actions/find_user_actions.ts +++ b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/user_actions/find_user_actions.ts @@ -15,6 +15,7 @@ import { ConnectorTypes, FindTypes, } from '@kbn/cases-plugin/common/api'; +import { MAX_USER_ACTIONS_PER_PAGE } from '@kbn/cases-plugin/common/constants'; import { globalRead, noKibanaPrivileges, @@ -43,7 +44,6 @@ import { } from '../../../../common/lib/api'; import { FtrProviderContext } from '../../../../common/ftr_provider_context'; -import { MAX_USER_ACTIONS_PER_PAGE } from '@kbn/cases-plugin/common/constants'; // eslint-disable-next-line import/no-default-export export default ({ getService }: FtrProviderContext): void => { From e549867ab92bb45c346893a43f08f1cb3b58ba83 Mon Sep 17 00:00:00 2001 From: adcoelho Date: Tue, 4 Jul 2023 17:28:01 +0200 Subject: [PATCH 5/8] Fixed tests by making paginationSchema exact. --- x-pack/plugins/cases/common/api/cases/case.ts | 12 ++++++------ .../plugins/cases/common/api/cases/comment/index.ts | 12 ++++++------ .../common/api/cases/user_actions/operations/find.ts | 12 ++++++------ x-pack/plugins/cases/common/schema/index.test.ts | 6 ++---- x-pack/plugins/cases/common/schema/index.ts | 5 ++++- x-pack/plugins/cases/common/schema/types.ts | 4 ++-- .../cases/server/client/attachments/get.test.ts | 2 +- 7 files changed, 27 insertions(+), 26 deletions(-) diff --git a/x-pack/plugins/cases/common/api/cases/case.ts b/x-pack/plugins/cases/common/api/cases/case.ts index 6a6cc17a889c6..ff6b8416f24dd 100644 --- a/x-pack/plugins/cases/common/api/cases/case.ts +++ b/x-pack/plugins/cases/common/api/cases/case.ts @@ -232,8 +232,8 @@ const CasesFindRequestSearchFieldsRt = rt.keyof({ 'updated_by.profile_uid': null, }); -export const CasesFindRequestRt = rt.exact( - rt.intersection([ +export const CasesFindRequestRt = rt.intersection([ + rt.exact( rt.partial({ /** * Tags to filter by @@ -335,10 +335,10 @@ export const CasesFindRequestRt = rt.exact( * The category of the case. */ category: rt.union([rt.array(rt.string), rt.string]), - }), - paginationSchema({ maxPerPage: MAX_CASES_PER_PAGE }), - ]) -); + }) + ), + paginationSchema({ maxPerPage: MAX_CASES_PER_PAGE }), +]); export const CasesDeleteRequestRt = limitedArraySchema({ codec: NonEmptyString, diff --git a/x-pack/plugins/cases/common/api/cases/comment/index.ts b/x-pack/plugins/cases/common/api/cases/comment/index.ts index 00254c536127a..f53e7d8828930 100644 --- a/x-pack/plugins/cases/common/api/cases/comment/index.ts +++ b/x-pack/plugins/cases/common/api/cases/comment/index.ts @@ -288,17 +288,17 @@ export const CommentsFindResponseRt = rt.strict({ export const CommentsRt = rt.array(CommentRt); -export const FindCommentsQueryParamsRt = rt.exact( - rt.intersection([ +export const FindCommentsQueryParamsRt = rt.intersection([ + rt.exact( rt.partial({ /** * Order to sort the response */ sortOrder: rt.union([rt.literal('desc'), rt.literal('asc')]), - }), - paginationSchema({ maxPerPage: MAX_COMMENTS_PER_PAGE }), - ]) -); + }) + ), + paginationSchema({ maxPerPage: MAX_COMMENTS_PER_PAGE }), +]); export const BulkCreateCommentRequestRt = rt.array(CommentRequestRt); diff --git a/x-pack/plugins/cases/common/api/cases/user_actions/operations/find.ts b/x-pack/plugins/cases/common/api/cases/user_actions/operations/find.ts index 7406feaed4492..90dc408b5e886 100644 --- a/x-pack/plugins/cases/common/api/cases/user_actions/operations/find.ts +++ b/x-pack/plugins/cases/common/api/cases/user_actions/operations/find.ts @@ -27,15 +27,15 @@ const FindTypeFieldRt = rt.keyof(FindTypes); export type FindTypeField = rt.TypeOf; -export const UserActionFindRequestRt = rt.exact( - rt.intersection([ +export const UserActionFindRequestRt = rt.intersection([ + rt.exact( rt.partial({ types: rt.array(FindTypeFieldRt), sortOrder: rt.union([rt.literal('desc'), rt.literal('asc')]), - }), - paginationSchema({ maxPerPage: MAX_USER_ACTIONS_PER_PAGE }), - ]) -); + }) + ), + paginationSchema({ maxPerPage: MAX_USER_ACTIONS_PER_PAGE }), +]); export type UserActionFindRequest = rt.TypeOf; diff --git a/x-pack/plugins/cases/common/schema/index.test.ts b/x-pack/plugins/cases/common/schema/index.test.ts index 06716fcfcaa51..2a8ac186a5fe6 100644 --- a/x-pack/plugins/cases/common/schema/index.test.ts +++ b/x-pack/plugins/cases/common/schema/index.test.ts @@ -267,12 +267,10 @@ describe('schema', () => { PathReporter.report(paginationSchema({ maxPerPage: 3 }).decode({ page: 'a', perPage: 'b' })) ).toMatchInlineSnapshot(` Array [ - "Invalid value \\"a\\" supplied to : Pagination/page: (number | NumberFromString | undefined)/0: number", + "Invalid value \\"a\\" supplied to : Pagination/page: (number | NumberFromString)/0: number", "cannot parse to a number", - "Invalid value \\"a\\" supplied to : Pagination/page: (number | NumberFromString | undefined)/2: undefined", - "Invalid value \\"b\\" supplied to : Pagination/perPage: (number | NumberFromString | undefined)/0: number", + "Invalid value \\"b\\" supplied to : Pagination/perPage: (number | NumberFromString)/0: number", "cannot parse to a number", - "Invalid value \\"b\\" supplied to : Pagination/perPage: (number | NumberFromString | undefined)/2: undefined", ] `); }); diff --git a/x-pack/plugins/cases/common/schema/index.ts b/x-pack/plugins/cases/common/schema/index.ts index bafa80bb1b85e..764119b06159b 100644 --- a/x-pack/plugins/cases/common/schema/index.ts +++ b/x-pack/plugins/cases/common/schema/index.ts @@ -126,7 +126,10 @@ export const paginationSchema = ({ maxPerPage }: { maxPerPage: number }) => ); } - return rt.success(params); + return rt.success({ + ...(params.page != null && { page: pageAsNumber }), + ...(params.perPage != null && { perPage: perPageAsNumber }), + }); }), rt.identity, undefined diff --git a/x-pack/plugins/cases/common/schema/types.ts b/x-pack/plugins/cases/common/schema/types.ts index 267df495ba8c3..5e0d69749f01f 100644 --- a/x-pack/plugins/cases/common/schema/types.ts +++ b/x-pack/plugins/cases/common/schema/types.ts @@ -8,7 +8,7 @@ import * as rt from 'io-ts'; import { NumberFromString } from '../api/saved_object'; -const PageTypeRt = rt.union([rt.number, NumberFromString, rt.undefined]); +const PageTypeRt = rt.union([rt.number, NumberFromString]); type PageNumberType = rt.TypeOf; export interface PaginationType { @@ -16,5 +16,5 @@ export interface PaginationType { perPage: PageNumberType; } -export const PaginationSchemaRt = rt.partial({ page: PageTypeRt, perPage: PageTypeRt }); +export const PaginationSchemaRt = rt.exact(rt.partial({ page: PageTypeRt, perPage: PageTypeRt })); export type PartialPaginationType = Partial; diff --git a/x-pack/plugins/cases/server/client/attachments/get.test.ts b/x-pack/plugins/cases/server/client/attachments/get.test.ts index fe12fb4b19a3a..d093793417c7a 100644 --- a/x-pack/plugins/cases/server/client/attachments/get.test.ts +++ b/x-pack/plugins/cases/server/client/attachments/get.test.ts @@ -36,7 +36,7 @@ describe('get', () => { await expect( findComment( // @ts-expect-error: excess attribute - { caseID: 'mock-id', findQueryParams: { foo: 'bar' } }, + { caseID: 'mock-id', findQueryParams: { page: 2, perPage: 9, foo: 'bar' } }, clientArgs ) ).rejects.toThrowErrorMatchingInlineSnapshot( From 76f739d7e66b3d4089cf2d8eb7bf0e5e638fc379 Mon Sep 17 00:00:00 2001 From: lcawl Date: Tue, 4 Jul 2023 09:24:30 -0700 Subject: [PATCH 6/8] [DOCS] Generate API preview --- .../cases/case-apis-passthru.asciidoc | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/docs/api-generated/cases/case-apis-passthru.asciidoc b/docs/api-generated/cases/case-apis-passthru.asciidoc index f35863d23eb88..8970186559032 100644 --- a/docs/api-generated/cases/case-apis-passthru.asciidoc +++ b/docs/api-generated/cases/case-apis-passthru.asciidoc @@ -859,7 +859,7 @@ Any modifications made to this file will be overwritten.
Query Parameter — The page number to return. default: 1
perPage (optional)
-
Query Parameter — The number of items to return. default: 20
sortOrder (optional)
+
Query Parameter — The number of items to return. Limited to 100 items. default: 20
sortOrder (optional)
Query Parameter — Determines the sort order. default: desc
types (optional)
@@ -951,7 +951,7 @@ Any modifications made to this file will be overwritten.
Query Parameter — The page number to return. default: 1
perPage (optional)
-
Query Parameter — The number of items to return. default: 20
sortOrder (optional)
+
Query Parameter — The number of items to return. Limited to 100 items. default: 20
sortOrder (optional)
Query Parameter — Determines the sort order. default: desc
types (optional)
@@ -1278,7 +1278,7 @@ Any modifications made to this file will be overwritten.
Query Parameter — The page number to return. default: 1
perPage (optional)
-
Query Parameter — The number of items to return. default: 20
reporters (optional)
+
Query Parameter — The number of items to return. Limited to 100 items. default: 20
reporters (optional)
Query Parameter — Filters the returned cases by the user name of the reporter. default: null
search (optional)
@@ -1471,7 +1471,7 @@ Any modifications made to this file will be overwritten.
Query Parameter — The page number to return. default: 1
perPage (optional)
-
Query Parameter — The number of items to return. default: 20
reporters (optional)
+
Query Parameter — The number of items to return. Limited to 100 items. default: 20
reporters (optional)
Query Parameter — Filters the returned cases by the user name of the reporter. default: null
search (optional)
@@ -4156,7 +4156,6 @@ Any modifications made to this file will be overwritten.
  • findCaseConnectorsDefaultSpace_200_response_inner_config -
  • findCasesDefaultSpace_200_response -
  • findCasesDefaultSpace_assignees_parameter -
    • -
  • findCasesDefaultSpace_category_parameter -
  • findCasesDefaultSpace_owner_parameter -
  • findCasesDefaultSpace_searchFields_parameter -
  • getCaseCommentDefaultSpace_200_response -
    • @@ -4580,6 +4579,7 @@ Any modifications made to this file will be overwritten.
    settings
    settings
    severity (optional)
    severity_property
    tags
    array[String] The words and phrases that help categorize cases. It can be an empty array.
    +
    category (optional)
    String Category for the case. It could be a word or a phrase to categorize the case.
    title
    String A title for the case.
    @@ -4659,12 +4659,6 @@ Any modifications made to this file will be overwritten.
    -
    -

    findCasesDefaultSpace_category_parameter - Up

    -
    -
    -
    -

    findCasesDefaultSpace_owner_parameter - Up

    @@ -5056,6 +5050,7 @@ Any modifications made to this file will be overwritten.
    severity (optional)
    severity_property
    status (optional)
    status
    tags (optional)
    array[String] The words and phrases that help categorize cases.
    +
    category (optional)
    String Category for the case. It could be a word or a phrase to categorize the case.
    title (optional)
    String A title for the case.
    version
    String The current version of the case. To determine this value, use the get case or find cases APIs.
    From 7c1ebf9925f59a55d4f97bff4300c756e7fb3879 Mon Sep 17 00:00:00 2001 From: adcoelho Date: Wed, 5 Jul 2023 11:52:04 +0200 Subject: [PATCH 7/8] Address PR. --- x-pack/plugins/cases/common/schema/index.test.ts | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/x-pack/plugins/cases/common/schema/index.test.ts b/x-pack/plugins/cases/common/schema/index.test.ts index 2a8ac186a5fe6..c553fd3216502 100644 --- a/x-pack/plugins/cases/common/schema/index.test.ts +++ b/x-pack/plugins/cases/common/schema/index.test.ts @@ -240,19 +240,7 @@ describe('schema', () => { `); }); - it(`fails when page * perPage > ${MAX_DOCS_PER_PAGE}`, () => { - expect( - PathReporter.report( - paginationSchema({ maxPerPage: 3 }).decode({ page: MAX_DOCS_PER_PAGE, perPage: 2 }) - ) - ).toMatchInlineSnapshot(` - Array [ - "The number of documents is too high. Paginating through more than 10000 documents is not possible.", - ] - `); - }); - - it('valid NumberFromString work correctly', () => { + it('validate params as strings work correctly', () => { expect( PathReporter.report(paginationSchema({ maxPerPage: 3 }).decode({ page: '1', perPage: '2' })) ).toMatchInlineSnapshot(` From a297f3f1dabdbe7ae0d6cef00bbeb7287d351555 Mon Sep 17 00:00:00 2001 From: adcoelho Date: Wed, 5 Jul 2023 12:19:30 +0200 Subject: [PATCH 8/8] Addressing PR comments 2 --- x-pack/plugins/cases/common/schema/index.test.ts | 2 +- x-pack/plugins/cases/common/schema/types.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/cases/common/schema/index.test.ts b/x-pack/plugins/cases/common/schema/index.test.ts index c553fd3216502..008cbc86cae80 100644 --- a/x-pack/plugins/cases/common/schema/index.test.ts +++ b/x-pack/plugins/cases/common/schema/index.test.ts @@ -207,7 +207,7 @@ describe('schema', () => { `); }); - it('fails when page > maxPerPage', () => { + it('fails when perPage > maxPerPage', () => { expect(PathReporter.report(paginationSchema({ maxPerPage: 3 }).decode({ perPage: 4 }))) .toMatchInlineSnapshot(` Array [ diff --git a/x-pack/plugins/cases/common/schema/types.ts b/x-pack/plugins/cases/common/schema/types.ts index 5e0d69749f01f..d2ece26504254 100644 --- a/x-pack/plugins/cases/common/schema/types.ts +++ b/x-pack/plugins/cases/common/schema/types.ts @@ -11,10 +11,10 @@ import { NumberFromString } from '../api/saved_object'; const PageTypeRt = rt.union([rt.number, NumberFromString]); type PageNumberType = rt.TypeOf; -export interface PaginationType { +export interface Pagination { page: PageNumberType; perPage: PageNumberType; } export const PaginationSchemaRt = rt.exact(rt.partial({ page: PageTypeRt, perPage: PageTypeRt })); -export type PartialPaginationType = Partial; +export type PartialPaginationType = Partial;