From 2f8b19cc5478b7d2bce8109689c4df12c8ab2e1d Mon Sep 17 00:00:00 2001 From: Jason Stoltzfus Date: Wed, 18 Jan 2023 16:23:26 -0500 Subject: [PATCH 1/4] Removed domain check from EUILink --- .../button_display/_button_display.test.tsx | 31 ++----- .../card/__snapshots__/card.test.tsx.snap | 2 +- .../__snapshots__/side_nav.test.tsx.snap | 2 +- .../get_secure_rel_for_target.test.ts | 85 +------------------ .../security/get_secure_rel_for_target.ts | 7 +- src/services/url.test.ts | 45 ---------- src/services/url.ts | 24 ------ 7 files changed, 13 insertions(+), 183 deletions(-) delete mode 100644 src/services/url.test.ts delete mode 100644 src/services/url.ts diff --git a/src/components/button/button_display/_button_display.test.tsx b/src/components/button/button_display/_button_display.test.tsx index 10e97d292ea..8e2ab7c95f2 100644 --- a/src/components/button/button_display/_button_display.test.tsx +++ b/src/components/button/button_display/_button_display.test.tsx @@ -94,30 +94,25 @@ describe('EuiButtonDisplay', () => { expect(container.querySelector('a')?.getAttribute('type')).toBeNull(); }); - it('inserts `rel=noreferrer` for non-Elastic urls ', () => { + it('inserts `rel=noreferrer`', () => { const { queryByTestSubject } = render( <> - ); - expect(queryByTestSubject('norel')?.getAttribute('rel')).toEqual(''); - expect(queryByTestSubject('badprotocol')?.getAttribute('rel')).toEqual( + expect(queryByTestSubject('rel')?.getAttribute('rel')).toEqual( 'noreferrer' ); - expect(queryByTestSubject('notelastic')?.getAttribute('rel')).toEqual( + expect(queryByTestSubject('norel')?.getAttribute('rel')).toEqual( 'noreferrer' ); }); @@ -128,20 +123,12 @@ describe('EuiButtonDisplay', () => { - ); - expect(queryByTestSubject('elastic')?.getAttribute('rel')).toEqual( - 'noopener' - ); - expect(queryByTestSubject('notelastic')?.getAttribute('rel')).toEqual( + expect(queryByTestSubject('blank')?.getAttribute('rel')).toEqual( 'noopener noreferrer' ); }); diff --git a/src/components/card/__snapshots__/card.test.tsx.snap b/src/components/card/__snapshots__/card.test.tsx.snap index 5a81425c32c..ce238ec7e05 100644 --- a/src/components/card/__snapshots__/card.test.tsx.snap +++ b/src/components/card/__snapshots__/card.test.tsx.snap @@ -36,7 +36,7 @@ exports[`EuiCard betaBadgeProps renders href 1`] = ` class="euiBetaBadge emotion-euiBetaBadge-hollow-m-baseline-euiCard__betaBadge" href="http://www.elastic.co/" id="generated-idBetaBadge" - rel="" + rel="noreferrer" title="Link" > Link diff --git a/src/components/side_nav/__snapshots__/side_nav.test.tsx.snap b/src/components/side_nav/__snapshots__/side_nav.test.tsx.snap index ffc72bb4285..52f4e605f30 100644 --- a/src/components/side_nav/__snapshots__/side_nav.test.tsx.snap +++ b/src/components/side_nav/__snapshots__/side_nav.test.tsx.snap @@ -348,7 +348,7 @@ exports[`EuiSideNav props items renders items which are links 1`] = ` { test('when target is not supplied', () => { expect( getSecureRelForTarget({ - href: undefined, target: undefined, rel: 'hello', }) @@ -23,7 +22,6 @@ describe('getSecureRelForTarget', () => { test('when target is empty', () => { expect( getSecureRelForTarget({ - href: undefined, target: '', rel: 'hello', }) @@ -33,7 +31,6 @@ describe('getSecureRelForTarget', () => { test('when target is not _blank', () => { expect( getSecureRelForTarget({ - href: undefined, target: '_self', rel: 'hello', }) @@ -41,11 +38,10 @@ describe('getSecureRelForTarget', () => { }); }); - describe('returns noopener noreferrer when domain is unsafe', () => { + describe('returns noopener noreferrer', () => { test('when href is not specified', () => { expect( getSecureRelForTarget({ - href: undefined, target: '_blank', rel: undefined, }) @@ -55,7 +51,6 @@ describe('getSecureRelForTarget', () => { test('when rel contains neither', () => { expect( getSecureRelForTarget({ - href: 'https://www.google.com/', target: '_blank', rel: undefined, }) @@ -65,7 +60,6 @@ describe('getSecureRelForTarget', () => { test('when rel contains both', () => { expect( getSecureRelForTarget({ - href: 'https://wwwelastic.co/', target: '_blank', rel: 'noopener noreferrer', }) @@ -75,7 +69,6 @@ describe('getSecureRelForTarget', () => { test('when rel contains noopener', () => { expect( getSecureRelForTarget({ - href: 'wss://www.elastic.co/', target: '_blank', rel: 'noopener', }) @@ -85,7 +78,6 @@ describe('getSecureRelForTarget', () => { test('when rel contains noreferrer', () => { expect( getSecureRelForTarget({ - href: 'smb://www.elastic.co/', target: '_blank', rel: 'noreferrer', }) @@ -95,85 +87,10 @@ describe('getSecureRelForTarget', () => { 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({ - href: 'https://www.elastic.co', - target: '_blank', - rel: undefined, - }) - ).toBe('noopener'); - }); - - test('when rel contains both', () => { - expect( - getSecureRelForTarget({ - href: 'https://www.elastic.co', - target: '_blank', - rel: 'noopener noreferrer', - }) - ).toBe('noopener'); - }); - - test('when rel contains noopener', () => { - expect( - getSecureRelForTarget({ - href: 'https://docs.elastic.co', - target: '_blank', - rel: 'noopener', - }) - ).toBe('noopener'); - }); - - test('when rel contains noreferrer', () => { - expect( - getSecureRelForTarget({ - href: 'https://elastic.co', - target: '_blank', - rel: 'noreferrer', - }) - ).toBe('noopener'); - }); - - test('including the original rel value', () => { - expect( - getSecureRelForTarget({ - href: 'http://discuss.elastic.co', - target: '_blank', - rel: 'nofollow', - }) - ).toBe('nofollow noopener'); - }); - }); - - describe('returns no noreferrer when domain is safe without target _blank', () => { - test('when target and rel is undefined', () => { - expect( - getSecureRelForTarget({ - href: 'http://discuss.elastic.co', - target: undefined, - rel: undefined, - }) - ).toBe(''); - }); - - test('when rel is specified', () => { - expect( - getSecureRelForTarget({ - href: 'https://discuss.elastic.co', - target: undefined, - rel: 'nofollow', - }) - ).toBe('nofollow'); - }); - }); }); diff --git a/src/services/security/get_secure_rel_for_target.ts b/src/services/security/get_secure_rel_for_target.ts index 365662eaaec..3a98127838e 100644 --- a/src/services/security/get_secure_rel_for_target.ts +++ b/src/services/security/get_secure_rel_for_target.ts @@ -10,10 +10,8 @@ * Secures outbound links. For more info: * https://www.jitbit.com/alexblog/256-targetblank---the-most-underestimated-vulnerability-ever/ */ -import { isDomainSecure } from '../url'; export const getSecureRelForTarget = ({ - href, target = '', rel, }: { @@ -21,14 +19,11 @@ export const getSecureRelForTarget = ({ target?: '_blank' | '_self' | '_parent' | '_top' | string; rel?: string; }) => { - const isElasticHref = !!href && isDomainSecure(href); const relParts = !!rel ? rel.split(' ').filter((part) => !!part.length && part !== 'noreferrer') : []; - if (!isElasticHref) { - relParts.push('noreferrer'); - } + relParts.push('noreferrer'); if (target.includes('_blank') && relParts.indexOf('noopener') === -1) { relParts.push('noopener'); diff --git a/src/services/url.test.ts b/src/services/url.test.ts deleted file mode 100644 index 12bb1147129..00000000000 --- a/src/services/url.test.ts +++ /dev/null @@ -1,45 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0 and the Server Side Public License, v 1; you may not use this file except - * in compliance with, at your election, the Elastic License 2.0 or the Server - * Side Public License, v 1. - */ - -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('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); - 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('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 deleted file mode 100644 index a61d5d2860c..00000000000 --- a/src/services/url.ts +++ /dev/null @@ -1,24 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0 and the Server Side Public License, v 1; you may not use this file except - * in compliance with, at your election, the Elastic License 2.0 or the Server - * Side Public License, v 1. - */ - -const isElasticHost = /^([a-zA-Z0-9]+\.)*elastic\.co$/; - -// 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); - const protocolMatches = - parsed.protocol === 'http:' || parsed.protocol === 'https:'; - const domainMatches = !!parsed.host.match(isElasticHost); - return protocolMatches && domainMatches; - } catch (e) { - return false; - } -}; From 830dd0c9f549f24e39278b8f01ba1d84df7627eb Mon Sep 17 00:00:00 2001 From: Jason Stoltzfus Date: Wed, 18 Jan 2023 16:44:19 -0500 Subject: [PATCH 2/4] Added changelog --- upcoming_changelogs/#6535.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 upcoming_changelogs/#6535.md diff --git a/upcoming_changelogs/#6535.md b/upcoming_changelogs/#6535.md new file mode 100644 index 00000000000..95bcf4205ba --- /dev/null +++ b/upcoming_changelogs/#6535.md @@ -0,0 +1 @@ +- Removed behavior from EUILink to *not* apply "noreferrer" to elastic.co domains \ No newline at end of file From b9a6dafddaca855f2f03122c99026375b36d65d6 Mon Sep 17 00:00:00 2001 From: Jason Stoltzfus Date: Wed, 18 Jan 2023 16:53:21 -0500 Subject: [PATCH 3/4] Renamed changelog --- upcoming_changelogs/{#6535.md => 6535.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename upcoming_changelogs/{#6535.md => 6535.md} (100%) diff --git a/upcoming_changelogs/#6535.md b/upcoming_changelogs/6535.md similarity index 100% rename from upcoming_changelogs/#6535.md rename to upcoming_changelogs/6535.md From 5cdb1bfed11840b3a572d28a5a2b43cf789eceb0 Mon Sep 17 00:00:00 2001 From: Jason Stoltzfus Date: Thu, 19 Jan 2023 08:49:13 -0500 Subject: [PATCH 4/4] Update upcoming_changelogs/6535.md Co-authored-by: Cee Chen <549407+cee-chen@users.noreply.github.com> --- upcoming_changelogs/6535.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/upcoming_changelogs/6535.md b/upcoming_changelogs/6535.md index 95bcf4205ba..b7cfaf438ff 100644 --- a/upcoming_changelogs/6535.md +++ b/upcoming_changelogs/6535.md @@ -1 +1,3 @@ -- Removed behavior from EUILink to *not* apply "noreferrer" to elastic.co domains \ No newline at end of file +**Breaking changes** + +- `EuiLink` now applies `rel="noreferrer"` to all domains, including `elastic.co` \ No newline at end of file