From 272c3d68347c0def0d6d041dde8fe698ec4b9240 Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Wed, 13 Apr 2022 09:22:02 -0600 Subject: [PATCH 01/13] Transform mailto: links in markdown to just the target address, as we unlink those values for security reasons --- .../plugins/markdown_link_validator.test.tsx | 15 +++++++++++++++ .../plugins/markdown_link_validator.tsx | 10 +++++++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/components/markdown_editor/plugins/markdown_link_validator.test.tsx b/src/components/markdown_editor/plugins/markdown_link_validator.test.tsx index a2318a468cf..f8e31fe9e10 100644 --- a/src/components/markdown_editor/plugins/markdown_link_validator.test.tsx +++ b/src/components/markdown_editor/plugins/markdown_link_validator.test.tsx @@ -72,4 +72,19 @@ describe('mutateLinkToText', () => { } `); }); + it('keeps only the email address when handling mailto: links', () => { + expect( + mutateLinkToText({ + type: 'link', + url: 'mailto:someone@elastic.co', + title: null, + children: [{ value: 'someone@elastic.co' }], + }) + ).toMatchInlineSnapshot(` + Object { + "type": "text", + "value": "someone@elastic.co", + } + `); + }); }); diff --git a/src/components/markdown_editor/plugins/markdown_link_validator.tsx b/src/components/markdown_editor/plugins/markdown_link_validator.tsx index 4a6c7a9a079..08d8669af32 100644 --- a/src/components/markdown_editor/plugins/markdown_link_validator.tsx +++ b/src/components/markdown_editor/plugins/markdown_link_validator.tsx @@ -30,7 +30,15 @@ export function markdownLinkValidator() { export function mutateLinkToText(node: LinkOrTextNode) { node.type = 'text'; - node.value = `[${node.children![0]?.value || ''}](${node.url})`; + + // https://github.com/elastic/eui/issues/5770 + // if this is a `mailto:` link only keep the target address + if (node.url?.startsWith('mailto:')) { + node.value = node.children![0]?.value; + } else { + node.value = `[${node.children![0]?.value || ''}](${node.url})`; + } + delete node.children; delete node.title; delete node.url; From 9daeb615454d8a3bc5b9e1002b627504ae24c6fa Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Wed, 13 Apr 2022 10:02:03 -0600 Subject: [PATCH 02/13] documentation --- .../markdown_editor/mardown_format_example.js | 27 +++++++++++++++++++ .../markdown_editor/markdown_format_links.js | 24 +++++++++++++++++ 2 files changed, 51 insertions(+) create mode 100644 src-docs/src/views/markdown_editor/markdown_format_links.js diff --git a/src-docs/src/views/markdown_editor/mardown_format_example.js b/src-docs/src/views/markdown_editor/mardown_format_example.js index dce19cfe922..32b86f92ffb 100644 --- a/src-docs/src/views/markdown_editor/mardown_format_example.js +++ b/src-docs/src/views/markdown_editor/mardown_format_example.js @@ -6,6 +6,7 @@ import { EuiMarkdownFormat, EuiText, EuiCode, + EuiCodeBlock, } from '../../../../src/components'; import { Link } from 'react-router-dom'; @@ -16,6 +17,9 @@ const markdownFormatSource = require('!!raw-loader!./markdown_format'); import MarkdownFormatStyles from './markdown_format_styles'; const markdownFormatStylesSource = require('!!raw-loader!./markdown_format_styles'); +import MarkdownFormatLinks, { markdownContent } from './markdown_format_links'; +const markdownFormatLinksSource = require('!!raw-loader!./markdown_format_links'); + import MarkdownFormatSink from './markdown_format_sink'; const markdownFormatSinkSource = require('!!raw-loader!./markdown_format_sink'); @@ -87,6 +91,29 @@ export const MarkdownFormatExample = { }, demo: , }, + { + source: [ + { + type: GuideSectionTypes.JS, + code: markdownFormatLinksSource, + }, + ], + title: 'Link validation for security', + text: ( + <> +

+ Markdown content often comes from untrusted sources like user generated content. To help with potential security issues, EuiMarkdownRenderer only renders links if they begin with https:, https, or /. Links that do not meet these requirements are rendered in the markdown link format, with the exception of mailto: where only the link copy is kept. +

+ + {markdownContent} + + + ), + props: { + EuiMarkdownFormat, + }, + demo: , + }, { source: [ { diff --git a/src-docs/src/views/markdown_editor/markdown_format_links.js b/src-docs/src/views/markdown_editor/markdown_format_links.js new file mode 100644 index 00000000000..5ea5537be4b --- /dev/null +++ b/src-docs/src/views/markdown_editor/markdown_format_links.js @@ -0,0 +1,24 @@ +import React from 'react'; + +import { EuiMarkdownFormat } from '../../../../src'; + +export const markdownContent = `**Links starting with http:, https:, and / are valid:** + +* https://elastic.com +* http://elastic.com +* https link to [elastic.co](https://elastic.co) +* http link to [elastic.co](http://elastic.co) +* relative link to [eui doc's homepage](${location.pathname}) + +**Other link protocols are kept as their markdown source:** +* ftp://elastic.co +* An [ftp link](ftp://elastic.co) + +**mailto: renders with only the link copy** +* Send a note to [someone](mailto:someone@elastic.co) +* Also applies to inline email addresses like someone@elastic.co +`; + +export default () => { + return {markdownContent}; +}; From c8c2ae867ef2a5a5ccae6c57b863bd13f258189f Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Wed, 13 Apr 2022 10:04:48 -0600 Subject: [PATCH 03/13] changelog --- upcoming_changelogs/5770.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 upcoming_changelogs/5770.md diff --git a/upcoming_changelogs/5770.md b/upcoming_changelogs/5770.md new file mode 100644 index 00000000000..f461ebe5667 --- /dev/null +++ b/upcoming_changelogs/5770.md @@ -0,0 +1,3 @@ +**Bug fixes** + +- Fixed `EuiMarkdownFormat` to not render email addresses as markdown link syntax From 04af037b3d7f0a49c3863525fa1bdbde3cbf7fd4 Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Wed, 13 Apr 2022 10:36:25 -0600 Subject: [PATCH 04/13] corrected changelog entry filename --- upcoming_changelogs/{5770.md => 5790.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename upcoming_changelogs/{5770.md => 5790.md} (100%) diff --git a/upcoming_changelogs/5770.md b/upcoming_changelogs/5790.md similarity index 100% rename from upcoming_changelogs/5770.md rename to upcoming_changelogs/5790.md From 15f4f7140269d54f1a6de3ad16b8fca6f593e57d Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Wed, 13 Apr 2022 11:40:25 -0600 Subject: [PATCH 05/13] Lint issues --- .../views/markdown_editor/mardown_format_example.js | 12 ++++++++---- .../views/markdown_editor/markdown_format_links.js | 5 ++++- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src-docs/src/views/markdown_editor/mardown_format_example.js b/src-docs/src/views/markdown_editor/mardown_format_example.js index 32b86f92ffb..422f47afad6 100644 --- a/src-docs/src/views/markdown_editor/mardown_format_example.js +++ b/src-docs/src/views/markdown_editor/mardown_format_example.js @@ -102,11 +102,15 @@ export const MarkdownFormatExample = { text: ( <>

- Markdown content often comes from untrusted sources like user generated content. To help with potential security issues, EuiMarkdownRenderer only renders links if they begin with https:, https, or /. Links that do not meet these requirements are rendered in the markdown link format, with the exception of mailto: where only the link copy is kept. + Markdown content often comes from untrusted sources like user + generated content. To help with potential security issues,{' '} + EuiMarkdownRenderer only renders links if they + begin with https:, https, or{' '} + /. Links that do not meet these requirements are + rendered in the markdown link format, with the exception of{' '} + mailto: where only the link copy is kept.

- - {markdownContent} - + {markdownContent} ), props: { diff --git a/src-docs/src/views/markdown_editor/markdown_format_links.js b/src-docs/src/views/markdown_editor/markdown_format_links.js index 5ea5537be4b..11c9b45535c 100644 --- a/src-docs/src/views/markdown_editor/markdown_format_links.js +++ b/src-docs/src/views/markdown_editor/markdown_format_links.js @@ -2,13 +2,16 @@ import React from 'react'; import { EuiMarkdownFormat } from '../../../../src'; +// eslint-disable-next-line no-restricted-globals +const locationPathname = location.pathname; + export const markdownContent = `**Links starting with http:, https:, and / are valid:** * https://elastic.com * http://elastic.com * https link to [elastic.co](https://elastic.co) * http link to [elastic.co](http://elastic.co) -* relative link to [eui doc's homepage](${location.pathname}) +* relative link to [eui doc's homepage](${locationPathname}) **Other link protocols are kept as their markdown source:** * ftp://elastic.co From 20bae8af4a014f0c8ff043ed0b8ba075f471fbfa Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Thu, 28 Apr 2022 14:28:54 -0600 Subject: [PATCH 06/13] Refactored markdownLinkValidator to be configurable --- .../markdown_editor.test.tsx.snap | 8 ++++- .../parsing_plugins.ts | 13 +++++-- .../plugins/markdown_link_validator.test.tsx | 35 +++++++++++++------ .../plugins/markdown_link_validator.tsx | 29 ++++++++++++--- 4 files changed, 66 insertions(+), 19 deletions(-) diff --git a/src/components/markdown_editor/__snapshots__/markdown_editor.test.tsx.snap b/src/components/markdown_editor/__snapshots__/markdown_editor.test.tsx.snap index a3a9686f634..b61e718f44d 100644 --- a/src/components/markdown_editor/__snapshots__/markdown_editor.test.tsx.snap +++ b/src/components/markdown_editor/__snapshots__/markdown_editor.test.tsx.snap @@ -29,7 +29,13 @@ exports[`EuiMarkdownEditor custom plugins are excluded and popover is rendered 1 ], Array [ [Function], - Object {}, + Object { + "allowProtocols": Array [ + "https:", + "http:", + ], + "allowRelative": true, + }, ], Array [ [Function], diff --git a/src/components/markdown_editor/plugins/markdown_default_plugins/parsing_plugins.ts b/src/components/markdown_editor/plugins/markdown_default_plugins/parsing_plugins.ts index 1afcb3c970b..8f03a0636c6 100644 --- a/src/components/markdown_editor/plugins/markdown_default_plugins/parsing_plugins.ts +++ b/src/components/markdown_editor/plugins/markdown_default_plugins/parsing_plugins.ts @@ -28,7 +28,10 @@ import breaks from 'remark-breaks'; import highlight from '../remark/remark_prismjs'; import * as MarkdownTooltip from '../markdown_tooltip'; import * as MarkdownCheckbox from '../markdown_checkbox'; -import { markdownLinkValidator } from '../markdown_link_validator'; +import { + markdownLinkValidator, + MarkdownLinkValidatorOptions, +} from '../markdown_link_validator'; export type DefaultEuiMarkdownParsingPlugins = PluggableList; @@ -41,7 +44,13 @@ export const getDefaultEuiMarkdownParsingPlugins = ({ [highlight, {}], [emoji, { emoticon: false }], [breaks, {}], - [markdownLinkValidator, {}], + [ + markdownLinkValidator, + { + allowRelative: true, + allowProtocols: ['https:', 'http:'], + } as MarkdownLinkValidatorOptions, + ], [MarkdownCheckbox.parser, {}], ]; diff --git a/src/components/markdown_editor/plugins/markdown_link_validator.test.tsx b/src/components/markdown_editor/plugins/markdown_link_validator.test.tsx index f8e31fe9e10..5ff2b3c03ec 100644 --- a/src/components/markdown_editor/plugins/markdown_link_validator.test.tsx +++ b/src/components/markdown_editor/plugins/markdown_link_validator.test.tsx @@ -6,38 +6,51 @@ * Side Public License, v 1. */ -import { validateUrl, mutateLinkToText } from './markdown_link_validator'; +import { + validateUrl, + mutateLinkToText, + MarkdownLinkValidatorOptions, +} from './markdown_link_validator'; import { validateHref } from '../../../services/security/href_validator'; +const defaultValidationOptions: MarkdownLinkValidatorOptions = { + allowRelative: true, + allowProtocols: ['http:', 'https:'], +}; + describe('validateURL', () => { it('approves of https:', () => { - expect(validateUrl('https:')).toBeTruthy(); + expect( + validateUrl('https://domain', defaultValidationOptions) + ).toBeTruthy(); }); it('approves of http:', () => { - expect(validateUrl('http:')).toBeTruthy(); + expect(validateUrl('http://domain', defaultValidationOptions)).toBeTruthy(); }); it('approves of absolute relative links', () => { - expect(validateUrl('/')).toBeTruthy(); + expect(validateUrl('/', defaultValidationOptions)).toBeTruthy(); }); it('approves of relative protocols', () => { - expect(validateUrl('//')).toBeTruthy(); + expect(validateUrl('//', defaultValidationOptions)).toBeTruthy(); }); it('rejects a url starting with http with not an s following', () => { - expect(validateUrl('httpm:')).toBeFalsy(); + expect(validateUrl('httpm:', defaultValidationOptions)).toBeFalsy(); }); it('rejects a directory relative link', () => { - expect(validateUrl('./')).toBeFalsy(); - expect(validateUrl('../')).toBeFalsy(); + expect(validateUrl('./', defaultValidationOptions)).toBeFalsy(); + expect(validateUrl('../', defaultValidationOptions)).toBeFalsy(); }); it('rejects a word', () => { - expect(validateUrl('word')).toBeFalsy(); + expect(validateUrl('word', defaultValidationOptions)).toBeFalsy(); }); it('rejects gopher', () => { - expect(validateUrl('gopher:')).toBeFalsy(); + expect( + validateUrl('gopher://domain', defaultValidationOptions) + ).toBeFalsy(); }); it('rejects javascript', () => { // eslint-disable-next-line no-script-url - expect(validateUrl('javascript:')).toBeFalsy(); + expect(validateUrl('javascript:', defaultValidationOptions)).toBeFalsy(); // eslint-disable-next-line no-script-url expect(validateHref('javascript:alert()')).toBeFalsy(); }); diff --git a/src/components/markdown_editor/plugins/markdown_link_validator.tsx b/src/components/markdown_editor/plugins/markdown_link_validator.tsx index 08d8669af32..90421daea91 100644 --- a/src/components/markdown_editor/plugins/markdown_link_validator.tsx +++ b/src/components/markdown_editor/plugins/markdown_link_validator.tsx @@ -16,12 +16,17 @@ interface LinkOrTextNode { children?: Array<{ value: string }>; } -export function markdownLinkValidator() { +export interface MarkdownLinkValidatorOptions { + allowRelative: boolean; + allowProtocols: string[]; +} + +export function markdownLinkValidator(options: MarkdownLinkValidatorOptions) { return (ast: any) => { visit(ast, 'link', (_node: unknown) => { const node = _node as LinkOrTextNode; - if (!validateUrl(node.url!)) { + if (!validateUrl(node.url!, options)) { mutateLinkToText(node); } }); @@ -45,7 +50,21 @@ export function mutateLinkToText(node: LinkOrTextNode) { return node; } -export function validateUrl(url: string) { - // A link is valid if it starts with http:, https:, or / - return /^(https?:|\/)/.test(url); +export function validateUrl( + url: string, + { allowRelative, allowProtocols }: MarkdownLinkValidatorOptions +) { + // relative captures both relative paths `/` and protocols `//` + const isRelative = url.startsWith('/'); + if (isRelative) { + return allowRelative; + } + + try { + const parsedUrl = new URL(url); + return allowProtocols.indexOf(parsedUrl.protocol) !== -1; + } catch (e) { + // failed to parse input as url + return false; + } } From fdac6a1c666c8370f2ac52542e157c3c90ad153d Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Thu, 5 May 2022 15:24:35 -0600 Subject: [PATCH 07/13] documentation, documentation, documentation --- .../markdown_link_validation.js | 34 ++++ .../markdown_plugin_example.js | 182 ++++++++++++++++++ src/components/markdown_editor/index.ts | 2 + .../parsing_plugins.ts | 8 +- .../plugins/markdown_link_validator.test.tsx | 4 +- .../plugins/markdown_link_validator.tsx | 8 +- src/services/url.test.ts | 4 + src/services/url.ts | 15 +- 8 files changed, 241 insertions(+), 16 deletions(-) create mode 100644 src-docs/src/views/markdown_editor/markdown_link_validation.js diff --git a/src-docs/src/views/markdown_editor/markdown_link_validation.js b/src-docs/src/views/markdown_editor/markdown_link_validation.js new file mode 100644 index 00000000000..ee6185544d8 --- /dev/null +++ b/src-docs/src/views/markdown_editor/markdown_link_validation.js @@ -0,0 +1,34 @@ +import React from 'react'; + +import { + getDefaultEuiMarkdownParsingPlugins, + euiMarkdownLinkValidator, + EuiMarkdownFormat, +} from '../../../../src/components'; + +// find the validation plugin and configure it to only allow https: and mailto: links +const parsingPlugins = getDefaultEuiMarkdownParsingPlugins(); +parsingPlugins.find(([plugin, config]) => { + const isValidationPlugin = plugin === euiMarkdownLinkValidator; + if (isValidationPlugin) { + config.allowProtocols = ['https:', 'mailto:']; + } + return isValidationPlugin; +}); + +const markdown = `**Standalone links** +https://example.com +http://example.com +someone@example.com + +**As markdown syntax** +[example.com, https](https://example.com) +[example.com, http](http://example.com) +[email someone@example.com](mailto:someone@example.com) +`; + +export default () => ( + + {markdown} + +); diff --git a/src-docs/src/views/markdown_editor/markdown_plugin_example.js b/src-docs/src/views/markdown_editor/markdown_plugin_example.js index dd8f26f7675..da49c886f79 100644 --- a/src-docs/src/views/markdown_editor/markdown_plugin_example.js +++ b/src-docs/src/views/markdown_editor/markdown_plugin_example.js @@ -19,6 +19,9 @@ import { Link } from 'react-router-dom'; import MarkdownEditorWithPlugins from './markdown_editor_with_plugins'; const markdownEditorWithPluginsSource = require('!!raw-loader!./markdown_editor_with_plugins'); +const linkValidationSource = require('!!raw-loader!./markdown_link_validation'); +import LinkValidation from './markdown_link_validation'; + const pluginSnippet = ` +

+ EUI provides additional plugins by default, but these can be omitted + or otherwise customized by providing the{' '} + parsingPluginList,{' '} + processingPluginList, and{' '} + uiPlugins props to the editor and formatter + components. +

+

The parsing plugins, responsible for parsing markdown are:

+
    +
  1. + + remark-parse + +
    2. +
  2. + + additional pre-processing for code blocks + +
    3. +
  3. + + remark-emoji + +
    4. +
  4. + + remark-breaks + +
    5. +
  5. + + link validation for security + +
    6. +
  6. + + injection of EuiCheckbox for markdown check boxes + +
    7. +
  7. + + tooltip plugin parser + +
    8. +
+

+ The above set provides an abstract syntax tree used by the editor to + provide feedback, and the renderer passes that output to the set of + processing plugins to allow it to be rendered: +

+
    +
  1. + + remark-rehype + +
    2. +
  2. + + rehype-react + +
    3. +
  3. + + tooltip plugin renderer + +
    4. +
+

+ The last set of plugin configuration - uiPlugins{' '} + - allows toolbar buttons to be defined and how they alter or inject + markdown and returns with only one plugin: +

+
    +
  1. + + tooltip plugin ui + +
    2. +
+

+ These plugin definitions can be obtained by calling{' '} + getDefaultEuiMarkdownParsingPlugins,{' '} + getDefaultEuiMarkdownProcessingPlugins, and{' '} + getDefaultEuiMarkdownUiPlugins respectively. Each + of these three functions take an optional configuration object with + an exclude key, an array of EUI-defaulted plugins + to disable. Currently the only option this configuration can take is{' '} + 'tooltip'. +

+ + ), + }, + { + source: [ + { + type: GuideSectionTypes.JS, + code: linkValidationSource, + }, + ], + title: 'Link validation & security', + text: ( + +

+ To enhance user and application security, the default behavior + removes links to URLs that don't begin with{' '} + https://, http://, or{' '} + / (relative). This validation can be further + configured or removed altogether. +

+

+ In this example only https: and{' '} + mailto: links are allowed. +

+ + ), + snippet: [ + `// change what link protocols are allowed +const parsingPlugins = getDefaultEuiMarkdownParsingPlugins(); +parsingPlugins.find(([plugin, config]) => { + const isValidationPlugin = plugin === euiMarkdownLinkValidator; + if (isValidationPlugin) { + config.allowProtocols = ['https:', 'mailto:']; + } + return isValidationPlugin; +});`, + `// filter out the link validation plugin +const parsingPlugins = getDefaultEuiMarkdownParsingPlugins().filter(([plugin]) => { + return plugin !== euiMarkdownLinkValidator; +});`, + `// disable relative urls +const parsingPlugins = getDefaultEuiMarkdownParsingPlugins(); +parsingPlugins.find(([plugin, config]) => { + const isValidationPlugin = plugin === euiMarkdownLinkValidator; + if (isValidationPlugin) { + config.allowRelative = false; + } + return isValidationPlugin; +});`, + ], + demo: , + }, { wrapText: false, text: ( diff --git a/src/components/markdown_editor/index.ts b/src/components/markdown_editor/index.ts index d3a62e4a6c0..d47f9ab3a72 100644 --- a/src/components/markdown_editor/index.ts +++ b/src/components/markdown_editor/index.ts @@ -26,3 +26,5 @@ export type { RemarkRehypeHandler, RemarkTokenizer, } from './markdown_types'; +export { euiMarkdownLinkValidator } from './plugins/markdown_link_validator'; +export type { EuiMarkdownLinkValidatorOptions } from './plugins/markdown_link_validator'; diff --git a/src/components/markdown_editor/plugins/markdown_default_plugins/parsing_plugins.ts b/src/components/markdown_editor/plugins/markdown_default_plugins/parsing_plugins.ts index 8f03a0636c6..b997fd459a2 100644 --- a/src/components/markdown_editor/plugins/markdown_default_plugins/parsing_plugins.ts +++ b/src/components/markdown_editor/plugins/markdown_default_plugins/parsing_plugins.ts @@ -29,8 +29,8 @@ import highlight from '../remark/remark_prismjs'; import * as MarkdownTooltip from '../markdown_tooltip'; import * as MarkdownCheckbox from '../markdown_checkbox'; import { - markdownLinkValidator, - MarkdownLinkValidatorOptions, + euiMarkdownLinkValidator, + EuiMarkdownLinkValidatorOptions, } from '../markdown_link_validator'; export type DefaultEuiMarkdownParsingPlugins = PluggableList; @@ -45,11 +45,11 @@ export const getDefaultEuiMarkdownParsingPlugins = ({ [emoji, { emoticon: false }], [breaks, {}], [ - markdownLinkValidator, + euiMarkdownLinkValidator, { allowRelative: true, allowProtocols: ['https:', 'http:'], - } as MarkdownLinkValidatorOptions, + } as EuiMarkdownLinkValidatorOptions, ], [MarkdownCheckbox.parser, {}], ]; diff --git a/src/components/markdown_editor/plugins/markdown_link_validator.test.tsx b/src/components/markdown_editor/plugins/markdown_link_validator.test.tsx index 5ff2b3c03ec..5c671015840 100644 --- a/src/components/markdown_editor/plugins/markdown_link_validator.test.tsx +++ b/src/components/markdown_editor/plugins/markdown_link_validator.test.tsx @@ -9,11 +9,11 @@ import { validateUrl, mutateLinkToText, - MarkdownLinkValidatorOptions, + EuiMarkdownLinkValidatorOptions, } from './markdown_link_validator'; import { validateHref } from '../../../services/security/href_validator'; -const defaultValidationOptions: MarkdownLinkValidatorOptions = { +const defaultValidationOptions: EuiMarkdownLinkValidatorOptions = { allowRelative: true, allowProtocols: ['http:', 'https:'], }; diff --git a/src/components/markdown_editor/plugins/markdown_link_validator.tsx b/src/components/markdown_editor/plugins/markdown_link_validator.tsx index 90421daea91..35c80138cdc 100644 --- a/src/components/markdown_editor/plugins/markdown_link_validator.tsx +++ b/src/components/markdown_editor/plugins/markdown_link_validator.tsx @@ -16,12 +16,14 @@ interface LinkOrTextNode { children?: Array<{ value: string }>; } -export interface MarkdownLinkValidatorOptions { +export interface EuiMarkdownLinkValidatorOptions { allowRelative: boolean; allowProtocols: string[]; } -export function markdownLinkValidator(options: MarkdownLinkValidatorOptions) { +export function euiMarkdownLinkValidator( + options: EuiMarkdownLinkValidatorOptions +) { return (ast: any) => { visit(ast, 'link', (_node: unknown) => { const node = _node as LinkOrTextNode; @@ -52,7 +54,7 @@ export function mutateLinkToText(node: LinkOrTextNode) { export function validateUrl( url: string, - { allowRelative, allowProtocols }: MarkdownLinkValidatorOptions + { allowRelative, allowProtocols }: EuiMarkdownLinkValidatorOptions ) { // relative captures both relative paths `/` and protocols `//` const isRelative = url.startsWith('/'); diff --git a/src/services/url.test.ts b/src/services/url.test.ts index 95bc9d174e0..12bb1147129 100644 --- a/src/services/url.test.ts +++ b/src/services/url.test.ts @@ -18,12 +18,14 @@ describe('url', () => { expect(isDomainSecure('https://docs.elastic.co')).toEqual(true); expect(isDomainSecure('https://stats.elastic.co')).toEqual(true); expect(isDomainSecure('https://lots.of.kids.elastic.co')).toEqual(true); + expect(isDomainSecure('http://docs.elastic.co')).toEqual(true); expect( isDomainSecure('https://elastic.co/cool/url/with?lots=of¶ms') ).toEqual(true); }); it('returns false for unsecure domains', () => { + expect(isDomainSecure('asdfasdf')).toEqual(false); expect(isDomainSecure('https://wwwelastic.co')).toEqual(false); expect(isDomainSecure('https://www.zelastic.co')).toEqual(false); expect(isDomainSecure('https://*elastic.co')).toEqual(false); @@ -31,11 +33,13 @@ describe('url', () => { expect(isDomainSecure('https://elastic.co.now')).toEqual(false); expect(isDomainSecure('elastic.co')).toEqual(false); expect(isDomainSecure('smb://www.elastic.co')).toEqual(false); + expect(isDomainSecure('smb://www.elastic.co:443')).toEqual(false); expect( isDomainSecure( 'https://wwwelastic.co/cool/url/with?lots=of¶ms/https://elastic.co' ) ).toEqual(false); + expect(isDomainSecure('https://example.com/.elastic.co')).toEqual(false); }); }); }); diff --git a/src/services/url.ts b/src/services/url.ts index e9bb196abac..cb751d300b2 100644 --- a/src/services/url.ts +++ b/src/services/url.ts @@ -6,19 +6,20 @@ * Side Public License, v 1. */ -const isElasticDomain = /(https?:\/\/(.+?\.)?elastic\.co((\/|\?)[A-Za-z0-9\-\._~:\/\?#\[\]@!$&'\(\)\*\+,;\=]*)?)/g; +const isElasticHost = /^([a-zA-Z0-9]+\.)*elastic\.co$/; // In order for the domain to be secure the regex // has to match _and_ the lengths of the match must // be _exact_ since URL's can have other URL's as // path or query params! export const isDomainSecure = (url: string = '') => { - const matches = url.match(isElasticDomain); - - if (!matches) { + try { + const parsed = new URL(url); + const protocolMatches = + parsed.protocol === 'http:' || parsed.protocol === 'https:'; + const domainMatches = !!parsed.host.match(isElasticHost); + return protocolMatches && domainMatches; + } catch (e) { return false; } - const [match] = matches; - - return match.length === url.length; }; From b5a96445f1319ac82af85137735374ebe8281134 Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Thu, 5 May 2022 15:35:10 -0600 Subject: [PATCH 08/13] PR feedback --- .../markdown_editor/plugins/markdown_link_validator.tsx | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/components/markdown_editor/plugins/markdown_link_validator.tsx b/src/components/markdown_editor/plugins/markdown_link_validator.tsx index 35c80138cdc..d7f4ea96dcb 100644 --- a/src/components/markdown_editor/plugins/markdown_link_validator.tsx +++ b/src/components/markdown_editor/plugins/markdown_link_validator.tsx @@ -39,11 +39,12 @@ export function mutateLinkToText(node: LinkOrTextNode) { node.type = 'text'; // https://github.com/elastic/eui/issues/5770 - // if this is a `mailto:` link only keep the target address + // if this is a `mailto:` link only keep the link text + const linkText = node.children?.[0]?.value || ''; if (node.url?.startsWith('mailto:')) { - node.value = node.children![0]?.value; + node.value = linkText; } else { - node.value = `[${node.children![0]?.value || ''}](${node.url})`; + node.value = `[${linkText || ''}](${node.url})`; } delete node.children; From d42cfe3cbfdfa25f5ed49de99279d8604e5efb0d Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Fri, 6 May 2022 11:32:03 -0600 Subject: [PATCH 09/13] enable mailto: links by default in markdown --- .../src/views/markdown_editor/markdown_plugin_example.js | 7 ++++--- .../__snapshots__/markdown_editor.test.tsx.snap | 1 + .../plugins/markdown_default_plugins/parsing_plugins.ts | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src-docs/src/views/markdown_editor/markdown_plugin_example.js b/src-docs/src/views/markdown_editor/markdown_plugin_example.js index da49c886f79..a70ae2539dd 100644 --- a/src-docs/src/views/markdown_editor/markdown_plugin_example.js +++ b/src-docs/src/views/markdown_editor/markdown_plugin_example.js @@ -315,9 +315,10 @@ export const MarkdownPluginExample = {

To enhance user and application security, the default behavior - removes links to URLs that don't begin with{' '} - https://, http://, or{' '} - / (relative). This validation can be further + removes links to URLs that aren't relative (beginning with{' '} + /) and don't use the{' '} + https:, http:, or{' '} + mailto: protocols. This validation can be further configured or removed altogether.

diff --git a/src/components/markdown_editor/__snapshots__/markdown_editor.test.tsx.snap b/src/components/markdown_editor/__snapshots__/markdown_editor.test.tsx.snap index b61e718f44d..50ca6a7a9a5 100644 --- a/src/components/markdown_editor/__snapshots__/markdown_editor.test.tsx.snap +++ b/src/components/markdown_editor/__snapshots__/markdown_editor.test.tsx.snap @@ -33,6 +33,7 @@ exports[`EuiMarkdownEditor custom plugins are excluded and popover is rendered 1 "allowProtocols": Array [ "https:", "http:", + "mailto:", ], "allowRelative": true, }, diff --git a/src/components/markdown_editor/plugins/markdown_default_plugins/parsing_plugins.ts b/src/components/markdown_editor/plugins/markdown_default_plugins/parsing_plugins.ts index b997fd459a2..d88c3625bf1 100644 --- a/src/components/markdown_editor/plugins/markdown_default_plugins/parsing_plugins.ts +++ b/src/components/markdown_editor/plugins/markdown_default_plugins/parsing_plugins.ts @@ -48,7 +48,7 @@ export const getDefaultEuiMarkdownParsingPlugins = ({ euiMarkdownLinkValidator, { allowRelative: true, - allowProtocols: ['https:', 'http:'], + allowProtocols: ['https:', 'http:', 'mailto:'], } as EuiMarkdownLinkValidatorOptions, ], [MarkdownCheckbox.parser, {}], From fd76cae1966aaec8f3217fe288cc42b41619ac84 Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Fri, 6 May 2022 11:34:45 -0600 Subject: [PATCH 10/13] updated comment --- src/services/url.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/services/url.ts b/src/services/url.ts index cb751d300b2..a61d5d2860c 100644 --- a/src/services/url.ts +++ b/src/services/url.ts @@ -8,10 +8,9 @@ const isElasticHost = /^([a-zA-Z0-9]+\.)*elastic\.co$/; -// In order for the domain to be secure the regex -// has to match _and_ the lengths of the match must -// be _exact_ since URL's can have other URL's as -// path or query params! +// In order for the domain to be secure it needs to be in a parsable format, +// with the protocol of http: or https: and the host matching elastic.co or +// of one its subdomains export const isDomainSecure = (url: string = '') => { try { const parsed = new URL(url); From 5b3e2f2ddd720d4b48d8e8e3e6ea26c434939c92 Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Fri, 6 May 2022 15:02:38 -0600 Subject: [PATCH 11/13] test to verify mailto protocol is supported --- .../plugins/markdown_link_validator.test.tsx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/components/markdown_editor/plugins/markdown_link_validator.test.tsx b/src/components/markdown_editor/plugins/markdown_link_validator.test.tsx index 5c671015840..0c77b338bad 100644 --- a/src/components/markdown_editor/plugins/markdown_link_validator.test.tsx +++ b/src/components/markdown_editor/plugins/markdown_link_validator.test.tsx @@ -15,7 +15,7 @@ import { validateHref } from '../../../services/security/href_validator'; const defaultValidationOptions: EuiMarkdownLinkValidatorOptions = { allowRelative: true, - allowProtocols: ['http:', 'https:'], + allowProtocols: ['http:', 'https:', 'mailto:'], }; describe('validateURL', () => { @@ -27,6 +27,11 @@ describe('validateURL', () => { it('approves of http:', () => { expect(validateUrl('http://domain', defaultValidationOptions)).toBeTruthy(); }); + it('approves of mailto:', () => { + expect( + validateUrl('mailto:someone@elastic.co', defaultValidationOptions) + ).toBeTruthy(); + }); it('approves of absolute relative links', () => { expect(validateUrl('/', defaultValidationOptions)).toBeTruthy(); }); From 4e31ab098639b0785ec7f036f219164d7068edab Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Fri, 6 May 2022 15:15:15 -0600 Subject: [PATCH 12/13] pr feedback --- .../markdown_editor/mardown_format_example.js | 6 ++---- .../markdown_editor/markdown_format_links.js | 8 +++----- .../plugins/markdown_link_validator.test.tsx | 19 +++++++++++++++++-- .../plugins/markdown_link_validator.tsx | 10 ++++++---- 4 files changed, 28 insertions(+), 15 deletions(-) diff --git a/src-docs/src/views/markdown_editor/mardown_format_example.js b/src-docs/src/views/markdown_editor/mardown_format_example.js index 422f47afad6..45f27331f01 100644 --- a/src-docs/src/views/markdown_editor/mardown_format_example.js +++ b/src-docs/src/views/markdown_editor/mardown_format_example.js @@ -105,10 +105,8 @@ export const MarkdownFormatExample = { Markdown content often comes from untrusted sources like user generated content. To help with potential security issues,{' '} EuiMarkdownRenderer only renders links if they - begin with https:, https, or{' '} - /. Links that do not meet these requirements are - rendered in the markdown link format, with the exception of{' '} - mailto: where only the link copy is kept. + begin with https:, http:,{' '} + mailto:, or /.

{markdownContent} diff --git a/src-docs/src/views/markdown_editor/markdown_format_links.js b/src-docs/src/views/markdown_editor/markdown_format_links.js index 11c9b45535c..2b28d6bd188 100644 --- a/src-docs/src/views/markdown_editor/markdown_format_links.js +++ b/src-docs/src/views/markdown_editor/markdown_format_links.js @@ -5,21 +5,19 @@ import { EuiMarkdownFormat } from '../../../../src'; // eslint-disable-next-line no-restricted-globals const locationPathname = location.pathname; -export const markdownContent = `**Links starting with http:, https:, and / are valid:** +export const markdownContent = `**Links starting with http:, https:, mailto:, and / are valid:** * https://elastic.com * http://elastic.com * https link to [elastic.co](https://elastic.co) * http link to [elastic.co](http://elastic.co) * relative link to [eui doc's homepage](${locationPathname}) +* someone@elastic.co +* [email me!](mailto:someone@elastic.co) **Other link protocols are kept as their markdown source:** * ftp://elastic.co * An [ftp link](ftp://elastic.co) - -**mailto: renders with only the link copy** -* Send a note to [someone](mailto:someone@elastic.co) -* Also applies to inline email addresses like someone@elastic.co `; export default () => { diff --git a/src/components/markdown_editor/plugins/markdown_link_validator.test.tsx b/src/components/markdown_editor/plugins/markdown_link_validator.test.tsx index 0c77b338bad..9b0a156f39d 100644 --- a/src/components/markdown_editor/plugins/markdown_link_validator.test.tsx +++ b/src/components/markdown_editor/plugins/markdown_link_validator.test.tsx @@ -90,7 +90,22 @@ describe('mutateLinkToText', () => { } `); }); - it('keeps only the email address when handling mailto: links', () => { + it('keeps only the link text when both text & url are the same value', () => { + expect( + mutateLinkToText({ + type: 'link', + url: 'ftp://www.example.com', + title: null, + children: [{ value: 'ftp://www.example.com' }], + }) + ).toMatchInlineSnapshot(` + Object { + "type": "text", + "value": "ftp://www.example.com", + } + `); + }); + it('renders with the markdown link syntax when link and url are not the same value', () => { expect( mutateLinkToText({ type: 'link', @@ -101,7 +116,7 @@ describe('mutateLinkToText', () => { ).toMatchInlineSnapshot(` Object { "type": "text", - "value": "someone@elastic.co", + "value": "[someone@elastic.co](mailto:someone@elastic.co)", } `); }); diff --git a/src/components/markdown_editor/plugins/markdown_link_validator.tsx b/src/components/markdown_editor/plugins/markdown_link_validator.tsx index d7f4ea96dcb..4f23051fd13 100644 --- a/src/components/markdown_editor/plugins/markdown_link_validator.tsx +++ b/src/components/markdown_editor/plugins/markdown_link_validator.tsx @@ -36,15 +36,17 @@ export function euiMarkdownLinkValidator( } export function mutateLinkToText(node: LinkOrTextNode) { + // this is an upsupported url, convert to a text node node.type = 'text'; - // https://github.com/elastic/eui/issues/5770 - // if this is a `mailto:` link only keep the link text + // and, if the link text matches the url there's only one value to show + // otherwise render as the markdown syntax so both text & url remain, unlinked const linkText = node.children?.[0]?.value || ''; - if (node.url?.startsWith('mailto:')) { + const linkUrl = node.url ?? ''; + if (linkText === linkUrl) { node.value = linkText; } else { - node.value = `[${linkText || ''}](${node.url})`; + node.value = `[${linkText}](${node.url})`; } delete node.children; From 0ce1ee944e41dd2af6910872e53a4981d7281d9d Mon Sep 17 00:00:00 2001 From: Constance Date: Fri, 6 May 2022 18:48:48 -0700 Subject: [PATCH 13/13] [PR feedback] Changelog --- upcoming_changelogs/5790.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/upcoming_changelogs/5790.md b/upcoming_changelogs/5790.md index f461ebe5667..9acf7310e1d 100644 --- a/upcoming_changelogs/5790.md +++ b/upcoming_changelogs/5790.md @@ -1,3 +1,2 @@ -**Bug fixes** - -- Fixed `EuiMarkdownFormat` to not render email addresses as markdown link syntax +- Updated `EuiMarkdownFormat` to allow `mailto:` links by default +- Updated `EuiMarkdownEditor`'s `euiMarkdownLinkValidator` parsing plugin to allow customization of link validation