(core) Revamp ForwardAuthLogin and unify with GRIST_PROXY_AUTH_HEADER

Summary:
By default, only respect GRIST_FORWARD_AUTH_HEADER on login endpoints; sessions are used elsewhere.

With GRIST_IGNORE_SESSION, do not use sessions, and respect GRIST_FORWARD_AUTH_HEADER on all endpoints.

GRIST_PROXY_AUTH_HEADER is now a synonym to GRIST_FORWARD_AUTH_HEADER.

Test Plan: Fixed tests. Tested first approach (no GRIST_IGNORE_SESSION) with grist-omnibus manually. Tested the second approach (with GRIST_IGNORE_SESSION) with a Apache-based setup enforcing http basic auth on all endpoints.

Reviewers: paulfitz, georgegevoian

Reviewed By: paulfitz, georgegevoian

Differential Revision: https://phab.getgrist.com/D4104

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 `<title>` 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:
 

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;

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

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,

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

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

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