[Roles] Added transformation of application * privilege to all #181400

elena-shostak · 2024-04-23T09:30:57Z

Summary

Added transformation of application wildcard * privilege to all to correctly filter and display roles as superuser.

[Screenshot] Kibana superuser role before change

[Screenshot] Kibana superuser role after change

Checklist

Unit or functional tests were updated or added to match the most common scenarios

For maintainers

This was checked for breaking API changes and was labeled appropriately

Fixes: #106561

Release note

Added transformation of application wildcard * privilege to all to correctly filter and display roles as superuser.

elena-shostak · 2024-04-23T09:32:48Z

/ci

elasticmachine · 2024-04-23T11:28:19Z

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

jeramysoucy

Nice work, Elena!

I just found one issue. If I create a role via ES API with wildcard application privileges (like superuser and system_indices_superuser both have), and then load and save the role in the UI (making no changes), the role ends up with an additional entry that correlates to the translated 'all' privilege.

"applications": [
    {
        "application": "*",
        "privileges": [
            "*"
        ],
        "resources": [
            "*"
        ]
    }
],

becomes

"applications": [
    {
        "application": "kibana-.kibana",
        "privileges": [
            "all"
        ],
        "resources": [
            "*"
        ]
    },
    {
        "application": "*",
        "privileges": [
            "*"
        ],
        "resources": [
            "*"
        ]
    }
],

And it also shows up in the UI:

It will also occur if a user edits the translated privileges in the UI. When the privilege is updated, it will retain the original wildcard as well as a new 'kibana':'all'.

We may want to intercept this on save/update and have a special case for when a role already contains the wild card so it is either retained (if assigning all) or overwritten (if changed), and potentially display a message to the user when a role contains the superuser style wildcard.

cc: @legrego any thoughts?

legrego · 2024-04-24T11:33:17Z

If I create a role via ES API with wildcard application privileges (like superuser and system_indices_superuser both have), and then load and save the role in the UI (making no changes), the role ends up with an additional entry that correlates to the translated 'all' privilege.

Great find, @jeramysoucy. Thanks for your attention to detail here.

We may want to intercept this on save/update and have a special case for when a role already contains the wild card so it is either retained (if assigning all) or overwritten (if changed), and potentially display a message to the user when a role contains the superuser style wildcard.

cc: @legrego any thoughts?

I'm not opposed to exploring some sort of interception if it's not an unreasonable amount of effort, and not too hacky. Otherwise, we should consider alternate approaches.

elena-shostak · 2024-04-24T11:57:32Z

@legrego @jeramysoucy privilege with "application": "kibana-.kibana" is a subset of "application": "*" in that case.
I think rules for overwriting it might be quite complex, like what if we want to overwrite the rule only for kibana-.kibana, should we just drop the wildcard privilege then?

The alternative can be if we return the application privilege with strongest weight in that case? Because in the end the strongest privileges will be applied. WDYT?

"applications": [
    {
        "application": "kibana-.kibana",
        "privileges": [
            "all"
        ],
        "resources": [
            "*"
        ]
    },
    {
        "application": "*",
        "privileges": [
            "*"
        ],
        "resources": [
            "*"
        ]
    }
],

elena-shostak · 2024-04-24T13:56:39Z

@jeramysoucy Thanks for bringing it in. Looked carefully at the setup, it seems that my normalization of application wildcard privilege on the server brought more problems than it eventually solved.

Discussed it with @legrego, we've decided to not normalize it to all, but rather display as * and disallow edit of such privileges in the UI. See attached screenshot below.

[Screenshot] Kibana wildcard role display

jeramysoucy · 2024-04-24T14:01:32Z

Discussed it with @legrego, we've decided to not normalize it to all, but rather display as * and disallow edit of such privileges in the UI. See attached screenshot below.

@elena-shostak Sounds good. Do we want to disallow the delete action as well?

elena-shostak · 2024-04-24T14:17:24Z

@elena-shostak Sounds good. Do we want to disallow the delete action as well?

I think we can, to make it consistent with superuser role display.

jeramysoucy

Sorry in advance. I found a couple of things.

Can we notify the user somehow that a base wildcard privilege is special? The action icons are removed, but there is nothing else to let the user know what's going on. Maybe an icon tip where the action icons usually are? cc: @legrego do we need to check with the design team or doc team for approach/wording?
If I view a role with the base wildcard and click Update, there is an error:

