[Cases] Create Bulk get cases internal API #147674

cnasikas · 2022-12-16T09:12:12Z

Summary

This PR creates the bulk get cases internal API. The endpoint is needed for the alerts table to be able to get all cases the alerts are attached to with one call.

Reference: #146864

Request

ids: (Required, array) An array of IDs of the retrieved cases.
fields: (Optional, array) The fields to return in the attributes key of the object response.

POST <kibana host>:<port>/internal/cases/_bulk_get
{
    "ids": ["case-id-1", "case-id-2", "123", "not-authorized"],
    "fields": ["title"]
}

Response

{
    "cases": [
     {
        "title": "case1",
        "owner": "securitySolution",
        "id": "case-id-1",
        "version": "WzIzMTU0NSwxNV0="
     },
     {
        "title": "case2",
        "owner": "observability",
        "id": "case-id-2",
        "version": "WzIzMTU0NSwxNV0="
      }
    ],
    "errors": [
        {
            "error": "Not Found",
            "message": "Saved object [cases/123] not found",
            "status": 404,
            "caseId": "123"
        },
        {
            "error": "Forbidden",
            "message": "Unauthorized to access case with owner: \"cases\"",
            "status": 403,
            "caseId": "not-authorized"
        }
    ]
}

Checklist

Delete any items that are not applicable to this PR.

Unit or functional tests were updated or added to match the most common scenarios

For maintainers

This was checked for breaking API changes and was labeled appropriately

elasticmachine · 2022-12-16T09:12:15Z

Pinging @elastic/response-ops (Team:ResponseOps)

elasticmachine · 2022-12-16T09:12:15Z

Pinging @elastic/response-ops-cases (Feature:Cases)

thomheymann

All looking good. Only thing I'd say is to update docs when you add new audit events: docs/user/security/audit-logging.asciidoc

x-pack/plugins/cases/server/authorization/authorization.ts

x-pack/plugins/cases/server/authorization/utils.test.ts

x-pack/plugins/cases/server/client/cases/bulk_get.ts

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

x-pack/test/cases_api_integration/security_and_spaces/tests/common/internal/bulk_get_cases.ts

jonathan-buttner

Looking good! Left a few comments and questions

jonathan-buttner · 2023-01-04T13:42:41Z

docs/user/security/audit-logging.asciidoc

+| `success` | User has accessed multiple case.
+| `failure` | User is not authorized to access multiple case.


jonathan-buttner · 2023-01-04T13:43:26Z

x-pack/plugins/cases/common/api/cases/case.ts

