Security - Role Mappings UI

[legrego]

Role Mapping UI 🔐 🗺

Adds a new management interface to allow users with the manage_security cluster privilege to manage their role mappings. This UI sits on top of the existing role mapping API.

A role mapping is a rule-based mechanism which tells Elasticsearch how to assign roles to users when authenticating from realms other than native and file.

Resolves #17981

Licensing

The role mapping interface is available to users with a Gold+ license (i.e., gold, platinum, trial, or enterprise). This is because role mappings cannot be used with any of the authentication realms available under the Basic license (native, file). See the ES Docs for more information.

Features

The role mapping UI supports the following operations:

View Role Mappings

Create/Edit Role Mapping

Delete Role Mappings

Exceptional conditions

Role mappings are complex entities, and the UI has several states which attempt to inform users of system conditions which impact their operation:

Insufficient privileges

Users without the manage_security cluster privilege will be presented with an "insufficient privileges" view instead of the actual management interfaces.

Testing: Login to Kibana with a user who does not have the manage_security cluster privilege, and attempt to manage role mappings

Incompatible realms

We display a warning message if Elasticsearch does not have any realms enabled which can take advantage of role mappings. This will not prevent role mappings from being managed, but rather makes them unnecessary, as the mappings won't be used.

Testing:

• Run with a Gold+ license, but without configuring any additional authentication realms in ES

To verify this message disappears, configure Elasticsearch with an external identity provider, such as:

• LDAP
• SAML
• Kerberos
• PKI
• OIDC

Inline scripts disabled in Elasticsearch

When inline scripts are disabled in Elasticsearch, then the traditional role templates cannot be used. We display a warning when editing an existing role mapping which contains these templates in this scenario.

Testing

1. Create a role mapping which uses the "Role template" template type
2. Stop Elasticsearch, and reconfigure to disable inline scripts by setting: scripts.allowed_types: ["stored"] in your elasticsearch.yml
3. Restart Elasticsearch, and attempt to edit this role mapping

Stored scripts disabled in Elasticsearch

When stored scripts are disabled in Elasticsearch, then the "stored script" role templates cannot be used. We display a warning when editing an existing role mapping which contains these templates in this scenario.

Testing

1. Create a role mapping which uses the "Stored script" template type
2. Stop Elasticsearch, and reconfigure to disable stored scripts by setting: scripts.allowed_types: ["inline"] in your elasticsearch.yml
3. Restart Elasticsearch, and attempt to edit this role mapping

Pending Tasks

• 🎨 Final design review
• 🖊 Copy Review
• 📝 Finish documentation (followup PR)
• 👨‍💻 Code review

[elasticmachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: 17d77c9

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

[elasticmachine]

Pinging @elastic/kibana-security (Team:Security)

[jportner]

Reviewing now!

[ryankeairns]

I think @andreadelrio worked on this, but she is out today so you're stuck with me :) There are some existing design patterns (form layouts) within Management that we can align to more tightly.

By default, EuiDescribedFormGroup defaults to a max-width: 800px. Clicking around Management, it seems we predominantly apply the fullWidth prop which, in this case, also seems to improve layout of the Role templates section when it is expanded.

In the screenshot above, I also see a lack of labels and/or help text for the Name input and the switch control. @gchaps can help with this, but my thoughts would be to add a 'Name' label above the text input (we could then re-name the bold heading in the left column to Mapping name) and then add a label next to the switch like 'Enable mapping'.

^ Gail, the wording in the Enable mapping section (left column) was particularly confusing to me... the title reads 'Enable mapping' while the help text below it explains the disabled state. Perhaps those should be in agreement?

Lastly, in the Roles section we use a default button style (with border) as opposed to an empty button. Should we do the same here? The empty button style gets a bit lost when it is surrounded by so much whitespace.

[gchaps]

+1 to including the label Name label above the text input and the label Enable Mapping above the toggle button.

For the Enable Mapping description, how about:
Map roles to users based on their username, groups, and other metadata. When false, ignore mappings.

[legrego]

Thanks @ryankeairns and @gchaps -- here's a screenshot with all suggestions applied:

[ryankeairns]

LGTM, appreciate the changes!

[kobelb]

ACK: Starting review

[jportner]

Seeing as this is a large PR and we now have two people reviewing it, I'll go ahead and leave a comment regarding my thoughts so far.

I tested most of the functionality, except for role templates. I also tried to break it every way I could think of. It works great! Feedback:

1. When the only non-native / non-file realm enabled is the PKI realm, creating a role mapping with the "groups" field will have no effect (reference). Maybe we can show a warning to the user in that case, and remove the "groups" option from the presets in the user input field?

2. When creating a brand new role mapping, you can unintentionally overwrite another one by using the same name. Can we prevent the user from doing this, or otherwise warn them when it's about to happen?

3. I like that the no_compatible_realms component provides a link to the Elasticsearch documentation for role mapping. I think it would be useful to provide this link when the user has compatible realms, too. It gives more information about role mapping that would be useful for someone who isn't familiar with it. What do you think?

That's all I have for now, along with a couple nits below.

[legrego]

When the only non-native / non-file realm enabled is the PKI realm, creating a role mapping with the "groups" field will have no effect (reference). Maybe we can show a warning to the user in that case, and remove the "groups" option from the presets in the user input field?

I'm hesitant to show a warning, as the rules section of the UI is already pretty dense, but I'm not opposed to hiding the "groups" option if we think there's enough benefit. Currently, the features check returns a simple flag indicating if compatible realms are enabled. We'd need to enhance this to return the set of enabled realms instead. This is certainly doable, but comes with a little added complexity to an already complex feature. I'll give this some more thought.

When creating a brand new role mapping, you can unintentionally overwrite another one by using the same name. Can we prevent the user from doing this, or otherwise warn them when it's about to happen?

For better or worse, the current behavior is consistent with the Roles UI today. The ES APIs for both do not distinguish between create and update, so a pre-flight check would be required on save. Again, this is certainly doable (subject to race conditions), but I was avoiding the additional implementation since we had a precedent for the less-than-ideal behavior 🙈

I like that the no_compatible_realms component provides a link to the Elasticsearch documentation for role mapping. I think it would be useful to provide this link when the user has compatible realms, too. It gives more information about role mapping that would be useful for someone who isn't familiar with it. What do you think?

That's not a bad idea -- I do attempt to provide links to more specific doc entries throughout the UI, but the overview might also be helpful. @andreadelrio / @gchaps do you have any thoughts on this?

[gchaps]

Although I am not opposed to the idea, I worry that we are putting too many links in our UIs. Where do you plan to add the link? Would it be appropriate to link to an overview topic from the Help menu?

[jportner]

Although I am not opposed to the idea, I worry that we are putting too many links in our UIs. Where do you plan to add the link? Would it be appropriate to link to an overview topic from the Help menu?

This is what I had in mind:

Just a simple link, "Read more here."

Edit: it would be more consistent with the rest of the Role Mapping UI for the link text to say "Learn about role mapping."

[gchaps]

A link in the intro works. I would use the text "Learn more." While I usually prefer the more specific text, I think it is clear that the link goes to the role mapping docs because "role mapping" appears in the title and intro sentence.

[jportner]

I got a bit confused when seeing this:

The two of these rules (except_all_rule except_any_rule and except_field_rule) are functionally equivalent. It seems that the only reason to include the except_field_rule is because the Elasticsearch API supports it, so we have to be able to render that type of rule.

There's already a lot going on in this UI, what do you think about potentially ditching the except_field_rule and just auto-converting that to an except_all_rule except_any_rule whenever the user is editing an existing role mapping?

I get that the intent of this is to provide a UI for the functionality that Elasticsearch provides, but some things just don't quite make sense when you translate them to a UI, in my opinion.

Note: it looks like except_all_rule should have the "None are true" message, but it currently doesn't. I mentioned that in a comment below. This comment assumes that's going to change. Edit: my mistake

[legrego]

There's already a lot going on in this UI, what do you think about potentially ditching the except_field_rule and just auto-converting that to an except_all_rule whenever the user is editing an existing role mapping?

That's an interesting idea! I do think that'd improve the usability and readability of the rules, but I'm also hesitant to automatically convert it, mostly because we expose the raw rule definition via the JSON editor, so users who are familiar with their rule set would see that the rules they previously created don't exactly match. Even within the same editing session, going from JSON editor -> Visual editor -> JSON editor would illustrate the difference.

I do agree that the current state can get confusing though, and you're absolutely right that they both exist solely because the Elasticsearch API supports it...

Do you think the potential confusion of this rewrite is worth the ux improvement?

[jportner]

Assuming this would be an easy change like it appears to be -- my personal opinion is that, if such a change was made, the total sum of confusion would be less 😁
How many people are familiar with role mapping and would notice (or care) if we converted the rule? Probably not many.

Maybe someone else can weigh in?

[legrego]

I tested this out: it does appear to be an easy fix and would actually simplify a couple of other code paths slightly. Let's see what others think!

[kobelb]

I'm also hesitant for us to automatically convert the except_field_rule to a except_any_rule because of the ability to toggle between JSON and visual editors...

Do we see any similarity to the fact that we allow users to specify the following via the JSON editor:

{
  "field": {
    "username": "*"
  }
}

Which we render as the following, even though the user can't create this rule via just the UI?

[legrego]

It’s kind of similar, in that the visual editor is limiting what you’re able to do in both cases. The only difference is that the visual editor (and the persisted model) will alter what is on screen if we perform the conversion from except_field_rule.

Joe does bring up a good point though, in that this is a bit of an edge case that most users won’t run into, let alone notice (we think)

[legrego]

@elasticmachine merge upstream

[legrego]

@gchaps as we discussed, I added "infomercial" style docs to this PR, linking to the existing ES docs where appropriate

Doc preview: http://kibana_53620.docs-preview.app.elstc.co/guide/en/kibana/master/role-mappings.html

[legrego]

@kobelb I think I addressed all of your feedback (🤞), so this is ready for another review whenever you are. I tried to reference commit hashes in my replies to your comments, so it's easier to find/review in isolation

[kobelb]

ACK: re-reviewing now

[legrego]

I'm going to merge ahead of docs approval to unblock other work, so I'll address any feedback in a followup PR.

@jportner your username is attached to a bunch of new snyk checks on this PR now, one of which is failing due to a missing manifest. Looks unrelated to any of the work in this PR, so I'm ignoring.