WIP modularise passport code, harden security, temporarily revert Rea…

…ct auth routing fix from #3302
theopensystemslab · Jul 12, 2024 · 7f9db74 · 7f9db74
1 parent c6521de
commit 7f9db74
Show file tree

Hide file tree

Showing 7 changed files with 116 additions and 77 deletions.
diff --git a/api.planx.uk/modules/auth/controller.ts b/api.planx.uk/modules/auth/controller.ts
@@ -1,16 +1,27 @@
 import { CookieOptions, RequestHandler, Response } from "express";
 import { Request } from "express-jwt";
 
+import { client } from './passport'
+
 export const failedLogin: RequestHandler = (_req, _res, next) =>
   next({
     status: 401,
     message: "User failed to authenticate",
   });
 
 export const logout: RequestHandler = (req, res) => {
+  console.log("INVOKING LOGOUT CONTROLLER")
+
+  console.log("REQ:")
+  console.log(req)
+
   req.logout(() => {
     // do nothing
   });
+  // TODO: redirect to logout URL with id_token_hint (and logout_hint ??)
+  // logout_url = client.endSessionUrl({
+  //   id_token_hint: 
+  // });
   res.redirect(process.env.EDITOR_URL_EXT!);
 };
 

diff --git a/api.planx.uk/modules/auth/middleware.ts b/api.planx.uk/modules/auth/middleware.ts
@@ -1,15 +1,16 @@
 import crypto from "crypto";
 import assert from "assert";
-import { ServerError } from "../../errors";
-import { Template } from "../../lib/notify";
 import { expressjwt } from "express-jwt";
-
-import passport from "passport";
-
 import { RequestHandler } from "http-proxy-middleware";
-import { Role } from "@opensystemslab/planx-core/types";
 import { AsyncLocalStorage } from "async_hooks";
 import { Request } from "express";
+import { generators } from 'openid-client'
+
+import { Role } from "@opensystemslab/planx-core/types";
+
+import { ServerError } from "../../errors";
+import { Template } from "../../lib/notify";
+import { passport } from "./passport";
 
 export const userContext = new AsyncLocalStorage<{ user: Express.User }>();
 
