EES-4823 - Add rel keywords to links based on externality

[sambiramairelogic]

This PR modifies the html attributes of anchor and link tags as they're displayed by the public site. It also fixes a minor bug because we're currently assessing whether a link is external by whether it includes explore-education-statistics.service.gov.uk, which means the admin site is being counted as an internal link.

Links to external sites which give 404s, 500s, or 401s (in the case of the admin site) are being followed by Googlebot and showing up as errors in the Google Search Console.

We should:

• Add noreferrer, and noopener to any links to external sites, including admin. This tells the browser not to provide a referrer when the user clicks the link (a security issue in older browsers), and to begin a new context when it does.
• Add nofollow to any external sites, including admin. This tells Googlebot not to follow them.
• Add external to any sites we don't trust. This tells Googlebot that we don't endorse (not that we necessarily condemn it either!) the target of this link. I've chosen to exempt admin and gov.uk from this, as we're all under the umbrella of the larger gov.uk domain.
• Add target="_blank" to any external links, including admin.
• Add (opens in a new tab) to the link body text to any external links, including admin.
• Continue to adhere to all existing link formatting, including maintaining upper case paths on external links, and trimming all links.

Pic for proof of new behaviour:

[sambiramairelogic]

I do appreciate there's lots of ways we could approach this - we may not need to make the distinction between external-admin and external-trusted, for example. We might also want an array of trusted sites so we can include more than just gov.uk, or we may want to drop this distinction entirely and only have internal and external.

I think this is a good starter for ten and allows us to tweak things going forward - but if anyone disagrees let's chat it through :)

[ntsim]

Fine with this approach 👍

[ntsim]

Could extract this into a module-scoped renderLink function to make this a bit easier to read

[ntsim]

Would be safer to keep the assertion like it was previously (i.e. using toHaveAttribute) there could be additional stuff inside of the rel that we're not expecting and could point to an issue.

The current assertions would fail to detect these issues.

[sambiramairelogic]

I'd rather not revert back to toHaveAttribute because I want to be order agnostic (i.e. the rel could be noopener noreferrer or noreferrer noopener and the test should still pass).

We could maybe do expect(rel.split(' ')).toHaveLength(3) to catch the issues you raise?

[ntsim]

Similar thing to above. Would use not.toHaveAttribute to be more precise. Also it's a bit strange to wrap assertions in a condition like this.

Same applies to test case further down as well.

[ntsim]

Could just drop the undefined parameter as it's optional

[ntsim]

Might want to test multiple values. One value could be a string that only contains spaces i.e. ' '

[ntsim]

As mentioned in other comments, an admin URL isn't always external (i.e. if you're already in admin), and vice-versa for public URLs. I think we need to go further and figure out what our host URL is and compare this with the external link's URL to figure out the externality.

Unfortunately, the implementation for this isn't super straightforward.

One initial idea was to use window.location, but I suspect this won't work well on the public frontend due to it being incompatible with SSR.

Environment variables don't really work as these can't be changed in the admin where they get baked into the build. As a result, I don't think they can be changed server or client-side in the admin.

Currently, I think the simplest approach would be to implement some common config that can set the host URL globally e.g.

// @common/utils/hostUrl.ts

let hostUrl: URL;

export function setHostUrl(value: string): void {
  hostUrl = new URL(value);
}

export function getHostUrl(): URL {
  return hostUrl;
}

We can then call the setHostUrl function in the admin and public frontend (in the correct places) to set the hostUrl.

In the public:

// pages/_app.tsx

// Call this at module-scope
setHostUrl(process.env.PUBLIC_URL);

In the admin:

// src/config.ts

import { setHostUrl } from '@common/utils/clientUrl';

export async function getConfig(): Promise<Config> {
  if (!config) {
    config = await fetch('/api/config').then(r => r.json());

    setHostUrl(config.url);
  }

  return config;
}

The admin will require us to add a new url property to the response from /api/config. This would need to be set from the AdminUri configuration variable, which is already set up for us (see appsettings.Development.json). It just needs to be fully qualified with a https://.

Once we have these bits in place, we should be able to check the URLs using getHostUrl e.g.

import { getHostUrl } from '@common/utils/hostUrl';

const url = new URL(options.url);
const hostUrl = getHostUrl();

if (url.host === hostUrl.host) {
  // internal
} else {
  // external
}

[ntsim]

These look like a mistake. We already have variables for the public API above.

[ntsim]

Given that we're in the frontend and we've already added the external linking props within the Link component, we can just simplify this all down to:

return (
  <li key={id} data-testid="other-release-item">
    <Link to={legacyLinkUrl}>{description}</Link>
  </li>
);

[ntsim]

Similar to above - can replace this with the Link component

[ntsim]

As per other comment - if we use an options object for the function parameter, we don't need to have an undefined here