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

Hide timeline bar if user does not have security solution crud capability #123775

Conversation

jamster10
Copy link
Contributor

@jamster10 jamster10 commented Jan 25, 2022

#122207

Summary

If a user does not have show capability for Security Solution from role permissions, but they do have access to cases (within Security Solution), they should not be able to interact with the bottom bar (Timelines).

As such, the bottom bar is now wrapped in a conditional to perform the check noted above.

A user with Cases but no show security solution permissions:

Before:
image

After:
image

Checklist

Delete any items that are not applicable to this PR.

  • Unit or functional tests were updated or added to match the most common scenarios
  • Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)

@jamster10 jamster10 added release_note:fix v8.0.0 Team:Threat Hunting Security Solution Threat Hunting Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting:Explore labels Jan 25, 2022
@jamster10 jamster10 self-assigned this Jan 25, 2022
@jamster10 jamster10 requested a review from a team as a code owner January 25, 2022 22:45
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-threat-hunting (Team:Threat Hunting)

@elasticmachine
Copy link
Contributor

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

@jamster10
Copy link
Contributor Author

jamster10 commented Jan 25, 2022

Hey, @paulewing @monina-n, I wasnt sure if a user only has Read permission, if they should be able to see the bottom bar. Currently if they do have read permissions to SecuritySolution, they do see the timeline bottom bar on most pages, as they would if they had full crud capability. I want to make sure this is expected.

My changes prevent a user with no read or write capability from seeing the timeline bar.

@jamster10 jamster10 force-pushed the 122207-bug-hide-bottom-timeline-if-not-permissioned branch from b3f71f7 to 2313a49 Compare January 25, 2022 23:36
bottomBar={
userHasSecuritySolutionVisible ? (
<SecuritySolutionBottomBar onAppLeave={onAppLeave} />
) : (
Copy link
Contributor

Choose a reason for hiding this comment

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

NIT: the bottomBar property is optional, we could also omit the prop or just pass undefined, instead of the empty fragment

@semd
Copy link
Contributor

semd commented Jan 26, 2022

Hey @jamster10, yes I agree we should make a decision about what to do with the timeline if the user only has the READ privilege.
I have been testing the current changes, everything works great when the user doesn't have ANY security solution privilege, but when the user has only the READ privilege I have spotted some problems:

  • The user is able to open the "Untitled Timeline" and save it, then an error arises in the console and the application enters an inconsistent state, when the application is refreshed we can see the timeline was not saved:
Gravacio.de.pantalla.2022-01-26.a.les.12.57.27.mov

If we are going to allow READ only access to the timeline, maybe we should disable the save button.

  • The user is able to click the "Investigate in timeline" button in the alerts table, when the timeline opens the same error appears in the console, and the application enters an inconsistent state:
Gravacio.de.pantalla.2022-01-26.a.les.12.58.59.mov

If we are going to allow READ only access to this section this error needs to be fixed, if we are not then maybe we should disable the "Investigate in timeline" button in the alerts table.

@paulewing @monina-n What do you think?

cc @YulNaumenko

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
securitySolution 4.6MB 4.6MB +63.0B

History

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

cc @jamster10

Copy link
Contributor

@semd semd left a comment

Choose a reason for hiding this comment

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

LGTM! 👌
Let's fix the "read-only" problems in another PR.

@jamster10 jamster10 merged commit 5aa26ed into elastic:main Jan 28, 2022
awahab07 pushed a commit to awahab07/kibana that referenced this pull request Jan 31, 2022
…lity (elastic#123775)

* Hide timeline bar if user does not have security solution crud capability

* change visibility to be based on show instead of crud

* PR fix

Co-authored-by: Kristof-Pierre Cummings <[email protected]>
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Feb 1, 2022
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 123775 or prevent reminders by adding the backport:skip label.

2 similar comments
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 123775 or prevent reminders by adding the backport:skip label.

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 123775 or prevent reminders by adding the backport:skip label.

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 123775 or prevent reminders by adding the backport:skip label.

12 similar comments
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 123775 or prevent reminders by adding the backport:skip label.

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 123775 or prevent reminders by adding the backport:skip label.

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 123775 or prevent reminders by adding the backport:skip label.

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 123775 or prevent reminders by adding the backport:skip label.

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 123775 or prevent reminders by adding the backport:skip label.

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 123775 or prevent reminders by adding the backport:skip label.

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 123775 or prevent reminders by adding the backport:skip label.

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 123775 or prevent reminders by adding the backport:skip label.

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 123775 or prevent reminders by adding the backport:skip label.

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 123775 or prevent reminders by adding the backport:skip label.

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 123775 or prevent reminders by adding the backport:skip label.

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 123775 or prevent reminders by adding the backport:skip label.

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 123775 or prevent reminders by adding the backport:skip label.

stephmilovic pushed a commit to stephmilovic/kibana that referenced this pull request Feb 23, 2022
…lity (elastic#123775)

* Hide timeline bar if user does not have security solution crud capability

* change visibility to be based on show instead of crud

* PR fix

Co-authored-by: Kristof-Pierre Cummings <[email protected]>
(cherry picked from commit 5aa26ed)
stephmilovic added a commit that referenced this pull request Feb 23, 2022
…lity (#123775) (#126289)

* Hide timeline bar if user does not have security solution crud capability

* change visibility to be based on show instead of crud

* PR fix

Co-authored-by: Kristof-Pierre Cummings <[email protected]>
(cherry picked from commit 5aa26ed)

Co-authored-by: Kristof C <[email protected]>
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Feb 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:fix Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting:Explore Team:Threat Hunting Security Solution Threat Hunting Team v8.0.1 v8.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Security Solution]Hide Bottom Timeline for Security privilege none and Case All custom user role
5 participants