(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
This commit is contained in:
Paul Fitzpatrick 2020-07-22 11:43:24 -04:00
parent a27032df3e
commit b71f2f2a10
4 changed files with 40 additions and 15 deletions

View File

@ -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;

View File

@ -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')}`);

View File

@ -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<string, Promise<string|undefined>>(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});
}
}

View File

@ -346,7 +346,11 @@ export async function fetchURL(url: string, accessId: string|null): Promise<Uplo
*/
async function _fetchURL(url: string, accessId: string|null, fileName: string,
headers?: {[key: string]: string}): Promise<UploadResult> {
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') || '';