Skip to content

Commit

Permalink
Set noreferrer also for non _blank links (#2008)
Browse files Browse the repository at this point in the history
* Set noreferrer also for non _blank links

* Add changelog entry
  • Loading branch information
timroes authored Jun 7, 2019
1 parent 2dcb20f commit a728779
Show file tree
Hide file tree
Showing 7 changed files with 43 additions and 25 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
## [`master`](https://github.com/elastic/eui/tree/master)

- Attach `noreferrer` also to links without `target="_blank"` ([#2008](https://github.com/elastic/eui/pull/2008))
- Convert observer utility components to TypeScript ([#2009](https://github.com/elastic/eui/pull/2009))

**Bug fixes**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ exports[`EuiBreadcrumbs is rendered 1`] = `
class="euiLink euiLink--subdued euiBreadcrumb customClass"
data-test-subj="breadcrumbsAnimals"
href="#"
rel="noreferrer"
title="[object Object]"
>
<span>
Expand All @@ -32,6 +33,7 @@ exports[`EuiBreadcrumbs is rendered 1`] = `
<a
class="euiLink euiLink--subdued euiBreadcrumb euiBreadcrumb--truncate"
href="#"
rel="noreferrer"
title="Boa constrictor"
>
Boa constrictor
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ exports[`EuiContextMenuItem props href renders a link 1`] = `
class="euiContextMenuItem testClass1 testClass2"
data-test-subj="test subject string"
href="url"
rel="noreferrer"
>
<span
class="euiContextMenu__itemLayout"
Expand Down Expand Up @@ -116,7 +117,7 @@ exports[`EuiContextMenuItem props rel is rendered 1`] = `
class="euiContextMenuItem testClass1 testClass2"
data-test-subj="test subject string"
href="url"
rel="help"
rel="help noreferrer"
>
<span
class="euiContextMenu__itemLayout"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ exports[`EuiHeaderBreadcrumbs is rendered 1`] = `
class="euiLink euiLink--subdued euiBreadcrumb customClass"
data-test-subj="breadcrumbsAnimals"
href="#"
rel="noreferrer"
title="[object Object]"
>
<span>
Expand All @@ -32,6 +33,7 @@ exports[`EuiHeaderBreadcrumbs is rendered 1`] = `
<a
class="euiLink euiLink--subdued euiBreadcrumb"
href="#"
rel="noreferrer"
title="Boa constrictor"
>
Boa constrictor
Expand Down
6 changes: 5 additions & 1 deletion src/components/link/__snapshots__/link.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ exports[`EuiLink it supports both href and onClick 1`] = `
<a
class="euiLink euiLink--primary"
href="/imalink"
rel="noreferrer"
/>
`;

Expand Down Expand Up @@ -76,6 +77,7 @@ exports[`EuiLink supports children 1`] = `
<a
class="euiLink euiLink--primary"
href="#"
rel="noreferrer"
>
<span>
Hiya!!!
Expand All @@ -87,21 +89,23 @@ exports[`EuiLink supports href 1`] = `
<a
class="euiLink euiLink--primary"
href="/baz/bing"
rel="noreferrer"
/>
`;

exports[`EuiLink supports rel 1`] = `
<a
class="euiLink euiLink--primary"
href="hoi"
rel="stylesheet"
rel="noreferrer stylesheet"
/>
`;

exports[`EuiLink supports target 1`] = `
<a
class="euiLink euiLink--primary"
href="#"
rel="noreferrer"
target="_parent"
/>
`;
Expand Down
40 changes: 26 additions & 14 deletions src/services/security/get_secure_rel_for_target.test.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
import { getSecureRelForTarget } from './get_secure_rel_for_target';

describe('getSecureRelForTarget', () => {
describe('returns rel', () => {
describe('returns rel and noreferrer', () => {
test('when target is not supplied', () => {
expect(
getSecureRelForTarget({
href: undefined,
target: undefined,
rel: 'hello',
})
).toBe('hello');
).toBe('hello noreferrer');
});

test('when target is empty', () => {
Expand All @@ -19,7 +19,7 @@ describe('getSecureRelForTarget', () => {
target: '',
rel: 'hello',
})
).toBe('hello');
).toBe('hello noreferrer');
});

test('when target is not _blank', () => {
Expand All @@ -29,17 +29,7 @@ describe('getSecureRelForTarget', () => {
target: '_self',
rel: 'hello',
})
).toBe('hello');
});

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

Expand Down Expand Up @@ -156,4 +146,26 @@ describe('getSecureRelForTarget', () => {
).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');
});
});
});
14 changes: 5 additions & 9 deletions src/services/security/get_secure_rel_for_target.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,30 +7,26 @@ import { isDomainSecure } from '../url';

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

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

if (relParts.indexOf('noopener') === -1) {
relParts.push('noopener');
}

if (!isElasticHref) {
relParts.push('noreferrer');
}

if (target.includes('_blank') && relParts.indexOf('noopener') === -1) {
relParts.push('noopener');
}

return relParts
.sort()
.join(' ')
Expand Down

0 comments on commit a728779

Please sign in to comment.