Introduce APP_HOME_INTERNAL_URL and fix duplicate docs

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.                                                                                                                                                                                                                                                                                                                  |

app/common/UserAPI.ts

@@ -773,11 +773,11 @@ export class UserAPIImpl extends BaseAPI implements UserAPI {
   }
 
   public async getWorker(key: string): Promise<string> {
-    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<DocWorkerAPI> {
@@ -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;
 }

app/common/gristUrls.ts

@@ -185,10 +185,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);
 }
 
 /**
@@ -208,7 +221,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';

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']) {

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
     }});

app/server/lib/Authorizer.ts

@@ -677,21 +677,25 @@ 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');
   const Organization = (req as RequestWithOrg).org;
   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<string, string> = {
     ...(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);

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 {

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) {