[Cases] Integrating file service and registering file kinds #152031

jonathan-buttner · 2023-02-23T19:46:05Z

This PR registers three file kinds for cases. One for each instance of cases (stack, observability, and security). Each solution needs separate http tags for the routes that are generated by the file service to implement RBAC.

I refactored the logic to remove some duplication across the three plugins since we're essentially registering the same http tags with slightly different names.

This PR shouldn't affect any of the current functionality.

Notable changes:

I split up the constants.ts file, really the only change is adding the file kinds logic to generate the http tags the rest is copy/paste
Refactored the logic to generate the api http tags for each plugin
Registered the three file kinds

Issues: #151780 #151933

…fix'

x-pack/plugins/cases/common/constants/mime_types.ts

x-pack/test/cases_api_integration/security_and_spaces/tests/common/files/apis.ts

x-pack/plugins/cases/common/constants/files.ts

adcoelho · 2023-02-24T11:18:18Z

x-pack/test/cases_api_integration/security_and_spaces/tests/common/files/apis.ts

+      for (const plugin of OWNERS) {
+        it(`should create and delete a file for plugin: ${plugin}`, async () => {
+          const createResult = await supertest
+            .post(FILES_API_ROUTES.fileKind.getCreateFileRoute(plugin))


I am trying to understand all this so please bear with me 😄

I get that our intent is

cases will register a unique file kind of each owner type.

and

On the UI when the user interacts with the files page to request all the files or view them, cases will deduce which plugin we are operating within and use that particular file kind for finding/opening the files.

Above, in x-pack/plugins/cases/common/constants/files.ts the HTTP tags were built but in these tests, all we are doing is calling FILES_API_ROUTES.fileKind.get*FileRoute(plugin)
which is something that is already provided by the files team(?) and already takes into account the plugin.

Is it precisely because you did registerCaseFileKinds that this now works? Otherwise, it wouldn't recognize the plugins?

And here the kind passed is only the plugin but in x-pack/plugins/cases/common/constants/files.ts it seems way more complicated. ${owner}FilesCases${operation}.

How does the API match plugin => ${owner}FilesCases...(left out the operation because I assume it comes from get*FileRoute)?

Haha no worries

Is it precisely because you did registerCaseFileKinds that this now works? Otherwise, it wouldn't recognize the plugins?

That's correct, he have to call their filesSetupPlugin.registerFileKind function to register the 3 owners otherwise we'd get an error if we tried to access routes for that kind because the routes wouldn't be created. I pass this function to a utility because we need to do it on the frontend and backend.

And here the kind passed is only the plugin but in x-pack/plugins/cases/common/constants/files.ts it seems way more complicated. ${owner}FilesCases${operation}.

Yeah so the http tags get used in two places. When registering the file kind to dictate which tags are need to use those routes (aka securitySolutionFilesCasesRead for the list, getById, download etc). Then we specify those http tags in this file: https://github.com/elastic/kibana/pull/152031/files#diff-ae9053bde29ec5baa7141b8c10a47c997d4c87319993f3c5a206ec9239956640R55

So when a user is granted the cases read feature in one of their roles they can then access any files API that also has that particular tag securitySolutionCasesFilesRead. The reason we want separate operations is because we split out the delete capability so a user who does not have delete access shouldn't be able to delete cases/comments or their files.

…nto cases-files-be

…fix'

…nto cases-files-be

jonathan-buttner · 2023-02-27T12:35:09Z

x-pack/plugins/cases/common/constants/index.ts

-export const CASE_VIEW_ALERT_TABLE_PATH =
-  `${CASE_VIEW_PATH}/?tabId=${CASE_VIEW_PAGE_TABS.ALERTS}` as const;
-export const CASE_VIEW_TAB_PATH = `${CASE_VIEW_PATH}/?tabId=:tabId` as const;
+export * from './owners';


Split this file up to move the file service constants to their own place and moved various other constants to avoid circular references.

jonathan-buttner · 2023-02-27T12:43:14Z

x-pack/test/cases_api_integration/common/lib/authentication/index.ts

-    await spacesService.create(space);
-  }
+
+  await Promise.all(spaces.map((space) => spacesService.create(space)));


Made some optimizations to speed up the creation of spaces, roles, and users for the tests

x-pack/plugins/cases/common/constants/mime_types.ts

elasticmachine · 2023-02-27T12:55:54Z

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

elasticmachine · 2023-02-27T12:55:56Z

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

x-pack/plugins/cases/server/files/mime_types.ts

cnasikas

Nice work! I did a first pass and left some comments.

x-pack/plugins/cases/common/constants/files.ts

x-pack/plugins/cases/common/constants/types.ts

x-pack/plugins/cases/common/utils/api_tags.ts

cnasikas · 2023-02-28T12:53:08Z

x-pack/plugins/cases/kibana.jsonc

@@ -25,7 +25,8 @@
      "management",
      "security",
      "notifications",
-      "ruleRegistry"
+      "ruleRegistry",
+      "files",


Do we need to same for o11y and the security solution? I think we would need it only if we need the file service from useKibana. We can do it on the UI PR. Just a reminder. @adcoelho

hmm I wonder if the build system figures that out because the observability plugin doesn't have "actions" specified in either of their required or optional plugins yet they can still use connectors within cases right?

If I remember correctly "actions" do not export any UI client. We fetch the connectors from our own API. Think about user profiles. We had to put the security everywhere so we use the security UI client. Basically, if we need a service through the useKibana hook it should be part of the kibana.jsonc file and passed in the KibanaContextProvider. Ideally, we should pass the case dependencies in our CaseContextProvider so consumers should not have to pass the dependencies themselves.

Ah I see. I can go ahead and add it then 👍

Security solution already has it as required to it's just observability that needs it.

x-pack/plugins/cases/public/files/index.ts

x-pack/plugins/cases/server/files/index.ts

x-pack/plugins/cases/server/files/mime_types.ts

x-pack/test/api_integration/apis/cases/files.ts

…nto cases-files-be

semd

Security Explore changes LGTM

js-jankisalvi · 2023-03-01T09:55:14Z

x-pack/plugins/cases/common/constants/mime_types.ts

+  'image/vnd.zbrush.pcx',
+  'image/webp',
+  'image/wmf',
+];


Wow, extensive!!

CoenWarmer

LGTM!

js-jankisalvi

Looks good. Great job adding api_integration tests 👍

x-pack/plugins/cases/server/files/index.ts

x-pack/test/api_integration/apis/cases/files.ts

cnasikas

Code LGTM 🚀 ! Is there any way I can test it manually?

kibana-ci · 2023-03-01T22:51:33Z

💚 Build Succeeded

Buildkite Build
Commit: 506d718

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`cases`	557	564	+7

Adoption-tracked APIs that are not used anywhere

APIs whose adoption is being tracked and are still not being used. This number should tend to zero.

id	before	after	diff
`files`	2	1	-1

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`cases`	75	77	+2

Async chunks

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

id	before	after	diff
`cases`	364.0KB	364.0KB	+6.0B

Page load bundle

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

id	before	after	diff
`cases`	130.9KB	133.2KB	+2.3KB

Unknown metric groups

API count

id	before	after	diff
`cases`	92	94	+2

ESLint disabled line counts

id	before	after	diff
`securitySolution`	428	430	+2

Total ESLint disabled count

id	before	after	diff
`securitySolution`	506	508	+2

History

💔 Build #111512 failed e8d2639
💚 Build #111223 succeeded d4dcc78
💔 Build #111135 failed fb62610
💚 Build #110865 succeeded b9cb6a5
💔 Build #110832 failed 71e8482
💔 Build #110727 failed c4e09a5

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

jonathan-buttner · 2023-03-02T12:34:58Z

Code LGTM 🚀 ! Is there any way I can test it manually?

I think we'd need to hit the file service APIs to test it. I can get you those APIs in a bit

…152031) This PR registers three file kinds for cases. One for each instance of cases (stack, observability, and security). Each solution needs separate http tags for the routes that are generated by the file service to implement RBAC. I refactored the logic to remove some duplication across the three plugins since we're essentially registering the same http tags with slightly different names. This PR shouldn't affect any of the current functionality. Notable changes: - I split up the constants.ts file, really the only change is adding the file kinds logic to generate the http tags the rest is copy/paste - Refactored the logic to generate the `api` http tags for each plugin - Registered the three file kinds Issues: elastic#151780 elastic#151933 --------- Co-authored-by: kibanamachine <[email protected]>