This is tricky, because if we disable the update button then no changes can be made to ES privileges for the role. So I think we have to trap the Kibana base wildcard on update, and only apply any ES changes.
If I view a role with the base wildcard, I still have options to add a Kibana privilege and view a privilege summary.

The summary shows all "none" (expected since this is a special case), but maybe we should remove the ability to view this is the base wildcard is the only privilege?

I think both of these options, Add Kibana privilege and View privilege summary, should be removed for roles with the base wildcard.

...les/edit_role/privileges/kibana/space_aware_privilege_section/privilege_space_table.test.tsx

legrego · 2024-04-25T13:03:14Z

Can we notify the user somehow that a base wildcard privilege is special? The action icons are removed, but there is nothing else to let the user know what's going on. Maybe an icon tip where the action icons usually are? cc: @legrego do we need to check with the design team or doc team for approach/wording?

I'm torn on this. On one hand, it makes sense to give knowledgeable administrators this information, but this could lead to more confusion for the majority of our administrators. To my knowledge, Kibana is the primary consumer of application privileges. There might be an enterprise search use case, but they have their own bespoke authorization model that's never been compatible with our role management UI/API. Telling them this grants access to additional application privileges in effect doesn't say much, because it's really just Kibana in practice.

If I view a role with the base wildcard and click Update, there is an error.
This is tricky, because if we disable the update button then no changes can be made to ES privileges for the role. So I think we have to trap the Kibana base wildcard on update, and only apply any ES changes.

I wonder if adding API support for other application privileges would be a helpful prerequisite? #21503

I think both of these options, Add Kibana privilege and View privilege summary, should be removed for roles with the base wildcard.

++

...ent/roles/edit_role/privileges/kibana/privilege_form_calculator/privilege_form_calculator.ts

elena-shostak · 2024-04-25T17:08:44Z

@jeramysoucy @legrego

Changes addressed:

Made the role non editable if there is a base privilege with the wildcard present.
Added tests to cover scenarios with edit/delete icon actions
Added tests to cover scenarios with View privilege summary, Add Kibana privilege actions

See screenshot

Let me know if that works and makes sense.

jeramysoucy

I just had one suggestion in the new test, to check for a read-only button render. I am approving now so that i don't hold up this PR any further.

I think making roles with the app wildcard privilege read-only in the UI is fine. These roles could only have been create/updated via the ES API anyway. I'm not sure how I feel about there not being any message to clarify this to the end user, but this is something we can come back to if needed.

Great work, Elena!

x-pack/plugins/security/public/management/roles/edit_role/edit_role_page.test.tsx

elena-shostak · 2024-04-30T09:06:29Z

@elasticmachine merge upstream

elena-shostak · 2024-04-30T09:31:32Z

@elasticmachine merge upstream

elena-shostak · 2024-04-30T13:00:24Z

@elasticmachine merge upstream

…181165) ## Summary Added endpoint `GET kbn:/internal/security/roles/{space-id}` to get all roles for provided space id. **Note**: changes needed for application `*` privileges were cherry-picked [to a separate PR].(#181400) ## Example Request `GET kbn:/internal/security/roles/space-b` Response ``` [ { "name": "role-a", "metadata": {}, "transient_metadata": { "enabled": true }, "elasticsearch": { "cluster": [ "all" ], "indices": [], "run_as": [] }, "kibana": [ { "base": [], "feature": { "dev_tools": [ "all" ] }, "spaces": [ "default", "space-b" ] } ], "_transform_error": [], "_unrecognized_applications": [] }, { "name": "superuser", "metadata": { "_reserved": true }, "transient_metadata": {}, "elasticsearch": { "cluster": [ "all" ], "indices": [ { "names": [ "*" ], "privileges": [ "all" ], "allow_restricted_indices": false }, { "names": [ "*" ], "privileges": [ "monitor", "read", "view_index_metadata", "read_cross_cluster" ], "allow_restricted_indices": true } ], "remote_indices": [ { "names": [ "*" ], "privileges": [ "all" ], "allow_restricted_indices": false, "clusters": [ "*" ] }, { "names": [ "*" ], "privileges": [ "monitor", "read", "view_index_metadata", "read_cross_cluster" ], "allow_restricted_indices": true, "clusters": [ "*" ] } ], "run_as": [ "*" ] }, "kibana": [ { "base": [ "all" ], "feature": {}, "spaces": [ "*" ] } ], "_transform_error": [], "_unrecognized_applications": [ "*" ] } ] ``` ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios ### For maintainers - [x] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) __Fixes: https://github.com/elastic/kibana/issues/180718__ --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

