Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ResponseOps] Alerts list is empty for Custom threshold rules created from Stack management #179082

Closed
umbopepato opened this issue Mar 20, 2024 · 3 comments · Fixed by #194615
Closed
Assignees
Labels
bug Fixes for quality problems that affect the customer experience Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams)

Comments

@umbopepato
Copy link
Member

Steps to reproduce

  1. Create a Custom threshold rule that fires alerts from Stack management > Rules
  2. Open the newly created rule detail page
  3. The alerts table appears empty, even when the History tab shows a positive number of active alerts
image image

Observations

The alerts table in the rule page looks for the alerts in the .alerts-stack.alerts-default index, but they are stored in .alerts-observability.threshold.alerts-default instead:

image
@umbopepato umbopepato added bug Fixes for quality problems that affect the customer experience Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) labels Mar 20, 2024
@elasticmachine
Copy link
Contributor

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

@ersin-erdal
Copy link
Contributor

We recently onboard this rule type with Framework alert-as-data (after this issue was created): #179284

Testing again could be helpful as we started to use the alerting framework to persist alerts.

@umbopepato
Copy link
Member Author

Thanks for the heads up @ersin-erdal! I'll check again but I'm pretty sure the issue is the Custom threshold rule type being registered with a wrong producer feature id (observability).

In the rule details page we extract the feature id to be sent to the alerts table from the rule.consumer/ruleType.producer couple. If the former is alerts we take the latter. For the Log threshold rule for example:

rule.consumer -> 'alerts'
ruleType.producer -> 'logs'

We select logs and the alerts show up because they're queried from the correct index.

In the Custom threshold rule instead:

rule.consumer -> 'stackAlerts'
ruleType.producer -> 'observability'

If we keep stackAlerts we don't see any alert because they actually end up in the logs indexes. If we fall back to observability the search strategy fails because observability cannot be used for querying alerts. It seems like the ruleType.producer should be logs in this case.