Adding file service into cases

81b27ad

jonathan-buttner added release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) Feature:Cases Cases feature v8.8.0 labels Feb 23, 2023

jonathan-buttner and others added 3 commits February 23, 2023 14:47

Merge branch 'main' of github.com:elastic/kibana into cases-files-be

8cabedd

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

b2f75f3

…fix'

Typing owner info

a6c62bf

jonathan-buttner mentioned this pull request Feb 23, 2023

[Cases] Implement files backend changes #151933

Closed

13 tasks

jonathan-buttner and others added 3 commits February 23, 2023 15:59

Adding more mime types

869474a

Adding initial tests

770253e

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

03e450b

…fix'

adcoelho reviewed Feb 24, 2023

View reviewed changes

x-pack/plugins/cases/common/constants/mime_types.ts Outdated Show resolved Hide resolved

adcoelho reviewed Feb 24, 2023

View reviewed changes

x-pack/test/cases_api_integration/security_and_spaces/tests/common/files/apis.ts Outdated Show resolved Hide resolved

adcoelho reviewed Feb 24, 2023

View reviewed changes

x-pack/plugins/cases/common/constants/files.ts Outdated Show resolved Hide resolved

adcoelho reviewed Feb 24, 2023

View reviewed changes

jonathan-buttner and others added 6 commits February 24, 2023 17:10

