diff --git a/README.md b/README.md index 2beeea35..296d7d0c 100644 --- a/README.md +++ b/README.md @@ -280,7 +280,7 @@ GRIST_MAX_UPLOAD_IMPORT_MB | max allowed size for imports (except .grist files) GRIST_OFFER_ALL_LANGUAGES | if set, all translated langauages are offered to the user (by default, only languages with a special 'good enough' key set are offered to user). GRIST_ORG_IN_PATH | if true, encode org in path rather than domain GRIST_PAGE_TITLE_SUFFIX | a string to append to the end of the `` in HTML documents. Defaults to `" - Grist"`. Set to `_blank` for no suffix at all. -GRIST_PROXY_AUTH_HEADER | header which will be set by a (reverse) proxy webserver with an authorized users' email. This can be used as an alternative to a SAML service. See also GRIST_FORWARD_AUTH_HEADER. +~GRIST_PROXY_AUTH_HEADER~ | Deprecated, and interpreted as a synonym for GRIST_FORWARD_AUTH_HEADER. GRIST_ROUTER_URL | optional url for an api that allows servers to be (un)registered with a load balancer GRIST_SERVE_SAME_ORIGIN | set to "true" to access home server and doc workers on the same protocol-host-port as the top-level page, same as for custom domains (careful, host header should be trustworthy) GRIST_SERVERS | the types of server to setup. Comma separated values which may contain "home", "docs", static" and/or "app". Defaults to "home,docs,static". @@ -339,16 +339,34 @@ GRIST_FORWARD_AUTH_HEADER | if set, trust the specified header (e.g. "x-forwarde GRIST_FORWARD_AUTH_LOGIN_PATH | if GRIST_FORWARD_AUTH_HEADER is set, Grist will listen at this path for logins. Defaults to `/auth/login`. GRIST_FORWARD_AUTH_LOGOUT_PATH | if GRIST_FORWARD_AUTH_HEADER is set, Grist will forward to this path when user logs out. -When using forward authentication, you may wish to also set the following variables: +Forward authentication supports two modes, distinguished by `GRIST_IGNORE_SESSION`: + +1. With sessions, and forward-auth on login endpoints. + + For example, using traefik reverse proxy with + [traefik-forward-auth](https://github.com/thomseddon/traefik-forward-auth) middleware: + + - `GRIST_IGNORE_SESSION`: do NOT set, or set to a falsy value. + - Make sure your reverse proxy applies the forward auth middleware to + `GRIST_FORWARD_AUTH_LOGIN_PATH` and `GRIST_FORWARD_AUTH_LOGOUT_PATH`. + - If you want to allow anonymous access in some cases, make sure all other paths are free of + the forward auth middleware. Grist will trigger it as needed by redirecting to + `GRIST_FORWARD_AUTH_LOGIN_PATH`. Once the user is logged in, Grist will use sessions to + identify the user until logout. - * GRIST_FORCE_LOGIN=true to disable anonymous access. - * GRIST_IGNORE_SESSION=true to ignore any user identity information in a cookie. - Only do this if you use forward authentication on all paths. - You may not want to use forward authentication on all paths if it makes - signing in required, and you are trying to permit anonymous access. +2. With no sessions, and forward-auth on all endpoints. + + For example, using HTTP Basic Auth and server configuration that sets the header (specified in + `GRIST_FORWARD_AUTH_HEADER`) to the logged-in user. + + - `GRIST_IGNORE_SESSION`: set to `true`. Grist sessions will not be used. + - Make sure your reverse proxy sets the header you specified for all requests that may need + login information. It is imperative that this header cannot be spoofed by the user, since + Grist will trust whatever is in it. + +When using forward authentication, you may wish to also set the following variables: -GRIST_FORWARD_AUTH_HEADER is similar to GRIST_PROXY_AUTH_HEADER, but enables -a login system (assuming you have some forward authentication set up). + * `GRIST_FORCE_LOGIN=true` to disable anonymous access. #### Plugins: diff --git a/app/server/lib/Authorizer.ts b/app/server/lib/Authorizer.ts index f13f5657..c1d53361 100644 --- a/app/server/lib/Authorizer.ts +++ b/app/server/lib/Authorizer.ts @@ -96,45 +96,39 @@ export function isSingleUserMode(): boolean { return process.env.GRIST_SINGLE_USER === '1'; } - /** * Returns a profile if it can be deduced from the request. This requires a - * header to specify the users' email address. The header to set comes from the - * environment variable GRIST_PROXY_AUTH_HEADER, or may be passed in. + * header to specify the users' email address. * A result of null means that the user should be considered known to be anonymous. * A result of undefined means we should go on to consider other authentication * methods (such as cookies). */ export function getRequestProfile(req: Request|IncomingMessage, - header?: string): UserProfile|null|undefined { - header = header || process.env.GRIST_PROXY_AUTH_HEADER; + header: string): UserProfile|null|undefined { let profile: UserProfile|null|undefined; - if (header) { - // Careful reading headers. If we have an IncomingMessage, there is no - // get() function, and header names are lowercased. - const headerContent = ('get' in req) ? req.get(header) : req.headers[header.toLowerCase()]; - if (headerContent) { - const userEmail = headerContent.toString(); - const [userName] = userEmail.split("@", 1); - if (userEmail && userName) { - profile = { - "email": userEmail, - "name": userName - }; - } - } - // If no profile at this point, and header was present, - // treat as anonymous user, represented by null value. - // Don't go on to look at session. - if (!profile && headerContent !== undefined) { - profile = null; + // Careful reading headers. If we have an IncomingMessage, there is no + // get() function, and header names are lowercased. + const headerContent = ('get' in req) ? req.get(header) : req.headers[header.toLowerCase()]; + if (headerContent) { + const userEmail = headerContent.toString(); + const [userName] = userEmail.split("@", 1); + if (userEmail && userName) { + profile = { + "email": userEmail, + "name": userName + }; } } + // If no profile at this point, and header was present, + // treat as anonymous user, represented by null value. + // Don't go on to look at session. + if (!profile && headerContent !== undefined) { + profile = null; + } return profile; } - /** * Returns the express request object with user information added, if it can be * found based on passed in headers or the session. Specifically, sets: @@ -144,13 +138,15 @@ export function getRequestProfile(req: Request|IncomingMessage, * as would typically be the case, credentials were not presented) * - req.users: set for org-and-session-based logins, with list of profiles in session */ -export async function addRequestUser(dbManager: HomeDBManager, permitStore: IPermitStore, - options: { - gristServer: GristServer, - skipSession?: boolean, - getProfile?(req: Request|IncomingMessage): Promise<UserProfile|null|undefined>, - }, - req: Request, res: Response, next: NextFunction) { +export async function addRequestUser( + dbManager: HomeDBManager, permitStore: IPermitStore, + options: { + gristServer: GristServer, + skipSession?: boolean, + overrideProfile?(req: Request|IncomingMessage): Promise<UserProfile|null|undefined>, + }, + req: Request, res: Response, next: NextFunction +) { const mreq = req as RequestWithLogin; let profile: UserProfile|undefined; @@ -236,21 +232,20 @@ export async function addRequestUser(dbManager: HomeDBManager, permitStore: IPer // If this is the case, we won't use session information. let skipSession: boolean = options.skipSession || authDone; if (!authDone && !mreq.userId) { - let candidate = await options.getProfile?.(mreq); - if (candidate === undefined) { - candidate = getRequestProfile(mreq); - } - if (candidate !== undefined) { + const candidateProfile = await options.overrideProfile?.(mreq); + if (candidateProfile !== undefined) { + // Either a valid or a null profile tells us that another login system determined the user, + // and that we should skip sessions. skipSession = true; - } - if (candidate) { - profile = candidate; - const user = await dbManager.getUserByLoginWithRetry(profile.email, {profile}); - if (user) { - mreq.user = user; - mreq.users = [profile]; - mreq.userId = user.id; - mreq.userIsAuthorized = true; + if (candidateProfile) { + profile = candidateProfile; + const user = await dbManager.getUserByLoginWithRetry(profile.email, {profile}); + if (user) { + mreq.user = user; + mreq.users = [profile]; + mreq.userId = user.id; + mreq.userIsAuthorized = true; + } } } } @@ -698,7 +693,7 @@ export function getTransitiveHeaders(req: Request): {[key: string]: string} { ...(XRequestedWith ? { 'X-Requested-With': XRequestedWith } : undefined), ...(Origin ? { Origin } : undefined), }; - const extraHeader = process.env.GRIST_PROXY_AUTH_HEADER; + const extraHeader = process.env.GRIST_FORWARD_AUTH_HEADER; const extraHeaderValue = extraHeader && req.get(extraHeader); if (extraHeader && extraHeaderValue) { result[extraHeader] = extraHeaderValue; diff --git a/app/server/lib/Comm.ts b/app/server/lib/Comm.ts index 4f8331c5..e9a3838d 100644 --- a/app/server/lib/Comm.ts +++ b/app/server/lib/Comm.ts @@ -42,7 +42,6 @@ import {parseFirstUrlPart} from 'app/common/gristUrls'; import {firstDefined, safeJsonParse} from 'app/common/gutil'; import {UserProfile} from 'app/common/LoginSessionAPI'; import * as version from 'app/common/version'; -import {getRequestProfile} from 'app/server/lib/Authorizer'; import {ScopedSession} from "app/server/lib/BrowserSession"; import {Client, ClientMethod} from "app/server/lib/Client"; import {Hosts, RequestWithOrg} from 'app/server/lib/extractOrg'; @@ -198,8 +197,7 @@ export class Comm extends EventEmitter { */ private async _getSessionProfile(scopedSession: ScopedSession, req: http.IncomingMessage): Promise<UserProfile|null> { return await firstDefined( - async () => this._options.loginMiddleware?.getProfile?.(req), - async () => getRequestProfile(req), + async () => this._options.loginMiddleware?.overrideProfile?.(req), async () => scopedSession.getSessionProfile(), ) || null; } diff --git a/app/server/lib/FlexServer.ts b/app/server/lib/FlexServer.ts index 0e937fe5..fdc25c7b 100644 --- a/app/server/lib/FlexServer.ts +++ b/app/server/lib/FlexServer.ts @@ -671,7 +671,7 @@ export class FlexServer implements GristServer { this._userIdMiddleware = expressWrap(addRequestUser.bind( null, this._dbManager, this._internalPermitStore, { - getProfile: this._loginMiddleware.getProfile?.bind(this._loginMiddleware), + overrideProfile: this._loginMiddleware.overrideProfile?.bind(this._loginMiddleware), // Set this to false to stop Grist using a cookie for authentication purposes. skipSession, gristServer: this, diff --git a/app/server/lib/ForwardAuthLogin.ts b/app/server/lib/ForwardAuthLogin.ts index d0912126..7095fc17 100644 --- a/app/server/lib/ForwardAuthLogin.ts +++ b/app/server/lib/ForwardAuthLogin.ts @@ -1,43 +1,78 @@ import { ApiError } from 'app/common/ApiError'; -import { UserProfile } from 'app/common/LoginSessionAPI'; import { appSettings } from 'app/server/lib/AppSettings'; import { getRequestProfile } from 'app/server/lib/Authorizer'; import { expressWrap } from 'app/server/lib/expressWrap'; -import { GristLoginSystem, GristServer, setUserInSession } from 'app/server/lib/GristServer'; +import { GristLoginMiddleware, GristLoginSystem, GristServer, setUserInSession } from 'app/server/lib/GristServer'; +import log from 'app/server/lib/log'; import { optStringParam } from 'app/server/lib/requestUtils'; import * as express from 'express'; -import { IncomingMessage } from 'http'; import trimEnd = require('lodash/trimEnd'); import trimStart = require('lodash/trimStart'); /** * Return a login system that can work in concert with middleware that - * does authentication and then passes identity in a header. An example - * of such middleware is traefik-forward-auth: + * does authentication and then passes identity in a header. + * There are two modes of operation, distinguished by whether GRIST_IGNORE_SESSION is set. * - * https://github.com/thomseddon/traefik-forward-auth + * 1. With sessions, and forward-auth on login endpoints. * - * To make it function: - * - Set GRIST_FORWARD_AUTH_HEADER to a header that will contain - * authorized user emails, say "x-forwarded-user" - * - Make sure /auth/login is processed by forward auth middleware - * - Set GRIST_FORWARD_AUTH_LOGOUT_PATH to a path that will trigger - * a logout (for traefik-forward-auth by default that is /_oauth/logout). - * - Make sure that logout path is processed by forward auth middleware - * - If you want to allow anonymous access in some cases, make sure all - * other paths are free of the forward auth middleware - Grist will - * trigger it as needed. - * - Optionally, tell the middleware where to forward back to after logout. - * (For traefik-forward-auth, you'd set LOGOUT_REDIRECT to .../signed-out) + * For example, using traefik reverse proxy with traefik-forward-auth middleware: + * + * https://github.com/thomseddon/traefik-forward-auth + * + * Grist environment: + * - GRIST_FORWARD_AUTH_HEADER: set to a header that will contain + * authorized user emails, say "x-forwarded-user" + * - GRIST_FORWARD_AUTH_LOGOUT_PATH: set to a path that will trigger + * a logout (for traefik-forward-auth by default that is /_oauth/logout). + * - GRIST_FORWARD_AUTH_LOGIN_PATH: optionally set to override the default (/auth/login). + * - GRIST_IGNORE_SESSION: do NOT set, or set to a falsy value. + * + * Reverse proxy: + * - Make sure your reverse proxy applies the forward auth middleware to + * GRIST_FORWARD_AUTH_LOGIN_PATH and GRIST_FORWARD_AUTH_LOGOUT_PATH. + * - If you want to allow anonymous access in some cases, make sure all + * other paths are free of the forward auth middleware - Grist will + * trigger it as needed by redirecting to /auth/login. + * - Grist only uses the configured header at login/logout. Once the user is logged in, Grist + * will use the session info to identify the user, until logout. + * - Optionally, tell the middleware where to forward back to after logout. + * (For traefik-forward-auth, you'd run it with LOGOUT_REDIRECT set to .../signed-out) + * + * 2. With no sessions, and forward-auth on all endpoints. + * + * For example, using HTTP Basic Auth and server configuration that sets a header to the + * logged-in user (e.g. to REMOTE_USER with Apache). + * + * Grist environment: + * - GRIST_IGNORE_SESSION: set to true. Grist sessions will not be used. + * - GRIST_FORWARD_AUTH_HEADER: set to to a header that will contain authorized user emails, say + * "x-remote-user". + * + * Reverse proxy: + * - Make sure your reverse proxy sets the header you specified for all requests that may need + * login information. It is imperative that this header cannot be spoofed by the user, since + * Grist will trust whatever is in it. + * + * GRIST_PROXY_AUTH_HEADER is deprecated in favor of GRIST_FORWARD_AUTH_HEADER. It is currently + * interpreted as a synonym, with a warning, but support for it may be dropped. * * Redirection logic currently assumes a single-site installation. */ export async function getForwardAuthLoginSystem(): Promise<GristLoginSystem|undefined> { const section = appSettings.section('login').section('system').section('forwardAuth'); - const header = section.flag('header').readString({ - envVar: 'GRIST_FORWARD_AUTH_HEADER', + const headerSetting = section.flag('header'); + const header = headerSetting.readString({ + envVar: ['GRIST_FORWARD_AUTH_HEADER', 'GRIST_PROXY_AUTH_HEADER'] }); - if (!header) { return; } + if (!header) { + return; + } + + if (headerSetting.describe().foundInEnvVar === 'GRIST_PROXY_AUTH_HEADER') { + log.warn("GRIST_PROXY_AUTH_HEADER is deprecated; interpreted as a synonym of GRIST_FORWARD_AUTH_HEADER"); + } + section.flag('active').set(true); const logoutPath = section.flag('logoutPath').readString({ envVar: 'GRIST_FORWARD_AUTH_LOGOUT_PATH' @@ -45,18 +80,23 @@ export async function getForwardAuthLoginSystem(): Promise<GristLoginSystem|unde const loginPath = section.flag('loginPath').requireString({ envVar: 'GRIST_FORWARD_AUTH_LOGIN_PATH', defaultValue: '/auth/login', - }) || ''; + }); + + const skipSession = appSettings.section('login').flag('skipSession').readBool({ + envVar: 'GRIST_IGNORE_SESSION', + }); + return { async getMiddleware(gristServer: GristServer) { async function getLoginRedirectUrl(req: express.Request, url: URL) { const target = new URL(trimEnd(gristServer.getHomeUrl(req), '/') + '/' + trimStart(loginPath, '/')); - // In lieu of sanatizing the next url, we include only the path + // In lieu of sanitizing the next url, we include only the path // component. This will only work for single-domain installations. target.searchParams.append('next', url.pathname); return target.href; } - return { + const middleware: GristLoginMiddleware = { getLoginRedirectUrl, getSignUpRedirectUrl: getLoginRedirectUrl, async getLogoutRedirectUrl(req: express.Request) { @@ -64,7 +104,7 @@ export async function getForwardAuthLoginSystem(): Promise<GristLoginSystem|unde trimStart(logoutPath, '/'); }, async addEndpoints(app: express.Express) { - app.get('/auth/login', expressWrap(async (req, res) => { + app.get(loginPath, expressWrap(async (req, res) => { const profile = getRequestProfile(req, header); if (!profile) { throw new ApiError('cannot find user', 401); @@ -77,12 +117,14 @@ export async function getForwardAuthLoginSystem(): Promise<GristLoginSystem|unde } res.redirect(target.href); })); - return "forward-auth"; - }, - async getProfile(req: express.Request|IncomingMessage): Promise<UserProfile|null|undefined> { - return getRequestProfile(req, header); + return skipSession ? "forward-auth-skip-session" : "forward-auth"; }, }; + if (skipSession) { + // With GRIST_IGNORE_SESSION, respect the header for all requests. + middleware.overrideProfile = async (req) => getRequestProfile(req, header); + } + return middleware; }, async deleteUser() { // If we could delete the user account in the external diff --git a/app/server/lib/GristServer.ts b/app/server/lib/GristServer.ts index 65e3cade..7c3e492e 100644 --- a/app/server/lib/GristServer.ts +++ b/app/server/lib/GristServer.ts @@ -79,10 +79,11 @@ export interface GristLoginMiddleware { getWildcardMiddleware?(): express.RequestHandler[]; // Returns arbitrary string for log. addEndpoints(app: express.Express): Promise<string>; - // Optionally, extract profile from request. Result can be a profile, - // or null if anonymous (and other methods of determining profile such - // as a cookie should not be used), or undefined to use other methods. - getProfile?(req: express.Request|IncomingMessage): Promise<UserProfile|null|undefined>; + // Normally, the profile is obtained from the user's session object, which is set at login, and + // is identified by a session cookie. When given, overrideProfile() will be called first to + // extract the profile from each request. Result can be a profile, or null if anonymous + // (sessions will then not be used), or undefined to fall back to using session info. + overrideProfile?(req: express.Request|IncomingMessage): Promise<UserProfile|null|undefined>; // Called on first visit to an app page after a signup, for reporting or telemetry purposes. onFirstVisit?(req: express.Request): void; } diff --git a/test/server/lib/Authorizer.ts b/test/server/lib/Authorizer.ts index 52cf189a..12625239 100644 --- a/test/server/lib/Authorizer.ts +++ b/test/server/lib/Authorizer.ts @@ -78,7 +78,10 @@ describe('Authorizer', function() { this.timeout(5000); setUpDB(this); oldEnv = new testUtils.EnvironmentSnapshot(); + // GRIST_PROXY_AUTH_HEADER now only affects requests directly when GRIST_IGNORE_SESSION is + // also set. process.env.GRIST_PROXY_AUTH_HEADER = 'X-email'; + process.env.GRIST_IGNORE_SESSION = 'true'; await createInitialDb(); await activateServer(server, docTools.getDocManager()); await loadFixtureDocs(); @@ -185,7 +188,9 @@ describe('Authorizer', function() { const applyUserActions = await cli.send("applyUserActions", 0, [["UpdateRecord", "Table1", 1, {A: nonce}]]); - assert.lengthOf(cli.messages, 1); // user actions pushed to client + // Skip messages with no actions (since docUsage may or may not appear by now) + const messagesWithActions = cli.messages.filter(m => m.data.docActions); + assert.lengthOf(messagesWithActions, 1); // user actions pushed to client assert.equal(applyUserActions.error, undefined); const fetchTable = await cli.send("fetchTable", 0, "Table1"); assert.equal(fetchTable.error, undefined);