From 49b1749e98c873c564c2463eb8eb3189d434eaca Mon Sep 17 00:00:00 2001 From: Louis Delbosc Date: Wed, 28 Sep 2022 18:33:53 +0200 Subject: [PATCH] Add function to allow hosts from environment variables (#287) * Add allowed host option to handle CORS requests * Update readme with new GRIST_ALLOWED_HOSTS environment variable --- README.md | 3 ++- app/server/lib/Authorizer.ts | 4 ++-- app/server/lib/Triggers.ts | 3 ++- app/server/lib/requestUtils.ts | 18 +++++++++++++++--- test/server/lib/DocApi.ts | 18 ++++++++++++++++++ 5 files changed, 39 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 0a8e6be9..10ebbf23 100644 --- a/README.md +++ b/README.md @@ -201,7 +201,8 @@ APP_HOME_URL | url prefix for home api (home and doc servers need this) APP_STATIC_URL | url prefix for static resources APP_STATIC_INCLUDE_CUSTOM_CSS | set to "true" to include custom.css (from APP_STATIC_URL) in static pages APP_UNTRUSTED_URL | URL at which to serve/expect plugin content. -GRIST_ADAPT_DOMAIN | set to "true" to support multiple base domains (careful, host header should be trustworthy) +GRIST_ADAPT_DOMAIN | set to "true" to support multiple base domains (careful, host header should be trustworthy) +GRIST_ALLOWED_HOSTS | comma-separated list of permitted domains origin for requests (e.g. my.site,another.com) GRIST_APP_ROOT | directory containing Grist sandbox and assets (specifically the sandbox and static subdirectories). GRIST_BACKUP_DELAY_SECS | wait this long after a doc change before making a backup GRIST_DATA_DIR | directory in which to store document caches. diff --git a/app/server/lib/Authorizer.ts b/app/server/lib/Authorizer.ts index 7375560c..1f544302 100644 --- a/app/server/lib/Authorizer.ts +++ b/app/server/lib/Authorizer.ts @@ -18,7 +18,7 @@ import {makeId} from 'app/server/lib/idUtils'; import log from 'app/server/lib/log'; import {IPermitStore, Permit} from 'app/server/lib/Permit'; import {AccessTokenInfo} from 'app/server/lib/AccessTokens'; -import {allowHost, getOriginUrl, optStringParam} from 'app/server/lib/requestUtils'; +import {allowHost, isEnvironmentAllowedHost, getOriginUrl, optStringParam} from 'app/server/lib/requestUtils'; import * as cookie from 'cookie'; import {NextFunction, Request, RequestHandler, Response} from 'express'; import {IncomingMessage} from 'http'; @@ -263,7 +263,7 @@ export async function addRequestUser(dbManager: HomeDBManager, permitStore: IPer // custom-domain owner could hijack such sessions. const allowedOrg = getAllowedOrgForSessionID(mreq.sessionID); if (allowedOrg) { - if (allowHost(req, allowedOrg.host)) { + if (allowHost(req, allowedOrg.host) || isEnvironmentAllowedHost(allowedOrg.host)) { customHostSession = ` custom-host-match ${allowedOrg.host}`; } else { // We need an exception for internal forwarding from home server to doc-workers. These use diff --git a/app/server/lib/Triggers.ts b/app/server/lib/Triggers.ts index 2622dccc..cac25651 100644 --- a/app/server/lib/Triggers.ts +++ b/app/server/lib/Triggers.ts @@ -14,6 +14,7 @@ import {promisifyAll} from 'bluebird'; import * as _ from 'lodash'; import fetch from 'node-fetch'; import {createClient, Multi, RedisClient} from 'redis'; +import {matchesBaseDomain} from 'app/server/lib/requestUtils'; promisifyAll(RedisClient.prototype); @@ -550,6 +551,6 @@ export function isUrlAllowed(urlString: string) { } return (process.env.ALLOWED_WEBHOOK_DOMAINS || "").split(",").some(domain => - domain && url.host.endsWith(domain) + domain && matchesBaseDomain(url.host, domain) ); } diff --git a/app/server/lib/requestUtils.ts b/app/server/lib/requestUtils.ts index 6aa61fa9..34c93d2e 100644 --- a/app/server/lib/requestUtils.ts +++ b/app/server/lib/requestUtils.ts @@ -9,6 +9,7 @@ import log from 'app/server/lib/log'; import {Permit} from 'app/server/lib/Permit'; import {Request, Response} from 'express'; import {URL} from 'url'; +import _ from 'lodash'; // log api details outside of dev environment (when GRIST_HOSTED_VERSION is set) const shouldLogApiDetails = Boolean(process.env.GRIST_HOSTED_VERSION); @@ -85,7 +86,7 @@ export function trustOrigin(req: Request, resp: Response): boolean { const origin = req.get('origin'); if (!origin) { return true; } // Not a CORS request. if (process.env.GRIST_HOST && req.hostname === process.env.GRIST_HOST) { return true; } - if (!allowHost(req, new URL(origin))) { return false; } + if (!allowHost(req, new URL(origin)) && !isEnvironmentAllowedHost(new URL(origin))) { return false; } // For a request to a custom domain, the full hostname must match. resp.header("Access-Control-Allow-Origin", origin); @@ -102,15 +103,26 @@ export function allowHost(req: Request, allowedHost: string|URL) { const allowedUrl = (typeof allowedHost === 'string') ? new URL(`${proto}://${allowedHost}`) : allowedHost; if (mreq.isCustomHost) { // For a request to a custom domain, the full hostname must match. - return actualUrl.hostname === allowedUrl.hostname; + return actualUrl.hostname === allowedUrl.hostname; } else { // For requests to a native subdomains, only the base domain needs to match. const allowedDomain = parseSubdomain(allowedUrl.hostname); const actualDomain = parseSubdomain(actualUrl.hostname); - return (actualDomain.base === allowedDomain.base); + return (!_.isEmpty(actualDomain) ? actualDomain.base === allowedDomain.base : allowedUrl.hostname === actualUrl.hostname); } } +export function matchesBaseDomain(domain: string, baseDomain: string) { + return domain === baseDomain || domain.endsWith("." + baseDomain); +} + +export function isEnvironmentAllowedHost(url: string|URL) { + const urlHost = (typeof url === 'string') ? url : url.host; + return (process.env.GRIST_ALLOWED_HOSTS || "").split(",").some(domain => + domain && matchesBaseDomain(urlHost, domain) + ); +} + export function isParameterOn(parameter: any): boolean { return gutil.isAffirmative(parameter); } diff --git a/test/server/lib/DocApi.ts b/test/server/lib/DocApi.ts index 5ad05631..dcc4a9b6 100644 --- a/test/server/lib/DocApi.ts +++ b/test/server/lib/DocApi.ts @@ -2773,6 +2773,23 @@ function testDocApi() { }); }); + describe("Allowed Origin", () => { + it('should allow only example.com', async () => { + async function checkOrigin(origin: string, status: number, error?: string) { + const resp = await axios.get(`${serverUrl}/api/docs/${docIds.Timesheets}/`, + {...chimpy, headers: {...chimpy.headers, "Origin": origin}} + ); + error && assert.deepEqual(resp.data, {error}); + assert.equal(resp.status, status); + } + await checkOrigin("https://www.toto.com", 500, "Unrecognized origin"); + await checkOrigin("https://badexample.com", 500, "Unrecognized origin"); + await checkOrigin("https://bad.com/example.com/toto", 500, "Unrecognized origin"); + await checkOrigin("https://example.com/path", 200); + await checkOrigin("https://good.example.com/toto", 200); + }) + }) + // PLEASE ADD MORE TESTS HERE } @@ -2866,6 +2883,7 @@ class TestServer { REDIS_URL: process.env.TEST_REDIS_URL, APP_HOME_URL: _homeUrl, ALLOWED_WEBHOOK_DOMAINS: `example.com,localhost:${webhooksTestPort}`, + GRIST_ALLOWED_HOSTS: `example.com,localhost:${webhooksTestPort}`, ...process.env };