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.

Sign up for GitHub

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

Jump to bottom

[Security Solution][Exceptions] Prevents value list entries from co-existing with non value list entries #72995

Merged
merged 4 commits into from
Jul 24, 2020
Merged

[Security Solution][Exceptions] Prevents value list entries from co-existing with non value list entries #72995

merged 4 commits into from
Jul 24, 2020

Conversation

spong
Copy link
Member

@spong spong commented Jul 23, 2020 •
edited
Loading

Summary

Fixes validation issue where value list exception entries could be added alongside non value list exception entries.

Once a value list operator (is in list or is not in list) is selected the nested button will disable, and subsequent and/ or's will only have the value list operators available to them:

If a value list is not selected in the first exception entry, all subsequent entries will no longer have the value list operators:

Adds validation for empty case to prevent network error when submitted no entries.
Add modal:

Edit modal:

Checklist

Delete any items that are not applicable to this PR.

@spong spong added bug Fixes for quality problems that affect the customer experience Team:SIEM v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.10.0 v7.9.0 labels Jul 23, 2020
@spong spong requested review from a team as code owners July 23, 2020 03:54
@spong spong self-assigned this Jul 23, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/siem (Team:SIEM)

@dhurley14
Copy link
Contributor

dhurley14 commented Jul 23, 2020 •
edited
Loading

@spong thanks for taking care of this!

We do support or/and for value list entries. This PR #72304 adds that functionality. That PR covers the following cases:

  1. where two exception items, each with a value list entry, are or'd together
  2. where one exception item has two entries, both containing value lists, which are anded together.

So I would say we could relax the restrictions for this PR to only preventing "anding" of value list exception entries with other types of exception entries.

yctercero
yctercero reviewed Jul 23, 2020
View reviewed changes
Copy link
Contributor

@yctercero yctercero left a comment

Choose a reason for hiding this comment

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

This looks great! Thank you, thank you for taking this one. Good to LGTM with the updated loosened restrictions.

yctercero
yctercero approved these changes Jul 23, 2020
View reviewed changes
Copy link
Contributor

@yctercero yctercero left a comment

Choose a reason for hiding this comment

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

LATM (looks amazing to me) - thank you so so much for the fixes, this makes the user experience so much better!

...plugins/security_solution/public/common/components/exceptions/edit_exception_modal/index.tsx
@@ -146,6 +146,11 @@ export const EditExceptionModal = memo(function EditExceptionModal({
}
}, [shouldDisableBulkClose]);

