Skip to content

Commit

Permalink
Adjust encoding for security management pages (elastic#83629)
Browse files Browse the repository at this point in the history
* Adjust encoding for security management pages

* introduce tryDecodeURIComponent
  • Loading branch information
legrego committed Nov 20, 2020
1 parent 3f90e72 commit 6beeba4
Show file tree
Hide file tree
Showing 8 changed files with 82 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -83,18 +83,18 @@ describe('roleMappingsManagementApp', () => {
});

it('mount() works for the `edit role mapping` page', async () => {
const roleMappingName = 'someRoleMappingName';
const roleMappingName = 'role@mapping';

const { setBreadcrumbs, container, unmount } = await mountApp('/', `/edit/${roleMappingName}`);

expect(setBreadcrumbs).toHaveBeenCalledTimes(1);
expect(setBreadcrumbs).toHaveBeenCalledWith([
{ href: `/`, text: 'Role Mappings' },
{ href: `/edit/${roleMappingName}`, text: roleMappingName },
{ href: `/edit/${encodeURIComponent(roleMappingName)}`, text: roleMappingName },
]);
expect(container).toMatchInlineSnapshot(`
<div>
Role Mapping Edit Page: {"name":"someRoleMappingName","roleMappingsAPI":{"http":{"basePath":{"basePath":"","serverBasePath":""},"anonymousPaths":{}}},"rolesAPIClient":{"http":{"basePath":{"basePath":"","serverBasePath":""},"anonymousPaths":{}}},"notifications":{"toasts":{}},"docLinks":{"esDocBasePath":"https://www.elastic.co/guide/en/elasticsearch/reference/mocked-test-branch/"},"history":{"action":"PUSH","length":1,"location":{"pathname":"/edit/someRoleMappingName","search":"","hash":""}}}
Role Mapping Edit Page: {"name":"role@mapping","roleMappingsAPI":{"http":{"basePath":{"basePath":"","serverBasePath":""},"anonymousPaths":{}}},"rolesAPIClient":{"http":{"basePath":{"basePath":"","serverBasePath":""},"anonymousPaths":{}}},"notifications":{"toasts":{}},"docLinks":{"esDocBasePath":"https://www.elastic.co/guide/en/elasticsearch/reference/mocked-test-branch/"},"history":{"action":"PUSH","length":1,"location":{"pathname":"/edit/role@mapping","search":"","hash":""}}}
</div>
`);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { StartServicesAccessor } from 'src/core/public';
import { RegisterManagementAppArgs } from '../../../../../../src/plugins/management/public';
import { PluginStartDependencies } from '../../plugin';
import { DocumentationLinksService } from './documentation_links';
import { tryDecodeURIComponent } from '../url_utils';

interface CreateParams {
getStartServices: StartServicesAccessor<PluginStartDependencies>;
Expand Down Expand Up @@ -70,10 +71,14 @@ export const roleMappingsManagementApp = Object.freeze({
const EditRoleMappingsPageWithBreadcrumbs = () => {
const { name } = useParams<{ name?: string }>();

// Additional decoding is a workaround for a bug in react-router's version of the `history` module.
// See https://github.com/elastic/kibana/issues/82440
const decodedName = name ? tryDecodeURIComponent(name) : undefined;

setBreadcrumbs([
...roleMappingsBreadcrumbs,
name
? { text: name, href: `/edit/${encodeURIComponent(name)}` }
? { text: decodedName, href: `/edit/${encodeURIComponent(name)}` }
: {
text: i18n.translate('xpack.security.roleMappings.createBreadcrumb', {
defaultMessage: 'Create',
Expand All @@ -83,7 +88,7 @@ export const roleMappingsManagementApp = Object.freeze({

return (
<EditRoleMappingPage
name={name}
name={decodedName}
roleMappingsAPI={roleMappingsAPIClient}
rolesAPIClient={new RolesAPIClient(http)}
notifications={notifications}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,18 +97,18 @@ describe('rolesManagementApp', () => {
});

it('mount() works for the `edit role` page', async () => {
const roleName = 'someRoleName';
const roleName = 'role@name';

const { setBreadcrumbs, container, unmount } = await mountApp('/', `/edit/${roleName}`);

expect(setBreadcrumbs).toHaveBeenCalledTimes(1);
expect(setBreadcrumbs).toHaveBeenCalledWith([
{ href: `/`, text: 'Roles' },
{ href: `/edit/${roleName}`, text: roleName },
{ href: `/edit/${encodeURIComponent(roleName)}`, text: roleName },
]);
expect(container).toMatchInlineSnapshot(`
<div>
Role Edit Page: {"action":"edit","roleName":"someRoleName","rolesAPIClient":{"http":{"basePath":{"basePath":"","serverBasePath":""},"anonymousPaths":{}}},"userAPIClient":{"http":{"basePath":{"basePath":"","serverBasePath":""},"anonymousPaths":{}}},"indicesAPIClient":{"http":{"basePath":{"basePath":"","serverBasePath":""},"anonymousPaths":{}}},"privilegesAPIClient":{"http":{"basePath":{"basePath":"","serverBasePath":""},"anonymousPaths":{}}},"http":{"basePath":{"basePath":"","serverBasePath":""},"anonymousPaths":{}},"notifications":{"toasts":{}},"fatalErrors":{},"license":{"features$":{"_isScalar":false}},"docLinks":{"esDocBasePath":"https://www.elastic.co/guide/en/elasticsearch/reference/mocked-test-branch/"},"uiCapabilities":{"catalogue":{},"management":{},"navLinks":{}},"history":{"action":"PUSH","length":1,"location":{"pathname":"/edit/someRoleName","search":"","hash":""}}}
Role Edit Page: {"action":"edit","roleName":"role@name","rolesAPIClient":{"http":{"basePath":{"basePath":"","serverBasePath":""},"anonymousPaths":{}}},"userAPIClient":{"http":{"basePath":{"basePath":"","serverBasePath":""},"anonymousPaths":{}}},"indicesAPIClient":{"http":{"basePath":{"basePath":"","serverBasePath":""},"anonymousPaths":{}}},"privilegesAPIClient":{"http":{"basePath":{"basePath":"","serverBasePath":""},"anonymousPaths":{}}},"http":{"basePath":{"basePath":"","serverBasePath":""},"anonymousPaths":{}},"notifications":{"toasts":{}},"fatalErrors":{},"license":{"features$":{"_isScalar":false}},"docLinks":{"esDocBasePath":"https://www.elastic.co/guide/en/elasticsearch/reference/mocked-test-branch/"},"uiCapabilities":{"catalogue":{},"management":{},"navLinks":{}},"history":{"action":"PUSH","length":1,"location":{"pathname":"/edit/role@name","search":"","hash":""}}}
</div>
`);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { RegisterManagementAppArgs } from '../../../../../../src/plugins/managem
import { SecurityLicense } from '../../../common/licensing';
import { PluginStartDependencies } from '../../plugin';
import { DocumentationLinksService } from './documentation_links';
import { tryDecodeURIComponent } from '../url_utils';

interface CreateParams {
fatalErrors: FatalErrorsSetup;
Expand Down Expand Up @@ -68,10 +69,14 @@ export const rolesManagementApp = Object.freeze({
const EditRolePageWithBreadcrumbs = ({ action }: { action: 'edit' | 'clone' }) => {
const { roleName } = useParams<{ roleName?: string }>();

// Additional decoding is a workaround for a bug in react-router's version of the `history` module.
// See https://github.com/elastic/kibana/issues/82440
const decodedRoleName = roleName ? tryDecodeURIComponent(roleName) : undefined;

setBreadcrumbs([
...rolesBreadcrumbs,
action === 'edit' && roleName
? { text: roleName, href: `/edit/${encodeURIComponent(roleName)}` }
? { text: decodedRoleName, href: `/edit/${encodeURIComponent(roleName)}` }
: {
text: i18n.translate('xpack.security.roles.createBreadcrumb', {
defaultMessage: 'Create',
Expand All @@ -82,7 +87,7 @@ export const rolesManagementApp = Object.freeze({
return (
<EditRolePage
action={action}
roleName={roleName}
roleName={decodedRoleName}
rolesAPIClient={rolesAPIClient}
userAPIClient={new UserAPIClient(http)}
indicesAPIClient={new IndicesAPIClient(http)}
Expand Down
21 changes: 21 additions & 0 deletions x-pack/plugins/security/public/management/uri_utils.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/*
* 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 { tryDecodeURIComponent } from './url_utils';

describe('tryDecodeURIComponent', () => {
it('properly decodes a URI Component', () => {
expect(
tryDecodeURIComponent('sample%26piece%3Dof%20text%40gmail.com%2520')
).toMatchInlineSnapshot(`"sample&piece=of text@gmail.com%20"`);
});

it('returns the original string undecoded if it is malformed', () => {
expect(tryDecodeURIComponent('sample&piece=of%text@gmail.com%20')).toMatchInlineSnapshot(
`"sample&piece=of%text@gmail.com%20"`
);
});
});
13 changes: 13 additions & 0 deletions x-pack/plugins/security/public/management/url_utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/*
* 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.
*/

export const tryDecodeURIComponent = (uriComponent: string) => {
try {
return decodeURIComponent(uriComponent);
} catch {
return uriComponent;
}
};
Original file line number Diff line number Diff line change
Expand Up @@ -86,18 +86,18 @@ describe('usersManagementApp', () => {
});

it('mount() works for the `edit user` page', async () => {
const userName = 'someUserName';
const userName = 'foo@bar.com';

const { setBreadcrumbs, container, unmount } = await mountApp('/', `/edit/${userName}`);

expect(setBreadcrumbs).toHaveBeenCalledTimes(1);
expect(setBreadcrumbs).toHaveBeenCalledWith([
{ href: `/`, text: 'Users' },
{ href: `/edit/${userName}`, text: userName },
{ href: `/edit/${encodeURIComponent(userName)}`, text: userName },
]);
expect(container).toMatchInlineSnapshot(`
<div>
User Edit Page: {"authc":{},"userAPIClient":{"http":{"basePath":{"basePath":"","serverBasePath":""},"anonymousPaths":{}}},"rolesAPIClient":{"http":{"basePath":{"basePath":"","serverBasePath":""},"anonymousPaths":{}}},"notifications":{"toasts":{}},"username":"someUserName","history":{"action":"PUSH","length":1,"location":{"pathname":"/edit/someUserName","search":"","hash":""}}}
User Edit Page: {"authc":{},"userAPIClient":{"http":{"basePath":{"basePath":"","serverBasePath":""},"anonymousPaths":{}}},"rolesAPIClient":{"http":{"basePath":{"basePath":"","serverBasePath":""},"anonymousPaths":{}}},"notifications":{"toasts":{}},"username":"foo@bar.com","history":{"action":"PUSH","length":1,"location":{"pathname":"/edit/foo@bar.com","search":"","hash":""}}}
</div>
`);

Expand All @@ -106,18 +106,23 @@ describe('usersManagementApp', () => {
expect(container).toMatchInlineSnapshot(`<div />`);
});

it('mount() properly encodes user name in `edit user` page link in breadcrumbs', async () => {
const username = 'some 安全性 user';

const { setBreadcrumbs } = await mountApp('/', `/edit/${username}`);

expect(setBreadcrumbs).toHaveBeenCalledTimes(1);
expect(setBreadcrumbs).toHaveBeenCalledWith([
{ href: `/`, text: 'Users' },
{
href: '/edit/some%20%E5%AE%89%E5%85%A8%E6%80%A7%20user',
text: username,
},
]);
const usernames = ['foo@bar.com', 'foo&bar.com', 'some 安全性 user'];
usernames.forEach((username) => {
it(
'mount() properly encodes user name in `edit user` page link in breadcrumbs for user ' +
username,
async () => {
const { setBreadcrumbs } = await mountApp('/', `/edit/${username}`);

expect(setBreadcrumbs).toHaveBeenCalledTimes(1);
expect(setBreadcrumbs).toHaveBeenCalledWith([
{ href: `/`, text: 'Users' },
{
href: `/edit/${encodeURIComponent(username)}`,
text: username,
},
]);
}
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { StartServicesAccessor } from 'src/core/public';
import { RegisterManagementAppArgs } from '../../../../../../src/plugins/management/public';
import { AuthenticationServiceSetup } from '../../authentication';
import { PluginStartDependencies } from '../../plugin';
import { tryDecodeURIComponent } from '../url_utils';

interface CreateParams {
authc: AuthenticationServiceSetup;
Expand Down Expand Up @@ -66,10 +67,14 @@ export const usersManagementApp = Object.freeze({
const EditUserPageWithBreadcrumbs = () => {
const { username } = useParams<{ username?: string }>();

// Additional decoding is a workaround for a bug in react-router's version of the `history` module.
// See https://github.com/elastic/kibana/issues/82440
const decodedUsername = username ? tryDecodeURIComponent(username) : undefined;

setBreadcrumbs([
...usersBreadcrumbs,
username
? { text: username, href: `/edit/${encodeURIComponent(username)}` }
? { text: decodedUsername, href: `/edit/${encodeURIComponent(username)}` }
: {
text: i18n.translate('xpack.security.users.createBreadcrumb', {
defaultMessage: 'Create',
Expand All @@ -83,7 +88,7 @@ export const usersManagementApp = Object.freeze({
userAPIClient={userAPIClient}
rolesAPIClient={new RolesAPIClient(http)}
notifications={notifications}
username={username}
username={decodedUsername}
history={history}
/>
);
Expand Down

0 comments on commit 6beeba4

Please sign in to comment.