Properly encode links to edit user page (#81562)

elastic · Oct 26, 2020 · 9f7ccc6 · 9f7ccc6
1 parent 7e34bf2
commit 9f7ccc6
Show file tree

Hide file tree

Showing 4 changed files with 63 additions and 6 deletions.
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
@@ -66,6 +66,11 @@ describe('<RolesGridPage />', () => {
         kibana: [{ base: [], spaces: [], feature: {} }],
         transient_metadata: { enabled: false },
       },
+      {
+        name: 'special%chars%role',
+        elasticsearch: { cluster: [], indices: [], run_as: [] },
+        kibana: [{ base: [], spaces: [], feature: {} }],
+      },
     ]);
   });
 
@@ -121,7 +126,7 @@ describe('<RolesGridPage />', () => {
     expect(wrapper.find(PermissionDenied)).toMatchSnapshot();
   });
 
-  it('renders role actions as appropriate', async () => {
+  it('renders role actions as appropriate, escaping when necessary', async () => {
     const wrapper = mountWithIntl(
       <RolesGridPage
         rolesAPIClient={apiClientMock}
@@ -137,16 +142,26 @@ describe('<RolesGridPage />', () => {
 
     expect(wrapper.find(PermissionDenied)).toHaveLength(0);
 
-    const editButton = wrapper.find('EuiButtonIcon[data-test-subj="edit-role-action-test-role-1"]');
+    let editButton = wrapper.find('EuiButtonIcon[data-test-subj="edit-role-action-test-role-1"]');
     expect(editButton).toHaveLength(1);
     expect(editButton.prop('href')).toBe('/edit/test-role-1');
 
-    const cloneButton = wrapper.find(
-      'EuiButtonIcon[data-test-subj="clone-role-action-test-role-1"]'
+    editButton = wrapper.find(
+      'EuiButtonIcon[data-test-subj="edit-role-action-special%chars%role"]'
     );
+    expect(editButton).toHaveLength(1);
+    expect(editButton.prop('href')).toBe('/edit/special%25chars%25role');
+
+    let cloneButton = wrapper.find('EuiButtonIcon[data-test-subj="clone-role-action-test-role-1"]');
     expect(cloneButton).toHaveLength(1);
     expect(cloneButton.prop('href')).toBe('/clone/test-role-1');
 
+    cloneButton = wrapper.find(
+      'EuiButtonIcon[data-test-subj="clone-role-action-special%chars%role"]'
+    );
+    expect(cloneButton).toHaveLength(1);
+    expect(cloneButton.prop('href')).toBe('/clone/special%25chars%25role');
+
     expect(
       wrapper.find('EuiButtonIcon[data-test-subj="edit-role-action-disabled-role"]')
     ).toHaveLength(1);
@@ -182,6 +197,11 @@ describe('<RolesGridPage />', () => {
         kibana: [{ base: [], spaces: [], feature: {} }],
         metadata: { _reserved: true },
       },
+      {
+        name: 'special%chars%role',
+        elasticsearch: { cluster: [], indices: [], run_as: [] },
+        kibana: [{ base: [], spaces: [], feature: {} }],
+      },
       {
         name: 'test-role-1',
         elasticsearch: { cluster: [], indices: [], run_as: [] },
@@ -198,6 +218,11 @@ describe('<RolesGridPage />', () => {
         kibana: [{ base: [], spaces: [], feature: {} }],
         transient_metadata: { enabled: false },
       },
+      {
+        name: 'special%chars%role',
+        elasticsearch: { cluster: [], indices: [], run_as: [] },
+        kibana: [{ base: [], spaces: [], feature: {} }],
+      },
       {
         name: 'test-role-1',
         elasticsearch: { cluster: [], indices: [], run_as: [] },

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
@@ -58,7 +58,7 @@ interface State {
 }
 
 const getRoleManagementHref = (action: 'edit' | 'clone', roleName?: string) => {
-  return `/${action}${roleName ? `/${roleName}` : ''}`;
+  return `/${action}${roleName ? `/${encodeURIComponent(roleName)}` : ''}`;
 };
 
 export class RolesGridPage extends Component<Props, State> {

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
@@ -71,6 +71,38 @@ describe('UsersGridPage', () => {
     expect(findTestSubject(wrapper, 'userDisabled')).toHaveLength(0);
   });
 
+  it('generates valid links when usernames contain special characters', async () => {
+    const apiClientMock = userAPIClientMock.create();
+    apiClientMock.getUsers.mockImplementation(() => {
+      return Promise.resolve<User[]>([
+        {
+          username: 'username with some fun characters!@#$%^&*()',
+          email: '[email protected]',
+          full_name: 'foo bar',
+          roles: ['kibana_user'],
+          enabled: true,
+        },
+      ]);
+    });
+
+    const wrapper = mountWithIntl(
+      <UsersGridPage
+        userAPIClient={apiClientMock}
+        rolesAPIClient={rolesAPIClientMock.create()}
+        notifications={coreStart.notifications}
+        history={history}
+        navigateToApp={coreStart.application.navigateToApp}
+      />
+    );
+
+    await waitForRender(wrapper);
+
+    const link = findTestSubject(wrapper, 'userRowUserName');
+    expect(link.props().href).toMatchInlineSnapshot(
+      `"/edit/username%20with%20some%20fun%20characters!%40%23%24%25%5E%26*()"`
+    );
+  });
+
   it('renders a forbidden message if user is not authorized', async () => {
     const apiClient = userAPIClientMock.create();
     apiClient.getUsers.mockRejectedValue({ body: { statusCode: 403 } });

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
@@ -112,7 +112,7 @@ export class UsersGridPage extends Component<Props, State> {
         render: (username: string) => (
           <EuiLink
             data-test-subj="userRowUserName"
-            {...reactRouterNavigate(this.props.history, `/edit/${username}`)}
+            {...reactRouterNavigate(this.props.history, `/edit/${encodeURIComponent(username)}`)}
           >
             {username}
           </EuiLink>