Support nonce and acr with OIDC + other improvements and tests (#883)

* Introduces new configuration variables for OIDC:
  - GRIST_OIDC_IDP_ENABLED_PROTECTIONS
  - GRIST_OIDC_IDP_ACR_VALUES
  - GRIST_OIDC_IDP_EXTRA_CLIENT_METADATA
* Implements all supported protections in oidc/Protections.ts
* Includes a better error page for failed OIDC logins
* Includes some other improvements, e.g. to logging, to OIDC
* Adds a large unit test for OIDCConfig
* Adds support for SERVER_NODE_OPTIONS for running tests
* Adds to documentation/develop.md info about GREP_TESTS, VERBOSE, and SERVER_NODE_OPTIONS.

app/client/ui/errorPages.ts

@@ -15,12 +15,19 @@ const testId = makeTestId('test-');
 
 const t = makeT('errorPages');
 
+function signInAgainButton() {
+  return cssButtonWrap(bigPrimaryButtonLink(
+    t("Sign in again"), {href: getLoginUrl()}, testId('error-signin')
+  ));
+}
+
 export function createErrPage(appModel: AppModel) {
   const {errMessage, errPage} = getGristConfig();
   return errPage === 'signed-out' ? createSignedOutPage(appModel) :
     errPage === 'not-found' ? createNotFoundPage(appModel, errMessage) :
     errPage === 'access-denied' ? createForbiddenPage(appModel, errMessage) :
     errPage === 'account-deleted' ? createAccountDeletedPage(appModel) :
+    errPage === 'signin-failed' ? createSigninFailedPage(appModel, errMessage) :
     createOtherErrorPage(appModel, errMessage);
 }
 
@@ -61,9 +68,7 @@ export function createSignedOutPage(appModel: AppModel) {
 
   return pagePanelsError(appModel, t("Signed out{{suffix}}", {suffix: ''}), [
     cssErrorText(t("You are now signed out.")),
-    cssButtonWrap(bigPrimaryButtonLink(
+    signInAgainButton(),
-      t("Sign in again"), {href: getLoginUrl()}, testId('error-signin')
-    ))
   ]);
 }
 
@@ -98,6 +103,18 @@ export function createNotFoundPage(appModel: AppModel, message?: string) {
   ]);
 }
 
+export function createSigninFailedPage(appModel: AppModel, message?: string) {
+  document.title = t("Sign-in failed{{suffix}}", {suffix: getPageTitleSuffix(getGristConfig())});
+  return pagePanelsError(appModel, t("Sign-in failed{{suffix}}", {suffix: ''}), [
+    cssErrorText(message ??
+      t("Failed to log in.{{separator}}Please try again or contact support.", {
+        separator: dom('br')
+    })),
+    signInAgainButton(),
+    cssButtonWrap(bigBasicButtonLink(t("Contact support"), {href: commonUrls.contactSupport})),
+  ]);
+}
+
 /**
  * Creates a generic error page with the given message.
  */

app/common/StringUnion.ts

@@ -1,3 +1,9 @@
+export class StringUnionError extends TypeError {
+  constructor(errMessage: string, public readonly actual: string, public readonly values: string[]) {
+    super(errMessage);
+  }
+}
+
 /**
  * TypeScript will infer a string union type from the literal values passed to
  * this function. Without `extends string`, it would instead generalize them
@@ -28,7 +34,7 @@ export const StringUnion = <UnionType extends string>(...values: UnionType[]) =>
     if (!guard(value)) {
       const actual = JSON.stringify(value);
       const expected = values.map(s => JSON.stringify(s)).join(' | ');
-      throw new TypeError(`Value '${actual}' is not assignable to type '${expected}'.`);
+      throw new StringUnionError(`Value '${actual}' is not assignable to type '${expected}'.`, actual, values);
     }
     return value;
   };
@@ -44,6 +50,6 @@ export const StringUnion = <UnionType extends string>(...values: UnionType[]) =>
     return value != null && guard(value) ? value : undefined;
   };
 
-  const unionNamespace = {guard, check, parse, values, checkAll};
+  const unionNamespace = { guard, check, parse, values, checkAll };
   return Object.freeze(unionNamespace as typeof unionNamespace & {type: UnionType});
 };

app/server/lib/BrowserSession.ts

@@ -69,12 +69,21 @@ export interface SessionObj {
                           // something they just added, without allowing the suer
                           // to edit other people's contributions).
 
-  oidc?: {
+  oidc?: SessionOIDCInfo;
-    // codeVerifier is used during OIDC authentication, to protect against attacks like CSRF.
+}
-    codeVerifier?: string;
+
-    state?: string;
+export interface SessionOIDCInfo {
-    targetUrl?: string;
+  // more details on protections are available here: https://danielfett.de/2020/05/16/pkce-vs-nonce-equivalent-or-not/#special-case-error-responses
-  }
+  // code_verifier is used during OIDC authentication for PKCE protection, to protect against attacks like CSRF.
+  // PKCE + state are currently the best combination to protect against CSRF and code injection attacks.
+  code_verifier?: string;
+  // much like code_verifier, for OIDC providers that do not support PKCE.
+  nonce?: string;
+  // state is used to protect against Error Responses spoofs.
+  state?: string;
+  targetUrl?: string;
+  // Stores user claims signed by the issuer, store it to allow loging out.
+  idToken?: string;
 }
 
 // Make an artificial change to a session to encourage express-session to set a cookie.

app/server/lib/FlexServer.ts

@@ -1036,7 +1036,7 @@ export class FlexServer implements GristServer {
       server: this,
       staticDir: getAppPathTo(this.appRoot, 'static'),
       tag: this.tag,
-      testLogin: allowTestLogin(),
+      testLogin: isTestLoginAllowed(),
       baseDomain: this._defaultBaseDomain,
     });
 
@@ -1207,7 +1207,7 @@ export class FlexServer implements GristServer {
     })));
     this.app.get('/signin', ...signinMiddleware, expressWrap(this._redirectToLoginOrSignup.bind(this, {})));
 
-    if (allowTestLogin()) {
+    if (isTestLoginAllowed()) {
       // This is an endpoint for the dev environment that lets you log in as anyone.
       // For a standard dev environment, it will be accessible at localhost:8080/test/login
       // and localhost:8080/o/<org>/test/login.  Only available when GRIST_TEST_LOGIN is set.
@@ -1989,7 +1989,9 @@ export class FlexServer implements GristServer {
   }
 
   public resolveLoginSystem() {
-    return process.env.GRIST_TEST_LOGIN ? getTestLoginSystem() : (this._getLoginSystem?.() || getLoginSystem());
+    return isTestLoginAllowed() ?
+      getTestLoginSystem() :
+      (this._getLoginSystem?.() || getLoginSystem());
   }
 
   public addUpdatesCheck() {
@@ -2519,8 +2521,8 @@ function configServer<T extends https.Server|http.Server>(server: T): T {
 }
 
 // Returns true if environment is configured to allow unauthenticated test logins.
-function allowTestLogin() {
+function isTestLoginAllowed() {
-  return Boolean(process.env.GRIST_TEST_LOGIN);
+  return isAffirmative(process.env.GRIST_TEST_LOGIN);
 }
 
 // Check OPTIONS requests for allowed origins, and return heads to allow the browser to proceed

app/server/lib/OIDCConfig.ts

@@ -35,6 +35,19 @@
  *    env GRIST_OIDC_SP_IGNORE_EMAIL_VERIFIED
  *        If set to "true", the user will be allowed to login even if the email is not verified by the IDP.
  *        Defaults to false.
+ *    env GRIST_OIDC_IDP_ENABLED_PROTECTIONS
+ *        A comma-separated list of protections to enable. Supported values are "PKCE", "STATE", "NONCE"
+ *        (or you may set it to "UNPROTECTED" alone, to disable any protections if you *really* know what you do!).
+ *        Defaults to "PKCE,STATE", which is the recommended settings.
+ *        It's highly recommended that you enable STATE, and at least one of PKCE or NONCE,
+ *        depending on what your OIDC provider requires/supports.
+ *    env GRIST_OIDC_IDP_ACR_VALUES
+ *        A space-separated list of ACR values to request from the IdP. Optional.
+ *    env GRIST_OIDC_IDP_EXTRA_CLIENT_METADATA
+ *        A JSON object with extra client metadata to pass to openid-client. Optional.
+ *        Be aware that setting this object may override any other values passed to the openid client.
+ *        More info: https://github.com/panva/node-openid-client/tree/main/docs#new-clientmetadata-jwks-options
+ *
  *
  * This version of OIDCConfig has been tested with Keycloak OIDC IdP following the instructions
  * at:
@@ -52,26 +65,61 @@
 
 import * as express from 'express';
 import { GristLoginSystem, GristServer } from './GristServer';
-import { Client, generators, Issuer, UserinfoResponse } from 'openid-client';
+import {
+  Client, ClientMetadata, Issuer, errors as OIDCError, TokenSet, UserinfoResponse
+} from 'openid-client';
 import { Sessions } from './Sessions';
 import log from 'app/server/lib/log';
-import { appSettings } from './AppSettings';
+import { AppSettings, appSettings } from './AppSettings';
 import { RequestWithLogin } from './Authorizer';
 import { UserProfile } from 'app/common/LoginSessionAPI';
+import { SendAppPageFunction } from 'app/server/lib/sendAppPage';
+import { StringUnionError } from 'app/common/StringUnion';
+import { EnabledProtection, EnabledProtectionString, ProtectionsManager } from './oidc/Protections';
+import { SessionObj } from './BrowserSession';
 
 const CALLBACK_URL = '/oauth2/callback';
 
+function formatTokenForLogs(token: TokenSet) {
+  const showValueInClear = ['token_type', 'expires_in', 'expires_at', 'scope'];
+  const result: Record<string, any> = {};
+  for (const [key, value] of Object.entries(token)) {
+    if (typeof value !== 'function') {
+      result[key] = showValueInClear.includes(key) ? value : 'REDACTED';
+    }
+  }
+  return result;
+}
+
+class ErrorWithUserFriendlyMessage extends Error {
+  constructor(errMessage: string, public readonly userFriendlyMessage: string) {
+    super(errMessage);
+  }
+}
+
 export class OIDCConfig {
-  private _client: Client;
+  /**
+   * Handy alias to create an OIDCConfig instance and initialize it.
+   */
+  public static async build(sendAppPage: SendAppPageFunction): Promise<OIDCConfig> {
+    const config = new OIDCConfig(sendAppPage);
+    await config.initOIDC();
+    return config;
+  }
+
+  protected _client: Client;
   private _redirectUrl: string;
   private _namePropertyKey?: string;
   private _emailPropertyKey: string;
   private _endSessionEndpoint: string;
   private _skipEndSessionEndpoint: boolean;
   private _ignoreEmailVerified: boolean;
