diff --git a/README.md b/README.md index c950d741..4582729d 100644 --- a/README.md +++ b/README.md @@ -235,10 +235,11 @@ Grist can be configured in many ways. Here are the main environment variables it | 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. | +| 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_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). It only makes sense to define this value in the doc worker. Defaults to `APP_DOC_URL`. | | APP_HOME_URL | url prefix for home api (home and doc servers need this) | +| APP_HOME_INTERNAL_URL | like `APP_HOME_URL` but used by the home and the doc servers to reach any home workers using an internal domain name resolution (like in a docker environment). Defaults to `APP_HOME_URL` | | 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. | diff --git a/app/common/UserAPI.ts b/app/common/UserAPI.ts index 28daa159..15bc3b61 100644 --- a/app/common/UserAPI.ts +++ b/app/common/UserAPI.ts @@ -773,11 +773,11 @@ export class UserAPIImpl extends BaseAPI implements UserAPI { } public async getWorker(key: string): Promise { - const json = await this.requestJson(`${this._url}/api/worker/${key}`, { + const json = (await this.requestJson(`${this._url}/api/worker/${key}`, { method: 'GET', credentials: 'include' - }); - return getDocWorkerUrl(this._homeUrl, json); + })) as PublicDocWorkerUrlInfo; + return getPublicDocWorkerUrl(this._homeUrl, json); } public async getWorkerAPI(key: string): Promise { @@ -1156,6 +1156,27 @@ export class DocAPIImpl extends BaseAPI implements DocAPI { } } +/** + * Represents information to build public doc worker url. + * + * Structure that may contain either **exclusively**: + * - a selfPrefix when no pool of doc worker exist. + * - a public doc worker url otherwise. + */ +export type PublicDocWorkerUrlInfo = { + selfPrefix: string; + docWorkerUrl: null; +} | { + selfPrefix: null; + docWorkerUrl: string; +} + +export function getUrlFromPrefix(homeUrl: string, prefix: string) { + const url = new URL(homeUrl); + url.pathname = prefix + url.pathname; + return url.href; +} + /** * Get a docWorkerUrl from information returned from backend. When the backend * is fully configured, and there is a pool of workers, this is straightforward, @@ -1164,19 +1185,13 @@ export class DocAPIImpl extends BaseAPI implements DocAPI { * use the homeUrl of the backend, with extra path prefix information * given by selfPrefix. At the time of writing, the selfPrefix contains a * doc-worker id, and a tag for the codebase (used in consistency checks). + * + * @param {string} homeUrl + * @param {string} docWorkerInfo The information to build the public doc worker url + * (result of the call to /api/worker/:docId) */ -export function getDocWorkerUrl(homeUrl: string, docWorkerInfo: { - docWorkerUrl: string|null, - selfPrefix?: string, -}): string { - if (!docWorkerInfo.docWorkerUrl) { - if (!docWorkerInfo.selfPrefix) { - // This should never happen. - throw new Error('missing selfPrefix for docWorkerUrl'); - } - const url = new URL(homeUrl); - url.pathname = docWorkerInfo.selfPrefix + url.pathname; - return url.href; - } - return docWorkerInfo.docWorkerUrl; +export function getPublicDocWorkerUrl(homeUrl: string, docWorkerInfo: PublicDocWorkerUrlInfo) { + return docWorkerInfo.selfPrefix !== null ? + getUrlFromPrefix(homeUrl, docWorkerInfo.selfPrefix) : + docWorkerInfo.docWorkerUrl; } diff --git a/app/common/gristUrls.ts b/app/common/gristUrls.ts index 182a1ea1..431fd49e 100644 --- a/app/common/gristUrls.ts +++ b/app/common/gristUrls.ts @@ -186,10 +186,23 @@ 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; +function hostMatchesUrl(host?: string, url?: string) { + return host !== undefined && url !== undefined && new URL(url).host === host; +} + +/** + * Returns true if: + * - the server is a home worker and the host matches APP_HOME_INTERNAL_URL; + * - or the server is a doc worker and the host matches APP_DOC_INTERNAL_URL; + * + * @param {string?} host The host to check + */ +function isOwnInternalUrlHost(host?: string) { + // Note: APP_HOME_INTERNAL_URL may also be defined in doc worker as well as in home worker + if (process.env.APP_HOME_INTERNAL_URL && hostMatchesUrl(host, process.env.APP_HOME_INTERNAL_URL)) { + return true; + } + return Boolean(process.env.APP_DOC_INTERNAL_URL) && hostMatchesUrl(host, process.env.APP_DOC_INTERNAL_URL); } /** @@ -209,7 +222,11 @@ export function getHostType(host: string, options: { const hostname = host.split(":")[0]; if (!options.baseDomain) { return 'native'; } - if (hostname === 'localhost' || isDocInternalUrl(host) || hostname.endsWith(options.baseDomain)) { + if ( + hostname === 'localhost' || + isOwnInternalUrlHost(host) || + hostname.endsWith(options.baseDomain) + ) { return 'native'; } return 'custom'; diff --git a/app/gen-server/lib/DocApiForwarder.ts b/app/gen-server/lib/DocApiForwarder.ts index 4be608af..3545a63a 100644 --- a/app/gen-server/lib/DocApiForwarder.ts +++ b/app/gen-server/lib/DocApiForwarder.ts @@ -104,7 +104,11 @@ export class DocApiForwarder { url.pathname = removeTrailingSlash(docWorkerUrl.pathname) + url.pathname; const headers: {[key: string]: string} = { - ...getTransitiveHeaders(req), + // At this point, we have already checked and trusted the origin of the request. + // See FlexServer#addApiMiddleware(). So don't include the "Origin" header. + // Including this header also would break features like form submissions, + // as the "Host" header is not retrieved when calling getTransitiveHeaders(). + ...getTransitiveHeaders(req, { includeOrigin: false }), 'Content-Type': req.get('Content-Type') || 'application/json', }; for (const key of ['X-Sort', 'X-Limit']) { diff --git a/app/server/lib/AppEndpoint.ts b/app/server/lib/AppEndpoint.ts index 05fd54e1..b327a4e1 100644 --- a/app/server/lib/AppEndpoint.ts +++ b/app/server/lib/AppEndpoint.ts @@ -9,17 +9,18 @@ import {ApiError} from 'app/common/ApiError'; import {getSlugIfNeeded, parseUrlId, SHARE_KEY_PREFIX} from 'app/common/gristUrls'; import {LocalPlugin} from "app/common/plugin"; import {TELEMETRY_TEMPLATE_SIGNUP_COOKIE_NAME} from 'app/common/Telemetry'; -import {Document as APIDocument} from 'app/common/UserAPI'; +import {Document as APIDocument, PublicDocWorkerUrlInfo} from 'app/common/UserAPI'; import {Document} from "app/gen-server/entity/Document"; import {HomeDBManager} from 'app/gen-server/lib/HomeDBManager'; import {assertAccess, getTransitiveHeaders, getUserId, isAnonymousUser, RequestWithLogin} from 'app/server/lib/Authorizer'; import {DocStatus, IDocWorkerMap} from 'app/server/lib/DocWorkerMap'; -import {customizeDocWorkerUrl, getWorker, useWorkerPool} from 'app/server/lib/DocWorkerUtils'; +import { + customizeDocWorkerUrl, getDocWorkerInfoOrSelfPrefix, getWorker, useWorkerPool +} from 'app/server/lib/DocWorkerUtils'; import {expressWrap} from 'app/server/lib/expressWrap'; import {DocTemplate, GristServer} from 'app/server/lib/GristServer'; import {getCookieDomain} from 'app/server/lib/gristSessions'; -import {getAssignmentId} from 'app/server/lib/idUtils'; import log from 'app/server/lib/log'; import {addOrgToPathIfNeeded, pruneAPIResult, trustOrigin} from 'app/server/lib/requestUtils'; import {ISendAppPageOptions} from 'app/server/lib/sendAppPage'; @@ -48,32 +49,18 @@ export function attachAppEndpoint(options: AttachOptions): void { app.get('/apiconsole', expressWrap(async (req, res) => sendAppPage(req, res, {path: 'apiconsole.html', status: 200, config: {}}))); - app.get('/api/worker/:assignmentId([^/]+)/?*', expressWrap(async (req, res) => { - if (!useWorkerPool()) { - // Let the client know there is not a separate pool of workers, - // so they should continue to use the same base URL for accessing - // documents. For consistency, return a prefix to add into that - // URL, as there would be for a pool of workers. It would be nice - // to go ahead and provide the full URL, but that requires making - // more assumptions about how Grist is configured. - // Alternatives could be: have the client to send their base URL - // in the request; or use headers commonly added by reverse proxies. - const selfPrefix = "/dw/self/v/" + gristServer.getTag(); - res.json({docWorkerUrl: null, selfPrefix}); - return; - } + app.get('/api/worker/:docId([^/]+)/?*', expressWrap(async (req, res) => { if (!trustOrigin(req, res)) { throw new Error('Unrecognized origin'); } res.header("Access-Control-Allow-Credentials", "true"); - if (!docWorkerMap) { - return res.status(500).json({error: 'no worker map'}); - } - const assignmentId = getAssignmentId(docWorkerMap, req.params.assignmentId); - const {docStatus} = await getWorker(docWorkerMap, assignmentId, '/status'); - if (!docStatus) { - return res.status(500).json({error: 'no worker'}); - } - res.json({docWorkerUrl: customizeDocWorkerUrl(docStatus.docWorker.publicUrl, req)}); + const {selfPrefix, docWorker} = await getDocWorkerInfoOrSelfPrefix( + req.params.docId, docWorkerMap, gristServer.getTag() + ); + const info: PublicDocWorkerUrlInfo = selfPrefix ? + { docWorkerUrl: null, selfPrefix } : + { docWorkerUrl: customizeDocWorkerUrl(docWorker!.publicUrl, req), selfPrefix: null }; + + return res.json(info); })); // Handler for serving the document landing pages. Expects the following parameters: @@ -160,7 +147,7 @@ export function attachAppEndpoint(options: AttachOptions): void { // TODO docWorkerMain needs to serve app.html, perhaps with correct base-href already set. const headers = { Accept: 'application/json', - ...getTransitiveHeaders(req), + ...getTransitiveHeaders(req, { includeOrigin: true }), }; const workerInfo = await getWorker(docWorkerMap, docId, `/${docId}/app.html`, {headers}); docStatus = workerInfo.docStatus; @@ -206,10 +193,16 @@ export function attachAppEndpoint(options: AttachOptions): void { }); } + // Without a public URL, we're in single server mode. + // Use a null workerPublicURL, to signify that the URL prefix serving the + // current endpoint is the only one available. + const publicUrl = docStatus?.docWorker?.publicUrl; + const workerPublicUrl = publicUrl !== undefined ? customizeDocWorkerUrl(publicUrl, req) : null; + await sendAppPage(req, res, {path: "", content: body.page, tag: body.tag, status: 200, googleTagManager: 'anon', config: { assignmentId: docId, - getWorker: {[docId]: customizeDocWorkerUrl(docStatus?.docWorker?.publicUrl, req)}, + getWorker: {[docId]: workerPublicUrl }, getDoc: {[docId]: pruneAPIResult(doc as unknown as APIDocument)}, plugins }}); diff --git a/app/server/lib/Authorizer.ts b/app/server/lib/Authorizer.ts index 761e76f2..b8c85981 100644 --- a/app/server/lib/Authorizer.ts +++ b/app/server/lib/Authorizer.ts @@ -677,7 +677,10 @@ export function assertAccess( * Pull out headers to pass along to a proxied service. Focused primarily on * authentication. */ -export function getTransitiveHeaders(req: Request): {[key: string]: string} { +export function getTransitiveHeaders( + req: Request, + { includeOrigin }: { includeOrigin: boolean } +): {[key: string]: string} { const Authorization = req.get('Authorization'); const Cookie = req.get('Cookie'); const PermitHeader = req.get('Permit'); @@ -685,13 +688,14 @@ export function getTransitiveHeaders(req: Request): {[key: string]: string} { const XRequestedWith = req.get('X-Requested-With'); const Origin = req.get('Origin'); // Pass along the original Origin since it may // play a role in granular access control. + const result: Record = { ...(Authorization ? { Authorization } : undefined), ...(Cookie ? { Cookie } : undefined), ...(Organization ? { Organization } : undefined), ...(PermitHeader ? { Permit: PermitHeader } : undefined), ...(XRequestedWith ? { 'X-Requested-With': XRequestedWith } : undefined), - ...(Origin ? { Origin } : undefined), + ...((includeOrigin && Origin) ? { Origin } : undefined), }; const extraHeader = process.env.GRIST_FORWARD_AUTH_HEADER; const extraHeaderValue = extraHeader && req.get(extraHeader); diff --git a/app/server/lib/BootProbes.ts b/app/server/lib/BootProbes.ts index 7cddb99f..8edc28ae 100644 --- a/app/server/lib/BootProbes.ts +++ b/app/server/lib/BootProbes.ts @@ -77,7 +77,7 @@ const _homeUrlReachableProbe: Probe = { id: 'reachable', name: 'Grist is reachable', apply: async (server, req) => { - const url = server.getHomeUrl(req); + const url = server.getHomeInternalUrl(); try { const resp = await fetch(url); if (resp.status !== 200) { @@ -102,7 +102,7 @@ const _statusCheckProbe: Probe = { id: 'health-check', name: 'Built-in Health check', apply: async (server, req) => { - const baseUrl = server.getHomeUrl(req); + const baseUrl = server.getHomeInternalUrl(); const url = new URL(baseUrl); url.pathname = removeTrailingSlash(url.pathname) + '/status'; try { diff --git a/app/server/lib/DocApi.ts b/app/server/lib/DocApi.ts index 95d54540..223b08bd 100644 --- a/app/server/lib/DocApi.ts +++ b/app/server/lib/DocApi.ts @@ -1098,10 +1098,11 @@ export class DocWorkerApi { if (req.body.sourceDocId) { options.sourceDocId = await this._confirmDocIdForRead(req, String(req.body.sourceDocId)); // Make sure that if we wanted to download the full source, we would be allowed. - const result = await fetch(this._grist.getHomeUrl(req, `/api/docs/${options.sourceDocId}/download?dryrun=1`), { + const homeUrl = this._grist.getHomeInternalUrl(`/api/docs/${options.sourceDocId}/download?dryrun=1`); + const result = await fetch(homeUrl, { method: 'GET', headers: { - ...getTransitiveHeaders(req), + ...getTransitiveHeaders(req, { includeOrigin: false }), 'Content-Type': 'application/json', } }); @@ -1111,10 +1112,10 @@ export class DocWorkerApi { } // We should make sure the source document has flushed recently. // It may not be served by the same worker, so work through the api. - await fetch(this._grist.getHomeUrl(req, `/api/docs/${options.sourceDocId}/flush`), { + await fetch(this._grist.getHomeInternalUrl(`/api/docs/${options.sourceDocId}/flush`), { method: 'POST', headers: { - ...getTransitiveHeaders(req), + ...getTransitiveHeaders(req, { includeOrigin: false }), 'Content-Type': 'application/json', } }); @@ -1170,12 +1171,16 @@ export class DocWorkerApi { const showDetails = isAffirmative(req.query.detail); const docSession = docSessionFromRequest(req); const {states} = await this._getStates(docSession, activeDoc); - const ref = await fetch(this._grist.getHomeUrl(req, `/api/docs/${req.params.docId2}/states`), { + const ref = await fetch(this._grist.getHomeInternalUrl(`/api/docs/${req.params.docId2}/states`), { headers: { - ...getTransitiveHeaders(req), + ...getTransitiveHeaders(req, { includeOrigin: false }), 'Content-Type': 'application/json', } }); + if (!ref.ok) { + res.status(ref.status).send(await ref.text()); + return; + } const states2: DocState[] = (await ref.json()).states; const left = states[0]; const right = states2[0]; @@ -1199,9 +1204,9 @@ export class DocWorkerApi { // Calculate changes from the (common) parent to the current version of the other document. const url = `/api/docs/${req.params.docId2}/compare?left=${parent.h}`; - const rightChangesReq = await fetch(this._grist.getHomeUrl(req, url), { + const rightChangesReq = await fetch(this._grist.getHomeInternalUrl(url), { headers: { - ...getTransitiveHeaders(req), + ...getTransitiveHeaders(req, { includeOrigin: false }), 'Content-Type': 'application/json', } }); @@ -1644,7 +1649,7 @@ export class DocWorkerApi { let uploadResult; try { const accessId = makeAccessId(req, getAuthorizedUserId(req)); - uploadResult = await fetchDoc(this._grist, sourceDocumentId, req, accessId, asTemplate); + uploadResult = await fetchDoc(this._grist, this._docWorkerMap, sourceDocumentId, req, accessId, asTemplate); globalUploadSet.changeUploadName(uploadResult.uploadId, accessId, `${documentName}.grist`); } catch (err) { if ((err as ApiError).status === 403) { diff --git a/app/server/lib/DocWorkerUtils.ts b/app/server/lib/DocWorkerUtils.ts index 1396b876..060cef6f 100644 --- a/app/server/lib/DocWorkerUtils.ts +++ b/app/server/lib/DocWorkerUtils.ts @@ -1,11 +1,12 @@ import {ApiError} from 'app/common/ApiError'; import {parseSubdomainStrictly} from 'app/common/gristUrls'; import {removeTrailingSlash} from 'app/common/gutil'; -import {DocStatus, IDocWorkerMap} from 'app/server/lib/DocWorkerMap'; +import {DocStatus, DocWorkerInfo, IDocWorkerMap} from 'app/server/lib/DocWorkerMap'; import log from 'app/server/lib/log'; import {adaptServerUrl} from 'app/server/lib/requestUtils'; import * as express from 'express'; import fetch, {Response as FetchResponse, RequestInit} from 'node-fetch'; +import {getAssignmentId} from './idUtils'; /** * This method transforms a doc worker's public url as needed based on the request. @@ -35,16 +36,7 @@ import fetch, {Response as FetchResponse, RequestInit} from 'node-fetch'; * TODO: doc worker registration could be redesigned to remove the assumption * of a fixed base domain. */ -export function customizeDocWorkerUrl( - docWorkerUrlSeed: string|undefined, - req: express.Request -): string|null { - if (!docWorkerUrlSeed) { - // When no doc worker seed, we're in single server mode. - // Return null, to signify that the URL prefix serving the - // current endpoint is the only one available. - return null; - } +export function customizeDocWorkerUrl( docWorkerUrlSeed: string, req: express.Request): string { const docWorkerUrl = new URL(docWorkerUrlSeed); const workerSubdomain = parseSubdomainStrictly(docWorkerUrl.hostname).org; adaptServerUrl(docWorkerUrl, req); @@ -152,6 +144,43 @@ export async function getWorker( } } +export type DocWorkerInfoOrSelfPrefix = { + docWorker: DocWorkerInfo, + selfPrefix?: never, +} | { + docWorker?: never, + selfPrefix: string +}; + +export async function getDocWorkerInfoOrSelfPrefix( + docId: string, + docWorkerMap?: IDocWorkerMap | null, + tag?: string +): Promise { + if (!useWorkerPool()) { + // Let the client know there is not a separate pool of workers, + // so they should continue to use the same base URL for accessing + // documents. For consistency, return a prefix to add into that + // URL, as there would be for a pool of workers. It would be nice + // to go ahead and provide the full URL, but that requires making + // more assumptions about how Grist is configured. + // Alternatives could be: have the client to send their base URL + // in the request; or use headers commonly added by reverse proxies. + const selfPrefix = "/dw/self/v/" + tag; + return { selfPrefix }; + } + + if (!docWorkerMap) { + throw new Error('no worker map'); + } + const assignmentId = getAssignmentId(docWorkerMap, docId); + const { docStatus } = await getWorker(docWorkerMap, assignmentId, '/status'); + if (!docStatus) { + throw new Error('no worker'); + } + return { docWorker: docStatus.docWorker }; +} + // Return true if document related endpoints are served by separate workers. export function useWorkerPool() { return process.env.GRIST_SINGLE_PORT !== 'true'; diff --git a/app/server/lib/FlexServer.ts b/app/server/lib/FlexServer.ts index 17ac46a3..59d8a3e5 100644 --- a/app/server/lib/FlexServer.ts +++ b/app/server/lib/FlexServer.ts @@ -295,6 +295,13 @@ export class FlexServer implements GristServer { return homeUrl; } + /** + * Same as getDefaultHomeUrl, but for internal use. + */ + public getDefaultHomeInternalUrl(): string { + return process.env.APP_HOME_INTERNAL_URL || this.getDefaultHomeUrl(); + } + /** * Get a url for the home server api, adapting it to match the base domain in the * requested url. This adaptation is important for cookie-based authentication. @@ -309,6 +316,14 @@ export class FlexServer implements GristServer { return homeUrl.href; } + /** + * Same as getHomeUrl, but for requesting internally. + */ + public getHomeInternalUrl(relPath: string = ''): string { + const homeUrl = new URL(relPath, this.getDefaultHomeInternalUrl()); + return homeUrl.href; + } + /** * Get a home url that is appropriate for the given document. For now, this * returns a default that works for all documents. That could change in future, @@ -316,7 +331,7 @@ export class FlexServer implements GristServer { * based on domain). */ public async getHomeUrlByDocId(docId: string, relPath: string = ''): Promise { - return new URL(relPath, this.getDefaultHomeUrl()).href; + return new URL(relPath, this.getDefaultHomeInternalUrl()).href; } // Get the port number the server listens on. This may be different from the port @@ -1429,12 +1444,12 @@ export class FlexServer implements GristServer { return this._sendAppPage(req, resp, {path: 'app.html', status: 200, config: {}}); })); - const createDoom = async (req: express.Request) => { + const createDoom = async () => { const dbManager = this.getHomeDBManager(); const permitStore = this.getPermitStore(); const notifier = this.getNotifier(); const loginSystem = await this.resolveLoginSystem(); - const homeUrl = this.getHomeUrl(req).replace(/\/$/, ''); + const homeUrl = this.getHomeInternalUrl().replace(/\/$/, ''); return new Doom(dbManager, permitStore, notifier, loginSystem, homeUrl); }; @@ -1458,7 +1473,7 @@ export class FlexServer implements GristServer { // Reuse Doom cli tool for account deletion. It won't allow to delete account if it has access // to other (not public) team sites. - const doom = await createDoom(req); + const doom = await createDoom(); await doom.deleteUser(userId); this.getTelemetry().logEvent(req as RequestWithLogin, 'deletedAccount'); return resp.status(200).json(true); @@ -1491,7 +1506,7 @@ export class FlexServer implements GristServer { } // Reuse Doom cli tool for org deletion. Note, this removes everything as a super user. - const doom = await createDoom(req); + const doom = await createDoom(); await doom.deleteOrg(org.id); this.getTelemetry().logEvent(req as RequestWithLogin, 'deletedSite', { @@ -1980,7 +1995,7 @@ export class FlexServer implements GristServer { // Add the handling for the /upload route. Most uploads are meant for a DocWorker: they are put // in temporary files, and the DocWorker needs to be on the same machine to have access to them. // This doesn't check for doc access permissions because the request isn't tied to a document. - addUploadRoute(this, this.app, this._trustOriginsMiddleware, ...basicMiddleware); + addUploadRoute(this, this.app, this._docWorkerMap, this._trustOriginsMiddleware, ...basicMiddleware); this.app.get('/attachment', ...docAccessMiddleware, expressWrap(async (req, res) => this._docWorker.getAttachment(req, res))); @@ -2418,10 +2433,10 @@ export class FlexServer implements GristServer { const workspace = workspaces.find(w => w.name === 'Home'); if (!workspace) { throw new Error('Home workspace not found'); } - const copyDocUrl = this.getHomeUrl(req, '/api/docs'); + const copyDocUrl = this.getHomeInternalUrl('/api/docs'); const response = await fetch(copyDocUrl, { headers: { - ...getTransitiveHeaders(req), + ...getTransitiveHeaders(req, { includeOrigin: false }), 'Content-Type': 'application/json', }, method: 'POST', diff --git a/app/server/lib/GristServer.ts b/app/server/lib/GristServer.ts index 9c53f347..f40bf109 100644 --- a/app/server/lib/GristServer.ts +++ b/app/server/lib/GristServer.ts @@ -35,6 +35,7 @@ export interface GristServer { settings?: Readonly>; getHost(): string; getHomeUrl(req: express.Request, relPath?: string): string; + getHomeInternalUrl(relPath?: string): string; getHomeUrlByDocId(docId: string, relPath?: string): Promise; getOwnUrl(): string; getOrgUrl(orgKey: string|number): Promise; @@ -127,6 +128,7 @@ export function createDummyGristServer(): GristServer { settings: {}, getHost() { return 'localhost:4242'; }, getHomeUrl() { return 'http://localhost:4242'; }, + getHomeInternalUrl() { return 'http://localhost:4242'; }, getHomeUrlByDocId() { return Promise.resolve('http://localhost:4242'); }, getMergedOrgUrl() { return 'http://localhost:4242'; }, getOwnUrl() { return 'http://localhost:4242'; }, diff --git a/app/server/lib/ITestingHooks-ti.ts b/app/server/lib/ITestingHooks-ti.ts index f1454e4b..15c34f5a 100644 --- a/app/server/lib/ITestingHooks-ti.ts +++ b/app/server/lib/ITestingHooks-ti.ts @@ -11,7 +11,6 @@ export const ClientJsonMemoryLimits = t.iface([], { }); export const ITestingHooks = t.iface([], { - "getOwnPort": t.func("number"), "getPort": t.func("number"), "setLoginSessionProfile": t.func("void", t.param("gristSidCookie", "string"), t.param("profile", t.union("UserProfile", "null")), t.param("org", "string", true)), "setServerVersion": t.func("void", t.param("version", t.union("string", "null"))), diff --git a/app/server/lib/ITestingHooks.ts b/app/server/lib/ITestingHooks.ts index 3f5fa8d7..4a0f20a0 100644 --- a/app/server/lib/ITestingHooks.ts +++ b/app/server/lib/ITestingHooks.ts @@ -7,7 +7,6 @@ export interface ClientJsonMemoryLimits { } export interface ITestingHooks { - getOwnPort(): Promise; getPort(): Promise; setLoginSessionProfile(gristSidCookie: string, profile: UserProfile|null, org?: string): Promise; setServerVersion(version: string|null): Promise; diff --git a/app/server/lib/TestingHooks.ts b/app/server/lib/TestingHooks.ts index 0fb2fb8e..3c695662 100644 --- a/app/server/lib/TestingHooks.ts +++ b/app/server/lib/TestingHooks.ts @@ -68,11 +68,6 @@ export class TestingHooks implements ITestingHooks { private _workerServers: FlexServer[] ) {} - public async getOwnPort(): Promise { - log.info("TestingHooks.getOwnPort called"); - return this._server.getOwnPort(); - } - public async getPort(): Promise { log.info("TestingHooks.getPort called"); return this._port; diff --git a/app/server/lib/requestUtils.ts b/app/server/lib/requestUtils.ts index 6424d6d9..94165405 100644 --- a/app/server/lib/requestUtils.ts +++ b/app/server/lib/requestUtils.ts @@ -1,5 +1,5 @@ import {ApiError} from 'app/common/ApiError'; -import {DEFAULT_HOME_SUBDOMAIN, isOrgInPathOnly, parseSubdomain, sanitizePathTail} from 'app/common/gristUrls'; +import { DEFAULT_HOME_SUBDOMAIN, isOrgInPathOnly, parseSubdomain, sanitizePathTail } from 'app/common/gristUrls'; import * as gutil from 'app/common/gutil'; import {DocScope, QueryResult, Scope} from 'app/gen-server/lib/HomeDBManager'; import {getUserId, RequestWithLogin} from 'app/server/lib/Authorizer'; diff --git a/app/server/lib/uploads.ts b/app/server/lib/uploads.ts index f4170da2..08c35852 100644 --- a/app/server/lib/uploads.ts +++ b/app/server/lib/uploads.ts @@ -1,7 +1,7 @@ import {ApiError} from 'app/common/ApiError'; import {InactivityTimer} from 'app/common/InactivityTimer'; import {FetchUrlOptions, FileUploadResult, UPLOAD_URL_PATH, UploadResult} from 'app/common/uploads'; -import {getDocWorkerUrl} from 'app/common/UserAPI'; +import {getUrlFromPrefix} from 'app/common/UserAPI'; import {getAuthorizedUserId, getTransitiveHeaders, getUserId, isSingleUserMode, RequestWithLogin} from 'app/server/lib/Authorizer'; import {expressWrap} from 'app/server/lib/expressWrap'; @@ -21,6 +21,8 @@ import * as multiparty from 'multiparty'; import fetch, {Response as FetchResponse} from 'node-fetch'; import * as path from 'path'; import * as tmp from 'tmp'; +import {IDocWorkerMap} from './DocWorkerMap'; +import {getDocWorkerInfoOrSelfPrefix} from './DocWorkerUtils'; // After some time of inactivity, clean up the upload. We give an hour, which seems generous, // except that if one is toying with import options, and leaves the upload in an open browser idle @@ -39,7 +41,12 @@ export interface FormResult { /** * Adds an upload route to the given express app, listening for POST requests at UPLOAD_URL_PATH. */ -export function addUploadRoute(server: GristServer, expressApp: Application, ...handlers: RequestHandler[]): void { +export function addUploadRoute( + server: GristServer, + expressApp: Application, + docWorkerMap: IDocWorkerMap, + ...handlers: RequestHandler[] +): void { // When doing a cross-origin post, the browser will check for access with options prior to posting. // We need to reassure it that the request will be accepted before it will go ahead and post. @@ -72,7 +79,7 @@ export function addUploadRoute(server: GristServer, expressApp: Application, ... if (!docId) { throw new Error('doc must be specified'); } const accessId = makeAccessId(req, getAuthorizedUserId(req)); try { - const uploadResult: UploadResult = await fetchDoc(server, docId, req, accessId, + const uploadResult: UploadResult = await fetchDoc(server, docWorkerMap, docId, req, accessId, req.query.template === '1'); if (name) { globalUploadSet.changeUploadName(uploadResult.uploadId, accessId, name); @@ -404,24 +411,21 @@ async function _fetchURL(url: string, accessId: string|null, options?: FetchUrlO * Fetches a Grist doc potentially managed by a different doc worker. Passes on credentials * supplied in the current request. */ -export async function fetchDoc(server: GristServer, docId: string, req: Request, accessId: string|null, - template: boolean): Promise { +export async function fetchDoc( + server: GristServer, + docWorkerMap: IDocWorkerMap, + docId: string, + req: Request, + accessId: string|null, + template: boolean +): Promise { // Prepare headers that preserve credentials of current user. - const headers = getTransitiveHeaders(req); - - // Passing the Origin header would serve no purpose here, as we are - // constructing an internal request to fetch from our own doc worker - // URL. Indeed, it may interfere, as it could incur a CORS check in - // `trustOrigin`, which we do not need. - delete headers.Origin; + const headers = getTransitiveHeaders(req, { includeOrigin: false }); // Find the doc worker responsible for the document we wish to copy. // The backend needs to be well configured for this to work. - const homeUrl = server.getHomeUrl(req); - const fetchUrl = new URL(`/api/worker/${docId}`, homeUrl); - const response: FetchResponse = await Deps.fetch(fetchUrl.href, {headers}); - await _checkForError(response); - const docWorkerUrl = getDocWorkerUrl(server.getOwnUrl(), await response.json()); + const { selfPrefix, docWorker } = await getDocWorkerInfoOrSelfPrefix(docId, docWorkerMap, server.getTag()); + const docWorkerUrl = docWorker ? docWorker.internalUrl : getUrlFromPrefix(server.getHomeInternalUrl(), selfPrefix); // Download the document, in full or as a template. const url = new URL(`api/docs/${docId}/download?template=${Number(template)}`, docWorkerUrl.replace(/\/*$/, '/')); diff --git a/test/server/lib/DocApi.ts b/test/server/lib/DocApi.ts index 156353f3..3d9b1f85 100644 --- a/test/server/lib/DocApi.ts +++ b/test/server/lib/DocApi.ts @@ -35,19 +35,13 @@ import {serveSomething, Serving} from 'test/server/customUtil'; import {prepareDatabase} from 'test/server/lib/helpers/PrepareDatabase'; import {prepareFilesystemDirectoryForTests} from 'test/server/lib/helpers/PrepareFilesystemDirectoryForTests'; import {signal} from 'test/server/lib/helpers/Signal'; -import {TestServer} from 'test/server/lib/helpers/TestServer'; +import {TestServer, TestServerReverseProxy} from 'test/server/lib/helpers/TestServer'; import * as testUtils from 'test/server/testUtils'; import {waitForIt} from 'test/server/wait'; import defaultsDeep = require('lodash/defaultsDeep'); import pick = require('lodash/pick'); import { getDatabase } from 'test/testUtils'; -const chimpy = configForUser('Chimpy'); -const kiwi = configForUser('Kiwi'); -const charon = configForUser('Charon'); -const nobody = configForUser('Anonymous'); -const support = configForUser('support'); - // some doc ids const docIds: { [name: string]: string } = { ApiDataRecordsTest: 'sampledocid_7', @@ -68,6 +62,18 @@ let hasHomeApi: boolean; let home: TestServer; let docs: TestServer; let userApi: UserAPIImpl; +let extraHeadersForConfig = {}; + +function makeConfig(username: string): AxiosRequestConfig { + const originalConfig = configForUser(username); + return { + ...originalConfig, + headers: { + ...originalConfig.headers, + ...extraHeadersForConfig + } + }; +} describe('DocApi', function () { this.timeout(30000); @@ -77,12 +83,7 @@ describe('DocApi', function () { before(async function () { oldEnv = new testUtils.EnvironmentSnapshot(); - // Clear redis test database if redis is in use. - if (process.env.TEST_REDIS_URL) { - const cli = createClient(process.env.TEST_REDIS_URL); - await cli.flushdbAsync(); - await cli.quitAsync(); - } + await flushAllRedis(); // Create the tmp dir removing any previous one await prepareFilesystemDirectoryForTests(tmpDir); @@ -136,6 +137,7 @@ describe('DocApi', function () { }); it('should not allow anonymous users to create new docs', async () => { + const nobody = makeConfig('Anonymous'); const resp = await axios.post(`${serverUrl}/api/docs`, null, nobody); assert.equal(resp.status, 403); }); @@ -158,6 +160,95 @@ describe('DocApi', function () { testDocApi(); }); + describe('behind a reverse-proxy', function () { + async function setupServersWithProxy(suitename: string, overrideEnvConf?: NodeJS.ProcessEnv) { + const proxy = new TestServerReverseProxy(); + const additionalEnvConfiguration = { + ALLOWED_WEBHOOK_DOMAINS: `example.com,localhost:${webhooksTestPort}`, + GRIST_DATA_DIR: dataDir, + APP_HOME_URL: await proxy.getServerUrl(), + GRIST_ORG_IN_PATH: 'true', + GRIST_SINGLE_PORT: '0', + ...overrideEnvConf + }; + const home = await TestServer.startServer('home', tmpDir, suitename, additionalEnvConfiguration); + const docs = await TestServer.startServer( + 'docs', tmpDir, suitename, additionalEnvConfiguration, home.serverUrl + ); + proxy.requireFromOutsideHeader(); + + await proxy.start(home, docs); + + homeUrl = serverUrl = await proxy.getServerUrl(); + hasHomeApi = true; + extraHeadersForConfig = { + Origin: serverUrl, + ...TestServerReverseProxy.FROM_OUTSIDE_HEADER, + }; + + return {proxy, home, docs}; + } + + async function tearDown(proxy: TestServerReverseProxy, servers: TestServer[]) { + proxy.stop(); + for (const server of servers) { + await server.stop(); + } + await flushAllRedis(); + } + + let proxy: TestServerReverseProxy; + + describe('should run usual DocApi test', function () { + setup('behind-proxy-testdocs', async () => { + ({proxy, home, docs} = await setupServersWithProxy(suitename)); + }); + + after(() => tearDown(proxy, [home, docs])); + + testDocApi(); + }); + + async function testCompareDocs(proxy: TestServerReverseProxy, home: TestServer) { + const chimpy = makeConfig('chimpy'); + const userApiServerUrl = await proxy.getServerUrl(); + const chimpyApi = home.makeUserApi( + ORG_NAME, 'chimpy', { serverUrl: userApiServerUrl, headers: chimpy.headers as Record } + ); + const ws1 = (await chimpyApi.getOrgWorkspaces('current'))[0].id; + const docId1 = await chimpyApi.newDoc({name: 'testdoc1'}, ws1); + const docId2 = await chimpyApi.newDoc({name: 'testdoc2'}, ws1); + const doc1 = chimpyApi.getDocAPI(docId1); + + return doc1.compareDoc(docId2); + } + + describe('with APP_HOME_INTERNAL_URL', function () { + setup('behind-proxy-with-apphomeinternalurl', async () => { + // APP_HOME_INTERNAL_URL will be set by TestServer. + ({proxy, home, docs} = await setupServersWithProxy(suitename)); + }); + after(() => tearDown(proxy, [home, docs])); + + it('should succeed to compare docs', async function () { + const res = await testCompareDocs(proxy, home); + assert.exists(res); + }); + }); + + describe('without APP_HOME_INTERNAL_URL', function () { + setup('behind-proxy-without-apphomeinternalurl', async () => { + ({proxy, home, docs} = await setupServersWithProxy(suitename, {APP_HOME_INTERNAL_URL: ''})); + }); + after(() => tearDown(proxy, [home, docs])); + + it('should succeed to compare docs', async function () { + const promise = testCompareDocs(proxy, home); + await assert.isRejected(promise, /TestServerReverseProxy: called public URL/); + }); + }); + }); + describe("should work directly with a docworker", async () => { setup('docs', async () => { const additionalEnvConfiguration = { @@ -233,6 +324,17 @@ describe('DocApi', function () { // Contains the tests. This is where you want to add more test. function testDocApi() { + let chimpy: AxiosRequestConfig, kiwi: AxiosRequestConfig, + charon: AxiosRequestConfig, nobody: AxiosRequestConfig, support: AxiosRequestConfig; + + before(function () { + chimpy = makeConfig('Chimpy'); + kiwi = makeConfig('Kiwi'); + charon = makeConfig('Charon'); + nobody = makeConfig('Anonymous'); + support = makeConfig('support'); + }); + async function generateDocAndUrl(docName: string = "Dummy") { const wid = (await userApi.getOrgWorkspaces('current')).find((w) => w.name === 'Private')!.id; const docId = await userApi.newDoc({name: docName}, wid); @@ -1341,7 +1443,7 @@ function testDocApi() { it(`GET /docs/{did}/tables/{tid}/data supports sorts and limits in ${mode}`, async function () { function makeQuery(sort: string[] | null, limit: number | null) { const url = new URL(`${serverUrl}/api/docs/${docIds.Timesheets}/tables/Table1/data`); - const config = configForUser('chimpy'); + const config = makeConfig('chimpy'); if (mode === 'url') { if (sort) { url.searchParams.append('sort', sort.join(',')); @@ -2615,6 +2717,18 @@ function testDocApi() { await worker1.copyDoc(docId, undefined, 'copy'); }); + it("POST /docs/{did} with sourceDocId copies a document", async function () { + const chimpyWs = await userApi.newWorkspace({name: "Chimpy's Workspace"}, ORG_NAME); + const resp = await axios.post(`${serverUrl}/api/docs`, { + sourceDocumentId: docIds.TestDoc, + documentName: 'copy of TestDoc', + asTemplate: false, + workspaceId: chimpyWs + }, chimpy); + assert.equal(resp.status, 200); + assert.isString(resp.data); + }); + it("GET /docs/{did}/download/csv serves CSV-encoded document", async function () { const resp = await axios.get(`${serverUrl}/api/docs/${docIds.Timesheets}/download/csv?tableId=Table1`, chimpy); assert.equal(resp.status, 200); @@ -2801,7 +2915,7 @@ function testDocApi() { }); it('POST /workspaces/{wid}/import handles empty filenames', async function () { - if (!process.env.TEST_REDIS_URL) { + if (!process.env.TEST_REDIS_URL || docs.proxiedServer) { this.skip(); } const worker1 = await userApi.getWorkerAPI('import'); @@ -2809,7 +2923,7 @@ function testDocApi() { const fakeData1 = await testUtils.readFixtureDoc('Hello.grist'); const uploadId1 = await worker1.upload(fakeData1, '.grist'); const resp = await axios.post(`${worker1.url}/api/workspaces/${wid}/import`, {uploadId: uploadId1}, - configForUser('Chimpy')); + makeConfig('Chimpy')); assert.equal(resp.status, 200); assert.equal(resp.data.title, 'Untitled upload'); assert.equal(typeof resp.data.id, 'string'); @@ -2855,11 +2969,11 @@ function testDocApi() { }); it("document is protected during upload-and-import sequence", async function () { - if (!process.env.TEST_REDIS_URL) { + if (!process.env.TEST_REDIS_URL || home.proxiedServer) { this.skip(); } // Prepare an API for a different user. - const kiwiApi = new UserAPIImpl(`${home.serverUrl}/o/Fish`, { + const kiwiApi = new UserAPIImpl(`${homeUrl}/o/Fish`, { headers: {Authorization: 'Bearer api_key_for_kiwi'}, fetch: fetch as any, newFormData: () => new FormData() as any, @@ -2875,18 +2989,18 @@ function testDocApi() { // Check that kiwi only has access to their own upload. let wid = (await kiwiApi.getOrgWorkspaces('current')).find((w) => w.name === 'Big')!.id; let resp = await axios.post(`${worker2.url}/api/workspaces/${wid}/import`, {uploadId: uploadId1}, - configForUser('Kiwi')); + makeConfig('Kiwi')); assert.equal(resp.status, 403); assert.deepEqual(resp.data, {error: "access denied"}); resp = await axios.post(`${worker2.url}/api/workspaces/${wid}/import`, {uploadId: uploadId2}, - configForUser('Kiwi')); + makeConfig('Kiwi')); assert.equal(resp.status, 200); // Check that chimpy has access to their own upload. wid = (await userApi.getOrgWorkspaces('current')).find((w) => w.name === 'Private')!.id; resp = await axios.post(`${worker1.url}/api/workspaces/${wid}/import`, {uploadId: uploadId1}, - configForUser('Chimpy')); + makeConfig('Chimpy')); assert.equal(resp.status, 200); }); @@ -2963,10 +3077,11 @@ function testDocApi() { }); it('filters urlIds by org', async function () { + if (home.proxiedServer) { this.skip(); } // Make two documents with same urlId const ws1 = (await userApi.getOrgWorkspaces('current'))[0].id; const doc1 = await userApi.newDoc({name: 'testdoc1', urlId: 'urlid'}, ws1); - const nasaApi = new UserAPIImpl(`${home.serverUrl}/o/nasa`, { + const nasaApi = new UserAPIImpl(`${homeUrl}/o/nasa`, { headers: {Authorization: 'Bearer api_key_for_chimpy'}, fetch: fetch as any, newFormData: () => new FormData() as any, @@ -2995,9 +3110,10 @@ function testDocApi() { it('allows docId access to any document from merged org', async function () { // Make two documents + if (home.proxiedServer) { this.skip(); } const ws1 = (await userApi.getOrgWorkspaces('current'))[0].id; const doc1 = await userApi.newDoc({name: 'testdoc1'}, ws1); - const nasaApi = new UserAPIImpl(`${home.serverUrl}/o/nasa`, { + const nasaApi = new UserAPIImpl(`${homeUrl}/o/nasa`, { headers: {Authorization: 'Bearer api_key_for_chimpy'}, fetch: fetch as any, newFormData: () => new FormData() as any, @@ -3125,11 +3241,17 @@ function testDocApi() { }); it("GET /docs/{did1}/compare/{did2} tracks changes between docs", async function () { - const ws1 = (await userApi.getOrgWorkspaces('current'))[0].id; - const docId1 = await userApi.newDoc({name: 'testdoc1'}, ws1); - const docId2 = await userApi.newDoc({name: 'testdoc2'}, ws1); - const doc1 = userApi.getDocAPI(docId1); - const doc2 = userApi.getDocAPI(docId2); + // Pass kiwi's headers as it contains both Authorization and Origin headers + // if run behind a proxy, so we can ensure that the Origin header check is not made. + const userApiServerUrl = docs.proxiedServer ? serverUrl : undefined; + const chimpyApi = home.makeUserApi( + ORG_NAME, 'chimpy', { serverUrl: userApiServerUrl, headers: chimpy.headers as Record } + ); + const ws1 = (await chimpyApi.getOrgWorkspaces('current'))[0].id; + const docId1 = await chimpyApi.newDoc({name: 'testdoc1'}, ws1); + const docId2 = await chimpyApi.newDoc({name: 'testdoc2'}, ws1); + const doc1 = chimpyApi.getDocAPI(docId1); + const doc2 = chimpyApi.getDocAPI(docId2); // Stick some content in column A so it has a defined type // so diffs are smaller and simpler. @@ -3327,6 +3449,9 @@ function testDocApi() { }); it('doc worker endpoints ignore any /dw/.../ prefix', async function () { + if (docs.proxiedServer) { + this.skip(); + } const docWorkerUrl = docs.serverUrl; let resp = await axios.get(`${docWorkerUrl}/api/docs/${docIds.Timesheets}/tables/Table1/data`, chimpy); assert.equal(resp.status, 200); @@ -4974,18 +5099,26 @@ function testDocApi() { } }); - const chimpyConfig = configForUser("Chimpy"); - const anonConfig = configForUser("Anonymous"); + const chimpyConfig = makeConfig("Chimpy"); + const anonConfig = makeConfig("Anonymous"); delete chimpyConfig.headers!["X-Requested-With"]; delete anonConfig.headers!["X-Requested-With"]; + let allowedOrigin; + // Target a more realistic Host than "localhost:port" - anonConfig.headers!.Host = chimpyConfig.headers!.Host = 'api.example.com'; + // (if behind a proxy, we already benefit from a custom and realistic host). + if (!home.proxiedServer) { + anonConfig.headers!.Host = chimpyConfig.headers!.Host = + 'api.example.com'; + allowedOrigin = 'http://front.example.com'; + } else { + allowedOrigin = serverUrl; + } const url = `${serverUrl}/api/docs/${docId}/tables/Table1/records`; const data = { records: [{ fields: {} }] }; - const allowedOrigin = 'http://front.example.com'; const forbiddenOrigin = 'http://evil.com'; // Normal same origin requests @@ -5217,6 +5350,7 @@ function setup(name: string, cb: () => Promise) { before(async function () { suitename = name; dataDir = path.join(tmpDir, `${suitename}-data`); + await flushAllRedis(); await fse.mkdirs(dataDir); await setupDataDir(dataDir); await cb(); @@ -5235,6 +5369,7 @@ function setup(name: string, cb: () => Promise) { // stop all servers await home.stop(); await docs.stop(); + extraHeadersForConfig = {}; }); } @@ -5263,3 +5398,12 @@ async function flushAuth() { await home.testingHooks.flushAuthorizerCache(); await docs.testingHooks.flushAuthorizerCache(); } + +async function flushAllRedis() { + // Clear redis test database if redis is in use. + if (process.env.TEST_REDIS_URL) { + const cli = createClient(process.env.TEST_REDIS_URL); + await cli.flushdbAsync(); + await cli.quitAsync(); + } +} diff --git a/test/server/lib/helpers/TestProxyServer.ts b/test/server/lib/helpers/TestProxyServer.ts index d5d171dc..e255d5e3 100644 --- a/test/server/lib/helpers/TestProxyServer.ts +++ b/test/server/lib/helpers/TestProxyServer.ts @@ -7,7 +7,6 @@ export class TestProxyServer { const server = new TestProxyServer(); await server._prepare(portNumber); return server; - } private _proxyCallsCounter: number = 0; @@ -38,7 +37,6 @@ export class TestProxyServer { } res.sendStatus(responseCode); res.end(); - //next(); }); }, portNumber); } diff --git a/test/server/lib/helpers/TestServer.ts b/test/server/lib/helpers/TestServer.ts index 51a5d39f..95946859 100644 --- a/test/server/lib/helpers/TestServer.ts +++ b/test/server/lib/helpers/TestServer.ts @@ -1,15 +1,20 @@ import {connectTestingHooks, TestingHooksClient} from "app/server/lib/TestingHooks"; import {ChildProcess, execFileSync, spawn} from "child_process"; +import * as http from "http"; import FormData from 'form-data'; import path from "path"; import * as fse from "fs-extra"; import * as testUtils from "test/server/testUtils"; import {UserAPIImpl} from "app/common/UserAPI"; -import {exitPromise} from "app/server/lib/serverUtils"; +import {exitPromise, getAvailablePort} from "app/server/lib/serverUtils"; import log from "app/server/lib/log"; import {delay} from "bluebird"; import fetch from "node-fetch"; import {Writable} from "stream"; +import express from "express"; +import { AddressInfo } from "net"; +import { isAffirmative } from "app/common/gutil"; +import httpProxy from 'http-proxy'; /** * This starts a server in a separate process. @@ -24,18 +29,26 @@ export class TestServer { options: {output?: Writable} = {}, // Pipe server output to the given stream ): Promise { - const server = new TestServer(serverTypes, tempDirectory, suitename); + const server = new this(serverTypes, tempDirectory, suitename); await server.start(_homeUrl, customEnv, options); return server; } public testingSocket: string; public testingHooks: TestingHooksClient; - public serverUrl: string; public stopped = false; + public get serverUrl() { + if (this._proxiedServer) { + throw new Error('Direct access to this test server is disallowed'); + } + return this._serverUrl; + } + public get proxiedServer() { return this._proxiedServer; } + private _serverUrl: string; private _server: ChildProcess; private _exitPromise: Promise; + private _proxiedServer: boolean = false; private readonly _defaultEnv; @@ -44,9 +57,6 @@ export class TestServer { GRIST_INST_DIR: this.rootDir, GRIST_DATA_DIR: path.join(this.rootDir, "data"), GRIST_SERVERS: this._serverTypes, - // with port '0' no need to hard code a port number (we can use testing hooks to find out what - // port server is listening on). - GRIST_PORT: '0', GRIST_DISABLE_S3: 'true', REDIS_URL: process.env.TEST_REDIS_URL, GRIST_TRIGGER_WAIT_DELAY: '100', @@ -68,9 +78,16 @@ export class TestServer { // Unix socket paths typically can't be longer than this. Who knew. Make the error obvious. throw new Error(`Path of testingSocket too long: ${this.testingSocket.length} (${this.testingSocket})`); } - const env = { - APP_HOME_URL: _homeUrl, + + const port = await getAvailablePort(); + this._serverUrl = `http://localhost:${port}`; + const homeUrl = _homeUrl ?? (this._serverTypes.includes('home') ? this._serverUrl : undefined); + + const env: NodeJS.ProcessEnv = { + APP_HOME_URL: homeUrl, + APP_HOME_INTERNAL_URL: homeUrl, GRIST_TESTING_SOCKET: this.testingSocket, + GRIST_PORT: String(port), ...this._defaultEnv, ...customEnv }; @@ -98,7 +115,7 @@ export class TestServer { .catch(() => undefined); await this._waitServerReady(); - log.info(`server ${this._serverTypes} up and listening on ${this.serverUrl}`); + log.info(`server ${this._serverTypes} up and listening on ${this._serverUrl}`); } public async stop() { @@ -125,11 +142,9 @@ export class TestServer { // create testing hooks and get own port this.testingHooks = await connectTestingHooks(this.testingSocket); - const port: number = await this.testingHooks.getOwnPort(); - this.serverUrl = `http://localhost:${port}`; // wait for check - return (await fetch(`${this.serverUrl}/status/hooks`, {timeout: 1000})).ok; + return (await fetch(`${this._serverUrl}/status/hooks`, {timeout: 1000})).ok; } catch (err) { log.warn("Failed to initialize server", err); return false; @@ -142,14 +157,32 @@ export class TestServer { // Returns the promise for the ChildProcess's signal or exit code. public getExitPromise(): Promise { return this._exitPromise; } - public makeUserApi(org: string, user: string = 'chimpy'): UserAPIImpl { - return new UserAPIImpl(`${this.serverUrl}/o/${org}`, { - headers: {Authorization: `Bearer api_key_for_${user}`}, + public makeUserApi( + org: string, + user: string = 'chimpy', + { + headers = {Authorization: `Bearer api_key_for_${user}`}, + serverUrl = this._serverUrl, + }: { + headers?: Record + serverUrl?: string, + } = { headers: undefined, serverUrl: undefined }, + ): UserAPIImpl { + return new UserAPIImpl(`${serverUrl}/o/${org}`, { + headers, fetch: fetch as unknown as typeof globalThis.fetch, newFormData: () => new FormData() as any, }); } + /** + * Assuming that the server is behind a reverse-proxy (like TestServerReverseProxy), + * disallow access to the serverUrl to prevent the tests to join the server directly. + */ + public disallowDirectAccess() { + this._proxiedServer = true; + } + private async _waitServerReady() { // It's important to clear the timeout, because it can prevent node from exiting otherwise, // which is annoying when running only this test for debugging. @@ -170,3 +203,103 @@ export class TestServer { } } } + +/** + * Creates a reverse-proxy for a home and a doc worker. + * + * The workers are then disallowed to be joined directly, the tests are assumed to + * pass through this reverse-proxy. + * + * You may use it like follow: + * ```ts + * const proxy = new TestServerReverseProxy(); + * // Create here a doc and a home workers with their env variables + * proxy.requireFromOutsideHeader(); // Optional + * await proxy.start(home, docs); + * ``` + */ +export class TestServerReverseProxy { + + // Use a different hostname for the proxy than the doc and home workers' + // so we can ensure that either we omit the Origin header (so the internal calls to home and doc workers + // are not considered as CORS requests), or otherwise we fail because the hostnames are different + // https://github.com/gristlabs/grist-core/blob/24b39c651b9590cc360cc91b587d3e1b301a9c63/app/server/lib/requestUtils.ts#L85-L98 + public static readonly HOSTNAME: string = 'grist-test-proxy.127.0.0.1.nip.io'; + + public static FROM_OUTSIDE_HEADER = {"X-FROM-OUTSIDE": true}; + + private _app = express(); + private _proxyServer: http.Server; + private _proxy: httpProxy = httpProxy.createProxy(); + private _address: Promise; + private _requireFromOutsideHeader = false; + + public get stopped() { return !this._proxyServer.listening; } + + public constructor() { + this._proxyServer = this._app.listen(0); + this._address = new Promise((resolve) => { + this._proxyServer.once('listening', () => { + resolve(this._proxyServer.address() as AddressInfo); + }); + }); + } + + /** + * Require the reverse-proxy to be called from the outside world. + * This assumes that every requests to the proxy includes the header + * provided in TestServerReverseProxy.FROM_OUTSIDE_HEADER + * + * If a call is done by a worker (assuming they don't include that header), + * the proxy rejects with a FORBIDEN http status. + */ + public requireFromOutsideHeader() { + this._requireFromOutsideHeader = true; + } + + public async start(homeServer: TestServer, docServer: TestServer) { + this._app.all(['/dw/dw1', '/dw/dw1/*'], (oreq, ores) => this._getRequestHandlerFor(docServer)); + this._app.all('/*', this._getRequestHandlerFor(homeServer)); + + // Forbid now the use of serverUrl property, so we don't allow the tests to + // call the workers directly + homeServer.disallowDirectAccess(); + docServer.disallowDirectAccess(); + + log.info('proxy server running on ', await this.getServerUrl()); + } + + public async getAddress() { + return this._address; + } + + public async getServerUrl() { + const address = await this.getAddress(); + return `http://${TestServerReverseProxy.HOSTNAME}:${address.port}`; + } + + public stop() { + if (this.stopped) { + return; + } + log.info("Stopping node TestServerReverseProxy"); + this._proxyServer.close(); + this._proxy.close(); + } + + private _getRequestHandlerFor(server: TestServer) { + const serverUrl = new URL(server.serverUrl); + + return (oreq: express.Request, ores: express.Response) => { + log.debug(`[proxy] Requesting (method=${oreq.method}): ${new URL(oreq.url, serverUrl).href}`); + + // See the requireFromOutsideHeader() method for the explanation + if (this._requireFromOutsideHeader && !isAffirmative(oreq.get("X-FROM-OUTSIDE"))) { + log.error('TestServerReverseProxy: called public URL from internal'); + return ores.status(403).json({error: "TestServerReverseProxy: called public URL from internal "}); + } + + this._proxy.web(oreq, ores, { target: serverUrl }); + }; + } +}