From d9f76adb7b591f0a97a2f3c8d81b23356e22e566 Mon Sep 17 00:00:00 2001 From: Dan Goss Date: Thu, 25 Jul 2024 10:11:16 +0100 Subject: [PATCH] [api] introduce top-level await for getting passport in server.ts --- api.planx.uk/modules/auth/controller.ts | 4 -- api.planx.uk/modules/auth/middleware.ts | 66 ++++++++++++------- api.planx.uk/modules/auth/passport.ts | 54 +++++++-------- api.planx.uk/modules/auth/routes.ts | 35 +++++----- .../modules/auth/strategy/microsoft-oidc.ts | 6 +- api.planx.uk/server.ts | 15 +++-- 6 files changed, 96 insertions(+), 84 deletions(-) diff --git a/api.planx.uk/modules/auth/controller.ts b/api.planx.uk/modules/auth/controller.ts index b5d2c8c724..9cb2ff7ffd 100644 --- a/api.planx.uk/modules/auth/controller.ts +++ b/api.planx.uk/modules/auth/controller.ts @@ -1,8 +1,6 @@ import type { CookieOptions, RequestHandler, Response } from "express"; import type { Request } from "express-jwt"; -import { microsoftOidcClient } from "./passport.js"; - export const failedLogin: RequestHandler = (_req, _res, next) => next({ status: 401, @@ -13,8 +11,6 @@ export const logout: RequestHandler = (req, res) => { req.logout(() => { // do nothing }); - // TODO: implement logout for Microsoft strategy (redirect to logout URL with hint) - // logout_url = microsoftOidcClient.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 index c70efdcf2d..ef0335ff68 100644 --- a/api.planx.uk/modules/auth/middleware.ts +++ b/api.planx.uk/modules/auth/middleware.ts @@ -4,7 +4,7 @@ import { ServerError } from "../../errors/index.js"; import type { Template } from "../../lib/notify/index.js"; import { expressjwt } from "express-jwt"; import { generators } from "openid-client"; -import { passportWithStrategies } from "./passport.js"; +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"; @@ -109,38 +109,54 @@ export const useJWT = expressjwt({ getToken: getToken, }); -export const useGoogleAuth: RequestHandler = (req, res, next) => { - req.session!.returnTo = req.get("Referrer"); - return passportWithStrategies.authenticate("google", { - scope: ["profile", "email"], - })(req, res, next); +export const getGoogleAuthHandler = ( + passport: Authenticator, +): RequestHandler => { + return (req, res, next) => { + req.session!.returnTo = req.get("Referrer"); + return passport.authenticate("google", { + scope: ["profile", "email"], + })(req, res, next); + }; }; -export const useGoogleCallbackAuth: RequestHandler = (req, res, next) => { - return passportWithStrategies.authenticate("google", { - failureRedirect: "/auth/login/failed", - })(req, res, next); +export const getGoogleCallbackAuthHandler = ( + passport: Authenticator, +): RequestHandler => { + return (req, res, next) => { + return passport.authenticate("google", { + failureRedirect: "/auth/login/failed", + })(req, res, next); + }; }; -export const useMicrosoftAuth: RequestHandler = (req, res, next) => { - req.session!.returnTo = req.get("Referrer"); +export const getMicrosoftAuthHandler = ( + passport: Authenticator, +): RequestHandler => { + return (req, res, next) => { + req.session!.returnTo = req.get("Referrer"); - // 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; + // 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; - // @ts-expect-error (method not typed to accept nonce, but it does pass it to the strategy) - return passportWithStrategies.authenticate("microsoft-oidc", { - prompt: "select_account", - nonce, - })(req, res, next); + // @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, + })(req, res, next); + }; }; -export const useMicrosoftCallbackAuth: RequestHandler = (req, res, next) => { - return passportWithStrategies.authenticate("microsoft-oidc", { - failureRedirect: "/auth/login/failed", - })(req, res, next); +export const getMicrosoftCallbackAuthHandler = ( + passport: Authenticator, +): RequestHandler => { + return (req, res, next) => { + return passport.authenticate("microsoft-oidc", { + failureRedirect: "/auth/login/failed", + })(req, res, next); + }; }; type UseRoleAuth = (authRoles: Role[]) => RequestHandler; diff --git a/api.planx.uk/modules/auth/passport.ts b/api.planx.uk/modules/auth/passport.ts index 21343a7d9f..2a9b0e2843 100644 --- a/api.planx.uk/modules/auth/passport.ts +++ b/api.planx.uk/modules/auth/passport.ts @@ -1,4 +1,5 @@ -import { custom, Issuer } from "openid-client"; +import { Issuer } from "openid-client"; +import type { Authenticator } from "passport"; import passport from "passport"; import { googleStrategy } from "./strategy/google.js"; @@ -8,41 +9,32 @@ import { MICROSOFT_OPENID_CONFIG_URL, } from "./strategy/microsoft-oidc.js"; -const setupPassport = () => { - // TODO: remove below config (timeout extended for local testing with poor connection) - custom.setHttpOptionsDefaults({ - timeout: 10000, - }); - - // explicitly instantiate new passport for clarity - const passportWithStrategies = new passport.Passport(); +export default async (): Promise => { + // explicitly instantiate new passport class for clarity + const customPassport = new passport.Passport(); - // build Microsoft OIDC client, and use it to build the related strategy - let microsoftOidcClient; - // TODO: need to block on fetch of issuer, but can't use top level await... - Issuer.discover(MICROSOFT_OPENID_CONFIG_URL).then((microsoftIssuer) => { - console.debug("Discovered issuer %s", microsoftIssuer.issuer); - const microsoftClientConfig = getMicrosoftClientConfig(); - microsoftOidcClient = new microsoftIssuer.Client(microsoftClientConfig); - console.debug("Built Microsoft client: %O", microsoftOidcClient); - passportWithStrategies.use( - "microsoft-oidc", - getMicrosoftOidcStrategy(microsoftOidcClient), - ); - }); + // instantiate Microsoft OIDC client, and use it to build the related strategy + const microsoftIssuer = await Issuer.discover(MICROSOFT_OPENID_CONFIG_URL); + console.debug("Discovered issuer %s", microsoftIssuer.issuer); + const microsoftOidcClient = new microsoftIssuer.Client( + getMicrosoftClientConfig(), + ); + console.debug("Built Microsoft client: %O", microsoftOidcClient); + customPassport.use( + "microsoft-oidc", + getMicrosoftOidcStrategy(microsoftOidcClient), + ); - // do any other aspects of passport setup which can be handled here - passportWithStrategies.use("google", googleStrategy); - passportWithStrategies.serializeUser((user: any, done) => { + // do other aspects of passport setup which can be handled here + // TODO: replace types here (e.g. user: Express.User - but verify this first) + customPassport.use("google", googleStrategy); + customPassport.serializeUser((user: any, done) => { done(null, user); }); - passportWithStrategies.deserializeUser((obj: any, done) => { + customPassport.deserializeUser((obj: any, done) => { done(null, obj); }); - return { passportWithStrategies, microsoftOidcClient }; + // tsc dislikes the use of 'this' in the passportjs codebase, so we cast explicitly + return customPassport as Authenticator; }; - -// instantiate and export the new passport class and Microsoft client as early as possible -const { passportWithStrategies, microsoftOidcClient } = setupPassport(); -export { passportWithStrategies, microsoftOidcClient }; diff --git a/api.planx.uk/modules/auth/routes.ts b/api.planx.uk/modules/auth/routes.ts index e8083afeec..e87b8f6790 100644 --- a/api.planx.uk/modules/auth/routes.ts +++ b/api.planx.uk/modules/auth/routes.ts @@ -1,22 +1,25 @@ import { Router } from "express"; +import type { Authenticator } from "passport"; import * as Middleware from "./middleware.js"; import * as Controller from "./controller.js"; -const router = Router(); +export default (passport: Authenticator): Router => { + const router = Router(); -router.get("/logout", Controller.logout); -router.get("/auth/login/failed", Controller.failedLogin); -router.get("/auth/google", Middleware.useGoogleAuth); -router.get( - "/auth/google/callback", - Middleware.useGoogleCallbackAuth, - Controller.handleSuccess, -); -router.get("/auth/microsoft", Middleware.useMicrosoftAuth); -router.post( - "/auth/microsoft/callback", - Middleware.useMicrosoftCallbackAuth, - Controller.handleSuccess, -); + router.get("/logout", Controller.logout); + router.get("/auth/login/failed", Controller.failedLogin); + router.get("/auth/google", Middleware.getGoogleAuthHandler(passport)); + router.get( + "/auth/google/callback", + Middleware.getGoogleCallbackAuthHandler(passport), + Controller.handleSuccess, + ); + router.get("/auth/microsoft", Middleware.getMicrosoftAuthHandler(passport)); + router.post( + "/auth/microsoft/callback", + Middleware.getMicrosoftCallbackAuthHandler(passport), + Controller.handleSuccess, + ); -export default router; + return router; +}; diff --git a/api.planx.uk/modules/auth/strategy/microsoft-oidc.ts b/api.planx.uk/modules/auth/strategy/microsoft-oidc.ts index 4fe8d88f87..46faf4e37d 100644 --- a/api.planx.uk/modules/auth/strategy/microsoft-oidc.ts +++ b/api.planx.uk/modules/auth/strategy/microsoft-oidc.ts @@ -26,20 +26,18 @@ export const getMicrosoftOidcStrategy = (client: Client): Strategy => { client: client, params: { scope: "openid email profile", + // TODO: verify that this is the appropriate response_mode response_mode: "form_post", }, // need the request in the verify callback to validate the returned nonce passReqToCallback: true, }, async (req: any, tokenSet: TokenSet, done: any): Promise => { - // TODO: use state to pass the redirectTo query param through the auth flow - const state = tokenSet.state; + // TODO: use tokenSet.state to pass the redirectTo query param through the auth flow const claims = tokenSet.claims(); const email = claims.email; const returned_nonce = claims.nonce; - // we grab login_hint to provide to the logout endpoint later (as per OpenID spec) - const login_hint = claims.login_hint; if (returned_nonce != req.session.nonce) { return done( diff --git a/api.planx.uk/server.ts b/api.planx.uk/server.ts index c013f868e9..10edff20fb 100644 --- a/api.planx.uk/server.ts +++ b/api.planx.uk/server.ts @@ -16,8 +16,8 @@ import { ServerError } from "./errors/index.js"; import airbrake from "./airbrake.js"; import { apiLimiter } from "./rateLimit.js"; import { registerSessionStubs } from "./session.js"; -import { passportWithStrategies } from "./modules/auth/passport.js"; -import authRoutes from "./modules/auth/routes.js"; +import getPassport from "./modules/auth/passport.js"; +import getAuthRoutes from "./modules/auth/routes.js"; import teamRoutes from "./modules/team/routes.js"; import miscRoutes from "./modules/misc/routes.js"; import userRoutes from "./modules/user/routes.js"; @@ -121,10 +121,17 @@ app.use( // register stubs after cookieSession middleware initialisation app.use(registerSessionStubs); -app.use(passportWithStrategies.initialize()); -app.use(passportWithStrategies.session()); + +// equip passport with auth strategies early on, so we can pass it to route handlers +const passport = await getPassport(); +app.use(passport.initialize()); +app.use(passport.session()); + app.use(bodyParser.urlencoded({ extended: true })); +// auth routes rely on the passport class we've just initialised +const authRoutes = await getAuthRoutes(passport); + // Setup API routes app.use(adminRoutes); app.use(analyticsRoutes);