[Security Solution][Detections] Loosen lists permissions #75378

rylnd · 2020-08-18T22:40:43Z

Summary

In 7.9 we required that a user has write permissions to the lists indexes in order to use Detections. This relaxes that requirement, leaving the following behaviors:

If the lists indexes do not exist, we show a configuration page if the user cannot create them (lists manage)
If the lists indexes exist but the user cannot write to them (lists write), we disable the "Upload Value Lists" modal button

Review Steps

Setup: a kibana instance with Security Solution enabled and set up
- namely: both the alerts and lists indexes are created
Create a user with the following permissions:
Navigate to detections and verify that:
1. No "needs configuration" screen is displayed, and instead you can interact with the plugin
2. The "Upload Value Lists" modal button is disabled

Checklist

Delete any items that are not applicable to this PR.

Documentation was added for features that require explanation or tutorials

For maintainers

This was checked for breaking API changes and was labeled appropriately

We were previously showing them a configuration page, which was incorrect. The only place we require the write permission is the value lists modal, which is now hidden if that permission is absent.

If the user does not have permission to write to the lists index, they should not be able to CRUD value lists.

* Replaces useCallback functions with inline anonymous functions * Renames the modal state to be more similar to existing analogous modal state

kibanamachine · 2020-08-19T00:24:27Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: f856762

Build metrics

async chunks size

id	value	diff	baseline
securitySolution	7.2MB	-201.0B	7.2MB

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

elasticmachine · 2020-08-19T00:35:42Z

Pinging @elastic/siem (Team:SIEM)

FrankHassanabad · 2020-08-19T15:12:06Z

...s/security_solution/public/detections/containers/detection_engine/lists/use_lists_config.tsx

  const needsIndexConfiguration =
    needsIndex && (canManageIndex === false || (canManageIndex === true && hasIndexError));
-  const needsConfiguration = !enabled || canWriteIndex === false || needsIndexConfiguration;
+  const needsConfiguration = !enabled || needsIndexConfiguration;


Only suggestions I would have would be if we can have tests around this area that would be pretty nice to add to the documentation and help future maintainers.

return { canManageIndex, canWriteIndex, enabled, loading, needsConfiguration };

This looks really good in that later we can push this to the "needs permissions" and make actionable UI's to tell the user what they're missing exactly and how to fix it. So this was really nice to see here.

I wholeheartedly agree on the tests. I had been waiting to write a functional test for "read-only lists user can use Detections," but in the interim I will add some unit tests around these hooks in a followup PR 👍

FrankHassanabad

LGTM, appreciate the speed and response to this on top of all the other #1 priorities going on.

* Allow read-only lists users to use Detections if lists indexes exist We were previously showing them a configuration page, which was incorrect. The only place we require the write permission is the value lists modal, which is now hidden if that permission is absent. * Disable the Value Lists modal when the user cannot write lists If the user does not have permission to write to the lists index, they should not be able to CRUD value lists. * style: Remove unnecessary useCallbacks * Replaces useCallback functions with inline anonymous functions * Renames the modal state to be more similar to existing analogous modal state

rylnd · 2020-08-19T16:26:56Z

@benskelker This removes the write permission from the lists indexes as a requirement to use Detections.

* Allow read-only lists users to use Detections if lists indexes exist We were previously showing them a configuration page, which was incorrect. The only place we require the write permission is the value lists modal, which is now hidden if that permission is absent. * Disable the Value Lists modal when the user cannot write lists If the user does not have permission to write to the lists index, they should not be able to CRUD value lists. * style: Remove unnecessary useCallbacks * Replaces useCallback functions with inline anonymous functions * Renames the modal state to be more similar to existing analogous modal state

…5459) * Allow read-only lists users to use Detections if lists indexes exist We were previously showing them a configuration page, which was incorrect. The only place we require the write permission is the value lists modal, which is now hidden if that permission is absent. * Disable the Value Lists modal when the user cannot write lists If the user does not have permission to write to the lists index, they should not be able to CRUD value lists. * style: Remove unnecessary useCallbacks * Replaces useCallback functions with inline anonymous functions * Renames the modal state to be more similar to existing analogous modal state

