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

[Security Solution] Validation is not triggered correctly in rule actions form #142217

Open
vitaliidm opened this issue Sep 29, 2022 · 2 comments
Labels
bug Fixes for quality problems that affect the customer experience Feature:Alerting/RuleActions Issues related to the Actions attached to Rules on the Alerting Framework Feature:Rule Actions Security Solution Rule Actions feature Feature:Rule Creation Security Solution Detection Rule Creation impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. Team:Detection Engine Security Solution Detection Engine Area Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. technical debt Improvement of the software architecture and operational architecture

Comments

@vitaliidm
Copy link
Contributor

vitaliidm commented Sep 29, 2022

Steps to reproduce

Issue was found, once working on #140593.
Rule actions form is marked as invalid, even though all fields are populated in slack action form

Screen.Recording.2022-09-26.at.12.36.53.mov
  1. open rule actions form
  2. add slack action
  3. remove slack action
  4. add again slack action
  5. press Save rule
  6. validation error is displayed

Technical overview

Once slack rule actions form is rendered:
First setActionIdByIndex in x-pack/plugins/security_solution/public/detections/components/rules/rule_actions_field/index.tsx fires update to actions field of form (but w/o params properties, only type of selector in action).
Then, subsequently setActionParamsProperty updates params in field actions of form with values displayed if form controls through calling field.setValue. In our case, it is a message form field in slack action form.

These updates trigger form validation, defined in schema:

  actions: {
    validations: [
      {
        validator: validateRuleActionsField(actionTypeRegistry),
      },
    ],
  },

validateRuleActionsField receives in curried function argument field value:

    const [{ value, path }] = data as [{ value: RuleAction[]; path: string }];

which doesn't have updates set in setActionParamsProperty, hence form is marked as invalid. It happens consistently when you add some action, remove it, add again it.

Here is logged form value of actions in functions calls

setActionIdByIndex [
  {
    "id": "50a49d70-34d4-11ed-87dc-f3f665e2eedb",
    "actionTypeId": ".slack",
    "group": "default",
    "params": {}
  }
]
validateRuleActionsField [
  {
    "id": "50a49d70-34d4-11ed-87dc-f3f665e2eedb",
    "actionTypeId": ".slack",
    "group": "default",
    "params": {}
  }
]
validateRuleActionsField [
  {
    "id": "50a49d70-34d4-11ed-87dc-f3f665e2eedb",
    "actionTypeId": ".slack",
    "group": "default",
    "params": {}
  }
]
setActionParamsProperty [
  {
    "id": "50a49d70-34d4-11ed-87dc-f3f665e2eedb",
    "actionTypeId": ".slack",
    "group": "default",
    "params": {
      "message": "Rule {{context.rule.name}} generated {{state.signals_count}} alerts"
    }
  }
]

From the log above: validateRuleActionsField is not getting called after setActionParamsProperty is called, instead validateRuleActionsField called twice after setActionIdByIndex has been triggered.

Expected behaviour

Validation to be triggered once new params values were updated through setActionParamsProperty.

Workaround

@vitaliidm vitaliidm added bug Fixes for quality problems that affect the customer experience triage_needed Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Detection Rule Management Security Detection Rule Management Team labels Sep 29, 2022
@elasticmachine
Copy link
Contributor

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

@vitaliidm vitaliidm added the technical debt Improvement of the software architecture and operational architecture label Sep 29, 2022
@banderror banderror added impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. Team:Detections and Resp Security Detection Response Team and removed triage_needed labels Sep 30, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detections-response (Team:Detections and Resp)

