[Case] Detection rules for case #88726

jonathan-buttner · 2021-01-19T18:32:27Z

This PR adds support in the case backend for attaching generated alerts from detection rules to a case.

Notable changes:

A case can be either an individual case or a collection
- Individual cases are the same pre 7.12
- Collection cases contain multiple sub cases and do not have a status
  - Collections can only be deleted using a force flag which defaults to false
Sub cases are attached to a case and share all the case information (title, tags, etc) except for status (they each have a status)
An individual case can be converted to a collection if it does not have any alerts attached to it
Detection rule alerts can be attached to sub cases and will have the type generated_alert (this allows the UI to distinguish between generated alerts and user added alerts)
APIs
- get sub case by ID
- find sub cases under a specific collection
- patch multiple sub cases within multiple collections
- delete sub cases

…es-for-case

michaelolo24 · 2021-02-11T20:30:37Z

x-pack/plugins/case/server/client/cases/update.ts

+  };
+
+  const requestsUpdatingTypeField = requests.filter((req) => req.type === CaseType.collection);
+  const casesAlertTotals = await Promise.all(


michaelolo24 · 2021-02-11T20:35:43Z

x-pack/plugins/case/server/client/cases/update.ts

+  });
+
+  return subCases.saved_objects.reduce((acc, subCase) => {
+    // log about the sub cases that we couldn't find


Is this a Todo to notify the user about situations where there is an error?

Yeah it's really a TODO to add logging later

michaelolo24 · 2021-02-11T20:38:00Z

x-pack/plugins/case/server/client/cases/update.ts

+  let status: CaseStatuses = CaseStatuses.open;
+  if (alertComment.attributes.associationType === AssociationType.case) {
+    const id = getID(alertComment, CASE_SAVED_OBJECT);
+    // We should log if we can't find the status


Another log comment. I agree, maybe a parameter sent with the response errors: ['We were unable to retrieve x, y, and z']

Yeah I'll add the logging in a follow up PR. I need to pass the plugin's logger around to do it I think.

michaelolo24 · 2021-02-11T20:43:27Z

x-pack/plugins/case/server/client/client.ts

+/**
+ * This class is a pass through for common case functionality (like creating, get a case).
+ */
+export class CaseClientHandler implements CaseClient {


Clean 👌🏾

michaelolo24 · 2021-02-11T20:48:44Z

x-pack/plugins/case/server/client/comments/add.ts

  });

-  // An alert cannot be attach to a closed case.
-  if (query.type === CommentType.alert && myCase.attributes.status === CaseStatuses.closed) {


Do we want to keep this check though?

Yeah, it should be in CommentableCase now haha.

michaelolo24 · 2021-02-11T20:53:09Z

x-pack/plugins/case/server/client/comments/add.ts

+    commentReq: query,
+  });
+
+  if (newComment.attributes.type === CommentType.alert && updatedCase.settings.syncAlerts) {


not necessary for this PR, but could be helpful to just have all these checks encapsulated in functions like isAlert() isCollection() isGeneratedAlert etc... especially if the criteria for any of these changes in the future

Great idea. I'll add it to the refactoring list 👍

michaelolo24 · 2021-02-11T20:57:44Z

x-pack/plugins/case/server/client/types.ts

 import { CaseClientGetAlertsResponse } from './alerts/types';

-export interface CaseClientCreate {


More of a question. We seem to be passing in the post and patch request types while the others are defined here? Is there a reason they're separated?

I think that might have been from more of a convenience thing where some parameter types were defined already by the io-ts types and we just pass those fields around where as others don't strictly align with an io-ts type.

michaelolo24 · 2021-02-11T21:00:28Z

x-pack/plugins/case/server/common/models/commentable_case.ts

+  }): Promise<NewCommentResp> {
+    if (commentReq.type === CommentType.alert) {
+      if (this.status === CaseStatuses.closed) {
+        throw Boom.badRequest('Alert cannot be attached to a closed case');


Ignore my previous comment about the removed check 😄

michaelolo24 · 2021-02-11T21:04:20Z

x-pack/plugins/case/server/common/utils.ts

+      if (alert.id) {
+        const totalAlerts = acc.get(alert.id);
+        if (totalAlerts !== undefined) {
+          acc.set(alert.id, totalAlerts + countAlerts(alertsInfo));


You can cache countAlerts const alertCount = countAlerts(alertsInfo) and use that on 105 maybe?

Good point, will update

michaelolo24 · 2021-02-11T21:08:31Z

x-pack/plugins/case/server/connectors/index.ts

+  );
+};
+
+export const isCommentAlert = (


nice 👍🏾, this was the idea for all the other types of checks as well that I mentioned earlier

nit: I see that the type is not used anywhere. Maybe we can remove it.

kibanamachine · 2021-02-12T01:57:53Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`securitySolution`	2187	2189	+2

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`securitySolution`	7.5MB	7.5MB	+796.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`securitySolution`	234.0KB	237.2KB	+3.1KB

Saved Objects .kibana field count

Every field in each saved object type adds overhead to Elasticsearch. Kibana needs to keep the total field count below Elasticsearch's default limit of 1000 fields. Only specify field mappings for the fields you wish to search on or query. See https://www.elastic.co/guide/en/kibana/master/development-plugin-saved-objects.html#_mappings

id	before	after	diff
`cases`	40	41	+1
`cases-comments`	20	21	+1
`cases-sub-case`	-	17	+17
total			+19

History

💛 Build #106145 was flaky 0c98e1c
💛 Build #105881 was flaky 575d849
💛 Build #105857 was flaky 57b8414
💚 Build #105459 succeeded 8a553c2
💔 Build #105420 failed 0cce723

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cnasikas · 2021-02-12T09:16:03Z

x-pack/plugins/case/server/client/comments/add.ts

+    });
+  }
+
+  decodeCommentRequest(comment);


Comment for another PR: TypeScript thinks that the comment is of type user and not alert | user.

cnasikas · 2021-02-12T09:20:27Z

x-pack/plugins/case/server/client/comments/add.ts

+  } = await commentableCase.createComment({ createdDate, user: userDetails, commentReq: query });
+
+  if (
+    (newComment.attributes.type === CommentType.alert ||


As you throw an error before if comment.type !== CommentType.generatedAlert it should be impossible the new comment to be of type anything other than CommentType.generatedAlert. Never hurts to double check though 😄

haha good point.

cnasikas · 2021-02-12T12:15:23Z

x-pack/plugins/case/server/common/models/commentable_case.ts

+      });
+
+      return CollectWithSubCaseResponseRt.encode({
+        subCase: flattenSubCaseSavedObject({


Shouldn't we return subCases: [flattenSubCaseSavedObject(...)]?

Yeah I talked with Xavier about this and we thought for now we'd keep it as a single entry rather than an array of sub cases for the add comment response.

cnasikas · 2021-02-12T12:22:20Z

x-pack/plugins/case/server/connectors/case/index.ts

-    response: kibanaResponseFactory,
+    scopedClusterClient,
+    // we might want the user information to be passed as part of the action request
+    user: nullUser,


I found out that you can scopedClusterClient.security.getUser(). Just for future reference in case we need it.

oh interesting! I'll add it to the list of stuff to refactor 👍

cnasikas · 2021-02-12T12:25:03Z

x-pack/plugins/case/server/connectors/index.ts

+): comment is ContextTypeGeneratedAlertType => {
+  return (
+    comment.type === CommentType.generatedAlert &&
+    'alerts' in comment &&


I think the type check is enough, right?

Just being extra careful since we need to access the alerts field.

cnasikas · 2021-02-12T12:31:45Z

x-pack/plugins/case/server/routes/api/cases/comments/find_comments.ts

        const args = query
          ? {
+              caseService,


nit:

const args = {caseService, client, id, options: { page: defaultPage, perPage: defaultPerPage, sortField: 'created_at', ...(query ? query : {}) }, associationType}

cnasikas · 2021-02-12T12:40:35Z

x-pack/plugins/case/server/routes/api/cases/delete_cases.ts

+
+  await Promise.all(
+    subCasesForCaseIds.saved_objects.map((subCaseSO) =>
+      caseService.deleteSubCase(client, subCaseSO.id)


For another PR: I think maybe is better to first delete the subcase and then the comments if the corresponding subcase has been deleted successfully. This will prevent a use case where all the comments of a subcase have been deleted and the deletion of the subcase faild.

cnasikas · 2021-02-12T12:59:54Z

x-pack/plugins/case/server/routes/api/utils.ts

+/**
+ * Builds an AlertInfo object accumulating the alert IDs and indices for the passed in alerts.
+ */
+export const getAlertIndicesAndIDs = (comments: CommentAttributes[] | undefined): AlertInfo => {


nit: getAlertIndicesAndIDs and getAlertIndicesAndIDsFromSO are almost identical. They can be combined in one function.

Apparently I don't even use getAlertIndicesAndIDsFromSO anywhere haha 🤦 I'll remove it in the next PR.

cnasikas · 2021-02-12T13:02:49Z

x-pack/plugins/case/server/routes/api/utils.ts

+
+/**
+ * This is used to test if the posted comment is an generated alert. A generated alert will have one or many alerts.
+ * An alert is essentially an object with a _id field. This differs from a regular attached alert because the _id is


nit: generated alerts do not have an _id field any more. Fields of alert and generated_alert is the same.

Yep thanks.

cnasikas · 2021-02-12T13:05:05Z

x-pack/plugins/case/server/services/alerts/index.ts

@@ -5,24 +5,26 @@
 * 2.0.
 */

+import _ from 'lodash';


nit: Better to use import { isEmpty } from lodash/fp

cnasikas

Very good PR! The code is very clean and readable. Very important for the complexity of this feature. Thank you for the effort you put on this. Great job!

I left some nit/code enhancement comments. Let's work together on another PR to do them.

Tests I preformed:

Add a user comment to a collection ✅
Add an alert to a collection: 400 error: Alert cannot be attached to a collection case ✅
Add a generated alert to a collection. The generated alert is added to the subcase and not to the collection ✅
Attach an alert and a user comment to a subcase ✅
Attach a generated alert to a sub case using the cases route: 404 error, case not found ✅.
Add an alert and user comment to an individual case ✅
Add generated alert to an individual case: 400 error ✅
Add connector to an individual case and to a collection case ✅
Add connector to a subcase: 406: All update fields are identical to current version. ✅
Push individual case to external service with alerts & user comments ✅
Push subcase to external service using the case route: 404 error ✅
Patching a subcase using the case route: 404 error ✅
Push a collection to an external service ✅
Update the status of an individual case with sync turned off does not update the alerts attached ✅
Updating the status of an individual case with sync turned on updates the alerts correctly ✅
Updating the status of a collection is not allowed ✅
Update the status of a subcase ✅
Case collection: Turn sync off, change the status of a subcase, turn sync on, all alerts attach to the subcase change their status accordingly ✅
Attach a generated_alert to a collection creates another sub case if all subcases are closed ✅
Attach a generated_alert to a collection does NOT creates another sub case if there is an open subcase ✅
Case collection, sync alert is off: two subcases, one in-progress, one closed. Turn sync on. All alerts are updated accordingly ✅.
Two subcases: one in-progess, one open. Attaching generated_alerts to the collection is added to the most recent subcase ✅
Update collection & individual case fields like title, tags, comments, connector fields etc from the UI ✅
Push updates to external service correctly ✅
Creates a case correctly from the UI. ✅
Converting a collection to individual throws an error ✅
Converting an individual case to a collection when having alerts attached throws an error ✅

Notes:

If I am working on a collection case (in the UI) and a subcase is generate behind the scene I have to refresh for me to be able to do a change. One day we have to have an observable/websocket to watch for cases updates and update on the fly as Github does.
We should use excess in the future when patching subcases & cases.

* Adding type field to client * Removing context and adding association type * Handle alerts from multiple indices * Adding flow for adding a sub case * Making progress on creating alerts from rules * Refactored add comment to handle case and sub case * Starting sub case API and refactoring of case client * Fleshing out find cases * Finished the find cases api * Filtering comments by association type * Fixing tests and types * Updating snapshots * Cleaning up comment references * Working unit tests * Fixing integration tests and got ES to work * Unit tests and api integration test working * Refactoring find and get_status * Starting patch, and update * script for sub cases * Removing converted_by and fixing type errors * Adding docs for script * Removing converted_by and fixing integration test * Adding sub case id to comment routes * Removing stringify comparison * Adding delete api and tests * Updating license * missed license files * Integration tests passing * Adding more tests for sub cases * Find int tests, scoped client, patch sub user actions * fixing types and call cluster * fixing get sub case param issue * Adding user actions for sub cases * Preventing alerts on collections and refactoring user * Allowing type to be updated for ind cases * Refactoring and writing tests * Fixing sub case status filtering * Adding more tests not allowing gen alerts patch * Working unit tests * Push to connector gets all sub case comments * Writing more tests and cleaning up * Updating push functionality for generated alerts and sub cases * Adding comment about updating collection sync * Refactoring update alert status for sub cases and removing request and cleaning up * Addressing alert service feedback * Fixing sub case sync bug and cleaning up comment types * Addressing more feedback Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Kibana Machine <[email protected]>

This reverts commit f8b8d5b.

jonathan-buttner added 5 commits January 13, 2021 14:17

Adding type field to client

1346445

Removing context and adding association type

9f82a3d

Handle alerts from multiple indices

edf97b3

Merge branch 'master' of github.com:elastic/kibana into detection-rul…

e10e620

…es-for-case

Adding flow for adding a sub case

ada3eb9

jonathan-buttner added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.12.0 Team:Threat Hunting Security Solution Threat Hunting Team Feature:Cases Cases feature labels Jan 19, 2021

jonathan-buttner added 20 commits January 19, 2021 17:34

Making progress on creating alerts from rules

695df14

Merge branch 'master' of github.com:elastic/kibana into detection-rul…

a01a70f

…es-for-case

Refactored add comment to handle case and sub case

7c012c7

Starting sub case API and refactoring of case client

914a263

Merge branch 'master' of github.com:elastic/kibana into detection-rul…

cc57dd9

…es-for-case

Fleshing out find cases

26a02fc

Finished the find cases api

2d1b4e9

Merge branch 'master' of github.com:elastic/kibana into detection-rul…

c5f47b3

…es-for-case

Filtering comments by association type

c58ddb0

Fixing tests and types

fcc40c0

Updating snapshots

bc62907

Cleaning up comment references

2507213

Working unit tests

d486483

Fixing integration tests and got ES to work

2ee61b0

Unit tests and api integration test working

34fb160

Refactoring find and get_status

435b741

Merge branch 'master' of github.com:elastic/kibana into detection-rul…

1b45ca5

…es-for-case

Starting patch, and update

a4f4114

Merge branch 'master' of github.com:elastic/kibana into detection-rul…

5f4bbb3

…es-for-case

Merge branch 'master' of github.com:elastic/kibana into detection-rul…

060db5b

…es-for-case

Merge branch 'master' of github.com:elastic/kibana into detection-rul…

0c98e1c

…es-for-case

michaelolo24 reviewed Feb 11, 2021

View reviewed changes

Addressing more feedback

99d37ae

cnasikas reviewed Feb 12, 2021

View reviewed changes

cnasikas approved these changes Feb 12, 2021

View reviewed changes

jonathan-buttner merged commit f8b8d5b into elastic:master Feb 12, 2021

This was referenced Feb 12, 2021

[7.x] [Case] Detection rules for case (#88726) #91290

Merged

[Security Solution][Case] Attach multiple alerts to case #91292

Closed

spalger added a commit to spalger/kibana that referenced this pull request Feb 13, 2021

Revert "[Case] Detection rules for case (elastic#88726)"

5a471d6

This reverts commit f8b8d5b.

cnasikas mentioned this pull request Mar 1, 2021

[Security Solution][Case] Create comments of type alert from case connector #85750

Closed

		import { CaseClientGetAlertsResponse } from './alerts/types';

		export interface CaseClientCreate {

[Case] Detection rules for case #88726

[Case] Detection rules for case #88726

Conversation

jonathan-buttner commented Jan 19, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cnasikas Feb 12, 2021 • edited Loading

Choose a reason for hiding this comment

kibanamachine commented Feb 12, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

Saved Objects .kibana field count

History

Choose a reason for hiding this comment

cnasikas Feb 12, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cnasikas Feb 12, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cnasikas left a comment • edited Loading

Choose a reason for hiding this comment

jonathan-buttner commented Jan 19, 2021 •

edited

Loading

cnasikas Feb 12, 2021 •

edited

Loading

cnasikas Feb 12, 2021 •

edited

Loading

cnasikas Feb 12, 2021 •

edited

Loading

cnasikas left a comment •

edited

Loading