diff --git a/CHANGELOG.md b/CHANGELOG.md index a71a76eedb3..0111a1b8f72 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) diff --git a/src/components/button/button.js b/src/components/button/button.js index a95d609c74c..2a5023b5ee2 100644 --- a/src/components/button/button.js +++ b/src/components/button/button.js @@ -113,7 +113,7 @@ export const EuiButton = ({ // 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 ( 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 ( 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 ( 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 = ( { 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'); }); }); }); diff --git a/src/services/security/get_secure_rel_for_target.ts b/src/services/security/get_secure_rel_for_target.ts index 11f42542119..7002d2167e9 100644 --- a/src/services/security/get_secure_rel_for_target.ts +++ b/src/services/security/get_secure_rel_for_target.ts @@ -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(); }; diff --git a/src/services/url.test.ts b/src/services/url.test.ts new file mode 100644 index 00000000000..b535362a51d --- /dev/null +++ b/src/services/url.test.ts @@ -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¶ms') + ).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¶ms/https://elastic.co' + ) + ).toEqual(false); + }); + }); +}); diff --git a/src/services/url.ts b/src/services/url.ts new file mode 100644 index 00000000000..1c4afafdb07 --- /dev/null +++ b/src/services/url.ts @@ -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; +};