[7.x] URL encoding for URL drilldown (#86902) (#87143)

* URL encoding for URL drilldown (#86902) * feat: 🎸 use EuiSwitch for "Open in new window" toggle * feat: 🎸 add "URL encoding" option and "Additional options" * feat: 🎸 make "Open in new window" true by default * feat: 🎸 respect encoding config setting * test: 💍 add encoding tests * feat: 🎸 add URI encoding Handlebars helpers * docs: ✏️ add URL encoding methods to URL Drilldown docs Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> * test: 💍 align 7.x branch with master Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
elastic · Jan 5, 2021 · 98485a5 · 98485a5
1 parent 3e50b5f
commit 98485a5
Show file tree

Hide file tree

Showing 9 changed files with 185 additions and 18 deletions.
diff --git a/docs/user/dashboard/url-drilldown.asciidoc b/docs/user/dashboard/url-drilldown.asciidoc
@@ -133,6 +133,12 @@ Example:
 
 `{{split event.value ","}}`
 
+|encodeURIComponent
+a|Escapes string using built in `encodeURIComponent` function.
+
+|encodeURIQuery
+a|Escapes string using built in `encodeURIComponent` function, while keeping "@", ":", "$", ",", and ";" characters as is.
+
 |===
 
 

diff --git a/x-pack/plugins/drilldowns/url_drilldown/public/lib/url_drilldown.test.ts b/x-pack/plugins/drilldowns/url_drilldown/public/lib/url_drilldown.test.ts
@@ -443,3 +443,77 @@ describe('UrlDrilldown', () => {
     });
   });
 });
+
+describe('encoding', () => {
+  const urlDrilldown = createDrilldown();
+  const context: ActionContext = {
+    data: {
+      data: mockDataPoints,
+    },
+    embeddable: mockEmbeddable,
+  };
+
+  test('encodes URL by default', async () => {
+    const config: Config = {
+      url: {
+        template: 'https://elastic.co?foo=head%26shoulders',
+      },
+      openInNewTab: false,
+    };
+    const url = await urlDrilldown.getHref(config, context);
+
+    expect(url).toBe('https://elastic.co?foo=head%2526shoulders');
+  });
+
+  test('encodes URL when encoding is enabled', async () => {
+    const config: Config = {
+      url: {
+        template: 'https://elastic.co?foo=head%26shoulders',
+      },
+      openInNewTab: false,
+      encodeUrl: true,
+    };
+    const url = await urlDrilldown.getHref(config, context);
+
+    expect(url).toBe('https://elastic.co?foo=head%2526shoulders');
+  });
+
+  test('does not encode URL when encoding is not enabled', async () => {
+    const config: Config = {
+      url: {
+        template: 'https://elastic.co?foo=head%26shoulders',
+      },
+      openInNewTab: false,
+      encodeUrl: false,
+    };
+    const url = await urlDrilldown.getHref(config, context);
+
+    expect(url).toBe('https://elastic.co?foo=head%26shoulders');
+  });
+
+  test('can encode URI component using "encodeURIComponent" Handlebars helper', async () => {
+    const config: Config = {
+      url: {
+        template: 'https://elastic.co?foo={{encodeURIComponent "head%26shoulders@gmail.com"}}',
+      },
+      openInNewTab: false,
+      encodeUrl: false,
+    };
+    const url = await urlDrilldown.getHref(config, context);
+
+    expect(url).toBe('https://elastic.co?foo=head%2526shoulders%40gmail.com');
+  });
+
+  test('can encode URI component using "encodeURIQuery" Handlebars helper', async () => {
+    const config: Config = {
+      url: {
+        template: 'https://elastic.co?foo={{encodeURIQuery "head%26shoulders@gmail.com"}}',
+      },
+      openInNewTab: false,
+      encodeUrl: false,
+    };
+    const url = await urlDrilldown.getHref(config, context);
+
+    expect(url).toBe('https://elastic.co?foo=head%2526shoulders@gmail.com');
+  });
+});
diff --git a/x-pack/plugins/drilldowns/url_drilldown/public/lib/url_drilldown.tsx b/x-pack/plugins/drilldowns/url_drilldown/public/lib/url_drilldown.tsx
@@ -104,7 +104,8 @@ export class UrlDrilldown implements Drilldown<Config, ActionContext, ActionFact
 
   public readonly createConfig = () => ({
     url: { template: '' },
-    openInNewTab: false,
+    openInNewTab: true,
+    encodeUrl: true,
   });
 
   public readonly isConfigValid = (config: Config): config is Config => {
@@ -133,7 +134,12 @@ export class UrlDrilldown implements Drilldown<Config, ActionContext, ActionFact
   };
 
   private buildUrl(config: Config, context: ActionContext): string {
-    const url = urlDrilldownCompileUrl(config.url.template, this.getRuntimeVariables(context));
+    const doEncode = config.encodeUrl ?? true;
+    const url = urlDrilldownCompileUrl(
+      config.url.template,
+      this.getRuntimeVariables(context),
+      doEncode
+    );
     return url;
   }
 

