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

[SIEM][Detections] Prevent removal of actions via the UI from breaking rule AAD #68184

Merged
merged 2 commits into from
Jun 4, 2020

Conversation

rylnd
Copy link
Contributor

@rylnd rylnd commented Jun 3, 2020

Summary

This fixes #64870 for realsies.

The issue ended up being caused by a conditional form field that mapped to a nested field on the rule's params: when a rule is created with an action, it has a meta.kibana_siem_app_url field. When the rule's actions were removed via the UI, that form field was also removed, which broke AAD and thus rule execution.

This fixes the issue by making that field unconditional, and also removes the previous workaround.

Testing this requires interacting with both the UI and task manager. I have not written an automated regression test, and I don't believe our cypress tests currently exercise rule execution. However, we've also created #68179 to discuss the AAD issue in general and how best to protect API users from encountering it. Adding a (currently failing) integration test there seems most appropriate.

For maintainers

This fixes elastic#64870 _for real_.

The issue ended up being caused by a
conditional form field that mapped to a nested field on the rule's
params: when a rule is created with an action, it has a
meta.kibana_siem_app_url field. When the rule's actions were removed via
the UI, that field was _also_ removed, which broke AAD and thus rule
execution.

This fixes the issue by making that field unconditional, and also
removes the previous workaround.
@rylnd rylnd added Team:SIEM v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.8.0 v7.9.0 v7.7.2 labels Jun 3, 2020
@rylnd rylnd self-assigned this Jun 3, 2020
Copy link
Contributor

@FrankHassanabad FrankHassanabad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@rylnd rylnd marked this pull request as ready for review June 4, 2020 16:07
@elasticmachine
Copy link
Contributor

Pinging @elastic/siem (Team:SIEM)

@rylnd rylnd requested review from a team as code owners June 4, 2020 16:32
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@rylnd rylnd merged commit 5fd6416 into elastic:master Jun 4, 2020
@rylnd rylnd deleted the fix_action_editing_bug branch June 4, 2020 18:38
rylnd added a commit to rylnd/kibana that referenced this pull request Jun 4, 2020
…#68184)

This fixes elastic#64870 _for real_.

The issue ended up being caused by a
conditional form field that mapped to a nested field on the rule's
params: when a rule is created with an action, it has a
meta.kibana_siem_app_url field. When the rule's actions were removed via
the UI, that field was _also_ removed, which broke AAD and thus rule
execution.

This fixes the issue by making that field unconditional, and also
removes the previous workaround.
# Conflicts:
#	x-pack/plugins/security_solution/public/alerts/components/rules/step_rule_actions/index.tsx
#	x-pack/plugins/security_solution/server/lib/detection_engine/rules/update_rules_notifications.ts
rylnd added a commit that referenced this pull request Jun 5, 2020
…#68300)

This fixes #64870 _for real_.

The issue ended up being caused by a
conditional form field that mapped to a nested field on the rule's
params: when a rule is created with an action, it has a
meta.kibana_siem_app_url field. When the rule's actions were removed via
the UI, that field was _also_ removed, which broke AAD and thus rule
execution.

This fixes the issue by making that field unconditional, and also
removes the previous workaround.
# Conflicts:
#	x-pack/plugins/security_solution/public/alerts/components/rules/step_rule_actions/index.tsx
#	x-pack/plugins/security_solution/server/lib/detection_engine/rules/update_rules_notifications.ts
rylnd added a commit that referenced this pull request Jun 5, 2020
…#68299)

This fixes #64870 _for real_.

The issue ended up being caused by a
conditional form field that mapped to a nested field on the rule's
params: when a rule is created with an action, it has a
meta.kibana_siem_app_url field. When the rule's actions were removed via
the UI, that field was _also_ removed, which broke AAD and thus rule
execution.

This fixes the issue by making that field unconditional, and also
removes the previous workaround.
rylnd added a commit that referenced this pull request Jun 9, 2020
…#68681)

This fixes #64870 _for real_.

The issue ended up being caused by a
conditional form field that mapped to a nested field on the rule's
params: when a rule is created with an action, it has a
meta.kibana_siem_app_url field. When the rule's actions were removed via
the UI, that field was _also_ removed, which broke AAD and thus rule
execution.

This fixes the issue by making that field unconditional, and also
removes the previous workaround.
@MindyRS MindyRS added the Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. label Sep 23, 2021
@elasticmachine
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:SIEM v7.7.2 v7.8.0 v7.9.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[SIEM] Removing a Rule's Action breaks Rule execution
5 participants