Support nonce and acr with OIDC + Tests

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(
-      t("Sign in again"), {href: getLoginUrl()}, testId('error-signin')
-    ))
+    signInAgainButton(),
   ]);
 }
 
@@ -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?: {
-    // codeVerifier is used during OIDC authentication, to protect against attacks like CSRF.
-    codeVerifier?: string;
-    state?: string;
-    targetUrl?: string;
-  }
+  oidc?: SessionOIDCInfo;
+}
+
+export interface SessionOIDCInfo {
+  // 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

@@ -1035,7 +1035,7 @@ export class FlexServer implements GristServer {
       server: this,
       staticDir: getAppPathTo(this.appRoot, 'static'),
       tag: this.tag,
-      testLogin: allowTestLogin(),
+      testLogin: isTestLoginAllowed(),
       baseDomain: this._defaultBaseDomain,
     });
 
@@ -1206,7 +1206,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.
@@ -1976,7 +1976,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() {
@@ -2506,8 +2508,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() {
-  return Boolean(process.env.GRIST_TEST_LOGIN);
+function isTestLoginAllowed() {
+  return isAffirmative(process.env.GRIST_TEST_LOGIN);
 }
 
 // Check OPTIONS requests for allowed origins, and return heads to allow the browser to proceed