+  private _protectionManager: ProtectionsManager;
+  private _acrValues?: string;
 
-  public constructor() {
+  protected constructor(
-  }
+    private _sendAppPage: SendAppPageFunction
+  ) {}
 
   public async initOIDC(): Promise<void> {
     const section = appSettings.section('login').section('system').section('oidc');
@@ -108,21 +156,27 @@ export class OIDCConfig {
       defaultValue: false,
     })!;
 
+    this._acrValues = section.flag('acrValues').readString({
+      envVar: 'GRIST_OIDC_IDP_ACR_VALUES',
+    })!;
+
     this._ignoreEmailVerified = section.flag('ignoreEmailVerified').readBool({
       envVar: 'GRIST_OIDC_SP_IGNORE_EMAIL_VERIFIED',
       defaultValue: false,
     })!;
 
-    const issuer = await Issuer.discover(issuerUrl);
+    const extraMetadata: Partial<ClientMetadata> = JSON.parse(section.flag('extraClientMetadata').readString({
+      envVar: 'GRIST_OIDC_IDP_EXTRA_CLIENT_METADATA',
+    }) || '{}');
+
+    const enabledProtections = this._buildEnabledProtections(section);
+    this._protectionManager = new ProtectionsManager(enabledProtections);
+
     this._redirectUrl = new URL(CALLBACK_URL, spHost).href;
