From 4de592839627533a3fe3191b396fc82fa7998ccd Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Wed, 27 Apr 2022 12:07:08 -0400 Subject: [PATCH] (core) when redirecting, use protocol in APP_HOME_URL if available Summary: Currently, Grist behind a reverse proxy will generate many needless redirects via `http`, and can't be used with only port 443. This diff centralizes generation of these redirects and uses the protocol in APP_HOME_URL if it is set. Test Plan: manually tested by rebuilding grist-core and doing a reverse proxy deployment that had no support for port 80. Prior to this change, there are lots of problems; after, the site works as expected. Reviewers: jarek Reviewed By: jarek Differential Revision: https://phab.getgrist.com/D3400 --- app/server/lib/Authorizer.ts | 4 ++-- app/server/lib/DiscourseConnect.ts | 5 +++-- app/server/lib/FlexServer.ts | 7 ++++--- app/server/lib/SamlConfig.ts | 3 ++- app/server/lib/extractOrg.ts | 3 ++- app/server/lib/requestUtils.ts | 23 ++++++++++++++++++----- 6 files changed, 31 insertions(+), 14 deletions(-) diff --git a/app/server/lib/Authorizer.ts b/app/server/lib/Authorizer.ts index 9ddcd7af..a6ab6c08 100644 --- a/app/server/lib/Authorizer.ts +++ b/app/server/lib/Authorizer.ts @@ -15,7 +15,7 @@ import {COOKIE_MAX_AGE, getAllowedOrgForSessionID, getCookieDomain, import {makeId} from 'app/server/lib/idUtils'; import * as log from 'app/server/lib/log'; import {IPermitStore, Permit} from 'app/server/lib/Permit'; -import {allowHost, optStringParam} from 'app/server/lib/requestUtils'; +import {allowHost, getOriginUrl, optStringParam} from 'app/server/lib/requestUtils'; import * as cookie from 'cookie'; import {NextFunction, Request, RequestHandler, Response} from 'express'; import {IncomingMessage} from 'http'; @@ -344,7 +344,7 @@ export function redirectToLoginUnconditionally( // logging out again, `users` will still be set. const signUp: boolean = (mreq.session.users === undefined); log.debug(`Authorizer: redirecting to ${signUp ? 'sign up' : 'log in'}`); - const redirectUrl = new URL(req.protocol + '://' + req.get('host') + req.originalUrl); + const redirectUrl = new URL(getOriginUrl(req) + req.originalUrl); if (signUp) { return resp.redirect(await getSignUpRedirectUrl(req, redirectUrl)); } else { diff --git a/app/server/lib/DiscourseConnect.ts b/app/server/lib/DiscourseConnect.ts index b517d41b..c07d6ff7 100644 --- a/app/server/lib/DiscourseConnect.ts +++ b/app/server/lib/DiscourseConnect.ts @@ -21,6 +21,7 @@ import type {Express, NextFunction, Request, RequestHandler, Response} from 'express'; import type {RequestWithLogin} from 'app/server/lib/Authorizer'; import {expressWrap} from 'app/server/lib/expressWrap'; +import {getOriginUrl} from 'app/server/lib/requestUtils'; import * as crypto from 'crypto'; const DISCOURSE_CONNECT_SECRET = process.env.DISCOURSE_CONNECT_SECRET; @@ -65,8 +66,8 @@ function discourseConnect(req: Request, resp: Response) { throw new Error('User is not authenticated'); } if (!req.query.user && mreq.users && mreq.users.length > 1) { - const origUrl = new URL(req.originalUrl, `${req.protocol}://${req.get('host')}`); - const redirectUrl = new URL('/welcome/select-account', `${req.protocol}://${req.get('host')}`); + const origUrl = new URL(req.originalUrl, getOriginUrl(req)); + const redirectUrl = new URL('/welcome/select-account', getOriginUrl(req)); redirectUrl.searchParams.set('next', origUrl.toString()); return resp.redirect(redirectUrl.toString()); } diff --git a/app/server/lib/FlexServer.ts b/app/server/lib/FlexServer.ts index 5a918216..dae23f2f 100644 --- a/app/server/lib/FlexServer.ts +++ b/app/server/lib/FlexServer.ts @@ -45,7 +45,8 @@ import {IPermitStore} from 'app/server/lib/Permit'; import {getAppPathTo, getAppRoot, getUnpackedAppRoot} from 'app/server/lib/places'; import {addPluginEndpoints, limitToPlugins} from 'app/server/lib/PluginEndpoint'; import {PluginManager} from 'app/server/lib/PluginManager'; -import {adaptServerUrl, addOrgToPath, addPermit, getOrgUrl, getScope, optStringParam, +import { + adaptServerUrl, addOrgToPath, addPermit, getOrgUrl, getOriginUrl, getScope, optStringParam, RequestWithGristInfo, stringParam, TEST_HTTPS_OFFSET, trustOrigin} from 'app/server/lib/requestUtils'; import {ISendAppPageOptions, makeGristConfig, makeMessagePage, makeSendAppPage} from 'app/server/lib/sendAppPage'; import {getDatabaseUrl} from 'app/server/lib/serverUtils'; @@ -1097,7 +1098,7 @@ export class FlexServer implements GristServer { const planRequired = task === 'signup' || task === 'updatePlan'; if (!BillingTask.guard(task) || (planRequired && !req.query.billingPlan)) { // If the payment task/plan are invalid, redirect to the summary page. - return resp.redirect(req.protocol + '://' + req.get('host') + `/billing`); + return resp.redirect(getOriginUrl(req) + `/billing`); } else { return this._sendAppPage(req, resp, {path: 'billing.html', status: 200, config: {}}); } @@ -1519,7 +1520,7 @@ export class FlexServer implements GristServer { private _getOrgRedirectUrl(req: RequestWithLogin, subdomain: string, pathname: string = req.originalUrl): string { const config = this.getGristConfig(); const {hostname, orgInPath} = getOrgUrlInfo(subdomain, req.get('host')!, config); - const redirectUrl = new URL(pathname, `${req.protocol}://${req.get('host')}`); + const redirectUrl = new URL(pathname, getOriginUrl(req)); if (hostname) { redirectUrl.hostname = hostname; } diff --git a/app/server/lib/SamlConfig.ts b/app/server/lib/SamlConfig.ts index cbed758a..593e1f53 100644 --- a/app/server/lib/SamlConfig.ts +++ b/app/server/lib/SamlConfig.ts @@ -61,6 +61,7 @@ import {expressWrap} from 'app/server/lib/expressWrap'; import {GristLoginSystem, GristServer} from 'app/server/lib/GristServer'; import * as log from 'app/server/lib/log'; import {Permit} from 'app/server/lib/Permit'; +import {getOriginUrl} from 'app/server/lib/requestUtils'; import {fromCallback} from 'app/server/lib/serverUtils'; import {Sessions} from 'app/server/lib/Sessions'; @@ -156,7 +157,7 @@ export class SamlConfig { // Starting point for login. It redirects to the IdP, and then to /saml/assert. app.get("/saml/login", expressWrap(async (req, res, next) => { - res.redirect(await this.getLoginRedirectUrl(req, new URL(req.protocol + "://" + req.get('host')))); + res.redirect(await this.getLoginRedirectUrl(req, new URL(getOriginUrl(req)))); })); // Assert endpoint for when the login completes as POST. diff --git a/app/server/lib/extractOrg.ts b/app/server/lib/extractOrg.ts index 9e53c8af..06ed7f57 100644 --- a/app/server/lib/extractOrg.ts +++ b/app/server/lib/extractOrg.ts @@ -3,6 +3,7 @@ import { mapGetOrSet, MapWithTTL } from 'app/common/AsyncCreate'; import { extractOrgParts, getHostType, getKnownOrg } from 'app/common/gristUrls'; import { Organization } from 'app/gen-server/entity/Organization'; import { HomeDBManager } from 'app/gen-server/lib/HomeDBManager'; +import { getOriginUrl } from 'app/server/lib/requestUtils'; import { NextFunction, Request, RequestHandler, Response } from 'express'; import { IncomingMessage } from 'http'; @@ -155,7 +156,7 @@ export class Hosts { return o && o.host || undefined; }); if (orgHost && orgHost !== req.hostname) { - const url = new URL(`${req.protocol}://${req.headers.host}${req.path}`); + const url = new URL(getOriginUrl(req) + req.path); url.hostname = orgHost; // assigning hostname rather than host preserves port. return resp.redirect(url.href); } diff --git a/app/server/lib/requestUtils.ts b/app/server/lib/requestUtils.ts index 45bf6eea..156d4cc1 100644 --- a/app/server/lib/requestUtils.ts +++ b/app/server/lib/requestUtils.ts @@ -70,7 +70,7 @@ export function addOrgToPath(req: RequestWithOrg, path: string): string { * Get url to the org associated with the request. */ export function getOrgUrl(req: Request, path: string = '/') { - return req.protocol + '://' + req.get('host') + addOrgToPathIfNeeded(req, path); + return getOriginUrl(req) + addOrgToPathIfNeeded(req, path); } /** @@ -97,8 +97,8 @@ export function trustOrigin(req: Request, resp: Response): boolean { // enough if only the base domains match. Differing ports are allowed, which helps in dev/testing. export function allowHost(req: Request, allowedHost: string|URL) { const mreq = req as RequestWithOrg; - const proto = req.protocol; - const actualUrl = new URL(`${proto}://${req.get('host')}`); + const proto = getEndUserProtocol(req); + const actualUrl = new URL(getOriginUrl(req)); const allowedUrl = (typeof allowedHost === 'string') ? new URL(`${proto}://${allowedHost}`) : allowedHost; if (mreq.isCustomHost) { // For a request to a custom domain, the full hostname must match. @@ -282,11 +282,24 @@ export interface RequestWithGristInfo extends Request { * https://docs.aws.amazon.com/elasticloadbalancing/latest/classic/x-forwarded-headers.html */ export function getOriginUrl(req: Request) { - const host = req.headers.host!; - const protocol = req.get("X-Forwarded-Proto") || req.protocol; + const host = req.get('host')!; + const protocol = getEndUserProtocol(req); return `${protocol}://${host}`; } +/** + * Get the protocol to use in Grist URLs that are intended to be reachable + * from a user's browser. Use the protocol in APP_HOME_URL if available, + * otherwise X-Forwarded-Proto is set on the provided request, otherwise + * the protocol of the request itself. + */ +export function getEndUserProtocol(req: Request) { + if (process.env.APP_HOME_URL) { + return new URL(process.env.APP_HOME_URL).protocol.replace(':', ''); + } + return req.get("X-Forwarded-Proto") || req.protocol; +} + /** * In some configurations, session information may be cached by the server. * When session information changes, give the server a chance to clear its