Skip to content

Commit

Permalink
[8.x] [Spaces UI] Role Editor Flyout Should Match in Roles Mgmt (#198182
Browse files Browse the repository at this point in the history
) (#202820)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Spaces UI] Role Editor Flyout Should Match in Roles Mgmt
(#198182)](#198182)

<!--- Backport version: 8.9.8 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Tim
Sullivan","email":"tsullivan@users.noreply.github.com"},"sourceCommit":{"committedDate":"2024-11-20T14:39:41Z","message":"[Spaces
UI] Role Editor Flyout Should Match in Roles Mgmt (#198182)\n\n##
Summary\r\n\r\nPart of
https://github.com/elastic/kibana-team/issues/1242\r\n\r\n**Fixes for
alignment of the Role editor flyout**\r\n1. Remove the warning callout
regarding global privileges that impact\r\nother privileges\r\n1. Unify
the info callouts regarding combination of privileges\r\n1. set
\"Customize\" as the default selected option when assigning
new\r\nprivileges\r\n1. update placeholders for selector box when
assigning privileges\r\n1. Hide privileges controls if no spaces are
selected\r\n1. Update button group label text to \"Define privileges\"
and align\r\nhelper texts below\r\n1. Align headers for assign/edit
states\r\n1. Remove descriptions under headers\r\n1. Update size of info
callout above button group to small\r\n1. Reduce text size for the
\"Manage roles\" link\r\n1. Remove the \"Additional Stack Management
permissions can be found\r\noutside of this menu...\" test for the
Spaces Management context.\r\n\r\n**Polish fixes**\r\n1. Remove features
visible column\r\n1. ~~Remove identifier column from spaces grid~~\r\n1.
Fix vertical alignment of non-current space name in table\r\n1. Ordered
the listing of assigned roles during and after search\r\n1. Removing a
role from the space shows a confirmation modal\r\n1. Update columns
widths in the spaces grid\r\n1. Remove the \"By default your current
view is Classic\" callout\r\n\r\n### Checklist\r\n\r\nDelete any items
that are not applicable to this PR.\r\n\r\n- [x] Any text added follows
[EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [x] Any UI
touched in this PR is usable by keyboard only (learn more\r\nabout
[keyboard accessibility](https://webaim.org/techniques/keyboard/))\r\n-
[x] Any UI touched in this PR does not create any new axe
failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n-
[x] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n-
[x] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)","sha":"226924eafebff8852a67723a49e8f4fdbb6ed869","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","backport
missing","v9.0.0","ci:cloud-deploy","backport:version","v8.17.0"],"number":198182,"url":"https://github.com/elastic/kibana/pull/198182","mergeCommit":{"message":"[Spaces
UI] Role Editor Flyout Should Match in Roles Mgmt (#198182)\n\n##
Summary\r\n\r\nPart of
https://github.com/elastic/kibana-team/issues/1242\r\n\r\n**Fixes for
alignment of the Role editor flyout**\r\n1. Remove the warning callout
regarding global privileges that impact\r\nother privileges\r\n1. Unify
the info callouts regarding combination of privileges\r\n1. set
\"Customize\" as the default selected option when assigning
new\r\nprivileges\r\n1. update placeholders for selector box when
assigning privileges\r\n1. Hide privileges controls if no spaces are
selected\r\n1. Update button group label text to \"Define privileges\"
and align\r\nhelper texts below\r\n1. Align headers for assign/edit
states\r\n1. Remove descriptions under headers\r\n1. Update size of info
callout above button group to small\r\n1. Reduce text size for the
\"Manage roles\" link\r\n1. Remove the \"Additional Stack Management
permissions can be found\r\noutside of this menu...\" test for the
Spaces Management context.\r\n\r\n**Polish fixes**\r\n1. Remove features
visible column\r\n1. ~~Remove identifier column from spaces grid~~\r\n1.
Fix vertical alignment of non-current space name in table\r\n1. Ordered
the listing of assigned roles during and after search\r\n1. Removing a
role from the space shows a confirmation modal\r\n1. Update columns
widths in the spaces grid\r\n1. Remove the \"By default your current
view is Classic\" callout\r\n\r\n### Checklist\r\n\r\nDelete any items
that are not applicable to this PR.\r\n\r\n- [x] Any text added follows
[EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [x] Any UI
touched in this PR is usable by keyboard only (learn more\r\nabout
[keyboard accessibility](https://webaim.org/techniques/keyboard/))\r\n-
[x] Any UI touched in this PR does not create any new axe
failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n-
[x] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n-
[x] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)","sha":"226924eafebff8852a67723a49e8f4fdbb6ed869"}},"sourceBranch":"main","suggestedTargetBranches":["8.17"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/198182","number":198182,"mergeCommit":{"message":"[Spaces
UI] Role Editor Flyout Should Match in Roles Mgmt (#198182)\n\n##
Summary\r\n\r\nPart of
https://github.com/elastic/kibana-team/issues/1242\r\n\r\n**Fixes for
alignment of the Role editor flyout**\r\n1. Remove the warning callout
regarding global privileges that impact\r\nother privileges\r\n1. Unify
the info callouts regarding combination of privileges\r\n1. set
\"Customize\" as the default selected option when assigning
new\r\nprivileges\r\n1. update placeholders for selector box when
assigning privileges\r\n1. Hide privileges controls if no spaces are
selected\r\n1. Update button group label text to \"Define privileges\"
and align\r\nhelper texts below\r\n1. Align headers for assign/edit
states\r\n1. Remove descriptions under headers\r\n1. Update size of info
callout above button group to small\r\n1. Reduce text size for the
\"Manage roles\" link\r\n1. Remove the \"Additional Stack Management
permissions can be found\r\noutside of this menu...\" test for the
Spaces Management context.\r\n\r\n**Polish fixes**\r\n1. Remove features
visible column\r\n1. ~~Remove identifier column from spaces grid~~\r\n1.
Fix vertical alignment of non-current space name in table\r\n1. Ordered
the listing of assigned roles during and after search\r\n1. Removing a
role from the space shows a confirmation modal\r\n1. Update columns
widths in the spaces grid\r\n1. Remove the \"By default your current
view is Classic\" callout\r\n\r\n### Checklist\r\n\r\nDelete any items
that are not applicable to this PR.\r\n\r\n- [x] Any text added follows
[EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [x] Any UI
touched in this PR is usable by keyboard only (learn more\r\nabout
[keyboard accessibility](https://webaim.org/techniques/keyboard/))\r\n-
[x] Any UI touched in this PR does not create any new axe
failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n-
[x] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n-
[x] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)","sha":"226924eafebff8852a67723a49e8f4fdbb6ed869"}},{"branch":"8.x","label":"v8.17.0","labelRegex":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->
  • Loading branch information
tsullivan authored Dec 5, 2024
1 parent aaf6e4f commit 3f8bbfa
Show file tree
Hide file tree
Showing 18 changed files with 273 additions and 497 deletions.
2 changes: 1 addition & 1 deletion x-pack/packages/security/ui_components/kibana.jsonc
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"type": "shared-common",
"type": "shared-browser",
"id": "@kbn/security-ui-components",
"owner": "@elastic/kibana-security",
"group": "platform",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ const setup = (config: TestConfig) => {
kibanaPrivileges={kibanaPrivileges}
onChange={onChange}
onChangeAll={onChangeAll}
showAdditionalPermissionsMessage={true}
canCustomizeSubFeaturePrivileges={config.canCustomizeSubFeaturePrivileges}
privilegeIndex={config.privilegeIndex}
allSpacesSelected={true}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,10 @@ interface Props {
privilegeIndex: number;
onChange: (featureId: string, privileges: string[]) => void;
onChangeAll: (privileges: string[]) => void;
showAdditionalPermissionsMessage: boolean;
canCustomizeSubFeaturePrivileges: boolean;
allSpacesSelected: boolean;
disabled?: boolean;
/**
* default is true, to remain backwards compatible
*/
showTitle?: boolean;
}

interface State {
Expand All @@ -62,7 +59,6 @@ export class FeatureTable extends Component<Props, State> {
public static defaultProps = {
privilegeIndex: -1,
showLocks: true,
showTitle: true,
};

private featureCategories: Map<string, SecuredFeature[]> = new Map();
Expand Down Expand Up @@ -189,20 +185,7 @@ export class FeatureTable extends Component<Props, State> {
return (
<div>
<EuiFlexGroup alignItems={'flexEnd'}>
<EuiFlexItem>
{this.props.showTitle && (
<EuiText size="xs">
<b>
{i18n.translate(
'xpack.security.management.editRole.featureTable.featureVisibilityTitle',
{
defaultMessage: 'Customize feature privileges',
}
)}
</b>
</EuiText>
)}
</EuiFlexItem>
<EuiFlexItem />
{!this.props.disabled && (
<EuiFlexItem grow={false}>
<ChangeAllPrivilegesControl
Expand Down Expand Up @@ -397,7 +380,7 @@ export class FeatureTable extends Component<Props, State> {
};

private getCategoryHelpText = (category: AppCategory) => {
if (category.id === 'management') {
if (category.id === 'management' && this.props.showAdditionalPermissionsMessage) {
return i18n.translate(
'xpack.security.management.editRole.featureTable.managementCategoryHelpText',
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@ export class SimplePrivilegeSection extends Component<Props, State> {
privilegeIndex={this.props.role.kibana.findIndex((k) =>
isGlobalPrivilegeDefinition(k)
)}
showAdditionalPermissionsMessage={true}
canCustomizeSubFeaturePrivileges={this.props.canCustomizeSubFeaturePrivileges}
allSpacesSelected
disabled={!this.props.editable}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import type { Space } from '@kbn/spaces-plugin/public';
import { findTestSubject, mountWithIntl } from '@kbn/test-jest-helpers';

import { PrivilegeSpaceForm } from './privilege_space_form';
import { SpaceSelector } from './space_selector';
import type { Role } from '../../../../../../../common';

const createRole = (kibana: Role['kibana'] = []): Role => {
Expand Down Expand Up @@ -57,7 +56,7 @@ const renderComponent = (props: React.ComponentProps<typeof PrivilegeSpaceForm>)
};

describe('PrivilegeSpaceForm', () => {
it('renders an empty form when the role contains no Kibana privileges', () => {
it('renders no form when no role is selected', () => {
const role = createRole();
const kibanaPrivileges = createKibanaPrivileges(kibanaFeatures);

Expand All @@ -71,40 +70,9 @@ describe('PrivilegeSpaceForm', () => {
onCancel: jest.fn(),
});

expect(
wrapper.find(EuiButtonGroup).filter('[name="basePrivilegeButtonGroup"]').props().idSelected
).toEqual(`basePrivilege_custom`);
expect(wrapper.find(FeatureTable).props().disabled).toEqual(true);
expect(getDisplayedFeaturePrivileges(wrapper)).toMatchInlineSnapshot(`
Object {
"excluded_from_base": Object {
"primaryFeaturePrivilege": "none",
"subFeaturePrivileges": Array [],
},
"no_sub_features": Object {
"primaryFeaturePrivilege": "none",
"subFeaturePrivileges": Array [],
},
"with_excluded_sub_features": Object {
"primaryFeaturePrivilege": "none",
"subFeaturePrivileges": Array [],
},
"with_require_all_spaces_for_feature_and_sub_features": Object {
"primaryFeaturePrivilege": "none",
"subFeaturePrivileges": Array [],
},
"with_require_all_spaces_sub_features": Object {
"primaryFeaturePrivilege": "none",
"subFeaturePrivileges": Array [],
},
"with_sub_features": Object {
"primaryFeaturePrivilege": "none",
"subFeaturePrivileges": Array [],
},
}
`);

expect(findTestSubject(wrapper, 'spaceFormGlobalPermissionsSupersedeWarning')).toHaveLength(0);
expect(wrapper.find(EuiButtonGroup).filter('[name="basePrivilegeButtonGroup"]')).toHaveLength(
0
);
});

it('renders when a base privilege is selected', () => {
Expand Down Expand Up @@ -232,43 +200,6 @@ describe('PrivilegeSpaceForm', () => {
expect(findTestSubject(wrapper, 'spaceFormGlobalPermissionsSupersedeWarning')).toHaveLength(0);
});

it('renders a warning when configuring a global privilege after space privileges are already defined', () => {
const role = createRole([
{
base: [],
feature: {
with_sub_features: ['read'],
},
spaces: ['foo'],
},
{
base: [],
feature: {
with_sub_features: ['all'],
},
spaces: ['*'],
},
]);

const kibanaPrivileges = createKibanaPrivileges(kibanaFeatures);

const wrapper = renderComponent({
role,
spaces: displaySpaces,
kibanaPrivileges,
privilegeIndex: -1,
canCustomizeSubFeaturePrivileges: true,
onChange: jest.fn(),
onCancel: jest.fn(),
});

wrapper.find(SpaceSelector).props().onChange(['*']);

wrapper.update();

expect(findTestSubject(wrapper, 'globalPrivilegeWarning')).toHaveLength(1);
});

it('renders a warning when space privileges are less permissive than configured global privileges', () => {
const role = createRole([
{
Expand Down
Loading

0 comments on commit 3f8bbfa

Please sign in to comment.