-    this._client = new issuer.Client({
+    await this._initClient({ issuerUrl, clientId, clientSecret, extraMetadata });
-      client_id: clientId,
+
-      client_secret: clientSecret,
-      redirect_uris: [ this._redirectUrl ],
-      response_types: [ 'code' ],
-    });
     if (this._client.issuer.metadata.end_session_endpoint === undefined &&
-        !this._endSessionEndpoint && !this._skipEndSessionEndpoint) {
+      !this._endSessionEndpoint && !this._skipEndSessionEndpoint) {
       throw new Error('The Identity provider does not propose end_session_endpoint. ' +
         'If that is expected, please set GRIST_OIDC_IDP_SKIP_END_SESSION_ENDPOINT=true ' +
         'or provide an alternative logout URL in GRIST_OIDC_IDP_END_SESSION_ENDPOINT');
@@ -135,28 +189,37 @@ export class OIDCConfig {
   }
 
   public async handleCallback(sessions: Sessions, req: express.Request, res: express.Response): Promise<void> {
-    const mreq = req as RequestWithLogin;
+    let mreq;
+    try {
+      mreq = this._getRequestWithSession(req);
+    } catch(err) {
+      log.warn("OIDCConfig callback:", err.message);
+      return this._sendErrorPage(req, res);
+    }
+
     try {
       const params = this._client.callbackParams(req);
-      const { state, targetUrl } = mreq.session?.oidc ?? {};
+      if (!mreq.session.oidc) {
-      if (!state) {
+        throw new Error('Missing OIDC information associated to this session');
-        throw new Error('Login or logout failed to complete');
       }
 
-      const codeVerifier = await this._retrieveCodeVerifierFromSession(req);
+      const { targetUrl } = mreq.session.oidc;
 
-      // The callback function will compare the state present in the params and the one we retrieved from the session.
+      const checks = this._protectionManager.getCallbackChecks(mreq.session.oidc);
-      // If they don't match, it will throw an error.
+
-      const tokenSet = await this._client.callback(
+      // The callback function will compare the protections present in the params and the ones we retrieved
-        this._redirectUrl,
+      // from the session. If they don't match, it will throw an error.
-        params,
+      const tokenSet = await this._client.callback(this._redirectUrl, params, checks);
-        { state, code_verifier: codeVerifier }
+      log.debug("Got tokenSet: %o", formatTokenForLogs(tokenSet));
-      );
 
       const userInfo = await this._client.userinfo(tokenSet);
+      log.debug("Got userinfo: %o", userInfo);
 
       if (!this._ignoreEmailVerified && userInfo.email_verified !== true) {
-        throw new Error(`OIDCConfig: email not verified for ${userInfo.email}`);
+        throw new ErrorWithUserFriendlyMessage(
+          `OIDCConfig: email not verified for ${userInfo.email}`,
+          req.t("oidc.emailNotVerifiedError")
+        );
       }
 
       const profile = this._makeUserProfileFromUserInfo(userInfo);
@@ -167,33 +230,47 @@ export class OIDCConfig {
         profile,
       }));
 
-      delete mreq.session.oidc;
+      // We clear the previous session info, like the states, nonce or the code verifier, which
+      // now that we are authenticated.
+      // We store the idToken for later, especially for the logout
+      mreq.session.oidc = {
+        idToken: tokenSet.id_token,
+      };
       res.redirect(targetUrl ?? '/');
     } catch (err) {
       log.error(`OIDC callback failed: ${err.stack}`);
-      // Delete the session data even if the login failed.
+      const maybeResponse = this._maybeExtractDetailsFromError(err);
+      if (maybeResponse) {
+        log.error('Response received: %o',  maybeResponse);
+      }
+
+      // Delete entirely the session data when the login failed.
       // This way, we prevent several login attempts.
       //
       // Also session deletion must be done before sending the response.
       delete mreq.session.oidc;
-      res.status(500).send(`OIDC callback failed.`);
+
+      await this._sendErrorPage(req, res, err.userFriendlyMessage);
     }
   }
 
   public async getLoginRedirectUrl(req: express.Request, targetUrl: URL): Promise<string> {
-    const { codeVerifier, state } = await this._generateAndStoreConnectionInfo(req, targetUrl.href);
+    const mreq = this._getRequestWithSession(req);
-    const codeChallenge = generators.codeChallenge(codeVerifier);
 
-    const authUrl = this._client.authorizationUrl({
+    mreq.session.oidc = {
+      targetUrl: targetUrl.href,
+      ...this._protectionManager.generateSessionInfo()
+    };
+
+    return this._client.authorizationUrl({
       scope: process.env.GRIST_OIDC_IDP_SCOPES || 'openid email profile',
-      code_challenge: codeChallenge,
+      acr_values: this._acrValues,
-      code_challenge_method: 'S256',
+      ...this._protectionManager.forgeAuthUrlParams(mreq.session.oidc),
-      state,
     });
-    return authUrl;
   }
 
   public async getLogoutRedirectUrl(req: express.Request, redirectUrl: URL): Promise<string> {
+    const session: SessionObj|undefined = (req as RequestWithLogin).session;
     // For IdPs that don't have end_session_endpoint, we just redirect to the logout page.
     if (this._skipEndSessionEndpoint) {
       return redirectUrl.href;
@@ -203,56 +280,112 @@ export class OIDCConfig {
       return this._endSessionEndpoint;
     }
     return this._client.endSessionUrl({
-      post_logout_redirect_uri: redirectUrl.href
+      post_logout_redirect_uri: redirectUrl.href,
+      id_token_hint: session?.oidc?.idToken,
     });
   }
 
-  private async _generateAndStoreConnectionInfo(req: express.Request, targetUrl: string) {
+  public supportsProtection(protection: EnabledProtectionString) {
-    const mreq = req as RequestWithLogin;
+    return this._protectionManager.supportsProtection(protection);
-    if (!mreq.session) { throw new Error('no session available'); }
-    const codeVerifier = generators.codeVerifier();
-    const state = generators.state();
-    mreq.session.oidc = {
-      codeVerifier,
-      state,
-      targetUrl
-    };
-
-    return { codeVerifier, state };
   }
 
-  private async _retrieveCodeVerifierFromSession(req: express.Request) {
+  protected async _initClient({ issuerUrl, clientId, clientSecret, extraMetadata }:
+    { issuerUrl: string, clientId: string, clientSecret: string, extraMetadata: Partial<ClientMetadata> }
+  ): Promise<void> {
+    const issuer = await Issuer.discover(issuerUrl);
+    this._client = new issuer.Client({
+      client_id: clientId,
+      client_secret: clientSecret,
+      redirect_uris: [this._redirectUrl],
+      response_types: ['code'],
+      ...extraMetadata,
+    });
+  }
+
+  private _sendErrorPage(req: express.Request, res: express.Response, userFriendlyMessage?: string) {
+    return this._sendAppPage(req, res, {
+      path: 'error.html',
+      status: 500,
+      config: {
+        errPage: 'signin-failed',
+        errMessage: userFriendlyMessage
+      },
+    });
+  }
+
+  private _getRequestWithSession(req: express.Request) {
     const mreq = req as RequestWithLogin;
     if (!mreq.session) { throw new Error('no session available'); }
-    const codeVerifier = mreq.session.oidc?.codeVerifier;
+
-    if (!codeVerifier) { throw new Error('Login is stale'); }
+    return mreq;
-    return codeVerifier;
+  }
+
+  private _buildEnabledProtections(section: AppSettings): Set<EnabledProtectionString> {
+    const enabledProtections = section.flag('enabledProtections').readString({
+      envVar: 'GRIST_OIDC_IDP_ENABLED_PROTECTIONS',
+      defaultValue: 'PKCE,STATE',
+    })!.split(',');
+    if (enabledProtections.length === 1 && enabledProtections[0] === 'UNPROTECTED') {
+      log.warn("You chose to enable OIDC connection with no protection, you are exposed to vulnerabilities." +
+        " Please never do that in production.");
+      return new Set();
+    }
+    try {
+      return new Set(EnabledProtection.checkAll(enabledProtections));
+    } catch (e) {
+      if (e instanceof StringUnionError) {
+        throw new TypeError(`OIDC: Invalid protection in GRIST_OIDC_IDP_ENABLED_PROTECTIONS: ${e.actual}.`+
+          ` Expected at least one of these values: "${e.values.join(",")}"`
+        );
+      }
+      throw e;
+    }
   }
 
   private _makeUserProfileFromUserInfo(userInfo: UserinfoResponse): Partial<UserProfile> {
     return {
-      email: String(userInfo[ this._emailPropertyKey ]),
+      email: String(userInfo[this._emailPropertyKey]),
       name: this._extractName(userInfo)
     };
   }
 
-  private _extractName(userInfo: UserinfoResponse): string|undefined {
+  private _extractName(userInfo: UserinfoResponse): string | undefined {
     if (this._namePropertyKey) {
-      return (userInfo[ this._namePropertyKey ] as any)?.toString();
+      return (userInfo[this._namePropertyKey] as any)?.toString();
     }
     const fname = userInfo.given_name ?? '';
     const lname = userInfo.family_name ?? '';
 
     return `${fname} ${lname}`.trim() || userInfo.name;
   }
+
+  /**
+   * Returns some response details from either OIDCClient's RPError or OPError,
+   * which are handy for error logging.
+   */
+  private _maybeExtractDetailsFromError(error: Error) {
+    if (error instanceof OIDCError.OPError || error instanceof OIDCError.RPError) {
+      const { response } = error;
+      if (response) {
+        // Ensure that we don't log a buffer (which might be noisy), at least for now, unless we're sure that
+        // would be relevant.
+        const isBodyPureObject = response.body && Object.getPrototypeOf(response.body) === Object.prototype;
+        return {
+          body: isBodyPureObject ? response.body : undefined,
+          statusCode: response.statusCode,
+          statusMessage: response.statusMessage,
+        };
+      }
+    }
+    return null;
+  }
 }
 
-export async function getOIDCLoginSystem(): Promise<GristLoginSystem|undefined> {
+export async function getOIDCLoginSystem(): Promise<GristLoginSystem | undefined> {
   if (!process.env.GRIST_OIDC_IDP_ISSUER) { return undefined; }
   return {
     async getMiddleware(gristServer: GristServer) {
-      const config = new OIDCConfig();
+      const config = await OIDCConfig.build(gristServer.sendAppPage.bind(gristServer));
-      await config.initOIDC();
       return {
         getLoginRedirectUrl: config.getLoginRedirectUrl.bind(config),
         getSignUpRedirectUrl: config.getLoginRedirectUrl.bind(config),
@@ -263,6 +396,6 @@ export async function getOIDCLoginSystem(): Promise<GristLoginSystem|undefined>
         },
       };
     },
-    async deleteUser() {},
+    async deleteUser() { },
   };
 }

app/server/lib/oidc/Protections.ts

@@ -0,0 +1,121 @@
+import { StringUnion } from 'app/common/StringUnion';
+import { SessionOIDCInfo } from 'app/server/lib/BrowserSession';
+import { AuthorizationParameters, generators, OpenIDCallbackChecks } from 'openid-client';
+
+export const EnabledProtection = StringUnion(
+  "STATE",
+  "NONCE",
+  "PKCE",
+);
+export type EnabledProtectionString = typeof EnabledProtection.type;
+
+interface Protection {
+  generateSessionInfo(): SessionOIDCInfo;
+  forgeAuthUrlParams(sessionInfo: SessionOIDCInfo): AuthorizationParameters;
+  getCallbackChecks(sessionInfo: SessionOIDCInfo): OpenIDCallbackChecks;
+}
+
+function checkIsSet(value: string|undefined, message: string): string {
+  if (!value) { throw new Error(message); }
+  return value;
+}
+
+class PKCEProtection implements Protection {
+  public generateSessionInfo(): SessionOIDCInfo {
+    return {
+      code_verifier: generators.codeVerifier()
+    };
+  }
+  public forgeAuthUrlParams(sessionInfo: SessionOIDCInfo): AuthorizationParameters {
+    return {
+      code_challenge: generators.codeChallenge(checkIsSet(sessionInfo.code_verifier, "Login is stale")),
+      code_challenge_method: 'S256'
+    };
+  }
+  public getCallbackChecks(sessionInfo: SessionOIDCInfo): OpenIDCallbackChecks {
+    return {
+      code_verifier: checkIsSet(sessionInfo.code_verifier, "Login is stale")
+    };
+  }
+}
+
+class NonceProtection implements Protection {
+  public generateSessionInfo(): SessionOIDCInfo {
+    return {
+      nonce: generators.nonce()
+    };
+  }
+  public forgeAuthUrlParams(sessionInfo: SessionOIDCInfo): AuthorizationParameters {
+    return {
+      nonce: sessionInfo.nonce
+    };
+  }
+  public getCallbackChecks(sessionInfo: SessionOIDCInfo): OpenIDCallbackChecks {
+    return {
+      nonce: checkIsSet(sessionInfo.nonce, "Login is stale")
+    };
+  }
+}
+
+class StateProtection implements Protection {
+  public generateSessionInfo(): SessionOIDCInfo {
+    return {
+      state: generators.state()
+    };
+  }
+  public forgeAuthUrlParams(sessionInfo: SessionOIDCInfo): AuthorizationParameters {
+    return {
+      state: sessionInfo.state
+    };
+  }
+  public getCallbackChecks(sessionInfo: SessionOIDCInfo): OpenIDCallbackChecks {
+    return {
+      state: checkIsSet(sessionInfo.state, "Login or logout failed to complete")
+    };
+  }
+}
+
+export class ProtectionsManager implements Protection {
+  private _protections: Protection[] = [];
+
+  constructor(private _enabledProtections: Set<EnabledProtectionString>) {
+    if (this._enabledProtections.has('STATE')) {
+      this._protections.push(new StateProtection());
+    }
+    if (this._enabledProtections.has('NONCE')) {
+      this._protections.push(new NonceProtection());
+    }
+    if (this._enabledProtections.has('PKCE')) {
+      this._protections.push(new PKCEProtection());
+    }
+  }
+
+  public generateSessionInfo(): SessionOIDCInfo {
+    const sessionInfo: SessionOIDCInfo = {};
+    for (const protection of this._protections) {
+      Object.assign(sessionInfo, protection.generateSessionInfo());
+    }
+    return sessionInfo;
+  }
+
+  public forgeAuthUrlParams(sessionInfo: SessionOIDCInfo): AuthorizationParameters {
+    const authParams: AuthorizationParameters = {};
+    for (const protection of this._protections) {
+      Object.assign(authParams, protection.forgeAuthUrlParams(sessionInfo));
+    }
+    return authParams;
+  }
+
+  public getCallbackChecks(sessionInfo: SessionOIDCInfo): OpenIDCallbackChecks {
+    const checks: OpenIDCallbackChecks = {};
+    for (const protection of this._protections) {
+      Object.assign(checks, protection.getCallbackChecks(sessionInfo));
+    }
+    return checks;
+  }
+
+  public supportsProtection(protection: EnabledProtectionString) {
+    return this._enabledProtections.has(protection);
+  }
+}
+

app/server/lib/sendAppPage.ts

@@ -121,15 +121,16 @@ export function makeMessagePage(staticDir: string) {
   };
 }
 
+export type SendAppPageFunction =
+  (req: express.Request, resp: express.Response, options: ISendAppPageOptions) => Promise<void>;
+
 /**
  * Send a simple template page, read from file at pagePath (relative to static/), with certain
  * placeholders replaced.
  */
-export function makeSendAppPage(opts: {
+export function makeSendAppPage({ server, staticDir, tag, testLogin, baseDomain }: {
-  server: GristServer, staticDir: string, tag: string, testLogin?: boolean,
+  server: GristServer, staticDir: string, tag: string, testLogin?: boolean, baseDomain?: string
-  baseDomain?: string
+}): SendAppPageFunction {
-}) {
-  const {server, staticDir, tag, testLogin} = opts;
 
   // If env var GRIST_INCLUDE_CUSTOM_SCRIPT_URL is set, load it in a <script> tag on all app pages.
   const customScriptUrl = process.env.GRIST_INCLUDE_CUSTOM_SCRIPT_URL;
@@ -140,7 +141,7 @@ export function makeSendAppPage(opts: {
     const config = makeGristConfig({
       homeUrl: !isSingleUserMode() ? server.getHomeUrl(req) : null,
       extra: options.config,
-      baseDomain: opts.baseDomain,
+      baseDomain,
       req,
       server,
     });

documentation/develop.md

@@ -122,6 +122,13 @@ You may run the tests using one of these commands:
  - `yarn test:docker` to run some end-to-end tests under docker
  - `yarn test:python` to run the data engine tests
 
+Also some options that may interest you:
+ - `GREP_TESTS="pattern"` in order to filter the tests to run, for example: `GREP_TESTS="Boot" yarn test:nbrowser`
+ - `VERBOSE=1` in order to view logs when a server is spawned (especially useful to debug the end-to-end and backend tests)
+ - `SERVER_NODE_OPTIONS="node options"` in order to pass options to the server being tested,
+   for example: `SERVER_NODE_OPTIONS="--inspect --inspect-brk" GREP_TESTS="Boot" yarn test:nbrowser` 
+   to run the tests with the debugger (you should close the debugger each time the node process should stop)
+
 ## Develop widgets
 
 Check out this repository: https://github.com/gristlabs/grist-widget#readme

static/locales/en.server.json

@@ -1,5 +1,8 @@
 {
   "sendAppPage": {
     "Loading": "Loading"
+  },
+  "oidc": {
+    "emailNotVerifiedError": "Please verify your email with the identity provider, and log in again."
   }
 }

test/nbrowser/homeUtil.ts

@@ -381,7 +381,7 @@ export class HomeUtil {
   /**
    * Waits for browser to navigate to a Grist login page.
    */
-   public async checkGristLoginPage(waitMs: number = 2000) {
+  public async checkGristLoginPage(waitMs: number = 2000) {
     await this.driver.wait(this.isOnGristLoginPage.bind(this), waitMs);
   }
 

test/nbrowser/testServer.ts

@@ -152,7 +152,10 @@ export class TestServerMerged extends EventEmitter implements IMochaServer {
       delete env.DOC_WORKER_COUNT;
     }
     this._server = spawn('node', [cmd], {
-      env,
+      env: {
+        ...env,
+        ...(process.env.SERVER_NODE_OPTIONS ? {NODE_OPTIONS: process.env.SERVER_NODE_OPTIONS} : {})
+      },
       stdio: quiet ? 'ignore' : ['inherit', serverLog, serverLog],
     });
     this._exitPromise = exitPromise(this._server);

test/server/lib/OIDCConfig.ts

@@ -0,0 +1,763 @@
+import {EnvironmentSnapshot} from "../testUtils";
+import {OIDCConfig} from "app/server/lib/OIDCConfig";
+import {SessionObj} from "app/server/lib/BrowserSession";
+import {Sessions} from "app/server/lib/Sessions";
+import log from "app/server/lib/log";
+import {assert} from "chai";
+import Sinon from "sinon";
+import {Client, generators, errors as OIDCError} from "openid-client";
+import express from "express";
+import _ from "lodash";
+import {RequestWithLogin} from "app/server/lib/Authorizer";
+import { SendAppPageFunction } from "app/server/lib/sendAppPage";
+
+const NOOPED_SEND_APP_PAGE: SendAppPageFunction = () => Promise.resolve();
+
+class OIDCConfigStubbed extends OIDCConfig {
+  public static async buildWithStub(client: Client = new ClientStub().asClient()) {
+    return this.build(NOOPED_SEND_APP_PAGE, client);
+  }
+  public static async build(sendAppPage: SendAppPageFunction, clientStub?: Client): Promise<OIDCConfigStubbed> {
+    const result = new OIDCConfigStubbed(sendAppPage);
+    if (clientStub) {
+      result._initClient = Sinon.spy(() => {
+        result._client = clientStub!;
+      });
+    }
+    await result.initOIDC();
+    return result;
+  }
+
+  public _initClient: Sinon.SinonSpy;
+}
+
+class ClientStub {
+  public static FAKE_REDIRECT_URL = 'FAKE_REDIRECT_URL';
+  public authorizationUrl = Sinon.stub().returns(ClientStub.FAKE_REDIRECT_URL);
+  public callbackParams = Sinon.stub().returns(undefined);
+  public callback = Sinon.stub().returns({});
+  public userinfo = Sinon.stub().returns(undefined);
+  public endSessionUrl = Sinon.stub().returns(undefined);
+  public issuer: {
+    metadata: {
+      end_session_endpoint: string | undefined;
+    }
+  } = {
+    metadata: {
+      end_session_endpoint: 'http://localhost:8484/logout',
+    }
+  };
+  public asClient() {
+    return this as unknown as Client;
+  }
+  public getAuthorizationUrlStub() {
+    return this.authorizationUrl;
+  }
+}
+
+describe('OIDCConfig', () => {
+  let oldEnv: EnvironmentSnapshot;
+  let sandbox: Sinon.SinonSandbox;
+  let logInfoStub: Sinon.SinonStub;
+  let logErrorStub: Sinon.SinonStub;
+  let logWarnStub: Sinon.SinonStub;
+  let logDebugStub: Sinon.SinonStub;
+
+  before(() => {
+    oldEnv = new EnvironmentSnapshot();
+  });
+
+  beforeEach(() => {
+    sandbox = Sinon.createSandbox();
+    logInfoStub = sandbox.stub(log, 'info');
+    logErrorStub = sandbox.stub(log, 'error');
+    logDebugStub = sandbox.stub(log, 'debug');
+    logWarnStub = sandbox.stub(log, 'warn');
+  });
+
+  afterEach(() => {
+    oldEnv.restore();
+    sandbox.restore();
+  });
+
+  function setEnvVars() {
+    // Prevent any environment variable from leaking into the test:
+    for (const envVar in process.env) {
+      if (envVar.startsWith('GRIST_OIDC_')) {
+        delete process.env[envVar];
+      }
+    }
+    process.env.GRIST_OIDC_SP_HOST = 'http://localhost:8484';
+    process.env.GRIST_OIDC_IDP_CLIENT_ID = 'client id';
+    process.env.GRIST_OIDC_IDP_CLIENT_SECRET = 'secret';
+    process.env.GRIST_OIDC_IDP_ISSUER = 'http://localhost:8000';
+  }
+
+  describe('build', () => {
+    function isInitializedLogCalled() {
+      return logInfoStub.calledWithExactly(`OIDCConfig: initialized with issuer ${process.env.GRIST_OIDC_IDP_ISSUER}`);
+    }
+
+    it('should reject when required env variables are not passed', async () => {
+      for (const envVar of [
+        'GRIST_OIDC_SP_HOST',
+        'GRIST_OIDC_IDP_ISSUER',
+        'GRIST_OIDC_IDP_CLIENT_ID',
+        'GRIST_OIDC_IDP_CLIENT_SECRET',
+      ]) {
+        setEnvVars();
+        delete process.env[envVar];
+        const promise = OIDCConfig.build(NOOPED_SEND_APP_PAGE);
+        await assert.isRejected(promise, `missing environment variable: ${envVar}`);
+      }
+    });
+
+    it('should reject when the client initialization fails', async () => {
+      setEnvVars();
+      sandbox.stub(OIDCConfigStubbed.prototype, '_initClient').rejects(new Error('client init failed'));
+      const promise = OIDCConfigStubbed.build(NOOPED_SEND_APP_PAGE);
+      await assert.isRejected(promise, 'client init failed');
+    });
+
+    it('should create a client with passed information', async () => {
+      setEnvVars();
+      const config = await OIDCConfigStubbed.buildWithStub();
+      assert.isTrue(config._initClient.calledOnce);
+      assert.deepEqual(config._initClient.firstCall.args, [{
+        clientId: process.env.GRIST_OIDC_IDP_CLIENT_ID,
+        clientSecret: process.env.GRIST_OIDC_IDP_CLIENT_SECRET,
+        issuerUrl: process.env.GRIST_OIDC_IDP_ISSUER,
+        extraMetadata: {},
+      }]);
+
+      assert.isTrue(isInitializedLogCalled());
+    });
+
+    it('should create a client with passed information with extra configuration', async () => {
+      setEnvVars();
+      const extraMetadata = {
+        userinfo_signed_response_alg: 'RS256',
+      };
+      process.env.GRIST_OIDC_IDP_EXTRA_CLIENT_METADATA = JSON.stringify(extraMetadata);
+      const config = await OIDCConfigStubbed.buildWithStub();
+      assert.isTrue(config._initClient.calledOnce);
+      assert.deepEqual(config._initClient.firstCall.args, [{
+        clientId: process.env.GRIST_OIDC_IDP_CLIENT_ID,
+        clientSecret: process.env.GRIST_OIDC_IDP_CLIENT_SECRET,
+        issuerUrl: process.env.GRIST_OIDC_IDP_ISSUER,
+        extraMetadata,
+      }]);
+    });
+
+    describe('End Session Endpoint', () => {
+      [
+        {
+          itMsg: 'should fulfill when the end_session_endpoint is not known ' +
+            'and GRIST_OIDC_IDP_SKIP_END_SESSION_ENDPOINT=true',
+          end_session_endpoint: undefined,
+          env: {
+            GRIST_OIDC_IDP_SKIP_END_SESSION_ENDPOINT: 'true'
+          }
+        },
+        {
+          itMsg: 'should fulfill when the end_session_endpoint is provided with GRIST_OIDC_IDP_END_SESSION_ENDPOINT',
+          end_session_endpoint: undefined,
+          env: {
+            GRIST_OIDC_IDP_END_SESSION_ENDPOINT: 'http://localhost:8484/logout'
+          }
+        },
+        {
+          itMsg: 'should fulfill when the end_session_endpoint is provided with the issuer',
+          end_session_endpoint: 'http://localhost:8484/logout',
+        },
+        {
+          itMsg: 'should reject when the end_session_endpoint is not known',
+          errorMsg: /If that is expected, please set GRIST_OIDC_IDP_SKIP_END_SESSION_ENDPOINT/,
+          end_session_endpoint: undefined,
+        }
+      ].forEach((ctx) => {
+        it(ctx.itMsg, async () => {
+          setEnvVars();
+          Object.assign(process.env, ctx.env);
+          const client = new ClientStub();
+          client.issuer.metadata.end_session_endpoint = ctx.end_session_endpoint;
+          const promise = OIDCConfigStubbed.buildWithStub(client.asClient());
+          if (ctx.errorMsg) {
+            await assert.isRejected(promise, ctx.errorMsg);
+            assert.isFalse(isInitializedLogCalled());
+          } else {
+            await assert.isFulfilled(promise);
+            assert.isTrue(isInitializedLogCalled());
+          }
+        });
+      });
+    });
+  });
+
+  describe('GRIST_OIDC_IDP_ENABLED_PROTECTIONS', () => {
+    async function checkRejection(promise: Promise<OIDCConfig>, actualValue: string) {
+      return assert.isRejected(
+        promise,
+        `OIDC: Invalid protection in GRIST_OIDC_IDP_ENABLED_PROTECTIONS: "${actualValue}". ` +
+          'Expected at least one of these values: "STATE,NONCE,PKCE"');
+    }
+    it('should reject when GRIST_OIDC_IDP_ENABLED_PROTECTIONS contains unsupported values', async () => {
+      setEnvVars();
+      process.env.GRIST_OIDC_IDP_ENABLED_PROTECTIONS = 'STATE,NONCE,PKCE,invalid';
+      const promise = OIDCConfig.build(NOOPED_SEND_APP_PAGE);
+      await checkRejection(promise, 'invalid');
+    });
+
+    it('should successfully change the supported protections', async function () {
+      setEnvVars();
+      process.env.GRIST_OIDC_IDP_ENABLED_PROTECTIONS = 'NONCE';
+      const config = await OIDCConfigStubbed.buildWithStub();
+      assert.isTrue(config.supportsProtection("NONCE"));
+      assert.isFalse(config.supportsProtection("PKCE"));
+      assert.isFalse(config.supportsProtection("STATE"));
+    });
+
+    it('should reject when set to an empty string', async function () {
+      setEnvVars();
+      process.env.GRIST_OIDC_IDP_ENABLED_PROTECTIONS = '';
+      const promise = OIDCConfigStubbed.buildWithStub();
+      await checkRejection(promise, '');
+    });
+
+    it('should accept to be set to "UNPROTECTED"', async function () {
+      setEnvVars();
+      process.env.GRIST_OIDC_IDP_ENABLED_PROTECTIONS = 'UNPROTECTED';
+      const config = await OIDCConfigStubbed.buildWithStub();
+      assert.isFalse(config.supportsProtection("NONCE"));
+      assert.isFalse(config.supportsProtection("PKCE"));
+      assert.isFalse(config.supportsProtection("STATE"));
+      assert.equal(logWarnStub.callCount, 1, 'a warning should be raised');
+      assert.match(logWarnStub.firstCall.args[0], /with no protection/);
+    });
+
+    it('should reject when set to "UNPROTECTED,PKCE"', async function () {
+      setEnvVars();
+      process.env.GRIST_OIDC_IDP_ENABLED_PROTECTIONS = 'UNPROTECTED,PKCE';
+      const promise = OIDCConfigStubbed.buildWithStub();
+      await checkRejection(promise, 'UNPROTECTED');
+    });
+
+    it('if omitted, should default to "STATE,PKCE"', async function () {
+      setEnvVars();
+      const config = await OIDCConfigStubbed.buildWithStub();
+      assert.isFalse(config.supportsProtection("NONCE"));
+      assert.isTrue(config.supportsProtection("PKCE"));
+      assert.isTrue(config.supportsProtection("STATE"));
+    });
+  });
+
+  describe('getLoginRedirectUrl', () => {
+    const FAKE_NONCE = 'fake-nonce';
+    const FAKE_STATE = 'fake-state';
+    const FAKE_CODE_VERIFIER = 'fake-code-verifier';
+    const FAKE_CODE_CHALLENGE = 'fake-code-challenge';
+    const TARGET_URL = 'http://localhost:8484/';
+
+    beforeEach(() => {
+      sandbox.stub(generators, 'nonce').returns(FAKE_NONCE);
+      sandbox.stub(generators, 'state').returns(FAKE_STATE);
+      sandbox.stub(generators, 'codeVerifier').returns(FAKE_CODE_VERIFIER);
+      sandbox.stub(generators, 'codeChallenge').returns(FAKE_CODE_CHALLENGE);
+    });
+
+    [
+      {
+        itMsg: 'should forge the url with default values',
+        expectedCalledWith: [{
+          scope: 'openid email profile',
+          acr_values: undefined,
+          code_challenge: FAKE_CODE_CHALLENGE,
+          code_challenge_method: 'S256',
+          state: FAKE_STATE,
+        }],
+        expectedSession: {
+          oidc: {
+            code_verifier: FAKE_CODE_VERIFIER,
+            state: FAKE_STATE,
+            targetUrl: TARGET_URL,
+          }
+        }
+      },
+      {
+        itMsg: 'should forge the URL with passed GRIST_OIDC_IDP_SCOPES',
+        env: {
+          GRIST_OIDC_IDP_SCOPES: 'my scopes',
+        },
+        expectedCalledWith: [{
+          scope: 'my scopes',
+          acr_values: undefined,
+          code_challenge: FAKE_CODE_CHALLENGE,
+          code_challenge_method: 'S256',
+          state: FAKE_STATE,
+        }],
+        expectedSession: {
+          oidc: {
+            code_verifier: FAKE_CODE_VERIFIER,
+            state: FAKE_STATE,
+            targetUrl: TARGET_URL,
+          }
+        }
+      },
+      {
+        itMsg: 'should pass the nonce when GRIST_OIDC_IDP_ENABLED_PROTECTIONS includes NONCE',
+        env: {
+          GRIST_OIDC_IDP_ENABLED_PROTECTIONS: 'STATE,NONCE,PKCE',
+        },
+        expectedCalledWith: [{
+          scope: 'openid email profile',
+          acr_values: undefined,
+          code_challenge: FAKE_CODE_CHALLENGE,
+          code_challenge_method: 'S256',
+          state: FAKE_STATE,
+          nonce: FAKE_NONCE,
+        }],
+        expectedSession: {
+          oidc: {
+            code_verifier: FAKE_CODE_VERIFIER,
+            nonce: FAKE_NONCE,
+            state: FAKE_STATE,
+            targetUrl: TARGET_URL,
+          }
+        }
+      },
+      {
+        itMsg: 'should not pass the code_challenge when PKCE is omitted in GRIST_OIDC_IDP_ENABLED_PROTECTIONS',
+        env: {
+          GRIST_OIDC_IDP_ENABLED_PROTECTIONS: 'STATE,NONCE',
+        },
+        expectedCalledWith: [{
+          scope: 'openid email profile',
+          acr_values: undefined,
+          state: FAKE_STATE,
+          nonce: FAKE_NONCE,
+        }],
+        expectedSession: {
+          oidc: {
+            nonce: FAKE_NONCE,
+            state: FAKE_STATE,
+            targetUrl: TARGET_URL,
+          }
+        }
+      },
+    ].forEach(ctx => {
+      it(ctx.itMsg, async () => {
+        setEnvVars();
+        Object.assign(process.env, ctx.env);
+        const clientStub = new ClientStub();
+        const config = await OIDCConfigStubbed.buildWithStub(clientStub.asClient());
+        const session = {};
+        const req = {
+          session
+        } as unknown as express.Request;
+        const url = await config.getLoginRedirectUrl(req, new URL(TARGET_URL));
+        assert.equal(url, ClientStub.FAKE_REDIRECT_URL);
+        assert.isTrue(clientStub.authorizationUrl.calledOnce);
+        assert.deepEqual(clientStub.authorizationUrl.firstCall.args, ctx.expectedCalledWith);
+        assert.deepEqual(session, ctx.expectedSession);
+      });
+    });
+  });
+
+  describe('handleCallback', () => {
+    const FAKE_STATE = 'fake-state';
+    const FAKE_NONCE = 'fake-nonce';
+    const FAKE_CODE_VERIFIER = 'fake-code-verifier';
+    const FAKE_USER_INFO = {
+      email: 'fake-email',
+      name: 'fake-name',
+      email_verified: true,
+    };
+    const DEFAULT_SESSION = {
+      oidc: {
+        code_verifier: FAKE_CODE_VERIFIER,
+        state: FAKE_STATE
+      }
+    } as SessionObj;
+    const DEFAULT_EXPECTED_CALLBACK_CHECKS = {
+      state: FAKE_STATE,
+      code_verifier: FAKE_CODE_VERIFIER,
+    };
+    let fakeRes: {
+      status: Sinon.SinonStub;
+      send: Sinon.SinonStub;
+      redirect: Sinon.SinonStub;
+    };
+    let fakeSessions: {
+      getOrCreateSessionFromRequest: Sinon.SinonStub
+    };
+    let fakeScopedSession: {
+      operateOnScopedSession: Sinon.SinonStub
+    };
+
+    beforeEach(() => {
+      fakeRes = {
+        redirect: Sinon.stub(),
+        status: Sinon.stub().returnsThis(),
+        send: Sinon.stub().returnsThis(),
+      };
+      fakeScopedSession = {
+        operateOnScopedSession: Sinon.stub().resolves(),
+      };
+      fakeSessions = {
+        getOrCreateSessionFromRequest: Sinon.stub().returns(fakeScopedSession),
+      };
+    });
+
+    function checkUserProfile(expectedUserProfile: object) {
+      return function ({user}: {user: any}) {
+        assert.deepEqual(user.profile, expectedUserProfile,
+          `user profile should have been populated with ${JSON.stringify(expectedUserProfile)}`);
+      };
+    }
+
+    function checkRedirect(expectedRedirection: string) {
+      return function ({fakeRes}: {fakeRes: any}) {
+        assert.deepEqual(fakeRes.redirect.firstCall.args, [expectedRedirection],
+          `should have redirected to ${expectedRedirection}`);
+      };
+    }
+
+    [
+      {
+        itMsg: 'should reject when no OIDC information is present in the session',
+        session: {},
+        expectedErrorMsg: /Missing OIDC information/
+      },
+      {
+        itMsg: 'should resolve when the state and the code challenge are found in the session',
+        session: DEFAULT_SESSION,
+      },
+      {
+        itMsg: 'should reject when the state is not found in the session',
+        session: {
+          oidc: {}
+        },
+        expectedErrorMsg: /Login or logout failed to complete/,
+      },
+      {
+        itMsg: 'should resolve when the state is missing and its check has been disabled (UNPROTECTED)',
+        session: DEFAULT_SESSION,
+        env: {
+          GRIST_OIDC_IDP_ENABLED_PROTECTIONS: 'UNPROTECTED',
+        },
+        expectedCbChecks: {},
+      },
+      {
+        itMsg: 'should reject when the code_verifier is missing from the session',
+        session: {
+          oidc: {
+            state: FAKE_STATE,
+            GRIST_OIDC_IDP_ENABLED_PROTECTIONS: 'STATE,PKCE'
+          }
+        },
+        expectedErrorMsg: /Login is stale/,
+      },
+      {
+        itMsg: 'should resolve when the code_verifier is missing and its check has been disabled',
+        session: {
+          oidc: {
+            state: FAKE_STATE,
+            nonce: FAKE_NONCE,
+          }
+        },
+        env: {
+          GRIST_OIDC_IDP_ENABLED_PROTECTIONS: 'STATE,NONCE',
+        },
+        expectedCbChecks: {
+          state: FAKE_STATE,
+          nonce: FAKE_NONCE,
+        },
+      },
+      {
+        itMsg: 'should reject when nonce is missing from the session despite its check being enabled',
+        session: DEFAULT_SESSION,
+        env: {
+          GRIST_OIDC_IDP_ENABLED_PROTECTIONS: 'STATE,NONCE,PKCE',
+        },
+        expectedErrorMsg: /Login is stale/,
+      }, {
+        itMsg: 'should resolve when nonce is present in the session and its check is enabled',
+        session: {
+          oidc: {
+            state: FAKE_STATE,
+            nonce: FAKE_NONCE,
+            code_verifier: undefined,
+          },
+        },
+        env: {
+          GRIST_OIDC_IDP_ENABLED_PROTECTIONS: 'STATE,NONCE',
+        },
+        expectedCbChecks: {
+          state: FAKE_STATE,
+          nonce: FAKE_NONCE,
+        },
+      },
+      {
+        itMsg: 'should reject when the userinfo mail is not verified',
+        session: DEFAULT_SESSION,
+        userInfo: {
+          ...FAKE_USER_INFO,
+          email_verified: false,
+        },
+        expectedErrorMsg: /email not verified for/,
+        extraChecks: function ({ sendAppPageStub }: { sendAppPageStub: Sinon.SinonStub }) {
+          assert.equal(sendAppPageStub.firstCall.args[2].config.errMessage, 'oidc.emailNotVerifiedError');
+        }
+      },
+      {
+        itMsg: 'should resolve when the userinfo mail is not verified but its check disabled',
+        session: DEFAULT_SESSION,
+        userInfo: {
+          ...FAKE_USER_INFO,
+          email_verified: false,
+        },
+        env: {
+          GRIST_OIDC_SP_IGNORE_EMAIL_VERIFIED: 'true',
+        }
+      },
+      {
+        itMsg: 'should resolve when the userinfo mail is not verified but its check disabled',
+        session: DEFAULT_SESSION,
+        userInfo: {
+          ...FAKE_USER_INFO,
+          email_verified: false,
+        },
+        env: {
+          GRIST_OIDC_SP_IGNORE_EMAIL_VERIFIED: 'true',
+        },
+      },
+      {
+        itMsg: 'should fill user profile with email and name',
+        session: DEFAULT_SESSION,
+        userInfo: FAKE_USER_INFO,
+        extraChecks: checkUserProfile({
+          email: FAKE_USER_INFO.email,
+          name: FAKE_USER_INFO.name,
+        })
+      },
+      {
+        itMsg: 'should fill user profile with name constructed using ' +
+          'given_name and family_name when GRIST_OIDC_SP_PROFILE_NAME_ATTR is not set',
+        session: DEFAULT_SESSION,
+        userInfo: {
+          ...FAKE_USER_INFO,
+          given_name: 'given_name',
+          family_name: 'family_name',
+        },
+        extrachecks: checkUserProfile({
+          email: 'fake-email',
+          name: 'given_name family_name',
+        })
+      },
+      {
+        itMsg: 'should fill user profile with email and name when ' +
+          'GRIST_OIDC_SP_PROFILE_NAME_ATTR and GRIST_OIDC_SP_PROFILE_EMAIL_ATTR are set',
+        session: DEFAULT_SESSION,
+        userInfo: {
+          ...FAKE_USER_INFO,
+          fooMail: 'fake-email2',
+          fooName: 'fake-name2',
+        },
+        env: {
+          GRIST_OIDC_SP_PROFILE_NAME_ATTR: 'fooName',
+          GRIST_OIDC_SP_PROFILE_EMAIL_ATTR: 'fooMail',
+        },
+        extraChecks: checkUserProfile({
+          email: 'fake-email2',
+          name: 'fake-name2',
+        }),
+      },
+      {
+        itMsg: 'should redirect by default to the root page',
+        session: DEFAULT_SESSION,
+        extraChecks: checkRedirect('/'),
+      },
+      {
+        itMsg: 'should redirect to the targetUrl when it is present in the session',
+        session: {
+          oidc: {
+            ...DEFAULT_SESSION.oidc,
+            targetUrl: 'http://localhost:8484/some/path'
+          }
+        },
+        extraChecks: checkRedirect('http://localhost:8484/some/path'),
+      },
+      {
+        itMsg: "should redact confidential information in the tokenSet in the logs",
+        session: DEFAULT_SESSION,
+        tokenSet: {
+          id_token: 'fake-id-token',
+          access_token: 'fake-access',
+          whatever: 'fake-whatever',
+          token_type: 'fake-token-type',
+          expires_at: 1234567890,
+          expires_in: 987654321,
+          scope: 'fake-scope',
+        },
+        extraChecks: function () {
+          assert.isTrue(logDebugStub.called);
+          assert.deepEqual(logDebugStub.firstCall.args, [
+            'Got tokenSet: %o', {
+              id_token: 'REDACTED',
+              access_token: 'REDACTED',
+              whatever: 'REDACTED',
+              token_type: this.tokenSet.token_type,
+              expires_at: this.tokenSet.expires_at,
+              expires_in: this.tokenSet.expires_in,
+              scope: this.tokenSet.scope,
+            }
+          ]);
+        }
+      },
+    ].forEach(ctx => {
+      it(ctx.itMsg, async () => {
+        setEnvVars();
+        Object.assign(process.env, ctx.env);
+        const clientStub = new ClientStub();
+        const sendAppPageStub = Sinon.stub().resolves();
+        const fakeParams = {
+          state: FAKE_STATE,
+        };
+        const config = await OIDCConfigStubbed.build(sendAppPageStub as SendAppPageFunction, clientStub.asClient());
+        const session = _.clone(ctx.session); // session is modified, so clone it
+        const req = {
+          session,
+          t: (key: string) => key
+        } as unknown as express.Request;
+        clientStub.callbackParams.returns(fakeParams);
+        const tokenSet = { id_token: 'id_token', ...ctx.tokenSet };
+        clientStub.callback.resolves(tokenSet);
+        clientStub.userinfo.returns(_.clone(ctx.userInfo ?? FAKE_USER_INFO));
+        const user: { profile?: object } = {};
+        fakeScopedSession.operateOnScopedSession.yields(user);
+
+        await config.handleCallback(
+          fakeSessions as unknown as Sessions,
+          req,
+          fakeRes as unknown as express.Response
+        );
+
+        if (ctx.expectedErrorMsg) {
+          assert.isTrue(logErrorStub.calledOnce);
+          assert.match(logErrorStub.firstCall.args[0], ctx.expectedErrorMsg);
+          assert.isTrue(sendAppPageStub.calledOnceWith(req, fakeRes));
+          assert.include(sendAppPageStub.firstCall.args[2], {
+            path: 'error.html',
+            status: 500,
+          });
+        } else {
+          assert.isFalse(logErrorStub.called, 'no error should be logged. Got: ' + logErrorStub.firstCall?.args[0]);
+          assert.isTrue(fakeRes.redirect.calledOnce, 'should redirect');
+          assert.isTrue(clientStub.callback.calledOnce);
+          assert.deepEqual(clientStub.callback.firstCall.args, [
+            'http://localhost:8484/oauth2/callback',
+            fakeParams,
+            ctx.expectedCbChecks ?? DEFAULT_EXPECTED_CALLBACK_CHECKS
+          ]);
+          assert.deepEqual(session, {
+            oidc: {
+              idToken: tokenSet.id_token,
+            }
+          }, 'oidc info should only keep state and id_token in the session and for the logout');
+        }
+        ctx.extraChecks?.({ fakeRes, user, sendAppPageStub });
+      });
+    });
+
+    it('should log err.response when userinfo fails to parse response body', async () => {
+      // See https://github.com/panva/node-openid-client/blob/47a549cb4e36ffe2ebfe2dc9d6b69a02643cc0a9/lib/client.js#L1293
+      setEnvVars();
+      const clientStub = new ClientStub();
+      const sendAppPageStub = Sinon.stub().resolves();
+      const config = await OIDCConfigStubbed.build(sendAppPageStub, clientStub.asClient());
+      const req = {
+        session: DEFAULT_SESSION,
+      } as unknown as express.Request;
+      clientStub.callbackParams.returns({state: FAKE_STATE});
+      const errorResponse = {
+        body: { property: 'response here' },
+        statusCode: 400,
+        statusMessage: 'statusMessage'
+      } as unknown as any;
+
+      const err = new OIDCError.OPError({error: 'userinfo failed'}, errorResponse);
+      clientStub.userinfo.rejects(err);
+
+      await config.handleCallback(
+        fakeSessions as unknown as Sessions,
+        req,
+        fakeRes as unknown as express.Response
+      );
+
+      assert.equal(logErrorStub.callCount, 2, 'logErrorStub show be called twice');
+      assert.include(logErrorStub.firstCall.args[0], err.message);
+      assert.include(logErrorStub.secondCall.args[0], 'Response received');
+      assert.deepEqual(logErrorStub.secondCall.args[1], errorResponse);
+      assert.isTrue(sendAppPageStub.calledOnce, "An error should have been sent");
+    });
+  });
+
+  describe('getLogoutRedirectUrl', () => {
+    const REDIRECT_URL = new URL('http://localhost:8484/docs/signed-out');
+    const URL_RETURNED_BY_CLIENT = 'http://localhost:8484/logout_url_from_issuer';
+    const ENV_VALUE_GRIST_OIDC_IDP_END_SESSION_ENDPOINT = 'http://localhost:8484/logout';
+    const FAKE_SESSION = {
+      oidc: {
+        idToken: 'id_token',
+      }
+    } as SessionObj;
+
+    [
+      {
+        itMsg: 'should skip the end session endpoint when GRIST_OIDC_IDP_SKIP_END_SESSION_ENDPOINT=true',
+        env: {
+          GRIST_OIDC_IDP_SKIP_END_SESSION_ENDPOINT: 'true',
+        },
+        expectedUrl: REDIRECT_URL.href,
+      }, {
+        itMsg: 'should use the GRIST_OIDC_IDP_END_SESSION_ENDPOINT when it is set',
+        env: {
+          GRIST_OIDC_IDP_END_SESSION_ENDPOINT: ENV_VALUE_GRIST_OIDC_IDP_END_SESSION_ENDPOINT
+        },
+        expectedUrl: ENV_VALUE_GRIST_OIDC_IDP_END_SESSION_ENDPOINT
+      }, {
+        itMsg: 'should call the end session endpoint with the expected parameters',
+        expectedUrl: URL_RETURNED_BY_CLIENT,
+        expectedLogoutParams: {
+          post_logout_redirect_uri: REDIRECT_URL.href,
+          id_token_hint: FAKE_SESSION.oidc!.idToken,
+        }
+      }, {
+        itMsg: 'should call the end session endpoint with no idToken if session is missing',
+        expectedUrl: URL_RETURNED_BY_CLIENT,
+        expectedLogoutParams: {
+          post_logout_redirect_uri: REDIRECT_URL.href,
+          id_token_hint: undefined,
+        },
+        session: null
+      }
+    ].forEach(ctx => {
+      it(ctx.itMsg, async () => {
+        setEnvVars();
+        Object.assign(process.env, ctx.env);
+        const clientStub = new ClientStub();
+        clientStub.endSessionUrl.returns(URL_RETURNED_BY_CLIENT);
+        const config = await OIDCConfigStubbed.buildWithStub(clientStub.asClient());
+        const req = {
+          session: 'session' in ctx ? ctx.session : FAKE_SESSION
+        } as unknown as RequestWithLogin;
+        const url = await config.getLogoutRedirectUrl(req, REDIRECT_URL);
+        assert.equal(url, ctx.expectedUrl);
+        if (ctx.expectedLogoutParams) {
+          assert.isTrue(clientStub.endSessionUrl.calledOnce);
+          assert.deepEqual(clientStub.endSessionUrl.firstCall.args, [ctx.expectedLogoutParams]);
+        }
+      });
+    });
+  });
+});