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

Removed domain check from EuiLink #6535

Merged
merged 4 commits into from
Jan 19, 2023
Merged
Show file tree
Hide file tree
Changes from 3 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
31 changes: 9 additions & 22 deletions src/components/button/button_display/_button_display.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(
<>
<EuiButtonDisplay
href="https://elastic.co"
data-test-subj="norel"
rel="noreferrer" // Removes this
/>
<EuiButtonDisplay
href="ftp://elastic.co"
data-test-subj="badprotocol"
data-test-subj="rel"
rel="noreferrer"
/>
<EuiButtonDisplay
href="https://hello.world"
data-test-subj="notelastic"
href="https://elastic.co"
data-test-subj="norel"
/>
</>
);

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'
);
});
Expand All @@ -128,20 +123,12 @@ describe('EuiButtonDisplay', () => {
<EuiButtonDisplay
href="https://elastic.co"
target="_blank"
data-test-subj="elastic"
/>
<EuiButtonDisplay
href="https://hello.world"
target="_blank"
data-test-subj="notelastic"
data-test-subj="blank"
/>
</>
);

expect(queryByTestSubject('elastic')?.getAttribute('rel')).toEqual(
'noopener'
);
expect(queryByTestSubject('notelastic')?.getAttribute('rel')).toEqual(
expect(queryByTestSubject('blank')?.getAttribute('rel')).toEqual(
'noopener noreferrer'
);
});
Expand Down
2 changes: 1 addition & 1 deletion src/components/card/__snapshots__/card.test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ exports[`EuiSideNav props items renders items which are links 1`] = `
<a
class="euiSideNavItemButton euiSideNavItemButton--isClickable"
href="http://www.elastic.co"
rel=""
rel="noreferrer"
>
<span
class="euiSideNavItemButton__content"
Expand Down
85 changes: 1 addition & 84 deletions src/services/security/get_secure_rel_for_target.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ describe('getSecureRelForTarget', () => {
test('when target is not supplied', () => {
expect(
getSecureRelForTarget({
href: undefined,
target: undefined,
rel: 'hello',
})
Expand All @@ -23,7 +22,6 @@ describe('getSecureRelForTarget', () => {
test('when target is empty', () => {
expect(
getSecureRelForTarget({
href: undefined,
target: '',
rel: 'hello',
})
Expand All @@ -33,19 +31,17 @@ describe('getSecureRelForTarget', () => {
test('when target is not _blank', () => {
expect(
getSecureRelForTarget({
href: undefined,
target: '_self',
rel: 'hello',
})
).toBe('hello noreferrer');
});
});

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,
})
Expand All @@ -55,7 +51,6 @@ describe('getSecureRelForTarget', () => {
test('when rel contains neither', () => {
expect(
getSecureRelForTarget({
href: 'https://www.google.com/',
target: '_blank',
rel: undefined,
})
Expand All @@ -65,7 +60,6 @@ describe('getSecureRelForTarget', () => {
test('when rel contains both', () => {
expect(
getSecureRelForTarget({
href: 'https://wwwelastic.co/',
target: '_blank',
rel: 'noopener noreferrer',
})
Expand All @@ -75,7 +69,6 @@ describe('getSecureRelForTarget', () => {
test('when rel contains noopener', () => {
expect(
getSecureRelForTarget({
href: 'wss://www.elastic.co/',
target: '_blank',
rel: 'noopener',
})
Expand All @@ -85,7 +78,6 @@ describe('getSecureRelForTarget', () => {
test('when rel contains noreferrer', () => {
expect(
getSecureRelForTarget({
href: 'smb://www.elastic.co/',
target: '_blank',
rel: 'noreferrer',
})
Expand All @@ -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', () => {
cee-chen marked this conversation as resolved.
Show resolved Hide resolved
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');
});
});
});
7 changes: 1 addition & 6 deletions src/services/security/get_secure_rel_for_target.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,25 +10,20 @@
* 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,
}: {
href?: string;
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');
Expand Down
45 changes: 0 additions & 45 deletions src/services/url.test.ts

This file was deleted.

24 changes: 0 additions & 24 deletions src/services/url.ts

This file was deleted.

1 change: 1 addition & 0 deletions upcoming_changelogs/6535.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Removed behavior from EUILink to *not* apply "noreferrer" to elastic.co domains
JasonStoltz marked this conversation as resolved.
Show resolved Hide resolved