Skip to content

Commit

Permalink
Proposal for not requiring changing trustOrigin
Browse files Browse the repository at this point in the history
  • Loading branch information
fflorent committed Apr 2, 2024
1 parent cba4e6c commit 93be827
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 13 deletions.
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -250,9 +250,9 @@ 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.
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). You may only 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 server to reach any home workers using an internal domain name resolution (like in a docker environment). Defaults to `APP_HOME_URL`
APP_HOME_INTERNAL_URL | like `APP_HOME_URL` but used by the home server to reach any home workers using an internal domain name resolution (like in a docker environment). Also both home and doc workers need this. 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.
Expand Down
11 changes: 5 additions & 6 deletions app/common/gristUrls.ts
Original file line number Diff line number Diff line change
Expand Up @@ -192,13 +192,12 @@ export function hostMatchesUrl(host?: string, url?: string) {
*
* @param {string?} host The host to check
*/
export function isOwnInternalUrlHost(host?: string) {
if (process.env.APP_HOME_INTERNAL_URL) {
return hostMatchesUrl(host, process.env.APP_HOME_INTERNAL_URL);
} else if (process.env.APP_DOC_INTERNAL_URL) {
return hostMatchesUrl(host, process.env.APP_DOC_INTERNAL_URL);
function isOwnInternalUrlHost(host?: string) {
// Note: APP_HOME_INTERNAL_URL may also 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 false;
return Boolean(process.env.APP_DOC_INTERNAL_URL) && hostMatchesUrl(host, process.env.APP_DOC_INTERNAL_URL);
}

/**
Expand Down
4 changes: 4 additions & 0 deletions app/server/lib/Authorizer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -685,13 +685,17 @@ 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 Host = req.get('Host'); // Also pass along the original Host, as we need it since
// the destination compares that with the Origin header.

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),
...(Host ? { Host } : undefined),
};
const extraHeader = process.env.GRIST_FORWARD_AUTH_HEADER;
const extraHeaderValue = extraHeader && req.get(extraHeader);
Expand Down
6 changes: 1 addition & 5 deletions app/server/lib/requestUtils.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import {ApiError} from 'app/common/ApiError';
import {
DEFAULT_HOME_SUBDOMAIN, isOrgInPathOnly, isOwnInternalUrlHost, 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 {getUser, getUserId, RequestWithLogin} from 'app/server/lib/Authorizer';
Expand Down Expand Up @@ -89,8 +87,6 @@ export function trustOrigin(req: Request, resp: Response): boolean {
if (!origin) { return true; } // Not a CORS request.
if (process.env.GRIST_HOST && req.hostname === process.env.GRIST_HOST) { return true; }

if (isOwnInternalUrlHost(req.get('Host'))) { return true; }

if (!allowHost(req, new URL(origin))) { return false; }

// For a request to a custom domain, the full hostname must match.
Expand Down

0 comments on commit 93be827

Please sign in to comment.