Remove the GRIST_ALLOWED_HOSTS environment variable

[jonathanperret]

Context

While reviewing @fflorent 's #895 and discussing with various people at ANCT we realized that the GRIST_ALLOWED_HOSTS environment variable was added in a PR submitted in 2022 by another developer working at the time with ANCT : #287.

It turns out that the use case that originally motivated the PR (#271) is now covered in two ways:

• since e590e65, cross-origin requests that do not require authentication are allowed, so that public documents can be queried just fine from any external domain ;
• allowing authenticated requests from external domains currently requires, in addition to including the domain in GRIST_ALLOWED_HOSTS, making an API key available in a client, which is definitely something one wants to discourage as pointed out by @dsagal in env var for controling allowed origins ? #271 (comment). Indeed we saw this mistake occur in the very web app that motivated the addition of GRIST_ALLOWED_HOSTS (the exposed API key has since been revoked).

We are now confident that we do not need this variable and have removed it from our deployment. Given the specificity of the use case it enables, and the lack of adequate documentation as evidenced by #895, it seems doubtful that any other Grist deployment would currently be depending on it, although it is of course difficult to be certain.

We also estimate that keeping it available in Grist has multiple downsides:

• as mentioned above, pretty much the only scenario it enables is the unsafe one where an API key is accessible to a public Web client;
• it adds unnecessary complexity to an already difficult-to-follow process of origin validation (GRIST_ALLOWED_HOSTS played a significant part in my bafflement while trying to implement origin checking for Support HTTP long polling as an alternative to WebSockets #859);
• while I have no evidence for this, I suspect a confused Grist self-hoster may end up setting GRIST_ALLOWED_HOSTS to bypass an otherwise broken configuration of APP_HOME_URL and the other relevant variables (which are unfortunately numerous).

Proposed solution

This PR mostly reverts commit 49b1749. A few notes on what was kept:

• in allowHost, the logic introduced by Add function to allow hosts from environment variables #287 that allows a domain name that doesn't match the standard <orgname>.<basedomain>.<tld> pattern enforced by parseSubdomain to match itself was kept, although with simplified logic that does not involve lodash. This might not be necessary, since requests whose Host match their Origin are by definition not cross-origin, and would therefore not end up in allowHost, but was kept out of an abundance of caution.
• the matchesBaseDomain function seemeed like an unrelated but useful addition of Add function to allow hosts from environment variables #287 (notably making the checks against ALLOWED_WEBHOOK_DOMAINS more robust), so it was kept as well.

[dsagal]

Looks good to me too. One note is to remove GRIST_ALLOWED_HOSTS from test/server/lib/helpers/TestServer.ts, where it no longer plays a role.

[jonathanperret]

Looks good to me too. One note is to remove GRIST_ALLOWED_HOSTS from test/server/lib/helpers/TestServer.ts, where it no longer plays a role.

Good catch, I had missed that one, it's removed now.

[paulfitz]

Thanks @jonathanperret and everyone involved in the detective work here.