…5460) * Allow read-only lists users to use Detections if lists indexes exist We were previously showing them a configuration page, which was incorrect. The only place we require the write permission is the value lists modal, which is now hidden if that permission is absent. * Disable the Value Lists modal when the user cannot write lists If the user does not have permission to write to the lists index, they should not be able to CRUD value lists. * style: Remove unnecessary useCallbacks * Replaces useCallback functions with inline anonymous functions * Renames the modal state to be more similar to existing analogous modal state

elasticmachine · 2021-09-23T14:27:05Z

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

rylnd added 3 commits August 18, 2020 12:48

Allow read-only lists users to use Detections if lists indexes exist

06e554e

We were previously showing them a configuration page, which was incorrect. The only place we require the write permission is the value lists modal, which is now hidden if that permission is absent.

Disable the Value Lists modal when the user cannot write lists

a119d84

If the user does not have permission to write to the lists index, they should not be able to CRUD value lists.

style: Remove unnecessary useCallbacks

f856762

* Replaces useCallback functions with inline anonymous functions * Renames the modal state to be more similar to existing analogous modal state

rylnd added release_note:enhancement Team:SIEM v8.0.0 v7.10.0 v7.9.1 labels Aug 18, 2020

rylnd self-assigned this Aug 18, 2020

rylnd marked this pull request as ready for review August 19, 2020 00:35

rylnd requested review from a team as code owners August 19, 2020 00:35

FrankHassanabad reviewed Aug 19, 2020

View reviewed changes

FrankHassanabad approved these changes Aug 19, 2020

View reviewed changes

rylnd merged commit 32c23ea into elastic:master Aug 19, 2020

rylnd deleted the loosen_lists_permissions branch August 19, 2020 16:25

This was referenced Aug 19, 2020

[7.x] [Security Solution][Detections] Loosen lists permissions (#75378) #75459

Merged

[7.9] [Security Solution][Detections] Loosen lists permissions (#75378) #75460

Merged

rylnd mentioned this pull request Aug 19, 2020

[Security Solution][Detections] Cleaning up mocks/tests #74920

Merged

3 tasks

spong added release_note:fix and removed release_note:enhancement labels Sep 3, 2020

MindyRS added the Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. label Sep 23, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Security Solution][Detections] Loosen lists permissions #75378

[Security Solution][Detections] Loosen lists permissions #75378

Uh oh!

rylnd commented Aug 18, 2020 •

edited

Loading

Uh oh!

kibanamachine commented Aug 19, 2020

Uh oh!

elasticmachine commented Aug 19, 2020

Uh oh!

FrankHassanabad Aug 19, 2020

Uh oh!

rylnd Aug 19, 2020

Uh oh!

FrankHassanabad left a comment

Uh oh!

rylnd commented Aug 19, 2020

Uh oh!

elasticmachine commented Sep 23, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

[Security Solution][Detections] Loosen lists permissions #75378

[Security Solution][Detections] Loosen lists permissions #75378

Uh oh!

Conversation

rylnd commented Aug 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Review Steps

Checklist

For maintainers

Uh oh!

kibanamachine commented Aug 19, 2020

💚 Build Succeeded

Build metrics

async chunks size

Uh oh!

elasticmachine commented Aug 19, 2020

Uh oh!

FrankHassanabad Aug 19, 2020

Choose a reason for hiding this comment

Uh oh!

rylnd Aug 19, 2020

Choose a reason for hiding this comment

Uh oh!

FrankHassanabad left a comment

Choose a reason for hiding this comment

Uh oh!

rylnd commented Aug 19, 2020

Uh oh!

elasticmachine commented Sep 23, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

rylnd commented Aug 18, 2020 •

edited

Loading