Revert "[api] security hardening for microsoft auth"

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

@@ -39,7 +39,6 @@ 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 { CookieOptions, Request } from "express";
+import type { Request } from "express";
 
 export const userContext = new AsyncLocalStorage<{ user: Express.User }>();
 
@@ -136,27 +136,15 @@ 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 (mitigates against CSRF attacks)
+    // generate a nonce to enable us to validate the response from OP
     const nonce = generators.nonce();
     console.debug(`Generated a nonce: %s`, nonce);
+    req.session!.nonce = nonce;
 
-    // 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)
+    // @ts-expect-error (method not typed to accept nonce, but it does pass it to the strategy)
     return passport.authenticate("microsoft-oidc", {
       prompt: "select_account",
-      nonce: hash,
+      nonce,
     })(req, res, next);
   };
 };

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

@@ -1,4 +1,3 @@
-import crypto from "crypto";
 import type {
   Client,
   ClientMetadata,
@@ -42,30 +41,18 @@ export const getMicrosoftOidcStrategy = (client: Client): Strategy<Client> => {
 };
 
 const verifyCallback: StrategyVerifyCallbackReq<Express.User> = async (
-  req: Http.IncomingMessageWithCookies,
+  req: Http.IncomingMessageWithSession,
   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();
-
-  // ensure the response is authentic by comparing nonce
+  const email = claims.email;
   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 (returned_nonce != req.session?.nonce) {
+    return done(new Error("Returned nonce does not match session nonce"));
+  }
   if (!email) {
     return done(new Error("Unable to authenticate without email"));
   }

api.planx.uk/server.ts

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