kibana-ci · 2024-04-30T15:19:52Z

💚 Build Succeeded

Buildkite Build
Commit: 21ee0be

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
`security`	574.6KB	574.9KB	+327.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`security`	67.6KB	67.8KB	+174.0B

Unknown metric groups

API count

id	before	after	diff
`security`	404	406	+2

History

💔 Build #206925 failed 46278dc
💔 Build #206846 failed abd7477
💔 Build #206828 failed d25cca8
💛 Build #206269 was flaky 2a1484a
💔 Build #206103 failed 87a1b19
💚 Build #205777 succeeded c8bc414

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

…lastic#181165) ## Summary Added endpoint `GET kbn:/internal/security/roles/{space-id}` to get all roles for provided space id. **Note**: changes needed for application `*` privileges were cherry-picked [to a separate PR].(elastic#181400) ## Example Request `GET kbn:/internal/security/roles/space-b` Response ``` [ { "name": "role-a", "metadata": {}, "transient_metadata": { "enabled": true }, "elasticsearch": { "cluster": [ "all" ], "indices": [], "run_as": [] }, "kibana": [ { "base": [], "feature": { "dev_tools": [ "all" ] }, "spaces": [ "default", "space-b" ] } ], "_transform_error": [], "_unrecognized_applications": [] }, { "name": "superuser", "metadata": { "_reserved": true }, "transient_metadata": {}, "elasticsearch": { "cluster": [ "all" ], "indices": [ { "names": [ "*" ], "privileges": [ "all" ], "allow_restricted_indices": false }, { "names": [ "*" ], "privileges": [ "monitor", "read", "view_index_metadata", "read_cross_cluster" ], "allow_restricted_indices": true } ], "remote_indices": [ { "names": [ "*" ], "privileges": [ "all" ], "allow_restricted_indices": false, "clusters": [ "*" ] }, { "names": [ "*" ], "privileges": [ "monitor", "read", "view_index_metadata", "read_cross_cluster" ], "allow_restricted_indices": true, "clusters": [ "*" ] } ], "run_as": [ "*" ] }, "kibana": [ { "base": [ "all" ], "feature": {}, "spaces": [ "*" ] } ], "_transform_error": [], "_unrecognized_applications": [ "*" ] } ] ``` ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios ### For maintainers - [x] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) __Fixes: https://github.com/elastic/kibana/issues/180718__ --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

…ic#181400) ## Summary Added transformation of application wildcard `*` privilege to `all` to correctly filter and display roles as `superuser`. <details> <summary>[Screenshot] Kibana superuser role before change</summary> <img width="1508" alt="Screenshot 2024-04-23 at 11 20 31" src="https://github.com/elastic/kibana/assets/165678770/594cf27c-64a7-49cf-bf4b-c63adca76a55"> </details> <details> <summary>[Screenshot] Kibana superuser role after change</summary> <img width="1504" alt="Screenshot 2024-04-23 at 11 17 54" src="https://github.com/elastic/kibana/assets/165678770/96dbdaa6-f5c9-4644-82ca-f88a0f6a15b3"> </details> ### Checklist - [X] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios ### For maintainers - [x] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) __Fixes: https://github.com/elastic/kibana/issues/106561__ ## Release note Added transformation of application wildcard `*` privilege to `all` to correctly filter and display roles as `superuser`. --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

jasonrhodes · 2024-05-06T18:28:49Z

Thank you all so much for this change, I'd been following an earlier issue that this PR closed and it was very happy news to see! <3

elena-shostak added bug Fixes for quality problems that affect the customer experience Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! Feature:Users/Roles/API Keys labels Apr 23, 2024

elena-shostak mentioned this pull request Apr 23, 2024

[Spaces] API endpoint for roles which have access to a given space #181165

Merged

2 tasks

elena-shostak marked this pull request as ready for review April 23, 2024 11:28

elena-shostak requested a review from a team as a code owner April 23, 2024 11:28

elena-shostak added the release_note:fix label Apr 23, 2024

jeramysoucy self-requested a review April 23, 2024 12:06

jeramysoucy requested changes Apr 24, 2024

View reviewed changes

elena-shostak force-pushed the 106561-superuser-role branch from 3225b5b to 5feadd8 Compare April 24, 2024 15:12

jeramysoucy requested changes Apr 25, 2024

View reviewed changes

...les/edit_role/privileges/kibana/space_aware_privilege_section/privilege_space_table.test.tsx Show resolved Hide resolved

