Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[Cases] Filter Cases API guardrails #160705

Merged
merged 6 commits into from
Jun 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions x-pack/plugins/cases/common/api/cases/case.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,12 @@ import { CommentRt } from './comment';
import { CasesStatusResponseRt, CaseStatusRt } from './status';
import { CaseConnectorRt } from '../connectors/connector';
import { CaseAssigneesRt } from './assignee';
import {
MAX_ASSIGNEES_FILTER_LENGTH,
MAX_REPORTERS_FILTER_LENGTH,
MAX_TAGS_FILTER_LENGTH,
} from '../../constants';
import { limitedArraySchema } from '../../schema';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not for this PR but I think we should put our io-ts validation functions/helpers in a different folder. Given all the conversation around schemas, types, versioned APIs, etc the name of the folder is confusing hahaha.


export const AttachmentTotalsRt = rt.strict({
alerts: rt.number,
Expand Down Expand Up @@ -207,7 +213,7 @@ export const CasesFindRequestRt = rt.exact(
/**
* Tags to filter by
*/
tags: rt.union([rt.array(rt.string), rt.string]),
tags: rt.union([limitedArraySchema(rt.string, 0, MAX_TAGS_FILTER_LENGTH, 'tags'), rt.string]),
/**
* The status of the case (open, closed, in-progress)
*/
Expand All @@ -219,11 +225,17 @@ export const CasesFindRequestRt = rt.exact(
/**
* The uids of the user profiles to filter by
*/
assignees: rt.union([rt.array(rt.string), rt.string]),
assignees: rt.union([
limitedArraySchema(rt.string, 0, MAX_ASSIGNEES_FILTER_LENGTH, 'assignees'),
rt.string,
]),
/**
* The reporters to filter by
*/
reporters: rt.union([rt.array(rt.string), rt.string]),
reporters: rt.union([
limitedArraySchema(rt.string, 0, MAX_REPORTERS_FILTER_LENGTH, 'reporters'),
rt.string,
]),
/**
* Operator to use for the `search` field
*/
Expand Down
3 changes: 3 additions & 0 deletions x-pack/plugins/cases/common/constants/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,9 @@ 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_CATEGORY_FILTER_LENGTH = 100 as const;
export const MAX_TAGS_FILTER_LENGTH = 100 as const;
export const MAX_ASSIGNEES_FILTER_LENGTH = 100 as const;
export const MAX_REPORTERS_FILTER_LENGTH = 100 as const;

/**
* Validation
Expand Down
9 changes: 6 additions & 3 deletions x-pack/plugins/cases/docs/openapi/bundled.json
Original file line number Diff line number Diff line change
Expand Up @@ -3826,7 +3826,8 @@
"type": "array",
"items": {
"type": "string"
}
},
"maxItems": 100
}
]
}
Expand Down Expand Up @@ -3922,7 +3923,8 @@
"type": "array",
"items": {
"type": "string"
}
},
"maxItems": 100
}
]
},
Expand Down Expand Up @@ -4023,7 +4025,8 @@
"type": "array",
"items": {
"type": "string"
}
},
"maxItems": 100
}
]
},
Expand Down
3 changes: 3 additions & 0 deletions x-pack/plugins/cases/docs/openapi/bundled.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2323,6 +2323,7 @@ components:
- type: array
items:
type: string
maxItems: 100
category:
in: query
name: category
Expand Down Expand Up @@ -2389,6 +2390,7 @@ components:
- type: array
items:
type: string
maxItems: 100
example: elastic
search:
in: query
Expand Down Expand Up @@ -2460,6 +2462,7 @@ components:
- type: array
items:
type: string
maxItems: 100
example: tag-1
to:
in: query
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,5 @@ schema:
- type: string
- type: array
items:
type: string
type: string
maxItems: 100
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,5 @@ schema:
- type: array
items:
type: string
example: elastic
maxItems: 100
example: elastic
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,5 @@ schema:
- type: array
items:
type: string
example: tag-1
maxItems: 100
example: tag-1
37 changes: 36 additions & 1 deletion x-pack/plugins/cases/server/client/cases/find.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,12 @@ import { v1 as uuidv1 } from 'uuid';

import type { Case } from '../../../common/api';

import { MAX_CATEGORY_FILTER_LENGTH } from '../../../common/constants';
import {
MAX_ASSIGNEES_FILTER_LENGTH,
MAX_CATEGORY_FILTER_LENGTH,
MAX_REPORTERS_FILTER_LENGTH,
MAX_TAGS_FILTER_LENGTH,
} from '../../../common/constants';
import { flattenCaseSavedObject } from '../../common/utils';
import { mockCases } from '../../mocks';
import { createCasesClientMockArgs, createCasesClientMockFindRequest } from '../mocks';
Expand Down Expand Up @@ -114,5 +119,35 @@ describe('find', () => {
`Error: Too many categories provided. The maximum allowed is ${MAX_CATEGORY_FILTER_LENGTH}`
);
});

it(`throws an error when the tags array has ${MAX_TAGS_FILTER_LENGTH} items`, async () => {
const tags = Array(MAX_TAGS_FILTER_LENGTH + 1).fill('foobar');

const findRequest = createCasesClientMockFindRequest({ tags });

await expect(find(findRequest, clientArgs)).rejects.toThrowError(
`Error: The length of the field tags is too long. Array must be of length <= ${MAX_TAGS_FILTER_LENGTH}`
);
});

it(`throws an error when the assignees array has ${MAX_ASSIGNEES_FILTER_LENGTH} items`, async () => {
const assignees = Array(MAX_ASSIGNEES_FILTER_LENGTH + 1).fill('foobar');

const findRequest = createCasesClientMockFindRequest({ assignees });

await expect(find(findRequest, clientArgs)).rejects.toThrowError(
`Error: The length of the field assignees is too long. Array must be of length <= ${MAX_ASSIGNEES_FILTER_LENGTH}`
);
});

it(`throws an error when the reporters array has ${MAX_REPORTERS_FILTER_LENGTH} items`, async () => {
const reporters = Array(MAX_REPORTERS_FILTER_LENGTH + 1).fill('foobar');

const findRequest = createCasesClientMockFindRequest({ reporters });

await expect(find(findRequest, clientArgs)).rejects.toThrowError(
`Error: The length of the field reporters is too long. Array must be of length <= ${MAX_REPORTERS_FILTER_LENGTH}.`
);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,13 @@
import { v1 as uuidv1 } from 'uuid';

import expect from '@kbn/expect';
import { CASES_URL, MAX_CATEGORY_FILTER_LENGTH } from '@kbn/cases-plugin/common/constants';
import {
CASES_URL,
MAX_ASSIGNEES_FILTER_LENGTH,
MAX_CATEGORY_FILTER_LENGTH,
MAX_REPORTERS_FILTER_LENGTH,
MAX_TAGS_FILTER_LENGTH,
} from '@kbn/cases-plugin/common/constants';
import { Case, CaseSeverity, CaseStatuses, CommentType } from '@kbn/cases-plugin/common/api';
import { ALERTING_CASES_SAVED_OBJECT_INDEX } from '@kbn/core-saved-objects-server';
import { FtrProviderContext } from '../../../../common/ftr_provider_context';
Expand Down Expand Up @@ -307,20 +313,6 @@ export default ({ getService }: FtrProviderContext): void => {
});
});

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,
});
});
}

it('sorts by severity', async () => {
const case4 = await createCase(supertest, {
...postCaseReq,
Expand Down Expand Up @@ -349,10 +341,37 @@ export default ({ getService }: FtrProviderContext): void => {
});
});

it('unhappy path - 400s when more than the maximum category fields are supplied', async () => {
const category = Array(MAX_CATEGORY_FILTER_LENGTH + 1).fill('foobar');
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,
});
});
}

await findCases({ supertest, query: { category }, 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', () => {
Expand Down