From b0307d4ceafa067883dc8568b9911ee744a84092 Mon Sep 17 00:00:00 2001 From: Dan Goss Date: Thu, 20 Jun 2024 19:26:38 +0100 Subject: [PATCH] fix bugs with Microsoft auth on local --- api.planx.uk/modules/auth/middleware.ts | 2 + api.planx.uk/modules/auth/routes.ts | 2 +- .../modules/auth/strategy/microsoft-oidc.ts | 55 +++++++++++++------ api.planx.uk/server.ts | 1 + docker-compose.yml | 3 + 5 files changed, 46 insertions(+), 17 deletions(-) diff --git a/api.planx.uk/modules/auth/middleware.ts b/api.planx.uk/modules/auth/middleware.ts index a7084a0fca..94ecdfa37b 100644 --- a/api.planx.uk/modules/auth/middleware.ts +++ b/api.planx.uk/modules/auth/middleware.ts @@ -124,6 +124,7 @@ 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"); return passport.authenticate("microsoft-oidc", { prompt: "select_account", @@ -131,6 +132,7 @@ export const useMicrosoftAuth: RequestHandler = (req, res, next) => { }; export const useMicrosoftCallbackAuth: RequestHandler = (req, res, next) => { + console.log("INVOKING MICROSOFT CALLBACK MIDDLEWARE") return passport.authenticate("microsoft-oidc", { failureRedirect: "/auth/login/failed", })(req, res, next); diff --git a/api.planx.uk/modules/auth/routes.ts b/api.planx.uk/modules/auth/routes.ts index 8570bcadc2..ec94f6209b 100644 --- a/api.planx.uk/modules/auth/routes.ts +++ b/api.planx.uk/modules/auth/routes.ts @@ -13,7 +13,7 @@ router.get( Controller.handleSuccess, ); router.get("/auth/microsoft", Middleware.useMicrosoftAuth) -router.get( +router.post( "/auth/microsoft/callback", Middleware.useMicrosoftCallbackAuth, Controller.handleSuccess, diff --git a/api.planx.uk/modules/auth/strategy/microsoft-oidc.ts b/api.planx.uk/modules/auth/strategy/microsoft-oidc.ts index 99292af329..7fd8a1658b 100644 --- a/api.planx.uk/modules/auth/strategy/microsoft-oidc.ts +++ b/api.planx.uk/modules/auth/strategy/microsoft-oidc.ts @@ -23,38 +23,59 @@ export const getMicrosoftOidcStrategy = ( 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: process.env.MICROSOFT_CLIENT_ID!, + 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 - microsoftClient.authorizationUrl({ - scope: "openid email profile", - response_mode: "form_post", // could also be 'query' or 'fragment' - nonce, - }); - console.log("Built Microsoft client:"); console.log(microsoftClient.metadata); // oidc = Open ID Connect - return new Strategy( - { client: microsoftClient }, - async (tokenset: TokenSet, userInfo: any, done: any): Promise => { - console.log("USER INFO:"); - console.log(userInfo); - + return new Strategy({ + client: microsoftClient, + 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 + }, + async (req: any, tokenSet: TokenSet, done: any): Promise => { + console.log("INVOKING STRATEGY CALLBACK!") + console.log("TOKEN SET:"); - console.log(tokenset); + console.log(tokenSet); + + console.log("ID TOKEN:") + console.log(tokenSet.id_token) + + console.log("CLAIMS:") + console.log(tokenSet.claims()) + + const id_token = tokenSet.id_token; + const state = tokenSet.state; + // TODO: do something with state?? + + const claims = tokenSet.claims(); + const email = claims.email + const returned_nonce = claims.nonce + // TODO: compare nonces - const email = "xxx"; if (!email) throw Error("Unable to authenticate without email"); const jwt = await buildJWT(email); @@ -66,7 +87,9 @@ export const getMicrosoftOidcStrategy = ( } as any); } - done(null, { jwt }); + return done(null, { jwt }); + + // TODO: handle error case i.e. done(err) }, ); }; diff --git a/api.planx.uk/server.ts b/api.planx.uk/server.ts index d045475953..b9015b28b7 100644 --- a/api.planx.uk/server.ts +++ b/api.planx.uk/server.ts @@ -124,6 +124,7 @@ 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); diff --git a/docker-compose.yml b/docker-compose.yml index 65a38b6e7d..5eb879aed5 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -153,6 +153,9 @@ services: UNIFORM_CLIENT_AYLESBURY_VALE: ${UNIFORM_CLIENT_AYLESBURY_VALE} UNIFORM_CLIENT_CHILTERN: ${UNIFORM_CLIENT_CHILTERN} UNIFORM_CLIENT_WYCOMBE: ${UNIFORM_CLIENT_WYCOMBE} + # microsoft-oidc strategy testing + MICROSOFT_CLIENT_ID: ${MICROSOFT_CLIENT_ID} + MICROSOFT_CLIENT_SECRET: ${MICROSOFT_CLIENT_SECRET} sharedb: restart: unless-stopped