@@ -325,6 +325,22 @@ export const AllTagsFindRequestRt = rt.partial({

 export const AllReportersFindRequestRt = AllTagsFindRequestRt;

+export const CasesBulkGetRequestRt = rt.type({
+  ids: rt.array(rt.string),
+  fields: rt.union([rt.undefined, rt.array(rt.string), rt.string]),


Should we make this optional instead of having it accept undefined?

To my understanding, this is how you define "optional" to io-ts. At least this is what we have for other optional query parameters in the find request. I tested without the fields query parameter and is working as expected. Are you aware of another way to do it?

Yeah that's weird, I don't know why that's working. I believe to make it optional we need to do an intersection and partial:

const CasesBulkGetRequestRt = t.intersection([ t.type({ ids: rt.array(rt.string), }), t.partial({ fields: rt.union([rt.undefined, rt.array(rt.string), rt.string]), }) ])

jonathan-buttner · 2023-01-04T13:58:57Z

x-pack/plugins/cases/server/authorization/authorization.ts

+        AuthorizationAuditLogger.createFailureMessage({ owners: authorizedOwners, operation })
+      );
+
+      this.auditLogger.log({ error, operation });


When a user attempts to delete multiple cases and the authorization fails we will log an entry for each entity (delete is an all or nothing so we would log for each entity in the request). How about we call this.logSavedObjects() and loop over the entities as well?

jonathan-buttner · 2023-01-04T14:02:17Z

x-pack/plugins/cases/server/authorization/utils.ts

+  savedObjects: Array<SavedObject<T>>,
+  authorizedEntities: OwnerEntity[]
+): [Array<SavedObject<T>>, Array<SavedObject<T>>] =>
+  partition(savedObjects, (so) =>


How about we return an object that names which is which? Like:

{ authorized: [...], unauthorized: [...] }

jonathan-buttner · 2023-01-04T14:03:22Z

x-pack/plugins/cases/server/authorization/utils.ts

@@ -64,3 +66,11 @@ export const includeFieldsRequiredForAuthentication = (fields?: string[]): strin
  }
  return uniq([...fields, OWNER_FIELD]);
 };
+
+export const getAuthorizedAndUnauthorizedSavedObjects = <T>(


How about we rename this to groupByAuthorization or partitionByAuthorization something like that.

jonathan-buttner · 2023-01-04T14:09:40Z

x-pack/plugins/cases/server/client/cases/bulk_get.ts

+      (caseInfo) => caseInfo.error === undefined
+    ) as [CaseSavedObject[], SOWithErrors];
+
+    const authorizedEntities = await authorization.getAuthorizedEntities({


hmm do you think we'll ever call getAuthorizedEntities without calling ensureAuthorized? I wonder if we should create a new method in the Authorization class that abstracts the calls to both and maybe returns the authorized and unauthorized cases like getAuthorizedAndUnauthorizedSavedObjects. That way it'd be harder to forget to call ensureAuthorized. We could also have this top level function handle throwing if the request resulted in no authorized entities and have getAuthorizedEntities simply return the information instead of needing to check if none of the owners were authorized.

Very good idea! I don't think we'll even call getAuthorizedEntities without calling ensureAuthorize. What about getAndEnsureAuthorizedEntities as a name for the new method?

Sounds good!

jonathan-buttner · 2023-01-04T14:21:28Z

x-pack/plugins/cases/server/client/typedoc_interfaces.ts

@@ -41,6 +42,7 @@ export interface ICaseResponse extends CaseResponse {}
 export interface ICaseResolveResponse extends CaseResolveResponse {}
 export interface ICasesResponse extends CasesResponse {}
 export interface ICasesFindResponse extends CasesFindResponse {}
+export interface ICasesBulkGetResponse extends CasesBulkGetResponse {}


Note to self we need to remove this file too 😬

jonathan-buttner · 2023-01-04T14:25:44Z

x-pack/plugins/cases/server/client/cases/bulk_get.ts

+    throwErrorIfCaseIdsReachTheLimit(request.ids);
+    throwErrorIfFieldsAreInvalid(fields);
+
+    const finalFields = fields?.length ? [...fields, 'id', 'version'] : fields;


hmm do we need to include id and version? I thought the fields parameter only governed fields we defined in our mapping.

I thought the same but if I do not add them they are not returned 🤷‍♀️.

Oh I think they aren't returned because of the pick at the end, I guess it doesn't hurt to have them here, might be a bit confusing though.

oh, you are right! I will fix it.

x-pack/plugins/cases/server/client/cases/bulk_get.ts

jonathan-buttner · 2023-01-04T14:45:24Z

x-pack/plugins/cases/server/authorization/authorization.test.ts

+    });
+
+    describe('hasAllRequested: true', () => {
+      it('returns correct entities', async () => {


Is this test showing that we filter out entities that don't have a valid registered owner? In this case owner c?

To my understanding, yes. If hasAllRequested === true then the valid owners are all owners returned by featuresStart.getKibanaFeatures and not the ones the user has access (the privlages returned by checkPrivileges is an empty array in this test). Then. we filter out. Basically, owner c is not a valid one because is not registered as a feature (security) by any plugin.

jonathan-buttner

Looking good, a few suggestions

jonathan-buttner · 2023-01-10T16:40:49Z

x-pack/plugins/cases/server/authorization/authorization.test.ts

    });

    describe('hasAllRequested: true', () => {
-      it('returns correct entities', async () => {
+      it('returns correct entities when the registered features (owners) are a and b', async () => {


nit: maybe change this to categorizes the registered owners a and b as authorized and the unregistered owner c as unauthorized. Something like that.

jonathan-buttner · 2023-01-10T16:47:35Z

x-pack/plugins/cases/server/client/cases/bulk_get.test.ts

+      await expect(bulkGet({ ids, fields: [field] }, clientArgs)).resolves.not.toThrow();
+    });
+
+    it('throws if the requested field is not valid', async () => {


nit: could we add a test that shows the dot notation is not supported?

jonathan-buttner · 2023-01-10T17:16:04Z

x-pack/plugins/cases/server/client/cases/bulk_get.ts

+
+  for (const theCase of unauthorizedCases) {
+    errors.push({
+      message: 'Unauthorized',


The object we return when a user is unauthorized for a retrieving an individual case looks like this:

{ "statusCode": 403, "error": "Forbidden", "message": "Unauthorized to access case with owners: \"observability\"" }

Should we add the nested error: 'Forbidden' field? I don't know if it adds much 🤷‍♂️ . It would probably be helpful make the message equivalent though. So have it be Unauthorized to access case with owner: <owner> (I don't know why we have it as plural in the single get case scenario 🤔

Sound good to me. What would you suggest for the text of the error attribute in the case of an error from the SO API (the loop before this loop)?

Better types

kibana-ci · 2023-01-10T21:45:35Z

💚 Build Succeeded

Buildkite Build
Commit: fa623f5

Metrics [docs]

Page load bundle

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

id	before	after	diff
`cases`	125.3KB	125.6KB	+248.0B

History

💔 Build #99294 failed c2abc71a3dc24341e4c8036e13f72cc7d2cb3e04
💔 Build #99249 failed 516447d
💔 Build #98878 failed b5bbea09ee45bb1ff2b2b82ea0ff6a9df40c9592
💚 Build #96613 succeeded 383eb89
💚 Build #95368 succeeded d9dd243

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

cc @cnasikas

## Summary This PR creates the bulk get cases internal API. The endpoint is needed for the alerts table to be able to get all cases the alerts are attached to with one call. Reference: elastic#146864 ### Request - ids: (Required, array) An array of IDs of the retrieved cases. - fields: (Optional, array) The fields to return in the attributes key of the object response. ``` POST <kibana host>:<port>/internal/cases/_bulk_get { "ids": ["case-id-1", "case-id-2", "123", "not-authorized"], "fields": ["title"] } ``` ### Response ``` { "cases": [ { "title": "case1", "owner": "securitySolution", "id": "case-id-1", "version": "WzIzMTU0NSwxNV0=" }, { "title": "case2", "owner": "observability", "id": "case-id-2", "version": "WzIzMTU0NSwxNV0=" } ], "errors": [ { "error": "Not Found", "message": "Saved object [cases/123] not found", "status": 404, "caseId": "123" }, { "error": "Forbidden", "message": "Unauthorized to access case with owner: \"cases\"", "status": 403, "caseId": "not-authorized" } ] } ``` ### Checklist Delete any items that are not applicable to this PR. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios ### For maintainers - [x] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

cnasikas added 4 commits December 15, 2022 12:59

Create bulk get cases internal route

7ce7e26

Decode with fields

7353010

Throw if field is invalid

f9a7eca

Add integration tests

b0c2a72

cnasikas added release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) Feature:Cases Cases feature v8.7.0 labels Dec 16, 2022

cnasikas requested a review from a team as a code owner December 16, 2022 09:12

cnasikas self-assigned this Dec 16, 2022

cnasikas mentioned this pull request Dec 16, 2022

[Cases] Link cases to alerts #146864

Closed

8 tasks

cnasikas added 2 commits December 16, 2022 13:38

Change response to contain errors

b96d291

Improve tests

d9dd243

thomheymann approved these changes Dec 16, 2022

View reviewed changes

js-jankisalvi reviewed Dec 22, 2022

View reviewed changes

adcoelho reviewed Dec 22, 2022

View reviewed changes

x-pack/plugins/cases/server/client/cases/client.ts Outdated Show resolved Hide resolved

adcoelho reviewed Dec 22, 2022

View reviewed changes

x-pack/test/cases_api_integration/security_and_spaces/tests/common/internal/bulk_get_cases.ts Show resolved Hide resolved

cnasikas added 2 commits December 22, 2022 16:26

Merge branch 'main' into bulk_get_cases

bf88a7e

PR feedback

383eb89

jonathan-buttner reviewed Jan 4, 2023

View reviewed changes

Merge branch 'main' into bulk_get_cases

fde4cf1

cnasikas force-pushed the bulk_get_cases branch from b5bbea0 to fcff3d0 Compare January 10, 2023 16:29

PR feedback

516447d

cnasikas force-pushed the bulk_get_cases branch from fcff3d0 to 516447d Compare January 10, 2023 16:31

jonathan-buttner reviewed Jan 10, 2023

View reviewed changes

cnasikas added 2 commits January 10, 2023 20:06

Better types

8d8454b

Better types

PR feedback

8e3a468

jonathan-buttner approved these changes Jan 10, 2023

View reviewed changes

Merge branch 'main' into bulk_get_cases

fa623f5

cnasikas force-pushed the bulk_get_cases branch from c2abc71 to fa623f5 Compare January 10, 2023 20:45

cnasikas merged commit a8902e1 into elastic:main Jan 11, 2023

cnasikas deleted the bulk_get_cases branch January 11, 2023 14:32

kibanamachine added the backport:skip This commit does not require backporting label Jan 11, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Cases] Create Bulk get cases internal API #147674

[Cases] Create Bulk get cases internal API #147674

cnasikas commented Dec 16, 2022 •

edited

Loading

elasticmachine commented Dec 16, 2022

elasticmachine commented Dec 16, 2022

thomheymann left a comment

jonathan-buttner left a comment

jonathan-buttner Jan 4, 2023

jonathan-buttner Jan 4, 2023

cnasikas Jan 9, 2023

jonathan-buttner Jan 9, 2023

jonathan-buttner Jan 4, 2023

jonathan-buttner Jan 4, 2023

jonathan-buttner Jan 4, 2023

jonathan-buttner Jan 4, 2023

cnasikas Jan 9, 2023 •

edited

Loading

jonathan-buttner Jan 9, 2023

jonathan-buttner Jan 4, 2023

jonathan-buttner Jan 4, 2023

cnasikas Jan 9, 2023

jonathan-buttner Jan 9, 2023

cnasikas Jan 9, 2023

jonathan-buttner Jan 4, 2023

cnasikas Jan 10, 2023 •

edited

Loading

jonathan-buttner left a comment

jonathan-buttner Jan 10, 2023

jonathan-buttner Jan 10, 2023

jonathan-buttner Jan 10, 2023

cnasikas Jan 10, 2023

kibana-ci commented Jan 10, 2023

		\| `success` \| User has accessed multiple case.
		\| `failure` \| User is not authorized to access multiple case.

[Cases] Create Bulk get cases internal API #147674

[Cases] Create Bulk get cases internal API #147674

Conversation

cnasikas commented Dec 16, 2022 • edited Loading

Summary

Request

Response

Checklist

For maintainers

elasticmachine commented Dec 16, 2022

elasticmachine commented Dec 16, 2022

thomheymann left a comment

Choose a reason for hiding this comment

jonathan-buttner left a 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 Jan 9, 2023 • 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 Jan 10, 2023 • edited Loading

Choose a reason for hiding this comment

jonathan-buttner left a 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

kibana-ci commented Jan 10, 2023

💚 Build Succeeded

Metrics [docs]

Page load bundle

History

cnasikas commented Dec 16, 2022 •

edited

Loading

cnasikas Jan 9, 2023 •

edited

Loading

cnasikas Jan 10, 2023 •

edited

Loading