[Security Solution] - Updating UI to work with new kibana privileges abstraction on alerts #108961

yctercero · 2021-08-17T18:05:17Z

Summary

++ @XavierM , @michaelolo24, @stephmilovic, @semd, @angorayc, @machadoum, @dhurley14 big thanks for helping push these changes through, fix tests, etc.

Holy moly.

What is happening in this PR? 🤷🏽‍♀️ Let's break it down:

Added a package @kbn/alerts - another one?! ...yes
- This is meant to add shared hooks and components around alerts as data
- useGetUserAlertsPermissions - accepts the Kibana capabilities object and returns whether the user has read and crud alerts privileges
- AlertsFeatureNoPermissions - component displayed when user does not have alerts privileges
UI changes for user with NO alerts privileges
- Alerts tab hidden in security solution side navigation
- Alerts tab hidden in rule details page
UI changes for user with alerts READ ONLY privileges
- alerts checkboxes hidden in alerts table
- alerts bulk actions hidden in alerts table
per discussions with @kobelb, @XavierM, @marshallmain, @dhurley14 , @peluja1012 moving forward with changes needed to detections read/create index routes to allow users with kibana privileges to move forward. Investigation is ongoing as to ways to apply any existing document level .siem-signals preferences on top of kibana privileges.
- 🚨 breaking change 🚨

TO DO

remove alerts tab from MAIN navigation when user has no alerts privileges
remove alerts link from Kibana search when user has no alerts privileges
update privileges warning banner to remove references to needing .siem-signals ES privileges
want to follow up with more robust tests

Notes

There is now some repetitive logic/code in the UI, where we're maintaining user privilege info in hooks, context, and redux. This will require cleanup, but would rather do once we have more tests in.
While coding, found a good chunk of dead code paths around the old alerts table utility bar

Screenshots

Alerts privileges NONE

Alerts privileges READ ONLY

Checklist

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

For maintainers

This was checked for breaking API changes and was labeled appropriately

yctercero · 2021-08-18T00:02:32Z

packages/kbn-rule-data-utils/src/alerts_as_data_rbac.ts

@@ -31,7 +31,7 @@ export const mapConsumerToIndexName: Record<AlertConsumers, string | string[]> =
  logs: '.alerts-observability.logs',
  infrastructure: '.alerts-observability.metrics',
  observability: '.alerts-observability',
-  siem: ['.alerts-security.alerts', '.siem-signals'],
+  siem: '.alerts-security.alerts',


.siem-signals is dynamically named and also aliased to .alerts-security.alerts-space so no need to query both.

XavierM · 2021-08-18T01:59:57Z

packages/kbn-alerts/src/hooks/use_get_alerts_permissions/index.ts

+
+export const useGetUserAlertsPermissions = (
+  uiCapabilities: RecursiveReadonly<Record<string, any>>,
+  featureId: string


I think this logic should be different to allow o11y to be able to use it. we need to remember that they can have multiple alerts from different feature IDs like logs, APM, Metrics etc. Make sense to us but not sure if it will be good for them.

x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts

x-pack/plugins/security_solution/public/app/deep_links/index.ts

x-pack/plugins/security_solution/public/detections/components/user_info/index.tsx

x-pack/plugins/security_solution/public/detections/pages/alerts/index.tsx

XavierM · 2021-08-18T02:25:00Z

x-pack/plugins/security_solution/public/plugin.tsx

+            query: { features: SERVER_APP_ID },
+          }
+        );
+        signal = { name: indexName[0] };


nit - let's add a comment for FUTURE DEVELOPER, that the expectation here is to only have one index for the alert index

Def! Will add.

I didn't do...BUT will add if CI fails or as follow up.

x-pack/plugins/timelines/server/search_strategy/index_fields/index.ts

XavierM

review it with 👓 🔎 👓, Tested locally, code looks good and it is working as expected.

Good job!!! 👍 👏 🍾

elasticmachine · 2021-08-18T02:39:11Z

Pinging @elastic/security-solution (Team: SecuritySolution)

…raff

yctercero · 2021-08-18T16:15:01Z

@elasticmachine merge upstream

stephmilovic · 2021-08-18T16:28:10Z

XavierM · 2021-08-18T18:33:30Z

