From 625fce5f65e8a1427a52d8e754255f148833a4b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaros=C5=82aw=20Sadzi=C5=84ski?= Date: Tue, 13 Jul 2021 09:29:38 +0200 Subject: [PATCH] (core) Refactoring google drive plugin Summary: Finishing implementation for google drive plugin. - Refactoring plugin code to make it more robust and to follow grist ux - Changing the way server hosts untrusted user content, from different domain to different port Test Plan: Browser tests Reviewers: dsagal, paulfitz Reviewed By: paulfitz Differential Revision: https://phab.getgrist.com/D2881 --- app/client/components/Importer.ts | 7 ++++-- app/common/gristUrls.ts | 31 +++++++++++++++++------- app/server/devServerMain.ts | 39 +++++++++++++++++++++++++++++++ app/server/lib/FlexServer.ts | 23 +++++++++++++++++- app/server/lib/extractOrg.ts | 19 ++++++++------- app/server/lib/sendAppPage.ts | 3 ++- app/server/mergedServerMain.ts | 4 ++++ test/nbrowser/testServer.ts | 1 + 8 files changed, 105 insertions(+), 22 deletions(-) diff --git a/app/client/components/Importer.ts b/app/client/components/Importer.ts index 6ee6ec9b..c85cb875 100644 --- a/app/client/components/Importer.ts +++ b/app/client/components/Importer.ts @@ -158,6 +158,9 @@ export class Importer extends Disposable { } } } catch (err) { + if (!this._openModalCtl) { + this._showImportDialog(); + } this._renderError(err.message); return; } @@ -267,7 +270,7 @@ export class Importer extends Disposable { const tableRowModel = this._gristDoc.docModel.dataTables[importResult.tables[0].hiddenTableId].tableMetaRow; await this._gristDoc.openDocPage(tableRowModel.primaryViewId()); } - this._openModalCtl!.close(); + this._openModalCtl?.close(); this.dispose(); } @@ -276,7 +279,7 @@ export class Importer extends Disposable { await this._docComm.cancelImportFiles( this._getTransformedDataSource(this._uploadResult), this._getHiddenTableIds()); } - this._openModalCtl!.close(); + this._openModalCtl?.close(); this.dispose(); } diff --git a/app/common/gristUrls.ts b/app/common/gristUrls.ts index c3d8f210..6e0bb9ed 100644 --- a/app/common/gristUrls.ts +++ b/app/common/gristUrls.ts @@ -96,28 +96,38 @@ export interface OrgUrlInfo { } /** - * Given host name, baseDomain, and pluginUrl, determine whether to interpret 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. */ -export function getHostType(hostname: string, options: { +export function getHostType(host: string, options: { baseDomain?: string, pluginUrl?: string }): 'native' | 'custom' | 'plugin' { - if (options.pluginUrl && hostname.toLowerCase() === new URL(options.pluginUrl).hostname.toLowerCase()) { - return 'plugin'; + + if (options.pluginUrl) { + const url = new URL(options.pluginUrl); + if (url.host.toLowerCase() === host.toLowerCase()) { + return 'plugin'; + } } + + const hostname = host.split(":")[0]; 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 { +export function getOrgUrlInfo(newOrg: string, currentHost: string, options: OrgUrlOptions): OrgUrlInfo { if (newOrg === options.singleOrg) { return {}; } - if (!options.baseDomain || currentHostname === 'localhost') { - return {orgInPath: newOrg}; + const hostType = getHostType(currentHost, options); + if (hostType !== 'plugin') { + const hostname = currentHost.split(":")[0]; + if (!options.baseDomain || hostname === 'localhost') { + return {orgInPath: newOrg}; + } } - if (newOrg === options.org && getHostType(currentHostname, options) !== 'native') { + if (newOrg === options.org && hostType !== 'native') { return {}; } return {hostname: newOrg + options.baseDomain}; @@ -140,7 +150,7 @@ export function encodeUrl(gristConfig: Partial, if (state.org) { // We figure out where to stick the org using the gristConfig and the current host. - const {hostname, orgInPath} = getOrgUrlInfo(state.org, baseLocation.hostname, gristConfig); + const {hostname, orgInPath} = getOrgUrlInfo(state.org, baseLocation.host, gristConfig); if (hostname) { url.hostname = hostname; } @@ -419,6 +429,9 @@ export interface GristLoadConfig { // The timestamp when this gristConfig was generated. timestampMs: number; + + // Google Client Id, used in Google integration (ex: Google Drive Plugin) + googleClientId?: string; } // Acceptable org subdomains are alphanumeric (hyphen also allowed) and of diff --git a/app/server/devServerMain.ts b/app/server/devServerMain.ts index 7f12ae7f..c59c4842 100644 --- a/app/server/devServerMain.ts +++ b/app/server/devServerMain.ts @@ -34,6 +34,22 @@ function getPort(envVarName: string, fallbackPort: number): number { return val ? parseInt(val, 10) : fallbackPort; } +// Checks whether to serve user content on same domain but on different port +function checkUserContentPort(): number | null { + if (process.env.APP_UNTRUSTED_URL && process.env.APP_HOME_URL) { + const homeUrl = new URL(process.env.APP_HOME_URL); + const pluginUrl = new URL(process.env.APP_UNTRUSTED_URL); + // If the hostname of both home and plugin url are the same, + // but the ports are different + if (homeUrl.hostname === pluginUrl.hostname && + homeUrl.port !== pluginUrl.port) { + const port = parseInt(pluginUrl.port || '80', 10); + return port; + } + } + return null; +} + export async function main() { log.info("=========================================================================="); log.info("== devServer"); @@ -76,6 +92,12 @@ export async function main() { } } + if (!process.env.GOOGLE_CLIENT_ID) { + // those key is only for development purposes + // and is no secret as it is publicly visible in a plugin page + process.env.GOOGLE_CLIENT_ID = '632317221841-ce66sfp00rf92dip4548dn4hf2ga79us.apps.googleusercontent.com'; + } + if (process.env.GRIST_SINGLE_PORT) { log.info("=========================================================================="); log.info("== mergedServer"); @@ -85,6 +107,14 @@ export async function main() { } const server = await mergedServerMain(port, ["home", "docs", "static"]); await server.addTestingHooks(); + // If plugin content is served from same host but on different port, + // run webserver on that port + const userPort = checkUserContentPort(); + if (userPort !== null) { + log.info("=========================================================================="); + log.info("== userContent"); + await server.startCopy('pluginServer', userPort); + } return; } @@ -118,6 +148,15 @@ export async function main() { await home.startCopy('webServer', webServerPort); } + // If plugin content is served from same host but on different port, + // run webserver on that port + const userPort = checkUserContentPort(); + if (userPort !== null) { + log.info("=========================================================================="); + log.info("== userContent"); + await home.startCopy('pluginServer', userPort); + } + // Bring up the docWorker(s) log.info("=========================================================================="); log.info("== docWorker"); diff --git a/app/server/lib/FlexServer.ts b/app/server/lib/FlexServer.ts index 5d780e6a..a728cae4 100644 --- a/app/server/lib/FlexServer.ts +++ b/app/server/lib/FlexServer.ts @@ -747,6 +747,27 @@ export class FlexServer implements GristServer { httpsServer: this.httpsServer, }); } + /** + * Add endpoint that servers a javascript file with various api keys that + * are used by the client libraries. + */ + public addClientSecrets() { + if (this._check('clientSecret')) { return; } + this.app.get('/client-secret.js', expressWrap(async (req, res) => { + const config = this.getGristConfig(); + // Currently we are exposing only Google keys. + // Those keys are eventually visible by the client, but should be usable + // only from Grist's domains. + const secrets = { + googleClientId : config.googleClientId, + }; + res.set('Content-Type', 'application/javascript'); + res.status(200); + res.send(` + window.gristClientSecret = ${JSON.stringify(secrets)} + `); + })); + } public addLoginRoutes() { if (this._check('login', 'org', 'sessions')) { return; } @@ -1416,7 +1437,7 @@ export class FlexServer implements GristServer { * path. */ private _getOrgRedirectUrl(req: RequestWithLogin, subdomain: string, pathname: string = req.originalUrl): string { - const {hostname, orgInPath} = getOrgUrlInfo(subdomain, req.hostname, { + const {hostname, orgInPath} = getOrgUrlInfo(subdomain, req.get('host')!, { org: req.org, baseDomain: this._defaultBaseDomain, pluginUrl: this._pluginUrl, diff --git a/app/server/lib/extractOrg.ts b/app/server/lib/extractOrg.ts index 1ddbec78..9e53c8af 100644 --- a/app/server/lib/extractOrg.ts +++ b/app/server/lib/extractOrg.ts @@ -64,9 +64,8 @@ export class Hosts { // Extract org info in a request. This applies to the low-level IncomingMessage type (rather // than express.Request that derives from it) to be usable with websocket requests too. public async getOrgInfo(req: IncomingMessage): Promise { - const host = req.headers.host || ''; - const hostname = host.split(':')[0]; // Strip out port (ignores IPv6 but is OK for us). - const info = await this.getOrgInfoFromParts(hostname, req.url!); + const host = req.headers.host!; + const info = await this.getOrgInfoFromParts(host, req.url!); // "Organization" header is used in proxying to doc worker, so respect it if // no org info found in url. if (!info.org && req.headers.organization) { @@ -77,7 +76,9 @@ export class Hosts { // Extract org, isCustomHost, and the URL with /o/ORG stripped away. Throws ApiError for // mismatching org or invalid custom domain. Hostname should not include port. - public async getOrgInfoFromParts(hostname: string, urlPath: string): Promise { + public async getOrgInfoFromParts(host: string, urlPath: string): Promise { + const hostname = host.split(':')[0]; // Strip out port (ignores IPv6 but is OK for us). + // Extract the org from the host and URL path. const parts = extractOrgParts(hostname, urlPath); @@ -87,7 +88,7 @@ export class Hosts { return {org: singleOrg, url: parts.pathRemainder, isCustomHost: false}; } - const hostType = this._getHostType(hostname); + const hostType = this._getHostType(host); if (hostType === 'native') { if (parts.mismatch) { throw new ApiError(`Wrong org for this domain: ` + @@ -147,14 +148,14 @@ export class Hosts { private async _redirectHost(req: Request, resp: Response, next: NextFunction) { const {org} = req as RequestWithOrg; - if (org && this._getHostType(req.hostname) === 'native' && !this._dbManager.isMergedOrg(org)) { + if (org && this._getHostType(req.headers.host!) === '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}); return o && o.host || undefined; }); if (orgHost && orgHost !== req.hostname) { - const url = new URL(`${req.protocol}://${req.get('host')}${req.path}`); + const url = new URL(`${req.protocol}://${req.headers.host}${req.path}`); url.hostname = orgHost; // assigning hostname rather than host preserves port. return resp.redirect(url.href); } @@ -162,7 +163,7 @@ export class Hosts { return next(); } - private _getHostType(hostname: string) { - return getHostType(hostname, {baseDomain: this._baseDomain, pluginUrl: this._pluginUrl}); + private _getHostType(host: string) { + return getHostType(host, {baseDomain: this._baseDomain, pluginUrl: this._pluginUrl}); } } diff --git a/app/server/lib/sendAppPage.ts b/app/server/lib/sendAppPage.ts index 49fa48c3..af4eb2ad 100644 --- a/app/server/lib/sendAppPage.ts +++ b/app/server/lib/sendAppPage.ts @@ -24,7 +24,7 @@ export function makeGristConfig(homeUrl: string|null, extra: Partial