From 39107a7f7476d009f9eb5607797c04c5e570cbf6 Mon Sep 17 00:00:00 2001 From: Dan Goss Date: Fri, 12 Jul 2024 09:55:18 +0100 Subject: [PATCH] WIP modularise passport code, harden security, temporarily revert React auth routing fix from #3302 --- api.planx.uk/modules/auth/controller.ts | 11 ++++ api.planx.uk/modules/auth/middleware.ts | 26 ++++++-- api.planx.uk/modules/auth/passport.ts | 56 +++++++++++++++++ api.planx.uk/modules/auth/service.ts | 9 ++- .../modules/auth/strategy/microsoft-oidc.ts | 63 +++++-------------- api.planx.uk/server.ts | 24 +------ editor.planx.uk/src/index.tsx | 10 ++- 7 files changed, 121 insertions(+), 78 deletions(-) create mode 100644 api.planx.uk/modules/auth/passport.ts diff --git a/api.planx.uk/modules/auth/controller.ts b/api.planx.uk/modules/auth/controller.ts index 458487ad6c..9d5c22c324 100644 --- a/api.planx.uk/modules/auth/controller.ts +++ b/api.planx.uk/modules/auth/controller.ts @@ -1,6 +1,8 @@ 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, @@ -8,9 +10,18 @@ export const failedLogin: RequestHandler = (_req, _res, next) => }); 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 index 94ecdfa37b..276f9f8cf7 100644 --- 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 new file mode 100644 index 0000000000..58a2cbe0a8 --- /dev/null +++ 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 }; \ No newline at end of file diff --git a/api.planx.uk/modules/auth/service.ts b/api.planx.uk/modules/auth/service.ts index d4a6c39e0a..7665ffe92e 100644 --- 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 => { + 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 index 7fd8a1658b..f947867564 100644 --- 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 => { - 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 => { 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,9 +33,17 @@ 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); @@ -84,7 +51,7 @@ export const getMicrosoftOidcStrategy = ( 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 index b9015b28b7..62a5857b38 100644 --- 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 index 9e178d4167..e29c79c4cf 100644 --- a/editor.planx.uk/src/index.tsx +++ b/editor.planx.uk/src/index.tsx @@ -37,18 +37,26 @@ 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 setCookie("jwt", jwtSearchParams); setCookie("auth", { loggedIn: true }); - window.history.go(-1); + console.log("CURRENT PAGE: " + window.location.href); + console.log("NAVIGATING TO PREVIOUS PAGE IN HISTORY"); + // window.history.go(-1); // history.go(-1) is equivalent to history.back() + window.location.href = "/"; + // location.reload() }; const Layout: React.FC<{