@banderror banderror added Feature:Rule Actions Security Solution Rule Actions feature Feature:Alerting/RuleActions Issues related to the Actions attached to Rules on the Alerting Framework labels Sep 30, 2022
@banderror banderror added Team:Detection Alerts Security Detection Alerts Area Team Feature:Rule Creation Security Solution Detection Rule Creation and removed Team:Detection Rule Management Security Detection Rule Management Team labels Oct 25, 2022
@banderror banderror changed the title [Security Solution][Detections] Validation is not triggered correctly in rule actions form [Security Solution] Validation is not triggered correctly in rule actions form Oct 25, 2022
spong added a commit that referenced this issue Feb 16, 2023
…ng Rule Action Message (#150823)

## Summary

Resolves: #149885

For additional details, please see
#141811 (comment) and
#142217.

As mentioned in #142217, this
issue is the result of managing stale events and timings (renderings +
field updates) between the Detections `RuleActionsField` component,
validation within the hooks-form lib, and field updates coming from the
`trigger_actions_ui` components.

Note: this behavior is explicit to the Edit Rule flow (`ActionsStepRule`
form), and not the Bulk Actions flyout (`RuleActionsFormData` form) as
events flow as intended in the latter, presumably because of all the
nested components and use of `useFormData` within the Edit Rule flow
(see [form libs
docs](https://docs.elastic.dev/form-lib/examples/listening-to-changes#forward-the-form-state-to-a-parent-component)).

The fix here is a modification of the fix provided in
#141811 with `setTimeout`, but
instead of always pushing the params update to be within a `setTimeout`,
it now only does it when initially loading `Actions` with empty
`Params`. Since this fix also has the unintended side-effect of
flickering the validation error callout when first adding an action,
validation is now throttled to 100ms intervals, which also helps with
extraneous re-renders.

Before initial fix (with cursor issue) / Before throttle fix w/ flashing
error callout:
<p align="center">
<img width="350"
src="https://user-images.githubusercontent.com/2946766/215585424-8c056440-5906-4168-b997-aea3690c7ea5.gif"
/> <img width="242"
src="https://user-images.githubusercontent.com/2946766/217961284-704013b7-2f0a-4a24-95a4-4183348448f4.gif"
/>
</p>

After:
<p align="center">
<img width="242"
src="https://user-images.githubusercontent.com/2946766/218591986-e52456e0-e374-4d8e-bc84-9fb6da69f1f9.gif"
/>
</p>
kibanamachine pushed a commit to kibanamachine/kibana that referenced this issue Feb 16, 2023
…ng Rule Action Message (elastic#150823)

## Summary

Resolves: elastic#149885

For additional details, please see
elastic#141811 (comment) and
elastic#142217.

As mentioned in elastic#142217, this
issue is the result of managing stale events and timings (renderings +
field updates) between the Detections `RuleActionsField` component,
validation within the hooks-form lib, and field updates coming from the
`trigger_actions_ui` components.

Note: this behavior is explicit to the Edit Rule flow (`ActionsStepRule`
form), and not the Bulk Actions flyout (`RuleActionsFormData` form) as
events flow as intended in the latter, presumably because of all the
nested components and use of `useFormData` within the Edit Rule flow
(see [form libs
docs](https://docs.elastic.dev/form-lib/examples/listening-to-changes#forward-the-form-state-to-a-parent-component)).

The fix here is a modification of the fix provided in
elastic#141811 with `setTimeout`, but
instead of always pushing the params update to be within a `setTimeout`,
it now only does it when initially loading `Actions` with empty
`Params`. Since this fix also has the unintended side-effect of
flickering the validation error callout when first adding an action,
validation is now throttled to 100ms intervals, which also helps with
extraneous re-renders.

Before initial fix (with cursor issue) / Before throttle fix w/ flashing
error callout:
<p align="center">
<img width="350"
src="https://user-images.githubusercontent.com/2946766/215585424-8c056440-5906-4168-b997-aea3690c7ea5.gif"
/> <img width="242"
src="https://user-images.githubusercontent.com/2946766/217961284-704013b7-2f0a-4a24-95a4-4183348448f4.gif"
/>
</p>

After:
<p align="center">
<img width="242"
src="https://user-images.githubusercontent.com/2946766/218591986-e52456e0-e374-4d8e-bc84-9fb6da69f1f9.gif"
/>
</p>

(cherry picked from commit 736759a)
kibanamachine added a commit that referenced this issue Feb 16, 2023
… editing Rule Action Message (#150823) (#151518)

# Backport

This will backport the following commits from `main` to `8.7`:
- [[Security Solution] Fix cursor jumping to end of text area when
editing Rule Action Message
(#150823)](#150823)

<!--- Backport version: 8.9.7 -->

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

<!--BACKPORT [{"author":{"name":"Garrett
Spong","email":"spong@users.noreply.github.com"},"sourceCommit":{"committedDate":"2023-02-16T21:05:26Z","message":"[Security
Solution] Fix cursor jumping to end of text area when editing Rule
Action Message (#150823)\n\n## Summary\r\n\r\nResolves:
#149885 additional
details, please
see\r\nhttps://github.com//pull/141811#discussion_r981346237
and\r\nhttps://github.com//issues/142217.\r\n\r\nAs
mentioned in #142217,
this\r\nissue is the result of managing stale events and timings
(renderings +\r\nfield updates) between the Detections
`RuleActionsField` component,\r\nvalidation within the hooks-form lib,
and field updates coming from the\r\n`trigger_actions_ui`
components.\r\n\r\nNote: this behavior is explicit to the Edit Rule flow
(`ActionsStepRule`\r\nform), and not the Bulk Actions flyout
(`RuleActionsFormData` form) as\r\nevents flow as intended in the
latter, presumably because of all the\r\nnested components and use of
`useFormData` within the Edit Rule flow\r\n(see [form
libs\r\ndocs](https://docs.elastic.dev/form-lib/examples/listening-to-changes#forward-the-form-state-to-a-parent-component)).\r\n\r\nThe
fix here is a modification of the fix provided
in\r\nhttps://github.com//pull/141811 with `setTimeout`,
but\r\ninstead of always pushing the params update to be within a
`setTimeout`,\r\nit now only does it when initially loading `Actions`
with empty\r\n`Params`. Since this fix also has the unintended
side-effect of\r\nflickering the validation error callout when first
adding an action,\r\nvalidation is now throttled to 100ms intervals,
which also helps with\r\nextraneous re-renders.\r\n\r\nBefore initial
fix (with cursor issue) / Before throttle fix w/ flashing\r\nerror
callout:\r\n<p align=\"center\">\r\n<img
width=\"350\"\r\nsrc=\"https://user-images.githubusercontent.com/2946766/215585424-8c056440-5906-4168-b997-aea3690c7ea5.gif\"\r\n/>
<img
width=\"242\"\r\nsrc=\"https://user-images.githubusercontent.com/2946766/217961284-704013b7-2f0a-4a24-95a4-4183348448f4.gif\"\r\n/>\r\n</p>\r\n\r\nAfter:\r\n<p
align=\"center\">\r\n<img
width=\"242\"\r\nsrc=\"https://user-images.githubusercontent.com/2946766/218591986-e52456e0-e374-4d8e-bc84-9fb6da69f1f9.gif\"\r\n/>\r\n</p>","sha":"736759af76623966684e6dbb06009f1517a8d8a8","branchLabelMapping":{"^v8.8.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:fix","Team:Detections
and Resp","Team: SecuritySolution","Feature:Rule
Actions","Team:Detection
Rules","v8.7.0","v8.8.0"],"number":150823,"url":"#150823
Solution] Fix cursor jumping to end of text area when editing Rule
Action Message (#150823)\n\n## Summary\r\n\r\nResolves:
#149885 additional
details, please
see\r\nhttps://github.com//pull/141811#discussion_r981346237
and\r\nhttps://github.com//issues/142217.\r\n\r\nAs
mentioned in #142217,
this\r\nissue is the result of managing stale events and timings
(renderings +\r\nfield updates) between the Detections
`RuleActionsField` component,\r\nvalidation within the hooks-form lib,
and field updates coming from the\r\n`trigger_actions_ui`
components.\r\n\r\nNote: this behavior is explicit to the Edit Rule flow
(`ActionsStepRule`\r\nform), and not the Bulk Actions flyout
(`RuleActionsFormData` form) as\r\nevents flow as intended in the
latter, presumably because of all the\r\nnested components and use of
`useFormData` within the Edit Rule flow\r\n(see [form
libs\r\ndocs](https://docs.elastic.dev/form-lib/examples/listening-to-changes#forward-the-form-state-to-a-parent-component)).\r\n\r\nThe
fix here is a modification of the fix provided
in\r\nhttps://github.com//pull/141811 with `setTimeout`,
but\r\ninstead of always pushing the params update to be within a
`setTimeout`,\r\nit now only does it when initially loading `Actions`
with empty\r\n`Params`. Since this fix also has the unintended
side-effect of\r\nflickering the validation error callout when first
adding an action,\r\nvalidation is now throttled to 100ms intervals,
which also helps with\r\nextraneous re-renders.\r\n\r\nBefore initial
fix (with cursor issue) / Before throttle fix w/ flashing\r\nerror
callout:\r\n<p align=\"center\">\r\n<img
width=\"350\"\r\nsrc=\"https://user-images.githubusercontent.com/2946766/215585424-8c056440-5906-4168-b997-aea3690c7ea5.gif\"\r\n/>
<img
width=\"242\"\r\nsrc=\"https://user-images.githubusercontent.com/2946766/217961284-704013b7-2f0a-4a24-95a4-4183348448f4.gif\"\r\n/>\r\n</p>\r\n\r\nAfter:\r\n<p
align=\"center\">\r\n<img
width=\"242\"\r\nsrc=\"https://user-images.githubusercontent.com/2946766/218591986-e52456e0-e374-4d8e-bc84-9fb6da69f1f9.gif\"\r\n/>\r\n</p>","sha":"736759af76623966684e6dbb06009f1517a8d8a8"}},"sourceBranch":"main","suggestedTargetBranches":["8.7"],"targetPullRequestStates":[{"branch":"8.7","label":"v8.7.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.8.0","labelRegex":"^v8.8.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/150823","number":150823,"mergeCommit":{"message":"[Security
Solution] Fix cursor jumping to end of text area when editing Rule
Action Message (#150823)\n\n## Summary\r\n\r\nResolves:
#149885 additional
details, please
see\r\nhttps://github.com//pull/141811#discussion_r981346237
and\r\nhttps://github.com//issues/142217.\r\n\r\nAs
mentioned in #142217,
this\r\nissue is the result of managing stale events and timings
(renderings +\r\nfield updates) between the Detections
`RuleActionsField` component,\r\nvalidation within the hooks-form lib,
and field updates coming from the\r\n`trigger_actions_ui`
components.\r\n\r\nNote: this behavior is explicit to the Edit Rule flow
(`ActionsStepRule`\r\nform), and not the Bulk Actions flyout
(`RuleActionsFormData` form) as\r\nevents flow as intended in the
latter, presumably because of all the\r\nnested components and use of
`useFormData` within the Edit Rule flow\r\n(see [form
libs\r\ndocs](https://docs.elastic.dev/form-lib/examples/listening-to-changes#forward-the-form-state-to-a-parent-component)).\r\n\r\nThe
fix here is a modification of the fix provided
in\r\nhttps://github.com//pull/141811 with `setTimeout`,
but\r\ninstead of always pushing the params update to be within a
`setTimeout`,\r\nit now only does it when initially loading `Actions`
with empty\r\n`Params`. Since this fix also has the unintended
side-effect of\r\nflickering the validation error callout when first
adding an action,\r\nvalidation is now throttled to 100ms intervals,
which also helps with\r\nextraneous re-renders.\r\n\r\nBefore initial
fix (with cursor issue) / Before throttle fix w/ flashing\r\nerror
callout:\r\n<p align=\"center\">\r\n<img
width=\"350\"\r\nsrc=\"https://user-images.githubusercontent.com/2946766/215585424-8c056440-5906-4168-b997-aea3690c7ea5.gif\"\r\n/>
<img
width=\"242\"\r\nsrc=\"https://user-images.githubusercontent.com/2946766/217961284-704013b7-2f0a-4a24-95a4-4183348448f4.gif\"\r\n/>\r\n</p>\r\n\r\nAfter:\r\n<p
align=\"center\">\r\n<img
width=\"242\"\r\nsrc=\"https://user-images.githubusercontent.com/2946766/218591986-e52456e0-e374-4d8e-bc84-9fb6da69f1f9.gif\"\r\n/>\r\n</p>","sha":"736759af76623966684e6dbb06009f1517a8d8a8"}}]}]
BACKPORT-->

Co-authored-by: Garrett Spong <spong@users.noreply.github.com>
@yctercero yctercero added Team:Detection Engine Security Solution Detection Engine Area and removed Team:Detection Alerts Security Detection Alerts Area Team labels May 13, 2023
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 Feature:Alerting/RuleActions Issues related to the Actions attached to Rules on the Alerting Framework Feature:Rule Actions Security Solution Rule Actions feature Feature:Rule Creation Security Solution Detection Rule Creation impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. Team:Detection Engine Security Solution Detection Engine Area Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. technical debt Improvement of the software architecture and operational architecture
Projects
None yet
Development

No branches or pull requests

4 participants