Feature Controls - Feature privileges besides all and read

[kobelb]

Presently, we only allow feature's to register an all or read privilege. This is rather limiting and doesn't allow features to grant more granular access to individual "sub features", for example the ability to create short urls. Another example could be for the various visualization types. Administrators might not want users to create certain visualizations. For example, Vega visualizations allow for raw ES queries to be written to power the visualizations, and that can lead to problems with the cluster if a user writes a very expensive query.
Or for index pattern management, administrators might want to restrict who can create scripted fields within the patterns.

[elasticmachine]

Pinging @elastic/kibana-security

[ryankeairns]

The following Figma prototype link contains the designs we discussed for sub-feature privileges:

Prototype

🖥 https://www.figma.com/proto/4N0Np3P0aSihitAqO1OLtJ/Spaces?node-id=10%3A1211&viewport=-3294%2C-9504%2C1.1121656894683838&scaling=scale-down-width

Screenshot preview

Compressed form controls

The full set of compressed EUI form controls can be seen at the bottom of this page: https://elastic.github.io/eui/#/forms/compressed-forms

Let me know if you need any assistance or feedback once you start building this out. Happy to help!

[tsg]

This one will likely become important for a set of RBAC-like features that we need in SIEM and the Endpoint app. For example, in the SIEM, we want to have users (say, Tier 1 analysts) that can view and close signals but that cannot manage the SIEM rules.

[legrego]

@ryankeairns I'm starting to pick this up again, and I'd like to get your thoughts:

In the screenshot, we have a "Mute alert messages" sub-feature privilege, which the user can toggle independently of the "All"/"Read"/"None" button group.

Selecting "All" will automatically select "Mute alert messages" (based on the Figma you sent), which is what I'd expect:

I'm curious what this will look like once there are more than a couple of sub-feature privileges in this section. For example, I could imagine Alert management having the following (hypothetical):

• Create/update alerts
• Mute alerts
• Acknowledge alerts
• Delete alerts

I fear that the UI will end up a bit crowded in this scenario, or visually hard to parse

[ryankeairns]

@legrego I don't recall making that connection, at the time, between "All" and "Mute". The mockups were showing a few different combinations, but I was imagining the button group (All | Read | None) had to do with CRUD permissions whereas Mute would affect the notifications that pop up.

Maybe we should zoom about this so that I'm following more closely. It might be that this gets split into more than one section or something.

[legrego]

I don't recall making that connection, at the time, between "All" and "Mute". The mockups were showing a few different combinations, but I was imagining the button group (All | Read | None) had to do with CRUD permissions whereas Mute would affect the notifications that pop up.

I chatted with @kobelb after posting this, and I think your initial understanding is correct: the button group for alerting would be for CRUD permissions, while the mute checkbox would be an additional action that could be selected.

[legrego]

I'd like to open a comment period for the overall UI which enables this feature. I have a working proof-of-concept which illustrates how the role management screen will function when configuring sub-feature privileges.

Motivation

The original feature controls implementation went through a lot of design iterations, and as a result, the underlying implementation became rather difficult to maintain and reason about. We didn't take the proper time to restructure the implementation following design changes, and I want to make sure we leave ourselves enough time to do it right this time around.

I am looking to get "approval" from the following stakeholders on the proposed approach before we go through the effort of getting this ready for final code and design reviews:

• @elastic/kibana-security
• @ryankeairns / design team as appropriate
• @arisonl

Out of scope

This is not a code review.

The code is not in a state to review. Please run this to get a sense of how Kibana will behave, without reviewing the underlying implementation.

This is mostly not a design review.

I have cleanup to do yet: There are controls which take up the wrong amount of space, and controls which overlap each other in certain conditions.

The types of controls and overall layout is all in scope, however.

In scope

The overall layout and experience of configuring privileges is in-scope for review at this point. This includes:

• All tooltips and messaging (message text is placeholder, but conveys the point)
• Displayed privileges
• Controls for editing privileges, including enabled/disabled states

Comment period

Please reply directly to this issue with your feedback.
I am looking for all feedback by EOD Friday, February 7th.

To test

Run from the security/sub-features-3 branch of my fork (legrego#security/sub-features-3)

After bootstrapping, view the role management page. For this test, I've added mock sub-feature privileges to the Discover and Visualize features.

Let me know if you need help getting your environment setup, I'm more than happy to help out here.

Rough set of commands

• `cd` to your local Kibana repository
• Switch to my branch: `git checkout security/sub-features-3`
• Bootstrap: `yarn kbn bootstrap`
• Start ES: `yarn es snapshot --license trial`
• Start Kibana: `./bin/kibana --dev`

A word of caution

[ryankeairns]

Checked this out locally and it looks good, nice progress.

A driving theme in this design was to make the interactions feel intentional. In reviewing this PR, I did feel as though I was being guided to make clear/cautious decisions when changing privileges with global and/or overriding effects.

[kobelb]

Nice work, this is working great! I noticed a few things when going through the UI/UX, some of which are quite possibly bugs or temporary short-cuts

1. Discover's "mute alerts" sub-feature privilege can be enabled when the user has no other alerting privileges

Was this intentional? If so, will that always be the behavior within sub-feature privileges?

2. There's no indication in the "privileges table" when a space has privileges less permissive than globally.

3. The privilege summary's use of either a label for the mutually exclusive options, and a check-box for the boolean options is rather inconsistent

4. Should we use something besides a warning logo when a feature has sub-feature privileges selected? It doesn't feel like a warning is the most accurate:

5. Toggling between "All" and "Read" forgets the sub-feature privilege selection, is this what we want?

6. The Discover feature is automatically expanded, is this intentional?

7. For these features without sub-feature privileges, do we want there to be the "expand"/"collapse" button next to them? If a feature doesn't have sub-feature privileges, having to click that and see you can't do anything is a bummer.

[legrego]

Thanks for the prompt feedback @kobelb / @ryankeairns!

Discover's "mute alerts" sub-feature privilege can be enabled when the user has no other alerting privileges. Was this intentional? If so, will that always be the behavior within sub-feature privileges?

It was intentional - it was my understanding that the "privilege groups" were not in any way linked to each other, but on the other hand, I can see this tripping up some users. In this specific example, it won't be very helpful to mute alerts if you can't even read them.

There's no indication in the "privileges table" when a space has privileges less permissive than globally.

👍 I can add this

The privilege summary's use of either a label for the mutually exclusive options, and a check-box for the boolean options is rather inconsistent

Should we use something besides a warning logo when a feature has sub-feature privileges selected? It doesn't feel like a warning is the most accurate:

I'll defer to @ryankeairns on these two points; I followed the Figma mockup for both.

Toggling between "All" and "Read" forgets the sub-feature privilege selection, is this what we want?

🤷‍♂ This was honestly the easier of the two options to implement. I like this approach because it respects the visual hierarchy - changing the primary feature privilege resets the form, which you can then choose to customize. It could be frustrating to lose your changes, but I sincerely hope that these sub-feature forms won't grow to be all that complex.

The Discover feature is automatically expanded, is this intentional?

This just saves me a click when doing the change -> refresh -> test cycle, and won't be in the final design.

For these features without sub-feature privileges, do we want there to be the "expand"/"collapse" button next to them? If a feature doesn't have sub-feature privileges, having to click that and see you can't do anything is a bummer.

That's a good point, it's rather useless for features which lack sub-features. I'll update to remove this. Should there be some other content there to signify that you can't change anything, or should we just leave it empty?

[ryankeairns]

The privilege summary's use of either a label for the mutually exclusive options, and a check-box for the boolean options is rather inconsistent

Should we use something besides a warning logo when a feature has sub-feature privileges selected? It doesn't feel like a warning is the most accurate:

I'll defer to @ryankeairns on these two points; I followed the Figma mockup for both.

I'm not sure I'm following the first point completely, but would it be possible still display a button group, but set it to disabled? Then we're using the same UI elements (button groups and checkboxes) in all situations.

To the second point, we could use the iInCircle icon to denote more information in a less urgent/negative way.

[legrego]

I'm not sure I'm following the first point completely, but would it be possible still display a button group, but set it to disabled? Then we're using the same UI elements (button groups and checkboxes) in all situations.

We certainly can. I tried this, but even when using buttonSize="compressed", and using an EuiModal with an unrestricted width (maxWidth={false}), we end up running out of room pretty quickly. Here's what it looks like when my window is maximized on my 15" laptop:

[jportner]

I'm late to the party, but to add my 2 cents to the feedback that was already given:

Toggling between "All" and "Read" forgets the sub-feature privilege selection, is this what we want?

🤷‍♂ This was honestly the easier of the two options to implement. I like this approach because it respects the visual hierarchy - changing the primary feature privilege resets the form, which you can then choose to customize. It could be frustrating to lose your changes, but I sincerely hope that these sub-feature forms won't grow to be all that complex.

I like the current implementation.

For these features without sub-feature privileges, do we want there to be the "expand"/"collapse" button next to them? If a feature doesn't have sub-feature privileges, having to click that and see you can't do anything is a bummer.

That's a good point, it's rather useless for features which lack sub-features. I'll update to remove this. Should there be some other content there to signify that you can't change anything, or should we just leave it empty?

I think leaving it empty would be best.

---

My own feedback:

1. What do you think about using EuiAccordion? This would move the "expand" arrow to the left side of the row, which I think would draw peoples' eyes and make it more evident when there are available sub-feature privileges to customize. This would also make the entirety of the text clickable to view the expanded row.
2. What do you think about an "open all" / "close all" button? Similar to the existing "change all" button. This could come in handy when you want to view everything that's available.

Also, I found a bug, in case you weren't aware of this behavior already:

[legrego]

What do you think about using EuiAccordion? This would move the "expand" arrow to the left side of the row, which I think would draw peoples' eyes and make it more evident when there are available sub-feature privileges to customize. This would also make the entirety of the text clickable to view the expanded row.

That's an interesting idea. Here's a quick-and-dirty implementation to show it off. My one hesitation here is getting features without sub-feature privileges to render correctly. We'd want them to render like an accordion, but without the expanding action or UI affordances for it. If I get some time after the more critical paths are in place, I'll revisit this to see what's possible.

What do you think about an "open all" / "close all" button? Similar to the existing "change all" button. This could come in handy when you want to view everything that's available.

Should be easy enough to do!

Also, I found a bug, in case you weren't aware of this behavior already:

Yep, thanks for reporting though!

[arisonl]

Great work, once again. A couple of thoughts/questions:

• The design is lean and simple but I wanted to note that: the distance between the sub-features and their respective controls, the lack of some type of separator between the sub-features, the different types and sizes of controls (buttons, checkboxes and labels) and potentially the number of the sub-features, might make the sub-features section as a whole a little difficult to parse:

• What is the behaviour of the sub-feature sections of the other apps, once one app is open? I presume you can still open them and work with them? In that case the first bullet-point becomes more challenging.
• The accordion is a nice alternative; having > on the left you immediately realise that it is expandable, without having to parse anything else.
• Are we moving from a model where: an application determines completely the privileges and the privileges are of three levels, to a model where: the application determines any number of levels as well as the sub-features and the admin is free to determine how these levels and sub-features "mix and match"? How will this work for other applications where more than three levels and different sub-features might be needed? What is the "framework"?

[legrego]

What is the behaviour of the sub-feature sections of the other apps, once one app is open? I presume you can still open them and work with them? In that case the first bullet-point becomes more challenging.

Yes, you can open and work on as many features as you'd like in this screen, so I agree it can make the sub-features sections a bit difficult to parse. @ryankeairns do you have any suggestions on how to make this easier to parse? Having a visual separation between the different sub-features might be helpful here, but I worry about drawing too many horizontal lines, since we are already rendered inside of a table (FWIW, I don't think this part is solved by accordions either).

We could also give more space between each sub-feature, but I then worry about the amount of scrolling folks will have to do in order to see everything when more than one row is expanded.

The accordion is a nice alternative; having > on the left you immediately realise that it is expandable, without having to parse anything else.

++

Are we moving from a model where: an application determines completely the privileges and the privileges are of three levels, to a model where: the application determines any number of levels as well as the sub-features and the admin is free to determine how these levels and sub-features "mix and match"? How will this work for other applications where more than three levels and different sub-features might be needed? What is the "framework"?

I think this is still up for debate. In other words, I considered it out-of-scope for this issue. The "All"/"Read"/"None" that we show at the feature-level will remain unchanged for now. Features (applications) will still have to fit their privilege model into the "All"/"Read"/"None" categories, but they will have a lot of control over the sub-features.

In general, the sub-feature privileges come in two flavors: independent and mutually-exclusive:

independent features privileges are the checkboxes that you see in the expanded region. These checkboxes can be selected in any combination, and do not have any dependencies on other sub-feature privileges.

mutually-exclusive feature privileges are the button groups that you see in the expanded region. Features (applications) have full control over how these are labeled, but there are some restrictions:

• We assume that the privileges are defined in descending order of permissive-ness. In other words, the left-most privilege must have more capabilities than the next left-most privilege. This is identical to how the "All"/"Read"/"None" controls behave today.
• Since they are mutually exclusive of each other, by definition you are only able to select a single privilege within the group.

[arisonl]

but I worry about drawing too many horizontal lines, since we are already rendered inside of a table

Totally agree, not easy to improve this. Curious if there is a way without explicitly drawing lines, e.g. slightly varying the background shades or something better. Also, with many sub-areas open, is the parent-child grouping of main privileges and sub-features maintained or does it get blurred?

In general, the sub-feature privileges come in two flavors: independent and mutually-exclusive...:

Makes sense, thanks.

[legrego]

Curious if there is a way without explicitly drawing lines, e.g. slightly varying the background shades or something better.

I'll see if we can some up with something better

with many sub-areas open, is the parent-child grouping of main privileges and sub-features maintained or does it get blurred?

I think the parent-child groupings are maintained here. There is enough color contrast and lines between the parent row and the expanded child region to delineate this