@Zacqary Zacqary moved this from Awaiting Triage to Todo in AppEx: ResponseOps - Rules & Alerts Management Apr 2, 2024
@cnasikas cnasikas assigned cnasikas and unassigned XavierM May 15, 2024
kibanamachine pushed a commit to kibanamachine/kibana that referenced this issue Oct 6, 2024
…ld rule types under stack alerts feature privilege (elastic#194615)

## Summary

In the ES query, anomaly detection, and custom threshold rule types
users can use the "Role visibility" dropdown to select where the rules
should be accessible. The "Role visibility" dropdown sets the `consumer`
which is paramount for alerting RBAC. For the anomaly detection and
custom threshold rule types if the `consumer` is set to `stackAlerts`
then the rules will not be accessible from any rule page even if the
user has access to the "Stack alerts" feature privilege. This PR fixes
this bug.

Fixes elastic#193549
Fixes elastic#191075
Fixes elastic#184422
Fixes elastic#179082

## Testing

1. Create an anomaly detection and custom threshold rule and set the
"Role visibility" to "Stack alerts".
2. Create a user with access only to "Stack alerts".
3. Login with the user created in Step 2.
4. Verify that you can see the rules from the stack management page.
5. Verify that you can see the alerts generated from the rules.
6. Create a user with roles `kibana_admin` and verify the same.

### Checklist

- [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
- [x] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed

### 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)

## Release notes

Fix bug where rule types with "Stack alerts" role visibility are not
being shown in the stack management page

(cherry picked from commit 8d83a07)
cnasikas added a commit to cnasikas/kibana that referenced this issue Oct 6, 2024
…ld rule types under stack alerts feature privilege (elastic#194615)

## Summary

In the ES query, anomaly detection, and custom threshold rule types
users can use the "Role visibility" dropdown to select where the rules
should be accessible. The "Role visibility" dropdown sets the `consumer`
which is paramount for alerting RBAC. For the anomaly detection and
custom threshold rule types if the `consumer` is set to `stackAlerts`
then the rules will not be accessible from any rule page even if the
user has access to the "Stack alerts" feature privilege. This PR fixes
this bug.

Fixes elastic#193549
Fixes elastic#191075
Fixes elastic#184422
Fixes elastic#179082

## Testing

1. Create an anomaly detection and custom threshold rule and set the
"Role visibility" to "Stack alerts".
2. Create a user with access only to "Stack alerts".
3. Login with the user created in Step 2.
4. Verify that you can see the rules from the stack management page.
5. Verify that you can see the alerts generated from the rules.
6. Create a user with roles `kibana_admin` and verify the same.

### Checklist

- [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
- [x] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed

### 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)

## Release notes

Fix bug where rule types with "Stack alerts" role visibility are not
being shown in the stack management page

(cherry picked from commit 8d83a07)

# Conflicts:
#	x-pack/test/alerting_api_integration/security_and_spaces/group1/tests/alerting/find.ts
cnasikas added a commit that referenced this issue Oct 7, 2024
…threshold rule types under stack alerts feature privilege (#194615) (#195173)

# Backport

This will backport the following commits from `main` to `8.15`:
- [[ResponseOps][Alerting] Register anomaly detection and custom
threshold rule types under stack alerts feature privilege
(#194615)](#194615)

<!--- Backport version: 8.9.8 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Christos
Nasikas","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-06T11:46:44Z","message":"[ResponseOps][Alerting]
Register anomaly detection and custom threshold rule types under stack
alerts feature privilege (#194615)\n\n## Summary\r\n\r\nIn the ES query,
anomaly detection, and custom threshold rule types\r\nusers can use the
\"Role visibility\" dropdown to select where the rules\r\nshould be
accessible. The \"Role visibility\" dropdown sets the
`consumer`\r\nwhich is paramount for alerting RBAC. For the anomaly
detection and\r\ncustom threshold rule types if the `consumer` is set to
`stackAlerts`\r\nthen the rules will not be accessible from any rule
page even if the\r\nuser has access to the \"Stack alerts\" feature
privilege. This PR fixes\r\nthis bug.\r\n\r\nFixes
https://github.com/elastic/kibana/issues/193549\r\nFixes
https://github.com/elastic/kibana/issues/191075\r\nFixes
https://github.com/elastic/kibana/issues/184422\r\nFixes
https://github.com/elastic/kibana/issues/179082\r\n\r\n##
Testing\r\n\r\n1. Create an anomaly detection and custom threshold rule
and set the\r\n\"Role visibility\" to \"Stack alerts\".\r\n2. Create a
user with access only to \"Stack alerts\".\r\n3. Login with the user
created in Step 2.\r\n4. Verify that you can see the rules from the
stack management page.\r\n5. Verify that you can see the alerts
generated from the rules.\r\n6. Create a user with roles `kibana_admin`
and verify the same.\r\n\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [x] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n\r\n### For maintainers\r\n\r\n- [x]
This was checked for breaking API changes and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n##
Release notes\r\n\r\nFix bug where rule types with \"Stack alerts\" role
visibility are not\r\nbeing shown in the stack management
page","sha":"8d83a075f6228bb5a6e35a9bb4654fe29cee0cff","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:fix","Team:ResponseOps","v9.0.0","Feature:Alerting/RulesFramework","backport:prev-major","v8.16.0","v8.15.3"],"number":194615,"url":"https://github.com/elastic/kibana/pull/194615","mergeCommit":{"message":"[ResponseOps][Alerting]
Register anomaly detection and custom threshold rule types under stack
alerts feature privilege (#194615)\n\n## Summary\r\n\r\nIn the ES query,
anomaly detection, and custom threshold rule types\r\nusers can use the
\"Role visibility\" dropdown to select where the rules\r\nshould be
accessible. The \"Role visibility\" dropdown sets the
`consumer`\r\nwhich is paramount for alerting RBAC. For the anomaly
detection and\r\ncustom threshold rule types if the `consumer` is set to
`stackAlerts`\r\nthen the rules will not be accessible from any rule
page even if the\r\nuser has access to the \"Stack alerts\" feature
privilege. This PR fixes\r\nthis bug.\r\n\r\nFixes
https://github.com/elastic/kibana/issues/193549\r\nFixes
https://github.com/elastic/kibana/issues/191075\r\nFixes
https://github.com/elastic/kibana/issues/184422\r\nFixes
https://github.com/elastic/kibana/issues/179082\r\n\r\n##
Testing\r\n\r\n1. Create an anomaly detection and custom threshold rule
and set the\r\n\"Role visibility\" to \"Stack alerts\".\r\n2. Create a
user with access only to \"Stack alerts\".\r\n3. Login with the user
created in Step 2.\r\n4. Verify that you can see the rules from the
stack management page.\r\n5. Verify that you can see the alerts
generated from the rules.\r\n6. Create a user with roles `kibana_admin`
and verify the same.\r\n\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [x] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n\r\n### For maintainers\r\n\r\n- [x]
This was checked for breaking API changes and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n##
Release notes\r\n\r\nFix bug where rule types with \"Stack alerts\" role
visibility are not\r\nbeing shown in the stack management
page","sha":"8d83a075f6228bb5a6e35a9bb4654fe29cee0cff"}},"sourceBranch":"main","suggestedTargetBranches":["8.15"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/194615","number":194615,"mergeCommit":{"message":"[ResponseOps][Alerting]
Register anomaly detection and custom threshold rule types under stack
alerts feature privilege (#194615)\n\n## Summary\r\n\r\nIn the ES query,
anomaly detection, and custom threshold rule types\r\nusers can use the
\"Role visibility\" dropdown to select where the rules\r\nshould be
accessible. The \"Role visibility\" dropdown sets the
`consumer`\r\nwhich is paramount for alerting RBAC. For the anomaly
detection and\r\ncustom threshold rule types if the `consumer` is set to
`stackAlerts`\r\nthen the rules will not be accessible from any rule
page even if the\r\nuser has access to the \"Stack alerts\" feature
privilege. This PR fixes\r\nthis bug.\r\n\r\nFixes
https://github.com/elastic/kibana/issues/193549\r\nFixes
https://github.com/elastic/kibana/issues/191075\r\nFixes
https://github.com/elastic/kibana/issues/184422\r\nFixes
https://github.com/elastic/kibana/issues/179082\r\n\r\n##
Testing\r\n\r\n1. Create an anomaly detection and custom threshold rule
and set the\r\n\"Role visibility\" to \"Stack alerts\".\r\n2. Create a
user with access only to \"Stack alerts\".\r\n3. Login with the user
created in Step 2.\r\n4. Verify that you can see the rules from the
stack management page.\r\n5. Verify that you can see the alerts
generated from the rules.\r\n6. Create a user with roles `kibana_admin`
and verify the same.\r\n\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [x] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n\r\n### For maintainers\r\n\r\n- [x]
This was checked for breaking API changes and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n##
Release notes\r\n\r\nFix bug where rule types with \"Stack alerts\" role
visibility are not\r\nbeing shown in the stack management
page","sha":"8d83a075f6228bb5a6e35a9bb4654fe29cee0cff"}},{"branch":"8.x","label":"v8.16.0","labelRegex":"^v8.16.0$","isSourceBranch":false,"url":"https://github.com/elastic/kibana/pull/195171","number":195171,"state":"MERGED","mergeCommit":{"sha":"a89a5388b4685304d2a1ec3e3d2ed8e75d833841","message":"[8.x]
[ResponseOps][Alerting] Register anomaly detection and custom threshold
rule types under stack alerts feature privilege (#194615) (#195171)\n\n#
Backport\n\nThis will backport the following commits from `main` to
`8.x`:\n- [[ResponseOps][Alerting] Register anomaly detection and
custom\nthreshold rule types under stack alerts feature
privilege\n(#194615)](https://github.com/elastic/kibana/pull/194615)\n\n<!---
Backport version: 9.4.3 -->\n\n### Questions ?\nPlease refer to the
[Backport
tool\ndocumentation](https://github.com/sqren/backport)\n\n<!--BACKPORT
[{\"author\":{\"name\":\"Christos\nNasikas\",\"email\":\"[email protected]\"},\"sourceCommit\":{\"committedDate\":\"2024-10-06T11:46:44Z\",\"message\":\"[ResponseOps][Alerting]\nRegister
anomaly detection and custom threshold rule types under stack\nalerts
feature privilege (#194615)\\n\\n## Summary\\r\\n\\r\\nIn the ES
query,\nanomaly detection, and custom threshold rule types\\r\\nusers
can use the\n\\\"Role visibility\\\" dropdown to select where the
rules\\r\\nshould be\naccessible. The \\\"Role visibility\\\" dropdown
sets the\n`consumer`\\r\\nwhich is paramount for alerting RBAC. For the
anomaly\ndetection and\\r\\ncustom threshold rule types if the
`consumer` is set to\n`stackAlerts`\\r\\nthen the rules will not be
accessible from any rule\npage even if the\\r\\nuser has access to the
\\\"Stack alerts\\\" feature\nprivilege. This PR fixes\\r\\nthis
bug.\\r\\n\\r\\nFixes\nhttps://github.com//issues/193549\\r\\nFixes\nhttps://github.com//issues/191075\\r\\nFixes\nhttps://github.com//issues/184422\\r\\nFixes\nhttps://github.com//issues/179082\\r\\n\\r\\n##\nTesting\\r\\n\\r\\n1.
Create an anomaly detection and custom threshold rule\nand set
the\\r\\n\\\"Role visibility\\\" to \\\"Stack alerts\\\".\\r\\n2. Create
a\nuser with access only to \\\"Stack alerts\\\".\\r\\n3. Login with the
user\ncreated in Step 2.\\r\\n4. Verify that you can see the rules from
the\nstack management page.\\r\\n5. Verify that you can see the
alerts\ngenerated from the rules.\\r\\n6. Create a user with roles
`kibana_admin`\nand verify the same.\\r\\n\\r\\n\\r\\n###
Checklist\\r\\n\\r\\n- [x] [Unit
or\nfunctional\\r\\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\\r\\nwere\nupdated
or added to match the most common scenarios\\r\\n- [x]
[Flaky\nTest\\r\\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)\nwas\\r\\nused
on any tests changed\\r\\n\\r\\n### For maintainers\\r\\n\\r\\n-
[x]\nThis was checked for breaking API changes and
was\n[labeled\\r\\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\\r\\n\\r\\n##\nRelease
notes\\r\\n\\r\\nFix bug where rule types with \\\"Stack alerts\\\"
role\nvisibility are not\\r\\nbeing shown in the stack
management\npage\",\"sha\":\"8d83a075f6228bb5a6e35a9bb4654fe29cee0cff\",\"branchLabelMapping\":{\"^v9.0.0$\":\"main\",\"^v8.16.0$\":\"8.x\",\"^v(\\\\d+).(\\\\d+).\\\\d+$\":\"$1.$2\"}},\"sourcePullRequest\":{\"labels\":[\"bug\",\"release_note:fix\",\"Team:ResponseOps\",\"v9.0.0\",\"Feature:Alerting/RulesFramework\",\"backport:prev-major\",\"v8.16.0\",\"v8.15.3\"],\"title\":\"[ResponseOps][Alerting]\nRegister
anomaly detection and custom threshold rule types under stack\nalerts
feature\nprivilege\",\"number\":194615,\"url\":\"https://github.com/elastic/kibana/pull/194615\",\"mergeCommit\":{\"message\":\"[ResponseOps][Alerting]\nRegister
anomaly detection and custom threshold rule types under stack\nalerts
feature privilege (#194615)\\n\\n## Summary\\r\\n\\r\\nIn the ES
query,\nanomaly detection, and custom threshold rule types\\r\\nusers
can use the\n\\\"Role visibility\\\" dropdown to select where the
rules\\r\\nshould be\naccessible. The \\\"Role visibility\\\" dropdown
sets the\n`consumer`\\r\\nwhich is paramount for alerting RBAC. For the
anomaly\ndetection and\\r\\ncustom threshold rule types if the
`consumer` is set to\n`stackAlerts`\\r\\nthen the rules will not be
accessible from any rule\npage even if the\\r\\nuser has access to the
\\\"Stack alerts\\\" feature\nprivilege. This PR fixes\\r\\nthis
bug.\\r\\n\\r\\nFixes\nhttps://github.com//issues/193549\\r\\nFixes\nhttps://github.com//issues/191075\\r\\nFixes\nhttps://github.com//issues/184422\\r\\nFixes\nhttps://github.com//issues/179082\\r\\n\\r\\n##\nTesting\\r\\n\\r\\n1.
Create an anomaly detection and custom threshold rule\nand set
the\\r\\n\\\"Role visibility\\\" to \\\"Stack alerts\\\".\\r\\n2. Create
a\nuser with access only to \\\"Stack alerts\\\".\\r\\n3. Login with the
user\ncreated in Step 2.\\r\\n4. Verify that you can see the rules from
the\nstack management page.\\r\\n5. Verify that you can see the
alerts\ngenerated from the rules.\\r\\n6. Create a user with roles
`kibana_admin`\nand verify the same.\\r\\n\\r\\n\\r\\n###
Checklist\\r\\n\\r\\n- [x] [Unit
or\nfunctional\\r\\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\\r\\nwere\nupdated
or added to match the most common scenarios\\r\\n- [x]
[Flaky\nTest\\r\\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)\nwas\\r\\nused
on any tests changed\\r\\n\\r\\n### For maintainers\\r\\n\\r\\n-
[x]\nThis was checked for breaking API changes and
was\n[labeled\\r\\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\\r\\n\\r\\n##\nRelease
notes\\r\\n\\r\\nFix bug where rule types with \\\"Stack alerts\\\"
role\nvisibility are not\\r\\nbeing shown in the stack
management\npage\",\"sha\":\"8d83a075f6228bb5a6e35a9bb4654fe29cee0cff\"}},\"sourceBranch\":\"main\",\"suggestedTargetBranches\":[\"8.x\",\"8.15\"],\"targetPullRequestStates\":[{\"branch\":\"main\",\"label\":\"v9.0.0\",\"branchLabelMappingKey\":\"^v9.0.0$\",\"isSourceBranch\":true,\"state\":\"MERGED\",\"url\":\"https://github.com/elastic/kibana/pull/194615\",\"number\":194615,\"mergeCommit\":{\"message\":\"[ResponseOps][Alerting]\nRegister
anomaly detection and custom threshold rule types under stack\nalerts
feature privilege (#194615)\\n\\n## Summary\\r\\n\\r\\nIn the ES
query,\nanomaly detection, and custom threshold rule types\\r\\nusers
can use the\n\\\"Role visibility\\\" dropdown to select where the
rules\\r\\nshould be\naccessible. The \\\"Role visibility\\\" dropdown
sets the\n`consumer`\\r\\nwhich is paramount for alerting RBAC. For the
anomaly\ndetection and\\r\\ncustom threshold rule types if the
`consumer` is set to\n`stackAlerts`\\r\\nthen the rules will not be
accessible from any rule\npage even if the\\r\\nuser has access to the
\\\"Stack alerts\\\" feature\nprivilege. This PR fixes\\r\\nthis
bug.\\r\\n\\r\\nFixes\nhttps://github.com//issues/193549\\r\\nFixes\nhttps://github.com//issues/191075\\r\\nFixes\nhttps://github.com//issues/184422\\r\\nFixes\nhttps://github.com//issues/179082\\r\\n\\r\\n##\nTesting\\r\\n\\r\\n1.
Create an anomaly detection and custom threshold rule\nand set
the\\r\\n\\\"Role visibility\\\" to \\\"Stack alerts\\\".\\r\\n2. Create
a\nuser with access only to \\\"Stack alerts\\\".\\r\\n3. Login with the
user\ncreated in Step 2.\\r\\n4. Verify that you can see the rules from
the\nstack management page.\\r\\n5. Verify that you can see the
alerts\ngenerated from the rules.\\r\\n6. Create a user with roles
`kibana_admin`\nand verify the same.\\r\\n\\r\\n\\r\\n###
Checklist\\r\\n\\r\\n- [x] [Unit
or\nfunctional\\r\\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\\r\\nwere\nupdated
or added to match the most common scenarios\\r\\n- [x]
[Flaky\nTest\\r\\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)\nwas\\r\\nused
on any tests changed\\r\\n\\r\\n### For maintainers\\r\\n\\r\\n-
[x]\nThis was checked for breaking API changes and
was\n[labeled\\r\\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\\r\\n\\r\\n##\nRelease
notes\\r\\n\\r\\nFix bug where rule types with \\\"Stack alerts\\\"
role\nvisibility are not\\r\\nbeing shown in the stack
management\npage\",\"sha\":\"8d83a075f6228bb5a6e35a9bb4654fe29cee0cff\"}},{\"branch\":\"8.x\",\"label\":\"v8.16.0\",\"branchLabelMappingKey\":\"^v8.16.0$\",\"isSourceBranch\":false,\"state\":\"NOT_CREATED\"},{\"branch\":\"8.15\",\"label\":\"v8.15.3\",\"branchLabelMappingKey\":\"^v(\\\\d+).(\\\\d+).\\\\d+$\",\"isSourceBranch\":false,\"state\":\"NOT_CREATED\"}]}]\nBACKPORT-->\n\nCo-authored-by:
Christos Nasikas
<[email protected]>"}},{"branch":"8.15","label":"v8.15.3","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->
tiansivive pushed a commit to tiansivive/kibana that referenced this issue Oct 7, 2024
…ld rule types under stack alerts feature privilege (elastic#194615)

## Summary

In the ES query, anomaly detection, and custom threshold rule types
users can use the "Role visibility" dropdown to select where the rules
should be accessible. The "Role visibility" dropdown sets the `consumer`
which is paramount for alerting RBAC. For the anomaly detection and
custom threshold rule types if the `consumer` is set to `stackAlerts`
then the rules will not be accessible from any rule page even if the
user has access to the "Stack alerts" feature privilege. This PR fixes
this bug.

Fixes elastic#193549
Fixes elastic#191075
Fixes elastic#184422
Fixes elastic#179082

## Testing

1. Create an anomaly detection and custom threshold rule and set the
"Role visibility" to "Stack alerts".
2. Create a user with access only to "Stack alerts".
3. Login with the user created in Step 2.
4. Verify that you can see the rules from the stack management page.
5. Verify that you can see the alerts generated from the rules.
6. Create a user with roles `kibana_admin` and verify the same.


### Checklist

- [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
- [x] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed

### 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)

## Release notes

Fix bug where rule types with "Stack alerts" role visibility are not
being shown in the stack management page
cnasikas added a commit that referenced this issue Dec 3, 2024
## Summary

This PR aims to decouple the feature IDs from the `consumer` attribute
of rules and alerts.

Towards: #187202
Fixes: #181559
Fixes: #182435

> [!NOTE]  
> Unfortunately, I could not break the PR into smaller pieces. The APIs
could not work anymore with feature IDs and had to convert them to use
rule type IDs. Also, I took the chance and refactored crucial parts of
the authorization class that in turn affected a lot of files. Most of
the changes in the files are minimal and easy to review. The crucial
changes are in the authorization class and some alerting APIs.

## Architecture

### Alerting RBAC model

The Kibana security uses Elasticsearch's [application
privileges](https://www.elastic.co/guide/en/elasticsearch/reference/current/security-api-put-privileges.html#security-api-put-privileges).
This way Kibana can represent and store its privilege models within
Elasticsearch roles. To do that, Kibana security creates actions that
are granted by a specific privilege. Alerting uses its own RBAC model
and is built on top of the existing Kibana security model. The Alerting
RBAC uses the `rule_type_id` and `consumer` attributes to define who
owns the rule and the alerts procured by the rule. To connect the
`rule_type_id` and `consumer` with the Kibana security actions the
Alerting RBAC registers its custom actions. They are constructed as
`alerting:<rule-type-id>/<feature-id>/<alerting-entity>/<operation>`.
Because to authorizate a resource an action has to be generated and
because the action needs a valid feature ID the value of the `consumer`
should be a valid feature ID. For example, the
`alerting:siem.esqlRule/siem/rule/get` action, means that a user with a
role that grants this action can get a rule of type `siem.esqlRule` with
consumer `siem`.

### Problem statement

At the moment the `consumer` attribute should be a valid feature ID.
Though this approach worked well so far it has its limitation.
Specifically:

- Rule types cannot support more than one consumer.
- To associate old rules with a new feature ID required a migration on
the rule's SOs and the alerts documents.
- The API calls are feature ID-oriented and not rule-type-oriented.
- The framework has to be aware of the values of the `consumer`
attribute.
- Feature IDs are tightly coupled with the alerting indices leading to
[bugs](#179082).
- Legacy consumers that are not a valid feature anymore can cause
[bugs](#184595).
- The framework has to be aware of legacy consumers to handle edge
cases.
- The framework has to be aware of specific consumers to handle edge
cases.

### Proposed solution

This PR aims to decouple the feature IDs from consumers. It achieves
that a) by changing the way solutions configure the alerting privileges
when registering a feature and b) by changing the alerting actions. The
schema changes as:

```
// Old formatting
id: 'siem', <--- feature ID
alerting:['siem.queryRule']

// New formatting
id: 'siem', <--- feature ID
alerting: [{ ruleTypeId: 'siem.queryRule', consumers: ['siem'] }] <-- consumer same as the feature ID in the old formatting
```

The new actions are constructed as
`alerting:<rule-type-id>/<consumer>/<alerting-entity>/<operation>`. For
example `alerting:rule-type-id/my-consumer/rule/get`. The new action
means that a user with a role that grants this action can get a rule of
type `rule-type` with consumer `my-consumer`. Changing the action
strings is not considered a breaking change as long as the user's
permission works as before. In our case, this is true because the
consumer will be the same as before (feature ID), and the alerting
security actions will be the same. For example:

**Old formatting**

Schema:
```
id: 'logs', <--- feature ID
alerting:['.es-query'] <-- rule type ID
```

Generated action:

```
alerting:.es-query/logs/rule/get
```

**New formatting**

Schema:
```
id: 'siem', <--- feature ID
alerting: [{ ruleTypeId: '.es-query', consumers: ['logs'] }] <-- consumer same as the feature ID in the old formatting
```

Generated action:

```
alerting:.es-query/logs/rule/get <--- consumer is set as logs and the action is the same as before
```

In both formating the actions are the same thus breaking changes are
avoided.

### Alerting authorization class
The alerting plugin uses and exports the alerting authorization class
(`AlertingAuthorization`). The class is responsible for handling all
authorization actions related to rules and alerts. The class changed to
handle the new actions as described in the above sections. A lot of
methods were renamed, removed, and cleaned up, all method arguments
converted to be an object, and the response signature of some methods
changed. These changes affected various pieces of the code. The changes
in this class are the most important in this PR especially the
`_getAuthorizedRuleTypesWithAuthorizedConsumers` method which is the
cornerstone of the alerting RBAC. Please review carefully.

### Instantiation of the alerting authorization class
The `AlertingAuthorizationClientFactory` is used to create instances of
the `AlertingAuthorization` class. The `AlertingAuthorization` class
needs to perform async operations upon instantiation. Because JS, at the
moment, does not support async instantiation of classes the
`AlertingAuthorization` class was assigning `Promise` objects to
variables that could be resolved later in other phases of the lifecycle
of the class. To improve readability and make the lifecycle of the class
clearer, I separated the construction of the class (initialization) from
the bootstrap process. As a result, getting the `AlertingAuthorization`
class or any client that depends on it (`getRulesClient` for example) is
an async operation.

### Filtering
A lot of routes use the authorization class to get the authorization
filter (`getFindAuthorizationFilter`), a filter that, if applied,
returns only the rule types and consumers the user is authorized to. The
method that returns the filter was built in a way to also support
filtering on top of the authorization filter thus coupling the
authorized filter with router filtering. I believe these two operations
should be decoupled and the filter method should return a filter that
gives you all the authorized rule types. It is the responsibility of the
consumer, router in our case, to apply extra filters on top of the
authorization filter. For that reason, I made all the necessary changes
to decouple them.

### Legacy consumers & producer
A lot of rules and alerts have been created and are still being created
from observability with the `alerts` consumer. When the Alerting RBAC
encounters a rule or alert with `alerts` as a consumer it falls back to
the `producer` of the rule type ID to construct the actions. For example
if a rule with `ruleTypeId: .es-query` and `consumer: alerts` the
alerting action will be constructed as
`alerting:.es-query/stackAlerts/rule/get` where `stackRules` is the
producer of the `.es-query` rule type. The `producer` is used to be used
in alerting authorization but due to its complexity, it was deprecated
and only used as a fallback for the `alerts` consumer. To avoid breaking
changes all feature privileges that specify access to rule types add the
`alerts` consumer when configuring their alerting privileges. By moving
the `alerts` consumer to the registration of the feature we can stop
relying on the `producer`. The `producer` is not used anymore in the
authorization class. In the next PRs the `producer` will removed
entirely.

### Routes
The following changes were introduced to the alerting routes:

- All related routes changed to be rule-type oriented and not feature ID
oriented.
- All related routes support the `ruleTypeIds` and the `consumers`
parameters for filtering. In all routes, the filters are constructed as
`ruleTypeIds: ['foo'] AND consumers: ['bar'] AND authorizationFilter`.
Filtering by consumers is important. In o11y for example, we do not want
to show ES rule types with the `stackAlerts` consumer even if the user
has access to them.
- The `/internal/rac/alerts/_feature_ids` route got deleted as it was
not used anywhere in the codebase and it was internal.

All the changes in the routes are related to internal routes and no
breaking changes are introduced.

### Constants
I moved the o11y and stack rule type IDs to `kbn-rule-data-utils` and
exported all security solution rule type IDs from
`kbn-securitysolution-rules`. I am not a fan of having a centralized
place for the rule type IDs. Ideally, consumers of the framework should
specify keywords like `observablility` (category or subcategory) or even
`apm.*` and the framework should know which rule type IDs to pick up. I
think it is out of the scope of the PR, and at the moment it seems the
most straightforward way to move forward. I will try to clean up as much
as possible in further iterations. If you are interested in the upcoming
work follow this issue #187202.

### Other notable code changes
- Change all instances of feature IDs to rule type IDs.
- `isSiemRuleType`: This is a temporary helper function that is needed
in places where we handle edge cases related to security solution rule
types. Ideally, the framework should be agnostic to the rule types or
consumers. The plan is to be removed entirely in further iterations.
- Rename alerting `PluginSetupContract` and `PluginStartContract` to
`AlertingServerSetup` and `AlertingServerStart`. This made me touch a
lot of files but I could not resist.
- `filter_consumers` was mistakenly exposed to a public API. It was
undocumented.
- Files or functions that were not used anywhere in the codebase got
deleted.
- Change the returned type of the `list` method of the
`RuleTypeRegistry` from `Set<RegistryRuleType>` to `Map<string,
RegistryRuleType>`.
- Assertion of `KueryNode` in tests changed to an assertion of KQL using
`toKqlExpression`.
- Removal of `useRuleAADFields` as it is not used anywhere.

## Testing

> [!CAUTION]
> It is very important to test all the areas of the application where
rules or alerts are being used directly or indirectly. Scenarios to
consider:
> - The correct rules, alerts, and aggregations on top of them are being
shown as expected as a superuser.
> - The correct rules, alerts, and aggregations on top of them are being
shown as expected by a user with limited access to certain features.
> - The changes in this PR are backward compatible with the previous
users' permissions.

### Solutions
Please test and verify that:
- All the rule types you own with all possible combinations of
permissions both in ESS and in Serverless.
- The consumers and rule types make sense when registering the features.
- The consumers and rule types that are passed to the components are the
intended ones.

### ResponseOps
The most important changes are in the alerting authorization class, the
search strategy, and the routes. Please test:
- The rules we own with all possible combinations of permissions.
- The stack alerts page and its solution filtering.
- The categories filtering in the maintenance window UI.

## Risks
> [!WARNING]
> The risks involved in this PR are related to privileges. Specifically:
> - Users with no privileges can access rules and alerts they do not
have access to.
> - Users with privileges cannot access rules and alerts they have
access to.
>
> An excessive list of integration tests is in place to ensure that the
above scenarios will not occur. In the case of a bug, we could a)
release an energy release for serverless and b) backport the fix in ESS.
Given that this PR is intended to be merged in 8.17 we have plenty of
time to test and to minimize the chances of risks.

## FQA

- I noticed that a lot of routes support the `filter` parameter where we
can pass an arbitrary KQL filter. Why we do not use this to filter by
the rule type IDs and the consumers and instead we introduce new
dedicated parameters?

The `filter` parameter should not be exposed in the first place. It
assumes that the consumer of the API knows the underlying structure and
implementation details of the persisted storage API (SavedObject client
API). For example, a valid filter would be
`alerting.attributes.rule_type_id`. In this filter the consumer should
know a) the name of the SO b) the keyword `attributes` (storage
implementation detail) and c) the name of the attribute as it is
persisted in ES (snake case instead of camel case as it is returned by
the APIs). As there is no abstraction layer between the SO and the API,
it makes it very difficult to make changes in the persistent schema or
the APIs. For all the above I decided to introduce new query parameters
where the alerting framework has total control over it.

- I noticed in the code a lot of instances where the consumer is used.
Should not remove any logic around consumers?

This PR is a step forward making the framework as agnostic as possible.
I had to keep the scope of the PR as contained as possible. We will get
there. It needs time :).

- I noticed a lot of hacks like checking if the rule type is `siem`.
Should not remove the hacks?

This PR is a step forward making the framework as agnostic as possible.
I had to keep the scope of the PR as contained as possible. We will get
there. It needs time :).

- I hate the "Role visibility" dropdown. Can we remove it?

I also do not like it. The goal is to remove it. Follow
#189997.

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Aleh Zasypkin <[email protected]>
Co-authored-by: Paula Borgonovi <[email protected]>
hop-dev pushed a commit to hop-dev/kibana that referenced this issue Dec 5, 2024
…183756)

## Summary

This PR aims to decouple the feature IDs from the `consumer` attribute
of rules and alerts.

Towards: elastic#187202
Fixes: elastic#181559
Fixes: elastic#182435

> [!NOTE]  
> Unfortunately, I could not break the PR into smaller pieces. The APIs
could not work anymore with feature IDs and had to convert them to use
rule type IDs. Also, I took the chance and refactored crucial parts of
the authorization class that in turn affected a lot of files. Most of
the changes in the files are minimal and easy to review. The crucial
changes are in the authorization class and some alerting APIs.

## Architecture

### Alerting RBAC model

The Kibana security uses Elasticsearch's [application
privileges](https://www.elastic.co/guide/en/elasticsearch/reference/current/security-api-put-privileges.html#security-api-put-privileges).
This way Kibana can represent and store its privilege models within
Elasticsearch roles. To do that, Kibana security creates actions that
are granted by a specific privilege. Alerting uses its own RBAC model
and is built on top of the existing Kibana security model. The Alerting
RBAC uses the `rule_type_id` and `consumer` attributes to define who
owns the rule and the alerts procured by the rule. To connect the
`rule_type_id` and `consumer` with the Kibana security actions the
Alerting RBAC registers its custom actions. They are constructed as
`alerting:<rule-type-id>/<feature-id>/<alerting-entity>/<operation>`.
Because to authorizate a resource an action has to be generated and
because the action needs a valid feature ID the value of the `consumer`
should be a valid feature ID. For example, the
`alerting:siem.esqlRule/siem/rule/get` action, means that a user with a
role that grants this action can get a rule of type `siem.esqlRule` with
consumer `siem`.

### Problem statement

At the moment the `consumer` attribute should be a valid feature ID.
Though this approach worked well so far it has its limitation.
Specifically:

- Rule types cannot support more than one consumer.
- To associate old rules with a new feature ID required a migration on
the rule's SOs and the alerts documents.
- The API calls are feature ID-oriented and not rule-type-oriented.
- The framework has to be aware of the values of the `consumer`
attribute.
- Feature IDs are tightly coupled with the alerting indices leading to
[bugs](elastic#179082).
- Legacy consumers that are not a valid feature anymore can cause
[bugs](elastic#184595).
- The framework has to be aware of legacy consumers to handle edge
cases.
- The framework has to be aware of specific consumers to handle edge
cases.

### Proposed solution

This PR aims to decouple the feature IDs from consumers. It achieves
that a) by changing the way solutions configure the alerting privileges
when registering a feature and b) by changing the alerting actions. The
schema changes as:

```
// Old formatting
id: 'siem', <--- feature ID
alerting:['siem.queryRule']

// New formatting
id: 'siem', <--- feature ID
alerting: [{ ruleTypeId: 'siem.queryRule', consumers: ['siem'] }] <-- consumer same as the feature ID in the old formatting
```

The new actions are constructed as
`alerting:<rule-type-id>/<consumer>/<alerting-entity>/<operation>`. For
example `alerting:rule-type-id/my-consumer/rule/get`. The new action
means that a user with a role that grants this action can get a rule of
type `rule-type` with consumer `my-consumer`. Changing the action
strings is not considered a breaking change as long as the user's
permission works as before. In our case, this is true because the
consumer will be the same as before (feature ID), and the alerting
security actions will be the same. For example:

**Old formatting**

Schema:
```
id: 'logs', <--- feature ID
alerting:['.es-query'] <-- rule type ID
```

Generated action:

```
alerting:.es-query/logs/rule/get
```

**New formatting**

Schema:
```
id: 'siem', <--- feature ID
alerting: [{ ruleTypeId: '.es-query', consumers: ['logs'] }] <-- consumer same as the feature ID in the old formatting
```

Generated action:

```
alerting:.es-query/logs/rule/get <--- consumer is set as logs and the action is the same as before
```

In both formating the actions are the same thus breaking changes are
avoided.

### Alerting authorization class
The alerting plugin uses and exports the alerting authorization class
(`AlertingAuthorization`). The class is responsible for handling all
authorization actions related to rules and alerts. The class changed to
handle the new actions as described in the above sections. A lot of
methods were renamed, removed, and cleaned up, all method arguments
converted to be an object, and the response signature of some methods
changed. These changes affected various pieces of the code. The changes
in this class are the most important in this PR especially the
`_getAuthorizedRuleTypesWithAuthorizedConsumers` method which is the
cornerstone of the alerting RBAC. Please review carefully.

### Instantiation of the alerting authorization class
The `AlertingAuthorizationClientFactory` is used to create instances of
the `AlertingAuthorization` class. The `AlertingAuthorization` class
needs to perform async operations upon instantiation. Because JS, at the
moment, does not support async instantiation of classes the
`AlertingAuthorization` class was assigning `Promise` objects to
variables that could be resolved later in other phases of the lifecycle
of the class. To improve readability and make the lifecycle of the class
clearer, I separated the construction of the class (initialization) from
the bootstrap process. As a result, getting the `AlertingAuthorization`
class or any client that depends on it (`getRulesClient` for example) is
an async operation.

### Filtering
A lot of routes use the authorization class to get the authorization
filter (`getFindAuthorizationFilter`), a filter that, if applied,
returns only the rule types and consumers the user is authorized to. The
method that returns the filter was built in a way to also support
filtering on top of the authorization filter thus coupling the
authorized filter with router filtering. I believe these two operations
should be decoupled and the filter method should return a filter that
gives you all the authorized rule types. It is the responsibility of the
consumer, router in our case, to apply extra filters on top of the
authorization filter. For that reason, I made all the necessary changes
to decouple them.

### Legacy consumers & producer
A lot of rules and alerts have been created and are still being created
from observability with the `alerts` consumer. When the Alerting RBAC
encounters a rule or alert with `alerts` as a consumer it falls back to
the `producer` of the rule type ID to construct the actions. For example
if a rule with `ruleTypeId: .es-query` and `consumer: alerts` the
alerting action will be constructed as
`alerting:.es-query/stackAlerts/rule/get` where `stackRules` is the
producer of the `.es-query` rule type. The `producer` is used to be used
in alerting authorization but due to its complexity, it was deprecated
and only used as a fallback for the `alerts` consumer. To avoid breaking
changes all feature privileges that specify access to rule types add the
`alerts` consumer when configuring their alerting privileges. By moving
the `alerts` consumer to the registration of the feature we can stop
relying on the `producer`. The `producer` is not used anymore in the
authorization class. In the next PRs the `producer` will removed
entirely.

### Routes
The following changes were introduced to the alerting routes:

- All related routes changed to be rule-type oriented and not feature ID
oriented.
- All related routes support the `ruleTypeIds` and the `consumers`
parameters for filtering. In all routes, the filters are constructed as
`ruleTypeIds: ['foo'] AND consumers: ['bar'] AND authorizationFilter`.
Filtering by consumers is important. In o11y for example, we do not want
to show ES rule types with the `stackAlerts` consumer even if the user
has access to them.
- The `/internal/rac/alerts/_feature_ids` route got deleted as it was
not used anywhere in the codebase and it was internal.

All the changes in the routes are related to internal routes and no
breaking changes are introduced.

### Constants
I moved the o11y and stack rule type IDs to `kbn-rule-data-utils` and
exported all security solution rule type IDs from
`kbn-securitysolution-rules`. I am not a fan of having a centralized
place for the rule type IDs. Ideally, consumers of the framework should
specify keywords like `observablility` (category or subcategory) or even
`apm.*` and the framework should know which rule type IDs to pick up. I
think it is out of the scope of the PR, and at the moment it seems the
most straightforward way to move forward. I will try to clean up as much
as possible in further iterations. If you are interested in the upcoming
work follow this issue elastic#187202.

### Other notable code changes
- Change all instances of feature IDs to rule type IDs.
- `isSiemRuleType`: This is a temporary helper function that is needed
in places where we handle edge cases related to security solution rule
types. Ideally, the framework should be agnostic to the rule types or
consumers. The plan is to be removed entirely in further iterations.
- Rename alerting `PluginSetupContract` and `PluginStartContract` to
`AlertingServerSetup` and `AlertingServerStart`. This made me touch a
lot of files but I could not resist.
- `filter_consumers` was mistakenly exposed to a public API. It was
undocumented.
- Files or functions that were not used anywhere in the codebase got
deleted.
- Change the returned type of the `list` method of the
`RuleTypeRegistry` from `Set<RegistryRuleType>` to `Map<string,
RegistryRuleType>`.
- Assertion of `KueryNode` in tests changed to an assertion of KQL using
`toKqlExpression`.
- Removal of `useRuleAADFields` as it is not used anywhere.

## Testing

> [!CAUTION]
> It is very important to test all the areas of the application where
rules or alerts are being used directly or indirectly. Scenarios to
consider:
> - The correct rules, alerts, and aggregations on top of them are being
shown as expected as a superuser.
> - The correct rules, alerts, and aggregations on top of them are being
shown as expected by a user with limited access to certain features.
> - The changes in this PR are backward compatible with the previous
users' permissions.

### Solutions
Please test and verify that:
- All the rule types you own with all possible combinations of
permissions both in ESS and in Serverless.
- The consumers and rule types make sense when registering the features.
- The consumers and rule types that are passed to the components are the
intended ones.

### ResponseOps
The most important changes are in the alerting authorization class, the
search strategy, and the routes. Please test:
- The rules we own with all possible combinations of permissions.
- The stack alerts page and its solution filtering.
- The categories filtering in the maintenance window UI.

## Risks
> [!WARNING]
> The risks involved in this PR are related to privileges. Specifically:
> - Users with no privileges can access rules and alerts they do not
have access to.
> - Users with privileges cannot access rules and alerts they have
access to.
>
> An excessive list of integration tests is in place to ensure that the
above scenarios will not occur. In the case of a bug, we could a)
release an energy release for serverless and b) backport the fix in ESS.
Given that this PR is intended to be merged in 8.17 we have plenty of
time to test and to minimize the chances of risks.

## FQA

- I noticed that a lot of routes support the `filter` parameter where we
can pass an arbitrary KQL filter. Why we do not use this to filter by
the rule type IDs and the consumers and instead we introduce new
dedicated parameters?

The `filter` parameter should not be exposed in the first place. It
assumes that the consumer of the API knows the underlying structure and
implementation details of the persisted storage API (SavedObject client
API). For example, a valid filter would be
`alerting.attributes.rule_type_id`. In this filter the consumer should
know a) the name of the SO b) the keyword `attributes` (storage
implementation detail) and c) the name of the attribute as it is
persisted in ES (snake case instead of camel case as it is returned by
the APIs). As there is no abstraction layer between the SO and the API,
it makes it very difficult to make changes in the persistent schema or
the APIs. For all the above I decided to introduce new query parameters
where the alerting framework has total control over it.

- I noticed in the code a lot of instances where the consumer is used.
Should not remove any logic around consumers?

This PR is a step forward making the framework as agnostic as possible.
I had to keep the scope of the PR as contained as possible. We will get
there. It needs time :).

- I noticed a lot of hacks like checking if the rule type is `siem`.
Should not remove the hacks?

This PR is a step forward making the framework as agnostic as possible.
I had to keep the scope of the PR as contained as possible. We will get
there. It needs time :).

- I hate the "Role visibility" dropdown. Can we remove it?

I also do not like it. The goal is to remove it. Follow
elastic#189997.

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Aleh Zasypkin <[email protected]>
Co-authored-by: Paula Borgonovi <[email protected]>
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this issue Dec 9, 2024
…183756)

## Summary

This PR aims to decouple the feature IDs from the `consumer` attribute
of rules and alerts.

Towards: elastic#187202
Fixes: elastic#181559
Fixes: elastic#182435

> [!NOTE]  
> Unfortunately, I could not break the PR into smaller pieces. The APIs
could not work anymore with feature IDs and had to convert them to use
rule type IDs. Also, I took the chance and refactored crucial parts of
the authorization class that in turn affected a lot of files. Most of
the changes in the files are minimal and easy to review. The crucial
changes are in the authorization class and some alerting APIs.

## Architecture

### Alerting RBAC model

The Kibana security uses Elasticsearch's [application
privileges](https://www.elastic.co/guide/en/elasticsearch/reference/current/security-api-put-privileges.html#security-api-put-privileges).
This way Kibana can represent and store its privilege models within
Elasticsearch roles. To do that, Kibana security creates actions that
are granted by a specific privilege. Alerting uses its own RBAC model
and is built on top of the existing Kibana security model. The Alerting
RBAC uses the `rule_type_id` and `consumer` attributes to define who
owns the rule and the alerts procured by the rule. To connect the
`rule_type_id` and `consumer` with the Kibana security actions the
Alerting RBAC registers its custom actions. They are constructed as
`alerting:<rule-type-id>/<feature-id>/<alerting-entity>/<operation>`.
Because to authorizate a resource an action has to be generated and
because the action needs a valid feature ID the value of the `consumer`
should be a valid feature ID. For example, the
`alerting:siem.esqlRule/siem/rule/get` action, means that a user with a
role that grants this action can get a rule of type `siem.esqlRule` with
consumer `siem`.

### Problem statement

At the moment the `consumer` attribute should be a valid feature ID.
Though this approach worked well so far it has its limitation.
Specifically:

- Rule types cannot support more than one consumer.
- To associate old rules with a new feature ID required a migration on
the rule's SOs and the alerts documents.
- The API calls are feature ID-oriented and not rule-type-oriented.
- The framework has to be aware of the values of the `consumer`
attribute.
- Feature IDs are tightly coupled with the alerting indices leading to
[bugs](elastic#179082).
- Legacy consumers that are not a valid feature anymore can cause
[bugs](elastic#184595).
- The framework has to be aware of legacy consumers to handle edge
cases.
- The framework has to be aware of specific consumers to handle edge
cases.

### Proposed solution

This PR aims to decouple the feature IDs from consumers. It achieves
that a) by changing the way solutions configure the alerting privileges
when registering a feature and b) by changing the alerting actions. The
schema changes as:

```
// Old formatting
id: 'siem', <--- feature ID
alerting:['siem.queryRule']

// New formatting
id: 'siem', <--- feature ID
alerting: [{ ruleTypeId: 'siem.queryRule', consumers: ['siem'] }] <-- consumer same as the feature ID in the old formatting
```

The new actions are constructed as
`alerting:<rule-type-id>/<consumer>/<alerting-entity>/<operation>`. For
example `alerting:rule-type-id/my-consumer/rule/get`. The new action
means that a user with a role that grants this action can get a rule of
type `rule-type` with consumer `my-consumer`. Changing the action
strings is not considered a breaking change as long as the user's
permission works as before. In our case, this is true because the
consumer will be the same as before (feature ID), and the alerting
security actions will be the same. For example:

**Old formatting**

Schema:
```
id: 'logs', <--- feature ID
alerting:['.es-query'] <-- rule type ID
```

Generated action:

```
alerting:.es-query/logs/rule/get
```

**New formatting**

Schema:
```
id: 'siem', <--- feature ID
alerting: [{ ruleTypeId: '.es-query', consumers: ['logs'] }] <-- consumer same as the feature ID in the old formatting
```

Generated action:

```
alerting:.es-query/logs/rule/get <--- consumer is set as logs and the action is the same as before
```

In both formating the actions are the same thus breaking changes are
avoided.

### Alerting authorization class
The alerting plugin uses and exports the alerting authorization class
(`AlertingAuthorization`). The class is responsible for handling all
authorization actions related to rules and alerts. The class changed to
handle the new actions as described in the above sections. A lot of
methods were renamed, removed, and cleaned up, all method arguments
converted to be an object, and the response signature of some methods
changed. These changes affected various pieces of the code. The changes
in this class are the most important in this PR especially the
`_getAuthorizedRuleTypesWithAuthorizedConsumers` method which is the
cornerstone of the alerting RBAC. Please review carefully.

### Instantiation of the alerting authorization class
The `AlertingAuthorizationClientFactory` is used to create instances of
the `AlertingAuthorization` class. The `AlertingAuthorization` class
needs to perform async operations upon instantiation. Because JS, at the
moment, does not support async instantiation of classes the
`AlertingAuthorization` class was assigning `Promise` objects to
variables that could be resolved later in other phases of the lifecycle
of the class. To improve readability and make the lifecycle of the class
clearer, I separated the construction of the class (initialization) from
the bootstrap process. As a result, getting the `AlertingAuthorization`
class or any client that depends on it (`getRulesClient` for example) is
an async operation.

### Filtering
A lot of routes use the authorization class to get the authorization
filter (`getFindAuthorizationFilter`), a filter that, if applied,
returns only the rule types and consumers the user is authorized to. The
method that returns the filter was built in a way to also support
filtering on top of the authorization filter thus coupling the
authorized filter with router filtering. I believe these two operations
should be decoupled and the filter method should return a filter that
gives you all the authorized rule types. It is the responsibility of the
consumer, router in our case, to apply extra filters on top of the
authorization filter. For that reason, I made all the necessary changes
to decouple them.

### Legacy consumers & producer
A lot of rules and alerts have been created and are still being created
from observability with the `alerts` consumer. When the Alerting RBAC
encounters a rule or alert with `alerts` as a consumer it falls back to
the `producer` of the rule type ID to construct the actions. For example
if a rule with `ruleTypeId: .es-query` and `consumer: alerts` the
alerting action will be constructed as
`alerting:.es-query/stackAlerts/rule/get` where `stackRules` is the
producer of the `.es-query` rule type. The `producer` is used to be used
in alerting authorization but due to its complexity, it was deprecated
and only used as a fallback for the `alerts` consumer. To avoid breaking
changes all feature privileges that specify access to rule types add the
`alerts` consumer when configuring their alerting privileges. By moving
the `alerts` consumer to the registration of the feature we can stop
relying on the `producer`. The `producer` is not used anymore in the
authorization class. In the next PRs the `producer` will removed
entirely.

### Routes
The following changes were introduced to the alerting routes:

- All related routes changed to be rule-type oriented and not feature ID
oriented.
- All related routes support the `ruleTypeIds` and the `consumers`
parameters for filtering. In all routes, the filters are constructed as
`ruleTypeIds: ['foo'] AND consumers: ['bar'] AND authorizationFilter`.
Filtering by consumers is important. In o11y for example, we do not want
to show ES rule types with the `stackAlerts` consumer even if the user
has access to them.
- The `/internal/rac/alerts/_feature_ids` route got deleted as it was
not used anywhere in the codebase and it was internal.

All the changes in the routes are related to internal routes and no
breaking changes are introduced.

### Constants
I moved the o11y and stack rule type IDs to `kbn-rule-data-utils` and
exported all security solution rule type IDs from
`kbn-securitysolution-rules`. I am not a fan of having a centralized
place for the rule type IDs. Ideally, consumers of the framework should
specify keywords like `observablility` (category or subcategory) or even
`apm.*` and the framework should know which rule type IDs to pick up. I
think it is out of the scope of the PR, and at the moment it seems the
most straightforward way to move forward. I will try to clean up as much
as possible in further iterations. If you are interested in the upcoming
work follow this issue elastic#187202.

### Other notable code changes
- Change all instances of feature IDs to rule type IDs.
- `isSiemRuleType`: This is a temporary helper function that is needed
in places where we handle edge cases related to security solution rule
types. Ideally, the framework should be agnostic to the rule types or
consumers. The plan is to be removed entirely in further iterations.
- Rename alerting `PluginSetupContract` and `PluginStartContract` to
`AlertingServerSetup` and `AlertingServerStart`. This made me touch a
lot of files but I could not resist.
- `filter_consumers` was mistakenly exposed to a public API. It was
undocumented.
- Files or functions that were not used anywhere in the codebase got
deleted.
- Change the returned type of the `list` method of the
`RuleTypeRegistry` from `Set<RegistryRuleType>` to `Map<string,
RegistryRuleType>`.
- Assertion of `KueryNode` in tests changed to an assertion of KQL using
`toKqlExpression`.
- Removal of `useRuleAADFields` as it is not used anywhere.

## Testing

> [!CAUTION]
> It is very important to test all the areas of the application where
rules or alerts are being used directly or indirectly. Scenarios to
consider:
> - The correct rules, alerts, and aggregations on top of them are being
shown as expected as a superuser.
> - The correct rules, alerts, and aggregations on top of them are being
shown as expected by a user with limited access to certain features.
> - The changes in this PR are backward compatible with the previous
users' permissions.

### Solutions
Please test and verify that:
- All the rule types you own with all possible combinations of
permissions both in ESS and in Serverless.
- The consumers and rule types make sense when registering the features.
- The consumers and rule types that are passed to the components are the
intended ones.

### ResponseOps
The most important changes are in the alerting authorization class, the
search strategy, and the routes. Please test:
- The rules we own with all possible combinations of permissions.
- The stack alerts page and its solution filtering.
- The categories filtering in the maintenance window UI.

## Risks
> [!WARNING]
> The risks involved in this PR are related to privileges. Specifically:
> - Users with no privileges can access rules and alerts they do not
have access to.
> - Users with privileges cannot access rules and alerts they have
access to.
>
> An excessive list of integration tests is in place to ensure that the
above scenarios will not occur. In the case of a bug, we could a)
release an energy release for serverless and b) backport the fix in ESS.
Given that this PR is intended to be merged in 8.17 we have plenty of
time to test and to minimize the chances of risks.

## FQA

- I noticed that a lot of routes support the `filter` parameter where we
can pass an arbitrary KQL filter. Why we do not use this to filter by
the rule type IDs and the consumers and instead we introduce new
dedicated parameters?

The `filter` parameter should not be exposed in the first place. It
assumes that the consumer of the API knows the underlying structure and
implementation details of the persisted storage API (SavedObject client
API). For example, a valid filter would be
`alerting.attributes.rule_type_id`. In this filter the consumer should
know a) the name of the SO b) the keyword `attributes` (storage
implementation detail) and c) the name of the attribute as it is
persisted in ES (snake case instead of camel case as it is returned by
the APIs). As there is no abstraction layer between the SO and the API,
it makes it very difficult to make changes in the persistent schema or
the APIs. For all the above I decided to introduce new query parameters
where the alerting framework has total control over it.

- I noticed in the code a lot of instances where the consumer is used.
Should not remove any logic around consumers?

This PR is a step forward making the framework as agnostic as possible.
I had to keep the scope of the PR as contained as possible. We will get
there. It needs time :).

- I noticed a lot of hacks like checking if the rule type is `siem`.
Should not remove the hacks?

This PR is a step forward making the framework as agnostic as possible.
I had to keep the scope of the PR as contained as possible. We will get
there. It needs time :).

- I hate the "Role visibility" dropdown. Can we remove it?

I also do not like it. The goal is to remove it. Follow
elastic#189997.

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Aleh Zasypkin <[email protected]>
Co-authored-by: Paula Borgonovi <[email protected]>
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this issue Dec 12, 2024
…183756)

## Summary

This PR aims to decouple the feature IDs from the `consumer` attribute
of rules and alerts.

Towards: elastic#187202
Fixes: elastic#181559
Fixes: elastic#182435

> [!NOTE]  
> Unfortunately, I could not break the PR into smaller pieces. The APIs
could not work anymore with feature IDs and had to convert them to use
rule type IDs. Also, I took the chance and refactored crucial parts of
the authorization class that in turn affected a lot of files. Most of
the changes in the files are minimal and easy to review. The crucial
changes are in the authorization class and some alerting APIs.

## Architecture

### Alerting RBAC model

The Kibana security uses Elasticsearch's [application
privileges](https://www.elastic.co/guide/en/elasticsearch/reference/current/security-api-put-privileges.html#security-api-put-privileges).
This way Kibana can represent and store its privilege models within
Elasticsearch roles. To do that, Kibana security creates actions that
are granted by a specific privilege. Alerting uses its own RBAC model
and is built on top of the existing Kibana security model. The Alerting
RBAC uses the `rule_type_id` and `consumer` attributes to define who
owns the rule and the alerts procured by the rule. To connect the
`rule_type_id` and `consumer` with the Kibana security actions the
Alerting RBAC registers its custom actions. They are constructed as
`alerting:<rule-type-id>/<feature-id>/<alerting-entity>/<operation>`.
Because to authorizate a resource an action has to be generated and
because the action needs a valid feature ID the value of the `consumer`
should be a valid feature ID. For example, the
`alerting:siem.esqlRule/siem/rule/get` action, means that a user with a
role that grants this action can get a rule of type `siem.esqlRule` with
consumer `siem`.

### Problem statement

At the moment the `consumer` attribute should be a valid feature ID.
Though this approach worked well so far it has its limitation.
Specifically:

- Rule types cannot support more than one consumer.
- To associate old rules with a new feature ID required a migration on
the rule's SOs and the alerts documents.
- The API calls are feature ID-oriented and not rule-type-oriented.
- The framework has to be aware of the values of the `consumer`
attribute.
- Feature IDs are tightly coupled with the alerting indices leading to
[bugs](elastic#179082).
- Legacy consumers that are not a valid feature anymore can cause
[bugs](elastic#184595).
- The framework has to be aware of legacy consumers to handle edge
cases.
- The framework has to be aware of specific consumers to handle edge
cases.

### Proposed solution

This PR aims to decouple the feature IDs from consumers. It achieves
that a) by changing the way solutions configure the alerting privileges
when registering a feature and b) by changing the alerting actions. The
schema changes as:

```
// Old formatting
id: 'siem', <--- feature ID
alerting:['siem.queryRule']

// New formatting
id: 'siem', <--- feature ID
alerting: [{ ruleTypeId: 'siem.queryRule', consumers: ['siem'] }] <-- consumer same as the feature ID in the old formatting
```

The new actions are constructed as
`alerting:<rule-type-id>/<consumer>/<alerting-entity>/<operation>`. For
example `alerting:rule-type-id/my-consumer/rule/get`. The new action
means that a user with a role that grants this action can get a rule of
type `rule-type` with consumer `my-consumer`. Changing the action
strings is not considered a breaking change as long as the user's
permission works as before. In our case, this is true because the
consumer will be the same as before (feature ID), and the alerting
security actions will be the same. For example:

**Old formatting**

Schema:
```
id: 'logs', <--- feature ID
alerting:['.es-query'] <-- rule type ID
```

Generated action:

```
alerting:.es-query/logs/rule/get
```

**New formatting**

Schema:
```
id: 'siem', <--- feature ID
alerting: [{ ruleTypeId: '.es-query', consumers: ['logs'] }] <-- consumer same as the feature ID in the old formatting
```

Generated action:

```
alerting:.es-query/logs/rule/get <--- consumer is set as logs and the action is the same as before
```

In both formating the actions are the same thus breaking changes are
avoided.

### Alerting authorization class
The alerting plugin uses and exports the alerting authorization class
(`AlertingAuthorization`). The class is responsible for handling all
authorization actions related to rules and alerts. The class changed to
handle the new actions as described in the above sections. A lot of
methods were renamed, removed, and cleaned up, all method arguments
converted to be an object, and the response signature of some methods
changed. These changes affected various pieces of the code. The changes
in this class are the most important in this PR especially the
`_getAuthorizedRuleTypesWithAuthorizedConsumers` method which is the
cornerstone of the alerting RBAC. Please review carefully.

### Instantiation of the alerting authorization class
The `AlertingAuthorizationClientFactory` is used to create instances of
the `AlertingAuthorization` class. The `AlertingAuthorization` class
needs to perform async operations upon instantiation. Because JS, at the
moment, does not support async instantiation of classes the
`AlertingAuthorization` class was assigning `Promise` objects to
variables that could be resolved later in other phases of the lifecycle
of the class. To improve readability and make the lifecycle of the class
clearer, I separated the construction of the class (initialization) from
the bootstrap process. As a result, getting the `AlertingAuthorization`
class or any client that depends on it (`getRulesClient` for example) is
an async operation.

### Filtering
A lot of routes use the authorization class to get the authorization
filter (`getFindAuthorizationFilter`), a filter that, if applied,
returns only the rule types and consumers the user is authorized to. The
method that returns the filter was built in a way to also support
filtering on top of the authorization filter thus coupling the
authorized filter with router filtering. I believe these two operations
should be decoupled and the filter method should return a filter that
gives you all the authorized rule types. It is the responsibility of the
consumer, router in our case, to apply extra filters on top of the
authorization filter. For that reason, I made all the necessary changes
to decouple them.

### Legacy consumers & producer
A lot of rules and alerts have been created and are still being created
from observability with the `alerts` consumer. When the Alerting RBAC
encounters a rule or alert with `alerts` as a consumer it falls back to
the `producer` of the rule type ID to construct the actions. For example
if a rule with `ruleTypeId: .es-query` and `consumer: alerts` the
alerting action will be constructed as
`alerting:.es-query/stackAlerts/rule/get` where `stackRules` is the
producer of the `.es-query` rule type. The `producer` is used to be used
in alerting authorization but due to its complexity, it was deprecated
and only used as a fallback for the `alerts` consumer. To avoid breaking
changes all feature privileges that specify access to rule types add the
`alerts` consumer when configuring their alerting privileges. By moving
the `alerts` consumer to the registration of the feature we can stop
relying on the `producer`. The `producer` is not used anymore in the
authorization class. In the next PRs the `producer` will removed
entirely.

### Routes
The following changes were introduced to the alerting routes:

- All related routes changed to be rule-type oriented and not feature ID
oriented.
- All related routes support the `ruleTypeIds` and the `consumers`
parameters for filtering. In all routes, the filters are constructed as
`ruleTypeIds: ['foo'] AND consumers: ['bar'] AND authorizationFilter`.
Filtering by consumers is important. In o11y for example, we do not want
to show ES rule types with the `stackAlerts` consumer even if the user
has access to them.
- The `/internal/rac/alerts/_feature_ids` route got deleted as it was
not used anywhere in the codebase and it was internal.

All the changes in the routes are related to internal routes and no
breaking changes are introduced.

### Constants
I moved the o11y and stack rule type IDs to `kbn-rule-data-utils` and
exported all security solution rule type IDs from
`kbn-securitysolution-rules`. I am not a fan of having a centralized
place for the rule type IDs. Ideally, consumers of the framework should
specify keywords like `observablility` (category or subcategory) or even
`apm.*` and the framework should know which rule type IDs to pick up. I
think it is out of the scope of the PR, and at the moment it seems the
most straightforward way to move forward. I will try to clean up as much
as possible in further iterations. If you are interested in the upcoming
work follow this issue elastic#187202.

### Other notable code changes
- Change all instances of feature IDs to rule type IDs.
- `isSiemRuleType`: This is a temporary helper function that is needed
in places where we handle edge cases related to security solution rule
types. Ideally, the framework should be agnostic to the rule types or
consumers. The plan is to be removed entirely in further iterations.
- Rename alerting `PluginSetupContract` and `PluginStartContract` to
`AlertingServerSetup` and `AlertingServerStart`. This made me touch a
lot of files but I could not resist.
- `filter_consumers` was mistakenly exposed to a public API. It was
undocumented.
- Files or functions that were not used anywhere in the codebase got
deleted.
- Change the returned type of the `list` method of the
`RuleTypeRegistry` from `Set<RegistryRuleType>` to `Map<string,
RegistryRuleType>`.
- Assertion of `KueryNode` in tests changed to an assertion of KQL using
`toKqlExpression`.
- Removal of `useRuleAADFields` as it is not used anywhere.

## Testing

> [!CAUTION]
> It is very important to test all the areas of the application where
rules or alerts are being used directly or indirectly. Scenarios to
consider:
> - The correct rules, alerts, and aggregations on top of them are being
shown as expected as a superuser.
> - The correct rules, alerts, and aggregations on top of them are being
shown as expected by a user with limited access to certain features.
> - The changes in this PR are backward compatible with the previous
users' permissions.

### Solutions
Please test and verify that:
- All the rule types you own with all possible combinations of
permissions both in ESS and in Serverless.
- The consumers and rule types make sense when registering the features.
- The consumers and rule types that are passed to the components are the
intended ones.

### ResponseOps
The most important changes are in the alerting authorization class, the
search strategy, and the routes. Please test:
- The rules we own with all possible combinations of permissions.
- The stack alerts page and its solution filtering.
- The categories filtering in the maintenance window UI.

## Risks
> [!WARNING]
> The risks involved in this PR are related to privileges. Specifically:
> - Users with no privileges can access rules and alerts they do not
have access to.
> - Users with privileges cannot access rules and alerts they have
access to.
>
> An excessive list of integration tests is in place to ensure that the
above scenarios will not occur. In the case of a bug, we could a)
release an energy release for serverless and b) backport the fix in ESS.
Given that this PR is intended to be merged in 8.17 we have plenty of
time to test and to minimize the chances of risks.

## FQA

- I noticed that a lot of routes support the `filter` parameter where we
can pass an arbitrary KQL filter. Why we do not use this to filter by
the rule type IDs and the consumers and instead we introduce new
dedicated parameters?

The `filter` parameter should not be exposed in the first place. It
assumes that the consumer of the API knows the underlying structure and
implementation details of the persisted storage API (SavedObject client
API). For example, a valid filter would be
`alerting.attributes.rule_type_id`. In this filter the consumer should
know a) the name of the SO b) the keyword `attributes` (storage
implementation detail) and c) the name of the attribute as it is
persisted in ES (snake case instead of camel case as it is returned by
the APIs). As there is no abstraction layer between the SO and the API,
it makes it very difficult to make changes in the persistent schema or
the APIs. For all the above I decided to introduce new query parameters
where the alerting framework has total control over it.

- I noticed in the code a lot of instances where the consumer is used.
Should not remove any logic around consumers?

This PR is a step forward making the framework as agnostic as possible.
I had to keep the scope of the PR as contained as possible. We will get
there. It needs time :).

- I noticed a lot of hacks like checking if the rule type is `siem`.
Should not remove the hacks?

This PR is a step forward making the framework as agnostic as possible.
I had to keep the scope of the PR as contained as possible. We will get
there. It needs time :).

- I hate the "Role visibility" dropdown. Can we remove it?

I also do not like it. The goal is to remove it. Follow
elastic#189997.

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Aleh Zasypkin <[email protected]>
Co-authored-by: Paula Borgonovi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams)
Projects
No open projects
5 participants