Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set noreferrer also for non _blank links #2008

Merged
merged 3 commits into from
Jun 7, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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