[api] security hardening for microsoft auth (#3895)

api.planx.uk/modules/auth/controller.ts

@@ -39,6 +39,7 @@ function setJWTCookie(returnTo: string, res: Response, req: Request) {
     maxAge: new Date(
       new Date().setFullYear(new Date().getFullYear() + 1),
     ).getTime(),
+    // pizzas rely on staging API for auth (due to static redirect URIs), so we have to allow cross-site
     sameSite: "none",
     secure: true,
   };

api.planx.uk/modules/auth/middleware.ts

@@ -8,7 +8,7 @@ import type { Authenticator } from "passport";
 import type { RequestHandler } from "http-proxy-middleware";
 import type { Role } from "@opensystemslab/planx-core/types";
 import { AsyncLocalStorage } from "async_hooks";
-import type { Request } from "express";
+import type { CookieOptions, Request } from "express";
 
 export const userContext = new AsyncLocalStorage<{ user: Express.User }>();
 
@@ -136,15 +136,27 @@ export const getMicrosoftAuthHandler = (
   return (req, res, next) => {
     req.session!.returnTo = req.get("Referrer");
 
-    // generate a nonce to enable us to validate the response from OP
+    // generate a nonce to enable us to validate the response from OP (mitigates against CSRF attacks)
     const nonce = generators.nonce();
     console.debug(`Generated a nonce: %s`, nonce);
-    req.session!.nonce = nonce;
 
-    // @ts-expect-error (method not typed to accept nonce, but it does pass it to the strategy)
+    // we hash the nonce to avoid sending it plaintext over the wire in our auth request
+    const hash = crypto.createHash("sha256").update(nonce).digest("hex");
+    console.debug(`Hashed nonce: %s`, hash);
+
+    // we store the original nonce in a short-lived, httpOnly but cross-site cookie
+    const httpOnlyCookieOptions: CookieOptions = {
+      maxAge: 15 * 60 * 1000, // 15 mins
+      sameSite: "none",
+      httpOnly: true,
+      secure: true,
+    };
+    res.cookie("ms-oidc-nonce", nonce, httpOnlyCookieOptions);
+
+    // @ts-expect-error (method not typed to accept nonce, but it does include it in the request)
     return passport.authenticate("microsoft-oidc", {
       prompt: "select_account",
-      nonce,
+      nonce: hash,
     })(req, res, next);
   };
 };

api.planx.uk/modules/auth/strategy/microsoft-oidc.ts

@@ -1,3 +1,4 @@
+import crypto from "crypto";
 import type {
   Client,
   ClientMetadata,
@@ -41,18 +42,30 @@ export const getMicrosoftOidcStrategy = (client: Client): Strategy<Client> => {
 };
 
 const verifyCallback: StrategyVerifyCallbackReq<Express.User> = async (
-  req: Http.IncomingMessageWithSession,
+  req: Http.IncomingMessageWithCookies,
   tokenSet,
   done,
 ): Promise<void> => {
   // TODO: use tokenSet.state to pass the redirectTo query param through the auth flow
+  // TODO: validate id_token sig with the public key from the jwks_uri (...v2.0/keys)
   const claims: IdTokenClaims = tokenSet.claims();
-  const email = claims.email;
-  const returned_nonce = claims.nonce;
 
-  if (returned_nonce != req.session?.nonce) {
-    return done(new Error("Returned nonce does not match session nonce"));
+  // ensure the response is authentic by comparing nonce
+  const returned_nonce = claims.nonce;
+  if (!req.cookies || !req.cookies["ms-oidc-nonce"]) {
+    return done(new Error("No nonce found in appropriate cookie"));
   }
+  const original_nonce = req.cookies["ms-oidc-nonce"];
+  const hash = crypto.createHash("sha256").update(original_nonce).digest("hex");
+  if (returned_nonce != hash) {
+    return done(
+      new Error(
+        "Returned nonce does not match nonce sent with original request",
+      ),
+    );
+  }
+
+  const email = claims.email;
   if (!email) {
     return done(new Error("Unable to authenticate without email"));
   }

api.planx.uk/server.ts

@@ -112,7 +112,8 @@ assert(process.env.UNIFORM_SUBMISSION_URL);
 // needed for storing original URL to redirect to in login flow
 app.use(
   cookieSession({
-    maxAge: 24 * 60 * 60 * 100,
+    // we don't need session to persist for long - it's only required for auth flow
+    maxAge: 2 * 60 * 60 * 1000, // 2hrs
     name: "session",
     secret: process.env.SESSION_SECRET,
   }),
@@ -198,9 +199,9 @@ declare global {
   }
 
   namespace Http {
-    interface IncomingMessageWithSession extends IncomingMessage {
-      session?: {
-        nonce: string;
+    interface IncomingMessageWithCookies extends IncomingMessage {
+      cookies?: {
+        "ms-oidc-nonce": string;
       };
     }
   }