const isSubmitButtonDisabled = useMemo(
Copy link
Contributor

Choose a reason for hiding this comment

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

Ahhh, nice 😉 I was gonna say maybe we need to return the errors from the builder, but really the only errors would be empty values or invalid values that would show with a red error state in the inputs.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

async chunks size

id value diff baseline
securitySolution 7.3MB +1.6KB 7.3MB

History

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

@spong spong merged commit 7f36bd7 into elastic:master Jul 24, 2020
@spong spong deleted the large-lists-and-validation branch July 24, 2020 21:50
@spong spong mentioned this pull request Jul 24, 2020
Merged
spong added a commit to spong/kibana that referenced this pull request Jul 24, 2020
@spong
[Security Solution][Exceptions] Prevents value list entries from co-e…
07369f6 
…xisting with non value list entries (elastic#72995)

## Summary

Fixes validation issue where value list exception entries could be added alongside non value list exception entries.


Once a value list operator (`is in list` or `is not in list`) is selected the `nested` button will disable, and subsequent `and`/ `or`'s will only have the value list operators available to them:

<p align="center">
  <img width="400" src="https://user-images.githubusercontent.com/2946766/88323026-d36a8180-ccde-11ea-97f0-16490bf9a671.gif" />
</p>


If a value list is not selected in the first exception entry, all subsequent entries will no longer have the value list operators:
<p align="center">
  <img width="400" src="https://user-images.githubusercontent.com/2946766/88250505-08d28900-cc65-11ea-9a6c-5b869e3d7f7f.gif" />
</p>


Adds validation for empty case to prevent network error when submitted no entries.
Add modal:
<p align="center">
  <img width="400" src="https://user-images.githubusercontent.com/2946766/88299601-c5593880-ccbf-11ea-9468-30796a0f9748.gif" />
</p>

Edit modal:
<p align="center">
  <img width="400" src="https://user-images.githubusercontent.com/2946766/88299609-c7bb9280-ccbf-11ea-943c-836b641d7ae3.gif" />
</p>




### Checklist

Delete any items that are not applicable to this PR.

- [x] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios
@spong spong mentioned this pull request Jul 24, 2020
Merged
spong added a commit to spong/kibana that referenced this pull request Jul 24, 2020
@spong
[Security Solution][Exceptions] Prevents value list entries from co-e…
c6f7b0f 
…xisting with non value list entries (elastic#72995)

## Summary

Fixes validation issue where value list exception entries could be added alongside non value list exception entries.


Once a value list operator (`is in list` or `is not in list`) is selected the `nested` button will disable, and subsequent `and`/ `or`'s will only have the value list operators available to them:

<p align="center">
  <img width="400" src="https://user-images.githubusercontent.com/2946766/88323026-d36a8180-ccde-11ea-97f0-16490bf9a671.gif" />
</p>


If a value list is not selected in the first exception entry, all subsequent entries will no longer have the value list operators:
<p align="center">
  <img width="400" src="https://user-images.githubusercontent.com/2946766/88250505-08d28900-cc65-11ea-9a6c-5b869e3d7f7f.gif" />
</p>


Adds validation for empty case to prevent network error when submitted no entries.
Add modal:
<p align="center">
  <img width="400" src="https://user-images.githubusercontent.com/2946766/88299601-c5593880-ccbf-11ea-9468-30796a0f9748.gif" />
</p>

Edit modal:
<p align="center">
  <img width="400" src="https://user-images.githubusercontent.com/2946766/88299609-c7bb9280-ccbf-11ea-943c-836b641d7ae3.gif" />
</p>




### Checklist

Delete any items that are not applicable to this PR.

- [x] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios
spong added a commit that referenced this pull request Jul 25, 2020
@spong
[Security Solution][Exceptions] Prevents value list entries from co-e…
559bd87 
…xisting with non value list entries (#72995) (#73217)

## Summary

Fixes validation issue where value list exception entries could be added alongside non value list exception entries.


Once a value list operator (`is in list` or `is not in list`) is selected the `nested` button will disable, and subsequent `and`/ `or`'s will only have the value list operators available to them:

<p align="center">
  <img width="400" src="https://user-images.githubusercontent.com/2946766/88323026-d36a8180-ccde-11ea-97f0-16490bf9a671.gif" />
</p>


If a value list is not selected in the first exception entry, all subsequent entries will no longer have the value list operators:
<p align="center">
  <img width="400" src="https://user-images.githubusercontent.com/2946766/88250505-08d28900-cc65-11ea-9a6c-5b869e3d7f7f.gif" />
</p>


Adds validation for empty case to prevent network error when submitted no entries.
Add modal:
<p align="center">
  <img width="400" src="https://user-images.githubusercontent.com/2946766/88299601-c5593880-ccbf-11ea-9468-30796a0f9748.gif" />
</p>

Edit modal:
<p align="center">
  <img width="400" src="https://user-images.githubusercontent.com/2946766/88299609-c7bb9280-ccbf-11ea-943c-836b641d7ae3.gif" />
</p>




### Checklist

Delete any items that are not applicable to this PR.

- [x] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios
spong added a commit that referenced this pull request Jul 25, 2020
@spong
[Security Solution][Exceptions] Prevents value list entries from co-e…
1cef6cf 
…xisting with non value list entries (#72995) (#73216)

## Summary

Fixes validation issue where value list exception entries could be added alongside non value list exception entries.


Once a value list operator (`is in list` or `is not in list`) is selected the `nested` button will disable, and subsequent `and`/ `or`'s will only have the value list operators available to them:

<p align="center">
  <img width="400" src="https://user-images.githubusercontent.com/2946766/88323026-d36a8180-ccde-11ea-97f0-16490bf9a671.gif" />
</p>


If a value list is not selected in the first exception entry, all subsequent entries will no longer have the value list operators:
<p align="center">
  <img width="400" src="https://user-images.githubusercontent.com/2946766/88250505-08d28900-cc65-11ea-9a6c-5b869e3d7f7f.gif" />
</p>


Adds validation for empty case to prevent network error when submitted no entries.
Add modal:
<p align="center">
  <img width="400" src="https://user-images.githubusercontent.com/2946766/88299601-c5593880-ccbf-11ea-9468-30796a0f9748.gif" />
</p>

Edit modal:
<p align="center">
  <img width="400" src="https://user-images.githubusercontent.com/2946766/88299609-c7bb9280-ccbf-11ea-943c-836b641d7ae3.gif" />
</p>




### Checklist

Delete any items that are not applicable to this PR.

- [x] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jul 27, 2020
@gmmorris
Merge branch 'master' into event-log/use-alerts-actions-clients
2d63a67 
* master: (111 commits)
  Remove flaky note from gauge tests (elastic#73240)
  Convert functional vega tests to ts and unskip tests (elastic#72238)
  [Graph] Unskip graph tests (elastic#72291)
  Add default Elasticsearch credentials to docs (elastic#72617)
  [APM] Read body from indicesStats in upload-telemetry-data (elastic#72732)
  The directory in the command was missing the /generated directory and would cause all definitions to be regenerated in the wrong place. (elastic#72766)
  [KP] use new ES client in SO service (elastic#72289)
  [Security Solution][Exceptions] Prevents value list entries from co-existing with non value list entries (elastic#72995)
  Return EUI CSS to Shareable Runtime (elastic#72990)
  Removed useless karma test (elastic#73190)
  [INGEST_MANAGER] Make package config name blank for endpoint on Package Config create (elastic#73082)
  [Ingest Manager] Support DEGRADED state in fleet agent event (elastic#73104)
  [Security Solution][Detections] Change detections breadcrumb title (elastic#73059)
  [ML] Fixing unnecessary deleting job polling (elastic#73087)
  [ML] Fixing recognizer wizard create job button (elastic#73025)
  [Composable template] Preview composite template (elastic#72598)
  [Uptime] Use manual intervals for ping histogram (elastic#72928)
  [Security Solution][Endpoint] Task/policy save modal text change, remove duplicate policy details text (elastic#73130)
  [Maps] fix tile layer attibution text and attribution link validation errors (elastic#73160)
  skip ingest pipeline api tests
  ...
@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
Reviewers

@yctercero yctercero yctercero approved these changes

@peluja1012 peluja1012 Awaiting requested review from peluja1012

@dhurley14 dhurley14 Awaiting requested review from dhurley14

Assignees

@spong spong

Labels
bug Fixes for quality problems that affect the customer experience 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.9.0 v7.10.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@spong @elasticmachine @dhurley14 @kibanamachine @yctercero @MindyRS