Fleet Privileges Display #204402

elena-shostak · 2024-12-16T13:49:18Z

Summary

Fixed privileges display for features/subFeatures that require all spaces.

Before

Role privileges display for only Default space selected

Privileges summary display for only Default space selected

After

Role privileges display for only Default space selected

Privileges summary display for only Default space selected

How to test

With Default space:

Navigate to Creating a new Role and assign Kibana privileges.
Set the Spaces to Default Space and the privilege level to All.
Navigate to Management category and verify that Fleet is set to None.
Click on "View privilege summary" and verify that Fleet is set to None.

With *All Spaces:

Navigate to Creating a new Role and assign Kibana privileges.
Set the Spaces to *All Spaces and the privilege level to All.
Navigate to Management category and verify that Fleet is set to All
Click on "View privilege summary" and verify that Fleet is set to All

Checklist

Check the PR satisfies following conditions.

Unit or functional tests were updated or added to match the most common scenarios
The PR description includes the appropriate Release Notes section, and the correct release_note:* label is applied per the guidelines

Fixes: #194686

Release Note

Fixed privileges display for features/subFeatures that require all spaces.

elena-shostak · 2024-12-16T13:49:32Z

/ci

elena-shostak · 2024-12-16T18:22:28Z

/ci

elena-shostak · 2024-12-17T12:43:47Z

@elasticmachine merge upstream

elena-shostak · 2024-12-17T12:50:09Z

/ci

elasticmachine · 2024-12-17T14:44:40Z

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

azasypkin · 2024-12-24T12:15:48Z

ACK: will review today, sorry for the delay!

azasypkin · 2024-12-24T13:18:59Z

.../edit_role/privileges/kibana/space_aware_privilege_section/space_aware_privilege_section.tsx

