Skip to content

Commit

Permalink
PLATUI-3270 remove backlink security checks (#428)
Browse files Browse the repository at this point in the history
  • Loading branch information
TimothyFothergill authored Dec 18, 2024
1 parent 590d3a7 commit 5eaab83
Show file tree
Hide file tree
Showing 7 changed files with 56 additions and 89 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,13 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

## [Unreleased]

## [6.46.0] - 2024-12-17

### Changed

- Removed backlink security checks which were causing user experience issues
- Created an ADR to confirm

## [6.45.0] - 2024-12-17

### Changed
Expand Down
37 changes: 37 additions & 0 deletions docs/adr/0008-remove-backlink-referer-checks.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
# Remove Backlink referrer checks

* Status: accepted
* Deciders: PlatUI
* Date: 2024-12-17
* Components: [back-link-helper](/src/components/back-link-helper/)

## Context and Problem Statement

As part of the back link helper, we offered the ability to ensure that the back link won't appear in certain scenarios:
* When the referrer is not on an allow-list
* When there is no referrer

This hasn't been adding much value - and in some cases, has caused issues and confusion. We have taken the decision to remove these checks.

## Decision Drivers

* A link from other government domains cause the back link to not appear.
* The security checks are not used widely by services that use JavaScript to make their back links behave like the browser back button.

## Considered Options

* Keeping the checks in place as the checks were based on another implementation on the platform.
* Removing the checks altogether.

## Decision Outcome

Chosen option: "Removing the checks altogether."

### Positive Consequences

* This reduces any potential issues from other government services linking to our services.
* Simpler user experience.

### Negative Consequences

* Users may be sent back to services and sites not in our domain
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "hmrc-frontend",
"version": "6.45.0",
"version": "6.46.0",
"description": "Design patterns for HMRC frontends",
"scripts": {
"start": "gulp dev",
Expand Down
34 changes: 0 additions & 34 deletions src/components/back-link-helper/back-link-helper.browser.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,40 +8,6 @@ describe('/components/back-link-helper', () => {
}

describe('When a JS-enabled back link is included on a page', () => {
it('should be hidden when referrer is on a different domain', async () => {
await render(page, withHmrcStylesAndScripts(`
<a href="#" class="govuk-back-link" data-module="hmrc-back-link">back</a>
`), {
referer: 'http://somewhere-else.com',
});
expect(await linkDisplayStyle()).toBe('none');
});

it('should be shown when referrer is on a different domain, but the domain has been allowlisted', async () => {
await render(page, withHmrcStylesAndScripts(`
<a href="#" class="govuk-back-link" data-module="hmrc-back-link">back</a>
`), {
referer: 'http://account.hmrc.gov.uk',
});
expect(await linkDisplayStyle()).not.toBe('none');
});

it('should be hidden when referrer is empty', async () => {
await render(page, withHmrcStylesAndScripts(`
<a href="#" class="govuk-back-link" data-module="hmrc-back-link">back</a>
`), {
referer: '',
});
expect(await linkDisplayStyle()).toBe('none');
});

it('should be visible when JS is enabled and there is a referrer on same domain', async () => {
await render(page, withHmrcStylesAndScripts(`
<a href="#" class="govuk-back-link" data-module="hmrc-back-link">back</a>
`), withRefererFromSameDomain);
expect(await linkDisplayStyle()).not.toBe('none');
});

it('should be hidden when JS is disabled', async () => {
await page.setJavaScriptEnabled(false);
await render(page, withHmrcStylesAndScripts(`
Expand Down
40 changes: 9 additions & 31 deletions src/components/back-link-helper/back-link-helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,39 +7,17 @@ function BackLinkHelper($module, window, document) {
BackLinkHelper.prototype.init = function init() {
// do nothing if History API is absent
if (this.window.history) {
/* TODO: Remains unclear whether a check for the same domain is necessary for security reasons.
There may be user research suggesting considerations regarding the visibility of the
back link on refresh.
Currently, a page refresh sets the referer to empty, leading to the back link being hidden
under our existing logic.
*/
const referrerAllowList = ['account-np.hmrc.gov.uk', 'account.hmrc.gov.uk'];
const shouldHideBackLink = () => {
const referer = this.document.referrer;
if (!referer) return true;
const referredFromDifferentDomain = () => referer.indexOf(this.window.location.host) === -1;
const referrerNotOnAllowList = () => !referrerAllowList.some(
(allowListedDomain) => referer.includes(allowListedDomain),
);
return referredFromDifferentDomain() && referrerNotOnAllowList();
};
// prevent resubmit warning
if (this.window.history.replaceState && typeof this.window.history.replaceState === 'function') {
this.window.history.replaceState(null, null, this.window.location.href);
}

// hide the backlink if the referrer is on a different domain or the referrer is not set
if (shouldHideBackLink()) {
this.$module.classList.add('hmrc-hidden-backlink');
} else {
// prevent resubmit warning
if (this.window.history.replaceState && typeof this.window.history.replaceState === 'function') {
this.window.history.replaceState(null, null, this.window.location.href);
this.$module.addEventListener('click', (event) => {
event.preventDefault();
if (this.window.history.back && typeof this.window.history.back === 'function') {
this.window.history.back();
}

this.$module.addEventListener('click', (event) => {
event.preventDefault();
if (this.window.history.back && typeof this.window.history.back === 'function') {
this.window.history.back();
}
});
}
});
}
};

Expand Down
21 changes: 0 additions & 21 deletions src/components/back-link-helper/back-link-helper.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,27 +71,6 @@ describe('/components/back-link-helper', () => {
expect(mockWindow.history.back).toHaveBeenCalled();
});

it('adds .hmrc-hidden-backlink class to backlink when document referer is empty', () => {
const sut = new BackLinkHelper(mockAnchorTag, mockWindow, { referrer: '' });
sut.init();

expect(mockAnchorTag.classList.add.mock.calls[0][0]).toBe('hmrc-hidden-backlink');
});

it('adds .hmrc-hidden-backlink class to backlink when document referer is null', () => {
const sut = new BackLinkHelper(mockAnchorTag, mockWindow, { referrer: null });
sut.init();

expect(mockAnchorTag.classList.add.mock.calls[0][0]).toBe('hmrc-hidden-backlink');
});

it('adds .hmrc-hidden-backlink class to backlink if document referer is on a different domain', () => {
const sut = new BackLinkHelper(mockAnchorTag, mockWindow, { referrer: 'https://some.other.site/some/referrer' });
sut.init();

expect(mockAnchorTag.classList.add.mock.calls[0][0]).toBe('hmrc-hidden-backlink');
});

it('does not throw if window.history.back() is not implemented', () => {
const ancientBrowser = { history: {}, location: { host: 'https://tax.service.gov.uk' } };
const sut = new BackLinkHelper(mockAnchorTag, ancientBrowser, mockDocument);
Expand Down

0 comments on commit 5eaab83

Please sign in to comment.