[Security Solution] [Cases] Introduce case observables (phase 0 & 1) #190237
Conversation
lgestc
force-pushed
the
cases_observables
branch
2 times, most recently
from
b7c8f0e
to
b70f7cc
Compare
lgestc
force-pushed
the
cases_observables
branch
2 times, most recently
from
3ea6323
to
1dfdc9b
Compare
lgestc
force-pushed
the
cases_observables
branch
2 times, most recently
from
ad86b09
to
fa8ed50
Compare
lgestc
changed the title
lgestc
added
Team:ResponseOps
lgestc
added
backport:skip
lgestc
force-pushed
the
cases_observables
branch
from
eb73134
to
e1c0dd4
Compare
elastic-vault-github-plugin-prod
bot
requested a review
from a team
as a code owner
lgestc
force-pushed
the
cases_observables
branch
from
cc054cc
to
cc94167
Compare
lgestc
force-pushed
the
cases_observables
branch
from
83f88e2
to
5646f2f
Compare
lgestc
force-pushed
the
cases_observables
branch
from
1a58c5d
to
050d36b
Compare
lgestc
force-pushed
the
cases_observables
branch
from
cfa120c
to
3f3b97f
Compare
cnasikas
removed
backport:skip
ioc and the other field is remove now, will improve the validation + the weird state issue is in progress |
|
I started testing and here are some first notes I quickly noticed: After adding an observable to a case, I do not see an entry in the Screen.Recording.2024-11-12.at.4.45.19.PM.movIn the Requirements section of this document it is written that observables should be searchable, but the UI does not contain a search bar. I see a search bar on the
I feel like we should improve slightly the values rendered in the
When adding an observable to a case, right now (unless I'm mistaken) we only allow users to create new observables. It could be nice to have a list of already created observables to pick from... But we might not be able to do that due to how observables are being saved. If they are only saved within a case, then we would have to fetch observables for all cases which could be inefficient... |
Yeah searchability has been skipped for now, as for the list - I am open to remove columns for instance, as what should be displayed in there has not really been described. |
|
added some simple client side validation |
There was a problem hiding this comment.
I started reviewing the frontend code from a cases perspective.
I tested the UI a bit too and have a couple of comments:
- We should introduce some validation when creating observables in a case. Having a
URLbe1234does not make sense. - I think it would make sense to limit the columns seen in the
Similarpage.Name,observables,tags. Something like this but not the same as the cases list page. - We should limit the number of observable types that the user can create.
- In the settings page if I edit an existing observable and close the flyout. If I click
Add new observablethe old observable will still be in the flyout. (See video below)
Screen.Recording.2024-11-13.at.14.46.07.mov
| @@ -60,6 +71,16 @@ export const convertCaseToCamelCase = (theCase: Case): CaseUI => { | |||
|
|
|||
| export const convertCasesToCamelCase = (cases: Cases): CasesUI => cases.map(convertCaseToCamelCase); | |||
|
|
|||
| export const convertSimilarCaseToCamelCase = (theCase: SimilarCase): SimilarCaseUI => { | |||
There was a problem hiding this comment.
nit: doesn't need to be exported if it's only used in this file
x-pack/plugins/cases/public/components/case_view/case_view_tabs.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Could you add a section on x-pack/plugins/cases/public/components/configure_cases/index.test.tsx to test these changes? There's one for connectors, custom fields, templates, etc.
| @@ -61,6 +68,35 @@ const FilesBadge = ({ | |||
|
|
|||
| FilesBadge.displayName = 'FilesBadge'; | |||
|
|
|||
| const ObservablesBadge = ({ | |||
There was a problem hiding this comment.
Could you update x-pack/plugins/cases/public/components/case_view/case_view_tabs.test.tsx to reflect the changes in this file?
| @@ -526,6 +613,26 @@ export const ConfigureCases: React.FC = React.memo(() => { | |||
| </CommonFlyout> | |||
| ) : null; | |||
|
|
|||
| const AddOrEditObservableTypeFlyout = | |||
There was a problem hiding this comment.
We should include some limit to the number of Observables Types that can be created. Right now there seems to be no limit.
x-pack/plugins/cases/public/components/observable_types/index.tsx
Outdated
Show resolved
Hide resolved
| const [error, setError] = useState<boolean>(false); | ||
|
|
||
| const onAddCustomField = useCallback(() => { | ||
| if (observableTypes.length === MAX_CUSTOM_FIELDS_PER_CASE && !error) { |
There was a problem hiding this comment.
MAX_CUSTOM_FIELDS_PER_CASE seems to be wrong here.
this should be fixed now:) |
There was a problem hiding this comment.
I did another pass only on the common and backend code. I left some comments. Also:
- As we discussed we should validate observable values in the UI.
- We should validate observable types (key and label) both in the UI and the API (they are public).
- We should validate the number of max observables per case in the UI.
- Missing unit tests:
- x-pack/plugins/cases/public/api/utils.ts
- x-pack/plugins/cases/server/client/cases/observables.ts
- x-pack/plugins/cases/server/routes/api/cases/similar.ts
- x-pack/plugins/cases/server/routes/api/observables/delete_observable.ts
- x-pack/plugins/cases/server/routes/api/observables/patch_observable.ts
- x-pack/plugins/cases/server/routes/api/observables/post_observable.ts
| @@ -83,7 +83,12 @@ export const INTERNAL_DELETE_FILE_ATTACHMENTS_URL = | |||
| export const INTERNAL_GET_CASE_CATEGORIES_URL = `${CASES_INTERNAL_URL}/categories` as const; | |||
| export const INTERNAL_CASE_METRICS_URL = `${CASES_INTERNAL_URL}/metrics` as const; | |||
| export const INTERNAL_CASE_METRICS_DETAILS_URL = `${CASES_INTERNAL_URL}/metrics/{case_id}` as const; | |||
| export const INTERNAL_CASE_SIMILAR_CASES_URL = `${CASES_INTERNAL_URL}/_similar/{case_id}` as const; | |||
There was a problem hiding this comment.
I think it is better to follow the REST principles and conventions we have in the codebase for the URL.
| export const INTERNAL_CASE_SIMILAR_CASES_URL = `${CASES_INTERNAL_URL}/_similar/{case_id}` as const; | |
| export const INTERNAL_CASE_SIMILAR_CASES_URL = `${CASES_INTERNAL_URL}/{case_id}/_similar` as const; |
| /** | ||
| * Exporting an array of built-in observable types for use in the application | ||
| */ | ||
| export const OBSERVABLE_TYPES_BUILTIN = [ |
There was a problem hiding this comment.
I thought about it more and it is ok to have the same keys across solutions and the same options to all solutions.
| */ | ||
|
|
||
| import * as rt from 'io-ts'; | ||
| import { CaseObservableRt, ObservablePatch } from '../../domain'; |
There was a problem hiding this comment.
It seems that the ObservablePatch is only related to APIs. Better to move the type in this file or just import CaseObservableBaseRt and use it as the payload of the requests. You can import from the domain as you like but you have to import from v1 like ../../domain/observables/v1.
| observable: ObservablePatch, | ||
| }); | ||
|
|
||
| export const BulkGetObservablesResponseRt = rt.strict({ |
There was a problem hiding this comment.
This type is not used anywhere.
| export type AddObservableRequest = rt.TypeOf<typeof AddObservableRequestRt>; | ||
| export type UpdateObservableRequest = rt.TypeOf<typeof UpdateObservableRequestRt>; | ||
|
|
||
| export type ObservableRequest = AddObservableRequest; |
There was a problem hiding this comment.
This type is not used anywhere.
|
|
||
| const newObservableData = { | ||
| value: 'test', | ||
| typeKey: '0ad4bf8e-575f-49ad-87ea-8bcafd5173e4', |
There was a problem hiding this comment.
The typeKey is not part of the configuration. Shouldn't the addObservable throw an error? Let's have the following tests:
- Valid configuration exists and you can add an observable of a custom observable type
- Valid configuration does not exist and you cannot add an observable
- You can add a build in observable type
| }); | ||
| }); | ||
|
|
||
| describe('shows similar cases', () => { |
There was a problem hiding this comment.
Tests for similar cases should be on each own file as it is not an API related to observables (in the sense of REST resource)
| const { cases: casesAfterObservablesAreAdded } = await similarCases({ | ||
| supertest, | ||
| body: { perPage: 10, page: 1 }, | ||
| caseId: caseA.id, |
There was a problem hiding this comment.
Let's do another call for caseB.
| }); | ||
| }); | ||
|
|
||
| describe('shows similar cases', () => { |
There was a problem hiding this comment.
Could you please add a test where cases from different solutions (different owner) are not returned even if they have the same observables?
| ]); | ||
|
|
||
| const newObservableData = { | ||
| value: '{"foo": "bar"}', |
There was a problem hiding this comment.
I do not think we should support this kind of values in the MVP. Also OBSERVABLE_TYPES_BUILTIN[0].key is an IPv4.
⏳ Build in-progress
History
|
Summary
Introducting Case Observables - phases 0 and 1
This pull request introduces case observables to Kibana, enhancing the platform's case management capabilities. It adds support for capturing and displaying observables (e.g., IP addresses, URLs, file hashes) linked to cases. The feature integrates with the Cases UI, allowing users to easily associate observables with cases for better tracking and analysis in incident response workflows. This improves investigative efficiency by correlating observables across multiple cases.
Requirements:
https://docs.google.com/document/d/12hZTpyn0eXy3Xnq8qLBd6_sJxBhNZoI7vXztxWHhUds/edit#heading=h.srf6mb8ifiad
Design document: https://docs.google.com/document/d/1MeDLl6OEWast1RC1M3_hQXnRCd8frrXdGkFnypIYKJQ/edit#heading=h.kb5lrp2j62id
Notable Cases sections are added in this pr:
1. Observables section in the case view, allowing for adding and listing up to 10 observables for the case
2. Similar cases view for every case, allowing for similar case discovery
3. Observable types management view in Cases settings
Original issue:
#180360
Things skipped for now from MVP: