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] [Detections] Adds scripts to create users + roles based on specific privileges #81866

Merged
merged 32 commits into from
Nov 19, 2020

Conversation

dhurley14
Copy link
Contributor

@dhurley14 dhurley14 commented Oct 27, 2020

Summary

As we add acute restrictions based on privileges to external plugins we need a quick, reproducible process to ensure these privilege checks and the UI restrictions put in place do not change unexpectedly. I have added scripts which will create a role + user to mimic the privileges described in the below chart. The originating spreadsheet contains descriptions of document-level security which has not been implemented yet and is not represented in the below recreation of that chart. This should get us pretty far though. Also included in this PR is a sample cypress test to provide guidance on how these scripts can be utilized to perform automated testing of privilege checks against certain features in Detections and across the Security Solution.

Role Data Sources SIEM ML Jobs/Results Lists Rules/Exceptions Action Connectors Signals/Alerts
T1 Analyst read read none read read read, write
T2 Analyst read read read read read read, write
Hunter / T3 Analyst read, write read read read, write read read, write
Rule Author / Manager / Detections Engineer read, write read read, write read, write read read, write, view_index_metadata
SOC Manager read, write read read, write read, write all read, write, manage
Platform Engineer (data ingest, cluster ops) read, write all all read, write all all

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@dhurley14 dhurley14 self-assigned this Oct 27, 2020
@dhurley14 dhurley14 force-pushed the detections-roles-users-scripts branch 3 times, most recently from 75b148f to ff483e2 Compare November 10, 2020 21:29
@dhurley14 dhurley14 added Feature:Detection Rules Anything related to Security Solution's Detection Rules release_note:skip Skip the PR/issue when compiling release notes review Team:Detections and Resp Security Detection Response Team v7.11.0 v8.0.0 labels Nov 10, 2020
@dhurley14 dhurley14 marked this pull request as ready for review November 10, 2020 23:24
@dhurley14 dhurley14 requested review from a team as code owners November 10, 2020 23:24
@dhurley14 dhurley14 force-pushed the detections-roles-users-scripts branch from 2d3c176 to d979011 Compare November 11, 2020 19:22
@dhurley14
Copy link
Contributor Author

@elasticmachine merge upstream

@dhurley14 dhurley14 force-pushed the detections-roles-users-scripts branch 2 times, most recently from e66e3a8 to 365ddea Compare November 12, 2020 00:03
Copy link
Member

@spong spong left a comment

Choose a reason for hiding this comment

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

Checked out, tested scripts locally and all is awesome! 😀

Thanks so much for taking the time to get this foundation in place @dhurley14! This is huge for expanding our test coverage and improving our confidence in testing features with less permissive users/roles. Really appreciate the additional readme's and sample cypress test/helpers to get people using these scripts in their tests too.

I did leave a few nits you'll probably want to address, but overall this LGTM so approving! 👍 🚀

I am curious what we can do to leverage these from within the x-pack/test/detection_engine_api_integration FTR tests -- any thoughts on how we could wire these up with supertest, or at least during test environment setup?

@dhurley14 dhurley14 force-pushed the detections-roles-users-scripts branch from 365ddea to 7c0c02a Compare November 16, 2020 15:00
@elastic elastic deleted a comment from dhurley14 Nov 17, 2020
@dhurley14 dhurley14 force-pushed the detections-roles-users-scripts branch from ce3a417 to 57d62f5 Compare November 17, 2020 21:30
@dhurley14 dhurley14 force-pushed the detections-roles-users-scripts branch from aac9642 to ed09038 Compare November 18, 2020 15:05
@dhurley14
Copy link
Contributor Author

@elasticmachine merge upstream

@dhurley14 dhurley14 force-pushed the detections-roles-users-scripts branch from d718ba8 to ed09038 Compare November 18, 2020 18:40
@dhurley14
Copy link
Contributor Author

@elasticmachine merge upstream

@@ -157,6 +161,79 @@ export default ({ getService }: FtrProviderContext) => {
);
expect(everySignalClosed).to.eql(true);
});

it('should NOT be able to close signals with t1 analyst user', async () => {
Copy link
Member

Choose a reason for hiding this comment

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

Looking good! Thanks for adding these functional tests -- I think this gives everyone what they need to test all the different permission permutations when building out new features. This'll go a long way @dhurley14! 🙂

@XavierM
Copy link
Contributor

XavierM commented Nov 19, 2020

@elasticmachine merge upstream

@dhurley14 dhurley14 force-pushed the detections-roles-users-scripts branch from 0388de3 to ab2c851 Compare November 19, 2020 15:27
@dhurley14
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Distributable file count

id before after diff
default 42901 42944 +43

History

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

@dhurley14 dhurley14 merged commit b3c334a into elastic:master Nov 19, 2020
@dhurley14 dhurley14 deleted the detections-roles-users-scripts branch November 19, 2020 21:02
dhurley14 added a commit to dhurley14/kibana that referenced this pull request Nov 19, 2020
… based on specific privileges (elastic#81866)

* shell scripts for creating roles + users for testing

* update readme's and updated privilege requirements based on testing with the users and inferring what the roles are supposed to do

* update role privileges based on feedback meeting yesterday

* updated scripts to accept filepath to role / user, added a test to ensure upload value list button is disabled

* updated role scripts to be parameterized

* adds login with role function and adds a sample test with a role to test that a t1 analyst user cannot upload a value list

* add object with corresponding roles

* fix spacing

* parameterize urls for basic auth with roles + users

* forgot to change the cy.visit string

* add KIBANA_URL env var for cli runner

* add env vars for curl script execution

* second script

* update readme's for each role and remove create_index from lists privilege for the soc manager role

* remove 'manage' cluster privilege for rule author

* remove 'create_index' privilege from soc_manager role since that is not parity with the security workflows spreadsheet

* update the login function logic with glo's feedback

* replace SIEM with Security Solution in markdown files

* make role param optional not just undefined

* remove unused file

* add copyright to scripts files

* update top-level README for roles scripts

* remove reference to internal spreadsheet and reference readme for this pr

* remove unnecessary -XPOST and remove verbose mode from post_detections_user script

* adds utils for running integration tests with other users and adds two sample tests showing example usage

* minor type updates and small refactor

* fix x-pack/test types

* use enum types instead of custom type

* fix path to json

Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: Xavier Mouligneau <[email protected]>
dhurley14 added a commit that referenced this pull request Nov 19, 2020
… roles based on specific privileges (#81866) (#83861)

* shell scripts for creating roles + users for testing

* update readme's and updated privilege requirements based on testing with the users and inferring what the roles are supposed to do

* update role privileges based on feedback meeting yesterday

* updated scripts to accept filepath to role / user, added a test to ensure upload value list button is disabled

* updated role scripts to be parameterized

* adds login with role function and adds a sample test with a role to test that a t1 analyst user cannot upload a value list

* add object with corresponding roles

* fix spacing

* parameterize urls for basic auth with roles + users

* forgot to change the cy.visit string

* add KIBANA_URL env var for cli runner

* add env vars for curl script execution

* second script

* update readme's for each role and remove create_index from lists privilege for the soc manager role

* remove 'manage' cluster privilege for rule author

* remove 'create_index' privilege from soc_manager role since that is not parity with the security workflows spreadsheet

* update the login function logic with glo's feedback

* replace SIEM with Security Solution in markdown files

* make role param optional not just undefined

* remove unused file

* add copyright to scripts files

* update top-level README for roles scripts

* remove reference to internal spreadsheet and reference readme for this pr

* remove unnecessary -XPOST and remove verbose mode from post_detections_user script

* adds utils for running integration tests with other users and adds two sample tests showing example usage

* minor type updates and small refactor

* fix x-pack/test types

* use enum types instead of custom type

* fix path to json

Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: Xavier Mouligneau <[email protected]>

Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: Xavier Mouligneau <[email protected]>
banderror added a commit that referenced this pull request Dec 15, 2020
…g to info so they persist when dismissed (#84904)

**Addresses:** #76587

## Summary

In this PR I'm doing basically 2 things:

1. Making readonly callouts we have in Detections `primary` instead of `warning` and thus persistable in local storage (if a callout is dismissed, we remember it there)
2. Creating a reusable implementation for that.

TODO:

- [x] Adjust all callouts used in Detections to be of type `primary`
- [x] Implement the local storage logic (dumb)
- [x] Implement the local storage logic (reusable)
- [x] Add a new user role: "Reader" (read-only user)
- [x] Add Cypress tests

Out of scope:

- Add unit tests. I'll probably address this in a follow-up PR. Do you think it's worth it or better to wait until the rework (see the next point)?
- Rework callouts to standardise them across Detections, Cases and other parts of the Security app? See my comment below in this PR.

## Screen recordings

Quick demo of how this implementation works:

- [primary callouts](https://drive.google.com/file/d/1SYQd_ihKPvzlVUxELI8qNEqLBOkL18Gd/view?usp=sharing)
- [warning, danger](https://drive.google.com/file/d/1lrAFPyXNjOYSiEsUXxY_fjXsvmyDcdWY/view?usp=sharing) (callout types here were manually adjusted)

## Additional notes

Cypress tests are based on the work done in #81866.

![](https://puu.sh/GXwOd/1c855cb03f.png)
banderror added a commit to banderror/kibana that referenced this pull request Dec 15, 2020
…g to info so they persist when dismissed (elastic#84904)

**Addresses:** elastic#76587

## Summary

In this PR I'm doing basically 2 things:

1. Making readonly callouts we have in Detections `primary` instead of `warning` and thus persistable in local storage (if a callout is dismissed, we remember it there)
2. Creating a reusable implementation for that.

TODO:

- [x] Adjust all callouts used in Detections to be of type `primary`
- [x] Implement the local storage logic (dumb)
- [x] Implement the local storage logic (reusable)
- [x] Add a new user role: "Reader" (read-only user)
- [x] Add Cypress tests

Out of scope:

- Add unit tests. I'll probably address this in a follow-up PR. Do you think it's worth it or better to wait until the rework (see the next point)?
- Rework callouts to standardise them across Detections, Cases and other parts of the Security app? See my comment below in this PR.

## Screen recordings

Quick demo of how this implementation works:

- [primary callouts](https://drive.google.com/file/d/1SYQd_ihKPvzlVUxELI8qNEqLBOkL18Gd/view?usp=sharing)
- [warning, danger](https://drive.google.com/file/d/1lrAFPyXNjOYSiEsUXxY_fjXsvmyDcdWY/view?usp=sharing) (callout types here were manually adjusted)

## Additional notes

Cypress tests are based on the work done in elastic#81866.

![](https://puu.sh/GXwOd/1c855cb03f.png)
spong pushed a commit that referenced this pull request Dec 16, 2020
…g to info so they persist when dismissed (#84904) (#86047)

**Addresses:** #76587

## Summary

In this PR I'm doing basically 2 things:

1. Making readonly callouts we have in Detections `primary` instead of `warning` and thus persistable in local storage (if a callout is dismissed, we remember it there)
2. Creating a reusable implementation for that.

TODO:

- [x] Adjust all callouts used in Detections to be of type `primary`
- [x] Implement the local storage logic (dumb)
- [x] Implement the local storage logic (reusable)
- [x] Add a new user role: "Reader" (read-only user)
- [x] Add Cypress tests

Out of scope:

- Add unit tests. I'll probably address this in a follow-up PR. Do you think it's worth it or better to wait until the rework (see the next point)?
- Rework callouts to standardise them across Detections, Cases and other parts of the Security app? See my comment below in this PR.

## Screen recordings

Quick demo of how this implementation works:

- [primary callouts](https://drive.google.com/file/d/1SYQd_ihKPvzlVUxELI8qNEqLBOkL18Gd/view?usp=sharing)
- [warning, danger](https://drive.google.com/file/d/1lrAFPyXNjOYSiEsUXxY_fjXsvmyDcdWY/view?usp=sharing) (callout types here were manually adjusted)

## Additional notes

Cypress tests are based on the work done in #81866.

![](https://puu.sh/GXwOd/1c855cb03f.png)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Detection Rules Anything related to Security Solution's Detection Rules release_note:skip Skip the PR/issue when compiling release notes review Team:Detections and Resp Security Detection Response Team v7.11.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants