From 7d3a6130c560a4c6bd66436abc6aae99ec12faf0 Mon Sep 17 00:00:00 2001 From: Larry Gregory <larry.gregory@elastic.co> Date: Mon, 24 Feb 2020 13:14:21 -0500 Subject: [PATCH] address PR feedback --- x-pack/plugins/security/common/model/role.ts | 18 +-- .../role_combo_box/role_combo_box.test.tsx | 110 ++++++++++++++++++ .../role_combo_box/role_combo_box.tsx | 20 +--- .../role_combo_box_option.test.tsx | 57 +++++++++ .../role_combo_box/role_combo_box_option.tsx | 31 +++++ .../roles/roles_grid/roles_grid_page.test.tsx | 53 ++++++++- .../roles/roles_grid/roles_grid_page.tsx | 1 + .../users/edit_user/edit_user_page.test.tsx | 60 ++++++++-- .../users/edit_user/edit_user_page.tsx | 10 +- .../users/users_grid/users_grid_page.test.tsx | 70 +++++++++++ .../users/users_grid/users_grid_page.tsx | 3 +- 11 files changed, 392 insertions(+), 41 deletions(-) create mode 100644 x-pack/plugins/security/public/management/role_combo_box/role_combo_box.test.tsx create mode 100644 x-pack/plugins/security/public/management/role_combo_box/role_combo_box_option.test.tsx create mode 100644 x-pack/plugins/security/public/management/role_combo_box/role_combo_box_option.tsx diff --git a/x-pack/plugins/security/common/model/role.ts b/x-pack/plugins/security/common/model/role.ts index 5790f7bcc658a..3ffa00ca0a571 100644 --- a/x-pack/plugins/security/common/model/role.ts +++ b/x-pack/plugins/security/common/model/role.ts @@ -71,15 +71,6 @@ export function isRoleDeprecated(role: Partial<Role>) { return role.metadata?._deprecated ?? false; } -/** - * Returns the reason this role is deprecated. - * - * @param role the Role as returned by roles API - */ -export function getRoleDeprecatedReason(role: Partial<Role>) { - return role.metadata?._deprecated_reason ?? ''; -} - /** * Returns the extended deprecation notice for the provided role. * @@ -124,3 +115,12 @@ export function prepareRoleClone(role: Role): Role { return clone; } + +/** + * Returns the reason this role is deprecated. + * + * @param role the Role as returned by roles API + */ +function getRoleDeprecatedReason(role: Partial<Role>) { + return role.metadata?._deprecated_reason ?? ''; +} diff --git a/x-pack/plugins/security/public/management/role_combo_box/role_combo_box.test.tsx b/x-pack/plugins/security/public/management/role_combo_box/role_combo_box.test.tsx new file mode 100644 index 0000000000000..b5c2659f99935 --- /dev/null +++ b/x-pack/plugins/security/public/management/role_combo_box/role_combo_box.test.tsx @@ -0,0 +1,110 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import React from 'react'; +import { mountWithIntl, nextTick } from 'test_utils/enzyme_helpers'; + +import { RoleComboBox } from '.'; +import { EuiComboBox } from '@elastic/eui'; +import { findTestSubject } from 'test_utils/find_test_subject'; + +describe('RoleComboBox', () => { + it('renders the provided list of roles via EuiComboBox options', () => { + const availableRoles = [ + { + name: 'role-1', + elasticsearch: { cluster: [], indices: [], run_as: [] }, + kibana: [], + metadata: {}, + }, + { + name: 'role-2', + elasticsearch: { cluster: [], indices: [], run_as: [] }, + kibana: [], + metadata: {}, + }, + ]; + const wrapper = mountWithIntl( + <RoleComboBox availableRoles={availableRoles} selectedRoleNames={[]} onChange={jest.fn()} /> + ); + + expect(wrapper.find(EuiComboBox).props().options).toMatchInlineSnapshot(` + Array [ + Object { + "color": "default", + "data-test-subj": "roleOption-role-1", + "label": "role-1", + "value": Object { + "isDeprecated": false, + }, + }, + Object { + "color": "default", + "data-test-subj": "roleOption-role-2", + "label": "role-2", + "value": Object { + "isDeprecated": false, + }, + }, + ] + `); + }); + + it('renders deprecated roles as such', () => { + const availableRoles = [ + { + name: 'role-1', + elasticsearch: { cluster: [], indices: [], run_as: [] }, + kibana: [], + metadata: { _deprecated: true }, + }, + ]; + const wrapper = mountWithIntl( + <RoleComboBox availableRoles={availableRoles} selectedRoleNames={[]} onChange={jest.fn()} /> + ); + + expect(wrapper.find(EuiComboBox).props().options).toMatchInlineSnapshot(` + Array [ + Object { + "color": "warning", + "data-test-subj": "roleOption-role-1", + "label": "role-1", + "value": Object { + "isDeprecated": true, + }, + }, + ] + `); + }); + + it('renders the selected role names in the expanded list, coded according to deprecated status', () => { + const availableRoles = [ + { + name: 'role-1', + elasticsearch: { cluster: [], indices: [], run_as: [] }, + kibana: [], + metadata: {}, + }, + { + name: 'role-2', + elasticsearch: { cluster: [], indices: [], run_as: [] }, + kibana: [], + metadata: {}, + }, + ]; + const wrapper = mountWithIntl( + <div> + <RoleComboBox availableRoles={availableRoles} selectedRoleNames={[]} onChange={jest.fn()} /> + </div> + ); + + findTestSubject(wrapper, 'comboBoxToggleListButton').simulate('click'); + + wrapper.find(EuiComboBox).setState({ isListOpen: true }); + + expect(findTestSubject(wrapper, 'rolesDropdown-renderOption')).toMatchInlineSnapshot(`null`); + }); +}); diff --git a/x-pack/plugins/security/public/management/role_combo_box/role_combo_box.tsx b/x-pack/plugins/security/public/management/role_combo_box/role_combo_box.tsx index 48744ff9ebfcd..65fd8a8324a7d 100644 --- a/x-pack/plugins/security/public/management/role_combo_box/role_combo_box.tsx +++ b/x-pack/plugins/security/public/management/role_combo_box/role_combo_box.tsx @@ -6,8 +6,9 @@ import React from 'react'; import { i18n } from '@kbn/i18n'; -import { EuiComboBox, EuiText } from '@elastic/eui'; +import { EuiComboBox } from '@elastic/eui'; import { Role, isRoleDeprecated } from '../../../common/model'; +import { RoleComboBoxOption } from './role_combo_box_option'; interface Props { availableRoles: Role[]; @@ -25,7 +26,7 @@ export const RoleComboBox = (props: Props) => { const roleNameToOption = (roleName: string) => { const roleDefinition = props.availableRoles.find(role => role.name === roleName); - const isDeprecated = roleDefinition && isRoleDeprecated(roleDefinition); + const isDeprecated: boolean = (roleDefinition && isRoleDeprecated(roleDefinition)) ?? false; return { color: isDeprecated ? 'warning' : 'default', 'data-test-subj': `roleOption-${roleName}`, @@ -54,20 +55,7 @@ export const RoleComboBox = (props: Props) => { isDisabled={props.isDisabled} options={options} selectedOptions={selectedOptions} - renderOption={option => { - const isDeprecated = option.value!.isDeprecated; - const deprecatedLabel = i18n.translate( - 'xpack.security.management.users.editUser.deprecatedRoleText', - { - defaultMessage: '(deprecated)', - } - ); - return ( - <EuiText color={option.color as any}> - {option.label} {isDeprecated ? deprecatedLabel : ''} - </EuiText> - ); - }} + renderOption={option => <RoleComboBoxOption option={option} />} /> ); }; diff --git a/x-pack/plugins/security/public/management/role_combo_box/role_combo_box_option.test.tsx b/x-pack/plugins/security/public/management/role_combo_box/role_combo_box_option.test.tsx new file mode 100644 index 0000000000000..c1ac381ba9994 --- /dev/null +++ b/x-pack/plugins/security/public/management/role_combo_box/role_combo_box_option.test.tsx @@ -0,0 +1,57 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import React from 'react'; +import { shallowWithIntl } from 'test_utils/enzyme_helpers'; +import { RoleComboBoxOption } from './role_combo_box_option'; + +describe('RoleComboBoxOption', () => { + it('renders a regular role correctly', () => { + const wrapper = shallowWithIntl( + <RoleComboBoxOption + option={{ + color: 'default', + label: 'role-1', + }} + /> + ); + + expect(wrapper).toMatchInlineSnapshot(` + <EuiText + color="default" + data-test-subj="rolesDropdown-renderOption" + > + role-1 + + </EuiText> + `); + }); + + it('renders a deprecated role correctly', () => { + const wrapper = shallowWithIntl( + <RoleComboBoxOption + option={{ + color: 'warning', + label: 'role-1', + value: { + isDeprecated: true, + }, + }} + /> + ); + + expect(wrapper).toMatchInlineSnapshot(` + <EuiText + color="warning" + data-test-subj="rolesDropdown-renderOption" + > + role-1 + + (deprecated) + </EuiText> + `); + }); +}); diff --git a/x-pack/plugins/security/public/management/role_combo_box/role_combo_box_option.tsx b/x-pack/plugins/security/public/management/role_combo_box/role_combo_box_option.tsx new file mode 100644 index 0000000000000..126a3151adf01 --- /dev/null +++ b/x-pack/plugins/security/public/management/role_combo_box/role_combo_box_option.tsx @@ -0,0 +1,31 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import React from 'react'; + +import { i18n } from '@kbn/i18n'; + +import { EuiComboBoxOptionProps, EuiText } from '@elastic/eui'; + +interface Props { + option: EuiComboBoxOptionProps<{ isDeprecated: boolean }>; +} + +export const RoleComboBoxOption = ({ option }: Props) => { + const isDeprecated = option.value?.isDeprecated ?? false; + const deprecatedLabel = i18n.translate( + 'xpack.security.management.users.editUser.deprecatedRoleText', + { + defaultMessage: '(deprecated)', + } + ); + + return ( + <EuiText color={option.color as any} data-test-subj="rolesDropdown-renderOption"> + {option.label} {isDeprecated ? deprecatedLabel : ''} + </EuiText> + ); +}; diff --git a/x-pack/plugins/security/public/management/roles/roles_grid/roles_grid_page.test.tsx b/x-pack/plugins/security/public/management/roles/roles_grid/roles_grid_page.test.tsx index b8a1c04e33a1e..410d5bc9f7643 100644 --- a/x-pack/plugins/security/public/management/roles/roles_grid/roles_grid_page.test.tsx +++ b/x-pack/plugins/security/public/management/roles/roles_grid/roles_grid_page.test.tsx @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import { EuiIcon } from '@elastic/eui'; +import { EuiIcon, EuiBasicTable } from '@elastic/eui'; import { ReactWrapper } from 'enzyme'; import React from 'react'; import { mountWithIntl } from 'test_utils/enzyme_helpers'; @@ -15,6 +15,7 @@ import { RolesGridPage } from './roles_grid_page'; import { coreMock } from '../../../../../../../src/core/public/mocks'; import { rolesAPIClientMock } from '../index.mock'; import { ReservedBadge, DisabledBadge } from '../../badges'; +import { findTestSubject } from 'test_utils/find_test_subject'; const mock403 = () => ({ body: { statusCode: 403 } }); @@ -140,4 +141,54 @@ describe('<RolesGridPage />', () => { wrapper.find('EuiButtonIcon[data-test-subj="clone-role-action-disabled-role"]') ).toHaveLength(1); }); + + it('hides reserved roles when instructed to', async () => { + const wrapper = mountWithIntl( + <RolesGridPage + rolesAPIClient={apiClientMock} + notifications={coreMock.createStart().notifications} + /> + ); + const initialIconCount = wrapper.find(EuiIcon).length; + + await waitForRender(wrapper, updatedWrapper => { + return updatedWrapper.find(EuiIcon).length > initialIconCount; + }); + + expect(wrapper.find(EuiBasicTable).props().items).toEqual([ + { + name: 'disabled-role', + elasticsearch: { cluster: [], indices: [], run_as: [] }, + kibana: [{ base: [], spaces: [], feature: {} }], + transient_metadata: { enabled: false }, + }, + { + name: 'reserved-role', + elasticsearch: { cluster: [], indices: [], run_as: [] }, + kibana: [{ base: [], spaces: [], feature: {} }], + metadata: { _reserved: true }, + }, + { + name: 'test-role-1', + elasticsearch: { cluster: [], indices: [], run_as: [] }, + kibana: [{ base: [], spaces: [], feature: {} }], + }, + ]); + + findTestSubject(wrapper, 'showReservedRolesSwitch').simulate('click'); + + expect(wrapper.find(EuiBasicTable).props().items).toEqual([ + { + name: 'disabled-role', + elasticsearch: { cluster: [], indices: [], run_as: [] }, + kibana: [{ base: [], spaces: [], feature: {} }], + transient_metadata: { enabled: false }, + }, + { + name: 'test-role-1', + elasticsearch: { cluster: [], indices: [], run_as: [] }, + kibana: [{ base: [], spaces: [], feature: {} }], + }, + ]); + }); }); diff --git a/x-pack/plugins/security/public/management/roles/roles_grid/roles_grid_page.tsx b/x-pack/plugins/security/public/management/roles/roles_grid/roles_grid_page.tsx index 587dd740819c1..04a74a1a9b99a 100644 --- a/x-pack/plugins/security/public/management/roles/roles_grid/roles_grid_page.tsx +++ b/x-pack/plugins/security/public/management/roles/roles_grid/roles_grid_page.tsx @@ -377,6 +377,7 @@ export class RolesGridPage extends Component<Props, State> { private renderToolsRight() { return ( <EuiSwitch + data-test-subj="showReservedRolesSwitch" label={ <FormattedMessage id="xpack.security.management.roles.showReservedRolesLabel" diff --git a/x-pack/plugins/security/public/management/users/edit_user/edit_user_page.test.tsx b/x-pack/plugins/security/public/management/users/edit_user/edit_user_page.test.tsx index 37b1d847e59d1..be7517ff892b5 100644 --- a/x-pack/plugins/security/public/management/users/edit_user/edit_user_page.test.tsx +++ b/x-pack/plugins/security/public/management/users/edit_user/edit_user_page.test.tsx @@ -15,13 +15,14 @@ import { mockAuthenticatedUser } from '../../../../common/model/authenticated_us import { securityMock } from '../../../mocks'; import { rolesAPIClientMock } from '../../roles/index.mock'; import { userAPIClientMock } from '../index.mock'; +import { findTestSubject } from 'test_utils/find_test_subject'; -const createUser = (username: string) => { +const createUser = (username: string, roles = ['idk', 'something']) => { const user: User = { username, full_name: 'my full name', email: 'foo@bar.com', - roles: ['idk', 'something'], + roles, enabled: true, }; @@ -34,9 +35,9 @@ const createUser = (username: string) => { return user; }; -const buildClients = () => { +const buildClients = (user: User) => { const apiClient = userAPIClientMock.create(); - apiClient.getUser.mockImplementation(async (username: string) => createUser(username)); + apiClient.getUser.mockResolvedValue(user); const rolesAPIClient = rolesAPIClientMock.create(); rolesAPIClient.getRoles.mockImplementation(() => { @@ -59,6 +60,18 @@ const buildClients = () => { }, kibana: [], }, + { + name: 'deprecated-role', + elasticsearch: { + cluster: [], + indices: [], + run_as: ['bar'], + }, + kibana: [], + metadata: { + _deprecated: true, + }, + }, ] as Role[]); }); @@ -83,11 +96,12 @@ function expectMissingSaveButton(wrapper: ReactWrapper<any, any>) { describe('EditUserPage', () => { it('allows reserved users to be viewed', async () => { - const { apiClient, rolesAPIClient } = buildClients(); + const user = createUser('reserved_user'); + const { apiClient, rolesAPIClient } = buildClients(user); const securitySetup = buildSecuritySetup(); const wrapper = mountWithIntl( <EditUserPage - username={'reserved_user'} + username={user.username} userAPIClient={apiClient} rolesAPIClient={rolesAPIClient} authc={securitySetup.authc} @@ -104,11 +118,12 @@ describe('EditUserPage', () => { }); it('allows new users to be created', async () => { - const { apiClient, rolesAPIClient } = buildClients(); + const user = createUser(''); + const { apiClient, rolesAPIClient } = buildClients(user); const securitySetup = buildSecuritySetup(); const wrapper = mountWithIntl( <EditUserPage - username={''} + username={user.username} userAPIClient={apiClient} rolesAPIClient={rolesAPIClient} authc={securitySetup.authc} @@ -125,11 +140,12 @@ describe('EditUserPage', () => { }); it('allows existing users to be edited', async () => { - const { apiClient, rolesAPIClient } = buildClients(); + const user = createUser('existing_user'); + const { apiClient, rolesAPIClient } = buildClients(user); const securitySetup = buildSecuritySetup(); const wrapper = mountWithIntl( <EditUserPage - username={'existing_user'} + username={user.username} userAPIClient={apiClient} rolesAPIClient={rolesAPIClient} authc={securitySetup.authc} @@ -142,8 +158,32 @@ describe('EditUserPage', () => { expect(apiClient.getUser).toBeCalledTimes(1); expect(securitySetup.authc.getCurrentUser).toBeCalledTimes(1); + expect(findTestSubject(wrapper, 'hasDeprecatedRolesAssignedHelpText')).toHaveLength(0); expectSaveButton(wrapper); }); + + it('warns when user is assigned a deprecated role', async () => { + const user = createUser('existing_user', ['deprecated-role']); + const { apiClient, rolesAPIClient } = buildClients(user); + const securitySetup = buildSecuritySetup(); + + const wrapper = mountWithIntl( + <EditUserPage + username={user.username} + userAPIClient={apiClient} + rolesAPIClient={rolesAPIClient} + authc={securitySetup.authc} + notifications={coreMock.createStart().notifications} + /> + ); + + await waitForRender(wrapper); + + expect(apiClient.getUser).toBeCalledTimes(1); + expect(securitySetup.authc.getCurrentUser).toBeCalledTimes(1); + + expect(findTestSubject(wrapper, 'hasDeprecatedRolesAssignedHelpText')).toHaveLength(1); + }); }); async function waitForRender(wrapper: ReactWrapper<any, any>) { diff --git a/x-pack/plugins/security/public/management/users/edit_user/edit_user_page.tsx b/x-pack/plugins/security/public/management/users/edit_user/edit_user_page.tsx index 4f9b27fd8e171..7b791840613d1 100644 --- a/x-pack/plugins/security/public/management/users/edit_user/edit_user_page.tsx +++ b/x-pack/plugins/security/public/management/users/edit_user/edit_user_page.tsx @@ -382,10 +382,12 @@ export class EditUserPage extends Component<Props, State> { }); const roleHelpText = hasAnyDeprecatedRolesAssigned ? ( - <FormattedMessage - id="xpack.security.management.users.editUser.deprecatedRolesAssignedWarning" - defaultMessage="This user is assigned a deprecated role. Please migrate to a supported role as soon as possible." - /> + <span data-test-subj="hasDeprecatedRolesAssignedHelpText"> + <FormattedMessage + id="xpack.security.management.users.editUser.deprecatedRolesAssignedWarning" + defaultMessage="This user is assigned a deprecated role. Please migrate to a supported role as soon as possible." + /> + </span> ) : ( undefined ); diff --git a/x-pack/plugins/security/public/management/users/users_grid/users_grid_page.test.tsx b/x-pack/plugins/security/public/management/users/users_grid/users_grid_page.test.tsx index bb086b1be68fb..4c00e59057ac7 100644 --- a/x-pack/plugins/security/public/management/users/users_grid/users_grid_page.test.tsx +++ b/x-pack/plugins/security/public/management/users/users_grid/users_grid_page.test.tsx @@ -13,6 +13,7 @@ import { userAPIClientMock } from '../index.mock'; import { coreMock } from '../../../../../../../src/core/public/mocks'; import { rolesAPIClientMock } from '../../roles/index.mock'; import { findTestSubject } from 'test_utils/find_test_subject'; +import { EuiBasicTable } from '@elastic/eui'; describe('UsersGridPage', () => { it('renders the list of users', async () => { @@ -167,6 +168,75 @@ describe('UsersGridPage', () => { } `); }); + + it('hides reserved users when instructed to', async () => { + const apiClientMock = userAPIClientMock.create(); + apiClientMock.getUsers.mockImplementation(() => { + return Promise.resolve<User[]>([ + { + username: 'foo', + email: 'foo@bar.net', + full_name: 'foo bar', + roles: ['kibana_user'], + enabled: true, + }, + { + username: 'reserved', + email: 'reserved@bar.net', + full_name: '', + roles: ['superuser'], + enabled: true, + metadata: { + _reserved: true, + }, + }, + ]); + }); + + const roleAPIClientMock = rolesAPIClientMock.create(); + + const wrapper = mountWithIntl( + <UsersGridPage + userAPIClient={apiClientMock} + rolesAPIClient={roleAPIClientMock} + notifications={coreMock.createStart().notifications} + /> + ); + + await waitForRender(wrapper); + + expect(wrapper.find(EuiBasicTable).props().items).toEqual([ + { + username: 'foo', + email: 'foo@bar.net', + full_name: 'foo bar', + roles: ['kibana_user'], + enabled: true, + }, + { + username: 'reserved', + email: 'reserved@bar.net', + full_name: '', + roles: ['superuser'], + enabled: true, + metadata: { + _reserved: true, + }, + }, + ]); + + findTestSubject(wrapper, 'showReservedUsersSwitch').simulate('click'); + + expect(wrapper.find(EuiBasicTable).props().items).toEqual([ + { + username: 'foo', + email: 'foo@bar.net', + full_name: 'foo bar', + roles: ['kibana_user'], + enabled: true, + }, + ]); + }); }); async function waitForRender(wrapper: ReactWrapper<any, any>) { diff --git a/x-pack/plugins/security/public/management/users/users_grid/users_grid_page.tsx b/x-pack/plugins/security/public/management/users/users_grid/users_grid_page.tsx index ed16372e55037..6837fcf430fe7 100644 --- a/x-pack/plugins/security/public/management/users/users_grid/users_grid_page.tsx +++ b/x-pack/plugins/security/public/management/users/users_grid/users_grid_page.tsx @@ -194,7 +194,7 @@ export class UsersGridPage extends Component<Props, State> { }; const sorting = { sort: { - field: 'full_name', + field: 'username', direction: 'asc', }, } as const; @@ -344,6 +344,7 @@ export class UsersGridPage extends Component<Props, State> { private renderToolsRight() { return ( <EuiSwitch + data-test-subj="showReservedUsersSwitch" label={ <FormattedMessage id="xpack.security.management.users.showReservedUsersLabel"