From b71f2f2a104d37ea27f4e778cfa73cb22ec6f391 Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Wed, 22 Jul 2020 11:43:24 -0400 Subject: [PATCH] (core) add a deployment test for Import-from-URL, and fix underlying issue Summary: * Adds a simple deployment test for the "Import from URL" button. * Makes server aware of plugin hostnames in the appropriate places. * Unrelated but convenient: allows following redirection when importing. Test Plan: Added tests. The `local_deployment` test works. A modified version of this works against `staging_deployment` (using a test url that doesn't require redirection; also staging currently has a hot fix that can hopefully be removed once the code fix included here is in). Reviewers: dsagal Reviewed By: dsagal Differential Revision: https://phab.getgrist.com/D2556 --- app/common/gristUrls.ts | 23 ++++++++++++++++++----- app/server/lib/FlexServer.ts | 8 +++++++- app/server/lib/extractOrg.ts | 18 ++++++++++-------- app/server/lib/uploads.ts | 6 +++++- 4 files changed, 40 insertions(+), 15 deletions(-) diff --git a/app/common/gristUrls.ts b/app/common/gristUrls.ts index ec80b91d..760c7b0a 100644 --- a/app/common/gristUrls.ts +++ b/app/common/gristUrls.ts @@ -75,6 +75,9 @@ export interface OrgUrlOptions { // In single-org mode, this is the single well-known org. singleOrg?: string; + + // Base URL used for accessing plugin material. + pluginUrl?: string; } // Result of getOrgUrlInfo(). @@ -83,9 +86,19 @@ export interface OrgUrlInfo { orgInPath?: string; // If /o/{orgInPath} should be used to access the requested org. } -export function isCustomHost(hostname: string, baseDomain: string) { - // .localtest.me is used by plugin tests for arcane reasons. - return hostname !== 'localhost' && !hostname.endsWith(baseDomain) && !hostname.endsWith('.localtest.me'); +/** + * Given host name, baseDomain, and pluginUrl, determine whether to interpret host + * as a custom domain, a native domain, or a plugin domain. + */ +export function getHostType(hostname: string, options: { + baseDomain?: string, pluginUrl?: string +}): 'native' | 'custom' | 'plugin' { + if (options.pluginUrl && hostname.toLowerCase() === new URL(options.pluginUrl).hostname.toLowerCase()) { + return 'plugin'; + } + if (!options.baseDomain) { return 'native'; } + if (hostname !== 'localhost' && !hostname.endsWith(options.baseDomain)) { return 'custom'; } + return 'native'; } export function getOrgUrlInfo(newOrg: string, currentHostname: string, options: OrgUrlOptions): OrgUrlInfo { @@ -95,7 +108,7 @@ export function getOrgUrlInfo(newOrg: string, currentHostname: string, options: if (!options.baseDomain || currentHostname === 'localhost') { return {orgInPath: newOrg}; } - if (newOrg === options.org && isCustomHost(currentHostname, options.baseDomain)) { + if (newOrg === options.org && getHostType(currentHostname, options) !== 'native') { return {}; } return {hostname: newOrg + options.baseDomain}; @@ -361,7 +374,7 @@ export interface GristLoadConfig { errMessage?: string; // URL for client to use for untrusted content. - pluginUrl?: string|null; + pluginUrl?: string; // Stripe API key for use on the client. stripeAPIKey?: string; diff --git a/app/server/lib/FlexServer.ts b/app/server/lib/FlexServer.ts index 48a94c84..21da9734 100644 --- a/app/server/lib/FlexServer.ts +++ b/app/server/lib/FlexServer.ts @@ -69,6 +69,8 @@ export interface FlexServerOptions { // Base domain for org hostnames, starting with ".". Defaults to the base domain of APP_HOME_URL. baseDomain?: string; + // Base URL for plugins, if permitted. Defaults to APP_UNTRUSTED_URL. + pluginUrl?: string; } export interface RequestWithGrist extends express.Request { @@ -97,6 +99,7 @@ export class FlexServer implements GristServer { public electronServerMethods: ElectronServerMethods; public readonly docsRoot: string; private _defaultBaseDomain: string|undefined; + private _pluginUrl: string|undefined; private _billing: IBilling; private _instanceRoot: string; private _docManager: DocManager; @@ -146,6 +149,8 @@ export class FlexServer implements GristServer { const homeUrl = process.env.APP_HOME_URL; this._defaultBaseDomain = options.baseDomain || (homeUrl && parseSubdomain(new URL(homeUrl).hostname).base); this.info.push(['defaultBaseDomain', this._defaultBaseDomain]); + this._pluginUrl = options.pluginUrl || process.env.APP_UNTRUSTED_URL; + this.info.push(['pluginUrl', this._pluginUrl]); this.app.use((req, res, next) => { (req as RequestWithGrist).gristServer = this; @@ -365,7 +370,7 @@ export class FlexServer implements GristServer { // Prepare cache for managing org-to-host relationship. public addHosts() { if (this._check('hosts', 'homedb')) { return; } - this._hosts = new Hosts(this._defaultBaseDomain, this.dbManager); + this._hosts = new Hosts(this._defaultBaseDomain, this.dbManager, this._pluginUrl); } public async initHomeDBManager() { @@ -1262,6 +1267,7 @@ export class FlexServer implements GristServer { const {hostname, orgInPath} = getOrgUrlInfo(subdomain, req.hostname, { org: req.org, baseDomain: this._defaultBaseDomain, + pluginUrl: this._pluginUrl, singleOrg: process.env.GRIST_SINGLE_ORG, }); const redirectUrl = new URL(pathname, `${req.protocol}://${req.get('host')}`); diff --git a/app/server/lib/extractOrg.ts b/app/server/lib/extractOrg.ts index 02fb931f..1ddbec78 100644 --- a/app/server/lib/extractOrg.ts +++ b/app/server/lib/extractOrg.ts @@ -1,6 +1,6 @@ import { ApiError } from 'app/common/ApiError'; import { mapGetOrSet, MapWithTTL } from 'app/common/AsyncCreate'; -import { extractOrgParts, getKnownOrg, isCustomHost } from 'app/common/gristUrls'; +import { extractOrgParts, getHostType, getKnownOrg } from 'app/common/gristUrls'; import { Organization } from 'app/gen-server/entity/Organization'; import { HomeDBManager } from 'app/gen-server/lib/HomeDBManager'; import { NextFunction, Request, RequestHandler, Response } from 'express'; @@ -39,7 +39,8 @@ export class Hosts { private _org2host = new MapWithTTL>(ORG_HOST_CACHE_TTL); // baseDomain should start with ".". It may be undefined for localhost or single-org mode. - constructor(private _baseDomain: string|undefined, private _dbManager: HomeDBManager) { + constructor(private _baseDomain: string|undefined, private _dbManager: HomeDBManager, + private _pluginUrl: string|undefined) { } /** @@ -86,14 +87,15 @@ export class Hosts { return {org: singleOrg, url: parts.pathRemainder, isCustomHost: false}; } - // Fake the protocol; it doesn't matter for parsing out the hostname. - if (this._isNativeDomain(hostname)) { + const hostType = this._getHostType(hostname); + if (hostType === 'native') { if (parts.mismatch) { throw new ApiError(`Wrong org for this domain: ` + `'${parts.orgFromPath}' does not match '${parts.orgFromHost}'`, 400); } return {org: parts.subdomain || '', url: parts.pathRemainder, isCustomHost: false}; - + } else if (hostType === 'plugin') { + return {org: '', url: parts.pathRemainder, isCustomHost: false}; } else { // Otherwise check for a custom host. const org = await mapGetOrSet(this._host2org, hostname, async () => { @@ -145,7 +147,7 @@ export class Hosts { private async _redirectHost(req: Request, resp: Response, next: NextFunction) { const {org} = req as RequestWithOrg; - if (org && this._isNativeDomain(req.hostname) && !this._dbManager.isMergedOrg(org)) { + if (org && this._getHostType(req.hostname) === 'native' && !this._dbManager.isMergedOrg(org)) { // Check if the org has a preferred host. const orgHost = await mapGetOrSet(this._org2host, org, async () => { const o = await this._dbManager.connection.manager.findOne(Organization, {domain: org}); @@ -160,7 +162,7 @@ export class Hosts { return next(); } - private _isNativeDomain(hostname: string) { - return !this._baseDomain || !isCustomHost(hostname, this._baseDomain); + private _getHostType(hostname: string) { + return getHostType(hostname, {baseDomain: this._baseDomain, pluginUrl: this._pluginUrl}); } } diff --git a/app/server/lib/uploads.ts b/app/server/lib/uploads.ts index af9b14da..6923889c 100644 --- a/app/server/lib/uploads.ts +++ b/app/server/lib/uploads.ts @@ -346,7 +346,11 @@ export async function fetchURL(url: string, accessId: string|null): Promise { - const response: FetchResponse = await Deps.fetch(url, {headers}); + const response: FetchResponse = await Deps.fetch(url, { + redirect: 'follow', + follow: 10, + headers + }); await _checkForError(response); if (fileName === '') { const disposition = response.headers.get('content-disposition') || '';