From 10822d3b860bd8a2f40c00fef80e396803a0f553 Mon Sep 17 00:00:00 2001 From: Florent Date: Mon, 6 Nov 2023 09:24:59 +0100 Subject: [PATCH] getHostType: consider APP_DOC_INTERNAL_URL as native (#715) The getHostType() now returns "native" when the host corresponds to the value of APP_DOC_INTERNAL_URL. T While trying to scale, with a different internal and public URL for doc workers, and having configured the org to be specified in the path (GRIST_ORG_IN_PATH=true), the APP_DOC_INTERNAL_URL parameter was not treated as internal which made the connection between home server and doc workers impossible. --------- https://github.com/gristlabs/grist-core/pull/715 Co-authored-by: Florent FAYOLLE --- README.md | 1 + app/common/gristUrls.ts | 12 +++++++-- test/common/gristUrls.ts | 55 +++++++++++++++++++++++++++++++++++++- test/server/lib/DocApi2.ts | 3 ++- 4 files changed, 67 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 53b11a3a..2beeea35 100644 --- a/README.md +++ b/README.md @@ -250,6 +250,7 @@ Variable | Purpose -------- | ------- ALLOWED_WEBHOOK_DOMAINS | comma-separated list of permitted domains to use in webhooks (e.g. webhook.site,zapier.com). You can set this to `*` to allow all domains, but if doing so, we recommend using a carefully locked-down proxy (see `GRIST_HTTPS_PROXY`) if you do not entirely trust users. Otherwise services on your internal network may become vulnerable to manipulation. APP_DOC_URL | doc worker url, set when starting an individual doc worker (other servers will find doc worker urls via redis) +APP_DOC_INTERNAL_URL | like `APP_DOC_URL` but used by the home server to reach the server using an internal domain name resolution (like in a docker environment). Defaults to `APP_DOC_URL` 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 diff --git a/app/common/gristUrls.ts b/app/common/gristUrls.ts index ef94a416..3ab254c9 100644 --- a/app/common/gristUrls.ts +++ b/app/common/gristUrls.ts @@ -167,6 +167,12 @@ export interface OrgUrlInfo { orgInPath?: string; // If /o/{orgInPath} should be used to access the requested org. } +function isDocInternalUrl(host: string) { + if (!process.env.APP_DOC_INTERNAL_URL) { return false; } + const internalUrl = new URL('/', process.env.APP_DOC_INTERNAL_URL); + return internalUrl.host === host; +} + /** * Given host (optionally with port), baseDomain, and pluginUrl, determine whether to interpret host * as a custom domain, a native domain, or a plugin domain. @@ -184,8 +190,10 @@ export function getHostType(host: string, options: { const hostname = host.split(":")[0]; if (!options.baseDomain) { return 'native'; } - if (hostname !== 'localhost' && !hostname.endsWith(options.baseDomain)) { return 'custom'; } - return 'native'; + if (hostname === 'localhost' || isDocInternalUrl(host) || hostname.endsWith(options.baseDomain)) { + return 'native'; + } + return 'custom'; } export function getOrgUrlInfo(newOrg: string, currentHost: string, options: OrgUrlOptions): OrgUrlInfo { diff --git a/test/common/gristUrls.ts b/test/common/gristUrls.ts index a12877a0..6ce7c5c5 100644 --- a/test/common/gristUrls.ts +++ b/test/common/gristUrls.ts @@ -1,5 +1,6 @@ -import {decodeUrl, IGristUrlState, parseFirstUrlPart} from 'app/common/gristUrls'; +import {decodeUrl, getHostType, IGristUrlState, parseFirstUrlPart} from 'app/common/gristUrls'; import {assert} from 'chai'; +import * as testUtils from 'test/server/testUtils'; describe('gristUrls', function() { @@ -76,4 +77,56 @@ describe('gristUrls', function() { assert.deepEqual(parseFirstUrlPart('o', ''), {path: ''}); }); }); + + describe('getHostType', function() { + const defaultOptions = { + baseDomain: 'getgrist.com', + pluginUrl: 'https://plugin.getgrist.com', + }; + + let oldEnv: testUtils.EnvironmentSnapshot; + + beforeEach(function () { + oldEnv = new testUtils.EnvironmentSnapshot(); + }); + + afterEach(function () { + oldEnv.restore(); + }); + + it('should interpret localhost as "native"', function() { + assert.equal(getHostType('localhost', defaultOptions), 'native'); + assert.equal(getHostType('localhost:8080', defaultOptions), 'native'); + }); + + it('should interpret base domain as "native"', function() { + assert.equal(getHostType('getgrist.com', defaultOptions), 'native'); + assert.equal(getHostType('www.getgrist.com', defaultOptions), 'native'); + assert.equal(getHostType('foo.getgrist.com', defaultOptions), 'native'); + assert.equal(getHostType('foo.getgrist.com:8080', defaultOptions), 'native'); + }); + + it('should interpret plugin domain as "plugin"', function() { + assert.equal(getHostType('plugin.getgrist.com', defaultOptions), 'plugin'); + assert.equal(getHostType('PLUGIN.getgrist.com', { pluginUrl: 'https://pLuGin.getgrist.com' }), 'plugin'); + }); + + it('should interpret other domains as "custom"', function() { + assert.equal(getHostType('foo.com', defaultOptions), 'custom'); + assert.equal(getHostType('foo.bar.com', defaultOptions), 'custom'); + }); + + it('should interpret doc internal url as "native"', function() { + process.env.APP_DOC_INTERNAL_URL = 'https://doc-worker-123.internal/path'; + assert.equal(getHostType('doc-worker-123.internal', defaultOptions), 'native'); + assert.equal(getHostType('doc-worker-123.internal:8080', defaultOptions), 'custom'); + assert.equal(getHostType('doc-worker-124.internal', defaultOptions), 'custom'); + + process.env.APP_DOC_INTERNAL_URL = 'https://doc-worker-123.internal:8080/path'; + assert.equal(getHostType('doc-worker-123.internal:8080', defaultOptions), 'native'); + assert.equal(getHostType('doc-worker-123.internal', defaultOptions), 'custom'); + assert.equal(getHostType('doc-worker-124.internal:8080', defaultOptions), 'custom'); + assert.equal(getHostType('doc-worker-123.internal:8079', defaultOptions), 'custom'); + }); + }); }); diff --git a/test/server/lib/DocApi2.ts b/test/server/lib/DocApi2.ts index fea62cb8..29345282 100644 --- a/test/server/lib/DocApi2.ts +++ b/test/server/lib/DocApi2.ts @@ -17,9 +17,10 @@ describe('DocApi2', function() { let owner: UserAPI; let wsId: number; testUtils.setTmpLogLevel('error'); - const oldEnv = new testUtils.EnvironmentSnapshot(); + let oldEnv: testUtils.EnvironmentSnapshot; before(async function() { + oldEnv = new testUtils.EnvironmentSnapshot(); const tmpDir = await createTmpDir(); process.env.GRIST_DATA_DIR = tmpDir; process.env.STRIPE_ENDPOINT_SECRET = 'TEST_WITHOUT_ENDPOINT_SECRET';