From 093b2239ad6147c1bdbef4051dd031ff25aa9349 Mon Sep 17 00:00:00 2001 From: Aleh Zasypkin Date: Wed, 15 Jan 2020 12:12:05 +0100 Subject: [PATCH] Review#2: remove role related methods from `UsersAPIClient`, use more appropriate naming format for the management app object names, fix feature catalogue link, properly handle role mapping name change in Edit Role Mapping page, properly encode parameters in breadcrumbs links. --- .../api_keys/api_keys_management_app.test.tsx | 20 +++++----- .../api_keys/api_keys_management_app.tsx | 2 +- .../public/management/api_keys/index.ts | 2 +- .../management/management_service.test.ts | 20 +++++----- .../public/management/management_service.ts | 39 ++++++------------- .../edit_role_mapping_page.tsx | 6 +++ .../public/management/role_mappings/index.ts | 2 +- .../role_mappings_management_app.test.tsx | 30 +++++++++++--- .../role_mappings_management_app.tsx | 4 +- .../security/public/management/roles/index.ts | 2 +- .../roles/roles_management_app.test.tsx | 35 +++++++++++++---- .../management/roles/roles_management_app.tsx | 4 +- .../users/edit_user/edit_user_page.test.tsx | 22 ++++++----- .../users/edit_user/edit_user_page.tsx | 6 ++- .../security/public/management/users/index.ts | 2 +- .../management/users/user_api_client.mock.ts | 2 - .../management/users/user_api_client.ts | 11 +----- .../users/users_management_app.test.tsx | 37 +++++++++++++----- .../management/users/users_management_app.tsx | 6 ++- x-pack/plugins/security/public/plugin.tsx | 5 +-- 20 files changed, 150 insertions(+), 107 deletions(-) diff --git a/x-pack/plugins/security/public/management/api_keys/api_keys_management_app.test.tsx b/x-pack/plugins/security/public/management/api_keys/api_keys_management_app.test.tsx index 54da081f6a16c..05427960d42b5 100644 --- a/x-pack/plugins/security/public/management/api_keys/api_keys_management_app.test.tsx +++ b/x-pack/plugins/security/public/management/api_keys/api_keys_management_app.test.tsx @@ -8,14 +8,14 @@ jest.mock('./api_keys_grid', () => ({ APIKeysGridPage: (props: any) => `Page: ${JSON.stringify(props)}`, })); -import { APIKeysManagementApp } from './api_keys_management_app'; +import { apiKeysManagementApp } from './api_keys_management_app'; import { coreMock } from '../../../../../../src/core/public/mocks'; -describe('APIKeysManagementApp', () => { +describe('apiKeysManagementApp', () => { it('create() returns proper management app descriptor', () => { const { getStartServices } = coreMock.createSetup(); - expect(APIKeysManagementApp.create({ getStartServices: getStartServices as any })) + expect(apiKeysManagementApp.create({ getStartServices: getStartServices as any })) .toMatchInlineSnapshot(` Object { "id": "api_keys", @@ -31,13 +31,13 @@ describe('APIKeysManagementApp', () => { const container = document.createElement('div'); const setBreadcrumbs = jest.fn(); - const unmount = await APIKeysManagementApp.create({ - getStartServices: getStartServices as any, - }).mount({ - basePath: '/some-base-path', - element: container, - setBreadcrumbs, - }); + const unmount = await apiKeysManagementApp + .create({ getStartServices: getStartServices as any }) + .mount({ + basePath: '/some-base-path', + element: container, + setBreadcrumbs, + }); expect(setBreadcrumbs).toHaveBeenCalledTimes(1); expect(setBreadcrumbs).toHaveBeenCalledWith([{ href: '#/some-base-path', text: 'API Keys' }]); diff --git a/x-pack/plugins/security/public/management/api_keys/api_keys_management_app.tsx b/x-pack/plugins/security/public/management/api_keys/api_keys_management_app.tsx index da2947b58fc12..35de732b84ce9 100644 --- a/x-pack/plugins/security/public/management/api_keys/api_keys_management_app.tsx +++ b/x-pack/plugins/security/public/management/api_keys/api_keys_management_app.tsx @@ -18,7 +18,7 @@ interface CreateParams { getStartServices: CoreSetup['getStartServices']; } -export const APIKeysManagementApp = Object.freeze({ +export const apiKeysManagementApp = Object.freeze({ id: 'api_keys', create({ getStartServices }: CreateParams) { return { diff --git a/x-pack/plugins/security/public/management/api_keys/index.ts b/x-pack/plugins/security/public/management/api_keys/index.ts index 2d42e367ee3ea..e15da7d5eb409 100644 --- a/x-pack/plugins/security/public/management/api_keys/index.ts +++ b/x-pack/plugins/security/public/management/api_keys/index.ts @@ -4,4 +4,4 @@ * you may not use this file except in compliance with the Elastic License. */ -export { APIKeysManagementApp } from './api_keys_management_app'; +export { apiKeysManagementApp } from './api_keys_management_app'; diff --git a/x-pack/plugins/security/public/management/management_service.test.ts b/x-pack/plugins/security/public/management/management_service.test.ts index 1326bccec6156..53c12ad7ab12c 100644 --- a/x-pack/plugins/security/public/management/management_service.test.ts +++ b/x-pack/plugins/security/public/management/management_service.test.ts @@ -8,14 +8,14 @@ import { BehaviorSubject } from 'rxjs'; import { ManagementApp } from '../../../../../src/plugins/management/public'; import { SecurityLicenseFeatures } from '../../common/licensing/license_features'; import { ManagementService } from './management_service'; -import { UsersManagementApp } from './users'; +import { usersManagementApp } from './users'; import { coreMock } from '../../../../../src/core/public/mocks'; import { licenseMock } from '../../common/licensing/index.mock'; import { securityMock } from '../mocks'; -import { RolesManagementApp } from './roles'; -import { APIKeysManagementApp } from './api_keys'; -import { RoleMappingsManagementApp } from './role_mappings'; +import { rolesManagementApp } from './roles'; +import { apiKeysManagementApp } from './api_keys'; +import { roleMappingsManagementApp } from './role_mappings'; describe('ManagementService', () => { describe('setup()', () => { @@ -119,10 +119,10 @@ describe('ManagementService', () => { } as unknown) as jest.Mocked; }; const mockApps = new Map>([ - [UsersManagementApp.id, getMockedApp()], - [RolesManagementApp.id, getMockedApp()], - [APIKeysManagementApp.id, getMockedApp()], - [RoleMappingsManagementApp.id, getMockedApp()], + [usersManagementApp.id, getMockedApp()], + [rolesManagementApp.id, getMockedApp()], + [apiKeysManagementApp.id, getMockedApp()], + [roleMappingsManagementApp.id, getMockedApp()], ] as Array<[string, jest.Mocked]>); service.start({ @@ -165,7 +165,7 @@ describe('ManagementService', () => { it('disables only Role Mappings app if `showLinks` is `true`, but `showRoleMappingsManagement` is `false` at `start`', () => { const { mockApps } = startService({ showLinks: true, showRoleMappingsManagement: false }); for (const [appId, mockApp] of mockApps) { - expect(mockApp.enabled).toBe(appId !== RoleMappingsManagementApp.id); + expect(mockApp.enabled).toBe(appId !== roleMappingsManagementApp.id); } }); @@ -197,7 +197,7 @@ describe('ManagementService', () => { updateFeatures({ showLinks: true, showRoleMappingsManagement: false }); for (const [appId, mockApp] of mockApps) { - expect(mockApp.enabled).toBe(appId !== RoleMappingsManagementApp.id); + expect(mockApp.enabled).toBe(appId !== roleMappingsManagementApp.id); } }); diff --git a/x-pack/plugins/security/public/management/management_service.ts b/x-pack/plugins/security/public/management/management_service.ts index a394ce60826e4..5ad3681590fbf 100644 --- a/x-pack/plugins/security/public/management/management_service.ts +++ b/x-pack/plugins/security/public/management/management_service.ts @@ -15,10 +15,10 @@ import { import { SecurityLicense } from '../../common/licensing'; import { AuthenticationServiceSetup } from '../authentication'; import { PluginStartDependencies } from '../plugin'; -import { APIKeysManagementApp } from './api_keys'; -import { RoleMappingsManagementApp } from './role_mappings'; -import { RolesManagementApp } from './roles'; -import { UsersManagementApp } from './users'; +import { apiKeysManagementApp } from './api_keys'; +import { roleMappingsManagementApp } from './role_mappings'; +import { rolesManagementApp } from './roles'; +import { usersManagementApp } from './users'; interface SetupParams { management: ManagementSetup; @@ -48,12 +48,12 @@ export class ManagementService { euiIconType: 'securityApp', }); - securitySection.registerApp(UsersManagementApp.create({ authc, getStartServices })); + securitySection.registerApp(usersManagementApp.create({ authc, getStartServices })); securitySection.registerApp( - RolesManagementApp.create({ fatalErrors, license, getStartServices }) + rolesManagementApp.create({ fatalErrors, license, getStartServices }) ); - securitySection.registerApp(APIKeysManagementApp.create({ getStartServices })); - securitySection.registerApp(RoleMappingsManagementApp.create({ getStartServices })); + securitySection.registerApp(apiKeysManagementApp.create({ getStartServices })); + securitySection.registerApp(roleMappingsManagementApp.create({ getStartServices })); } start({ management }: StartParams) { @@ -61,11 +61,11 @@ export class ManagementService { const securitySection = management.sections.getSection('security')!; const securityManagementAppsStatuses: Array<[ManagementApp, boolean]> = [ - [securitySection.getApp(UsersManagementApp.id)!, features.showLinks], - [securitySection.getApp(RolesManagementApp.id)!, features.showLinks], - [securitySection.getApp(APIKeysManagementApp.id)!, features.showLinks], + [securitySection.getApp(usersManagementApp.id)!, features.showLinks], + [securitySection.getApp(rolesManagementApp.id)!, features.showLinks], + [securitySection.getApp(apiKeysManagementApp.id)!, features.showLinks], [ - securitySection.getApp(RoleMappingsManagementApp.id)!, + securitySection.getApp(roleMappingsManagementApp.id)!, features.showLinks && features.showRoleMappingsManagement, ], ]; @@ -92,19 +92,4 @@ export class ManagementService { this.licenseFeaturesSubscription = undefined; } } - - // TODO: DO WE STILL NEED THIS? - /* private checkLicense({ - management, - notifications, - }: Pick) { - const { showLinks, linksMessage } = this.license.getFeatures(); - if (!showLinks) { - notifications.toasts.addDanger({ title: linksMessage }); - management.sections.navigateToApp('management'); - return false; - } - - return true; - }*/ } diff --git a/x-pack/plugins/security/public/management/role_mappings/edit_role_mapping/edit_role_mapping_page.tsx b/x-pack/plugins/security/public/management/role_mappings/edit_role_mapping/edit_role_mapping_page.tsx index f9f19da675330..142b53cbb50f2 100644 --- a/x-pack/plugins/security/public/management/role_mappings/edit_role_mapping/edit_role_mapping_page.tsx +++ b/x-pack/plugins/security/public/management/role_mappings/edit_role_mapping/edit_role_mapping_page.tsx @@ -78,6 +78,12 @@ export class EditRoleMappingPage extends Component { this.loadAppData(); } + public async componentDidUpdate(prevProps: Props) { + if (prevProps.name !== this.props.name) { + await this.loadAppData(); + } + } + public render() { const { loadState } = this.state; diff --git a/x-pack/plugins/security/public/management/role_mappings/index.ts b/x-pack/plugins/security/public/management/role_mappings/index.ts index 4893a9aa00fca..f670ea6181038 100644 --- a/x-pack/plugins/security/public/management/role_mappings/index.ts +++ b/x-pack/plugins/security/public/management/role_mappings/index.ts @@ -4,4 +4,4 @@ * you may not use this file except in compliance with the Elastic License. */ -export { RoleMappingsManagementApp } from './role_mappings_management_app'; +export { roleMappingsManagementApp } from './role_mappings_management_app'; diff --git a/x-pack/plugins/security/public/management/role_mappings/role_mappings_management_app.test.tsx b/x-pack/plugins/security/public/management/role_mappings/role_mappings_management_app.test.tsx index 65134902f6344..86f54bca88dbd 100644 --- a/x-pack/plugins/security/public/management/role_mappings/role_mappings_management_app.test.tsx +++ b/x-pack/plugins/security/public/management/role_mappings/role_mappings_management_app.test.tsx @@ -12,7 +12,7 @@ jest.mock('./edit_role_mapping', () => ({ EditRoleMappingPage: (props: any) => `Role Mapping Edit Page: ${JSON.stringify(props)}`, })); -import { RoleMappingsManagementApp } from './role_mappings_management_app'; +import { roleMappingsManagementApp } from './role_mappings_management_app'; import { coreMock } from '../../../../../../src/core/public/mocks'; @@ -20,17 +20,17 @@ async function mountApp(basePath: string) { const container = document.createElement('div'); const setBreadcrumbs = jest.fn(); - const unmount = await RoleMappingsManagementApp.create({ - getStartServices: coreMock.createSetup().getStartServices as any, - }).mount({ basePath, element: container, setBreadcrumbs }); + const unmount = await roleMappingsManagementApp + .create({ getStartServices: coreMock.createSetup().getStartServices as any }) + .mount({ basePath, element: container, setBreadcrumbs }); return { unmount, container, setBreadcrumbs }; } -describe('RoleMappingsManagementApp', () => { +describe('roleMappingsManagementApp', () => { it('create() returns proper management app descriptor', () => { expect( - RoleMappingsManagementApp.create({ + roleMappingsManagementApp.create({ getStartServices: coreMock.createSetup().getStartServices as any, }) ).toMatchInlineSnapshot(` @@ -106,4 +106,22 @@ describe('RoleMappingsManagementApp', () => { expect(container).toMatchInlineSnapshot(`
`); }); + + it('mount() properly encodes role mapping name in `edit role mapping` page link in breadcrumbs', async () => { + const basePath = '/some-base-path/role_mappings'; + const roleMappingName = 'some 安全性 role mapping'; + window.location.hash = `${basePath}/edit/${roleMappingName}`; + + const { setBreadcrumbs } = await mountApp(basePath); + + expect(setBreadcrumbs).toHaveBeenCalledTimes(1); + expect(setBreadcrumbs).toHaveBeenCalledWith([ + { href: `#${basePath}`, text: 'Role Mappings' }, + { + href: + '#/some-base-path/role_mappings/edit/some%20%E5%AE%89%E5%85%A8%E6%80%A7%20role%20mapping', + text: roleMappingName, + }, + ]); + }); }); diff --git a/x-pack/plugins/security/public/management/role_mappings/role_mappings_management_app.tsx b/x-pack/plugins/security/public/management/role_mappings/role_mappings_management_app.tsx index d87bb46d7bedb..af1572cedbade 100644 --- a/x-pack/plugins/security/public/management/role_mappings/role_mappings_management_app.tsx +++ b/x-pack/plugins/security/public/management/role_mappings/role_mappings_management_app.tsx @@ -21,7 +21,7 @@ interface CreateParams { getStartServices: CoreSetup['getStartServices']; } -export const RoleMappingsManagementApp = Object.freeze({ +export const roleMappingsManagementApp = Object.freeze({ id: 'role_mappings', create({ getStartServices }: CreateParams) { return { @@ -60,7 +60,7 @@ export const RoleMappingsManagementApp = Object.freeze({ setBreadcrumbs([ ...roleMappingsBreadcrumbs, name - ? { text: name, href: `#${basePath}/edit/${name}` } + ? { text: name, href: `#${basePath}/edit/${encodeURIComponent(name)}` } : { text: i18n.translate('xpack.security.roleMappings.createBreadcrumb', { defaultMessage: 'Create', diff --git a/x-pack/plugins/security/public/management/roles/index.ts b/x-pack/plugins/security/public/management/roles/index.ts index c149b1350be48..6339d438e1f2f 100644 --- a/x-pack/plugins/security/public/management/roles/index.ts +++ b/x-pack/plugins/security/public/management/roles/index.ts @@ -4,5 +4,5 @@ * you may not use this file except in compliance with the Elastic License. */ -export { RolesManagementApp } from './roles_management_app'; +export { rolesManagementApp } from './roles_management_app'; export { RolesAPIClient } from './roles_api_client'; diff --git a/x-pack/plugins/security/public/management/roles/roles_management_app.test.tsx b/x-pack/plugins/security/public/management/roles/roles_management_app.test.tsx index 84aadce9f2642..48bc1a6580a93 100644 --- a/x-pack/plugins/security/public/management/roles/roles_management_app.test.tsx +++ b/x-pack/plugins/security/public/management/roles/roles_management_app.test.tsx @@ -14,7 +14,7 @@ jest.mock('./edit_role', () => ({ EditRolePage: (props: any) => `Role Edit Page: ${JSON.stringify(props)}`, })); -import { RolesManagementApp } from './roles_management_app'; +import { rolesManagementApp } from './roles_management_app'; import { coreMock } from '../../../../../../src/core/public/mocks'; @@ -23,21 +23,23 @@ async function mountApp(basePath: string) { const container = document.createElement('div'); const setBreadcrumbs = jest.fn(); - const unmount = await RolesManagementApp.create({ - license: licenseMock.create(), - fatalErrors, - getStartServices: jest.fn().mockResolvedValue([coreMock.createStart(), { data: {} }]), - }).mount({ basePath, element: container, setBreadcrumbs }); + const unmount = await rolesManagementApp + .create({ + license: licenseMock.create(), + fatalErrors, + getStartServices: jest.fn().mockResolvedValue([coreMock.createStart(), { data: {} }]), + }) + .mount({ basePath, element: container, setBreadcrumbs }); return { unmount, container, setBreadcrumbs }; } -describe('RolesManagementApp', () => { +describe('rolesManagementApp', () => { it('create() returns proper management app descriptor', () => { const { fatalErrors, getStartServices } = coreMock.createSetup(); expect( - RolesManagementApp.create({ + rolesManagementApp.create({ license: licenseMock.create(), fatalErrors, getStartServices: getStartServices as any, @@ -138,4 +140,21 @@ describe('RolesManagementApp', () => { expect(container).toMatchInlineSnapshot(`
`); }); + + it('mount() properly encodes role name in `edit role` page link in breadcrumbs', async () => { + const basePath = '/some-base-path/roles'; + const roleName = 'some 安全性 role'; + window.location.hash = `${basePath}/edit/${roleName}`; + + const { setBreadcrumbs } = await mountApp(basePath); + + expect(setBreadcrumbs).toHaveBeenCalledTimes(1); + expect(setBreadcrumbs).toHaveBeenCalledWith([ + { href: `#${basePath}`, text: 'Roles' }, + { + href: '#/some-base-path/roles/edit/some%20%E5%AE%89%E5%85%A8%E6%80%A7%20role', + text: roleName, + }, + ]); + }); }); diff --git a/x-pack/plugins/security/public/management/roles/roles_management_app.tsx b/x-pack/plugins/security/public/management/roles/roles_management_app.tsx index dd4dcd02750f3..4e8c95b61c2f1 100644 --- a/x-pack/plugins/security/public/management/roles/roles_management_app.tsx +++ b/x-pack/plugins/security/public/management/roles/roles_management_app.tsx @@ -26,7 +26,7 @@ interface CreateParams { getStartServices: CoreSetup['getStartServices']; } -export const RolesManagementApp = Object.freeze({ +export const rolesManagementApp = Object.freeze({ id: 'roles', create({ license, fatalErrors, getStartServices }: CreateParams) { return { @@ -58,7 +58,7 @@ export const RolesManagementApp = Object.freeze({ setBreadcrumbs([ ...rolesBreadcrumbs, action === 'edit' && roleName - ? { text: roleName, href: `#${basePath}/edit/${roleName}` } + ? { text: roleName, href: `#${basePath}/edit/${encodeURIComponent(roleName)}` } : { text: i18n.translate('xpack.security.roles.createBreadcrumb', { defaultMessage: 'Create', 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 50cfabab61166..543d20bb92afe 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 @@ -14,6 +14,7 @@ import { ReactWrapper } from 'enzyme'; import { coreMock } from '../../../../../../../src/core/public/mocks'; import { mockAuthenticatedUser } from '../../../../common/model/authenticated_user.mock'; import { securityMock } from '../../../mocks'; +import { rolesAPIClientMock } from '../../roles/index.mock'; import { userAPIClientMock } from '../index.mock'; const createUser = (username: string) => { @@ -34,12 +35,12 @@ const createUser = (username: string) => { return user; }; -const buildClient = () => { - const apiClientMock = userAPIClientMock.create(); +const buildClients = () => { + const apiClient = userAPIClientMock.create(); + apiClient.getUser.mockImplementation(async (username: string) => createUser(username)); - apiClientMock.getUser.mockImplementation(async (username: string) => createUser(username)); - - apiClientMock.getRoles.mockImplementation(() => { + const rolesAPIClient = rolesAPIClientMock.create(); + rolesAPIClient.getRoles.mockImplementation(() => { return Promise.resolve([ { name: 'role 1', @@ -62,7 +63,7 @@ const buildClient = () => { ] as Role[]); }); - return apiClientMock; + return { apiClient, rolesAPIClient }; }; function buildSecuritySetup() { @@ -83,12 +84,13 @@ function expectMissingSaveButton(wrapper: ReactWrapper) { describe('EditUserPage', () => { it('allows reserved users to be viewed', async () => { - const apiClient = buildClient(); + const { apiClient, rolesAPIClient } = buildClients(); const securitySetup = buildSecuritySetup(); const wrapper = mountWithIntl( @@ -103,12 +105,13 @@ describe('EditUserPage', () => { }); it('allows new users to be created', async () => { - const apiClient = buildClient(); + const { apiClient, rolesAPIClient } = buildClients(); const securitySetup = buildSecuritySetup(); const wrapper = mountWithIntl( @@ -123,12 +126,13 @@ describe('EditUserPage', () => { }); it('allows existing users to be edited', async () => { - const apiClient = buildClient(); + const { apiClient, rolesAPIClient } = buildClients(); const securitySetup = buildSecuritySetup(); const wrapper = mountWithIntl( 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 77cad1134a2dd..576f3ff9e6008 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 @@ -32,6 +32,7 @@ import { NotificationsStart } from 'src/core/public'; import { User, EditUser, Role } from '../../../../common/model'; import { AuthenticationServiceSetup } from '../../../authentication'; import { USERS_PATH } from '../../management_urls'; +import { RolesAPIClient } from '../../roles'; import { ConfirmDeleteUsers, ChangePasswordForm } from '../components'; import { UserValidator, UserValidationResult } from './validate_user'; import { UserAPIClient } from '..'; @@ -39,6 +40,7 @@ import { UserAPIClient } from '..'; interface Props { username?: string; apiClient: PublicMethodsOf; + rolesAPIClient: PublicMethodsOf; authc: AuthenticationServiceSetup; notifications: NotificationsStart; } @@ -97,7 +99,7 @@ export class EditUserPage extends Component { } private async setCurrentUser() { - const { username, apiClient, notifications, authc } = this.props; + const { username, apiClient, rolesAPIClient, notifications, authc } = this.props; let { user, currentUser } = this.state; if (username) { try { @@ -120,7 +122,7 @@ export class EditUserPage extends Component { let roles: Role[] = []; try { - roles = await apiClient.getRoles(); + roles = await rolesAPIClient.getRoles(); } catch (err) { notifications.toasts.addDanger({ title: i18n.translate('xpack.security.management.users.editUser.errorLoadingRolesTitle', { diff --git a/x-pack/plugins/security/public/management/users/index.ts b/x-pack/plugins/security/public/management/users/index.ts index f29a6aa1bd46f..c8b4d41973da6 100644 --- a/x-pack/plugins/security/public/management/users/index.ts +++ b/x-pack/plugins/security/public/management/users/index.ts @@ -5,4 +5,4 @@ */ export { UserAPIClient } from './user_api_client'; -export { UsersManagementApp } from './users_management_app'; +export { usersManagementApp } from './users_management_app'; diff --git a/x-pack/plugins/security/public/management/users/user_api_client.mock.ts b/x-pack/plugins/security/public/management/users/user_api_client.mock.ts index cbebb422e384a..7223f78d57fdc 100644 --- a/x-pack/plugins/security/public/management/users/user_api_client.mock.ts +++ b/x-pack/plugins/security/public/management/users/user_api_client.mock.ts @@ -10,8 +10,6 @@ export const userAPIClientMock = { getUser: jest.fn(), deleteUser: jest.fn(), saveUser: jest.fn(), - getRoles: jest.fn(), - getRole: jest.fn(), changePassword: jest.fn(), }), }; diff --git a/x-pack/plugins/security/public/management/users/user_api_client.ts b/x-pack/plugins/security/public/management/users/user_api_client.ts index a0fa415248bee..61dd09d2c5e3d 100644 --- a/x-pack/plugins/security/public/management/users/user_api_client.ts +++ b/x-pack/plugins/security/public/management/users/user_api_client.ts @@ -5,10 +5,9 @@ */ import { HttpStart } from 'src/core/public'; -import { Role, User, EditUser } from '../../../common/model'; +import { User, EditUser } from '../../../common/model'; const usersUrl = '/internal/security/users'; -const rolesUrl = '/api/security/role'; export class UserAPIClient { constructor(private readonly http: HttpStart) {} @@ -31,14 +30,6 @@ export class UserAPIClient { }); } - public async getRoles() { - return await this.http.get(rolesUrl); - } - - public async getRole(name: string) { - return await this.http.get(`${rolesUrl}/${encodeURIComponent(name)}`); - } - public async changePassword(username: string, password: string, currentPassword: string) { const data: Record = { newPassword: password, diff --git a/x-pack/plugins/security/public/management/users/users_management_app.test.tsx b/x-pack/plugins/security/public/management/users/users_management_app.test.tsx index b4f619368d599..48ffcfc550a84 100644 --- a/x-pack/plugins/security/public/management/users/users_management_app.test.tsx +++ b/x-pack/plugins/security/public/management/users/users_management_app.test.tsx @@ -12,7 +12,7 @@ jest.mock('./edit_user', () => ({ EditUserPage: (props: any) => `User Edit Page: ${JSON.stringify(props)}`, })); -import { UsersManagementApp } from './users_management_app'; +import { usersManagementApp } from './users_management_app'; import { coreMock } from '../../../../../../src/core/public/mocks'; import { securityMock } from '../../mocks'; @@ -21,18 +21,20 @@ async function mountApp(basePath: string) { const container = document.createElement('div'); const setBreadcrumbs = jest.fn(); - const unmount = await UsersManagementApp.create({ - authc: securityMock.createSetup().authc, - getStartServices: coreMock.createSetup().getStartServices as any, - }).mount({ basePath, element: container, setBreadcrumbs }); + const unmount = await usersManagementApp + .create({ + authc: securityMock.createSetup().authc, + getStartServices: coreMock.createSetup().getStartServices as any, + }) + .mount({ basePath, element: container, setBreadcrumbs }); return { unmount, container, setBreadcrumbs }; } -describe('UsersManagementApp', () => { +describe('usersManagementApp', () => { it('create() returns proper management app descriptor', () => { expect( - UsersManagementApp.create({ + usersManagementApp.create({ authc: securityMock.createSetup().authc, getStartServices: coreMock.createSetup().getStartServices as any, }) @@ -78,7 +80,7 @@ describe('UsersManagementApp', () => { ]); expect(container).toMatchInlineSnapshot(`
- User Edit Page: {"authc":{},"apiClient":{"http":{"basePath":{"basePath":""},"anonymousPaths":{}}},"notifications":{"toasts":{}}} + User Edit Page: {"authc":{},"apiClient":{"http":{"basePath":{"basePath":""},"anonymousPaths":{}}},"rolesAPIClient":{"http":{"basePath":{"basePath":""},"anonymousPaths":{}}},"notifications":{"toasts":{}}}
`); @@ -101,7 +103,7 @@ describe('UsersManagementApp', () => { ]); expect(container).toMatchInlineSnapshot(`
- User Edit Page: {"authc":{},"apiClient":{"http":{"basePath":{"basePath":""},"anonymousPaths":{}}},"notifications":{"toasts":{}},"username":"someUserName"} + User Edit Page: {"authc":{},"apiClient":{"http":{"basePath":{"basePath":""},"anonymousPaths":{}}},"rolesAPIClient":{"http":{"basePath":{"basePath":""},"anonymousPaths":{}}},"notifications":{"toasts":{}},"username":"someUserName"}
`); @@ -109,4 +111,21 @@ describe('UsersManagementApp', () => { expect(container).toMatchInlineSnapshot(`
`); }); + + it('mount() properly encodes user name in `edit user` page link in breadcrumbs', async () => { + const basePath = '/some-base-path/users'; + const username = 'some 安全性 user'; + window.location.hash = `${basePath}/edit/${username}`; + + const { setBreadcrumbs } = await mountApp(basePath); + + expect(setBreadcrumbs).toHaveBeenCalledTimes(1); + expect(setBreadcrumbs).toHaveBeenCalledWith([ + { href: `#${basePath}`, text: 'Users' }, + { + href: '#/some-base-path/users/edit/some%20%E5%AE%89%E5%85%A8%E6%80%A7%20user', + text: username, + }, + ]); + }); }); diff --git a/x-pack/plugins/security/public/management/users/users_management_app.tsx b/x-pack/plugins/security/public/management/users/users_management_app.tsx index f1d4f11f8b760..9aebb396ce9a9 100644 --- a/x-pack/plugins/security/public/management/users/users_management_app.tsx +++ b/x-pack/plugins/security/public/management/users/users_management_app.tsx @@ -12,6 +12,7 @@ import { CoreSetup } from 'src/core/public'; import { RegisterManagementAppArgs } from '../../../../../../src/plugins/management/public'; import { AuthenticationServiceSetup } from '../../authentication'; import { PluginStartDependencies } from '../../plugin'; +import { RolesAPIClient } from '../roles'; import { UserAPIClient } from './user_api_client'; import { UsersGridPage } from './users_grid'; import { EditUserPage } from './edit_user'; @@ -21,7 +22,7 @@ interface CreateParams { getStartServices: CoreSetup['getStartServices']; } -export const UsersManagementApp = Object.freeze({ +export const usersManagementApp = Object.freeze({ id: 'users', create({ authc, getStartServices }: CreateParams) { return { @@ -49,7 +50,7 @@ export const UsersManagementApp = Object.freeze({ setBreadcrumbs([ ...usersBreadcrumbs, username - ? { text: username, href: `#${basePath}/edit/${username}` } + ? { text: username, href: `#${basePath}/edit/${encodeURIComponent(username)}` } : { text: i18n.translate('xpack.security.users.createBreadcrumb', { defaultMessage: 'Create', @@ -61,6 +62,7 @@ export const UsersManagementApp = Object.freeze({ diff --git a/x-pack/plugins/security/public/plugin.tsx b/x-pack/plugins/security/public/plugin.tsx index 5f84e2386aeaa..394e23cbbf646 100644 --- a/x-pack/plugins/security/public/plugin.tsx +++ b/x-pack/plugins/security/public/plugin.tsx @@ -87,8 +87,7 @@ export class SecurityPlugin }); } - // Home is an optional plugin, and if it's disabled we shouldn't try to fill feature catalog. - if (home) { + if (management && home) { home.featureCatalogue.register({ id: 'security', title: i18n.translate('xpack.security.registerFeature.securitySettingsTitle', { @@ -99,7 +98,7 @@ export class SecurityPlugin 'Protect your data and easily manage who has access to what with users and roles.', }), icon: 'securityApp', - path: '/app/kibana#/management/security', + path: '/app/kibana#/management/security/users', showOnHomePage: true, category: FeatureCatalogueCategory.ADMIN, });