@@ -126,13 +127,26 @@ export const useGoogleCallbackAuth: RequestHandler = (req, res, next) => {
 export const useMicrosoftAuth: RequestHandler = (req, res, next) => {
   console.log("INVOKING MICROSOFT MIDDLEWARE")
   req.session!.returnTo = req.get("Referrer");
+  console.log("REFERRER:")
+  console.log(req.get("Referrer"))
+  const nonce = generators.nonce();
+  console.log(`Generated a nonce: ${nonce}`);
+  req.session!.nonce = nonce
+  // @ts-expect-error - method not typed to accept nonce (but it does deliver it to the strategy)
+  // DOES IT ??
   return passport.authenticate("microsoft-oidc", {
     prompt: "select_account",
+    nonce: nonce,
   })(req, res, next);
 };
 
 export const useMicrosoftCallbackAuth: RequestHandler = (req, res, next) => {
   console.log("INVOKING MICROSOFT CALLBACK MIDDLEWARE")
+  console.log("THIS ALMOST WORKS, BUT THEN REDIRECTS BACK TO MICROSOFT AFTER HITTING /")
+
+  console.log("REQ:")
+  console.log(req)
+
   return passport.authenticate("microsoft-oidc", {
     failureRedirect: "/auth/login/failed",
   })(req, res, next);

diff --git a/api.planx.uk/modules/auth/passport.ts b/api.planx.uk/modules/auth/passport.ts
@@ -0,0 +1,56 @@
+import { custom, Issuer } from 'openid-client';
+import passport from 'passport';
+
+import { googleStrategy } from './strategy/google';
+import { getMicrosoftOidcStrategy } from './strategy/microsoft-oidc';
+
+const MICROSOFT_OPENID_CONFIG_URL = "https://login.microsoftonline.com/common/v2.0/.well-known/openid-configuration";
+
+// we need the client in various files, so it needs to be instantiated early on, and exported
+let client;
+
+// TODO: reset timeout (extended for local testing with poor connection)
+custom.setHttpOptionsDefaults({
+  timeout: 10000,
+})
+console.log("ATTEMPTING TO DISCOVER METADATA DOC")
+Issuer.discover(MICROSOFT_OPENID_CONFIG_URL).then(microsoftIssuer => {
+  console.log(
+    "Discovered issuer %s %O",
+    microsoftIssuer.issuer,
+    microsoftIssuer.metadata,
+  );
+
+  const client_id = process.env.MICROSOFT_CLIENT_ID!;
+  if (typeof client_id !== 'string') {
+    throw new Error('No MICROSOFT_CLIENT_ID in the environment');
+  }
+
+  client = new microsoftIssuer.Client({
+    client_id: client_id,
+    client_secret: process.env.MICROSOFT_CLIENT_SECRET!,
+    redirect_uris: [`${process.env.API_URL_EXT}/auth/microsoft/callback`],
+    post_logout_redirect_uris: [process.env.EDITOR_URL_EXT!],
+    response_types: ["id_token"],
+  });
+
+  console.log("Built Microsoft client:");
+  console.log(client);
+
+  passport.use('google', googleStrategy);
+  passport.use('microsoft-oidc', getMicrosoftOidcStrategy(client));
+
+  passport.serializeUser((user: any, done) => {
+    console.log("SERIALIZING USER")
+    console.log(user)
+    done(null, user);
+  });
+
+  passport.deserializeUser((obj: any, done) => {
+    console.log("DESERIALIZING USER")
+    console.log(obj)
+    done(null, obj);
+  });
+});
+
+export { passport, client };
diff --git a/api.planx.uk/modules/auth/service.ts b/api.planx.uk/modules/auth/service.ts
@@ -3,14 +3,21 @@ import { $api } from "../../client";
 import { User, Role } from "@opensystemslab/planx-core/types";
 
 export const buildJWT = async (email: string): Promise<string | undefined> => {
+  console.log("FETCHING USER FROM HASURA BY EMAIL")
   await checkUserCanAccessEnv(email, process.env.NODE_ENV);
   const user = await $api.user.getByEmail(email);
+  console.log("USER:")
+  console.log(user)
   if (!user) return;
 
+  const claims = generateHasuraClaimsForUser(user);
+  console.log("CLAIMS:")
+  console.log(claims)
+
   const data = {
     sub: user.id.toString(),
     email,
-    "https://hasura.io/jwt/claims": generateHasuraClaimsForUser(user),
+    "https://hasura.io/jwt/claims": claims,
   };
 
   const jwt = sign(data, process.env.JWT_SECRET!);

diff --git a/api.planx.uk/modules/auth/strategy/microsoft-oidc.ts b/api.planx.uk/modules/auth/strategy/microsoft-oidc.ts
@@ -1,56 +1,18 @@
-import { Strategy, TokenSet, Issuer, generators, Client } from "openid-client";
+import { Strategy, TokenSet, Client } from "openid-client";
 import { buildJWT } from "../service";
 
-const MICROSOFT_OAUTH_BASE_URL =
-  "https://login.microsoftonline.com/common/v2.0";
-const OPENID_METADATA_DOCUMENT_ENDPOINT = "/.well-known/openid-configuration";
-
-export const getMicrosoftIssuer = async (): Promise<Issuer> => {
-  const microsoftIssuer = await Issuer.discover(
-    MICROSOFT_OAUTH_BASE_URL + OPENID_METADATA_DOCUMENT_ENDPOINT,
-  );
-  console.log(
-    "Discovered issuer %s %O",
-    microsoftIssuer.issuer,
-    microsoftIssuer.metadata,
-  );
-  return microsoftIssuer;
-};
-
 export const getMicrosoftOidcStrategy = (
-  microsoftIssuer: Issuer,
+  client: Client,
 ): Strategy<Client> => {
   console.log("redirect uri domain:");
   console.log(process.env.API_URL_EXT);
 
-  const client_id = process.env.MICROSOFT_CLIENT_ID!;
-  if (typeof client_id !== 'string') {
-    throw new Error('No MICROSOFT_CLIENT_ID in the environment');
-  }
-
-  const microsoftClient = new microsoftIssuer.Client({
-    client_id: client_id,
-    client_secret: process.env.MICROSOFT_CLIENT_SECRET!,
-    redirect_uris: [`${process.env.API_URL_EXT}/auth/microsoft/callback`],
-    post_logout_redirect_uris: [`${process.env.API_URL_EXT}/logout`],
-    response_types: ["id_token"],
-  });
-
-  // should nonce be generated here, or in middleware functions?
-  const nonce = generators.nonce();
-  console.log(`Generated a nonce: ${nonce}`);
-  // TODO: store nonce (encrypted and httpOnly) in session
-
-  console.log("Built Microsoft client:");
-  console.log(microsoftClient.metadata);
-
-  // oidc = Open ID Connect
+  // oidc = OpenID Connect
   return new Strategy({
-      client: microsoftClient,
+      client: client,
       params: {
         scope: "openid email profile",
         response_mode: "form_post", // could also be 'query' or 'fragment'
-        nonce,
       },
       passReqToCallback: true,
       // usePKCE: false, // whether to use PKCE - defaults to true, according to docs
@@ -61,9 +23,6 @@ export const getMicrosoftOidcStrategy = (
       console.log("TOKEN SET:");
       console.log(tokenSet);
 
-      console.log("ID TOKEN:")
-      console.log(tokenSet.id_token)
-
       console.log("CLAIMS:")
       console.log(tokenSet.claims())
 
@@ -74,17 +33,25 @@ export const getMicrosoftOidcStrategy = (
       const claims = tokenSet.claims();
       const email = claims.email
       const returned_nonce = claims.nonce
-      // TODO: compare nonces
 
-      if (!email) throw Error("Unable to authenticate without email");
+      console.log("SESSION:")
+      console.log(req.session)
+
+      if (returned_nonce != req.session.nonce) {
+        return done(new Error("Returned nonce does not match session nonce"), null)
+      };
+
+      if (!email) {
+        return done (new Error("Unable to authenticate without email"), null)
+      };
 
       const jwt = await buildJWT(email);
 
       if (!jwt) {
         return done({
           status: 404,
           message: `User (${email}) not found. Do you need to log in to a different Microsoft Account?`,
-        } as any);
+        } as any, null);
       }
 
       return done(null, { jwt });

diff --git a/api.planx.uk/server.ts b/api.planx.uk/server.ts
@@ -9,17 +9,11 @@ import express, { ErrorRequestHandler } from "express";
 import noir from "pino-noir";
 import pinoLogger from "express-pino-logger";
 import { Server } from "http";
-import passport from "passport";
 import helmet from "helmet";
 import { ServerError } from "./errors";
 import airbrake from "./airbrake";
 import { apiLimiter } from "./rateLimit";
-import { googleStrategy } from "./modules/auth/strategy/google";
-import {
-  getMicrosoftIssuer,
-  getMicrosoftOidcStrategy,
-} from "./modules/auth/strategy/microsoft-oidc";
-import { Issuer } from "openid-client";
+import { passport } from "./modules/auth/passport";
 import authRoutes from "./modules/auth/routes";
 import teamRoutes from "./modules/team/routes";
 import miscRoutes from "./modules/misc/routes";
@@ -121,23 +115,9 @@ app.use(
   }),
 );
 
-// we have to fetch the Microsoft OpenID issuer to pass to our strategy constructor
-// TODO: handle failure to fetch issuer
-getMicrosoftIssuer().then((microsoftIssuer: Issuer) => {
-  console.log("GOT MS ISSUER - SETTING UP STRATEGY")
-  passport.use("microsoft-oidc", getMicrosoftOidcStrategy(microsoftIssuer));
-});
-passport.use("google", googleStrategy);
-
-passport.serializeUser(function (user, cb) {
-  cb(null, user);
-});
-
-passport.deserializeUser(function (obj: Express.User, cb) {
-  cb(null, obj);
-});
 app.use(passport.initialize());
 app.use(passport.session());
+
 app.use(urlencoded({ extended: true }));
 
 // Setup API routes

diff --git a/editor.planx.uk/src/index.tsx b/editor.planx.uk/src/index.tsx
@@ -37,12 +37,16 @@ if (!window.customElements.get("my-map")) {
 const hasJWT = (): boolean | void => {
   // This cookie indicates the presence of the secure httpOnly "jwt" cookie
   const authCookie = getCookie("auth");
+  console.log("AUTH COOKIE:");
+  console.log(authCookie);
   if (authCookie) return true;
 
   // If JWT not set via cookie, check search params
+  console.log("LOOKING FOR JWT IN QUERY PARAMS");
   const jwtSearchParams = new URLSearchParams(window.location.search).get(
     "jwt",
   );
+  console.log(jwtSearchParams);
   if (!jwtSearchParams) return false;
 
   // Remove JWT from URL, and re-run this function