diff --git a/..._enhanced/public/drilldowns/url_drilldown/components/url_drilldown_collect_config/i18n.ts b/..._enhanced/public/drilldowns/url_drilldown/components/url_drilldown_collect_config/i18n.ts
@@ -34,7 +34,7 @@ export const txtAddVariableButtonTitle = i18n.translate(
 export const txtUrlTemplateLabel = i18n.translate(
   'xpack.uiActionsEnhanced.drilldowns.urlDrilldownCollectConfig.urlTemplateLabel',
   {
-    defaultMessage: 'Enter URL template:',
+    defaultMessage: 'Enter URL:',
   }
 );
 
@@ -76,6 +76,27 @@ export const txtUrlTemplatePreviewLinkText = i18n.translate(
 export const txtUrlTemplateOpenInNewTab = i18n.translate(
   'xpack.uiActionsEnhanced.drilldowns.urlDrilldownCollectConfig.openInNewTabLabel',
   {
-    defaultMessage: 'Open in new tab',
+    defaultMessage: 'Open in new window',
+  }
+);
+
+export const txtUrlTemplateAdditionalOptions = i18n.translate(
+  'xpack.uiActionsEnhanced.drilldowns.urlDrilldownCollectConfig.additionalOptions',
+  {
+    defaultMessage: 'Additional options',
+  }
+);
+
+export const txtUrlTemplateEncodeUrl = i18n.translate(
+  'xpack.uiActionsEnhanced.drilldowns.urlDrilldownCollectConfig.encodeUrl',
+  {
+    defaultMessage: 'Encode URL',
+  }
+);
+
+export const txtUrlTemplateEncodeDescription = i18n.translate(
+  'xpack.uiActionsEnhanced.drilldowns.urlDrilldownCollectConfig.encodeDescription',
+  {
+    defaultMessage: 'If enabled, URL will be escaped using percent encoding',
   }
 );
diff --git a/...ns/url_drilldown/components/url_drilldown_collect_config/url_drilldown_collect_config.tsx b/...ns/url_drilldown/components/url_drilldown_collect_config/url_drilldown_collect_config.tsx
@@ -6,7 +6,6 @@
 
 import React, { useRef, useState } from 'react';
 import {
-  EuiCheckbox,
   EuiFormRow,
   EuiIcon,
   EuiLink,
@@ -17,6 +16,11 @@ import {
   EuiText,
   EuiTextArea,
   EuiSelectableOption,
+  EuiSwitch,
+  EuiAccordion,
+  EuiSpacer,
+  EuiPanel,
+  EuiTextColor,
 } from '@elastic/eui';
 import { UrlDrilldownConfig } from '../../types';
 import './index.scss';
@@ -28,6 +32,9 @@ import {
   txtUrlTemplateLabel,
   txtUrlTemplateOpenInNewTab,
   txtUrlTemplatePlaceholder,
+  txtUrlTemplateAdditionalOptions,
+  txtUrlTemplateEncodeUrl,
+  txtUrlTemplateEncodeDescription,
 } from './i18n';
 
 export interface UrlDrilldownCollectConfig {
@@ -110,15 +117,39 @@ export const UrlDrilldownCollectConfig: React.FC<UrlDrilldownCollectConfig> = ({
           inputRef={textAreaRef}
         />
       </EuiFormRow>
-      <EuiFormRow hasChildLabel={false}>
-        <EuiCheckbox
-          id="openInNewTab"
-          name="openInNewTab"
-          label={txtUrlTemplateOpenInNewTab}
-          checked={config.openInNewTab}
-          onChange={() => onConfig({ ...config, openInNewTab: !config.openInNewTab })}
-        />
-      </EuiFormRow>
+      <EuiSpacer size={'l'} />
+      <EuiAccordion
+        id="accordion_url_drilldown_additional_options"
+        buttonContent={txtUrlTemplateAdditionalOptions}
+      >
+        <EuiSpacer size={'s'} />
+        <EuiPanel color="subdued" borderRadius="none" hasShadow={false} style={{ border: 'none' }}>
+          <EuiFormRow hasChildLabel={false}>
+            <EuiSwitch
+              id="openInNewTab"
+              name="openInNewTab"
+              label={txtUrlTemplateOpenInNewTab}
+              checked={config.openInNewTab}
+              onChange={() => onConfig({ ...config, openInNewTab: !config.openInNewTab })}
+            />
+          </EuiFormRow>
+          <EuiFormRow hasChildLabel={false} fullWidth>
+            <EuiSwitch
+              id="encodeUrl"
+              name="encodeUrl"
+              label={
+                <>
+                  {txtUrlTemplateEncodeUrl}
+                  <EuiSpacer size={'s'} />
+                  <EuiTextColor color="subdued">{txtUrlTemplateEncodeDescription}</EuiTextColor>
+                </>
+              }
+              checked={config.encodeUrl ?? true}
+              onChange={() => onConfig({ ...config, encodeUrl: !(config.encodeUrl ?? true) })}
+            />
+          </EuiFormRow>
+        </EuiPanel>
+      </EuiAccordion>
     </>
   );
 };

diff --git a/x-pack/plugins/ui_actions_enhanced/public/drilldowns/url_drilldown/types.ts b/x-pack/plugins/ui_actions_enhanced/public/drilldowns/url_drilldown/types.ts
@@ -7,6 +7,7 @@
 export type UrlDrilldownConfig = {
   url: { format?: 'handlebars_v1'; template: string };
   openInNewTab: boolean;
+  encodeUrl?: boolean;
 };
 
 /**

diff --git a/x-pack/plugins/ui_actions_enhanced/public/drilldowns/url_drilldown/url_template.test.ts b/x-pack/plugins/ui_actions_enhanced/public/drilldowns/url_drilldown/url_template.test.ts
@@ -12,6 +12,18 @@ test('should compile url without variables', () => {
   expect(compile(url, {})).toBe(url);
 });
 
+test('by default, encodes URI', () => {
+  const url = 'https://elastic.co?foo=head%26shoulders';
+  expect(compile(url, {})).not.toBe(url);
+  expect(compile(url, {})).toBe('https://elastic.co?foo=head%2526shoulders');
+});
+
+test('when URI encoding is disabled, should not encode URI', () => {
+  const url =
+    'https://xxxxx.service-now.com/nav_to.do?uri=incident.do%3Fsys_id%3D-1%26sysparm_query%3Dshort_description%3DHello';
+  expect(compile(url, {}, false)).toBe(url);
+});
+
 test('should fail on unknown syntax', () => {
   const url = 'https://elastic.co/{{}';
   expect(() => compile(url, {})).toThrowError();

diff --git a/x-pack/plugins/ui_actions_enhanced/public/drilldowns/url_drilldown/url_template.ts b/x-pack/plugins/ui_actions_enhanced/public/drilldowns/url_drilldown/url_template.ts
@@ -9,6 +9,7 @@ import { encode, RisonValue } from 'rison-node';
 import dateMath from '@elastic/datemath';
 import moment, { Moment } from 'moment';
 import numeral from '@elastic/numeral';
+import { url } from '../../../../../../src/plugins/kibana_utils/public';
 
 const handlebars = createHandlebars();
 
@@ -116,7 +117,22 @@ handlebars.registerHelper('replace', (...args) => {
   return String(str).split(searchString).join(valueString);
 });
 
-export function compile(url: string, context: object): string {
-  const template = handlebars.compile(url, { strict: true, noEscape: true });
-  return encodeURI(template(context));
+handlebars.registerHelper('encodeURIComponent', (component: unknown) => {
+  const str = String(component);
+  return encodeURIComponent(str);
+});
+handlebars.registerHelper('encodeURIQuery', (component: unknown) => {
+  const str = String(component);
+  return url.encodeUriQuery(str);
+});
+
+export function compile(urlTemplate: string, context: object, doEncode: boolean = true): string {
+  const handlebarsTemplate = handlebars.compile(urlTemplate, { strict: true, noEscape: true });
+  let processedUrl: string = handlebarsTemplate(context);
+
+  if (doEncode) {
+    processedUrl = encodeURI(processedUrl);
+  }
+
+  return processedUrl;
 }
diff --git a/x-pack/test/functional/apps/dashboard/drilldowns/dashboard_to_url_drilldown.ts b/x-pack/test/functional/apps/dashboard/drilldowns/dashboard_to_url_drilldown.ts
@@ -25,7 +25,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
       await PageObjects.dashboard.preserveCrossAppState();
     });
 
-    it('should create dashboard to URL drilldown and use it to navigate to discover', async () => {
+    it.skip('should create dashboard to URL drilldown and use it to navigate to discover', async () => {
       await PageObjects.dashboard.gotoDashboardEditMode(
         dashboardDrilldownsManage.DASHBOARD_WITH_AREA_CHART_NAME
       );