[Spaces and Roles] Fix rules on showing `Permissions` tab #196442

tsullivan · 2024-10-15T20:43:32Z

Summary

Some deployment types don't have custom roles. This PR hides the Permissions tab in Spaces Management for projects without role management enabled. The solution uses the isRoleManagementEnabled function from the security plugin’s authz service for this.

…) === true

…fix'

jeramysoucy · 2024-10-16T07:24:29Z

x-pack/plugins/spaces/public/management/edit_space/edit_space_tabs.tsx

@@ -92,7 +93,11 @@ export const getTabs = ({
    },
  ];

-  if (canUserViewRoles) {
+  const canUserViewRoles = Boolean(capabilities?.roles?.view);
+  const isRoleManagementEnabled = authz && authz.isRoleManagementEnabled() !== false;


Suggested change

const isRoleManagementEnabled = authz && authz.isRoleManagementEnabled() !== false;

const isRoleManagementEnabled = authz && authz.isRoleManagementEnabled();

or

Suggested change

const isRoleManagementEnabled = authz && authz.isRoleManagementEnabled() !== false;

const isRoleManagementEnabled = authz && authz.isRoleManagementEnabled() === true;

@jeramysoucy I think we examined this further in Slack, and agreed that the config.roleManagementEnabled is only available in serverless, which makes the authz.isRoleManagementEnabled() return undefined in stateful, when it actually should be interpreted as being enabled.

So how about:

/* * config.roleManagementEnabled is an offering-based config. In serverless, the value will be true or false. * In stateful, role management is enabled but the config setting is expected to be undefined. */ const isRoleManagementEnabled = authz && (typeof authz.isRoleManagementEnabled() === 'undefined' || authz.isRoleManagementEnabled());

Alternatively, we could update the implementation of authz.isRoleManagementEnabled in

kibana/x-pack/plugins/security/public/authorization/authorization_service.ts

Line 21 in 4339f84

const isRoleManagementEnabled = () => config.roleManagementEnabled;

Moved this logic to x-pack/plugins/spaces/public/management/edit_space/provider/edit_space_provider.tsx

Thanks, Tim! Sorry for the confusion.

eokoneyo

Left some nit comments

x-pack/plugins/spaces/public/management/spaces_management_app.tsx

x-pack/plugins/spaces/tsconfig.json

elasticmachine · 2024-10-16T19:51:29Z

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

jeramysoucy

LGTM. Just one nit/suggestion to add a unit test case.

jeramysoucy · 2024-10-17T07:55:09Z

x-pack/plugins/spaces/public/management/edit_space/edit_space_roles_tab.test.tsx

Nit: we could a test here to confirm that the Permissions tab does not render when getIsRoleManagementEnabled resolves false.

Wanted to avoid merge before feedback is reviewed.

tsullivan · 2024-10-17T20:24:19Z

@jeramysoucy I have addressed the feedback by adding tests to unit tests that deal with the permissions tab.

I have also updated the code because the original fix didn't account for an HTTP request that results in a 404 when role management is disabled: 3795630

…fix'

elasticmachine · 2024-10-17T22:24:31Z

💚 Build Succeeded

Buildkite Build
Commit: 9efeb59

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
`spaces`	256.2KB	256.4KB	+234.0B

Page load bundle

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

id	before	after	diff
`spaces`	33.7KB	34.1KB	+347.0B

History

💚 Build #243279 succeeded eeb43a8
💔 Build #243213 failed db8c2f9
💔 Build #242914 failed a35c0d3
💔 Build #242898 failed 783a973
💔 Build #242872 failed 5b58f75

jeramysoucy

Thanks!

kibanamachine · 2024-10-18T15:40:19Z

Starting backport for target branches: 8.16, 8.x

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

kibanamachine · 2024-10-18T15:44:54Z

💔 All backports failed

Status	Branch	Result
❌	8.16	Backport failed because of merge conflicts
❌	8.x	Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 196442

Questions ?

Please refer to the Backport tool documentation

…6442) ## Summary Some deployment types don't have custom roles. This PR hides the `Permissions` tab in Spaces Management for projects without role management enabled. The solution uses the isRoleManagementEnabled function from the security plugin’s authz service for this. --------- Co-authored-by: kibanamachine <[email protected]> (cherry picked from commit 4ed9e51)

tsullivan · 2024-10-18T18:45:00Z

💚 All backports created successfully

Status	Branch	Result
✅	8.x
✅	8.16

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

Questions ?

Please refer to the Backport tool documentation

tsullivan and others added 3 commits October 15, 2024 13:00

Fix: “Permission” tab should appear if authz.isRoleManagementEnabled(…

6ca1965

…) === true

refine the fix

5b58f75

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

783a973

…fix'

tsullivan added release_note:skip Skip the PR/issue when compiling release notes v8.16.0 backport:version Backport to applied version labels Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! labels Oct 15, 2024