jeramysoucy reviewed Apr 25, 2024

View reviewed changes

...ent/roles/edit_role/privileges/kibana/privilege_form_calculator/privilege_form_calculator.ts Show resolved Hide resolved

elena-shostak added 9 commits April 26, 2024 11:31

[Roles] Added transformation of application * privilege to all

9ab437e

[Roles] Moved wildcard normalization to UI

8652bba

Disallow editing wildcard app privilege

ffee1e3

[Roles] Disabled delete action

8d33ae8

[Roles] Added test

202f86a

fixup

a13aa77

[Roles] Disallowed edit actions for role with wildcard privilege

da44264

[Roles] Added test for PrivilegeFormCalculator

2513c78

[Roles] fixup

2a1484a

elena-shostak force-pushed the 106561-superuser-role branch from 1d2f18f to 2a1484a Compare April 26, 2024 09:32

jeramysoucy approved these changes Apr 29, 2024

View reviewed changes

x-pack/plugins/security/public/management/roles/edit_role/edit_role_page.test.tsx Outdated Show resolved Hide resolved

Updated test

f9b7fe5

Merge branch 'main' into 106561-superuser-role

d25cca8

Merge branch 'main' into 106561-superuser-role

abd7477

kibanamachine and others added 2 commits April 30, 2024 09:00

Merge branch 'main' into 106561-superuser-role

46278dc

Type fix

21ee0be

elena-shostak added the 8.15 candidate label Apr 30, 2024

elena-shostak merged commit 4023f92 into elastic:main Apr 30, 2024
19 checks passed

kibanamachine added v8.15.0 backport:skip This commit does not require backporting labels Apr 30, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Roles] Added transformation of application * privilege to all #181400

[Roles] Added transformation of application * privilege to all #181400

elena-shostak commented Apr 23, 2024 •

edited

Loading

elena-shostak commented Apr 23, 2024

elasticmachine commented Apr 23, 2024

jeramysoucy left a comment •

edited

Loading

legrego commented Apr 24, 2024

elena-shostak commented Apr 24, 2024

elena-shostak commented Apr 24, 2024 •

edited

Loading

jeramysoucy commented Apr 24, 2024 •

edited

Loading

elena-shostak commented Apr 24, 2024 •

edited

Loading

jeramysoucy left a comment •

edited

Loading

legrego commented Apr 25, 2024 •

edited

Loading

elena-shostak commented Apr 25, 2024 •

edited

Loading

jeramysoucy left a comment

elena-shostak commented Apr 30, 2024

elena-shostak commented Apr 30, 2024

elena-shostak commented Apr 30, 2024

kibana-ci commented Apr 30, 2024

API count

jasonrhodes commented May 6, 2024

[Roles] Added transformation of application * privilege to all #181400

[Roles] Added transformation of application * privilege to all #181400

Conversation

elena-shostak commented Apr 23, 2024 • edited Loading

Summary

Checklist

For maintainers

Release note

elena-shostak commented Apr 23, 2024

elasticmachine commented Apr 23, 2024

jeramysoucy left a comment • edited Loading

Choose a reason for hiding this comment

legrego commented Apr 24, 2024

elena-shostak commented Apr 24, 2024

elena-shostak commented Apr 24, 2024 • edited Loading

jeramysoucy commented Apr 24, 2024 • edited Loading

elena-shostak commented Apr 24, 2024 • edited Loading

jeramysoucy left a comment • edited Loading

Choose a reason for hiding this comment

legrego commented Apr 25, 2024 • edited Loading

elena-shostak commented Apr 25, 2024 • edited Loading

jeramysoucy left a comment

Choose a reason for hiding this comment

elena-shostak commented Apr 30, 2024

elena-shostak commented Apr 30, 2024

elena-shostak commented Apr 30, 2024

kibana-ci commented Apr 30, 2024

💚 Build Succeeded

Metrics [docs]

Async chunks

Page load bundle

API count

History

jasonrhodes commented May 6, 2024

elena-shostak commented Apr 23, 2024 •

edited

Loading

jeramysoucy left a comment •

edited

Loading

elena-shostak commented Apr 24, 2024 •

edited

Loading

jeramysoucy commented Apr 24, 2024 •

edited

Loading

elena-shostak commented Apr 24, 2024 •

edited

Loading

jeramysoucy left a comment •

edited

Loading

legrego commented Apr 25, 2024 •

edited

Loading

elena-shostak commented Apr 25, 2024 •

edited

Loading