we need tags 7.15 and 7.16 for the auto-backport to work

kibanamachine · 2021-08-18T18:48:28Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`securitySolution`	2362	2371	+9

Async chunks

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

id	before	after	diff
`apm`	4.4MB	4.4MB	-18.0B
`observability`	490.5KB	490.5KB	-18.0B
`securitySolution`	6.5MB	6.5MB	+8.0KB
`timelines`	437.5KB	437.6KB	+93.0B
total			+8.0KB

Page load bundle

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

id	before	after	diff
`infra`	149.7KB	149.6KB	-18.0B
`securitySolution`	211.7KB	212.1KB	+450.0B
`uptime`	36.6KB	36.6KB	-18.0B
total			+414.0B

History

💔 Build #146682 failed e510c41
💔 Build #146673 failed 43aa0eb
💔 Build #146665 failed b0bf078
💚 Build #146644 succeeded 85e15c6
💔 Build #146589 failed e4927fe

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

cc @yctercero

…abstraction on alerts (elastic#108961) ## Summary Holy moly. What is happening in this PR? 🤷🏽‍♀️ Let's break it down: - Added a package `@kbn/alerts` - another one?! ...yes - This is meant to add shared hooks and components around alerts as data - `useGetUserAlertsPermissions` - accepts the Kibana capabilities object and returns whether the user has `read` and `crud` alerts privileges - `AlertsFeatureNoPermissions` - component displayed when user does not have alerts privileges - UI changes for user with NO alerts privileges - `Alerts` tab hidden in security solution side navigation - `Alerts` tab hidden in rule details page - UI changes for user with alerts READ ONLY privileges - alerts checkboxes hidden in alerts table - alerts bulk actions hidden in alerts table

kibanamachine · 2021-08-18T18:52:40Z

💚 Backport successful

Status	Branch	Result
✅	7.15
✅	7.x

The backport PRs will be merged automatically after passing CI.

…abstraction on alerts (#108961) (#109156) ## Summary Holy moly. What is happening in this PR? 🤷🏽‍♀️ Let's break it down: - Added a package `@kbn/alerts` - another one?! ...yes - This is meant to add shared hooks and components around alerts as data - `useGetUserAlertsPermissions` - accepts the Kibana capabilities object and returns whether the user has `read` and `crud` alerts privileges - `AlertsFeatureNoPermissions` - component displayed when user does not have alerts privileges - UI changes for user with NO alerts privileges - `Alerts` tab hidden in security solution side navigation - `Alerts` tab hidden in rule details page - UI changes for user with alerts READ ONLY privileges - alerts checkboxes hidden in alerts table - alerts bulk actions hidden in alerts table Co-authored-by: Yara Tercero <yctercero@users.noreply.github.com>

…abstraction on alerts (#108961) (#109155) ## Summary Holy moly. What is happening in this PR? 🤷🏽‍♀️ Let's break it down: - Added a package `@kbn/alerts` - another one?! ...yes - This is meant to add shared hooks and components around alerts as data - `useGetUserAlertsPermissions` - accepts the Kibana capabilities object and returns whether the user has `read` and `crud` alerts privileges - `AlertsFeatureNoPermissions` - component displayed when user does not have alerts privileges - UI changes for user with NO alerts privileges - `Alerts` tab hidden in security solution side navigation - `Alerts` tab hidden in rule details page - UI changes for user with alerts READ ONLY privileges - alerts checkboxes hidden in alerts table - alerts bulk actions hidden in alerts table Co-authored-by: Yara Tercero <yctercero@users.noreply.github.com>

yctercero added 4 commits August 16, 2021 23:52

updating to return alerts as data alias to security solution redux

90a2509

add alerts privileges hook in package

baa1222

not yet working, pushing up to share

cfdde2a

update index routes to use internal and add ui for alerts none and read

8fe33d0

yctercero commented Aug 18, 2021

View reviewed changes

yctercero added 3 commits August 17, 2021 17:07

cleanup

aaa8c90

Merge branch 'master' of github.com:elastic/kibana into alerts_privs_ui

339abbd

thanks X for helping with read only tgrid changes

ac60f60

yctercero marked this pull request as ready for review August 18, 2021 01:49

yctercero requested review from a team as code owners August 18, 2021 01:49