@@ -218,7 +218,7 @@ export class SpaceAwarePrivilegeSection extends Component<Props, State> {
    const viewMatrixButton = (
      <PrivilegeSummary
        role={this.props.role}
-        spaces={this.getDisplaySpaces()}
+        spaces={this.getSelectedSpaces()}


Caution
There is something definitely off here when I select different access variations. See the video. Do you see the same?

Screen.Recording.2024-12-24.at.14.16.13.mov

Confirmed, can reproduce, will take a look

6c631b4 pushed fix, but please hold on review for now, need to check tests

@azasypkin ready for review 🙂

Nice! I'll review on Monday

It looks like we have another issue now. If Read/All access to Fleet is granted through a Kibana privilege entry with “All Spaces”, the privilege summary for any space-specific privileges should reflect what is granted by the “All Spaces” privilege.

Screen.Recording.2024-12-30.at.16.01.07.mov

Screen.Recording.2024-12-31.at.11.34.58.mov

@azasypkin pushed a fix and checked it with multiple spaces, should work as expected 🙌

elena-shostak · 2024-12-27T09:36:08Z

@elasticmachine merge upstream

azasypkin · 2024-12-30T13:51:49Z

...c/management/roles/edit_role/privileges/kibana/privilege_summary/privilege_summary_table.tsx

@@ -52,7 +52,7 @@ function showPrivilege(allSpacesSelected: boolean, primaryFeature?: PrimaryFeatu
  if (
    primaryFeature?.name == null ||
    primaryFeature?.disabled ||
-    (primaryFeature.requireAllSpaces && !allSpacesSelected)
+    (primaryFeature?.requireAllSpaces && !allSpacesSelected)
  ) {
    return 'None';


question: I know it was here before your PR, but shouldn't None be a localized string instead of hardcoded English one?

As far as I see we don't localize other privileges as well (Read, All). And some feature/subfeature names.

Interesting, thanks for checking. I wonder if that was a deliberate decision. In any case, it’s outside the scope of this PR.

azasypkin · 2024-12-30T13:57:04Z

...c/management/roles/edit_role/privileges/kibana/privilege_summary/privilege_summary_table.tsx

@@ -210,11 +212,20 @@ export const PrivilegeSummaryTable = (props: PrivilegeSummaryTableProps) => {
      </EuiFlexGroup>
    );

+    const categoryPrivileges = Object.keys(privileges).reduce((acc, key) => {


optional nit: since you retrieve value using the key anyway, Object.values might have been a slightly better choice. Or even something like this would be simpler:

const categoryPrivileges = Object.fromEntries( Object.entries(privileges).map(([key, [, featurePrivileges]]) => [key, featurePrivileges]) );

azasypkin · 2024-12-30T15:19:40Z

.../edit_role/privileges/kibana/space_aware_privilege_section/space_aware_privilege_section.tsx

@@ -218,7 +218,7 @@ export class SpaceAwarePrivilegeSection extends Component<Props, State> {
    const viewMatrixButton = (
      <PrivilegeSummary
        role={this.props.role}
-        spaces={this.getDisplaySpaces()}
+        spaces={this.getSelectedSpaces()}


It looks like we have another issue now. If Read/All access to Fleet is granted through a Kibana privilege entry with “All Spaces”, the privilege summary for any space-specific privileges should reflect what is granted by the “All Spaces” privilege.

Screen.Recording.2024-12-30.at.16.01.07.mov

azasypkin

Looks like we have a new weird behavior 😬

There are so many edge cases, and it’s very tricky to handle them all. It would be even trickier to ensure they aren’t accidentally broken in the future. Thanks for updating the tests to include more cases. We might need to incorporate all these cases into our unit or functional tests, even if they are specifically around Fleet.

Screen.Recording.2025-01-06.at.15.41.15.mov

elena-shostak · 2025-01-07T10:23:57Z

@elasticmachine merge upstream

azasypkin

Looks great, thanks! Tested various scenarios and everything worked as expected.

azasypkin · 2025-01-08T13:50:27Z

...c/management/roles/edit_role/privileges/kibana/privilege_summary/privilege_summary_table.tsx

@@ -48,14 +48,27 @@ function getColumnKey(entry: RoleKibanaPrivilege) {
  return `privilege_entry_${entry.spaces.join('|')}`;
 }

-function showPrivilege(allSpacesSelected: boolean, primaryFeature?: PrimaryFeaturePrivilege) {
+function showPrivilege({


🎖️ Thanks for switching to object/struct arguments! Using booleans directly in function arguments makes it really hard to follow the flow.

azasypkin · 2025-01-08T14:12:02Z

...c/management/roles/edit_role/privileges/kibana/privilege_summary/privilege_summary_table.tsx

  ) {
    return 'None';
  }
+
+  if (primaryFeature?.requireAllSpaces && allSpacesSelected) {
+    return globalPrimaryFeature?.name ?? 'None';


nit: it'd be helpful if you can add a comment here with the example when the primary feature requires all spaces, all spaces are selected, but we cannot rely on then name of the primary feature and should use the name of the global primary feature instead

azasypkin · 2025-01-08T14:14:11Z

...c/management/roles/edit_role/privileges/kibana/privilege_summary/privilege_summary_table.tsx

@@ -210,11 +235,20 @@ export const PrivilegeSummaryTable = (props: PrivilegeSummaryTableProps) => {
      </EuiFlexGroup>
    );

+    const categoryPrivileges = Object.keys(privileges).reduce((acc, key) => {


question: any reason this suggestion didn't work? #204402 (comment)

it worked, I thought I actually committed the change, will push it

elena-shostak · 2025-01-09T08:55:41Z

@elasticmachine merge upstream

elasticmachine · 2025-01-09T11:01:52Z

💚 Build Succeeded

Buildkite Build
Commit: 061bc06

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`	542.2KB	543.3KB	+1.1KB
`spaces`	257.0KB	257.0KB	+76.0B
total			+1.1KB

History

💛 Build #264673 was flaky c132937
💔 Build #264411 failed f0c1c9b
💔 Build #264381 failed 45cfb91
💚 Build #263439 succeeded 47c7a6f
💛 Build #263102 was flaky b3f1efd
💔 Build #262967 failed 6c631b4

kibanamachine · 2025-01-09T13:23:10Z

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/12690767211

## Summary Fixed privileges display for features/subFeatures that require all spaces. ### Before Role privileges display for only `Default` space selected <img width="728" alt="Screenshot 2024-12-17 at 13 32 17" src="https://github.com/user-attachments/assets/151b7012-aa1a-430c-be22-cc91e64362e3" /> Privileges summary display for only `Default` space selected <img width="471" alt="Screenshot 2024-12-17 at 13 32 50" src="https://github.com/user-attachments/assets/964c2223-163d-4081-a37d-196f5df5df5c" /> ### After Role privileges display for only `Default` space selected <img width="739" alt="Screenshot 2024-12-17 at 13 30 00" src="https://github.com/user-attachments/assets/0f98a9d7-211d-46ec-82c6-25d29a44be6b" /> Privileges summary display for only `Default` space selected <img width="569" alt="Screenshot 2024-12-17 at 13 30 19" src="https://github.com/user-attachments/assets/932771fd-6486-4b7e-9de5-6cd34ab74dc9" /> ### How to test With `Default` space: 1. Navigate to Creating a new Role and assign Kibana privileges. 2. Set the Spaces to `Default` Space and the privilege level to All. 3. Navigate to Management category and verify that Fleet is set to `None`. 4. Click on "View privilege summary" and verify that Fleet is set to `None`. With `*All Spaces`: 1. Navigate to Creating a new Role and assign Kibana privileges. 2. Set the Spaces to `*All Spaces` and the privilege level to All. 3. Navigate to Management category and verify that Fleet is set to `All` 4. Click on "View privilege summary" and verify that Fleet is set to `All` ### Checklist Check the PR satisfies following conditions. - [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 - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) __Fixes: https://github.com/elastic/kibana/issues/194686__ ## Release Note Fixed privileges display for features/subFeatures that require all spaces. --------- Co-authored-by: Elastic Machine <[email protected]> (cherry picked from commit d4196cd)

kibanamachine · 2025-01-09T13:28:19Z

💚 All backports created successfully

Status	Branch	Result
✅	8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

# Backport This will backport the following commits from `main` to `8.x`: - [Fleet Privileges Display (#204402)](#204402)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Elena Shostak <[email protected]>

## Summary Fixed privileges display for features/subFeatures that require all spaces. ### Before Role privileges display for only `Default` space selected <img width="728" alt="Screenshot 2024-12-17 at 13 32 17" src="https://github.com/user-attachments/assets/151b7012-aa1a-430c-be22-cc91e64362e3" /> Privileges summary display for only `Default` space selected <img width="471" alt="Screenshot 2024-12-17 at 13 32 50" src="https://github.com/user-attachments/assets/964c2223-163d-4081-a37d-196f5df5df5c" /> ### After Role privileges display for only `Default` space selected <img width="739" alt="Screenshot 2024-12-17 at 13 30 00" src="https://github.com/user-attachments/assets/0f98a9d7-211d-46ec-82c6-25d29a44be6b" /> Privileges summary display for only `Default` space selected <img width="569" alt="Screenshot 2024-12-17 at 13 30 19" src="https://github.com/user-attachments/assets/932771fd-6486-4b7e-9de5-6cd34ab74dc9" /> ### How to test With `Default` space: 1. Navigate to Creating a new Role and assign Kibana privileges. 2. Set the Spaces to `Default` Space and the privilege level to All. 3. Navigate to Management category and verify that Fleet is set to `None`. 4. Click on "View privilege summary" and verify that Fleet is set to `None`. With `*All Spaces`: 1. Navigate to Creating a new Role and assign Kibana privileges. 2. Set the Spaces to `*All Spaces` and the privilege level to All. 3. Navigate to Management category and verify that Fleet is set to `All` 4. Click on "View privilege summary" and verify that Fleet is set to `All` ### Checklist Check the PR satisfies following conditions. - [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 - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) __Fixes: https://github.com/elastic/kibana/issues/194686__ ## Release Note Fixed privileges display for features/subFeatures that require all spaces. --------- Co-authored-by: Elastic Machine <[email protected]>

elena-shostak added Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! release_note:fix backport:prev-minor Backport to (9.0) the previous minor version (i.e. one version back from main) Feature:Users/Roles/API Keys labels Dec 17, 2024

elena-shostak marked this pull request as ready for review December 17, 2024 14:44

elena-shostak requested a review from a team as a code owner December 17, 2024 14:44

azasypkin self-requested a review December 20, 2024 14:37

azasypkin reviewed Dec 24, 2024

View reviewed changes

azasypkin self-requested a review December 27, 2024 13:47

azasypkin reviewed Dec 30, 2024

View reviewed changes

elena-shostak force-pushed the 194686-fleet-privileges branch from 64cb61d to 017dad8 Compare December 31, 2024 11:06

azasypkin self-requested a review January 3, 2025 12:35

elena-shostak force-pushed the 194686-fleet-privileges branch from 017dad8 to ab901f5 Compare January 6, 2025 13:42

azasypkin reviewed Jan 6, 2025

View reviewed changes

elena-shostak added 9 commits January 6, 2025 17:01

Fleet privileges display fixes

1be1c8d

Test

93d8926

jest test updates

ce36a81

Test updates

4ca4dd9

Fleet privileges display fix

5738148

Test fixes

347a678

Fixes

4efce4e

Fixes

47ad806

Fix

4666e00

Tests

f0c1c9b

elena-shostak force-pushed the 194686-fleet-privileges branch from 45cfb91 to f0c1c9b Compare January 6, 2025 16:03

Merge branch 'main' into 194686-fleet-privileges

c132937

elena-shostak requested a review from azasypkin January 7, 2025 11:51

azasypkin approved these changes Jan 8, 2025

View reviewed changes

Fixes

f9bdbaf

Merge branch 'main' into 194686-fleet-privileges

061bc06

elena-shostak merged commit d4196cd into elastic:main Jan 9, 2025
8 checks passed

kibanamachine added the v9.0.0 label Jan 9, 2025

kibanamachine mentioned this pull request Jan 9, 2025

[8.x] Fleet Privileges Display (#204402) #206055

Merged

kibanamachine added the v8.18.0 label Jan 9, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fleet Privileges Display #204402

Fleet Privileges Display #204402

elena-shostak commented Dec 16, 2024 •

edited by kibanamachine

Loading

elena-shostak commented Dec 16, 2024

elena-shostak commented Dec 16, 2024

elena-shostak commented Dec 17, 2024

elena-shostak commented Dec 17, 2024

elasticmachine commented Dec 17, 2024

azasypkin commented Dec 24, 2024

azasypkin Dec 24, 2024

elena-shostak Dec 24, 2024

elena-shostak Dec 24, 2024

elena-shostak Dec 27, 2024

azasypkin Dec 27, 2024

azasypkin Dec 30, 2024

elena-shostak Dec 31, 2024

elena-shostak commented Dec 27, 2024

azasypkin Dec 30, 2024

elena-shostak Dec 31, 2024

azasypkin Dec 31, 2024

azasypkin Dec 30, 2024

azasypkin Dec 30, 2024

azasypkin left a comment

elena-shostak commented Jan 7, 2025

azasypkin left a comment

azasypkin Jan 8, 2025

azasypkin Jan 8, 2025

azasypkin Jan 8, 2025

elena-shostak Jan 8, 2025

elena-shostak commented Jan 9, 2025

elasticmachine commented Jan 9, 2025

kibanamachine commented Jan 9, 2025

kibanamachine commented Jan 9, 2025

Fleet Privileges Display #204402

Fleet Privileges Display #204402

Conversation

elena-shostak commented Dec 16, 2024 • edited by kibanamachine Loading

Summary

Before

After

How to test

Checklist

Release Note

elena-shostak commented Dec 16, 2024

elena-shostak commented Dec 16, 2024

elena-shostak commented Dec 17, 2024

elena-shostak commented Dec 17, 2024

elasticmachine commented Dec 17, 2024

azasypkin commented Dec 24, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elena-shostak commented Dec 27, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

azasypkin left a comment

Choose a reason for hiding this comment

elena-shostak commented Jan 7, 2025

azasypkin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elena-shostak commented Jan 9, 2025

elasticmachine commented Jan 9, 2025

💚 Build Succeeded

Metrics [docs]

Async chunks

History

kibanamachine commented Jan 9, 2025

kibanamachine commented Jan 9, 2025

💚 All backports created successfully

Questions ?

elena-shostak commented Dec 16, 2024 •

edited by kibanamachine

Loading