Merge branch 'main' into spaces/use-role-management-flag

a35c0d3

jeramysoucy reviewed Oct 16, 2024

View reviewed changes

eokoneyo reviewed Oct 16, 2024

View reviewed changes

x-pack/plugins/spaces/public/management/spaces_management_app.tsx Outdated Show resolved Hide resolved

x-pack/plugins/spaces/tsconfig.json Outdated Show resolved Hide resolved

tsullivan added 3 commits October 16, 2024 08:33

Merge branch 'main' into spaces/use-role-management-flag

3cdb292

proper use of flag for stateful

6faaacc

fix tests

db8c2f9

tsullivan force-pushed the spaces/use-role-management-flag branch from 85247fd to db8c2f9 Compare October 16, 2024 16:37

Switch to using an isRoleManagementEnabled boolean field

eeb43a8

tsullivan force-pushed the spaces/use-role-management-flag branch from 4e26ce3 to eeb43a8 Compare October 16, 2024 19:43

tsullivan marked this pull request as ready for review October 16, 2024 19:51

tsullivan requested a review from a team as a code owner October 16, 2024 19:51

jeramysoucy previously approved these changes Oct 17, 2024

View reviewed changes

tsullivan added 3 commits October 17, 2024 11:23

Merge branch 'main' into spaces/use-role-management-flag

67537da

Ensure that roles are not fetched if role management is disabled

3795630

improve tests

c64a87d

tsullivan requested review from eokoneyo and jeramysoucy October 17, 2024 20:24

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

9efeb59

…fix'

tsullivan added the v8.17.0 label Oct 17, 2024

jeramysoucy approved these changes Oct 18, 2024

View reviewed changes

tsullivan merged commit 4ed9e51 into elastic:main Oct 18, 2024
24 checks passed

tsullivan deleted the spaces/use-role-management-flag branch October 18, 2024 15:40

kibanamachine added the v9.0.0 label Oct 18, 2024

tsullivan mentioned this pull request Oct 18, 2024

[8.x] [Spaces and Roles] Fix rules on showing Permissions tab (#196442) #196937

Open

tsullivan mentioned this pull request Oct 18, 2024

[8.16] [Spaces and Roles] Fix rules on showing Permissions tab (#196442) #196938

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Spaces and Roles] Fix rules on showing `Permissions` tab #196442

[Spaces and Roles] Fix rules on showing `Permissions` tab #196442

tsullivan commented Oct 15, 2024 •

edited by kibanamachine

Loading

jeramysoucy Oct 16, 2024 •

edited

Loading

tsullivan Oct 16, 2024

tsullivan Oct 16, 2024

jeramysoucy Oct 17, 2024

eokoneyo left a comment

elasticmachine commented Oct 16, 2024

jeramysoucy left a comment

jeramysoucy Oct 17, 2024

tsullivan commented Oct 17, 2024

elasticmachine commented Oct 17, 2024

jeramysoucy left a comment

kibanamachine commented Oct 18, 2024

kibanamachine commented Oct 18, 2024

tsullivan commented Oct 18, 2024

	const isRoleManagementEnabled = authz && authz.isRoleManagementEnabled() !== false;
	const isRoleManagementEnabled = authz && authz.isRoleManagementEnabled();

[Spaces and Roles] Fix rules on showing Permissions tab #196442

[Spaces and Roles] Fix rules on showing Permissions tab #196442

Conversation

tsullivan commented Oct 15, 2024 • edited by kibanamachine Loading

Summary

jeramysoucy Oct 16, 2024 • edited Loading

Choose a reason for hiding this comment

tsullivan Oct 16, 2024

Choose a reason for hiding this comment

tsullivan Oct 16, 2024

Choose a reason for hiding this comment

jeramysoucy Oct 17, 2024

Choose a reason for hiding this comment

eokoneyo left a comment

Choose a reason for hiding this comment

elasticmachine commented Oct 16, 2024

jeramysoucy left a comment

Choose a reason for hiding this comment

jeramysoucy Oct 17, 2024

Choose a reason for hiding this comment

tsullivan commented Oct 17, 2024

elasticmachine commented Oct 17, 2024

💚 Build Succeeded

Metrics [docs]

Async chunks

Page load bundle

History

jeramysoucy left a comment

Choose a reason for hiding this comment

kibanamachine commented Oct 18, 2024

kibanamachine commented Oct 18, 2024

💔 All backports failed

Manual backport

Questions ?

tsullivan commented Oct 18, 2024

💚 All backports created successfully

Questions ?

[Spaces and Roles] Fix rules on showing `Permissions` tab #196442

[Spaces and Roles] Fix rules on showing `Permissions` tab #196442

tsullivan commented Oct 15, 2024 •

edited by kibanamachine

Loading

jeramysoucy Oct 16, 2024 •

edited

Loading