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

elasticmachine · 2025-01-07T11:51:13Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: c132937

Failed CI Steps

FTR Configs #37

Test Failures

[job] [logs] FTR Configs #37 / Observability AI Assistant API tests knowledge_base/knowledge_base_user_instructions.spec.ts Knowledge base user instructions when a user instruction exists and a conversation is created does not add the instruction conversation for other users

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 #264411 failed f0c1c9b
💔 Build #264381 failed 45cfb91
💚 Build #263439 succeeded 47c7a6f
💛 Build #263102 was flaky b3f1efd
💔 Build #262967 failed 6c631b4

elena-shostak added Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! release_note:fix backport:prev-minor Backport to (8.x) 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

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

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

elasticmachine commented Jan 7, 2025

Fleet Privileges Display #204402

Are you sure you want to change the base?

Fleet Privileges Display #204402

Conversation

elena-shostak commented Dec 16, 2024 • edited 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

elasticmachine commented Jan 7, 2025

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

Async chunks

History

elena-shostak commented Dec 16, 2024 •

edited

Loading