Skip to content

Commit

Permalink
Expands the secure_rel utility to handle elastic domains (#1565)
Browse files Browse the repository at this point in the history
* Expands the getSecureRelForTarget utility to let referrer's pass-through to elastic.co domains.
  • Loading branch information
joelgriffith authored Feb 14, 2019
1 parent 98e5cbf commit 99b52fd
Show file tree
Hide file tree
Showing 11 changed files with 207 additions and 42 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
- Adjusted set of Elastic Logos in `EuiIcon` to look better in dark mode. ([#1462](https://github.com/elastic/eui/pull/1562))
- Added `isCopyable` prop to `EuiCodeBlock` ([#1556](https://github.com/elastic/eui/pull/1556))
- Added optional `Snippet` tab to docs and renamed demo tabs ([#1556](https://github.com/elastic/eui/pull/1556))
- Expanded `getSecureRelForTarget` to handle elastic.co domains as a referrer whitelist ([#1565](https://github.com/elastic/eui/pull/1565))
- New `url` utility for verifying if a URL is a referrer whitelist ([#1565](https://github.com/elastic/eui/pull/1565))

## [`7.0.0`](https://github.com/elastic/eui/tree/v7.0.0)

Expand Down
2 changes: 1 addition & 1 deletion src/components/button/button.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ export const EuiButton = ({
// <a> elements don't respect the `disabled` attribute. So if we're disabled, we'll just pretend
// this is a button and piggyback off its disabled styles.
if (href && !isDisabled) {
const secureRel = getSecureRelForTarget(target, rel);
const secureRel = getSecureRelForTarget({ href, target, rel });

return (
<a
Expand Down
2 changes: 1 addition & 1 deletion src/components/button/button_empty/button_empty.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ export const EuiButtonEmpty = ({
// <a> elements don't respect the `disabled` attribute. So if we're disabled, we'll just pretend
// this is a button and piggyback off its disabled styles.
if (href && !isDisabled) {
const secureRel = getSecureRelForTarget(target, rel);
const secureRel = getSecureRelForTarget({ href, target, rel });

return (
<a
Expand Down
2 changes: 1 addition & 1 deletion src/components/button/button_icon/button_icon.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export const EuiButtonIcon = ({
// <a> elements don't respect the `disabled` attribute. So if we're disabled, we'll just pretend
// this is a button and piggyback off its disabled styles.
if (href && !isDisabled) {
const secureRel = getSecureRelForTarget(target, rel);
const secureRel = getSecureRelForTarget({ href, target, rel });

return (
<a
Expand Down
2 changes: 1 addition & 1 deletion src/components/card/card.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ export const EuiCard = ({

let secureRel;
if (href) {
secureRel = getSecureRelForTarget(target, rel);
secureRel = getSecureRelForTarget({ href, target, rel });
}

let imageNode;
Expand Down
2 changes: 1 addition & 1 deletion src/components/context_menu/context_menu_item.js
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ export class EuiContextMenuItem extends Component {
// <a> elements don't respect the `disabled` attribute. So if we're disabled, we'll just pretend
// this is a button and piggyback off its disabled styles.
if (href && !disabled) {
const secureRel = getSecureRelForTarget(target, rel);
const secureRel = getSecureRelForTarget({ href, target, rel });

button = (
<a
Expand Down
2 changes: 1 addition & 1 deletion src/components/link/link.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export const EuiLink = ({
);
}

const secureRel = getSecureRelForTarget(target, rel);
const secureRel = getSecureRelForTarget({ href, target, rel });

return (
<a
Expand Down
144 changes: 126 additions & 18 deletions src/services/security/get_secure_rel_for_target.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,49 +3,157 @@ import { getSecureRelForTarget } from './get_secure_rel_for_target';
describe('getSecureRelForTarget', () => {
describe('returns rel', () => {
test('when target is not supplied', () => {
expect(getSecureRelForTarget(undefined, 'hello')).toBe('hello');
expect(
getSecureRelForTarget({
href: undefined,
target: undefined,
rel: 'hello',
})
).toBe('hello');
});

test('when target is empty', () => {
expect(getSecureRelForTarget('', 'hello')).toBe('hello');
expect(
getSecureRelForTarget({
href: undefined,
target: '',
rel: 'hello',
})
).toBe('hello');
});

test('when target is not _blank', () => {
expect(getSecureRelForTarget('_self', 'hello')).toBe('hello');
expect(
getSecureRelForTarget({
href: undefined,
target: '_self',
rel: 'hello',
})
).toBe('hello');
});

test('when target is undefined', () => {
expect(getSecureRelForTarget('_self', 'hello')).toBe('hello');
expect(
getSecureRelForTarget({
href: undefined,
target: undefined,
rel: 'hello',
})
).toBe('hello');
});
});

describe('returns noopener noreferrer', () => {
describe('returns noopener noreferrer when domain is unsafe', () => {
test('when href is not specified', () => {
expect(
getSecureRelForTarget({
href: undefined,
target: '_blank',
rel: undefined,
})
).toBe('noopener noreferrer');
});

test('when rel contains neither', () => {
expect(
getSecureRelForTarget({
href: 'https://www.google.com/',
target: '_blank',
rel: undefined,
})
).toBe('noopener noreferrer');
});

test('when rel contains both', () => {
expect(
getSecureRelForTarget({
href: 'https://wwwelastic.co/',
target: '_blank',
rel: 'noopener noreferrer',
})
).toBe('noopener noreferrer');
});

test('when rel contains noopener', () => {
expect(
getSecureRelForTarget({
href: 'wss://www.elastic.co/',
target: '_blank',
rel: 'noopener',
})
).toBe('noopener noreferrer');
});

test('when rel contains noreferrer', () => {
expect(
getSecureRelForTarget({
href: 'smb://www.elastic.co/',
target: '_blank',
rel: 'noreferrer',
})
).toBe('noopener noreferrer');
});

test('including the original rel value', () => {
expect(
getSecureRelForTarget({
href: '/foo/bar',
target: '_blank',
rel: 'nofollow',
})
).toBe('nofollow noopener noreferrer');
});
});

describe('returns noopener when domain is safe', () => {
test('when rel contains neither', () => {
expect(getSecureRelForTarget('_blank')).toBe('noopener noreferrer');
expect(
getSecureRelForTarget({
href: 'https://www.elastic.co',
target: '_blank',
rel: undefined,
})
).toBe('noopener');
});

test('when rel contains both', () => {
expect(getSecureRelForTarget('_blank', 'noopener noreferrer')).toBe(
'noopener noreferrer'
);
expect(
getSecureRelForTarget({
href: 'https://www.elastic.co',
target: '_blank',
rel: 'noopener noreferrer',
})
).toBe('noopener');
});

test('when rel contains noopener', () => {
expect(getSecureRelForTarget('_blank', 'noopener')).toBe(
'noopener noreferrer'
);
expect(
getSecureRelForTarget({
href: 'https://docs.elastic.co',
target: '_blank',
rel: 'noopener',
})
).toBe('noopener');
});

test('when rel contains noreferrer', () => {
expect(getSecureRelForTarget('_blank', 'noreferrer')).toBe(
'noreferrer noopener'
);
expect(
getSecureRelForTarget({
href: 'https://elastic.co',
target: '_blank',
rel: 'noreferrer',
})
).toBe('noopener');
});

test('including the original rel value', () => {
expect(getSecureRelForTarget('_blank', 'nofollow')).toBe(
'nofollow noopener noreferrer'
);
expect(
getSecureRelForTarget({
href: 'http://discuss.elastic.co',
target: '_blank',
rel: 'nofollow',
})
).toBe('nofollow noopener');
});
});
});
42 changes: 24 additions & 18 deletions src/services/security/get_secure_rel_for_target.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,31 +2,37 @@
* Secures outbound links. For more info:
* https://www.jitbit.com/alexblog/256-targetblank---the-most-underestimated-vulnerability-ever/
*/
export const getSecureRelForTarget = (
target?: '_blank' | '_self' | '_parent' | '_top' | string,
rel?: string
) => {
if (!target) {
return rel;
}
import filter from 'lodash/filter';
import { isDomainSecure } from '../url';

if (!target.includes('_blank')) {
export const getSecureRelForTarget = ({
href,
target,
rel,
}: {
href?: string;
target?: '_blank' | '_self' | '_parent' | '_top' | string;
rel?: string;
}) => {
if (!target || !target.includes('_blank')) {
return rel;
}

if (!rel) {
return 'noopener noreferrer';
}

let secureRel = rel;
const isElasticHref = !!href && isDomainSecure(href);
const relParts = !!rel
? filter(rel.split(' '), part => !!part.length && part !== 'noreferrer')
: [];

if (!secureRel.includes('noopener')) {
secureRel = `${secureRel} noopener`;
if (relParts.indexOf('noopener') === -1) {
relParts.push('noopener');
}

if (!secureRel.includes('noreferrer')) {
secureRel = `${secureRel} noreferrer`;
if (!isElasticHref) {
relParts.push('noreferrer');
}

return secureRel.trim();
return relParts
.sort()
.join(' ')
.trim();
};
33 changes: 33 additions & 0 deletions src/services/url.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { isDomainSecure } from './url';

describe(`url`, () => {
describe(`#isDomainSecure`, () => {
it(`returns true for secure domains`, () => {
expect(isDomainSecure('https://elastic.co')).toEqual(true);
expect(isDomainSecure('https://elastic.co?foo=bar')).toEqual(true);
expect(isDomainSecure('https://elastic.co/')).toEqual(true);
expect(isDomainSecure('https://www.elastic.co')).toEqual(true);
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('https://elastic.co/cool/url/with?lots=of&params')
).toEqual(true);
});

it(`returns false for unsecure domains`, () => {
expect(isDomainSecure('https://wwwelastic.co')).toEqual(false);
expect(isDomainSecure('https://www.zelastic.co')).toEqual(false);
expect(isDomainSecure('https://*elastic.co')).toEqual(false);
expect(isDomainSecure('http://elastic.com')).toEqual(false);
expect(isDomainSecure('https://elastic.co.now')).toEqual(false);
expect(isDomainSecure('elastic.co')).toEqual(false);
expect(isDomainSecure('smb://www.elastic.co')).toEqual(false);
expect(
isDomainSecure(
'https://wwwelastic.co/cool/url/with?lots=of&params/https://elastic.co'
)
).toEqual(false);
});
});
});
16 changes: 16 additions & 0 deletions src/services/url.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
const isElasticDomain = /(https?:\/\/(.+?\.)?elastic\.co((\/|\?)[A-Za-z0-9\-\._~:\/\?#\[\]@!$&'\(\)\*\+,;\=]*)?)/g;

// 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) {
return false;
}
const [match] = matches;

return match.length === url.length;
};

0 comments on commit 99b52fd

Please sign in to comment.