Moving to integration directory and adding more

3314aa9

Merge branch 'cases-files-be' of github.com:jonathan-buttner/kibana i…

75e71d5

…nto cases-files-be

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

50f806a

…fix'

Adding delete test

73d55bc

Merge branch 'cases-files-be' of github.com:jonathan-buttner/kibana i…

4e6b026

…nto cases-files-be

Merge branch 'main' of github.com:elastic/kibana into cases-files-be

a8e0344

jonathan-buttner commented Feb 27, 2023

View reviewed changes

jonathan-buttner marked this pull request as ready for review February 27, 2023 12:55

jonathan-buttner requested review from a team as code owners February 27, 2023 12:55

jonathan-buttner added 4 commits February 27, 2023 11:18

Merge branch 'main' into cases-files-be

c4e09a5

Merge branch 'main' into cases-files-be

89c50f7

Merge branch 'main' into cases-files-be

71e8482

Merge branch 'main' into cases-files-be

b9cb6a5

adcoelho reviewed Feb 28, 2023

View reviewed changes

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

cnasikas reviewed Feb 28, 2023

View reviewed changes

jonathan-buttner and others added 5 commits February 28, 2023 11:51

Addressing feedback

218b2ac

Merge branch 'cases-files-be' of github.com:jonathan-buttner/kibana i…

69aab41

…nto cases-files-be

Making files plugin required for observability

fb62610

Merge branch 'main' into cases-files-be

9828325

Merge branch 'main' into cases-files-be

d4dcc78

semd approved these changes Mar 1, 2023

View reviewed changes

js-jankisalvi reviewed Mar 1, 2023

View reviewed changes

CoenWarmer approved these changes Mar 1, 2023

View reviewed changes

js-jankisalvi approved these changes Mar 1, 2023

View reviewed changes

cnasikas reviewed Mar 1, 2023

View reviewed changes

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

cnasikas reviewed Mar 1, 2023

View reviewed changes

x-pack/plugins/cases/server/files/index.ts Show resolved Hide resolved

cnasikas reviewed Mar 1, 2023

View reviewed changes

x-pack/test/api_integration/apis/cases/files.ts Show resolved Hide resolved

cnasikas approved these changes Mar 1, 2023

View reviewed changes

jonathan-buttner added 3 commits March 1, 2023 15:25

Merge branch 'main' of github.com:elastic/kibana into cases-files-be

5bd1018

Addressing feedback

e8d2639

Fixing tests

506d718

jonathan-buttner merged commit 7d81cf2 into elastic:main Mar 2, 2023

jonathan-buttner deleted the cases-files-be branch March 2, 2023 12:35

cnasikas mentioned this pull request Apr 3, 2023

[Cases][Meta]: File attachments #151595

Closed

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Cases] Integrating file service and registering file kinds #152031

[Cases] Integrating file service and registering file kinds #152031

jonathan-buttner commented Feb 23, 2023 •

edited

Loading

adcoelho Feb 24, 2023

jonathan-buttner Feb 24, 2023

jonathan-buttner Feb 27, 2023

jonathan-buttner Feb 27, 2023

elasticmachine commented Feb 27, 2023

elasticmachine commented Feb 27, 2023

cnasikas left a comment

cnasikas Feb 28, 2023

jonathan-buttner Feb 28, 2023

cnasikas Feb 28, 2023

jonathan-buttner Feb 28, 2023

jonathan-buttner Feb 28, 2023

semd left a comment

js-jankisalvi Mar 1, 2023

CoenWarmer left a comment

js-jankisalvi left a comment

cnasikas left a comment

kibana-ci commented Mar 1, 2023

API count

ESLint disabled line counts

Total ESLint disabled count

jonathan-buttner commented Mar 2, 2023

[Cases] Integrating file service and registering file kinds #152031

[Cases] Integrating file service and registering file kinds #152031

Conversation

jonathan-buttner commented Feb 23, 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

elasticmachine commented Feb 27, 2023

elasticmachine commented Feb 27, 2023

cnasikas 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

semd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CoenWarmer left a comment

Choose a reason for hiding this comment

js-jankisalvi left a comment

Choose a reason for hiding this comment

cnasikas left a comment

Choose a reason for hiding this comment

kibana-ci commented Mar 1, 2023

💚 Build Succeeded

Metrics [docs]

Module Count

Adoption-tracked APIs that are not used anywhere

Public APIs missing comments

Async chunks

Page load bundle

API count

ESLint disabled line counts

Total ESLint disabled count

History

jonathan-buttner commented Mar 2, 2023

jonathan-buttner commented Feb 23, 2023 •

edited

Loading