Skip to content

Commit

Permalink
[EuiMarkdownEditor] Add a markdown link validator (#4362)
Browse files Browse the repository at this point in the history
* Add a markdown link validator

* Add href validation to EuiLink

* Disabled href handling when the href uses the javascript protocol

* changelog

Co-authored-by: Chandler Prall <[email protected]>
  • Loading branch information
glitteringkatie and chandlerprall authored Dec 9, 2020
1 parent 90a0280 commit 0031003
Show file tree
Hide file tree
Showing 26 changed files with 398 additions and 32 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
- Fixed propagation of `esc` key presses closing parent popovers ([#4336](https://github.com/elastic/eui/pull/4336))
- Fixed overwritten `isDisabled` prop on `EuiListGroupItem` `extraAction` config ([#4359](https://github.com/elastic/eui/pull/4359))
- Fixed `inputRef` for `EuiCheckbox` ([#4298](https://github.com/elastic/eui/pull/4298))
- Limited the links allowed in `EuiMarkdownEditor` to http, https, or starting with a forward slash ([#4362](https://github.com/elastic/eui/pull/4362))
- Aligned components with an `href` prop to React's practice of disallowing `javascript:` protocols ([#4362](https://github.com/elastic/eui/pull/4362))

**Theme: Amsterdam**

Expand Down
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@
"tabbable": "^3.0.0",
"text-diff": "^1.0.1",
"unified": "^9.2.0",
"url-parse": "^1.4.7",
"uuid": "^8.3.0",
"vfile": "^4.2.0"
},
Expand Down Expand Up @@ -108,6 +109,7 @@
"@types/react-router-dom": "^5.1.5",
"@types/resize-observer-browser": "^0.1.3",
"@types/tabbable": "^3.1.0",
"@types/url-parse": "^1.4.3",
"@types/uuid": "^8.3.0",
"@typescript-eslint/eslint-plugin": "^3.10.1",
"@typescript-eslint/parser": "^3.10.1",
Expand Down
34 changes: 34 additions & 0 deletions src-docs/src/views/link/link_example.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,17 @@ import linkConfig from './playground';

import Link from './link';
import { LinkDisable } from './link_disable';
import { LinkValidation } from './link_validation';

const linkSource = require('!!raw-loader!./link');
const linkHtml = renderToHtml(Link);

const linkDisableSource = require('!!raw-loader!./link_disable');
const linkDisableHtml = renderToHtml(LinkDisable);

const linkValidationSource = require('!!raw-loader!./link_validation');
const linkValidationHtml = renderToHtml(LinkValidation);

const linkSnippet = [
`<EuiLink href="#"><!-- Link text --></EuiLink>
`,
Expand Down Expand Up @@ -77,6 +81,36 @@ export const LinkExample = {
props: { EuiLink },
demo: <LinkDisable />,
},
{
title: 'Link validation',
source: [
{
type: GuideSectionTypes.JS,
code: linkValidationSource,
},
{
type: GuideSectionTypes.HTML,
code: linkValidationHtml,
},
],
text: (
<p>
To make links more secure for users, <strong>EuiLink</strong> and
other components that accept an <EuiCode>href</EuiCode> prop become
disabled if that <EuiCode>href</EuiCode> uses the{' '}
<EuiCode>javascript:</EuiCode> protocol. This helps protect consuming
applications from cross-site scripting (XSS) attacks and mirrors
React&apos;s{' '}
<EuiLink
href="https://github.com/facebook/react/blob/940f48b999a3131e77b2545bd7ae252ef27ae6d1/packages/react-dom/src/shared/sanitizeURL.js#L37"
target="_blank">
planned behavior
</EuiLink>{' '}
to prevent rendering of <EuiCode>javascript:</EuiCode> links.
</p>
),
demo: <LinkValidation />,
},
],
playground: linkConfig,
};
25 changes: 25 additions & 0 deletions src-docs/src/views/link/link_validation.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import React from 'react';
import { EuiLink } from '../../../../src/components';

const urls = [
'https://elastic.co',
'//elastic.co',
'relative/url/somewhere',
'http://username:[email protected]/',
// eslint-disable-next-line no-script-url
'javascript:alert()',
];

export const LinkValidation = () => {
return (
<>
{urls.map((url) => (
<div key={url}>
<EuiLink color="secondary" href={url}>
{url}
</EuiLink>
</div>
))}
</>
);
};
7 changes: 6 additions & 1 deletion src/components/badge/badge.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import {
import { EuiInnerText } from '../inner_text';
import { EuiIcon, IconColor, IconType } from '../icon';
import { chromaValid, parseColor } from '../color_picker/utils';
import { validateHref } from '../../services/security/href_validator';

type IconSide = 'left' | 'right';

Expand Down Expand Up @@ -136,7 +137,7 @@ export const EuiBadge: FunctionComponent<EuiBadgeProps> = ({
iconType,
iconSide = 'left',
className,
isDisabled,
isDisabled: _isDisabled,
onClick,
iconOnClick,
onClickAriaLabel,
Expand All @@ -150,6 +151,9 @@ export const EuiBadge: FunctionComponent<EuiBadgeProps> = ({
}) => {
checkValidColor(color);

const isHrefValid = !href || validateHref(href);
const isDisabled = _isDisabled || !isHrefValid;

let optionalCustomStyles: object | undefined = style;
let textColor = null;
// TODO - replace with variable once https://github.com/elastic/eui/issues/2731 is closed
Expand Down Expand Up @@ -215,6 +219,7 @@ export const EuiBadge: FunctionComponent<EuiBadgeProps> = ({
'euiBadge__icon',
closeButtonProps && closeButtonProps.className
);

const Element = href && !isDisabled ? 'a' : 'button';
const relObj: {
href?: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,7 @@ exports[`EuiInMemoryTable behavior pagination 1`] = `
<button
className="euiButtonEmpty euiButtonEmpty--text euiButtonEmpty--xSmall"
data-test-subj="tablePaginationPopoverButton"
disabled={false}
onClick={[Function]}
type="button"
>
Expand Down
9 changes: 7 additions & 2 deletions src/components/button/button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import {
EuiButtonContentType,
EuiButtonContent,
} from './button_content';
import { validateHref } from '../../services/security/href_validator';

export type ButtonColor =
| 'primary'
Expand Down Expand Up @@ -250,15 +251,19 @@ export type Props = ExclusiveUnion<
>;

export const EuiButton: FunctionComponent<Props> = ({
isDisabled,
disabled,
isDisabled: _isDisabled,
disabled: _disabled,
href,
target,
rel,
type = 'button',
buttonRef,
...rest
}) => {
const isHrefValid = !href || validateHref(href);
const disabled = _disabled || !isHrefValid;
const isDisabled = _isDisabled || !isHrefValid;

const buttonIsDisabled = rest.isLoading || isDisabled || disabled;
const element = href && !isDisabled ? 'a' : 'button';

Expand Down
9 changes: 7 additions & 2 deletions src/components/button/button_empty/button_empty.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
EuiButtonContentProps,
EuiButtonContentType,
} from '../button_content';
import { validateHref } from '../../../services/security/href_validator';

export type EuiButtonEmptyColor =
| 'primary'
Expand Down Expand Up @@ -126,8 +127,8 @@ export const EuiButtonEmpty: FunctionComponent<EuiButtonEmptyProps> = ({
color = 'primary',
size,
flush,
isDisabled,
disabled,
isDisabled: _isDisabled,
disabled: _disabled,
isLoading,
href,
target,
Expand All @@ -139,6 +140,10 @@ export const EuiButtonEmpty: FunctionComponent<EuiButtonEmptyProps> = ({
isSelected,
...rest
}) => {
const isHrefValid = !href || validateHref(href);
const disabled = _disabled || !isHrefValid;
const isDisabled = _isDisabled || !isHrefValid;

// If in the loading state, force disabled to true
const buttonIsDisabled = isLoading || isDisabled || disabled;

Expand Down
6 changes: 5 additions & 1 deletion src/components/button/button_icon/button_icon.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import {
import { IconType, IconSize, EuiIcon } from '../../icon';

import { ButtonSize } from '../button';
import { validateHref } from '../../../services/security/href_validator';

export type EuiButtonIconColor =
| 'accent'
Expand Down Expand Up @@ -104,7 +105,7 @@ export const EuiButtonIcon: FunctionComponent<Props> = ({
iconType,
iconSize = 'm',
color = 'primary',
isDisabled,
isDisabled: _isDisabled,
href,
type = 'button',
target,
Expand All @@ -113,6 +114,9 @@ export const EuiButtonIcon: FunctionComponent<Props> = ({
isSelected,
...rest
}) => {
const isHrefValid = !href || validateHref(href);
const isDisabled = _isDisabled || !isHrefValid;

const ariaHidden = rest['aria-hidden'];
const isAriaHidden = ariaHidden === 'true' || ariaHidden === true;

Expand Down
6 changes: 5 additions & 1 deletion src/components/card/card.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import {
euiCardSelectableColor,
} from './card_select';
import { htmlIdGenerator } from '../../services/accessibility';
import { validateHref } from '../../services/security/href_validator';

type CardAlignment = 'left' | 'center' | 'right';

Expand Down Expand Up @@ -176,7 +177,7 @@ export const SIZES = keysOf(paddingSizeToClassNameMap);
export const EuiCard: FunctionComponent<EuiCardProps> = ({
className,
description,
isDisabled,
isDisabled: _isDisabled,
title,
titleElement = 'span',
titleSize = 's',
Expand All @@ -198,6 +199,9 @@ export const EuiCard: FunctionComponent<EuiCardProps> = ({
paddingSize = 'm',
...rest
}) => {
const isHrefValid = !href || validateHref(href);
const isDisabled = _isDisabled || !isHrefValid;

/**
* For a11y, we simulate the same click that's provided on the title when clicking the whole card
* without having to make the whole card a button or anchor tag.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,7 @@ exports[`EuiContextMenuPanel updating items and content updates to items should
<EuiResizeObserver onResize={[Function: onResize]}>
<div>
<EuiContextMenuItem data-counter={0} buttonRef={[Function: bound ]}>
<button disabled={[undefined]} className=\\"euiContextMenuItem\\" type=\\"button\\" data-counter={0}>
<button disabled={false} className=\\"euiContextMenuItem\\" type=\\"button\\" data-counter={0}>
<span className=\\"euiContextMenu__itemLayout\\">
<span className=\\"euiContextMenuItem__text\\">
Option A
Expand All @@ -458,7 +458,7 @@ exports[`EuiContextMenuPanel updating items and content updates to items should
</button>
</EuiContextMenuItem>
<EuiContextMenuItem data-counter={1} buttonRef={[Function: bound ]}>
<button disabled={[undefined]} className=\\"euiContextMenuItem\\" type=\\"button\\" data-counter={1}>
<button disabled={false} className=\\"euiContextMenuItem\\" type=\\"button\\" data-counter={1}>
<span className=\\"euiContextMenu__itemLayout\\">
<span className=\\"euiContextMenuItem__text\\">
Option B
Expand All @@ -480,7 +480,7 @@ exports[`EuiContextMenuPanel updating items and content updates to items should
<EuiResizeObserver onResize={[Function: onResize]}>
<div>
<EuiContextMenuItem data-counter={0} buttonRef={[Function: bound ]}>
<button disabled={[undefined]} className=\\"euiContextMenuItem\\" type=\\"button\\" data-counter={0}>
<button disabled={false} className=\\"euiContextMenuItem\\" type=\\"button\\" data-counter={0}>
<span className=\\"euiContextMenu__itemLayout\\">
<span className=\\"euiContextMenuItem__text\\">
Option A
Expand All @@ -489,7 +489,7 @@ exports[`EuiContextMenuPanel updating items and content updates to items should
</button>
</EuiContextMenuItem>
<EuiContextMenuItem data-counter={1} buttonRef={[Function: bound ]}>
<button disabled={[undefined]} className=\\"euiContextMenuItem\\" type=\\"button\\" data-counter={1}>
<button disabled={false} className=\\"euiContextMenuItem\\" type=\\"button\\" data-counter={1}>
<span className=\\"euiContextMenu__itemLayout\\">
<span className=\\"euiContextMenuItem__text\\">
Option B
Expand Down Expand Up @@ -539,7 +539,7 @@ exports[`EuiContextMenuPanel updating items and content updates to items should
<EuiResizeObserver onResize={[Function: onResize]}>
<div>
<EuiContextMenuItem data-counter={0} buttonRef={[Function: bound ]}>
<button disabled={[undefined]} className=\\"euiContextMenuItem\\" type=\\"button\\" data-counter={0}>
<button disabled={false} className=\\"euiContextMenuItem\\" type=\\"button\\" data-counter={0}>
<span className=\\"euiContextMenu__itemLayout\\">
<span className=\\"euiContextMenuItem__text\\">
Option A
Expand All @@ -548,7 +548,7 @@ exports[`EuiContextMenuPanel updating items and content updates to items should
</button>
</EuiContextMenuItem>
<EuiContextMenuItem data-counter={1} buttonRef={[Function: bound ]}>
<button disabled={[undefined]} className=\\"euiContextMenuItem\\" type=\\"button\\" data-counter={1}>
<button disabled={false} className=\\"euiContextMenuItem\\" type=\\"button\\" data-counter={1}>
<span className=\\"euiContextMenu__itemLayout\\">
<span className=\\"euiContextMenuItem__text\\">
Option B
Expand All @@ -570,7 +570,7 @@ exports[`EuiContextMenuPanel updating items and content updates to items should
<EuiResizeObserver onResize={[Function: onResize]}>
<div>
<EuiContextMenuItem data-counter={2} buttonRef={[Function: bound ]}>
<button disabled={[undefined]} className=\\"euiContextMenuItem\\" type=\\"button\\" data-counter={2}>
<button disabled={false} className=\\"euiContextMenuItem\\" type=\\"button\\" data-counter={2}>
<span className=\\"euiContextMenu__itemLayout\\">
<span className=\\"euiContextMenuItem__text\\">
Option A
Expand All @@ -579,7 +579,7 @@ exports[`EuiContextMenuPanel updating items and content updates to items should
</button>
</EuiContextMenuItem>
<EuiContextMenuItem data-counter={3} buttonRef={[Function: bound ]}>
<button disabled={[undefined]} className=\\"euiContextMenuItem\\" type=\\"button\\" data-counter={3}>
<button disabled={false} className=\\"euiContextMenuItem\\" type=\\"button\\" data-counter={3}>
<span className=\\"euiContextMenu__itemLayout\\">
<span className=\\"euiContextMenuItem__text\\">
Option B
Expand Down
6 changes: 5 additions & 1 deletion src/components/context_menu/context_menu_item.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import { EuiIcon } from '../icon';
import { EuiToolTip, ToolTipPositions } from '../tool_tip';

import { getSecureRelForTarget } from '../../services';
import { validateHref } from '../../services/security/href_validator';

export type EuiContextMenuItemIcon = ReactElement<any> | string | HTMLElement;

Expand Down Expand Up @@ -90,7 +91,7 @@ export class EuiContextMenuItem extends Component<Props> {
hasPanel,
icon,
buttonRef,
disabled,
disabled: _disabled,
layoutAlign = 'center',
toolTipTitle,
toolTipContent,
Expand All @@ -102,6 +103,9 @@ export class EuiContextMenuItem extends Component<Props> {
} = this.props;
let iconInstance;

const isHrefValid = !href || validateHref(href);
const disabled = _disabled || !isHrefValid;

if (icon) {
switch (typeof icon) {
case 'string':
Expand Down
Loading

0 comments on commit 0031003

Please sign in to comment.